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

feat: enforce configurable relay trust policy - #1939

Open
kalvinnchau wants to merge 21 commits into
mainfrom
am/relay-trust-policy
Open

kalvinnchau wants to merge 21 commits into
mainfrom
am/relay-trust-policy

Conversation

@kalvinnchau

@kalvinnchau kalvinnchau commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Builds can pin one relay and allow a fixed set of others through the BUZZ_BUILD_RELAY_POLICY build variable. When the variable is unset (OSS builds), behavior is unchanged and every relay is allowed.

Configuration

export BUZZ_BUILD_RELAY_POLICY='{"pinned":"relay.example.com","allowlist":["other.example.com:8443"]}'
bin/pnpm tauri build
  • pinned: the relay every identity starts on. It is always allowed and cannot be left or removed.
  • allowlist: additional relays users may join. An empty array allows only the pin.
  • Each entry is host or host:port and allows that host over both wss:// and https://. A bare host means port 443.
  • Subdomains, suffixes and other ports are different relays.
  • An allowlist entry *.suffix or *.suffix:port allows any host exactly one DNS label under suffix on that port, such as a.communities.example.com for *.communities.example.com. It does not allow suffix itself or deeper subdomains, the suffix needs at least two labels, and the pin is always exact.
  • Malformed input fails the Vite and Cargo builds; it never falls back to unrestricted.

Behavior

  • Startup: the pin is added to the saved communities. A fresh identity, or a saved selection that is now blocked, lands on the pin. A saved Personal space choice is kept.
  • Blocked relays: joins, invites, account-sync additions and direct transport calls are refused. Saved memberships to blocked relays are hidden and never contacted.
  • Preservation: stored agent settings, teams and unfinished joins for blocked relays stay on disk, so a later build that allows the relay resumes them.
  • Enforcement points: shared community/transport code (communityDestination), the development relay broker, and the native host (relay origin, signing, HTTP, pairing, agents, login gate).

Tests

  • Vitest relay-policy.test.ts: absent policy is unrestricted, malformed policy fails, host matching across schemes and ports, fresh-install pin, blocked join and pinned leave refused, blocked saved relay never contacted but kept in the stored record and restored by a build that allows it, legacy import re-offers only policy-skipped relays and keeps observed removals, Personal space kept, blocked unfinished joins retained.
  • Vitest agents/service.test.ts: blocked agents keep settings and teams and never bind or publish to blocked relays.
  • Rust relay_policy unit tests: parsing, host/port matching, pinned removal.
  • Chromium/WebKit journey relay-policy.spec.mjs: compiled policy through the app and broker, including browser storage restoration.

Deferred

  • Human behavior confirmation.
  • Packaged native GUI and OS-store integration, real-relay membership, long-running blocked-agent timers.
  • Native IPC call-site wiring has no compiled-policy test; the shared native checker is unit-tested.

@kalvinnchau
kalvinnchau force-pushed the am/relay-trust-policy branch from 8ce150c to face144 Compare October 31, 2026 18:29
@kalvinnchau
kalvinnchau marked this pull request as ready for review October 31, 2026 18:37
@kalvinnchau
kalvinnchau requested a review from a team as a code owner October 31, 2026 18:37

@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.

🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

One P2 finding is attached: enforce the compiled policy at the native socket IPC boundary.

Optional UI polish: disable or hide Leave/Remove for the pinned community and explain why it is required; the current refusal correctly runs before side effects and does not report success.

Reviewed head 5f8450a26fc8b8f571f17741c0bfc951372003b2 against base d58bb14a1bf3c577081ffd3e7c42830f43ee1016. Source-only: no PR code or tests executed locally. A completed hosted Chromium flake job reports 34 passes, including ten policy-journey repetitions (6.6–7.5s each); the broader CI snapshot still had running jobs. Packaged native behavior, compiled-policy native IPC wiring, OS integration and human acceptance remain unverified. The public PR description has no attachments; no concrete disclosure issue found in the inspected description or commit messages.

Comment thread src/features/relay/live.ts Resolved

@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.

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

The prior P2 is resolved in source. relay_socket_connect now checks the compiled policy immediately after canonicalization, before reserving a socket or entering discovery, assertion acquisition or upgrade. Rejection also cannot trigger the renderer’s public-WebSocket fallback, which requires a successful public response. No new actionable findings in this one-line correction.

Reviewed head 422aba104de46235c0f634f7242272d943afc73d against base d58bb14a1bf3c577081ffd3e7c42830f43ee1016, focusing on the delta from previously reviewed 5f8450a26fc8b8f571f17741c0bfc951372003b2. Source-only: no tests, builds or app execution performed. The compiled-policy IPC regression remains explicitly deferred; the existing checker tests do not establish call-site wiring. Author-reported checks/probe were not independently reproduced, and packaged native behavior remains unverified.

@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.

Verdict: one correctness blocker; otherwise looks good. I traced every native relay call path I could find through relay::origin/request_url, relay_socket_connect, pairing::community and AppAgentHost::agent, and found no policy bypass. Policy parsing fails closed and matches between TS and Rust. The blocker is inline: blocked saved communities are deleted from storage rather than hidden.

Merge criteria: blocked memberships survive a persisted write and a policy-relaxing upgrade, with a test that asserts the stored record.

Validation gaps (non-blocking, already noted in the PR): there's no compiled-policy coverage of the native IPC call sites, and no long-running blocked-agent check. This review was done by reading the source; I didn't run it.

Comment thread src/features/communities/service.ts Resolved

@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.

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

The saved-membership erasure is fixed: blocked memberships survive persisted writes and return when a later build allows them. The added test checks the stored record and restart. The earlier native socket policy fix remains intact. One new low-priority P3 is attached: the legacy retry marker can forget an observed removal and offer that community again after policy relaxation. Import still requires explicit consent; no automatic restoration or policy bypass was found in this scenario.

Reviewed head 68d0948586926bd1ec26815f036c31c9877a9be4 against base d58bb14a1bf3c577081ffd3e7c42830f43ee1016, focusing on changes since 422aba104de46235c0f634f7242272d943afc73d. Source-only: no tests, builds or app execution. Author-reported checks were not independently reproduced. The removal/retry transition lacks regression coverage; packaged native behavior and compiled-policy IPC wiring remain unverified. A single CI snapshot had two failed Chromium jobs and other jobs still running; those failures were not triaged in this source-only pass.

@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.

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

The prior P3 is resolved in source. Legacy relay addresses are normalized independently of policy and checked against observed removals before entering the skipped list. Pending partial markers are pruned when new removals are observed, and the final write excludes removals observed during consent/save. No new actionable findings in this correction.

The added regression assertions cover an existing removal marker under a restrictive policy and pruning a partial marker before a failed read. They were inspected, not executed; the full acknowledgement/restart journey was not exercised here.

Reviewed head a292e984fea05d990f418445ffea4a1d8065848a against base d58bb14a1bf3c577081ffd3e7c42830f43ee1016, focusing on the two-file delta from 68d0948586926bd1ec26815f036c31c9877a9be4. Source-only: no tests, builds or app execution. Author-reported checks were not independently reproduced; packaged native behavior and compiled-policy IPC wiring remain unverified.

@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.

🤖 Star Lord’s automated follow-up source review via Wes’s account.

No actionable findings. The browser journey now expects https://blocked.example to remain in the persisted membership record after selecting Secondary, matching the reviewed preservation behavior. The existing assertion still requires that blocked membership to be absent from the switcher, so the correction checks storage retention without weakening the UI exclusion.

Reviewed head 750530483d450903bbb89c9246b9994aad059218 against base d58bb14a1bf3c577081ffd3e7c42830f43ee1016. This solo follow-up covers only the one browser-test expectation changed since team-reviewed a292e984fea05d990f418445ffea4a1d8065848a; production code is unchanged. Source-only: no tests, builds or app execution. Browser results were not independently reproduced, and the previously disclosed native/runtime validation gaps remain.

@kalvinnchau
kalvinnchau force-pushed the am/relay-trust-policy branch from 7505304 to 6223f53 Compare November 2, 2026 22:43

