Repository navigation
security(ci): the required-context shim trusts a path filter the pull request itself can rewrite #14408
Description
Summary
The required-context shim pattern decides whether a test suite applies by reading a path-filter file out of the pull request's own checkout. That file is ordinary repo content, so the pull request being gated controls the rule that decides whether it gets gated.
This is not hypothetical for one filter and theoretical for the other — it is the same mechanism in both, and one of them already gates a required context in production.
Mechanism
Both pairs work identically:
| Filter | Real suite | Shim | Context | Required today |
|---|---|---|---|---|
.github/filters/frontend-paths.yml |
frontend-test.yml job unit-tests |
frontend-required-context.yml job unit-tests-skip |
Unit & Integration Tests |
yes |
.github/filters/python-paths.yml |
ci.yml job python-suite |
python-required-context.yml job python-suite |
python-suite |
not yet (#14353) |
Both workflows trigger on pull_request (not pull_request_target), and dorny/paths-filter is pointed at the filter path, so it resolves against the PR's merge commit.
The bypass follows from the complement condition:
- A pull request edits a real source file and narrows the filter so that file's path no longer matches.
- The real workflow still triggers — its trigger-level
paths:is GitHub-side config and is not tamperable (and after ci: the backend Python suite gates nothing — python-suite is not a required check #14353,ci.ymlhas nopull_request.pathsat all). - Its
changesjob evaluates the tampered filter, computesfalse, and the real suite skips. - The shim evaluates the same tampered filter, computes
false, so its complementif:is true — it runs and reports the context green.
Net: a real source change merges with a fabricated green for a suite that never ran. The gate reports verified for something it did not verify, which is worse than having no gate, because the green is what a reviewer trusts.
Why the obvious fix does not work
Adding a guard test that pins the filter's patterns against a floor does not close this, because those tests live in repo_tests/ and run inside the very job the tampered filter skips. The check would be absent exactly when it is needed — an empty result reading as a clean result.
A floor check inside the shim is also insufficient on its own: the shim is repo content too, so the same pull request can narrow the floor list alongside the filter.
What actually holds today
Human review of .github/. CODEOWNERS covers the path and the ruleset requires review, so exploiting this requires a reviewer to approve a diff that visibly narrows a path filter next to a source change. That is a real control, and it is the only one — worth stating plainly rather than assuming the automation covers it.
What is needed
The decision is where the trust anchor should live. Options, roughly in increasing strength:
- Make tampering loud rather than silent. A step in the shim that recomputes the decision from a floor pattern set and fails the context instead of publishing green when the filter no longer covers the obvious source paths. Defeatable by editing two files, but it converts a silent green into a visible red for the accidental-narrowing case, which is the likely one.
- Move the decision out of PR-controlled content. Compute the changed-path decision from the diff directly in the shim with an inline pattern set that is protected by the same review gate, or have the shim refuse to publish when the filter file itself is among the changed paths.
- Accept and document. Record that
.github/filters/*.ymlis branch-protection-adjacent configuration whose review is the control, and make that explicit where the files live.
Any of these is legitimate. Shipping a required gate while leaving this undocumented is the part that should not stand.
Acceptance criteria
- A decision is recorded on which anchor applies
- The chosen control covers both filter files, not just the python one
- A pull request that narrows a filter while changing a matching source file does not produce a green required context — asserted, not argued
- The residual trust boundary (whatever it ends up being) is documented in the filter files themselves
Activity