mirror of
https://github.com/binaricat/Netcatty.git
synced 2026-10-09 15:58:34 +00:00
db69d5ac39
* fix: harden sync overwrite recovery * refactor: separate backup retention settings * refactor: align backup retention controls * refactor: simplify backup retention card * fix: address PR #720 deep-review findings - Close the cross-window restore race by holding a time-bounded barrier in localStorage during every destructive apply; useAutoSync skips pushes while it's set, preventing a pre-restore snapshot from clobbering just-restored cloud data. - Round-trip startup three-way merges so merged-in local additions actually reach the cloud instead of living only on the device that ran the merge until the next edit. - Upgrade sync signatures from a 64-char ciphertext prefix to full SHA-256 (v3), closing the tail-mutation replay weakness. - Harden the vault-backup IPC: payload size cap, enum-validated reason, sanitized version strings, strict maxCount, concurrent-call mutex, monotonic createdAt to avoid same-ms ordering ties. - Extract the anchor-change decision into a pure module with unit tests covering no-anchor, resource-id drift, and signature mismatch paths. - Capture the protective backup from the pre-apply closure snapshot so it reflects what's being replaced rather than what was imported. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR #720 follow-up review findings Make protective backup abort-on-failure (was best-effort console.error), preserve nested syncedAt in fingerprint, use UTF-8 byte length for size guard, throw on conflict-inspect failure so stale uploads can't leak through, treat unreadable remote as changed, canonical-JSON signature meta, and hold the version stamp on transient backup failures so the retry path still fires. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address second-pass review findings on PR #720 - Hold version-change stamp when payload is non-meaningful (covers the startup vault-rehydrate race where a transient empty snapshot would permanently skip the upgrade backup). - readBackupRecord stat-checks before readFile so an oversized file in the backup dir cannot OOM the renderer on enumeration. - Reject maxBackups input outside 1..100 instead of silently clamping (matches the i18n error copy and the main-process sanitizer bound). - Wrap USE_LOCAL conflict-resolution push in withRestoreBarrier so a concurrent auto-sync in another window cannot interleave. - sha256Hex throws SyncSignatureUnavailableError on missing WebCrypto subtle; createSyncedFileSignature returns null, forcing the unreadable-remote → three-way-merge path instead of a weak length-only pseudo-signature. - Document that array order in normalizePayloadForHash is an invariant enforced by producers, not the hash function. - Drop three-way-merge completion logs from console.log to console.info. - Comment the implicit restore → store-listener refresh chain so future refactors don't silently break the UI reload path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address third-pass review findings on PR #720 Resolves I-3 through I-8 and related cleanup items identified in the deep review. Highlights: - replace setTimeout(0) post-merge round-trip with a direct syncAllProviders call using the already-computed merged payload, removing the React-commit race - resolve the empty-vault confirmation promise on unmount so a mid-dialog window teardown doesn't leak the resolver - retry the version-change backup as hosts/keys hydrate, instead of latching on the first (possibly empty) snapshot - heartbeat-refresh the cross-window restore barrier so long applies cannot expose a post-60s window to concurrent auto-sync - add a diagnostic warning when connected providers hold divergent bases (multi-account configurations) - surface a user-visible "Sync paused" toast when startup inspect fails, replacing the previous silent gate-open - tie-break backup list sort by id when createdAt collides - extract applyProtectedSyncPayload so the main and settings windows cannot drift on restore-barrier / protective-backup handling Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address deep-review findings on PR #720 Deep re-review surfaced six Important issues that survived the prior four review rounds. All are hardened here: - I1: fsync the protective backup file AND its directory before the rename completes, so a system crash between backup creation and the restore it guards cannot leave a torn/zero-length safety net. - I3: persist an apply-in-progress sentinel across the non-atomic localStorage writes in applySyncPayload. A crash mid-apply now surfaces on the next startup (toast + refuse auto-push) instead of silently pushing the half-applied state over an intact cloud copy. - I2: only open the auto-sync gate (remoteCheckDoneRef) when the startup inspect validated cleanly. Add a bounded exponential-backoff retry so a transient inspect failure self-heals instead of wedging auto-sync until restart. - I5: save the sync base BEFORE advancing the per-provider anchor inside uploadToProvider. A renderer crash between the two writes now degrades to "stale anchor forces re-inspect on next run," which re-merges against the fresh base — eliminating the silent base-drift window where a 3rd-device race could misclassify entries. - I6: main process broadcasts a vaultBackups:changed IPC event on every mutation; useLocalVaultBackups subscribes so protective backups created from the main window show up in the Settings backup list without manual refresh. - I4: update PR description + code comment to match the actual (safer) design: auto-sync gate opens on vault init, with hasMeaningfulSyncData + restore barrier preventing empty-push; the version-change backup is best-effort and retries as data hydrates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: serialize startup checkRemoteVersion and stabilize its deps Re-review flagged that checkRemoteVersion's useCallback depended on `config` — a fresh object literal from App.tsx on every render — so the retry effect restarted with attempt=0 on every vault edit and could spawn overlapping in-flight inspect+apply runs. Two concurrent commitRemoteInspection + onApplyPayload calls could race on the apply-in-progress sentinel around interleaved writes. Route `buildPayload`, `config.onApplyPayload`, and `config.startupReady` through refs so checkRemoteVersion's identity no longer churns with unrelated App state. Add an in-flight guard that returns early when a previous invocation is still awaiting the network, closing the same-window re-entry gap that withRestoreBarrier intentionally doesn't cover. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: release in-flight lock on no-connected-provider early return Third-pass review caught that `checkRemoteInFlightRef` was acquired before the `!connectedProvider` check, so that early return leaked the lock and every subsequent retry-timer tick silently no-op'd. Move the acquisition past the early return so the only path that takes the lock reaches the finally-release. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
131 lines
5.4 KiB
JavaScript
131 lines
5.4 KiB
JavaScript
/**
|
|
* syncSignature - Provider-agnostic remote snapshot fingerprint.
|
|
*
|
|
* Stable, order-independent signature of a SyncedFile used by
|
|
* CloudSyncManager to decide whether a remote has changed since we last
|
|
* observed it. Must produce the same value for semantically-identical
|
|
* remotes and a different value for any ciphertext/metadata change.
|
|
*
|
|
* Kept as a plain ESM .js file (JSDoc-typed) so it works seamlessly with
|
|
* both Vite's bundler in the renderer AND Node's `node --test` harness
|
|
* without needing a TypeScript test runner. CloudSyncManager.ts imports
|
|
* it via a normal ESM import.
|
|
*
|
|
* The previous implementation in CloudSyncManager only hashed
|
|
* `[version, updatedAt, deviceId, iv, salt]`. That meant:
|
|
* - a misbehaving adapter could replay those five fields while
|
|
* mutating algorithm/kdf/appVersion and the anchor would treat the
|
|
* remote as unchanged;
|
|
* - deviceId (a field the remote controls) was weighted as strongly
|
|
* as iv/salt;
|
|
* - ciphertext changes with metadata held constant could slip past.
|
|
*
|
|
* v3 hashes the full meta object (sorted for stability) plus the
|
|
* SHA-256 of the full payload ciphertext so any of those mutations flip
|
|
* the anchor. v2 used only a 64-char prefix of the ciphertext, which is
|
|
* easily defeated by an adversary that controls the remote and can
|
|
* tail-mutate while preserving the prefix. v3 is resistant to any
|
|
* ciphertext mutation.
|
|
*
|
|
* Version prefixes are part of the signature string itself (`v3:`) so
|
|
* an older anchor persisted from a previous build will simply never
|
|
* compare equal to a fresh signature from this build, forcing a
|
|
* single-cycle safe re-detection (treated as "remote changed" which
|
|
* triggers three-way merge) rather than a silent mismatch.
|
|
*
|
|
* INVARIANT: `meta` values must be primitives (strings, numbers,
|
|
* booleans, null/undefined). Nested objects or arrays in meta would
|
|
* serialize via JSON.stringify, which does NOT sort keys — breaking
|
|
* signature stability. All current SyncedFile meta fields satisfy this.
|
|
*/
|
|
|
|
/**
|
|
* Sentinel error for a missing WebCrypto subtle digest — see
|
|
* `sha256Hex` and `createSyncedFileSignature` for the fail-closed
|
|
* handling.
|
|
*/
|
|
class SyncSignatureUnavailableError extends Error {
|
|
constructor() {
|
|
super('WebCrypto subtle.digest is unavailable; signature cannot be computed safely.');
|
|
this.name = 'SyncSignatureUnavailableError';
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Compute SHA-256 of a UTF-8 string, returning lowercase hex.
|
|
*
|
|
* Uses `globalThis.crypto.subtle` (Web Crypto API) which is available in
|
|
* both the Electron renderer and Node.js ≥ 19 (the repo's runtime targets
|
|
* both, and CI/tests run under Node). Keeping to the Web Crypto API also
|
|
* avoids pulling `node:crypto` into the renderer bundle.
|
|
*
|
|
* Throws `SyncSignatureUnavailableError` when subtle.digest is missing.
|
|
* Earlier revisions returned a length-only fallback string (`nosha-N`),
|
|
* which would produce a short, truncation-trivial pseudo-signature that
|
|
* an attacker controlling the remote could alias against a legitimate
|
|
* v3 signature of the same length. Failing loudly here lets the caller
|
|
* in `createSyncedFileSignature` return `null`, which routes through
|
|
* the "unreadable remote → treat as changed → three-way merge or
|
|
* surface decrypt error" path — strictly safer than a weak signature.
|
|
*
|
|
* @param {string} input
|
|
* @returns {Promise<string>}
|
|
*/
|
|
async function sha256Hex(input) {
|
|
const subtle = globalThis.crypto?.subtle;
|
|
if (!subtle?.digest) {
|
|
throw new SyncSignatureUnavailableError();
|
|
}
|
|
const bytes = new globalThis.TextEncoder().encode(input);
|
|
const buf = await subtle.digest('SHA-256', bytes);
|
|
const arr = new Uint8Array(buf);
|
|
let hex = '';
|
|
for (let i = 0; i < arr.length; i += 1) {
|
|
hex += arr[i].toString(16).padStart(2, '0');
|
|
}
|
|
return hex;
|
|
}
|
|
|
|
/**
|
|
* @param {import('../../domain/sync').SyncedFile | null} syncedFile
|
|
* @returns {Promise<string | null>}
|
|
*/
|
|
export async function createSyncedFileSignature(syncedFile) {
|
|
if (!syncedFile) return null;
|
|
const { meta, payload } = syncedFile;
|
|
if (!meta || typeof meta !== 'object') return null;
|
|
|
|
// Serialize meta as a canonical JSON object with keys sorted. Earlier
|
|
// versions joined `${key}=${JSON.stringify(...)}` with `|`, which left
|
|
// the `=` separator unescaped: a future meta key containing `=` in its
|
|
// name (or a string value that mimics the separator syntax) could
|
|
// alias with a different key/value pair. JSON.stringify of a sorted
|
|
// plain object is injection-proof because string values are quoted
|
|
// and escaped by the serializer.
|
|
const metaKeys = Object.keys(meta).sort();
|
|
const canonicalMeta = {};
|
|
for (const key of metaKeys) {
|
|
canonicalMeta[key] = meta[key] ?? null;
|
|
}
|
|
const metaSerialized = JSON.stringify(canonicalMeta);
|
|
|
|
const payloadStr = typeof payload === 'string' ? payload : '';
|
|
const payloadLen = payloadStr.length;
|
|
let payloadHash;
|
|
try {
|
|
payloadHash = payloadStr ? await sha256Hex(payloadStr) : 'empty';
|
|
} catch (error) {
|
|
if (error instanceof SyncSignatureUnavailableError) {
|
|
// Fail closed: no signature → decideRemoteChanged's
|
|
// `currentSignature === null` branch treats the remote as
|
|
// "unreadable" and routes through three-way merge. That is the
|
|
// safe behavior vs. a weak pseudo-signature that could silently
|
|
// alias against another payload of the same length.
|
|
return null;
|
|
}
|
|
throw error;
|
|
}
|
|
|
|
return `v3:${metaSerialized}|len=${payloadLen}|sha256=${payloadHash}`;
|
|
}
|