mirror of https://github.com/openclaw/openclaw.git
refactor(exec): split safe-bin semantics
This commit is contained in:
parent
d551d8b8f7
commit
0ac939059e
|
|
@ -259,6 +259,7 @@ High-signal `checkId` values you will most likely see in real deployments (not e
|
|||
| `tools.exec.auto_allow_skills_enabled` | warn | Exec approvals trust skill bins implicitly | `~/.openclaw/exec-approvals.json` | no |
|
||||
| `tools.exec.allowlist_interpreter_without_strict_inline_eval` | warn | Interpreter allowlists permit inline eval without forced reapproval | `tools.exec.strictInlineEval`, `agents.list[].tools.exec.strictInlineEval`, exec approvals allowlist | no |
|
||||
| `tools.exec.safe_bins_interpreter_unprofiled` | warn | Interpreter/runtime bins in `safeBins` without explicit profiles broaden exec risk | `tools.exec.safeBins`, `tools.exec.safeBinProfiles`, `agents.list[].tools.exec.*` | no |
|
||||
| `tools.exec.safe_bins_broad_behavior` | warn | Broad-behavior tools in `safeBins` weaken the low-risk stdin-filter trust model | `tools.exec.safeBins`, `agents.list[].tools.exec.safeBins` | no |
|
||||
| `skills.workspace.symlink_escape` | warn | Workspace `skills/**/SKILL.md` resolves outside workspace root (symlink-chain drift) | workspace `skills/**` filesystem state | no |
|
||||
| `security.exposure.open_channels_with_exec` | warn/critical | Shared/public rooms can reach exec-enabled agents | `channels.*.dmPolicy`, `channels.*.groupPolicy`, `tools.exec.*`, `agents.list[].tools.exec.*` | no |
|
||||
| `security.exposure.open_groups_with_elevated` | critical | Open groups + elevated tools create high-impact prompt-injection paths | `channels.*.groupPolicy`, `tools.elevated.*` | no |
|
||||
|
|
|
|||
|
|
@ -215,7 +215,10 @@ etc.) so inner executables are persisted instead of multiplexer binaries. If a w
|
|||
multiplexer cannot be safely unwrapped, no allowlist entry is persisted automatically.
|
||||
If you allowlist interpreters like `python3` or `node`, prefer `tools.exec.strictInlineEval=true` so inline eval still requires an explicit approval.
|
||||
|
||||
Default safe bins: `cut`, `uniq`, `head`, `tail`, `tr`, `wc`.
|
||||
Default safe bins:
|
||||
[//]: # "SAFE_BIN_DEFAULTS:START"
|
||||
`cut`, `uniq`, `head`, `tail`, `tr`, `wc`
|
||||
[//]: # "SAFE_BIN_DEFAULTS:END"
|
||||
|
||||
`grep` and `sort` are not in the default list. If you opt in, keep explicit allowlist entries for
|
||||
their non-stdin workflows.
|
||||
|
|
|
|||
|
|
@ -144,6 +144,7 @@ Use the two controls for different jobs:
|
|||
|
||||
Do not treat `safeBins` as a generic allowlist, and do not add interpreter/runtime binaries (for example `python3`, `node`, `ruby`, `bash`). If you need those, use explicit allowlist entries and keep approval prompts enabled.
|
||||
`openclaw security audit` warns when interpreter/runtime `safeBins` entries are missing explicit profiles, and `openclaw doctor --fix` can scaffold missing custom `safeBinProfiles` entries.
|
||||
`openclaw security audit` and `openclaw doctor` also warn when you explicitly add broad-behavior bins such as `jq` back into `safeBins`.
|
||||
If you explicitly allowlist interpreters, enable `tools.exec.strictInlineEval` so inline code-eval forms still require a fresh approval.
|
||||
|
||||
For full policy details and examples, see [Exec approvals](/tools/exec-approvals#safe-bins-stdin-only) and [Safe bins versus allowlist](/tools/exec-approvals#safe-bins-versus-allowlist).
|
||||
|
|
|
|||
|
|
@ -28,21 +28,36 @@ describe("doctor exec safe bin helpers", () => {
|
|||
},
|
||||
} as OpenClawConfig);
|
||||
|
||||
expect(hits).toEqual([{ scopePath: "tools.exec", bin: "node", isInterpreter: true }]);
|
||||
expect(hits).toEqual([
|
||||
{ scopePath: "tools.exec", bin: "node", kind: "missingProfile", isInterpreter: true },
|
||||
{
|
||||
scopePath: "tools.exec",
|
||||
bin: "jq",
|
||||
kind: "riskySemantics",
|
||||
warning:
|
||||
"jq supports broad jq programs and builtins (for example `env`), so prefer explicit allowlist entries or approval-gated runs instead of safeBins.",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("formats coverage warnings", () => {
|
||||
const warnings = collectExecSafeBinCoverageWarnings({
|
||||
hits: [
|
||||
{ scopePath: "tools.exec", bin: "node", isInterpreter: true },
|
||||
{ scopePath: "agents.list.runner.tools.exec", bin: "jq", isInterpreter: false },
|
||||
{ scopePath: "tools.exec", bin: "node", kind: "missingProfile", isInterpreter: true },
|
||||
{
|
||||
scopePath: "agents.list.runner.tools.exec",
|
||||
bin: "jq",
|
||||
kind: "riskySemantics",
|
||||
warning:
|
||||
"jq supports broad jq programs and builtins (for example `env`), so prefer explicit allowlist entries or approval-gated runs instead of safeBins.",
|
||||
},
|
||||
],
|
||||
doctorFixCommand: "openclaw doctor --fix",
|
||||
});
|
||||
|
||||
expect(warnings).toEqual([
|
||||
expect.stringContaining("tools.exec.safeBins includes interpreter/runtime 'node'"),
|
||||
expect.stringContaining("agents.list.runner.tools.exec.safeBins entry 'jq'"),
|
||||
expect.stringContaining("agents.list.runner.tools.exec.safeBins includes 'jq'"),
|
||||
expect.stringContaining('Run "openclaw doctor --fix"'),
|
||||
]);
|
||||
});
|
||||
|
|
@ -60,6 +75,7 @@ describe("doctor exec safe bin helpers", () => {
|
|||
"- tools.exec.safeBinProfiles.jq: added scaffold profile {} (review and tighten flags/positionals).",
|
||||
]);
|
||||
expect(result.warnings).toEqual([
|
||||
"- tools.exec.safeBins includes 'jq': jq supports broad jq programs and builtins (for example `env`), so prefer explicit allowlist entries or approval-gated runs instead of safeBins.",
|
||||
"- tools.exec.safeBins includes interpreter/runtime 'node' without profile; remove it from safeBins or use explicit allowlist entries.",
|
||||
]);
|
||||
expect(result.config.tools?.exec?.safeBinProfiles).toEqual({ jq: {} });
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import {
|
|||
listInterpreterLikeSafeBins,
|
||||
resolveMergedSafeBinProfileFixtures,
|
||||
} from "../../../infra/exec-safe-bin-runtime-policy.js";
|
||||
import { listRiskyConfiguredSafeBins } from "../../../infra/exec-safe-bin-semantics.js";
|
||||
import {
|
||||
getTrustedSafeBinDirs,
|
||||
isTrustedSafeBinPath,
|
||||
|
|
@ -15,7 +16,9 @@ import { asObjectRecord } from "./object.js";
|
|||
export type ExecSafeBinCoverageHit = {
|
||||
scopePath: string;
|
||||
bin: string;
|
||||
isInterpreter: boolean;
|
||||
kind: "missingProfile" | "riskySemantics";
|
||||
isInterpreter?: boolean;
|
||||
warning?: string;
|
||||
};
|
||||
|
||||
type ExecSafeBinScopeRef = {
|
||||
|
|
@ -119,9 +122,18 @@ export function scanExecSafeBinCoverage(cfg: OpenClawConfig): ExecSafeBinCoverag
|
|||
hits.push({
|
||||
scopePath: scope.scopePath,
|
||||
bin,
|
||||
kind: "missingProfile",
|
||||
isInterpreter: interpreterBins.has(bin),
|
||||
});
|
||||
}
|
||||
for (const hit of listRiskyConfiguredSafeBins(scope.safeBins)) {
|
||||
hits.push({
|
||||
scopePath: scope.scopePath,
|
||||
bin: hit.bin,
|
||||
kind: "riskySemantics",
|
||||
warning: hit.warning,
|
||||
});
|
||||
}
|
||||
}
|
||||
return hits;
|
||||
}
|
||||
|
|
@ -161,8 +173,13 @@ export function collectExecSafeBinCoverageWarnings(params: {
|
|||
if (params.hits.length === 0) {
|
||||
return [];
|
||||
}
|
||||
const interpreterHits = params.hits.filter((hit) => hit.isInterpreter);
|
||||
const customHits = params.hits.filter((hit) => !hit.isInterpreter);
|
||||
const interpreterHits = params.hits.filter(
|
||||
(hit) => hit.kind === "missingProfile" && hit.isInterpreter,
|
||||
);
|
||||
const customHits = params.hits.filter(
|
||||
(hit) => hit.kind === "missingProfile" && !hit.isInterpreter,
|
||||
);
|
||||
const riskyHits = params.hits.filter((hit) => hit.kind === "riskySemantics");
|
||||
const lines: string[] = [];
|
||||
if (interpreterHits.length > 0) {
|
||||
for (const hit of interpreterHits.slice(0, 5)) {
|
||||
|
|
@ -186,6 +203,16 @@ export function collectExecSafeBinCoverageWarnings(params: {
|
|||
lines.push(`- ${customHits.length - 5} more custom safeBins entries are missing profiles.`);
|
||||
}
|
||||
}
|
||||
if (riskyHits.length > 0) {
|
||||
for (const hit of riskyHits.slice(0, 5)) {
|
||||
lines.push(
|
||||
`- ${sanitizeForLog(hit.scopePath)}.safeBins includes '${sanitizeForLog(hit.bin)}': ${sanitizeForLog(hit.warning ?? "prefer explicit allowlist entries or approval-gated runs.")}`,
|
||||
);
|
||||
}
|
||||
if (riskyHits.length > 5) {
|
||||
lines.push(`- ${riskyHits.length - 5} more safeBins entries should not use the low-risk safeBins fast path.`);
|
||||
}
|
||||
}
|
||||
lines.push(
|
||||
`- Run "${params.doctorFixCommand}" to scaffold missing custom safeBinProfiles entries.`,
|
||||
);
|
||||
|
|
@ -224,6 +251,11 @@ export function maybeRepairExecSafeBinProfiles(cfg: OpenClawConfig): {
|
|||
|
||||
for (const scope of collectExecSafeBinScopes(next)) {
|
||||
const interpreterBins = new Set(listInterpreterLikeSafeBins(scope.safeBins));
|
||||
for (const hit of listRiskyConfiguredSafeBins(scope.safeBins)) {
|
||||
warnings.push(
|
||||
`- ${scope.scopePath}.safeBins includes '${hit.bin}': ${hit.warning}`,
|
||||
);
|
||||
}
|
||||
const missingBins = scope.safeBins.filter((bin) => !scope.mergedProfiles[bin]);
|
||||
if (missingBins.length === 0) {
|
||||
continue;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
import path from "node:path";
|
||||
import {
|
||||
DEFAULT_SAFE_BINS,
|
||||
analyzeShellCommand,
|
||||
isWindowsPlatform,
|
||||
matchAllowlist,
|
||||
|
|
@ -13,6 +12,7 @@ import {
|
|||
} from "./exec-approvals-analysis.js";
|
||||
import type { ExecAllowlistEntry } from "./exec-approvals.js";
|
||||
import {
|
||||
DEFAULT_SAFE_BINS,
|
||||
SAFE_BIN_PROFILES,
|
||||
type SafeBinProfile,
|
||||
validateSafeBinArgv,
|
||||
|
|
@ -31,7 +31,7 @@ function hasShellLineContinuation(command: string): boolean {
|
|||
return /\\(?:\r\n|\n|\r)/.test(command);
|
||||
}
|
||||
|
||||
export function normalizeSafeBins(entries?: string[]): Set<string> {
|
||||
export function normalizeSafeBins(entries?: readonly string[]): Set<string> {
|
||||
if (!Array.isArray(entries)) {
|
||||
return new Set();
|
||||
}
|
||||
|
|
@ -41,7 +41,7 @@ export function normalizeSafeBins(entries?: string[]): Set<string> {
|
|||
return new Set(normalized);
|
||||
}
|
||||
|
||||
export function resolveSafeBins(entries?: string[] | null): Set<string> {
|
||||
export function resolveSafeBins(entries?: readonly string[] | null): Set<string> {
|
||||
if (entries === undefined) {
|
||||
return normalizeSafeBins(DEFAULT_SAFE_BINS);
|
||||
}
|
||||
|
|
@ -92,7 +92,7 @@ export function isSafeBinUsage(params: {
|
|||
if (!profile) {
|
||||
return false;
|
||||
}
|
||||
return validateSafeBinArgv(argv, profile);
|
||||
return validateSafeBinArgv(argv, profile, { binName: execName });
|
||||
}
|
||||
|
||||
function isPathScopedExecutableToken(token: string): boolean {
|
||||
|
|
|
|||
|
|
@ -3,9 +3,9 @@ import {
|
|||
resolveCommandResolutionFromArgv,
|
||||
type CommandResolution,
|
||||
} from "./exec-command-resolution.js";
|
||||
export { DEFAULT_SAFE_BINS } from "./exec-safe-bin-policy.js";
|
||||
|
||||
export {
|
||||
DEFAULT_SAFE_BINS,
|
||||
matchAllowlist,
|
||||
parseExecArgvToken,
|
||||
resolveAllowlistCandidatePath,
|
||||
|
|
|
|||
|
|
@ -6,8 +6,6 @@ import { resolveDispatchWrapperExecutionPlan } from "./exec-wrapper-resolution.j
|
|||
import { resolveExecutablePath as resolveExecutableCandidatePath } from "./executable-path.js";
|
||||
import { expandHomePrefix } from "./home-dir.js";
|
||||
|
||||
export const DEFAULT_SAFE_BINS = ["cut", "uniq", "head", "tail", "tr", "wc"];
|
||||
|
||||
export type CommandResolution = {
|
||||
rawExecutable: string;
|
||||
resolvedPath?: string;
|
||||
|
|
|
|||
|
|
@ -3,7 +3,6 @@ export type SafeBinProfile = {
|
|||
maxPositional?: number;
|
||||
allowedValueFlags?: ReadonlySet<string>;
|
||||
deniedFlags?: ReadonlySet<string>;
|
||||
positionalValidator?: (positional: readonly string[]) => boolean;
|
||||
// Precomputed long-option metadata for GNU abbreviation resolution.
|
||||
knownLongFlags?: readonly string[];
|
||||
knownLongFlagsSet?: ReadonlySet<string>;
|
||||
|
|
@ -21,6 +20,8 @@ export type SafeBinProfileFixtures = Readonly<Record<string, SafeBinProfileFixtu
|
|||
|
||||
const NO_FLAGS: ReadonlySet<string> = new Set();
|
||||
|
||||
export const DEFAULT_SAFE_BINS = ["cut", "uniq", "head", "tail", "tr", "wc"] as const;
|
||||
|
||||
const toFlagSet = (flags?: readonly string[]): ReadonlySet<string> => {
|
||||
if (!flags || flags.length === 0) {
|
||||
return NO_FLAGS;
|
||||
|
|
@ -69,18 +70,7 @@ export function buildLongFlagPrefixMap(
|
|||
return prefixMap;
|
||||
}
|
||||
|
||||
const JQ_ENV_FILTER_PATTERN = /(^|[^.$A-Za-z0-9_])env([^A-Za-z0-9_]|$)/;
|
||||
|
||||
function validateJqSafeBinPositional(positional: readonly string[]): boolean {
|
||||
for (const token of positional) {
|
||||
if (JQ_ENV_FILTER_PATTERN.test(token)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function compileSafeBinProfile(name: string, fixture: SafeBinProfileFixture): SafeBinProfile {
|
||||
function compileSafeBinProfile(fixture: SafeBinProfileFixture): SafeBinProfile {
|
||||
const allowedValueFlags = toFlagSet(fixture.allowedValueFlags);
|
||||
const deniedFlags = toFlagSet(fixture.deniedFlags);
|
||||
const knownLongFlags = collectKnownLongFlags(allowedValueFlags, deniedFlags);
|
||||
|
|
@ -89,7 +79,6 @@ function compileSafeBinProfile(name: string, fixture: SafeBinProfileFixture): Sa
|
|||
maxPositional: fixture.maxPositional,
|
||||
allowedValueFlags,
|
||||
deniedFlags,
|
||||
positionalValidator: name === "jq" ? validateJqSafeBinPositional : undefined,
|
||||
knownLongFlags,
|
||||
knownLongFlagsSet: new Set(knownLongFlags),
|
||||
longFlagPrefixMap: buildLongFlagPrefixMap(knownLongFlags),
|
||||
|
|
@ -100,7 +89,7 @@ function compileSafeBinProfiles(
|
|||
fixtures: Record<string, SafeBinProfileFixture>,
|
||||
): Record<string, SafeBinProfile> {
|
||||
return Object.fromEntries(
|
||||
Object.entries(fixtures).map(([name, fixture]) => [name, compileSafeBinProfile(name, fixture)]),
|
||||
Object.entries(fixtures).map(([name, fixture]) => [name, compileSafeBinProfile(fixture)]),
|
||||
) as Record<string, SafeBinProfile>;
|
||||
}
|
||||
|
||||
|
|
@ -326,3 +315,9 @@ export function renderSafeBinDeniedFlagsDocBullets(
|
|||
.map((bin) => `- \`${bin}\`: ${deniedByBin[bin].map((flag) => `\`${flag}\``).join(", ")}`)
|
||||
.join("\n");
|
||||
}
|
||||
|
||||
export function renderDefaultSafeBinsDocText(
|
||||
defaults: readonly string[] = DEFAULT_SAFE_BINS,
|
||||
): string {
|
||||
return defaults.map((bin) => `\`${bin}\``).join(", ");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { parseExecArgvToken } from "./exec-approvals-analysis.js";
|
||||
import { validateSafeBinSemantics } from "./exec-safe-bin-semantics.js";
|
||||
import {
|
||||
buildLongFlagPrefixMap,
|
||||
collectKnownLongFlags,
|
||||
|
|
@ -130,10 +131,10 @@ function validatePositionalCount(positional: string[], profile: SafeBinProfile):
|
|||
if (typeof profile.maxPositional === "number" && positional.length > profile.maxPositional) {
|
||||
return false;
|
||||
}
|
||||
return profile.positionalValidator?.(positional) ?? true;
|
||||
return true;
|
||||
}
|
||||
|
||||
export function validateSafeBinArgv(args: string[], profile: SafeBinProfile): boolean {
|
||||
function collectPositionalTokens(args: string[], profile: SafeBinProfile): string[] | null {
|
||||
const allowedValueFlags = profile.allowedValueFlags ?? NO_FLAGS;
|
||||
const deniedFlags = profile.deniedFlags ?? NO_FLAGS;
|
||||
const knownLongFlags =
|
||||
|
|
@ -185,7 +186,7 @@ export function validateSafeBinArgv(args: string[], profile: SafeBinProfile): bo
|
|||
longFlagPrefixMap,
|
||||
});
|
||||
if (nextIndex < 0) {
|
||||
return false;
|
||||
return null;
|
||||
}
|
||||
i = nextIndex;
|
||||
continue;
|
||||
|
|
@ -200,10 +201,28 @@ export function validateSafeBinArgv(args: string[], profile: SafeBinProfile): bo
|
|||
deniedFlags,
|
||||
});
|
||||
if (nextIndex < 0) {
|
||||
return false;
|
||||
return null;
|
||||
}
|
||||
i = nextIndex;
|
||||
}
|
||||
|
||||
return validatePositionalCount(positional, profile);
|
||||
return positional;
|
||||
}
|
||||
|
||||
export function validateSafeBinArgv(
|
||||
args: string[],
|
||||
profile: SafeBinProfile,
|
||||
options?: { binName?: string },
|
||||
): boolean {
|
||||
const positional = collectPositionalTokens(args, profile);
|
||||
if (!positional) {
|
||||
return false;
|
||||
}
|
||||
if (!validatePositionalCount(positional, profile)) {
|
||||
return false;
|
||||
}
|
||||
return validateSafeBinSemantics({
|
||||
binName: options?.binName,
|
||||
positional,
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,14 +2,18 @@ import fs from "node:fs";
|
|||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
DEFAULT_SAFE_BINS,
|
||||
SAFE_BIN_PROFILE_FIXTURES,
|
||||
SAFE_BIN_PROFILES,
|
||||
buildLongFlagPrefixMap,
|
||||
collectKnownLongFlags,
|
||||
renderDefaultSafeBinsDocText,
|
||||
renderSafeBinDeniedFlagsDocBullets,
|
||||
validateSafeBinArgv,
|
||||
} from "./exec-safe-bin-policy.js";
|
||||
|
||||
const SAFE_BIN_DOC_DEFAULTS_START = '[//]: # "SAFE_BIN_DEFAULTS:START"';
|
||||
const SAFE_BIN_DOC_DEFAULTS_END = '[//]: # "SAFE_BIN_DEFAULTS:END"';
|
||||
const SAFE_BIN_DOC_DENIED_FLAGS_START = '[//]: # "SAFE_BIN_DENIED_FLAGS:START"';
|
||||
const SAFE_BIN_DOC_DENIED_FLAGS_END = '[//]: # "SAFE_BIN_DENIED_FLAGS:END"';
|
||||
|
||||
|
|
@ -48,14 +52,14 @@ describe("exec safe bin policy jq", () => {
|
|||
const jqProfile = SAFE_BIN_PROFILES.jq;
|
||||
|
||||
it("allows normal jq field filters", () => {
|
||||
expect(validateSafeBinArgv([".foo"], jqProfile)).toBe(true);
|
||||
expect(validateSafeBinArgv([".env"], jqProfile)).toBe(true);
|
||||
expect(validateSafeBinArgv([".foo"], jqProfile, { binName: "jq" })).toBe(true);
|
||||
expect(validateSafeBinArgv([".env"], jqProfile, { binName: "jq" })).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks jq env builtin filters in safe-bin mode", () => {
|
||||
expect(validateSafeBinArgv(["env"], jqProfile)).toBe(false);
|
||||
expect(validateSafeBinArgv(["env.FOO"], jqProfile)).toBe(false);
|
||||
expect(validateSafeBinArgv([".foo | env"], jqProfile)).toBe(false);
|
||||
expect(validateSafeBinArgv(["env"], jqProfile, { binName: "jq" })).toBe(false);
|
||||
expect(validateSafeBinArgv(["env.FOO"], jqProfile, { binName: "jq" })).toBe(false);
|
||||
expect(validateSafeBinArgv([".foo | env"], jqProfile, { binName: "jq" })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -160,6 +164,18 @@ describe("exec safe bin policy denied-flag matrix", () => {
|
|||
});
|
||||
|
||||
describe("exec safe bin policy docs parity", () => {
|
||||
it("keeps default safe-bin docs in sync with policy defaults", () => {
|
||||
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_DEFAULTS_START);
|
||||
const end = docs.indexOf(SAFE_BIN_DOC_DEFAULTS_END);
|
||||
expect(start).toBeGreaterThanOrEqual(0);
|
||||
expect(end).toBeGreaterThan(start);
|
||||
const actual = docs.slice(start + SAFE_BIN_DOC_DEFAULTS_START.length, end).trim();
|
||||
const expected = renderDefaultSafeBinsDocText(DEFAULT_SAFE_BINS);
|
||||
expect(actual).toBe(expected);
|
||||
});
|
||||
|
||||
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");
|
||||
|
|
|
|||
|
|
@ -1,9 +1,11 @@
|
|||
export {
|
||||
DEFAULT_SAFE_BINS,
|
||||
SAFE_BIN_PROFILE_FIXTURES,
|
||||
SAFE_BIN_PROFILES,
|
||||
buildLongFlagPrefixMap,
|
||||
collectKnownLongFlags,
|
||||
normalizeSafeBinProfileFixtures,
|
||||
renderDefaultSafeBinsDocText,
|
||||
renderSafeBinDeniedFlagsDocBullets,
|
||||
resolveSafeBinDeniedFlags,
|
||||
resolveSafeBinProfiles,
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import {
|
|||
type SafeBinProfileFixture,
|
||||
type SafeBinProfileFixtures,
|
||||
} from "./exec-safe-bin-policy.js";
|
||||
import { normalizeSafeBinName } from "./exec-safe-bin-semantics.js";
|
||||
import {
|
||||
getTrustedSafeBinDirs,
|
||||
listWritableExplicitTrustedSafeBinDirs,
|
||||
|
|
@ -59,16 +60,6 @@ const INTERPRETER_LIKE_PATTERNS = [
|
|||
/^node\d+(?:\.\d+)?$/,
|
||||
];
|
||||
|
||||
function normalizeSafeBinName(raw: string): string {
|
||||
const trimmed = raw.trim().toLowerCase();
|
||||
if (!trimmed) {
|
||||
return "";
|
||||
}
|
||||
const tail = trimmed.split(/[\\/]/).at(-1);
|
||||
const normalized = tail ?? trimmed;
|
||||
return normalized.replace(/\.(?:exe|cmd|bat|com)$/i, "");
|
||||
}
|
||||
|
||||
export function isInterpreterLikeSafeBin(raw: string): boolean {
|
||||
const normalized = normalizeSafeBinName(raw);
|
||||
if (!normalized) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,60 @@
|
|||
export type SafeBinSemanticValidationParams = {
|
||||
binName?: string;
|
||||
positional: readonly string[];
|
||||
};
|
||||
|
||||
type SafeBinSemanticRule = {
|
||||
validate?: (params: SafeBinSemanticValidationParams) => boolean;
|
||||
configWarning?: string;
|
||||
};
|
||||
|
||||
const JQ_ENV_FILTER_PATTERN = /(^|[^.$A-Za-z0-9_])env([^A-Za-z0-9_]|$)/;
|
||||
|
||||
const SAFE_BIN_SEMANTIC_RULES: Readonly<Record<string, SafeBinSemanticRule>> = {
|
||||
jq: {
|
||||
validate: ({ positional }) =>
|
||||
!positional.some((token) => JQ_ENV_FILTER_PATTERN.test(token)),
|
||||
configWarning:
|
||||
"jq supports broad jq programs and builtins (for example `env`), so prefer explicit allowlist entries or approval-gated runs instead of safeBins.",
|
||||
},
|
||||
};
|
||||
|
||||
export function normalizeSafeBinName(raw: string): string {
|
||||
const trimmed = raw.trim().toLowerCase();
|
||||
if (!trimmed) {
|
||||
return "";
|
||||
}
|
||||
const tail = trimmed.split(/[\\/]/).at(-1);
|
||||
const normalized = tail ?? trimmed;
|
||||
return normalized.replace(/\.(?:exe|cmd|bat|com)$/i, "");
|
||||
}
|
||||
|
||||
export function getSafeBinSemanticRule(binName?: string): SafeBinSemanticRule | undefined {
|
||||
const normalized = typeof binName === "string" ? normalizeSafeBinName(binName) : "";
|
||||
return normalized ? SAFE_BIN_SEMANTIC_RULES[normalized] : undefined;
|
||||
}
|
||||
|
||||
export function validateSafeBinSemantics(params: SafeBinSemanticValidationParams): boolean {
|
||||
return getSafeBinSemanticRule(params.binName)?.validate?.(params) ?? true;
|
||||
}
|
||||
|
||||
export function listRiskyConfiguredSafeBins(entries: Iterable<string>): Array<{
|
||||
bin: string;
|
||||
warning: string;
|
||||
}> {
|
||||
const hits = new Map<string, string>();
|
||||
for (const entry of entries) {
|
||||
const normalized = normalizeSafeBinName(entry);
|
||||
if (!normalized || hits.has(normalized)) {
|
||||
continue;
|
||||
}
|
||||
const warning = getSafeBinSemanticRule(normalized)?.configWarning;
|
||||
if (!warning) {
|
||||
continue;
|
||||
}
|
||||
hits.set(normalized, warning);
|
||||
}
|
||||
return Array.from(hits.entries())
|
||||
.map(([bin, warning]) => ({ bin, warning }))
|
||||
.toSorted((a, b) => a.bin.localeCompare(b.bin));
|
||||
}
|
||||
|
|
@ -686,6 +686,46 @@ description: test skill
|
|||
);
|
||||
});
|
||||
|
||||
it("warns when risky broad-behavior bins are explicitly added to safeBins", async () => {
|
||||
const cases: Array<{
|
||||
name: string;
|
||||
cfg: OpenClawConfig;
|
||||
expected: boolean;
|
||||
}> = [
|
||||
{
|
||||
name: "jq configured globally",
|
||||
cfg: {
|
||||
tools: {
|
||||
exec: {
|
||||
safeBins: ["jq"],
|
||||
},
|
||||
},
|
||||
},
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "jq not configured",
|
||||
cfg: {
|
||||
tools: {
|
||||
exec: {
|
||||
safeBins: ["cut"],
|
||||
},
|
||||
},
|
||||
},
|
||||
expected: false,
|
||||
},
|
||||
];
|
||||
await Promise.all(
|
||||
cases.map(async (testCase) => {
|
||||
const res = await audit(testCase.cfg);
|
||||
expect(
|
||||
hasFinding(res, "tools.exec.safe_bins_broad_behavior", "warn"),
|
||||
testCase.name,
|
||||
).toBe(testCase.expected);
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("evaluates safeBinTrustedDirs risk findings", async () => {
|
||||
const riskyGlobalTrustedDirs =
|
||||
process.platform === "win32"
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import {
|
|||
listInterpreterLikeSafeBins,
|
||||
resolveMergedSafeBinProfileFixtures,
|
||||
} from "../infra/exec-safe-bin-runtime-policy.js";
|
||||
import { listRiskyConfiguredSafeBins } from "../infra/exec-safe-bin-semantics.js";
|
||||
import { normalizeTrustedSafeBinDirs } from "../infra/exec-safe-bin-trust.js";
|
||||
import { isBlockedHostnameOrIp, isPrivateNetworkAllowedByPolicy } from "../infra/net/ssrf.js";
|
||||
import { DEFAULT_AGENT_ID } from "../routing/session-key.js";
|
||||
|
|
@ -1111,6 +1112,7 @@ function collectExecRuntimeFindings(cfg: OpenClawConfig): SecurityAuditFinding[]
|
|||
}
|
||||
|
||||
const interpreterHits: string[] = [];
|
||||
const riskySemanticSafeBinHits: string[] = [];
|
||||
const globalSafeBins = normalizeConfiguredSafeBins(globalExec?.safeBins);
|
||||
if (globalSafeBins.length > 0) {
|
||||
const merged = resolveMergedSafeBinProfileFixtures({ global: globalExec }) ?? {};
|
||||
|
|
@ -1118,6 +1120,9 @@ function collectExecRuntimeFindings(cfg: OpenClawConfig): SecurityAuditFinding[]
|
|||
if (interpreters.length > 0) {
|
||||
interpreterHits.push(`- tools.exec.safeBins: ${interpreters.join(", ")}`);
|
||||
}
|
||||
for (const hit of listRiskyConfiguredSafeBins(globalSafeBins)) {
|
||||
riskySemanticSafeBinHits.push(`- tools.exec.safeBins: ${hit.bin} (${hit.warning})`);
|
||||
}
|
||||
}
|
||||
|
||||
for (const entry of agents) {
|
||||
|
|
@ -1136,11 +1141,21 @@ function collectExecRuntimeFindings(cfg: OpenClawConfig): SecurityAuditFinding[]
|
|||
}) ?? {};
|
||||
const interpreters = listInterpreterLikeSafeBins(agentSafeBins).filter((bin) => !merged[bin]);
|
||||
if (interpreters.length === 0) {
|
||||
for (const hit of listRiskyConfiguredSafeBins(agentSafeBins)) {
|
||||
riskySemanticSafeBinHits.push(
|
||||
`- agents.list.${entry.id}.tools.exec.safeBins: ${hit.bin} (${hit.warning})`,
|
||||
);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
interpreterHits.push(
|
||||
`- agents.list.${entry.id}.tools.exec.safeBins: ${interpreters.join(", ")}`,
|
||||
);
|
||||
for (const hit of listRiskyConfiguredSafeBins(agentSafeBins)) {
|
||||
riskySemanticSafeBinHits.push(
|
||||
`- agents.list.${entry.id}.tools.exec.safeBins: ${hit.bin} (${hit.warning})`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (interpreterHits.length > 0) {
|
||||
|
|
@ -1156,6 +1171,19 @@ function collectExecRuntimeFindings(cfg: OpenClawConfig): SecurityAuditFinding[]
|
|||
});
|
||||
}
|
||||
|
||||
if (riskySemanticSafeBinHits.length > 0) {
|
||||
findings.push({
|
||||
checkId: "tools.exec.safe_bins_broad_behavior",
|
||||
severity: "warn",
|
||||
title: "safeBins includes binaries with broader semantics than low-risk stream filters",
|
||||
detail:
|
||||
`Detected risky safeBins entries:\n${riskySemanticSafeBinHits.join("\n")}\n` +
|
||||
"These tools expose semantics that do not fit the low-risk stdin-filter fast path.",
|
||||
remediation:
|
||||
"Remove these binaries from safeBins and prefer explicit allowlist entries or approval-gated execution.",
|
||||
});
|
||||
}
|
||||
|
||||
if (riskyTrustedDirHits.length > 0) {
|
||||
findings.push({
|
||||
checkId: "tools.exec.safe_bin_trusted_dirs_risky",
|
||||
|
|
|
|||
Loading…
Reference in New Issue