feat: hide nostrconnect UI behind dev menu + add log export - #4
Merged
Conversation
7 tasks
DocNR added a commit
that referenced
this pull request
May 2, 2026
Brainstorm review of design-system.md against shipped code surfaced 9 inconsistencies + 1 anti-pattern still present. Fixed everything in one batch so the next TestFlight archive carries it all. Code: - HomeView: drop .padding(.bottom, 8) on SlimIdentityBar invocation — slim banner owns its outer bottom padding (12pt); stacking another 8pt on top was double-counting (review #4) - HomeView: drop .padding(.bottom, 8) inside statsRow — listSectionSpacing(0) carries the gap to Connected Clients; the residual padding kept the visible gap excessive after polish round 2 (review #2) - AccountDetailView: avatarLarge letter fallback opacity 0.25 → 0.22 to align with SlimIdentityBar's 0.22 (review #3) - ConnectSheet: add .presentationBackground(Color(.systemGroupedBackground)) — was the last sheet still defaulting to translucent (review #9) - ApprovalSheet: rename @State capExceeded → showConnectionCapAlert for naming convention parity with HomeView (review #7) design-system.md: - New "Cross-platform applicability" section at the top — clarifies what carries directly to clave.casa web companion (color tokens, displayLabel rule, identity-vs-functional zone philosophy, avatar treatments, copy patterns, anti-patterns) vs what's iOS-only (SwiftUI modifiers, haptics, sheet/toolbar conventions) - §3 Typography: corrected initial-letter font scale — AvatarView uses size*0.35 mono (pubkey) or size*0.4 proportional (name); was wrongly documented as a single 0.37 (review #1) - §4 Avatars: added Treatment Selection Rule table (B on neutral bg, C on saturated theme gradient) + clarified 1-vs-2 letter behavior (review #5, #6) - §4 Sizing scale: expanded table to include initial font + border thickness per slot, with the ~5% border scaling rule (review #8) - §5 Spacing: explicit "single source of truth" note on slim banner bottom padding; new "Stats row" subsection capturing the ultraThinMaterial-on-small-cards-OK rule (review gap #10, #4) - §6 HomeView gradient: documented palette[0] defensive fallback when currentAccount is nil (review #13) - §7 Patterns: new "State variable naming" subsection with the showCapAlert / showAccountCapAlert / showConnectionCapAlert convention (review #7 doc side) - §11 Anti-patterns: audit-point note that ConnectSheet was the last surface to acquire .presentationBackground (review #9 doc side) Build green on iOS Simulator 26.4. pbxproj still 41 — assumes user hasn't yet archived 41; bump to 42 if needed before re-archive. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DocNR added a commit
that referenced
this pull request
Jun 7, 2026
Closes a spec-compliance gap surfaced by external code review of the TypeScript port (same bug existed verbatim in the Swift port). Spec algorithm step 4 says: before MAC verify, fail if the embedded kind/scope on the wire don't match the caller-supplied expected kind/scope. Our implementation skipped this check on the assumption that MAC verify covered it — that's only half right. Why MAC verify alone isn't sufficient: - The MAC is computed over `nonce || u32_be(kind) || u32_be(scope_len) || scope || chacha20_ct`. - At encrypt time, the encryptor uses (kind, scope) consistent with what they signed in to the wire's embedded fields. - On decrypt, our implementation computes the verification MAC using the CALLER's `context.kind / context.scope`, NOT the embedded `parts.kind / parts.scope` from `Ciphertext.decode`. - For a legitimate wire, caller's context == embedded == encryptor's choice, so MAC matches and decrypt succeeds. - For a wire whose embedded kind/scope bytes are tampered IN TRANSIT but whose MAC tag is left intact, our MAC computation still uses the (untampered) caller context. If that context matches what the encryptor originally signed in, MAC matches and decrypt succeeds — silently accepting a tampered wire. The exploit surface in current usage is narrow because we never expose `parts.kind / parts.scope` from the public decrypt API (the caller already knows kind+scope; they passed them as context). But it's a real spec deviation and weakens the "embedded context is authenticated" property the v3 design claims. Fix: explicit `parts.kind == context.kind` check + constant-time `parts.scope == context.scope` bytes equality, before MAC verify. Both branches throw `.decryptionFailed` (same case as MAC failure) so a network observer can't oracle which mismatch tripped it. Constant-time helper duplicated inline rather than imported from Encryption.swift because cross-file private exposure for a 12-line helper adds little. Two new XCTest cases in NIP44v3Tests.swift verify rejection: - testDecryptRejectsTamperedEmbeddedKind — flips byte 68 of a valid wire (low byte of u32 kind), MAC untouched, expects .decryptionFailed. - testDecryptRejectsTamperedEmbeddedScope — flips byte 73 (first scope byte) of a valid non-empty-scope wire, MAC untouched, expects .decryptionFailed. Both would have silently SUCCEEDED before this commit. Build bumped 92 → 93. Companion Spectr-side fix lands in DocNR/spectr@feat/nip44v3-port in a separate commit covering the same defect + #2/#3/#4 from the same review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
<iso-timestamp> [category] message, copies to clipboard. Fetch runs onTask.detachedto avoid UI hang.Why
Recent debugging surfaced three clients that break on nostrconnect (zap.cooking custom wrapper, plebsvszombies old nostr-tools, Coracle without NIP-65). Bunker flow works reliably everywhere. For the upcoming announcement, lead with bunker-first positioning and hide the partially-working feature until the proxy-per-client-relay work (in BACKLOG) lands. Log export makes future debugging sessions faster — less idevicesyslog + SSH gymnastics, more copy-paste into chat.
Known limitations
.currentProcessIdentifierscope captures main-app logs only. NSE runs in a separate process and is not captured. Developer-menu text calls this out. For NSE logs, user still needs Xcode Console or idevicesyslog.SharedConstants.sharedDefaultsbacking means the dev-menu unlock persists across app restarts (intentional — once unlocked, stays unlocked until user taps "Lock Developer Menu").Follow-ups (non-blocking, noted by final code review)
LogExporter.allCategoriesdeclares"apns"but no logger uses that category today (push delegate logs under"app"). Harmless — constant isn't referenced at the call site. Drop or start using it in a follow-up.SettingsView.swift:202-203aroundLogExportercalls fromTask.detached. Benign under Swift 5. Fix by addingSendableconformance toLogExporter.Entry/ makingLogExporterexplicitly Sendable.DeveloperSettings(current tests cover the puretapGateSatisfiedhelper only).initaccepts adefaults:override, so this is easy to add.Test plan
@State devSettingsobservation)Commits (will squash-merge)
🤖 Generated with Claude Code