diff --git a/packages/core/src/shell/select.ts b/packages/core/src/shell/select.ts index 8086b230485..39e4af1b87e 100644 --- a/packages/core/src/shell/select.ts +++ b/packages/core/src/shell/select.ts @@ -173,7 +173,7 @@ export function args(file: string, command: string) { if (n === "nu" || n === "fish") return ["-c", command] if (n === "zsh" || n === "bash") return ["-c", command] if (n === "cmd") return ["/c", command] - if (ps(file)) return ["-NoProfile", "-Command", command] + if (ps(file)) return ["-NoLogo", "-NoProfile", "-NonInteractive", "-Command", command] return ["-c", command] } diff --git a/packages/core/src/tool/plugin/shell.ts b/packages/core/src/tool/plugin/shell.ts index 384f60e461a..84723863acd 100644 --- a/packages/core/src/tool/plugin/shell.ts +++ b/packages/core/src/tool/plugin/shell.ts @@ -1,6 +1,5 @@ export * as ShellTool from "./shell" -import path from "path" import { ToolFailure } from "@opencode-ai/ai" import type { Content } from "@opencode-ai/schema/tool" import type { Context as PluginContext } from "@opencode-ai/plugin/effect/plugin" @@ -66,18 +65,14 @@ const Output = Schema.Struct({ ...StructuredOutput.fields, output: Schema.String, status: Schema.optionalKey(Schema.Literals(["completed", "running"])), - warnings: Schema.optionalKey(Schema.Array(Schema.String)), }) type Output = typeof Output.Type const modelOutput = (output: Output): string | undefined => { - const warnings = output.warnings?.length - ? `\n\nWarnings:\n${output.warnings.map((warning) => `- ${warning}`).join("\n")}` - : "" - if (output.status === "running") return `${warnings.trimStart()}${warnings ? "\n\n" : ""}${BACKGROUND_INSTRUCTION}` - if (output.timeout) return `${warnings.trimStart()}${warnings ? "\n\n" : ""}Command timed out before completion.` - return `${warnings.trimStart()}${warnings ? "\n\n" : ""}Command exited with code ${output.exit}.` + if (output.status === "running") return BACKGROUND_INSTRUCTION + if (output.timeout) return "Command timed out before completion." + return `Command exited with code ${output.exit}.` } /** @@ -86,31 +81,12 @@ const modelOutput = (output: Output): string | undefined => { */ // TODO: Port tree-sitter bash / PowerShell parser-based approval reduction. // TODO: Port BashArity reusable command-prefix approvals. -// TODO: Replace token-based command-argument external-directory advisories with parser-based detection. // TODO: Add plugin shell.env environment augmentation once plugin hooks exist. // TODO: Persist job status and define restart recovery before exposing remote observation. // TODO: Add HTTP job observation only after durable status, restart recovery, and authorization are defined. // TODO: Revisit process-group cleanup and platform coverage with shell-specific tests if current AppProcess semantics do not fully cover it. // TODO: Revisit binary output handling if stdout/stderr decoding is text-only. -const shellTokens = (command: string) => command.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g) ?? [] -const unquote = (value: string) => value.replace(/^(['"])(.*)\1$/, "$2") -const externalCommandDirectories = Effect.fn("ShellTool.externalCommandDirectories")(function* ( - fs: FSUtil.Interface, - command: string, - cwd: string, -) { - const directories = new Set() - for (const token of shellTokens(command)) { - const value = unquote(token).replace(/[;,|&]+$/, "") - if (!path.isAbsolute(value)) continue - const resolved = yield* fs.resolve(value) - if (FSUtil.contains(cwd, resolved)) continue - directories.add(yield* fs.resolve(path.dirname(resolved))) - } - return [...directories] -}) - export const Plugin = { id: "opencode.tool.shell", effect: Effect.fn("ShellTool.Plugin")(function* (ctx: PluginContext) { @@ -179,10 +155,6 @@ export const Plugin = { agent: context.agent, source, }) - const warnings = (yield* externalCommandDirectories(fsUtil, input.command, target.canonical)).map( - (directory) => - `Command argument references external directory ${path.join(directory, "*").replaceAll("\\", "/")}. Shell runs with host-user filesystem, process, and network authority; this scan is advisory only.`, - ) yield* permission.assert({ action: name, resources: [input.command], @@ -264,7 +236,6 @@ export const Plugin = { shellID: info.id, truncated: false, status: "running" as const, - ...(warnings.length ? { warnings } : {}), } } @@ -279,14 +250,13 @@ export const Plugin = { shellID: info.id, truncated: false, status: "running" as const, - ...(warnings.length ? { warnings } : {}), } } if (result?.info.status === "error") return yield* Effect.fail(new Error(result.info.error ?? "Command failed")) if (result?.info.status === "cancelled") return yield* Effect.fail(new Error("Command cancelled")) - return { ...(yield* Deferred.await(settled)), ...(warnings.length ? { warnings } : {}) } + return yield* Deferred.await(settled) }).pipe( Effect.map((output) => { const content: Array = [{ type: "text", text: output.output }] diff --git a/packages/core/test/shell.test.ts b/packages/core/test/shell.test.ts index 78782442255..8725e615fa6 100644 --- a/packages/core/test/shell.test.ts +++ b/packages/core/test/shell.test.ts @@ -59,6 +59,13 @@ describe("shell", () => { expect(ShellSelect.args("/usr/bin/fish", "echo hi")).toEqual(["-c", "echo hi"]) expect(ShellSelect.args("/bin/zsh", "echo hi")).toEqual(["-c", "echo hi"]) expect(ShellSelect.args("/bin/bash", "echo hi")).toEqual(["-c", "echo hi"]) + expect(ShellSelect.args("pwsh", "Write-Output hi")).toEqual([ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-Command", + "Write-Output hi", + ]) }) if (process.platform === "win32") { diff --git a/packages/core/test/tool-shell.test.ts b/packages/core/test/tool-shell.test.ts index df53ae94036..2b540d75385 100644 --- a/packages/core/test/tool-shell.test.ts +++ b/packages/core/test/tool-shell.test.ts @@ -346,33 +346,6 @@ describe("ShellTool", () => { ), ) - it.live("reports external command arguments as advisory warnings without enforcing approval", () => - Effect.acquireUseRelease( - Effect.promise(() => Promise.all([tmpdir(), tmpdir()])), - ([active, outside]) => { - reset() - denyAction = "external_directory" - const target = path.join(outside.path, "secret.txt") - return withSession(active.path, (registry) => executeTool(registry, call({ command: `cat ${target}` }))).pipe( - Effect.andThen((settled) => - Effect.sync(() => { - expect(assertions.map((item) => item.action)).toEqual(["shell"]) - expect(settled.metadata).not.toHaveProperty("warnings") - expect(settled.content?.[1]).toMatchObject({ - type: "text", - text: expect.stringContaining("Warnings:"), - }) - }), - ), - ) - }, - ([active, outside]) => - Effect.promise(() => - Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined), - ), - ), - ) - it.live("keeps non-zero exits useful", () => Effect.acquireUseRelease( Effect.promise(() => tmpdir()),