refactor(security): consolidate path containment onto canonical fs-safe guard (#124870)

* refactor(security): expose canonical path containment modes

* refactor(agents): use canonical path containment guard

* refactor(security): consolidate core path containment sites

* refactor(plugins): consolidate path containment sites

* test(security): cover canonical path containment behavior
This commit is contained in:
Peter Steinberger
2026-08-16 15:37:02 -07:00
committed by GitHub
parent 9b468ccaa2
commit add30d455d
30 changed files with 148 additions and 301 deletions
+2 -14
View File
@@ -7,7 +7,7 @@ import fs from "node:fs/promises";
import path from "node:path";
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
import type { ExecHost } from "../infra/exec-approvals.js";
import { safeStatSync } from "../infra/path-guards.js";
import { isPathInside, safeStatSync } from "../infra/path-guards.js";
import type { BashSandboxConfig } from "./bash-tools.shared.js";
import { assertSandboxPath } from "./sandbox-paths.js";
@@ -63,13 +63,6 @@ function resolveExistingHostWorkdir(workdir: string): string | null {
return stats?.isDirectory() ? workdir : null;
}
function isHostPathInsideRoot(params: { root: string; candidate: string }): boolean {
const root = path.resolve(params.root);
const candidate = path.resolve(params.candidate);
const relative = path.relative(root, candidate);
return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
}
function safeCurrentCwd(): string | null {
try {
return process.cwd();
@@ -275,12 +268,7 @@ function resolveBackendHostWorkdirCandidate(params: {
if (readOnlySkillMapping) {
return { ...readOnlySkillMapping, failIfInvalid: false };
}
if (
isHostPathInsideRoot({
root: params.sandbox.workspaceDir,
candidate: hostPath,
})
) {
if (isPathInside(path.resolve(params.sandbox.workspaceDir), hostPath)) {
return {
hostPath,
hostRoot: path.resolve(params.sandbox.workspaceDir),
+2 -7
View File
@@ -142,7 +142,7 @@ export function resolveWritableSandboxBindHostRoots(
if (
!parsed.writable ||
seen.has(parsed.hostRoot) ||
readonlyRoots.some((root) => isHostPathWithinOrEqual(parsed.hostRoot, root))
readonlyRoots.some((root) => isPathInside(parsed.hostRoot, root))
) {
continue;
}
@@ -166,7 +166,7 @@ export function hasSandboxBindReadonlyHostShadows(binds: readonly string[] | und
const writableRoots = parsedBinds.filter((bind) => bind.writable).map((bind) => bind.hostRoot);
const readonlyRoots = parsedBinds.filter((bind) => !bind.writable).map((bind) => bind.hostRoot);
return writableRoots.some((writableRoot) =>
readonlyRoots.some((readonlyRoot) => isHostPathWithinOrEqual(writableRoot, readonlyRoot)),
readonlyRoots.some((readonlyRoot) => isPathInside(writableRoot, readonlyRoot)),
);
}
@@ -397,11 +397,6 @@ function isPathInsideHost(root: string, target: string): boolean {
return isPathInside(canonicalRoot, canonicalTarget);
}
function isHostPathWithinOrEqual(root: string, target: string): boolean {
const relative = path.relative(path.resolve(root), path.resolve(target));
return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
}
function toHostSegments(relativePosix: string): string[] {
return relativePosix.split("/").filter(Boolean);
}
+5 -9
View File
@@ -15,9 +15,10 @@ import {
statSync,
} from "node:fs";
import { homedir } from "node:os";
import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path";
import { basename, dirname, join, relative, resolve } from "node:path";
import { minimatch } from "minimatch";
import { isDefaultStateDir } from "../../config/paths.js";
import { isPathInside } from "../../infra/path-guards.js";
import {
addIgnoreRules,
normalizeNativePathSeparators,
@@ -550,13 +551,8 @@ function resolveRealPathIfPossible(path: string): string {
}
}
function isPathWithinRoot(root: string, candidate: string): boolean {
const rel = relative(root, candidate);
return rel === "" || (rel !== "" && !rel.startsWith("..") && !isAbsolute(rel));
}
function isRealPathWithinRoot(root: string, candidate: string): boolean {
return isPathWithinRoot(
return isPathInside(
resolveRealPathIfPossible(resolve(root)),
resolveRealPathIfPossible(candidate),
);
@@ -1209,10 +1205,10 @@ export class DefaultPackageManager implements PackageManager {
const realRoot = resolveRealPathIfPossible(resolvedRoot);
return paths.filter((path) => {
const resolvedPath = resolve(path);
if (!isPathWithinRoot(resolvedRoot, resolvedPath)) {
if (!isPathInside(resolvedRoot, resolvedPath)) {
return false;
}
return isPathWithinRoot(realRoot, resolveRealPathIfPossible(resolvedPath));
return isPathInside(realRoot, resolveRealPathIfPossible(resolvedPath));
});
}
+2 -9
View File
@@ -2,6 +2,7 @@
import fs from "node:fs/promises";
import path from "node:path";
import { uniqueStrings } from "@openclaw/normalization-core/string-normalization";
import { isPathInside } from "../../infra/path-guards.js";
import { createSubsystemLogger } from "../../logging/subsystem.js";
const QA_PARENT_PID_ENV = "OPENCLAW_QA_PARENT_PID";
@@ -70,14 +71,6 @@ function resolveQaCleanupRoots(env: NodeJS.ProcessEnv): string[] {
);
}
function pathContains(root: string, candidate: string): boolean {
const relative = path.relative(root, candidate);
return (
relative === "" ||
(relative.length > 0 && !relative.startsWith("..") && !path.isAbsolute(relative))
);
}
export function installQaParentWatchdog(
deps: QaParentWatchdogDeps = {},
): QaParentWatchdogHandle | null {
@@ -133,7 +126,7 @@ export function installQaParentWatchdog(
void (async () => {
const currentCwd = path.resolve(cwd());
const activeCwdRoot = qaCleanupRoots.find((cleanupRoot) =>
pathContains(cleanupRoot, currentCwd),
isPathInside(cleanupRoot, currentCwd),
);
if (activeCwdRoot) {
const safeCwd = path.dirname(activeCwdRoot);
+2 -4
View File
@@ -11,6 +11,7 @@ import { migratePersistedImplicitMainRoster } from "../config/legacy.roster.js";
import { CONFIG_PATH } from "../config/paths.js";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { callGateway } from "../gateway/call.js";
import { isPathInside } from "../infra/path-guards.js";
import type { RuntimeEnv } from "../runtime.js";
import {
noteImplicitFallbackClobberWarnings,
@@ -49,10 +50,7 @@ function collectInvalidHookTransformsDirWarnings(
const resolved = path.isAbsolute(transformsDir)
? path.resolve(transformsDir)
: path.resolve(transformsRoot, transformsDir);
const relative = path.relative(transformsRoot, resolved);
const escapesRoot =
relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative);
if (!escapesRoot) {
if (isPathInside(transformsRoot, resolved)) {
return [];
}
return [
+2 -4
View File
@@ -20,6 +20,7 @@ import type { SessionEntry } from "../config/sessions/types.js";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { resolveStoredSessionOwnerAgentId } from "../gateway/session-store-key.js";
import { readFileDescriptorBoundedSync } from "../infra/boundary-file-read.js";
import { isPathInside } from "../infra/path-guards.js";
import { resolveSqliteDatabaseFilePaths } from "../infra/sqlite-files.js";
import { normalizeLegacySessionEntryDelivery as normalizeSessionEntryDelivery } from "../infra/state-migrations.legacy-session-store.js";
import { LEGACY_IMPLICIT_AGENT_ID, normalizeAgentId } from "../routing/session-key.js";
@@ -197,10 +198,7 @@ function resolveDoctorSessionSqliteMaintenanceRoots(
}
function isPathWithin(rootPath: string, candidatePath: string): boolean {
const relativePath = path.relative(rootPath, path.resolve(candidatePath));
return (
relativePath === "" || (!relativePath.startsWith(`..${path.sep}`) && relativePath !== "..")
);
return isPathInside(rootPath, path.resolve(candidatePath));
}
function commonPathAncestor(leftPath: string, rightPath: string): string {
@@ -1822,71 +1822,5 @@ describe("ensureOnboardingPluginInstalled", () => {
expect(captured?.options).toEqual([{ value: "skip", label: "Skip for now" }]);
});
});
it("rejects local install paths when relative resolution looks cross-drive", async () => {
await withTestDir({ prefix: "openclaw-onboarding-install-cross-drive-" }, async (temp) => {
const workspaceDir = path.join(temp, "workspace");
const pluginDir = path.join(workspaceDir, "plugins", "demo");
await fs.mkdir(path.join(workspaceDir, ".git"), { recursive: true });
await fs.mkdir(pluginDir, { recursive: true });
const realWorkspaceDir = await fs.realpath(workspaceDir);
const originalRelative = path.relative;
const originalIsAbsolute = path.isAbsolute;
const relativeSpy = vi.spyOn(path, "relative").mockImplementation((from, to) => {
if (
typeof from === "string" &&
typeof to === "string" &&
from === realWorkspaceDir &&
to === path.join(realWorkspaceDir, "plugins", "demo")
) {
return "D:\\evil";
}
return originalRelative(from, to);
});
const isAbsoluteSpy = vi.spyOn(path, "isAbsolute").mockImplementation((value) => {
if (value === "D:\\evil") {
return true;
}
return originalIsAbsolute(value);
});
try {
let captured:
| {
options: Array<{
value: "clawhub" | "npm" | "local" | "skip";
label: string;
hint?: string;
}>;
}
| undefined;
await ensureOnboardingPluginInstalled({
cfg: {},
entry: {
pluginId: "demo-plugin",
label: "Demo Plugin",
install: {
localPath: "plugins/demo",
},
},
prompter: {
select: vi.fn(async (input) => {
captured = input;
return "skip";
}),
} as never,
runtime: {} as never,
workspaceDir,
});
expect(captured?.options).toEqual([{ value: "skip", label: "Skip for now" }]);
} finally {
relativeSpy.mockRestore();
isAbsoluteSpy.mockRestore();
}
});
});
});
/* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */
+4 -14
View File
@@ -15,6 +15,7 @@ import { assertConfigWriteAllowedInCurrentMode } from "../config/nix-mode-write-
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { parseClawHubPluginSpec } from "../infra/clawhub-spec.js";
import { isOpenClawOrgNpmSpec, parseRegistryNpmSpec } from "../infra/npm-registry-spec.js";
import { isPathInside } from "../infra/path-guards.js";
import { normalizeUpdateChannel, resolveRegistryUpdateChannel } from "../infra/update-channels.js";
import {
findBundledPluginSourceInMap,
@@ -175,14 +176,6 @@ function resolveGitDirectoryMarker(dir: string): string | null {
}
}
function isWithinBaseDirectory(baseDir: string, targetPath: string): boolean {
const relative = path.relative(baseDir, targetPath);
return (
relative === "" ||
(!path.isAbsolute(relative) && !relative.startsWith(`..${path.sep}`) && relative !== "..")
);
}
function hasTrustedGitWorkspace(root: string): boolean {
const realRoot = resolveRealDirectory(root);
if (!realRoot) {
@@ -241,11 +234,8 @@ function formatPortableLocalPath(localPath: string, workspaceDir?: string): stri
if (!realBase) {
continue;
}
const relative = path.relative(realBase, localPath);
if (
relative === "" ||
(!path.isAbsolute(relative) && !relative.startsWith(`..${path.sep}`) && relative !== "..")
) {
if (isPathInside(realBase, localPath)) {
const relative = path.relative(realBase, localPath);
const portable = relative.split(path.sep).join("/");
return portable ? `./${portable}` : ".";
}
@@ -302,7 +292,7 @@ function resolveLocalPath(params: {
if (
!bases.some((base) => {
const realBase = resolveRealDirectory(base);
return realBase ? isWithinBaseDirectory(realBase, resolved) : false;
return realBase ? isPathInside(realBase, resolved) : false;
})
) {
continue;
+5 -12
View File
@@ -15,6 +15,7 @@ import {
import { createBackupLinkCache } from "../infra/backup-volatile-stat-cache.js";
import { formatErrorMessage as errorMessage } from "../infra/errors.js";
import { root as fsSafeRoot } from "../infra/fs-safe.js";
import { isPathInside } from "../infra/path-guards.js";
import {
cellAuthSecretDir,
cellNetworkName,
@@ -120,14 +121,6 @@ async function canonicalizeForContainment(targetPath: string): Promise<string> {
}
}
function isWithin(candidate: string, root: string): boolean {
const relative = path.relative(root, candidate);
return (
relative === "" ||
(!relative.startsWith(`..${path.sep}`) && relative !== ".." && !path.isAbsolute(relative))
);
}
function remapArchivePath(
entryPath: string,
manifestPath: string,
@@ -138,11 +131,11 @@ function remapArchivePath(
if (resolved === manifestPath) {
return "manifest.json";
}
if (isWithin(resolved, dataTarget)) {
if (isPathInside(dataTarget, resolved)) {
const relative = path.relative(dataTarget, resolved).split(path.sep).join(path.posix.sep);
return relative ? path.posix.join("data", relative) : "data";
}
if (isWithin(resolved, authTarget)) {
if (isPathInside(authTarget, resolved)) {
const relative = path.relative(authTarget, resolved).split(path.sep).join(path.posix.sep);
return relative ? path.posix.join("auth", relative) : "auth";
}
@@ -206,7 +199,7 @@ export async function backupFleetCell(params: {
);
const canonicalOutput = await canonicalizeForContainment(archivePath);
const roots = [dataTarget, authTarget];
if (roots.some((root) => isWithin(canonicalOutput, root))) {
if (roots.some((root) => isPathInside(root, canonicalOutput))) {
throw new Error(
"Fleet backup output must not be written inside the cell data or auth directory.",
);
@@ -475,7 +468,7 @@ export async function restoreFleetCell(params: {
canonicalizeForContainment(params.record.dataDir),
canonicalizeForContainment(cellAuthSecretDir(params.stateDir, params.record.tenantId)),
]);
if (restoreRoots.some((root) => isWithin(canonicalArchive, root))) {
if (restoreRoots.some((root) => isPathInside(root, canonicalArchive))) {
throw new Error(
"Fleet restore archive must not be stored inside the cell data or auth directory.",
);
@@ -4,6 +4,7 @@ import path from "node:path";
import { listAgentEntries } from "../agents/agent-scope-config.js";
import { resolveStateDir } from "../config/paths.js";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { isPathInside } from "../infra/path-guards.js";
import { resolveUserPath } from "../utils.js";
import type { HealthCheck, HealthRepairEffect } from "./health-checks.js";
@@ -55,14 +56,6 @@ async function canonicalPath(target: string): Promise<string> {
}
}
function isSameOrDescendant(parent: string, candidate: string): boolean {
const relative = path.relative(parent, candidate);
return (
relative === "" ||
(!path.isAbsolute(relative) && !relative.startsWith(`..${path.sep}`) && relative !== "..")
);
}
async function configuredAgentWorkspaceCollisions(
cfg: OpenClawConfig,
target: string,
@@ -89,8 +82,8 @@ async function configuredAgentWorkspaceCollisions(
return resolvedEntries
.filter(
(entry) =>
isSameOrDescendant(resolvedTarget, entry.resolvedWorkspace) ||
isSameOrDescendant(entry.resolvedWorkspace, resolvedTarget),
isPathInside(resolvedTarget, entry.resolvedWorkspace) ||
isPathInside(entry.resolvedWorkspace, resolvedTarget),
)
.map((entry) => entry.label);
}
+3 -7
View File
@@ -9,6 +9,7 @@ import {
import { resolveConfigPathCandidate } from "../config/paths.js";
import type { HookMappingConfig, HooksConfig, HookSessionMode } from "../config/types.hooks.js";
import { importFileModule, resolveFunctionModuleExport } from "../hooks/module-loader.js";
import { isPathInside } from "../infra/path-guards.js";
import type { HookMessageChannel } from "./hooks.types.js";
export type HookMappingResolved = {
@@ -444,11 +445,6 @@ 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 {
// Hook containment prefers native canonicalization when Node exposes it.
@@ -481,7 +477,7 @@ function resolveContainedPath(baseDir: string, target: string, label: string): s
throw new Error(`${label} module path is required`);
}
const resolved = resolvePath(base, trimmed);
if (escapesBase(base, resolved)) {
if (!isPathInside(base, resolved)) {
throw new Error(`${label} module path must be within ${base}: ${target}`);
}
@@ -493,7 +489,7 @@ function resolveContainedPath(baseDir: string, target: string, label: string): s
if (
baseRealpath &&
existingAncestorRealpath &&
escapesBase(baseRealpath, existingAncestorRealpath)
!isPathInside(baseRealpath, existingAncestorRealpath)
) {
throw new Error(`${label} module path must be within ${base}: ${target}`);
}
+3 -10
View File
@@ -24,6 +24,7 @@ import { runGit } from "../../agents/worktrees/git.js";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import { FsSafeError } from "../../infra/fs-safe.js";
import { pruneMapToMaxSize } from "../../infra/map-size.js";
import { isPathInside } from "../../infra/path-guards.js";
import { normalizeAgentId, parseAgentSessionKey } from "../../routing/session-key.js";
import { resolveRequestedSessionAgentId } from "../session-request-agent.js";
import {
@@ -313,14 +314,6 @@ function toDisplayPath(root: string, resolved: string): string {
return relative.split(path.sep).join("/");
}
function isInsideRoot(root: string, candidate: string): boolean {
const relative = path.relative(root, candidate);
return (
relative === "" ||
(relative !== ".." && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative))
);
}
function resolveTouchedFilePath(params: {
root: string | undefined;
fileRoot: string | undefined;
@@ -331,7 +324,7 @@ function resolveTouchedFilePath(params: {
}
const base = params.fileRoot ?? params.root;
const resolved = resolveSessionToolPathToCwd(params.filePath, base);
if (!isInsideRoot(params.root, resolved)) {
if (!isPathInside(params.root, resolved)) {
return undefined;
}
return resolved;
@@ -349,7 +342,7 @@ function resolveFileRoot(params: {
}
const resolvedCwd = path.resolve(params.spawnedCwd);
const resolvedRoot = path.resolve(params.root);
return isInsideRoot(resolvedRoot, resolvedCwd) ? params.spawnedCwd : params.root;
return isPathInside(resolvedRoot, resolvedCwd) ? params.spawnedCwd : params.root;
}
function relevanceForKind(kind: FileKind): SessionFileRelevance {
+35 -25
View File
@@ -505,33 +505,43 @@ describe("ensureOpenClawCliOnPath", () => {
expect(updated).not.toContain(path.join("npm-prefix", "bin"));
});
it("ignores package-manager env roots derived from the active workspace", () => {
const homeDir = abs("/tmp/openclaw-path/home");
const cwd = path.join(homeDir, "workspace");
const appBinDir = path.join(homeDir, "app-bin");
const appCli = path.join(appBinDir, "openclaw");
const pnpmHome = path.join(cwd, ".pnpm");
const npmPrefix = path.join(cwd, ".npm-prefix");
for (const dir of [homeDir, cwd, appBinDir, pnpmHome, path.join(pnpmHome, "bin"), npmPrefix]) {
setDir(dir);
}
setDir(path.join(npmPrefix, "bin"));
setExe(appCli);
resetBootstrapEnv("/usr/bin:/bin");
process.env.PNPM_HOME = pnpmHome;
process.env.NPM_CONFIG_PREFIX = npmPrefix;
it.each([".pnpm", "..cache"])(
"ignores package-manager env roots derived from the active workspace (%s)",
(packageManagerDir) => {
const homeDir = abs("/tmp/openclaw-path/home");
const cwd = path.join(homeDir, "workspace");
const appBinDir = path.join(homeDir, "app-bin");
const appCli = path.join(appBinDir, "openclaw");
const pnpmHome = path.join(cwd, packageManagerDir);
const npmPrefix = path.join(cwd, ".npm-prefix");
for (const dir of [
homeDir,
cwd,
appBinDir,
pnpmHome,
path.join(pnpmHome, "bin"),
npmPrefix,
]) {
setDir(dir);
}
setDir(path.join(npmPrefix, "bin"));
setExe(appCli);
resetBootstrapEnv("/usr/bin:/bin");
process.env.PNPM_HOME = pnpmHome;
process.env.NPM_CONFIG_PREFIX = npmPrefix;
const updated = bootstrapPath({
execPath: appCli,
cwd,
homeDir,
platform: "linux",
});
const updated = bootstrapPath({
execPath: appCli,
cwd,
homeDir,
platform: "linux",
});
expect(updated).not.toContain(pnpmHome);
expect(updated).not.toContain(path.join(pnpmHome, "bin"));
expect(updated).not.toContain(path.join(npmPrefix, "bin"));
});
expect(updated).not.toContain(pnpmHome);
expect(updated).not.toContain(path.join(pnpmHome, "bin"));
expect(updated).not.toContain(path.join(npmPrefix, "bin"));
},
);
it("ignores package-manager env roots whose existing parent resolves into the workspace", () => {
const homeDir = abs("/tmp/openclaw-path/home");
+3 -7
View File
@@ -8,6 +8,7 @@ import {
} from "@openclaw/normalization-core/string-normalization";
import { resolveBrewPathDirs } from "./brew.js";
import { isTruthyEnvValue } from "./env.js";
import { isPathInside } from "./path-guards.js";
import { tryProcessCwd } from "./safe-cwd.js";
type EnsureOpenClawPathOpts = {
@@ -67,11 +68,6 @@ function realpathExistingPath(candidate: string): string | undefined {
}
}
function isSameOrChildPath(candidate: string, parent: string): boolean {
const relative = path.relative(parent, candidate);
return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
}
function isFilesystemRoot(dirPath: string): boolean {
return path.dirname(dirPath) === dirPath;
}
@@ -98,7 +94,7 @@ function normalizeTrustedPackageManagerRoot(params: {
if (cwd === homeDir || isFilesystemRoot(cwd)) {
return normalized;
}
if (isSameOrChildPath(normalized, cwd)) {
if (isPathInside(cwd, normalized)) {
return undefined;
}
@@ -110,7 +106,7 @@ function normalizeTrustedPackageManagerRoot(params: {
realCwd !== realHome &&
!isFilesystemRoot(realCwd) &&
realCandidate &&
isSameOrChildPath(realCandidate, realCwd)
isPathInside(realCwd, realCandidate)
) {
return undefined;
}
+25
View File
@@ -3,6 +3,7 @@ import { afterEach, describe, expect, it, vi } from "vitest";
import { mockProcessPlatform } from "../test-utils/vitest-spies.js";
import {
isPathInside,
isPathStrictlyInside,
normalizeWindowsPathForComparison,
normalizeWindowsPathPreservingCase,
} from "./path-guards.js";
@@ -60,6 +61,7 @@ describe("isPathInside", () => {
["/workspace/root", "/workspace/root", true],
["/workspace/root", "/workspace/root/nested/file.txt", true],
["/workspace/root", "/workspace/root/..file.txt", true],
["/workspace/root", "/workspace/root/..cache/cache.json", true],
["/workspace/root", "/workspace/root/../escape.txt", false],
["/workspace/root", "/workspace/rootless/file.txt", false],
["/workspace/root", "/workspace/root/a/b/c/d/e/file.txt", true],
@@ -88,3 +90,26 @@ describe("isPathInside", () => {
}
});
});
describe("isPathStrictlyInside", () => {
it.each([
["/workspace/root", "/workspace/root", false],
["/workspace/root", "/workspace/root/child", true],
["/workspace/root", "/workspace/root/..cache/cache.json", true],
["/workspace/root", "/workspace/root/../escape", false],
])("checks strict posix containment %s -> %s", (basePath, targetPath, expected) => {
expect(isPathStrictlyInside(basePath, targetPath)).toBe(expected);
});
it("uses win32 path semantics for strict containment checks", () => {
setPlatform("win32");
for (const [basePath, targetPath, expected] of [
[String.raw`C:\workspace\root`, String.raw`C:\workspace\root`, false],
[String.raw`C:\workspace\root`, String.raw`C:\workspace\root\..cache\file.txt`, true],
[String.raw`C:\workspace\root`, String.raw`D:\workspace\root\file.txt`, false],
] as const) {
expect(isPathStrictlyInside(basePath, targetPath)).toBe(expected);
}
});
});
+7 -1
View File
@@ -1,15 +1,21 @@
// Exposes generic path guard helpers with fs-safe defaults.
import path from "node:path";
import { isPathInside } from "@openclaw/fs-safe/path";
import "./fs-safe-defaults.js";
// Generic path guard facade for containment checks and safe relative paths.
export {
hasNodeErrorCode,
isNotFoundPathError,
isPathInside,
normalizeWindowsPathForComparison,
safeStatSync,
} from "@openclaw/fs-safe/path";
export { isPathInside };
/** Returns true only when target is a descendant of root, not root itself. */
export function isPathStrictlyInside(root: string, target: string): boolean {
return isPathInside(root, target) && !isPathInside(target, root);
}
/**
* Normalize a Windows path for boundary math whose result is handed back to callers.
+5 -17
View File
@@ -25,7 +25,7 @@ import {
} from "../infra/kysely-sync.js";
import { assertNoWindowsNetworkPath, safeFileURLToPath } from "../infra/local-file-access.js";
import type { PinnedDispatcherPolicy, SsrFPolicy } from "../infra/net/ssrf.js";
import { isNotFoundPathError } from "../infra/path-guards.js";
import { isNotFoundPathError, isPathInside } from "../infra/path-guards.js";
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
import { getActivePluginHttpRouteRegistry } from "../plugins/runtime.js";
import type { DB as OpenClawStateKyselyDatabase } from "../state/openclaw-state-db.generated.js";
@@ -300,19 +300,9 @@ function getValidatedHostReadText(buffer?: Buffer): string | undefined {
return printableRatio > 0.95 ? text : undefined;
}
function isPathInsideRoot(filePath: string | undefined, root: string): boolean {
if (!filePath) {
return false;
}
const relative = path.relative(path.resolve(root), path.resolve(filePath));
return (
relative === "" || (relative !== "" && !relative.startsWith("..") && !path.isAbsolute(relative))
);
}
function resolveLocalMediaFileName(filePath: string): string | undefined {
const fileName = basenameFromAnyPath(filePath) || undefined;
return fileName && isPathInsideRoot(filePath, getMediaDir())
return fileName && isPathInside(getMediaDir(), filePath)
? extractOriginalFilename(fileName)
: fileName;
}
@@ -375,15 +365,13 @@ async function resolveTrustedGeneratedHostReadHtml(
}
// Outbound staging always requires provenance, even when a custom state dir
// places media/outbound underneath the otherwise trusted temp root.
if (outboundRoot && isPathInsideRoot(resolvedFilePath, outboundRoot)) {
if (outboundRoot && isPathInside(outboundRoot, resolvedFilePath)) {
const marker = await getTrustedGeneratedHtmlMarker(resolvedFilePath);
return marker
? { source: "outbound", expectedSha256: marker.sha256, expectedSize: marker.size }
: undefined;
}
return tmpRoot && isPathInsideRoot(resolvedFilePath, tmpRoot)
? { source: "temp-root" }
: undefined;
return tmpRoot && isPathInside(tmpRoot, resolvedFilePath) ? { source: "temp-root" } : undefined;
}
/** Records exact-byte provenance for a trusted generated HTML staged outbound. */
@@ -393,7 +381,7 @@ export async function markTrustedGeneratedHtmlPath(
): Promise<void> {
const resolvedFilePath = await realpath(filePath);
const outboundRoot = await realpath(path.join(getMediaDir(), "outbound")).catch(() => undefined);
if (!outboundRoot || !isPathInsideRoot(resolvedFilePath, outboundRoot)) {
if (!outboundRoot || !isPathInside(outboundRoot, resolvedFilePath)) {
throw new Error(
`markTrustedGeneratedHtmlPath: refusing path outside outbound staging: ${resolvedFilePath}`,
);
+7 -2
View File
@@ -567,6 +567,7 @@ describe("loadEnabledBundleMcpConfig", () => {
$schema: AGENT_MCP_SCHEMA,
mcpServers: {
valid: { type: "stdio", command: "node", cwd: "./child" },
dotPrefixedChild: { type: "stdio", command: "node", cwd: "./..cache" },
relativeEscape: { type: "stdio", command: "node", cwd: "./../escape" },
placeholderEscape: {
type: "stdio",
@@ -575,7 +576,7 @@ describe("loadEnabledBundleMcpConfig", () => {
},
},
},
textFiles: { "child/.keep": "" },
textFiles: { "child/.keep": "", "..cache/.keep": "" },
});
const rootRealPath = await fs.realpath(pluginRoot);
const relativeProcessCwd = path.relative(rootRealPath, await fs.realpath(process.cwd()));
@@ -588,11 +589,15 @@ describe("loadEnabledBundleMcpConfig", () => {
cfg: createEnabledBundleConfig(["portable-cwd"]),
});
expect(Object.keys(loaded.config.mcpServers)).toEqual(["valid"]);
expect(Object.keys(loaded.config.mcpServers)).toEqual(["valid", "dotPrefixedChild"]);
await expectResolvedPathEqual(
loaded.config.mcpServers.valid?.cwd,
path.join(pluginRoot, "child"),
);
await expectResolvedPathEqual(
loaded.config.mcpServers.dotPrefixedChild?.cwd,
path.join(pluginRoot, "..cache"),
);
expect(loaded.diagnostics).toHaveLength(2);
expect(loaded.diagnostics.map((entry) => entry.message)).toEqual([
expect.stringContaining('invalid MCP server "relativeEscape"'),
+3 -10
View File
@@ -6,6 +6,7 @@ import { resolveMcpTransportConfig } from "../agents/mcp-transport-config.js";
import { applyMergePatch } from "../config/merge-patch.js";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { readRootJsonObjectSync } from "../infra/json-files.js";
import { isPathInside } from "../infra/path-guards.js";
import { isRecord } from "../utils.js";
import {
loadEnabledBundleConfig,
@@ -240,20 +241,12 @@ function hasOnlyKeys(raw: Record<string, unknown>, allowed: ReadonlySet<string>)
return Object.keys(raw).every((key) => allowed.has(key));
}
function isPathWithin(baseDir: string, targetPath: string): boolean {
const relative = path.relative(baseDir, targetPath);
return (
relative === "" ||
(!path.isAbsolute(relative) && relative !== ".." && !relative.startsWith(`..${path.sep}`))
);
}
function isValidAgentCommand(command: unknown, rootDir: string): command is string {
if (typeof command !== "string" || command.length === 0) {
return false;
}
if (command.startsWith("./")) {
return command.length > 2 && isPathWithin(rootDir, path.resolve(rootDir, command));
return command.length > 2 && isPathInside(rootDir, path.resolve(rootDir, command));
}
return !/[\s/\\]/.test(command);
}
@@ -279,7 +272,7 @@ function isValidAgentCwd(cwd: unknown, rootDir: string, pluginDataDir: string):
return false;
}
const expanded = expandBundleRootPlaceholders({ value: cwd, rootDir, pluginDataDir });
return isPathWithin(baseDir, path.resolve(baseDir, expanded));
return isPathInside(baseDir, path.resolve(baseDir, expanded));
}
function validateAgentMcpServer(params: {
+3 -8
View File
@@ -4,6 +4,7 @@ import path from "node:path";
import { fileURLToPath } from "node:url";
import { uniqueStrings } from "@openclaw/normalization-core/string-normalization";
import { tryReadJsonSync } from "../infra/json-files.js";
import { isPathInside } from "../infra/path-guards.js";
import { collectBundledChannelConfigsCore } from "./bundled-channel-config-metadata.js";
import {
collectBundledPluginPublicSurfaceArtifacts,
@@ -218,11 +219,6 @@ function listBundledPluginEntryBaseDirs(params: {
return uniqueStrings(baseDirs);
}
function isPathInsideRoot(rootDir: string, targetPath: string): boolean {
const relative = path.relative(rootDir, targetPath);
return relative !== ".." && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative);
}
function listBundledPluginEntryRoots(params: {
rootDir: string;
pluginDirName?: string;
@@ -257,7 +253,7 @@ function listBundledPluginEntrySearchPaths(
}
const normalizedEntry = path.normalize(rawEntry);
for (const root of roots) {
if (!isPathInsideRoot(root, normalizedEntry)) {
if (!isPathInside(root, normalizedEntry)) {
continue;
}
const relativeEntry = path.relative(root, normalizedEntry);
@@ -314,8 +310,7 @@ function resolveBundledPluginEntryCandidate(baseDir: string, entryPath: string):
const candidate = path.isAbsolute(normalizedEntryPath)
? path.normalize(normalizedEntryPath)
: path.resolve(baseDir, normalizedEntryPath);
const relative = path.relative(baseDir, candidate);
if (relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) {
if (!isPathInside(baseDir, candidate)) {
return null;
}
return candidate;
@@ -1,4 +1,5 @@
import path from "node:path";
import { isPathInside } from "../infra/path-guards.js";
import { resolveUserPath } from "../utils.js";
import {
isInstalledPluginIndexInstallOwnerAmbiguous,
@@ -127,8 +128,7 @@ function installRecordPathMatchesPluginRoot(
}
const candidatePath = path.resolve(resolveUserPath(candidate, env));
const resolvedCandidate = safeRealpathSync(candidatePath, realpathCache) ?? candidatePath;
const relative = path.relative(resolvedCandidate, resolvedRoot);
return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
return isPathInside(resolvedCandidate, resolvedRoot);
});
}
+4 -8
View File
@@ -3,6 +3,7 @@ import fs from "node:fs";
import path from "node:path";
import { isRecord } from "@openclaw/normalization-core/record-coerce";
import { safePathSegmentHashed } from "../infra/install-safe-path.js";
import { isPathInside } from "../infra/path-guards.js";
import {
isPluginNpmProjectDir,
resolveDefaultPluginNpmDir,
@@ -126,11 +127,6 @@ export async function markRetainedManagedNpmInstall(params: {
return true;
}
function isPathEqualOrInside(parentPath: string, childPath: string): boolean {
const relative = path.relative(path.resolve(parentPath), path.resolve(childPath));
return relative === "" || (relative !== ".." && !relative.startsWith(`..${path.sep}`));
}
function listManagedNpmPackageDirs(npmRoot: string): string[] {
const nodeModulesDir = path.join(npmRoot, "node_modules");
let entries: fs.Dirent[];
@@ -185,7 +181,7 @@ async function cleanupRetainedLegacyNpmPackages(params: {
if (
!hasRetainedManagedNpmInstallMarker(packageDir) ||
markerPreservesPackageFiles(resolveRetainedManagedNpmInstallMarkerPath(packageDir)) ||
params.activeInstallPaths.some((installPath) => isPathEqualOrInside(packageDir, installPath))
params.activeInstallPaths.some((installPath) => isPathInside(packageDir, installPath))
) {
continue;
}
@@ -243,13 +239,13 @@ export async function cleanupRetainedManagedNpmInstallGenerations(
markerEntries.some((entry) =>
markerPreservesPackageFiles(path.join(markerDir, entry.name)),
) ||
!isPathEqualOrInside(projectsDir, projectRoot) ||
!isPathInside(projectsDir, projectRoot) ||
!isOwnedManagedNpmProject({
markerNames: new Set(markerEntries.map((entry) => entry.name)),
npmDir,
projectRoot,
}) ||
activeInstallPaths.some((installPath) => isPathEqualOrInside(projectRoot, installPath))
activeInstallPaths.some((installPath) => isPathInside(projectRoot, installPath))
) {
continue;
}
+2 -4
View File
@@ -957,8 +957,7 @@ async function downloadUrlToTempFile(
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-marketplace-download-"));
const createdTmpDir = tmpDir;
const targetPath = path.resolve(createdTmpDir, fileName);
const relativeTargetPath = path.relative(createdTmpDir, targetPath);
if (relativeTargetPath === ".." || relativeTargetPath.startsWith(`..${path.sep}`)) {
if (!isPathInside(createdTmpDir, targetPath)) {
throw new Error("invalid download filename");
}
await streamMarketplaceResponseToFile({
@@ -995,8 +994,7 @@ async function ensureInsideMarketplaceRoot(
): Promise<{ ok: true; path: string } | { ok: false; error: string }> {
const resolved = path.resolve(rootDir, candidate);
const resolvedExists = await pathExists(resolved);
const relative = path.relative(rootDir, resolved);
if (relative === ".." || relative.startsWith(`..${path.sep}`)) {
if (!isPathInside(rootDir, resolved)) {
return {
ok: false,
error: `plugin source escapes marketplace root: ${candidate}`,
+2 -6
View File
@@ -3,6 +3,7 @@ import fs from "node:fs";
import Module, { createRequire } from "node:module";
import path from "node:path";
import { pathToFileURL } from "node:url";
import { isPathInside } from "../infra/path-guards.js";
const nodeRequire = createRequire(import.meta.url);
type ResolveFilename = (
@@ -130,7 +131,7 @@ function clearRequireCacheSubtree(
const cached = nodeRequire.cache[resolvedPath];
if (cached) {
for (const child of cached.children) {
if (isPathInsideOrSame(dependencyRoot, child.id)) {
if (isPathInside(dependencyRoot, child.id)) {
clearRequireCacheSubtree(child.id, dependencyRoot, seen);
}
}
@@ -138,11 +139,6 @@ function clearRequireCacheSubtree(
delete nodeRequire.cache[resolvedPath];
}
function isPathInsideOrSame(root: string, target: string): boolean {
const relative = path.relative(root, target);
return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
}
function requireWithOptionalAliases(
modulePath: string,
aliasMap: Record<string, string> | undefined,
+4 -4
View File
@@ -4,6 +4,7 @@ import Module from "node:module";
import path from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
import { resolveRealpathOrAbsolute } from "../infra/boundary-path.js";
import { isPathInside, isPathStrictlyInside } from "../infra/path-guards.js";
import { PluginLruCache } from "./plugin-cache-primitives.js";
import { registerPluginMetadataProcessMemoLifecycleClear } from "./plugin-metadata-lifecycle.js";
import {
@@ -163,10 +164,10 @@ function findBundledPluginRoot(modulePath: string): string | undefined {
const packageRoot = normalizePathForBoundary(resolveLoaderPackageRootFromModulePath(modulePath));
for (const relativeRoot of ["extensions", "dist/extensions", "dist-runtime/extensions"]) {
const bundledRoot = path.join(packageRoot, relativeRoot);
const relative = path.relative(bundledRoot, resolvedModulePath);
if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) {
if (!isPathStrictlyInside(bundledRoot, resolvedModulePath)) {
continue;
}
const relative = path.relative(bundledRoot, resolvedModulePath);
const [pluginId] = relative.split(path.sep);
if (pluginId) {
return path.join(bundledRoot, pluginId);
@@ -237,8 +238,7 @@ function resolveAllowedParentRoots(
}
function isWithinRoot(candidate: string, root: string): boolean {
const relative = path.relative(root, normalizePathForBoundary(candidate));
return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
return isPathInside(root, normalizePathForBoundary(candidate));
}
function resolveAliasTargetForParent(
+2 -2
View File
@@ -1,6 +1,7 @@
import { realpathSync } from "node:fs";
import path from "node:path";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { isPathInside } from "../infra/path-guards.js";
import { resetPluginSlotsToDefaults } from "./slots.js";
export type PluginConfigUninstallActions = {
@@ -39,11 +40,10 @@ export function resolveComparableUninstallPathInternal(value: string): string {
}
export function isUninstallPathInsideOrEqualInternal(parent: string, child: string): boolean {
const relative = path.relative(
return isPathInside(
resolveComparableUninstallPathInternal(parent),
resolveComparableUninstallPathInternal(child),
);
return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
}
export function resolveUninstallChannelConfigKeysInternal(
+6 -13
View File
@@ -7,6 +7,7 @@ import type {
ManualExecSecretProviderConfig,
PluginIntegrationSecretProviderConfig,
} from "../config/types.secrets.js";
import { isPathInside } from "../infra/path-guards.js";
import { normalizePluginsConfig, type NormalizedPluginsConfig } from "../plugins/config-state.js";
import { shouldRejectHardlinkedPluginFiles } from "../plugins/hardlink-policy.js";
import { isActivatedManifestOwner } from "../plugins/manifest-owner-policy.js";
@@ -38,17 +39,9 @@ type SecretProviderIntegrationResolution =
const NODE_COMMAND_PLACEHOLDER = "${node}";
const PLUGIN_INTEGRATION_PROVIDER_ID_MAX_LENGTH = 128;
function isPathInsideOrEqual(rootDir: string, candidate: string): boolean {
const relative = path.relative(path.resolve(rootDir), path.resolve(candidate));
return (
relative === "" ||
(relative.length > 0 && !relative.startsWith("..") && !path.isAbsolute(relative))
);
}
function resolvePluginRelativePath(value: string, pluginRoot: string): string | undefined {
const resolved = path.resolve(pluginRoot, value);
return isPathInsideOrEqual(pluginRoot, resolved) ? resolved : undefined;
return isPathInside(pluginRoot, resolved) ? resolved : undefined;
}
function isPluginRelativeEntrypoint(value: string): boolean {
@@ -85,13 +78,13 @@ function isSecurePosixPathStat(stat: fs.Stats): boolean {
}
function pathSegmentsBetween(rootDir: string, targetDir: string): string[] | undefined {
if (!isPathInside(rootDir, targetDir)) {
return undefined;
}
const relative = path.relative(rootDir, targetDir);
if (relative === "") {
return [];
}
if (relative.startsWith("..") || path.isAbsolute(relative)) {
return undefined;
}
return relative.split(path.sep).filter(Boolean);
}
@@ -182,7 +175,7 @@ function resolveNodeEntrypointArg(params: {
}
try {
const realpath = fs.realpathSync(resolved);
if (!isPathInsideOrEqual(pluginRootRealpath, realpath)) {
if (!isPathInside(pluginRootRealpath, realpath)) {
return undefined;
}
if (
+2 -9
View File
@@ -8,6 +8,7 @@ import {
type ClawHubSkillsShTrustState,
} from "../../infra/clawhub-skills.js";
import { formatErrorMessage } from "../../infra/errors.js";
import { isPathInside } from "../../infra/path-guards.js";
import { normalizeTrackedSkillSlug, resolveWorkspaceSkillInstallDir } from "./archive-install.js";
import {
normalizeDownloadedArtifactLock,
@@ -251,14 +252,6 @@ export function resolveClawHubSkillStatusLinkSync(params: {
};
}
function isPathInsideDir(child: string, parent: string): boolean {
const relative = path.relative(parent, child);
return (
relative === "" ||
(relative.length > 0 && !relative.startsWith("..") && !path.isAbsolute(relative))
);
}
function readLocalSkillCardSync(
skillDir: string,
includeContent = false,
@@ -277,7 +270,7 @@ function readLocalSkillCardSync(
try {
const rootRealPath = fsSync.realpathSync.native(skillDir);
const cardRealPath = fsSync.realpathSync.native(cardPath);
if (!isPathInsideDir(cardRealPath, rootRealPath)) {
if (!isPathInside(rootRealPath, cardRealPath)) {
return undefined;
}
fd = fsSync.openSync(cardPath, fsSync.constants.O_RDONLY | (fsSync.constants.O_NOFOLLOW ?? 0));
+1 -7
View File
@@ -6,6 +6,7 @@ import { normalizeOptionalString } from "@openclaw/normalization-core/string-coe
import chokidar, { type FSWatcher } from "chokidar";
import { isDefaultStateDir } from "../../config/paths.js";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import { isPathInside } from "../../infra/path-guards.js";
import { createSubsystemLogger } from "../../logging/subsystem.js";
import { CONFIG_DIR, resolveUserPath } from "../../utils.js";
import { resolvePluginSkillDirs } from "../loading/plugin-skills.js";
@@ -366,13 +367,6 @@ function watchDepthForPath(raw: string, depth: number): number {
return depth + missingSegments;
}
function isPathInside(parent: string, child: string): boolean {
const relative = path.relative(parent, child);
return (
relative === "" || (relative !== "" && !relative.startsWith("..") && !path.isAbsolute(relative))
);
}
function isPathInsideAnyRoot(roots: readonly string[], child: string): boolean {
return roots.some((root) => isPathInside(root, child));
}
+2 -10
View File
@@ -3,7 +3,7 @@ import fs from "node:fs/promises";
import path from "node:path";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { readDurableJsonFile, writeJsonAtomic } from "../infra/json-files.js";
import { isNotFoundPathError } from "../infra/path-guards.js";
import { isNotFoundPathError, isPathInside } from "../infra/path-guards.js";
import type { MigrationApplyResult, MigrationPlan } from "../plugins/types.js";
import { hashSetupMigrationConfig } from "./setup.migration-canonical.js";
import { SetupMigrationTargetChangedError } from "./setup.migration-snapshot.js";
@@ -452,14 +452,6 @@ async function canonicalizePromotionPath(
}
}
function pathsOverlap(left: string, right: string): boolean {
const relative = path.relative(left, right);
return (
relative.length === 0 ||
(!relative.startsWith(`..${path.sep}`) && relative !== ".." && !path.isAbsolute(relative))
);
}
export async function assertSupportedStagedStateTree(params: {
stagedStateDir: string;
agentId: string;
@@ -518,7 +510,7 @@ export async function assertDisjointPromotionTargets(
};
const currentPath = normalizePath(current.path.path);
const otherPath = normalizePath(other.path.path);
if (pathsOverlap(currentPath, otherPath) || pathsOverlap(otherPath, currentPath)) {
if (isPathInside(currentPath, otherPath) || isPathInside(otherPath, currentPath)) {
throw new Error(
`Migration promotion targets overlap: ${current.component.finalPath} and ${other.component.finalPath}.`,
);