From 2d599769b8a9f1feca7afb53efa7b3db8a769651 Mon Sep 17 00:00:00 2001 From: prathikcannotcode Date: Tue, 10 Nov 2026 15:42:10 +0530 Subject: [PATCH] Close four write-guard bypasses - package.json leaves the layout. `npm test` is pre-approved, so an agent that could rewrite the script could run any command without a prompt. - Both hooks resolve the target as the disk spells it before matching a rule. On Windows and macOS, catalog/Provisions.json reached the read-only catalog and FLAGS.jsonl rewrote the append-only log. - The expert gate parses the file as it would be saved, so an Edit that swaps only the value, or a \u escape, is blocked. - The calculator's --out writes only a new matters//calc/.json and never overwrites one. It runs through Bash, outside the hooks. Co-Authored-By: Claude Fable 5.2 --- .claude/schemas/layout.json | 1 - .claude/scripts/hea-rea.mjs | 17 ++++++++++----- .claude/scripts/lib.mjs | 11 ++++++++-- .claude/scripts/tests/guardrails.test.mjs | 19 +++++++++++++++-- .claude/scripts/tests/hea-rea.test.mjs | 26 ++++++++++++++++++----- .claude/scripts/validate-on-write.mjs | 6 +++--- .claude/scripts/write-guard.mjs | 12 +++++++---- SECURITY.md | 2 +- 8 files changed, 71 insertions(+), 23 deletions(-) diff --git a/.claude/schemas/layout.json b/.claude/schemas/layout.json index 372d674..78a8263 100644 --- a/.claude/schemas/layout.json +++ b/.claude/schemas/layout.json @@ -22,7 +22,6 @@ { "pattern": "CITATION.cff" }, { "pattern": "SECURITY.md" }, { "pattern": "LICENSE" }, - { "pattern": "package.json" }, { "pattern": ".gitignore" }, { "pattern": ".gitattributes" } ] diff --git a/.claude/scripts/hea-rea.mjs b/.claude/scripts/hea-rea.mjs index 24b2277..ebb7b49 100644 --- a/.claude/scripts/hea-rea.mjs +++ b/.claude/scripts/hea-rea.mjs @@ -17,9 +17,9 @@ */ import { readFileSync, writeFileSync, mkdirSync, realpathSync } from 'node:fs'; -import { dirname } from 'node:path'; +import { dirname, resolve, relative } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { loadSchema, validate } from './lib.mjs'; +import { loadSchema, validate, loadLayout, matchRule, realPath } from './lib.mjs'; const ROOT = fileURLToPath(new URL('../..', import.meta.url)); const DISCLAIMER = @@ -165,9 +165,16 @@ if (isMain()) { console.error(summarise(result)); const outIdx = process.argv.indexOf('--out'); if (outIdx !== -1 && process.argv[outIdx + 1]) { - mkdirSync(dirname(process.argv[outIdx + 1]), { recursive: true }); - writeFileSync(process.argv[outIdx + 1], JSON.stringify(result, null, 2) + '\n'); - console.log(`Wrote ${process.argv[outIdx + 1]}`); + const out = process.argv[outIdx + 1]; + // --out runs outside the write-guard, so it takes only a new calc file in this workspace. + const rel = relative(realPath(ROOT), realPath(resolve(out))).replace(/\\/g, '/'); + if (matchRule(loadLayout(ROOT), rel)?.schema !== 'calc-result.schema.json') { + throw new Error(`--out must be matters//calc/.json in this workspace, got ${out}`); + } + mkdirSync(dirname(out), { recursive: true }); + try { writeFileSync(out, JSON.stringify(result, null, 2) + '\n', { flag: 'wx' }); } + catch (e) { throw e.code === 'EEXIST' ? new Error(`${out} already exists. A run is a record: pick a new run name.`) : e; } + console.log(`Wrote ${out}`); } else { console.log(JSON.stringify(result, null, 2)); } diff --git a/.claude/scripts/lib.mjs b/.claude/scripts/lib.mjs index 2219e26..79e1707 100644 --- a/.claude/scripts/lib.mjs +++ b/.claude/scripts/lib.mjs @@ -3,8 +3,8 @@ * Schema validation. Node built-ins only, so a fresh clone needs no install. */ -import { readFileSync, readdirSync, statSync, existsSync } from 'node:fs'; -import { join, extname, relative, resolve, dirname } from 'node:path'; +import { readFileSync, readdirSync, statSync, existsSync, realpathSync } from 'node:fs'; +import { join, extname, relative, resolve, dirname, basename } from 'node:path'; const SCHEMA_DIR = '.claude/schemas'; const SKIP_DIRS = new Set(['.git', 'node_modules']); @@ -17,6 +17,13 @@ export function workspaceRoot(start = process.cwd()) { } } +/** The path as the disk spells it, so another case or a symlink cannot dodge a layout rule. */ +export function realPath(path) { + if (existsSync(path)) return realpathSync.native(path); + const parent = dirname(path); + return parent === path ? path : join(realPath(parent), basename(path)); +} + /** Problems with a SKILL.md frontmatter. A bare `: ` or ` #` in a value is not valid YAML. */ export function frontmatterErrors(text, name) { const front = text.match(/^---\r?\n([\s\S]*?)\r?\n---/)?.[1] ?? ''; diff --git a/.claude/scripts/tests/guardrails.test.mjs b/.claude/scripts/tests/guardrails.test.mjs index 55195d1..b7754f9 100644 --- a/.claude/scripts/tests/guardrails.test.mjs +++ b/.claude/scripts/tests/guardrails.test.mjs @@ -1,7 +1,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { mkdtempSync, mkdirSync, copyFileSync, writeFileSync, readFileSync, rmSync } from 'node:fs'; +import { mkdtempSync, mkdirSync, copyFileSync, writeFileSync, readFileSync, rmSync, existsSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -49,7 +49,7 @@ test('layout matches data paths and rejects strays', () => { test('agents cannot write the guards that bind them', () => { const layout = loadLayout(repo); - for (const p of ['.claude/scripts/write-guard.mjs', '.claude/schemas/layout.json', '.claude/schemas/calc-result.schema.json', '.claude/settings.json', '.claude/settings.local.json']) { + for (const p of ['.claude/scripts/write-guard.mjs', '.claude/schemas/layout.json', '.claude/schemas/calc-result.schema.json', '.claude/settings.json', '.claude/settings.local.json', 'package.json']) { // package.json: `npm test` is pre-approved, so its scripts are a guard too assert.equal(matchRule(layout, p), undefined, p); } assert.ok(matchRule(layout, '.claude/skills/env-catalog/SKILL.md')); @@ -80,6 +80,21 @@ test('write-guard stops an agent recording expert validation', () => { const r = run('write-guard.mjs', { cwd: dir, input: { tool_name: 'Write', tool_input: { file_path: 'matters/m1/calc/run-1.json', content: '{ "validation_status": "expert_validated" }' } } }); assert.equal(r.status, 2); assert.match(r.stderr, /only a named human expert/); + + const file = 'matters/m1/calc/run-1.json'; + const guard = (tool_name, tool_input) => run('write-guard.mjs', { cwd: dir, input: { tool_name, tool_input: { file_path: file, ...tool_input } } }).status; + writeFileSync(join(dir, file), '{ "validation_status": "pending_expert_review", "label": "x" }'); + assert.equal(guard('Edit', { old_string: 'pending_expert_review', new_string: 'expert_validated' }), 2, 'an Edit that swaps only the value'); + assert.equal(guard('Write', { content: '{ "validation_status": "expert\\u005fvalidated" }' }), 2, 'a unicode escape'); + assert.equal(guard('Edit', { old_string: '"x"', new_string: '"y"' }), 0, 'an ordinary edit still passes'); + rmSync(dir, { recursive: true }); +}); + +test('another spelling of a guarded path is still guarded', { skip: !existsSync(join(repo, 'PACKAGE.JSON')) && 'case-sensitive disk' }, () => { + const dir = workspace(); + const guard = (file_path, content) => run('write-guard.mjs', { cwd: dir, input: { tool_name: 'Write', tool_input: { file_path, content } } }).status; + assert.equal(guard('catalog/Provisions.json', '{}'), 2); + assert.equal(guard('matters/m1/FLAGS.jsonl', ''), 2); rmSync(dir, { recursive: true }); }); diff --git a/.claude/scripts/tests/hea-rea.test.mjs b/.claude/scripts/tests/hea-rea.test.mjs index dea2f8b..b709ff8 100644 --- a/.claude/scripts/tests/hea-rea.test.mjs +++ b/.claude/scripts/tests/hea-rea.test.mjs @@ -136,11 +136,27 @@ test('CLI reports bad input plainly and exits 1', () => { } }); -test('CLI creates the output folder', () => { - const r = cli(worked, '--out', '/matters/new-matter/calc/run-1.json'); - assert.equal(r.status, 0, r.stderr); - assert.ok(existsSync(join(r.dir, 'matters/new-matter/calc/run-1.json'))); - rmSync(r.dir, { recursive: true }); +test('CLI writes only a new calc file inside the workspace', () => { + const matter = join(repo, `matters/zz-test-${process.pid}`); // gitignored + const out = join(matter, 'calc/run-1.json'); + try { + const first = cli(worked, '--out', out); + assert.equal(first.status, 0, first.stderr); + assert.ok(existsSync(out), 'creates the output folder'); + const again = cli(worked, '--out', out); + assert.equal(again.status, 1); + assert.match(again.stderr, /already exists/); + for (const dir of [first.dir, again.dir]) rmSync(dir, { recursive: true }); + + for (const bad of ['/matters/m/calc/run-1.json', join(repo, 'catalog/provisions.json'), join(matter, 'flags.jsonl'), join(repo, '.claude/settings.json')]) { + const r = cli(worked, '--out', bad); + assert.equal(r.status, 1, bad); + assert.match(r.stderr, /--out must be matters/); + rmSync(r.dir, { recursive: true }); + } + } finally { + rmSync(matter, { recursive: true, force: true }); + } }); test('CLI still runs when reached through a symlink', { skip: process.platform === 'win32' }, () => { diff --git a/.claude/scripts/validate-on-write.mjs b/.claude/scripts/validate-on-write.mjs index 7c9c3b0..ae65656 100644 --- a/.claude/scripts/validate-on-write.mjs +++ b/.claude/scripts/validate-on-write.mjs @@ -7,7 +7,7 @@ */ import { resolve, relative } from 'node:path'; -import { checkFile, workspaceRoot } from './lib.mjs'; +import { checkFile, workspaceRoot, realPath } from './lib.mjs'; let raw = ''; process.stdin.setEncoding('utf8'); @@ -21,9 +21,9 @@ process.stdin.on('end', () => { const start = [event.cwd, process.cwd(), process.env.CLAUDE_PROJECT_DIR].find(dir => dir && workspaceRoot(dir)); if (!start) process.exit(0); - const root = workspaceRoot(start); + const root = realPath(workspaceRoot(start)); - const rel = relative(root, resolve(start, filePath)).replace(/\\/g, '/'); + const rel = relative(root, realPath(resolve(start, filePath))).replace(/\\/g, '/'); const errors = checkFile(rel, root); if (errors.length) { console.error(`Validation failed for ${rel}:\n ${errors.join('\n ')}`); diff --git a/.claude/scripts/write-guard.mjs b/.claude/scripts/write-guard.mjs index 708f174..0fe2cde 100644 --- a/.claude/scripts/write-guard.mjs +++ b/.claude/scripts/write-guard.mjs @@ -14,7 +14,7 @@ import { readFileSync, existsSync } from 'node:fs'; import { resolve, relative, isAbsolute } from 'node:path'; -import { loadLayout, matchRule, workspaceRoot } from './lib.mjs'; +import { loadLayout, matchRule, workspaceRoot, realPath } from './lib.mjs'; const SECRET_PATTERNS = [ /(?:api[_-]?key|apikey)["']?\s*[:=]\s*["']?[A-Za-z0-9_\-]{20,}/i, @@ -63,9 +63,9 @@ function guard(event) { // The event's cwd follows Claude; the others cover a cwd this platform can't read. const start = [event.cwd, process.cwd(), process.env.CLAUDE_PROJECT_DIR].find(dir => dir && workspaceRoot(dir)); if (!start) block('no workspace layout found at or above the working directory'); - const root = workspaceRoot(start); + const root = realPath(workspaceRoot(start)); - const abs = resolve(start, filePath); + const abs = realPath(resolve(start, filePath)); const rel = relative(root, abs).replace(/\\/g, '/'); if (rel === '..' || rel.startsWith('../') || isAbsolute(rel)) block(`path outside workspace: ${filePath}`); @@ -81,7 +81,11 @@ function guard(event) { } const content = [input.content, ...editsOf(input).map(e => e.new_string)].filter(Boolean).join('\n'); - if (/"validation_status"\s*:\s*"expert_validated"/.test(content)) { + // The file as it would be saved, parsed, so an Edit that swaps only the value or a \u escape is still caught. + let status; + try { status = JSON.parse(contentAfter(existsSync(abs) ? readFileSync(abs, 'utf8') : '', event.tool_name, input)).validation_status; } + catch { /* not a JSON file */ } + if (status === 'expert_validated' || /"validation_status"\s*:\s*"expert_validated"/.test(content)) { block('only a named human expert records expert_validated, by hand, with validated_by and validated_at.'); } if (SECRET_PATTERNS.some(p => p.test(content))) { diff --git a/SECURITY.md b/SECURITY.md index 594660e..77e71dd 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -14,7 +14,7 @@ Include what you found, how to reproduce it, and the impact. ## Known limits -- Hooks guard Claude Code's Write, Edit and MultiEdit tools. A shell command run through the Bash tool is not intercepted, and that includes the calculator's `--out`; `npm run validate` still checks what it wrote. Keep Bash permissions narrow (`.claude/settings.json` allows only the validate, test and calculator commands by default). +- Hooks guard Claude Code's Write, Edit and MultiEdit tools. A shell command run through the Bash tool is not intercepted. The calculator's `--out` checks itself: it writes only a new `matters//calc/.json` and never overwrites one. Keep Bash permissions narrow (`.claude/settings.json` allows only the validate, test and calculator commands by default). - `onFailure: block` needs Claude Code 2.1.295 or newer. On older versions, a hook that cannot start at all does not block the write. - The hook command looks for the guard under `${CLAUDE_PROJECT_DIR}`, the folder a session started in, then relative to the working directory. A session that moved here from elsewhere and then changed into a subdirectory can't find it, so writes are blocked until it returns to the repo root. - On Windows without Git Bash, Claude Code runs hooks in PowerShell, which can't parse this command. The write-guard then blocks every write, and post-write validation does not run. Install Git for Windows.