From a3bbfd8aa418bfb5f77cfc200e11c06b595d1bd9 Mon Sep 17 00:00:00 2001 From: StepenkoAnatoli Date: Wed, 14 Oct 2026 22:38:55 +0300 Subject: [PATCH] fix: distinguish invalid snapshot ages from stale evidence what changed Researcher-release validates snapshot-max-age-ms before resolving or reading evidence. The release and snapshot facades classify a non-finite, negative or nonnumeric allowance as INPUT-SNAPSHOT-MAX-AGE / INCOMPLETE. Added CLI and facade regressions and updated the architecture map, changelog and inventory. why Fixes #343. NaN or a negative allowance was reported as stale snapshot evidence; a valid package could instead pass if it had no snapshot to judge. Input errors now remain input errors, while a finite non-negative allowance retains real freshness checks. what it touched research-kit/bin/researcher-release.mjs, research-kit/lib/release-validator.mjs, research-kit/test/release-validator.test.mjs, docs/ARCHITECTURE.md, CHANGELOG.md and research-kit/README.md. The error uses the existing input-error reduction, with no new flag, schema or publication behavior. what you verified verified: the added regressions were red on the base (14 passed, 2 failed) and the focused release-validator group passed after the fix (16 passed, 0 failed). expected: the strengthened valid-package fixture and full offline suite will run through the normal commit gate; final results accompany the PR. what you got wrong and fixed mistake: the initial facade fixture named an artifact in its registry but did not write that record. where: research-kit/test/release-validator.test.mjs, invalid snapshot-age facade regression. impact: the initial fixture could not prove that the same package passes with a valid allowance; no shipped validator behavior was affected. cause: fixture setup wrote the registry and role roster but omitted the artifact file. fix: wrote the valid envelope and added a baseline PASS assertion before exercising invalid allowances. verified: the initial focused group passed 16 tests; expected: the normal commit gate covers the strengthened final fixture and full suite. --- CHANGELOG.md | 4 ++ docs/ARCHITECTURE.md | 6 +- research-kit/README.md | 2 +- research-kit/bin/researcher-release.mjs | 8 ++- research-kit/lib/release-validator.mjs | 10 ++- research-kit/test/release-validator.test.mjs | 66 ++++++++++++++++++++ 6 files changed, 92 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00e0a471..bf7c64d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ Each release of Research-Kit, newest first. The version is `KIT_VERSION` in ## Unreleased +- Invalid `--snapshot-max-age-ms` values are usage errors with exit 2. Library callers + receive `INPUT-SNAPSHOT-MAX-AGE` / `INCOMPLETE` rather than a false stale-snapshot + verdict; finite non-negative allowances, including zero, retain their meaning (#343). + - ZIP archives declare UTF-8 names in both headers, so ordinary extractors preserve non-ASCII paths declared by the package manifest (#345). - Machine config saves preserve unpatched raw role and posture values, including typos diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a35be350..1180fe1e 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -287,7 +287,11 @@ environment variable at all**, so it cannot depend on a credential or a config. maps to exit code — `PASS` 0, `FAIL`/`REOPEN` 1, `INCOMPLETE` 2, `BLOCKED` 3, unknown 4 — and a **usage error exits 2 from every subcommand**, printing the usage block to stderr; its argument parser names a value flag without a value before resolving any path, -including when the next token is another flag; +including when the next token is another flag. `validate` refuses blank, non-finite or +negative `--snapshot-max-age-ms` values as usage errors before judging evidence (#343). +The `validateRelease` and `validateSnapshotEvidence` library boundaries report an invalid +age allowance as `INPUT-SNAPSHOT-MAX-AGE` / `INCOMPLETE`; `SNAPSHOT-STALE` is reserved for +an actual snapshot older than a valid allowance; `test/support-policy.test.mjs` pins that by running the CLI. It writes exactly one thing, only when asked: `fi-validate --report `), `selftest.mjs` (every requested group must match a test file before any test starts; an unmatched group is named and refused with exit 2, including beside a valid group; #332. CLI regressions isolate child report paths from the parent suite; the whole suite, queued by the harness — which **awaits** every test, diff --git a/research-kit/README.md b/research-kit/README.md index c06356d9..5119cf4f 100644 --- a/research-kit/README.md +++ b/research-kit/README.md @@ -568,7 +568,7 @@ node research-kit/bin/selftest.mjs # all of it node research-kit/bin/selftest.mjs gate hook # just these files ``` -1845 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means +1847 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means the assertions settled (ADR-0021), and each test is raced against a watchdog (`RESEARCH_KIT_TEST_TIMEOUT`, default 60s). diff --git a/research-kit/bin/researcher-release.mjs b/research-kit/bin/researcher-release.mjs index 17d34d11..8ac95b91 100644 --- a/research-kit/bin/researcher-release.mjs +++ b/research-kit/bin/researcher-release.mjs @@ -54,6 +54,12 @@ function main(argv = process.argv.slice(2)) { if (parsed.error) { console.error(`${parsed.error}\n${usage()}`); return 2; } let result; if (parsed.command === 'validate') { + const snapshotMaxAgeMs = parsed.snapshotmaxagems === undefined ? undefined : Number(parsed.snapshotmaxagems); + if (parsed.snapshotmaxagems !== undefined + && (!parsed.snapshotmaxagems.trim() || !Number.isFinite(snapshotMaxAgeMs) || snapshotMaxAgeMs < 0)) { + console.error(`--snapshot-max-age-ms must be a finite non-negative number\n${usage()}`); + return 2; + } if (!parsed.root || !parsed.package || !parsed.registry || !parsed.roles || !parsed.pointers) { console.error(usage()); return 2; @@ -78,7 +84,7 @@ function main(argv = process.argv.slice(2)) { headPath: parsed.head ? path.resolve(parsed.head) : null, genesisHash: parsed.genesishash ?? '0'.repeat(64), publicKeys, - snapshotMaxAgeMs: parsed.snapshotmaxagems === undefined ? undefined : Number(parsed.snapshotmaxagems), + snapshotMaxAgeMs, }); } else if (parsed.command === 'conform') { if (!parsed.schema) { console.error(usage()); return 2; } diff --git a/research-kit/lib/release-validator.mjs b/research-kit/lib/release-validator.mjs index 00c5fb1d..cc3c0f72 100644 --- a/research-kit/lib/release-validator.mjs +++ b/research-kit/lib/release-validator.mjs @@ -694,6 +694,10 @@ function snapshotRefValue(ref) { */ export function validateSnapshotEvidence(record, { root = null, now = Date.now(), maxAgeMs = SNAPSHOT_MAX_AGE_MS } = {}) { const errors = []; + if (!Number.isFinite(maxAgeMs) || maxAgeMs < 0) { + add(errors, 'INPUT-SNAPSHOT-MAX-AGE', 'snapshot age allowance must be a finite non-negative number', '--snapshot-max-age-ms', 'INCOMPLETE'); + return { status: resultStatus(errors), errors }; + } const meta = metaOf(record); const refs = snapshotRefs(record); if (!refs || typeof refs !== 'object' || Array.isArray(refs)) { @@ -719,7 +723,7 @@ export function validateSnapshotEvidence(record, { root = null, now = Date.now() const capturedMs = typeof capturedAt === 'string' && UTC.test(capturedAt) ? Date.parse(capturedAt) : NaN; if (!Number.isFinite(capturedMs)) add(errors, 'SNAPSHOT-TIME', `${phase} capturedAt must be UTC ISO-8601 with Z`, `${at}.capturedAt`, 'INCOMPLETE'); else if (capturedMs > now) add(errors, 'SNAPSHOT-FUTURE', `${phase} snapshot is dated in the future`, `${at}.capturedAt`); - else if (!Number.isFinite(maxAgeMs) || maxAgeMs < 0 || now - capturedMs > maxAgeMs) add(errors, 'SNAPSHOT-STALE', `${phase} path-authority snapshot is older than the allowed ${maxAgeMs}ms`, `${at}.capturedAt`); + else if (now - capturedMs > maxAgeMs) add(errors, 'SNAPSHOT-STALE', `${phase} path-authority snapshot is older than the allowed ${maxAgeMs}ms`, `${at}.capturedAt`); const fileName = ref.path ?? ref.file ?? ref.evidencePath; if (fileName === undefined) { @@ -987,6 +991,10 @@ export function validateRelease({ root, package: packageName, registryPath, role recoveryReceiptIds: [], errors, }; + if (!Number.isFinite(snapshotMaxAgeMs) || snapshotMaxAgeMs < 0) { + add(errors, 'INPUT-SNAPSHOT-MAX-AGE', 'snapshot age allowance must be a finite non-negative number', '--snapshot-max-age-ms', 'INCOMPLETE'); + return output; + } if (!root || !fs.existsSync(root) || !inside(process.cwd(), root) && !path.isAbsolute(root)) add(errors, 'INPUT-ROOT', 'root must name an existing directory', '--root', 'INCOMPLETE'); if (!PACKAGES.has(packageName)) add(errors, 'INPUT-PACKAGE', 'package must be R28, R29, R30, R31, or R32', '--package', 'INCOMPLETE'); const registryLoaded = readJson(registryPath); diff --git a/research-kit/test/release-validator.test.mjs b/research-kit/test/release-validator.test.mjs index fc01a5c8..4c34a2b0 100644 --- a/research-kit/test/release-validator.test.mjs +++ b/research-kit/test/release-validator.test.mjs @@ -205,6 +205,72 @@ test('researcher-release names a missing value before resolving paths', () => { } }); +test('researcher-release refuses invalid snapshot ages as usage before judging a valid package', async () => { + const root = tempRoot(); + try { + const registryPath = path.join(root, 'registry.json'); + const rolesPath = path.join(root, 'roles.json'); + const pointers = path.join(root, 'pointers'); + const record = envelope(); + record.payloadSha256 = await sha256(canonical(record.payload)); + writeJson(path.join(root, 'records', 'R28-01.json'), record); + writeJson(registryPath, registryFor()); + writeJson(rolesPath, roster()); + fs.mkdirSync(pointers); + const args = ['research-kit/bin/researcher-release.mjs', 'validate', '--root', root, + '--package', 'R28', '--registry', registryPath, '--roles', rolesPath, '--pointers', pointers, '--json']; + for (const value of ['abc', '', ' ', 'NaN', 'Infinity', '1e309', '-1']) { + const run = spawnSync(process.execPath, [...args, '--snapshot-max-age-ms', value], { encoding: 'utf8' }); + assertEqual(run.status, 2, run.stdout || run.stderr); + assert(run.stderr.includes('--snapshot-max-age-ms'), run.stderr); + assert(run.stderr.includes('Usage:'), run.stderr); + assertEqual(run.stdout, '', 'usage must not produce an evidence verdict'); + assert(!/SNAPSHOT-STALE|allowed .*ms/.test(run.stderr), run.stderr); + } + for (const value of ['0', '1', '1.5', '1e3']) { + const run = spawnSync(process.execPath, [...args, '--snapshot-max-age-ms', value], { encoding: 'utf8' }); + assertEqual(run.status, 0, run.stdout || run.stderr); + assertEqual(JSON.parse(run.stdout).status, 'PASS'); + } + } finally { cleanup(root); } +}); + +test('release and snapshot facades distinguish an invalid age allowance from stale evidence', async () => { + const { validateRelease, validateSnapshotEvidence } = await import('../lib/release-validator.mjs'); + const root = tempRoot(); + try { + const registryPath = path.join(root, 'registry.json'); + const rolesPath = path.join(root, 'roles.json'); + writeJson(registryPath, registryFor()); + writeJson(rolesPath, roster()); + const now = Date.parse('2026-01-01T00:00:00Z'); + const baseRecord = envelope(); + baseRecord.payloadSha256 = await sha256(canonical(baseRecord.payload)); + writeJson(path.join(root, 'records', 'R28-01.json'), baseRecord); + const baseline = validateRelease({ root, package: 'R28', registryPath, rolesPath }); + assertEqual(baseline.status, 'PASS', JSON.stringify(baseline)); + const refs = Object.fromEntries(['before', 'after', 'recovery'].map((phase) => [phase, { + snapshotId: phase, capturedAt: '2026-01-01T00:00:00Z', status: 'PASS', envelopeCanonicalSha256: 'c'.repeat(64), + }])); + const record = envelope({ payload: { pathAuthoritySnapshots: refs } }); + for (const value of [NaN, Infinity, -Infinity, -1, null, '0']) { + const results = [ + validateRelease({ root, package: 'R28', registryPath, rolesPath, snapshotMaxAgeMs: value }), + validateSnapshotEvidence(record, { root, now, maxAgeMs: value }), + ]; + for (const result of results) { + assertEqual(result.status, 'INCOMPLETE', JSON.stringify(result)); + assert(result.errors.some((error) => error.code === 'INPUT-SNAPSHOT-MAX-AGE'), JSON.stringify(result)); + assert(!result.errors.some((error) => error.code === 'SNAPSHOT-STALE'), JSON.stringify(result)); + } + } + const fresh = validateSnapshotEvidence(record, { root, now, maxAgeMs: 0 }); + assert(!fresh.errors.some((error) => error.code === 'INPUT-SNAPSHOT-MAX-AGE' || error.code === 'SNAPSHOT-STALE'), JSON.stringify(fresh)); + const stale = validateSnapshotEvidence(record, { root, now: now + 1, maxAgeMs: 0 }); + assertEqual(stale.errors.filter((error) => error.code === 'SNAPSHOT-STALE').length, 3, JSON.stringify(stale)); + } finally { cleanup(root); } +}); + test('researcher-release conform validates explicit files and maps schema failure to exit 1', async () => { const root = tempRoot(); try {