feat(commands): a write that leaves the repo says so before you approve it (M-BUG-41)
The chain observed configuration across repos but presented every write it then proposed as though it landed where the session stands. STATE named two arms; measuring found five, and two of them are worse than the two already known: - implement — the approval prompt named NO path at all, only a count, so a plan editing ~/.claude/CLAUDE.md and one editing ./CLAUDE.md produced byte-identical prompts. - rollback — the file list rendered `.claude/settings.json`, a repo-relative FORM, while the restore writes to the absolute original. The other arms were silent; this one pointed the wrong way. - fix — paths were visible but unclassified, and --global mixed machine-wide and project rows into one unmarked table. The gate's strength comes from the target's scope class, never from the command asking: five command-owned policies would drift apart the way five copies of the lever table did. SCOPE_CLASSES is one source for class, gate, wording and predicate; templates render `disclosures[]` from the CLI instead of restating what a class means. Two orderings in that table are load-bearing, and both were measured: - plugin-managed before user-scope. Both ~/.claude/config-audit/ and the legacy ~/.config-audit/ are live, and every command writes session state there. The other order fires the gate on every write ever made and gets it switched off, which is worse than no gate. - user-scope before cross-repo. ~/.claude/.git EXISTS, so a plain .git-upward walk answers "another repo" for ~/.claude/CLAUDE.md and silently downgrades the strongest gate on the subtraction axis's primary target to disclosure. disclose is not require-ok: campaign export is cross-repo by design, so the gate there says so rather than refusing. Distinct from require-target-dir.mjs, which asks whether a scan ROOT is readable (exit 3) — a different invariant, left unmerged along with its four inline copies. Also structural, both found while building this: the hand-maintained GUARDED list in the unknown-flag sweep now derives its completeness from the directory (measured complete at 14 of 14 first, so nothing was hiding — but the 15th CLI would have been swept by nothing); and prose shape-guards use whitespace- tolerant patterns, after one went red against a command file that did say the right thing, line-wrapped. Gated: implement, fix, rollback, plan, campaign export. Suite 1596 -> 1625/0, frozen v5.0.0 and default-output baselines 0 changed files. No new GAP dimension, no lever, no finding code — utilization denominators untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013941cEohSD5Aw56FVAtBgZ
This commit is contained in:
parent
49bae2657f
commit
1543830c52
12 changed files with 821 additions and 9 deletions
160
tests/commands/write-scope-gate-shape.test.mjs
Normal file
160
tests/commands/write-scope-gate-shape.test.mjs
Normal file
|
|
@ -0,0 +1,160 @@
|
|||
/**
|
||||
* M-BUG-41 — scope-gate wiring across the write surface.
|
||||
*
|
||||
* The chain lacked a gate between "observation across repos" and "write action
|
||||
* outside the repo the session stands in". Five arms were measured carrying it
|
||||
* (`docs/mbug41-scope-gate-fasit.local.md` §2). STATE named two of them; the
|
||||
* three found by measuring were `implement`, `fix` and `rollback`, and two are
|
||||
* worse than the two originally known:
|
||||
*
|
||||
* - `implement` — the approval prompt named NO path at all, only a count, so a
|
||||
* plan editing `~/.claude/CLAUDE.md` and one editing `./CLAUDE.md` produced
|
||||
* byte-identical prompts.
|
||||
* - `rollback` — the file list rendered `.claude/settings.json`, a
|
||||
* repo-relative FORM, while the restore writes to the absolute original.
|
||||
* The other arms were silent; this one pointed the wrong way.
|
||||
* - `fix` — paths were visible, but unclassified, and `--global` mixed
|
||||
* machine-wide and project rows into one unmarked table.
|
||||
*
|
||||
* A CLI carries the answer instead of each template restating the class table,
|
||||
* because five prose paraphrases of one policy are five policies drifting apart
|
||||
* — the shape that put the lever table in five copies (#61).
|
||||
*/
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { readFile, readdir } from 'node:fs/promises';
|
||||
import { resolve, dirname } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const COMMANDS_DIR = resolve(__dirname, '..', '..', 'commands');
|
||||
const SCANNERS_DIR = resolve(__dirname, '..', '..', 'scanners');
|
||||
|
||||
/**
|
||||
* Every command whose approval surface precedes a write that can leave the repo.
|
||||
* `gatesApproval` marks the ones that own an `AskUserQuestion` and must
|
||||
* therefore branch on `requiresApproval`; `plan` has no confirmation step at all
|
||||
* — its summary IS the approval surface — and `campaign` writes cross-repo by
|
||||
* design, where the gate discloses rather than blocks.
|
||||
*/
|
||||
const GATED_COMMANDS = [
|
||||
{ file: 'implement.md', gatesApproval: true },
|
||||
{ file: 'fix.md', gatesApproval: true },
|
||||
{ file: 'rollback.md', gatesApproval: true },
|
||||
{ file: 'plan.md', gatesApproval: false },
|
||||
{ file: 'campaign.md', gatesApproval: false },
|
||||
];
|
||||
|
||||
async function readCommand(name) {
|
||||
return await readFile(resolve(COMMANDS_DIR, name), 'utf-8');
|
||||
}
|
||||
|
||||
for (const { file, gatesApproval } of GATED_COMMANDS) {
|
||||
test(`${file} classifies its write targets before the approval surface`, async () => {
|
||||
const content = await readCommand(file);
|
||||
|
||||
assert.match(
|
||||
content,
|
||||
/write-scope-cli\.mjs/,
|
||||
`${file} presents a write without classifying where it lands. Showing a count — or even a\n` +
|
||||
'path — does not tell the user the write leaves this project.',
|
||||
);
|
||||
assert.match(
|
||||
content,
|
||||
/\$\{CLAUDE_PLUGIN_ROOT\}\/scanners\/write-scope-cli\.mjs/,
|
||||
`${file} must anchor the CLI at \${CLAUDE_PLUGIN_ROOT} — a relative path resolves against\n` +
|
||||
"the user's working directory, not the plugin.",
|
||||
);
|
||||
assert.match(
|
||||
content,
|
||||
/write-scope-cli\.mjs[^\n]*--output-file[^\n]*2>\/dev\/null/,
|
||||
`${file} must invoke the CLI as \`--output-file <path> 2>/dev/null\` (ux-rules rule 2), or\n` +
|
||||
'the user sees raw payload and diagnostics.',
|
||||
);
|
||||
assert.match(
|
||||
content,
|
||||
/disclosures/,
|
||||
`${file} must render the payload's \`disclosures[]\`. Wording kept in the payload stays in\n` +
|
||||
'one place; wording paraphrased per command is a policy copy that drifts.',
|
||||
);
|
||||
});
|
||||
|
||||
if (gatesApproval) {
|
||||
test(`${file} branches its approval question on requiresApproval`, async () => {
|
||||
const content = await readCommand(file);
|
||||
assert.match(
|
||||
content,
|
||||
/requiresApproval/,
|
||||
`${file} owns an AskUserQuestion before a write, so it must branch on\n` +
|
||||
'`requiresApproval` — a machine-wide change must not share a prompt with a local one.',
|
||||
);
|
||||
assert.match(
|
||||
content,
|
||||
/outside\s+this\s+project/,
|
||||
`${file} must say the write leaves this project, in words. The scope class name alone\n` +
|
||||
'is vocabulary the user has not been taught.',
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
test('rollback shows absolute originals, not a repo-relative-looking form', async () => {
|
||||
const content = await readCommand('rollback.md');
|
||||
|
||||
// The measured defect: the result list rendered `- .claude/settings.json`,
|
||||
// which reads as project-local while the restore targets an absolute path.
|
||||
assert.doesNotMatch(
|
||||
content,
|
||||
/^\s*-\s+\.claude\/settings\.json/m,
|
||||
'rollback renders a restore target in repo-relative form. The restore writes to the absolute\n' +
|
||||
'original recorded at backup time, which may be machine-wide — the display must not imply\n' +
|
||||
'otherwise.',
|
||||
);
|
||||
});
|
||||
|
||||
test('campaign discloses the cross-repo write and does not refuse it', async () => {
|
||||
const content = await readCommand('campaign.md');
|
||||
|
||||
assert.match(
|
||||
content,
|
||||
/different\s+project/,
|
||||
'campaign export writes into another repo by design, so the gate has to SAY so.',
|
||||
);
|
||||
assert.match(
|
||||
content,
|
||||
/does\s+not\s+refuse|not\s+turn\s+this\s+into\s+a\s+refusal/,
|
||||
'The template must record that this arm discloses rather than blocks — a later edit that\n' +
|
||||
'"tightens" it into a refusal would break a by-design feature (fasit §6 A1).',
|
||||
);
|
||||
});
|
||||
|
||||
test('the scope-class table lives in exactly ONE module', async () => {
|
||||
const entries = await readdir(SCANNERS_DIR, { recursive: true });
|
||||
const owners = [];
|
||||
for (const name of entries) {
|
||||
if (!name.endsWith('.mjs')) continue;
|
||||
const src = await readFile(resolve(SCANNERS_DIR, name), 'utf-8');
|
||||
if (src.includes('plugin-managed')) owners.push(name);
|
||||
}
|
||||
|
||||
assert.deepEqual(
|
||||
owners,
|
||||
['lib/write-scope.mjs'],
|
||||
'A second module naming the scope classes is a second copy of the policy. The class, its\n' +
|
||||
'gate, its disclosure wording and its predicate belong in one export (#61: the lever table\n' +
|
||||
'was found in five copies).',
|
||||
);
|
||||
});
|
||||
|
||||
test('no command file restates the class table in prose', async () => {
|
||||
for (const { file } of GATED_COMMANDS) {
|
||||
const content = await readCommand(file);
|
||||
assert.doesNotMatch(
|
||||
content,
|
||||
/plugin-managed/,
|
||||
`${file} names an internal scope class. Templates render what the payload returns; a\n` +
|
||||
'template that enumerates the classes is a policy copy in prose.',
|
||||
);
|
||||
}
|
||||
});
|
||||
185
tests/lib/write-scope.test.mjs
Normal file
185
tests/lib/write-scope.test.mjs
Normal file
|
|
@ -0,0 +1,185 @@
|
|||
/**
|
||||
* M-BUG-41 — write-scope classification tests.
|
||||
*
|
||||
* The chain lacked a gate between "observation across repos" and "write action
|
||||
* outside the repo the session stands in". Five arms were measured carrying the
|
||||
* same hole (`docs/mbug41-scope-gate-fasit.local.md` §2): `implement` (approval
|
||||
* prompt names no path at all), `rollback` (renders repo-relative-looking paths
|
||||
* while writing to absolute originals), `fix` (`--global` mixes user-scope and
|
||||
* repo rows in one unmarked table), `plan`, and `campaign export`.
|
||||
*
|
||||
* The design decision the fasit settled: the gate's STRENGTH comes from the
|
||||
* target's scope CLASS, never from which command is asking — five command-owned
|
||||
* policies would drift apart the way five copies of the lever table did (#61).
|
||||
*
|
||||
* Two ordering rules in `SCOPE_CLASSES` are load-bearing, both measured:
|
||||
*
|
||||
* 1. `plugin-managed` BEFORE `user-scope`. Both `~/.claude/config-audit/` and
|
||||
* the legacy `~/.config-audit/` exist on the operator's machine, and every
|
||||
* command writes session state there. A naive "under ~/.claude ⇒ require
|
||||
* approval" rule would fire on every single write and be switched off.
|
||||
*
|
||||
* 2. `user-scope` BEFORE `cross-repo`. Measured: `~/.claude/.git` EXISTS —
|
||||
* the operator's `~/.claude` is a git repo with a `.gitignore` of `*`. So a
|
||||
* naive `.git`-upward-walk classifies `~/.claude/CLAUDE.md` — the single
|
||||
* most important subtraction-write target — as merely cross-repo
|
||||
* ("disclose") when it must be user-scope ("require explicit OK").
|
||||
*/
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { join } from 'node:path';
|
||||
import { homedir } from 'node:os';
|
||||
|
||||
import {
|
||||
SCOPE_CLASSES,
|
||||
classifyWriteTarget,
|
||||
} from '../../scanners/lib/write-scope.mjs';
|
||||
|
||||
const HOME = homedir();
|
||||
const REPO = '/Users/example/repos/my-plugin';
|
||||
|
||||
// A repo-root predicate is injected so classification stays hermetic: the tests
|
||||
// never touch the filesystem, and `~/.claude` is declared a repo root here
|
||||
// precisely because it IS one on the real machine.
|
||||
function fakeIsRepoRoot(dir) {
|
||||
return dir === REPO
|
||||
|| dir === '/Users/example/repos/other-plugin'
|
||||
|| dir === join(HOME, '.claude');
|
||||
}
|
||||
|
||||
function classify(target) {
|
||||
return classifyWriteTarget(target, REPO, { isRepoRoot: fakeIsRepoRoot });
|
||||
}
|
||||
|
||||
test('SCOPE_CLASSES carries class, gate and disclosure in ONE export', () => {
|
||||
const names = Object.keys(SCOPE_CLASSES);
|
||||
assert.deepEqual(
|
||||
names,
|
||||
['plugin-managed', 'in-repo', 'user-scope', 'cross-repo', 'outside'],
|
||||
'declaration order IS the match order — reordering changes behaviour',
|
||||
);
|
||||
|
||||
for (const [name, spec] of Object.entries(SCOPE_CLASSES)) {
|
||||
assert.ok(
|
||||
['silent', 'disclose', 'require-ok'].includes(spec.gate),
|
||||
`${name} must carry a known gate, got ${spec.gate}`,
|
||||
);
|
||||
if (spec.gate === 'silent') {
|
||||
assert.equal(spec.disclosure, null, `${name} is silent, so it has no disclosure text`);
|
||||
} else {
|
||||
assert.ok(
|
||||
typeof spec.disclosure === 'string' && spec.disclosure.length > 0,
|
||||
`${name} gates the write, so it MUST carry disclosure text in this same export`,
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// §5 taus-liste — four DIFFERENT reasons for silence, four fixtures. One
|
||||
// "nothing fired" assertion would pass on any of them (C4's P5 lesson).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('T1 — plugin bookkeeping under the canonical root is silent', () => {
|
||||
const r = classify(join(HOME, '.claude', 'config-audit', 'sessions', 'x', 'state.yaml'));
|
||||
assert.equal(r.scopeClass, 'plugin-managed');
|
||||
assert.equal(r.gate, 'silent');
|
||||
});
|
||||
|
||||
test('T2 — plugin bookkeeping under the LEGACY root is silent for the same reason', () => {
|
||||
const r = classify(join(HOME, '.config-audit', 'backups', 'x', 'manifest.yaml'));
|
||||
assert.equal(r.scopeClass, 'plugin-managed');
|
||||
assert.equal(r.gate, 'silent');
|
||||
});
|
||||
|
||||
test('T3 — a target inside the session repo is silent', () => {
|
||||
const r = classify(join(REPO, 'CLAUDE.md'));
|
||||
assert.equal(r.scopeClass, 'in-repo');
|
||||
assert.equal(r.gate, 'silent');
|
||||
});
|
||||
|
||||
test('T4 — the repo root itself counts as in-repo, not as "outside"', () => {
|
||||
const r = classify(REPO);
|
||||
assert.equal(r.scopeClass, 'in-repo');
|
||||
assert.equal(r.gate, 'silent');
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// The gated classes
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('user-scope config requires explicit approval, not mere disclosure', () => {
|
||||
const r = classify(join(HOME, '.claude', 'CLAUDE.md'));
|
||||
assert.equal(r.scopeClass, 'user-scope');
|
||||
assert.equal(r.gate, 'require-ok');
|
||||
assert.ok(r.disclosure.length > 0);
|
||||
});
|
||||
|
||||
test('ORDERING — ~/.claude is a real git repo, and must STILL classify as user-scope', () => {
|
||||
// The regression this pins: a `.git`-upward walk sees ~/.claude/.git (it
|
||||
// exists on the operator's machine) and would answer "cross-repo" =>
|
||||
// disclose-only, silently downgrading the gate on the subtraction axis's
|
||||
// primary target.
|
||||
const r = classify(join(HOME, '.claude', 'skills', 'some-skill', 'SKILL.md'));
|
||||
assert.equal(r.scopeClass, 'user-scope', 'user-scope must be matched BEFORE cross-repo');
|
||||
assert.equal(r.gate, 'require-ok');
|
||||
});
|
||||
|
||||
test('ORDERING — plugin state under ~/.claude does NOT inherit the user-scope gate', () => {
|
||||
// The regression this pins: dropping `plugin-managed` (or matching it after
|
||||
// `user-scope`) makes every session write demand approval.
|
||||
const r = classify(join(HOME, '.claude', 'config-audit', 'backups', 'b', 'files', 'CLAUDE.md'));
|
||||
assert.equal(r.scopeClass, 'plugin-managed');
|
||||
assert.equal(r.gate, 'silent');
|
||||
});
|
||||
|
||||
test('another repo is disclosed, never blocked — campaign export is cross-repo by design', () => {
|
||||
const r = classify('/Users/example/repos/other-plugin/docs/config-audit-plan-x.md');
|
||||
assert.equal(r.scopeClass, 'cross-repo');
|
||||
assert.equal(r.gate, 'disclose', 'blocking here would break a by-design feature (fasit §6 A1)');
|
||||
assert.ok(r.disclosure.length > 0);
|
||||
});
|
||||
|
||||
test('a path in no repo and no known root requires approval', () => {
|
||||
const r = classify('/tmp/somewhere/else.md');
|
||||
assert.equal(r.scopeClass, 'outside');
|
||||
assert.equal(r.gate, 'require-ok');
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Robustness of the boundary itself
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('a sibling whose name merely PREFIXES the repo root is not in-repo', () => {
|
||||
// `/Users/example/repos/my-plugin-2` starts with the repo root string; a
|
||||
// `startsWith` implementation would call it in-repo and skip the gate.
|
||||
const r = classify('/Users/example/repos/my-plugin-2/CLAUDE.md');
|
||||
assert.notEqual(r.scopeClass, 'in-repo');
|
||||
});
|
||||
|
||||
test('relative and unnormalised targets are resolved before classification', () => {
|
||||
const viaDots = classify(join(REPO, 'docs', '..', 'CLAUDE.md'));
|
||||
assert.equal(viaDots.scopeClass, 'in-repo');
|
||||
|
||||
const escaping = classify(join(REPO, '..', 'other-plugin', 'docs', 'x.md'));
|
||||
assert.equal(escaping.scopeClass, 'cross-repo', 'a `..` that leaves the repo must be caught');
|
||||
});
|
||||
|
||||
test('every classification result is internally consistent with SCOPE_CLASSES', () => {
|
||||
const targets = [
|
||||
join(HOME, '.claude', 'config-audit', 'x'),
|
||||
join(HOME, '.config-audit', 'x'),
|
||||
join(REPO, 'CLAUDE.md'),
|
||||
join(HOME, '.claude', 'CLAUDE.md'),
|
||||
'/Users/example/repos/other-plugin/docs/x.md',
|
||||
'/tmp/x.md',
|
||||
];
|
||||
for (const t of targets) {
|
||||
const r = classify(t);
|
||||
const spec = SCOPE_CLASSES[r.scopeClass];
|
||||
assert.ok(spec, `${r.scopeClass} must exist in SCOPE_CLASSES`);
|
||||
assert.equal(r.gate, spec.gate, `gate for ${t} must come from the single source`);
|
||||
assert.equal(r.disclosure, spec.disclosure, `disclosure for ${t} must come from the single source`);
|
||||
}
|
||||
});
|
||||
|
|
@ -60,8 +60,38 @@ const GUARDED = [
|
|||
{ cli: 'token-hotspots-cli.mjs', argv: [], valueFlag: '--output-file' },
|
||||
{ cli: 'whats-active.mjs', argv: [], valueFlag: '--output-file' },
|
||||
{ cli: 'self-audit.mjs', argv: [], valueFlag: null }, // no value-taking flag
|
||||
{ cli: 'write-scope-cli.mjs', argv: ['--target', 'x'], valueFlag: '--output-file' },
|
||||
];
|
||||
|
||||
/**
|
||||
* GUARDED was hand-maintained, and a hand-maintained list of what to sweep is a
|
||||
* premise rather than a measurement — the shape that made `KNOWN_OPEN` wrong
|
||||
* about its own breadth (#57). Measured when this guard was added: the list was
|
||||
* complete at 14 of 14, so nothing was hiding. It is the NEXT CLI that is at
|
||||
* risk, so the coverage is derived from the directory instead of asserted about
|
||||
* a literal.
|
||||
*/
|
||||
test('every argv-reading CLI in scanners/ appears in GUARDED', async () => {
|
||||
const entries = await readdir(SCANNERS_DIR);
|
||||
const clis = [];
|
||||
for (const name of entries) {
|
||||
if (!name.endsWith('.mjs')) continue;
|
||||
const src = await readFile(resolve(SCANNERS_DIR, name), 'utf-8');
|
||||
if (src.includes('process.argv')) clis.push(name);
|
||||
}
|
||||
|
||||
const guarded = new Set(GUARDED.map((g) => g.cli));
|
||||
const missing = clis.filter((c) => !guarded.has(c)).sort();
|
||||
|
||||
assert.deepEqual(
|
||||
missing,
|
||||
[],
|
||||
'A CLI that reads process.argv but is absent from GUARDED is swept by nothing: it can\n' +
|
||||
'accept an unknown flag, or write a file named after one, and both arms of this test\n' +
|
||||
'would still be green. Add it to GUARDED rather than relaxing this check.',
|
||||
);
|
||||
});
|
||||
|
||||
function run(cli, argv, cwd = resolve(__dirname, '..', '..')) {
|
||||
return new Promise((res) => {
|
||||
const child = spawn(process.execPath, [resolve(SCANNERS_DIR, cli), ...argv], { cwd });
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue