git-guard silently weakens protection on mistyped config (on_protected "deny", non-array protected_branches) #38

New issue

Closed

opened 2026-07-23 14:23:46 +00:00 by crenshawdev · 0 comments

crenshawdev commented

2026-07-23 14:23:46 +00:00

(Migrated from github.com)

Severity: high · Area: git-guard · surfaced in the post-v1.2.0 cross-model review sweep

The protected-branch hook reads two config values and trusts both without validation, so a plausible typo silently makes the rail weaker than configured. There is a config validate command, but git-guard reads the raw merged config, not a validated one.

1. `on_protected: "deny"` silently degrades to a soft prompt

The code recognizes only "refuse" as the value for a hard block; every other value (including "deny") falls through to a soft "ask".

cadence-core/bin/git-guard.mjs:132: const onProtected = git.on_protected || 'ask';
git-guard.mjs:139: decide(onProtected === 'refuse' ? 'deny' : 'ask', ...) — anything but refuse maps to ask.

The trap: the file's own header comment (git-guard.mjs:19-20) documents the setting as ask (default) | refuse ("deny") | allow, which reads like deny is a valid alias for the hard block. A user who sets on_protected: "deny" expecting commits to main to be blocked instead gets only a prompt, with no signal that the value was unrecognized.

2. A non-array `protected_branches` silently guards the wrong branches

git-guard.mjs:128-129: Array.isArray(git.protected_branches) ? git.protected_branches : ['main','master'].

If a user writes "protected_branches": "release" (a string rather than a list), the guard falls back to the built-in ['main','master']. The branch the user meant to protect (release) is unguarded, and branches they did not configure are guarded — no error surfaced. (The same non-array fallback exists in git-publish.mjs.)

Fix direction

Validate the specific keys git-guard depends on before acting on them: enum-check on_protected (and either accept deny as an alias for refuse or surface an unrecognized value), and array-check protected_branches with a surfaced fallback rather than a silent one. Fix the header comment at :19-20 so the documented value and the accepted value agree. Both manifestations share one root and one fix.

**Severity:** high · **Area:** git-guard · surfaced in the post-v1.2.0 cross-model review sweep The protected-branch hook reads two config values and trusts both without validation, so a plausible typo silently makes the rail weaker than configured. There is a `config validate` command, but `git-guard` reads the raw merged config, not a validated one. ## 1. `on_protected: "deny"` silently degrades to a soft prompt The code recognizes only `"refuse"` as the value for a hard block; every other value (including `"deny"`) falls through to a soft `"ask"`. - `cadence-core/bin/git-guard.mjs:132`: `const onProtected = git.on_protected || 'ask';` - `git-guard.mjs:139`: `decide(onProtected === 'refuse' ? 'deny' : 'ask', ...)` — anything but `refuse` maps to `ask`. The trap: the file's own header comment (`git-guard.mjs:19-20`) documents the setting as `ask (default) | refuse ("deny") | allow`, which reads like `deny` is a valid alias for the hard block. A user who sets `on_protected: "deny"` expecting commits to `main` to be blocked instead gets only a prompt, with no signal that the value was unrecognized. ## 2. A non-array `protected_branches` silently guards the wrong branches - `git-guard.mjs:128-129`: `Array.isArray(git.protected_branches) ? git.protected_branches : ['main','master']`. If a user writes `"protected_branches": "release"` (a string rather than a list), the guard falls back to the built-in `['main','master']`. The branch the user meant to protect (`release`) is unguarded, and branches they did not configure are guarded — no error surfaced. (The same non-array fallback exists in `git-publish.mjs`.) ## Fix direction Validate the specific keys `git-guard` depends on before acting on them: enum-check `on_protected` (and either accept `deny` as an alias for `refuse` or surface an unrecognized value), and array-check `protected_branches` with a surfaced fallback rather than a silent one. Fix the header comment at `:19-20` so the documented value and the accepted value agree. Both manifestations share one root and one fix.