feat(review): one reader of the brief's threat model (decision 164, 218)
New lib/review/threat-model.mjs, the one reader both gates use:
readThreatModel(briefText) -> {present, inside, outside, problems} through
brief-validator's parseThreatModel; usable(briefPath, repo) = present, well
formed, and the intent approval current by the two checks /trekplan Phase 1
runs (checkIntentApprovalContent, then verifyLog kind intent with the
hash's sha256: stripped and the subject normalized), else not usable with
the reason in `why`; backedLine(finding, tm) = the Outside: text the
finding's threat_model_ref equals exactly, else null. Only a security
finding not from an engine can be outside (218): a finding with an
`engine`, or carrying the engines' key SECURITY_REVIEW_VULNERABILITY, is
never backed.
Part 2, step 2.2 of the E plan.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
74f75dcb7d
commit
bec4500b8f
2 changed files with 329 additions and 0 deletions
110
lib/review/threat-model.mjs
Normal file
110
lib/review/threat-model.mjs
Normal file
|
|
@ -0,0 +1,110 @@
|
|||
// lib/review/threat-model.mjs
|
||||
// One reader of the brief's optional `## Threat Model` (E, decisions 164, 218),
|
||||
// for both gates: the plan gate (lib/validators/plan-gate.mjs) and the review's
|
||||
// verdict gate (lib/review/verdict-gate.mjs).
|
||||
//
|
||||
// The section is `- Inside: <text>` / `- Outside: <text>` bullets
|
||||
// (brief-validator.mjs `parseThreatModel`, the same reader the validator uses).
|
||||
// Voyage's own security findings (plan-critic, scope-guardian, the code
|
||||
// reviewer's part for command texts) block inside it; one that quotes an
|
||||
// `Outside:` line's text exactly in `threat_model_ref` is backed by that line
|
||||
// and becomes a listed known issue instead of a blocker. Engine findings
|
||||
// (Claude Code's `/security-review`, Claude Security) block as before: only a
|
||||
// security finding not from an engine can be outside (218).
|
||||
//
|
||||
// readThreatModel(briefText) -> {present, inside, outside, problems}
|
||||
// usable(briefPath, repo) -> {usable, why, threatModel}
|
||||
// usable = the section is present and well formed, AND the intent approval
|
||||
// is current: the two checks /trekplan Phase 1 runs (the marker,
|
||||
// checkIntentApprovalContent; and the approval log, verifyLog kind
|
||||
// `intent` with the hash's `sha256:` stripped and the subject normalized).
|
||||
// Otherwise every security finding counts, and `why` says why.
|
||||
// backedLine(finding, tm) -> the `Outside:` text the finding's
|
||||
// `threat_model_ref` equals exactly, else null.
|
||||
//
|
||||
// Lifting a finding back in is the operator's: change (or remove) that
|
||||
// `Outside:` line and approve the intent again. Both gates re-read the brief on
|
||||
// every run, so a finding whose line is gone counts in full.
|
||||
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { parseDocument } from '../util/frontmatter.mjs';
|
||||
import { verifyLog, normalizeHex, normalizeSubject, resolveRepo } from '../util/approval-log.mjs';
|
||||
import { checkIntentApprovalContent } from '../validators/intent-approval.mjs';
|
||||
import { parseThreatModel } from '../validators/brief-validator.mjs';
|
||||
import { isSecurityKey, keysOf } from './plan-review-dedup.mjs';
|
||||
|
||||
/** The engines' catalogue key (/security-review, Claude Security): never Voyage's own. */
|
||||
export const ENGINES_SECURITY_KEY = 'SECURITY_REVIEW_VULNERABILITY';
|
||||
|
||||
const EMPTY = Object.freeze({ present: false, inside: [], outside: [], problems: [] });
|
||||
|
||||
/**
|
||||
* Read the brief's `## Threat Model`.
|
||||
* @param {string} briefText - the whole brief (frontmatter optional)
|
||||
* @returns {{present: boolean, inside: string[], outside: string[], problems: string[]}}
|
||||
*/
|
||||
export function readThreatModel(briefText) {
|
||||
const text = typeof briefText === 'string' ? briefText : '';
|
||||
const doc = parseDocument(text);
|
||||
const body = doc.parsed?.body ?? text;
|
||||
return parseThreatModel(body);
|
||||
}
|
||||
|
||||
/**
|
||||
* May the brief's threat model be applied? Present, well formed, and the
|
||||
* intent approval current (marker and log). Otherwise every security finding
|
||||
* counts, and `why` says why.
|
||||
* @param {string} briefPath
|
||||
* @param {string} [repo] - the repo whose approval log to read (default: the git top level of the working directory)
|
||||
* @returns {{usable: boolean, why: string|null, threatModel: {present: boolean, inside: string[], outside: string[], problems: string[]}}}
|
||||
*/
|
||||
export function usable(briefPath, repo) {
|
||||
const no = (why, threatModel = EMPTY) => ({ usable: false, why, threatModel });
|
||||
if (typeof briefPath !== 'string' || briefPath === '') return no('no brief path was given');
|
||||
let text;
|
||||
try { text = readFileSync(briefPath, 'utf8'); } catch (e) { return no(`cannot read the brief ${briefPath}: ${e.message}`); }
|
||||
const tm = readThreatModel(text);
|
||||
if (!tm.present) return no('the brief has no ## Threat Model', tm);
|
||||
if (tm.problems.length > 0) return no(`the brief's ## Threat Model is malformed: ${tm.problems.join('; ')}`, tm);
|
||||
const marker = checkIntentApprovalContent(text);
|
||||
if (!marker.valid) {
|
||||
return no(`the intent approval is not current (${marker.errors.map((e) => e.code).join(', ')})`, tm);
|
||||
}
|
||||
const sha = normalizeHex(marker.parsed?.frontmatter?.intent_approved_hash);
|
||||
if (!sha) return no('the brief carries no intent approval hash', tm);
|
||||
let log;
|
||||
try {
|
||||
const root = resolveRepo(repo);
|
||||
log = verifyLog({ repo: root, kind: 'intent', subject: normalizeSubject(root, briefPath), sha });
|
||||
} catch (e) {
|
||||
return no(`the approval log could not be read: ${e.message}`, tm);
|
||||
}
|
||||
if (log.exit !== 0) {
|
||||
return no(`the intent approval is not in the approval log (${log.status}${log.detail ? `: ${log.detail}` : ''})`, tm);
|
||||
}
|
||||
return { usable: true, why: null, threatModel: tm };
|
||||
}
|
||||
|
||||
/** A security finding that is Voyage's own: a security key, no engine, not the engines' key (218). */
|
||||
export function isOwnSecurityFinding(f) {
|
||||
if (!f || typeof f !== 'object') return false;
|
||||
if (typeof f.engine === 'string' && f.engine.trim() !== '') return false;
|
||||
const keys = keysOf(f);
|
||||
if (keys.some((k) => k.trim().toUpperCase() === ENGINES_SECURITY_KEY)) return false;
|
||||
return keys.some(isSecurityKey);
|
||||
}
|
||||
|
||||
/**
|
||||
* The `Outside:` text a finding is backed by: its `threat_model_ref` equals it
|
||||
* exactly. Only Voyage's own security findings can be backed.
|
||||
* @param {object} finding
|
||||
* @param {{present?: boolean, outside?: string[]}|null} tm - from readThreatModel / usable
|
||||
* @returns {string|null}
|
||||
*/
|
||||
export function backedLine(finding, tm) {
|
||||
if (!tm || tm.present !== true || !Array.isArray(tm.outside)) return null;
|
||||
if (!isOwnSecurityFinding(finding)) return null;
|
||||
const ref = finding.threat_model_ref;
|
||||
if (typeof ref !== 'string' || ref === '') return null;
|
||||
return tm.outside.find((line) => line === ref) ?? null;
|
||||
}
|
||||
219
tests/lib/threat-model.test.mjs
Normal file
219
tests/lib/threat-model.test.mjs
Normal file
|
|
@ -0,0 +1,219 @@
|
|||
// tests/lib/threat-model.test.mjs
|
||||
// E 2.2 — one reader of the brief's `## Threat Model` for both gates
|
||||
// (decisions 164, 218): readThreatModel reads the section; usable says whether
|
||||
// it may be applied (present, and the intent approval current: the marker
|
||||
// check and the approval log, the two checks /trekplan Phase 1 runs), and why
|
||||
// not; backedLine finds the `Outside:` line a finding quotes exactly. Only a
|
||||
// security finding not from an engine can be outside.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const ROOT = join(HERE, '..', '..');
|
||||
const load = () => import(join(ROOT, 'lib', 'review', 'threat-model.mjs'));
|
||||
const approval = () => import(join(ROOT, 'lib', 'validators', 'intent-approval.mjs'));
|
||||
const log = () => import(join(ROOT, 'lib', 'util', 'approval-log.mjs'));
|
||||
|
||||
const OUTSIDE = "a local user who can already edit the repo's hooks";
|
||||
const INSIDE = 'a crafted file name in a pull request reaches the shell';
|
||||
const TM = `## Threat Model\n\n- Inside: ${INSIDE}\n- Outside: ${OUTSIDE}\n`;
|
||||
|
||||
const brief = (threat = TM, type = 'trekbrief') => `---
|
||||
type: ${type}
|
||||
brief_version: "2.2"
|
||||
created: 2026-11-08
|
||||
task: "Add an upload hook"
|
||||
slug: upload-hook
|
||||
project_dir: .claude/projects/2026-11-08-upload-hook/
|
||||
research_topics: 0
|
||||
research_status: complete
|
||||
framing: new-direction
|
||||
---
|
||||
|
||||
# Task
|
||||
|
||||
## TL;DR
|
||||
|
||||
An upload hook.
|
||||
|
||||
## Intent
|
||||
|
||||
Uploads arrive from strangers.
|
||||
|
||||
## Goal
|
||||
|
||||
A hook that checks each upload.
|
||||
|
||||
${threat}
|
||||
## Success Criteria
|
||||
|
||||
- It checks.
|
||||
`;
|
||||
|
||||
const own = (extra = {}) => ({ id: 'f1', rule_key: 'SECURITY_INJECTION', severity: 'blocker', text: 'shell built from a name', threat_model_ref: OUTSIDE, ...extra });
|
||||
|
||||
// ---- readThreatModel ----
|
||||
|
||||
test('readThreatModel: present with its Inside and Outside texts; absent is not present', async () => {
|
||||
const { readThreatModel } = await load();
|
||||
const tm = readThreatModel(brief());
|
||||
assert.equal(tm.present, true);
|
||||
assert.deepEqual(tm.inside, [INSIDE]);
|
||||
assert.deepEqual(tm.outside, [OUTSIDE]);
|
||||
const none = readThreatModel(brief(''));
|
||||
assert.equal(none.present, false);
|
||||
assert.deepEqual(none.inside, []);
|
||||
assert.deepEqual(none.outside, []);
|
||||
});
|
||||
|
||||
test('readThreatModel: a malformed section is present with its problems; text without frontmatter still reads', async () => {
|
||||
const { readThreatModel } = await load();
|
||||
const bad = readThreatModel(brief('## Threat Model\n\n- Scope: all\n'));
|
||||
assert.equal(bad.present, true);
|
||||
assert.ok(bad.problems.length > 0, 'the problem is named');
|
||||
const bare = readThreatModel(`# Brief\n\n${TM}`);
|
||||
assert.deepEqual(bare.outside, [OUTSIDE]);
|
||||
assert.deepEqual(readThreatModel(undefined).outside, []);
|
||||
});
|
||||
|
||||
// ---- usable ----
|
||||
|
||||
async function approvedRepo(text) {
|
||||
const { stampIntentApproval } = await approval();
|
||||
const { appendApproval, normalizeHex, normalizeSubject, resolveRepo } = await log();
|
||||
const dir = mkdtempSync(join(tmpdir(), 'threat-model-'));
|
||||
const p = join(dir, 'brief.md');
|
||||
const s = stampIntentApproval(text, new Date('2026-11-08T08:00:00Z'));
|
||||
writeFileSync(p, s.stamped ? s.text : text);
|
||||
const repo = resolveRepo(dir);
|
||||
const logIt = (hash = s.hash) => appendApproval({ repo, kind: 'intent', subject: normalizeSubject(repo, p), sha: normalizeHex(hash) });
|
||||
return { dir, p, s, logIt };
|
||||
}
|
||||
|
||||
test('usable: present, stamped and logged → usable', async () => {
|
||||
const { usable } = await load();
|
||||
const { dir, p, logIt } = await approvedRepo(brief());
|
||||
try {
|
||||
assert.equal(logIt().appended, true);
|
||||
const u = usable(p, dir);
|
||||
assert.equal(u.usable, true, u.why);
|
||||
assert.equal(u.why, null);
|
||||
assert.deepEqual(u.threatModel.outside, [OUTSIDE]);
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test('usable: no ## Threat Model → not usable, and it says why', async () => {
|
||||
const { usable } = await load();
|
||||
const { dir, p, logIt } = await approvedRepo(brief(''));
|
||||
try {
|
||||
logIt();
|
||||
const u = usable(p, dir);
|
||||
assert.equal(u.usable, false);
|
||||
assert.match(u.why, /no ## Threat Model/);
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test('usable: the threat model edited after the approval → not usable (stale)', async () => {
|
||||
const { usable } = await load();
|
||||
const { dir, p, s, logIt } = await approvedRepo(brief());
|
||||
try {
|
||||
logIt();
|
||||
writeFileSync(p, s.text.replace(OUTSIDE, 'anyone at all'));
|
||||
const u = usable(p, dir);
|
||||
assert.equal(u.usable, false);
|
||||
assert.match(u.why, /BRIEF_INTENT_APPROVAL_STALE/);
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test('usable: stamped but not in the approval log → not usable (not-logged)', async () => {
|
||||
const { usable } = await load();
|
||||
const { dir, p } = await approvedRepo(brief());
|
||||
try {
|
||||
const u = usable(p, dir);
|
||||
assert.equal(u.usable, false);
|
||||
assert.match(u.why, /not-logged/);
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test('usable: the log covers other intent text → not usable (mismatch)', async () => {
|
||||
const { usable } = await load();
|
||||
const { dir, p, logIt } = await approvedRepo(brief());
|
||||
try {
|
||||
logIt(`sha256:${'a'.repeat(64)}`);
|
||||
const u = usable(p, dir);
|
||||
assert.equal(u.usable, false);
|
||||
assert.match(u.why, /mismatch/);
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
test('usable: unapproved, malformed, unreadable or a trekreview brief → not usable', async () => {
|
||||
const { usable } = await load();
|
||||
const dir = mkdtempSync(join(tmpdir(), 'threat-model-'));
|
||||
try {
|
||||
const p = join(dir, 'brief.md');
|
||||
writeFileSync(p, brief());
|
||||
let u = usable(p, dir);
|
||||
assert.equal(u.usable, false);
|
||||
assert.match(u.why, /BRIEF_INTENT_NOT_APPROVED/);
|
||||
writeFileSync(p, brief('## Threat Model\n\n- Scope: all\n'));
|
||||
u = usable(p, dir);
|
||||
assert.equal(u.usable, false);
|
||||
assert.match(u.why, /malformed/);
|
||||
u = usable(join(dir, 'missing.md'), dir);
|
||||
assert.equal(u.usable, false);
|
||||
assert.match(u.why, /cannot read/);
|
||||
writeFileSync(p, brief(TM, 'trekreview'));
|
||||
u = usable(p, dir);
|
||||
assert.equal(u.usable, false, 'a trekreview brief carries no approved intent of its own');
|
||||
assert.equal(usable(undefined, dir).usable, false);
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
// ---- backedLine ----
|
||||
|
||||
test('backedLine: a Voyage security finding quoting an Outside line exactly → that line', async () => {
|
||||
const { backedLine, readThreatModel } = await load();
|
||||
const tm = readThreatModel(brief());
|
||||
assert.equal(backedLine(own(), tm), OUTSIDE);
|
||||
assert.equal(backedLine(own({ rule_key: 'security/shell', rule_keys: ['security/shell', 'SECURITY_INJECTION'] }), tm), OUTSIDE,
|
||||
'a merged finding carrying a security key counts as a security finding');
|
||||
});
|
||||
|
||||
test('backedLine: not exact, an Inside line, no ref, or no threat model → null', async () => {
|
||||
const { backedLine, readThreatModel } = await load();
|
||||
const tm = readThreatModel(brief());
|
||||
assert.equal(backedLine(own({ threat_model_ref: INSIDE }), tm), null);
|
||||
assert.equal(backedLine(own({ threat_model_ref: `${OUTSIDE} ` }), tm), null);
|
||||
assert.equal(backedLine(own({ threat_model_ref: OUTSIDE.toUpperCase() }), tm), null);
|
||||
assert.equal(backedLine(own({ threat_model_ref: `Outside: ${OUTSIDE}` }), tm), null);
|
||||
assert.equal(backedLine(own({ threat_model_ref: undefined }), tm), null);
|
||||
assert.equal(backedLine(own(), readThreatModel(brief(''))), null);
|
||||
assert.equal(backedLine(own(), null), null);
|
||||
assert.equal(backedLine(null, tm), null);
|
||||
});
|
||||
|
||||
test('backedLine: a finding whose Outside line is gone counts in full (the brief is re-read)', async () => {
|
||||
const { backedLine, readThreatModel } = await load();
|
||||
const changed = readThreatModel(brief(`## Threat Model\n\n- Inside: ${INSIDE}\n- Outside: anyone at all\n`));
|
||||
assert.equal(backedLine(own(), changed), null);
|
||||
});
|
||||
|
||||
test('backedLine: only a security finding not from an engine can be outside (218)', async () => {
|
||||
const { backedLine, readThreatModel, isOwnSecurityFinding } = await load();
|
||||
const tm = readThreatModel(brief());
|
||||
assert.equal(isOwnSecurityFinding(own()), true);
|
||||
const engine = own({ engine: 'security-review' });
|
||||
assert.equal(isOwnSecurityFinding(engine), false);
|
||||
assert.equal(backedLine(engine, tm), null, 'an engine finding blocks as today');
|
||||
const enginesKey = own({ rule_key: 'SECURITY_REVIEW_VULNERABILITY' });
|
||||
assert.equal(backedLine(enginesKey, tm), null, "the engines' key is never Voyage's own");
|
||||
const notSecurity = own({ rule_key: 'MISSING_TEST' });
|
||||
assert.equal(isOwnSecurityFinding(notSecurity), false);
|
||||
assert.equal(backedLine(notSecurity, tm), null, 'only a security finding can be outside');
|
||||
assert.equal(backedLine(own({ rule_key: 'security_injection' }), tm), OUTSIDE, 'key case is ignored, as the gate reads it');
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue