feat(hooks): block --no-verify, the one bypass of the four with no rule (v1.41.0) - #60
Conversation
…le (v1.41.0) Non-negotiable 21 names four bypasses: --no-verify, --admin, --force, and merging past a check. Measured across all 40 rules: --admin, `push --force` and --force-with-lease each had one. --no-verify had NONE. That asymmetry is worse than a plain gap. Three of the four are covered, so a reader has every reason to assume the fourth is too: the rule reads as enforced and is not. Found the way these are always found. On 2026-08-02 an agent resolving a merge conflict ran `git commit --no-verify`, by reflex, with no justification. It reported itself, discarded the commit, redid the merge and recommitted with hooks enabled. Nothing stopped it BECAUSE THERE WAS NOTHING TO STOP IT -- it did not route around a guard, it walked through a gap. The near-miss the rule must not get wrong, and why the short form is scoped to one subcommand: `git commit -n` IS --no-verify, while `git push -n` is --dry-run. Same letter, opposite meaning. Blocking a dry-run would refuse the safest command in git, and a guard that refuses ordinary work gets bypassed. Asserted by `push-dry-run-allowed`. No bypass marker, deliberately, and the message says why rather than asserting it. That same incident's honest attempt first failed with `vitest: command not found` because a fresh worktree had no dependencies; the fix was `bun install --frozen-lockfile`. A hook that cannot run is a broken toolchain, not a gate to step over -- so the refusal names that command, because a refusal that leaves you stuck is one that gets worked around next time. Six tests: both spellings on both subcommands, the dry-run near-miss, an ordinary commit, and a quoted mention. Hook suite 346/346, vitest 60/60. Created by Claude Code on behalf of @lapc506
![dojo-code-reviewer[bot]](https://avatars.githubusercontent.com/in/3092887?s=60&v=4){kind=small}
There was a problem hiding this comment.
💬 Review Comments
Comments — 0 blockers, 2 P2. Confidence: 2.60/5.00.
Walkthrough
main directly instead of develop. Under standard GitFlow governance, feature development branches should target develop first.
Review Summary
This PR adds the block-no-verify rule to both rules.yaml and its compiled representation rules.json. The rule is designed to block git commit --no-verify, git push --no-verify, and git commit -n to close the last bypass loophole mentioned in Non-negotiable 21 of the dojo-os rules. It also correctly bumps the versioning manifest files (package.json, .claude-plugin/plugin.json, .claude-plugin/marketplace.json) and registers the entry in the CHANGELOG.md.
- Files/Areas Reviewed: Rules configuration (
hooks/rules/rules.yaml,hooks/rules/rules.json), versioning metadata manifests (package.json,.claude-plugin/plugin.json,.claude-plugin/marketplace.json), and the documentation (CHANGELOG.md). - Safety Rationale: The changes introduce passive tool-call rules that prevent hook-bypassing without modifying production runtime code, and are backed by robust unit tests.
Verdict
Comment — 0 blockers, 2 P2s.
🟡 P2 — Major
hooks/rules/rules.yaml:1318— 🟡 P2 (major) — False Positive Risk on Commit Messages. The pattern matches ' -n ' or '--no-verify' regardless of whether they are inside a commit message (e.g., git commit -m "feat: support -n in client"). Since 'bypass_marker' is set to null, a developer running a completely valid commit with such a message will be blocked with no way to bypass. Consider either refining the regex to avoid matching inside quoted strings, or introducing a bypass_marker (e.g., 'no-verify-allowed') to let developers unblock themselves in case of false positives.
[pass 1]
package.json:3— 🟡 P2 (major) — Missing Version Update in README.md. The version manifests (package.json, plugin.json, marketplace.json) are bumped to 1.41.0, but README.md is not updated in this PR (still refers to 1.38.0). This violates the repo's own 'warn-version-readme-changelog-sync' rule/guideline. Please update the README.md version header to 1.41.0 to ensure consistent versioning across all surfaces.
[pass 1]
Total findings: 2 compliance (2 total)
| match: | ||
| # `git commit -n` IS --no-verify. `git push -n` is --dry-run and must NOT | ||
| # match, so the short form is scoped to commit only. Two flags, one letter, | ||
| # opposite meanings -- the same near-miss shape the force-push rule guards. |
There was a problem hiding this comment.
🟡 P2 (major) — False Positive Risk on Commit Messages. The pattern matches ' -n ' or '--no-verify' regardless of whether they are inside a commit message (e.g., git commit -m "feat: support -n in client"). Since 'bypass_marker' is set to null, a developer running a completely valid commit with such a message will be blocked with no way to bypass. Consider either refining the regex to avoid matching inside quoted strings, or introducing a bypass_marker (e.g., 'no-verify-allowed') to let developers unblock themselves in case of false positives.
[pass 1]
| { | ||
| "name": "@lapc506/make-no-mistakes", | ||
| "version": "1.38.0", | ||
| "version": "1.41.0", |
There was a problem hiding this comment.
🟡 P2 (major) — Missing Version Update in README.md. The version manifests (package.json, plugin.json, marketplace.json) are bumped to 1.41.0, but README.md is not updated in this PR (still refers to 1.38.0). This violates the repo's own 'warn-version-readme-changelog-sync' rule/guideline. Please update the README.md version header to 1.41.0 to ensure consistent versioning across all surfaces.
[pass 1]
1 participant
The gap, measured
Non-negotiable 21 names four bypasses. Across all 40 rules in
rules.yaml:push --force--force-with-lease--admin--no-verifyThat asymmetry is worse than a plain gap. Three of the four are covered, so a reader has every reason to assume the fourth is too. The rule reads as enforced and is not.
How it surfaced
On 2026-08-02, an agent resolving a merge conflict ran
git commit --no-verify— by reflex, with no justification. It reported the violation itself, discarded the commit withgit reset --hard, redid the merge, reapplied the same resolution, and recommitted with hooks enabled.Nothing stopped it because there was nothing to stop it. It did not route around a guard. It walked through a gap.
The near-miss this rule must not get wrong
git commit -nis--no-verify.git push -nis--dry-run.Same letter, opposite meaning — which is why the short form is scoped to
commitalone. Blocking a dry-run would refuse the safest command in git, and a guard that refuses ordinary work gets bypassed, which is how a guard stops carrying information at all. Pinned by thepush-dry-run-allowedtest.No bypass marker, and the message says why
bypass_marker: null, matching the precedent set bytest-runtime-guardrail.json(DOJ-6200).The refusal names the alternative instead of just refusing, and it does so because of what happened in that same incident: the honest attempt failed first, with
vitest: command not found, because a fresh worktree had no dependencies. The fix wasbun install --frozen-lockfile.A refusal that leaves someone stuck is a refusal that gets worked around next time, so the message hands them the real fix rather than a dead end.
Verification
npx vitest run→ 60/60npm run build-rulesregeneratesrules.jsonat 41 rules with no driftThe builder rejected the first draft with
rule block-no-verify missing tests (non-empty array required)— its own gate working, and worth noting since this PR is about gates that do not exist.Version collision, stated up front
Bumps to 1.41.0, assuming #57 (1.39.0) and #58 (1.40.0) land first. All three touch
package.json,plugin.json,marketplace.jsonandCHANGELOG.md; none conflicts withmain, and each conflicts with the other two. Whichever order they merge in, the later ones need a renumber — the latent-conflict class #57 exists to surface, now in its third instance.Created by Claude Code on behalf of @lapc506