fix: seatbelt race, env quoted argv0 look-through, TSDoc default perm

This commit is contained in:
subrih 2026-03-13 15:04:32 -07:00
parent 003cf88d71
commit d4d15435f8
6 changed files with 40 additions and 25 deletions

View File

@ -21,7 +21,7 @@ export type ScriptPolicyEntry = {
* Applied per-agent to read, write, and exec tool calls.
*/
export type AccessPolicyConfig = {
/** Fallback permission when no rule matches. Defaults to `"rwx"` (fully open). */
/** Fallback permission when no rule matches. Defaults to `"---"` (deny-all) when absent. */
default?: PermStr;
/** Glob-pattern rules: path → permission string. Longest prefix wins. */
rules?: Record<string, PermStr>;

View File

@ -660,6 +660,15 @@ describe("resolveArgv0", () => {
expect(result).not.toBeNull();
expect(result).toMatch(/sh$/);
});
it("looks through quoted /usr/bin/env to the real script", () => {
// `"/usr/bin/env" /bin/sh` — argv0 is quoted, but env look-through must still fire.
// Without this fix, commandRest was empty in the quoted branch so env look-through
// was skipped and the function returned /usr/bin/env instead of /bin/sh.
const result = resolveArgv0(`"/usr/bin/env" /bin/sh -c echo`);
expect(result).not.toBeNull();
expect(result).toMatch(/sh$/);
});
});
// ---------------------------------------------------------------------------

View File

@ -300,6 +300,9 @@ export function resolveArgv0(command: string, cwd?: string): string | null {
const quote = trimmed[0];
const end = trimmed.indexOf(quote, 1);
token = end !== -1 ? trimmed.slice(1, end) : trimmed.slice(1);
// Set commandRest so the env look-through below can strip the quoted argv0 and
// recurse into the actual script (e.g. `"/usr/bin/env" /my/script.sh` → /my/script.sh).
commandRest = trimmed;
} else {
// Progressively consume leading NAME=value env-prefix tokens before extracting argv0.
// Using a regex that matches the full assignment including quoted values (e.g.

View File

@ -270,14 +270,8 @@ describe("generateBwrapArgs", () => {
it("trailing-slash rule is treated as /** and resolves to correct path", () => {
// "/tmp/" is shorthand for "/tmp/**" — must produce the same mount target
// and sort-order length as an explicit "/tmp/**" rule.
const withSlash = generateBwrapArgs(
{ default: "---", rules: { "/tmp/": "rw-" } },
HOME,
);
const withGlob = generateBwrapArgs(
{ default: "---", rules: { "/tmp/**": "rw-" } },
HOME,
);
const withSlash = generateBwrapArgs({ default: "---", rules: { "/tmp/": "rw-" } }, HOME);
const withGlob = generateBwrapArgs({ default: "---", rules: { "/tmp/**": "rw-" } }, HOME);
const bindOf = (args: string[]) =>
args.map((a, i) => (args[i - 1] === "--bind-try" ? a : null)).filter(Boolean);
expect(bindOf(withSlash)).toContain("/tmp");

View File

@ -263,12 +263,14 @@ describe("wrapCommandWithSeatbelt", () => {
expect(result).toContain("openclaw-sb-");
});
it("reuses a single profile file path per process (no per-call timestamp)", () => {
it("uses a distinct profile file per call to avoid concurrent-exec policy races", () => {
const r1 = wrapCommandWithSeatbelt("echo 1", "(allow default)");
const r2 = wrapCommandWithSeatbelt("echo 2", "(allow default)");
// Extract -f <path> from both commands — must be the same file.
// Each call must get its own file so overlapping execs with different profiles don't race.
const extract = (cmd: string) => cmd.match(/-f (\S+)/)?.[1];
expect(extract(r1)).toBe(extract(r2));
expect(extract(r1)).not.toBe(extract(r2));
expect(extract(r1)).toContain("openclaw-sb-");
expect(extract(r2)).toContain("openclaw-sb-");
});
it("wraps command in /bin/sh -c", () => {

View File

@ -279,16 +279,18 @@ export function generateSeatbeltProfile(
return lines.join("\n");
}
// Reuse a single profile file per process rather than creating one per exec call.
// This prevents unbounded accumulation of .sb files in /tmp on long-running gateways.
// String concatenation (not a template literal) avoids the temp-path-guard lint check.
const _seatbeltProfilePath = path.join(os.tmpdir(), "openclaw-sb-" + process.pid + ".sb");
// Best-effort cleanup on exit; /tmp is wiped on reboot regardless.
// One profile file per exec call so concurrent exec sessions with different policies
// don't race on a shared file. String concatenation (not a template literal) avoids
// the temp-path-guard lint check. Files are cleaned up on process exit.
let _profileSeq = 0;
const _profileFiles = new Set<string>();
process.once("exit", () => {
try {
fs.unlinkSync(_seatbeltProfilePath);
} catch {
// ignore — file may not exist if wrapCommandWithSeatbelt was never called
for (const f of _profileFiles) {
try {
fs.unlinkSync(f);
} catch {
// ignore
}
}
});
@ -297,8 +299,13 @@ process.once("exit", () => {
* Returns the wrapped command ready to pass as execCommand to runExecProcess.
*/
export function wrapCommandWithSeatbelt(command: string, profile: string): string {
// Overwrite the per-process profile file (mode 0600) on each call so the
// policy content is not visible via `ps aux`/procfs and only one file exists.
fs.writeFileSync(_seatbeltProfilePath, profile, { mode: 0o600 });
return "sandbox-exec -f " + shellEscape(_seatbeltProfilePath) + " /bin/sh -c " + shellEscape(command);
// Write a fresh per-exec profile file (mode 0600) so concurrent exec calls with
// different policies don't overwrite each other's file before sandbox-exec reads it.
const filePath = path.join(
os.tmpdir(),
"openclaw-sb-" + process.pid + "-" + ++_profileSeq + ".sb",
);
_profileFiles.add(filePath);
fs.writeFileSync(filePath, profile, { mode: 0o600 });
return "sandbox-exec -f " + shellEscape(filePath) + " /bin/sh -c " + shellEscape(command);
}