From b9fbc57bbd040ee642f3f56073b8cf75d0988179 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Gondhi Date: Fri, 15 May 2026 14:55:37 +0530 Subject: [PATCH] Bind shell script operands after combined options [AI] (#81882) * fix: bind shell script operands after combined options * addressing codex review * docs: add changelog entry for PR merge --- CHANGELOG.md | 1 + src/infra/shell-inline-command.ts | 2 +- src/node-host/invoke-system-run-plan.test.ts | 137 ++++++++++++++++--- src/node-host/invoke-system-run-plan.ts | 20 ++- 4 files changed, 140 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3eacbe1ee1a..b1e372ee27a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Bind shell script operands after combined options [AI]. (#81882) Thanks @pgondhi987. - fix(canvas): validate snapshot response formats [AI]. (#81881) Thanks @pgondhi987. - Constrain provider catalog entry paths [AI]. (#81884) Thanks @pgondhi987. - Require canonical node platform IDs [AI]. (#81880) Thanks @pgondhi987. diff --git a/src/infra/shell-inline-command.ts b/src/infra/shell-inline-command.ts index 89449439bea..ed6ff7a8045 100644 --- a/src/infra/shell-inline-command.ts +++ b/src/infra/shell-inline-command.ts @@ -135,7 +135,7 @@ function isPosixShortOption(token: string, option: string): boolean { return hasOption; } -function advancePosixInlineOptionScan(token: string): number { +export function advancePosixInlineOptionScan(token: string): number { const combinedValueCount = combinedSeparateValueOptionCount(token); if (combinedValueCount > 0) { return 1 + combinedValueCount; diff --git a/src/node-host/invoke-system-run-plan.test.ts b/src/node-host/invoke-system-run-plan.test.ts index cd692181039..4d80fa8e6e9 100644 --- a/src/node-host/invoke-system-run-plan.test.ts +++ b/src/node-host/invoke-system-run-plan.test.ts @@ -965,22 +965,127 @@ describe("hardenApprovedExecutionPaths", () => { }); it("captures the real shell script operand after value-taking shell flags", () => { - const tmp = createFixtureDir("openclaw-shell-option-value-"); - const scriptPath = path.join(tmp, "run.sh"); - fs.writeFileSync(scriptPath, "#!/bin/sh\necho SAFE\n"); - fs.writeFileSync(path.join(tmp, "errexit"), "decoy\n"); - const snapshot = resolveMutableFileOperandSnapshotSync({ - argv: ["/bin/bash", "-o", "errexit", "./run.sh"], - cwd: tmp, - shellCommand: null, - }); - expect(snapshot).toEqual({ - ok: true, - snapshot: { - argvIndex: 3, - path: fs.realpathSync(scriptPath), - sha256: sha256FileSync(scriptPath), + const cases = [ + { + name: "separate set option", + argv: ["/bin/bash", "-o", "errexit", "./run.sh"], + decoyName: "errexit", + expectedArgvIndex: 3, }, - }); + { + name: "combined set option", + argv: ["/bin/bash", "-eo", "pipefail", "./run.sh"], + decoyName: "pipefail", + expectedArgvIndex: 3, + }, + { + name: "combined trace option", + argv: ["/bin/bash", "-xo", "errexit", "./run.sh"], + decoyName: "errexit", + expectedArgvIndex: 3, + }, + { + name: "combined unset option", + argv: ["/bin/bash", "-uo", "nounset", "./run.sh"], + decoyName: "nounset", + expectedArgvIndex: 3, + }, + { + name: "plus set option", + argv: ["/bin/bash", "+o", "histexpand", "./run.sh"], + decoyName: "histexpand", + expectedArgvIndex: 3, + }, + { + name: "plus shopt option", + argv: ["/bin/bash", "+O", "extglob", "./run.sh"], + decoyName: "extglob", + expectedArgvIndex: 3, + }, + { + name: "combined plus set option", + argv: ["/bin/bash", "+eo", "pipefail", "./run.sh"], + decoyName: "pipefail", + expectedArgvIndex: 3, + }, + ]; + + for (const testCase of cases) { + runNamedCase(testCase.name, () => { + const tmp = createFixtureDir("openclaw-shell-option-value-"); + const scriptPath = path.join(tmp, "run.sh"); + fs.writeFileSync(scriptPath, "#!/bin/sh\necho SAFE\n"); + fs.writeFileSync(path.join(tmp, testCase.decoyName), "decoy\n"); + const snapshot = resolveMutableFileOperandSnapshotSync({ + argv: testCase.argv, + cwd: tmp, + shellCommand: null, + }); + expect(snapshot).toEqual({ + ok: true, + snapshot: { + argvIndex: testCase.expectedArgvIndex, + path: fs.realpathSync(scriptPath), + sha256: sha256FileSync(scriptPath), + }, + }); + if (!snapshot.ok || snapshot.snapshot === null) { + throw new Error("expected mutable file operand snapshot"); + } + fs.writeFileSync(scriptPath, "#!/bin/sh\necho CHANGED\n"); + expect( + revalidateApprovedMutableFileOperand({ + snapshot: snapshot.snapshot, + argv: testCase.argv, + cwd: tmp, + }), + ).toBe(false); + }); + } + }); + + it("captures fish script operands with plus-prefixed filenames", () => { + const cases = [ + { + name: "plus-prefixed fish script", + argv: ["fish", "+setup.fish"], + }, + { + name: "plus-prefixed fish script before script args", + argv: ["fish", "+setup.fish", "-c", "echo arg"], + }, + ]; + + for (const testCase of cases) { + runNamedCase(testCase.name, () => { + const tmp = createFixtureDir("openclaw-fish-plus-script-"); + const scriptPath = path.join(tmp, "+setup.fish"); + fs.writeFileSync(scriptPath, "echo SAFE\n"); + const snapshot = resolveMutableFileOperandSnapshotSync({ + argv: testCase.argv, + cwd: tmp, + shellCommand: null, + }); + expect(snapshot).toEqual({ + ok: true, + snapshot: { + argvIndex: 1, + path: fs.realpathSync(scriptPath), + sha256: sha256FileSync(scriptPath), + }, + }); + if (!snapshot.ok || snapshot.snapshot === null) { + throw new Error("expected mutable file operand snapshot"); + } + fs.writeFileSync(scriptPath, "echo CHANGED\n"); + expect( + revalidateApprovedMutableFileOperand({ + snapshot: snapshot.snapshot, + argv: testCase.argv, + cwd: tmp, + }), + ).toBe(false); + }); + } }); }); diff --git a/src/node-host/invoke-system-run-plan.ts b/src/node-host/invoke-system-run-plan.ts index 40fbe0ce3d3..cb1ce27ff60 100644 --- a/src/node-host/invoke-system-run-plan.ts +++ b/src/node-host/invoke-system-run-plan.ts @@ -16,6 +16,7 @@ import { } from "../infra/exec-wrapper-resolution.js"; import { sameFileIdentity } from "../infra/fs-safe-advanced.js"; import { + advancePosixInlineOptionScan, POSIX_INLINE_COMMAND_FLAGS, resolveInlineCommandMatch, } from "../infra/shell-inline-command.js"; @@ -156,9 +157,18 @@ const POSIX_SHELL_OPTIONS_WITH_VALUE = new Set([ "--init-file", "--rcfile", "--startup-script", + "-O", "-o", + "+O", + "+o", ]); +const POSIX_SHELLS_WITH_PLUS_OPTIONS = new Set(["ash", "bash", "dash", "ksh", "sh", "zsh"]); + +function isPosixShellOptionToken(token: string, supportsPlusOptions: boolean): boolean { + return token.startsWith("-") || (supportsPlusOptions && token.startsWith("+")); +} + const NPM_EXEC_OPTIONS_WITH_VALUE = new Set([ "--cache", "--package", @@ -580,10 +590,13 @@ function unwrapNpmExecInvocation(argv: string[]): string[] | null { return unwrapDirectPackageExecInvocation(["npx", ...tail]); } -function resolvePosixShellScriptOperandIndex(argv: string[]): number | null { +function resolvePosixShellScriptOperandIndex(argv: string[], executable: string): number | null { + const supportsPlusOptions = POSIX_SHELLS_WITH_PLUS_OPTIONS.has(executable); if ( resolveInlineCommandMatch(argv, POSIX_INLINE_COMMAND_FLAGS, { allowCombinedC: true, + isOptionToken: (token) => isPosixShellOptionToken(token, supportsPlusOptions), + stopAtFirstNonOption: true, }).valueTokenIndex !== null ) { return null; @@ -604,7 +617,7 @@ function resolvePosixShellScriptOperandIndex(argv: string[]): number | null { if (!afterDoubleDash && token === "-s") { return null; } - if (!afterDoubleDash && token.startsWith("-")) { + if (!afterDoubleDash && isPosixShellOptionToken(token, supportsPlusOptions)) { const flag = normalizeOptionFlag(token); if (POSIX_SHELL_OPTIONS_WITH_VALUE.has(flag)) { if (!token.includes("=")) { @@ -612,6 +625,7 @@ function resolvePosixShellScriptOperandIndex(argv: string[]): number | null { } continue; } + i += advancePosixInlineOptionScan(token) - 1; continue; } return i; @@ -866,7 +880,7 @@ function resolveMutableFileOperandIndex(argv: string[], cwd: string | undefined) return null; } if ((POSIX_SHELL_WRAPPERS as ReadonlySet).has(executable)) { - const shellIndex = resolvePosixShellScriptOperandIndex(unwrapped.argv); + const shellIndex = resolvePosixShellScriptOperandIndex(unwrapped.argv, executable); return shellIndex === null ? null : unwrapped.baseIndex + shellIndex; } if (MUTABLE_ARGV1_INTERPRETER_PATTERNS.some((pattern) => pattern.test(executable))) {