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 a6c5853..37ffd45 100644 --- a/scripts/qualification_policy.py +++ b/scripts/qualification_policy.py @@ -1319,6 +1319,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 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 + 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() + 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( + 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(): @@ -1702,11 +1764,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 01fe6a7..6983ef9 100644 --- a/tests/test_qualification_policy.py +++ b/tests/test_qualification_policy.py @@ -423,12 +423,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 @@ -448,6 +450,71 @@ 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'", + "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( + PolicyError, "cache restore.*external fork" + ): + self.scan(candidate) + + 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) + candidate = source.replace(self.TRUSTED_READ, "github.ref == 'refs/heads/main'") + with self.assertRaisesRegex(PolicyError, "cache restore.*external fork"): + self.scan(candidate) + def test_pull_request_cache_without_partition_refuses_untrusted_writes_and_prefixes(self) -> None: source = self.trusted_main_cache() key = "{{ hashFiles('Cargo.lock') }}"