Skip to content

feat: open pubky auth links - #722

Open
ovitrif wants to merge 23 commits into
codex/pubky-ring-signupfrom
codex/715-pubkyauth-links
Open

feat: open pubky auth links#722
ovitrif wants to merge 23 commits into
codex/pubky-ring-signupfrom
codex/715-pubkyauth-links

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Closes #715

This PR extends the #697#724 signup/auth stack with Bitkit-targeted Pubky marketplace setup-link delivery and explicit watch-only account authorization.

Description

  • Applies Bitkit claim validation only to the uniquely targetable bitkit://pubky-auth/setup wrapper and normalizes that wrapper to Paykit rc51's pubkyauth://signin_grant request while preserving its raw query.
  • Accepts lightning:/lnurl*:-prefixed raw pubkyauth:// auth and signup requests from OS links through the same parsing path used by scanner and clipboard paste.
  • Retains setup requests, Ring callbacks, BIP21, BOLT11, and gift links through startup, restoration, PIN, scene, and app lifecycle gates until their destination UI can handle them.
  • Preserves feat: support Pubky signup #724's signup parsing, identity checks, scanner dismissal, loading state, and profile-setup continuation as the source of truth.
  • Requires wrapper requests to carry the supported watch-only-account-v1 claim with the exact server/private read-write capability set and rejects missing, duplicate, unsupported, or malformed authorization parameters before account material can be authorized.
  • Displays the parsed authorization relay origin on watch-only consent and authorization without representing it as a verified service identity.
  • Uses the separate pubkyring://signin scheme only for Bitkit's outbound Ring handoff.

Linked Issues/Tasks

Stack Note

#697 and #724 are currently seven master commits behind, while #722's preserved history already contains those commits. Until the parent branches sync with master, GitHub's stacked diff also shows those inherited changes. The #722-specific behavior is the marketplace wrapper, lifecycle routing, watch-only validation, relay display, Ring handoff, and journey coverage described above.

Preview

QA Notes

Manual Tests

  • 1. Terminated or PIN-locked wallet with Paykit UI and a Bitkit-generated Pubky identity → open a valid setup link → finish startup/unlock: watch-only consent shows the authorization relay origin.
  • 2. Watch-only consent → Cancel: returns without authorizing or exporting account material.
  • 3. Offline wallet → open bitkit://gift-…: Gift Loading opens and owns node failure presentation.

These external-handoff scenarios remain open; the automated validation below does not mark them complete.

Automated Checks

  • Focused clean-simulator tests: 135 passed across PubkyAuthRequestTests, PubkyAuthURLSchemeTests, SceneDelegateTests, PubkyProfileManagerTests, PubkyAuthApprovalSheetTests, PendingProfileSetupResumeTests, and ShopPaymentRequestTests on bcc07259.
  • E2E Debug simulator build-and-run: passed on bcc07259; the app launched and produced a semantic UI snapshot.
  • SwiftFormat, translation validation, and git diff --check: passed on bcc07259.
  • No local validation was rerun for documentation-only review fix c2de3f05, per implementation-only direction.
  • GitHub Actions remain on the normal PR workflow; no reruns were triggered.

@greptile-apps

This comment has been minimized.

Comment thread Bitkit/Info.plist Outdated
ovitrif

This comment was marked as off-topic.

Comment thread Bitkit/AppScene.swift Outdated
Comment thread Bitkit/Models/PubkyAuthRequest.swift
@ovitrif
ovitrif force-pushed the codex/715-pubkyauth-links branch from d8110c0 to 48b2191 Compare September 2, 2026 21:43
@ovitrif
ovitrif requested a review from ben-kaufman September 2, 2026 21:45
Comment thread Bitkit/ViewModels/AppViewModel.swift
@ovitrif ovitrif changed the title feat: open Pubky auth links feat: open pubky auth links Sep 3, 2026
@ovitrif
ovitrif force-pushed the codex/715-pubkyauth-links branch from 48b2191 to c108dee Compare September 3, 2026 21:51
@ovitrif
ovitrif requested a review from ben-kaufman September 3, 2026 21:52
Comment thread Bitkit/ViewModels/AppViewModel.swift
@ovitrif
ovitrif requested a review from ben-kaufman September 3, 2026 22:45
Comment thread Bitkit/ViewModels/AppViewModel.swift

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the link as an attacker-controlled trust boundary. Validation is strict — exact scheme/host/path, no userinfo/port/fragment, exactly one x-bitkit-claim=watch-only-account-v1, caps must equal the fixed watch-only set, duplicate relay/secret rejected — and I could not escalate through it. The wrapper forces requiresBitkitClaim, so ordinaryApproval (the only path that forwards URL-supplied capabilities) is unreachable from the bitkit:// wrapper; the companion path passes the hardcoded watchOnlyAccountCapabilities constant. Raw pubkyauth:// requests are a separate matter: handleScannedData strips a lightning:/lnurl*: prefix before classification, so a lightning:pubkyauth://signin?...&caps=<anything> link does reach ordinaryApproval with URL-supplied caps — behind the consent sheet, the Paykit flag, and local auth where enabled. The claim payload is version|accountIndex|addressType|78-byte xpub — public material, no spending key. Secrets are not logged, and retained-link handling is single-slot and PIN-gated.

