Skip to content

Require external fork guards before cache restoration - #271

Merged
rmcdaniel merged 5 commits into
mainfrom
fix/270-fork-cache-restores-20261101
Nov 2, 2026
Merged

rmcdaniel merged 5 commits into
mainfrom
fix/270-fork-cache-restores-20261101

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

Pull-request cache restores can consume executable output saved by an earlier revision of the same external PR. Require a job or step condition that excludes external forks before restoring a cache; exact or event-partitioned keys alone do not establish trust.

The evaluator recognizes event/repository comparisons, grouped negation and Boolean composition. Unknown terms, including Unicode-confusable literals and main-looking refs, cannot establish trust. Protected-main writers, exact keys/paths, action pins and privileged-consumer restrictions remain unchanged.

Repository prerequisites are merged: Server #929 (1908dd4f), Waterline #254 (15aa0275), Workflow #973 (95f7ec01). A fresh immutable snapshot observed November 2 at 01:45:49 UTC passes all 97 workflows across 11 public-main targets, without candidate overlays; private Cloud is excluded. This is a workflow trust scan, not each product's complete test suite.

Combined source 4ec72c2ac8619f7fd4cf66a00f65588d95eac2a7 normally merges delivered dispatch-parser #273; the 11 normal Python modules pass 122 tests with two local npm/Cargo-client skips. Fork-policy controls previously produced 21 expected failing subcases on the old implementation, and Unicode-confusable controls preserve unknown/fail-closed behavior. Fresh current-head hosted checks are required before merge.

Closes #270. No permissions, required checks, cache capacity or spending limits are expanded.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Implemented in draft PR #271 at 83bd7cee5d21b019ab77ad360318c0d6d3ad24cf.

  • All 27 focused policy tests pass. The previous policy produces 21 expected failing subcases under the same new tests.
  • All 11 Python test modules complete: 120 tests, with the two npm/Cargo client controls skipped because those binaries are absent from the local disposable image. Hosted CI retains both controls and is required before merge.
  • The public catalog snapshot contains 97 workflow files across 11 targets. Eight current snapshots pass the workflow trust scan. Server requires six guards, Waterline three, and Workflow four.
  • The combined candidate catalog passes all 11 trust scans with Server PR Rust #325: skip cache restores on fork pull requests server#929, Waterline PR CI: exclude external forks from Composer caches waterline#254, Workflow PR CI: exclude external forks from Composer caches workflow#973 and the SDK-Rust cache PR CI: reuse trusted Rust dependency builds sdk-rust#148 overlaid. This is candidate workflow validation; those prerequisites have not yet merged, and it is not a full repository-test claim.
  • Workflow and Waterline have only the intended seven step conditions changed, verified by parsed workflow comparison. Their 35 event controls and complete candidate central-policy validation pass.

Next: complete Security/required CI and merge the repository prerequisites, refresh the public main catalog, then mark #271 ready only after that catalog satisfies the new policy. Existing permissions, check selection, writers and cache limits remain unchanged.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Both Sonar findings concerned exception-test scope. Fixed at d5e97b77e4257b410213812e2e9d7ff1db4bbede: construct each rejected workflow before entering assertRaisesRegex, leaving only the policy scan inside the expected-error block.

The policy implementation and all fixture inputs are unchanged from 83bd7cee. All 27 focused policy tests pass again. Fresh hosted checks are running for the new test-only head. The draft and repository-prerequisite merge conditions remain in force.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-head review of d5e97b7: accepted. The evaluator is conservative: unknown expressions remain potentially runnable, and a restore is accepted only when the job or step condition proves it cannot execute for an external fork. Coverage includes job inheritance, OR/AND grouping, case-insensitive event literals, negation precedence, and exact repository identity. The later Sonar cleanup does not change policy behavior. Keep the PR draft until the catalog is compatible through Server #929, Waterline #254, and Workflow #973; once those prerequisites are satisfied, there is no Security hold on this policy head.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Tightened the supported literal boundary at 4a006513a78d4f27999e9531260cea6c72b77c2c: known Boolean spellings use simple case conversion, and non-ASCII event literals remain unknown. This prevents Unicode lookalikes from being treated as proof of fork exclusion.

Three new negative fixtures fail the previous head and pass now. All 27 focused policy tests and all 11 local Python modules pass: 120 tests with two npm/Cargo client skips. The combined 11-target candidate trust scan passes. Existing ASCII guards and the draft's repository prerequisites are unchanged. This remains separate from the dispatch correction in #273.

@rmcdaniel

Copy link
Copy Markdown
Member Author

The dispatch correction #273 is now merged. A normal local merge of its main commit into this fork-policy branch is 4ec72c2ac8619f7fd4cf66a00f65588d95eac2a7; the public draft head remains 4a006513 while prerequisites are outstanding. The combined normal Python modules complete 122 tests with two local client skips.

