fix(peers): close 50+ audit findings across P2P sync system#133
Merged
fix(peers): close 50+ audit findings across P2P sync system#133
Conversation
…HMAC, drop plaintext PIN from session
… set https timeouts
… privkey from PeerIdentity surface
Audit refs: tmp/audit/01-security.md
- peer-identity.ts:39-43 — plaintext private key fallback no 0o600
- peer-identity.ts:73 — PeerIdentity.privkey exported but unused
Changes:
- Add IdentityOpts { allowPlaintext?: boolean } and require explicit
opt-in when safeStorage.isEncryptionAvailable() is false. Throw
before generating the keypair so failure path doesn't leak entropy.
Error message instructs users to set
ADC_PEERS_ALLOW_PLAINTEXT_IDENTITY=1 to opt in.
- Log a serviceLogger warning when writing in plaintext mode.
- Persist identity file with mode 0o600 (was default 0o644 minus umask).
Mirrors peer-tls.ts pattern.
- Drop privkey from the exported PeerIdentity interface — only sign()
is used by callers (verified via git grep: no consumers of
PeerIdentity.privkey in src/main/features/peers or assistant). The
on-disk JSON format is unchanged (still pubkey/privkey/useSafeStorage),
only the in-memory surface is minimized.
- peers-service.ts: pass { allowPlaintext: process.env
.ADC_PEERS_ALLOW_PLAINTEXT_IDENTITY === '1' } at the single call site.
Tests:
- New tests/unit/peers/peer-identity.test.ts (8 cases, 1 skipped on
win32) — covers throw-without-opt-in, no-leak-on-failure-path,
0o600 mode (POSIX), reload symmetry, sign() signature shape, and
asserts 'privkey' not in identity.
- Update tests/integration/peers/peer-identity.test.ts to pass
allowPlaintext: true (mock keeps safeStorage unavailable) and
remove .privkey assertions; add a throw-without-opt-in case.
peer-tls.ts ripple: none. peer-tls.ts derives its own X.509
keypair via @peculiar/x509 and never reads PeerIdentity.privkey.
…and permanent-fail on fingerprint mismatch
… incoming sockets, dedupe peerStore/pairing
…rip peerIdShort from GC frontier, WALL_PAD=13
Audit 04 C1+C2: PeersService bootstrap was a fire-and-forget IIFE that left handlers throwing "peers-service not yet initialized" if any IPC call landed during the boot window, and disposePeerTransport was a no-op if shutdown raced ahead of the IIFE — leaking the TLS listener + mDNS. - Add wrapAsyncPeersService(): a Proxy that wraps Promise<PeersService> and returns Promise-shaped methods that await the inner promise before delegating. Source-compatible cast to PeersService at the wrap-site. - Replace the IIFE in service-registry.ts with a single peersServicePromise + wrapper. disposePeerTransport now awaits the same promise so half-constructed resources still get cleaned up. - Add a peersDisposed guard so a second dispose during the same window is a no-op. - Update peers-handlers.ts comments — handlers continue to use Promise.resolve(...) which transparently chains the wrapper Promise for sync-typed methods (listPaired/listDiscovered/getIdentity/revoke) and keeps the existing await-thenable lint clean. - Tests: 6 unit (wrapAsyncPeersService) + 3 integration (bootstrap-race) covering forward/race-safe/dispose-before-resolve/rejected-bootstrap. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…edPeer source-of-truth Closes audit H1-H5 from tmp/audit/04-service-ipc.md: - H1: validatedHandle helper parses both input and output schemas (output validated only in non-production for zero prod overhead). - H2: peers-handlers reaches through peersInvoke[channel] map instead of importing individual schemas. - H3: HostnameSchema regex (DNS / IPv4 / IPv6 / zone-id / brackets) replaces arbitrary z.string() on host fields, closing SSRF surface. - H4: displayName normalized to z.string().nullable() across DiscoveredPeerSchema, PairInitInputSchema, PairConfirmInputSchema, PinIssuedEventSchema. Drops .optional() permutations. - H5: PairedPeer is now exported from @shared/ipc/peers/contract.ts; peer-store re-exports it. Single source of truth. Also normalize DiscoveredPeerWithPaired.displayName to string|null and coalesce ad.displayName ?? null in enrichDiscovered so output schema validation passes in dev. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…casts and inline keys
… visibility Closes audit H4. Previously <IncomingPinDialog /> was mounted inside SettingsPage, so a PIN issued by a remote initiator while the user was on any non-Settings route was silently lost (the useIncomingPin hook only listens while mounted). Hoisted the dialog to RootLayout alongside other always-mounted overlays (AssistantWidget, WorkflowPermissionModal, notifications). Mounted in the post-onboarding shell only — incoming pair invites have no meaning before the workspace is initialized. Removed the corresponding TODO(p2p-phase4) comment and the now-unused IncomingPinDialog import from SettingsPage. Backstop test added at tests/unit/renderer/peers/incoming-pin-global.test.ts. The repo's vitest config uses environment: 'node' (no jsdom), so the test falls back to static-analysis assertions that verify the dialog is imported and rendered in RootLayout and absent from SettingsPage. To be replaced with a render test if/when a jsdom-enabled vitest project is added.
…ePeerListPanel) + format helpers Closes audit C3, C4, M2, M5 (and L3 partially) from tmp/audit/05-renderer.md. - Extract useOutgoingPair hook owning Stage state machine, session token, PIN value (sanitized via sanitizePin), and pair-init/pair-confirm mutations. OutgoingPairDialog becomes render-only. - Extract usePeerListPanel hook owning inviteTarget state and the four query/mutation hooks. PeerListPanel becomes render-only and the renderSelfBody / renderPairedBody / renderDiscoveredBody helpers are now typed sub-components <SelfBody/>, <PairedList/>, <DiscoveredList/>. - Replace lib/truncate.ts with lib/format.ts adding peerLabel() and sanitizePin(); re-export truncate. Removes inline (peer.displayName ?? truncate(peer.peerId)) duplication across all three dialogs. - Add src/shared/ipc/peers/constants.ts with PIN_LENGTH=6 (M5) and PEER_ID_DISPLAY_MAX=16. format.ts and the hook import the shared constant; main + renderer agree on PIN size from one source. - IncomingPinDialog (L3): replace Heading as="h1" for the PIN value with Text size="lg" + className="text-3xl font-mono tracking-widest" (Text size tokens are sm/md/lg only — visual scale comes from the Tailwind utility). Uses peerLabel() helper. - index.ts: re-export the hooks, peerLabel, sanitizePin, truncate; drop truncate.ts barrel entry. Tests (44 added/touched, all passing): - format.test.ts (15 real unit tests on truncate/peerLabel/sanitizePin) - useOutgoingPair.test.ts (10 static-analysis assertions, T13/T14 style) - usePeerListPanel.test.ts (10 static-analysis assertions) Verification: vitest peers/ green (44/44), npx tsc --noEmit clean, npx eslint clean on all touched files. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ase1 deprecations Adds src/main/features/peers/peer-constants.ts as the single source of truth for protocol-level + runtime constants shared across the peers module. Updates consumers to import from there: - peer-pairing.ts: SESSION_TTL_MS, SESSION_MAX_ATTEMPTS, SESSION_SOFT_LIMIT (drops local DEFAULT_TTL_MS / DEFAULT_MAX_ATTEMPTS / DEFAULT_MAX_ACTIVE_SESSIONS) - pair-server.ts: PAIR_BODY_MAX_BYTES, PAIR_REQUEST_TIMEOUT_MS, PAIR_HEADERS_TIMEOUT_MS, PAIR_KEEPALIVE_TIMEOUT_MS, PAIR_BODY_READ_TIMEOUT_MS, LOOPBACK_HOST - ws-transport.ts: WS_CLOSE_CODES (SCHEMA_MISMATCH, FINGERPRINT_MISMATCH, MALFORMED_FRAME, UNTRUSTED), MAX_INBOUND_SOCKETS, LOOPBACK_HOST - peer-mdns.ts: MDNS_SERVICE_TYPE, MDNS_PROTOCOL, PEER_ID_SHORT_LEN - peers-service.ts: GC_INTERVAL_MS, LOOPBACK_HOST - outbound-dialer.ts: WS_RECONNECT_BASE_MS / WS_RECONNECT_MAX_MS / WS_RECONNECT_JITTER as defaults (still overridable via opts) Removes the Phase1PeerConfig type alias and loadPhase1PeerConfig() function from peer-config.ts. Grep across src/ + tests/ found zero non-self callers. Pure mechanical refactor — no value changes, no behavior changes. Audit refs: 02-transport.md H2/H3/L4/L7, 01-security.md "magic numbers" section. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…peer-state schema, drop peerId fallbacks, schema-hash uses node:crypto, cross-device-query DI, relocate peerKeys - Move migration-tags.ts from src/main/features/peers/ to src/main/db/ (audit 01/Low — generic Drizzle journal logic, not peer-specific). - Clear revokedAt on re-pair in peerStore.upsert (audit 01/Medium — silent re-pair failure on previously-revoked peers). - Merge peer-state-schema.ts into peers/schema.ts (audit 03/Low — one schema file per domain). - Drop dev-default 'aaaaaaaa' / 'peer-a' peerId fallbacks in service-registry; resolve identity once via getOrCreatePeerIdentity and pass through to peers-service + replicationEngine. Throws if identity is unresolved (audit 04/L3). - cross-device-query receives PeerStore via DI rather than constructing its own; reuses the registry-owned singleton (audit 04/M6). - schema-hash.ts switches to node:crypto and is now sync — drops unnecessary async API at the only call site (audit 03/M8). - Relocate peerKeys to @shared/ipc/peers/queryKeys.ts; renderer feature re-exports for source compat. EventBridge imports from @shared/ipc/peers, removing the boundaries/dependencies disable (audit 05/T13). Tests: new tests/unit/peers/peer-store-revoked-reset.test.ts; schema-hash + migration-tags + cross-device-query tests updated. typecheck + lint pass on touched files. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Multi-agent audit + fix sprint on the new device-to-device (peers / P2P) sync system. Closes every Critical and High finding across security, transport, replication, service/IPC, and renderer layers.
tmp/audit/0[1-5]-*.md+tmp/audit/SUMMARY.mddocs/superpowers/plans/2026-04-26-peers-audit-fixes.mdKey Fixes
Security —
crypto.randomIntPIN,timingSafeEqualon decoded HMAC, per-IP rate-limit on/pair/*, slowloris timeouts, safeStorage-required identity with0o600mode,checkServerIdentityfor TLS pinning (no more post-handshake fingerprint hacks).Transport —
OutboundDialerstate machine (backoff + jitter + cap +permanently_failed),wss/socket'error'listeners, signed Ed25519 HELLO inbound peer auth, boundincomingSockets, Zod-validated wire frames.Replication —
peer_state.last_seen_hlcactually written (op-log GC was a no-op before),lastHlcseeded fromop_logon engine restart, GC frontier stripspeerIdShortsuffix,WALL_PAD=13, dedup-first inapplyRemoteOp,op_log(hlc)index, per-table column allowlist viaSYNC_TABLE_DEFS.Service / IPC — Awaitable peers bootstrap (kills the IIFE race + dispose race),
validatedHandleenforcing input AND output schemas,HostnameSchemaregex (closes SSRF surface), singlePairedPeersource-of-truth,postJsonPinned/safeFanOut/runGcTickextracted.Renderer —
useIpcEvent(no morewindow.api.on),peerKeysfactory used everywhere (relocated to@shared/ipc/peersto remove FSD boundary disable),IncomingPinDialoghoisted toRootLayout(was scoped to Settings only), MVP presentation hooksuseOutgoingPair+usePeerListPanel,peerLabel/sanitizePinhelpers,PIN_LENGTHconstant.Cleanup —
peer-constants.tsmodule,Phase1PeerConfigdeprecation removed,migration-tagsrelocated to@main/db,peer-state-schemamerged intoschema.ts, dev-defaultpeer-apeerId fallbacks dropped,cross-device-queryacceptspeerStorevia DI,schema-hashusesnode:crypto.Test plan
npx tsc --noEmit— PASSnpx eslinton all touched dirs — PASS (0 warnings)🤖 Generated with Claude Code