@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.

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

No new actionable findings. The rebase preserves the previously reviewed policy changes and the fixes for native socket enforcement, blocked-membership retention and legacy-removal tracking. We checked the updated host, community and agent integration paths; the policy gates remain ahead of the inspected relay side effects.

Reviewed head 6223f537853b974b79549aa5ff9f2487c65d889a against base 263b618e6c17c3b2bf787997b05f98d65d4ac330, comparing with previously reviewed 750530483d450903bbb89c9246b9994aad059218. Source-only: no tests, builds or apps executed, and no CI assessment in this cycle. Packaged native behavior, compiled-policy IPC call-site coverage, OS-store integration, long-running agent behavior and human acceptance remain unverified. The public description and commit metadata were inspected; the description has no attached images.

@kalvinnchau
kalvinnchau force-pushed the am/relay-trust-policy branch from 6223f53 to 82895bc Compare November 3, 2026 04:38

@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.

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

No new actionable findings. The rebase preserves the policy changes and the earlier fixes for native socket enforcement, blocked-membership retention and legacy-removal tracking. We checked integration with the updated native HTTP cancellation/signature-verification paths, broker streaming and agent tabs; the inspected relay operations remain behind the policy gates. Previously declined optional UI polish and compiled-policy IPC coverage remain deferred.

Reviewed head 82895bcf4f1d5665973656a722a079f5e5ee4f42 against base b12cad3e718140afab8989699aa9a9604192f853, comparing with previously reviewed 6223f537853b974b79549aa5ff9f2487c65d889a. Source-only: no tests, builds or apps executed; CI was not assessed. Packaged native behavior, compiled-policy IPC call-site coverage, OS-store integration, long-running agent behavior, runtime focus, performance and human acceptance remain unverified. The public description and commit metadata were inspected; the description has no attached images.

@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. Full Court review (native, frontend and adversarial challenge lanes).

Verdict: the policy itself holds; one user-facing defect worth fixing before merge, plus two smaller ones (inline). No policy bypass found. Native relay HTTP, sockets, signing, media, pairing, admin and agent paths all reach a policy gate. The broker gates scoped and unscoped dispatch. Account-sync rows for blocked relays are dropped, never enqueued as removals. TS/Rust canonicalization matches on scheme, default port, case, bracketed IPv6, trailing dots and credentials.

Merge criteria: in a policy build, typing or pasting a blocked relay on the Welcome and Join pages shows the policy message without a Retry button. The agent findings are optional.

Product question for the author or Wes, not a finding: the seeded pin is a local membership, so mergeList (known-communities.ts:117-123) uploads it to the Builderlab account list, and the user's unrestricted devices then add it too. If that's intended, a sentence in docs/configuration.md would help.

Optional: if someone downgrades to a pre-policy build, it reads the 1:<addresses> legacy marker as an unfinished import and may offer removed legacy communities again. Joining still needs consent.

Validation: source review at 82895bc against b12cad3. The frontend issue was reproduced with a throwaway Vitest file that stubbed the policy; nothing else was run locally. CI snapshot: 43 checks passing, Windows native skipped. Compiled-policy native IPC, packaged GUI and human acceptance remain deferred, as the PR states.

Comment thread src/features/communities/destination.ts Resolved
Comment thread src/features/agents/service.ts Resolved
Comment thread src/bundled/agents/AgentDetails.tsx Resolved
@kalvinnchau

kalvinnchau commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Responses to the two review-body points:

  • Downgrade to a pre-policy build: declining. A pre-policy build reads the 1:<addresses> marker as an unfinished import and may offer those legacy communities again. Joining still requires consent, so nothing joins silently. Making older builds read a newer marker would need a change in those older builds.
  • Seeded pin in the account list: fixed in 30f78c1. The pin is still added on each device, but mergeList no longer uploads it to the account list. This is documented in docs/configuration.md.

@kalvinnchau