One medium, about provenance rather than authorisation. One thing I could not settle offline: whether Pubky Ring actually registers pubkyring:// — if not, Ring sign-in reports "not installed".

Minor gap while you are here: duplicate caps parameters are not rejected the way duplicate relay/secret are. Not exploitable (the companion path never reads URL caps, so a divergence yields rejection), but inconsistent with the other duplicate checks.

Comment thread Bitkit/ViewModels/AppViewModel.swift Outdated
@ovitrif
ovitrif force-pushed the codex/715-pubkyauth-links branch from 4f7a770 to 151170c Compare September 5, 2026 01:53
@ovitrif ovitrif self-assigned this Sep 5, 2026
ben-kaufman
ben-kaufman previously approved these changes Sep 6, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed this specifically as a deeplink-gating problem, because I just confirmed a HIGH-severity hole of exactly this shape on the Android side (synonymdev/bitkit-android#1224 — a pubkyauth://direct_signup?hs=<attacker> link binds the wallet's seed-derived identity to an attacker's homeserver with no sheet and no PIN). iOS does not have that class of bug here. Recording the trace since it's the kind of thing worth having on file.

Every URL entry point, and its gating:

Entry point Effect Gate
.onOpenURL (AppScene.swift:149) retainDeepLink(url) — store only none needed, no side effects
Cold start scene(_:willConnectTo:) (SceneDelegate.swift:40, :51-54) DeepLinkRouter.forward → store store only
Warm scene(_:openURLContexts:) (SceneDelegate.swift:69-73) store store only
AppDelegate.application(_:open:) (BitkitApp.swift:43-50) store (not invoked in a scene lifecycle) store only
AppScene.onAppear router drain (:287-289) store store only
Sole consumerMainNavView .task(id:) / .onChange (:326-333:651-712) handleScannedData / ring callback MainNavView mounts only when walletExists, not initializing/restoring/migrating, and isPinVerified || !pinEnabled (AppScene.swift:326-429)

Same gate as the QR scanner, so no mismatch — which is precisely where Android went wrong. Backgrounding sets isPinVerified = false (AppScene.swift:743), unmounting MainNavView and cancelling its .task.

Why the Android primitive isn't reachable: the Android hole was a direct_signup link binding the identity with no sheet and no PIN. iOS always presents the approval sheet, which is the part that matters.

The reachable link surface is wider than the bitkit:// wrapper, though. Info.plist registers lightning/LIGHTNING, and handleScannedData strips those prefixes before classification (AppViewModel.swift:492), so lightning:pubkyauth://direct_signup?hs=<z32> and lightning:pubkyauth://signin?...&caps=<anything> are both OS-deliverable and both skip requiresBitkitClaim (PubkyAuthRequest.swift:126). The claim contract — exactly one x-bitkit-claim=watch-only-account-v1, caps equal to the fixed watch-only set, host hardcoded to signin_grant — applies only to bitkit://pubky-auth/setup. So an hs= rebinding param and a generic capability grant are link-reachable; what stops them is the consent sheet, not the parser.

On that sheet the attacker-supplied homeserver z32 is rendered (PubkyAuthApprovalSheet.swift:262-274) and the requested capabilities are itemised (:253-255). Claim-carrying wrapper requests take two taps (watch-only consent, then authorize); non-claim requests open directly in .authorize (:84), so one tap. Local auth is required whenever PIN or biometrics is enabled, and skipped via PubkyApprovalLocalAuthMode.none when the user has neither (:22-24, :408-409). Everything is behind PaykitFeatureFlags.isUIEnabled (:553), default false. Ring callbacks are dropped unless the nonce matches the active attempt (PubkyProfileManager.swift:607-613).

Also checked and ruled out: .task(id:) cancelling an in-flight handler (bitkit-core's uniffiRustCallAsync has no cancellation handler and handleScannedData has no checkpoints, so the sheet still presents); duplicate delivery via onOpenURL + scene delegate (deduped by .onChange equality); routePendingDeepLinkIfReady clearing pendingDeepLinkURL synchronously before the first await (AppViewModel.swift:124-131), so concurrent callers can't double-handle. relay is displayed as origin only and its query is stripped from logs (MainNavView.swift:723-742).

One caveat on my own work: I read pubky 0.9.3 from the cargo registry to confirm Signer::approve_auth only posts an encrypted token to the relay and never calls signup; the lock pins 0.8.0, which I did not have locally.

Optional, non-blocking, and deliberately not touching the retain/replay design you three already settled: a retained URL has no expiry. A link tapped while locked or pre-onboarding is replayed whenever MainNavView next mounts — potentially days later, e.g. a payment link opening the Send sheet right after wallet creation. Consent taps still apply so it is not a bypass, but a staleness check in routePendingDeepLinkIfReady would tighten it.

An uncertain broadcast may still confirm, so discarding the signed
transaction on the way out must not delete the proof prepared for it.
Leaving the pending row behind is recoverable; deleting it is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J1dzLxvzqbhAMSTyXamczc
@ovitrif
ovitrif requested a review from jvsena42 September 7, 2026 22:58
@ovitrif
ovitrif changed the base branch from master to codex/pubky-ring-signup September 8, 2026 17:05

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One LOW inline — a documentation claim rather than a code defect. Not blocking.

Scope note, so nobody reviews #729 twice: the four hardware-wallet files (HwFundingSigner.swift, HwSendSignView.swift, HwFundingSignerTests.swift, changelog.d/next/729.fixed.md) plus the .broadcastConnectivity toast strings are byte-identical to origin/mastergit diff --quiet pr722 origin/master passes for all four. They're #729, merged to master at 12:27Z, showing up here only because the base branch codex/pubky-ring-signup hasn't synced master. The whole block vanishes once #724 merges master. Nothing about what the device signs changed.

Similarly, bcc07259a re-carries #724's signup parsing under this branch; I assessed it only for interaction with this PR's link delivery, not as new work.

Delta since my last review: the branch was rebased, then formatting-only 4d0a0b659 and the #724 re-stack. My relay-origin thread is addressed and survives the rebase unchanged. No over-correction, and nothing in the retention/gating design moved.

Checked and clean:

  • Request pinning (TOCTOU). The sheet reads only config.request (an immutable let) and passes config.request / config.request.rawUrl to performAuthorization. A second link arriving mid-approval goes through showSheet, which calls hideSheet() first — the .sheet(item:) tears the view and its @State down, dismissing the AuthCheck cover, and re-presents after 0.7s starting fresh at .watchOnlyConsent. An approval already past PIN completes on the captured request, never the new one. The signup path additionally guards on rawUrl equality. This is the approve-A-grant-B case and it's genuinely closed.
  • Lock-screen bypass. All five entry points only store. The sole consumer is MainNavView, mounted only when walletExists, not initializing/restoring, and isPinVerified || !pinEnabled; backgrounding resets isPinVerified, which unmounts it and the sheet. routePendingDeepLinkIfReady clears the slot before its first await, so the .task(id:) + .onChange double-trigger can't double-handle.
  • Wrapper trust boundary. isBitkitSetupHandoff requires exact scheme/host/path, no userinfo/port/fragment, non-empty query not starting with ?. normalizedProtocolURL hardcodes host signin_grant and passes raw query bytes through. parse rejects duplicate relay/secret, then forces the claim check — exactly one x-bitkit-claim=watch-only-account-v1 and caps set-equal to the fixed constant. parseSignup throws if a wrapper carried a claim, and a wrapper can't be a signup URL anyway. Companion approval passes the hardcoded watchOnlyAccountCapabilities, never request.capabilities.
  • Rendered attacker text. relayOrigin is rebuilt as scheme://host[:port] from URLComponents — no path, query or userinfo — so it can't contain < to break the <accent> markup. sanitizedDeeplinkDescription strips query and userinfo, and for opaque lightning: URLs logs only the scheme, so secret never reaches logs.
  • URL-scheme hijack. No security-relevant inbound secret rides on a claimable scheme. The one outbound URL carrying a secret is the Ring handoff, where Bitkit is the requester — a hijacker of pubkyring:// could only act as the signer, the same exposure pubkyauth:// had before this PR. Ring callbacks are nonce-pinned.
  • Lifecycle. The .task(id:) restarting when LDK reaches .running can't lose a pubky link: handleScannedData reaches showSheet with no await before it, and BitkitCore's uniffi async has no cancellation handler on the decode path.

One thing I could not settle, stated as unverified rather than cleared: my local paykit-rs checkout is older than the pinned rc51, and pubky_session.rs delegates host handling to pubky::DeepLink::from_str, so while PubkyAuthRequestTests proves rc51 parses signin_grant, I couldn't confirm the Rust approval path accepts it — the E2E journey deliberately cancels before approval. Worth one manual approve on rc51 to close.

Cross-repo: the alias-gating concern raised on synonymdev/bitkit-android#1224 is structural here rather than a bug — iOS registers URL schemes statically in Info.plist and can't runtime-gate them the way Android's manifest aliases can.

I've corrected two statements in my own earlier review bodies on this PR that the trace below showed were wrong.

Comment thread journeys/pubky-auth/README.md Outdated
@@ -0,0 +1,14 @@
# Pubky auth

This suite covers the uniquely targetable `bitkit://pubky-auth/setup` OS handoff into Bitkit. The wrapper carries the Paykit grant-auth requester fields and normalizes to `pubkyauth://signin_grant`; raw Pubky auth and signup requests remain supported through QR scanning and clipboard paste.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"raw Pubky auth and signup requests remain supported through QR scanning and clipboard paste" is incomplete — they're also reachable as OS links, and the same wording in the PR description ("Accepts only the uniquely targetable bitkit://pubky-auth/setup wrapper") reads as a security property the code doesn't implement.

Info.plist registers lightning/LIGHTNING, and handleScannedData strips that prefix before classification (AppViewModel.swift:492). I verified the transform directly rather than assuming: lightning:pubkyauth://direct_signup?hs=abc123 parses as scheme lightning with the absoluteString round-tripping byte-identical, and after removingLightningSchemes() becomes scheme pubkyauth, host direct_signup, query intact. Case-insensitive, so LIGHTNING: works too.

So both of these are OS-deliverable and skip requiresBitkitClaim (PubkyAuthRequest.swift:126, false whenever the scheme isn't bitkit):

  • lightning:pubkyauth://direct_signup?hs=<attacker-z32> -> signup consent sheet -> on approval, registerIdentity(…, homeserverZ32: <attacker>)
  • lightning:pubkyauth://signin?relay=…&secret=…&caps=/pub/anything/:rw -> opens directly in .authorize -> ordinaryApproval(authUrl, request.capabilities, …) with the URL's caps

This is not a hole you opened. The signin variant is on master; the signup variant is already reachable on the base branch (#724 head), which has the same onOpenURL -> handleScannedData -> removingLightningSchemes -> isProtocolURL -> signup-branch chain. This PR only changed delivery to retain-and-replay. And the consent boundary holds throughout: the attacker's homeserver z32 is rendered on the sheet, capabilities are itemised, an explicit Authorize tap is required, local auth applies whenever PIN or biometrics is enabled, and the whole surface is behind PaykitFeatureFlags.isUIEnabled (default false, Dev Settings only). That is why this is LOW and why it isn't the Android no-sheet bug.

What's wrong is only the description of the contract. Narrowest fix is two sentences: say here, and in the PR description, that the wrapper is the only link form receiving claim validation, and that lightning:/lnurl*:-prefixed raw pubkyauth:// requests are accepted from links exactly as the scanner and paste paths accept them.

Separately, and genuinely as a question rather than a request: should a raw pubky request arrive via a tapped link at all? If the team's threat model is "no pubky request without the claim contract from a link", the smallest change is one guard at the top of MainNavView.handleDeepLink — if url.scheme != "bitkit" and PubkyAuthRequest.isProtocolURL(url.absoluteString.removingLightningSchemes()), toast and return. No need to expose isBitkitSetupHandoff (the wrapper's scheme is bitkit, which the strip never touches) and no need to touch requiresLightningNode. That's a product call, not a defect.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected in c2de3f0. The journey README and PR description now state that Bitkit claim validation applies only to the bitkit://pubky-auth/setup wrapper, while lightning:/lnurl*:-prefixed raw Pubky auth and signup links follow the scanner and paste path. I left the separate policy question unchanged for a team product decision.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed fixed in c2de3f056. The README now scopes claim validation to the wrapper and names the lightning:/lnurl*: link form explicitly, and PR description bullets 1 and 2 match. Nothing over-corrected — the QR/paste sentence is preserved rather than replaced.

Leaving the open question from my comment as a question: whether raw pubky requests should arrive via a tapped link at all is still a product call, and nothing here forecloses it.

@ovitrif
ovitrif requested a review from jvsena42 September 8, 2026 19:07
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.

feat: open Pubky auth links from iOS

3 participants