diff --git a/docs/wiki/Config-Backups.md b/docs/wiki/Config-Backups.md index e743327c..f3dd9962 100644 --- a/docs/wiki/Config-Backups.md +++ b/docs/wiki/Config-Backups.md @@ -138,7 +138,10 @@ Only the tracked names are ever written, whatever a manifest lists. Restoring `settings.json` keeps the four backup settings (on or off, folder, how many, how many days) at their current values, so a restore from before a folder change does not point the list -at the old folder and hide the `pre-restore` copy it just made. +at the old folder and hide the `pre-restore` copy it just made. The exception is a live +`settings.json` that has none of the four (a reset file): the restore then keeps the backup's own +how-many and how-many-days, so the next pass does not fall back to 20 copies and 30 days. The +`pre-restore` copy records the retention in force too. Settings are re-read on the next request, so reload the page. `.env` and `push-keys.json` are read once per process: **restart Codeman** for those to take effect (the response lists which). @@ -191,7 +194,11 @@ query and fragment. Choose a private location, or turn the feature off if you cannot. - **Retention survives a bad `settings.json`.** If it does not parse, a pass still takes its snapshot but deletes nothing. If the four backup settings are missing (a reset file), the pass - prunes with the retention the newest backup recorded, not the 20 copies and 30 days defaults. + prunes with the retention the newest backup that recorded one holds, not the 20 copies and 30 + days defaults, and Settings shows those values until you save. While `settings.json` does not + parse, a pass reads it as empty, so its snapshot goes to the default folder even if a custom + folder is set or backups are switched off; nothing is widened (same modes, inside the data + directory). - **Anyone who can write to a custom backup folder can plant a backup that Restore applies.** Restore refuses symlinks and files others can write, but a file of your own user's making passes, so keep the folder private. diff --git a/src/config-backup.ts b/src/config-backup.ts index 9912950e..c641af37 100644 --- a/src/config-backup.ts +++ b/src/config-backup.ts @@ -404,11 +404,30 @@ export function sweepConfigBackupTemps(dir: string, now: number = Date.now()): v /** The retention recorded in this instance's newest snapshot, or null when it recorded none. */ export function latestBackupRetention(dir: string, origin: string): { keepCount: number; keepDays: number } | null { - const [newest] = listConfigBackups(dir, origin); - const m = newest ? readManifest(dir, newest.id, origin) : null; - return m && typeof m.keepCount === 'number' && typeof m.keepDays === 'number' - ? { keepCount: m.keepCount, keepDays: m.keepDays } - : null; + // The newest manifest that recorded one: a snapshot from before this field existed must not hide it. + for (const b of listConfigBackups(dir, origin)) { + const m = readManifest(dir, b.id, origin); + if (m && typeof m.keepCount === 'number' && typeof m.keepDays === 'number') { + return { keepCount: m.keepCount, keepDays: m.keepDays }; + } + } + return null; +} + +/** + * The settings in effect when `raw` (the parsed settings.json) has none of the four `configBackup*` + * keys, as after a reset: the retention the newest backup recorded instead of the defaults. One + * place for the timer pass and for what the GET route reports, so they cannot disagree. + */ +export function withRecordedRetention( + settings: ConfigBackupSettings, + raw: Record, + dir: string, + origin: string | null +): ConfigBackupSettings { + if (hasConfigBackupSettings(raw) || !origin) return settings; + const kept = latestBackupRetention(dir, origin); + return kept ? { ...settings, ...kept } : settings; } export interface PruneOptions { @@ -473,19 +492,25 @@ function assertTrustedSnapshotEntry(path: string, label: string, checkMode: bool * Whether the backup folder's filesystem keeps a file's mode: vfat/exFAT, CIFS shares and WSL's * `/mnt/d` report 0755 or 0777 for everything, so the group/world test would refuse every backup * there even though snapshots succeed (`createConfigBackup` already expects mode-less filesystems). - * Probed with a 0600 temp file; if the probe cannot run, the strict test stays on. + * Probed with a 0600 temp file made with `wx` under a random name; if the probe cannot run, the + * strict test stays on. */ function folderKeepsModes(dir: string): boolean { if (process.platform === 'win32') return false; - const probe = join(dir, `.mode-probe-${process.pid}`); + // A random name and 'wx' (fail if anything is there): a planted symlink or directory with a + // guessable name must neither be written through nor break the probe. Only a file this call + // created is removed. + const probe = join(dir, `.mode-probe-${randomUUID()}`); + let created = false; try { - writeFileSync(probe, '', { mode: 0o600 }); + writeFileSync(probe, '', { mode: 0o600, flag: 'wx' }); + created = true; chmodSync(probe, 0o600); return (lstatSync(probe).mode & 0o777 & 0o077) === 0; } catch { return true; } finally { - rmSync(probe, { force: true }); + if (created) rmSync(probe, { force: true }); } } @@ -498,7 +523,11 @@ function keepCurrentBackupSettings(snapshot: Buffer, livePath: string): Buffer { const live = JSON.parse(readFileSync(livePath, 'utf8')) as Record; if (!restored || typeof restored !== 'object' || !live || typeof live !== 'object') return snapshot; let changed = false; + // A reset live file has none of the four keys. Taking "absent" for the retention too would bring + // the defaults back (20 copies, 30 days) on the next pass, so the backup's own retention stays. + const liveHasSettings = hasConfigBackupSettings(live); for (const key of BACKUP_SETTING_KEYS) { + if (!liveHasSettings && (key === 'configBackupKeepCount' || key === 'configBackupKeepDays')) continue; if (JSON.stringify(restored[key]) === JSON.stringify(live[key])) continue; changed = true; if (key in live) restored[key] = live[key]; @@ -535,7 +564,7 @@ export function restoreConfigBackup(opts: { }): RestoreResult { const origin = readConfigBackupOrigin(opts.dataDir); const manifest = origin ? readManifest(opts.backupDir, opts.id, origin) : null; - if (!manifest) throw new ConfigBackupNotFoundError(`No config backup named ${opts.id}`); + if (!manifest || !origin) throw new ConfigBackupNotFoundError(`No config backup named ${opts.id}`); const allowed = new Set(CONFIG_BACKUP_FILES); const loaded: Array<{ name: string; data: Buffer }> = []; @@ -563,6 +592,8 @@ export function restoreConfigBackup(opts: { appVersion: opts.appVersion, force: true, now: opts.now, + // The retention in force, so this newest snapshot does not hide it from the next pass. + ...(latestBackupRetention(opts.backupDir, origin) ?? {}), }); mkdirSync(opts.dataDir, { recursive: true }); diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 30a475b8..03d3e239 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -1388,6 +1388,13 @@ Object.assign(CodemanApp.prototype, { const data = body.data; const dirInput = this.$('appSettingsConfigBackupDir') || document.getElementById('appSettingsConfigBackupDir'); if (dirInput && data.defaultDir) dirInput.placeholder = `default: ${data.defaultDir}`; + // A reset settings.json has none of the four keys, but the passes keep the retention the newest backup + // recorded: show that, or a Save would write 20 / 30 over it. + const saved = this.loadAppSettingsFromStorage(); + const countInput = document.getElementById('appSettingsConfigBackupKeepCount'); + const daysInput = document.getElementById('appSettingsConfigBackupKeepDays'); + if (countInput && saved.configBackupKeepCount === undefined && data.settings) countInput.value = data.settings.keepCount; + if (daysInput && saved.configBackupKeepDays === undefined && data.settings) daysInput.value = data.settings.keepDays; if (!data.backups.length) { out.innerHTML = `No backups yet in ${escapeHtml(data.dir)}.`; return; diff --git a/src/web/routes/config-backup-routes.ts b/src/web/routes/config-backup-routes.ts index 1a1cc3e4..602e0673 100644 --- a/src/web/routes/config-backup-routes.ts +++ b/src/web/routes/config-backup-routes.ts @@ -40,8 +40,7 @@ import { resolveConfigBackupDir, restoreConfigBackup, configBackupOrigin, - hasConfigBackupSettings, - latestBackupRetention, + withRecordedRetention, readConfigBackupOrigin, sweepConfigBackupTemps, type ConfigBackupResult, @@ -73,8 +72,10 @@ export interface ConfigBackupStatus { export async function readEffectiveBackupSettings(): Promise<{ settings: ConfigBackupSettings; dir: string }> { const raw = await readJsonConfig>(SETTINGS_PATH, 'settings.json', {}); - const settings = readConfigBackupSettings(raw); - return { settings, dir: resolveConfigBackupDir(settings, getDataDir()) }; + const base = readConfigBackupSettings(raw); + const dir = resolveConfigBackupDir(base, getDataDir()); + // The same fallback the timer pass uses, so the form never shows 20 / 30 while a pass keeps 100 / 0. + return { settings: withRecordedRetention(base, raw, dir, readConfigBackupOrigin(getDataDir())), dir }; } /** A user-chosen backup folder that is not safe to touch (bounded probe, as for case paths, #516). */ @@ -128,10 +129,7 @@ export async function runConfigBackup( sweepConfigBackupTemps(dir); const origin = configBackupOrigin(getDataDir()); // No backup settings in the file (reset, or unreadable): keep the retention the newest snapshot recorded. - const settings = - unreadable || !hasConfigBackupSettings(raw) - ? { ...fromFile, ...(latestBackupRetention(dir, origin) ?? {}) } - : fromFile; + const settings = withRecordedRetention(fromFile, unreadable ? {} : raw, dir, origin); const result = createConfigBackup({ dataDir: getDataDir(), backupDir: dir, diff --git a/test/config-backup-restore-ui.test.ts b/test/config-backup-restore-ui.test.ts index 3e79e972..256e4479 100644 --- a/test/config-backup-restore-ui.test.ts +++ b/test/config-backup-restore-ui.test.ts @@ -62,3 +62,47 @@ describe('restoreConfigBackup (Settings UI)', () => { expect(confirms[0]).toMatch(/users\.json, \.env\)/); }); }); + +describe('loadConfigBackups (retention fields)', () => { + function load(saved: Record) { + const inputs: Record = { + appSettingsConfigBackupKeepCount: { value: '20', placeholder: '' }, + appSettingsConfigBackupKeepDays: { value: '30', placeholder: '' }, + appSettingsConfigBackupDir: { value: '', placeholder: '' }, + }; + function CodemanApp(this: unknown) {} + const ctx = vm.createContext({ + CodemanApp, + window: {}, + document: { getElementById: (id: string) => inputs[id] ?? null }, + escapeHtml: (s: string) => s, + }); + vm.runInContext(SRC, ctx); + const app = Object.create((CodemanApp as unknown as { prototype: object }).prototype) as Record; + Object.assign(app, { + $: () => ({ textContent: '', innerHTML: '' }), + loadAppSettingsFromStorage: () => saved, + _api: async () => ({ + ok: true, + json: async () => ({ + success: true, + data: { settings: { keepCount: 100, keepDays: 0 }, dir: '/d', defaultDir: '/d', backups: [] }, + }), + }), + }); + return { app, inputs }; + } + + it('shows the retention the passes use when settings.json has none of the keys (a reset file)', async () => { + const { app, inputs } = load({}); + await (app.loadConfigBackups as () => Promise)(); + expect(inputs.appSettingsConfigBackupKeepCount.value).toBe(100); + expect(inputs.appSettingsConfigBackupKeepDays.value).toBe(0); + }); + + it('leaves the form alone when the saved settings state the retention', async () => { + const { app, inputs } = load({ configBackupKeepCount: 7, configBackupKeepDays: 9 }); + await (app.loadConfigBackups as () => Promise)(); + expect(inputs.appSettingsConfigBackupKeepCount.value).toBe('20'); // the open-settings code owns it + }); +}); diff --git a/test/config-backup.test.ts b/test/config-backup.test.ts index fe56474f..a045939e 100644 --- a/test/config-backup.test.ts +++ b/test/config-backup.test.ts @@ -32,6 +32,7 @@ import { resolveConfigBackupDir, restoreConfigBackup, configBackupOrigin, + latestBackupRetention, sweepConfigBackupTemps, } from '../src/config-backup.js'; @@ -541,6 +542,19 @@ describe('restore trusts only what this module wrote', () => { expect(() => restoreConfigBackup({ dataDir: data, backupDir: backups, id, keepsModes: false })).toThrow(/symlink/); }); + it('the mode probe never writes through a planted symlink or trips over a planted directory', () => { + put('settings.json', '{"a":1}'); + const { id } = made(); + const target = join(root, 'victim.txt'); + writeFileSync(target, 'precious', { mode: 0o644 }); + symlinkSync(target, join(backups, `.mode-probe-${process.pid}`)); + mkdirSync(join(backups, '.mode-probe-0')); + expect(() => restoreConfigBackup({ dataDir: data, backupDir: backups, id })).not.toThrow(); + expect(() => diffConfigBackups({ dataDir: data, backupDir: backups, from: id, to: 'current' })).not.toThrow(); + expect(readFileSync(target, 'utf8')).toBe('precious'); + expect(statSync(target).mode & 0o777).toBe(0o644); + }); + it('compare applies the same check to a snapshot file', () => { put('settings.json', '{"a":1}'); const { id } = made(); @@ -559,6 +573,35 @@ describe('restore trusts only what this module wrote', () => { expect(() => restoreConfigBackup({ dataDir: data, backupDir: backups, id })).not.toThrow(); }); + it('keeps the backup retention when the live settings.json was reset and has none of the four keys', () => { + put('settings.json', JSON.stringify({ displayName: 'old', configBackupKeepCount: 100, configBackupKeepDays: 0 })); + const { id } = made(); + put('settings.json', '{}'); + restoreConfigBackup({ dataDir: data, backupDir: backups, id }); + expect(JSON.parse(readFileSync(join(data, 'settings.json'), 'utf8'))).toEqual({ + displayName: 'old', + configBackupKeepCount: 100, + configBackupKeepDays: 0, + }); + }); + + it('latestBackupRetention skips a newer snapshot that recorded none, and a pre-restore copy records it', () => { + put('settings.json', '{"a":1}'); + createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0, keepCount: 100, keepDays: 0 }); + put('settings.json', '{"a":2}'); + createConfigBackup({ dataDir: data, backupDir: backups, reason: 'auto', now: T0 + 1000 }); // an older build + const origin = configBackupOrigin(data); + expect(latestBackupRetention(backups, origin)).toEqual({ keepCount: 100, keepDays: 0 }); + const id = listConfigBackups(backups, origin).at(-1)!.id; + const r = restoreConfigBackup({ dataDir: data, backupDir: backups, id, now: T0 + 2000 }); + expect(r.safetySnapshot).toBeTruthy(); + const pre = listConfigBackups(backups, origin).find((b) => b.id === r.safetySnapshot)!; + expect(JSON.parse(readFileSync(join(backups, pre.id, 'manifest.json'), 'utf8'))).toMatchObject({ + keepCount: 100, + keepDays: 0, + }); + }); + it('keeps the current folder and retention when settings.json is restored', () => { put('settings.json', JSON.stringify({ displayName: 'old', configBackupDir: '/old', configBackupKeepCount: 5 })); const { id } = made(); diff --git a/test/routes/config-backup-routes.test.ts b/test/routes/config-backup-routes.test.ts index 1dc1704b..a87d71be 100644 --- a/test/routes/config-backup-routes.test.ts +++ b/test/routes/config-backup-routes.test.ts @@ -189,12 +189,16 @@ describe('retention survives an unreadable or reset settings.json', () => { const BASE = Date.UTC(2025, 0, 1); const dir = () => defaultConfigBackupDir(getDataDir()); /** 30 snapshots a day apart, made under a long-history retention (100 copies, no age limit). */ - function seed() { + function seed(withKeys = true) { settings({ configBackupKeepCount: 100, configBackupKeepDays: 0 }); for (let i = 0; i < 30; i++) { writeFileSync( SETTINGS_PATH, - JSON.stringify({ displayName: `v${i}`, configBackupKeepCount: 100, configBackupKeepDays: 0 }) + JSON.stringify( + withKeys + ? { displayName: `v${i}`, configBackupKeepCount: 100, configBackupKeepDays: 0 } + : { displayName: `v${i}` } + ) ); createConfigBackup({ dataDir: getDataDir(), @@ -226,6 +230,26 @@ describe('retention survives an unreadable or reset settings.json', () => { expect(listConfigBackups(dir())).toHaveLength(31); }); + it.each([true, false])( + 'a restore after a reset keeps the retention (the backup %s the four keys)', + async (withKeys) => { + seed(withKeys); + const oldest = listConfigBackups(dir()).at(-1)!.id; + writeFileSync(SETTINGS_PATH, '{}'); // the reset + expect((await runConfigBackup('auto'))?.removed).toEqual([]); + + const { app } = await createRouteTestHarness(registerConfigBackupRoutes); + const res = await app.inject({ method: 'POST', url: `/api/config-backups/${oldest}/restore` }); + expect(res.statusCode).toBe(200); + // the restored file carries the retention, or the recorded one still applies via the pre-restore copy + const out = await runConfigBackup('auto'); + expect(out?.removed).toEqual([]); + expect(listConfigBackups(dir()).length).toBeGreaterThanOrEqual(32); + const status = (await app.inject({ method: 'GET', url: '/api/config-backups' })).json().data.settings; + expect(status).toMatchObject({ keepCount: 100, keepDays: 0 }); // what the form shows is what the pass uses + } + ); + it('a settings.json that sets the retention still wins', async () => { seed(); writeFileSync(SETTINGS_PATH, JSON.stringify({ configBackupKeepCount: 5, configBackupKeepDays: 0 }));