Skip to content

ref(typing): Type request.user with service accounts everywhere in sentry - #129640

Merged
gricha merged 1 commit into
ref/sa-integration-issue-configfrom
ref/drop-request-user-allowlist
Nov 4, 2026
Merged

gricha merged 1 commit into
ref/sa-integration-issue-configfrom
ref/drop-request-user-allowlist

Conversation

@gricha

@gricha gricha commented Nov 3, 2026

Copy link
Copy Markdown
Member

Every file is off the request.user allowlist, so this deletes it. mypy now types DRF request.user as User | AnonymousUser | ServiceAccountPrincipal everywhere in sentry.

  • tools/mypy_helpers/request_user_allowlist.txt is gone, along with the plugin's allowlist reader, its dmypy cache handling (report_config_data) and the allowlist ratchet test.
  • Files outside the sentry repo still keep today's type. getsentry loads this plugin through a tools -> ../sentry/tools symlink, and test_drf_request_user_keeps_its_type_outside_sentry still covers that.
  • src/AGENTS.md drops the note about the list, and the ServiceAccountId guard's docstring now says it covers code mypy doesn't check this way, such as getsentry.

Full mypy passes (8475 files) and so do the plugin and principal tests (108 passed).

@gricha
gricha added this pull request to stack #129572 November 3, 2026 01:49
@gricha
gricha requested review from a team as code owners November 3, 2026 01:49
@gricha
gricha requested review from cvxluo and skaasten November 3, 2026 01:49
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 3, 2026
@sentry

sentry Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Sentry Snapshot Testing

Name Added Removed Changed Renamed Unchanged Skipped Status
sentry-frontend
sentry-frontend
0 0 0 0 780 0 ✅ Unchanged

⚙️ sentry-frontend Snapshot Settings

@gricha
gricha force-pushed the ref/drop-request-user-allowlist branch from d821e2d to ea701e6 Compare November 3, 2026 02:27
@gricha
gricha force-pushed the ref/drop-request-user-allowlist branch from ea701e6 to c92ee41 Compare November 3, 2026 02:30
@gricha
gricha force-pushed the ref/drop-request-user-allowlist branch from c92ee41 to 88d673f Compare November 3, 2026 04:03
@gricha
gricha force-pushed the ref/drop-request-user-allowlist branch from 88d673f to 46ebf87 Compare November 3, 2026 04:41
@gricha
gricha removed this pull request from stack #129572 November 3, 2026 06:52
@gricha
gricha added this pull request to stack #129650 November 3, 2026 06:52
@gricha
gricha force-pushed the ref/drop-request-user-allowlist branch from 46ebf87 to e7d1563 Compare November 3, 2026 19:53
@gricha
gricha removed this pull request from stack #129650 November 3, 2026 19:53
@gricha
gricha added this pull request to stack #129747 November 3, 2026 19:54
@gricha
gricha requested review from a team as code owners November 3, 2026 20:03
@gricha
gricha requested review from a team as code owners November 3, 2026 20:03
@gricha
gricha force-pushed the ref/drop-request-user-allowlist branch 2 times, most recently from 2b4a9f2 to 2c29f9d Compare November 3, 2026 20:09
@gricha
gricha force-pushed the ref/drop-request-user-allowlist branch 3 times, most recently from f63da29 to b95e21e Compare November 3, 2026 23:19
@gricha
gricha force-pushed the ref/drop-request-user-allowlist branch from b95e21e to 261db9c Compare November 4, 2026 00:45
…ntry

Every file is off the request.user allowlist, so delete it. The mypy
plugin types DRF request.user as User | AnonymousUser |
ServiceAccountPrincipal in all of sentry; getsentry, which loads the
plugin through a tools symlink, keeps the old type. Drops the allowlist
reader, its dmypy cache handling, the allowlist ratchet test and the
AGENTS.md note about the list.
@gricha
gricha force-pushed the ref/drop-request-user-allowlist branch from 261db9c to a70aa46 Compare November 4, 2026 01:15
@gricha
gricha merged commit 2f1e6fe into master Nov 4, 2026
78 checks passed
@gricha
gricha deleted the ref/drop-request-user-allowlist branch November 4, 2026 03:39
gricha added a commit that referenced this pull request Nov 4, 2026
… and client config (#129630)

Four one-off `request.user` reads, which drops their files from the
allowlist (10 → 6). What's left is member management (#129638), data
export (#129637) and `group_integration_details.py` (#129639); #129640
then deletes the allowlist.

- **Agent tokens:** `resolve_minting_principal()` rejects any caller
with no user (`has_user`), not just `AnonymousUser`. A service account
used to resolve to a *user* principal carrying its `ServiceAccountId`
and fail later; it now gets the same "Minting requires a user principal"
403 as an anonymous caller. The new assertion fails without this change:
  ```
E AssertionError: assert AgentPrincipal(type=<AgentPrincipalType.USER:
'user'>, id=ServiceAccountId(service_account_id=1)) is
<MintingPrincipalRejection.UNSUPPORTED: 'unsupported'>
  ```
- **Release serialization** takes a `RequestPrincipal`. It only forwards
the caller to `serialize()` and `serialize_generic_user()`, which
already take one, so this is type-only.
- **Notebooks** read the caller through `acting_user_id()`; same value
for users and token callers.
- **Client config** is built only from Django views, so its request is
typed as `HttpRequest`. Type-only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants