git-guard: catch destructive-local ops (reset --hard, clean -f, checkout ., branch -D) #25

New issue

Closed

opened 2026-07-19 00:25:18 +00:00 by crenshawdev · 2 comments

crenshawdev commented

2026-07-19 00:25:18 +00:00

(Migrated from github.com)

Source: skills-analysis session, 2026-07-18 (comparison against Matt Pocock's git-guardrails-claude-code).

Cadence's git-guard.mjs PreToolUse hook guards push (always ask) and commit on a protected branch (git.on_protected). It does not guard destructive-local commands that can irreversibly wipe uncommitted work: git reset --hard, git clean -f/-fd, git checkout . / git restore ., git branch -D.

This is the one class of git command that destroys work with no recovery, and it's currently unguarded.

Do NOT adopt Matt's skill — his is a bash+jq substring-grep blocklist that false-triggers on echo "git push" and can't tell a dangerous flag combo from a safe one. Cadence's parser is strictly better. This is an extension of the existing git-guard.mjs, not a replacement.

Approach:

Extend gitSubcommands() (or add a sibling check) to recognize the destructive set, flag-aware:
- reset only when --hard is present (soft/mixed reset is fine)
- clean only with -f/-fd
- checkout / restore only when the pathspec is . or --
- branch -D
New config key git.on_destructive: ask | deny | allow (default ask), mirroring on_protected.
Same philosophy as the rest of the rails: ask, don't hard-block; the harness decides.
Stays scoped to Cadence projects (existing planningRoot walk-up); silent elsewhere.

Benefit: the "oh no" git commands stop and ask before running, so a stray reset --hard can't silently vaporize uncommitted work.

Scoped for v1.2.0.

**Source:** skills-analysis session, 2026-07-18 (comparison against Matt Pocock's `git-guardrails-claude-code`). Cadence's `git-guard.mjs` PreToolUse hook guards **push** (always ask) and **commit on a protected branch** (`git.on_protected`). It does **not** guard destructive-local commands that can irreversibly wipe uncommitted work: `git reset --hard`, `git clean -f`/`-fd`, `git checkout .` / `git restore .`, `git branch -D`. This is the one class of git command that destroys work with no recovery, and it's currently unguarded. **Do NOT adopt Matt's skill** — his is a bash+jq substring-grep blocklist that false-triggers on `echo "git push"` and can't tell a dangerous flag combo from a safe one. Cadence's parser is strictly better. This is an **extension of the existing `git-guard.mjs`**, not a replacement. **Approach:** - Extend `gitSubcommands()` (or add a sibling check) to recognize the destructive set, **flag-aware**: - `reset` only when `--hard` is present (soft/mixed reset is fine) - `clean` only with `-f`/`-fd` - `checkout` / `restore` only when the pathspec is `.` or `--` - `branch -D` - New config key `git.on_destructive: ask | deny | allow` (default `ask`), mirroring `on_protected`. - Same philosophy as the rest of the rails: **ask, don't hard-block**; the harness decides. - Stays scoped to Cadence projects (existing `planningRoot` walk-up); silent elsewhere. **Benefit:** the "oh no" git commands stop and ask before running, so a stray `reset --hard` can't silently vaporize uncommitted work. _Scoped for v1.2.0._

john commented

2026-07-30 05:13:37 +00:00

Owner

Picked into the cycle after v2.0.0, as the requirement TOK-02, and it changed that requirement's shape rather than riding along with it.

TOK-02 was scoped as a straight rip-out: delete lib/shell-tokens.mjs, its test file, and references/git.md's rail-3 grammar and out-of-grammar table, and go back to a small regex that asks on a plain git push. The motivation was proportionality. Three review rounds in a row kept generating rail-3 evasion findings (&>, !(...), brace-in-subcommand), against a rail whose own documentation concedes it is "a detection widener, not a security boundary" and whose adversary is the model issuing the command.

This issue is why the rip-out is wrong as scoped. It needs precisely the parsing the rip-out deletes: reset only on --hard, clean only on -f/-fd, checkout/restore only when the pathspec is . or --. A twenty-line regex cannot make those distinctions without becoming the substring blocklist this issue explicitly says not to adopt.

And it guards the stronger case. Rail 3 guards push, where a miss costs a recoverable remote commit. This guards the one class that destroys uncommitted work with no recovery. The parser is currently spent entirely on the weaker case, which is the actual proportionality complaint, stated more precisely than the rip-out stated it.

So TOK-02 becomes a redirection, not a deletion. Rail 3's evasion grammar and out-of-grammar table go. What remains is a small flag-aware parser, and it is spent here: git.on_destructive (ask | deny | allow, default ask) firing on reset --hard, clean -f/-fd, checkout/restore with a . or -- pathspec, and branch -D.

Two things from this issue's own text are kept as written: ask rather than hard-block, and extend git-guard.mjs rather than adopt an external blocklist. One thing changes: the framing here is "an extension of the existing parser". It is now a redirection of it, because the surface it was serving is being removed in the same requirement.

Superseding TOK-01's rail-3 claim rather than extending it, which is also why the cycle ships as a minor rather than a patch.

Picked into the cycle after v2.0.0, as the requirement **TOK-02**, and it changed that requirement's shape rather than riding along with it. TOK-02 was scoped as a straight rip-out: delete `lib/shell-tokens.mjs`, its test file, and `references/git.md`'s rail-3 grammar and out-of-grammar table, and go back to a small regex that asks on a plain `git push`. The motivation was proportionality. Three review rounds in a row kept generating rail-3 evasion findings (`&>`, `!(...)`, brace-in-subcommand), against a rail whose own documentation concedes it is "a detection widener, not a security boundary" and whose adversary is the model issuing the command. This issue is why the rip-out is wrong as scoped. It needs precisely the parsing the rip-out deletes: `reset` only on `--hard`, `clean` only on `-f`/`-fd`, `checkout`/`restore` only when the pathspec is `.` or `--`. A twenty-line regex cannot make those distinctions without becoming the substring blocklist this issue explicitly says not to adopt. And it guards the stronger case. Rail 3 guards `push`, where a miss costs a recoverable remote commit. This guards the one class that destroys uncommitted work with no recovery. The parser is currently spent entirely on the weaker case, which is the actual proportionality complaint, stated more precisely than the rip-out stated it. **So TOK-02 becomes a redirection, not a deletion.** Rail 3's evasion grammar and out-of-grammar table go. What remains is a small flag-aware parser, and it is spent here: `git.on_destructive` (`ask` | `deny` | `allow`, default `ask`) firing on `reset --hard`, `clean -f`/`-fd`, `checkout`/`restore` with a `.` or `--` pathspec, and `branch -D`. Two things from this issue's own text are kept as written: ask rather than hard-block, and extend `git-guard.mjs` rather than adopt an external blocklist. One thing changes: the framing here is "an extension of the existing parser". It is now a redirection of it, because the surface it was serving is being removed in the same requirement. Superseding TOK-01's rail-3 claim rather than extending it, which is also why the cycle ships as a minor rather than a patch.

john modified the milestone from v1.2.0-rc.1 to (deleted)

2026-07-30 05:13:38 +00:00

john modified the milestone from (deleted) to v2.1.0

2026-07-30 05:15:36 +00:00

john commented

2026-08-05 17:08:57 +00:00

Owner

Closing as decided against, not as done.

The destructive-local rail was built and then deliberately removed by TOK-02 in v2.2.0. cadence-core/bin/lib/destructive-git.mjs and lib/shell-tokens.mjs are deleted along with git.on_destructive from the config schema, and references/git.md's rail-3 grammar went with them — 2,251 lines in total.

The reason was measurement, not principle. The scan was O(K x N) in memory: 3.1 GB at 224 KB of input, V8 abort at 280 KB, inside a PreToolUse hook that runs on every Bash call and fails OPEN. So a long enough command line switched the guard off entirely and let the operation inside it run unprompted — the guard was least reliable exactly when the command was most unusual. Alongside that, three consecutive blocking review panels each found new escape shapes behind bash -c, $(...) and aliases, and the reader still went silent on git switch -f main.

What stands today is a ~30-line anchored reader (lib/git-segments.mjs) guarding push and commit-on-protected only, with the missed shapes pinned as explicit test rows rather than left as an absence. It is a detection widener, not a security boundary, and it says so.

Re-open if the trade is worth revisiting — but it should be re-argued from the performance finding rather than resumed as filed.

Verified against the tree at the v2.3.0 close: destructive-git.mjs absent, no on_destructive in the schema or cadence-core/bin.

Closing as decided against, not as done. The destructive-local rail was built and then deliberately removed by TOK-02 in v2.2.0. `cadence-core/bin/lib/destructive-git.mjs` and `lib/shell-tokens.mjs` are deleted along with `git.on_destructive` from the config schema, and `references/git.md`'s rail-3 grammar went with them — 2,251 lines in total. The reason was measurement, not principle. The scan was O(K x N) in memory: 3.1 GB at 224 KB of input, V8 abort at 280 KB, inside a PreToolUse hook that runs on every Bash call and fails OPEN. So a long enough command line switched the guard off entirely and let the operation inside it run unprompted — the guard was least reliable exactly when the command was most unusual. Alongside that, three consecutive blocking review panels each found new escape shapes behind `bash -c`, `$(...)` and aliases, and the reader still went silent on `git switch -f main`. What stands today is a ~30-line anchored reader (`lib/git-segments.mjs`) guarding push and commit-on-protected only, with the missed shapes pinned as explicit test rows rather than left as an absence. It is a detection widener, not a security boundary, and it says so. Re-open if the trade is worth revisiting — but it should be re-argued from the performance finding rather than resumed as filed. Verified against the tree at the v2.3.0 close: `destructive-git.mjs` absent, no `on_destructive` in the schema or `cadence-core/bin`.

john closed this issue

2026-08-05 17:08:59 +00:00