feat(mint)!: harden repos scope against cross-repo minting - #5826
Conversation
Restrict same-org mints to the requesting repo by default, allow only empty repos on foreign mints, and gate org-mode exceptions behind PER_ORG_FOREIGN_COMPAT so workflows keep working when enabled. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
When mint deploy uses on-disk source and CLI commitSHA is still unset, stamp git HEAD into the mint and show Version/Commit in the summary. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Expose PER_ORG_FOREIGN_COMPAT on GET /v1/status and print it from traffic env in fullsend mint status (absent = off for older mints). Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Capture the PER_ORG_FOREIGN_COMPAT repos policy as an accepted ADR and point ADR 0060 / architecture.md at it. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Finished Review · ✅ Success · Started 8:59 AM UTC · Completed 9:18 AM UTC |
PR Summary by QodoHarden mint repos scoping to prevent cross-repo minting
AI Description
Diagram
High-Level Assessment
Files changed (21)
|
Site previewPreview: https://60c0ccef-site.fullsend-ai.workers.dev Commit: |
Code Review by Qodo
1.
|
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
Previous run (3)ReviewFindingsMedium
Low
Previous run (4)ReviewFindingsMedium
Low
Previous run (5)ReviewFindingsMedium
Low
Previous run (6)ReviewReason: stale-head The review agent reviewed commit Previous run (7)ReviewFindingsMedium
Low
Labels: PR modifies mint core authorization logic (internal/mintcore/) and adds security hardening for repos scope |
Update embedded zip file count, wire PER_ORG_FOREIGN_COMPAT into the CF Worker config JSON, stop enabling compat in TestMain by default, return distinct repos-scope errors, and warn when checkout commit resolution fails. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review · |
Include gofmt fix for handler_test.go map alignment that failed CI lint. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Finished Review · ✅ Success · Started 9:44 AM UTC · Completed 10:00 AM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Keep early source-dir resolution for dry-run from fullsend-ai#5763 and deploy-commit stamping from this branch; retain both sets of deploy tests. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Finished Review · ✅ Success · Started 7:06 AM UTC · Completed 7:21 AM UTC |
rh-hemartin
left a comment
There was a problem hiding this comment.
In general looks good to me, the only stuff I'm not sure about is the nil returning as []string type, otherwise the rest is purely cosmetic. Since there are no changes to the actions I guess they are properly configured and always pass repos to their requests.
Rename the cross-org boolean to isTargetForeign, use strings.Split for bare repo names, and add a blank line before the two-repo compat check. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
merge: sync harden-mint-repos-scope with upstream/main
|
🤖 Finished Review · ✅ Success · Started 9:29 AM UTC · Completed 9:44 AM UTC |
Return a shape label from validateReposScope and log it when a compat exception allows broader-than-self same-org repos for auditability. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
merge: sync harden-mint-repos-scope with upstream/main
|
🤖 Finished Review · ✅ Success · Started 10:16 AM UTC · Completed 10:35 AM UTC |
There was a problem hiding this comment.
See the review comment for full details.
Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:
internal/mintcore/handler.go:244: [low] breaking-api-behavior
POST /v1/token introduces two intentional breaking changes documented in ADR 0077: (1) same-org requests with empty repos or broad repo lists now return 403, (2) cross-org requests with non-empty repos now return 403. Properly documented with PER_ORG_FOREIGN_COMPAT migration path and ! suffix in PR title.
internal/mintcore/handler_test.go(file-level): Line 2433 · [low] test-integrity
TestHandler_PerRepoUnregistered sends repos test-repo but OIDC claim is test-org/unregistered-repo. If OIDC ever passed, repos scope check would reject with 403 instead of expected 401. Pre-existing pattern, not introduced by this PR.
internal/mintcore/handler.go:49: [low] response-schema-change
GET /v1/status adds new always-present field per_org_foreign_compat (bool, no omitempty). Additive and backward-compatible.
internal/mintcore/handler.go:96: [low] initialization-asymmetry
NewHandler reads PER_ORG_FOREIGN_COMPAT from os.Getenv but NewHandlerFromConfig does not accept a compat parameter. ParseWorkerConfig patches afterward. Three constructor paths initialize differently. Currently safe.
internal/mintcore/repos_scope.go:19: [low] input-normalization
repos * is normalized to empty repos before validation. On same-org path this means 403. Documented in ADR 0077.
|
🤖 Finished Retro · ✅ Success · Started 11:30 AM UTC · Completed 11:44 AM UTC |
Retro: PR #5826 — feat(mint)!: harden repos scope against cross-repo mintingWorkflow overviewThis was a human-authored (via Cursor) security-critical PR by @ifireball: 22 files changed, +1180/−93 lines, 14 commits over ~26 hours. The PR hardened mint Agent activity: 9 review agent runs (7 successful, 2 cancelled via stale-head detection), 0 code/fix/triage runs (human-authored PR). Agents repo: Review quality assessmentThis PR demonstrated strong complementarity between human and bot review — neither alone would have caught all issues:
The most impactful finding — a real correctness bug where Cloudflare Worker deployments would silently fail to honor 6 bot findings were fixed by the author. The human reviewer's 3 naming/idiom catches were also all fixed. One human finding (nil vs empty slice) was a false positive that the author corrected with a Go semantics explanation. Evidence for existing issues
What went well
Proposals filed |
resolveE2EToken in pkg/e2etest/auth.go previously omitted the Repos field, relying on the legacy empty-repos signal for org-wide tokens. Now that mint accepts "*" as the explicit org-wide form (PR fullsend-ai#5826), pass Repos: ["*"] so the e2e suite is ready for repos becoming mandatory (fullsend-ai#5831). Closes fullsend-ai#5830
Summary
reposauthorization: same-org mints default to the requesting repo only; foreign (cross-org) mints require emptyrepos(with["*"]as an empty alias). Org-mode shapes stay available only whenPER_ORG_FOREIGN_COMPATis enabled (ADR 0077).mint deployuses an on-disk checkout and the CLI commit is still unset/dev, stamp the mint withgit HEADfrom that source and surface Version/Commit in the deploy summary.PER_ORG_FOREIGN_COMPATonGET /v1/status(per_org_foreign_compat) and print it from traffic env infullsend mint status(missing → off, compatible with older mints).Fixes #5828
Fixes #5829
Accepted ADR edits
reposconsequences at ADR 0077. FOREIGN allowlists andtarget_orgare unchanged.Test plan
go testininternal/mintcore,internal/cli,internal/dispatch/gcf(repos scope, status field, deploy-commit resolution, mint status on/off, embedded zip file count)internal/dispatch/gcf/mintsrc/mintcore/matchinternal/mintcore/PER_ORG_FOREIGN_COMPAT=true: same-org org-mode /.fullsendshapes still mint; without the flag, same-org broad lists and install-wide are denied as expectedfullsend mint statusshowsPer-org foreign compat: on|offmatching traffic envdeve2e/behaviourjobs on this PR (CLI changes are covered by unit tests; fullmake e2e-testruns in the PR e2e workflow)BREAKING CHANGE: same-org mint requests that previously used installation-wide or broad org-mode
reposlists are denied unlessPER_ORG_FOREIGN_COMPATis enabled (and still never allow same-org installation-wide tokens). Foreign mints must use emptyrepos.