feat: store-backed workload identity lookup - #6285
Closed
Conversation
The database side of the admission interface: given an organization, the caller's project, a workload issuer row and a subject, whether an undeleted admission recognises that workload. EXISTS rather than the row, because that is the whole of what admission needs and returning a row invites a caller to read something else off it, widening the security boundary by accident. The tenancy predicate matches the issuer resolver exactly, and for the same reasons: organization_id unconditionally, so a project-tier row cannot answer outside its organization, and the project arm reads the caller's own project or the organization tier. A NULL @project_id makes that arm false rather than true, which is what stops a project's private admission from answering a caller that named no project. Key components are checked before the store rather than left to the query so a tenancy hole cannot depend on a data property a seed or fixture could break, and an empty subject can never match a row holding one. Eleven cases cover it, including both tiers, a sibling project and a withdrawn row. Two fail if the project arm is made unconditionally true. Claude-Session: https://claude.ai/code/session_01KYwFta55KqHStFGvWUHiJc
Contributor
🦋 Changeset detectedLatest commit: e86178d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Running ultrareview automatically — Security-critical admission lookup with subtle tenancy/NULL logic; a missed bug could grant or deny workload access incorrectly.. I'll post findings when complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Ultrareview completed in 3m 48s
No issues found across 5 files
Linked issue analysis
Linked issue: AIM-255: feat: repo-backed workload identity lookup
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | An admitted organization/project/issuer/subject tuple resolves, while changing any key component does not. | The adapter validates required inputs, and the SQL matches organization, issuer, subject, and project tier. Tests cover admission and mutations of each key component. |
| ✅ | Organization-tier admissions resolve for callers in any project and for callers with no project. | The SQL permits organization-tier rows with project_id IS NULL, and tests cover both another project and an organization-scoped caller. |
| ✅ | Project-tier admissions resolve only for their own project, not for an organization-scoped caller or sibling project. | The project predicate requires project_id = @project_id, while organization-scoped callers can match only project_id IS NULL. Tests cover own-project, sibling-project, and unset-project behavior. |
| ✅ | Soft-deleted admissions do not resolve. | The query filters deleted IS FALSE, with an integration test withdrawing an admission before lookup. |
| ✅ | Database errors surface as errors rather than being treated as non-admission. | The adapter returns a wrapped error from the generated repository call and does not convert failures into false, matching the required failure semantics. |
Contributor
|
Found 1 test failure on Blacksmith runners: Failure
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light.svg)
aa-wong
deleted the
waaronwong/aim-255-feat-repo-backed-workload-identity-lookup
branch
September 10, 2026 22:39
aa-wong
changed the title
Collaborator
Author
|
Superseded by #6294 — same commits, same base. The branch was renamed from |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
AIM-255
Summary
The database side of the admission interface #5952 defines and wires empty. One query and the function over it, in
server/internal/workloadidentitybesideResolveIssuerByURL, so both reads on the workload path share onerepo.DBTX.WorkloadIdentityIsAdmittedreturnsEXISTS, not the row — the boolean is the whole of what admission needs, and returning a row invites a caller to read something else off it.The tenancy predicate is deliberately identical to the issuer resolver's:
organization_idunconditionally, so a project-tier row can never answer outside its organization, and a project arm reading the caller's own project or the organization tier. The subject is matched by exact equality, with no expression around the column that would make the lookup index unusable.The two nulls
project_idis nullable on the table and on the query, and they mean different things — an unset row is the organization tier and answers every project, an unset query is an organization-scoped caller a project-tier row must not answer. SQL gives this for free:project_id = @project_idis not true when the parameter is NULL. Both directions are tested.Failing closed before the store
An empty organization, nil issuer or empty subject returns
(false, nil)without querying, for the same reasonResolveIssuerByURLchecks its organization: a tenancy hole should not depend on a data property a seed could break. A store error propagates as an error and never as non-admission.Motivation
#5952 ships the admission logic with no storage so the security logic stays independently testable. This supplies the store behind it. No production caller yet — AIM-259 is the grant that will call it.
Stacking
Based on #5952, which defines the types. Deliberately not stacked behind #6282 or #5878: this is the last Urgent item in the verification milestone and only needs those types.