Repository navigation
fix(security): the org's signed-commits ruleset is active but enforces nothing #132
Description
🤖 Generated by the Agentic Engineer
Delivered and verified (2026-09-08)
PR #190 merged as 7130742c555d4a225414043a47dd2c24df7c4d7c and shipped in tag v1.23.6. The signed artifact reached the existing controller through normal Flux reconciliation. Organization ruleset 5397812, Require signed commits, is now Disabled, retains its original identity, empty ref coverage, signature rule and empty bypass list, and permits only Observe and Update. GitHub settings and the healthy controller independently confirm the result.
The complete before/after effective-rules responses on main for .github, actions, platform, monorepo, and ksail are identical: all existing pull-request, linear-history and required-status-check protections remain. There are still zero native signature rules in every case. The September 6 sampled default-branch commits all reported verified: true, reason: valid with committer web-flow. That is historical signed-commit outcome evidence, not proof of native signature enforcement.
The control-integrity defect is resolved: the retained rule no longer advertises active protection that applies nowhere.
The old one-click enforcement recommendation missed a compatibility constraint
GitHub's current signed-commit rules documentation says squash merging into a branch requiring signatures is restricted to the pull-request author. A branch-signing audit alone therefore does not establish compatibility with the engineer merging bot and contributor PRs.
Enabling the rule on ~DEFAULT_BRANCH must not be called a zero-cost change until those merge paths are demonstrated. The earlier audit remains useful historical evidence in the comments, but does not prove this additional condition.
Approved retirement — delivered
The issue's original expected behavior explicitly allowed either enforcement or retirement. The scoped retirement was explicitly approved in the September 8 session. PR #190 retains the already-inert object as a disabled record:
- managementPolicies: ["Observe"]
+ managementPolicies: ["Observe", "Update"]
- enforcement: active
+ enforcement: disabledBefore allowing Update, the implementation backfilled every writable field from the complete observed shape: branch target, original name and external ID, empty ref include/exclude, repository selector ~ALL with no exclusions and protected: false, no bypass actors, and the existing signature rule plus its observed false boolean rule fields. Never permit Create or Delete. This preserves zero effective coverage and does not remove any effective PR, history, or required-check control.
The compensating protection remains the effective pull-request/status/linear-history controls and GitHub-created signed squash commits. The disabled retained record makes the advertised posture honest; it does not implement native signature enforcement.
Historical approval rejection — now resolved
Automatic approval review initially rejected even the local configuration edit because the specific security-policy change lacked explicit authorization. The user has now approved this exact retained-disabled, Observe-and-Update-only proposal. The actual session question and answer were verified, and the independently reviewed patch was applied successfully in the approval-bearing session context. The signed change has now completed review, release and production verification.
Acceptance criteria
- Obtain explicit approval for the scoped retirement while preserving all effective branch protections.
- Complete current-head review and CI, and deliver the fully backfilled disabled resource declaratively.
- Verify the released artifact and the existing live ruleset's disabled state, retained identity and healthy controller status.
- Compare complete effective branch protections with the saved baseline and record the final disposition here.
All ten repository tests, the installed-CRD schema check and the no-deletion/release guards passed. The change retains all 99 rendered identities and changes only the signing-rule record; 16 unsafe variations are rejected by the new guard. Full-range review found no remaining issues at the final head, and hosted CI passed. Independent signature verification passed for the immutable OCI digest, all 127 packaged source files matched the merged commit byte for byte, and Flux verified and applied that same release. The controller retained the same resource identity and observed the disabled state.
No personal action or delivery work remains for this issue. After disabling the rule, repository-inherited ruleset reads no longer returned it; the authenticated organization settings page and the controller observation supplied the independent disabled-state checks. The complete effective-protection comparison passed for all five repositories.
Metadata
Metadata
Assignees
Labels
Type
Fields
Projects
- StatusShow more project fields✅ Done
Activity
coderabbitai commentedon Aug 2, 2026
🔗 Related PRs
#74 - feat(rulesets): require the CodeRabbit review status check org-wide [merged]
#75 - revert(rulesets): drop the provider-uncreatable CodeRabbit required-check ruleset [merged]
#88 - feat: archived-repo pattern + reusable-workflows archival groundwork [merged]
#127 - fix(repositories): keep org-enforced signoff out of update payloads [merged]
#128 - fix(repositories): stop declaring org-enforced signoff so updates can apply [merged]
📝 Issue Planner
Check the box below or use the
@coderabbitai plancommand to generate an implementation plan and prompts that you can use with your favorite coding assistant.🧪 Issue enrichment is currently in open beta.
You can configure auto-planning by selecting labels in the issue_enrichment configuration.
To disable automatic issue enrichment, add the following to your
.coderabbit.yaml:💬 Have feedback or questions? Drop into our discord!
devantler commentedon Aug 7, 2026
Criterion 4, measured — and it decides the disposition
The open question was whether requiring signatures would start refusing pushes from any lane that
does not sign today. It would. Measured 2026-08-07 on the newest commit of a live branch per lane:
claude/*cursor/*codex/*reason: unsignedchore/template-sync_*(the template-sync App)reason: unsignedSo the two dispositions the issue framed are not equally priced, and neither is quite right:
That is the DevEx tax criterion 4 exists to catch, and it lands on two automation lanes at once.
Recommendation: enforce, scoped to the default branch only. Everything that reaches
mainis aGitHub-created squash commit, which GitHub signs — so on
mainthe rule refuses nothing that happenstoday, closes the stated-vs-actual gap, and makes an unsigned commit on
mainimpossible in future.Branch commits stay unsigned-tolerant, so no lane breaks. That is "the secure path is also the easy
path" rather than a gate to route around.
Fixing the two unsigned lanes is worth doing on its own merits, but it should be its own work and
not a precondition — coupling it here is what would turn a free control into a tax.
The implementation is not a
deploy/edit — and that is the same defect as #141deploy/organization-rulesets/require-signed-commits.yamlcarriesmanagementPolicies: ["Observe"]and an identity-onlyforProvider(name, target, enforcement). Itwrites nothing, so changing the declared ref condition there would be inert — the config would
read as if the rule were scoped while live stayed unchanged. That is exactly the class filed as #141
against
Observe-onlyRepositoryresources; here it applies to the rulesets.Two ways forward, and picking one is the remaining decision:
deploy/records the live shape. Fastest,and consistent with the file's own warning that promoting past
Observeneeds astatus.atProviderbackfill because the round-trip is lossy.
declaratively. Correct long-term, larger, and blocked on the same lossy-round-trip work the
README flags for all ten adopted rulesets.
Criterion 3's control — an unsigned commit refused on a covered scratch branch, a signed one accepted
— should run after whichever path lands, against
mainrather than a scratch branch, since that iswhere the recommended scope applies.
chore: agent-claim #132 nonce=4aab4ae744e7b5f148233dac2a178800
devantler commentedon Aug 20, 2026
Disposition: enforce, scoped to
~DEFAULT_BRANCH— and the root cause is a single empty fieldThe premise re-verified, the cause located, and criterion 4 (the DevEx-tax check) measured. What is
left is one org-level edit, which is maintainer authority.
Root cause — an empty
includelist matches no refsThe ruleset is not misconfigured in some subtle way; its ref condition is simply empty:
Control — the same field on a ruleset that is effective:
That is the whole difference.
activedescribes the ruleset;includedecides whether it everapplies. An empty include matches nothing, so the rule is live and inert at the same time — which is
exactly the stated-versus-actual gap this issue is about.
Premise re-verified (the original evidence was 18 days old)
Re-measured today across 22 non-archived repositories (up from 18):
required_signatureseffective on default branchrequired_linear_historyeffective (positive control)The control matters: an instrument that reported nothing for every rule would make the zero
meaningless. It reports 22/22 for a neighbouring rule, so the zero is a measurement.
Criterion 4 — the lane-signing audit, which is what decides the scope
Sampled 105 commits across recent merged PRs in
monorepo,platformandksail, plus 43 onopen PRs. Unsigned commits exist on branches today, from three distinct paths:
app/ksail-botrelease lane (chore/copilot-plugin-v*)devantler-tech-bot[bot], current through 2026-08-19megalinter-botauto-fix pushes ontoclaude/*branchesclaude/*local commitscodex/*,app/cursor, dependabot/renovateThis is the finding that sets the scope. Enforcing on all branches (
~ALL) would startrefusing pushes from the ksail-bot release lane outright and from MegaLinter's auto-fix, and would
intermittently refuse this engineer's own lane. That is a real DevEx tax, and it is the second half
of good DevEx is easy and secure.
Enforcing on
~DEFAULT_BRANCHcosts nothing: every change lands as a GitHub-created squashcommit, which GitHub signs itself, so the everyday path already satisfies the rule. None of the three
unsigned paths touches the default branch directly.
Why enforce rather than retire
Retiring is honest but throws away a control that is free to switch on.
~DEFAULT_BRANCHmatcheswhat every other org ruleset already uses, removes the false signal, and adds defence in depth behind
the
pull_requestrule — at zero cost to any lane measured above. Retiring would be the right callonly if enforcement were expensive, and it is not.
Deliberately not
~ALL. That is the version with teeth for branch commits, and the audit says itwould break three paths today. If it is ever wanted, the three paths above must be fixed first — that
is a separate piece of work, not a condition to slip into this change.
The remaining step is maintainer authority
Applying it needs
admin:org, which this engineer does not hold (GET /orgs/{org}/rulesets→403 needs admin:org), so it is genuinely out of reach rather than deferred by choice:Then criteria 2 and 3 are verifiable and this engineer can run both arms:
repos/{owner}/{repo}/rules/branches/mainreportsrequired_signaturesfor all 22 repos.accepted — both arms, since a rule that rejects nothing reads identically to a rule that works.
Blocker: devantler-tech org ruleset 5397812 (maintainer authority —
admin:org) | last-verified2026-08-20: not applied (ref_name.include still empty; effective on 0/22).
devantler commentedon Aug 25, 2026
Blast radius for this change — a third lane now fails verification
Flagging an ordering constraint measured today, because enabling
required_signatureswhile itholds would take a write lane offline rather than merely tightening provenance.
devantler-tech/monorepo#3046records that the Cursor cloud lane's newest commit does notverify:
devantler-tech/actions@22ef0a55(2026-08-25, open PRactions#1054) reportsverified=false, reason=unknown_keyundercursor[bot]@users.noreply.github.com. Two earliercommits from that lane verify cleanly under a different address. Every
cursor/*ref acrossmonorepo,actions,platformandksailwas enumerated: exactly one Cursor-authored commitexists since 2026-08-01, and it is the failing one.
So the lanes now show three different outcomes —
valid,bad_email(monorepo#3020, theclaudelane) and
unknown_key(thecursorlane). Turning onrequired_signaturesbefore those areresolved would reject pushes from at least one lane that is actively opening PRs today.
This does not argue against the change — the ruleset enforcing nothing is exactly the defect this
issue is about. It argues for sequencing: resolve
#3046andmonorepo#3020first, then enable,so a security control does not land as an availability incident.
The blocker recorded on this issue (maintainer authority for the org-level ruleset change) is
unchanged and was last verified 2026-08-24; I did not re-measure it this run.
devantler commentedon Sep 2, 2026
Acceptance criterion 4 is now DONE: every lane that pushes commits already signs
This is the criterion the issue itself names as the long pole and as ordinary agent work — the check that decides whether enforcing becomes a DevEx tax. Measured 2026-09-02T02:2xZ.
Answer: enforcing
required_signatureson branch pushes would refuse nothing that any current lane produces. The second half of good DevEx is easy and secure passes.Lane census
480 pull requests across
platform,monorepo,ksail,agent-plugins,actionsandworld-at-ruinresolve to six identities that push branch commits —devantler(both machine-local instances, 214),app/dependabot(176),app/botantler-1(53),app/ksail-bot(17),app/renovate(14),app/cursor(6).github-actions[bot]authored none in that sample; see the residual below.Evidence
app/renovatewidened (2026-08-26 → 09-01)ksail-botsync commit onto a Dependabot branch (ksail#6826head)platformada9e41b)Commits created through the GitHub API report committer
web-flowand carry GitHub's own signature; locally-created ones (devantler,cursoragent) carry the lane's key. Both verifyvalid.Negative control — the instrument discriminates
An all-
trueresult is worthless unless the field can reportfalse, so:ksailcommit history page 50 holds 28 commits withverified: false, dated 2024-06-14 → 2024-10-25, committerrenovate[bot], reasonsunsignedandno_user.That is the same discrimination argument the issue makes for the rules endpoint, and it lands on the most useful lane: Renovate did push unsigned commits historically and no longer does — 14 of 14 of its recent heads verify. So the risk this criterion exists to find is real and is already closed, rather than never having existed.
Residual — what this does not prove
github-actions[bot]authored no PR in the 480 sampled, so if a workflow pushes commits on a path that never opens a PR, that path is uncovered here.Neither residual weakens the conclusion much, because the mechanism is structural: every lane except the two machine-local ones creates commits through the GitHub API, which signs them unconditionally.
Blocker re-verified
required_signaturesis still ABSENT from the effective rules formainon.github,actions,platform,monorepoandksail, whilerequired_linear_historyis PRESENT on all five — so the endpoint still discriminates and the null result still means what it says.What remains
Only the maintainer act: applying the ref condition to the org-level
Require signed commitsruleset (or retiring it). Criterion 4's risk is now measured rather than assumed, so that step is a one-click whose blast radius is known: no current lane breaks. Criteria 2 and 3 — reading the effective-rules endpoint back, and the two-armed accept/refuse control on a scratch branch — are verification I can run immediately after it is applied.chore: agent-claim #132 nonce=804c495492be556a9b7a412c182bdad0