diff --git a/src/hooks/non-interactive-env/index.test.ts b/src/hooks/non-interactive-env/index.test.ts index 7b4502a7..370feee9 100644 --- a/src/hooks/non-interactive-env/index.test.ts +++ b/src/hooks/non-interactive-env/index.test.ts @@ -1,9 +1,31 @@ -import { describe, test, expect } from "bun:test" +import { describe, test, expect, beforeEach, afterEach } from "bun:test" import { createNonInteractiveEnvHook, NON_INTERACTIVE_ENV } from "./index" describe("non-interactive-env hook", () => { const mockCtx = {} as Parameters[0] + let originalPlatform: NodeJS.Platform + let originalEnv: Record + + beforeEach(() => { + originalPlatform = process.platform + originalEnv = { + SHELL: process.env.SHELL, + PSModulePath: process.env.PSModulePath, + } + }) + + afterEach(() => { + Object.defineProperty(process, "platform", { value: originalPlatform }) + for (const [key, value] of Object.entries(originalEnv)) { + if (value !== undefined) { + process.env[key] = value + } else { + delete process.env[key] + } + } + }) + describe("git command modification", () => { test("#given git command #when hook executes #then prepends export statement", async () => { const hook = createNonInteractiveEnvHook(mockCtx) @@ -147,4 +169,147 @@ describe("non-interactive-env hook", () => { expect(output.message).toBeUndefined() }) }) + + describe("cross-platform shell support", () => { + test("#given macOS platform #when git command executes #then uses unix export syntax", async () => { + delete process.env.PSModulePath + process.env.SHELL = "/bin/zsh" + Object.defineProperty(process, "platform", { value: "darwin" }) + + const hook = createNonInteractiveEnvHook(mockCtx) + const output: { args: Record; message?: string } = { + args: { command: "git status" }, + } + + await hook["tool.execute.before"]( + { tool: "bash", sessionID: "test", callID: "1" }, + output + ) + + const cmd = output.args.command as string + expect(cmd).toStartWith("export ") + expect(cmd).toContain(";") + expect(cmd).not.toContain("$env:") + expect(cmd).not.toContain("set ") + }) + + test("#given Linux platform #when git command executes #then uses unix export syntax", async () => { + delete process.env.PSModulePath + process.env.SHELL = "/bin/bash" + Object.defineProperty(process, "platform", { value: "linux" }) + + const hook = createNonInteractiveEnvHook(mockCtx) + const output: { args: Record; message?: string } = { + args: { command: "git commit -m 'test'" }, + } + + await hook["tool.execute.before"]( + { tool: "bash", sessionID: "test", callID: "1" }, + output + ) + + const cmd = output.args.command as string + expect(cmd).toStartWith("export ") + expect(cmd).toContain("; git commit") + }) + + test("#given Windows with PowerShell #when git command executes #then uses powershell $env syntax", async () => { + process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules" + Object.defineProperty(process, "platform", { value: "win32" }) + + const hook = createNonInteractiveEnvHook(mockCtx) + const output: { args: Record; message?: string } = { + args: { command: "git status" }, + } + + await hook["tool.execute.before"]( + { tool: "bash", sessionID: "test", callID: "1" }, + output + ) + + const cmd = output.args.command as string + expect(cmd).toContain("$env:") + expect(cmd).toContain("; git status") + expect(cmd).not.toStartWith("export ") + expect(cmd).not.toContain("set ") + }) + + test("#given Windows without PowerShell #when git command executes #then uses cmd set syntax", async () => { + delete process.env.PSModulePath + delete process.env.SHELL + Object.defineProperty(process, "platform", { value: "win32" }) + + const hook = createNonInteractiveEnvHook(mockCtx) + const output: { args: Record; message?: string } = { + args: { command: "git log" }, + } + + await hook["tool.execute.before"]( + { tool: "bash", sessionID: "test", callID: "1" }, + output + ) + + const cmd = output.args.command as string + expect(cmd).toContain("set ") + expect(cmd).toContain("&&") + expect(cmd).not.toStartWith("export ") + expect(cmd).not.toContain("$env:") + }) + + test("#given PowerShell #when values contain quotes #then escapes correctly", async () => { + process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules" + Object.defineProperty(process, "platform", { value: "win32" }) + + const hook = createNonInteractiveEnvHook(mockCtx) + const output: { args: Record; message?: string } = { + args: { command: "git status" }, + } + + await hook["tool.execute.before"]( + { tool: "bash", sessionID: "test", callID: "1" }, + output + ) + + const cmd = output.args.command as string + expect(cmd).toMatch(/\$env:\w+='[^']*'/) + }) + + test("#given cmd.exe #when values contain spaces #then escapes correctly", async () => { + delete process.env.PSModulePath + delete process.env.SHELL + Object.defineProperty(process, "platform", { value: "win32" }) + + const hook = createNonInteractiveEnvHook(mockCtx) + const output: { args: Record; message?: string } = { + args: { command: "git status" }, + } + + await hook["tool.execute.before"]( + { tool: "bash", sessionID: "test", callID: "1" }, + output + ) + + const cmd = output.args.command as string + expect(cmd).toMatch(/set \w+="[^"]*"/) + }) + + test("#given PowerShell #when chained git commands #then env vars apply to all commands", async () => { + process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules" + Object.defineProperty(process, "platform", { value: "win32" }) + + const hook = createNonInteractiveEnvHook(mockCtx) + const output: { args: Record; message?: string } = { + args: { command: "git add file && git commit -m 'test'" }, + } + + await hook["tool.execute.before"]( + { tool: "bash", sessionID: "test", callID: "1" }, + output + ) + + const cmd = output.args.command as string + expect(cmd).toContain("$env:") + expect(cmd).toContain("; git add file && git commit") + }) + }) }) diff --git a/src/hooks/non-interactive-env/index.ts b/src/hooks/non-interactive-env/index.ts index a7f51d6d..e6c7e56c 100644 --- a/src/hooks/non-interactive-env/index.ts +++ b/src/hooks/non-interactive-env/index.ts @@ -1,6 +1,7 @@ import type { PluginInput } from "@opencode-ai/plugin" import { HOOK_NAME, NON_INTERACTIVE_ENV, SHELL_COMMAND_PATTERNS } from "./constants" -import { log } from "../../shared" +import { isNonInteractive } from "./detector" +import { log, detectShellType, buildEnvPrefix } from "../../shared" export * from "./constants" export * from "./detector" @@ -19,35 +20,6 @@ function detectBannedCommand(command: string): string | undefined { return undefined } -/** - * Shell-escape a value for use in VAR=value prefix. - * Wraps in single quotes if contains special chars. - */ -function shellEscape(value: string): string { - // Empty string needs quotes - if (value === "") return "''" - // If contains special chars, wrap in single quotes (escape existing single quotes) - if (/[^a-zA-Z0-9_\-.:\/]/.test(value)) { - return `'${value.replace(/'/g, "'\\''")}'` - } - return value -} - -/** - * Build export statement for environment variables. - * Uses `export VAR1=val1 VAR2=val2;` format to ensure variables - * apply to ALL commands in a chain (e.g., `cmd1 && cmd2`). - * - * Previous approach used VAR=value prefix which only applies to the first command. - * OpenCode's bash tool ignores args.env, so we must prepend to command. - */ -function buildEnvPrefix(env: Record): string { - const exports = Object.entries(env) - .map(([key, value]) => `${key}=${shellEscape(value)}`) - .join(" ") - return `export ${exports};` -} - export function createNonInteractiveEnvHook(_ctx: PluginInput) { return { "tool.execute.before": async ( @@ -74,11 +46,12 @@ export function createNonInteractiveEnvHook(_ctx: PluginInput) { return } - // OpenCode's bash tool uses hardcoded `...process.env` in spawn(), - // ignoring any args.env we might set. Prepend export statement to command. - // Uses `export VAR=val;` format to ensure variables apply to ALL commands - // in a chain (e.g., `git add file && git rebase --continue`). - const envPrefix = buildEnvPrefix(NON_INTERACTIVE_ENV) + if (!isNonInteractive()) { + return + } + + const shellType = detectShellType() + const envPrefix = buildEnvPrefix(NON_INTERACTIVE_ENV, shellType) output.args.command = `${envPrefix} ${command}` log(`[${HOOK_NAME}] Prepended non-interactive env vars to git command`, { diff --git a/src/shared/index.ts b/src/shared/index.ts index 79cc7024..c0e6d0bb 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -23,3 +23,4 @@ export * from "./external-plugin-detector" export * from "./zip-extractor" export * from "./agent-variant" export * from "./session-cursor" +export * from "./shell-env" diff --git a/src/shared/shell-env.test.ts b/src/shared/shell-env.test.ts new file mode 100644 index 00000000..c0e53306 --- /dev/null +++ b/src/shared/shell-env.test.ts @@ -0,0 +1,278 @@ +import { describe, test, expect, beforeEach, afterEach } from "bun:test" +import { detectShellType, shellEscape, buildEnvPrefix } from "./shell-env" + +describe("shell-env", () => { + let originalPlatform: NodeJS.Platform + let originalEnv: Record + + beforeEach(() => { + originalPlatform = process.platform + originalEnv = { + SHELL: process.env.SHELL, + PSModulePath: process.env.PSModulePath, + } + }) + + afterEach(() => { + Object.defineProperty(process, "platform", { value: originalPlatform }) + for (const [key, value] of Object.entries(originalEnv)) { + if (value !== undefined) { + process.env[key] = value + } else { + delete process.env[key] + } + } + }) + + describe("detectShellType", () => { + test("#given SHELL env var set to /bin/bash #when detectShellType is called #then returns unix", () => { + delete process.env.PSModulePath + process.env.SHELL = "/bin/bash" + Object.defineProperty(process, "platform", { value: "linux" }) + + const result = detectShellType() + + expect(result).toBe("unix") + }) + + test("#given SHELL env var set to /bin/zsh #when detectShellType is called #then returns unix", () => { + delete process.env.PSModulePath + process.env.SHELL = "/bin/zsh" + Object.defineProperty(process, "platform", { value: "darwin" }) + + const result = detectShellType() + + expect(result).toBe("unix") + }) + + test("#given PSModulePath is set #when detectShellType is called #then returns powershell", () => { + process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules" + Object.defineProperty(process, "platform", { value: "win32" }) + + const result = detectShellType() + + expect(result).toBe("powershell") + }) + + test("#given Windows platform without PSModulePath #when detectShellType is called #then returns cmd", () => { + delete process.env.PSModulePath + delete process.env.SHELL + Object.defineProperty(process, "platform", { value: "win32" }) + + const result = detectShellType() + + expect(result).toBe("cmd") + }) + + test("#given non-Windows platform without SHELL env var #when detectShellType is called #then returns unix", () => { + delete process.env.PSModulePath + delete process.env.SHELL + Object.defineProperty(process, "platform", { value: "linux" }) + + const result = detectShellType() + + expect(result).toBe("unix") + }) + + test("#given PSModulePath takes priority over SHELL #when both are set #then returns powershell", () => { + process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules" + process.env.SHELL = "/bin/bash" + Object.defineProperty(process, "platform", { value: "win32" }) + + const result = detectShellType() + + expect(result).toBe("powershell") + }) + }) + + describe("shellEscape", () => { + describe("unix shell", () => { + test("#given plain alphanumeric string #when shellEscape is called with unix #then returns unquoted string", () => { + const result = shellEscape("simple123", "unix") + expect(result).toBe("simple123") + }) + + test("#given empty string #when shellEscape is called with unix #then returns single quotes", () => { + const result = shellEscape("", "unix") + expect(result).toBe("''") + }) + + test("#given string with spaces #when shellEscape is called with unix #then wraps in single quotes", () => { + const result = shellEscape("has spaces", "unix") + expect(result).toBe("'has spaces'") + }) + + test("#given string with single quote #when shellEscape is called with unix #then escapes with backslash", () => { + const result = shellEscape("it's", "unix") + expect(result).toBe("'it'\\''s'") + }) + + test("#given string with colon and slash #when shellEscape is called with unix #then returns unquoted", () => { + const result = shellEscape("/usr/bin:/bin", "unix") + expect(result).toBe("/usr/bin:/bin") + }) + + test("#given string with newline #when shellEscape is called with unix #then preserves newline in quotes", () => { + const result = shellEscape("line1\nline2", "unix") + expect(result).toBe("'line1\nline2'") + }) + }) + + describe("powershell", () => { + test("#given plain alphanumeric string #when shellEscape is called with powershell #then wraps in single quotes", () => { + const result = shellEscape("simple123", "powershell") + expect(result).toBe("'simple123'") + }) + + test("#given empty string #when shellEscape is called with powershell #then returns single quotes", () => { + const result = shellEscape("", "powershell") + expect(result).toBe("''") + }) + + test("#given string with spaces #when shellEscape is called with powershell #then wraps in single quotes", () => { + const result = shellEscape("has spaces", "powershell") + expect(result).toBe("'has spaces'") + }) + + test("#given string with single quote #when shellEscape is called with powershell #then escapes with double quote", () => { + const result = shellEscape("it's", "powershell") + expect(result).toBe("'it''s'") + }) + + test("#given string with dollar sign #when shellEscape is called with powershell #then wraps to prevent expansion", () => { + const result = shellEscape("$var", "powershell") + expect(result).toBe("'$var'") + }) + + test("#given Windows path with backslashes #when shellEscape is called with powershell #then preserves backslashes", () => { + const result = shellEscape("C:\\path", "powershell") + expect(result).toBe("'C:\\path'") + }) + + test("#given string with colon #when shellEscape is called with powershell #then wraps in quotes", () => { + const result = shellEscape("key:value", "powershell") + expect(result).toBe("'key:value'") + }) + }) + + describe("cmd.exe", () => { + test("#given plain alphanumeric string #when shellEscape is called with cmd #then wraps in double quotes", () => { + const result = shellEscape("simple123", "cmd") + expect(result).toBe('"simple123"') + }) + + test("#given empty string #when shellEscape is called with cmd #then returns double quotes", () => { + const result = shellEscape("", "cmd") + expect(result).toBe('""') + }) + + test("#given string with spaces #when shellEscape is called with cmd #then wraps in double quotes", () => { + const result = shellEscape("has spaces", "cmd") + expect(result).toBe('"has spaces"') + }) + + test("#given string with double quote #when shellEscape is called with cmd #then escapes with double quote", () => { + const result = shellEscape('say "hello"', "cmd") + expect(result).toBe('"say ""hello"""') + }) + + test("#given string with percent signs #when shellEscape is called with cmd #then escapes percent signs", () => { + const result = shellEscape("%PATH%", "cmd") + expect(result).toBe('"%%PATH%%"') + }) + + test("#given Windows path with backslashes #when shellEscape is called with cmd #then preserves backslashes", () => { + const result = shellEscape("C:\\path", "cmd") + expect(result).toBe('"C:\\path"') + }) + + test("#given string with colon #when shellEscape is called with cmd #then wraps in double quotes", () => { + const result = shellEscape("key:value", "cmd") + expect(result).toBe('"key:value"') + }) + }) + }) + + describe("buildEnvPrefix", () => { + describe("unix shell", () => { + test("#given single environment variable #when buildEnvPrefix is called with unix #then builds export statement", () => { + const result = buildEnvPrefix({ VAR: "value" }, "unix") + expect(result).toBe("export VAR=value;") + }) + + test("#given multiple environment variables #when buildEnvPrefix is called with unix #then builds export statement with all vars", () => { + const result = buildEnvPrefix({ VAR1: "val1", VAR2: "val2" }, "unix") + expect(result).toBe("export VAR1=val1 VAR2=val2;") + }) + + test("#given env var with special chars #when buildEnvPrefix is called with unix #then escapes value", () => { + const result = buildEnvPrefix({ PATH: "/usr/bin:/bin" }, "unix") + expect(result).toBe("export PATH=/usr/bin:/bin;") + }) + + test("#given env var with spaces #when buildEnvPrefix is called with unix #then escapes with quotes", () => { + const result = buildEnvPrefix({ MSG: "has spaces" }, "unix") + expect(result).toBe("export MSG='has spaces';") + }) + + test("#given empty env object #when buildEnvPrefix is called with unix #then returns empty string", () => { + const result = buildEnvPrefix({}, "unix") + expect(result).toBe("") + }) + }) + + describe("powershell", () => { + test("#given single environment variable #when buildEnvPrefix is called with powershell #then builds $env assignment", () => { + const result = buildEnvPrefix({ VAR: "value" }, "powershell") + expect(result).toBe("$env:VAR='value';") + }) + + test("#given multiple environment variables #when buildEnvPrefix is called with powershell #then builds multiple assignments", () => { + const result = buildEnvPrefix({ VAR1: "val1", VAR2: "val2" }, "powershell") + expect(result).toBe("$env:VAR1='val1'; $env:VAR2='val2';") + }) + + test("#given env var with special chars #when buildEnvPrefix is called with powershell #then escapes value", () => { + const result = buildEnvPrefix({ MSG: "it's working" }, "powershell") + expect(result).toBe("$env:MSG='it''s working';") + }) + + test("#given env var with dollar sign #when buildEnvPrefix is called with powershell #then escapes to prevent expansion", () => { + const result = buildEnvPrefix({ VAR: "$test" }, "powershell") + expect(result).toBe("$env:VAR='$test';") + }) + + test("#given empty env object #when buildEnvPrefix is called with powershell #then returns empty string", () => { + const result = buildEnvPrefix({}, "powershell") + expect(result).toBe("") + }) + }) + + describe("cmd.exe", () => { + test("#given single environment variable #when buildEnvPrefix is called with cmd #then builds set command", () => { + const result = buildEnvPrefix({ VAR: "value" }, "cmd") + expect(result).toBe('set VAR="value" &&') + }) + + test("#given multiple environment variables #when buildEnvPrefix is called with cmd #then builds multiple set commands", () => { + const result = buildEnvPrefix({ VAR1: "val1", VAR2: "val2" }, "cmd") + expect(result).toBe('set VAR1="val1" && set VAR2="val2" &&') + }) + + test("#given env var with special chars #when buildEnvPrefix is called with cmd #then escapes value", () => { + const result = buildEnvPrefix({ MSG: "has spaces" }, "cmd") + expect(result).toBe('set MSG="has spaces" &&') + }) + + test("#given env var with double quotes #when buildEnvPrefix is called with cmd #then escapes quotes", () => { + const result = buildEnvPrefix({ MSG: 'say "hello"' }, "cmd") + expect(result).toBe('set MSG="say ""hello""" &&') + }) + + test("#given empty env object #when buildEnvPrefix is called with cmd #then returns empty string", () => { + const result = buildEnvPrefix({}, "cmd") + expect(result).toBe("") + }) + }) + }) +}) diff --git a/src/shared/shell-env.ts b/src/shared/shell-env.ts new file mode 100644 index 00000000..b074baf5 --- /dev/null +++ b/src/shared/shell-env.ts @@ -0,0 +1,111 @@ +export type ShellType = "unix" | "powershell" | "cmd" + +/** + * Detect the current shell type based on environment variables. + * + * Detection priority: + * 1. PSModulePath → PowerShell + * 2. SHELL env var → Unix shell + * 3. Platform fallback → win32: cmd, others: unix + */ +export function detectShellType(): ShellType { + if (process.env.PSModulePath) { + return "powershell" + } + + if (process.env.SHELL) { + return "unix" + } + + return process.platform === "win32" ? "cmd" : "unix" +} + +/** + * Shell-escape a value for use in environment variable assignment. + * + * @param value - The value to escape + * @param shellType - The target shell type + * @returns Escaped value appropriate for the shell + */ +export function shellEscape(value: string, shellType: ShellType): string { + if (value === "") { + return shellType === "cmd" ? '""' : "''" + } + + switch (shellType) { + case "unix": + if (/[^a-zA-Z0-9_\-.:\/]/.test(value)) { + return `'${value.replace(/'/g, "'\\''")}'` + } + return value + + case "powershell": + return `'${value.replace(/'/g, "''")}'` + + case "cmd": + // Escape % first (for environment variable expansion), then " (for quoting) + return `"${value.replace(/%/g, '%%').replace(/"/g, '""')}"` + + default: + return value + } +} + +/** + * Build environment variable prefix command for the target shell. + * + * @param env - Record of environment variables to set + * @param shellType - The target shell type + * @returns Command prefix string to prepend to the actual command + * + * @example + * ```ts + * // Unix: "export VAR1=val1 VAR2=val2; command" + * buildEnvPrefix({ VAR1: "val1", VAR2: "val2" }, "unix") + * // => "export VAR1=val1 VAR2=val2;" + * + * // PowerShell: "$env:VAR1='val1'; $env:VAR2='val2'; command" + * buildEnvPrefix({ VAR1: "val1", VAR2: "val2" }, "powershell") + * // => "$env:VAR1='val1'; $env:VAR2='val2';" + * + * // cmd.exe: "set VAR1=val1 && set VAR2=val2 && command" + * buildEnvPrefix({ VAR1: "val1", VAR2: "val2" }, "cmd") + * // => "set VAR1=\"val1\" && set VAR2=\"val2\" &&" + * ``` + */ +export function buildEnvPrefix( + env: Record, + shellType: ShellType +): string { + const entries = Object.entries(env) + + if (entries.length === 0) { + return "" + } + + switch (shellType) { + case "unix": { + const assignments = entries + .map(([key, value]) => `${key}=${shellEscape(value, shellType)}`) + .join(" ") + return `export ${assignments};` + } + + case "powershell": { + const assignments = entries + .map(([key, value]) => `$env:${key}=${shellEscape(value, shellType)}`) + .join("; ") + return `${assignments};` + } + + case "cmd": { + const assignments = entries + .map(([key, value]) => `set ${key}=${shellEscape(value, shellType)}`) + .join(" && ") + return `${assignments} &&` + } + + default: + return "" + } +}