fix(ci): run fork gates from the trusted receiving repository - #461
fix(ci): run fork gates from the trusted receiving repository#461rita-aga wants to merge 4 commits into
Conversation
|
@greptile review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| steps: | ||
| - name: Start the gate on the contributor commit | ||
| id: contributor_check | ||
| uses: actions/github-script@v7 |
There was a problem hiding this comment.
These trusted pull_request_target workflows use mutable references such as actions/github-script@v7 and actions/checkout@v6.0.2. If an upstream tag is compromised or repointed, its code runs with the receiving repository’s write-capable token and can forge contributor checks. A compromised checkout action could also persist environment or PATH changes that intercept the later secret-bearing Stack step. Pin every action used by these trusted workflows to a full commit SHA. The same pattern appears in the planning, review, and verification workflows.
How this was verified: The mutable action references execute in trusted jobs granted checks: write and pull-requests: write, before later steps receive STACK_TOKEN.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/sdlc-decisions.yml
Line: 23
Comment:
**Mutable Actions In Trusted Gates**
These trusted `pull_request_target` workflows use mutable references such as `actions/github-script@v7` and `actions/checkout@v6.0.2`. If an upstream tag is compromised or repointed, its code runs with the receiving repository’s write-capable token and can forge contributor checks. A compromised checkout action could also persist environment or PATH changes that intercept the later secret-bearing Stack step. Pin every action used by these trusted workflows to a full commit SHA. The same pattern appears in the planning, review, and verification workflows.
**How this was verified:** The mutable action references execute in trusted jobs granted `checks: write` and `pull-requests: write`, before later steps receive `STACK_TOKEN`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| types: [opened, edited, synchronize, reopened] | ||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
| jobs: | ||
| decision-log: | ||
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
This PR introduces a new trusted pull_request_target validation and contributor-check reporting architecture across four workflows, but adds effort documentation instead of the required ADR under docs/adrs/. The repository requires an ADR before code for material architecture changes and new patterns. Document the security boundary, permissions, commit binding, and rollout order in an ADR before merging. The same issue applies to the planning, review, and verification workflows.
Context Used: CLAUDE.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/sdlc-decisions.yml
Line: 9-16
Comment:
**Architecture Change Lacks ADR**
This PR introduces a new trusted `pull_request_target` validation and contributor-check reporting architecture across four workflows, but adds effort documentation instead of the required ADR under `docs/adrs/`. The repository requires an ADR before code for material architecture changes and new patterns. Document the security boundary, permissions, commit binding, and rollout order in an ADR before merging. The same issue applies to the planning, review, and verification workflows.
**Context Used:** CLAUDE.md ([source](https://github.com/nerdsane/temper/blob/main/CLAUDE.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
ARN-498 review triageGreptile — Temper bf8b5f6
|
|
@greptile review |
Confirmation triageThe current Greptile findings do not require further source changes:
|
ARN-498 independent verificationVerified Stack
The first equivalence probe compared all job permissions literally and stopped because standalone Temper workflows and the combined Stack workflow differ in extra permissions. The corrected probe checks the permissions required by the behavior ( No confirmed defect. These are executed local regressions and comparison of the actual Temper workflow scripts against the tested scripts, with mocked GitHub/CLI boundaries in the tests. No live GitHub Actions execution is claimed. Source worktrees remained clean at the specified SHAs. This proof covers the local gate implementation and workflow wiring. The live acceptance case is the corrected receiving-repository Actions run on Nick’s unchanged fork heads after this repair lands; that result will be recorded separately. No application image changes or deployment are part of this workflow/docs repair. |
Third-round review status — incomplete panelCurrent-head Codex review: no findings. Independent verification:92tests and10subtests passed. Fable could not run because its subscription quota is exhausted. Grok failed to return a verdict within the bounded1200second attempt. These failures are not approvals. The normal one-model skip rule is not met. The three-round stop has been reached. Arbitration is assessing the existing evidence; no fourth panel or broader tooling repair is being started. The owning task retains the user's authorization to merge, but does not reinterpret that as a panel-quorum waiver. |
VerdictThe implementation loop is technically closed: every confirmed defect was fixed, the final heads passed available review and tests, and no implementation finding remains. The unresolved issue is panel availability, so a human must explicitly accept a one-time reviewer-minimum exception or pause. WhyFindings concentrated in rounds 1–2 on concrete workflow defects; each was corrected and independently reverified. The fixes remained narrow, added only the bounded watcher/rerun needed for evidence correctness, introduced none of the prohibited machinery, and still match the authorized intent. Round 3 found no defect: Codex was clean, 92 tests plus 10 subtests passed, and workflow equivalence and Temper CI passed. Fable exhausted quota and Grok timed out, leaving quorum unavailable rather than an adverse review result. Recurring threads
Verification limitsNo source checkout was available, so this judgment relies solely on the supplied history and reported evidence; the diff and test artifacts were not directly inspected. Full panel quorum was not achieved, no PR has merged, and live corrected-fork Actions acceptance can only be verified after landing. Options
Recommendation: Accept the one-time exception; another review round would violate the stopping decision without addressing an open implementation defect. Human questionDo you authorize a one-time reviewer-minimum exception for |
1 participant
Allow original fork contributions to use the existing private Stack gates. Trusted receiving-repository workflows fetch Stack, inspect contributor Git objects as data, and report results on the contributor commit. Evidence reruns follow the associated workflow. This fixes the STACK_TOKEN fork-secret mismatch without rehosting contributions.
Scope is token access and the required workflow/data-read/check-reporting changes only. Rita rejected and requested removal of the broader repair; the separate terminal-review schema/validator extension has been removed. Existing review requirements are unchanged. No MCP, proof-submission API, replacement chains, evidence-backend detection, or review-runner changes are included.
Tracks ARN-498. Companion: https://github.com/arni-labs/stack/pull/19 and #461. Verification after subtraction:68tests and10subtests passed. Prior review records remain historical and must not be represented as approval of this changed head. The previous exception request for the old heads is superseded by this scope correction. No merge or live fork-gate acceptance is claimed.
Decisions & Tradeoffs
Decisions
Decision: Run secret-bearing validation on the trusted receiving branch and inspect contribution objects without executing them.
Came up because: Fork pull_request jobs cannot receive STACK_TOKEN needed to fetch private Stack.
Options: Rehost contributions; distribute gate copies; correct workflow execution.
Chose workflow correction because: It preserves original contributions and the existing private tooling without another service. Builds stay unprivileged.
Where: Stack gates and Temper .github/workflows/sdlc-*.yml.
Decision: Use local isolated worktrees and GitHub directly for this effort.
Came up because: Temper MCP and health endpoint return HTTP 503.
Options: Wait indefinitely; repair the platform; use the local exception Rita explicitly approved.
Chose the approved exception because: It completes the narrow objective without reopening the broader platform work.
Where: Codex parent task 01a08157-a252-7b43-b1d9-facd84cd2695.
Decision: Preserve automatic-merge revocation when risk flags or workflow changes require human authorization, while never enabling auto-merge from the privileged validator.
Came up because: The original extraction removed that revocation together with automatic merge enabling.
Options: Remove both behaviors; retain revocation only.
Chose revocation only because: It preserves human control without granting the validator a new merge path. Revocation and Git/API failures remain visible failures.
Where: Stack gates/sdlc.yml and Temper sdlc-review.yml.
Decision: Use the documented Check Runs update fields and distinguish an absent feature map from a declared empty map.
Came up because: Review found a create-only field in the update request and a reproduced empty-map validation regression.
Options: Rely on undocumented extra-field handling and empty-set truthiness; preserve the existing contracts explicitly.
Chose explicit contracts because: A check is updated by its ID, while a declared feature map must reject unknown names even when it contains no features. The alleged HTTP422 was not observed; the API field correction follows GitHub's documented update contract.
Where: Contributor result reporters; Stack proof/validate.py and focused regressions.
Decision: Remove all additions outside the fork-token correction, including the terminal-review record extension.
Came up because: Rita explicitly rejected the extra work and authorized its cleanup.
Options: Retain the separate review-policy extension as a merge prerequisite; remove it.
Chose removal because: The accepted change is token access and its required workflow wiring. An unrelated gate limitation does not authorize another feature.
Where: Stack review files restored to the pre-effort versions; broad Stack17 and TemperPaw510 withdrawn; own additions to Temper436 removed.
Implemented and cleaned up by Codex (Astra), Codex desktop harness.
Greptile Summary
The PR moves private Stack validation into trusted
pull_request_targetworkflows while treating contributor Git objects as data and publishing gate results against the contributor commit.Confidence Score: 4/5
The implementation has no newly introduced blocking defect since the previous review, but the outstanding repository requirement for an ADR must be satisfied before merging.
The earlier mutable-action finding remains outstanding because the trusted workflows still use version tags such as
actions/github-script@v7andactions/checkout@v6.0.2. The architecture-documentation finding also remains outstanding: the effort documents do not provide the required ADR underdocs/adrs/for the new trusted validation and contributor-check architecture. The previous base-commit finding was manually resolved without explanation.Important Files Changed
Sequence Diagram
sequenceDiagram participant Fork as Contributor fork participant GitHub as GitHub pull request participant Gate as Trusted receiving-repository gate participant Stack as Private Stack tooling participant Checks as Contributor commit checks Fork->>GitHub: Push contributor commit GitHub->>Gate: Trigger pull_request_target Gate->>Checks: Create in-progress check on contributor SHA Gate->>GitHub: Fetch PR Git objects without checkout Gate->>Stack: Fetch trusted validator with STACK_TOKEN Gate->>Gate: Validate contributor objects as data Gate->>GitHub: Confirm PR head is unchanged Gate->>Checks: Complete check on contributor SHAReviews (4): Last reviewed commit: "docs: restrict gate correction to fork t..." | Re-trigger Greptile