diff --git a/CONTEXT.md b/CONTEXT.md index 6b9f2da..df27ad3 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -21,8 +21,10 @@ program that actually runs. _Avoid_: safe wrapper **Non-transparent wrapper**: -A wrapper whose modifier args change the inner command's resolution or effect -(e.g. `env`, whose `PATH=` or `-i` can make the inner name resolve to a -different binary). Stripping the modifiers and re-evaluating the inner command -is unsound: the verdict may apply to a different program than the one that runs. +A wrapper whose modifiers change what the inner command actually does in a way +the command string does not capture — e.g. `env` (its `PATH=` / `LD_PRELOAD` +make the inner name resolve to or load a different program) or `xargs` (its +inner command's arguments are read from stdin). Stripping the modifiers and +re-evaluating the inner command is unsound: the verdict applies to inputs that +are not knowable from the command string. _Avoid_: unsafe wrapper diff --git a/docs/adr/0003-xargs-defers-and-typically-asks.md b/docs/adr/0003-xargs-defers-and-typically-asks.md new file mode 100644 index 0000000..57e45b7 --- /dev/null +++ b/docs/adr/0003-xargs-defers-and-typically-asks.md @@ -0,0 +1,38 @@ +--- +status: accepted +--- + +# Defer `xargs`; it is non-transparent even to the AI judge + +`xargs` is a third command type considered for `pi-permission-inner-cmd`. Like +`env` (ADR 0002), inner-cmd **always defers** it and never unwraps. Unlike `env`, +the non-transparency is in the inner command's *arguments*, not its environment. + +## Why + +`xargs [options] [command [initial-args]]` reads tokens from stdin (or `-a +FILE`) and appends them as arguments to `command`. The command string therefore +holds only the command *name* (plus any initial args); the bulk of what the +command actually does — its runtime arguments — comes from a separate channel +that is absent from the string entirely. Re-evaluating the inner command is +unsound: the verdict would apply to arguments that are not even knowable from the +input (`xargs rm` can become `rm` over any list of files). `-I` / `-i`, `-a`, +`-P`, `-o` further alter behavior. + +There is no sound unwrap subset (unlike `env`'s rare bare form): `xargs` always +draws its arguments from an external source, so the inner command is never fully +determined by the string. + +## Consequences + +- inner-cmd registers an `xargs` handler that claims `xargs`-leading commands and + defers them (mirroring `env`), for observability. Note that `xargs` usually + appears mid-pipeline (`find … | xargs rm`), where inner-cmd's leading-program + check never reaches it and it defers by default regardless. +- The deferred command reaches the AI judge — but the AI judge *also* cannot see + stdin, so it cannot know the actual arguments either. `xargs` commands + therefore typically warrant a human **ask** rather than an auto-allow, even + with AI judgement; the sound outcome is human confirmation. + +See ADR 0002 for the sibling `env` decision and `CONTEXT.md` for the +non-transparent wrapper distinction. diff --git a/packages/pi-permission-inner-cmd/src/handlers/index.ts b/packages/pi-permission-inner-cmd/src/handlers/index.ts index b2be538..677ecac 100644 --- a/packages/pi-permission-inner-cmd/src/handlers/index.ts +++ b/packages/pi-permission-inner-cmd/src/handlers/index.ts @@ -1,5 +1,6 @@ import { envHandler } from "./env"; import { timeoutHandler } from "./timeout"; +import { xargsHandler } from "./xargs"; import type { CommandHandler } from "./types"; /** @@ -7,4 +8,8 @@ import type { CommandHandler } from "./types"; * claims the command; the rest are not consulted. Add a handler here (and a * new file under `handlers/`) to support a new command type. */ -export const handlers: readonly CommandHandler[] = [timeoutHandler, envHandler]; +export const handlers: readonly CommandHandler[] = [ + timeoutHandler, + envHandler, + xargsHandler, +]; diff --git a/packages/pi-permission-inner-cmd/src/handlers/xargs.ts b/packages/pi-permission-inner-cmd/src/handlers/xargs.ts new file mode 100644 index 0000000..1952106 --- /dev/null +++ b/packages/pi-permission-inner-cmd/src/handlers/xargs.ts @@ -0,0 +1,27 @@ +import type { CommandHandler } from "./types"; + +/** Matches a command whose leading program is `xargs`. */ +const XARGS_PREFIX = /^xargs(?:[ \t]|$)/; + +/** + * The `xargs` wrapper handler (ADR 0003). + * + * `xargs` is non-transparent in a way distinct from `env`: the inner command's + * name is known, but its arguments are read from stdin (or `-a FILE`) at run + * time, so they are absent from the command string entirely. Re-evaluating the + * inner command is unsound — the verdict would apply to arguments that are not + * even knowable from the input. `xargs` is claimed but always deferred; it + * never unwraps. (`xargs` usually appears mid-pipeline, so inner-cmd's + * leading-program check rarely reaches it; this handler covers the rarer + * `xargs`-as-leading-program case for observability.) + */ +export const xargsHandler: CommandHandler = { + id: "xargs", + decide({ command, log }) { + if (!XARGS_PREFIX.test(command)) { + return undefined; + } + log.debug("inner_cmd.xargs_non_transparent", { command }); + return { kind: "defer" }; + }, +}; diff --git a/packages/pi-permission-inner-cmd/test/authorizer.test.ts b/packages/pi-permission-inner-cmd/test/authorizer.test.ts index 44cdf9f..212ac1b 100644 --- a/packages/pi-permission-inner-cmd/test/authorizer.test.ts +++ b/packages/pi-permission-inner-cmd/test/authorizer.test.ts @@ -432,6 +432,36 @@ describe("authorizeInnerCommand — env wrapper", () => { }); }); +describe("authorizeInnerCommand — xargs wrapper", () => { + it("defers on a leading xargs with a debug log (non-transparent args)", async () => { + const { verdict, log, check } = await run({ + recoveredCommand: "xargs rm", + states: { rm: "allow" }, + }); + expect(verdict.kind).toBe("defer"); + // xargs arguments come from stdin, so the inner command is never + // re-evaluated through the deterministic policy. + expect(check).toEqual([]); + expect(log).toEqual([ + { + level: "debug", + event: "inner_cmd.xargs_non_transparent", + details: { command: "xargs rm" }, + }, + ]); + }); + + it("does not claim a command where xargs appears mid-pipeline", async () => { + // Starts with find, not xargs -> no handler claims it -> silent defer. + const { verdict, log } = await run({ + recoveredCommand: "find . -name '*.tmp' | xargs rm", + states: {}, + }); + expect(verdict.kind).toBe("defer"); + expect(log).toEqual([]); + }); +}); + describe("authorizeInnerCommand — exceptions defer with a debug log", () => { it("defers when reading the session id throws (logs only safe data)", async () => { const { verdict, log } = await run({