LLM/moltbot
1
0
Fork 0
mirror of https://github.com/moltbot/moltbot.git synced 2026-03-07 22:44:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: tighten safe-bin policy model and docs parity

Browse Source
This commit is contained in:
Peter Steinberger
2026-02-21 19:24:23 +01:00
parent c730d4dd72
commit 89aad7b922
4 changed files with 227 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
docs/tools/exec-approvals.md
View File

@@ -131,6 +131,16 @@ File-oriented options are denied for default safe bins (for example `sort -o`, `
`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).
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`
- `wc`: `--files0-from`
<!-- SAFE_BIN_DENIED_FLAGS:END -->
Safe bins also force argv tokens to be treated as **literal text** at execution time (no globbing
and no `$VARS` expansion) for stdin-only segments, so patterns like `*` or `$HOME/...` cannot be
used to smuggle file reads.

158
src/infra/exec-approvals.test.ts
View File

@@ -519,6 +519,103 @@ describe("exec approvals safe bins", () => {
setup?: (cwd: string) => void;
};
function buildDeniedFlagVariantCases(params: {
executableName: string;
resolvedPath: string;
safeBins?: string[];
flag: string;
takesValue: boolean;
label: string;
}): SafeBinCase[] {
const value = "blocked";
const argvVariants: string[][] = [];
if (!params.takesValue) {
argvVariants.push([params.executableName, params.flag]);
} else if (params.flag.startsWith("--")) {
argvVariants.push([params.executableName, `${params.flag}=${value}`]);
argvVariants.push([params.executableName, params.flag, value]);
} else if (params.flag.startsWith("-")) {
argvVariants.push([params.executableName, `${params.flag}${value}`]);
argvVariants.push([params.executableName, params.flag, value]);
} else {
argvVariants.push([params.executableName, params.flag, value]);
}
return argvVariants.map((argv) => ({
name: `${params.label} (${argv.slice(1).join(" ")})`,
argv,
resolvedPath: params.resolvedPath,
expected: false,
safeBins: params.safeBins ?? [params.executableName],
executableName: params.executableName,
}));
}
const deniedFlagCases: SafeBinCase[] = [
...buildDeniedFlagVariantCases({
executableName: "sort",
resolvedPath: "/usr/bin/sort",
flag: "-o",
takesValue: true,
label: "blocks sort output flag",
}),
...buildDeniedFlagVariantCases({
executableName: "sort",
resolvedPath: "/usr/bin/sort",
flag: "--output",
takesValue: true,
label: "blocks sort output flag",
}),
...buildDeniedFlagVariantCases({
executableName: "sort",
resolvedPath: "/usr/bin/sort",
flag: "--compress-program",
takesValue: true,
label: "blocks sort external program flag",
}),
...buildDeniedFlagVariantCases({
executableName: "grep",
resolvedPath: "/usr/bin/grep",
flag: "-R",
takesValue: false,
label: "blocks grep recursive flag",
}),
...buildDeniedFlagVariantCases({
executableName: "grep",
resolvedPath: "/usr/bin/grep",
flag: "--recursive",
takesValue: false,
label: "blocks grep recursive flag",
}),
...buildDeniedFlagVariantCases({
executableName: "grep",
resolvedPath: "/usr/bin/grep",
flag: "--file",
takesValue: true,
label: "blocks grep file-pattern flag",
}),
...buildDeniedFlagVariantCases({
executableName: "jq",
resolvedPath: "/usr/bin/jq",
flag: "-f",
takesValue: true,
label: "blocks jq file-program flag",
}),
...buildDeniedFlagVariantCases({
executableName: "jq",
resolvedPath: "/usr/bin/jq",
flag: "--from-file",
takesValue: true,
label: "blocks jq file-program flag",
}),
...buildDeniedFlagVariantCases({
executableName: "wc",
resolvedPath: "/usr/bin/wc",
flag: "--files0-from",
takesValue: true,
label: "blocks wc file-list flag",
}),
];
const cases: SafeBinCase[] = [
{
name: "allows safe bins with non-path args",
@@ -540,54 +637,7 @@ describe("exec approvals safe bins", () => {
expected: false,
cwd: "/tmp",
},
{
name: "blocks sort output path via -o <file>",
argv: ["sort", "-o", "malicious.sh"],
resolvedPath: "/usr/bin/sort",
expected: false,
safeBins: ["sort"],
executableName: "sort",
},
{
name: "blocks sort output path via attached short option (-ofile)",
argv: ["sort", "-omalicious.sh"],
resolvedPath: "/usr/bin/sort",
expected: false,
safeBins: ["sort"],
executableName: "sort",
},
{
name: "blocks sort output path via --output=file",
argv: ["sort", "--output=malicious.sh"],
resolvedPath: "/usr/bin/sort",
expected: false,
safeBins: ["sort"],
executableName: "sort",
},
{
name: "blocks sort external program flag via --compress-program=<prog>",
argv: ["sort", "--compress-program=sh"],
resolvedPath: "/usr/bin/sort",
expected: false,
safeBins: ["sort"],
executableName: "sort",
},
{
name: "blocks sort external program flag via --compress-program <prog>",
argv: ["sort", "--compress-program", "sh"],
resolvedPath: "/usr/bin/sort",
expected: false,
safeBins: ["sort"],
executableName: "sort",
},
{
name: "blocks grep recursive flags that read cwd",
argv: ["grep", "-R", "needle"],
resolvedPath: "/usr/bin/grep",
expected: false,
safeBins: ["grep"],
executableName: "grep",
},
...deniedFlagCases,
{
name: "blocks grep file positional when pattern uses -e",
argv: ["grep", "-e", "needle", ".env"],
@@ -690,13 +740,13 @@ describe("exec approvals safe bins", () => {
for (const [name, fixture] of Object.entries(SAFE_BIN_PROFILE_FIXTURES)) {
const profile = SAFE_BIN_PROFILES[name];
expect(profile).toBeDefined();
const fixtureBlockedFlags = fixture.blockedFlags ?? [];
const compiledBlockedFlags = profile?.blockedFlags ?? new Set<string>();
for (const blockedFlag of fixtureBlockedFlags) {
expect(compiledBlockedFlags.has(blockedFlag)).toBe(true);
const fixtureDeniedFlags = fixture.deniedFlags ?? [];
const compiledDeniedFlags = profile?.deniedFlags ?? new Set<string>();
for (const deniedFlag of fixtureDeniedFlags) {
expect(compiledDeniedFlags.has(deniedFlag)).toBe(true);
}
expect(Array.from(compiledBlockedFlags).toSorted()).toEqual(
[...fixtureBlockedFlags].toSorted(),
expect(Array.from(compiledDeniedFlags).toSorted()).toEqual(
[...fixtureDeniedFlags].toSorted(),
);
}
});

52
src/infra/exec-safe-bin-policy.test.ts
View File

@@ -1,5 +1,26 @@
import fs from "node:fs";
import path from "node:path";
import { describe, expect, it } from "vitest";
import { SAFE_BIN_PROFILES, validateSafeBinArgv } from "./exec-safe-bin-policy.js";
import {
SAFE_BIN_PROFILE_FIXTURES,
SAFE_BIN_PROFILES,
renderSafeBinDeniedFlagsDocBullets,
validateSafeBinArgv,
} from "./exec-safe-bin-policy.js";
const SAFE_BIN_DOC_DENIED_FLAGS_START = "<!-- SAFE_BIN_DENIED_FLAGS:START -->";
const SAFE_BIN_DOC_DENIED_FLAGS_END = "<!-- SAFE_BIN_DENIED_FLAGS:END -->";
function buildDeniedFlagArgvVariants(flag: string): string[][] {
const value = "blocked";
if (flag.startsWith("--")) {
return [[`${flag}=${value}`], [flag, value], [flag]];
}
if (flag.startsWith("-")) {
return [[`${flag}${value}`], [flag, value], [flag]];
}
return [[flag]];
}
describe("exec safe bin policy grep", () => {
const grepProfile = SAFE_BIN_PROFILES.grep;
@@ -34,3 +55,32 @@ describe("exec safe bin policy sort", () => {
expect(validateSafeBinArgv(["--compress-program", "sh"], sortProfile)).toBe(false);
});
});
describe("exec safe bin policy denied-flag matrix", () => {
for (const [binName, fixture] of Object.entries(SAFE_BIN_PROFILE_FIXTURES)) {
const profile = SAFE_BIN_PROFILES[binName];
const deniedFlags = fixture.deniedFlags ?? [];
for (const deniedFlag of deniedFlags) {
const variants = buildDeniedFlagArgvVariants(deniedFlag);
for (const variant of variants) {
it(`${binName} denies ${deniedFlag} (${variant.join(" ")})`, () => {
expect(validateSafeBinArgv(variant, profile)).toBe(false);
});
}
}
}
});
describe("exec safe bin policy docs parity", () => {
it("keeps denied-flag docs in sync with policy fixtures", () => {
const docsPath = path.resolve(process.cwd(), "docs/tools/exec-approvals.md");
const docs = fs.readFileSync(docsPath, "utf8").replaceAll("\r\n", "\n");
const start = docs.indexOf(SAFE_BIN_DOC_DENIED_FLAGS_START);
const end = docs.indexOf(SAFE_BIN_DOC_DENIED_FLAGS_END);
expect(start).toBeGreaterThanOrEqual(0);
expect(end).toBeGreaterThan(start);
const actual = docs.slice(start + SAFE_BIN_DOC_DENIED_FLAGS_START.length, end).trim();
const expected = renderSafeBinDeniedFlagsDocBullets();
expect(actual).toBe(expected);
});
});

113
src/infra/exec-safe-bin-policy.ts
View File

@@ -26,15 +26,15 @@ function hasGlobToken(value: string): boolean {
export type SafeBinProfile = {
minPositional?: number;
maxPositional?: number;
valueFlags?: ReadonlySet<string>;
blockedFlags?: ReadonlySet<string>;
allowedValueFlags?: ReadonlySet<string>;
deniedFlags?: ReadonlySet<string>;
};
export type SafeBinProfileFixture = {
minPositional?: number;
maxPositional?: number;
valueFlags?: readonly string[];
blockedFlags?: readonly string[];
allowedValueFlags?: readonly string[];
deniedFlags?: readonly string[];
};
const NO_FLAGS: ReadonlySet<string> = new Set();
@@ -50,8 +50,8 @@ function compileSafeBinProfile(fixture: SafeBinProfileFixture): SafeBinProfile {
return {
minPositional: fixture.minPositional,
maxPositional: fixture.maxPositional,
valueFlags: toFlagSet(fixture.valueFlags),
blockedFlags: toFlagSet(fixture.blockedFlags),
allowedValueFlags: toFlagSet(fixture.allowedValueFlags),
deniedFlags: toFlagSet(fixture.deniedFlags),
};
}
@@ -68,19 +68,8 @@ export const SAFE_BIN_GENERIC_PROFILE_FIXTURE: SafeBinProfileFixture = {};
export const SAFE_BIN_PROFILE_FIXTURES: Record<string, SafeBinProfileFixture> = {
jq: {
maxPositional: 1,
valueFlags: [
"--arg",
"--argjson",
"--argstr",
"--argfile",
"--rawfile",
"--slurpfile",
"--from-file",
"--library-path",
"-L",
"-f",
],
blockedFlags: [
allowedValueFlags: ["--arg", "--argjson", "--argstr"],
deniedFlags: [
"--argfile",
"--rawfile",
"--slurpfile",
@@ -95,30 +84,25 @@ export const SAFE_BIN_PROFILE_FIXTURES: Record<string, SafeBinProfileFixture> =
// Allowing one positional is ambiguous because -e consumes the pattern and
// frees the positional slot for a filename.
maxPositional: 0,
valueFlags: [
allowedValueFlags: [
"--regexp",
"--file",
"--max-count",
"--after-context",
"--before-context",
"--context",
"--devices",
"--directories",
"--binary-files",
"--exclude",
"--exclude-from",
"--include",
"--label",
"-e",
"-f",
"-m",
"-A",
"-B",
"-C",
"-D",
"-d",
],
blockedFlags: [
deniedFlags: [
"--file",
"--exclude-from",
"--dereference-recursive",
@@ -132,7 +116,7 @@ export const SAFE_BIN_PROFILE_FIXTURES: Record<string, SafeBinProfileFixture> =
},
cut: {
maxPositional: 0,
valueFlags: [
allowedValueFlags: [
"--bytes",
"--characters",
"--fields",
@@ -146,7 +130,7 @@ export const SAFE_BIN_PROFILE_FIXTURES: Record<string, SafeBinProfileFixture> =
},
sort: {
maxPositional: 0,
valueFlags: [
allowedValueFlags: [
"--key",
"--field-separator",
"--buffer-size",
@@ -154,28 +138,33 @@ export const SAFE_BIN_PROFILE_FIXTURES: Record<string, SafeBinProfileFixture> =
"--parallel",
"--batch-size",
"--random-source",
"--files0-from",
"--output",
"-k",
"-t",
"-S",
"-T",
"-o",
],
// --compress-program can invoke an external executable and breaks stdin-only guarantees.
blockedFlags: ["--compress-program", "--files0-from", "--output", "-o"],
deniedFlags: ["--compress-program", "--files0-from", "--output", "-o"],
},
uniq: {
maxPositional: 0,
valueFlags: ["--skip-fields", "--skip-chars", "--check-chars", "--group", "-f", "-s", "-w"],
allowedValueFlags: [
"--skip-fields",
"--skip-chars",
"--check-chars",
"--group",
"-f",
"-s",
"-w",
],
},
head: {
maxPositional: 0,
valueFlags: ["--lines", "--bytes", "-n", "-c"],
allowedValueFlags: ["--lines", "--bytes", "-n", "-c"],
},
tail: {
maxPositional: 0,
valueFlags: [
allowedValueFlags: [
"--lines",
"--bytes",
"--sleep-interval",
@@ -191,8 +180,7 @@ export const SAFE_BIN_PROFILE_FIXTURES: Record<string, SafeBinProfileFixture> =
},
wc: {
maxPositional: 0,
valueFlags: ["--files0-from"],
blockedFlags: ["--files0-from"],
deniedFlags: ["--files0-from"],
},
};
@@ -201,6 +189,29 @@ export const SAFE_BIN_GENERIC_PROFILE = compileSafeBinProfile(SAFE_BIN_GENERIC_P
export const SAFE_BIN_PROFILES: Record<string, SafeBinProfile> =
compileSafeBinProfiles(SAFE_BIN_PROFILE_FIXTURES);
export function resolveSafeBinDeniedFlags(
fixtures: Readonly<Record<string, SafeBinProfileFixture>> = SAFE_BIN_PROFILE_FIXTURES,
): Record<string, string[]> {
const out: Record<string, string[]> = {};
for (const [name, fixture] of Object.entries(fixtures)) {
const denied = Array.from(new Set(fixture.deniedFlags ?? [])).toSorted();
if (denied.length > 0) {
out[name] = denied;
}
}
return out;
}
export function renderSafeBinDeniedFlagsDocBullets(
fixtures: Readonly<Record<string, SafeBinProfileFixture>> = SAFE_BIN_PROFILE_FIXTURES,
): string {
const deniedByBin = resolveSafeBinDeniedFlags(fixtures);
const bins = Object.keys(deniedByBin).toSorted();
return bins
.map((bin) => `- \`${bin}\`: ${deniedByBin[bin].map((flag) => `\`${flag}\``).join(", ")}`)
.join("\n");
}
function isSafeLiteralToken(value: string): boolean {
if (!value || value === "-") {
return true;
@@ -217,16 +228,16 @@ function consumeLongOptionToken(
index: number,
flag: string,
inlineValue: string | undefined,
valueFlags: ReadonlySet<string>,
blockedFlags: ReadonlySet<string>,
allowedValueFlags: ReadonlySet<string>,
deniedFlags: ReadonlySet<string>,
): number {
if (blockedFlags.has(flag)) {
if (deniedFlags.has(flag)) {
return -1;
}
if (inlineValue !== undefined) {
return isSafeLiteralToken(inlineValue) ? index + 1 : -1;
}
if (!valueFlags.has(flag)) {
if (!allowedValueFlags.has(flag)) {
return index + 1;
}
return isInvalidValueToken(args[index + 1]) ? -1 : index + 2;
@@ -238,15 +249,15 @@ function consumeShortOptionClusterToken(
raw: string,
cluster: string,
flags: string[],
valueFlags: ReadonlySet<string>,
blockedFlags: ReadonlySet<string>,
allowedValueFlags: ReadonlySet<string>,
deniedFlags: ReadonlySet<string>,
): number {
for (let j = 0; j < flags.length; j += 1) {
const flag = flags[j];
if (blockedFlags.has(flag)) {
if (deniedFlags.has(flag)) {
return -1;
}
if (!valueFlags.has(flag)) {
if (!allowedValueFlags.has(flag)) {
continue;
}
const inlineValue = cluster.slice(j + 1);
@@ -275,8 +286,8 @@ function validatePositionalCount(positional: string[], profile: SafeBinProfile):
}
export function validateSafeBinArgv(args: string[], profile: SafeBinProfile): boolean {
const valueFlags = profile.valueFlags ?? NO_FLAGS;
const blockedFlags = profile.blockedFlags ?? NO_FLAGS;
const allowedValueFlags = profile.allowedValueFlags ?? NO_FLAGS;
const deniedFlags = profile.deniedFlags ?? NO_FLAGS;
const positional: string[] = [];
let i = 0;
while (i < args.length) {
@@ -315,8 +326,8 @@ export function validateSafeBinArgv(args: string[], profile: SafeBinProfile): bo
i,
token.flag,
token.inlineValue,
valueFlags,
blockedFlags,
allowedValueFlags,
deniedFlags,
);
if (nextIndex < 0) {
return false;
@@ -331,8 +342,8 @@ export function validateSafeBinArgv(args: string[], profile: SafeBinProfile): bo
token.raw,
token.cluster,
token.flags,
valueFlags,
blockedFlags,
allowedValueFlags,
deniedFlags,
);
if (nextIndex < 0) {
return false;