kalvinnchau commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Correction to my earlier comment: this is now fixed in 30f78c1, not deferred. The seeded pin should not reach the account's other devices.

  • mergeList takes the relay policy, with the build's policy as the default, and skips the pin when it queues unsynced memberships for upload. Other memberships still upload. If the account list already contains the pin, nothing changes.
  • An explicit join through the join flow still queues its usual upload. Only the seeded pin is excluded.
  • docs/configuration.md now says the seeded pin is never uploaded to the Builderlab account list.
  • known-communities.test.ts has a new regression test, and it fails without the change.

Verification: tsc and biome are clean, and Vitest passes for src/features/communities, src/features/identity and src/bundled/builderlab (1,385 tests).

@kalvinnchau
kalvinnchau force-pushed the am/relay-trust-policy branch from 4e91d0b to 7285530 Compare November 3, 2026 18:31

@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.

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

Two P2 findings in the new agent-deletion preflight are detailed inline. The blocked-address lookup now explains the policy refusal without offering Retry, and the initial account-sync merge excludes the seeded pin. Previously declined optional membership-display and native IPC coverage suggestions remain deferred.

Reviewed head 30f78c1f82087b44a5edf3529d272b64f364efa0, base 589c2a489fe87ab45315359014d7d46d2df4a240 (merge-base b12cad3e718140afab8989699aa9a9604192f853), with the follow-up compared directly to 82895bcf4f1d5665973656a722a079f5e5ee4f42. Source-only: no tests, builds or apps executed. The CI snapshot was still running, with Windows native validation skipped. Packaged native behavior, compiled-policy IPC, actual deletion/restart, runtime focus, performance and human acceptance remain unverified. Public description and commit metadata inspected; no description attachments.

Comment thread src/features/agents/service.ts Resolved
Comment thread src/features/agents/service.ts Resolved

@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.

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

The prior empty-config deletion finding is resolved in source. No new independent findings. The existing P2 storage-key finding remains: policy-dependent wss:// versus https:// keys can hide a persisted config binding. With the home departed, the narrowed guard can consequently skip the required home-config archive. The new test injects bindingFor, so it does not cover that persisted lookup. The existing repair advice and regression criteria still apply; no duplicate inline comment added.

Reviewed head 6bee6f1a608629923714ebc8e5e28ad0ad0edb3c, base 589c2a489fe87ab45315359014d7d46d2df4a240, focusing on the two-file delta from 30f78c1f82087b44a5edf3529d272b64f364efa0. Source-only: no tests, builds or apps executed; CI was not assessed. Actual deletion/restart, packaged native behavior, runtime focus, accessibility and human acceptance remain unverified. Public description and commit metadata inspected; no description attachments.

@kalvinnchau
kalvinnchau force-pushed the am/relay-trust-policy branch from ba5d9c2 to 781602f Compare November 3, 2026 20:15

@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.

🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

No new actionable findings. The prior storage-key P2 is resolved in source: membership/home identity now canonicalizes independently of relay policy, so persisted HTTPS config bindings are found before the Delete preflight, while connection paths retain policy checks. The regression seeds an actual persisted config with a native WSS home row and asserts refusal before native deletion starts. The rebase preserves the feature patches; the affected integration paths were inspected. Previously declined membership-display and native IPC coverage suggestions remain deferred.

Reviewed head 781602f9843a0a4cd34ee92e305f8ecc0221497e, base 066e34f12fcad50e8d636e7da02cbade3b5e6fad, following the published review at 6bee6f1a608629923714ebc8e5e28ad0ad0edb3c. Source-only: no tests, builds or apps executed; CI was not assessed. Actual deletion/restart, packaged native IPC, runtime focus/accessibility and human acceptance remain unverified. Public description and commit messages inspected; no description attachments.

@kalvinnchau
kalvinnchau force-pushed the am/relay-trust-policy branch from 781602f to 16961a6 Compare November 5, 2026 21:14

@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.

🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

No new actionable findings in the rebase integration reviewed at head 16961a6631fdaca9fd68511adea7ab65b3a43662, base f6b0c2c62cec1536324cc07de65cf706ed9ea35a (merge-base 54cffc14e42a0ba54493566856f13a25e964c0de), relative to the previously reviewed 781602f9843a0a4cd34ee92e305f8ecc0221497e.

The prior native socket, blocked-membership preservation, legacy-removal, pin-sync and agent home-key/Delete repairs remain intact. We traced the inherited native/broker and runner changes, community lookup/auto-join, and legacy-import/backup-prompt coordination against the policy boundaries. Previously declined or deferred suggestions remain deferred.

Non-blocking product clarification: should a policy-seeded Builderlab-hosted pin count toward automatic backup consent? src/features/communities/service.ts:345–354 adds the pin to saved memberships even with Personal space selected. On a non-managed, backup-configured build, an existing key with no saved choice and a Builderlab session can therefore reach decideBackup()’s save branch and setBackupChoice(..., true, true) in src/features/identity/BackupConsent.tsx, without a prompt or an explicit join to that pin. This follows the literal saved-host rule in docs/identity.md; we are not treating it as a demonstrated contract violation. Native upload still requires the account’s linked key to match (src-tauri/src/nsec_backup.rs:571–578), and an existing saved choice stands. Making the intended seeded-pin behavior explicit would clarify the interaction.

Validation: immutable source and regression assertions inspected only; no tests, builds, apps or PR code executed. CI, packaged native/OS-store behavior, actual backup upload, runtime focus/accessibility and long-running agent behavior were not verified. The compiled-policy native IPC coverage remains deferred. Public PR description inspected; no attached images were present.

@kalvinnchau
kalvinnchau force-pushed the am/relay-trust-policy branch from 16961a6 to 616709d Compare November 5, 2026 23:26

@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.

🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

No new actionable findings in the focused rebase integration at head 616709d4443788d1e34c09c079bbff1680c68477, base 028e5a13d3b85eb8f042b80109e96514d622c783 (merge-base 7037441ad50e6dc327ab6a2b1fe40fedfb0d67f1), compared with the previously reviewed 16961a6631fdaca9fd68511adea7ab65b3a43662.

The blocked-home Delete check still finds persisted config through policy-independent home keys and refuses before native deletion starts. The inherited refresh recovery and missing-config fallback preserve that order; remote inventory metadata retains blocked rows. Native workflow-coordinate deletion still passes the relay-policy check before signing, and the changed no-community 404 classification remains downstream of the lookup’s early blocked-relay refusal. Earlier repairs and explicitly declined/deferred suggestions remain intact. The prior seeded-pin backup clarification remains optional; this rebase does not change that interaction.

Validation: immutable source and relevant regression assertions inspected only. No tests, builds, apps or PR code executed; CI was not assessed. Actual deletion/restart, packaged native signing/IPC and OS-store behavior, runtime focus/accessibility, macOS window activation and long-running agent behavior remain unverified. Public PR description and feature commit messages inspected; no description attachments.

@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.

🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

No new actionable code findings in the wildcard follow-up at head 3f07a9ecc5f5b538842b53c82982fe3bfc669de3, base 028e5a13d3b85eb8f042b80109e96514d622c783, compared with reviewed 616709d4443788d1e34c09c079bbff1680c68477.

The frontend/broker and native implementations compare canonical suffix origins including the port, require one nonempty additional label, and reject the bare suffix, deeper subdomains and different ports. The pin remains exact, and existing destination, lookup, import and native boundaries retain their checks. Prior repaired and explicitly declined/deferred items remain preserved.

Non-blocking documentation note: the PR description still says “There are no wildcards.” Please align it with docs/configuration.md: allowlist entries now support one-label *.suffix[:port] patterns, while the pin stays exact.

Validation: source and mirrored frontend/Rust regression assertions inspected only. No tests, builds, apps or PR code executed; CI was not assessed. Packaged native enforcement, real-relay connections, restoration/restart and runtime UI behavior remain unverified. Public description and new commit message inspected; no description attachments.

@kalvinnchau

Copy link
Copy Markdown
Contributor Author

🤖 Updated the PR description to match docs/configuration.md. Allowlist entries may now be one-label *.suffix or *.suffix:port patterns. These don't allow the bare suffix or deeper subdomains, the suffix needs at least two labels, and the pin stays exact.

