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
This commit is contained in:
Pavan Kumar Gondhi
2026-05-15 14:55:37 +05:30
committed by GitHub
parent 238b0fc76f
commit b9fbc57bbd
4 changed files with 140 additions and 20 deletions

View File

@@ -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.

View File

@@ -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;

View File

@@ -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);
});
}
});
});

View File

@@ -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<string>).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))) {