diff --git a/browser-host/relay-broker-live.test.mjs b/browser-host/relay-broker-live.test.mjs index acf0b8e956..54f8b803f4 100644 --- a/browser-host/relay-broker-live.test.mjs +++ b/browser-host/relay-broker-live.test.mjs @@ -1054,6 +1054,11 @@ test("production interest controls preserve socket/global routes and pending wri test.each([ ["restricted: not a member", true, "Relay request failed (503)"], + [ + 'invalid: an open channel named "Launch" already exists', + true, + 'An open channel named "Launch" already exists', + ], [ "error: internal server error", false, @@ -1828,13 +1833,20 @@ test.each([ ]); await h.sockets[0].receive(["OK", event.id, false, reason]); const response = await result; - expect(response.status).toBe(503); - expect(await response.json()).toEqual({ - error: summary.startsWith("rate-limited:") - ? summary - : "Socket publication could not be confirmed", - ...(!sent ? { sent: false } : {}), - }); + // A proven validation refusal answers this client as the relay's HTTP + // API does, with its own text; the log keeps only the safe summary. + const validation = !sent && reason.startsWith("invalid:"); + expect(response.status).toBe(validation ? 400 : 503); + expect(await response.json()).toEqual( + validation + ? { error: reason } + : { + error: summary.startsWith("rate-limited:") + ? summary + : "Socket publication could not be confirmed", + ...(!sent ? { sent: false } : {}), + }, + ); expect(lines).toContain( `publication id=${event.id} kind=9000 stage=socket sent=${sent} reason=Relay publication not confirmed refusal=${summary}`, ); diff --git a/browser-host/relay-broker.mjs b/browser-host/relay-broker.mjs index 62449caf3b..d9df5ae464 100644 --- a/browser-host/relay-broker.mjs +++ b/browser-host/relay-broker.mjs @@ -2809,6 +2809,18 @@ export function relayBrokerPlugin({ const conflict = failure?.sent === false && failure.refusal?.startsWith("conflict:"); + // Answer a proven validation refusal as the relay's HTTP API does, + // 400 with its bounded text, so every publish path shows the same + // reason. The text goes to this client only, never to the log. + if ( + failure?.sent === false && + failure.reason?.startsWith("invalid:") && + !( + readPublishing && + failure.refusal === READ_STATE_TIMESTAMP_REFUSAL + ) + ) + return json(res, 400, { error: failure.reason }); return json(res, conflict ? 409 : 503, { error: failure?.sent === false && diff --git a/src/features/relay/http-admission.ts b/src/features/relay/http-admission.ts index 2f4a46c64d..97c12c5eda 100644 --- a/src/features/relay/http-admission.ts +++ b/src/features/relay/http-admission.ts @@ -84,7 +84,7 @@ export function apiFailure(status: number, value: unknown): ApiFailure { * `reason` may keep an exact, caller-allowed upstream refusal from that body. */ export async function readApiFailure( response: Response, - reason?: (body: unknown) => string | undefined, + reason?: (body: unknown, status: number) => string | undefined, ): Promise { if (!response.body) return apiFailure(response.status, undefined); const reader = response.body.getReader(); @@ -108,7 +108,7 @@ export async function readApiFailure( return apiFailure(response.status, undefined); } const failure = apiFailure(response.status, body); - const allowed = failure.quota ? undefined : reason?.(body); + const allowed = failure.quota ? undefined : reason?.(body, response.status); return allowed ? { ...failure, error: allowed } : failure; } finally { await reader.cancel().catch(() => {}); @@ -248,7 +248,7 @@ export async function admittedApiRequest( request: () => Promise, signal?: AbortSignal, priority: Ticket["priority"] = "foreground", - reason?: (body: unknown) => string | undefined, + reason?: (body: unknown, status: number) => string | undefined, ): Promise { return lane.run( async () => { diff --git a/src/features/relay/native.test.ts b/src/features/relay/native.test.ts index 3130ee6ee4..4247be7bd7 100644 --- a/src/features/relay/native.test.ts +++ b/src/features/relay/native.test.ts @@ -342,7 +342,7 @@ it.each([true, false])( status: accepted ? 200 : 403, body: accepted ? { accepted: true, event_id: JSON.parse(request.body ?? "{}").id } - : { error: "Registration denied" }, + : { error: "restricted: registration denied" }, }); expect(outbox.supports(30177)).toBe(true); const id = outbox.send({ kind: 30177, tags: [["d", agent]], content }); @@ -351,7 +351,7 @@ it.each([true, false])( outbox.snapshot().find((item) => item.event.id === id), ).toMatchObject({ delivery: accepted ? "accepted" : "failed", - ...(accepted ? {} : { error: "Relay rejected the message (403)" }), + ...(accepted ? {} : { error: "Registration denied" }), }), ); const publishes = requests.filter( diff --git a/src/features/relay/socket-requests.ts b/src/features/relay/socket-requests.ts index b43e489528..4e2e7ee91b 100644 --- a/src/features/relay/socket-requests.ts +++ b/src/features/relay/socket-requests.ts @@ -6,12 +6,15 @@ import { MAX_EVENT_BYTES as CATALOG_EVENT_BYTES, } from "../agents/catalog-envelope.ts"; -/** False means proven non-delivery, not merely a negative or missing OK. */ +/** False means proven non-delivery, not merely a negative or missing OK. + * `refusal` is the log-safe summary; `reason` is the relay's own bounded text + * for a proven refusal, for the client only and never for logs. */ export class SocketRequestError extends Error { constructor( message: string, readonly sent: boolean, readonly refusal?: string, + readonly reason?: string, ) { super(message); } @@ -126,6 +129,7 @@ export function createSocketPublications(wake: () => void) { "Relay publication not confirmed", !rejected, publicationRefusal(data[3]), + rejected ? data[3] : undefined, ), ); } diff --git a/src/features/relay/transport.test.ts b/src/features/relay/transport.test.ts index 3f0be4ea99..c3082ae4bb 100644 --- a/src/features/relay/transport.test.ts +++ b/src/features/relay/transport.test.ts @@ -2,6 +2,7 @@ import { assert, afterEach, expect, it, vi } from "vitest"; import { connectBrokerTransport, connectSignedTransport, + acceptPublish, acceptReadStatePublish, } from "./transport"; import { @@ -136,6 +137,120 @@ it("distinguishes explicit rejection from invalid or missing delivery receipts", transport.writer.publish(event, new AbortController().signal), ).rejects.toBeInstanceOf(PublishRejected); }); +const refusal = (status: number, body: unknown) => + acceptPublish(Response.json(body, { status }), "id").catch( + (reason: unknown) => reason, + ); +it.each([ + [ + 'invalid: an open channel named "Launch" already exists', + 'An open channel named "Launch" already exists', + ], + ["blocked: you are banned", "You are banned"], + ["restricted: not a channel member", "Not a channel member"], + ["error: internal failure", "Internal failure"], + ["pow: difficulty too low", "Difficulty too low"], + ["duplicate: already accepted", "Already accepted"], + ["invalid: ", "Relay rejected the message (400)"], + // Prefixes writers match on carry meaning: left as sent. + ["conflict: stale prev", "conflict: stale prev"], + ["rate-limited: slow down", "rate-limited: slow down"], + ["auth-required: sign in", "auth-required: sign in"], +])( + "shows the relay's refusal text without its NIP-01 prefix: %s", + async (error, message) => { + const result = await refusal(400, { error }); + expect(result).toBeInstanceOf(PublishRejected); + expect(result).toMatchObject({ message, reason: error }); + }, +); +it.each([ + [400, {}], + [403, { error: 7 }], + [422, { error: "" }], + [404, "not an object"], + // A gateway or VPN edge can answer these statuses with JSON `{error}`; text + // without a NIP-01 prefix is not the relay's, so it is never shown. + [403, { error: "Forbidden by proxy" }], + [400, { error: "Plain relay text" }], + [401, { error: "Relay request failed (401)" }], +])( + "falls back to the status summary when a %s refusal has no relay text: %j", + async (status, body) => { + const result = await refusal(status, body); + expect(result).toBeInstanceOf(PublishRejected); + expect(result).toHaveProperty( + "message", + `Relay rejected the message (${status})`, + ); + }, +); +it("carries the relay's refusal text through both publish transports", async () => { + const error = 'invalid: an open channel named "Launch" already exists'; + const shown = 'An open channel named "Launch" already exists'; + const event = signed(key, { kind: 9, content: "hello", tags: [["h", "c"]] }); + const publishes = async ( + transport: Awaited>, + ) => { + assert.exists(transport.writer); + return transport.writer + .publish(event, new AbortController().signal) + .catch((reason: unknown) => reason); + }; + // Admission reduces an upstream body to a status summary unless the + // publish keeps the relay's refusal. + const signedTransport = await connectSignedTransport( + hostSigner(key), + "https://relay.test", + "relay", + ); + vi.stubGlobal("fetch", async () => Response.json({ error }, { status: 400 })); + expect(await publishes(signedTransport)).toMatchObject({ + message: shown, + reason: error, + }); + // Without relay text, admission's own summary must not replace the + // status summary `acceptPublish` shows. + for (const [status, body] of [ + [400, {}], + [400, { error: 7 }], + [403, { error: "Forbidden by proxy" }], + ] as const) { + vi.stubGlobal("fetch", async () => Response.json(body, { status })); + expect(await publishes(signedTransport)).toHaveProperty( + "message", + `Relay rejected the message (${status})`, + ); + } + // Other statuses stay a status summary, never upstream text. + vi.stubGlobal("fetch", async () => Response.json({ error }, { status: 500 })); + expect(await publishes(signedTransport)).toHaveProperty( + "message", + "Relay delivery could not be confirmed (500)", + ); + vi.stubGlobal("fetch", async (url: string) => + url.endsWith("/session") + ? Response.json({ + viewer: key.pubkey, + relayAuthor: "relay", + writeKinds: [9], + }) + : Response.json({ error }, { status: 400 }), + ); + expect(await publishes(await connectBrokerTransport())).toMatchObject({ + message: shown, + reason: error, + }); +}); +it("keeps the conflict refusal and the raw reason for classifying writers", async () => { + const conflict = await refusal(409, { error: "invalid: whatever" }); + expect(conflict).toHaveProperty( + "message", + "conflict: the relay state changed; reload before writing again", + ); + const refused = await refusal(400, { error: "restricted: nope" }); + expect(refused).toHaveProperty("reason", "restricted: nope"); +}); it.each([ [ "rate-limited: quota exceeded; retry in 17s", diff --git a/src/features/relay/transport.ts b/src/features/relay/transport.ts index b9c231369b..785b5f63c5 100644 --- a/src/features/relay/transport.ts +++ b/src/features/relay/transport.ts @@ -1424,6 +1424,9 @@ export async function connectSignedTransport( profiling, event.id, principal().api, + "foreground", + false, + refusalText, ).catch((error) => { if (error instanceof ApiPaused || error instanceof ApiNotSent) throw new PublishNotSent(error.message); @@ -1491,11 +1494,12 @@ async function signedPost( admission: Parameters[0], priority: "foreground" | "background" = "foreground", optionalPresence = false, + reason?: (body: unknown, status: number) => string | undefined, ) { const dispatch = (request: () => Promise) => optionalPresence ? admission.prepare(request) - : admittedApiRequest(admission, request, signal, priority); + : admittedApiRequest(admission, request, signal, priority, reason); signal?.throwIfAborted(); return admission.prepare(async () => { const body = JSON.stringify(value); @@ -1530,6 +1534,39 @@ export async function acceptReadStatePublish(response: Response, id: string) { return acceptPublish(response, id); } +const REFUSAL_STATUSES = [400, 401, 403, 404, 413, 422]; +/** NIP-01 machine-readable prefixes: the shape of a relay's own refusal. */ +const NIP01_PREFIX = + /^(invalid|blocked|restricted|error|pow|duplicate|rate-limited|conflict|auth-required):\s*/; +/** Prefixes writers match on, so the text is shown as sent. */ +const KEPT_PREFIXES = new Set(["rate-limited", "conflict", "auth-required"]); +/** The relay's own `{error}` text for a refused write. Admission otherwise + * reduces every upstream body to a status summary, so a signed publish asks + * it to keep this one, bounded by `readApiFailure`. A gateway or VPN edge + * can answer the same statuses with JSON `{error}`, so only relay-shaped + * text (a NIP-01 prefix) is kept. The prefix is a shape, not proof of origin. */ +const refusalText = (body: unknown, status: number) => { + const error = (body as { error?: unknown } | null)?.error; + return REFUSAL_STATUSES.includes(status) && + typeof error === "string" && + NIP01_PREFIX.test(error) + ? error + : undefined; +}; + +/** The display text for an HTTP refusal: the relay's own words without their + * NIP-01 machine prefix. `reason` keeps the raw text for classification. Text + * with a prefix writers match on (`conflict:`, `rate-limited:`) is shown as + * sent. Text without a prefix is not relay-shaped, so the status summary stays. */ +function refusalMessage(status: number, reason: string | undefined) { + const prefix = reason ? NIP01_PREFIX.exec(reason) : null; + if (!reason || !prefix) return `Relay rejected the message (${status})`; + if (KEPT_PREFIXES.has(prefix[1] ?? "")) return reason.trim(); + const text = reason.slice(prefix[0].length).trim(); + if (!text) return `Relay rejected the message (${status})`; + return text.charAt(0).toUpperCase() + text.slice(1); +} + /** A transport failure is an unknown outcome; only a definitive rejection is a failed write. */ export async function acceptPublish(response: Response, id: string) { if (!response.ok) { @@ -1540,7 +1577,7 @@ export async function acceptPublish(response: Response, id: string) { throw new PublishRejected( "conflict: the relay state changed; reload before writing again", ); - if ([400, 401, 403, 404, 413, 422].includes(response.status)) { + if (REFUSAL_STATUSES.includes(response.status)) { // Keep the relay's bounded `{error}` text for writers that classify it. let reason: string | undefined; await readApiFailure(response, (value) => { @@ -1549,7 +1586,7 @@ export async function acceptPublish(response: Response, id: string) { return undefined; }); throw new PublishRejected( - `Relay rejected the message (${response.status})`, + refusalMessage(response.status, reason), reason, ); }