Skip to content

CI gates are executed from the PR checkout, so a PR can neuter the check that certifies it #137

Description

@mbeacom

The gap

Every CI gate in this repository is executed from the pull request's own merge checkout, so a pull request can neuter the check that is supposed to certify it and still produce a green required status.

Raised by review on #133 against the new dco job, but it is not specific to DCO. The same holds for check-deps, audit-gate, check-freeze-hashes, check-doc-cli-versions, check-changelog, the schema emit-parity gate, the packages/ci/dist bundle diff, and adr check in self-dogfood.

Measured, not assumed

pull_request workflows run the PR's ci.yml, not main's. PR #98's clean-clone-builds job executed steps named Typecheck (network denied), Build (network denied), and Verify publishable package tarballs (network denied) — steps that exist only on that branch. main's ci.yml contains zero occurrences of network-denied:

$ gh api repos/mbeacom/adrkit/actions/runs/31604519629/jobs \
    --jq '.jobs[] | select(.name=="clean-clone-builds") | .steps[] | select(.name|test("network denied")) | .name'
Typecheck (network denied)
Build (network denied)
Verify publishable package tarballs (network denied)

$ grep -c 'network-denied' .github/workflows/ci.yml   # on main
0

This is why the partial mitigation does not work. Moving a script to a trusted base revision (git show origin/main:scripts/check-dco.ts) leaves the workflow step that invokes it under the pull request's control, so the same change simply edits the step. A control that looks like a control and is not one is worse than a documented gap — see ADR-0016.

What actually holds the line today

Not review. The main ruleset (19149458) carries only deletion, non_fast_forward, and required_status_checksthere is no pull_request rule, so no review is required. CODEOWNERS gates /docs/adr/, /schema/, LICENSE, NOTICE, and CONTRIBUTING.md, but not /scripts/ or /.github/workflows/.

What remains is that merging requires write access, and a diff that disables a gate is visible to whoever merges. That is a real control, but it is attention, which ADR-0016 Option B already argues is not the constraint.

Why it matters more for DCO than for the rest

A neutered check-deps yields a bad dependency edge, fixable in a follow-up. A neutered DCO check yields unsigned commits in the permanent history of an Apache-2.0 project — and ADR-0006 calls licensing "the most irreversible decision in the project." The blast radius is asymmetric even though the mechanism is shared.

Options

  1. Required review on the main ruleset, plus CODEOWNERS entries for /scripts/ and /.github/workflows/. Cheapest, and it upgrades the control from "whoever merges happens to look" to "a review is required". Does not make the gate tamper-proof.
  2. A pinned external action for the gates that matter, referenced by immutable SHA. Still invoked from a PR-controlled workflow, so it only closes the script half.
  3. Organization-level required workflows, which run outside the PR's control. Not available to a personal-namespace repository — and per ADR-0006 the namespace is deliberately personal.
  4. A gate that asserts the gates are unmodified relative to base. Self-referential, and it blocks every legitimate change to a check (including ci(dco): enforce DCO sign-off as a required check #133 itself).
  5. Document the property and accept it, on the grounds that merge access is the real boundary.

Option 1 looks like the honest first move; it is the only one that changes anything without pretending to close the hole.

Done when

  • A decision is recorded — this touches the ADR-0007 IP boundary and ADR-0006's provenance claim, so it likely wants an ADR rather than a silent settings change
  • Whatever control is chosen is observed working, per ADR-0016
  • scripts/check-dco.ts drops its "known limitation" note if the limitation stops being true

Found in review of #133 (DCO enforcement, ADR-0006 action item 2).

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestgithub_actionsPull requests that update GitHub Actions code

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions