Skip to content

Restrict release cache writes to verified main warm runs - #71

Merged
rmcdaniel merged 4 commits into
mainfrom
fix/70-release-cache-boundary-20261101
Nov 1, 2026
Merged

rmcdaniel merged 4 commits into
mainfrom
fix/70-release-cache-boundary-20261101

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

Release jobs previously checked out a dynamically selected source revision while retaining automatic cache-save capability for the default branch. Split restore/save actions, restrict saves to verified protected-main warming, and make every checkout in cache-writing native jobs use the trusted workflow event SHA with credentials disabled. Release payloads remain exact source-verified PHAR artifacts; release jobs restore and verify caches without saving them.

Merged as 639355dc92d49825b4806ee39b4e4203a31218f1, with a tree identical to qualified head 50a1d135. Normal PHP 8.2–8.5, PHAR/reproducibility, CodeQL and actual Sonar checks passed, with Security acceptance. Default-branch alert 9 is fixed from source without dismissal.

Delivered and independently accepted in CLI 2.3.3. All four main warm platforms passed; the normal release restored those exact cache keys and skipped all seven save-step instances. All 14 published asset hashes/provenance subjects, Linux native/PHAR execution, pinned and actual website installs, signed 2.3.2→2.3.3 upgrade, profile preservation and no-op behavior passed. Task-owned resources and the merged head branch were cleaned up. Complete delivery evidence and timings.

Closes #70.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security review at exact head 5f0f1840c4dfa60beb06da642b46e097c529ac77 found one blocking result: required aggregate CodeQL check 124740788870 failed with three new high-severity “Cache Poisoning via caching of untrusted files” findings on the explicit save steps at release workflow lines 625–640, 856–872, and 1461–1477. The semantic save guards are appropriately narrow, but static analysis does not prove that the dynamic resolver outputs equal the trusted event SHA on schedule and workflow-dispatch paths.

The cache-writing jobs do not need to check out the selected release source: their release payload is the separately built/downloaded PHAR, and the only root-source use is the POSIX launcher qualification script. Remove the release-source checkout from both cache-writing jobs; use literal ${{ github.sha }} for the trusted control checkout, include/invoke the launcher test from that checkout, and add a policy assertion that neither cache-writing job checks out needs.resolve-release.outputs.commit. Then rerun CodeQL. Alert 9 must close from corrected merged source rather than dismissal. No additional security blocker was found in the reviewed diff at this head.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security re-review accepted at exact successor f1088443a8e92ae8cfbee210d535ecb508673090 / tree 135ff6ca29bf08bec65d7c0d5639e435b3611334. Both cache-writing jobs now use literal ${{ github.sha }} for root and control checkouts with persisted credentials disabled; no resolver-derived source/control checkout remains in either job. The regression policy covers all four checkouts and passes 6/6 locally.

Required hosted evidence is now green on this exact head: aggregate CodeQL check 124741809405 succeeded with zero annotations and no open PR-ref code-scanning alerts; all other listed checks also succeeded. The prior failed head and three HIGH findings remain preserved as the counterexample. No security blocker remains for protected merge. Default-branch alert 9 still needs source-driven closure and the next normal warm/release-path acceptance after merge, without dismissal.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security re-acceptance at final exact head e82007aabf6b78617f8d43f977ad3070278327dc / tree 03a3ba27ce440a17274f0c4e42c019f1c7cd9af1: the only delta from accepted f1088443 is the semantics-preserving dictionary-comprehension rewrite in the policy test. git diff --check is clean and the policy suite passes 6/6 locally.

Fresh exact-head evidence is terminal: aggregate CodeQL check 124742293972 succeeded with zero annotations and zero open PR-ref alerts; Sonar and every listed hosted check succeeded. The cache-writing checkout boundary remains unchanged and accepted. No security blocker remains for protected merge. Post-merge acceptance remains source-driven closure of default-branch alert 9 plus the next normal cache-warm and release-path evidence; no dismissal.

@sonarqubecloud

sonarqubecloud Bot commented Nov 1, 2026

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security final-head acceptance at 50a1d13537180cb8f2ea4aa83b7e0230bf04edbe / tree 3e3aeb0fd515ebc2b00c88be5315f68c909358fe: the only delta from e82007aa replaces split with partition in test-only resolver-output parsing. The workflow and cache trust boundary are unchanged; git diff --check is clean and the policy suite passes 6/6 locally.

Every exact-head hosted check is now terminal success. Aggregate CodeQL 124742885286 has zero annotations and the PR ref has zero open code-scanning alerts; Sonar is green. No security blocker remains for protected merge. As before, verify default-branch alert 9 closes from corrected source and accept the next normal cache-warm/release paths without dismissal.

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 1, 2026 03:52
@rmcdaniel
rmcdaniel merged commit 639355d into main Nov 1, 2026
16 checks passed
@rmcdaniel
rmcdaniel deleted the fix/70-release-cache-boundary-20261101 branch November 1, 2026 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep release-source execution out of save-capable cache jobs

1 participant