From 247740fc7750b79792c176d0a34a4b0085b912f3 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:20:38 +0800 Subject: [PATCH] fix(config-backup): redact webhook and intents, bounded folder probe, per-instance origin, reload intents on restore Review round on #661: webhook.json and intents.json are redacted in compare and URL values lose userinfo/query; a custom folder is probed (bounded, pastCap) before each pass and in each route and only created when its parent exists; manifests record an origin and change check, prune, list, restore and diff are scoped to it; a restore reloads IntentStore, lists untouched files and treats settings.json as restart-required; remote-cases.json is tracked; stale .tmp-cfg-* folders are swept and a failed restore unlinks its temp; the group is hidden from non-admins; an empty days field saves 30. --- CLAUDE.md | 2 +- docs/wiki/Config-Backups.md | 21 ++++-- src/config-backup.ts | 92 +++++++++++++++++++----- src/intent-store.ts | 5 ++ src/web/public/settings-ui.js | 11 +-- src/web/routes/config-backup-routes.ts | 62 +++++++++++++--- test/config-backup.test.ts | 57 +++++++++++++++ test/routes/config-backup-routes.test.ts | 27 ++++++- 8 files changed, 240 insertions(+), 37 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d0f72db6..f43a20cf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -156,7 +156,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Auto-resume on usage limit** (opt-in per session, top of the Respawn tab): when Claude halts on a subscription limit, `usage-limit-patterns.ts` (pure, unit-tested) parses the reset time and `SessionAutoOps` arms a timer for reset+2min, then sends Esc + `continue`. ⚠️ Respawn cycles are blocked while paused (`isLimitPaused` guard in `onIdleDetected`), which is what prevents `/clear` from wiping the paused conversation. Claude-mode only. → [architecture-invariants#auto-resume-on-usage-limit](docs/architecture-invariants.md#auto-resume-on-usage-limit) -**Config backups** (`src/config-backup.ts`, `routes/config-backup-routes.ts`; `configBackupEnabled` absent = ON, `configBackupDir`, `configBackupKeepCount` 1-500 default 20, `configBackupKeepDays` 0-3650 default 30; `docs/wiki/Config-Backups.md`): timestamped snapshots `cfg-YYYYMMDD-HHMMSS/` (+ `manifest.json` with SHA-256s, `reason`, `appVersion`) of a FIXED allowlist (`CONFIG_BACKUP_FILES`: settings, clis, custom-model-hosts, webhook, intents, linked-cases, remote/docker hosts+cases, webviews, push-keys, users, `.env`) into `/backups/config` or a custom absolute folder. ⚠️ `state.json` is deliberately NOT tracked (rewritten per session event; it also holds cron jobs and respawn presets). One snapshot per real content change (digest of the file set vs the latest manifest), at startup + a 5-minute timer, skipped in test mode. ⚠️ Retention (`pruneConfigBackups`) only deletes folders matching the name pattern AND holding a parseable manifest, and never the newest; a custom folder equal to the data dir falls back to the default. ⚠️ Restore verifies every file's SHA-256 BEFORE writing, takes a `pre-restore` snapshot, writes atomically at 0600 and only allowlisted names; the id is validated against the name pattern (no traversal). Snapshots are 0700/0600 because `.env`/`users.json`/`custom-model-hosts.json` carry credentials; all routes are admin-only in multi-user mode. → [docs/wiki/Config-Backups.md](docs/wiki/Config-Backups.md) +**Config backups** (`src/config-backup.ts`, `routes/config-backup-routes.ts`; `configBackupEnabled` absent = ON, `configBackupDir`, `configBackupKeepCount` 1-500 default 20, `configBackupKeepDays` 0-3650 default 30; `docs/wiki/Config-Backups.md`): timestamped snapshots `cfg-YYYYMMDD-HHMMSS/` (+ `manifest.json` with SHA-256s, `reason`, `appVersion`) of a FIXED allowlist (`CONFIG_BACKUP_FILES`: settings, clis, custom-model-hosts, webhook, intents, linked-cases, remote/docker hosts+cases (incl. `remote-cases.json`), webviews, push-keys, users, `.env`) into `/backups/config` or a custom absolute folder. ⚠️ `state.json` is deliberately NOT tracked (rewritten per session event; it also holds cron jobs and respawn presets). One snapshot per real content change (digest of the file set vs the latest manifest), at startup + a 5-minute timer, skipped in test mode. ⚠️ Retention (`pruneConfigBackups`) only deletes folders matching the name pattern AND holding a parseable manifest, and never the newest; a custom folder equal to the data dir falls back to the default. ⚠️ Restore verifies every file's SHA-256 BEFORE writing, takes a `pre-restore` snapshot, writes atomically at 0600 and only allowlisted names; the id is validated against the name pattern (no traversal). Snapshots are 0700/0600 because `.env`/`users.json`/`custom-model-hosts.json` carry credentials; all routes are admin-only in multi-user mode. ⚠️ A custom folder is probed with `probePathKind(dir, { pastCap: true })` before every pass and in each route (#516: it may be a network mount; `unknown` skips the pass / answers 503 `OPERATION_FAILED`), and a missing custom folder is only created when its parent is present. ⚠️ Each manifest records an `origin` (hostname + data dir); change check, prune, list, restore and diff are scoped to it so instances sharing one folder do not snapshot or prune each other. Restoring `intents.json` calls `intentStore.reload()`; `settings.json` applies its watcher toggles after a restart. → [docs/wiki/Config-Backups.md](docs/wiki/Config-Backups.md) **Plan-usage chip** (`showPlanUsageLimits`, per-device: desktop default **ON**, handhelds OFF): resolve DISPLAY only through `planUsageChipEnabled()` (settings-ui.js). The same setting is the server-side COLLECTION switch, read fresh by `readPlanUsageTelemetryEnabled()` (hooks-config.ts) at every claude create/respawn. ⚠️ An ABSENT key reads as ON in the reader; `GET /api/settings` must never write. ⚠️ A save sends `showPlanUsageLimits` ONLY when it flips the chip on that device (`planUsageCollectionFlip()`), or a phone switches collection off for every desktop. Claude data comes from the statusLine exporter, injected as an EPHEMERAL `claude --settings` flag (`resolveStatusLineCliCommand`), never written to disk, WRAPPING a user's own statusLine, posting to `POST /api/status-telemetry`. Codex comes from a read-only `account/rateLimits/read` poll (main bucket only). The statusline's `prompt_cache` object (Claude Code v2.1.251+) is kept by `StatusTelemetrySchema` (every documented scalar field, null-tolerant; `parsePromptCache()` in usage-telemetry.ts normalizes only the stored subset) and shown in the footer as `cache:until HH:MM` / `cache:cold` (only when the user has no statusline of their own) and in Session Options → Respawn (`#promptCacheStatus`, fed by `Session.promptCache` in `toState()` plus the change-only `session:promptCache` SSE event, worded by the pure `CodemanPromptCache.format`): warm-until/cold, the re-cache stake, the hit rate, the last miss's causes, and a `Context compacted at HH:MM` note after a compaction or a tool-result clearing (stamped on the first warm report whose `recache_tokens_if_cold` is null after one that carried it, never from `expected_rebuilds`, which Claude Code bumps only at the next request; NEVER an alert). ⚠️ `Session.setPromptCache()` stamps `compactedAt` onto a NEW object (never mutating the parsed report) and the route broadcasts the STORED object; the last report is restored on restart (constructor `promptCache`, like `displayModel`) and the readout is time-aware (a passed `expiresAt` reads cold, not stale-warm). It is the source of truth for the cache TTL, never the plan tier. → [architecture-invariants#plan-usage-chip-statusline-telemetry](docs/architecture-invariants.md#plan-usage-chip-statusline-telemetry), `docs/usage-limits-display-plan.md` diff --git a/docs/wiki/Config-Backups.md b/docs/wiki/Config-Backups.md index 73eab2ed..295a4b9c 100644 --- a/docs/wiki/Config-Backups.md +++ b/docs/wiki/Config-Backups.md @@ -17,7 +17,7 @@ A fixed list of files from the data directory (`~/.codeman/`, or the directory o | `webhook.json` | Webhook notification target. | | `intents.json` | Read My Mind per-case profiles. | | `linked-cases.json` | Registered cases. | -| `remote-hosts.json`, `docker-hosts.json`, `docker-cases.json` | Remote SSH and Docker overlay configuration. | +| `remote-hosts.json`, `remote-cases.json`, `docker-hosts.json`, `docker-cases.json` | Remote SSH and Docker overlay configuration. | | `webviews.json` | Saved dashboard URLs. | | `push-keys.json` | Web-push keys (regenerating them drops every device's subscription). | | `users.json` | Multi-user accounts. **Holds password hashes.** | @@ -122,7 +122,14 @@ Only the tracked names are ever written, whatever a manifest lists. Settings are re-read on the next request, so reload the page. `clis.json`, `users.json`, `.env` and `push-keys.json` are read once per process: **restart Codeman** for those to take -effect (the response lists which). A restore does not touch running sessions. +effect (the response lists which). Restoring `settings.json` also does not re-run the switches +that saving in Settings applies at once (file watchers and the like), so those apply after a +restart, and `settings.json` is listed with the restart-required files. A restore does not touch +running sessions. + +When the backup folder is shared (a synced folder used by two machines, or prod and a beta), +each snapshot records the host and data directory that made it, and every instance only +snapshots, lists, prunes, compares and restores its own. You can also restore by hand: the files in a backup folder are the originals, so copying one back over the file in the data directory is equivalent (stop Codeman first if you are replacing @@ -133,7 +140,8 @@ over the file in the data directory is equivalent (stop Codeman first if you are Before you restore, or to find out what changed and when, use **Compare** in the list: - **Compare with current** on a backup shows what differs between that backup and the config - running now, which is also what a restore of it would undo. + running now. A restore puts back the files the backup holds; files created since are left as + they are, and the restore response names them (`untouched`). - **Changes in this backup** shows what differs between the backup before it and this one. - The **Compare … with … Show changes** pickers compare any two backups, or a backup with the current config. @@ -144,9 +152,10 @@ that are the same are listed on one line. A file that is not JSON shows only tha a file with more than 200 changes is cut short, so use the files themselves for a full diff. A self-refreshed discovery timestamp in `custom-model-hosts.json` is not counted as a change. -**Credential values are never shown.** `.env`, `users.json` and `push-keys.json` list only the -names that changed, and in every other file a key that looks like a token, secret, password, -credential or key is shown as "changed" without its value. +**Credential values are never shown.** `.env`, `users.json`, `push-keys.json`, `webhook.json` (the +URL is a bearer secret) and `intents.json` (captured prompts) list only the names that changed, and in every other file a key that looks like a token, secret, password, +credential or key is shown as "changed" without its value. URL values lose any `user:password@`, +query and fragment. ## Security diff --git a/src/config-backup.ts b/src/config-backup.ts index f4c67ffd..befbfc7c 100644 --- a/src/config-backup.ts +++ b/src/config-backup.ts @@ -25,6 +25,7 @@ import { createHash } from 'node:crypto'; import { chmodSync, existsSync, + lstatSync, mkdirSync, readdirSync, readFileSync, @@ -33,7 +34,7 @@ import { statSync, writeFileSync, } from 'node:fs'; -import { homedir } from 'node:os'; +import { homedir, hostname } from 'node:os'; import { isAbsolute, join, resolve, sep } from 'node:path'; /** @@ -55,6 +56,7 @@ export const CONFIG_BACKUP_FILES = [ 'intents.json', 'linked-cases.json', 'remote-hosts.json', + 'remote-cases.json', 'docker-hosts.json', 'docker-cases.json', 'webviews.json', @@ -94,6 +96,8 @@ export interface ConfigBackupManifest { appVersion: string; /** Digest of the set of files, so an unchanged state is not snapshotted twice. */ hash: string; + /** Host and data dir that made it, so instances sharing one folder keep to their own snapshots. */ + origin?: string; files: ConfigBackupFile[]; } @@ -152,6 +156,9 @@ export function resolveConfigBackupDir(settings: ConfigBackupSettings, dataDir: return dir; } +/** Who owns a snapshot: this machine's name plus the data dir (prod and a beta differ by the latter). */ +export const configBackupOrigin = (dataDir: string): string => `${hostname()}:${resolve(dataDir)}`; + const sha256 = (data: Buffer | string): string => createHash('sha256').update(data).digest('hex'); /** @@ -196,21 +203,25 @@ function stamp(now: number): string { return `${d.getFullYear()}${p(d.getMonth() + 1)}${p(d.getDate())}-${p(d.getHours())}${p(d.getMinutes())}${p(d.getSeconds())}`; } -function readManifest(dir: string, id: string): ConfigBackupManifest | null { +function readManifest(dir: string, id: string, origin?: string): ConfigBackupManifest | null { if (!SNAPSHOT_ID_RE.test(id)) return null; try { const m = JSON.parse(readFileSync(join(dir, id, MANIFEST), 'utf8')) as ConfigBackupManifest; if (!m || m.id !== id || !Array.isArray(m.files) || typeof m.createdAt !== 'number' || typeof m.hash !== 'string') { return null; } + if (origin !== undefined && m.origin !== origin) return null; return m; } catch { return null; } } -/** Snapshots this module made in `dir`, newest first. Foreign folders and files are ignored. */ -export function listConfigBackups(dir: string): ConfigBackupSummary[] { +/** + * Snapshots this module made in `dir`, newest first. Foreign folders and files are ignored, and so + * are other instances' snapshots when `origin` is given (see {@link configBackupOrigin}). + */ +export function listConfigBackups(dir: string, origin?: string): ConfigBackupSummary[] { let names: string[]; try { names = readdirSync(dir); @@ -219,7 +230,7 @@ export function listConfigBackups(dir: string): ConfigBackupSummary[] { } const out: ConfigBackupSummary[] = []; for (const id of names) { - const m = readManifest(dir, id); + const m = readManifest(dir, id, origin); if (!m) continue; const files = m.files.map((f) => ({ name: f.name, bytes: f.bytes })); out.push({ @@ -267,9 +278,10 @@ export function createConfigBackup(opts: CreateConfigBackupOptions): ConfigBacku if (present.length === 0) return { status: 'empty' }; const hash = sha256(present.map((f) => `${f.name}\0${f.change}`).join('\n')); - const existing = listConfigBackups(opts.backupDir); + const origin = configBackupOrigin(opts.dataDir); + const existing = listConfigBackups(opts.backupDir, origin); if (!opts.force && existing[0]) { - const latest = readManifest(opts.backupDir, existing[0].id); + const latest = readManifest(opts.backupDir, existing[0].id, origin); if (latest && latest.hash === hash) return { status: 'unchanged', id: latest.id }; } @@ -288,6 +300,7 @@ export function createConfigBackup(opts: CreateConfigBackupOptions): ConfigBacku reason: opts.reason, appVersion: opts.appVersion ?? '', hash, + origin, files: present.map((f) => ({ name: f.name, bytes: f.data.length, sha256: f.sha })), }; for (const f of present) writeFileSync(join(tmp, f.name), f.data, { mode: 0o600 }); @@ -305,9 +318,26 @@ export function createConfigBackup(opts: CreateConfigBackupOptions): ConfigBacku return { status: 'created', id, files: present.length, bytes: present.reduce((n, f) => n + f.data.length, 0) }; } +/** Remove `.tmp-cfg-*` build folders a crashed snapshot left behind (older than an hour, so a live one on a shared folder is safe). */ +export function sweepConfigBackupTemps(dir: string, now: number = Date.now()): void { + let names: string[]; + try { + names = readdirSync(dir); + } catch { + return; + } + for (const n of names) { + if (!/^\.tmp-cfg-\d{8}-\d{6}(?:-\d{1,3})?-\d+$/.test(n)) continue; + const st = lstatSync(join(dir, n), { throwIfNoEntry: false }); + if (st && now - st.mtimeMs > 3_600_000) rmSync(join(dir, n), { recursive: true, force: true }); + } +} + export interface PruneOptions { keepCount: number; keepDays: number; + /** Only this instance's snapshots are pruned; omitted = every snapshot in the folder. */ + origin?: string; now?: number; } @@ -317,7 +347,7 @@ export interface PruneOptions { */ export function pruneConfigBackups(dir: string, opts: PruneOptions): string[] { const now = opts.now ?? Date.now(); - const all = listConfigBackups(dir); + const all = listConfigBackups(dir, opts.origin); const cutoff = opts.keepDays > 0 ? now - opts.keepDays * 86_400_000 : null; const removed: string[] = []; all.forEach((b, index) => { @@ -343,6 +373,8 @@ export class ConfigBackupCorruptError extends Error {} export interface RestoreResult { restored: string[]; + /** Tracked files present now that the snapshot did not hold; a restore leaves them as they are. */ + untouched: string[]; /** The snapshot taken of the live files before they were overwritten. */ safetySnapshot: string | null; } @@ -359,7 +391,7 @@ export function restoreConfigBackup(opts: { appVersion?: string; now?: number; }): RestoreResult { - const manifest = readManifest(opts.backupDir, opts.id); + const manifest = readManifest(opts.backupDir, opts.id, configBackupOrigin(opts.dataDir)); if (!manifest) throw new ConfigBackupNotFoundError(`No config backup named ${opts.id}`); const allowed = new Set(CONFIG_BACKUP_FILES); @@ -392,11 +424,20 @@ export function restoreConfigBackup(opts: { for (const f of loaded) { const target = join(opts.dataDir, f.name); const tmp = `${target}.restore-${process.pid}`; - writeFileSync(tmp, f.data, { mode: 0o600 }); - renameSync(tmp, target); + try { + writeFileSync(tmp, f.data, { mode: 0o600 }); + renameSync(tmp, target); + } catch (err) { + rmSync(tmp, { force: true }); + throw err; + } restored.push(f.name); } - return { restored, safetySnapshot: safety.status === 'created' ? safety.id : null }; + // A restore never deletes: tracked files the snapshot did not hold stay as they are. + const untouched = CONFIG_BACKUP_FILES.filter( + (n) => !restored.includes(n) && lstatSync(join(opts.dataDir, n), { throwIfNoEntry: false })?.isFile() + ); + return { restored, untouched, safetySnapshot: safety.status === 'created' ? safety.id : null }; } /** The live files in the data dir, as a side of a diff. */ @@ -436,7 +477,13 @@ const MAX_DIFF_CHANGES = 200; const MAX_DIFF_VALUE_CHARS = 200; /** Files whose every value is a credential or key: only names are ever shown. */ -const REDACT_ALL_FILES: ReadonlySet = new Set(['.env', 'users.json', 'push-keys.json']); +const REDACT_ALL_FILES: ReadonlySet = new Set([ + '.env', + 'users.json', + 'push-keys.json', + 'webhook.json', + 'intents.json', +]); /** Elsewhere a value is hidden when its own key looks like a credential. */ const SECRET_KEY_RE = /(token|secret|passw|credential|auth|key$)/i; @@ -478,6 +525,19 @@ function leavesOf(name: string, data: Buffer): Leaves | null { } } +/** A URL value loses its userinfo, query and fragment: they carry passwords and tokens. */ +function scrubUrl(jsonText: string): string { + try { + const v = JSON.parse(jsonText) as unknown; + if (typeof v !== 'string' || !/^[a-z][a-z0-9+.-]*:\/\//i.test(v)) return jsonText; + const u = new URL(v); + if (!u.username && !u.password && !u.search && !u.hash) return jsonText; + return JSON.stringify(`${u.protocol}//${u.host}${u.pathname}`); + } catch { + return jsonText; + } +} + const clip = (s: string): string => (s.length > MAX_DIFF_VALUE_CHARS ? `${s.slice(0, MAX_DIFF_VALUE_CHARS)}…` : s); function diffLeaves(name: string, a: Leaves, b: Leaves): { changes: ConfigDiffChange[]; truncated: boolean } { @@ -494,8 +554,8 @@ function diffLeaves(name: string, a: Leaves, b: Leaves): { changes: ConfigDiffCh changes.push({ path, kind, - ...(before !== undefined && { before: clip(before) }), - ...(after !== undefined && { after: clip(after) }), + ...(before !== undefined && { before: clip(scrubUrl(before)) }), + ...(after !== undefined && { after: clip(scrubUrl(after)) }), }); } } @@ -516,7 +576,7 @@ function loadDiffSide(side: string, dataDir: string, backupDir: string): Map(CONFIG_BACKUP_FILES); for (const f of manifest.files) { diff --git a/src/intent-store.ts b/src/intent-store.ts index d1592bf9..432a318f 100644 --- a/src/intent-store.ts +++ b/src/intent-store.ts @@ -108,6 +108,11 @@ export class IntentStore { // ----- Public API ----- + /** Drop the in-memory copy so the next call re-reads intents.json (after a config restore replaced it). */ + reload(): void { + this.profiles = null; + } + /** * The profile for a session's case. Never persists on read: an absent * profile returns an empty transient one (`updatedAt: 0`). diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 6fcab2ac..78f41506 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -1364,13 +1364,13 @@ Object.assign(CodemanApp.prototype, { * wired to `codeman:me` for the same late-resolving role as the groups above. */ _applyDoctorAdminGate() { - const group = document.getElementById('doctorGroup'); - if (!group) return; const me = window.__codemanUser || {}; - group.style.display = me.multiUser && me.role !== 'admin' ? 'none' : ''; + for (const id of ['doctorGroup', 'configBackupGroup']) { + const group = document.getElementById(id); + if (group) group.style.display = me.multiUser && me.role !== 'admin' ? 'none' : ''; + } }, - /** Preview (apply=false) or run (apply=true) the MCP server sync across enabled CLIs. */ // ── Config backups ────────────────────────────────────────────────────────── // Server-side files, so these read the SAVED settings: a folder or retention edit applies after Save. @@ -1496,6 +1496,7 @@ Object.assign(CodemanApp.prototype, { this.loadConfigBackups(); }, + /** Preview (apply=false) or run (apply=true) the MCP server sync across enabled CLIs. */ async mcpSync(apply) { const out = this.$('mcpSyncResult'); const show = (html, hint = '') => { @@ -2871,7 +2872,7 @@ Object.assign(CodemanApp.prototype, { configBackupEnabled: document.getElementById('appSettingsConfigBackup').checked, configBackupDir: document.getElementById('appSettingsConfigBackupDir').value.trim(), configBackupKeepCount: Math.min(500, Math.max(1, parseInt(document.getElementById('appSettingsConfigBackupKeepCount').value, 10) || 20)), - configBackupKeepDays: Math.min(3650, Math.max(0, parseInt(document.getElementById('appSettingsConfigBackupKeepDays').value, 10) || 0)), + configBackupKeepDays: Math.min(3650, Math.max(0, Number.isNaN(parseInt(document.getElementById('appSettingsConfigBackupKeepDays').value, 10)) ? 30 : parseInt(document.getElementById('appSettingsConfigBackupKeepDays').value, 10))), readMyMindEnabled: document.getElementById('appSettingsReadMyMind').checked, ultracodeFloatingWindows: document.getElementById('appSettingsUltracodeFloatingWindows').checked, showMultiMonitorButton: document.getElementById('appSettingsShowMultiMonitorButton').checked, diff --git a/src/web/routes/config-backup-routes.ts b/src/web/routes/config-backup-routes.ts index fd0e2c7d..3494d602 100644 --- a/src/web/routes/config-backup-routes.ts +++ b/src/web/routes/config-backup-routes.ts @@ -20,6 +20,9 @@ import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } import { isAdmin, readJsonConfig, SETTINGS_PATH } from '../route-helpers.js'; import { isMultiUserMode } from '../../config/multiuser.js'; import { getDataDir } from '../../config/instance.js'; +import { intentStore } from '../../intent-store.js'; +import { describeUnknownPath, probePathKind } from '../../utils/bounded-path-probe.js'; +import { dirname } from 'node:path'; import { CONFIG_BACKUP_FILES, CONFIG_DIFF_CURRENT, @@ -33,6 +36,8 @@ import { readConfigBackupSettings, resolveConfigBackupDir, restoreConfigBackup, + configBackupOrigin, + sweepConfigBackupTemps, type ConfigBackupResult, type ConfigBackupSettings, type ConfigBackupSummary, @@ -49,7 +54,7 @@ const APP_VERSION: string = (() => { })(); /** Files whose running-process copy only reloads on restart (the rest are read per request). */ -const RESTART_REQUIRED = new Set(['clis.json', 'users.json', '.env', 'push-keys.json']); +const RESTART_REQUIRED = new Set(['clis.json', 'users.json', '.env', 'push-keys.json', 'settings.json']); export interface ConfigBackupStatus { settings: ConfigBackupSettings; @@ -65,6 +70,29 @@ export async function readEffectiveBackupSettings(): Promise<{ settings: ConfigB return { settings, dir: resolveConfigBackupDir(settings, getDataDir()) }; } +/** A user-chosen backup folder that is not safe to touch (bounded probe, as for case paths, #516). */ +export class BackupDirUnavailableError extends Error {} + +/** + * Probe a custom backup folder without blocking the event loop on a dead mount. The default folder + * needs no probe. Returns whether the folder exists; throws when it did not answer, or is not a + * folder. A missing custom folder is only creatable when its parent is present. + */ +export async function probeBackupDir(settings: ConfigBackupSettings, dir: string): Promise { + if (!settings.dir || dir === defaultConfigBackupDir(getDataDir())) return true; + const kind = await probePathKind(dir, { pastCap: true }); + if (kind === 'unknown') + throw new BackupDirUnavailableError(describeUnknownPath('Backup folder', dir, { pastCap: true })); + if (kind === 'file') throw new BackupDirUnavailableError(`Backup folder is a file, not a folder: ${dir}`); + if (kind === 'directory') return true; + const parent = await probePathKind(dirname(dir), { pastCap: true }); + if (parent === 'unknown') { + throw new BackupDirUnavailableError(describeUnknownPath('Backup folder', dirname(dir), { pastCap: true })); + } + if (parent !== 'directory') throw new BackupDirUnavailableError(`Backup folder's parent is missing: ${dirname(dir)}`); + return false; +} + /** Snapshot (when changed) and apply retention; shared by the timer and the manual route. */ export async function runConfigBackup( reason: string, @@ -72,6 +100,8 @@ export async function runConfigBackup( ): Promise<{ result: ConfigBackupResult; removed: string[] } | null> { const { settings, dir } = await readEffectiveBackupSettings(); if (!settings.enabled && !opts.force) return null; + await probeBackupDir(settings, dir); + sweepConfigBackupTemps(dir); const result = createConfigBackup({ dataDir: getDataDir(), backupDir: dir, @@ -79,7 +109,11 @@ export async function runConfigBackup( appVersion: APP_VERSION, force: opts.force, }); - const removed = pruneConfigBackups(dir, { keepCount: settings.keepCount, keepDays: settings.keepDays }); + const removed = pruneConfigBackups(dir, { + keepCount: settings.keepCount, + keepDays: settings.keepDays, + origin: configBackupOrigin(getDataDir()), + }); return { result, removed }; } @@ -96,6 +130,13 @@ export function registerConfigBackupRoutes(app: FastifyInstance): void { const denied = deny(req, reply); if (denied) return denied; const { settings, dir } = await readEffectiveBackupSettings(); + let present: boolean; + try { + present = await probeBackupDir(settings, dir); + } catch (err) { + reply.code(503); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getErrorMessage(err)); + } return { success: true, data: { @@ -103,7 +144,7 @@ export function registerConfigBackupRoutes(app: FastifyInstance): void { dir, defaultDir: defaultConfigBackupDir(getDataDir()), tracked: CONFIG_BACKUP_FILES, - backups: listConfigBackups(dir), + backups: present ? listConfigBackups(dir, configBackupOrigin(getDataDir())) : [], }, }; }); @@ -115,7 +156,7 @@ export function registerConfigBackupRoutes(app: FastifyInstance): void { const out = await runConfigBackup('manual', { force: true }); return { success: true, data: { ...out!.result, removed: out!.removed } }; } catch (err) { - reply.code(500); + reply.code(err instanceof BackupDirUnavailableError ? 503 : 500); return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `Backup failed: ${getErrorMessage(err)}`); } }); @@ -129,7 +170,8 @@ export function registerConfigBackupRoutes(app: FastifyInstance): void { return createErrorResponse(ApiErrorCode.INVALID_INPUT, '`from` is required (a backup id or "current")'); } try { - const { dir } = await readEffectiveBackupSettings(); + const { settings, dir } = await readEffectiveBackupSettings(); + await probeBackupDir(settings, dir); const data = diffConfigBackups({ dataDir: getDataDir(), backupDir: dir, @@ -146,7 +188,7 @@ export function registerConfigBackupRoutes(app: FastifyInstance): void { reply.code(409); return createErrorResponse(ApiErrorCode.CONFLICT, err.message); } - reply.code(500); + reply.code(err instanceof BackupDirUnavailableError ? 503 : 500); return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `Diff failed: ${getErrorMessage(err)}`); } }); @@ -156,12 +198,16 @@ export function registerConfigBackupRoutes(app: FastifyInstance): void { if (denied) return denied; const { id } = req.params as { id: string }; try { - const { dir } = await readEffectiveBackupSettings(); + const { settings, dir } = await readEffectiveBackupSettings(); + await probeBackupDir(settings, dir); const out = restoreConfigBackup({ dataDir: getDataDir(), backupDir: dir, id, appVersion: APP_VERSION }); + // IntentStore caches the file and rewrites it whole on the next prompt, which would undo the restore. + if (out.restored.includes('intents.json')) intentStore.reload(); return { success: true, data: { restored: out.restored, + untouched: out.untouched, safetySnapshot: out.safetySnapshot, restartRequired: out.restored.filter((n) => RESTART_REQUIRED.has(n)), }, @@ -175,7 +221,7 @@ export function registerConfigBackupRoutes(app: FastifyInstance): void { reply.code(409); return createErrorResponse(ApiErrorCode.CONFLICT, `${err.message}. Nothing was changed.`); } - reply.code(500); + reply.code(err instanceof BackupDirUnavailableError ? 503 : 500); return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `Restore failed: ${getErrorMessage(err)}`); } }); diff --git a/test/config-backup.test.ts b/test/config-backup.test.ts index fefcb16b..4e3ffcc7 100644 --- a/test/config-backup.test.ts +++ b/test/config-backup.test.ts @@ -30,6 +30,8 @@ import { readConfigBackupSettings, resolveConfigBackupDir, restoreConfigBackup, + configBackupOrigin, + sweepConfigBackupTemps, } from '../src/config-backup.js'; let root: string; @@ -335,6 +337,21 @@ describe('diffConfigBackups', () => { expect(diff(a, a).changed).toBe(0); }); + it('never returns a webhook URL, captured prompts, or URL userinfo and query', () => { + put('webhook.json', JSON.stringify({ url: 'https://hooks.slack.com/services/T0/B0/oldsecret' })); + put('intents.json', JSON.stringify({ profiles: [{ key: 'k', prompts: ['my password is pw1'] }] })); + put('settings.json', JSON.stringify({ baseUrl: 'https://user:pw@host/p?token=t1#f' })); + const a = snap(T0); + put('webhook.json', JSON.stringify({ url: 'https://hooks.slack.com/services/T0/B0/newsecret' })); + put('intents.json', JSON.stringify({ profiles: [{ key: 'k', prompts: ['my password is pw2'] }] })); + put('settings.json', JSON.stringify({ baseUrl: 'https://user:pw@host/p?token=t2#f' })); + const d = diff(a, 'current'); + const json = JSON.stringify(d); + for (const s of ['secret', 'pw1', 'pw2', 'user:pw', 'token=', 'hooks.slack']) expect(json).not.toContain(s); + expect(file(d, 'webhook.json').changes.every((c) => c.redacted)).toBe(true); + expect(file(d, 'intents.json').changes.every((c) => c.redacted)).toBe(true); + }); + it('never returns a credential value, only that it differs', () => { put('.env', 'SECRET_ONE=hunter2\nOTHER=abc\n'); put('users.json', JSON.stringify({ admin: { name: 'root' } })); @@ -388,3 +405,43 @@ describe('diffConfigBackups', () => { expect(() => diff(a, 'current')).toThrow(ConfigBackupCorruptError); }); }); + +describe('instances sharing one backup folder', () => { + it('keeps to its own snapshots: no churn, no prune, no restore of the other', () => { + const other = join(root, 'other-data'); + mkdirSync(other, { recursive: true }); + put('settings.json', '{"a":1}'); + writeFileSync(join(other, 'settings.json'), '{"b":2}'); + const mine = createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 }); + const theirs = createConfigBackup({ dataDir: other, backupDir: backups, reason: 'auto', now: T0 + 1000 }); + expect(theirs.status).toBe('created'); + // Each side's change check compares with its own newest snapshot, so a second tick is a no-op. + expect(createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 + 2000 }).status).toBe( + 'unchanged' + ); + expect(createConfigBackup({ dataDir: other, backupDir: backups, reason: 'auto', now: T0 + 3000 }).status).toBe( + 'unchanged' + ); + expect(listConfigBackups(backups, configBackupOrigin(data))).toHaveLength(1); + expect( + pruneConfigBackups(backups, { keepCount: 1, keepDays: 1, now: T0 + 999 * DAY, origin: configBackupOrigin(data) }) + ).toEqual([]); + expect(listConfigBackups(backups)).toHaveLength(2); + const theirId = (theirs as { id: string }).id; + expect(() => restoreConfigBackup({ dataDir: data, backupDir: backups, id: theirId })).toThrow( + ConfigBackupNotFoundError + ); + expect((mine as { id: string }).id).not.toBe(theirId); + }); + + it('a restore reports tracked files it left alone, and sweeps only old temp folders', () => { + put('settings.json', '{"a":1}'); + const made = createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 }) as { id: string }; + put('webhook.json', '{"url":"u"}'); + expect(restoreConfigBackup({ dataDir: data, backupDir: backups, id: made.id }).untouched).toEqual(['webhook.json']); + const stale = join(backups, '.tmp-cfg-20200101-000000-99999'); + mkdirSync(stale); + sweepConfigBackupTemps(backups, Date.now() + 2 * 3_600_000); + expect(existsSync(stale)).toBe(false); + }); +}); diff --git a/test/routes/config-backup-routes.test.ts b/test/routes/config-backup-routes.test.ts index cc76cb35..3d2cceba 100644 --- a/test/routes/config-backup-routes.test.ts +++ b/test/routes/config-backup-routes.test.ts @@ -13,6 +13,7 @@ import { registerConfigBackupRoutes } from '../../src/web/routes/config-backup-r import { SETTINGS_PATH } from '../../src/web/route-helpers.js'; import { getDataDir } from '../../src/config/instance.js'; import { defaultConfigBackupDir } from '../../src/config-backup.js'; +import { intentStore } from '../../src/intent-store.js'; let custom: string; const settings = (extra: Record = {}) => { @@ -84,11 +85,35 @@ describe('/api/config-backups', () => { const d = res.json().data; expect(d.restored).toEqual(expect.arrayContaining(['settings.json', 'clis.json'])); expect(d.restartRequired).toContain('clis.json'); - expect(d.restartRequired).not.toContain('settings.json'); + expect(d.restartRequired).toContain('settings.json'); // its watcher toggles are not re-run by a restore expect(d.safetySnapshot).toBeTruthy(); expect(JSON.parse(readFileSync(SETTINGS_PATH, 'utf8')).displayName).toBe('before'); }); + it('a restored intents.json is not reverted by the next captured prompt', async () => { + const { app } = await createRouteTestHarness(registerConfigBackupRoutes); + const dir = mkdtempSync(join(tmpdir(), 'cfg-backup-intent-')); + try { + intentStore.recordPrompt(undefined, dir, 's1', 'first prompt about the build pipeline'); + const id = (await app.inject({ method: 'POST', url: '/api/config-backups' })).json().data.id; + intentStore.recordPrompt(undefined, dir, 's1', 'second prompt about the build pipeline'); + await app.inject({ method: 'POST', url: `/api/config-backups/${id}/restore` }); + intentStore.recordPrompt(undefined, dir, 's1', 'third prompt about the build pipeline'); + expect(intentStore.getProfile(undefined, dir).recentPrompts).toHaveLength(2); + } finally { + intentStore.deleteProfile(undefined, dir); + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('refuses a custom folder whose parent is missing instead of creating it', async () => { + settings({ configBackupDir: join(custom, 'no', 'such', 'dir') }); + const { app } = await createRouteTestHarness(registerConfigBackupRoutes); + const res = await app.inject({ method: 'POST', url: '/api/config-backups' }); + expect(res.statusCode).toBe(503); + expect(existsSync(join(custom, 'no'))).toBe(false); + }); + it('answers 404 for an unknown backup and 404 for a traversal attempt', async () => { const { app } = await createRouteTestHarness(registerConfigBackupRoutes); for (const id of ['cfg-19990101-000000', '..%2F..%2Fetc', 'photos']) {