Skip to content

security(ci): the required-context shim trusts a path filter the pull request itself can rewrite #14408

Open

Description

@mrveiss

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:

  1. A pull request edits a real source file and narrows the filter so that file's path no longer matches.
  2. 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.yml has no pull_request.paths at all).
  3. Its changes job evaluates the tampered filter, computes false, and the real suite skips.
  4. The shim evaluates the same tampered filter, computes false, so its complement if: 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/*.yml is 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

Refs #14353, #13405, #13388

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions