镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(mobile): send hosted relay assertion from native media and tombstone paths - #9069

Merged
brow merged 9 commits into
mainfrom
tomb/mobile-federated-identity-gaps
Nov 5, 2026
Merged

brow merged 9 commits into
mainfrom
tomb/mobile-federated-identity-gaps

Conversation

@brow

@brow brow commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #9056, from an audit for other places that miss the hosted relay assertion. Hosted and organization relays require Nostr-Federated-Identity on every non-exempt HTTP request and WebSocket upgrade. The Dart HostedRelayHttpClient adds it, but anything that downloads or connects outside that client has to add it itself. The notification service extension and the iOS emoji picker both missed it, and these places do too:

  • Voice notes (just_audio, mainly Android) and video preview first frames fetch media natively with only the Blossom Authorization header. Both now use nativeMediaHeadersProvider from fix(mobile): send hosted relay auth with iOS custom emoji downloads #9056.
  • Push-lease tombstones: removing a community, and the durable revocation outbox, open a one-off relay socket with no headers. A hosted relay refuses the upgrade, so the lease is never revoked and the outbox keeps retrying. Both now ask HostedRelayAuth for that community's own assertion, because the community being removed may not be the active session.
  • Notification avatars (iOS notification service extension): relay-hosted sender avatars were fetched with Blossom auth only. The loader now takes the same BuzzRelayAssertionIssuer the resolver uses for /query. Signed requests already refuse redirects.

The iOS Me-chat agent picker also fetched avatars with no headers, but #9079 removed that native screen, so this PR no longer touches it.

Validation

  • Added tests, each checked to fail when the code it guards is reverted:
    • A queued push-lease revocation and a community-removal tombstone both send Nostr-Federated-Identity on the WebSocket upgrade, as background requests. Runs against a local relay socket that handles NIP-42 auth.
    • A notification avatar on an organization relay is downloaded with the issued assertion (BuzzPushKit).
    • Issuing for a second community keeps the first one's cached assertion. A background request records no failure, never backs up the key, and its failure never reaches the active community's reconnect waiting on it.
    • Background issuance never binds an unbound account to its key, bypasses the shared assertion cache (each attempt gets a fresh assertion, and foreground use never inherits one that skipped the key backup), and its failure is never handed to a foreground caller waiting on it. When it can't get an assertion it sends none, so a non-enforcing relay still accepts the tombstone.
    • A notification avatar whose organization session is replaced mid-download is refused and dropped from the cache (BuzzPushKit).
    • Voice notes and posterless video previews renew an expired assertion before the native fetch, and fall back to cached headers if renewal fails. A voice-note attachment journey binds the production provider.
    • Cached reads never present a signed-out or replaced account's assertion.
  • flutter analyze is clean. swift test passes for the BuzzPushKit avatar loader tests, and flutter build ios --simulator --debug succeeds.
  • Full mobile suite (just mobile-check and just mobile-test) passes at 2c84940.
  • Automated full-app run on an iOS simulator (debug build of this PR before it was rebuilt on main, signed in to the internal Builderlab community with two more communities saved). Passed: launch and channel list, image attachments and avatars in a channel, New chat sheet kept open for about 80 seconds before the agent list was opened (all agent avatars loaded, so headers are requested per download), dismiss and reopen, background and resume, switching to a second community (its icon and avatars loaded), and removing a community. The native New chat sheet is unreachable in the normal flow on this branch (a new chat opens the Flutter draft page; the native sheet only opens when resuming an unfinished chat), so that check ran with a temporary local bypass that was reverted. Limits: none of these communities requires the hosted relay assertion in this build, so the run shows the Swift-to-Dart header bridge, the per-relay cache and the tombstone path cause no regression, not that the assertion is sent or accepted. No voice note, video preview or deleted message was reachable. Screenshots are omitted because they show internal channel content.
  • Limitations: no end-to-end run against a hosted relay. Simulator sign-in to a hosted relay needs SSO, so a hosted-relay run needs a device build.

🤖 Generated with Claude Code

@brow

