Skip to content

Commit 01f36b0

Browse files
committed
refactor(shell): share transparent command peeling
1 parent 6d07b36 commit 01f36b0

5 files changed

Lines changed: 716 additions & 76 deletions

File tree

‎src/permission/classify-security.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,16 @@ describe("content inside an env -S payload never receives a weaker tier than it
780780
);
781781
expect(rule?.name).toBe("dependency-install");
782782
});
783+
784+
test("trailing env terminal flags remain arguments to split payloads", () => {
785+
expect(
786+
autoShellRuleForCall(shellCall(`env -S "rm -rf /" --version`))?.name,
787+
).toBe("recursive-rm");
788+
expect(
789+
autoShellRuleForCall(shellCall(`env -S "npm install left-pad" --help`))
790+
?.name,
791+
).toBe("dependency-install");
792+
});
783793
});
784794

785795
describe("upload-shaped network shell commands force ask in auto mode", () => {

‎src/shell/run-shell-authz.test.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,35 @@ describe("authz hard-deny peels glued and trailing env -S forms", () => {
388388
);
389389
});
390390

391+
test("split payloads own trailing terminal-looking arguments", () => {
392+
const cases = [
393+
{
394+
command: `env -S "find ." --help`,
395+
subject: "find . --help",
396+
reason: openEnded,
397+
},
398+
{
399+
command: `env -S "rm -rf /" --version`,
400+
subject: "rm -rf / --version",
401+
reason: destructive,
402+
},
403+
{
404+
command: `env -S "npm install left-pad" --help`,
405+
subject: "npm install left-pad --help",
406+
},
407+
];
408+
409+
for (const { command, subject, reason } of cases) {
410+
expect(expandShellSubjects(command)).toEqual({
411+
subjects: [command, subject],
412+
opaque: false,
413+
});
414+
if (reason !== undefined) {
415+
expect(runShellAuthzBlockReason(command)).toMatch(reason);
416+
}
417+
}
418+
});
419+
391420
test("G7: soft-allow non-catastrophic rm inside -S is not hard-denied", () => {
392421
expect(
393422
runShellAuthzBlockReason(`env -S "rm -rf node_modules"`),
@@ -433,6 +462,71 @@ describe("authz hard-deny peels glued and trailing env -S forms", () => {
433462
);
434463
});
435464

465+
test("transparent time options and end-of-options expose hard-denied utilities", () => {
466+
expect(runShellAuthzBlockReason(`time -p find .`)).toMatch(openEnded);
467+
expect(runShellAuthzBlockReason(`time -- find .`)).toMatch(openEnded);
468+
expect(runShellAuthzBlockReason(`/usr/bin/time -o report find .`)).toMatch(
469+
openEnded,
470+
);
471+
expect(runShellAuthzBlockReason(`/usr/bin/time -ao report find .`)).toMatch(
472+
openEnded,
473+
);
474+
expect(runShellAuthzBlockReason(`time -f %e find .`)).toMatch(openEnded);
475+
});
476+
477+
test("clustered env and timeout value options expose hard-denied utilities", () => {
478+
expect(runShellAuthzBlockReason(`env -iu PATH find .`)).toMatch(openEnded);
479+
expect(runShellAuthzBlockReason(`timeout -vs KILL 1 find .`)).toMatch(
480+
openEnded,
481+
);
482+
});
483+
484+
test("valid GNU timeout durations expose hard-denied utilities", () => {
485+
for (const duration of [
486+
".5s",
487+
"1",
488+
"1e3",
489+
"1e3s",
490+
"0x1p4",
491+
"2m",
492+
"3h",
493+
"4d",
494+
"inf",
495+
"infinity",
496+
]) {
497+
expect(runShellAuthzBlockReason(`timeout ${duration} find .`)).toMatch(
498+
openEnded,
499+
);
500+
}
501+
});
502+
503+
test("env value operands are parsed before terminal modes", () => {
504+
expect(runShellAuthzBlockReason(`env -u --help find .`)).toMatch(openEnded);
505+
expect(runShellAuthzBlockReason(`env -C --version find .`)).toMatch(
506+
openEnded,
507+
);
508+
});
509+
510+
test("env continues assignment parsing after end-of-options", () => {
511+
expect(runShellAuthzBlockReason(`env -- FILE=x find .`)).toMatch(openEnded);
512+
expect(runShellAuthzBlockReason(`env -i -- FILE=x find .`)).toMatch(
513+
openEnded,
514+
);
515+
});
516+
517+
test("terminal wrapper modes do not peel their operands as commands", () => {
518+
for (const command of [
519+
"command --help find .",
520+
"command -p -v find",
521+
"command -pv find",
522+
"env --help find",
523+
"nice --help find",
524+
"timeout --help find",
525+
]) {
526+
expect(runShellAuthzBlockReason(command)).toBeUndefined();
527+
}
528+
});
529+
436530
test("G13: empty/whitespace -S payload with trailing utility is hard-denied", () => {
437531
// Runtime still executes the trailing utility; do not opaque-drop it.
438532
expect(runShellAuthzBlockReason(`env -S " " find /`)).toMatch(openEnded);

‎src/shell/run-shell-authz.ts‎

Lines changed: 15 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22
// enforcement owner (hard deny at the top of its verdict path).
33

44
import { splitChainedCommand, tokenize } from "../permission/command.js";
5+
import {
6+
peelTransparentCommand,
7+
programBasename,
8+
} from "./transparent-command.js";
9+
10+
export { programBasename } from "./transparent-command.js";
511

612
function skipMatching(
713
tokens: readonly string[],
@@ -318,16 +324,6 @@ const RECURSIVE_FLAG = /^(--recursive|-[A-Za-z]*[rR][A-Za-z]*)$/;
318324
// Interpreters whose `-c` / `--command` payload is an independent shell subject.
319325
// Exported so tests and callers share one explicit list with the peeler.
320326
export const SHELL_INTERPRETERS = new Set(["bash", "sh", "zsh", "dash", "ksh"]);
321-
// Transparent prefixes that sit in front of a real program without changing it.
322-
const PREFIX_WRAPPERS = new Set([
323-
"command",
324-
"env",
325-
"builtin",
326-
"time",
327-
"nice",
328-
"nohup",
329-
"timeout",
330-
]);
331327
// Max recursive peel depth for nested wrappers. Exported so the depth cap is a
332328
// named policy knob tests can assert against, not a magic number.
333329
export const MAX_PEEL_DEPTH = 4;
@@ -375,12 +371,6 @@ function isDangerousTarget(token: string): boolean {
375371
return false;
376372
}
377373

378-
export function programBasename(token: string): string {
379-
const bare = token.replace(/['"]/g, "");
380-
const slash = bare.lastIndexOf("/");
381-
return slash >= 0 ? bare.slice(slash + 1) : bare;
382-
}
383-
384374
// Payload we cannot statically inspect: empty, a bare expansion, or a leading
385375
// command substitution. Argument-position expansions like `rm -rf $HOME` stay
386376
// parseable so catastrophic-target checks still fire.
@@ -684,7 +674,8 @@ function finishEnvSplitPayload(
684674
raw = payload;
685675
} else {
686676
if (isOpaquePayload(rest.join(" "))) return { kind: "opaque" };
687-
raw = rejoinTokens([payload, ...rest]);
677+
const trailing = rejoinTokens(rest);
678+
raw = trailing === null ? null : `${payload} ${trailing}`;
688679
}
689680
if (raw === null) return { kind: "opaque" };
690681
return peelEnvSplitUtility(raw);
@@ -811,66 +802,14 @@ function skipEnvFlagsAndAssignments(tokens: string[], start: number): number {
811802
// single segment.
812803
function peelOnce(segment: string): PeelOutcome {
813804
const tokens = tokenize(segment);
814-
let i = 0;
815-
i = skipMatching(tokens, i, (t) => ENV_ASSIGNMENT.test(t));
816-
817-
let strippedPrefix = false;
818-
while (i < tokens.length) {
819-
const current = tokens[i];
820-
if (current === undefined) break;
821-
const base = programBasename(current);
822-
if (base === "env") {
823-
// Prefer split-string peel: the whole payload is one quoted argument
824-
// that env re-splits itself, so the transparent-prefix path below
825-
// would only rejoin `-S '…'` and leave the real command invisible.
826-
const splitPeel = peelEnvSplitString(tokens, i + 1);
827-
if (splitPeel.kind !== "none") return splitPeel;
828-
strippedPrefix = true;
829-
i = skipEnvFlagsAndAssignments(tokens, i + 1);
830-
continue;
831-
}
832-
if (base === "timeout") {
833-
strippedPrefix = true;
834-
i++;
835-
// Optional duration (10, 30s, 1m, …) and common long/short flags.
836-
while (i < tokens.length) {
837-
const t = tokens[i];
838-
if (t === undefined) break;
839-
if (/^\d/.test(t)) {
840-
i++;
841-
continue;
842-
}
843-
if (t.startsWith("-") && t !== "-") {
844-
// Flags that take a value: -k / --kill-after / -s / --signal.
845-
if (
846-
t === "-k" ||
847-
t === "--kill-after" ||
848-
t === "-s" ||
849-
t === "--signal" ||
850-
t.startsWith("--kill-after=") ||
851-
t.startsWith("--signal=")
852-
) {
853-
i++;
854-
if (!t.includes("=") && i < tokens.length) {
855-
const next = tokens[i];
856-
if (next !== undefined && !next.startsWith("-")) i++;
857-
}
858-
continue;
859-
}
860-
i++;
861-
continue;
862-
}
863-
break;
864-
}
865-
continue;
866-
}
867-
if (PREFIX_WRAPPERS.has(base) && base !== "env" && base !== "timeout") {
868-
strippedPrefix = true;
869-
i++;
870-
continue;
871-
}
872-
break;
805+
const transparent = peelTransparentCommand(tokens);
806+
for (const wrapperIndex of transparent.wrapperIndexes) {
807+
if (programBasename(tokens[wrapperIndex] ?? "") !== "env") continue;
808+
const splitPeel = peelEnvSplitString(tokens, wrapperIndex + 1);
809+
if (splitPeel.kind !== "none") return splitPeel;
873810
}
811+
const i = transparent.executableIndex;
812+
const strippedPrefix = transparent.wrapperIndexes.length > 0;
874813

875814
if (i >= tokens.length)
876815
return strippedPrefix ? { kind: "opaque" } : { kind: "none" };

0 commit comments

Comments
 (0)