mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-09 12:57:40 +00:00
perf(security): bound regex input in filters and redaction
This commit is contained in:
@@ -318,6 +318,17 @@ describe("DiscordExecApprovalHandler.shouldHandle", () => {
|
|||||||
expect(handler.shouldHandle(createRequest({ sessionKey: `${"a".repeat(28)}!` }))).toBe(false);
|
expect(handler.shouldHandle(createRequest({ sessionKey: `${"a".repeat(28)}!` }))).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("matches long session keys with tail-bounded regex checks", () => {
|
||||||
|
const handler = createHandler({
|
||||||
|
enabled: true,
|
||||||
|
approvers: ["123"],
|
||||||
|
sessionFilter: ["discord:tail$"],
|
||||||
|
});
|
||||||
|
expect(
|
||||||
|
handler.shouldHandle(createRequest({ sessionKey: `${"x".repeat(5000)}discord:tail` })),
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
it("filters by discord account when session store includes account", () => {
|
it("filters by discord account when session store includes account", () => {
|
||||||
writeStore({
|
writeStore({
|
||||||
"agent:test-agent:discord:channel:999888777": {
|
"agent:test-agent:discord:channel:999888777": {
|
||||||
|
|||||||
@@ -34,6 +34,7 @@ import { createDiscordClient, stripUndefinedFields } from "../send.shared.js";
|
|||||||
import { DiscordUiContainer } from "../ui.js";
|
import { DiscordUiContainer } from "../ui.js";
|
||||||
|
|
||||||
const EXEC_APPROVAL_KEY = "execapproval";
|
const EXEC_APPROVAL_KEY = "execapproval";
|
||||||
|
const SESSION_FILTER_REGEX_MAX_INPUT = 2048;
|
||||||
|
|
||||||
export type { ExecApprovalRequest, ExecApprovalResolved };
|
export type { ExecApprovalRequest, ExecApprovalResolved };
|
||||||
|
|
||||||
@@ -367,12 +368,28 @@ export class DiscordExecApprovalHandler {
|
|||||||
if (!session) {
|
if (!session) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
const head = session.slice(0, SESSION_FILTER_REGEX_MAX_INPUT);
|
||||||
|
const tail =
|
||||||
|
session.length > SESSION_FILTER_REGEX_MAX_INPUT
|
||||||
|
? session.slice(-SESSION_FILTER_REGEX_MAX_INPUT)
|
||||||
|
: "";
|
||||||
const matches = config.sessionFilter.some((p) => {
|
const matches = config.sessionFilter.some((p) => {
|
||||||
if (session.includes(p)) {
|
if (session.includes(p)) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
const regex = compileSafeRegex(p);
|
const regex = compileSafeRegex(p);
|
||||||
return regex ? regex.test(session) : false;
|
if (!regex) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
regex.lastIndex = 0;
|
||||||
|
if (regex.test(head)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (tail) {
|
||||||
|
regex.lastIndex = 0;
|
||||||
|
return regex.test(tail);
|
||||||
|
}
|
||||||
|
return false;
|
||||||
});
|
});
|
||||||
if (!matches) {
|
if (!matches) {
|
||||||
return false;
|
return false;
|
||||||
|
|||||||
@@ -194,6 +194,34 @@ describe("exec approval forwarder", () => {
|
|||||||
expect(deliver).not.toHaveBeenCalled();
|
expect(deliver).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("matches long session keys with tail-bounded regex checks", async () => {
|
||||||
|
const cfg = {
|
||||||
|
approvals: {
|
||||||
|
exec: {
|
||||||
|
enabled: true,
|
||||||
|
mode: "session",
|
||||||
|
sessionFilter: ["discord:tail$"],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
} as OpenClawConfig;
|
||||||
|
|
||||||
|
const { deliver, forwarder } = createForwarder({
|
||||||
|
cfg,
|
||||||
|
resolveSessionTarget: () => ({ channel: "slack", to: "U1" }),
|
||||||
|
});
|
||||||
|
|
||||||
|
const request = {
|
||||||
|
...baseRequest,
|
||||||
|
request: {
|
||||||
|
...baseRequest.request,
|
||||||
|
sessionKey: `${"x".repeat(5000)}discord:tail`,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
await expect(forwarder.handleRequested(request)).resolves.toBe(true);
|
||||||
|
expect(deliver).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
it("returns false when all targets are skipped", async () => {
|
it("returns false when all targets are skipped", async () => {
|
||||||
await expectDiscordSessionTargetRequest({
|
await expectDiscordSessionTargetRequest({
|
||||||
cfg: makeSessionCfg({ discordExecApprovalsEnabled: true }),
|
cfg: makeSessionCfg({ discordExecApprovalsEnabled: true }),
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ import { deliverOutboundPayloads } from "./outbound/deliver.js";
|
|||||||
import { resolveSessionDeliveryTarget } from "./outbound/targets.js";
|
import { resolveSessionDeliveryTarget } from "./outbound/targets.js";
|
||||||
|
|
||||||
const log = createSubsystemLogger("gateway/exec-approvals");
|
const log = createSubsystemLogger("gateway/exec-approvals");
|
||||||
|
const SESSION_FILTER_REGEX_MAX_INPUT = 2048;
|
||||||
|
|
||||||
export type { ExecApprovalRequest, ExecApprovalResolved };
|
export type { ExecApprovalRequest, ExecApprovalResolved };
|
||||||
|
|
||||||
@@ -56,12 +57,28 @@ function normalizeMode(mode?: ExecApprovalForwardingConfig["mode"]) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function matchSessionFilter(sessionKey: string, patterns: string[]): boolean {
|
function matchSessionFilter(sessionKey: string, patterns: string[]): boolean {
|
||||||
|
const head = sessionKey.slice(0, SESSION_FILTER_REGEX_MAX_INPUT);
|
||||||
|
const tail =
|
||||||
|
sessionKey.length > SESSION_FILTER_REGEX_MAX_INPUT
|
||||||
|
? sessionKey.slice(-SESSION_FILTER_REGEX_MAX_INPUT)
|
||||||
|
: "";
|
||||||
return patterns.some((pattern) => {
|
return patterns.some((pattern) => {
|
||||||
if (sessionKey.includes(pattern)) {
|
if (sessionKey.includes(pattern)) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
const regex = compileSafeRegex(pattern);
|
const regex = compileSafeRegex(pattern);
|
||||||
return regex ? regex.test(sessionKey) : false;
|
if (!regex) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
regex.lastIndex = 0;
|
||||||
|
if (regex.test(head)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (tail) {
|
||||||
|
regex.lastIndex = 0;
|
||||||
|
return regex.test(tail);
|
||||||
|
}
|
||||||
|
return false;
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -102,6 +102,15 @@ describe("redactSensitiveText", () => {
|
|||||||
expect(output).toBe(input);
|
expect(output).toBe(input);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("redacts large payloads with bounded regex passes", () => {
|
||||||
|
const input = `${"x".repeat(40_000)} OPENAI_API_KEY=sk-1234567890abcdef ${"y".repeat(40_000)}`;
|
||||||
|
const output = redactSensitiveText(input, {
|
||||||
|
mode: "tools",
|
||||||
|
patterns: defaults,
|
||||||
|
});
|
||||||
|
expect(output).toContain("OPENAI_API_KEY=sk-123…cdef");
|
||||||
|
});
|
||||||
|
|
||||||
it("skips redaction when mode is off", () => {
|
it("skips redaction when mode is off", () => {
|
||||||
const input = "OPENAI_API_KEY=sk-1234567890abcdef";
|
const input = "OPENAI_API_KEY=sk-1234567890abcdef";
|
||||||
const output = redactSensitiveText(input, {
|
const output = redactSensitiveText(input, {
|
||||||
|
|||||||
@@ -10,6 +10,8 @@ const DEFAULT_REDACT_MODE: RedactSensitiveMode = "tools";
|
|||||||
const DEFAULT_REDACT_MIN_LENGTH = 18;
|
const DEFAULT_REDACT_MIN_LENGTH = 18;
|
||||||
const DEFAULT_REDACT_KEEP_START = 6;
|
const DEFAULT_REDACT_KEEP_START = 6;
|
||||||
const DEFAULT_REDACT_KEEP_END = 4;
|
const DEFAULT_REDACT_KEEP_END = 4;
|
||||||
|
const REDACT_REGEX_CHUNK_THRESHOLD = 32_768;
|
||||||
|
const REDACT_REGEX_CHUNK_SIZE = 16_384;
|
||||||
|
|
||||||
const DEFAULT_REDACT_PATTERNS: string[] = [
|
const DEFAULT_REDACT_PATTERNS: string[] = [
|
||||||
// ENV-style assignments.
|
// ENV-style assignments.
|
||||||
@@ -94,12 +96,26 @@ function redactMatch(match: string, groups: string[]): string {
|
|||||||
return match.replace(token, masked);
|
return match.replace(token, masked);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function replacePatternWithBounds(text: string, pattern: RegExp): string {
|
||||||
|
const apply = (value: string) =>
|
||||||
|
value.replace(pattern, (...args: string[]) =>
|
||||||
|
redactMatch(args[0], args.slice(1, args.length - 2)),
|
||||||
|
);
|
||||||
|
if (text.length <= REDACT_REGEX_CHUNK_THRESHOLD) {
|
||||||
|
return apply(text);
|
||||||
|
}
|
||||||
|
|
||||||
|
let output = "";
|
||||||
|
for (let index = 0; index < text.length; index += REDACT_REGEX_CHUNK_SIZE) {
|
||||||
|
output += apply(text.slice(index, index + REDACT_REGEX_CHUNK_SIZE));
|
||||||
|
}
|
||||||
|
return output;
|
||||||
|
}
|
||||||
|
|
||||||
function redactText(text: string, patterns: RegExp[]): string {
|
function redactText(text: string, patterns: RegExp[]): string {
|
||||||
let next = text;
|
let next = text;
|
||||||
for (const pattern of patterns) {
|
for (const pattern of patterns) {
|
||||||
next = next.replace(pattern, (...args: string[]) =>
|
next = replacePatternWithBounds(next, pattern);
|
||||||
redactMatch(args[0], args.slice(1, args.length - 2)),
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
return next;
|
return next;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user