Files
Netcatty/infrastructure/services/syncAnchorDecision.js
T
陈大猫 db69d5ac39 [codex] Harden sync overwrite protection and add local restore history (#720)
* 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>
2026-04-15 03:09:55 +08:00

74 lines
3.3 KiB
JavaScript

/**
* syncAnchorDecision — pure "has the remote changed since we last saw it?"
* logic extracted from CloudSyncManager so it can be exercised by
* `node --test` without standing up the full manager harness.
*
* Called from CloudSyncManager.inspectProviderRemoteState after the
* remote has been downloaded and its signature computed. Given the
* previous anchor and the current state, decides whether the remote
* looks different enough to warrant re-merging.
*
* Four decisions matter for data integrity:
*
* 1. Anchor missing + remote empty → not changed (first sync, nothing
* to merge from). Callers MUST still guard against pushing an empty
* local vault (see useAutoSync `hasMeaningfulSyncData`) — that guard
* is orthogonal to this decision.
* 2. Anchor missing + remote non-empty → changed (first sync, remote
* has data we've never observed → three-way merge with empty base).
* 3. Anchor present + resourceId drift → changed (provider created a
* fresh file; reuse of the old anchor would be meaningless).
* 4. Anchor present + signature mismatch → changed (same resource, new
* ciphertext — standard drift).
*
* Any other state is "unchanged", and callers short-circuit the merge.
*
* @param {{
* currentSignature: string | null,
* currentResourceId: string | null,
* anchor: { signature?: string | null, resourceId?: string | null } | null,
* hasRemoteFile: boolean,
* }} input
* @returns {{ remoteChanged: boolean, reason: string }}
*/
export function decideRemoteChanged(input) {
const { currentSignature, currentResourceId, anchor, hasRemoteFile } = input;
if (!anchor) {
// No anchor means we've never observed this provider.
if (!hasRemoteFile) {
// Remote has no file at all → nothing to merge.
return { remoteChanged: false, reason: 'no-anchor-no-remote' };
}
if (currentSignature === null) {
// hasRemoteFile=true but the signature computed to null — the
// file exists but we can't hash its meta (malformed shape, newer
// schema, partial download). Treat as CHANGED so the caller
// routes through the three-way merge / decrypt path rather than
// silently short-circuiting and letting the next upload overwrite
// an unreadable-but-extant remote file. If the payload is
// decryptable the merge will succeed; if it isn't, the decrypt
// error surfaces to the user, which is strictly safer than a
// silent stomp.
return { remoteChanged: true, reason: 'unreadable-remote' };
}
return { remoteChanged: true, reason: 'no-anchor-remote-has-data' };
}
// Resource identity drift: provider returned a different resource
// (e.g. a freshly-created gist, or the user reconnected and the
// adapter picked a new file). The previous anchor's signature is
// meaningless once the resource id changes.
const anchorResourceId = anchor.resourceId ?? null;
if (anchorResourceId !== currentResourceId) {
return { remoteChanged: true, reason: 'resource-id-changed' };
}
// Same resource, different signature → new ciphertext/meta.
if ((anchor.signature ?? null) !== currentSignature) {
return { remoteChanged: true, reason: 'signature-mismatch' };
}
return { remoteChanged: false, reason: 'anchor-matches' };
}