From c4253805b5c57126a4e27c46996a812e12a96665 Mon Sep 17 00:00:00 2001 From: Souptik Chakraborty Date: Wed, 29 Jul 2026 08:36:12 +0530 Subject: [PATCH] docs(gateguard): address review - split full-disable, pin glob semantics CodeRabbit review on #2611, all four findings: - GATEGUARD_DISABLED sat in a table introduced as 'these do not disable the gate'. Moved to its own full-disable section with ECC_GATEGUARD, and corrected the accepted values against ECC_DISABLE_VALUES (0/false/off/disabled/disable - the earlier draft would have implied 'no' works, which it does not). - Documented that a leading **/ compiles to .*/ and so needs a preceding separator: verified by reproducing the hook's glob->regex translation, **/tests/** matches /repo/tests/foo.js but not a bare relative tests/foo.js. Docs now say so and the example carries both forms. Matcher behaviour deliberately unchanged - widening it is a behaviour change, not a docs fix. - Reverse-drift check now compares documented names against the parsed env reads instead of hookSource.includes(), so a name surviving only in a comment or error string no longer satisfies it. - readGateguardEnvNames builds one Set from collected matches instead of mutating via Set#add, per the repo's no-in-place-mutation guideline. --- skills/gateguard/SKILL.md | 35 +++++++++++++++++------ tests/ci/gateguard-env-documented.test.js | 21 +++++++------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/skills/gateguard/SKILL.md b/skills/gateguard/SKILL.md index f244fa667..2c37994a7 100644 --- a/skills/gateguard/SKILL.md +++ b/skills/gateguard/SKILL.md @@ -108,8 +108,9 @@ command after presenting facts never re-triggers the gate. #### Graduated controls -`ECC_GATEGUARD=off` disables the whole gate. The variables below narrow it -instead, so the load-bearing destructive-Bash checks keep running: +`ECC_GATEGUARD=off` (or `GATEGUARD_DISABLED=1`) turns the gate off entirely. +The variables in this table do **not** — each narrows one behaviour while the +load-bearing destructive-Bash checks keep running: | Variable | Default | Effect | |---|---|---| @@ -117,23 +118,39 @@ instead, so the load-bearing destructive-Bash checks keep running: | `GATEGUARD_EXEMPT_GLOBS` | unset (no exemptions) | Comma-separated globs; a matching Edit/Write/MultiEdit target skips first-touch fact-forcing. Intended for low-import-value trees (tests, generated artifacts, scratch dirs) where "who imports this / what schema" carries no signal. | | `GATEGUARD_FACT_FORCE_FULL_DENIALS` | `3` | How many denials emit the full four-fact block before later ones condense to a single line. `0` condenses from the very first denial. | | `GATEGUARD_BASH_EXTRA_DESTRUCTIVE` | unset | Extra destructive-command patterns, as regex source, added to the built-in set. A malformed regex is treated as unset (built-ins still apply) and logged once to stderr. | -| `GATEGUARD_DISABLED` | unset | `1` disables the gate entirely — equivalent to `ECC_GATEGUARD=off`. | | `GATEGUARD_STATE_DIR` | `~/.gateguard` | Where per-session gate state is kept. If state cannot be persisted the gate allows the operation rather than looping, and names this variable in the warning. | `GATEGUARD_BASH_ROUTINE_DISABLED` accepts `1`, `true`, `on`, `enabled`, `enable`, or `yes` (case- and whitespace-insensitive); any other value -leaves the gate on. `GATEGUARD_DISABLED` recognises `1` only. +leaves the gate on. -`GATEGUARD_EXEMPT_GLOBS` patterns are matched against the normalized -(forward-slash, lowercased) file path: `*` matches within a path segment, -`**` across segments, `?` a single character. Matching is fail-open — a -malformed pattern is dropped rather than raising. +#### Turning the gate off completely + +| Variable | Effect | +|---|---| +| `ECC_GATEGUARD=off` | Disables GateGuard for the session. Accepts `0`, `false`, `off`, `disabled`, or `disable`. | +| `GATEGUARD_DISABLED=1` | Same effect. Recognises `1` only — the spellings above do **not** apply here. | + +For hook-level control, keep using `ECC_DISABLED_HOOKS` with the GateGuard hook ID. + +#### Glob semantics for `GATEGUARD_EXEMPT_GLOBS` + +Patterns are matched, unanchored, against the target path with backslashes +normalized to `/` and the whole string lowercased — the path exactly as the +hook receives it, which for Claude Code tool payloads is absolute. `*` matches +within a path segment, `**` across segments, `?` a single character. Matching +is fail-open: a malformed pattern is dropped rather than raising. + +Note that a leading `**/` compiles to `.*/`, so it requires at least one +preceding separator: `**/tests/**` exempts `/repo/tests/foo.js` but would not +match a bare relative `tests/foo.js`. Add the separator-free form too if you +pass relative paths: ```json { "env": { "GATEGUARD_BASH_ROUTINE_DISABLED": "1", - "GATEGUARD_EXEMPT_GLOBS": "**/tests/**,**/*.test.*,**/docs/**,**/dist/**" + "GATEGUARD_EXEMPT_GLOBS": "**/tests/**,tests/**,**/*.test.*,**/docs/**,**/dist/**" } } ``` diff --git a/tests/ci/gateguard-env-documented.test.js b/tests/ci/gateguard-env-documented.test.js index 4f6b4744b..6148a9c7e 100644 --- a/tests/ci/gateguard-env-documented.test.js +++ b/tests/ci/gateguard-env-documented.test.js @@ -38,13 +38,12 @@ function test(name, fn) { function readGateguardEnvNames(source) { // process.env.GATEGUARD_X and process.env['GATEGUARD_X'] - const names = new Set(); - const dotted = /process\.env\.(GATEGUARD_[A-Z0-9_]+)/g; - const bracketed = /process\.env\[\s*['"](GATEGUARD_[A-Z0-9_]+)['"]\s*\]/g; - let m; - while ((m = dotted.exec(source)) !== null) names.add(m[1]); - while ((m = bracketed.exec(source)) !== null) names.add(m[1]); - return names; + const dotted = source.match(/process\.env\.GATEGUARD_[A-Z0-9_]+/g) || []; + const bracketed = source.match(/process\.env\[\s*['"]GATEGUARD_[A-Z0-9_]+['"]\s*\]/g) || []; + const names = [...dotted, ...bracketed] + .map(hit => (hit.match(/GATEGUARD_[A-Z0-9_]+/) || [])[0]) + .filter(Boolean); + return new Set(names); } console.log('\nGateGuard env-var documentation surface\n'); @@ -73,10 +72,10 @@ if (test('every GATEGUARD_* variable the hook reads is documented', () => { if (test('the documented knobs are the ones the hook actually reads', () => { // Guards the reverse drift: a doc naming a knob the hook no longer reads. - const documented = [...new Set( - (skillDoc.match(/GATEGUARD_[A-Z0-9_]+/g) || []) - )]; - const stale = documented.filter(name => !hookSource.includes(name)).sort(); + // Compared against the parsed env reads, not raw source — a name surviving + // only in a comment or error string must not satisfy this. + const documented = [...new Set(skillDoc.match(/GATEGUARD_[A-Z0-9_]+/g) || [])]; + const stale = documented.filter(name => !envNames.has(name)).sort(); assert.deepStrictEqual(stale, [], `documented but unread by the hook: ${stale.join(', ')}`); })) passed++; else failed++;