Harden archives, snapshots, locks, and search against corruption and races
Template tests / tests (pull_request) Failing after 33s

Phase 1/2 of the improvement plan (PR 6 of the sequence). Recovery and
resource-limit hardening for the storage-adjacent modules.

ZIP resource limits (core/zip.js):
- unzipSync now enforces entry-count, total compressed, total inflated, and
  per-entry inflated budgets, and caps inflation with inflateRawSync
  maxOutputLength so a deflate bomb can't exhaust memory. Exact inflated-size
  match (not "at least") and CRC verification are kept. Import uses the
  default limits.

Transactional archive import (core/archive.js):
- The import validates the guide AND every step before writing anything, then
  stages the whole guide in a temp directory and publishes it with a single
  atomic rename. A corrupt step no longer leaves a partial guide in the
  library; a failure cleans up the staging directory.

Atomic snapshot restore (core/snapshots.js):
- Restore extracts and validates into a temp directory first; only then does
  it swap content in, moving live content aside so a mid-swap failure rolls
  back. A corrupt/truncated snapshot can no longer destroy the live guide
  (the old restore deleted live content before extracting).
- Fixed snapshot filename collisions: names kept milliseconds so two backups
  in the same second no longer overwrite each other.

Automatic backups (core/snapshots.js):
- Implemented the previously-dead backups.automatic/everyNSaves/keepLast
  settings: autoSnapshotIfDue snapshots every N saves and prunes to keepLast,
  wired into the save choke point in main.js. Never throws — a backup failure
  cannot break the save that triggered it.

Exclusive locks (core/locks.js):
- acquireLock uses O_CREAT|O_EXCL (flag 'wx') so only one writer wins the
  race; the old read-then-write left a window where two writers both believed
  they held the lock. Added a per-acquisition token so release only removes
  the exact lock it took (never one a force-steal replaced). Same-process
  re-acquire still succeeds; cross-process fresh locks conflict.

Search reconciliation (core/search.js):
- New reconcile(store) rebuilds/repairs the index against the library at
  startup using per-guide fingerprints (updatedAt+revision): reindexes new/
  changed guides, drops entries for deleted ones, and exposes a recovery
  status ('ok'|'reset'|'reconciled'). A missing/corrupt/version-mismatched
  index recovers instead of silently returning nothing. Wired into startup.

Recovery surface:
- New recovery:status IPC + preload method returns quarantined files (this
  session) and the search index status so the UI can surface data issues.