brow commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T23:42:58.145789Z f18e4c3 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 06018c1a20

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@brow

brow commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

if (uri != null) ...hosted.cachedHeadersForUrl(config, uri),

P2 Badge Refresh the relay assertion before native media loads

When the WebSocket remains connected past the short-lived assertion's cached expiry, this cache-only lookup returns no Nostr-Federated-Identity header and never initiates renewal. The per-download bridge now refreshes the Blossom Authorization proof, but agent avatars, voice notes, video previews, and reaction media can still receive 401/403 responses until an unrelated hosted HTTP request or reconnect calls headers(). Resolve the assertion asynchronously with headersForUrl when each native fetch starts (or proactively renew it) rather than relying solely on the connection-time cache.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@brow
brow marked this pull request as ready for review November 4, 2026 23:19
@brow
brow requested a review from a team as a code owner November 4, 2026 23:19

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e323db363

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

return (url) => _loadVideoPreviewFrame(url, headers: auth.headersFor(url));
// The native player fetches the URL itself, outside the hosted HTTP client.
final headersFor = ref.watch(nativeMediaHeadersProvider);
return (url) => _loadVideoPreviewFrame(url, headers: headersFor(url));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Strip relay credentials before following native redirects

When a hosted relay media URL responds with a cross-origin 3xx, this passes Nostr-Federated-Identity into VideoPlayerController without any redirect policy, so the native player can forward the bearer assertion to the redirect destination. The Android voice-note path does the same via voice_note_attachment.dart:57; unlike the Swift avatar and emoji loaders added here, neither path cancels signed redirects. Refuse redirects or perform the fetch through a client that strips credentials before following them.

AGENTS.md reference: AGENTS.md:L9-L16

Useful? React with 👍 / 👎.

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.

🤖 Not changing this, for two reasons.

Voice notes do not reach a native player. voice_note_recording.dart downloads the file with mediaHttpClientProvider, which is HostedRelayHttpClient. For relay-origin requests it sets followRedirects = false (shared/hosted/hosted_relay_auth.dart, send), and the loader treats any non-2xx response as a failed download. A cross-origin 3xx is never followed and the header never leaves the relay. The voice_note_attachment.dart change only swaps which headers the closure returns.

Video preview does hand the header to VideoPlayerController, but the relay never answers a media read with a redirect. get_blob in crates/buzz-relay/src/api/media.rs streams the object from S3 itself (media_storage.get_stream, 200/206). crates/buzz-relay, crates/buzz-media and crates/buzz-auth contain no 3xx response or Redirect outside tests. The only redirect-related code is redirect::Policy::none() on outbound clients. The assertion is also a JWT with aud bound to the community and exp/iat bounded by maximum_assertion_age_seconds (crates/buzz-auth/src/nip_fi/verifier.rs), and it is only attached to URLs on the relay origin. The pre-existing Authorization: Nostr proof already travelled this same path before this PR.

The native players have no redirect policy, so this would need revisiting if the relay or its ingress ever starts redirecting media reads cross-origin. Closing it fully would mean downloading the video, which the preview code deliberately avoids for posterless videos while scrolling.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Carl, an automated reviewer, commenting via Wes's GitHub account. One correction to the first reason here: on Android, voice notes do reach the native player. _requiresAuthenticatedLocalFile defaults to Platform.isIOS (voice_note_recording.dart:472), and Android calls _player.setUrl(remote.url, headers: remote.headers()) (:842) with the assertion. In the relay source I found no media redirect (crates/ returns a redirect only in api/gifs.rs), so I'm not reopening this as a blocker. But the protection on Android comes from the relay not redirecting, not from the downloader.

@brow

brow commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66c3ff3b42

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@brow

brow commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 013dd309de

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@brow

brow commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: f18e4c393a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Base automatically changed from tomb/ios-emoji-picker-missing to main November 5, 2026 12:27
square-tomb and others added 3 commits November 5, 2026 04:29
…one paths

Voice notes, video preview frames, notification avatars and Me-chat agent
avatars fetch media outside HostedRelayHttpClient, and push-lease
tombstones open their own relay socket. Hosted relays refuse all of these
without Nostr-Federated-Identity. Pass the assertion on each, scoped to
the relay origin, and refuse redirects for signed native requests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Tom Brow <tomb@squareup.com>
HostedRelayAuth kept one cached assertion, so issuing for an inactive
community (a background push-lease revocation after removing it) replaced
the active community's assertion. Native media paths read only the cache,
so voice notes, video previews and agent avatars then went out without
Nostr-Federated-Identity until another auth operation restored it. Key the
cache by relay and key, and let a refusal evict only the refused token.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Tom Brow <tomb@squareup.com>
The notification avatar loader checked the community scope before asking the
relay assertion issuer for a header, but not again once issuance finished. If
the community was removed or its account key changed meanwhile, the signed
avatar request still went out with the revoked Blossom proof and assertion;
the post-response check only kept the result out of the cache.

Recheck the scope immediately before starting the download and complete with
the revoked failure instead. The transfer is now an internal seam so a test
can observe that no request is started.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Tom Brow <tomb@squareup.com>
@brow
brow force-pushed the tomb/mobile-federated-identity-gaps branch from f18e4c3 to 0f37920 Compare November 5, 2026 12:34
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is 6793cbd25b7f1fa0c3e4750423bbbf4039e2a279...963e8faa3adf514dbcbff013a0ee66bacd33d4c2.
A new review must complete for this exact range. When manual authorization
is required, a user with write access must comment exactly
@buzz-security-review 963e8faa3adf514dbcbff013a0ee66bacd33d4c2 to authorize a new review.
Any previous review applies only to its recorded range.

