Skip to content

fix(artifactbundle): reject non-string debug ids - #129435

Draft
joshuarli wants to merge 1 commit into
masterfrom
fix/artifactbundle-debug-id-shape
Draft

fix(artifactbundle): reject non-string debug ids#129435
joshuarli wants to merge 1 commit into
masterfrom
fix/artifactbundle-debug-id-shape

Conversation

@joshuarli

@joshuarli joshuarli commented Oct 31, 2026

Copy link
Copy Markdown
Member

Behavioral split from #129431 (cast audit).

ArtifactBundleArchive.normalize_debug_id accepted only str | None, while callers read the debug-id header out of an uploaded manifest and cast it to that type. Accept object and treat any non-string value as absent before it reaches symbolic.

This removes the trust-boundary casts in models/artifactbundle.py and debug_files/artifact_bundles.py and adds a direct regression test.

@joshuarli
fix(artifactbundle): reject non-string debug ids
fa61c4e 
Manifest headers come from uploaded archives. Treat any non-string
debug-id value as absent instead of passing it into symbolic, and drop
the casts that assumed the header value was always a string.
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 31, 2026
joshuarli added a commit to joshuarli/sentry that referenced this pull request Oct 31, 2026
@joshuarli
fix(artifactbundle): reject non-string debug ids
a189274 
Manifest headers come from uploaded archives. Treat any non-string
debug-id value as absent instead of passing it into symbolic, and drop
the casts that assumed the header value was always a string.

Review split: getsentry#129435.
This was referenced Oct 31, 2026
Closed
Closed
This was referenced Nov 3, 2026
Merged
Draft
joshuarli added a commit that referenced this pull request Nov 3, 2026
@sentry-junior @joshuarli
ref(typing)!: enforce strict mypy across Sentry's Python codebase (ty…
ae29b66 
…pes only) (#129614)

This is the types-only version of #129447. It turns on `strict = true`
for the whole codebase and empties the mypy weaklist. It keeps **today's
master runtime behavior**. The behavior fixes that were folded into
#129447 come back as separate PRs stacked on this branch.

**Changes from #129447**
- `strict = true` with `warn_return_any = false`. The `no-any-return`
errors were most of the work, and fixing them added `return cast(...)`
calls, `isinstance`/`raise TypeError` guards, and `# justification:`
comments. Those are gone.
- Where strict types and master behavior conflict, master behavior stays
and the types give way: a `cast`, an `Any`, or a scoped `# type:
ignore[...]`. Examples:
- The paginator `order_b` typo is still there behind `# type:
ignore[attr-defined]`. #129615 fixes it.
- Fallbacks like `or 0`, coercions with `str()`/`int()`/`bool()`,
defensive copies that changed in-place mutation, and serializer output
are back to master.
- `get_mapping`, `post_mapping` and `get_sequence` in
`shared_integrations/client/base.py` are pure casts. They no longer
raise `ApiError` when a response has an unexpected shape.
- `Bounded*Field` are bare subclasses again, with `# type:
ignore[type-arg]` on 7 class lines. The `[int | None, int | None]`
subscripts are gone from call sites. These fields type-check as `Any`,
as they do on master.
- These are removed and can come back as separate PRs if we still want
them: `check_typing_campaign.py` with its hook and tests, the weaklist
sort tooling, the `.github/workflows/pre-commit.yml` rewrite, and the
`inviteStatus` nullability change in
`static/app/types/organization.tsx`.
- I tried `allow_redefinition = true`. Under mypy 2.1 it adds about 150
errors in existing code, so it's off and the variable renames stay.

**Rebased on master** (`e39c07b6`). Conflicts were resolved in master's
favor, with types added on top. Master code added since the old base
needed a few more stub entries in `fixtures/stubs-for-mypy`
(`PGAccessExclusive`/`PGLock`, two selenium exceptions,
`WebDriverWait.until` accepting `Literal[False]`) and annotations on a
handful of new tests.

**Deliberately left in**
- About 95 `assert x is not None` lines for type narrowing. I haven't
checked each one against master behavior.
- `UNBOUNDED_DATETIME = cast("datetime", None)` in
`tests/snuba/search/test_backend.py`. Widening `snuba_search` to
`datetime | None` spreads into `run_eap_group_search` and
`_prepare_params_for_categories`.

**Stack** (all based on this branch, land after it):
- #129615: paginator `order_by` typo (was #127799)
- #129616: msteams missing conversation id (was #129427)
- #129617: blank origin values (was #129428)
- #129618: release serializer defaults (was #129429)
- #129619: jira query required (was #129430)
- #129620: artifact manifest JSON shape (was #129432)
- #129621: dashboard threshold validation (was #129434; now accepts
decimal maximums)
- #129622: non-string debug ids (was #129435)
- #129623: non-string sourcemap headers (was #129436)
- #129624: blank allowed domains (was #129438; no longer splits strings
into characters)

#129433 isn't restacked. Master's `owned_by_acting_user()` already
returns `Q(pk__in=[])` for anonymous users, so those queries are skipped
there.

**No getsentry change needed.** getsentry's `backend typing` flagged 7
errors against an earlier push of this branch. Each one was a Sentry
signature this PR had made narrower than master's contract, so they're
fixed here, not in getsentry:
-
`BaseApiClient.request`/`get`/`get_cached`/`post`/`put`/`patch`/`delete`/`head_cached`
return `Any` again, as on master. The `get_mapping`/`get_sequence`
helpers keep their typed returns.
- `OAuth2Provider.get_oauth_client_id()` returns `Any`. getsentry
overrides it with an option typed `str | int`.
- `AbstractFile.getfile()` returns `Any`. getsentry's uploader treats it
as a `BufferedIOBase`.
- `BidirectionalMapping` is generic in its values
(`BidirectionalMapping[KT, VT]`), so lookups infer `str` instead of
`object`.
- `BitField` subclasses `BigIntegerField[Any, Any]`, so
`subscription.flags.<name>` resolves the way it does on master.

The `BitField` fix also restores two master behaviors the earlier push
had changed: plain string flags are added to `labels` again, and
`_arg_flags` holds the normalized flag list again, which is what
`deconstruct()` writes into migrations.

getsentry/getsentry#22868 is no longer needed for this PR.

**Verification**
- `mypy` (full repo, strict): `Success: no issues found in 8527 source
files`, on this branch and on this branch with all 10 stacked PRs
applied.
- `ruff check` / `ruff format --check` on the changed Python files, plus
`ruff --select F821,F401` across `src` and `tests`.
- getsentry master's mypy run against this branch reports the same 9
errors it reports against Sentry master: 6 are local-checkout gaps
(`scripts.*` and `devlocal` modules missing in my sandbox) and 3 are
already in `getsentry/jobs/migrate_legacy_seer.py`. None come from this
PR.
- I did not run pytest locally because there's no Postgres or Redis in
my sandbox. CI is the test signal.

Supersedes #129447 (fork branch, so I couldn't push to it).

<!-- junior-request-attribution:start -->
via @joshuarli.
<!-- junior-request-attribution:end -->

<!-- junior-session-footer:start -->
<!-- junior-conversation-id:slack%3AD0BQC9WSQ2E%3A1793660555.334709 -->

--

[View
Session](https://junior-prod.sentry.dev/conversations/slack%3AD0BQC9WSQ2E%3A1793660555.334709)
[[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AD0BQC9WSQ2E%3A1793660555.334709/?project=4510944073809921)

<!-- junior-session-footer:end -->

---------

Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Co-authored-by: Joshua Li <14209781+joshuarli@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@joshuarli