The refreshed 97-workflow public catalog passes nine of 11 targets without overlays. Server and Waterline are the two remaining incompatibilities, owned by #929 and #254. Adding those exact candidate guards, together with SDK-Rust #151, passes all 11 targets. This is a candidate scan, not proof those prerequisites are merged. Keep this PR draft and refresh current mains before enabling enforcement.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-head review of 4a00651: accepted. Replacing Unicode case folding with ASCII-compatible literal handling prevents non-ASCII event and Boolean lookalikes from becoming trusted facts; those expressions remain unknown and therefore potentially runnable. Existing ASCII guards retain their prior truth model, the new negative fixtures cover the confusable cases, and current checks are green. Keep the PR draft until Server #929 lands; no Security hold after that catalog prerequisite.

@rmcdaniel

Copy link
Copy Markdown
Member Author

All prerequisites are merged: Server #929 as 1908dd4f7e28cf580accecf2a028434fc55c4c93, Waterline #254 as 15aa02757a39c8ed8d1af095e78d33fb02e48b0f, and Workflow #973 as 95f7ec01b5d1ab7c91573a918c795b58eb982d68.

A fresh immutable public-main workflow snapshot observed 2026-11-02 01:45:49 UTC passes 11/11 targets, 97 workflows, with no candidate overlays and private Cloud excluded. The first local scan lacked the existing PyYAML mount and did not run; the corrected invocation with both existing dependency mounts passes. This is actual main compatibility, not complete product-suite acceptance.

Combined normal-merge head 4ec72c2ac8619f7fd4cf66a00f65588d95eac2a7 is now pushed. Its combined normal 11 Python modules already completed 122 tests with two local missing npm/Cargo-client skips; current central main remains the same merged #273 commit. The 4a006513..4ec72c2a source delta is exactly the independently accepted #273 dispatch-parser change. Fresh hosted checks and bounded review carry-forward precede protected merge.

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 2, 2026 01:50
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security acceptance carries to combined current head 4ec72c2ac8619f7fd4cf66a00f65588d95eac2a7.

I independently reproduced the recorded tree as the clean merge of previously accepted fork-policy head 4a006513a78d4f27999e9531260cea6c72b77c2c and current main 07d5d6a3df4242f9b09550329f6abf22f461cd29; there are no conflict/manual merge edits. Main's tree is byte-identical to accepted #273 source tree f8e8487576d061251db7ffc934da913d00a33f2c, so the only delta is the already accepted manual-dispatch parser/tests combined with the accepted external-fork restore policy. git diff --check passes.

All seven current hosted checks are now green, including shared contract tests, Sonar and CodeQL. The recorded immutable 97-workflow/11-public-main snapshot supplies the catalog-wide behavior evidence. No new Security finding or hold.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security source acceptance carries to final exact head a948e64bbab36b12bcb2524e23e1d4d7cfbcdc6e.

The sole delta from accepted 4ec72c2a replaces the external-fork evaluator's inline predicate with the already accepted _supports_condition_negation(operand) helper. The helper implements the same literal-or-balanced-group test byte for byte in meaning, and is already used by the manual-dispatch evaluator. Truth outcomes, fail-unknown behavior, workflow inventory, and tests are unchanged. git diff --check passes; repeated 122 tests and the same 97-workflow/11-main scan are sufficient focused evidence.

Fresh hosted checks remain the normal merge gate and were still running at review time. No new Security finding or hold.

@sonarqubecloud

sonarqubecloud Bot commented Nov 2, 2026

Copy link
Copy Markdown

@rmcdaniel
rmcdaniel merged commit 76f3338 into main Nov 2, 2026
7 checks passed
@rmcdaniel

Copy link
Copy Markdown
Member Author

Delivered by protected merge 76f333804e68725059039fd54df241d40d855cd3 at 2026-11-02 01:56:26 UTC, exact source a948e64bbab36b12bcb2524e23e1d4d7cfbcdc6e. All seven source checks passed; actual final PR Sonar open findings: 0. Security accepted the exact final helper delta in comment 6567206523.

Normal main follow-through passed: Repository checks 41375056250, Sonar 41375056208, CodeQL 41375055836. The fresh immutable 97-workflow / 11-public-main catalog passed without overlays, and the repeated normal Python modules passed 122 tests with two local npm/Cargo-client skips. Source prerequisites #929/#254/#973 are merged; #270 is closed. No permissions, required checks, cache capacity or spending limits were expanded.

@rmcdaniel
rmcdaniel deleted the fix/270-fork-cache-restores-20261101 branch November 2, 2026 01:59
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.

Enforce the fork boundary before restoring executable CI caches

1 participant