mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-07 22:44:16 +00:00
fix(security): harden safeBins long-option validation
This commit is contained in:
@@ -131,17 +131,20 @@ Custom safe bins must define an explicit profile in `tools.exec.safeBinProfiles.
|
||||
Validation is deterministic from argv shape only (no host filesystem existence checks), which
|
||||
prevents file-existence oracle behavior from allow/deny differences.
|
||||
File-oriented options are denied for default safe bins (for example `sort -o`, `sort --output`,
|
||||
`sort --files0-from`, `sort --compress-program`, `wc --files0-from`, `jq -f/--from-file`,
|
||||
`sort --files0-from`, `sort --compress-program`, `sort --random-source`,
|
||||
`sort --temporary-directory`/`-T`, `wc --files0-from`, `jq -f/--from-file`,
|
||||
`grep -f/--file`).
|
||||
Safe bins also enforce explicit per-binary flag policy for options that break stdin-only
|
||||
behavior (for example `sort -o/--output/--compress-program` and grep recursive flags).
|
||||
Long options are validated fail-closed in safe-bin mode: unknown flags and ambiguous
|
||||
abbreviations are rejected.
|
||||
Denied flags by safe-bin profile:
|
||||
|
||||
<!-- SAFE_BIN_DENIED_FLAGS:START -->
|
||||
|
||||
- `grep`: `--dereference-recursive`, `--directories`, `--exclude-from`, `--file`, `--recursive`, `-R`, `-d`, `-f`, `-r`
|
||||
- `jq`: `--argfile`, `--from-file`, `--library-path`, `--rawfile`, `--slurpfile`, `-L`, `-f`
|
||||
- `sort`: `--compress-program`, `--files0-from`, `--output`, `-o`
|
||||
- `sort`: `--compress-program`, `--files0-from`, `--output`, `--random-source`, `--temporary-directory`, `-T`, `-o`
|
||||
- `wc`: `--files0-from`
|
||||
<!-- SAFE_BIN_DENIED_FLAGS:END -->
|
||||
|
||||
|
||||
@@ -81,6 +81,41 @@ describe("exec approvals safe bins", () => {
|
||||
takesValue: true,
|
||||
label: "blocks sort external program flag",
|
||||
}),
|
||||
...buildDeniedFlagVariantCases({
|
||||
executableName: "sort",
|
||||
resolvedPath: "/usr/bin/sort",
|
||||
flag: "--compress-prog",
|
||||
takesValue: true,
|
||||
label: "blocks sort denied flag abbreviations",
|
||||
}),
|
||||
...buildDeniedFlagVariantCases({
|
||||
executableName: "sort",
|
||||
resolvedPath: "/usr/bin/sort",
|
||||
flag: "--files0-fro",
|
||||
takesValue: true,
|
||||
label: "blocks sort denied flag abbreviations",
|
||||
}),
|
||||
...buildDeniedFlagVariantCases({
|
||||
executableName: "sort",
|
||||
resolvedPath: "/usr/bin/sort",
|
||||
flag: "--random-source",
|
||||
takesValue: true,
|
||||
label: "blocks sort filesystem-dependent flags",
|
||||
}),
|
||||
...buildDeniedFlagVariantCases({
|
||||
executableName: "sort",
|
||||
resolvedPath: "/usr/bin/sort",
|
||||
flag: "--temporary-directory",
|
||||
takesValue: true,
|
||||
label: "blocks sort filesystem-dependent flags",
|
||||
}),
|
||||
...buildDeniedFlagVariantCases({
|
||||
executableName: "sort",
|
||||
resolvedPath: "/usr/bin/sort",
|
||||
flag: "-T",
|
||||
takesValue: true,
|
||||
label: "blocks sort filesystem-dependent flags",
|
||||
}),
|
||||
...buildDeniedFlagVariantCases({
|
||||
executableName: "grep",
|
||||
resolvedPath: "/usr/bin/grep",
|
||||
@@ -123,6 +158,13 @@ describe("exec approvals safe bins", () => {
|
||||
takesValue: true,
|
||||
label: "blocks wc file-list flag",
|
||||
}),
|
||||
...buildDeniedFlagVariantCases({
|
||||
executableName: "wc",
|
||||
resolvedPath: "/usr/bin/wc",
|
||||
flag: "--files0-fro",
|
||||
takesValue: true,
|
||||
label: "blocks wc denied flag abbreviations",
|
||||
}),
|
||||
];
|
||||
|
||||
const cases: SafeBinCase[] = [
|
||||
@@ -163,6 +205,22 @@ describe("exec approvals safe bins", () => {
|
||||
safeBins: ["grep"],
|
||||
executableName: "grep",
|
||||
},
|
||||
{
|
||||
name: "rejects unknown long options in safe-bin mode",
|
||||
argv: ["sort", "--totally-unknown=1"],
|
||||
resolvedPath: "/usr/bin/sort",
|
||||
expected: false,
|
||||
safeBins: ["sort"],
|
||||
executableName: "sort",
|
||||
},
|
||||
{
|
||||
name: "rejects ambiguous long-option abbreviations in safe-bin mode",
|
||||
argv: ["sort", "--f=1"],
|
||||
resolvedPath: "/usr/bin/sort",
|
||||
expected: false,
|
||||
safeBins: ["sort"],
|
||||
executableName: "sort",
|
||||
},
|
||||
];
|
||||
|
||||
for (const testCase of cases) {
|
||||
|
||||
@@ -48,12 +48,32 @@ describe("exec safe bin policy sort", () => {
|
||||
it("allows stdin-only sort flags", () => {
|
||||
expect(validateSafeBinArgv(["-S", "1M"], sortProfile)).toBe(true);
|
||||
expect(validateSafeBinArgv(["--key=1,1"], sortProfile)).toBe(true);
|
||||
expect(validateSafeBinArgv(["--ke=1,1"], sortProfile)).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks sort --compress-program in safe-bin mode", () => {
|
||||
expect(validateSafeBinArgv(["--compress-program=sh"], sortProfile)).toBe(false);
|
||||
expect(validateSafeBinArgv(["--compress-program", "sh"], sortProfile)).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks denied long-option abbreviations in safe-bin mode", () => {
|
||||
expect(validateSafeBinArgv(["--compress-prog=sh"], sortProfile)).toBe(false);
|
||||
expect(validateSafeBinArgv(["--files0-fro=list.txt"], sortProfile)).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects unknown or ambiguous long options in safe-bin mode", () => {
|
||||
expect(validateSafeBinArgv(["--totally-unknown=1"], sortProfile)).toBe(false);
|
||||
expect(validateSafeBinArgv(["--f=1"], sortProfile)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("exec safe bin policy wc", () => {
|
||||
const wcProfile = SAFE_BIN_PROFILES.wc;
|
||||
|
||||
it("blocks wc --files0-from abbreviations in safe-bin mode", () => {
|
||||
expect(validateSafeBinArgv(["--files0-fro=list.txt"], wcProfile)).toBe(false);
|
||||
expect(validateSafeBinArgv(["--files0-fro", "list.txt"], wcProfile)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("exec safe bin policy denied-flag matrix", () => {
|
||||
|
||||
@@ -134,17 +134,23 @@ export const SAFE_BIN_PROFILE_FIXTURES: Record<string, SafeBinProfileFixture> =
|
||||
"--key",
|
||||
"--field-separator",
|
||||
"--buffer-size",
|
||||
"--temporary-directory",
|
||||
"--parallel",
|
||||
"--batch-size",
|
||||
"--random-source",
|
||||
"-k",
|
||||
"-t",
|
||||
"-S",
|
||||
"-T",
|
||||
],
|
||||
// --compress-program can invoke an external executable and breaks stdin-only guarantees.
|
||||
deniedFlags: ["--compress-program", "--files0-from", "--output", "-o"],
|
||||
// --random-source/--temporary-directory/-T are filesystem-dependent and not stdin-only.
|
||||
deniedFlags: [
|
||||
"--compress-program",
|
||||
"--files0-from",
|
||||
"--output",
|
||||
"--random-source",
|
||||
"--temporary-directory",
|
||||
"-T",
|
||||
"-o",
|
||||
],
|
||||
},
|
||||
uniq: {
|
||||
maxPositional: 0,
|
||||
@@ -293,6 +299,38 @@ function isInvalidValueToken(value: string | undefined): boolean {
|
||||
return !value || !isSafeLiteralToken(value);
|
||||
}
|
||||
|
||||
function collectKnownLongFlags(
|
||||
allowedValueFlags: ReadonlySet<string>,
|
||||
deniedFlags: ReadonlySet<string>,
|
||||
): string[] {
|
||||
const known = new Set<string>();
|
||||
for (const flag of allowedValueFlags) {
|
||||
if (flag.startsWith("--")) {
|
||||
known.add(flag);
|
||||
}
|
||||
}
|
||||
for (const flag of deniedFlags) {
|
||||
if (flag.startsWith("--")) {
|
||||
known.add(flag);
|
||||
}
|
||||
}
|
||||
return Array.from(known);
|
||||
}
|
||||
|
||||
function resolveCanonicalLongFlag(flag: string, knownLongFlags: string[]): string | null {
|
||||
if (!flag.startsWith("--") || flag.length <= 2) {
|
||||
return null;
|
||||
}
|
||||
if (knownLongFlags.includes(flag)) {
|
||||
return flag;
|
||||
}
|
||||
const matches = knownLongFlags.filter((candidate) => candidate.startsWith(flag));
|
||||
if (matches.length !== 1) {
|
||||
return null;
|
||||
}
|
||||
return matches[0] ?? null;
|
||||
}
|
||||
|
||||
function consumeLongOptionToken(
|
||||
args: string[],
|
||||
index: number,
|
||||
@@ -301,13 +339,22 @@ function consumeLongOptionToken(
|
||||
allowedValueFlags: ReadonlySet<string>,
|
||||
deniedFlags: ReadonlySet<string>,
|
||||
): number {
|
||||
if (deniedFlags.has(flag)) {
|
||||
const knownLongFlags = collectKnownLongFlags(allowedValueFlags, deniedFlags);
|
||||
const canonicalFlag = resolveCanonicalLongFlag(flag, knownLongFlags);
|
||||
if (!canonicalFlag) {
|
||||
return -1;
|
||||
}
|
||||
if (deniedFlags.has(canonicalFlag)) {
|
||||
return -1;
|
||||
}
|
||||
const expectsValue = allowedValueFlags.has(canonicalFlag);
|
||||
if (inlineValue !== undefined) {
|
||||
if (!expectsValue) {
|
||||
return -1;
|
||||
}
|
||||
return isSafeLiteralToken(inlineValue) ? index + 1 : -1;
|
||||
}
|
||||
if (!allowedValueFlags.has(flag)) {
|
||||
if (!expectsValue) {
|
||||
return index + 1;
|
||||
}
|
||||
return isInvalidValueToken(args[index + 1]) ? -1 : index + 2;
|
||||
|
||||
Reference in New Issue
Block a user