@wpfleger96 wpfleger96 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.

🤖 Combined review from two automated reviewers working for Will (one did a source read, the other ran the startup and account-merge logic at this head in a focused Node probe with the surrounding services stubbed).

Verdict: two correctness defects block approval. Both come from how the policy interacts with the existing packaged default, BUZZ_DEFAULT_COMMUNITY_URL. Details are inline.

  1. On a fresh identity, an allowed packaged default is selected instead of the pin.
  2. When the packaged default is the pin itself, the pin membership is created without seeded: true, so mergeList uploads it to the account list without any explicit join.

Both are reachable in production builds: vite.config.ts compiles BUZZ_DEFAULT_COMMUNITY_URL into VITE_BUZZ_OPEN_RELAY for build, and src/app/services.ts passes it to createCommunities. A build that sets both the default and the pin to the same relay (likely the common setup) hits the second defect. The policy startup helper in relay-policy.test.ts always passes an empty openRelay, so the tests miss this. Regressions for an empty, matching, allowed-but-different and blocked default would cover it.

Everything else holds up: the TS and Rust parsers agree on canonicalization and the wildcard rule, malformed input fails both builds, the native gates (relay::origin/request_url, relay_socket_connect, pairing::community, agent commands) run before any network call, leaving the pinned relay is refused on the signing, HTTP and socket-frame paths, and an explicit joined() clears the seeded flag.

One nonblocking nit is inline (IP-literal wildcard suffixes).

Comment thread src/features/communities/service.ts Resolved
Comment thread src/features/communities/service.ts Resolved
Comment thread src/features/communities/destination.ts Resolved

@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.

🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

Follow-up at head 157d6261b7a7deba51a268b57c65f4a01f9568fc, base 028e5a13d3b85eb8f042b80109e96514d622c783, compared with reviewed 3f07a9ecc5f5b538842b53c82982fe3bfc669de3.

No new actionable findings. All three independent lanes completed their source and context checks and stopped.

The two packaged-default defects identified in Will’s review are repaired in source: a fresh identity selects the pin even with another allowed default, and a default equal to the pin gets the persisted seeded marker. Account merging excludes that marker after the policy changes or disappears; ordinary join callers replace the membership and queue its upload. The new regressions cover these combinations, which our earlier pass missed.

The native socket retains its admitted relay and checks a leave against that target before enqueueing it. Native socket EVENT frames and HTTP /events now reject non-integer or out-of-range kinds before dispatch. Both policy parsers reject IP-literal wildcard suffixes. The prior wildcard-description note is resolved; earlier repaired and explicitly declined/deferred items remain preserved.

Validation is source-only: regression assertions inspected, no tests, builds, apps or PR code executed; CI was not assessed. Packaged IPC wiring, real-relay leave behavior and runtime storage/restoration remain unverified. The socket protection covers native sends; the existing public WebSocket fallback remains outside that check, while normal Leave calls retain the signing/HTTP guards. Public description and new commit messages inspected; no description attachments.

@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.

🤖 Star Lord automated source review via Wes’s account, with independent Rocket, Mantis and Groot lanes, at b0a4402a0c774b94302772ee238e4852cfa539f5 (base cd9090ee07dcffe8737543dcef74bd8d5899962d), following 487a5554.

The local-team migration P2 is repaired in source; no new actionable findings. homesOf now counts the viewer’s retained, non-deleted native memberships at blocked relays using the same canonical origin parser as admitted communities. For allowed A plus blocked B, both migration and teamSync see multiple communities: the team stays local before any migration journal, remote Save, template replacement or local-copy removal. Ownership filtering and connection admission remain intact; unrestricted builds retain the prior calculation.

The new service regression combines dual saved memberships with a policy blocking B and config support at A. It checks the multi-community status, no migration Save or journal, and retained local storage. We inspected those assertions; the author reports fail-before/pass-after execution, which we did not independently run. Policy relaxation and a later B-only member edit are supported by the source path retaining the local team, rather than an exercised workflow in this review. The repair covers saved native memberships; it cannot infer memberships known only from unavailable hosted listings at a blocked community.

This was a focused two-file repair follow-up. Earlier coverage, repairs and declined/deferred decisions remain, including the separate inherited external ownerAccess issue. No tests, builds or app workflows were executed. Compiled-policy native IPC, packaged upgrades and runtime behavior remain unverified.

@tellaho

tellaho commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

🤖 Drip design review at b0a4402a0c774b94302772ee238e4852cfa539f5: no design blockers found in source.

This keeps people using a managed build within its permitted communities while retaining saved work. Blocked addresses and invites receive a policy refusal without Retry; pinned Leave refuses before side effects or a device-removal offer. Existing join, error and agent settings surfaces handle the change.

Source review only. No screenshots were available in the inspected PR description/comments. Tests and app workflows were not run; rendered states, light/dark themes, widths/enlarged text, keyboard/focus, many-agent layouts and packaged native behavior remain unverified.

@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.

🤖 Star Lord automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage, at b3fa56a7c4c15d5b62a66d7e1f6abe6b87b3802f (base/merge-base 3a1eb5601e997811009d01601786df8af9393a7f).

No new actionable findings in this bounded integration follow-up. The rebase retains the 21 reviewed feature patches, including the saved-membership/team-migration and owner-pairing repairs. Invitation parsing remains separate from permission: blocked destinations reach the policy refusal before relay requests. The new submitted community-join caller checks policy before opening the flow. Changed artifact discovery, remote-refusal classification and credential-prompt lifecycle retain the existing host admission/signing gates.

Earlier findings, repairs and explicitly declined/deferred items remain as previously recorded. This checks meaningful integration since the covered ef05e8202 rebase; it is not a fresh audit of every upstream change. The inherited external ownerAccess issue remains a separate follow-up.

Source-only: source and relevant regression assertions were inspected, not executed. No tests, builds, app workflows or CI checks were run. Compiled-policy native IPC, packaged upgrades, actual network behavior and rendered focus/accessibility remain unverified. Public PR description and feature commit messages were inspected; the description has no attached images.

@tellaho

tellaho commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

🤖 Could you add a screenshot or short video of the change to the PR? Design uses these for faster reviews.

kalvinnchau and others added 21 commits November 9, 2026 20:31
compile an optional pinned relay and allowlist into frontend and native hosts. enforce policy at destination, persistence, transport, broker and signing boundaries.

Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
retain blocked agent identities and unfinished joins while gating runtime access. cover policy refusals and run compiled native ipc enforcement in ci.

Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
filter import sources and shared-settings destinations by relay policy. preserve complete membership counts and render blocked portraits with existing fallbacks.

Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
declare the shared build input for message journeys so collection succeeds across browser projects.

Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
policy entries are host or host:port and match both wss and https. drop the
compiled-policy ci step, native ipc test, shared url corpus, broker matrix and
agent render tests; consolidate the remaining functional coverage.

Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
relay_policy canonicalizes origins with it; main gated the module to tests after removing its last app caller.

Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
relay_socket_connect canonicalized the relay but reached discovery, assertion and upgrade without consulting the compiled policy, so a direct ipc call could contact a blocked relay. check the policy before reserving the socket.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
saved memberships blocked by the relay policy were dropped on load, so the next persisted write erased them and a build that allows the relay could not restore them. keep them in the unresolved membership list with aliases, hidden but stored. a legacy import that skipped blocked relays now records them and offers only those once a build allows them, instead of marking the import complete.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
a relay removed before or during a policy-skipped import could land in the retained skipped list and be offered again once a build allowed it. match removals by policy-independent address before recording a skipped relay, and prune newly observed removals from a pending skipped list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
the relay policy journey asserted the blocked membership was dropped from storage; it is now kept hidden so a build that allows the relay restores it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
find a community refuses a blocked relay as a non-retryable "blocked"
failure before any request, instead of reporting it unreachable with a
retry that can never succeed. invite parsing resolves the relay without
the policy so blocked invites reach the join dialog and lookup, which
show the policy message, rather than failing as invalid links.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
a blocked home never connects, so delete could never archive the config
kept there and every retry failed blaming the connection. refuse up front
with the policy reason before anything is saved.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
a policy build seeds its pinned community as a local membership, so mergeList uploaded it to the builderlab account list and the account's unrestricted devices added it too. skip the pin when queueing unsynced memberships for upload; other memberships still upload.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
archiving the home config is a no-op without a config binding, so an
agent that left a blocked home with no synced config still deletes
cleanly. limit the refusal to agents whose config the blocked home keeps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
homeOf and membership keys fell back to the raw relay string when the
policy-aware origin check refused a blocked home, so a native wss:// row
missed config bindings saved under its canonical https:// origin. delete
then passed its preflight and stuck after saving, and allowed memberships
could miss their synced config. storage identity now resolves without the
policy; network access keeps the policy checks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
allowlist entries may be *.suffix or *.suffix:port. a wildcard permits
hosts exactly one dns label under the suffix on that port, not the
suffix itself or deeper subdomains. suffixes need two labels, other "*"
use is rejected, and the pin stays exact. rust and frontend parsers
match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a nip-43 leave names no relay, so relay_socket_send now checks
relay_policy::removable against the socket's own relay before
forwarding a kind 28936 event frame. a leave signed for another
community can no longer leave the pin through its native socket.

