From 83bd7cee5d21b019ab77ad360318c0d6d3ad24cf Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Sun, 1 Nov 2026 21:33:34 +0000 Subject: [PATCH 1/4] Require cache restore guards against external fork PRs --- qualification/README.md | 4 ++ scripts/qualification_policy.py | 73 ++++++++++++++++++++++++++++-- tests/test_qualification_policy.py | 62 +++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 4 deletions(-) diff --git a/qualification/README.md b/qualification/README.md index e040e15..accd4e8 100644 --- a/qualification/README.md +++ b/qualification/README.md @@ -29,6 +29,10 @@ scanner, with the analysis token only on the final scanner step. It may produce coverage first using trusted repository code and approved actions. Other steps cannot reference secrets; existing cache restrictions still apply. Forks do not run this job. +Cache restores in pull-request workflows must also exclude external forks at +the job or step condition. An earlier revision can populate the same PR cache, +so event-partitioned keys alone do not protect a later revision's build. An exact +shared key remains available for guarded restores and protected-main-only saves. The scanner rejects `pull_request_target`, mutable container images, unreviewed `workflow_run` consumers, and privileged artifact consumers without exact producer and digest provenance. diff --git a/scripts/qualification_policy.py b/scripts/qualification_policy.py index 1f41f03..ae2002b 100644 --- a/scripts/qualification_policy.py +++ b/scripts/qualification_policy.py @@ -1303,6 +1303,68 @@ def _condition_truth_for_unprotected_dispatch(expression: str, protected_ref: st return value, not value +def _condition_truth_for_external_fork(expression: str) -> tuple[bool, bool]: + """Return possible truth values for an external fork's pull-request event. + + Only the event and unequal repository identities are fixed. Ref checks and + other terms remain unknown, so a main-looking ref cannot establish trust. + """ + + expression = _strip_balanced_parentheses(expression) + disjunction = _split_condition(expression, "||") + if disjunction: + values = [_condition_truth_for_external_fork(part) for part in disjunction] + return any(value[0] for value in values), all(value[1] for value in values) + conjunction = _split_condition(expression, "&&") + if conjunction: + values = [_condition_truth_for_external_fork(part) for part in conjunction] + return all(value[0] for value in values), any(value[1] for value in values) + if expression.startswith("!") and not expression.startswith("!="): + operand = expression[1:].strip() + # ! binds before comparison in Actions expressions. Only invert a + # grouped expression or literal, never an ungrouped comparison. + if operand.casefold() not in {"true", "false"} and ( + not operand.startswith("(") or _strip_balanced_parentheses(operand) == operand + ): + return True, True + can_be_true, can_be_false = _condition_truth_for_external_fork(operand) + return can_be_false, can_be_true + if expression.casefold() in {"true", "false"}: + value = expression.casefold() == "true" + return value, not value + + event = re.fullmatch(r"github\.event_name\s*(==|!=)\s*(['\"])(.*?)\2", expression) + if event is not None: + operator, _quote, expected = event.groups() + equality = expected.casefold() == "pull_request" + value = equality if operator == "==" else not equality + return value, not value + repository = re.fullmatch( + r"(github\.event\.pull_request\.head\.repo\.full_name|github\.repository)\s*" + r"(==|!=)\s*(github\.event\.pull_request\.head\.repo\.full_name|github\.repository)", + expression, + ) + if repository is not None: + left, operator, right = repository.groups() + if left != right: + value = operator == "!=" + return value, not value + return True, True + + +def _cache_restore_excludes_external_forks(job: Mapping[str, Any], step: Mapping[str, Any]) -> bool: + # Both guards must pass before a step runs; either can exclude a fork. + for scope in (job, step): + condition = scope.get("if") + if isinstance(condition, str) and condition.strip(): + can_run, _can_skip = _condition_truth_for_external_fork( + _condition_without_expression_wrapper(condition) + ) + if not can_run: + return True + return False + + def _protects_manual_dispatch(job: dict[str, Any], protected_ref: str) -> bool: condition = job.get("if") if not isinstance(condition, str) or not condition.strip(): @@ -1686,11 +1748,14 @@ def _scan_workflow_trust( if not isinstance(path, str) or _unsafe_cache_path(path): raise PolicyError(f"{label} cache step in job {job_name!r} has an unsafe cache path") if pull_request: - # Trusted writes, untrusted reads: a restore-only step, or a - # save only a push to the protected branch can run, may share - # one exact key across events. A broad restore prefix could - # substitute stale output, so a shared key allows none. variant = parsed[1] if parsed is not None else "" + if variant != "save" and not _cache_restore_excludes_external_forks(job, step): + raise PolicyError( + f"{label} pull-request cache restore in job {job_name!r} " + "must exclude external forks at the job or step condition" + ) + # Trusted reads and protected writes may share one exact + # key. A broad restore prefix could substitute stale output. shared_key = variant == "restore" or ( variant == "save" and _requires_protected_push(step.get("if"), protected_ref) ) diff --git a/tests/test_qualification_policy.py b/tests/test_qualification_policy.py index c7fd3b7..3f15cc9 100644 --- a/tests/test_qualification_policy.py +++ b/tests/test_qualification_policy.py @@ -381,12 +381,14 @@ def test_pull_request_cache_rejects_broad_paths(self) -> None: self.scan(source) TRUSTED_SAVE = "github.event_name == 'push' && github.ref == 'refs/heads/main'" + TRUSTED_READ = "github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository" def trusted_main_cache(self) -> str: commit, version = self.approved_action("actions/cache") save_if = f"success() && {self.TRUSTED_SAVE} && steps.cache.outputs.cache-hit != 'true'" return workflow( f"""- id: cache + if: ${{{{ {self.TRUSTED_READ} }}}} uses: actions/cache/restore@{commit} # {version} with: path: build-cache @@ -406,6 +408,66 @@ def test_pull_request_cache_shares_an_exact_key_written_only_by_protected_pushes reordered = "github.ref == 'refs/heads/main' && (github.event_name == 'push')" self.scan(source.replace(self.TRUSTED_SAVE, reordered)) + def test_cache_restores_require_a_non_fork_guard_even_with_partitioned_keys(self) -> None: + source = self.trusted_main_cache() + guard = f" if: ${{{{ {self.TRUSTED_READ} }}}}\n" + for path in ("target/debug/deps", "vendor", "node_modules", "~/.cargo/registry/cache"): + candidate = source.replace("path: build-cache", f"path: {path}") + for partitioned in (False, True): + changed = candidate.replace(guard, "") + if partitioned: + changed = changed.replace("key: build-v1-", "key: build-${{ github.event_name }}-") + with self.subTest(path=path, partitioned=partitioned), self.assertRaisesRegex( + PolicyError, "cache restore.*external fork" + ): + self.scan(changed) + + def test_cache_restore_guard_rejects_fork_and_main_ref_bypasses(self) -> None: + source = self.trusted_main_cache() + same_repository = "github.event.pull_request.head.repo.full_name == github.repository" + for condition in ( + "true", + "github.ref == 'refs/heads/main'", + "github.event_name == 'pull_request'", + "github.event_name == 'PULL_REQUEST'", + "github.event_name != 'workflow_dispatch'", + f"{same_repository} || github.event_name == 'pull_request'", + f"{same_repository} || github.ref == 'refs/heads/main'", + f"{same_repository} || success()", + f"!({same_repository})", + "!github.event_name != 'push'", + "!github.event.pull_request.head.repo.full_name != github.repository", + "github.event_name == 'workflow_dispatch' || github.ref == 'refs/heads/main'", + ): + with self.subTest(condition=condition), self.assertRaisesRegex( + PolicyError, "cache restore.*external fork" + ): + self.scan(source.replace(self.TRUSTED_READ, condition)) + + def test_cache_restore_guards_accept_protected_and_same_repository_contexts(self) -> None: + source = self.trusted_main_cache() + same_repository = "github.event.pull_request.head.repo.full_name == github.repository" + for condition in ( + same_repository, + "github.repository == github.event.pull_request.head.repo.full_name", + "!(github.event.pull_request.head.repo.full_name != github.repository)", + f"success() && ({same_repository})", + f"({self.TRUSTED_SAVE}) || (github.event_name == 'pull_request' && {same_repository})", + f"always() && ({self.TRUSTED_READ})", + self.TRUSTED_SAVE, + "false", + ): + with self.subTest(condition=condition): + self.scan(source.replace(self.TRUSTED_READ, condition)) + + def test_cache_restore_can_inherit_the_job_non_fork_guard(self) -> None: + source = self.trusted_main_cache() + source = source.replace(f" if: ${{{{ {self.TRUSTED_READ} }}}}\n", "") + source = source.replace(" runs-on:", f" if: ${{{{ {self.TRUSTED_READ} }}}}\n runs-on:") + self.scan(source) + with self.assertRaisesRegex(PolicyError, "cache restore.*external fork"): + self.scan(source.replace(self.TRUSTED_READ, "github.ref == 'refs/heads/main'")) + def test_pull_request_cache_without_partition_refuses_untrusted_writes_and_prefixes(self) -> None: source = self.trusted_main_cache() key = "{{ hashFiles('Cargo.lock') }}" From d5e97b77e4257b410213812e2e9d7ff1db4bbede Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Sun, 1 Nov 2026 22:07:30 +0000 Subject: [PATCH 2/4] Construct rejected workflow fixtures before exception assertions --- tests/test_qualification_policy.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_qualification_policy.py b/tests/test_qualification_policy.py index 3f15cc9..01b4989 100644 --- a/tests/test_qualification_policy.py +++ b/tests/test_qualification_policy.py @@ -439,10 +439,11 @@ def test_cache_restore_guard_rejects_fork_and_main_ref_bypasses(self) -> None: "!github.event.pull_request.head.repo.full_name != github.repository", "github.event_name == 'workflow_dispatch' || github.ref == 'refs/heads/main'", ): + candidate = source.replace(self.TRUSTED_READ, condition) with self.subTest(condition=condition), self.assertRaisesRegex( PolicyError, "cache restore.*external fork" ): - self.scan(source.replace(self.TRUSTED_READ, condition)) + self.scan(candidate) def test_cache_restore_guards_accept_protected_and_same_repository_contexts(self) -> None: source = self.trusted_main_cache() @@ -465,8 +466,9 @@ def test_cache_restore_can_inherit_the_job_non_fork_guard(self) -> None: source = source.replace(f" if: ${{{{ {self.TRUSTED_READ} }}}}\n", "") source = source.replace(" runs-on:", f" if: ${{{{ {self.TRUSTED_READ} }}}}\n runs-on:") self.scan(source) + candidate = source.replace(self.TRUSTED_READ, "github.ref == 'refs/heads/main'") with self.assertRaisesRegex(PolicyError, "cache restore.*external fork"): - self.scan(source.replace(self.TRUSTED_READ, "github.ref == 'refs/heads/main'")) + self.scan(candidate) def test_pull_request_cache_without_partition_refuses_untrusted_writes_and_prefixes(self) -> None: source = self.trusted_main_cache() From 4a006513a78d4f27999e9531260cea6c72b77c2c Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Sun, 1 Nov 2026 23:07:53 +0000 Subject: [PATCH 3/4] Reject unsupported literals when proving fork cache trust --- scripts/qualification_policy.py | 10 ++++++---- tests/test_qualification_policy.py | 3 +++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/scripts/qualification_policy.py b/scripts/qualification_policy.py index ae2002b..1e1a563 100644 --- a/scripts/qualification_policy.py +++ b/scripts/qualification_policy.py @@ -1323,20 +1323,22 @@ def _condition_truth_for_external_fork(expression: str) -> tuple[bool, bool]: operand = expression[1:].strip() # ! binds before comparison in Actions expressions. Only invert a # grouped expression or literal, never an ungrouped comparison. - if operand.casefold() not in {"true", "false"} and ( + if operand.lower() not in {"true", "false"} and ( not operand.startswith("(") or _strip_balanced_parentheses(operand) == operand ): return True, True can_be_true, can_be_false = _condition_truth_for_external_fork(operand) return can_be_false, can_be_true - if expression.casefold() in {"true", "false"}: - value = expression.casefold() == "true" + if expression.lower() in {"true", "false"}: + value = expression.lower() == "true" return value, not value event = re.fullmatch(r"github\.event_name\s*(==|!=)\s*(['\"])(.*?)\2", expression) if event is not None: operator, _quote, expected = event.groups() - equality = expected.casefold() == "pull_request" + if not expected.isascii(): + return True, True + equality = expected.lower() == "pull_request" value = equality if operator == "==" else not equality return value, not value repository = re.fullmatch( diff --git a/tests/test_qualification_policy.py b/tests/test_qualification_policy.py index 01b4989..5fad7e5 100644 --- a/tests/test_qualification_policy.py +++ b/tests/test_qualification_policy.py @@ -438,6 +438,9 @@ def test_cache_restore_guard_rejects_fork_and_main_ref_bypasses(self) -> None: "!github.event_name != 'push'", "!github.event.pull_request.head.repo.full_name != github.repository", "github.event_name == 'workflow_dispatch' || github.ref == 'refs/heads/main'", + "github.event_name != 'pull_request'", + "!(github.event_name == 'pull_request')", + "falſe", ): candidate = source.replace(self.TRUSTED_READ, condition) with self.subTest(condition=condition), self.assertRaisesRegex( From a948e64bbab36b12bcb2524e23e1d4d7cfbcdc6e Mon Sep 17 00:00:00 2001 From: Richard McDaniel <1130888+rmcdaniel@users.noreply.github.com> Date: Mon, 2 Nov 2026 01:53:33 +0000 Subject: [PATCH 4/4] Reuse condition negation helper in fork policy evaluation --- scripts/qualification_policy.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/qualification_policy.py b/scripts/qualification_policy.py index c52c50a..37ffd45 100644 --- a/scripts/qualification_policy.py +++ b/scripts/qualification_policy.py @@ -1339,9 +1339,7 @@ def _condition_truth_for_external_fork(expression: str) -> tuple[bool, bool]: operand = expression[1:].strip() # ! binds before comparison in Actions expressions. Only invert a # grouped expression or literal, never an ungrouped comparison. - if operand.lower() not in {"true", "false"} and ( - not operand.startswith("(") or _strip_balanced_parentheses(operand) == operand - ): + if not _supports_condition_negation(operand): return True, True can_be_true, can_be_false = _condition_truth_for_external_fork(operand) return can_be_false, can_be_true