From 362e2f419d4bec438eae62b180b6c8a376a45a2a Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 15 Aug 2026 21:14:21 -0400 Subject: [PATCH] test: build a real OAuthStorage in the EMA leak test (#2018) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot round 1: the new EMA regression test constructed its inner provider's storage with `as unknown as OAuthStorage`, an unjustified double cast — the rule in AGENTS.md is that such a cast must carry a justification or be replaced. Replaced with the genuine article: `OAuthStorageBase` over an `OAuthMemoryStore` and a no-op persist backend, so the stub type-checks against the real interface the provider is handed and a future `OAuthStorage` contract change cannot pass silently. Reusing `emaConfig.storage`, as suggested, would not have addressed the objection — that value is itself a cast, and typed as `EmaFlowConfig["storage"]` rather than `OAuthStorage`. The test's assertion is unchanged and was re-verified to fail against pre-fix code (`expected 'corp-idp' to be null`). Signed-off-by: cliffhall --- .../core/auth/ema/transportProvider.test.ts | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/clients/web/src/test/core/auth/ema/transportProvider.test.ts b/clients/web/src/test/core/auth/ema/transportProvider.test.ts index 7cf3acce..b6a3b32a 100644 --- a/clients/web/src/test/core/auth/ema/transportProvider.test.ts +++ b/clients/web/src/test/core/auth/ema/transportProvider.test.ts @@ -6,6 +6,9 @@ import { MutableRedirectUrlProvider, } from "@inspector/core/auth/providers.js"; import type { OAuthStorage } from "@inspector/core/auth/storage.js"; +import { OAuthStorageBase } from "@inspector/core/auth/oauth-storage.js"; +import { OAuthMemoryStore } from "@inspector/core/auth/store.js"; +import type { OAuthPersistBackend } from "@inspector/core/auth/oauth-persist.js"; import type { EmaFlowConfig } from "@inspector/core/auth/ema/emaFlow.js"; import { EmaTransportOAuthProvider } from "@inspector/core/auth/ema/transportProvider.js"; import { @@ -23,6 +26,20 @@ const startIdpMock = vi.mocked(startEmaIdpAuthorization); const SERVER_URL = "http://127.0.0.1:9999/mcp"; +/** + * A real `OAuthStorage` — in-memory state over a no-op persist backend — for the + * one test below that drives an actual `BaseOAuthClientProvider`. Building the + * genuine article costs two lines and avoids an `as unknown as` stub that would + * stop type-checking against the interface the provider is handed. + */ +function makeRealStorage(): OAuthStorage { + const backend: OAuthPersistBackend = { + read: async () => null, + write: async () => {}, + }; + return new OAuthStorageBase(new OAuthMemoryStore(), backend); +} + function jwtWithExp(expSec: number): string { const payload = btoa(JSON.stringify({ exp: expSec })) .replace(/\+/g, "-") @@ -200,9 +217,10 @@ describe("EmaTransportOAuthProvider", () => { it("does not leak custom authorization params onto the IdP authorize URL", async () => { const navCallback = vi.fn(); const realInner = new BaseOAuthClientProvider(SERVER_URL, { - storage: { - getScope: vi.fn().mockResolvedValue(undefined), - } as unknown as OAuthStorage, + // A real OAuthStorage rather than a cast stub: the redirect path never + // reads it, and a genuine implementation keeps this test honest if the + // storage contract changes. + storage: makeRealStorage(), redirectUrlProvider: new MutableRedirectUrlProvider(), navigation: new CallbackNavigation(navCallback), authorizationParams: { kc_idp_hint: "corp-idp", audience: "api://mcp" },