diff --git a/DOCS.md b/DOCS.md index 22cb77ac..5336ac9d 100644 --- a/DOCS.md +++ b/DOCS.md @@ -1978,6 +1978,8 @@ vg scan [path] [--vulns] [--full] [--iac] [--format text|json|sarif|md] [--out < | `--force` | — | Always create a fresh ingest, even when the repository is unchanged since the last scan | | `--quiet` | — | Suppress promotional output; scan results are unaffected | +Switches (flags that take no value, such as `--vulns`, `--offline` or `--no-graph`) are off unless you pass them. They do not accept a value or an invented `--no-` form: `--vulns=false`, `--vulns=true` and `--no-vulns` stop with exit code `5` and name the form that works. In a CI template, add or leave out the flag itself rather than passing `true` or `false`. + By default, the scan writes `.vibgrate/scan_result.json`. Use `--no-local-artifacts` or `--max-privacy` to suppress local JSON artifact files. For offline drift scoring, pass `--package-manifest ` with a downloaded manifest bundle such as `https://github.com/vibgrate/manifests/latest-packages.zip`. The manifest shape, the fail-closed errors, and what offline mode skips are in [Offline scan with a package-version manifest](#offline-scan-with-a-package-version-manifest). diff --git a/src/cli-switch-values.test.ts b/src/cli-switch-values.test.ts new file mode 100644 index 00000000..25ead9b3 --- /dev/null +++ b/src/cli-switch-values.test.ts @@ -0,0 +1,137 @@ +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { Command, CommanderError } from 'commander'; +import { describe, expect, it } from 'vitest'; +import { buildProgram } from './cli.js'; +import { switchValueError } from './cli-switch-values.js'; + +/** + * CI templates often spell switches out explicitly (`--vulns=false`, + * `--no-vulns`), usually from a templated variable. `vg` must never read such + * an input as "on", and when it rejects one the error must name the switch and + * show a form that works. + */ + +const SWITCHES = ['--vulns', '--offline'] as const; +const attr = (flag: string): string => flag.slice(2); + +class Parsed extends Error { + constructor(readonly opts: Record) { + super('parsed'); + } +} + +/** + * Parse `vg scan ...args` through the real program and return the + * options commander resolved, or the error it rejected the input with. The + * scan itself never runs. + */ +async function parseScan(args: string[]): Promise | CommanderError> { + const scan = buildProgram().commands.find((c) => c.name() === 'scan') as Command; + scan.exitOverride(); + scan.configureOutput({ writeErr: () => {}, writeOut: () => {} }); + scan.hook('preAction', (_cmd, action) => { + throw new Parsed(action.opts()); + }); + try { + await scan.parseAsync([os.tmpdir(), ...args], { from: 'user' }); + } catch (e) { + if (e instanceof Parsed) return e.opts; + if (e instanceof CommanderError) return e; + throw e; + } + throw new Error('scan did not reach its action'); +} + +describe('vg scan switches parse as documented', () => { + for (const flag of SWITCHES) { + describe(flag, () => { + it('is off when absent', async () => { + const opts = await parseScan([]); + expect(opts).not.toBeInstanceOf(CommanderError); + expect((opts as Record)[attr(flag)]).toBeUndefined(); + }); + + it(`is on with ${flag}`, async () => { + const opts = await parseScan([flag]); + expect((opts as Record)[attr(flag)]).toBe(true); + }); + + for (const input of [`${flag}=true`, `${flag}=false`, `${flag}=maybe`, `${flag}=`, `--no-${attr(flag)}`]) { + it(`rejects ${input} and never turns the switch on`, async () => { + const res = await parseScan([input]); + expect(res).toBeInstanceOf(CommanderError); + expect((res as CommanderError).code).toBe('commander.unknownOption'); + }); + } + }); + } +}); + +describe('switchValueError', () => { + const program = buildProgram(); + + for (const flag of SWITCHES) { + for (const value of ['true', 'false', 'maybe', '']) { + it(`explains ${flag}=${value} with the switch and a working example`, () => { + const msg = switchValueError(program, ['scan', '.', `${flag}=${value}`]); + expect(msg).toContain(`${flag} is an on/off switch and takes no value`); + expect(msg).toContain(`got ${flag}=${value}`); + expect(msg).toContain(`vg scan ${flag}`); + }); + } + + it(`explains --no-${attr(flag)} without suggesting the opposite`, () => { + const msg = switchValueError(program, ['scan', `--no-${attr(flag)}`, '.']); + expect(msg).toContain(`--no-${attr(flag)} is not an option`); + expect(msg).toContain(`${flag} is off unless you pass it`); + expect(msg).toContain(`vg scan ${flag}`); + }); + } + + it('explains a value on a --no- switch the command does define', () => { + expect(switchValueError(program, ['scan', '--no-graph=1'])).toContain( + '--no-graph is an on/off switch and takes no value (got --no-graph=1)', + ); + }); + + it.each([ + [['scan', '.', '--vulns', '--offline']], + [['scan', '.', '--format=json']], + [['scan', '.', '--bogus=1']], + [['scan', '.', '--no-graph']], + [['scan', '.', '--', '--vulns=false']], + [['scan', '.', '--no-such-switch']], + ])('leaves valid or unrelated input to commander: %j', (args) => { + expect(switchValueError(program, args)).toBeUndefined(); + }); + + it('does not inspect arguments a pass-through command forwards', () => { + // `vg serve ...` hands everything after the agent name to the agent. + // `--http` is a serve switch, so without the pass-through rule this would match. + expect(switchValueError(program, ['serve', 'claude', '--http=1'])).toBeUndefined(); + }); +}); + +describe('vg scan with a switch value, end to end', () => { + const pkgRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-switch-')); + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"fixture","version":"1.0.0"}\n'); + + const run = (flag: string) => + spawnSync( + process.execPath, + ['--import', 'tsx', path.join(pkgRoot, 'src/cli.ts'), 'scan', dir, '--offline', '--no-graph', '--no-daemon', '--quiet', '--no-local-artifacts', flag], + { cwd: pkgRoot, encoding: 'utf8', timeout: 45_000, env: { ...process.env, NO_COLOR: '1', VIBGRATE_NO_KERNEL: '1', VIBGRATE_DSN: '' } }, + ); + + it.each(['--vulns=false', '--no-vulns'])('%s exits with the usage code and a working example', (flag) => { + const res = run(flag); + expect(res.status).toBe(5); + expect(res.stderr).toContain('vg scan --vulns'); + expect(res.stderr).not.toContain('Did you mean'); + }, 60_000); +}); diff --git a/src/cli-switch-values.ts b/src/cli-switch-values.ts new file mode 100644 index 00000000..b3ab4319 --- /dev/null +++ b/src/cli-switch-values.ts @@ -0,0 +1,83 @@ +import type { Command, Option } from 'commander'; + +/** + * Explain an on/off switch written with a value (`--vulns=false`) or with a + * `--no-` prefix the command does not define (`--no-vulns`). + * + * Commander rejects both as an unknown option, which is safe but unhelpful: + * `--vulns=false` reads as "unknown option" for a switch that exists, and for + * `--no-vulns` it suggests `--vulns`, the opposite of what was asked. Returns + * a usage message for the first such argument, or `undefined` to leave the + * input to commander. Only inputs commander would reject anyway are matched. + */ +export function switchValueError(program: Command, args: string[]): string | undefined { + const { cmd, rest } = resolveCommand(program, args); + if (isPassThrough(cmd)) return undefined; + const usage = commandPath(cmd); + + for (const arg of rest) { + if (arg === '--') break; + if (!arg.startsWith('--')) continue; + + const eq = arg.indexOf('='); + if (eq > 0) { + const flag = arg.slice(0, eq); + const opt = switchNamed(cmd, flag); + if (!opt || (!opt.negate && opt.defaultValue !== undefined)) continue; + const hint = flag.startsWith('--no-') + ? `use \`${usage} ${flag}\`, or leave it out` + : `it is off unless you pass it: use \`${usage} ${flag}\` to turn it on, or leave it out`; + return `${flag} is an on/off switch and takes no value (got ${arg}); ${hint}.`; + } + + if (arg.startsWith('--no-') && !findOption(cmd, arg)) { + const positive = `--${arg.slice('--no-'.length)}`; + const opt = switchNamed(cmd, positive); + if (!opt || opt.negate || opt.defaultValue !== undefined) continue; + return `${arg} is not an option. ${positive} is off unless you pass it, so leave it out (to turn it on: \`${usage} ${positive}\`).`; + } + } + return undefined; +} + +/** Walk leading subcommand names (`models catalog ...`) down to the leaf. */ +function resolveCommand(program: Command, args: string[]): { cmd: Command; rest: string[] } { + let cmd = program; + let i = 0; + for (; i < args.length; i++) { + const sub = cmd.commands.find((c) => c.name() === args[i] || c.aliases().includes(args[i])); + if (!sub) break; + cmd = sub; + } + return { cmd, rest: args.slice(i) }; +} + +/** + * A pass-through command (`vg serve ...`) forwards options after its + * first operand to another program, so their spelling is not ours to judge. + * Commander exposes the setter but not a getter for this. + */ +function isPassThrough(cmd: Command): boolean { + return (cmd as unknown as { _passThroughOptions?: boolean })._passThroughOptions === true; +} + +/** + * Only the leaf's own options count: the program enables positional options, + * so a parent's flag is not accepted after a subcommand name (global flags are + * declared on every subcommand instead, see cli-options.applyGlobalOptions). + */ +function findOption(cmd: Command, long: string): Option | undefined { + return cmd.options.find((o) => o.long === long); +} + +/** The option named `long` if it takes no value: `--vulns`, and also `--no-graph`. */ +function switchNamed(cmd: Command, long: string): Option | undefined { + const opt = findOption(cmd, long); + return opt && !opt.required && !opt.optional ? opt : undefined; +} + +function commandPath(cmd: Command): string { + const names: string[] = []; + for (let c: Command | null = cmd; c; c = c.parent) names.unshift(c.name()); + return names.join(' '); +} diff --git a/src/cli.ts b/src/cli.ts index e66260f2..23a45c54 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -45,13 +45,14 @@ import { registerReview } from './commands/review.js'; import { ConfigFileError } from './core-open/config.js'; import { LockfileParseError } from './core-open/utils/lockfile-parse.js'; import { UnsafeRootError } from './core-open/utils/root-safety.js'; -import { CliError, ExitCode } from './util/exit.js'; +import { CliError, ExitCode, usageError } from './util/exit.js'; import { c, info, disableColor, exitAfterFlush } from './util/output.js'; // Drift-reporting commands (merged from the Vibgrate CLI). These run on the // open base engine (`@vibgrate/core-open`) — no proprietary kernel. import { initCommand } from './reporting/commands/init.js'; import { scanCommand } from './reporting/commands/scan.js'; +import { switchValueError } from './cli-switch-values.js'; import { fixCommand } from './reporting/commands/fix.js'; import { baselineCommand } from './reporting/commands/baseline.js'; import { reportCommand } from './reporting/commands/report.js'; @@ -398,6 +399,9 @@ export async function main(argv = process.argv): Promise { // dispatch() can throw (e.g. a moved-command hint) — keep it inside the // handler so those surface as a clean `error:` line, not a raw stack. const dispatched = dispatch(raw, cwd); + // Commander would reject these as an unknown option; say what works instead. + const switchError = switchValueError(program, dispatched); + if (switchError) throw usageError(switchError); await program.parseAsync(dispatched, { from: 'user' }); } catch (err) { handleError(err);