From 3770719dccca77e722955b8ae3b0f2f2a23194fc Mon Sep 17 00:00:00 2001 From: StepenkoAnatoli Date: Wed, 14 Oct 2026 11:43:31 +0300 Subject: [PATCH] fix(cli): refuse missing validator flag values by name Fixes #288 what changed: path-authority and researcher-release now check value tokens before their handwritten parsers consume them. Missing, empty or next-flag tokens produce a named usage error on stderr with exit 2, before path resolution or input reading. Added a regression table in each CLI's existing validator test file for --file, --schema and --root at the end of the command or before --json. why: A bare --file reached path.resolve(undefined), surfacing a Node internal message or stack trace and, for researcher-release, exit 1 instead of a usage refusal (#288). Apply the existing usage-error boundary; no alternative design was set aside. what it touched: the two validator CLI parsers, path-authority/release-validator tests, their architecture descriptions, changelog and README test inventory. Repeated --file values remain supported; validation statuses and their exit-code mapping are unchanged. what you verified: verified - red-first 38 passed and two expected failures, reproducing the path-authority JSON-READ/paths[0] message and researcher-release TypeError/exit 1. Then path-authority, release-validator and support-policy passed 40 tests with zero failures in 210.9 seconds, including named exit-2 refusals and normal CLI conformance/validation. Architecture-map passed 21 tests; handoff OK (33 ledger entries, 27 evidence rows), preflight PASS (28 passing, one warning, no blockers), doctor READY collector/firecrawl-cli. After refresh onto merged main 82a02cb, the same 40 focused tests passed with zero failures and zero unsupported cases in 14.5 seconds. The patch was then advanced to main 194bcfb8 after PR411 landed; staged code/test blobs match the focused-tested source, while shared documentation includes that fix and the 1,838-test inventory. The normal full commit gate runs before acceptance; GitHub CI must verify the published head before merge. what you got wrong and fixed: nothing to report. --- CHANGELOG.md | 2 ++ docs/ARCHITECTURE.md | 5 ++++- research-kit/README.md | 2 +- research-kit/bin/path-authority.mjs | 4 ++++ research-kit/bin/researcher-release.mjs | 5 +++++ .../test/path-authority-validator.test.mjs | 14 ++++++++++++++ research-kit/test/release-validator.test.mjs | 17 +++++++++++++++++ 7 files changed, 47 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47c4fba3..eb022486 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ Each release of Research-Kit, newest first. The version is `KIT_VERSION` in ## Unreleased +- `path-authority` and `researcher-release` name a value flag with no value and print usage with exit 2 before resolving paths, including when another flag follows (#288). + - `property-replay --all` refuses a missing regression directory with exit 2 and names it; an existing empty directory explicitly reports no captured regressions. JSON results identify the directory and zero count (#307). diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5d0a4cc2..8b59c3af 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -277,7 +277,8 @@ markers - they wrote `research/` into any folder until 2026-09-28, `prior` a PRI `doctor --fix-arity` and `install` name a write the system refused - `could not write : (why)`, exit 2, via `writeFailure` - where they printed a stack trace, 2026-09-28), `path-authority.mjs` (offline Git-origin/path-authority snapshot validation and -schema conformance), +schema conformance; refuses a missing value for `--file`, `--schema` or `--root` by +name before reading any input, as a usage error with exit 2), `researcher-release.mjs` (ported 2026-09-20, ADR-0029 — three subcommands over the two validators: `validate` for R28–R32 release evidence, `conform` for schema-only checking, `fi-validate` for an FI bundle. It **duplicates no validation**: every check comes from @@ -285,6 +286,8 @@ validators: `validate` for R28–R32 release evidence, `conform` for schema-only environment variable at all**, so it cannot depend on a credential or a config. Status 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; `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 ad2fb474..269f36f4 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 ``` -1836 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means +1838 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/path-authority.mjs b/research-kit/bin/path-authority.mjs index b26ecdac..50fda9ca 100644 --- a/research-kit/bin/path-authority.mjs +++ b/research-kit/bin/path-authority.mjs @@ -22,6 +22,10 @@ function parse(argv) { for (let i = 1; i < argv.length; i += 1) { const arg = argv[i]; if (arg === '--json') { result.json = true; continue; } + if (['--file', '--schema', '--root'].includes(arg) + && (argv[i + 1] === undefined || argv[i + 1] === '' || argv[i + 1].startsWith('--'))) { + return { error: `${arg} needs a value` }; + } if (arg === '--file') { result.files.push(argv[++i]); continue; } if (arg === '--schema') { result.schema = argv[++i]; continue; } if (arg === '--root') { result.root = argv[++i]; continue; } diff --git a/research-kit/bin/researcher-release.mjs b/research-kit/bin/researcher-release.mjs index 608172cb..17d34d11 100644 --- a/research-kit/bin/researcher-release.mjs +++ b/research-kit/bin/researcher-release.mjs @@ -29,6 +29,10 @@ function argsAfterCommand(argv) { for (let i = 1; i < argv.length; i += 1) { const arg = argv[i]; if (arg === '--json') { result.json = true; continue; } + if (arg.startsWith('--') + && (argv[i + 1] === undefined || argv[i + 1] === '' || argv[i + 1].startsWith('--'))) { + return { error: `${arg} needs a value` }; + } if (arg === '--file') { result.files ??= []; result.files.push(argv[++i]); continue; } if (arg.startsWith('--')) { result[arg.slice(2).replaceAll('-', '')] = argv[++i]; continue; } result._.push(arg); @@ -47,6 +51,7 @@ function exitFor(status) { function main(argv = process.argv.slice(2)) { if (argv.includes('--help') || argv.includes('-h')) { process.stdout.write(`${usage()}\n`); return 0; } const parsed = argsAfterCommand(argv); + if (parsed.error) { console.error(`${parsed.error}\n${usage()}`); return 2; } let result; if (parsed.command === 'validate') { if (!parsed.root || !parsed.package || !parsed.registry || !parsed.roles || !parsed.pointers) { diff --git a/research-kit/test/path-authority-validator.test.mjs b/research-kit/test/path-authority-validator.test.mjs index 816e6a6d..f3e3a9c4 100644 --- a/research-kit/test/path-authority-validator.test.mjs +++ b/research-kit/test/path-authority-validator.test.mjs @@ -102,6 +102,20 @@ test('path-authority validator rejects out-of-root paths, symlink claims, host m assert(result.errors.some((error) => error.code === 'SNAP-ORIGIN-UNPROVEN'), JSON.stringify(result.errors)); }); +test('path-authority names a missing value before validating any input', () => { + for (const flag of ['--file', '--schema', '--root']) { + for (const tail of [[], ['--json']]) { + const args = ['research-kit/bin/path-authority.mjs', 'validate', flag, ...tail]; + const run = spawnSync(process.execPath, args, { encoding: 'utf8' }); + const output = `${run.stdout}${run.stderr}`; + assertEqual(run.status, 2, `${args.join(' ')}: ${output}`); + assert(run.stderr.includes(`${flag} needs a value`), output); + assert(run.stderr.includes('Usage:'), output); + assert(!/TypeError|paths\[0\]|ERR_INVALID_ARG_TYPE/.test(output), output); + } + } +}); + test('path-authority CLI conform and validate are offline, deterministic, and read-only', async () => { const root = tempRoot(); try { diff --git a/research-kit/test/release-validator.test.mjs b/research-kit/test/release-validator.test.mjs index aa60e07b..fc01a5c8 100644 --- a/research-kit/test/release-validator.test.mjs +++ b/research-kit/test/release-validator.test.mjs @@ -188,6 +188,23 @@ test('researcher-release validate emits the contract status and exit code withou } }); +test('researcher-release names a missing value before resolving paths', () => { + for (const flag of ['--file', '--schema', '--root']) { + for (const tail of [[], ['--json']]) { + const args = [ + 'research-kit/bin/researcher-release.mjs', 'conform', '--schema', 'missing-schema.json', + '--file', 'missing-record.json', flag, ...tail, + ]; + const run = spawnSync(process.execPath, args, { encoding: 'utf8' }); + const output = `${run.stdout}${run.stderr}`; + assertEqual(run.status, 2, `${args.join(' ')}: ${output}`); + assert(run.stderr.includes(`${flag} needs a value`), output); + assert(run.stderr.includes('Usage:'), output); + assert(!/TypeError|paths\[0\]|ERR_INVALID_ARG_TYPE/.test(output), output); + } + } +}); + test('researcher-release conform validates explicit files and maps schema failure to exit 1', async () => { const root = tempRoot(); try {