You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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'sci.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:
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_checks — there 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
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.
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.
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.
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).
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
dcojob, but it is not specific to DCO. The same holds forcheck-deps,audit-gate,check-freeze-hashes,check-doc-cli-versions,check-changelog, the schema emit-parity gate, thepackages/ci/distbundle diff, andadr checkinself-dogfood.Measured, not assumed
pull_requestworkflows run the PR'sci.yml, notmain's. PR #98'sclean-clone-buildsjob executed steps namedTypecheck (network denied),Build (network denied), andVerify publishable package tarballs (network denied)— steps that exist only on that branch.main'sci.ymlcontains zero occurrences ofnetwork-denied: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
mainruleset (19149458) carries onlydeletion,non_fast_forward, andrequired_status_checks— there is nopull_requestrule, so no review is required.CODEOWNERSgates/docs/adr/,/schema/,LICENSE,NOTICE, andCONTRIBUTING.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-depsyields 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
mainruleset, plusCODEOWNERSentries 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.Option 1 looks like the honest first move; it is the only one that changes anything without pretending to close the hole.
Done when
scripts/check-dco.tsdrops its "known limitation" note if the limitation stops being trueFound in review of #133 (DCO enforcement, ADR-0006 action item 2).