ref(search): Resolve issue: short ids org-wide with explicit project_ids=None#117398
Merged
Conversation
When an issue:PROJECT-123 filter is resolved to a group id, pass the query's project scope into by_qualified_short_id_bulk so a short id for a project outside the search scope does not resolve. filter.py passes params['project_id'] (None when absent -> org-wide query); discover.py and eap/resolver.py pass SnubaParams.project_ids. The resolved ids already feed a Snuba query scoped to those projects, so this is defense-in-depth that fails closed at resolution time. Co-Authored-By: Claude <noreply@anthropic.com>
github-actions
Bot
added
the
Scope: Backend
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
cvxluo
reviewed
Jun 11, 2026
Comment on lines
+259
to
+260
| # Scope to the projects the query is restricted to so a short id for a | ||
| # project outside the search scope does not resolve. |
Contributor
There was a problem hiding this comment.
Suggested change
| # Scope to the projects the query is restricted to so a short id for a | |
| # project outside the search scope does not resolve. |
cvxluo
reviewed
Jun 11, 2026
| short_ids_raw=list(collected), | ||
| # Scope to the projects the query is restricted to so a short id for a | ||
| # project outside the search scope does not resolve. | ||
| project_ids=self.params.project_ids, |
Contributor
There was a problem hiding this comment.
i don't think the params are same as projects that the caller has access to here. e.g. you can write a search query for project_ids 1, 2, 3, even if you don't have access to project id 1.
Member
Author
There was a problem hiding this comment.
i think you're right, will ruminate a bit with the agent here
… lookup SnubaParams.project_ids returns [] when a query has no projects. Passing [] to by_qualified_short_id_bulk filters to zero projects, so the short id never resolves and the bulk lookup raises Group.DoesNotExist -> InvalidIssueSearchQuery, erroring a query that previously returned empty results. Fall back to None (no pre-filter) for an empty scope, which is safe because the downstream Snuba query still enforces the project filter. Non-empty scopes are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
…ids=None Supersedes scoping issue: short id resolution to the query's project_ids. SnubaParams.project_ids is the query's project scope, not the caller's access set, and project enforcement is unnecessary here: the resolved group id is only applied inside a Snuba query already scoped to the requested projects (RequestMeta.project_ids), with access enforced upstream by get_projects(). So the search sites resolve org-wide and pass project_ids=None explicitly -- no behavior change from master, but the org-wide intent is explicit and forward-compatible with making project_ids required. Co-Authored-By: Claude <noreply@anthropic.com>
oioki
changed the title
cvxluo
reviewed
Jun 11, 2026
| organization_id=self.params.organization_id, short_ids_raw=list(collected) | ||
| organization_id=self.params.organization_id, | ||
| short_ids_raw=list(collected), | ||
| # org-wide: the Snuba query is already scoped to the requested projects. |
Contributor
There was a problem hiding this comment.
hm ok. it seems like we should just have a by_qualified_short_id_bulk_unscoped sort of function instead, so you have one function that does validation and one that explicitly doesn't
Member
Author
There was a problem hiding this comment.
i'm afraid people might misuse by_qualified_short_id_bulk_unscoped and an IDOR slips through. having a single method where a developer has to set project_ids explicitly is more secure imo
cvxluo
approved these changes
Jun 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates the three
issue:PROJECT-123short-id resolution sites (filter.py,datasets/discover.py,eap/resolver.py) to pass an explicitproject_ids=Noneintoby_qualified_short_id_bulk, preparing for the upcoming change that makesproject_idsa required argument.Why
None(org-wide), not the query's projects? Project-level enforcement is unnecessary at this resolution step:RequestMeta.project_idsin the EAP resolver; aproject_idcondition in discover/filter). A short id for a project outside the query simply matches zero rows.SnubaParamsviaget_projects()(which runshas_project_access). Andby_qualified_short_idis itself org-scoped, so cross-org never resolves.SnubaParams.project_idsis the query's project scope, not the caller's access set (internal callers can construct arbitrary project_ids), so scoping resolution to it would be both unnecessary and semantically misleading.So these sites resolve org-wide and document that intent with
project_ids=None. No behavior change from master — it just makes the org-wide choice explicit and forward-compatible with the required-project_idshardening. (Earlier revisions scoped toparams.project_ids; superseded per review — the genuinely protective scoping lives in the issue mutation path, not search.)