Tests: ZIP bomb/limits, transactional import abort with no partial guide,
atomic snapshot restore preserving the live guide on corruption, exclusive
lock conflict/steal/release-by-token, same-process re-acquire, search
reconcile (rebuild/drop/reindex/corrupt-reset), and automatic backup
cadence/pruning. 255 unit tests pass; startup smoke and workflow E2E pass.

Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
2026-07-03 23:10:21 -05:00
co-authored by Claude Fable 5
parent f31f1407a5
commit 8aa9756b8a
8 changed files with 622 additions and 41 deletions
+32 -8
View File
@@ -124,18 +124,40 @@ function importGuideArchive(store, file, { mode = 'copy' } = {}) {
}
function finalizeImport(store, newGuide, idMap, stepJsons, stepFiles) {
// Transactional import: validate the guide and EVERY step first, then write
// the whole guide into a temporary staging directory, and only publish it
// with a single atomic rename. Previously guide.json was written before the
// steps validated, so a bad step left a partial guide in the library.
validateGuide(newGuide);
writeJsonSync(path.join(store.guideDir(newGuide.guideId), 'guide.json'), newGuide);
const normalizedSteps = [];
for (const [stepId, { raw }] of stepJsons) {
const step = normalizeStep({ ...raw, stepId });
step.parentStepId = raw.parentStepId ? idMap.get(raw.parentStepId) || null : null;
validateStep(step);
const dir = store.stepDir(newGuide.guideId, stepId);
writeJsonSync(path.join(dir, 'step.json'), step);
for (const { name, data } of stepFiles.get(stepId) || []) {
atomicWriteFileSync(path.join(dir, name), data);
validateStep(step); // throws before anything is written on a bad step
normalizedSteps.push([stepId, step]);
}
const finalDir = store.guideDir(newGuide.guideId);
if (fs.existsSync(finalDir)) throw new Error(`guide already exists: ${newGuide.guideId}`);
const stagingDir = `${finalDir}.importing-${Date.now()}`;
fs.rmSync(stagingDir, { recursive: true, force: true });
try {
fs.mkdirSync(stagingDir, { recursive: true });
writeJsonSync(path.join(stagingDir, 'guide.json'), newGuide);
for (const [stepId, step] of normalizedSteps) {
const dir = path.join(stagingDir, 'steps', stepId);
fs.mkdirSync(dir, { recursive: true });
writeJsonSync(path.join(dir, 'step.json'), step);
for (const { name, data } of stepFiles.get(stepId) || []) {
atomicWriteFileSync(path.join(dir, name), data);
}
}
// Publish atomically. If the final dir appeared meanwhile, fail cleanly.
if (fs.existsSync(finalDir)) throw new Error(`guide already exists: ${newGuide.guideId}`);
fs.renameSync(stagingDir, finalDir);
} catch (err) {
fs.rmSync(stagingDir, { recursive: true, force: true });
throw err;
}
return store.getGuide(newGuide.guideId);
}
@@ -160,7 +182,9 @@ function saveLinkedGuide(store, guideId, { force = false } = {}) {
store.saveGuide(guide, { touch: false });
return { saved: true, path: target };
} finally {
releaseLock(target);
// Release by our acquisition token so we never remove a lock a concurrent
// force-steal replaced with theirs.
releaseLock(target, { lock: result.lock });
}
}
+56 -10
View File
@@ -20,18 +20,39 @@ function lockPathFor(archivePath) {
return path.join(dir, `${stem}.lock-sfgz`);
}
function currentHolder() {
function currentProcess() {
return { host: os.hostname(), user: os.userInfo().username, pid: process.pid };
}
function currentHolder() {
return {
...currentProcess(),
// Random per-acquisition token so two processes that happen to share
// host+user+pid space (containers, pid reuse) still compare distinctly,
// and so a steal can be detected by the previous holder.
token: `${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}`,
};
}
function readLock(archivePath) {
return readJsonIfExists(lockPathFor(archivePath), null);
}
function sameHolder(a, b) {
// Process identity (host+user+pid). Used to decide whether an existing lock is
// held by *this process* (safe to re-acquire) or someone else (a conflict).
function sameProcess(a, b) {
return a && b && a.host === b.host && a.user === b.user && a.pid === b.pid;
}
// Exact-acquisition identity via the per-acquisition token. Used by release so
// a caller only removes the lock it actually took (never one a force-steal
// replaced with its own).
function sameAcquisition(existing, owner) {
if (!existing || !owner) return false;
if (owner.token) return existing.token === owner.token;
return sameProcess(existing, owner);
}
function isStale(lock, now = Date.now()) {
const t = Date.parse(lock && lock.acquiredAt);
return !Number.isFinite(t) || now - t > STALE_AFTER_MS;
@@ -44,24 +65,49 @@ function isStale(lock, now = Date.now()) {
*/
function acquireLock(archivePath, { force = false } = {}) {
const file = lockPathFor(archivePath);
const existing = readLock(archivePath);
const me = currentHolder();
if (existing && !sameHolder(existing, me) && !isStale(existing) && !force) {
const lock = { ...me, acquiredAt: nowIso() };
const payload = JSON.stringify(lock, null, 2);
// Fast path: exclusive create. Only one writer wins the O_CREAT|O_EXCL race,
// so two processes can't both believe they hold the lock (the old
// read-then-write left exactly that window open).
try {
fs.writeFileSync(file, payload, { flag: 'wx' });
return { acquired: true, lock };
} catch (err) {
if (err.code !== 'EEXIST') throw err;
}
// A lock already exists. We may take it over only if this process already
// holds it, it is stale, or the caller is force-stealing (user confirmed).
const existing = readLock(archivePath);
if (existing && !sameProcess(existing, me) && !isStale(existing) && !force) {
return { acquired: false, conflict: existing };
}
const lock = { ...me, acquiredAt: nowIso() };
fs.writeFileSync(file, JSON.stringify(lock, null, 2));
// Overwrite to claim ownership (our token now identifies the lock).
fs.writeFileSync(file, payload);
return { acquired: true, lock };
}
/** Release only if we are the holder (or force). */
function releaseLock(archivePath, { force = false } = {}) {
/**
* Release only if we are the holder (or force). Pass the `lock` (or its
* `token`) returned by acquireLock so ownership is matched by token — the
* per-acquisition token means a fresh currentHolder() would not match.
*/
function releaseLock(archivePath, { force = false, lock = null, token = null } = {}) {
const file = lockPathFor(archivePath);
const existing = readLock(archivePath);
if (!existing) return true;
if (!force && !sameHolder(existing, currentHolder())) return false;
// With no explicit lock/token, fall back to process identity (the legacy
// "release my own lock" path) rather than a fresh token that can't match.
const owner = lock || (token ? { token } : currentProcess());
if (!force && !sameAcquisition(existing, owner)) return false;
fs.rmSync(file, { force: true });
return true;
}
module.exports = { lockPathFor, readLock, acquireLock, releaseLock, isStale, STALE_AFTER_MS };
module.exports = {
lockPathFor, readLock, acquireLock, releaseLock, isStale, STALE_AFTER_MS,
sameProcess, sameAcquisition,
};
+69 -5
View File
@@ -14,7 +14,7 @@ const { blockText } = require('./blocks');
* specific step in the editor.
*/
const INDEX_VERSION = 1;
const INDEX_VERSION = 2;
function tokenize(text) {
if (!text) return [];
@@ -27,20 +27,82 @@ function tokenize(text) {
class SearchIndex {
constructor(indexDir) {
this.file = path.join(indexDir, 'search-index.json');
// Per-guide source fingerprints so a startup reconcile can tell which
// guides changed while the app was closed, without re-reading every step.
this.fingerprints = {}; // guideId -> fingerprint string
// Recovery status surfaced to the UI: 'ok' | 'reset' (missing/corrupt/
// version mismatch) | 'reconciled' (rebuilt from the store at startup).
this.status = 'ok';
const fileExisted = require('node:fs').existsSync(this.file);
const stored = readJsonIfExists(this.file, null);
if (stored && stored.version === INDEX_VERSION) {
if (stored && stored.version === INDEX_VERSION && stored.docs && typeof stored.docs === 'object') {
this.docs = stored.docs;
this.fingerprints = stored.fingerprints || {};
} else {
// Missing, corrupt, or an older index version: start empty and mark it,
// so reconcile() rebuilds from the store instead of silently staying
// blank (which made search "work" but return nothing). A file that
// existed but could not be used is a 'reset' (recovery-worthy); a
// genuinely absent index on first run is just 'ok'.
this.docs = {}; // docKey -> { guideId, stepId, title, text, updatedAt }
this.status = fileExisted ? 'reset' : 'ok';
}
}
persist() {
writeJsonSync(this.file, { version: INDEX_VERSION, docs: this.docs });
writeJsonSync(this.file, {
version: INDEX_VERSION,
docs: this.docs,
fingerprints: this.fingerprints,
});
}
static fingerprint(guide) {
return `${guide.updatedAt || ''}:${Number.isInteger(guide.revision) ? guide.revision : 0}`;
}
/**
* Reconcile the index against the store at startup: reindex guides that are
* new or changed (by fingerprint), and drop index entries for guides that no
* longer exist. Returns a summary with a recovery status for the UI.
*/
reconcile(store) {
const guides = store.listGuides();
const liveIds = new Set(guides.map((g) => g.guideId));
let reindexed = 0;
let removed = 0;
// Drop docs/fingerprints for guides that are gone.
for (const key of Object.keys(this.fingerprints)) {
if (!liveIds.has(key)) {
this.removeGuide(key, { persist: false });
delete this.fingerprints[key];
removed += 1;
}
}
for (const guide of guides) {
const fp = SearchIndex.fingerprint(guide);
const indexed = this.fingerprints[guide.guideId];
const hasDoc = Boolean(this.docs[`g:${guide.guideId}`]);
if (indexed === fp && hasDoc) continue; // unchanged
try {
this.indexGuide(guide, store.listSteps(guide.guideId), { persist: false });
reindexed += 1;
} catch {
// A single unreadable guide must not abort the whole reconcile.
}
}
this.persist();
if (this.status === 'reset' || reindexed > 0 || removed > 0) {
this.status = this.status === 'reset' ? 'reset' : 'reconciled';
}
return { status: this.status, reindexed, removed, total: guides.length };
}
/** (Re)index one guide and all of its steps. */
indexGuide(guide, stepsMap) {
indexGuide(guide, stepsMap, { persist = true } = {}) {
this.removeGuide(guide.guideId, { persist: false });
const placeholderText = Object.entries(guide.placeholders || {})
@@ -69,13 +131,15 @@ class SearchIndex {
updatedAt: guide.updatedAt,
};
}
this.persist();
this.fingerprints[guide.guideId] = SearchIndex.fingerprint(guide);
if (persist) this.persist();
}
removeGuide(guideId, { persist = true } = {}) {
for (const key of Object.keys(this.docs)) {
if (this.docs[key].guideId === guideId) delete this.docs[key];
}
delete this.fingerprints[guideId];
if (persist) this.persist();
}
+97 -9
View File
@@ -3,7 +3,8 @@
const fs = require('node:fs');
const path = require('node:path');
const { zipDirSync, extractZipSync } = require('./zip');
const { atomicWriteFileSync } = require('./util');
const { atomicWriteFileSync, readJsonSync } = require('./util');
const { validateGuide } = require('./schema');
/**
* Snapshot backups: a zip of the guide directory (excluding history/) stored
@@ -16,7 +17,11 @@ function snapshotsDir(store, guideId) {
}
function snapshotName(label) {
const stamp = new Date().toISOString().replace(/[:.]/g, '-').replace(/-\d{3}Z$/, 'Z');
// Keep milliseconds: stripping them made two snapshots taken within the same
// second collide on filename (the second silently overwrote the first, so
// rapid automatic backups produced only one file). ms keeps names unique and
// still chronologically sortable.
const stamp = new Date().toISOString().replace(/[:.]/g, '-');
return label ? `${stamp}-${label.replace(/[^A-Za-z0-9_-]+/g, '_')}.zip` : `${stamp}.zip`;
}
@@ -50,20 +55,103 @@ function pruneSnapshots(store, guideId, keepLast) {
/**
* Restore a snapshot: replaces the guide's current content (guide.json and
* steps/) with the snapshot's, keeping the history/ directory intact.
*
* The extraction is staged and validated BEFORE any live content is touched:
* a corrupt or truncated snapshot can no longer destroy the current guide.
* The swap itself moves the old content aside, moves the new content in, then
* deletes the old — so a failure mid-swap leaves a recoverable state.
*/
function restoreSnapshot(store, guideId, name) {
const file = path.join(snapshotsDir(store, guideId), path.basename(name));
if (!fs.existsSync(file)) throw new Error(`snapshot not found: ${name}`);
const buf = fs.readFileSync(file);
const guideDir = store.guideDir(guideId);
// Safety: snapshot the pre-restore state too, so a restore is undoable.
createSnapshot(store, guideId, { label: 'pre-restore' });
for (const entry of fs.readdirSync(guideDir)) {
if (entry === 'history') continue;
fs.rmSync(path.join(guideDir, entry), { recursive: true, force: true });
// 1. Extract + validate into a temp staging dir. Nothing live is touched yet.
const staging = `${guideDir}.restoring-${Date.now()}`;
fs.rmSync(staging, { recursive: true, force: true });
try {
fs.mkdirSync(staging, { recursive: true });
extractZipSync(buf, staging);
const guideJson = path.join(staging, 'guide.json');
if (!fs.existsSync(guideJson)) throw new Error('snapshot is missing guide.json');
validateGuide(readJsonSync(guideJson)); // throws on a corrupt snapshot
} catch (err) {
fs.rmSync(staging, { recursive: true, force: true });
throw new Error(`snapshot restore aborted (snapshot invalid): ${err.message}`);
}
extractZipSync(buf, guideDir);
// 2. Snapshot the pre-restore state so the restore is itself undoable.
createSnapshot(store, guideId, { label: 'pre-restore' });
// 3. Swap in the validated content, preserving history/. Move live content
// aside first so we can roll back if a step fails.
const backup = `${guideDir}.prev-${Date.now()}`;
const liveEntries = fs.readdirSync(guideDir).filter((e) => e !== 'history');
fs.mkdirSync(backup, { recursive: true });
try {
for (const entry of liveEntries) {
fs.renameSync(path.join(guideDir, entry), path.join(backup, entry));
}
for (const entry of fs.readdirSync(staging)) {
if (entry === 'history') continue;
fs.renameSync(path.join(staging, entry), path.join(guideDir, entry));
}
} catch (err) {
// Roll back: restore whatever we moved aside.
for (const entry of fs.readdirSync(backup)) {
const dest = path.join(guideDir, entry);
fs.rmSync(dest, { recursive: true, force: true });
fs.renameSync(path.join(backup, entry), dest);
}
fs.rmSync(backup, { recursive: true, force: true });
fs.rmSync(staging, { recursive: true, force: true });
throw err;
}
fs.rmSync(backup, { recursive: true, force: true });
fs.rmSync(staging, { recursive: true, force: true });
return store.getGuide(guideId);
}
module.exports = { createSnapshot, listSnapshots, pruneSnapshots, restoreSnapshot, snapshotsDir };
/**
* Automatic backup policy. Every guide keeps a small save counter in its
* history dir; once `everyNSaves` saves accumulate (and backups.automatic is
* on) an automatic snapshot is taken and old ones pruned to backups.keepLast.
* Returns the snapshot name when one was taken, else null. Never throws — a
* backup failure must not break the save that triggered it.
*/
function autoSnapshotIfDue(store, guideId, settings) {
try {
const backups = (settings && settings.get && settings.get('backups')) || {};
if (backups.automatic === false) return null;
const everyN = Number.isInteger(backups.everyNSaves) && backups.everyNSaves > 0 ? backups.everyNSaves : 25;
const keepLast = Number.isInteger(backups.keepLast) && backups.keepLast > 0 ? backups.keepLast : 10;
const dir = path.join(store.guideDir(guideId), 'history');
fs.mkdirSync(dir, { recursive: true });
const counterFile = path.join(dir, 'autosave-counter.json');
let count = 0;
try {
count = JSON.parse(fs.readFileSync(counterFile, 'utf8')).count || 0;
} catch { count = 0; }
count += 1;
if (count >= everyN) {
createSnapshot(store, guideId, { label: 'auto', keepLast });
count = 0;
atomicWriteFileSync(counterFile, JSON.stringify({ count }));
return true;
}
atomicWriteFileSync(counterFile, JSON.stringify({ count }));
return null;
} catch (err) {
// Best effort: report, never break the caller's save.
console.error(`[stepforge] automatic backup failed for ${guideId}: ${err && err.message}`);
return null;
}
}
module.exports = {
createSnapshot, listSnapshots, pruneSnapshots, restoreSnapshot, snapshotsDir,
autoSnapshotIfDue,
};
+44 -8
View File
@@ -121,8 +121,22 @@ function zipSync(entries, { date = new Date(2026, 0, 1) } = {}) {
return Buffer.concat([...localParts, centralBuf, eocd]);
}
/** Parse a zip buffer into [{ name, data }] with CRC verification. */
function unzipSync(buffer) {
// Resource limits for untrusted archives (share files, snapshots). These cap
// memory and disk work so a ZIP bomb can't exhaust the machine. Callers that
// build archives themselves may relax them; imports use the defaults.
const DEFAULT_UNZIP_LIMITS = {
maxEntries: 50000,
maxTotalCompressed: 1024 * 1024 * 1024, // 1 GiB of stored bytes
maxTotalUncompressed: 4 * 1024 * 1024 * 1024, // 4 GiB inflated total
maxEntryUncompressed: 512 * 1024 * 1024, // 512 MiB per entry
};
/**
* Parse a zip buffer into [{ name, data }] with CRC verification and hard
* resource limits. `limits` overrides DEFAULT_UNZIP_LIMITS.
*/
function unzipSync(buffer, { limits = {} } = {}) {
const lim = { ...DEFAULT_UNZIP_LIMITS, ...limits };
if (!Buffer.isBuffer(buffer) || buffer.length < 22) throw new Error('zip: too small');
// Find end-of-central-directory record (scan backwards over the comment).
let eocd = -1;
@@ -132,11 +146,14 @@ function unzipSync(buffer) {
}
if (eocd < 0) throw new Error('zip: end record not found');
const count = buffer.readUInt16LE(eocd + 10);
if (count > lim.maxEntries) throw new Error(`zip: too many entries (${count} > ${lim.maxEntries})`);
let pos = buffer.readUInt32LE(eocd + 16);
const entries = [];
let totalCompressed = 0;
let totalUncompressed = 0;
for (let i = 0; i < count; i++) {
if (buffer.readUInt32LE(pos) !== 0x02014b50) throw new Error('zip: bad central header');
if (pos + 46 > buffer.length || buffer.readUInt32LE(pos) !== 0x02014b50) throw new Error('zip: bad central header');
const method = buffer.readUInt16LE(pos + 10);
const crc = buffer.readUInt32LE(pos + 16);
const compSize = buffer.readUInt32LE(pos + 20);
@@ -151,17 +168,33 @@ function unzipSync(buffer) {
assertSafeEntryName(name);
if (name.endsWith('/')) continue; // directory entry
// Budget checks BEFORE allocating/inflating: the declared sizes are
// attacker-controlled, so reject oversize claims up front.
if (uncompSize > lim.maxEntryUncompressed) {
throw new Error(`zip: entry too large (${uncompSize} > ${lim.maxEntryUncompressed}): ${name}`);
}
totalCompressed += compSize;
totalUncompressed += uncompSize;
if (totalCompressed > lim.maxTotalCompressed) throw new Error('zip: total compressed size exceeds limit');
if (totalUncompressed > lim.maxTotalUncompressed) throw new Error('zip: total inflated size exceeds limit');
if (buffer.readUInt32LE(localOffset) !== 0x04034b50) throw new Error('zip: bad local header');
const lNameLen = buffer.readUInt16LE(localOffset + 26);
const lExtraLen = buffer.readUInt16LE(localOffset + 28);
const dataStart = localOffset + 30 + lNameLen + lExtraLen;
if (dataStart + compSize > buffer.length) throw new Error(`zip: entry data out of range: ${name}`);
const raw = buffer.subarray(dataStart, dataStart + compSize);
let data;
if (method === 0) data = Buffer.from(raw);
else if (method === 8) data = zlib.inflateRawSync(raw);
else throw new Error(`zip: unsupported method ${method} for ${name}`);
else if (method === 8) {
// Cap inflation so a small deflate stream can't expand to gigabytes —
// even if the declared uncompSize lied, this is the real guard.
data = zlib.inflateRawSync(raw, { maxOutputLength: lim.maxEntryUncompressed });
} else throw new Error(`zip: unsupported method ${method} for ${name}`);
// Exact length match (not "at least"): the inflated bytes must equal the
// declared uncompressed size, and the CRC must verify.
if (data.length !== uncompSize) throw new Error(`zip: size mismatch for ${name}`);
if (crc32(data) !== crc) throw new Error(`zip: CRC mismatch for ${name}`);
entries.push({ name, data });
@@ -170,10 +203,10 @@ function unzipSync(buffer) {
}
/** Extract a zip buffer under destDir; every path is traversal-checked. */
function extractZipSync(buffer, destDir) {
function extractZipSync(buffer, destDir, { limits = {} } = {}) {
const resolvedDest = path.resolve(destDir);
const written = [];
for (const { name, data } of unzipSync(buffer)) {
for (const { name, data } of unzipSync(buffer, { limits })) {
const target = path.resolve(resolvedDest, name);
if (target !== resolvedDest && !target.startsWith(resolvedDest + path.sep)) {
throw new Error(`zip: entry escapes destination: ${name}`);
@@ -203,4 +236,7 @@ function zipDirSync(dir, { filter = () => true, prefix = '' } = {}) {
return zipSync(entries);
}
module.exports = { crc32, zipSync, unzipSync, extractZipSync, zipDirSync, assertSafeEntryName };
module.exports = {
crc32, zipSync, unzipSync, extractZipSync, zipDirSync, assertSafeEntryName,
DEFAULT_UNZIP_LIMITS,
};