From 407c84e5737f01cecb1311498cfb023d70c9de41 Mon Sep 17 00:00:00 2001 From: Brad Date: Fri, 1 May 2026 14:11:44 -0700 Subject: [PATCH] Allow config includes from approved roots (#75746) * Allow config includes from approved roots Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add changelog for include roots Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Tighten include realpath handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: ificator Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .env.example | 6 ++ CHANGELOG.md | 1 + docs/gateway/configuration.md | 6 ++ docs/help/environment.md | 11 +-- src/config/includes.test.ts | 140 +++++++++++++++++++++++++++++++++- src/config/includes.ts | 111 ++++++++++++++++++++++----- src/config/io.ts | 56 ++++++++------ src/config/paths.test.ts | 31 ++++++++ src/config/paths.ts | 40 ++++++++++ 9 files changed, 356 insertions(+), 46 deletions(-) diff --git a/.env.example b/.env.example index de02b73a595..a643b98ddf9 100644 --- a/.env.example +++ b/.env.example @@ -29,6 +29,12 @@ OPENCLAW_GATEWAY_TOKEN= # OPENCLAW_CONFIG_PATH=~/.openclaw/openclaw.json # OPENCLAW_HOME=~ +# Allowlist of extra directories that `$include` directives in openclaw.json may +# resolve files from. Path-list separated (':' on POSIX, ';' on Windows). Each +# entry is tilde-expanded. Without this, `$include` is confined to the directory +# containing openclaw.json. +# OPENCLAW_INCLUDE_ROOTS=/etc/openclaw/shared:~/.openclaw/shared + # Optional: import missing keys from your login shell profile. # OPENCLAW_LOAD_SHELL_ENV=1 # OPENCLAW_SHELL_ENV_TIMEOUT_MS=15000 diff --git a/CHANGELOG.md b/CHANGELOG.md index 732c6f38417..4838bd9bafb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ Docs: https://docs.openclaw.ai - Agents/Codex: default Codex app-server dynamic tools to native-first, keeping OpenClaw integration tools while leaving file, patch, exec, and process ownership to the Codex harness. (#75308) Thanks @pashpashpash. - Agents/Codex: default Codex-harness direct source replies to the OpenClaw `message` tool when visible reply delivery is not explicitly configured, keeping channel-visible output as a deliberate tool call. (#75765) Thanks @pashpashpash. - Heartbeats/agents: add a structured `heartbeat_respond` tool for tool-capable heartbeat runs so agents can record quiet outcomes or explicit notification text without relying only on `HEARTBEAT_OK` parsing. (#75765) Thanks @pashpashpash. +- Gateway/config: allow `$include` directives to read files from operator-approved `OPENCLAW_INCLUDE_ROOTS` directories while preserving default config-directory confinement. Thanks @ificator. ### Fixes diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index 4f3395e63c1..3dd3b67af77 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -522,6 +522,12 @@ cannot roll back unrelated user settings. - **Unsupported write-through**: root includes, include arrays, and includes with sibling overrides fail closed for OpenClaw-owned writes instead of flattening the config + - **Confinement**: `$include` paths must resolve under the directory holding + `openclaw.json`. To share a tree across machines or users, set + `OPENCLAW_INCLUDE_ROOTS` to a path-list (`:` on POSIX, `;` on Windows) of + additional directories that includes may reference. Symlinks are resolved + and re-checked, so a path that lexically lives in a config dir but whose + real target escapes every allowed root is still rejected. - **Error handling**: clear errors for missing files, parse errors, and circular includes diff --git a/docs/help/environment.md b/docs/help/environment.md index e35996b2c58..7a91acbbb35 100644 --- a/docs/help/environment.md +++ b/docs/help/environment.md @@ -103,11 +103,12 @@ Both resolve from process env at activation time. SecretRef details are document ## Path-related env vars -| Variable | Purpose | -| ---------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `OPENCLAW_HOME` | Override the home directory used for all internal path resolution (`~/.openclaw/`, agent dirs, sessions, credentials). Useful when running OpenClaw as a dedicated service user. | -| `OPENCLAW_STATE_DIR` | Override the state directory (default `~/.openclaw`). | -| `OPENCLAW_CONFIG_PATH` | Override the config file path (default `~/.openclaw/openclaw.json`). | +| Variable | Purpose | +| ------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `OPENCLAW_HOME` | Override the home directory used for all internal path resolution (`~/.openclaw/`, agent dirs, sessions, credentials). Useful when running OpenClaw as a dedicated service user. | +| `OPENCLAW_STATE_DIR` | Override the state directory (default `~/.openclaw`). | +| `OPENCLAW_CONFIG_PATH` | Override the config file path (default `~/.openclaw/openclaw.json`). | +| `OPENCLAW_INCLUDE_ROOTS` | Path-list of directories where `$include` directives may resolve files outside the config directory (default: none — `$include` is confined to the config dir). Tilde-expanded. | ## Logging diff --git a/src/config/includes.test.ts b/src/config/includes.test.ts index 1da0910b2fe..e3c360dc11e 100644 --- a/src/config/includes.test.ts +++ b/src/config/includes.test.ts @@ -1,6 +1,7 @@ +import nodeFs from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { withTempDir } from "../test-helpers/temp-dir.js"; import { CircularIncludeError, @@ -614,6 +615,33 @@ describe("security: path traversal protection (CWE-22)", () => { }); }); + it("fails closed when include realpath resolution fails for reasons other than ENOENT", () => { + const includePath = configPath("denied.json"); + const originalRealpathSync = nodeFs.realpathSync; + const realpathSpy = vi.spyOn(nodeFs, "realpathSync").mockImplementation((target) => { + if (path.normalize(String(target)) === includePath) { + const error = new Error("permission denied") as NodeJS.ErrnoException; + error.code = "EACCES"; + throw error; + } + return originalRealpathSync(target); + }); + + try { + expectResolveIncludeError( + () => + resolveConfigIncludes( + { $include: "./denied.json" }, + DEFAULT_BASE_PATH, + createMockResolver({ [includePath]: { leaked: true } }), + ), + /Failed to resolve include file realpath/, + ); + } finally { + realpathSpy.mockRestore(); + } + }); + it("rejects include files that are hardlinked aliases", async () => { if (process.platform === "win32") { return; @@ -659,3 +687,113 @@ describe("security: path traversal protection (CWE-22)", () => { }); }); }); + +describe("OPENCLAW_INCLUDE_ROOTS allowlist", () => { + it("permits an include outside the config directory when its root is allowed", () => { + const sharedFile = sharedPath("common.json"); + const files = { [sharedFile]: { shared: true } }; + expect( + resolveConfigIncludes( + { $include: sharedFile }, + DEFAULT_BASE_PATH, + createMockResolver(files), + { allowedRoots: [SHARED_DIR] }, + ), + ).toEqual({ shared: true }); + }); + + it("still rejects include paths that fall outside every allowed root", () => { + const obj = { $include: etcOpenClawPath("agents.json") }; + expect(() => + resolveConfigIncludes(obj, DEFAULT_BASE_PATH, createMockResolver({}), { + allowedRoots: [SHARED_DIR], + }), + ).toThrow(/escapes config directory/); + }); + + it.each([ + { name: "unset", allowedRoots: undefined }, + { name: "empty", allowedRoots: [] as string[] }, + ])( + "preserves the original config-directory boundary when allowedRoots is $name", + ({ allowedRoots }) => { + const obj = { $include: sharedPath("common.json") }; + expect(() => + resolveConfigIncludes(obj, DEFAULT_BASE_PATH, createMockResolver({}), { allowedRoots }), + ).toThrow(/escapes config directory/); + }, + ); + + it("ignores non-absolute or empty allowedRoots entries while honoring valid ones", () => { + const sharedFile = sharedPath("common.json"); + const files = { [sharedFile]: { shared: true } }; + expect( + resolveConfigIncludes( + { $include: sharedFile }, + DEFAULT_BASE_PATH, + createMockResolver(files), + { allowedRoots: ["", "./relative", SHARED_DIR] }, + ), + ).toEqual({ shared: true }); + }); + + it("resolves a symlinked include whose realpath lands inside an allowed root", async () => { + await withTempDir({ prefix: "openclaw-includes-allowed-symlink-" }, async (tempRoot) => { + const configDir = path.join(tempRoot, "config"); + const sharedDir = path.join(tempRoot, "shared"); + await fs.mkdir(configDir, { recursive: true }); + await fs.mkdir(sharedDir, { recursive: true }); + const sharedTarget = path.join(sharedDir, "extra.json5"); + await fs.writeFile(sharedTarget, "{ logging: { redactSensitive: 'tools' } }\n", "utf-8"); + const linkInConfig = path.join(configDir, "extra.json5"); + await fs.symlink( + sharedTarget, + linkInConfig, + process.platform === "win32" ? "file" : undefined, + ); + + const result = resolveConfigIncludes( + { $include: "./extra.json5" }, + path.join(configDir, "openclaw.json"), + undefined, + { allowedRoots: [sharedDir] }, + ); + expect(result).toEqual({ logging: { redactSensitive: "tools" } }); + }); + }); + + it("rejects a symlinked include that escapes both the config directory and every allowed root", async () => { + await withTempDir({ prefix: "openclaw-includes-allowed-escape-" }, async (tempRoot) => { + const configDir = path.join(tempRoot, "config"); + const allowedDir = path.join(tempRoot, "allowed"); + const offRootDir = path.join(tempRoot, "off-limits"); + await fs.mkdir(configDir, { recursive: true }); + await fs.mkdir(allowedDir, { recursive: true }); + await fs.mkdir(offRootDir, { recursive: true }); + const offRootTarget = path.join(offRootDir, "secret.json5"); + await fs.writeFile(offRootTarget, "{ leaked: true }\n", "utf-8"); + const linkInConfig = path.join(configDir, "secret.json5"); + try { + await fs.symlink( + offRootTarget, + linkInConfig, + process.platform === "win32" ? "file" : undefined, + ); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "EPERM") { + return; + } + throw err; + } + + expect(() => + resolveConfigIncludes( + { $include: "./secret.json5" }, + path.join(configDir, "openclaw.json"), + undefined, + { allowedRoots: [allowedDir] }, + ), + ).toThrow(/resolves outside config directory/); + }); + }); +}); diff --git a/src/config/includes.ts b/src/config/includes.ts index 9486aabdf1f..43cb31f9b99 100644 --- a/src/config/includes.ts +++ b/src/config/includes.ts @@ -40,6 +40,21 @@ export type IncludeFileReadParams = { maxBytes?: number; }; +export type ResolveConfigIncludesOptions = { + /** + * Additional directories outside the config directory that `$include` paths + * may resolve into. Typically populated from `OPENCLAW_INCLUDE_ROOTS`. + * Each entry must be an absolute path; symlinks are resolved before the + * containment check, consistent with the config-directory boundary check. + */ + allowedRoots?: ReadonlyArray; +}; + +type IncludeRoot = { + rootDir: string; + rootRealDir: string; +}; + // ============================================================================ // Errors // ============================================================================ @@ -91,17 +106,26 @@ export function deepMerge(target: unknown, source: unknown): unknown { class IncludeProcessor { private visited = new Set(); private depth = 0; - private readonly rootDir: string; - private readonly rootRealDir: string; + private readonly configRoot: IncludeRoot; + private readonly allowedRoots: ReadonlyArray; constructor( private basePath: string, private resolver: IncludeResolver, rootDir?: string, + allowedRoots?: ReadonlyArray, ) { this.visited.add(path.normalize(basePath)); - this.rootDir = path.normalize(rootDir ?? path.dirname(basePath)); - this.rootRealDir = path.normalize(safeRealpath(this.rootDir)); + const configRootDir = path.normalize(rootDir ?? path.dirname(basePath)); + this.configRoot = { + rootDir: configRootDir, + rootRealDir: path.normalize(safeRealpath(configRootDir)), + }; + this.allowedRoots = allowedRoots ?? []; + } + + private get rootDir(): string { + return this.configRoot.rootDir; } process(obj: unknown): unknown { @@ -176,49 +200,79 @@ class IncludeProcessor { } private loadFile(includePath: string): unknown { - const resolvedPath = this.resolvePath(includePath); + const { resolvedPath, root } = this.resolvePath(includePath); this.checkCircular(resolvedPath); this.checkDepth(includePath); - const raw = this.readFile(includePath, resolvedPath); + const raw = this.readFile(includePath, resolvedPath, root); const parsed = this.parseFile(includePath, resolvedPath, raw); return this.processNested(resolvedPath, parsed); } - private resolvePath(includePath: string): string { + private resolvePath(includePath: string): { resolvedPath: string; root: IncludeRoot } { const configDir = path.dirname(this.basePath); const resolved = path.isAbsolute(includePath) ? includePath : path.resolve(configDir, includePath); const normalized = path.normalize(resolved); - // SECURITY: Reject paths outside top-level config directory (CWE-22: Path Traversal) - if (!isPathInside(this.rootDir, normalized)) { + // SECURITY: Reject paths outside the config directory and any caller-allowed + // roots (CWE-22: Path Traversal). Allowed roots come from + // OPENCLAW_INCLUDE_ROOTS and let operators opt into shared include trees + // without weakening the default lock-down. + const lexicalMatch = this.findContainingRoot(normalized, "rootDir"); + if (!lexicalMatch) { throw new ConfigIncludeError( `Include path escapes config directory: ${includePath} (root: ${this.rootDir})`, includePath, ); } - // SECURITY: Resolve symlinks and re-validate to prevent symlink bypass + // SECURITY: Resolve symlinks and re-validate to prevent symlink bypass. + // The realpath may legitimately land in a different allowed root than the + // lexical path (e.g. config dir contains a symlink into an allowed root), + // so we recheck across all roots rather than pinning to the lexical match. try { const real = fs.realpathSync(normalized); - if (!isPathInside(this.rootRealDir, real)) { + const realMatch = this.findContainingRoot(real, "rootRealDir"); + if (!realMatch) { throw new ConfigIncludeError( `Include path resolves outside config directory (symlink): ${includePath} (root: ${this.rootDir})`, includePath, ); } + return { resolvedPath: normalized, root: realMatch }; } catch (err) { if (err instanceof ConfigIncludeError) { throw err; } - // File doesn't exist yet - normalized path check above is sufficient + if (isNotFoundError(err)) { + // File doesn't exist yet - lexical containment check above is sufficient. + return { resolvedPath: normalized, root: lexicalMatch }; + } + throw new ConfigIncludeError( + `Failed to resolve include file realpath: ${includePath} (resolved: ${normalized})`, + includePath, + err instanceof Error ? err : undefined, + ); } + } - return normalized; + private findContainingRoot( + candidate: string, + field: "rootDir" | "rootRealDir", + ): IncludeRoot | null { + if (isPathInside(this.configRoot[field], candidate)) { + return this.configRoot; + } + for (const root of this.allowedRoots) { + if (isPathInside(root[field], candidate)) { + return root; + } + } + return null; } private checkCircular(resolvedPath: string): void { @@ -236,13 +290,15 @@ class IncludeProcessor { } } - private readFile(includePath: string, resolvedPath: string): string { + private readFile(includePath: string, resolvedPath: string, root: IncludeRoot): string { try { if (this.resolver.readFileWithGuards) { + // This guard revalidates the opened file against root.rootRealDir, so + // symlink swaps between resolvePath() and read are rejected at open time. return this.resolver.readFileWithGuards({ includePath, resolvedPath, - rootRealDir: this.rootRealDir, + rootRealDir: root.rootRealDir, }); } return this.resolver.readFile(resolvedPath); @@ -271,7 +327,12 @@ class IncludeProcessor { } private processNested(resolvedPath: string, parsed: unknown): unknown { - const nested = new IncludeProcessor(resolvedPath, this.resolver, this.rootDir); + const nested = new IncludeProcessor( + resolvedPath, + this.resolver, + this.rootDir, + this.allowedRoots, + ); nested.visited = new Set([...this.visited, resolvedPath]); nested.depth = this.depth + 1; return nested.process(parsed); @@ -286,6 +347,15 @@ function safeRealpath(target: string): string { } } +function isNotFoundError(error: unknown): boolean { + return Boolean( + error && + typeof error === "object" && + "code" in error && + (error as { code?: unknown }).code === "ENOENT", + ); +} + export function readConfigIncludeFileWithGuards(params: IncludeFileReadParams): string { const ioFs = params.ioFs ?? fs; const maxBytes = params.maxBytes ?? MAX_INCLUDE_FILE_BYTES; @@ -341,6 +411,13 @@ export function resolveConfigIncludes( obj: unknown, configPath: string, resolver: IncludeResolver = defaultResolver, + options: ResolveConfigIncludesOptions = {}, ): unknown { - return new IncludeProcessor(configPath, resolver).process(obj); + const allowedRoots = (options.allowedRoots ?? []) + .filter((entry) => typeof entry === "string" && entry.length > 0 && path.isAbsolute(entry)) + .map((entry) => { + const rootDir = path.normalize(entry); + return { rootDir, rootRealDir: path.normalize(safeRealpath(rootDir)) }; + }); + return new IncludeProcessor(configPath, resolver, undefined, allowedRoots).process(obj); } diff --git a/src/config/io.ts b/src/config/io.ts index f6bd3c9160e..eea54238b21 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -81,7 +81,7 @@ import { materializeRuntimeConfig, } from "./materialize.js"; import { applyMergePatch } from "./merge-patch.js"; -import { resolveConfigPath, resolveStateDir } from "./paths.js"; +import { resolveConfigPath, resolveIncludeRoots, resolveStateDir } from "./paths.js"; import { extractShippedPluginInstallConfigRecords, stripShippedPluginInstallConfigRecords, @@ -1103,17 +1103,22 @@ function resolveConfigIncludesForRead( configPath: string, deps: Required, ): unknown { - return resolveConfigIncludes(parsed, configPath, { - readFile: (candidate) => deps.fs.readFileSync(candidate, "utf-8"), - readFileWithGuards: ({ includePath, resolvedPath, rootRealDir }) => - readConfigIncludeFileWithGuards({ - includePath, - resolvedPath, - rootRealDir, - ioFs: deps.fs, - }), - parseJson: (raw) => deps.json5.parse(raw), - }); + return resolveConfigIncludes( + parsed, + configPath, + { + readFile: (candidate) => deps.fs.readFileSync(candidate, "utf-8"), + readFileWithGuards: ({ includePath, resolvedPath, rootRealDir }) => + readConfigIncludeFileWithGuards({ + includePath, + resolvedPath, + rootRealDir, + ioFs: deps.fs, + }), + parseJson: (raw) => deps.json5.parse(raw), + }, + { allowedRoots: resolveIncludeRoots(deps.env, deps.homedir) }, + ); } function resolveConfigForRead( @@ -1998,17 +2003,22 @@ export function createConfigIO( unsetPaths, }); try { - const resolvedIncludes = resolveConfigIncludes(snapshot.parsed, configPath, { - readFile: (candidate) => deps.fs.readFileSync(candidate, "utf-8"), - readFileWithGuards: ({ includePath, resolvedPath, rootRealDir }) => - readConfigIncludeFileWithGuards({ - includePath, - resolvedPath, - rootRealDir, - ioFs: deps.fs, - }), - parseJson: (raw) => deps.json5.parse(raw), - }); + const resolvedIncludes = resolveConfigIncludes( + snapshot.parsed, + configPath, + { + readFile: (candidate) => deps.fs.readFileSync(candidate, "utf-8"), + readFileWithGuards: ({ includePath, resolvedPath, rootRealDir }) => + readConfigIncludeFileWithGuards({ + includePath, + resolvedPath, + rootRealDir, + ioFs: deps.fs, + }), + parseJson: (raw) => deps.json5.parse(raw), + }, + { allowedRoots: resolveIncludeRoots(deps.env, deps.homedir) }, + ); const collected = new Map(); collectEnvRefPaths(resolvedIncludes, "", collected); if (collected.size > 0) { diff --git a/src/config/paths.test.ts b/src/config/paths.test.ts index df8e7cd35a4..f2d14c41b3d 100644 --- a/src/config/paths.test.ts +++ b/src/config/paths.test.ts @@ -8,6 +8,7 @@ import { resolveConfigPathCandidate, resolveConfigPath, resolveGatewayPort, + resolveIncludeRoots, resolveOAuthDir, resolveOAuthPath, resolveStateDir, @@ -194,3 +195,33 @@ describe("state + config path candidates", () => { }); }); }); + +describe("resolveIncludeRoots", () => { + const HOME = path.parse(process.cwd()).root + "fakehome"; + + it("returns an empty list when OPENCLAW_INCLUDE_ROOTS is unset or blank", () => { + expect(resolveIncludeRoots(envWith({}), () => HOME)).toEqual([]); + expect(resolveIncludeRoots(envWith({ OPENCLAW_INCLUDE_ROOTS: "" }), () => HOME)).toEqual([]); + expect(resolveIncludeRoots(envWith({ OPENCLAW_INCLUDE_ROOTS: " " }), () => HOME)).toEqual([]); + }); + + it("splits on the platform path delimiter and resolves each entry to an absolute path", () => { + const a = path.resolve(path.parse(process.cwd()).root, "shared", "a"); + const b = path.resolve(path.parse(process.cwd()).root, "shared", "b"); + const env = envWith({ OPENCLAW_INCLUDE_ROOTS: [a, b].join(path.delimiter) }); + expect(resolveIncludeRoots(env, () => HOME)).toEqual([a, b]); + }); + + it("expands a leading tilde in each entry using the resolved home dir", () => { + const env = envWith({ OPENCLAW_INCLUDE_ROOTS: "~/share/openclaw" }); + expect(resolveIncludeRoots(env, () => HOME)).toEqual([path.join(HOME, "share", "openclaw")]); + }); + + it("drops empty entries and preserves de-duplicated order for repeated roots", () => { + const a = path.resolve(path.parse(process.cwd()).root, "shared", "a"); + const env = envWith({ + OPENCLAW_INCLUDE_ROOTS: ["", a, " ", a].join(path.delimiter), + }); + expect(resolveIncludeRoots(env, () => HOME)).toEqual([a]); + }); +}); diff --git a/src/config/paths.ts b/src/config/paths.ts index d71773196c6..7682d248ad1 100644 --- a/src/config/paths.ts +++ b/src/config/paths.ts @@ -96,6 +96,46 @@ function resolveUserPath( return resolveHomeRelativePath(input, { env, homedir }); } +/** + * Optional allowlist of directories that `$include` directives may resolve + * outside the config directory. Set via `OPENCLAW_INCLUDE_ROOTS` as a + * platform-delimited path list (`:` on POSIX, `;` on Windows). + * + * Each entry is tilde-expanded and resolved to an absolute path. Entries that + * cannot be resolved or that are not absolute after expansion are dropped. + * + * Returns an empty array when the var is unset or contains no usable entries, + * preserving the historical behavior where `$include` is confined to the + * directory containing `openclaw.json`. + */ +export function resolveIncludeRoots( + env: NodeJS.ProcessEnv = process.env, + homedir: () => string = envHomedir(env), +): string[] { + const raw = env.OPENCLAW_INCLUDE_ROOTS?.trim(); + if (!raw) { + return []; + } + const effectiveHomedir = () => resolveRequiredHomeDir(env, homedir); + const seen = new Set(); + const roots: string[] = []; + for (const entry of raw.split(path.delimiter)) { + const trimmed = entry.trim(); + if (!trimmed) { + continue; + } + const resolved = path.resolve( + resolveHomeRelativePath(trimmed, { env, homedir: effectiveHomedir }), + ); + if (!path.isAbsolute(resolved) || seen.has(resolved)) { + continue; + } + seen.add(resolved); + roots.push(resolved); + } + return roots; +} + export const STATE_DIR = resolveStateDir(); /**