diff --git a/src/config/includes.test.ts b/src/config/includes.test.ts index 4c7f6b6be92..6c6b506e95f 100644 --- a/src/config/includes.test.ts +++ b/src/config/includes.test.ts @@ -67,13 +67,12 @@ describe("resolveConfigIncludes", () => { }); }); - it("resolves absolute path $include", () => { + it("rejects absolute path outside config directory (CWE-22)", () => { const absolute = etcOpenClawPath("agents.json"); const files = { [absolute]: { list: [{ id: "main" }] } }; const obj = { agents: { $include: absolute } }; - expect(resolve(obj, files)).toEqual({ - agents: { list: [{ id: "main" }] }, - }); + expect(() => resolve(obj, files)).toThrow(ConfigIncludeError); + expect(() => resolve(obj, files)).toThrow(/escapes config directory/); }); it("resolves array $include with deep merge", () => { @@ -278,12 +277,15 @@ describe("resolveConfigIncludes", () => { }); }); - it("resolves parent directory references", () => { + it("rejects parent directory traversal escaping config directory (CWE-22)", () => { const files = { [sharedPath("common.json")]: { shared: true } }; const obj = { $include: "../../shared/common.json" }; - expect(resolve(obj, files, configPath("sub", "openclaw.json"))).toEqual({ - shared: true, - }); + expect(() => resolve(obj, files, configPath("sub", "openclaw.json"))).toThrow( + ConfigIncludeError, + ); + expect(() => resolve(obj, files, configPath("sub", "openclaw.json"))).toThrow( + /escapes config directory/, + ); }); }); @@ -359,3 +361,171 @@ describe("real-world config patterns", () => { }); }); }); +describe("security: path traversal protection (CWE-22)", () => { + describe("absolute path attacks", () => { + it("rejects /etc/passwd", () => { + const obj = { $include: "/etc/passwd" }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + expect(() => resolve(obj, {})).toThrow(/escapes config directory/); + }); + + it("rejects /etc/shadow", () => { + const obj = { $include: "/etc/shadow" }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + expect(() => resolve(obj, {})).toThrow(/escapes config directory/); + }); + + it("rejects home directory SSH key", () => { + const obj = { $include: `${process.env.HOME}/.ssh/id_rsa` }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + }); + + it("rejects /tmp paths", () => { + const obj = { $include: "/tmp/malicious.json" }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + }); + + it("rejects root directory", () => { + const obj = { $include: "/" }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + }); + }); + + describe("relative traversal attacks", () => { + it("rejects ../../etc/passwd", () => { + const obj = { $include: "../../etc/passwd" }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + expect(() => resolve(obj, {})).toThrow(/escapes config directory/); + }); + + it("rejects ../../../etc/shadow", () => { + const obj = { $include: "../../../etc/shadow" }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + }); + + it("rejects deeply nested traversal", () => { + const obj = { $include: "../../../../../../../../etc/passwd" }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + }); + + it("rejects traversal to parent of config directory", () => { + const obj = { $include: "../sibling-dir/secret.json" }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + }); + + it("rejects mixed absolute and traversal", () => { + const obj = { $include: "/config/../../../etc/passwd" }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + }); + }); + + describe("legitimate includes (should work)", () => { + it("allows relative include in same directory", () => { + const files = { [configPath("sub.json")]: { key: "value" } }; + const obj = { $include: "./sub.json" }; + expect(resolve(obj, files)).toEqual({ key: "value" }); + }); + + it("allows include without ./ prefix", () => { + const files = { [configPath("sub.json")]: { key: "value" } }; + const obj = { $include: "sub.json" }; + expect(resolve(obj, files)).toEqual({ key: "value" }); + }); + + it("allows include in subdirectory", () => { + const files = { [configPath("sub", "nested.json")]: { nested: true } }; + const obj = { $include: "./sub/nested.json" }; + expect(resolve(obj, files)).toEqual({ nested: true }); + }); + + it("allows deeply nested subdirectory", () => { + const files = { [configPath("a", "b", "c", "deep.json")]: { deep: true } }; + const obj = { $include: "./a/b/c/deep.json" }; + expect(resolve(obj, files)).toEqual({ deep: true }); + }); + + // Note: Upward traversal from nested configs is restricted for security. + // Each config file can only include files from its own directory and subdirectories. + // This prevents potential path traversal attacks even in complex nested scenarios. + }); + + describe("error properties", () => { + it("throws ConfigIncludeError with correct type", () => { + const obj = { $include: "/etc/passwd" }; + try { + resolve(obj, {}); + expect.fail("Should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(ConfigIncludeError); + expect(err).toHaveProperty("name", "ConfigIncludeError"); + } + }); + + it("includes offending path in error", () => { + const maliciousPath = "/etc/shadow"; + const obj = { $include: maliciousPath }; + try { + resolve(obj, {}); + expect.fail("Should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(ConfigIncludeError); + expect((err as ConfigIncludeError).includePath).toBe(maliciousPath); + } + }); + + it("includes descriptive message", () => { + const obj = { $include: "../../etc/passwd" }; + try { + resolve(obj, {}); + expect.fail("Should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(ConfigIncludeError); + expect((err as Error).message).toContain("escapes config directory"); + expect((err as Error).message).toContain("../../etc/passwd"); + } + }); + }); + + describe("array includes with malicious paths", () => { + it("rejects array with one malicious path", () => { + const files = { [configPath("good.json")]: { good: true } }; + const obj = { $include: ["./good.json", "/etc/passwd"] }; + expect(() => resolve(obj, files)).toThrow(ConfigIncludeError); + }); + + it("rejects array with multiple malicious paths", () => { + const obj = { $include: ["/etc/passwd", "/etc/shadow"] }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + }); + + it("allows array with all legitimate paths", () => { + const files = { + [configPath("a.json")]: { a: 1 }, + [configPath("b.json")]: { b: 2 }, + }; + const obj = { $include: ["./a.json", "./b.json"] }; + expect(resolve(obj, files)).toEqual({ a: 1, b: 2 }); + }); + }); + + describe("edge cases", () => { + it("rejects null bytes in path", () => { + const obj = { $include: "./file\x00.json" }; + // Path with null byte should be rejected or handled safely + expect(() => resolve(obj, {})).toThrow(); + }); + + it("rejects double slashes", () => { + const obj = { $include: "//etc/passwd" }; + expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + }); + + it("handles config at filesystem root edge case", () => { + const rootConfigPath = path.join(path.parse(process.cwd()).root, "test.json"); + const files = { [rootConfigPath]: { root: true } }; + // Even at root, absolute paths to other locations should be rejected + const obj = { $include: "/etc/passwd" }; + expect(() => resolve(obj, files, rootConfigPath)).toThrow(ConfigIncludeError); + }); + }); +}); diff --git a/src/config/includes.ts b/src/config/includes.ts index 53e4c81acf9..174e41bd2db 100644 --- a/src/config/includes.ts +++ b/src/config/includes.ts @@ -167,10 +167,37 @@ class IncludeProcessor { } private resolvePath(includePath: string): string { + const configDir = path.dirname(this.basePath); const resolved = path.isAbsolute(includePath) ? includePath - : path.resolve(path.dirname(this.basePath), includePath); - return path.normalize(resolved); + : path.resolve(configDir, includePath); + const normalized = path.normalize(resolved); + + // SECURITY: Reject paths outside config directory (CWE-22: Path Traversal) + if (!normalized.startsWith(configDir + path.sep) && normalized !== configDir) { + throw new ConfigIncludeError( + `Include path escapes config directory: ${includePath}`, + includePath, + ); + } + + // SECURITY: Resolve symlinks and re-validate to prevent symlink bypass + try { + const real = fs.realpathSync(normalized); + if (!real.startsWith(configDir + path.sep) && real !== configDir) { + throw new ConfigIncludeError( + `Include path resolves outside config directory (symlink): ${includePath}`, + includePath, + ); + } + } catch (err) { + if (err instanceof ConfigIncludeError) { + throw err; + } + // File doesn't exist yet - normalized path check above is sufficient + } + + return normalized; } private checkCircular(resolvedPath: string): void {