From ad0d9183ff16d1cf203b1fe3c4e70fa3c2130410 Mon Sep 17 00:00:00 2001 From: Jeffrey Montoya Date: Tue, 29 Sep 2026 14:04:26 -0400 Subject: [PATCH] feat(gateguard): pass comment and whitespace-only edits The file stays unchecked, so the next code change is still gated; counted in trivial_allows. --- docs/gateguard/design-notes.md | 22 ++- docs/ja-JP/skills/gateguard/SKILL.md | 1 + docs/zh-CN/skills/gateguard/SKILL.md | 1 + manifests/context-packs/skill-triggers@1.json | 2 +- scripts/hooks/gateguard-fact-force.js | 55 ++++++- scripts/lib/gateguard-state.js | 5 + skills/gateguard/SKILL.md | 49 +++++- tests/hooks/gateguard-fact-force.test.js | 154 ++++++++++++++++++ tests/lib/gateguard-state.test.js | 9 + 9 files changed, 276 insertions(+), 22 deletions(-) diff --git a/docs/gateguard/design-notes.md b/docs/gateguard/design-notes.md index 58c2108bb..ef17ba30e 100644 --- a/docs/gateguard/design-notes.md +++ b/docs/gateguard/design-notes.md @@ -34,14 +34,14 @@ headings stable, since they are the anchors. ## Fail to deny Every allowance other than a retry of an already-gated target (prior-search -credit, sibling collapse, the denial cap) is an exception to the first-touch +credit, the trivial-edit pass, sibling collapse, the denial cap) is an exception to the first-touch denial, so each one falls back to the denial on any doubt: an unreadable transcript, a parse failure, an unresolvable path, or an exception. The lib exports that feed those decisions never throw (they catch and return a fallback that denies, or no scan at all), so an error cannot escape into credit or -sibling logic. Allowances are never an `allow` permission decision: credit and -sibling passes return `additionalContext`, and the cap pass returns the input -unchanged. +sibling logic. Allowances are never an `allow` permission decision: credit, +trivial and sibling passes return `additionalContext`, and the cap pass returns +the input unchanged. ## Read is not evidence @@ -52,8 +52,10 @@ Only `Glob`, `Grep`, `LS` and shell search commands count. ## Sensitive targets Secrets, keys, auth/payment code, migrations and CI workflows always draw the -first-touch denial: prior-search credit, sibling collapse and the denial cap do -not apply to them, and the denial says so. +first-touch denial with the full questions: prior-search credit, the change +profile, the trivial-edit pass, sibling collapse and the denial cap do not +apply to them, and the denial says so (its note names the three older +exceptions). - Basename, extension and segment rules are exact, never substrings. - `.github/workflows/` is also matched below an ancestor (an unverified @@ -341,7 +343,13 @@ Anything the lexer cannot read with confidence makes the entry non-trivial: Make), the trigraph `??/`, whitespace after a line-continuation `\`, a `/*` inside a block comment, and an unterminated block comment or string. -Write is never trivial. The profile sees only the snippet, so a comment-looking +Write is never trivial. In the hook, a trivial Edit (or a MultiEdit whose +entries for that file are all trivial) of an unchecked code, test or prose +target passes with an `additionalContext` note after prior-search credit and +before sibling collapse and the denial cap. It is not marked checked and does +not touch the denial count or ordinal, so the next change that alters code is +gated as a first touch; `trivial_allows` counts the passes (merged by maximum +like the other counters). The profile sees only the snippet, so a comment-looking line inside a multi-line string that opens and closes outside the snippet reads as a comment; the trivial pass never marks the target checked and never applies to sensitive, instruction or config targets, so the next non-trivial diff --git a/docs/ja-JP/skills/gateguard/SKILL.md b/docs/ja-JP/skills/gateguard/SKILL.md index 996079758..b0abb1ecf 100644 --- a/docs/ja-JP/skills/gateguard/SKILL.md +++ b/docs/ja-JP/skills/gateguard/SKILL.md @@ -130,6 +130,7 @@ Triggers on: `rm -rf`, `git reset --hard`, `git push --force`, `drop table`, etc - **ディレクトリ一致:** まだ存在しないファイルを作成する Write だけが、対象のディレクトリそのものを指す検索でも一致します(Glob の `path` とリテラル接頭辞、Grep/LS の `path`、シェルのパス引数)。シェル引数では grep 系のパターン引数や値を取るフラグの値を除き、今実在するディレクトリだけを数えます。ターン内のいずれかの Bash/PowerShell 呼び出しに `cd`/`pushd`/`Set-Location` などがあれば、そのターンのシェルのディレクトリ一致は無効です。Edit、MultiEdit、既存ファイルを上書きする Write は語幹一致のみです。 - 相対パスはツールの `cwd`、次に `CLAUDE_PROJECT_DIR`、次にプロセスのディレクトリを基準に解決します。トランスクリプトが読めない等の問題があれば拒否に戻ります。 - **同一ターンの兄弟ファイル作成の集約:** まだ存在しないファイルへの Write(code、test、prose クラスのみ)が拒否された後、**同じクラス・同じディレクトリ**への新規ファイル Write は、同じターン内であれば注記付きで許可されます。ターン ID はターン開始レコード自身の `promptId`、なければその `uuid`、なければそのハッシュです(末尾範囲にターン開始がない場合は、範囲内のユーザーレコードの `promptId` がすべて一致するときだけ使います)。120 秒ルールはトランスクリプトのパスがまったくない場合に限られ、トランスクリプトが存在しない・読めない・ターン ID が得られない場合は集約されません。config・instruction ファイル、`.` で始まるセグメント(ドットファイルとすべてのドットディレクトリ)や 8.3 短縮名を含むパスは集約されません。ディレクトリはシンボリックリンクを解決した実体でも同じ判定を行い、実体のディレクトリをキーにします。Edit/MultiEdit も集約されません。 +- **コメント・空白のみの Edit:** 対応言語の機密でない code/test/prose ファイルに対し、コメントと空白だけが変わる Edit(MultiEdit はそのファイルの全エントリ)は注記付きで通過し、`trivial_allows` に数えます。チェック済みにはしないため、コードを変える次の変更は通常どおりゲートされます(Write、Python のインデント変更、複数行文字列などを含む変更は対象外)。 - **正規化パスキー:** 相対パスはツールの `cwd` を基準に解決され、`a.py`、`./a.py`、絶対パスは同じファイルとして扱われ、初回タッチは 1 回だけです。 - **セッション内カウンター:** 状態ファイルに `fact_force_credited`、`denials_by_class`、`credited_by_class`、`sibling_allows`、`dir_gates`、`cap_allows` を記録します(クラス名のみ記録し、パスは記録しません。`dir_gates` はサニタイズ済みの最初のファイル名のみ保持、最大 50 件)。 - **拒否の上限(オプトイン):** `GATEGUARD_FACT_FORCE_MAX_DENIALS` を設定すると、セッション内の初回タッチ拒否がその回数に達した後の新しいパスは拒否されずに通過します(`cap_allows` に記録)。`GATEGUARD_FACT_FORCE_FULL_DENIALS` はメッセージの詳しさ(メッセージ予算)だけを変えますが、こちらはブロックするかどうか(拒否予算)を変えます。事前検索のクレジットと兄弟ファイル集約が先に判定され、上限を消費しません。不正な値は上限なしとして扱い、stderr に 1 回警告します。 diff --git a/docs/zh-CN/skills/gateguard/SKILL.md b/docs/zh-CN/skills/gateguard/SKILL.md index 009b901af..966695a7a 100644 --- a/docs/zh-CN/skills/gateguard/SKILL.md +++ b/docs/zh-CN/skills/gateguard/SKILL.md @@ -93,6 +93,7 @@ LLM 的自我评估不起作用。问"你是否违反了任何策略?"答案 * **目标类别:** 首次编辑/写入按项目相对路径(去掉开头的 `.claude/worktrees//`)依次判定为 instruction(`CLAUDE.md`、`AGENTS.md`、`SKILL.md`、`.mdc`、`.cursorrules`、`.github/copilot-instructions.md` 等)、test、prose(`.md` 等)、config(`.json`、`.yaml`、`.env` / `.env.*` 等,`.env.example.ts` 仍属 code)或 code,并提出相应的问题。 * **按变更内容提问:** 对 code 类目标还会读取变更内容:未触及公开声明的编辑改问"本文件或其模块中依赖所改行为的调用点",不涉及数据的变更省略数据格式问题;无法可靠判断(不支持的扩展名、超过 64 KiB 等)以及敏感目标时仍问原来的四个问题。 * **事先搜索抵免:** 若当前人类回合内已有成功完成(非错误结果)的 `Glob`、`Grep`、`LS` 或 `rg`/`grep`/`find` 等搜索命中该文件,则以附注代替拒绝。`Read` 从不计入;与被门控调用同批次的搜索、重复的 tool_use id、压缩(compaction)之前的搜索均不计入。文件名词干须按单词边界匹配,且目标须位于所搜索目录之内(shell 片段按其路径参数或 `cwd` 判定范围;参数前的输入重定向 `<` 视为读取 stdin)。仅"创建尚不存在的文件"的写入可按目录匹配,且 shell 参数须是真实存在的目录;回合内出现 `cd` 等切换目录命令时不再按 shell 目录匹配。相对路径按工具的 `cwd` 解析。排除规则从不计入:`Grep` 的 `glob` 中以 `!` 开头的项,以及 shell 中 `rg -g '!x'`、`grep --exclude`/`--exclude-dir`、`fd -E`、`ls -I`、`tree -I`、`git ls-files -x`、git `:!x` 路径规格、带 `-not`/`!` 或后接 `-prune` 的 `find -name`/`-path`、PowerShell `-Exclude` 都不作为词干来源;若排除项覆盖目标(文件名、路径上的目录或词干),该搜索完全不能为其抵免(`rg -g '!widget.py' TODO .` 不抵免 `widget.py`,`rg -g '!*.md' widget .` 仍抵免)。含 `--exclude-from`/`--ignore-file` 的片段不抵免;用 `*.py`、`--include`、`find -name` 等文件名 glob 限定的搜索,只有在其匹配目标文件名时词干才计入。 +* **仅注释/空白的编辑:** 对受支持语言中非敏感的 code/test/prose 文件,只改动注释和空白的编辑(MultiEdit 须该文件的所有条目都如此)以附注放行并计入 `trivial_allows`,但不标记为已检查,下一次改动代码的编辑仍按首次触碰门控(写入、Python 缩进变化、多行字符串等从不算作此类编辑)。 * **同回合同级文件合并:** 同一目录、同一类别(仅 code、test、prose)的新文件在同一回合内(完全没有 transcript 路径时为 120 秒内)只拒绝第一个,其余以附注放行。config、instruction,以及任何以 `.` 开头的路径段(点文件和所有点目录)或 8.3 短文件名下的文件从不合并;目录按解析符号链接后的真实位置判定和记录;有 transcript 路径但文件缺失、无法读取或得不到回合 ID 时不合并;编辑从不合并。 * **拒绝上限(可选):** 设置 `GATEGUARD_FACT_FORCE_MAX_DENIALS` 后,会话内首次触碰的拒绝次数达到该值后,新路径直接放行(计入 `cap_allows`)。`GATEGUARD_FACT_FORCE_FULL_DENIALS` 只决定消息详略(消息预算),而此变量决定是否拦截(拒绝预算)。事先搜索抵免和同级合并先于上限判定,不消耗上限;格式错误的值视为无上限,并在 stderr 警告一次。 * **敏感目标:** `.env`/`.env.*`、`*.pem`/`*.key`/`*.p12`/`*.pfx`、`id_rsa*`/`id_ed25519*`/`id_ecdsa*`/`id_dsa*`/`.netrc`/`.pgpass`/`credentials*`/`secrets.*`、任一路径段恰为 `auth`、`authn`、`authz`、`security`、`secrets`、`payment`、`payments`、`billing`、`migrations`,以及 `.github/workflows/` 下的文件,不适用抵免、合并和上限,首次触碰总是拒绝。按字面路径和解析符号链接后的真实位置同时判定(`src/tools -> ../auth` 时 `src/tools/login.py` 也是敏感目标),真实位置无法解析时视为敏感。判定顺序:exempt → subagent → checked → 敏感? → 抵免 → 同级合并 → 上限 → 拒绝。 diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index 8f9f4e0e5..341870f86 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -1 +1 @@ -{"coverage":{"skills":293,"withTriggers":32},"generatedAt":"2026-09-24T23:51:22.784Z","id":"skill-triggers@1","model":{"effort":null,"id":"hand-seeded","source":"manual-curation-pending-regeneration"},"registryDigest":"ba200e80f4b96c71c8dd46029e2565769248955da20ccf849479f2e6613a4f92","schemaVersion":1,"triggers":{"skill:api-connector-builder":["add api integration","new provider connector","match existing integration pattern"],"skill:api-design":["rest endpoint design","pagination api","status codes","api versioning","rate limiting api","resource naming","filtering api","api error responses","offset pagination","limit query parameter","pagination defaults"],"skill:backend-patterns":["express api","node backend architecture","nextjs api routes","server side patterns","data access layer","static file server","url path handling","file server"],"skill:browser-qa":["deployed feature test","visual regression screenshots","core web vitals check","axe accessibility audit","ship do not ship","staging verification"],"skill:canary-watch":["post deploy monitoring","smoke test url","production url check","console errors production","sse stream check","after deploy verification"],"skill:code-tour":["onboarding walkthrough","explain subsystem","architecture tour","pr walkthrough","rca tour"],"skill:coding-standards":["code review standards","naming conventions","readability review","immutability conventions","fix naming typo","export naming","consistent exports"],"skill:content-hash-cache-pattern":["cache file processing","content addressed cache","sha256 hash cache"],"skill:database-migrations":["zero downtime migration","schema change production","add column large table","backfill data","expand contract","concurrent index","migration rollback","prisma migration","django migration"],"skill:deployment-patterns":["ci cd setup","dockerize app","health checks","rollback strategy","production readiness","deploy pipeline","containerize application"],"skill:design-system":["design tokens","visual consistency audit","css custom properties","ui audit","design system bootstrap"],"skill:django-patterns":["django orm","drf api","django rest framework","django caching","django signals","django middleware"],"skill:django-security":["django authentication","csrf protection","sql injection prevention","xss prevention","django deployment security","role based access control","authorization middleware","permissions checks"],"skill:docker-patterns":["dockerfile review","docker compose setup","container security","multi service orchestration"],"skill:error-handling":["error types","retry logic","circuit breaker","user facing errors","exception handling patterns","typed errors","error boundaries","go error handling","custom error class","error codes","config validation"],"skill:evm-token-decimals":["token decimals","wei conversion","erc20 balance off","bridge token precision"],"skill:frontend-a11y":["aria attributes","screen reader support","focus management","semantic html","form labeling","keyboard navigation react","a11y lint errors"],"skill:git-workflow":["merge vs rebase","commit conventions","resolve merge conflict","branching strategy","clean up commits","pull request cleanup","git history tidy"],"skill:hexagonal-architecture":["ports and adapters","dependency injection boundaries","decouple domain from io"],"skill:kubernetes-patterns":["kubernetes manifests","kubectl debugging","pod probes","k8s rbac","autoscaling config","configmap secrets"],"skill:orch-fix-defect":["fix a bug","broken behavior","regression fix","reproduce bug","defect repair"],"skill:postgres-patterns":["slow postgres query","query optimization","index design","rls policies","supabase schema","postgres indexing","database performance","schema design postgres","postgres driver","node postgres","query planner"],"skill:python-patterns":["pythonic code","pep 8","type hints python","python code review","idiomatic python"],"skill:python-testing":["pytest fixtures","mocking python","parametrized tests","coverage python","tdd python"],"skill:redis-patterns":["cache aside pattern","distributed lock","redis rate limiting","cache invalidation"],"skill:regex-vs-llm-structured-text":["parse invoice","extract receipt data","text extraction pipeline","parse form fields","cheap document parser","extract table data","parse log lines","parse access logs","common log format","log line parsing"],"skill:rust-patterns":["rust ownership","borrow checker","rust error handling","traits rust","rust concurrency","idiomatic rust"],"skill:search-first":["find existing library","npm package research","before writing custom code","evaluate existing tools","add dependency research"],"skill:security-review":["security audit","authentication review","sanitize user input","secrets handling","payment security checklist","prevent injection attacks","secure api endpoints","authn authz review","vulnerability checklist","input validation security","parameterized queries","sql injection"],"skill:security-scan":["audit claude config","claudemd security","mcp server audit","agentshield scan","hook configuration audit","settings json security"],"skill:tdd-workflow":["write test first","failing test","red green refactor","test driven development","regression test first","write a regression test"],"skill:verification-loop":["pre pr checks","verification report","quality gates","build lint test coverage","before creating a pr"]},"triggersDigest":"25b97a9e06fc336c7cf95ab854ed1a41033a54bcd6e1fb1cf69dc906332462aa"} +{"coverage":{"skills":293,"withTriggers":32},"generatedAt":"2026-09-24T23:51:22.784Z","id":"skill-triggers@1","model":{"effort":null,"id":"hand-seeded","source":"manual-curation-pending-regeneration"},"registryDigest":"34402347e0722c2f86fda65f067dafd4ede2bfa7ef3cf241c23143fd6eb6b49b","schemaVersion":1,"triggers":{"skill:api-connector-builder":["add api integration","new provider connector","match existing integration pattern"],"skill:api-design":["rest endpoint design","pagination api","status codes","api versioning","rate limiting api","resource naming","filtering api","api error responses","offset pagination","limit query parameter","pagination defaults"],"skill:backend-patterns":["express api","node backend architecture","nextjs api routes","server side patterns","data access layer","static file server","url path handling","file server"],"skill:browser-qa":["deployed feature test","visual regression screenshots","core web vitals check","axe accessibility audit","ship do not ship","staging verification"],"skill:canary-watch":["post deploy monitoring","smoke test url","production url check","console errors production","sse stream check","after deploy verification"],"skill:code-tour":["onboarding walkthrough","explain subsystem","architecture tour","pr walkthrough","rca tour"],"skill:coding-standards":["code review standards","naming conventions","readability review","immutability conventions","fix naming typo","export naming","consistent exports"],"skill:content-hash-cache-pattern":["cache file processing","content addressed cache","sha256 hash cache"],"skill:database-migrations":["zero downtime migration","schema change production","add column large table","backfill data","expand contract","concurrent index","migration rollback","prisma migration","django migration"],"skill:deployment-patterns":["ci cd setup","dockerize app","health checks","rollback strategy","production readiness","deploy pipeline","containerize application"],"skill:design-system":["design tokens","visual consistency audit","css custom properties","ui audit","design system bootstrap"],"skill:django-patterns":["django orm","drf api","django rest framework","django caching","django signals","django middleware"],"skill:django-security":["django authentication","csrf protection","sql injection prevention","xss prevention","django deployment security","role based access control","authorization middleware","permissions checks"],"skill:docker-patterns":["dockerfile review","docker compose setup","container security","multi service orchestration"],"skill:error-handling":["error types","retry logic","circuit breaker","user facing errors","exception handling patterns","typed errors","error boundaries","go error handling","custom error class","error codes","config validation"],"skill:evm-token-decimals":["token decimals","wei conversion","erc20 balance off","bridge token precision"],"skill:frontend-a11y":["aria attributes","screen reader support","focus management","semantic html","form labeling","keyboard navigation react","a11y lint errors"],"skill:git-workflow":["merge vs rebase","commit conventions","resolve merge conflict","branching strategy","clean up commits","pull request cleanup","git history tidy"],"skill:hexagonal-architecture":["ports and adapters","dependency injection boundaries","decouple domain from io"],"skill:kubernetes-patterns":["kubernetes manifests","kubectl debugging","pod probes","k8s rbac","autoscaling config","configmap secrets"],"skill:orch-fix-defect":["fix a bug","broken behavior","regression fix","reproduce bug","defect repair"],"skill:postgres-patterns":["slow postgres query","query optimization","index design","rls policies","supabase schema","postgres indexing","database performance","schema design postgres","postgres driver","node postgres","query planner"],"skill:python-patterns":["pythonic code","pep 8","type hints python","python code review","idiomatic python"],"skill:python-testing":["pytest fixtures","mocking python","parametrized tests","coverage python","tdd python"],"skill:redis-patterns":["cache aside pattern","distributed lock","redis rate limiting","cache invalidation"],"skill:regex-vs-llm-structured-text":["parse invoice","extract receipt data","text extraction pipeline","parse form fields","cheap document parser","extract table data","parse log lines","parse access logs","common log format","log line parsing"],"skill:rust-patterns":["rust ownership","borrow checker","rust error handling","traits rust","rust concurrency","idiomatic rust"],"skill:search-first":["find existing library","npm package research","before writing custom code","evaluate existing tools","add dependency research"],"skill:security-review":["security audit","authentication review","sanitize user input","secrets handling","payment security checklist","prevent injection attacks","secure api endpoints","authn authz review","vulnerability checklist","input validation security","parameterized queries","sql injection"],"skill:security-scan":["audit claude config","claudemd security","mcp server audit","agentshield scan","hook configuration audit","settings json security"],"skill:tdd-workflow":["write test first","failing test","red green refactor","test driven development","regression test first","write a regression test"],"skill:verification-loop":["pre pr checks","verification report","quality gates","build lint test coverage","before creating a pr"]},"triggersDigest":"25b97a9e06fc336c7cf95ab854ed1a41033a54bcd6e1fb1cf69dc906332462aa"} diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 741cf144d..7e1ab7c7a 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -47,6 +47,7 @@ const { getDenialCount, getCreditedCount, getCapAllowCount, + getTrivialAllowCount, getSiblingAllowCount, getClassCounts, mergeClassCounts, @@ -1451,6 +1452,7 @@ function saveState(state) { let mergedCreditedByClass = getClassCounts(state, 'credited_by_class'); let mergedSiblingAllows = getSiblingAllowCount(state); let mergedCapAllows = getCapAllowCount(state); + let mergedTrivialAllows = getTrivialAllowCount(state); try { if (fs.existsSync(stateFile)) { @@ -1468,6 +1470,7 @@ function saveState(state) { mergedCreditedByClass = mergeClassCounts(getClassCounts(diskState, 'credited_by_class'), mergedCreditedByClass); mergedSiblingAllows = Math.max(mergedSiblingAllows, getSiblingAllowCount(diskState)); mergedCapAllows = Math.max(mergedCapAllows, getCapAllowCount(diskState)); + mergedTrivialAllows = Math.max(mergedTrivialAllows, getTrivialAllowCount(diskState)); } } catch (_) { /* ignore malformed or transient disk state */ @@ -1482,7 +1485,8 @@ function saveState(state) { denials_by_class: mergedDenialsByClass, credited_by_class: mergedCreditedByClass, sibling_allows: mergedSiblingAllows, - cap_allows: mergedCapAllows + cap_allows: mergedCapAllows, + trivial_allows: mergedTrivialAllows }; // Atomic write: temp file + rename prevents partial reads @@ -1618,6 +1622,20 @@ function markCheckedAndCountCredit(key, cls) { })).ok; } +// --- Trivial edits --- +// see docs/gateguard/design-notes.md#trivial-edits + +const TRIVIAL_CLASSES = new Set(['code', 'test', 'prose']); + +function countTrivialAllow() { + const state = loadState(); + return saveState({ ...state, trivial_allows: getTrivialAllowCount(state) + 1 }); +} + +function isTrivialChange(cls, sensitive, profile) { + return !sensitive && TRIVIAL_CLASSES.has(cls) && Boolean(profile) && profile.known === true && profile.trivial === true; +} + // --- Sibling gates --- const SIBLING_NO_TURN_WINDOW_MS = 120 * 1000; @@ -2001,6 +2019,13 @@ function newFileGateKey(filePath, data, cls) { } } +function trivialNote(filePath) { + return ( + `[Fact-Forcing Gate] Comment or whitespace-only change to ${sanitizePath(filePath)}; no first-touch check needed. ` + + 'The next change to this file that alters code is still checked.' + ); +} + function siblingNote(gate) { return ( `[Fact-Forcing Gate] Sibling of ${sanitizePath(gate.first)} (gated earlier at denial #${gate.ordinal} this session); ` + @@ -2066,6 +2091,13 @@ function run(rawInput) { } return { additionalContext: creditNote(credit, filePath), exitCode: 0 }; } + const profile = sensitive ? null : changeProfileFor(toolName, filePath, [toolInput], toolInput.content); + if (toolName === 'Edit' && isTrivialChange(cls, sensitive, profile)) { + if (!countTrivialAllow()) { + return allowWithStateWarning(); + } + return { additionalContext: trivialNote(filePath), exitCode: 0 }; + } const turnId = isNewFile && !sensitive ? currentTurnId(getTurnScan()) : null; const collapsible = isNewFile && !sensitive && (turnId !== null || !transcriptPathFor(data)); const gateKey = collapsible ? newFileGateKey(filePath, data, cls) : null; @@ -2086,7 +2118,6 @@ function run(rawInput) { // see docs/gateguard/design-notes.md#denial-cap return rawInput; } - const profile = sensitive ? null : changeProfileFor(toolName, filePath, [toolInput], toolInput.content); return firstTouchDenial(filePath, { isWrite: toolName === 'Write', denials, cls, sensitive, profile }); } @@ -2099,11 +2130,13 @@ function run(rawInput) { } const edits = toolInput.edits || []; - const creditNotes = []; + const notes = []; + const trivialKeys = new Set(); for (const edit of edits) { const filePath = edit.file_path || ''; if (!filePath || isClaudeSettingsPath(filePath) || isExemptPath(filePath, data)) continue; const fileKey = canonicalPathKey(filePath, data); + if (trivialKeys.has(fileKey)) continue; if (!isChecked([fileKey, filePath])) { const cls = classifyTargetFor(filePath, data); const sensitive = isSensitiveTargetFor(filePath, data); @@ -2112,7 +2145,16 @@ function run(rawInput) { if (!markCheckedAndCountCredit(fileKey, cls)) { return allowWithStateWarning(); } - creditNotes.push(creditNote(credit, filePath)); + notes.push(creditNote(credit, filePath)); + continue; + } + const profile = sensitive ? null : changeProfileFor('Edit', filePath, entriesFor(edits, fileKey, data)); + if (isTrivialChange(cls, sensitive, profile)) { + if (!countTrivialAllow()) { + return allowWithStateWarning(); + } + trivialKeys.add(fileKey); + notes.push(trivialNote(filePath)); continue; } const cap = sensitive ? Number.POSITIVE_INFINITY : getMaxDenialBudget(); @@ -2124,12 +2166,11 @@ function run(rawInput) { // see docs/gateguard/design-notes.md#denial-cap continue; } - const profile = sensitive ? null : changeProfileFor('Edit', filePath, entriesFor(edits, fileKey, data)); return firstTouchDenial(filePath, { isWrite: false, denials, cls, sensitive, profile }); } } - if (creditNotes.length > 0) { - return { additionalContext: creditNotes, exitCode: 0 }; + if (notes.length > 0) { + return { additionalContext: notes, exitCode: 0 }; } return rawInput; // allow } diff --git a/scripts/lib/gateguard-state.js b/scripts/lib/gateguard-state.js index 41b23425f..51338bb85 100644 --- a/scripts/lib/gateguard-state.js +++ b/scripts/lib/gateguard-state.js @@ -22,6 +22,10 @@ function getCapAllowCount(state) { return toCount(state && state.cap_allows); } +function getTrivialAllowCount(state) { + return toCount(state && state.trivial_allows); +} + // --- Sibling dir gates --- const MAX_DIR_GATES = 50; @@ -123,6 +127,7 @@ module.exports = { getDenialCount, getCreditedCount, getCapAllowCount, + getTrivialAllowCount, getSiblingAllowCount, getClassCounts, mergeClassCounts, diff --git a/skills/gateguard/SKILL.md b/skills/gateguard/SKILL.md index 7900f07b0..0a37eccb3 100644 --- a/skills/gateguard/SKILL.md +++ b/skills/gateguard/SKILL.md @@ -90,7 +90,7 @@ Item 3 is dropped when the new content handles no data. For **code** targets the gate reads the change it is about to allow (Edit `old_string`/`new_string`, every MultiEdit entry for the file, Write -`content`) and decides two things: +`content`) and decides three things: - **Public surface:** a line on either side declares or exposes a public name. JS/TS: `export`, `exports`, `public`, `declare` (`.d.ts` files always); Python: @@ -104,6 +104,7 @@ For **code** targets the gate reads the change it is about to allow (Edit - **Data:** words for file I/O, formats (`json`, `csv`, `yaml`, …), serialisation, schemas, SQL and dates on either side. Without them the data question is dropped. +- **Trivial:** see [Comment and whitespace-only edits](#comment-and-whitespace-only-edits). Supported extensions: `.js .mjs .cjs .jsx .ts .tsx .mts .cts .py .pyi .go .rs .java .kt .kts .cs .c .h .cc .cpp .cxx .hpp .hh .hxx`. Any other extension, a @@ -217,19 +218,22 @@ this order; the first rule that applies wins: 1. **Exempt** (`GATEGUARD_EXEMPT_GLOBS`, Claude settings files) — allowed. 2. **Subagent** call — allowed (the parent session was already gated). 3. **Already checked** this session — allowed. -4. **Sensitive target?** — if so, skip straight to the denial (step 8). +4. **Sensitive target?** — if so, skip straight to the denial (step 9). 5. **Prior-search credit** — allowed with a note. -6. **Sibling collapse** — allowed with a note. -7. **Denial cap** (`GATEGUARD_FACT_FORCE_MAX_DENIALS`, opt-in) — passes +6. **Comment or whitespace-only Edit** — allowed with a note, not marked + checked. +7. **Sibling collapse** — allowed with a note. +8. **Denial cap** (`GATEGUARD_FACT_FORCE_MAX_DENIALS`, opt-in) — passes through once the session's denials have reached the cap. -8. **Deny** — counted in `fact_force_denials`. +9. **Deny** — counted in `fact_force_denials`. -Credit and sibling allows never consume the denial cap. +Credit, trivial and sibling allows never consume the denial cap. ### Sensitive targets Some files are too costly to change without the full check, so prior-search -credit, sibling collapse, and the denial cap never apply to them: they are +credit, the comment or whitespace-only pass, the change profile, sibling +collapse, and the denial cap never apply to them: they are denied on first touch exactly as without these rules, and the denial counts as usual. The denial carries one extra line: "Sensitive target: prior-search credit, sibling collapse, and the denial cap do not apply." A target is @@ -355,6 +359,36 @@ An Edit, Write, or MultiEdit entry is allowed, with an - **Falls back to deny:** a missing, unreadable, or non-file transcript, garbage records, or any internal error mean no credit. +### Comment and whitespace-only edits + +An Edit whose old and new text differ only in comments and whitespace changes +no behaviour, so the first-touch questions carry no signal. Such an Edit (or a +MultiEdit whose entries for that file are all such changes) of a `code`, +`test`, or `prose` target that is not sensitive passes with the note "Comment +or whitespace-only change to ``". The file is **not** marked checked: +the next change to it that alters code meets the normal first-touch gate. The +pass is counted in `trivial_allows` and never touches the denial count. + +It applies only to the extensions listed in +[Questions fit the change](#questions-fit-the-change), and only when every +entry reads cleanly: + +- comments are `//` and `/* */` (C family) or `#` (Python); C preprocessor + lines are code; whitespace inside strings, line breaks between code, and + the presence of whitespace between tokens are code (`a+b` → `a + b` is not + trivial); +- Python indentation of code lines is code; +- never trivial: a Write; any multi-line or raw string form (backticks, triple + quotes, `r"`, `R"`, `@"`, `$"`, f-strings, Kotlin `$`), a string running to + the end of a line, a line comment ending in `\`, `??/`, a nested or + unterminated block comment; in JS/TS any `/` outside a comment (regex versus + division), JSX-like tags, and `-->`; +- config, instruction, and sensitive targets never pass this way. + +Only the snippet is read, so a comment-looking line inside a multi-line string +that opens and closes outside the snippet reads as a comment; because the +file stays unchecked, the next real change is still gated. + ### Same-turn sibling creation collapse When several new files are created in one directory, the answers for each @@ -412,6 +446,7 @@ the sanitized first file per directory): | `sibling_allows` | Number of Writes allowed by sibling collapse | | `dir_gates` | Per class and directory, the denial that opened a sibling window (turn, time, first file, ordinal); capped at 50 entries | | `cap_allows` | Number of first touches passed through by the denial cap (`GATEGUARD_FACT_FORCE_MAX_DENIALS`); these never count as denials | +| `trivial_allows` | Number of comment or whitespace-only edits passed without the first-touch check; these never mark the file checked | Missing or malformed fields load as empty or zero, so older state files keep working. diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index f940f3aae..8ed5f2bb6 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -4560,6 +4560,160 @@ function runTests() { passed++; else failed++; + // --- Comment and whitespace-only edits --- + const b3Run = (toolName, tool_input, env = {}) => { + const result = runHook({ tool_name: toolName, tool_input }, { ...B2_ENV, ...env }); + const output = parseOutput(result.stdout); + const hso = output && output.hookSpecificOutput ? output.hookSpecificOutput : {}; + const context = Array.isArray(hso.additionalContext) ? hso.additionalContext.join('\n') : String(hso.additionalContext || ''); + return { result, decision: hso.permissionDecision, context, reason: hso.permissionDecisionReason || '' }; + }; + const b3State = () => (fs.existsSync(stateFile) ? JSON.parse(fs.readFileSync(stateFile, 'utf8')) : {}); + const b3Key = rel => `/proj-b2/${rel}`; + const B3_NOTE = 'Comment or whitespace-only change to'; + + clearState(); + if ( + test('a comment-only Edit of an unchecked code file passes with a note and is not marked checked', () => { + const out = b3Run('Edit', { file_path: 'src/t1.js', old_string: '// old note\nfoo();', new_string: '// new note\nfoo();' }); + assert.notStrictEqual(out.decision, 'deny', out.result.stdout); + assert.ok(out.context.includes(`${B3_NOTE} src/t1.js`), out.context); + assert.strictEqual(out.result.code, 0); + const state = b3State(); + assert.ok(!(state.checked || []).includes(b3Key('src/t1.js')), 'not marked checked'); + assert.strictEqual(state.trivial_allows, 1); + assert.strictEqual(state.fact_force_denials || 0, 0, 'denial count untouched'); + assert.ok(!out.result.stdout.includes('"allow"'), 'never an allow decision'); + }) + ) + passed++; + else failed++; + + if ( + test('a later code change to the same file is still gated', () => { + const out = b3Run('Edit', { file_path: 'src/t1.js', old_string: 'foo();', new_string: 'bar();' }); + assert.strictEqual(out.decision, 'deny', out.result.stdout); + const state = b3State(); + assert.strictEqual(state.fact_force_denials, 1); + assert.strictEqual(state.trivial_allows, 1, 'trivial counter kept'); + }) + ) + passed++; + else failed++; + + clearState(); + if ( + test('whitespace-only reindents and trivial test-file edits pass', () => { + const js = b3Run('Edit', { file_path: 'src/t2.js', old_string: 'if (x) {\n foo();\n}', new_string: 'if (x) {\n foo();\n}' }); + assert.ok(js.context.includes(B3_NOTE), js.result.stdout); + const spec = b3Run('Edit', { file_path: 'src/t2.test.js', old_string: "it('a', () => {}); // old", new_string: "it('a', () => {}); // new" }); + assert.ok(spec.context.includes(B3_NOTE), spec.result.stdout); + assert.strictEqual(b3State().trivial_allows, 2); + }) + ) + passed++; + else failed++; + + clearState(); + if ( + test('trivial-looking edits are still denied where the pass does not apply', () => { + const cases = [ + ['sensitive target', { file_path: 'src/auth/session.js', old_string: '// old\nfoo();', new_string: '// new\nfoo();' }], + ['comment plus code', { file_path: 'src/d1.js', old_string: '// old\nfoo(1);', new_string: '// new\nfoo(2);' }], + ['python indentation', { file_path: 'src/d2.py', old_string: 'if x:\n y()', new_string: 'if x:\n y()' }], + ['unknown extension', { file_path: 'src/d3.rb', old_string: '# old', new_string: '# new' }], + ['config target', { file_path: 'config/d4.yaml', old_string: '# old', new_string: '# new' }], + ['instruction target', { file_path: 'CLAUDE.md', old_string: '', new_string: '' }], + ['instruction code file', { file_path: '.claude/hooks/d5.md', old_string: '// old', new_string: '// new' }], + ['template literal', { file_path: 'src/d6.js', old_string: '// old\n`;', new_string: '// new\n`;' }], + ['over the bound', { file_path: 'src/d7.js', old_string: `// ${'x'.repeat(70 * 1024)}`, new_string: '// y' }], + ['missing new_string', { file_path: 'src/d8.js', old_string: '// old' }] + ]; + for (const [label, input] of cases) { + clearState(); + const out = b3Run('Edit', input); + assert.strictEqual(out.decision, 'deny', `${label}: ${out.result.stdout}`); + assert.ok(!out.context.includes(B3_NOTE), `${label}: no trivial note`); + assert.strictEqual(b3State().trivial_allows || 0, 0, `${label}: no trivial count`); + } + }) + ) + passed++; + else failed++; + + clearState(); + if ( + test('a Write is never a trivial edit', () => { + const out = b3Run('Write', { file_path: 'src/w1.js', content: '// only a comment\n' }); + assert.strictEqual(out.decision, 'deny', out.result.stdout); + }) + ) + passed++; + else failed++; + + clearState(); + if ( + test('MultiEdit passes a file only when all of its entries are trivial', () => { + const allTrivial = b3Run('MultiEdit', { + edits: [ + { file_path: 'src/m1.js', old_string: '// a', new_string: '// b' }, + { file_path: 'src/m1.js', old_string: 'foo(); // c', new_string: 'foo(); // d' } + ] + }); + assert.notStrictEqual(allTrivial.decision, 'deny', allTrivial.result.stdout); + assert.ok(allTrivial.context.includes(`${B3_NOTE} src/m1.js`), allTrivial.context); + assert.strictEqual(b3State().trivial_allows, 1, 'one pass per file'); + clearState(); + const mixed = b3Run('MultiEdit', { + edits: [ + { file_path: 'src/m2.js', old_string: '// a', new_string: '// b' }, + { file_path: 'src/m2.js', old_string: 'foo(1);', new_string: 'foo(2);' } + ] + }); + assert.strictEqual(mixed.decision, 'deny', 'a non-trivial entry for the file denies it'); + clearState(); + const twoFiles = b3Run('MultiEdit', { + edits: [ + { file_path: 'src/m3.js', old_string: '// a', new_string: '// b' }, + { file_path: 'src/m4.js', old_string: 'foo(1);', new_string: 'foo(2);' } + ] + }); + assert.strictEqual(twoFiles.decision, 'deny'); + assert.ok(twoFiles.reason.includes('src/m4.js'), 'the non-trivial file is the one denied'); + const state = b3State(); + assert.ok(!(state.checked || []).includes(b3Key('src/m3.js')), 'trivial file not marked checked'); + }) + ) + passed++; + else failed++; + + clearState(); + if ( + test('the trivial pass comes before the denial cap and never consumes it', () => { + const out = b3Run('Edit', { file_path: 'src/cap1.js', old_string: '// a', new_string: '// b' }, { GATEGUARD_FACT_FORCE_MAX_DENIALS: '0' }); + assert.ok(out.context.includes(B3_NOTE), out.result.stdout); + const state = b3State(); + assert.strictEqual(state.cap_allows || 0, 0); + assert.strictEqual(state.trivial_allows, 1); + }) + ) + passed++; + else failed++; + + clearState(); + if ( + test('trivial_allows merges by maximum with a concurrent state file', () => { + writeState({ checked: [], last_active: Date.now(), trivial_allows: 5 }); + b3Run('Edit', { file_path: 'src/merge1.js', old_string: '// a', new_string: '// b' }); + assert.strictEqual(b3State().trivial_allows, 6); + writeState({ checked: [], last_active: Date.now(), trivial_allows: 'junk' }); + b3Run('Edit', { file_path: 'src/merge2.js', old_string: '// a', new_string: '// b' }); + assert.strictEqual(b3State().trivial_allows, 1, 'malformed counter reads as zero'); + }) + ) + passed++; + else failed++; + clearState(); if ( test('scripts/hooks/x.js is code, not instruction', () => { diff --git a/tests/lib/gateguard-state.test.js b/tests/lib/gateguard-state.test.js index bd8466cd5..9510900da 100644 --- a/tests/lib/gateguard-state.test.js +++ b/tests/lib/gateguard-state.test.js @@ -11,6 +11,7 @@ const path = require('path'); const { getDenialCount, getCapAllowCount, + getTrivialAllowCount, getClassCounts, mergeClassCounts, incrementClassCount, @@ -36,6 +37,14 @@ function test(desc, fn) { } } +test('trivial_allows reads as a non-negative integer', () => { + assert.strictEqual(getTrivialAllowCount({ trivial_allows: 3 }), 3); + for (const bad of [undefined, null, -1, 'x', NaN, Infinity, {}]) { + assert.strictEqual(getTrivialAllowCount({ trivial_allows: bad }), 0, String(bad)); + } + assert.strictEqual(getTrivialAllowCount(null), 0); +}); + test('malformed or missing counters read as zero', () => { assert.strictEqual(getDenialCount(null), 0); assert.strictEqual(getDenialCount({ fact_force_denials: 'x' }), 0);