Repository navigation
Harden pr-merge's run-cache restore in the token-holding jobs #9455
Copy link
Copy link
Open
Labels
needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.Human-in-the-loop required; autonomous sessions must not resolve items carrying this.status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.
Description
Problem
Two jobs of .github/workflows/pr-merge.yml restore the pr-merge-run-cache-* actions/cache entry by bare prefix (restore-keys: pr-merge-run-cache-) after they hold the lanes App merge token:
- the
mergejob ("Restore the verdict reads the last run kept"); - the
enable-auto-mergejob, on its scheduled and hand-started runs, added by fix(pr-merge): require a pr-verify pass before a lane pull request merges natively #9450 for parity.
Any job in the default branch's cache scope can write a newer key under that prefix. carry-run-cache.mjs already filters the restored JSON, so a poisoned entry's contents can only make the gate refuse; they cannot forge a pass.
Two risks are not covered:
- Extraction path (UNVERIFIED). The security review of fix(pr-merge): require a pr-verify pass before a lane pull request merges natively #9450 raised this concern from published cache-poisoning research (Cacheract): actions/cache may extract a crafted archive with entries outside the declared
path. If it does, a poisoned entry could overwrite files under.base/(for examplecarry-run-cache.mjsorenable-auto-merge.mjs) before they run with the lane token andid-token: write. This has not been verified foractions/cache/restorev6.1.0. - Who can write the key. It has not been checked which default-branch jobs could be made to run attacker-influenced code that writes that key.
Proposal
Pick one, after verifying the extraction behaviour:
- Verify the archive. Restore into an empty directory in a step or job that holds no token. List the archive entries, refuse any path outside the single expected file, and only then copy the file into place.
- Move to the artifact-read pattern. Use the pattern perf(source-control): carry untied FAIL head refs between pr-merge runs #9416 uses for the head-ref map: an artifact uploaded only by a scheduled or hand-started run of main's pr-merge.yml, found through that run's record. That removes the shared cache scope from the trust path.
Either change should land in both jobs together.
Done when
- The extraction behaviour of the pinned cache restore is verified, and the source is cited.
- Neither token-holding job extracts a cache archive a non-pr-merge job could write, or the archive's paths are checked first.
carry-run-cache.mjs's content filter is kept.
Refs: #9450
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.Human-in-the-loop required; autonomous sessions must not resolve items carrying this.status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.
Type
Fields
No fields configured for Task.
Activity
github-actions commentedon Oct 23, 2026
Proposed work class: none
Thanks for the report. Automated triage handed this issue to a maintainer.
Unconfirmed model suggestion: work-class: structural
Basis: automated intake triage; a maintainer confirms or changes it.
Automated triage. A maintainer reviews it.
kyle-sexton commentedon Oct 23, 2026
The claude-security-review lane on #9450 (#9450 (comment)) suggests an integrity check on the carried run cache: an HMAC, or a check that a main-branch
pr-mergerun saved it. It also suggests reading the cache back through the artifact path. Today the carried sections can only count a FAIL (carry-run-cache.mjs:9-11;Reader._run_statereads a PASS's run fresh), so this protects the refuse direction and the extraction path, not the pass direction.