From 4d25c884c7e68527f4e531aa305f7eb7fc06bec3 Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Sun, 19 Jul 2026 13:19:29 +0300 Subject: [PATCH] emit moderation labeler only via global appLabelers The per-client labelers list also carried api.moderation.did, which the header builder emits plain alongside the global did;redact entry - a duplicate that could weaken redaction. Per-client lists now carry only account subscriptions. Co-Authored-By: Claude Fable 5 --- .../session/__tests__/clients-bundle-test.ts | 29 ++++--- src/state/session/clients.ts | 74 ++++++++---------- src/state/session/moderation.ts | 78 ++++++++++++------- 3 files changed, 102 insertions(+), 79 deletions(-) diff --git a/src/state/session/__tests__/clients-bundle-test.ts b/src/state/session/__tests__/clients-bundle-test.ts index 6a798e8f02..1f3265ca47 100644 --- a/src/state/session/__tests__/clients-bundle-test.ts +++ b/src/state/session/__tests__/clients-bundle-test.ts @@ -124,7 +124,7 @@ function makeSession( } describe('buildAppviewClient', () => { - it('sets the appview atproto-proxy header and includes the moderation DID in labelers', async () => { + it('sets the appview atproto-proxy header and includes only the per-instance labelers', async () => { const {seen, fetchMock} = makeCapturingFetch() const session = makeSession(fetchMock) const client = buildAppviewClient(session, [CUSTOM_LABELER]) @@ -134,8 +134,14 @@ describe('buildAppviewClient', () => { expect(seen.length).toBe(1) expect(seen[0].headers.get('atproto-proxy')).toBe(APPVIEW_PROXY) const labelers = seen[0].headers.get('atproto-accept-labelers') ?? '' - expect(labelers).toContain(api.moderation.did) expect(labelers).toContain(CUSTOM_LABELER) + /* + * The moderation DID is NOT a per-instance labeler: it flows only through + * the global Client.appLabelers (unset in this test), where lex-client + * merges it into the header with `;redact` on every request. See the + * labeler-header regression guard below for the merged composition. + */ + expect(labelers).not.toContain(api.moderation.did) }) it('routes through the session fetchHandler with the bearer token', async () => { @@ -147,10 +153,6 @@ describe('buildAppviewClient', () => { expect(fetchMock).toHaveBeenCalledTimes(1) expect(seen[0].headers.get('authorization')).toBe('Bearer access-jwt') - /* the moderation DID is always re-asserted even with no custom labelers */ - expect(seen[0].headers.get('atproto-accept-labelers')).toContain( - api.moderation.did, - ) }) }) @@ -288,10 +290,12 @@ describe('getPublicLexClient', () => { describe('labeler-header regression guard', () => { it('appview client emits the global Bluesky labeler redacted and the per-instance labeler plain', async () => { /* - * buildAppviewClient re-asserts api.moderation.did as a base labeler; the - * global Client.appLabelers carry the `;redact` suffix. Configure the global - * appLabelers to the Bluesky moderation DID (matching switchToBskyAppLabeler - * in moderation.ts) so the composition matches production. + * The moderation DID flows only through the global Client.appLabelers, + * which lex-client merges into the header per request with the `;redact` + * suffix; buildAppviewClient no longer lists it as a per-instance labeler. + * Configure the global appLabelers to the Bluesky moderation DID (matching + * switchToBskyAppLabeler in moderation.ts) so the composition matches + * production. */ Client.configure({appLabelers: [api.moderation.did]}) @@ -305,6 +309,11 @@ describe('labeler-header regression guard', () => { /* the global Bluesky moderation labeler is redacted */ expect(header).toContain(`${api.moderation.did};redact`) + /* ...and appears exactly once (no duplicate plain entry) */ + const occurrences = (header ?? '') + .split(',') + .filter(entry => entry.includes(api.moderation.did)) + expect(occurrences).toHaveLength(1) /* the per-instance custom labeler is present and plain (no redact) */ expect(header).toContain(CUSTOM_LABELER) expect(header).not.toContain(`${CUSTOM_LABELER};redact`) diff --git a/src/state/session/clients.ts b/src/state/session/clients.ts index 1004a98488..2abde9d94e 100644 --- a/src/state/session/clients.ts +++ b/src/state/session/clients.ts @@ -1,18 +1,17 @@ -import {Client} from '@atproto/lex' +import {type Client} from '@atproto/lex' import {type PasswordSession} from '@atproto/lex-password-session' -import {api} from '@bsky.app/sdk' import { BLUESKY_PROXY_HEADER, CHAT_PROXY_SERVICE, PUBLIC_BSKY_SERVICE, } from '#/lib/constants' +import {createLexClient} from '#/lib/lexClient' import {networkAwareFetch} from './session-core' /** * Lazily-constructed unauthenticated client pointed at the public appview. It - * hits {@link PUBLIC_BSKY_SERVICE} directly, mirroring `createPublicAgent`'s - * service URL, so no proxying is required. + * hits {@link PUBLIC_BSKY_SERVICE} directly, so no proxying is required. */ let publicClient: Client | undefined @@ -21,7 +20,7 @@ export function getPublicLexClient(): Client { * Pass networkAwareFetch so the unauthenticated public path feeds the same * reachability signal as the session-backed clients (see session-core). */ - publicClient ??= new Client({ + publicClient ??= createLexClient({ service: PUBLIC_BSKY_SERVICE, fetch: networkAwareFetch, }) @@ -31,15 +30,14 @@ export function getPublicLexClient(): Client { /** * Build the account (PDS) client over a {@link PasswordSession}. Writes and * record mutations go here - no `atproto-proxy` header, so requests hit the - * user's PDS directly (the session's `fetchHandler` resolves the PDS origin - * per request from the didDoc, falling back to `service`). + * user's PDS directly (the session's `fetchHandler` resolves the PDS origin per + * request from the didDoc, falling back to `service`). * - * The session already owns its own `fetch` (networkAwareFetch, set at - * construction), so we intentionally do NOT pass `fetch` here: a `Client` - * built over an existing `Agent`/session uses that agent's fetch. + * We intentionally do NOT pass `fetch` here: a client built over a session uses + * that session's own `fetch` (networkAwareFetch, set at construction). */ export function buildAccountClient(session: PasswordSession): Client { - return new Client(session) + return createLexClient(session) } /** @@ -47,14 +45,13 @@ export function buildAccountClient(session: PasswordSession): Client { * * {@link CHAT_PROXY_SERVICE} (`${CHAT_PROXY_DID}#bsky_chat`, default * `did:web:api.bsky.chat#bsky_chat`) is passed as the client's `service`, so - * lex-client sets `atproto-proxy: ` on every request. This is - * exactly what the old per-call `DM_SERVICE_HEADERS` did, once and centrally, so - * `chat.bsky.*` calls are proxied to the chat service. It is read from the + * lex-client sets `atproto-proxy: ` on every request and + * `chat.bsky.*` calls are proxied to the chat service. The DID is read from the * env-configurable `CHAT_PROXY_DID` (via `EXPO_PUBLIC_CHAT_PROXY_DID`) rather - * than the hard-coded SDK constant, restoring the old routing override. + * than the hard-coded SDK constant, so it can be retargeted per environment. */ export function buildChatClient(session: PasswordSession): Client { - return new Client(session, {service: CHAT_PROXY_SERVICE}) + return createLexClient(session, {service: CHAT_PROXY_SERVICE}) } /** Thrown when a write/auth-only client is used with no active session. */ @@ -67,20 +64,18 @@ export class NotAuthenticatedError extends Error { /** * A stable {@link Client} that throws {@link NotAuthenticatedError} on any - * request, before any network I/O. Used as the logged-out value of write/auth - * -only hooks (`usePdsClient`/`useChatClient`) so an unauthenticated write or - * chat call fails immediately and legibly instead of silently hitting the - * public appview (which would 404/405 with an opaque error). + * request, before any network I/O. Used as the logged-out value of the + * write/auth-only hooks (`usePdsClient`/`useChatClient`) so an unauthenticated + * call fails immediately and legibly instead of silently hitting the public + * appview (which would 404/405 with an opaque error). * * A lazily-constructed process-wide singleton, so its identity is stable across - * renders - safe to use in React Query keys and as a hook return value. The - * `did` is `undefined` (logged out) and the `fetchHandler` throws before - * touching the network. + * renders - safe to use in React Query keys and as a hook return value. */ let unauthedClient: Client | undefined export function getUnauthenticatedClient(): Client { - unauthedClient ??= new Client({ + unauthedClient ??= createLexClient({ did: undefined, fetchHandler: () => { throw new NotAuthenticatedError() @@ -90,33 +85,26 @@ export function getUnauthenticatedClient(): Client { } /** - * Build the authed appview client over a {@link PasswordSession}. + * Build the authed appview client over a {@link PasswordSession}. Requests are + * proxied to the Bluesky appview and carry the per-instance labelers. * - * Requests are proxied to the Bluesky appview and carry the per-instance - * labelers. The Bluesky moderation labeler (`api.moderation.did`) is always - * included as a base labeler because sending ANY `atproto-accept-labelers` - * header replaces the server-side default - so we must re-assert it to keep it - * active. + * The Bluesky moderation labeler (`api.moderation.did`) is deliberately NOT + * listed here - it must flow only through the global `Client.appLabelers` (see + * moderation.ts) so it carries the `;redact` suffix; adding it here would + * produce a duplicate, non-redact header entry. * - * The proxy `service` is read from `BLUESKY_PROXY_HEADER.get()` (rather than - * hard-coding `api.app.service`). Its default value equals `api.app.service` - * (`did:web:api.bsky.app#bsky_appview`), so production is unchanged; the getter - * exists so the e2e `TestCtrls` hack can retarget the appview by calling - * `BLUESKY_PROXY_HEADER.set()` before sign-in (the client is built at - * sign-in, so it picks up the override). + * The proxy `service` is read from `BLUESKY_PROXY_HEADER.get()`, whose default + * equals `api.app.service`. The getter exists so the e2e `TestCtrls` hack can + * retarget the appview via `BLUESKY_PROXY_HEADER.set()` before sign-in (the + * client is built at sign-in, so it picks up the override). */ export function buildAppviewClient( session: PasswordSession, labelerDids: string[], ): Client { - return new Client(session, { - /* BLUESKY_PROXY_HEADER.get() is a `did:...#...` ProxyHeaderValue, assignable to Service */ + return createLexClient(session, { service: BLUESKY_PROXY_HEADER.get(), - /* labelerDids are validated DID strings; cast to the DidString template type */ - labelers: [ - api.moderation.did, - ...labelerDids, - ] as `did:${string}:${string}`[], + labelers: labelerDids as `did:${string}:${string}`[], }) } diff --git a/src/state/session/moderation.ts b/src/state/session/moderation.ts index abc4de1652..a5d83b7735 100644 --- a/src/state/session/moderation.ts +++ b/src/state/session/moderation.ts @@ -3,17 +3,17 @@ import {api} from '@bsky.app/sdk' import {IS_TEST_USER} from '#/lib/constants' import {com} from '#/lexicons' +import {account as accountStorage} from '#/storage' import {configureAdditionalModerationAuthorities} from './additional-moderation-authorities' -import {readLabelers} from './agent-config' import {type SessionBundle} from './session-core' import {type SessionAccount} from './types' /* - * The Bluesky moderation labeler DID is `api.moderation.did` (from - * '@bsky.app/sdk'), value `did:plc:ar7c4by46qjdydhdevvrndac`. We use it - * everywhere: the global appLabelers config, the per-account filter, and the - * appview client's base labeler (matching `buildAppviewClient`); all resolve to - * identical `atproto-accept-labelers` headers. + * The Bluesky moderation labeler (`api.moderation.did`) flows ONLY through the + * global `Client.appLabelers` config: lex-client merges the static appLabelers + * with each client's per-instance `labelers` into the `atproto-accept-labelers` + * header on every request, and appLabelers entries carry the `;redact` suffix + * (redaction authority) while plain per-instance labelers don't. */ /** @@ -24,25 +24,43 @@ function configureGlobalAppLabelers(dids: string[]) { Client.configure({appLabelers: dids as `did:${string}:${string}`[]}) } +/** + * Cache an account's subscribed labeler DIDs. Called on every preferences + * fetch, so the cache is eventually consistent with the server. + */ +export function saveLabelers(did: string, value: string[]) { + accountStorage.set([did, 'labelers'], value) +} + +/** + * Read the cached labeler DIDs for an account, or `undefined` if none have + * been cached yet (first session on this device) or the entry is unreadable. + */ +export function readLabelers(did: string): string[] | undefined { + try { + return accountStorage.get([did, 'labelers']) + } catch { + /* a corrupt entry fails JSON.parse inside Storage.get; treat as no cache */ + return undefined + } +} + /** * Apply an account's subscribed labeler DIDs to a live appview client. The lex - * `Client` rebuilds the `atproto-accept-labelers` header per request, so this - * takes effect on the very next request without a client rebuild. + * `Client` rebuilds the header per request, so this takes effect on the next + * request without a client rebuild. * - * The Bluesky moderation labeler is always re-asserted as the base: sending ANY - * `atproto-accept-labelers` header replaces the server-side default, and - * `setLabelers` clears then re-adds, so the moderation DID must be included - * explicitly to stay active. + * We filter out the Bluesky moderation labeler: it is already asserted globally + * via `Client.appLabelers` (with `;redact`), and a user "subscribing" to it + * must not add a second, plain (non-redact) header entry alongside the redacted + * one. */ export function applyLabelersToClient( client: Client, subscribedDids: string[], ) { const perAccount = subscribedDids.filter(did => did !== api.moderation.did) - client.setLabelers([ - api.moderation.did, - ...perAccount, - ] as `did:${string}:${string}`[]) + client.setLabelers(perAccount as `did:${string}:${string}`[]) } export function configureModerationForGuest() { @@ -53,12 +71,14 @@ export function configureModerationForGuest() { } /** - * Configure moderation labelers for a signed-in account. + * Configure moderation labelers for a signed-in account. Fully synchronous: + * the labeler cache is a local MMKV read, so the bundle leaves here with its + * per-account labelers already applied, in the same tick. * * Takes the whole {@link SessionBundle} so it can apply per-account labelers to * the authed appview client (`bundle.appviewClient`, backing `useLexClient()`). */ -export async function configureModerationForAccount( +export function configureModerationForAccount( bundle: SessionBundle, account: SessionAccount, ) { @@ -66,17 +86,24 @@ export async function configureModerationForAccount( // Don't add any other global behavior here! switchToBskyAppLabeler() if (IS_TEST_USER(account.handle)) { - await trySwitchToTestAppLabeler(bundle) + /* + * Fire-and-forget: this resolves a handle over the network and only runs + * in the test environment. Requests made before it lands use the standard + * Bluesky app labeler; that race is acceptable for tests. + */ + void trySwitchToTestAppLabeler(bundle) } // The code below is actually relevant to production (and isn't global). - const labelerDids = await readLabelers(account.did).catch(_ => {}) + const labelerDids = readLabelers(account.did) if (labelerDids) { - // Apply the per-account labelers to the appview client. applyLabelersToClient(bundle.appviewClient, labelerDids) } else { - // If there are no headers in the storage, we'll not send them on the initial requests. - // If we wanted to fix this, we could block on the preferences query here. + /* + * No cached labelers yet (first session on this device), so the initial + * requests go out without them. We could block on the preferences query + * here to fix that, but choose not to. + */ } configureAdditionalModerationAuthorities() @@ -88,9 +115,8 @@ function switchToBskyAppLabeler() { /** * In the test environment, swap the global app labeler for the test-env - * moderation authority. The handle is resolved via the bundle's authed appview - * client; `client.call` returns the response body directly (no `{data}` - * wrapper), so `resolveHandle`'s output is `{did}`. + * moderation authority, resolving its handle via the bundle's authed appview + * client. */ async function trySwitchToTestAppLabeler(bundle: SessionBundle) { const did = (