diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d43bece492..584558653ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -104,6 +104,7 @@ Docs: https://docs.openclaw.ai - Control UI/Sessions: make the compaction count a compact `N Checkpoint(s)` disclosure and show expanded session-level details with modern checkpoint history cards across responsive table layouts. Thanks @BunsDev. - Control UI/performance: keep chat and channel tabs responsive while history payloads and channel probes are slow, label partial channel status, and record slow chat/config render timings in the event log. Thanks @BunsDev. - Control UI/sessions: fire the documented `/new` command and lifecycle hooks only for explicit Control UI session creation, restoring session-memory and custom hook capture without changing SDK parent-session creates. Fixes #76957. Thanks @BunsDev. +- Exec approvals: fall back to a guarded copy when Windows rejects rename-overwrite for `exec-approvals.json`, while preserving symlink, hard-link, and owner-only permission safeguards. Fixes #77785. (#77907) Thanks @Alex-Alaniz and @MilleniumGenAI. - Slack: preserve Socket Mode SDK error context and structured Slack API fields in reconnect logs, so startup failures no longer collapse to a bare `unknown error`. - iOS pairing: allow setup-code and manual `ws://` connects for private LAN and `.local` gateways while keeping Tailscale/public routes on `wss://`, and prefer explicit gateway passwords over stale bootstrap tokens in mixed-auth reconnects. Fixes #47887; carries forward #65185. Thanks @draix and @BunsDev. - Plugins/diagnostics: make source-only TypeScript package warnings actionable by explaining that missing compiled runtime output is a publisher packaging issue and pointing users to update/reinstall or disable/uninstall the plugin. Fixes #77835. Thanks @googlerest. diff --git a/src/infra/exec-approvals-store.test.ts b/src/infra/exec-approvals-store.test.ts index 1dac65a2bc2..933325800f5 100644 --- a/src/infra/exec-approvals-store.test.ts +++ b/src/infra/exec-approvals-store.test.ts @@ -79,6 +79,14 @@ function readApprovalsFile(homeDir: string): ExecApprovalsFile { return JSON.parse(fs.readFileSync(approvalsFilePath(homeDir), "utf8")) as ExecApprovalsFile; } +function listExecApprovalTempFiles(homeDir: string): string[] { + const dir = path.dirname(approvalsFilePath(homeDir)); + if (!fs.existsSync(dir)) { + return []; + } + return fs.readdirSync(dir).filter((name) => name.endsWith(".tmp")); +} + describe("exec approvals store helpers", () => { it("expands home-prefixed default file and socket paths", () => { const dir = createHomeDir(); @@ -173,6 +181,189 @@ describe("exec approvals store helpers", () => { expect(fs.statSync(approvalsPath).ino).not.toBe(fs.statSync(linkedPath).ino); }); + it("normalizes successful rename writes to owner-only permissions", () => { + const dir = createHomeDir(); + const actualWriteFileSync = fs.writeFileSync.bind(fs); + vi.spyOn(fs, "writeFileSync").mockImplementation((file, data, options) => { + const result = actualWriteFileSync(file, data, options as never); + const filePath = String(file); + if ( + typeof file !== "number" && + filePath.includes(".exec-approvals.") && + filePath.endsWith(".tmp") + ) { + fs.chmodSync(file, 0o000); + } + return result; + }); + + saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} }); + + expect(fs.readFileSync(approvalsFilePath(dir), "utf8")).toContain('"security": "full"'); + expect(fs.statSync(approvalsFilePath(dir)).mode & 0o777).toBe(0o600); + }); + + it("normalizes the approvals directory to owner-only permissions", () => { + const dir = createHomeDir(); + const approvalsDir = path.dirname(approvalsFilePath(dir)); + fs.mkdirSync(approvalsDir, { recursive: true }); + fs.chmodSync(approvalsDir, 0o777); + + saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} }); + + expect(fs.readFileSync(approvalsFilePath(dir), "utf8")).toContain('"security": "full"'); + expect(fs.statSync(approvalsDir).mode & 0o777).toBe(0o700); + }); + + it("falls back to copying when rename cannot overwrite the approvals file", () => { + const dir = createHomeDir(); + const approvalsPath = approvalsFilePath(dir); + fs.mkdirSync(path.dirname(approvalsPath), { recursive: true }); + fs.writeFileSync(approvalsPath, '{"version":1,"agents":{}}\n', "utf8"); + const actualRenameSync = fs.renameSync.bind(fs); + const rename = vi.spyOn(fs, "renameSync").mockImplementation((from, to) => { + if (String(to) === approvalsPath) { + const error = Object.assign(new Error("locked target"), { code: "EPERM" }); + throw error; + } + return actualRenameSync(from, to); + }); + + saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} }); + + expect(rename).toHaveBeenCalled(); + expect(fs.readFileSync(approvalsPath, "utf8")).toContain('"security": "full"'); + expect(fs.statSync(approvalsPath).mode & 0o777).toBe(0o600); + expect(listExecApprovalTempFiles(dir)).toEqual([]); + }); + + it("normalizes fallback temp files before copying", () => { + const dir = createHomeDir(); + const approvalsPath = approvalsFilePath(dir); + fs.mkdirSync(path.dirname(approvalsPath), { recursive: true }); + fs.writeFileSync(approvalsPath, '{"version":1,"agents":{}}\n', "utf8"); + const actualWriteFileSync = fs.writeFileSync.bind(fs); + vi.spyOn(fs, "writeFileSync").mockImplementation((file, data, options) => { + const result = actualWriteFileSync(file, data, options as never); + const filePath = String(file); + if ( + typeof file !== "number" && + filePath.includes(".exec-approvals.") && + filePath.endsWith(".tmp") + ) { + fs.chmodSync(file, 0o000); + } + return result; + }); + const actualRenameSync = fs.renameSync.bind(fs); + vi.spyOn(fs, "renameSync").mockImplementation((from, to) => { + if (String(to) === approvalsPath) { + const error = Object.assign(new Error("locked target"), { code: "EPERM" }); + throw error; + } + return actualRenameSync(from, to); + }); + + saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} }); + + expect(fs.readFileSync(approvalsPath, "utf8")).toContain('"security": "full"'); + expect(fs.statSync(approvalsPath).mode & 0o777).toBe(0o600); + expect(listExecApprovalTempFiles(dir)).toEqual([]); + }); + + it("restores the previous approvals file when fallback copy fails", () => { + const dir = createHomeDir(); + const approvalsPath = approvalsFilePath(dir); + const previousRaw = '{"version":1,"defaults":{"security":"deny"},"agents":{}}\n'; + fs.mkdirSync(path.dirname(approvalsPath), { recursive: true }); + fs.writeFileSync(approvalsPath, previousRaw, { encoding: "utf8", mode: 0o600 }); + const actualRenameSync = fs.renameSync.bind(fs); + vi.spyOn(fs, "renameSync").mockImplementation((from, to) => { + if (String(to) === approvalsPath) { + const error = Object.assign(new Error("locked target"), { code: "EPERM" }); + throw error; + } + return actualRenameSync(from, to); + }); + const actualFtruncateSync = fs.ftruncateSync.bind(fs); + let forcedFallbackFailure = false; + vi.spyOn(fs, "ftruncateSync").mockImplementation((fd, len) => { + if (!forcedFallbackFailure && len === 0) { + forcedFallbackFailure = true; + actualFtruncateSync(fd, len); + const error = Object.assign(new Error("copy failed after opening destination"), { + code: "ENOSPC", + }); + throw error; + } + return actualFtruncateSync(fd, len); + }); + + expect(() => + saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} }), + ).toThrow(/copy failed after opening destination/); + expect(fs.readFileSync(approvalsPath, "utf8")).toBe(previousRaw); + expect(fs.statSync(approvalsPath).mode & 0o777).toBe(0o600); + expect(listExecApprovalTempFiles(dir)).toEqual([]); + }); + + it("does not follow a symlink swapped in before fallback copy", () => { + const dir = createHomeDir(); + const approvalsPath = approvalsFilePath(dir); + const targetPath = path.join(dir, "elsewhere.json"); + fs.mkdirSync(path.dirname(approvalsPath), { recursive: true }); + fs.writeFileSync(approvalsPath, '{"version":1,"agents":{}}\n', "utf8"); + fs.writeFileSync(targetPath, '{"sentinel":true}\n', "utf8"); + const actualRenameSync = fs.renameSync.bind(fs); + vi.spyOn(fs, "renameSync").mockImplementation((from, to) => { + if (String(to) === approvalsPath) { + const error = Object.assign(new Error("locked target"), { code: "EPERM" }); + throw error; + } + return actualRenameSync(from, to); + }); + const actualStatSync = fs.statSync.bind(fs); + let swappedDestination = false; + vi.spyOn(fs, "statSync").mockImplementation((file, options) => { + const result = actualStatSync(file, options as never); + if (!swappedDestination && String(file) === approvalsPath) { + swappedDestination = true; + fs.rmSync(approvalsPath); + fs.symlinkSync(targetPath, approvalsPath); + } + return result; + }); + + expect(() => + saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} }), + ).toThrow(/symlink|ELOOP/); + expect(fs.readFileSync(targetPath, "utf8")).toBe('{"sentinel":true}\n'); + expect(listExecApprovalTempFiles(dir)).toEqual([]); + }); + + it("does not use the copy fallback for hard-linked approvals files", () => { + const dir = createHomeDir(); + const approvalsPath = approvalsFilePath(dir); + const linkedPath = path.join(dir, "linked.json"); + fs.mkdirSync(path.dirname(approvalsPath), { recursive: true }); + fs.writeFileSync(linkedPath, '{"sentinel":true}\n', "utf8"); + fs.linkSync(linkedPath, approvalsPath); + const actualRenameSync = fs.renameSync.bind(fs); + vi.spyOn(fs, "renameSync").mockImplementation((from, to) => { + if (String(to) === approvalsPath) { + const error = Object.assign(new Error("locked target"), { code: "EPERM" }); + throw error; + } + return actualRenameSync(from, to); + }); + + expect(() => + saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} }), + ).toThrow(/hard-linked exec approvals file/); + expect(fs.readFileSync(linkedPath, "utf8")).toBe('{"sentinel":true}\n'); + expect(listExecApprovalTempFiles(dir)).toEqual([]); + }); + it("refuses to write approvals through a symlink destination", () => { const dir = createHomeDir(); const approvalsPath = approvalsFilePath(dir); diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index 6e732ab9ead..ebefb855a05 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -15,7 +15,6 @@ import type { ExecAllowlistEntry } from "./exec-approvals.types.js"; import { assertNoSymlinkParentsSync } from "./fs-safe-advanced.js"; import { expandHomePrefix, resolveRequiredHomeDir } from "./home-dir.js"; import { requestJsonlSocket } from "./jsonl-socket.js"; -import { privateFileStoreSync } from "./private-file-store.js"; export * from "./exec-approvals-analysis.js"; export * from "./exec-approvals-allowlist.js"; export type { ExecAllowlistEntry } from "./exec-approvals.types.js"; @@ -266,6 +265,13 @@ function ensureDir(filePath: string) { if (!dirStat.isDirectory() || dirStat.isSymbolicLink()) { throw new Error(`Refusing to use unsafe exec approvals directory: ${dir}`); } + try { + fs.chmodSync(dir, 0o700); + } catch (err) { + if (process.platform !== "win32") { + throw err; + } + } return dir; } @@ -291,6 +297,201 @@ function assertSafeExecApprovalsDestination(filePath: string): void { } } +function assertSafeExecApprovalsOverwriteFallback(filePath: string): void { + assertSafeExecApprovalsDestination(filePath); + try { + const stat = fs.statSync(filePath); + if (stat.nlink > 1) { + throw new Error(`Refusing copy fallback for hard-linked exec approvals file: ${filePath}`); + } + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "ENOENT") { + throw err; + } + } +} + +type ExecApprovalsFallbackDestination = { + existed: boolean; + fd: number; + snapshot: Buffer | null; +}; + +function sameFilesystemEntry(left: fs.Stats, right: fs.Stats): boolean { + return left.dev === right.dev && left.ino === right.ino; +} + +function readExecApprovalsFallbackSnapshotFromFd(fd: number): Buffer { + const chunks: Buffer[] = []; + const buffer = Buffer.alloc(64 * 1024); + let position = 0; + while (true) { + const bytesRead = fs.readSync(fd, buffer, 0, buffer.length, position); + if (bytesRead === 0) { + break; + } + chunks.push(Buffer.from(buffer.subarray(0, bytesRead))); + position += bytesRead; + } + return Buffer.concat(chunks); +} + +function validateExecApprovalsFallbackFd(filePath: string, fd: number): fs.Stats { + const linkStat = fs.lstatSync(filePath); + if (linkStat.isSymbolicLink()) { + throw new Error(`Refusing to write exec approvals via symlink: ${filePath}`); + } + const pathStat = fs.statSync(filePath); + const fdStat = fs.fstatSync(fd); + if (!fdStat.isFile()) { + throw new Error(`Refusing copy fallback for non-file exec approvals path: ${filePath}`); + } + if (fdStat.nlink > 1) { + throw new Error(`Refusing copy fallback for hard-linked exec approvals file: ${filePath}`); + } + if (!sameFilesystemEntry(pathStat, fdStat)) { + throw new Error(`Refusing copy fallback after exec approvals path changed: ${filePath}`); + } + return fdStat; +} + +function openExistingExecApprovalsFallbackDestination( + filePath: string, +): ExecApprovalsFallbackDestination { + const noFollowFlag = fs.constants.O_NOFOLLOW ?? 0; + const fd = fs.openSync(filePath, fs.constants.O_RDWR | noFollowFlag, 0o600); + try { + validateExecApprovalsFallbackFd(filePath, fd); + return { + existed: true, + fd, + snapshot: readExecApprovalsFallbackSnapshotFromFd(fd), + }; + } catch (err) { + try { + fs.closeSync(fd); + } catch { + // best-effort after validation failure + } + throw err; + } +} + +function createExecApprovalsFallbackDestination( + filePath: string, +): ExecApprovalsFallbackDestination { + const noFollowFlag = fs.constants.O_NOFOLLOW ?? 0; + try { + const fd = fs.openSync( + filePath, + fs.constants.O_RDWR | fs.constants.O_CREAT | fs.constants.O_EXCL | noFollowFlag, + 0o600, + ); + try { + validateExecApprovalsFallbackFd(filePath, fd); + return { existed: false, fd, snapshot: null }; + } catch (err) { + try { + fs.closeSync(fd); + } catch { + // best-effort after validation failure + } + throw err; + } + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "EEXIST") { + return openExistingExecApprovalsFallbackDestination(filePath); + } + throw err; + } +} + +function openExecApprovalsFallbackDestination(filePath: string): ExecApprovalsFallbackDestination { + try { + return openExistingExecApprovalsFallbackDestination(filePath); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") { + return createExecApprovalsFallbackDestination(filePath); + } + throw err; + } +} + +function writeExecApprovalsFallbackBuffer(fd: number, contents: Buffer): void { + fs.ftruncateSync(fd, 0); + let written = 0; + while (written < contents.length) { + written += fs.writeSync(fd, contents, written, contents.length - written, written); + } + fs.ftruncateSync(fd, contents.length); + try { + fs.fchmodSync(fd, 0o600); + } catch { + // best-effort on platforms without chmod + } +} + +function restoreExecApprovalsFallbackDestination( + filePath: string, + destination: ExecApprovalsFallbackDestination, +): void { + if (!destination.existed) { + try { + const pathStat = fs.statSync(filePath); + const fdStat = fs.fstatSync(destination.fd); + if (sameFilesystemEntry(pathStat, fdStat)) { + fs.rmSync(filePath, { force: true }); + } + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "ENOENT") { + throw err; + } + } + return; + } + writeExecApprovalsFallbackBuffer(destination.fd, destination.snapshot ?? Buffer.alloc(0)); +} + +function copyExecApprovalsFallback(tempPath: string, filePath: string): void { + const contents = fs.readFileSync(tempPath); + const destination = openExecApprovalsFallbackDestination(filePath); + try { + writeExecApprovalsFallbackBuffer(destination.fd, contents); + validateExecApprovalsFallbackFd(filePath, destination.fd); + } catch (copyErr) { + try { + restoreExecApprovalsFallbackDestination(filePath, destination); + } catch (restoreErr) { + throw new Error( + `Failed to restore exec approvals after copy fallback failure for ${filePath}: ${String( + copyErr, + )}`, + { cause: restoreErr }, + ); + } + throw copyErr; + } finally { + fs.closeSync(destination.fd); + } +} + +function renameExecApprovalsWithFallback(tempPath: string, filePath: string): void { + try { + fs.renameSync(tempPath, filePath); + return; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + // Windows can reject rename-overwrite when another process has a transient + // handle on the target approvals file. + if (code !== "EPERM" && code !== "EEXIST") { + throw err; + } + assertSafeExecApprovalsOverwriteFallback(filePath); + copyExecApprovalsFallback(tempPath, filePath); + fs.rmSync(tempPath, { force: true }); + } +} + // Coerce legacy/corrupted allowlists into `ExecAllowlistEntry[]` before we spread // entries to add ids (spreading strings creates {"0":"l","1":"s",...}). function coerceAllowlistEntries(allowlist: unknown): ExecAllowlistEntry[] | undefined { @@ -500,7 +701,27 @@ export function saveExecApprovals(file: ExecApprovalsFile) { function writeExecApprovalsRaw(filePath: string, raw: string) { const dir = ensureDir(filePath); assertSafeExecApprovalsDestination(filePath); - privateFileStoreSync(dir).writeText(path.basename(filePath), raw); + const tempPath = path.join(dir, `.exec-approvals.${process.pid}.${crypto.randomUUID()}.tmp`); + let tempWritten = false; + try { + fs.writeFileSync(tempPath, raw, { mode: 0o600, flag: "wx" }); + try { + fs.chmodSync(tempPath, 0o600); + } catch { + // best-effort on platforms without chmod + } + tempWritten = true; + renameExecApprovalsWithFallback(tempPath, filePath); + } finally { + if (tempWritten && fs.existsSync(tempPath)) { + fs.rmSync(tempPath, { force: true }); + } + } + try { + fs.chmodSync(filePath, 0o600); + } catch { + // best-effort on platforms without chmod + } } export function restoreExecApprovalsSnapshot(snapshot: ExecApprovalsSnapshot): void {