feat(review): the critic's own security key SECURITY_RISK (decision 167)

lib/review/rule-catalogue.mjs gains SECURITY_RISK (BLOCKER, category
security, memory route rule): Voyage's own security key for a finding by
its own reviewers that SECURITY_INJECTION does not cover.
SECURITY_REVIEW_VULNERABILITY is described as the engines' key only.
plan-critic and scope-guardian name SECURITY_INJECTION or SECURITY_RISK
and no longer the engines' key.

The count moves from 17 to 18 in every pin: rule-catalogue.test,
claude-md-scope [CAT] (which now also refuses a stale 17), doc-consistency,
review-prose-pins, docs/architecture.md and the two reviewer agents.
critic-severity-rules pins three security keys; its "every security key
named" check leaves out the engines' key, and a new pin says the critic
never names it.

Existing tests changed for the planned rule: the four count pins above
(17 -> 18), and critic-severity-rules' two-key test (now three keys).
tests/lib/engine-findings.test.mjs: a comment naming the "(17 keys)" pin
now says 18.

Part 2, step 2.3 of the E plan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-11-08 08:55:34 +01:00
commit 5cb2918baa
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
12 changed files with 67 additions and 31 deletions

View file

@ -42,19 +42,19 @@ test('[CAT] the catalogue holds CLAUDE_MD_DEVIATION and CLAUDE_MD_STALE at MINOR
);
});
test('[CAT] the catalogue has 17 keys and docs/architecture.md and both reviewer agents say 17-key', () => {
assert.equal(RULE_CATALOGUE.length, 17);
assert.equal(RULE_KEYS.size, 17);
test('[CAT] the catalogue has 18 keys and docs/architecture.md and both reviewer agents say 18-key', () => {
assert.equal(RULE_CATALOGUE.length, 18);
assert.equal(RULE_KEYS.size, 18);
const header = read('lib/review/rule-catalogue.mjs').split('\n').slice(0, 10).join('\n');
assert.match(header, /\b17 rule keys\b/, 'the catalogue header counts 17 rule keys');
assert.match(header, /\b18 rule keys\b/, 'the catalogue header counts 18 rule keys');
const arch = read('docs/architecture.md');
assert.match(arch, /rule catalogue \(17 keys\)/);
assert.match(arch, /the 17-key rule catalogue/);
assert.doesNotMatch(arch, /\b(12|15)-key rule catalogue|rule catalogue \((12|15) keys\)/);
assert.match(arch, /rule catalogue \(18 keys\)/);
assert.match(arch, /the 18-key rule catalogue/);
assert.doesNotMatch(arch, /\b(12|15|17)-key rule catalogue|rule catalogue \((12|15|17) keys\)/);
for (const agent of ['agents/code-correctness-reviewer.md', 'agents/brief-conformance-reviewer.md']) {
const text = read(agent);
assert.match(text, /the 17-key catalogue/, `${agent} says 17-key`);
assert.doesNotMatch(text, /\b(12|15)-key catalogue/, `${agent} no longer says 12-key`);
assert.match(text, /the 18-key catalogue/, `${agent} says 18-key`);
assert.doesNotMatch(text, /\b(12|15|17)-key catalogue/, `${agent} no longer says 17-key`);
}
});

View file

@ -2,7 +2,8 @@
// The two plan reviewers (plan-critic, scope-guardian) must state rule B word for
// word as lib/review/critic-diff.mjs exports it, keep the 2026-10-08 rule that a
// test which cannot fail on a wrong implementation is major, tag security
// findings with the catalogue's security keys, and carry the round-2 fields
// findings with Voyage's own security keys (never the engines' key, decision
// 167, E 2.3), and carry the round-2 fields
// (`previous`, `line_end`, `lowered_from`, `caught_by`) in their json template.
// The name is the brief's.
@ -30,9 +31,12 @@ function jsonTemplate(text, rel) {
return JSON.parse(m[1]);
}
test('the catalogue has exactly two security keys: SECURITY_INJECTION and SECURITY_REVIEW_VULNERABILITY', () => {
// The engines' key (/security-review, Claude Security): never the critic's (decision 167).
const ENGINES_KEY = 'SECURITY_REVIEW_VULNERABILITY';
test('the catalogue has exactly three security keys: SECURITY_INJECTION, SECURITY_REVIEW_VULNERABILITY and SECURITY_RISK', () => {
const keys = RULE_CATALOGUE.filter(r => r.category === 'security').map(r => r.rule_key).sort();
assert.deepEqual(keys, ['SECURITY_INJECTION', 'SECURITY_REVIEW_VULNERABILITY']);
assert.deepEqual(keys, ['SECURITY_INJECTION', 'SECURITY_REVIEW_VULNERABILITY', 'SECURITY_RISK']);
});
for (const rel of AGENTS) {
@ -56,11 +60,15 @@ for (const rel of AGENTS) {
test(`${rel}: a security finding uses the catalogue's security rule_key`, () => {
const t = norm(read(rel));
const keys = RULE_CATALOGUE.filter(r => r.category === 'security').map(r => r.rule_key);
const keys = RULE_CATALOGUE.filter(r => r.category === 'security' && r.rule_key !== ENGINES_KEY).map(r => r.rule_key);
for (const k of keys) assert.ok(t.includes(`\`${k}\``), `${rel} must name \`${k}\``);
assert.ok(t.includes("A security finding uses the catalogue's security `rule_key`"));
});
test(`${rel}: the critic never names the engines' key (decision 167)`, () => {
assert.ok(!read(rel).includes(ENGINES_KEY), `${rel} must not name ${ENGINES_KEY}: it is the engines' key only`);
});
test(`${rel}: [STP] rule_key names the class, never the instance (the stop rule's class, decision 217)`, () => {
const t = norm(read(rel));
assert.ok(t.includes('`rule_key` names the class, never the instance'), `${rel} must say rule_key names the class, never the instance`);

View file

@ -240,12 +240,12 @@ test('HANDOVER-CONTRACTS.md Handover 7 § Lifecycle names --cleanup and produced
// file" (commands/trekcontinue.md exists, so that loop already asserts the
// /trekcontinue reference). Single-command existence pin → redundant.
test('rule-catalogue has exactly 17 entries', async () => {
test('rule-catalogue has exactly 18 entries', async () => {
const mod = await import('../../lib/review/rule-catalogue.mjs');
assert.strictEqual(
mod.RULE_CATALOGUE.length,
17,
'lib/review/rule-catalogue.mjs RULE_CATALOGUE size invariant: must be 17 (12 v1.0 baseline + 2 engine keys, ordre 5, + CODE_REVIEW_IMPROVEMENT, lekerepo trial 2, + CLAUDE_MD_DEVIATION and CLAUDE_MD_STALE, decision 98)',
18,
'lib/review/rule-catalogue.mjs RULE_CATALOGUE size invariant: must be 18 (12 v1.0 baseline + 2 engine keys, ordre 5, + CODE_REVIEW_IMPROVEMENT, lekerepo trial 2, + CLAUDE_MD_DEVIATION and CLAUDE_MD_STALE, decision 98, + SECURITY_RISK, E 2.3)',
);
});

View file

@ -170,7 +170,7 @@ test('engine-findings — /trekreview asks the transcription for the engine\'s f
assert.match(p, /never\s+write\s+one\s+the\s+engine\s+did\s+not\s+give/);
});
// The docs/architecture.md "rule catalogue (17 keys)" pin of this test lives in
// The docs/architecture.md "rule catalogue (18 keys)" pin of this test lives in
// tests/lib/claude-md-scope.test.mjs ([CAT]).
test('engine-findings — the docs: the engine rule-key count and the Declined-to-Judge rule match the code', () => {
const read = (p) => readFileSync(join(ROOT, p), 'utf8');

View file

@ -285,7 +285,7 @@ function subsection(text, heading) {
return text.slice(start, next === -1 ? undefined : start + heading.length + next);
}
test('[CMD] agents/code-correctness-reviewer.md names the instruction-file dimension, the 8-dimension heading and 17-key', () => {
test('[CMD] agents/code-correctness-reviewer.md names the instruction-file dimension, the 8-dimension heading and 18-key', () => {
const text = read(CC_AGENT);
const fm = /^---\n([\s\S]*?)\n---\n/.exec(text)?.[1] ?? '';
assert.match(flat(fm), /across 8 dimensions/, `the description says 8 dimensions: ${flat(fm)}`);
@ -303,7 +303,7 @@ test('[CMD] agents/code-correctness-reviewer.md names the instruction-file dimen
const rows = text.split('\n').filter((l) => l.startsWith('| Instruction files |'));
assert.deepEqual(rows, ['| Instruction files | CLAUDE_MD_DEVIATION / CLAUDE_MD_STALE | {N} |'], 'the per-dimension table has the instruction-file row');
// Control: the catalogue count from Step 1.
assert.ok(text.includes('the 17-key catalogue'), 'Control: the agent says the 17-key catalogue');
assert.ok(text.includes('the 18-key catalogue'), 'Control: the agent says the 18-key catalogue');
});
test('[CMD] every text that counts the code-correctness dimensions says 8, and /trekreview Phase 5 passes the instruction set', () => {

View file

@ -39,9 +39,23 @@ test('RULE_CATALOGUE — all category values within enum', () => {
}
});
test('RULE_KEYS.size === RULE_CATALOGUE.length (== 17) — pinned by doc-consistency', () => {
test('RULE_KEYS.size === RULE_CATALOGUE.length (== 18) — pinned by doc-consistency', () => {
assert.equal(RULE_KEYS.size, RULE_CATALOGUE.length);
assert.equal(RULE_CATALOGUE.length, 17);
assert.equal(RULE_CATALOGUE.length, 18);
});
test('SECURITY_RISK is Voyage\'s own security key; SECURITY_REVIEW_VULNERABILITY is the engines\' key only (E 2.3, decision 167)', () => {
const own = getRule('SECURITY_RISK');
assert.ok(own, 'SECURITY_RISK is in the catalogue');
assert.equal(own.severity, 'BLOCKER');
assert.equal(own.category, 'security');
assert.equal(own.memory_route, 'rule');
assert.match(own.description, /Voyage's own/);
const engines = getRule('SECURITY_REVIEW_VULNERABILITY');
assert.equal(engines.severity, 'BLOCKER');
assert.equal(engines.category, 'security');
assert.match(engines.description, /engines' key only/);
assert.match(engines.description, /SECURITY_RISK/);
});
test('CLAUDE_MD_DEVIATION and CLAUDE_MD_STALE are MINOR', () => {