-
Notifications
You must be signed in to change notification settings - Fork 103
docs: Add ADR 074 with a single-auth #5649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| --- | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] ADR number 0075 collides with open PR #5562, which independently also claims 0075 Verified via Suggestion: Run the /renumber-adr skill (or |
||
| title: "75. Lite auth mode" | ||
| status: Accepted | ||
| relates_to: | ||
| - agent-architecture | ||
| - security-threat-model | ||
| topics: | ||
| - identity | ||
| - per-repo | ||
| - least-privilege | ||
| --- | ||
|
|
||
| # 75. Lite auth mode | ||
|
|
||
| Date: 2026-07-22 | ||
|
|
||
| ## Status | ||
|
|
||
| Accepted | ||
|
|
||
| <!-- ADRs are point-in-time records, but not fully frozen after acceptance. | ||
| Minor annotations are welcome: cross-references to related ADRs, short | ||
| notes linking to newer decisions, or clarifying remarks. However, do not | ||
| substantially rewrite the Context, Decision, or Consequences sections. If | ||
| the decision itself needs to change, write a new ADR that supersedes this | ||
| one. For evolving design narrative, use docs/architecture.md. --> | ||
|
|
||
| ## Context | ||
|
|
||
| Per-repo installation ([ADR 0033](0033-per-repo-installation-mode.md)) historically required provisioning per-role GitHub Apps ([ADR 0007](0007-per-role-github-apps.md)) and a token mint ([ADR 0029](0029-central-token-mint-secretless-fullsend.md)), creating nontrivial setup friction. While the Per-role App + Mint architecture remains the default for scaled, highly isolated environments, some per-repo adopters need a zero-ceremony opt-in path using the default `secrets.GITHUB_TOKEN`. | ||
|
|
||
| A design spike investigated using `secrets.GITHUB_TOKEN` (`github-actions[bot]`). Because GitHub suppresses events triggered by `GITHUB_TOKEN`, standard stage handoffs (`labeled`, `pull_request_review.submitted`) cannot initiate subsequent workflow steps natively. However, `workflow_dispatch` is exempt from event suppression and can be invoked securely to chain stages without over-privileged scopes (maintaining `contents: read` for triage and review, per `internal/mintcore/github.go`). | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] internal/mintcore/github.go citation implies mint enforcement that Lite Auth Mode explicitly bypasses Context justifies keeping Suggestion: Cite |
||
|
|
||
| Additionally, native auto-merge requires `APPROVE` reviews, which fail with a 422 error if the same identity that authored the PR (e.g., `GITHUB_TOKEN`) tries to approve it. Exchanging a Bring-Your-Own-App (BYOA) PEM for a separate token bypasses this restriction, but exposes the PEM to prompt-injection or runner-compromise threats if not properly isolated. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] "BYOA" acronym collides with the established "Bring Your Own Agent" term This ADR defines "BYOA" to mean "Bring-Your-Own-App"/"Bring Your Own App" (a dedicated Reviewer GitHub App) at lines 34, 42, 50, and 51, echoed in Suggestion: Rename the new concept to avoid the collision — e.g. "Bring Your Own Reviewer (App)" or "BYO Reviewer App" — in both |
||
|
|
||
| ## Options | ||
|
|
||
| - **`GITHUB_TOKEN` with native event triggers:** Rejected because GitHub suppresses downstream workflows triggered by `GITHUB_TOKEN` events. | ||
| - **`GITHUB_TOKEN` with `repository_dispatch` handoffs:** Rejected because `repository_dispatch` requires `contents: write` on triage/review, violating least-privilege role definitions. | ||
| - **`GITHUB_TOKEN` with `workflow_dispatch` handoffs:** Chosen. Only requires `actions: write` (already held), preserving `contents: read` for triage/review. | ||
| - **Single self-owned App with native handoffs (no mint):** Rejected for default lite mode because storing the PEM as a static secret introduces standing-credential risk. | ||
| - **Bring Your Own App (BYOA) for Review:** Chosen for auto-merge. Bypasses the 422 self-approval error by using a dedicated Reviewer App token for the review approval step. | ||
|
|
||
| ## Decision | ||
|
|
||
| Introduce **Lite Auth Mode** as an opt-in alternative to the default Per-role GitHub App + Mint architecture for per-repo installations. In Lite Auth Mode: | ||
|
|
||
| 1. **Authentication:** The default `GITHUB_TOKEN` is used for triage, code, and review steps instead of a mint-provided token. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Stage-coverage list omits fix/retro, inconsistent with the rest of the ADR Decision item 1 says the default GITHUB_TOKEN "is used for triage, code, and review steps" — omitting fix and retro. But Decision item 2 (line 49) calls for updating post-retro.sh, implying retro is in-pipeline under Lite Auth Mode, and Consequences (line 58) states "only code and fix carry contents: write," implying fix is in-pipeline too. It's never stated whether fix and retro also run under the default GITHUB_TOKEN — which is needed for the "zero GitHub-credential secrets" claim in Consequences (line 55) to actually hold across the full pipeline — or authenticate some other way. Suggestion: Enumerate all pipeline stages Lite Auth Mode covers (triage, code, review, fix, retro) consistently everywhere a stage list appears in this ADR, or explicitly scope fix/retro out and explain their authentication path. |
||
| 2. **Handoffs:** Stage handoffs use `workflow_dispatch` (`gh workflow run`) rather than `repository_dispatch` or native events. This requires only `actions: write`, allowing triage and review to maintain least-privilege `contents: read`. `post-review.sh` and `post-retro.sh` will also be updated to use `workflow_dispatch` to close pipeline loops. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] ADR assigns post-review.sh/post-retro.sh work to a repo where those scripts no longer exist Decision item 2 states: "post-review.sh and post-retro.sh will also be updated to use workflow_dispatch to close pipeline loops." Verified directly against origin/main: Suggestion: Add a note to Decision item 2 (or Consequences) that the post-review.sh/post-retro.sh workflow_dispatch updates are fullsend-ai/agents-side follow-up work, tracked separately per the ADR-0058 Phase 4 repo split. |
||
| 3. **Auto-Merge (BYOA):** To bypass GitHub's self-approval 422 restriction, operators can provide a dedicated "Reviewer" App ID and PEM as repository secrets (`FULLSEND_REVIEWER_APP_ID`, `FULLSEND_REVIEWER_APP_PEM`). | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] BYOA Reviewer App's required permission scope for satisfying branch-protection approvals is unspecified Decision item 3 has operators provision a dedicated "Reviewer" App ID/PEM but never states what GitHub permission level that App needs (e.g. Suggestion: Specify the required permission scope for the Reviewer App (explicitly call out
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] FULLSEND_REVIEWER_APP_ID described as a secret, contradicting the established App-ID-as-variable convention Decision item 3 has operators provide "a dedicated 'Reviewer' App ID and PEM as repository secrets ( Suggestion: Store the App ID as a repository variable (e.g. |
||
| 4. **BYOA Security Architecture:** To mitigate prompt-injection threats, the Reviewer App token **MUST** be minted in an isolated workflow step or job *after* the LLM agent has safely exited, ensuring the PEM is never exposed to the LLM execution context. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] Reviewer-App-token isolation requirement contradicts itself on step vs. job, and "step" alone doesn't defend the named threat Decision item 4 requires the Reviewer App token be minted "in an isolated workflow step or job" after the LLM agent exits — but Consequences (line 57) narrows the same claim to only "a post-execution job," eliminating "step" as an option. The ADR contradicts itself on its own central security control. This isn't pedantic: Context (line 34) names "runner-compromise" as one of the two threats being defended against, and GitHub Actions steps within a single job share the same runner filesystem/process environment as every prior step in that job — a later step does not isolate the PEM from a compromised or prompt-injected agent that ran earlier in the same job; only a separate job (fresh runner) does. As written, an implementation can satisfy the letter of the MUST clause ("a step... after the agent has safely exited") while remaining exposed to the exact threat the ADR names, and the Consequences section's stronger claim ("eliminates the threat") is not actually guaranteed by what Decision item 4 requires. Suggestion: Resolve the contradiction by requiring a separate job specifically (drop "step" as a satisfying option) in Decision item 4, matching what Consequences already claims and reserving any step-level option for steps that themselves run in a freshly provisioned container/runner. |
||
|
|
||
| ## Consequences | ||
|
|
||
| - Per-repo installations can adopt Lite Auth Mode with zero GitHub-credential secrets, though GCP inference credentials are still required. | ||
| - Auto-merging PRs requires provisioning a dedicated Reviewer App to bypass the 422 self-approval restriction. | ||
| - Isolating the Reviewer App token exchange in a post-execution job eliminates the threat of exposing the PEM to an untrusted LLM context. | ||
| - Triage and review jobs maintain least-privilege `contents: read` scopes, while only code and fix carry `contents: write`. | ||
| - The reusable-dispatch workflow and shim configurations require updating to handle `workflow_dispatch` inputs and routing hints. | ||
| - Per-role least-privilege scoping is lost at the identity level, but GHA job-level permissions still restrict the ephemeral `GITHUB_TOKEN`. | ||
| - Lite Auth Mode remains a strictly opt-in mechanism, preserving the Per-role GitHub App + Mint mode as the default for scaled installations. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MEDIUM] ADR 0033 annotation substantially re-argues the superseded rationale instead of a short clarifying note
The block added at lines 69-81 doesn't just cross-reference ADR 0075 — it re-derives the technical reasoning and explicitly states the original "Rejected" verdict for Alternative 2 rested on an incorrect premise ("the fix→review loop this alternative worried about is not actually broken by sharing one App", "not because sharing one App is technically broken"). This appears technically accurate (GitHub's documented event-suppression is specific to GITHUB_TOKEN and does exempt App tokens/PATs), but it goes well beyond a "short note" or "clarifying remark." It's inconsistent with this repo's own ADR-immutability convention: ADR 0029 states "Once this ADR is Accepted, its content is frozen. Do not edit the Context, Decision, or Consequences sections... Only status changes and links to superseding ADRs should be added," and this same PR's new ADR 0075 codifies an equivalent norm in its own template comment ("Minor annotations are welcome... However, do not substantially rewrite the Context, Decision, or Consequences sections"). ADR 0033 itself carries no such comment, so nothing is technically violated, but the edit sets a precedent inconsistent with the standard this very PR introduces elsewhere.
Suggestion: Trim the ADR 0033 annotation to a short pointer (e.g. "Clarification (2026-07): the GITHUB_TOKEN-suppression reasoning above was imprecise — see ADR 0075 for the corrected analysis") and keep the full technical re-argument only in ADR 0075's own Context, which already restates it.