fix(security): block hook transform symlink escapes
This commit is contained in:
@@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Security/Hooks auth: normalize hook auth rate-limit client IP keys so IPv4 and IPv4-mapped IPv6 addresses share one throttle bucket, preventing dual-form auth-attempt budget bypasses. This ships in the next npm release. Thanks @aether-ai-agent for reporting.
|
||||
- Security/Exec approvals: treat `env` and shell-dispatch wrappers as transparent during allowlist analysis on node-host and macOS companion paths so policy checks match the effective executable/inline shell payload instead of the wrapper binary, blocking wrapper-smuggled allowlist bypasses. This ships in the next npm release. Thanks @tdjackey for reporting.
|
||||
- Security/Channels: harden Slack external menu token handling by switching to CSPRNG tokens, validating token shape, requiring user identity for external option lookups, and avoiding fabricated timestamp `trigger_id` fallbacks; also switch Tlon Urbit channel IDs to CSPRNG UUIDs, centralize secure ID/token generation via shared infra helpers, and add a guardrail test to block new runtime `Date.now()+Math.random()` token/id patterns.
|
||||
- Security/Hooks transforms: enforce symlink-safe containment for webhook transform module paths (including `hooks.transformsDir` and `hooks.mappings[].transform.module`) by resolving existing-path ancestors via realpath before import, while preserving in-root symlink support; add regression coverage for both escape and allow cases. This ships in the next npm release. Thanks @aether-ai-agent for reporting.
|
||||
- Telegram/WSL2: disable `autoSelectFamily` by default on WSL2 and memoize WSL2 detection in Telegram network decision logic to avoid repeated sync `/proc/version` probes on fetch/send paths. (#21916) Thanks @MizukiMachine.
|
||||
- Telegram/Streaming: preserve archived draft preview mapping after flush and clean superseded reasoning preview bubbles so multi-message preview finals no longer cross-edit or orphan stale messages under send/rotation races. (#23202) Thanks @obviyus.
|
||||
- Slack/Slash commands: preserve the Bolt app receiver when registering external select options handlers so monitor startup does not crash on runtimes that require bound `app.options` calls. (#23209) Thanks @0xgaia.
|
||||
|
||||
@@ -240,6 +240,92 @@ describe("hooks mapping", () => {
|
||||
const result = await applyNullTransformFromTempConfig({ configDir, transformsDir: "subdir" });
|
||||
expectSkippedTransformResult(result);
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"rejects transform module symlink escape outside transformsDir",
|
||||
() => {
|
||||
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-symlink-module-"));
|
||||
const transformsRoot = path.join(configDir, "hooks", "transforms");
|
||||
fs.mkdirSync(transformsRoot, { recursive: true });
|
||||
const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-outside-module-"));
|
||||
const outsideModule = path.join(outsideDir, "evil.mjs");
|
||||
fs.writeFileSync(outsideModule, 'export default () => ({ kind: "wake", text: "owned" });');
|
||||
fs.symlinkSync(outsideModule, path.join(transformsRoot, "linked.mjs"));
|
||||
expect(() =>
|
||||
resolveHookMappings(
|
||||
{
|
||||
mappings: [
|
||||
{
|
||||
match: { path: "custom" },
|
||||
action: "agent",
|
||||
transform: { module: "linked.mjs" },
|
||||
},
|
||||
],
|
||||
},
|
||||
{ configDir },
|
||||
),
|
||||
).toThrow(/must be within/);
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"rejects transformsDir symlink escape outside transforms root",
|
||||
() => {
|
||||
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-symlink-dir-"));
|
||||
const transformsRoot = path.join(configDir, "hooks", "transforms");
|
||||
fs.mkdirSync(transformsRoot, { recursive: true });
|
||||
const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-outside-dir-"));
|
||||
fs.writeFileSync(path.join(outsideDir, "transform.mjs"), "export default () => null;");
|
||||
fs.symlinkSync(outsideDir, path.join(transformsRoot, "escape"), "dir");
|
||||
expect(() =>
|
||||
resolveHookMappings(
|
||||
{
|
||||
transformsDir: "escape",
|
||||
mappings: [
|
||||
{
|
||||
match: { path: "custom" },
|
||||
action: "agent",
|
||||
transform: { module: "transform.mjs" },
|
||||
},
|
||||
],
|
||||
},
|
||||
{ configDir },
|
||||
),
|
||||
).toThrow(/Hook transformsDir/);
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(process.platform !== "win32")("accepts in-root transform module symlink", async () => {
|
||||
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-symlink-ok-"));
|
||||
const transformsRoot = path.join(configDir, "hooks", "transforms");
|
||||
const nestedDir = path.join(transformsRoot, "nested");
|
||||
fs.mkdirSync(nestedDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(nestedDir, "transform.mjs"), "export default () => null;");
|
||||
fs.symlinkSync(path.join(nestedDir, "transform.mjs"), path.join(transformsRoot, "linked.mjs"));
|
||||
|
||||
const mappings = resolveHookMappings(
|
||||
{
|
||||
mappings: [
|
||||
{
|
||||
match: { path: "skip" },
|
||||
action: "agent",
|
||||
transform: { module: "linked.mjs" },
|
||||
},
|
||||
],
|
||||
},
|
||||
{ configDir },
|
||||
);
|
||||
|
||||
const result = await applyHookMappings(mappings, {
|
||||
payload: {},
|
||||
headers: {},
|
||||
url: new URL("http://127.0.0.1:18789/hooks/skip"),
|
||||
path: "skip",
|
||||
});
|
||||
|
||||
expectSkippedTransformResult(result);
|
||||
});
|
||||
|
||||
it("treats null transform as a handled skip", async () => {
|
||||
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-skip-"));
|
||||
const result = await applyNullTransformFromTempConfig({ configDir });
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { CONFIG_PATH, type HookMappingConfig, type HooksConfig } from "../config/config.js";
|
||||
import { importFileModule, resolveFunctionModuleExport } from "../hooks/module-loader.js";
|
||||
@@ -355,6 +356,34 @@ function resolvePath(baseDir: string, target: string): string {
|
||||
return path.isAbsolute(target) ? path.resolve(target) : path.resolve(baseDir, target);
|
||||
}
|
||||
|
||||
function escapesBase(baseDir: string, candidate: string): boolean {
|
||||
const relative = path.relative(baseDir, candidate);
|
||||
return relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative);
|
||||
}
|
||||
|
||||
function safeRealpathSync(candidate: string): string | null {
|
||||
try {
|
||||
const nativeRealpath = fs.realpathSync.native as ((path: string) => string) | undefined;
|
||||
return nativeRealpath ? nativeRealpath(candidate) : fs.realpathSync(candidate);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function resolveExistingAncestor(candidate: string): string | null {
|
||||
let current = path.resolve(candidate);
|
||||
while (true) {
|
||||
if (fs.existsSync(current)) {
|
||||
return current;
|
||||
}
|
||||
const parent = path.dirname(current);
|
||||
if (parent === current) {
|
||||
return null;
|
||||
}
|
||||
current = parent;
|
||||
}
|
||||
}
|
||||
|
||||
function resolveContainedPath(baseDir: string, target: string, label: string): string {
|
||||
const base = path.resolve(baseDir);
|
||||
const trimmed = target?.trim();
|
||||
@@ -362,8 +391,20 @@ function resolveContainedPath(baseDir: string, target: string, label: string): s
|
||||
throw new Error(`${label} module path is required`);
|
||||
}
|
||||
const resolved = resolvePath(base, trimmed);
|
||||
const relative = path.relative(base, resolved);
|
||||
if (relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) {
|
||||
if (escapesBase(base, resolved)) {
|
||||
throw new Error(`${label} module path must be within ${base}: ${target}`);
|
||||
}
|
||||
|
||||
// Block symlink escapes for existing path segments while preserving current
|
||||
// behavior for not-yet-created files.
|
||||
const baseRealpath = safeRealpathSync(base);
|
||||
const existingAncestor = resolveExistingAncestor(resolved);
|
||||
const existingAncestorRealpath = existingAncestor ? safeRealpathSync(existingAncestor) : null;
|
||||
if (
|
||||
baseRealpath &&
|
||||
existingAncestorRealpath &&
|
||||
escapesBase(baseRealpath, existingAncestorRealpath)
|
||||
) {
|
||||
throw new Error(`${label} module path must be within ${base}: ${target}`);
|
||||
}
|
||||
return resolved;
|
||||
|
||||
Reference in New Issue
Block a user