From b29812271c349e04f833dff93a539ed9c33c306e Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 13 Jul 2026 07:13:26 +0000 Subject: [PATCH 1/4] CodeRabbit Generated Unit Tests: Add Generated Unit Tests for PR Changes --- tests/api-rate-limit-fallback.test.ts | 115 ++++++++++++++++++ .../document-viewer-navigation-source.test.ts | 32 +++++ tests/production-metadata-source.test.ts | 100 +++++++++++++++ tests/public-api-access.test.ts | 65 ++++++++++ 4 files changed, 312 insertions(+) create mode 100644 tests/document-viewer-navigation-source.test.ts create mode 100644 tests/production-metadata-source.test.ts diff --git a/tests/api-rate-limit-fallback.test.ts b/tests/api-rate-limit-fallback.test.ts index cba820e8a..cc12fef66 100644 --- a/tests/api-rate-limit-fallback.test.ts +++ b/tests/api-rate-limit-fallback.test.ts @@ -148,4 +148,119 @@ describe("document_upload fail-closed limiter", () => { // document_read degrades to the per-instance limiter instead of failing closed. expect(result.limited).toBe(false); }); + + it("enforces a global anonymous upload quota as well as the caller quota", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.doMock("@/lib/env", () => ({ + isLocalNoAuthMode: () => false, + })); + const { consumeSubjectApiRateLimit } = await import("../src/lib/api-rate-limit"); + const rpc = vi.fn(async (_name: string, args: Record) => ({ + data: { + limited: false, + limit_value: args.p_limit, + remaining: Number(args.p_limit) - 1, + retry_after_seconds: 60, + reset_at: new Date(Date.now() + 60_000).toISOString(), + }, + error: null, + })); + + await consumeSubjectApiRateLimit({ + supabase: { rpc } as never, + subject: { kind: "anonymous", subjectKey: "anon:caller" }, + bucket: "document_upload", + }); + + expect(rpc).toHaveBeenCalledTimes(2); + expect(rpc.mock.calls.map(([, args]) => args.p_subject_key)).toEqual( + expect.arrayContaining(["anon:caller", "anon:document_upload:global"]), + ); + }); + + it("uses the document_upload bucket's own authenticated allowance as the global ceiling", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.doMock("@/lib/env", () => ({ + isLocalNoAuthMode: () => false, + })); + const { consumeSubjectApiRateLimit } = await import("../src/lib/api-rate-limit"); + const rpc = vi.fn(async (_name: string, args: Record) => ({ + data: { + limited: false, + limit_value: args.p_limit, + remaining: Number(args.p_limit) - 1, + retry_after_seconds: 60, + reset_at: new Date(Date.now() + 60_000).toISOString(), + }, + error: null, + })); + + await consumeSubjectApiRateLimit({ + supabase: { rpc } as never, + subject: { kind: "anonymous", subjectKey: "anon:caller" }, + bucket: "document_upload", + }); + + const globalCall = rpc.mock.calls.find(([, args]) => args.p_subject_key === "anon:document_upload:global"); + // document_upload's authenticated allowance (12/60s), not answer's (30/60s), bounds the + // aggregate anonymous ceiling for this bucket. + expect(globalCall?.[1]).toMatchObject({ p_limit: 12, p_window_seconds: 60 }); + }); + + it("does not consume the global upload quota after the caller quota denies the request", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.doMock("@/lib/env", () => ({ + isLocalNoAuthMode: () => false, + })); + const { consumeSubjectApiRateLimit } = await import("../src/lib/api-rate-limit"); + const rpc = vi.fn(async () => ({ + data: { + limited: true, + limit_value: 3, + remaining: 0, + retry_after_seconds: 60, + reset_at: new Date(Date.now() + 60_000).toISOString(), + }, + error: null, + })); + + const result = await consumeSubjectApiRateLimit({ + supabase: { rpc } as never, + subject: { kind: "anonymous", subjectKey: "anon:caller" }, + bucket: "document_upload", + }); + + expect(result.limited).toBe(true); + expect(rpc).toHaveBeenCalledTimes(1); + expect(rpc.mock.calls[0]?.[1]).toMatchObject({ p_subject_key: "anon:caller" }); + }); + + it("does not apply the dual-quota global ceiling to buckets other than answer/document_upload", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.doMock("@/lib/env", () => ({ + isLocalNoAuthMode: () => false, + })); + const { consumeSubjectApiRateLimit } = await import("../src/lib/api-rate-limit"); + const rpc = vi.fn(async (_name: string, args: Record) => ({ + data: { + limited: false, + limit_value: args.p_limit, + remaining: Number(args.p_limit) - 1, + retry_after_seconds: 60, + reset_at: new Date(Date.now() + 60_000).toISOString(), + }, + error: null, + })); + + await consumeSubjectApiRateLimit({ + supabase: { rpc } as never, + subject: { kind: "anonymous", subjectKey: "anon:caller" }, + bucket: "registry", + }); + + // A single per-caller check is still sufficient for buckets that aren't fail-closed, + // expensive paid/ingestion paths. + expect(rpc).toHaveBeenCalledTimes(1); + expect(rpc.mock.calls[0]?.[1]).toMatchObject({ p_subject_key: "anon:caller" }); + }); }); diff --git a/tests/document-viewer-navigation-source.test.ts b/tests/document-viewer-navigation-source.test.ts new file mode 100644 index 000000000..fbfd480f2 --- /dev/null +++ b/tests/document-viewer-navigation-source.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; + +import { documentPageHref } from "../src/lib/document-viewer-navigation"; + +describe("document viewer useful-page navigation", () => { + it("creates a destination-page URL without carrying an unrelated citation chunk", () => { + const href = documentPageHref("document/id", 3); + + expect(href).toBe("/documents/document%2Fid?page=3#pdf-preview-section"); + expect(href).not.toContain("chunk="); + }); + + it("normalizes invalid page numbers to the first page", () => { + expect(documentPageHref("document-id", 0)).toBe("/documents/document-id?page=1#pdf-preview-section"); + expect(documentPageHref("document-id", Number.NaN)).toBe("/documents/document-id?page=1#pdf-preview-section"); + expect(documentPageHref("document-id", Number.POSITIVE_INFINITY)).toBe( + "/documents/document-id?page=1#pdf-preview-section", + ); + }); + + it("normalizes negative page numbers to the first page", () => { + expect(documentPageHref("document-id", -5)).toBe("/documents/document-id?page=1#pdf-preview-section"); + }); + + it("truncates fractional page numbers toward zero", () => { + expect(documentPageHref("document-id", 3.9)).toBe("/documents/document-id?page=3#pdf-preview-section"); + }); + + it("encodes documentId characters that are meaningful in a URL", () => { + expect(documentPageHref("doc id&value", 2)).toBe("/documents/doc%20id%26value?page=2#pdf-preview-section"); + }); +}); diff --git a/tests/production-metadata-source.test.ts b/tests/production-metadata-source.test.ts new file mode 100644 index 000000000..5134474e1 --- /dev/null +++ b/tests/production-metadata-source.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it } from "vitest"; + +import { resolveMetadataBase } from "../src/lib/metadata-base"; + +describe("production metadata origin", () => { + it.each(["http://clinical.test", "https://clinical.test"])( + "uses a configured HTTP(S) origin: %s", + (configuredOrigin) => { + expect(resolveMetadataBase(new Headers(), { configuredSiteUrl: configuredOrigin })?.href).toBe( + `${configuredOrigin}/`, + ); + }, + ); + + it("uses Railway's trusted deployment domain when configuration is absent", () => { + expect(resolveMetadataBase(new Headers(), { trustedDeploymentDomain: "clinical-kb.up.railway.app" })?.href).toBe( + "https://clinical-kb.up.railway.app/", + ); + }); + + it("uses request headers only when the caller explicitly allows a development fallback", () => { + const requestHeaders = new Headers({ + host: "internal.test:3000", + "x-forwarded-host": "clinical.example.org, proxy.internal", + "x-forwarded-proto": "https, http", + }); + + expect(resolveMetadataBase(requestHeaders, { allowRequestOrigin: true })?.href).toBe( + "https://clinical.example.org/", + ); + }); + + it("falls back to Railway's trusted domain when the configured value is malformed", () => { + const options = { + configuredSiteUrl: "not a valid URL", + trustedDeploymentDomain: "clinical-kb.up.railway.app", + }; + + expect(() => resolveMetadataBase(new Headers(), options)).not.toThrow(); + expect(resolveMetadataBase(new Headers(), options)?.href).toBe("https://clinical-kb.up.railway.app/"); + }); + + it("does not trust request-controlled host headers in production", () => { + const requestHeaders = new Headers({ + host: "internal.test:3000", + "x-forwarded-host": "attacker.example", + "x-forwarded-proto": "https", + }); + + expect(resolveMetadataBase(requestHeaders)).toBeUndefined(); + }); + + it("uses the trusted deployment domain as-is when it already carries an explicit scheme", () => { + expect( + resolveMetadataBase(new Headers(), { trustedDeploymentDomain: "http://staging.up.railway.app" })?.href, + ).toBe("http://staging.up.railway.app/"); + }); + + it("rejects a configured origin with a non-HTTP(S) protocol and falls through", () => { + expect( + resolveMetadataBase(new Headers(), { + configuredSiteUrl: "ftp://clinical.test", + trustedDeploymentDomain: "clinical-kb.up.railway.app", + })?.href, + ).toBe("https://clinical-kb.up.railway.app/"); + }); + + it("treats a whitespace-only configured site URL as absent", () => { + expect( + resolveMetadataBase(new Headers(), { + configuredSiteUrl: " ", + trustedDeploymentDomain: "clinical-kb.up.railway.app", + })?.href, + ).toBe("https://clinical-kb.up.railway.app/"); + }); + + it("falls back to the bare host header when x-forwarded-host is absent", () => { + const requestHeaders = new Headers({ host: "clinical.example.org" }); + + expect(resolveMetadataBase(requestHeaders, { allowRequestOrigin: true })?.href).toBe( + "https://clinical.example.org/", + ); + }); + + it("defaults to https when x-forwarded-proto is missing or not http/https", () => { + const missingProto = new Headers({ host: "clinical.example.org" }); + const invalidProto = new Headers({ host: "clinical.example.org", "x-forwarded-proto": "ws" }); + + expect(resolveMetadataBase(missingProto, { allowRequestOrigin: true })?.href).toBe( + "https://clinical.example.org/", + ); + expect(resolveMetadataBase(invalidProto, { allowRequestOrigin: true })?.href).toBe( + "https://clinical.example.org/", + ); + }); + + it("returns undefined when the dev fallback is allowed but no host header is present", () => { + expect(resolveMetadataBase(new Headers(), { allowRequestOrigin: true })).toBeUndefined(); + }); +}); diff --git a/tests/public-api-access.test.ts b/tests/public-api-access.test.ts index edaec20df..2d8378ee7 100644 --- a/tests/public-api-access.test.ts +++ b/tests/public-api-access.test.ts @@ -10,6 +10,19 @@ function anonymousRequest(ip: string, userAgent: string) { }); } +function forwardedRequest(forwardedFor: string, cloudflareIp: string) { + return new Request("http://localhost/api/upload", { + headers: { + "x-forwarded-for": forwardedFor, + "cf-connecting-ip": cloudflareIp, + }, + }); +} + +function headersOnlyRequest(headers: Record) { + return new Request("http://localhost/api/upload", { headers }); +} + describe("anonymous API rate-limit identity", () => { it("does not let callers rotate the quota by changing user-agent", () => { const first = anonymousApiSubjectKey(anonymousRequest("198.51.100.10", "client-a")); @@ -24,4 +37,56 @@ describe("anonymous API rate-limit identity", () => { expect(second).not.toBe(first); }); + + it("ignores caller-controlled identity entries before Railway's appended address", () => { + const first = anonymousApiSubjectKey(forwardedRequest("192.0.2.10, 198.51.100.20", "203.0.113.10")); + const second = anonymousApiSubjectKey(forwardedRequest("192.0.2.99, 198.51.100.20", "203.0.113.99")); + + expect(second).toBe(first); + }); + + it("keeps distinct Railway-appended client addresses separate", () => { + const first = anonymousApiSubjectKey(forwardedRequest("192.0.2.10, 198.51.100.20", "203.0.113.10")); + const second = anonymousApiSubjectKey(forwardedRequest("192.0.2.10, 198.51.100.99", "203.0.113.10")); + + expect(second).not.toBe(first); + }); + + it("never trusts cf-connecting-ip: callers sharing only that header collapse to the unknown-ip bucket", () => { + const first = anonymousApiSubjectKey(headersOnlyRequest({ "cf-connecting-ip": "203.0.113.10" })); + const second = anonymousApiSubjectKey(headersOnlyRequest({ "cf-connecting-ip": "203.0.113.99" })); + + // Prior to trusting only the deployment proxy's entry, distinct cf-connecting-ip values + // would have produced distinct keys. Now both fall through to the shared "unknown-ip" signal. + expect(second).toBe(first); + }); + + it("falls back to a shared unknown-ip signal when no proxy header is present", () => { + const first = anonymousApiSubjectKey(headersOnlyRequest({ "user-agent": "client-a" })); + const second = anonymousApiSubjectKey(headersOnlyRequest({ "user-agent": "client-b" })); + + expect(second).toBe(first); + }); + + it("prefers x-forwarded-for over x-real-ip when both are present", () => { + const request = new Request("http://localhost/api/upload", { + headers: { + "x-forwarded-for": "192.0.2.10, 198.51.100.20", + "x-real-ip": "203.0.113.55", + }, + }); + const usesForwardedFor = anonymousApiSubjectKey(request); + const matchesForwardedForTail = anonymousApiSubjectKey(forwardedRequest("203.0.113.10, 198.51.100.20", "unused")); + const matchesRealIpOnly = anonymousApiSubjectKey(headersOnlyRequest({ "x-real-ip": "203.0.113.55" })); + + expect(usesForwardedFor).toBe(matchesForwardedForTail); + expect(usesForwardedFor).not.toBe(matchesRealIpOnly); + }); + + it("trims whitespace and ignores empty entries in x-forwarded-for", () => { + const padded = anonymousApiSubjectKey(headersOnlyRequest({ "x-forwarded-for": " 198.51.100.20 , " })); + const clean = anonymousApiSubjectKey(headersOnlyRequest({ "x-forwarded-for": "198.51.100.20" })); + + expect(padded).toBe(clean); + }); }); From 42c1c4e80084d15f78414263cc7f92db326667c2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Jul 2026 15:27:57 +0000 Subject: [PATCH 2/4] test: format production metadata source tests for CI --- tests/production-metadata-source.test.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/production-metadata-source.test.ts b/tests/production-metadata-source.test.ts index 5134474e1..f8f2d9ef8 100644 --- a/tests/production-metadata-source.test.ts +++ b/tests/production-metadata-source.test.ts @@ -51,9 +51,9 @@ describe("production metadata origin", () => { }); it("uses the trusted deployment domain as-is when it already carries an explicit scheme", () => { - expect( - resolveMetadataBase(new Headers(), { trustedDeploymentDomain: "http://staging.up.railway.app" })?.href, - ).toBe("http://staging.up.railway.app/"); + expect(resolveMetadataBase(new Headers(), { trustedDeploymentDomain: "http://staging.up.railway.app" })?.href).toBe( + "http://staging.up.railway.app/", + ); }); it("rejects a configured origin with a non-HTTP(S) protocol and falls through", () => { @@ -86,12 +86,8 @@ describe("production metadata origin", () => { const missingProto = new Headers({ host: "clinical.example.org" }); const invalidProto = new Headers({ host: "clinical.example.org", "x-forwarded-proto": "ws" }); - expect(resolveMetadataBase(missingProto, { allowRequestOrigin: true })?.href).toBe( - "https://clinical.example.org/", - ); - expect(resolveMetadataBase(invalidProto, { allowRequestOrigin: true })?.href).toBe( - "https://clinical.example.org/", - ); + expect(resolveMetadataBase(missingProto, { allowRequestOrigin: true })?.href).toBe("https://clinical.example.org/"); + expect(resolveMetadataBase(invalidProto, { allowRequestOrigin: true })?.href).toBe("https://clinical.example.org/"); }); it("returns undefined when the dev fallback is allowed but no host header is present", () => { From 3d9b039587e1173934a73b5af1c38b314f93daba Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Tue, 14 Jul 2026 00:29:09 +0800 Subject: [PATCH 3/4] test: type rate-limit RPC mock arguments --- tests/api-rate-limit-fallback.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/api-rate-limit-fallback.test.ts b/tests/api-rate-limit-fallback.test.ts index cc12fef66..5e53c72aa 100644 --- a/tests/api-rate-limit-fallback.test.ts +++ b/tests/api-rate-limit-fallback.test.ts @@ -213,7 +213,7 @@ describe("document_upload fail-closed limiter", () => { isLocalNoAuthMode: () => false, })); const { consumeSubjectApiRateLimit } = await import("../src/lib/api-rate-limit"); - const rpc = vi.fn(async () => ({ + const rpc = vi.fn(async (_functionName: string, _args: Record) => ({ data: { limited: true, limit_value: 3, From 67e396f553d098156ae9de693f021fb9ed73fd94 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Tue, 14 Jul 2026 00:41:34 +0800 Subject: [PATCH 4/4] test: avoid unused rate-limit mock parameters --- tests/api-rate-limit-fallback.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/api-rate-limit-fallback.test.ts b/tests/api-rate-limit-fallback.test.ts index 5e53c72aa..a0ce75871 100644 --- a/tests/api-rate-limit-fallback.test.ts +++ b/tests/api-rate-limit-fallback.test.ts @@ -213,7 +213,7 @@ describe("document_upload fail-closed limiter", () => { isLocalNoAuthMode: () => false, })); const { consumeSubjectApiRateLimit } = await import("../src/lib/api-rate-limit"); - const rpc = vi.fn(async (_functionName: string, _args: Record) => ({ + const rpc = vi.fn(async () => ({ data: { limited: true, limit_value: 3, @@ -232,7 +232,8 @@ describe("document_upload fail-closed limiter", () => { expect(result.limited).toBe(true); expect(rpc).toHaveBeenCalledTimes(1); - expect(rpc.mock.calls[0]?.[1]).toMatchObject({ p_subject_key: "anon:caller" }); + const calls = rpc.mock.calls as unknown as Array<[string, Record]>; + expect(calls[0]?.[1]).toMatchObject({ p_subject_key: "anon:caller" }); }); it("does not apply the dual-quota global ceiling to buckets other than answer/document_upload", async () => {