Skip to content

fix(data-export): Refuse exports from callers with no user - #129637

Merged
gricha merged 1 commit into
ref/sa-oneoffsfrom
fix/data-export-requires-user
Nov 4, 2026
Merged

gricha merged 1 commit into
ref/sa-oneoffsfrom
fix/data-export-requires-user

Conversation

@gricha

@gricha gricha commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Data export POSTs from a caller with no user (a legacy API key or an org auth token) created an export nobody could get, then returned 500.

An export is emailed to and downloaded by its user. For a caller with no user the endpoint still ran get_or_create(user_id=None, ...), scheduled the export task, and then crashed serializing the response, because the serializer looks the user up. Each attempt left an export that no one could download (the details endpoint needs a user since #129442). The user_id=None lookup also matched other no-user callers' unfinished exports with the same query.

require_user() now refuses these callers before anything is created: 401 for API keys, 403 user-required for a service account. (Org auth tokens only carry org:ci, so the permission check already returned 403.) Users are unaffected. Production had no export POST without a user in the last 30 days (0 of 3,968 sampled transactions), so no working integration relies on this.

test_api_key_cannot_export fails without the fix:

E   AssertionError: b'{"detail":"Internal Error","errorId":null}'
E   assert 500 == 401
FAILED tests/sentry/data_export/endpoints/test_data_export.py::DataExportTest::test_api_key_cannot_export

With it, test_data_export.py passes: 49 passed. Also drops data_export.py from the request.user allowlist (6 → 5).

@gricha
gricha requested review from a team as code owners November 3, 2026 01:49
@gricha
gricha added this pull request to stack #129572 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
@gricha
gricha force-pushed the fix/data-export-requires-user branch from 250b3ae to 3bfabab Compare November 3, 2026 04:03
@gricha
gricha requested review from cvxluo and skaasten November 3, 2026 04:39
@gricha
gricha force-pushed the fix/data-export-requires-user branch from 3bfabab to 8dccd62 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 fix/data-export-requires-user branch from 8dccd62 to 2ef4b60 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:02
@gricha
gricha requested review from a team as code owners November 3, 2026 20:02
@gricha
gricha force-pushed the fix/data-export-requires-user branch from 2ef4b60 to 315e058 Compare November 3, 2026 20:02
@gricha
gricha force-pushed the fix/data-export-requires-user branch from 315e058 to c5e54ea Compare November 3, 2026 20:09
@gricha
gricha force-pushed the fix/data-export-requires-user branch from c5e54ea to 045cf03 Compare November 3, 2026 20:51
@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 fix/data-export-requires-user branch 3 times, most recently from 89d8511 to af9ae26 Compare November 4, 2026 00:44
An export is emailed to and downloaded by its user. For an API key or org
token, the endpoint created the export and scheduled its task, then
returned 500 because the response serializer looks the user up. Each
attempt left an export nobody could fetch, and the get_or_create on
user_id=None matched other no-user callers' unfinished exports.

require_user() refuses these callers up front (401; 403 for a service
account), and data_export.py leaves the request.user allowlist.
@gricha
gricha force-pushed the fix/data-export-requires-user branch from af9ae26 to b8a07c8 Compare November 4, 2026 01:15
@gricha
gricha merged commit 83c0770 into master Nov 4, 2026
78 checks passed
@gricha
gricha deleted the fix/data-export-requires-user branch November 4, 2026 03:39
gricha added a commit that referenced this pull request Nov 4, 2026
…129629)

Two personal-state checks go through `sentry.auth.principal`, which
drops them from the `request.user` allowlist (12 → 10).

- **Prompt dismissal** (`PUT prompts-activity`) is a personal write, so
it calls `require_user()`. A legacy API key got 201 while nothing was
saved (the insert with no user failed and the error was swallowed); it
now gets 401. Production had no PUT without a user in the last 30 days
(0 of 3,456 sampled). A service account still gets 403 `user-required`.
- **Dashboard edit permission** reads the caller through
`acting_user_id()`, the same value for users and token callers. A
service account on a restricted dashboard still gets 403, now the
generic permission error instead of `user-required`.

Data export stays on the allowlist here: callers with no user created
exports nobody could get, which #129637 fixes on its own.

`test_api_key_cannot_dismiss` fails without this change:

```
E   assert 201 == 401
FAILED tests/sentry/api/endpoints/test_prompts_activity.py::PutPromptsActivityTest::test_api_key_cannot_dismiss
```

With it, the prompts and dashboards suites pass.
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.

2 participants