the policy-seeded pin membership now persists a seeded marker, and
mergeList skips seeded memberships instead of comparing against the
current build's pin. the exclusion survives a later build that drops
or changes the policy; an explicit join replaces the membership and
uploads it as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
the relay narrows an event kind to u16, so a leave sent as kind 94472
or 28936 + 2^32 passed the raw u64 comparison yet arrived as a validly
signed 28936. relay::event_kind now yields the kind only as a u16 and
refuses non-integer or out-of-range kinds; the native socket's
leave_frame and the http /events gate both use it. leave detection now
runs before the socket lock is taken.

a default or auto-open community equal to the policy pin was inserted
unmarked, so the policy seed skipped it and account sync uploaded it.
that insertion now carries the seeded marker when it is the pin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a fresh record seeded by an allowed packaged default kept that default
selected, so the policy branch never selected the pin. a fresh install
now always selects the pin; the default stays a membership, and a
restored selection or personal space is unchanged.

wildcard allowlist suffixes must be dns names: an ipv4 literal such as
*.1.2.3.4 can never match and now fails the build in both parsers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
a pairing token restored from an earlier build reached owner_call
without checking its relay against the compiled policy, so after an
upgrade to a build that blocks that relay it could still obtain human
signatures for it. owner_request now refuses a blocked record relay
right after admission, before any owner key use, as the agent path
already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
homesOf derived a member's communities only from bound ones, and the
relay policy never binds a blocked community. a local team whose agent
is saved in an allowed and a blocked community therefore looked
single-community and migrated to the allowed one, changing its editing
rules permanently. homesOf now also counts the viewer's live
memberships on blocked relays; builds without a policy are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
@kalvinnchau
kalvinnchau force-pushed the am/relay-trust-policy branch from b3fa56a to 5a3e3e7 Compare November 10, 2026 04:36
@kalvinnchau

Copy link
Copy Markdown
Contributor Author

🤖 @tellaho here are screenshots of the policy states, at head 5a3e3e7. They come from Chromium running the relay-policy browser journey build with {"pinned":"primary.example","allowlist":["secondary.example"]}, at 1280×800.

1. The community switcher with a blocked saved community. The saved membership on blocked.example was selected at startup. The app restores the pinned relay instead, and the blocked community isn't listed.

switcher, light
switcher, dark

2. Join a community, with a blocked address. The field error explains the policy refusal. There is no Retry and no request goes to the relay.

join refused, light
join refused, dark

3. Leave on the pinned community. It's refused with a toast before any side effects.

leave refused, light
leave refused, dark

These cover only light and dark at one width. Narrow widths, enlarged text and the native app weren't captured.

This branch has not been deployed

No deployments
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.

4 participants