From 43557668d2400ac02e436693aaa40d96ced178f8 Mon Sep 17 00:00:00 2001 From: scoootscooob Date: Mon, 23 Mar 2026 00:14:09 -0700 Subject: [PATCH] Infra: support shell carrier allow-always approvals --- src/infra/exec-approvals-allow-always.test.ts | 28 ++++++++ src/infra/exec-approvals-allowlist.ts | 68 ++++++++++++++++++- test/vitest-scoped-config.test.ts | 17 +++-- vitest.extensions.config.ts | 13 ++-- vitest.gateway.config.ts | 10 ++- 5 files changed, 120 insertions(+), 16 deletions(-) diff --git a/src/infra/exec-approvals-allow-always.test.ts b/src/infra/exec-approvals-allow-always.test.ts index 114bf002c65..72479ec3e4b 100644 --- a/src/infra/exec-approvals-allow-always.test.ts +++ b/src/infra/exec-approvals-allow-always.test.ts @@ -221,6 +221,34 @@ describe("resolveAllowAlwaysPatterns", () => { }); }); + it("persists carried executables for shell-wrapper positional argv carriers", () => { + if (process.platform === "win32") { + return; + } + const dir = makeTempDir(); + const touch = makeExecutable(dir, "touch"); + const env = { PATH: `${dir}${path.delimiter}${process.env.PATH ?? ""}` }; + const safeBins = resolveSafeBins(undefined); + + const { persisted } = resolvePersistedPatterns({ + command: `sh -lc '$0 "$1"' touch ${path.join(dir, "marker")}`, + dir, + env, + safeBins, + }); + expect(persisted).toEqual([touch]); + + const second = evaluateShellAllowlist({ + command: `sh -lc '$0 "$1"' touch ${path.join(dir, "second-marker")}`, + allowlist: [{ pattern: touch }], + safeBins, + cwd: dir, + env, + platform: process.platform, + }); + expect(second.allowlistSatisfied).toBe(true); + }); + it("does not treat inline shell commands as persisted script paths", () => { if (process.platform === "win32") { return; diff --git a/src/infra/exec-approvals-allowlist.ts b/src/infra/exec-approvals-allowlist.ts index 5467fa45b41..e7bfabc333d 100644 --- a/src/infra/exec-approvals-allowlist.ts +++ b/src/infra/exec-approvals-allowlist.ts @@ -21,9 +21,11 @@ import { isTrustedSafeBinPath } from "./exec-safe-bin-trust.js"; import { extractShellWrapperInlineCommand, isShellWrapperExecutable, + normalizeExecutableToken, } from "./exec-wrapper-resolution.js"; import { resolveExecWrapperTrustPlan } from "./exec-wrapper-trust-plan.js"; import { expandHomePrefix } from "./home-dir.js"; +import { POSIX_INLINE_COMMAND_FLAGS, resolveInlineCommandMatch } from "./shell-inline-command.js"; function hasShellLineContinuation(command: string): boolean { return /\\(?:\r\n|\n|\r)/.test(command); @@ -113,6 +115,7 @@ type ExecAllowlistContext = { safeBins: Set; safeBinProfiles?: Readonly>; cwd?: string; + env?: NodeJS.ProcessEnv; platform?: string | null; trustedSafeBinDirs?: ReadonlySet; skillBins?: readonly SkillBinTrustEntry[]; @@ -125,6 +128,7 @@ function pickExecAllowlistContext(params: ExecAllowlistContext): ExecAllowlistCo safeBins: params.safeBins, safeBinProfiles: params.safeBinProfiles, cwd: params.cwd, + env: params.env, platform: params.platform, trustedSafeBinDirs: params.trustedSafeBinDirs, skillBins: params.skillBins, @@ -224,6 +228,18 @@ function evaluateSegments( : segment.resolution; const executableMatch = matchAllowlist(params.allowlist, candidateResolution); const inlineCommand = extractShellWrapperInlineCommand(allowlistSegment.argv); + const shellPositionalArgvCandidatePath = resolveShellWrapperPositionalArgvCandidatePath({ + segment: allowlistSegment, + cwd: params.cwd, + env: params.env, + }); + const shellPositionalArgvMatch = shellPositionalArgvCandidatePath + ? matchAllowlist(params.allowlist, { + rawExecutable: shellPositionalArgvCandidatePath, + resolvedPath: shellPositionalArgvCandidatePath, + executableName: path.basename(shellPositionalArgvCandidatePath), + }) + : null; const shellScriptCandidatePath = inlineCommand === null ? resolveShellWrapperScriptCandidatePath({ @@ -238,7 +254,7 @@ function evaluateSegments( executableName: path.basename(shellScriptCandidatePath), }) : null; - const match = executableMatch ?? shellScriptMatch; + const match = executableMatch ?? shellPositionalArgvMatch ?? shellScriptMatch; if (match) { matches.push(match); } @@ -408,6 +424,47 @@ function resolveShellWrapperScriptCandidatePath(params: { return path.resolve(base, expanded); } +function resolveShellWrapperPositionalArgvCandidatePath(params: { + segment: ExecCommandSegment; + cwd?: string; + env?: NodeJS.ProcessEnv; +}): string | undefined { + if (!isShellWrapperSegment(params.segment)) { + return undefined; + } + + const argv = params.segment.argv; + if (!Array.isArray(argv) || argv.length < 4) { + return undefined; + } + + const wrapper = normalizeExecutableToken(argv[0] ?? ""); + if (!["ash", "bash", "dash", "fish", "ksh", "sh", "zsh"].includes(wrapper)) { + return undefined; + } + + const inlineMatch = resolveInlineCommandMatch(argv, POSIX_INLINE_COMMAND_FLAGS, { + allowCombinedC: true, + }); + if (inlineMatch.valueTokenIndex === null || !inlineMatch.command) { + return undefined; + } + if (!/(?:^|[^\\$])\$(?:0|\{0\})/.test(inlineMatch.command)) { + return undefined; + } + + const carriedExecutable = argv + .slice(inlineMatch.valueTokenIndex + 1) + .map((token) => token.trim()) + .find((token) => token.length > 0); + if (!carriedExecutable) { + return undefined; + } + + const resolution = resolveCommandResolutionFromArgv([carriedExecutable], params.cwd, params.env); + return resolveAllowlistCandidatePath(resolution, params.cwd); +} + function collectAllowAlwaysPatterns(params: { segment: ExecCommandSegment; cwd?: string; @@ -441,6 +498,15 @@ function collectAllowAlwaysPatterns(params: { params.out.add(candidatePath); return; } + const positionalArgvPath = resolveShellWrapperPositionalArgvCandidatePath({ + segment, + cwd: params.cwd, + env: params.env, + }); + if (positionalArgvPath) { + params.out.add(positionalArgvPath); + return; + } const inlineCommand = trustPlan.shellInlineCommand ?? extractShellWrapperInlineCommand(segment.argv); if (!inlineCommand) { diff --git a/test/vitest-scoped-config.test.ts b/test/vitest-scoped-config.test.ts index c33a30530c2..bd9dc99745b 100644 --- a/test/vitest-scoped-config.test.ts +++ b/test/vitest-scoped-config.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import channelsConfig from "../vitest.channels.config.ts"; -import extensionsConfig from "../vitest.extensions.config.ts"; -import gatewayConfig from "../vitest.gateway.config.ts"; +import { createExtensionsVitestConfig } from "../vitest.extensions.config.ts"; +import { createGatewayVitestConfig } from "../vitest.gateway.config.ts"; import { createScopedVitestConfig, resolveVitestIsolation } from "../vitest.scoped-config.ts"; describe("resolveVitestIsolation", () => { @@ -42,21 +42,24 @@ describe("createScopedVitestConfig", () => { }); describe("scoped vitest configs", () => { + const defaultExtensionsConfig = createExtensionsVitestConfig({}); + const defaultGatewayConfig = createGatewayVitestConfig(); + it("defaults channel tests to non-isolated mode", () => { expect(channelsConfig.test?.isolate).toBe(false); }); it("defaults extension tests to non-isolated mode", () => { - expect(extensionsConfig.test?.isolate).toBe(false); + expect(defaultExtensionsConfig.test?.isolate).toBe(false); }); it("normalizes extension include patterns relative to the scoped dir", () => { - expect(extensionsConfig.test?.dir).toBe("extensions"); - expect(extensionsConfig.test?.include).toEqual(["**/*.test.ts"]); + expect(defaultExtensionsConfig.test?.dir).toBe("extensions"); + expect(defaultExtensionsConfig.test?.include).toEqual(["**/*.test.ts"]); }); it("normalizes gateway include patterns relative to the scoped dir", () => { - expect(gatewayConfig.test?.dir).toBe("src/gateway"); - expect(gatewayConfig.test?.include).toEqual(["**/*.test.ts"]); + expect(defaultGatewayConfig.test?.dir).toBe("src/gateway"); + expect(defaultGatewayConfig.test?.include).toEqual(["**/*.test.ts"]); }); }); diff --git a/vitest.extensions.config.ts b/vitest.extensions.config.ts index d63c83bf27b..40febcbe0f3 100644 --- a/vitest.extensions.config.ts +++ b/vitest.extensions.config.ts @@ -20,9 +20,10 @@ export function loadIncludePatternsFromEnv( return loadPatternListFile(includeFile, "OPENCLAW_VITEST_INCLUDE_FILE"); } -export default createScopedVitestConfig( - loadIncludePatternsFromEnv() ?? ["extensions/**/*.test.ts"], - { +export function createExtensionsVitestConfig( + env: Record = process.env, +) { + return createScopedVitestConfig(loadIncludePatternsFromEnv(env) ?? ["extensions/**/*.test.ts"], { dir: "extensions", pool: "threads", passWithNoTests: true, @@ -30,5 +31,7 @@ export default createScopedVitestConfig( // vitest.channels.config.ts (pnpm test:channels) which provides // the heavier mock scaffolding they need. exclude: channelTestExclude.filter((pattern) => pattern.startsWith("extensions/")), - }, -); + }); +} + +export default createExtensionsVitestConfig(); diff --git a/vitest.gateway.config.ts b/vitest.gateway.config.ts index 4e7d2fc20f5..e18315d68ef 100644 --- a/vitest.gateway.config.ts +++ b/vitest.gateway.config.ts @@ -1,5 +1,9 @@ import { createScopedVitestConfig } from "./vitest.scoped-config.ts"; -export default createScopedVitestConfig(["src/gateway/**/*.test.ts"], { - dir: "src/gateway", -}); +export function createGatewayVitestConfig() { + return createScopedVitestConfig(["src/gateway/**/*.test.ts"], { + dir: "src/gateway", + }); +} + +export default createGatewayVitestConfig();