mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
fix(infra): prevent $-pattern injection in home directory tilde expansion (#122991)
* fix(infra): prevent dollar-pattern injection in home dir tilde expansion String.replace interprets dollar-amp/dollar-1/dollar-backtick in the replacement string. When the home directory contains these sequences (e.g. a username with a dollar sign), tilde expansion via .replace(/^~/, fallbackHome) corrupts the path silently. Use a function replacement so the home path is treated literally, matching the pattern already fixed in terminal-core/display-string (#111398). Two call sites: resolveRawHomeDir and expandHomePrefix. * fix(daemon): prevent dollar-pattern injection in state dir tilde expansion Address review rank-up: the daemon state-path expansion deliberately does not use the core helper and still passed home as a string replacement. Apply the callback form here too and add a literal-dollar regression to the existing service-env suite (fails on the string form, passes with the callback). * fix(launcher): keep literal $ patterns when expanding tilde OPENCLAW_HOME * fix(ui): keep literal $ patterns in local media tilde expansion * test(ui): prove literal-$ tilde local media preview through Control UI e2e * test(ui): align literal-dollar media proof with compact attachment contract Preserve the current authenticated metadata and ticket-scoped download behavior while exercising the real Chromium Control UI under a literal-dollar home. Co-authored-by: liyuanbin <li.yuanbin1@xydigit.com> --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
+1
-1
@@ -509,7 +509,7 @@ const resolveLauncherHomeDir = () => {
|
||||
const explicit = normalizeLauncherHomeValue(process.env.OPENCLAW_HOME);
|
||||
const rawHome =
|
||||
explicit && (explicit === "~" || explicit.startsWith("~/") || explicit.startsWith("~\\"))
|
||||
? explicit.replace(/^~(?=$|[\\/])/, resolveLauncherOsHomeDir())
|
||||
? explicit.replace(/^~(?=$|[\\/])/, () => resolveLauncherOsHomeDir())
|
||||
: (explicit ?? resolveLauncherOsHomeDir());
|
||||
return path.resolve(rawHome);
|
||||
};
|
||||
|
||||
+1
-1
@@ -25,7 +25,7 @@ function resolveUserPathWithHome(input: string, home?: string): string {
|
||||
if (!home) {
|
||||
throw new Error("Missing HOME");
|
||||
}
|
||||
const expanded = trimmed.replace(/^~(?=$|[\\/])/, home);
|
||||
const expanded = trimmed.replace(/^~(?=$|[\\/])/, () => home);
|
||||
return path.resolve(expanded);
|
||||
}
|
||||
if (windowsAbsolutePath.test(trimmed) || windowsUncPath.test(trimmed)) {
|
||||
|
||||
@@ -970,6 +970,11 @@ describe("resolveGatewayStateDir", () => {
|
||||
expect(resolveGatewayStateDir(env)).toBe(path.resolve("/Users/test/openclaw-state"));
|
||||
});
|
||||
|
||||
it("does not interpret $ patterns in HOME when expanding ~ in OPENCLAW_STATE_DIR", () => {
|
||||
const env = { HOME: "/home/$&user", OPENCLAW_STATE_DIR: "~/openclaw-state" };
|
||||
expect(resolveGatewayStateDir(env)).toBe(path.resolve("/home/$&user/openclaw-state"));
|
||||
});
|
||||
|
||||
it("preserves Windows absolute paths without HOME", () => {
|
||||
const env = { OPENCLAW_STATE_DIR: "C:\\State\\openclaw" };
|
||||
expect(resolveGatewayStateDir(env)).toBe("C:\\State\\openclaw");
|
||||
|
||||
@@ -139,6 +139,15 @@ describe("resolveEffectiveHomeDir", () => {
|
||||
|
||||
expect(resolveEffectiveHomeDir(env)).toBe(path.resolve("/home/alice/svc"));
|
||||
});
|
||||
|
||||
it("does not interpret $ patterns in HOME when expanding OPENCLAW_HOME tilde", () => {
|
||||
const env = {
|
||||
OPENCLAW_HOME: "~/state",
|
||||
HOME: "/home/$&user",
|
||||
} as NodeJS.ProcessEnv;
|
||||
|
||||
expect(resolveEffectiveHomeDir(env)).toBe(path.resolve("/home/$&user/state"));
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveRequiredHomeDir", () => {
|
||||
@@ -230,6 +239,12 @@ describe("expandHomePrefix", () => {
|
||||
input: "/tmp/x",
|
||||
expected: "/tmp/x",
|
||||
},
|
||||
{
|
||||
name: "does not interpret $ patterns in home when expanding tilde",
|
||||
input: "~/x",
|
||||
opts: { home: "/home/$&user" },
|
||||
expected: "/home/$&user/x",
|
||||
},
|
||||
])("$name", ({ input, opts, expected }) => {
|
||||
expect(expandHomePrefix(input, opts)).toBe(expected);
|
||||
});
|
||||
|
||||
@@ -48,7 +48,7 @@ function resolveRawHomeDir(env: NodeJS.ProcessEnv, homedir: () => string): strin
|
||||
}
|
||||
if (explicitHome === "~" || explicitHome.startsWith("~/") || explicitHome.startsWith("~\\")) {
|
||||
const fallbackHome = resolveRawOsHomeDir(env, homedir);
|
||||
return fallbackHome ? explicitHome.replace(/^~(?=$|[\\/])/, fallbackHome) : undefined;
|
||||
return fallbackHome ? explicitHome.replace(/^~(?=$|[\\/])/, () => fallbackHome) : undefined;
|
||||
}
|
||||
return explicitHome;
|
||||
}
|
||||
@@ -117,7 +117,7 @@ export function expandHomePrefix(
|
||||
if (!home) {
|
||||
return input;
|
||||
}
|
||||
return input.replace(/^~(?=$|[\\/])/, home);
|
||||
return input.replace(/^~(?=$|[\\/])/, () => home);
|
||||
}
|
||||
|
||||
/** Resolves a user-supplied path after trimming and expanding against the effective home. */
|
||||
|
||||
@@ -634,6 +634,38 @@ describe("openclaw launcher", () => {
|
||||
expect(result.stdout).not.toContain("PRECOMPUTED");
|
||||
});
|
||||
|
||||
it("keeps literal $ patterns in HOME when expanding a tilde OPENCLAW_HOME", async () => {
|
||||
const fixtureRoot = await makeLauncherFixture(fixtureRoots);
|
||||
const home = path.join(fixtureRoot, "home$&d");
|
||||
const configDir = path.join(home, "oc", ".openclaw");
|
||||
await fs.mkdir(configDir, { recursive: true });
|
||||
await fs.writeFile(
|
||||
path.join(fixtureRoot, "dist", "cli-startup-metadata.json"),
|
||||
JSON.stringify({ rootHelpText: "PRECOMPUTED memory help\n" }),
|
||||
"utf8",
|
||||
);
|
||||
await fs.writeFile(
|
||||
path.join(fixtureRoot, "dist", "entry.js"),
|
||||
"process.stdout.write('RUNTIME ENTRY\\n');\n",
|
||||
"utf8",
|
||||
);
|
||||
await fs.writeFile(
|
||||
path.join(configDir, "openclaw.json"),
|
||||
JSON.stringify({ plugins: { slots: { memory: "memory-lancedb" } } }),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
const result = spawnSync(process.execPath, [path.join(fixtureRoot, "openclaw.mjs"), "--help"], {
|
||||
cwd: fixtureRoot,
|
||||
env: launcherEnv({ HOME: home, OPENCLAW_HOME: "~/oc" }),
|
||||
encoding: "utf8",
|
||||
});
|
||||
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.stdout).toBe("RUNTIME ENTRY\n");
|
||||
expect(result.stdout).not.toContain("PRECOMPUTED");
|
||||
});
|
||||
|
||||
it("checks legacy config candidates before using precomputed root help", async () => {
|
||||
const fixtureRoot = await makeLauncherFixture(fixtureRoots);
|
||||
const home = path.join(fixtureRoot, "home");
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
import { mkdir } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { expect, it } from "vitest";
|
||||
import { createChatFlowE2eSuite, installMockGateway } from "./chat-flow.test-support.ts";
|
||||
|
||||
const suite = createChatFlowE2eSuite();
|
||||
|
||||
suite.define(() => {
|
||||
it("allows tilde local media previews when the preview root home contains a literal $ pattern", async () => {
|
||||
const artifactDir = process.env.OPENCLAW_UI_E2E_ARTIFACT_DIR?.trim();
|
||||
const context = await suite.newBrowserContext({
|
||||
locale: "en-US",
|
||||
serviceWorkers: "block",
|
||||
viewport: { height: 900, width: 1280 },
|
||||
});
|
||||
const page = await context.newPage();
|
||||
const source = "~/media/report-voice.mp3";
|
||||
const requestedMediaUrls: URL[] = [];
|
||||
|
||||
await page.route("**/__openclaw__/assistant-media?**", async (route) => {
|
||||
const url = new URL(route.request().url());
|
||||
requestedMediaUrls.push(url);
|
||||
if (url.searchParams.get("meta") === "1") {
|
||||
expect(route.request().headers().authorization).toBe("Bearer e2e-device-token");
|
||||
await route.fulfill({
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({
|
||||
available: true,
|
||||
mediaTicket: "ticket-dollar-home",
|
||||
mediaTicketExpiresAt: new Date(Date.now() + 5 * 60_000).toISOString(),
|
||||
}),
|
||||
});
|
||||
return;
|
||||
}
|
||||
await route.fulfill({
|
||||
contentType: "audio/mpeg",
|
||||
body: Buffer.from("ID3\u0003\u0000\u0000\u0000\u0000\u0000\u0000"),
|
||||
});
|
||||
});
|
||||
|
||||
await installMockGateway(page, {
|
||||
localMediaPreviewRoots: ["/home/us$&r/media"],
|
||||
historyMessages: [
|
||||
{
|
||||
id: "assistant-dollar-home-audio",
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "text", text: "Your recording" },
|
||||
{
|
||||
type: "attachment",
|
||||
attachment: {
|
||||
kind: "audio",
|
||||
label: "report-voice.mp3",
|
||||
mimeType: "audio/mpeg",
|
||||
url: source,
|
||||
},
|
||||
},
|
||||
],
|
||||
timestamp: Date.now(),
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
try {
|
||||
await page.goto(`${suite.server.baseUrl}chat`);
|
||||
const attachment = page.locator(".chat-assistant-attachment-card--compact");
|
||||
await attachment.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await expect.poll(() => requestedMediaUrls.length, { timeout: 10_000 }).toBe(1);
|
||||
expect(requestedMediaUrls[0]?.searchParams.get("meta")).toBe("1");
|
||||
expect(requestedMediaUrls[0]?.searchParams.get("source")).toBe(source);
|
||||
const downloadHref = await attachment
|
||||
.locator(".chat-assistant-attachment-card__download")
|
||||
.getAttribute("href");
|
||||
expect(downloadHref).toBeTruthy();
|
||||
const downloadUrl = new URL(downloadHref ?? "", suite.server.baseUrl);
|
||||
expect(downloadUrl.searchParams.get("mediaTicket")).toBe("ticket-dollar-home");
|
||||
expect(downloadUrl.searchParams.get("source")).toBe(source);
|
||||
expect(await attachment.locator("audio, video").count()).toBe(0);
|
||||
expect(await page.getByText("Outside allowed folders").count()).toBe(0);
|
||||
if (artifactDir) {
|
||||
await mkdir(artifactDir, { recursive: true });
|
||||
await page.screenshot({
|
||||
fullPage: true,
|
||||
path: path.join(artifactDir, "local-media-dollar-home-allowed.png"),
|
||||
});
|
||||
}
|
||||
} finally {
|
||||
await suite.closeBrowserContext(context);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,11 @@
|
||||
// Control UI tests cover local media preview path policy.
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { isLocalAttachmentPreviewAllowed } from "./chat-message-local-media.ts";
|
||||
|
||||
describe("isLocalAttachmentPreviewAllowed", () => {
|
||||
it("keeps literal $ patterns in home when expanding tilde sources", () => {
|
||||
const roots = ["/home/us$&r/media"];
|
||||
expect(isLocalAttachmentPreviewAllowed("~/media/report.png", roots)).toBe(true);
|
||||
expect(isLocalAttachmentPreviewAllowed("~/elsewhere/report.png", roots)).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -96,7 +96,7 @@ export function isLocalAttachmentPreviewAllowed(
|
||||
? [canonicalizeLocalPathForComparison(normalizedSource)]
|
||||
: source.trim().startsWith("~")
|
||||
? resolveHomeCandidatesFromRoots(localMediaPreviewRoots).map((home) =>
|
||||
canonicalizeLocalPathForComparison(source.trim().replace(/^~(?=$|[\\/])/, home)),
|
||||
canonicalizeLocalPathForComparison(source.trim().replace(/^~(?=$|[\\/])/, () => home)),
|
||||
)
|
||||
: [];
|
||||
if (comparableSources.length === 0) {
|
||||
|
||||
@@ -346,6 +346,8 @@ export type ControlUiMockGatewayScenario = {
|
||||
cliAgentsEnabled?: boolean;
|
||||
workspace?: string;
|
||||
workspaceGit?: boolean;
|
||||
/** Local media preview roots served in the bootstrap config; tilde sources expand against these. */
|
||||
localMediaPreviewRoots?: string[];
|
||||
};
|
||||
|
||||
type NormalizedControlUiMockGatewayScenario = Required<ControlUiMockGatewayScenario>;
|
||||
@@ -919,6 +921,7 @@ function normalizeScenario(
|
||||
cliAgentsEnabled: scenario.cliAgentsEnabled ?? false,
|
||||
workspace: scenario.workspace ?? "",
|
||||
workspaceGit: scenario.workspaceGit ?? false,
|
||||
localMediaPreviewRoots: scenario.localMediaPreviewRoots ?? [],
|
||||
};
|
||||
}
|
||||
|
||||
@@ -933,7 +936,7 @@ export function createControlUiMockBootstrapConfig(scenario: ControlUiMockGatewa
|
||||
basePath: normalizedScenario.basePath,
|
||||
devGitBranch: normalizedScenario.devGitBranch || undefined,
|
||||
embedSandbox: "scripts",
|
||||
localMediaPreviewRoots: [],
|
||||
localMediaPreviewRoots: normalizedScenario.localMediaPreviewRoots,
|
||||
serverVersion: normalizedScenario.serverVersion,
|
||||
serverBuildId: normalizedScenario.serverBuildId,
|
||||
terminalEnabled: normalizedScenario.terminalEnabled,
|
||||
|
||||
Reference in New Issue
Block a user