From 1736aa99658927e62e03ade4fb4a2f965b0fca92 Mon Sep 17 00:00:00 2001 From: shitikyan Date: Wed, 22 Jul 2026 12:09:24 +0400 Subject: [PATCH] fix(web2): stop stamping an empty Authorization on every proxied request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The outbound Authorization header was keyed on `requireAuthForAll`, which actually governs INBOUND access to the DAHR proxy (the x-dahr-session-id check in isAuthorizedRequest). That flag defaults to true on production, so every proxied request went out carrying `Bearer undefined` regardless of whether the caller supplied a token. Permissive targets ignore the bogus header — which is why httpbin.org proxied fine and DAHR looked healthy — while targets that validate it reject the request: api.github.com answers 401 "Bad credentials" and raw.githubusercontent.com answers 404. Keying the header on the flag also meant a token passed off production was silently dropped. Forward the header only when the caller actually supplied a token. The inbound session-id control is untouched. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/features/web2/proxy/Proxy.test.ts | 77 +++++++++++++++++++++++++++ src/features/web2/proxy/Proxy.ts | 27 +++------- 2 files changed, 85 insertions(+), 19 deletions(-) create mode 100644 src/features/web2/proxy/Proxy.test.ts diff --git a/src/features/web2/proxy/Proxy.test.ts b/src/features/web2/proxy/Proxy.test.ts new file mode 100644 index 000000000..c9e3e1431 --- /dev/null +++ b/src/features/web2/proxy/Proxy.test.ts @@ -0,0 +1,77 @@ +// Importing Proxy pulls the node runtime in (SharedState -> chain -> logger -> +// PeerManager). Both config objects are passed explicitly below, so none of it +// is actually exercised — stub the modules so the suite stays a unit test. +jest.mock("@/utilities/sharedState", () => ({ + __esModule: true, + default: { getInstance: () => ({ PROD: false }) }, +})) +jest.mock("@/utilities/logger", () => ({ + __esModule: true, + default: { error: jest.fn(), info: jest.fn(), warn: jest.fn(), debug: jest.fn() }, +})) +jest.mock("src/libs/crypto/hashing", () => ({ + __esModule: true, + default: { sha256: (v: string) => v }, +})) + +import { Proxy } from "./Proxy" + +/** + * Build a Proxy with both config objects supplied so construction never reaches + * SharedState — these tests are about header shaping, not runtime environment. + */ +function makeProxy(requireAuthForAll: boolean) { + return new Proxy( + "test-session-id", + "localhost", + { requireAuthForAll, exceptions: [] }, + { verifyCertificates: false }, + ) +} + +/** `createHeaders` is private; TS visibility is compile-time only. */ +function headersFor( + proxy: Proxy, + targetAuthorization: string, +): Record { + return (proxy as any).createHeaders( + "GET", + {}, + targetAuthorization, + ) as Record +} + +describe("Proxy outbound Authorization header", () => { + it("is omitted when the caller supplied no token, even in production", () => { + // The regression: `requireAuthForAll` is true on production, and the + // outbound header used to be keyed on it. Every proxied request then + // carried `Bearer undefined`, which GitHub rejects (401 on + // api.github.com, 404 on raw.githubusercontent.com) while permissive + // targets like httpbin ignore it — hence "DAHR works but GitHub 401s". + const headers = headersFor(makeProxy(true), "") + + expect(headers).not.toHaveProperty("Authorization") + expect(Object.values(headers).join(" ")).not.toContain("undefined") + }) + + it("forwards the token the caller did supply", () => { + const headers = headersFor(makeProxy(true), "ghp_realtoken") + + expect(headers["Authorization"]).toBe("Bearer ghp_realtoken") + }) + + it("forwards a supplied token off production too, rather than dropping it", () => { + // Keying the header on the inbound-access flag also meant a token + // passed in development was silently discarded. + const headers = headersFor(makeProxy(false), "ghp_realtoken") + + expect(headers["Authorization"]).toBe("Bearer ghp_realtoken") + }) + + it("still stamps the session id that gates inbound proxy access", () => { + // The inbound control must be untouched by the outbound change. + expect(headersFor(makeProxy(true), "")["x-dahr-session-id"]).toBe( + "test-session-id", + ) + }) +}) diff --git a/src/features/web2/proxy/Proxy.ts b/src/features/web2/proxy/Proxy.ts index 87714837c..77d5f7ebf 100644 --- a/src/features/web2/proxy/Proxy.ts +++ b/src/features/web2/proxy/Proxy.ts @@ -78,7 +78,6 @@ export class Proxy { targetMethod, targetHeaders, targetAuthorization, - targetUrl, ) const req = http.request({ @@ -429,7 +428,6 @@ export class Proxy { targetMethod: Web2Method, targetHeaders: IWeb2Request["raw"]["headers"], targetAuthorization: string, - targetUrl: string, ): IWeb2Request["raw"]["headers"] { // Base headers - only essential ones const headers: IWeb2Request["raw"]["headers"] = { @@ -461,8 +459,14 @@ export class Proxy { headers["Accept-Encoding"] = "identity" } - // Add Authorization if required - if (this.requiresAuthorization(targetUrl, targetMethod)) { + // Only forward an Authorization the caller actually supplied. + // `requireAuthForAll` governs INBOUND access to this proxy (the + // x-dahr-session-id check in isAuthorizedRequest) and says nothing + // about what the target site should receive. Keying the outbound + // header on it stamps `Bearer undefined` onto every proxied request, + // which any site that validates the header rejects — GitHub answers + // 401 on api.github.com and 404 on raw.githubusercontent.com. + if (targetAuthorization) { headers["Authorization"] = `Bearer ${targetAuthorization}` } @@ -511,19 +515,4 @@ export class Proxy { entries.sort((a, b) => (a.key < b.key ? -1 : a.key > b.key ? 1 : 0)) return entries.map(e => `${e.key}:${e.value}`).join("\n") } - - private requiresAuthorization(url: string, method: Web2Method): boolean { - if (this._authConfig.requireAuthForAll) { - for (const exception of this._authConfig.exceptions) { - if ( - exception.urlPattern.test(url) && - exception.methods.includes(method) - ) { - return false - } - } - return true - } - return false - } }