feat(scanners): the write gate now runs in code, not in the templates' prose
`write-scope.mjs` has existed since M-BUG-41, but only one writer ever called it. Measured 2026-08-12: 9 files under `scanners/` write to disk, 1 imported the gate; 21 command templates, 17 mention a write, 5 call `write-scope-cli`. Five templates paraphrasing one policy is the shape that put the lever table in five copies (#61) — one level up. The defect was never "8 ungated writers = 8 bugs". Four of them write the plugin's own bookkeeping and must STAY ungated: a gate that fires on every run gets switched off, and then it guards nothing. The defect is that nothing declared WHICH, so the question was answered by reading, and answered differently each time it was asked. `tests/lib/write-gate-coverage.test.mjs` makes the answer structural: every writer either imports the gate or holds an EXEMPT entry naming where the bytes land. Seen RED against today's tree before the fix (4 ungated writers), and each of its four assertions was separately seen red against its own defect. Two premises in the plan text were falsified by measuring them first: - `scan-orchestrator` was carried as "plugin-managed, legitimately exempt". `--save-baseline` derives its path from the SCAN TARGET, so `--global` lands `~/.claude/.config-audit-baseline.json` — user-scope, require-ok. It is gated. `lib/baseline.mjs` is the genuinely exempt one. - the first sweep scored 9 writers with a regex that could not match `writeFileSync(`, so `lib/backup.mjs` — a real writer — read as clean. The guard covers sync and async forms, strips comments before matching, and asserts non-emptiness so a regex that stops matching cannot make every other assertion vacuously green (#63, #64). Gated: fix-engine, rollback-engine, campaign-export-cli, scan-orchestrator. All five call sites share ONE reduction, `evaluateWriteTargets` — four copies of classify/strongestGate/dedup is the drift this exists to prevent. `campaign export` still DISCLOSES rather than refuses: cross-repo is by design there, and tightening it into a refusal would break the feature. A dry run is still not a write, so it is never gated (#63). A refusal is a verdict about a config that WAS examined, so it rides in the payload and keeps the 0/1/2 exit contract (#62) — and the verdict now reaches the success payload too, since stderr is discarded by `2>/dev/null` (F3's class). commands/fix.md carries `--approve-scope` from the answer the user gives, with the rule stated where it can be read: classifying is not approving. Dogfooded end to end: a target outside the session root refuses with zero bytes written, then applies under `--approve-scope`. Suite 1703 -> 1707/0. Frozen v5.0.0 + default-output snapshots: 0 changed files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pkn22uGCgk6QZA738zNmHL
This commit is contained in:
parent
e60b80978b
commit
749b710de7
16 changed files with 520 additions and 34 deletions
|
|
@ -42,6 +42,7 @@ import {
|
|||
defaultLedgerPath,
|
||||
} from './lib/campaign-ledger.mjs';
|
||||
import { planExportPath, buildPlanExportDocument } from './lib/campaign-export.mjs';
|
||||
import { evaluateWriteTargets } from './lib/write-scope.mjs';
|
||||
|
||||
const DATE_RE = /^\d{4}-\d{2}-\d{2}$/;
|
||||
|
||||
|
|
@ -62,7 +63,7 @@ function defaultSessionsDir() {
|
|||
}
|
||||
|
||||
function parseArgs(argv) {
|
||||
const flags = { repo: null, ledgerFile: null, sessionsDir: null, referenceDate: null, outputFile: null, write: false };
|
||||
const flags = { repo: null, ledgerFile: null, sessionsDir: null, referenceDate: null, outputFile: null, write: false, approveScope: false, sessionRoot: null };
|
||||
for (let i = 0; i < argv.length; i++) {
|
||||
const a = argv[i];
|
||||
if (a === '--repo' && argv[i + 1] !== undefined) flags.repo = argv[++i];
|
||||
|
|
@ -71,6 +72,8 @@ function parseArgs(argv) {
|
|||
else if (a === '--reference-date' && argv[i + 1] !== undefined) flags.referenceDate = argv[++i];
|
||||
else if (a === '--output-file' && argv[i + 1] !== undefined) flags.outputFile = argv[++i];
|
||||
else if (a === '--write') flags.write = true;
|
||||
else if (a === '--approve-scope') flags.approveScope = true;
|
||||
else if (a === '--session-root' && argv[i + 1] !== undefined) flags.sessionRoot = argv[++i];
|
||||
else if (a.startsWith('--')) fail(`unknown flag "${a}"`);
|
||||
else fail(`unexpected argument "${a}"`);
|
||||
}
|
||||
|
|
@ -147,6 +150,32 @@ async function main() {
|
|||
now,
|
||||
});
|
||||
|
||||
// Q1 — the scope gate, in code rather than in commands/campaign.md's prose.
|
||||
//
|
||||
// `--repo` here names the repo being EXPORTED TO, which is the write target's
|
||||
// repo, not the session's. The session root is where the operator stands, so
|
||||
// it comes from `--session-root` (default cwd) — reading it off `--repo`
|
||||
// would make every export look "in-repo" and silence the gate by
|
||||
// construction (#63).
|
||||
//
|
||||
// Export into another project is `cross-repo`, whose gate is `disclose`, NOT
|
||||
// `require-ok`: campaign export is cross-repo BY DESIGN, and tightening it
|
||||
// into a refusal breaks the feature. So the disclosure always rides in the
|
||||
// payload, and only a `require-ok` class (machine-wide config, or a path in
|
||||
// no project at all) actually withholds the write.
|
||||
const scope = evaluateWriteTargets([targetPath], resolve(flags.sessionRoot ?? process.cwd()));
|
||||
|
||||
if (flags.write && scope.requiresApproval && !flags.approveScope) {
|
||||
return emit(
|
||||
{ status: 'refused', action: 'export', reason: 'scope-gate', repo: repoInfo,
|
||||
sessionId: repo.sessionId, sourcePlanPath, exportable: true, problems: [],
|
||||
written: false, targetPath, gate: scope.gate, requiresApproval: true,
|
||||
disclosures: scope.disclosures },
|
||||
flags.outputFile,
|
||||
0,
|
||||
);
|
||||
}
|
||||
|
||||
let written = false;
|
||||
if (flags.write) {
|
||||
await mkdir(dirname(targetPath), { recursive: true });
|
||||
|
|
@ -156,7 +185,9 @@ async function main() {
|
|||
|
||||
return emit(
|
||||
{ status: 'ok', action: 'export', repo: repoInfo, sessionId: repo.sessionId, sourcePlanPath,
|
||||
exportable: true, problems: [], written, targetPath, document },
|
||||
exportable: true, problems: [], written, targetPath, document,
|
||||
gate: scope.gate, requiresApproval: scope.requiresApproval,
|
||||
disclosures: scope.disclosures },
|
||||
flags.outputFile,
|
||||
0,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -19,8 +19,8 @@ import { humanizeFinding } from './lib/humanizer.mjs';
|
|||
// `--dry-run` is a no-op alias: dry-run is already the default. It exists because
|
||||
// commands/fix.md documents it in argument-hint, and a documented flag that the
|
||||
// CLI silently drops is the same fail-silent class as the unknown-flag sink below.
|
||||
const BOOL_FLAGS = ['--apply', '--dry-run', '--json', '--raw', '--global'];
|
||||
const VALUE_FLAGS = ['--output-file'];
|
||||
const BOOL_FLAGS = ['--apply', '--dry-run', '--json', '--raw', '--global', '--approve-scope'];
|
||||
const VALUE_FLAGS = ['--output-file', '--repo'];
|
||||
|
||||
async function main() {
|
||||
const args = process.argv.slice(2);
|
||||
|
|
@ -30,6 +30,12 @@ async function main() {
|
|||
let rawMode = false;
|
||||
let includeGlobal = false;
|
||||
let outputFile = null;
|
||||
let approveScope = false;
|
||||
// The session's root, never the scan target (#63). `--global` fixes files
|
||||
// under `~/.claude` while the session still stands somewhere else, so reading
|
||||
// the root off the target would classify a machine-wide write as "in-repo"
|
||||
// and silence the strongest gate exactly where it matters.
|
||||
let repoRoot = process.cwd();
|
||||
|
||||
// Same defect class as M-BUG-21 in drift-cli: this loop used to end in
|
||||
// `else if (!args[i].startsWith('-')) targetPath = args[i]` with no
|
||||
|
|
@ -44,13 +50,15 @@ async function main() {
|
|||
else if (arg === '--json') jsonMode = true;
|
||||
else if (arg === '--raw') rawMode = true;
|
||||
else if (arg === '--global') includeGlobal = true;
|
||||
else if (arg === '--approve-scope') approveScope = true;
|
||||
// --dry-run: default behaviour, accepted so it is not silently dropped.
|
||||
} else if (VALUE_FLAGS.includes(arg)) {
|
||||
const value = args[i + 1];
|
||||
if (value === undefined || value.startsWith('-')) {
|
||||
throw new Error(`Option ${arg} requires a value.`);
|
||||
}
|
||||
outputFile = value;
|
||||
if (arg === '--repo') repoRoot = value;
|
||||
else outputFile = value;
|
||||
i++;
|
||||
} else if (arg.startsWith('-')) {
|
||||
throw new Error(
|
||||
|
|
@ -134,6 +142,12 @@ async function main() {
|
|||
let verified = [];
|
||||
let regressions = [];
|
||||
let backupId = null;
|
||||
// The engine's scope verdict, carried out to the payload. A `disclose` class
|
||||
// writes without withholding anything, so the ONLY place the command can
|
||||
// learn that a write left the project is here — stderr is discarded by
|
||||
// `2>/dev/null` (ux-rules rule 2), which is F3's defect class.
|
||||
let scopeGate = null;
|
||||
let scopeDisclosures = [];
|
||||
|
||||
if (fixes.length === 0) {
|
||||
const output = { planned: [], applied: [], failed: [], verified: [], regressions: [], manual, backupId: null };
|
||||
|
|
@ -159,9 +173,43 @@ async function main() {
|
|||
process.stderr.write(` Applying ${fixes.length} fixes...\n\n`);
|
||||
}
|
||||
|
||||
const result = await applyFixes(fixes, { dryRun: false, backupDir: backup.backupPath });
|
||||
const result = await applyFixes(fixes, {
|
||||
dryRun: false,
|
||||
backupDir: backup.backupPath,
|
||||
repoRoot,
|
||||
approveScope,
|
||||
});
|
||||
applied = result.applied;
|
||||
failed = result.failed;
|
||||
scopeGate = result.gate ?? null;
|
||||
scopeDisclosures = result.disclosures ?? [];
|
||||
|
||||
// A refused set is a verdict about a config that WAS examined, not a tool
|
||||
// failure — so it rides in the payload and keeps the normal exit contract
|
||||
// (#62). Anything a command must act on has to reach it through
|
||||
// `--output-file`; stderr alone is invisible to the command layer (F3).
|
||||
if (result.requiresApproval && result.refused.length > 0) {
|
||||
const payload = {
|
||||
status: 'refused',
|
||||
reason: 'scope-gate',
|
||||
gate: result.gate,
|
||||
requiresApproval: true,
|
||||
disclosures: result.disclosures,
|
||||
refused: result.refused,
|
||||
backupId,
|
||||
};
|
||||
const json = JSON.stringify(payload, null, 2) + '\n';
|
||||
if (machineMode) process.stdout.write(json);
|
||||
if (outputFile) await writeOutputFile(outputFile, json, 'utf-8');
|
||||
if (!machineMode) {
|
||||
for (const line of result.disclosures) process.stderr.write(`\n ${line}\n`);
|
||||
process.stderr.write(
|
||||
`\n Refused ${result.refused.length} fix(es) pending your go-ahead.`
|
||||
+ ' Re-run with --approve-scope to apply them.\n',
|
||||
);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (!machineMode) {
|
||||
process.stderr.write(` Results: ${applied.length} applied, ${failed.length} failed\n`);
|
||||
|
|
@ -235,6 +283,8 @@ async function main() {
|
|||
recommendation: m.recommendation,
|
||||
})),
|
||||
backupId,
|
||||
gate: scopeGate,
|
||||
disclosures: scopeDisclosures,
|
||||
};
|
||||
const serialized = JSON.stringify(output, null, 2) + '\n';
|
||||
if (machineMode) process.stdout.write(serialized);
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import { readFile, writeFile, rename, stat } from 'node:fs/promises';
|
|||
import { dirname } from 'node:path';
|
||||
import { parseJson, parseFrontmatter } from './lib/yaml-parser.mjs';
|
||||
import { createBackup } from './lib/backup.mjs';
|
||||
import { evaluateWriteTargets } from './lib/write-scope.mjs';
|
||||
import { runAllScanners } from './scan-orchestrator.mjs';
|
||||
import { VALID_EFFORT_LEVELS as SETTINGS_EFFORT_LEVELS } from './settings-validator.mjs';
|
||||
|
||||
|
|
@ -237,6 +238,42 @@ export async function applyFixes(fixPlans, opts = {}) {
|
|||
throw new Error('backupDir is required when not in dryRun mode');
|
||||
}
|
||||
|
||||
// Q1 — the scope gate, in code rather than in the command template's prose.
|
||||
//
|
||||
// A file-rename writes TWO paths: the source disappears and `newPath`
|
||||
// appears. Classifying only `plan.file` would let a rename move a repo file
|
||||
// to a machine-wide destination under a `silent` gate.
|
||||
//
|
||||
// `!dryRun` is load-bearing and is the same rule the subtraction axis
|
||||
// settled (#63): the gate guards a WRITE, and a dry run is not one.
|
||||
// `requiresApproval` is reported either way, so a caller planning a run still
|
||||
// learns that approval will be owed before anything is applied.
|
||||
const scope = evaluateWriteTargets(
|
||||
fixPlans.flatMap((p) => (p.newPath ? [p.file, p.newPath] : [p.file])),
|
||||
opts.repoRoot ?? null,
|
||||
opts.home ? { home: opts.home } : {},
|
||||
);
|
||||
|
||||
if (scope.requiresApproval && !opts.approveScope && !opts.dryRun) {
|
||||
// A refused write is a VERDICT about a config that was examined, not a
|
||||
// tool failure (#62) — the caller renders the disclosure and asks. Nothing
|
||||
// is applied, and this is not an exit-3 situation.
|
||||
return {
|
||||
applied: [],
|
||||
failed: [],
|
||||
gate: scope.gate,
|
||||
requiresApproval: true,
|
||||
disclosures: scope.disclosures,
|
||||
refused: fixPlans.map((plan) => ({
|
||||
findingId: plan.findingId,
|
||||
file: plan.file,
|
||||
status: 'refused',
|
||||
reason: 'scope-gate',
|
||||
type: plan.type,
|
||||
})),
|
||||
};
|
||||
}
|
||||
|
||||
for (const plan of fixPlans) {
|
||||
if (opts.dryRun) {
|
||||
applied.push({
|
||||
|
|
@ -269,7 +306,14 @@ export async function applyFixes(fixPlans, opts = {}) {
|
|||
}
|
||||
}
|
||||
|
||||
return { applied, failed };
|
||||
return {
|
||||
applied,
|
||||
failed,
|
||||
gate: scope.gate,
|
||||
requiresApproval: scope.requiresApproval,
|
||||
disclosures: scope.disclosures,
|
||||
refused: [],
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -165,6 +165,42 @@ export function strongestGate(targets) {
|
|||
return worst;
|
||||
}
|
||||
|
||||
/**
|
||||
* Classify a whole write SET and reduce it to one verdict (Q1).
|
||||
*
|
||||
* Every gated arm needs the same four things — classify each target, take the
|
||||
* strongest gate, de-duplicate the disclosures, report whether approval is owed
|
||||
* — and `lib/subtraction-write.mjs` was the only arm that had them, written
|
||||
* inline. Four more call sites copying those four lines is precisely the shape
|
||||
* `SCOPE_CLASSES` exists to prevent one level down: the copies drift, and the
|
||||
* drift is invisible because each one still looks correct on its own.
|
||||
*
|
||||
* This decides nothing about whether a write is a good idea, and it never
|
||||
* writes. It answers "what does this set of targets oblige you to say?".
|
||||
*
|
||||
* Note the strict default: `sessionRepoRoot` of `null` means the `in-repo`
|
||||
* class can never match, so an omitted repo root fails toward MORE disclosure,
|
||||
* not less. A caller that forgets to pass it gets a noisier gate rather than a
|
||||
* silent one.
|
||||
*
|
||||
* @param {string[]} paths - Paths about to be written. Duplicates are fine.
|
||||
* @param {string|null} sessionRepoRoot - Repo root of the current session.
|
||||
* @param {object} [options] - Forwarded to `classifyWriteTarget`.
|
||||
* @returns {{gate: string, requiresApproval: boolean, disclosures: string[], targets: object[]}}
|
||||
*/
|
||||
export function evaluateWriteTargets(paths, sessionRepoRoot, options = {}) {
|
||||
const unique = [...new Set(paths.map((p) => resolve(p)))];
|
||||
const targets = unique.map((p) => classifyWriteTarget(p, sessionRepoRoot, options));
|
||||
const gate = strongestGate(targets);
|
||||
|
||||
return {
|
||||
gate,
|
||||
requiresApproval: gate === 'require-ok',
|
||||
disclosures: [...new Set(targets.map((t) => t.disclosure).filter(Boolean))],
|
||||
targets,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Classify a write target relative to the repo the session stands in.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@
|
|||
import { readFile, writeFile, readdir, stat, rm } from 'node:fs/promises';
|
||||
import { join } from 'node:path';
|
||||
import { getBackupDir, getLegacyBackupDir, parseManifest, checksum } from './lib/backup.mjs';
|
||||
import { evaluateWriteTargets } from './lib/write-scope.mjs';
|
||||
|
||||
/**
|
||||
* Resolve a backup id to its directory, canonical root first, then the
|
||||
|
|
@ -100,6 +101,35 @@ export async function restoreBackup(backupId, opts = {}) {
|
|||
const restored = [];
|
||||
const failed = [];
|
||||
|
||||
// Q1 — the scope gate, in code. `rollback` is one of the five arms M-BUG-41
|
||||
// measured: it renders repo-relative-looking paths while writing to the
|
||||
// ABSOLUTE originals recorded in the manifest, so what the operator reads and
|
||||
// what the run touches are not the same set. A backup taken under `--global`
|
||||
// restores `~/.claude/…`, which is `user-scope` / `require-ok`.
|
||||
const scope = evaluateWriteTargets(
|
||||
manifest.files.map((f) => f.originalPath),
|
||||
opts.repoRoot ?? null,
|
||||
opts.home ? { home: opts.home } : {},
|
||||
);
|
||||
|
||||
// The gate guards a WRITE; a dry run is not one (#63). `requiresApproval` is
|
||||
// returned either way, so a caller previewing a restore still learns that
|
||||
// approval will be owed.
|
||||
if (scope.requiresApproval && !opts.approveScope && !opts.dryRun) {
|
||||
return {
|
||||
restored: [],
|
||||
failed: [],
|
||||
gate: scope.gate,
|
||||
requiresApproval: true,
|
||||
disclosures: scope.disclosures,
|
||||
refused: manifest.files.map((f) => ({
|
||||
originalPath: f.originalPath,
|
||||
status: 'refused',
|
||||
reason: 'scope-gate',
|
||||
})),
|
||||
};
|
||||
}
|
||||
|
||||
// A manifest with entries that parsed to nothing would restore nothing while
|
||||
// reporting success. Fail loudly instead.
|
||||
if (manifest.files.length === 0 && /^\s+-\s/m.test(manifestContent)) {
|
||||
|
|
@ -167,7 +197,16 @@ export async function restoreBackup(backupId, opts = {}) {
|
|||
// Files implement CREATED are absent from the backup by definition, so they
|
||||
// survive the restore. Report them — a half-restored target is only dangerous
|
||||
// when it is also silent.
|
||||
return { restored, failed, createdNotRemoved: manifest.created, legacy: resolved.legacy };
|
||||
return {
|
||||
restored,
|
||||
failed,
|
||||
createdNotRemoved: manifest.created,
|
||||
legacy: resolved.legacy,
|
||||
gate: scope.gate,
|
||||
requiresApproval: scope.requiresApproval,
|
||||
disclosures: scope.disclosures,
|
||||
refused: [],
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@
|
|||
import { resolve, sep } from 'node:path';
|
||||
import { readFile, writeFile } from 'node:fs/promises';
|
||||
import { writeOutputFile } from './lib/write-output.mjs';
|
||||
import { evaluateWriteTargets } from './lib/write-scope.mjs';
|
||||
import { requireTargetDir } from './lib/require-target-dir.mjs';
|
||||
import { envelope } from './lib/output.mjs';
|
||||
import { discoverConfigFiles, discoverConfigFilesMulti, discoverFullMachinePaths } from './lib/file-discovery.mjs';
|
||||
|
|
@ -42,6 +43,7 @@ const ARG_SPEC = {
|
|||
boolean: [
|
||||
'--json', '--raw', '--global', '--full-machine', '--no-suppress',
|
||||
'--include-fixtures', '--exclude-cache', '--no-exclude-cache', '--save-baseline',
|
||||
'--approve-scope',
|
||||
],
|
||||
value: ['--output-file', '--context-window', '--baseline'],
|
||||
};
|
||||
|
|
@ -241,6 +243,7 @@ async function main() {
|
|||
let targetPath = '.';
|
||||
let outputFile = null;
|
||||
let saveBaseline = false;
|
||||
let approveScope = false;
|
||||
let baselinePath = null;
|
||||
let contextWindow = null;
|
||||
|
||||
|
|
@ -249,6 +252,8 @@ async function main() {
|
|||
outputFile = args[++i];
|
||||
} else if (args[i] === '--context-window' && args[i + 1]) {
|
||||
contextWindow = args[++i];
|
||||
} else if (args[i] === '--approve-scope') {
|
||||
approveScope = true;
|
||||
} else if (args[i] === '--save-baseline') {
|
||||
saveBaseline = true;
|
||||
} else if (args[i] === '--baseline' && args[i + 1]) {
|
||||
|
|
@ -314,10 +319,26 @@ async function main() {
|
|||
|
||||
if (saveBaseline) {
|
||||
const bPath = baselinePath || resolve(targetPath, '.config-audit-baseline.json');
|
||||
// Always save baselines as raw v5.0.0-shape envelope so future humanizer
|
||||
// changes don't trigger false-positive drift findings.
|
||||
await writeFile(bPath, JSON.stringify(result, null, 2), 'utf-8');
|
||||
process.stderr.write(`Baseline saved to ${bPath}\n`);
|
||||
|
||||
// Q1 — the scope gate, in code. This write was carried in the plan text as
|
||||
// one of the plugin's own artifacts, legitimately exempt — measured false: the
|
||||
// default path is derived from the SCAN TARGET, not from a plugin root, so
|
||||
// `--global --save-baseline` lands `~/.claude/.config-audit-baseline.json`,
|
||||
// which is `user-scope` / `require-ok`. `lib/baseline.mjs` is the genuinely
|
||||
// exempt one — it writes only under `~/.config-audit/baselines`.
|
||||
const scope = evaluateWriteTargets([bPath], process.cwd());
|
||||
|
||||
if (scope.requiresApproval && !approveScope) {
|
||||
for (const line of scope.disclosures) process.stderr.write(`\n${line}\n`);
|
||||
process.stderr.write(
|
||||
`Baseline NOT saved to ${bPath} — re-run with --approve-scope to write it.\n`,
|
||||
);
|
||||
} else {
|
||||
// Always save baselines as raw v5.0.0-shape envelope so future humanizer
|
||||
// changes don't trigger false-positive drift findings.
|
||||
await writeFile(bPath, JSON.stringify(result, null, 2), 'utf-8');
|
||||
process.stderr.write(`Baseline saved to ${bPath}\n`);
|
||||
}
|
||||
}
|
||||
|
||||
// Summary
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue