From a8ad242f885de2225455ae00f8564c83c7c4b162 Mon Sep 17 00:00:00 2001 From: Dominic <43616264+dominicnunez@users.noreply.github.com> Date: Mon, 26 Jan 2026 18:27:53 -0600 Subject: [PATCH] fix(security): properly test Windows ACL audit for config includes (#2403) * fix(security): properly test Windows ACL audit for config includes The test expected fs.config_include.perms_writable on Windows but chmod 0o644 has no effect on Windows ACLs. Use icacls to grant Everyone write access, which properly triggers the security check. Also stubs execIcacls to return proper ACL output so the audit can parse permissions without running actual icacls on the system. Adds cleanup via try/finally to remove temp directory containing world-writable test file. Fixes checks-windows CI failure. * test: isolate heartbeat runner tests from user workspace * docs: update changelog for #2403 --------- Co-authored-by: Tyler Yust --- CHANGELOG.md | 1 + ...tbeat-runner.returns-default-unset.test.ts | 7 +- src/security/audit.test.ts | 79 +++++++++++-------- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e57c7b4b1..4667e60d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Status: unreleased. ### Fixes - Security: pin npm overrides to keep tar@7.5.4 for install toolchains. +- Security: properly test Windows ACL audit for config includes. (#2403) Thanks @dominicnunez. - BlueBubbles: coalesce inbound URL link preview messages. (#1981) Thanks @tyler6204. - Cron: allow payloads containing "heartbeat" in event filter. (#2219) Thanks @dwfinkelstein. - Agents: include memory.md when bootstrapping memory context. (#2318) Thanks @czekaj. diff --git a/src/infra/heartbeat-runner.returns-default-unset.test.ts b/src/infra/heartbeat-runner.returns-default-unset.test.ts index 621f895fa..595cbaed7 100644 --- a/src/infra/heartbeat-runner.returns-default-unset.test.ts +++ b/src/infra/heartbeat-runner.returns-default-unset.test.ts @@ -333,6 +333,7 @@ describe("runHeartbeatOnce", () => { const cfg: ClawdbotConfig = { agents: { defaults: { + workspace: tmpDir, heartbeat: { every: "5m", target: "whatsapp" }, }, }, @@ -461,6 +462,7 @@ describe("runHeartbeatOnce", () => { const cfg: ClawdbotConfig = { agents: { defaults: { + workspace: tmpDir, heartbeat: { every: "5m", target: "last", @@ -542,6 +544,7 @@ describe("runHeartbeatOnce", () => { const cfg: ClawdbotConfig = { agents: { defaults: { + workspace: tmpDir, heartbeat: { every: "5m", target: "whatsapp" }, }, }, @@ -597,6 +600,7 @@ describe("runHeartbeatOnce", () => { const cfg: ClawdbotConfig = { agents: { defaults: { + workspace: tmpDir, heartbeat: { every: "5m", target: "whatsapp", @@ -668,6 +672,7 @@ describe("runHeartbeatOnce", () => { const cfg: ClawdbotConfig = { agents: { defaults: { + workspace: tmpDir, heartbeat: { every: "5m", target: "whatsapp", @@ -737,7 +742,7 @@ describe("runHeartbeatOnce", () => { try { const cfg: ClawdbotConfig = { agents: { - defaults: { heartbeat: { every: "5m" } }, + defaults: { workspace: tmpDir, heartbeat: { every: "5m" } }, list: [{ id: "work", default: true }], }, channels: { whatsapp: { allowFrom: ["*"] } }, diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index deebf7c70..3a43ff4cc 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -857,51 +857,62 @@ describe("security audit", () => { const includePath = path.join(stateDir, "extra.json5"); await fs.writeFile(includePath, "{ logging: { redactSensitive: 'off' } }\n", "utf-8"); - await fs.chmod(includePath, 0o644); + if (isWindows) { + // Grant "Everyone" write access to trigger the perms_writable check on Windows + const { execSync } = await import("node:child_process"); + execSync(`icacls "${includePath}" /grant Everyone:W`, { stdio: "ignore" }); + } else { + await fs.chmod(includePath, 0o644); + } const configPath = path.join(stateDir, "clawdbot.json"); await fs.writeFile(configPath, `{ "$include": "./extra.json5" }\n`, "utf-8"); await fs.chmod(configPath, 0o600); - const cfg: ClawdbotConfig = { logging: { redactSensitive: "off" } }; - const user = "DESKTOP-TEST\\Tester"; - const execIcacls = isWindows - ? async (_cmd: string, args: string[]) => { - const target = args[0]; - if (target === includePath) { + try { + const cfg: ClawdbotConfig = { logging: { redactSensitive: "off" } }; + const user = "DESKTOP-TEST\\Tester"; + const execIcacls = isWindows + ? async (_cmd: string, args: string[]) => { + const target = args[0]; + if (target === includePath) { + return { + stdout: `${target} NT AUTHORITY\\SYSTEM:(F)\n BUILTIN\\Users:(W)\n ${user}:(F)\n`, + stderr: "", + }; + } return { - stdout: `${target} NT AUTHORITY\\SYSTEM:(F)\n BUILTIN\\Users:(W)\n ${user}:(F)\n`, + stdout: `${target} NT AUTHORITY\\SYSTEM:(F)\n ${user}:(F)\n`, stderr: "", }; } - return { - stdout: `${target} NT AUTHORITY\\SYSTEM:(F)\n ${user}:(F)\n`, - stderr: "", - }; - } - : undefined; - const res = await runSecurityAudit({ - config: cfg, - includeFilesystem: true, - includeChannelSecurity: false, - stateDir, - configPath, - platform: isWindows ? "win32" : undefined, - env: isWindows - ? { ...process.env, USERNAME: "Tester", USERDOMAIN: "DESKTOP-TEST" } - : undefined, - execIcacls, - }); + : undefined; + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath, + platform: isWindows ? "win32" : undefined, + env: isWindows + ? { ...process.env, USERNAME: "Tester", USERDOMAIN: "DESKTOP-TEST" } + : undefined, + execIcacls, + }); - const expectedCheckId = isWindows - ? "fs.config_include.perms_writable" - : "fs.config_include.perms_world_readable"; + const expectedCheckId = isWindows + ? "fs.config_include.perms_writable" + : "fs.config_include.perms_world_readable"; - expect(res.findings).toEqual( - expect.arrayContaining([ - expect.objectContaining({ checkId: expectedCheckId, severity: "critical" }), - ]), - ); + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ checkId: expectedCheckId, severity: "critical" }), + ]), + ); + } finally { + // Clean up temp directory with world-writable file + await fs.rm(tmp, { recursive: true, force: true }); + } }); it("flags extensions without plugins.allow", async () => {