diff --git a/src/config/backup-rotation.ts b/src/config/backup-rotation.ts index d6c3035ebef..1883cde2045 100644 --- a/src/config/backup-rotation.ts +++ b/src/config/backup-rotation.ts @@ -1,11 +1,17 @@ +import path from "node:path"; + export const CONFIG_BACKUP_COUNT = 5; +export interface BackupRotationFs { + unlink: (path: string) => Promise; + rename: (from: string, to: string) => Promise; + chmod?: (path: string, mode: number) => Promise; + readdir?: (path: string) => Promise; +} + export async function rotateConfigBackups( configPath: string, - ioFs: { - unlink: (path: string) => Promise; - rename: (from: string, to: string) => Promise; - }, + ioFs: BackupRotationFs, ): Promise { if (CONFIG_BACKUP_COUNT <= 1) { return; @@ -24,3 +30,76 @@ export async function rotateConfigBackups( // best-effort }); } + +/** + * Harden file permissions on all .bak files in the rotation ring. + * copyFile does not guarantee permission preservation on all platforms + * (e.g. Windows, some NFS mounts), so we explicitly chmod each backup + * to owner-only (0o600) to match the main config file. + */ +export async function hardenBackupPermissions( + configPath: string, + ioFs: BackupRotationFs, +): Promise { + if (!ioFs.chmod) { + return; + } + const backupBase = `${configPath}.bak`; + // Harden the primary .bak + await ioFs.chmod(backupBase, 0o600).catch(() => { + // best-effort + }); + // Harden numbered backups + for (let i = 1; i < CONFIG_BACKUP_COUNT; i++) { + await ioFs.chmod(`${backupBase}.${i}`, 0o600).catch(() => { + // best-effort + }); + } +} + +/** + * Remove orphan .bak files that fall outside the managed rotation ring. + * These can accumulate from interrupted writes, manual copies, or PID-stamped + * backups (e.g. openclaw.json.bak.1772352289, openclaw.json.bak.before-marketing). + * + * Only files matching `.bak.*` are considered; the primary + * `.bak` and numbered `.bak.1` through `.bak.{N-1}` are preserved. + */ +export async function cleanOrphanBackups( + configPath: string, + ioFs: BackupRotationFs, +): Promise { + if (!ioFs.readdir) { + return; + } + const dir = path.dirname(configPath); + const base = path.basename(configPath); + const bakPrefix = `${base}.bak.`; + + // Build the set of valid numbered suffixes: "1", "2", ..., "{N-1}" + const validSuffixes = new Set(); + for (let i = 1; i < CONFIG_BACKUP_COUNT; i++) { + validSuffixes.add(String(i)); + } + + let entries: string[]; + try { + entries = await ioFs.readdir(dir); + } catch { + return; // best-effort + } + + for (const entry of entries) { + if (!entry.startsWith(bakPrefix)) { + continue; + } + const suffix = entry.slice(bakPrefix.length); + if (validSuffixes.has(suffix)) { + continue; + } + // This is an orphan — remove it + await ioFs.unlink(path.join(dir, entry)).catch(() => { + // best-effort + }); + } +} diff --git a/src/config/config.backup-rotation.test.ts b/src/config/config.backup-rotation.test.ts index cf55025d80a..8e9159beef3 100644 --- a/src/config/config.backup-rotation.test.ts +++ b/src/config/config.backup-rotation.test.ts @@ -1,7 +1,11 @@ import fs from "node:fs/promises"; import path from "node:path"; import { describe, expect, it } from "vitest"; -import { rotateConfigBackups } from "./backup-rotation.js"; +import { + rotateConfigBackups, + hardenBackupPermissions, + cleanOrphanBackups, +} from "./backup-rotation.js"; import { withTempHome } from "./test-helpers.js"; import type { OpenClawConfig } from "./types.js"; @@ -49,4 +53,63 @@ describe("config backup rotation", () => { await expect(fs.stat(`${configPath}.bak.5`)).rejects.toThrow(); }); }); + + it("hardenBackupPermissions sets 0o600 on all backup files", async () => { + await withTempHome(async () => { + const stateDir = process.env.OPENCLAW_STATE_DIR?.trim(); + if (!stateDir) { + throw new Error("Expected OPENCLAW_STATE_DIR to be set by withTempHome"); + } + const configPath = path.join(stateDir, "openclaw.json"); + + // Create .bak and .bak.1 with permissive mode + await fs.writeFile(`${configPath}.bak`, "secret", { mode: 0o644 }); + await fs.writeFile(`${configPath}.bak.1`, "secret", { mode: 0o644 }); + + await hardenBackupPermissions(configPath, fs); + + const bakStat = await fs.stat(`${configPath}.bak`); + const bak1Stat = await fs.stat(`${configPath}.bak.1`); + + // Owner-only permissions (0o600) + expect(bakStat.mode & 0o777).toBe(0o600); + expect(bak1Stat.mode & 0o777).toBe(0o600); + }); + }); + + it("cleanOrphanBackups removes stale files outside the rotation ring", async () => { + await withTempHome(async () => { + const stateDir = process.env.OPENCLAW_STATE_DIR?.trim(); + if (!stateDir) { + throw new Error("Expected OPENCLAW_STATE_DIR to be set by withTempHome"); + } + const configPath = path.join(stateDir, "openclaw.json"); + + // Create valid backups + await fs.writeFile(configPath, "current"); + await fs.writeFile(`${configPath}.bak`, "backup-0"); + await fs.writeFile(`${configPath}.bak.1`, "backup-1"); + await fs.writeFile(`${configPath}.bak.2`, "backup-2"); + + // Create orphans + await fs.writeFile(`${configPath}.bak.1772352289`, "orphan-pid"); + await fs.writeFile(`${configPath}.bak.before-marketing`, "orphan-manual"); + await fs.writeFile(`${configPath}.bak.99`, "orphan-overflow"); + + await cleanOrphanBackups(configPath, fs); + + // Valid backups preserved + await expect(fs.stat(`${configPath}.bak`)).resolves.toBeDefined(); + await expect(fs.stat(`${configPath}.bak.1`)).resolves.toBeDefined(); + await expect(fs.stat(`${configPath}.bak.2`)).resolves.toBeDefined(); + + // Orphans removed + await expect(fs.stat(`${configPath}.bak.1772352289`)).rejects.toThrow(); + await expect(fs.stat(`${configPath}.bak.before-marketing`)).rejects.toThrow(); + await expect(fs.stat(`${configPath}.bak.99`)).rejects.toThrow(); + + // Main config untouched + await expect(fs.readFile(configPath, "utf-8")).resolves.toBe("current"); + }); + }); }); diff --git a/src/config/io.ts b/src/config/io.ts index 9a051249221..3bec12c3a11 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -15,7 +15,11 @@ import { } from "../infra/shell-env.js"; import { VERSION } from "../version.js"; import { DuplicateAgentDirError, findDuplicateAgentDirs } from "./agent-dirs.js"; -import { rotateConfigBackups } from "./backup-rotation.js"; +import { + rotateConfigBackups, + hardenBackupPermissions, + cleanOrphanBackups, +} from "./backup-rotation.js"; import { applyCompactionDefaults, applyContextPruningDefaults, @@ -1245,6 +1249,8 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { await deps.fs.promises.copyFile(configPath, `${configPath}.bak`).catch(() => { // best-effort }); + await hardenBackupPermissions(configPath, deps.fs.promises); + await cleanOrphanBackups(configPath, deps.fs.promises); } try {