diff --git a/packages/cli/src/flags.ts b/packages/cli/src/flags.ts index 93a6142..d3dac28 100644 --- a/packages/cli/src/flags.ts +++ b/packages/cli/src/flags.ts @@ -31,7 +31,15 @@ export const BY_COMMAND: Record = { quickstart: ['dir', 'full'], // push/pull reach a gateway the user names. `--force` means two different deliberate overrides: // on push, store despite a secret-scan finding; on pull, replace a local run that already exists. - push: ['gateway', 'force'], + // + // `--fs` belongs here because `pushCommand` reads it (`args.bool('fs')`) and the help text + // offers it. Leaving it out did not make push ignore the flag — it made push REFUSE it, before + // the implementation ever saw it, and the refusal named only the two flags that were listed. So + // the workspace snapshots could not be shipped by any invocation, every run on the gateway came + // back with `blob_count: 0`, and forking a pulled run died in `git read-tree` on a tree object + // that had never been sent. This list is meant to be counted from the `args.*` call sites; that + // one was missed. + push: ['gateway', 'force', 'fs'], pull: ['gateway', 'force'], record: ['fs', 'shell', 'agent-spans', 'mcp-config', ...RETRIEVAL, ...TLS, ...UPSTREAM], attach: [ diff --git a/packages/cli/test/flags.test.ts b/packages/cli/test/flags.test.ts index 8bfa67a..f9a83e9 100644 --- a/packages/cli/test/flags.test.ts +++ b/packages/cli/test/flags.test.ts @@ -106,6 +106,52 @@ describe('the allowlist against the source it mirrors', () => { expect(unreachable).toEqual([]); }); + /** + * The one above flattens every command's flags into one set and asks whether SOMEONE allows the + * name. That is too weak by exactly one level, and the gap shipped: `push` reads `args.bool('fs')` + * and the help text offers `--fs`, but `push`'s own entry listed only `gateway` and `force`. + * Because `record` and `replay` both allow `fs`, the union check was satisfied and said nothing. + * + * What the user got was not a flag quietly ignored — it was `orca push --fs` REFUSED, with an + * error naming only the two flags that were listed. The snapshots could therefore not be shipped + * by any invocation, so every run on a gateway carried `blob_count: 0`, and forking a pulled run + * died inside `git read-tree` on a tree object that had never left the machine. + * + * The help text is the contract the user reads, so hold the allowlist to it per command rather + * than in aggregate. `sync.test.ts` could not catch this: it calls `pushCommand` with args it + * built itself, which never passes through the gate this file guards. + */ + it('accepts, for each command, every flag the help text offers it', () => { + const help = readFileSync(join(SRC, 'main.ts'), 'utf8').split(/\r?\n/); + const documented = new Map>(); + let command: string | undefined; + for (const line of help) { + // ` orca push [run] send a run to the gateway` — two spaces, then the command. + const heading = line.match(/^ {2}orca ([a-z-]+)\b(.*)$/); + if (heading) { + command = heading[1]!; + const flags = documented.get(command) ?? new Set(); + // A usage line can carry a flag itself: `orca replay [run] --from N`. + for (const m of heading[2]!.matchAll(/--([a-z][a-z0-9-]*)/g)) flags.add(m[1]!); + documented.set(command, flags); + continue; + } + // ` --fs include the workspace snapshots` — eight spaces, under the last heading. + const flag = command === undefined ? null : line.match(/^ {8}--([a-z][a-z0-9-]*)/); + if (flag) documented.get(command!)!.add(flag[1]!); + } + // The parse itself has to keep working, or the check passes by finding nothing. + expect(documented.get('push'), 'the help block stopped parsing').toBeDefined(); + expect([...documented.get('push')!].sort()).toContain('fs'); + + const refused: string[] = []; + for (const [cmd, flags] of documented) { + const allowed = new Set([...(BY_COMMAND[cmd] ?? []), ...GLOBAL]); + for (const name of flags) if (!allowed.has(name)) refused.push(`orca ${cmd} --${name}`); + } + expect(refused.sort()).toEqual([]); + }); + /** * A flag the code reads as a boolean has to be declared valueless, or the parser hands it the * next word and the positional it belonged to disappears.