submitSignedEventOnce(
wsUrl: wsUrl,
// A hosted relay refuses the socket upgrade without an assertion.
headers: await relayAuth.headers(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 P2 · Background revocation hides active auth recovery

This uses the shared HostedRelayAuth for a removed community. If the active community has stopped reconnecting after a terminal denial, a later revocation attempt whose saved key is refused overwrites its single failure slot. The sequence guard permits that newer failure. The recovery banner requires the failure to match the active relay and key, while RelaySession has already disabled automatic reconnects.

User/system effect: The active community remains disconnected, but its Retry and account-change actions disappear because background cleanup failed for another community.

Test: Record an active community's terminal denial, then fail assertion issuance for a queued revocation in another community. The active community's recovery actions should remain available.

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.

🤖 Addressed in 6ec277c: the revocation outbox and community removal now call HostedRelayAuth.headers(config, background: true), which neither records nor clears the shared failure state. The org relay auth test now records the active relay's terminal refusal, fails a background issuance for the other community, and checks the active failure is still there. That step fails without the fix.

…ecovery

Push-lease revocations and community removal issue assertions for a
community that may not be active. Their failures overwrote the single
failure slot, hiding the active community's Retry and account actions
after a terminal refusal. Background callers now neither record nor
clear that state.

Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Tom Brow <tomb@squareup.com>
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
…or backing up keys

Signed-off-by: Tom Brow <tomb@squareup.com>
Signed-off-by: Tom Brow <tomb@squareup.com>
Signed-off-by: Tom Brow <tomb@squareup.com>
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
headers: await relayAuth.headers(
RelayConfig(baseUrl: record.relayUrl, nsec: record.nsec),
background: true,
),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 P2 · Background cleanup can bind an account to a discarded key

A queued revocation can survive removal until the user signs into an organization account with no identity binding. This background call reaches the unbound-organization branch, then relayAssertion establishes the first binding using the removed community’s retained key. Background mode suppresses backup, and successful publication removes the outbox record holding that key.

User/system effect: Cleanup selects the account’s identity before foreground authentication can use the intended different key. That foreground key is subsequently refused by the binding check, and the discarded bound key can become unrecoverable.

Test: Resume a queued removed-community revocation under an unbound organization account, then authenticate a retained community with a different key.

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.

🤖 Addressed in b147bf9: background issuance now refuses an account with no bound identity (denied), so cleanup never creates a binding. The durable revocation keeps its record and retries later. Covered by the new step in organization_relay_auth_test.dart (no assertion is requested).

pending.pubkey == pubkey &&
failedRelay == config.baseUrl &&
failedPubkey == pubkey) {
rethrow;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 P2 · A foreground waiter can inherit an unrecorded terminal failure

The historical failure scope can match this pending operation even when that operation is background work. After a foreground attempt records unavailable, let a background issuance for the same relay/key fail with denied while foreground reconnect waits here. This branch rethrows that denial, but background issuance skips _record, leaving the recorded failure as unavailable.

User/system effect: Reconnect stops on the terminal denial, while the signed-in organization banner hides recovery for unavailable. The same-key waiter therefore remains a recovery gap after the background-state fix.

Test: Record a transient foreground failure, overlap background and foreground requests for that scope, then refuse the background issuance. Recovery should expose the current terminal failure.

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.

🤖 Addressed in b147bf9: pending operations now carry a background flag, and a waiter never rethrows a background operation's failure. It issues for itself instead. The new test records a foreground failure, overlaps a denied background issuance with a foreground reconnect for the same relay and key, and expects the foreground assertion; it fails without the guard.

// A background caller may be removing this community, so its key must
// not be backed up after the user discarded it.
if (!background && identical(account, organization)) {
unawaited(organizationBackup?.afterAssertion(config.nsec!, pubkey));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 P2 · Foreground reuse can skip the organization key backup

Background assertions enter the shared cache without calling afterAssertion. A later foreground headers() call returns that token from the cache and also skips the backup hook. For an inactive retained community with an unbacked key, such as after an earlier backup failure followed by an opt-out tombstone, switching to it before expiry makes foreground authentication succeed without starting the expected backup attempt. The accepted assertion lifetime can be long (the auth table includes a day).

User/system effect: The user actively uses an identity whose recoverable backup has not been established, potentially throughout that cached assertion’s lifetime. This differs from cleanup for a removed community, where backup suppression is intended.

Test: Issue a background assertion for a retained community with an unbacked key, then consume it in foreground authentication and observe that the backup hook is never called.

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.

🤖 Addressed in b147bf9: background assertions no longer enter the shared cache, so foreground use always issues (or reuses) its own assertion and runs afterAssertion. The test checks the backup hook fires on the first foreground use after background issuance.

headers: await relayAuth.headers(
RelayConfig(baseUrl: record.relayUrl, nsec: record.nsec),
background: true,
),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 P2 · Tombstone retries reuse an assertion the relay has refused

Both one-off tombstone paths propagate an HTTP 401/403 WebSocket upgrade refusal without invalidating the assertion used here. If a cached token becomes unacceptable before its advertised expiry, retries keep receiving it from headers(). The live-session path already evicts that token on upgrade refusal, but neither cleanup path does.

User/system effect: An inactive community’s push revocation cannot obtain working credentials until cache expiry. Its lease stays active and notifications can continue despite the cleanup retries.

Test: Cache an assertion, refuse it on the upgrade while allowing a newly issued token, and trigger the next durable revocation attempt. It currently resends the refused token.

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.

🤖 Addressed in b147bf9: background callers bypass the shared cache in both directions, so every tombstone attempt (outbox and removal) gets a freshly issued assertion and never resends a refused or session-cached one. The test checks consecutive background calls, including one after a foreground assertion is cached, each get a new token.

Background issuance (push-lease tombstones for removed or inactive
communities) now never binds an unbound account to the key it carries,
bypasses the shared assertion cache so each attempt gets a fresh
assertion and foreground use never inherits one that skipped the key
backup, and never hands its unrecorded failure to a foreground caller
waiting on it.

Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Tom Brow <tomb@squareup.com>
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
headers: await relayAuth.headers(
RelayConfig(baseUrl: record.relayUrl, nsec: record.nsec),
background: true,
),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 P2 · Optional assertion failure prevents valid push cleanup

During the documented client-before-enforcement rollout, a relay in Off or Shadow mode still accepts NIP-42-authenticated tombstones. This awaited assertion lookup can instead throw first, for example when an organization account is unbound or assertion issuance is unavailable. The socket is never opened, and every durable retry repeats that prerequisite. The community-removal/notification-opt-out publisher in community_provider.dart has the same behavior.

User/system effect: A push lease remains active and notifications can continue even though the relay would accept its signed revocation. Keeping background issuance from binding or backing up keys is necessary; cleanup can still let the relay decide whether an assertion-free signed request is acceptable.

Test: With mobile hosted auth enabled, an unavailable issuer and a non-enforcing relay, removing a community or opting out should deliver the NIP-42 tombstone. An enforcing relay should reject it and retain the retry record.

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.

🤖 Addressed in 963e8fa: a background request that can't get an assertion (unbound account, issuer unavailable, any AccountFailure) now returns no assertion header instead of throwing, so both tombstone publishers (outbox and community removal/opt-out) still open the socket and the relay decides. A non-enforcing relay accepts the NIP-42 tombstone; an enforcing one refuses and the durable retry tries again. Binding and backup stay suppressed. organization_relay_auth_test.dart now expects empty headers in those cases and fails if the fallback is reverted.

A background (cleanup) request that cannot obtain an assertion now sends
no assertion header instead of failing before the socket opens. A relay
that does not enforce assertions accepts the NIP-42 authenticated
tombstone; an enforcing relay refuses it and the durable retry tries
again.

Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Tom Brow <tomb@squareup.com>
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
if let assertion {
request.setValue(assertion.header, forHTTPHeaderField: "Nostr-Federated-Identity")
}
self.download(request, scope: scope, completion: completion)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 P2 · Avatar completion outlives its issuing organization session

If the shared organization session is cleared or replaced while this avatar download is in flight, the saved community and signing key can remain unchanged. download therefore still passes its community-only isCurrent(scope) check and returns success. BuzzNotificationEnrichmentOperation then persists the bytes through cache.saveAvatar, whose authorization guard checks the same community scope. The assertion issuer already exposes isCurrent(assertion) specifically to reject responses from a replaced session, but this path discards that assertion before download completion.

User/system effect: Protected avatar bytes can enter the durable notification cache after their issuing account session has been retired. The resolver's later session check prevents immediate notification delivery, but does not prevent this earlier cache write.

Test: Hold an authenticated avatar response, replace only the shared organization session, then release the response while retaining the community/key. The loader should reject it and leave the avatar cache unchanged.

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.

🤖 Addressed in 850baf1: when the download finishes, the avatar loader checks that the assertion's organization session is still current. If it isn't, it removes the cached response and fails as revoked. The new test replaces the session mid-download, and it fails with the guard disabled.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Carl, an automated reviewer, commenting via Wes's GitHub account.

One recommended blocker at 963e8faa3a: voice notes on Android and video previews only read the cached assertion, so they stop sending it once it expires. Details are in the inline thread. The other inline note is optional.

I re-checked the earlier fixes in HostedRelayAuth (per-relay cache, background waiters, no binding or backup, no recording, empty headers on failure), in the tombstone paths, and in the iOS avatar loader. All of them hold. One correction is in the redirect thread.

Validation gaps: no run against a hosted relay that enforces the assertion; source review plus mutation testing of the HostedRelayAuth guards. Android Mobile and two Swift CI jobs were pending at review time.

Merge criteria: native voice note and video preview reads get a fresh assertion when the cached one has expired (or the limitation is explicitly accepted and tracked), and CI is green.

source,
headers: () =>
ref.read(mediaGetAuthServiceProvider).headersFor(source),
headers: () => ref.read(nativeMediaHeadersProvider)(source),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 P2 · Recommended blocker · Native media reads never refresh an expired assertion

nativeMediaHeadersProvider only reads HostedRelayAuth.cachedHeadersForUrl. Nothing in this PR asks for a new assertion. The cache is refreshed only by a socket reconnect (relay_session.dart:627) or by a HostedRelayHttpClient request. Once the cached assertion passes its expiry (hosted_relay_auth.dart:141-142), cachedHeaders returns null and this closure sends no Nostr-Federated-Identity.

User effect: on Android with an enforcing relay, open a timeline and leave it idle past the assertion lifetime while the socket stays up. Then play a voice note. Android hands the headers straight to the native player (voice_note_recording.dart:842, since requiresAuthenticatedLocalFile is only true on iOS). The relay refuses the request, and every retry hits the same empty cache. Video preview first frames (message_content/video_preview.dart:34) have the same gap. These are the two paths this PR sets out to fix.

Fix: await HostedRelayAuth.headersForUrl at the native boundary. Both call sites are already async: _loadVideoPreviewFrame, and the _load(() => _player.setUrl(...)) closure once the headers callback becomes Future<Map<String, String>> Function(). Treat an AccountFailure as "no assertion" so the relay still decides. The provider test supplies a cached token that never expires, so add an expired-cache case to the existing test rather than a new file. If you'd rather keep the synchronous path, document the limitation and track the refresh as a follow-up.

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.

🤖 Addressed in c08ce7e: voice notes and posterless video previews now use freshNativeMediaHeadersProvider, which awaits headersForUrl and so renews an expired assertion. If the renewal fails, it falls back to the cached headers and the relay decides. The player's headers callback is now FutureOr, and the voice-note download awaits it before its generation check. The tests cover renewal and fallback for the provider, plus a voice-note attachment journey that fails when it is switched back to the synchronous provider. Native reaction-detail avatars still use the cached form; when it has expired they fall back to initials.

relay: Uri.parse(config.wsUrl).replace(path: '').toString(),
pubkey: pubkey,
)];
if (cached == null ||

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Low · Invariant gap: the account check on cachedHeaders is untested

A full-suite mutation run (CI's BUZZ_PUSH_GATEWAY_URL define, green baseline) found that removing !identical(cached.account, _accountFor(config)?.account) fails no test. The map now holds one assertion per community. If this check regressed, native media and saved-profile reads would keep presenting a signed-out or replaced account's assertion until it expires.

This is a security rule, so it fits the focused-invariant exception in AGENTS.md (Mobile testing conventions): add a step to the existing organization_relay_auth_test.dart coverage checking that cachedHeaders is null after sign-out and after an account swap. Optional.

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.

🤖 Addressed in c08ce7e: the cached-reads test now signs out and then restores a replacement account. Both times cachedHeaders must be null. Removing the account check fails it.

@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
@brow
brow force-pushed the tomb/mobile-federated-identity-gaps branch from 2c84940 to 963e8fa Compare November 5, 2026 20:07
@brow
brow merged commit 97ede70 into main Nov 5, 2026
101 of 102 checks passed
@brow
brow deleted the tomb/mobile-federated-identity-gaps branch November 5, 2026 20:32
brow added a commit that referenced this pull request Nov 5, 2026
…ed-session avatars (#9134)

Follow-up to [#9069 media
auth](#9069), which merged at its
approved head. This PR carries the fixes for the last review round on
that PR.

### Changes

- **Native media reads renew an expired assertion** (Carl's [recommended
blocker](#9069 (comment))).
Voice notes and posterless video previews hand headers to native code.
They now use `freshNativeMediaHeadersProvider`, which awaits
`HostedRelayAuth.headersForUrl` and so asks for a new assertion once the
cached one has expired. Before, while the socket stayed up they sent
none and the relay refused the read. If the renewal fails, the read
falls back to cached headers and the relay decides. The voice-note
player's `headers` callback is now `FutureOr`. Both the iOS
authenticated download and the Android native load await it, then
recheck their generation, so a cancel during a slow renewal never starts
the stale load.
- **Notification avatars drop a retired organization session** ([review
thread](#9069 (comment))).
When an avatar download finishes after the organization session that
issued its assertion was replaced, BuzzPushKit removes the cached
response and refuses the avatar.
- **Test:** cached reads never present a signed-out or replaced
account's assertion ([optional
note](#9069 (comment))).

Not changed: native reaction-detail avatars still use the cached
assertion. After it expires they fall back to initials.

### Validation

- Each new test was checked to fail with the code it guards reverted:
- `replacedOrganizationSessionRefusesAvatar` (BuzzPushKit): a session
replaced mid-download must be refused.
- `fresh native media headers renew an expired assertion`: covers
renewal and the fallback when renewal fails.
- `remote voice notes renew an expired relay assertion`: a journey
through `VoiceNoteAttachment`; it fails when the attachment is switched
back to the synchronous provider.
- `Android defers remote auth until playback starts`: now cancels during
a slow renewal and expects no native load.
- `an issued assertion serves cached reads for the relay`: now signs out
and restores a replacement account. It fails without the account check.
- `swift test` (69 tests) passes. The full mobile suite (`just
mobile-install mobile-check mobile-test`) passes with these commits on
main as of [#9069 media auth](#9069).
- The automated full-app simulator run from [#9069 media
auth](#9069) covers the surrounding
flows. This change is limited to the header source for voice notes and
video previews, and none of the test communities enforce the hosted
assertion, so a simulator run would not show the difference. No
end-to-end run against a relay that enforces the assertion.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Tom Brow <tomb@squareup.com>
Co-authored-by: Tom Brow <tomb@squareup.com>
Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
brow added a commit that referenced this pull request Nov 7, 2026
…9225, #9228 to 0.27) (#9232)

Backport to `mobile-v0.27` of the mobile fixes that let iOS release builds turn on hosted NIP-FI (`BUZZ_HOSTED_RELAY_AUTH`) for the Nov 10 launch. [squareup/buzz-releases#142](squareup/buzz-releases#142) turns the flag on for 0.27.1 candidates after this lands.

## Picks

Each is `git cherry-pick -x --signoff` of the squash commit on main, in this order:

- #9056 (6b8a72e): send hosted relay auth with iOS custom emoji downloads.
- #9069 (97ede70): send the hosted relay assertion from native media and tombstone paths.
- #9134 (1b9b307): renew expired assertions for native media and drop retired-session avatars.
- #9225 (b58ee08): request a hosted relay assertion only when the relay's NIP-11 document requires NIP-FI. Without it, the flag-on build is denied by every hosted community that hasn't enabled NIP-FI.
- #9228 (794ee64): the notification extension mints hosted assertions after a relay's NIP-FI challenge, so hosted pushes keep their text once a community enforces.

One 0.27-only commit, `test(mobile): drop org key backup checks from the #9069 backport`: 0.27 has no organization key backup (#9029), so the backported organization relay tests keep only their assertion and cache checks. No production code differs from main's versions.

All picks applied without conflicts.

## Risk

Hosted assertions are sent only when the build sets `BUZZ_HOSTED_RELAY_AUTH`, which current App Store builds don't. With the flag off, the one new behavior from #9228 is clearing a hosted session an earlier build might have shared with the notification extension. Organization relays keep their issuer. Each PR's description on main covers its own testing.

## Testing

- `dart format --set-exit-if-changed`, `flutter analyze`: clean.
- Full `flutter test` with CI's `BUZZ_PUSH_GATEWAY_URL` define: all pass (4,386).
- CI's configured-auth suite (`BUZZ_AUTH_DOMAINS=buzz.xyz,builderlab.xyz,builderlabstaging.xyz`) plus `test/shared/hosted`, `test/features/onboarding` and `test/shared/verified_profiles`: all pass (355).
- `swift test` in `ios/BuzzPushKit`: all pass (110 XCTest, 70 Swift Testing).
- Automated app run: an iOS simulator build of this branch with the App Store defines plus `BUZZ_HOSTED_RELAY_AUTH=true`, installed over a signed-in hosted account. The hosted community connects with no denial banner, the channel list loads, and a channel's history, avatars and media load. The notification extension can't be exercised on the simulator; BuzzPushKit tests cover it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)


Signed-off-by: Tom Brow <tomb@squareup.com>

This branch was successfully deployed

No deployments
codex-review — 963e8faa Deployed Nov 5, 2026 by brow via Run Codex Security Review #12988
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.

5 participants