mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-03 21:31:26 +00:00
fix: scope #57584 to shell allowlist changes
This commit is contained in:
Peter Steinberger
@@ -182,11 +182,6 @@ export function handleMessageStart(
|
||||
// may deliver late text_end updates after message_end, which would otherwise
|
||||
// re-trigger block replies.
|
||||
ctx.resetAssistantMessageState(ctx.state.assistantTexts.length);
|
||||
// Resolve text-repetition-guard config once per message to avoid re-parsing on every tick.
|
||||
ctx.state.resolvedTextRepetitionGuardConfig = resolveTextRepetitionGuardConfig({
|
||||
cfg: ctx.params.config,
|
||||
agentId: ctx.params.agentId,
|
||||
});
|
||||
// Use assistant message_start as the earliest "writing" signal for typing.
|
||||
void ctx.params.onAssistantMessageStart?.();
|
||||
}
|
||||
@@ -287,21 +282,6 @@ export function handleMessageUpdate(
|
||||
}
|
||||
}
|
||||
|
||||
// Text repetition guard: throttle checks to every N chars of new content.
|
||||
const guardConfig = ctx.state.resolvedTextRepetitionGuardConfig;
|
||||
if (guardConfig && guardConfig.enabled) {
|
||||
const checkInterval = guardConfig.checkIntervalChars;
|
||||
if (ctx.state.deltaBuffer.length - ctx.state.textRepetitionLastCheckedLen >= checkInterval) {
|
||||
ctx.state.textRepetitionLastCheckedLen = ctx.state.deltaBuffer.length;
|
||||
const result = detectTextRepetition(ctx.state.deltaBuffer, guardConfig);
|
||||
if (result.looping) {
|
||||
ctx.state.abortedByTextRepetitionGuard = true;
|
||||
void ctx.params.session.abort();
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (ctx.state.streamReasoning) {
|
||||
// Handle partial <think> tags: stream whatever reasoning is visible so far.
|
||||
ctx.emitReasoningStream(extractThinkingFromTaggedStream(ctx.state.deltaBuffer));
|
||||
@@ -538,10 +518,10 @@ export function handleMessageEnd(
|
||||
text &&
|
||||
onBlockReply
|
||||
) {
|
||||
if (ctx.blockChunker?.hasBuffered() && !ctx.state.abortedByTextRepetitionGuard) {
|
||||
if (ctx.blockChunker?.hasBuffered()) {
|
||||
ctx.blockChunker.drain({ force: true, emit: ctx.emitBlockChunk });
|
||||
ctx.blockChunker.reset();
|
||||
} else if (text !== ctx.state.lastBlockReplyText && !ctx.state.abortedByTextRepetitionGuard) {
|
||||
} else if (text !== ctx.state.lastBlockReplyText) {
|
||||
// Check for duplicates before emitting (same logic as emitBlockChunk).
|
||||
const normalizedText = normalizeTextForComparison(text);
|
||||
if (
|
||||
|
||||
@@ -367,73 +367,44 @@ function resolveTrustedSkillExecutionIds(params: {
|
||||
return skillIds;
|
||||
}
|
||||
|
||||
/**
|
||||
* Maximum recursion depth for evaluating shell wrapper inline commands.
|
||||
* Prevents infinite recursion from nested shell wrappers (e.g. `sh -c "sh -c '...'"`)
|
||||
* while allowing reasonable nesting for skill-constructed compound commands.
|
||||
*/
|
||||
const MAX_SHELL_WRAPPER_INLINE_EVAL_DEPTH = 3;
|
||||
|
||||
/**
|
||||
* Recursively evaluates a shell wrapper's inline command (the `-c` payload) against the
|
||||
* allowlist by parsing it into sub-commands and checking each one.
|
||||
*
|
||||
* Returns the evaluation result when the inline command is parseable and all sub-commands
|
||||
* are satisfied, or `null` when evaluation fails or any sub-command is unsatisfied.
|
||||
*/
|
||||
function evaluateShellWrapperInlineCommand(
|
||||
inlineCommand: string,
|
||||
params: ExecAllowlistContext,
|
||||
inlineDepth: number,
|
||||
precomputedChainParts?: string[],
|
||||
): { matches: ExecAllowlistEntry[]; segmentSatisfiedBy: ExecSegmentSatisfiedBy[] } | null {
|
||||
if (inlineDepth >= MAX_SHELL_WRAPPER_INLINE_EVAL_DEPTH) {
|
||||
function evaluateShellWrapperInlineChain(params: {
|
||||
inlineCommand: string;
|
||||
context: ExecAllowlistContext;
|
||||
inlineDepth: number;
|
||||
precomputedChainParts?: string[];
|
||||
}): { matches: ExecAllowlistEntry[] } | null {
|
||||
if (params.inlineDepth >= MAX_SHELL_WRAPPER_INLINE_EVAL_DEPTH) {
|
||||
return null;
|
||||
}
|
||||
if (isWindowsPlatform(params.context.platform)) {
|
||||
return null;
|
||||
}
|
||||
const chainParts = params.precomputedChainParts ?? splitCommandChain(params.inlineCommand);
|
||||
if (!chainParts || chainParts.length <= 1) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const chainParts =
|
||||
precomputedChainParts ?? (isWindowsPlatform(params.platform) ? null : splitCommandChain(inlineCommand));
|
||||
|
||||
if (!chainParts) {
|
||||
// Single command or pipeline (no chain operators).
|
||||
const analysis = analyzeShellCommand({
|
||||
command: inlineCommand,
|
||||
cwd: params.cwd,
|
||||
env: params.env,
|
||||
platform: params.platform,
|
||||
});
|
||||
if (!analysis.ok) {
|
||||
return null;
|
||||
}
|
||||
const result = evaluateSegments(analysis.segments, params, inlineDepth);
|
||||
return result.satisfied
|
||||
? { matches: result.matches, segmentSatisfiedBy: result.segmentSatisfiedBy }
|
||||
: null;
|
||||
}
|
||||
|
||||
// Multiple commands joined by chain operators (&&, ||, ;).
|
||||
const allMatches: ExecAllowlistEntry[] = [];
|
||||
const allSegmentSatisfiedBy: ExecSegmentSatisfiedBy[] = [];
|
||||
const matches: ExecAllowlistEntry[] = [];
|
||||
for (const part of chainParts) {
|
||||
const analysis = analyzeShellCommand({
|
||||
command: part,
|
||||
cwd: params.cwd,
|
||||
env: params.env,
|
||||
platform: params.platform,
|
||||
cwd: params.context.cwd,
|
||||
env: params.context.env,
|
||||
platform: params.context.platform,
|
||||
});
|
||||
if (!analysis.ok) {
|
||||
return null;
|
||||
}
|
||||
const result = evaluateSegments(analysis.segments, params, inlineDepth);
|
||||
const result = evaluateSegments(analysis.segments, params.context, params.inlineDepth);
|
||||
if (!result.satisfied) {
|
||||
return null;
|
||||
}
|
||||
allMatches.push(...result.matches);
|
||||
allSegmentSatisfiedBy.push(...result.segmentSatisfiedBy);
|
||||
matches.push(...result.matches);
|
||||
}
|
||||
return { matches: allMatches, segmentSatisfiedBy: allSegmentSatisfiedBy };
|
||||
return { matches };
|
||||
}
|
||||
|
||||
function evaluateSegments(
|
||||
segments: ExecCommandSegment[],
|
||||
params: ExecAllowlistContext,
|
||||
@@ -550,34 +521,24 @@ function evaluateSegments(
|
||||
: skillAllow
|
||||
? "skills"
|
||||
: null;
|
||||
|
||||
// When no direct match and the segment is a shell wrapper with an inline *compound*
|
||||
// command (e.g. `/bin/sh -c "cat SKILL.md && gog-wrapper calendar events"`),
|
||||
// recursively parse and evaluate each sub-command inside the inline payload.
|
||||
// This prevents shell-wrapped skill exec commands from being silently rejected
|
||||
// because the allowlist check was comparing `/bin/sh` instead of the actual target
|
||||
// binaries inside the compound command.
|
||||
//
|
||||
// Only recurse when the inline command contains chain operators (&&, ||, ;) —
|
||||
// single-command shell wrappers (e.g. `bash -lc 'script.sh'`) keep the original
|
||||
// behavior to preserve allow-always persisted-pattern security constraints.
|
||||
const inlineChainParts = by === null && inlineCommand ? splitCommandChain(inlineCommand) : null;
|
||||
if (inlineChainParts !== null && inlineChainParts.length > 1) {
|
||||
const inlineResult = evaluateShellWrapperInlineCommand(
|
||||
inlineCommand,
|
||||
params,
|
||||
inlineDepth + 1,
|
||||
inlineChainParts,
|
||||
);
|
||||
if (inlineResult !== null) {
|
||||
matches.push(...inlineResult.matches);
|
||||
// Preserve the per-sub-command satisfaction reasons from the inline evaluation
|
||||
// rather than collapsing them into a single "allowlist" entry for the wrapper segment.
|
||||
segmentSatisfiedBy.push(...inlineResult.segmentSatisfiedBy);
|
||||
return true;
|
||||
if (by === null && inlineCommand) {
|
||||
const inlineChainParts = splitCommandChain(inlineCommand);
|
||||
if (inlineChainParts && inlineChainParts.length > 1) {
|
||||
const inlineResult = evaluateShellWrapperInlineChain({
|
||||
inlineCommand,
|
||||
context: params,
|
||||
inlineDepth: inlineDepth + 1,
|
||||
precomputedChainParts: inlineChainParts,
|
||||
});
|
||||
if (inlineResult) {
|
||||
matches.push(...inlineResult.matches);
|
||||
// Keep per-segment metadata aligned with segments: one satisfaction marker
|
||||
// for this wrapper segment, even when the inline payload has multiple parts.
|
||||
segmentSatisfiedBy.push("allowlist");
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
segmentSatisfiedBy.push(by);
|
||||
return Boolean(by);
|
||||
});
|
||||
|
||||
@@ -645,7 +645,7 @@ describe("exec approvals shell analysis", () => {
|
||||
env,
|
||||
});
|
||||
expect(result.analysisOk).toBe(true);
|
||||
expect(result.allowlistSatisfied).toBe(false);
|
||||
expect(result.allowlistSatisfied).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
@@ -654,138 +654,101 @@ describe("exec approvals shell analysis", () => {
|
||||
it("normalizes safe bin names", () => {
|
||||
expect([...normalizeSafeBins([" jq ", "", "JQ", " sort "])]).toEqual(["jq", "sort"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("shell wrapper inline command evaluation (#57377)", () => {
|
||||
it("satisfies allowlist for shell-wrapped compound command when all inner binaries match", () => {
|
||||
const dir = makeTempDir();
|
||||
const catPath = path.join(dir, "cat");
|
||||
const printfPath = path.join(dir, "printf");
|
||||
const gogPath = path.join(dir, "gog-wrapper");
|
||||
const shPath = path.join(dir, "sh");
|
||||
fs.writeFileSync(catPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(printfPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(gogPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(shPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
const env = makePathEnv(dir);
|
||||
try {
|
||||
const result = evaluateShellAllowlist({
|
||||
command: `${shPath} -c "cat SKILL.md && printf '---CMD---' && gog-wrapper calendar events"`,
|
||||
allowlist: [{ pattern: catPath }, { pattern: printfPath }, { pattern: gogPath }],
|
||||
safeBins: new Set(),
|
||||
cwd: dir,
|
||||
env,
|
||||
});
|
||||
expect(result.analysisOk).toBe(true);
|
||||
expect(result.allowlistSatisfied).toBe(true);
|
||||
expect(result.allowlistMatches.length).toBe(3);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
describe("shell wrapper inline compound allowlist", () => {
|
||||
const commonShells = ["sh", "bash", "zsh", "dash", "ksh", "fish", "ash"] as const;
|
||||
|
||||
function writeExecutable(filePath: string) {
|
||||
fs.writeFileSync(filePath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
}
|
||||
});
|
||||
|
||||
it("satisfies allowlist for shell-wrapped compound command with safe bins", () => {
|
||||
const dir = makeTempDir();
|
||||
const headPath = path.join(dir, "head");
|
||||
const tailPath = path.join(dir, "tail");
|
||||
const gogPath = path.join(dir, "gog-wrapper");
|
||||
const shPath = path.join(dir, "sh");
|
||||
fs.writeFileSync(headPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(tailPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(gogPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(shPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
const env = makePathEnv(dir);
|
||||
try {
|
||||
const result = evaluateShellAllowlist({
|
||||
command: `${shPath} -c "head -n 1 && tail -n 1 && gog-wrapper calendar events"`,
|
||||
allowlist: [{ pattern: gogPath }],
|
||||
safeBins: normalizeSafeBins(["head", "tail"]),
|
||||
trustedSafeBinDirs: new Set([dir]),
|
||||
cwd: dir,
|
||||
env,
|
||||
});
|
||||
expect(result.analysisOk).toBe(true);
|
||||
// Safe bins are disabled on Windows
|
||||
it.each(commonShells)("evaluates inner chain commands for %s -c wrappers", (shellBinary) => {
|
||||
if (process.platform === "win32") {
|
||||
expect(result.allowlistSatisfied).toBe(false);
|
||||
return;
|
||||
}
|
||||
expect(result.allowlistSatisfied).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
const dir = makeTempDir();
|
||||
const shellPath = path.join(dir, shellBinary);
|
||||
const catPath = path.join(dir, "cat");
|
||||
const printfPath = path.join(dir, "printf");
|
||||
const gogPath = path.join(dir, "gog-wrapper");
|
||||
writeExecutable(shellPath);
|
||||
writeExecutable(catPath);
|
||||
writeExecutable(printfPath);
|
||||
writeExecutable(gogPath);
|
||||
const env = makePathEnv(dir);
|
||||
try {
|
||||
const result = evaluateShellAllowlist({
|
||||
command: `${shellPath} -c "cat SKILL.md && printf '---CMD---' && gog-wrapper calendar events"`,
|
||||
allowlist: [{ pattern: catPath }, { pattern: printfPath }, { pattern: gogPath }],
|
||||
safeBins: new Set(),
|
||||
cwd: dir,
|
||||
env,
|
||||
});
|
||||
expect(result.analysisOk).toBe(true);
|
||||
expect(result.allowlistSatisfied).toBe(true);
|
||||
expect(result.allowlistMatches.length).toBe(3);
|
||||
expect(result.segmentSatisfiedBy).toEqual(["allowlist"]);
|
||||
expect(result.segmentAllowlistEntries).toEqual([null]);
|
||||
expect(result.segmentSatisfiedBy.length).toBe(result.segments.length);
|
||||
expect(result.segmentAllowlistEntries.length).toBe(result.segments.length);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects shell-wrapped compound command when an inner binary is not in allowlist", () => {
|
||||
const dir = makeTempDir();
|
||||
const catPath = path.join(dir, "cat");
|
||||
const gogPath = path.join(dir, "gog-wrapper");
|
||||
const rmPath = path.join(dir, "rm");
|
||||
const shPath = path.join(dir, "sh");
|
||||
fs.writeFileSync(catPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(gogPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(rmPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(shPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
const env = makePathEnv(dir);
|
||||
try {
|
||||
const result = evaluateShellAllowlist({
|
||||
command: `${shPath} -c "cat SKILL.md && rm -rf / && gog-wrapper calendar events"`,
|
||||
allowlist: [{ pattern: catPath }, { pattern: gogPath }],
|
||||
safeBins: new Set(),
|
||||
cwd: dir,
|
||||
env,
|
||||
});
|
||||
expect(result.analysisOk).toBe(true);
|
||||
expect(result.allowlistSatisfied).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
it("rejects wrapper chain when any inner command misses the allowlist", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const dir = makeTempDir();
|
||||
const shellPath = path.join(dir, "sh");
|
||||
const catPath = path.join(dir, "cat");
|
||||
const rmPath = path.join(dir, "rm");
|
||||
const gogPath = path.join(dir, "gog-wrapper");
|
||||
writeExecutable(shellPath);
|
||||
writeExecutable(catPath);
|
||||
writeExecutable(rmPath);
|
||||
writeExecutable(gogPath);
|
||||
const env = makePathEnv(dir);
|
||||
try {
|
||||
const result = evaluateShellAllowlist({
|
||||
command: `${shellPath} -c "cat SKILL.md && rm -rf / && gog-wrapper calendar events"`,
|
||||
allowlist: [{ pattern: catPath }, { pattern: gogPath }],
|
||||
safeBins: new Set(),
|
||||
cwd: dir,
|
||||
env,
|
||||
});
|
||||
expect(result.analysisOk).toBe(true);
|
||||
expect(result.allowlistSatisfied).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("handles single command inside shell wrapper (no chain operators — not recursed)", () => {
|
||||
const dir = makeTempDir();
|
||||
const gogPath = path.join(dir, "gog-wrapper");
|
||||
const shPath = path.join(dir, "sh");
|
||||
fs.writeFileSync(gogPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(shPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
const env = makePathEnv(dir);
|
||||
try {
|
||||
// Single-command shell wrappers are NOT recursively evaluated to preserve
|
||||
// allow-always persisted-pattern security constraints.
|
||||
const result = evaluateShellAllowlist({
|
||||
command: `${shPath} -c "gog-wrapper calendar events"`,
|
||||
allowlist: [{ pattern: gogPath }],
|
||||
safeBins: new Set(),
|
||||
cwd: dir,
|
||||
env,
|
||||
});
|
||||
expect(result.analysisOk).toBe(true);
|
||||
expect(result.allowlistSatisfied).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("handles shell wrapper without -c flag (script invocation) unchanged", () => {
|
||||
const dir = makeTempDir();
|
||||
const shPath = path.join(dir, "sh");
|
||||
const scriptPath = path.join(dir, "myscript.sh");
|
||||
fs.writeFileSync(shPath, "#!/bin/sh\n", { mode: 0o755 });
|
||||
fs.writeFileSync(scriptPath, "#!/bin/sh\necho hello\n", { mode: 0o755 });
|
||||
const env = makePathEnv(dir);
|
||||
try {
|
||||
const result = evaluateShellAllowlist({
|
||||
command: `${shPath} ${scriptPath}`,
|
||||
allowlist: [{ pattern: scriptPath }],
|
||||
safeBins: new Set(),
|
||||
cwd: dir,
|
||||
env,
|
||||
});
|
||||
expect(result.analysisOk).toBe(true);
|
||||
expect(result.allowlistSatisfied).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
it("keeps single-command wrappers unchanged (no recursive allowlist lookup)", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const dir = makeTempDir();
|
||||
const shellPath = path.join(dir, "sh");
|
||||
const gogPath = path.join(dir, "gog-wrapper");
|
||||
writeExecutable(shellPath);
|
||||
writeExecutable(gogPath);
|
||||
const env = makePathEnv(dir);
|
||||
try {
|
||||
const result = evaluateShellAllowlist({
|
||||
command: `${shellPath} -c "gog-wrapper calendar events"`,
|
||||
allowlist: [{ pattern: gogPath }],
|
||||
safeBins: new Set(),
|
||||
cwd: dir,
|
||||
env,
|
||||
});
|
||||
expect(result.analysisOk).toBe(true);
|
||||
expect(result.allowlistSatisfied).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user