fix(review): the engine of a payload is read from an own property

Review finding 3799ea6c; the same lookup in engine-findings.mjs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-13 14:17:07 +02:00
commit d492abb5e9
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
3 changed files with 32 additions and 2 deletions

View file

@ -224,7 +224,7 @@ export function ingest(reviewerPayloads) {
// field a reviewer wrote on its own finding (decision 13, operator 12.10):
// an engine payload's findings get `engine` from its `reviewer`, every other
// payload's findings lose any `engine` they carry.
const eng = ENGINE_OF_PAYLOAD[payload.reviewer];
const eng = Object.hasOwn(ENGINE_OF_PAYLOAD, payload.reviewer) ? ENGINE_OF_PAYLOAD[payload.reviewer] : null;
for (const f of payload.findings) {
const { engine: _claimed, ...rest } = f;
findings.push({

View file

@ -90,7 +90,7 @@ export const IMPROVEMENT_CATEGORIES = Object.freeze(new Set(['simplification', '
*/
export function toReviewerPayload(input, opts = {}) {
const errors = [];
const spec = ENGINES[input?.engine];
const spec = Object.hasOwn(ENGINES, String(input?.engine ?? '')) ? ENGINES[input.engine] : null;
if (!spec) return { ok: false, errors: [`engine must be one of ${Object.keys(ENGINES).join('|')}, got ${JSON.stringify(input?.engine)}`] };
if (!Array.isArray(input.findings)) return { ok: false, errors: ['findings must be an array'] };
if (opts.scopeFiles && opts.scopeFiles.length === 0) {

View file

@ -20,6 +20,7 @@ import { join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
import * as m from '../../lib/review/coordinator-contract.mjs';
import { extractFindingsBlock } from '../../lib/review/findings-schema.mjs';
import { toReviewerPayload } from '../../lib/review/engine-findings.mjs';
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
const GATE = join(ROOT, 'lib', 'review', 'verdict-gate.mjs');
@ -535,3 +536,32 @@ test('decision 13 — the prose states the tie rule (coordinator Pass 1, Handove
assert.match(h6, /takes? `engine` from the payload a finding arrived in/);
assert.match(h6, /on a severity tie[^.]*a reviewer-agent finding survives[^.]*over an engine finding/i);
});
// ---- The engine of a payload is read from an own property (review 3799ea6c) --
//
// A payload `reviewer` named after an Object.prototype member must not pick up
// an inherited function or object as its engine: such a payload is a reviewer
// payload, its findings carry no `engine`, and the contract's output is plain
// JSON. The same lookup in engine-findings.mjs refuses an engine named after
// one with the existing "engine must be one of" error.
for (const name of ['toString', 'constructor', 'hasOwnProperty', 'valueOf', '__proto__']) {
test(`own property — a payload reviewer named ${name} gives findings no engine, and runContract's output survives a JSON round trip`, () => {
const { engine: _e, ...finding } = codeReviewDefect();
const ingested = m.ingest(payload(name, [finding]));
assert.equal(ingested.findings.length, 1);
assert.equal('engine' in ingested.findings[0], false,
`a payload reviewer named ${name} is not an engine payload`);
const result = m.runContract(payload(name, [finding]));
assert.deepEqual(JSON.parse(JSON.stringify(result)), result,
'runContract output is plain JSON (no inherited function or object as engine)');
assertAllOutputLegal(result);
});
}
test('own property — toReviewerPayload refuses engine toString with the "engine must be one of" error', () => {
const r = toReviewerPayload({ engine: 'toString', findings: [] });
assert.equal(r.ok, false, 'toString is not an engine');
assert.equal(r.errors.length, 1);
assert.match(r.errors[0], /^engine must be one of code-review\|security-review, got "toString"$/);
});