diff --git a/packages/core/src/tool/plugin/shell.ts b/packages/core/src/tool/plugin/shell.ts index a5abb1d7cd4b..dcebf6b7a114 100644 --- a/packages/core/src/tool/plugin/shell.ts +++ b/packages/core/src/tool/plugin/shell.ts @@ -130,15 +130,18 @@ export const Plugin = { }), ) yield* access.authorizeExternal([target, ...directories], context) - if (parsed.commands.length > 0) - yield* permission.assert({ - action: name, - resources: parsed.commands.map((command) => command.resource), - save: parsed.commands.map((command) => command.save), - sessionID: context.sessionID, - agent: context.agent, - source, - }) + // A command the scanner finds no commands in still runs, so fall back to the raw + // invocation rather than skipping the check. A bare redirect is the clearest case: + // `> file` parses to zero commands but truncates the file when the shell runs it. + const scanned = parsed.commands.length > 0 + yield* permission.assert({ + action: name, + resources: scanned ? parsed.commands.map((command) => command.resource) : [invocation.command], + save: scanned ? parsed.commands.map((command) => command.save) : [invocation.command], + sessionID: context.sessionID, + agent: context.agent, + source, + }) // Approval can outlive the directory, so validate immediately before spawning. const workdir = yield* Environment.typeFollowing(environment.files, target.absolute).pipe( Effect.catchTag("Environment.NotFound", () => diff --git a/packages/core/test/permission.test.ts b/packages/core/test/permission.test.ts index 3690b39f1a20..9b62419c48ba 100644 --- a/packages/core/test/permission.test.ts +++ b/packages/core/test/permission.test.ts @@ -707,3 +707,36 @@ describe("shell scanner permission impact", () => { } } }) + +describe("permission.shell scanner gaps", () => { + for (const portable of [false, true]) { + for (const command of ["> victim.txt", ">> victim.txt"]) { + it.effect(`denies ${JSON.stringify(command)} the scanner finds no commands in (portable=${portable})`, () => + Effect.gen(function* () { + // A bare redirect is valid POSIX and truncates or creates the file, but both + // scanners report zero commands for it. Falling back to the raw invocation keeps + // it inside the permission check instead of running unchecked. + const parsed = yield* ShellParse.scan(command, "/bin/bash", "/project", { portable }) + expect(parsed.commands.length).toBe(0) + + yield* setup([{ action: "*", resource: "*", effect: "deny" }]) + const service = yield* Permission.Service + const scanned = parsed.commands.length > 0 + const result = yield* service.ask( + assertion({ + action: "shell", + resources: scanned ? parsed.commands.map((entry) => entry.resource) : [command], + save: scanned ? parsed.commands.map((entry) => entry.save) : [command], + }), + ) + expect(result.effect).toBe("deny") + + // Without that fallback the resource list is empty, and an empty list evaluates + // to allow under a deny-all ruleset - the fail-open this guards against. + const empty = yield* service.ask(assertion({ action: "shell", resources: [], save: [] })) + expect(empty.effect).toBe("allow") + }), + ) + } + } +})