Skip to content

fix: end-to-end bug audit — deploy/seed-phrase/lifecycle hardening - #30

Open
Sentinel-Bluebuilder wants to merge 4 commits into
sentinel-official:mainfrom
Sentinel-Bluebuilder:fix/e2e-audit-2026-06-18
Open

fix: end-to-end bug audit — deploy/seed-phrase/lifecycle hardening#30
Sentinel-Bluebuilder wants to merge 4 commits into
sentinel-official:mainfrom
Sentinel-Bluebuilder:fix/e2e-audit-2026-06-18

Conversation

@Sentinel-Bluebuilder

Copy link
Copy Markdown
Contributor

Summary

End-to-end audit of the desktop node deployer surfaced and fixed a batch of
correctness, security, and lifecycle bugs spanning the main process and the
renderer. Fixes are grouped into three single-concern commits.

High severity

  • H-1 node-manager live-status single-fire + status reconciliation.
  • H-2 Docker pipe/health detection and error surfacing.
  • H-3 / H-4 deploy cancel + lifecycle races.
  • H-5 IPC enum / handler wiring mismatch.
  • H-6 renderer window listeners leaked (online/offline/keydown now torn down).
  • H-7 replayed done frames no longer skip the recovery-phrase backup flow.
  • H-8 clicking Cancel can no longer discard an unacked recovery phrase that
    landed in the same tick.

Medium severity

  • M-7 / M-8 Docker health detection edge cases.
  • M-9..M-11, M-15 node-manager status / event correctness.
  • M-12 mnemonic carry-through during deploy.
  • M-13 shared input validators extracted (src/main/validate.ts).
  • M-16 TOCTOU between the slow nodes.status RPC and the fast live-status
    push (per-node freshness clock); Nodes refreshes only newly-seen ids;
    DeploySshBatch clears its deploy poll intervals on unmount.

Low severity

  • L-3 known-hosts loader distinguishes ENOENT from real read errors.
  • L-7 single-instance lock + clean updater teardown on quit.
  • L-10 live-stats subscribers refcounted by webContents and auto-released
    on destroy (no leak across renderer reloads).
  • L-12 real fs.statfs disk probe (was a hardcoded 50GB/diskOk:true stub);
    recovery phrase rendered only once revealed (was CSS-blurred but live in DOM);
    ProgressRing peak moved out of render into state; NaN/Infinity/invalid-ISO
    guards in formatters.

Testing

Built and exercised locally on Windows 11 (Electron). Full CLI + UI
end-to-end pass to follow.

Notes

  • No secrets touched: mnemonics remain in the OS keychain (safeStorage/DPAPI);
    only public addresses are ever surfaced.
  • RPC-first preserved for all Sentinel chain access.

End-to-end audit fixes across the main-process services:

- deploy.ts: cancel/lifecycle races, mnemonic carry-through (H-3, H-4, M-12)
- docker.ts: pipe/health detection + error surfacing (H-2, M-7, M-8)
- node-manager.ts: live-status single-fire + status reconciliation
  (H-1, M-9, M-10, M-11, M-15)
- cli-registry.ts / node-specs.ts / ssh.ts: input validation + correctness
- shared/types.ts + ipc.ts: IPC enum + handler wiring (H-5, L-5, L-6)
- validate.ts: shared validators extracted for reuse (M-13)
…10,L-12)

- index.ts: single-instance lock + clean updater teardown on quit (L-7)
- updater.ts: window-aware install dialog + cancellable startup check (L-12)
- host-keys.ts: distinguish ENOENT from real read errors (L-3)
- live-stats.ts: refcount subscribers by webContents, auto-release on
  destroy instead of leaking across renderer reloads (L-10)
- system-report.ts: real fs.statfs disk probe replacing the hardcoded
  50GB/diskOk stub, degrading to unknown rather than fabricating (L-12)
- logger/settings/store/chain/geoip/wallet/sentinel-client: secret
  redaction, RPC-first fallbacks, and defensive guards
…6..H-8,M-16,L-12)

- store/app.ts: per-node liveStatusAt freshness clock so a slow nodes.status
  RPC can't overwrite a fresher live-status push (M-16 TOCTOU); named
  window listeners with teardown (H-6); seed-backup redirect runs before
  the done-dedup guard so replayed done frames still drive the seed flow (H-7)
- Progress.tsx: Cancel re-reads live progress and bails if an unacked
  recovery phrase is present, instead of clearing it (H-8)
- Nodes.tsx: refresh status only for newly-seen node ids (M-16)
- DeploySshBatch.tsx: track + clear deploy poll intervals on unmount and
  gate row writes behind an alive flag (M-16)
- WalletSetup.tsx: render the recovery phrase only once revealed instead
  of leaving it in the DOM behind a CSS blur (L-12)
- ProgressRing.tsx: move monotonic peak into state advanced from an effect
  instead of mutating a ref during render (L-12)
- format.ts: guard NaN/Infinity in fmtAmount/fmtUSD and invalid ISO in
  relativeTime (L-12)
wallet-setup.test.tsx: the L-12 fix stops rendering mnemonic words into
the DOM until the user explicitly reveals them (each tile shows "••••••"
pre-reveal so the secret never sits in the a11y tree / DevTools). Update
the test to assert the word is ABSENT first, click "Click to reveal
phrase", then assert it renders — matching the corrected behavior.

metrics.test.ts: better-sqlite3 is a native addon built for a single
NODE_MODULE_VERSION. The app ships on Electron (ABI 140), so
`npm run rebuild:electron` produces an Electron-ABI binary the vitest
runner (Node, ABI 137) cannot dlopen — the metrics store then degrades
to a no-op and the assertions can't run. Probe the binding once and
describe.skip WITH A LOGGED REASON when it can't load under the runner,
instead of silently passing against a disabled store. Full logic still
runs whenever the runner ABI matches the built binary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant