Merge remote-tracking branch 'origin/codex/effect-lint-core-sdk-oauth' into codex/effect-lint-integration

Rhys Sullivan committed 6 months ago

Commit fbcc52b2 parents dc75098 0bbd7564 files · +103 −123Browse files

Unified · Split

4 files

packages/core/sdk/src/oauth-discovery.test.ts +16 −7
packages/core/sdk/src/oauth-discovery.ts +63 −91
packages/core/sdk/src/oauth-helpers.test.ts +8 −6
packages/core/sdk/src/oauth-helpers.ts +16 −19

packages/core/sdk/src/oauth-discovery.test.ts+16−7

 import { afterEach, describe, expect, it, vi } from "@effect/vitest";
-import { Cause, Effect, Exit } from "effect";
+import { Cause, Effect, Exit, Schema } from "effect";
 import {
   OAuthDiscoveryError,
 type Handler = (url: string, init: RequestInit) => Response | Promise<Response>;
+const DcrRequestBody = Schema.Struct({
+  redirect_uris: Schema.Array(Schema.String),
+  token_endpoint_auth_method: Schema.String,
+});
+const decodeDcrRequestBody = Schema.decodeUnknownSync(
+  Schema.fromJsonString(DcrRequestBody),
+);
 const installFetchRouter = (
   handlers: readonly { match: (url: string) => boolean; handle: Handler }[],
 ): { calls: Array<{ url: string; init: RequestInit }> } => {
     const call = calls[0]!;
     expect(call.init.method).toBe("POST");
-    const body = JSON.parse(call.init.body as string);
+    const body = decodeDcrRequestBody(call.init.body);
     expect(body.redirect_uris).toEqual(["https://app.example.com/cb"]);
     expect(body.token_endpoint_auth_method).toBe("none");
   });
     expect(Exit.isFailure(exit)).toBe(true);
     if (!Exit.isFailure(exit)) return;
     const reason = exit.cause.reasons.find(Cause.isFailReason);
-    if (!(reason?.error instanceof OAuthDiscoveryError)) {
+    const error = reason?.error;
-      throw new Error("expected OAuthDiscoveryError");
+    expect(error).toEqual(expect.objectContaining({
-    }
+      _tag: "OAuthDiscoveryError",
-    expect(reason.error.status).toBe(400);
+      status: 400,
-    expect(reason.error.message).toMatch(/invalid_client_metadata/);
+      message: expect.stringMatching(/invalid_client_metadata/),
+    }));
   });
 });

packages/core/sdk/src/oauth-discovery.ts+63−91

 // callers actually need.
 // ---------------------------------------------------------------------------
-import { Data, Effect, Result, Schema } from "effect";
+import { Data, Effect, Option, Result, Schema } from "effect";
 import * as oauth from "oauth4webapi";
 import {
   readonly cause?: unknown;
 }> {}
-const discoveryError = (
-  message: string,
-  options: { status?: number; cause?: unknown } = {},
-): OAuthDiscoveryError =>
-  new OAuthDiscoveryError({
-    message,
-    status: options.status,
-    cause: options.cause,
-  });
 // ---------------------------------------------------------------------------
 // Schemas (narrow structural parsing — the RFCs leave many fields
 // optional; we validate only the subset consumers read)
 // ---------------------------------------------------------------------------
 const StringArray = Schema.Array(Schema.String);
+const JsonUnknownFromString = Schema.fromJsonString(Schema.Unknown);
+const decodeJsonUnknownSync = Schema.decodeUnknownSync(JsonUnknownFromString);
+const decodeJsonUnknownOption = Schema.decodeUnknownOption(JsonUnknownFromString);
 export const OAuthProtectedResourceMetadataSchema = Schema.Struct({
   resource: Schema.optional(Schema.String),
 const MCP_PROTOCOL_VERSION_HEADER = "mcp-protocol-version";
 const isLoopbackHttpUrl = (value: string): boolean => {
-  try {
+  if (!URL.canParse(value)) return false;
-    const url = new URL(value);
+  const url = new URL(value);
-    if (url.protocol !== "http:") return false;
+  if (url.protocol !== "http:") return false;
-    const hostname = url.hostname.toLowerCase();
+  const hostname = url.hostname.toLowerCase();
-    return (
+  return (
-      hostname === "localhost" ||
+    hostname === "localhost" ||
-      hostname === "0.0.0.0" ||
+    hostname === "0.0.0.0" ||
-      hostname === "::1" ||
+    hostname === "::1" ||
-      hostname === "[::1]" ||
+    hostname === "[::1]" ||
-      hostname.startsWith("127.")
+    hostname.startsWith("127.")
-    );
+  );
-  } catch {
-    return false;
-  }
 };
 const oauth4webapiOptions = (
           }
           const text = await response.text();
           if (text.length === 0) return "skip" as const;
-          return { status: response.status, body: JSON.parse(text) } as const;
+          return { status: response.status, body: decodeJsonUnknownSync(text) } as const;
         },
         catch: (cause) =>
-          discoveryError(
+          new OAuthDiscoveryError({
-            `Failed to fetch ${url}: ${cause instanceof Error ? cause.message : String(cause)}`,
+            message: `Failed to fetch protected resource metadata at ${url}`,
-            { cause },
+            cause,
-          ),
+          }),
       });
       if (result === "skip") continue;
       if (!("body" in result)) {
-        return yield* Effect.fail(
+        return yield* new OAuthDiscoveryError({
-          discoveryError(
+          message: `Protected resource metadata returned status ${result.status}`,
-            `Protected resource metadata returned status ${result.status}`,
+          status: result.status,
-            { status: result.status },
+        });
-          ),
-        );
       }
       const metadata = yield* decodeResourceMetadata(result.body).pipe(
         Effect.mapError(
           (err) =>
             new OAuthDiscoveryError({
-              message: `Protected resource metadata is malformed: ${
+              message: "Protected resource metadata is malformed",
-                Schema.isSchemaError(err) ? err.message : String(err)
-              }`,
               cause: err,
             }),
         ),
             raw: as,
           };
         },
-        catch: (cause) => {
+        catch: (cause) =>
-          if (cause instanceof OAuthDiscoveryError) return cause;
+          new OAuthDiscoveryError({
-          return discoveryError(
+            message: `Discovery (${algorithm}) failed for ${issuer}`,
-            `Discovery (${algorithm}) failed for ${issuer}: ${
+            cause,
-              cause instanceof Error ? cause.message : String(cause)
+          }),
-            }`,
-            { cause },
-          );
-        },
       }).pipe(
         // If one algorithm fails mid-roundtrip (network, parse, issuer
         // mismatch) we still want to try the other before giving up.
         Effect.mapError(
           (err) =>
             new OAuthDiscoveryError({
-              message: `Authorization server metadata is malformed: ${
+              message: "Authorization server metadata is malformed",
-                Schema.isSchemaError(err) ? err.message : String(err)
-              }`,
               cause: err,
             }),
         ),
 ): DcrErrorBody | DcrTransport => {
   // RFC 6749 error envelope: `{error, error_description?}` with 4xx.
   if (status >= 400 && status < 500) {
-    const parsed = Result.try({
+    const body = text ? Option.getOrNull(decodeJsonUnknownOption(text)) : null;
-      try: () => (text ? (JSON.parse(text) as unknown) : null),
-      catch: () => null,
-    });
-    const body = Result.isSuccess(parsed) ? parsed.success : null;
     if (
       body &&
       typeof body === "object" &&
         }),
       catch: (cause) =>
         new DcrTransport({
-          message: `Dynamic Client Registration request failed: ${cause instanceof Error ? cause.message : String(cause)}`,
+          message: "Dynamic Client Registration request failed",
           cause,
         }),
     });
     // Accept both 200 and 201 as success — RFC 7591 mandates 201, but
     // Todoist (and others) return 200 OK with the client information body.
     if (response.status !== 200 && response.status !== 201) {
-      const text = yield* Effect.promise(() =>
+      const text = yield* Effect.tryPromise({
-        response.text().catch(() => ""),
+        try: () => response.text(),
-      );
+        catch: () => "",
+      });
       return yield* interpretDcrFailure(response.status, text);
     }
           cause,
         }),
     });
-    const json = yield* Effect.try({
+    const json = yield* Schema.decodeUnknownEffect(JsonUnknownFromString)(text).pipe(
-      try: () => JSON.parse(text) as unknown,
+      Effect.mapError((cause) =>
-      catch: (cause) =>
         new DcrTransport({
           message: "Dynamic Client Registration response was not valid JSON",
           status: response.status,
           cause,
         }),
-    });
+      ),
+    );
     return yield* decodeClientInformation(json).pipe(
       Effect.mapError(
         (err) =>
           new OAuthDiscoveryError({
-            message: `Dynamic Client Registration response is malformed: ${
+            message: "Dynamic Client Registration response is malformed",
-              Schema.isSchemaError(err) ? err.message : String(err)
-            }`,
             cause: err,
           }),
       ),
     Effect.catchTags({
       DcrErrorBody: (err) =>
         Effect.fail(
-          discoveryError(
+          new OAuthDiscoveryError({
-            `Dynamic Client Registration failed: ${err.error}${
+            message: `Dynamic Client Registration failed: ${err.error}${
-              err.error_description ? ` — ${err.error_description}` : ""
+              err.error_description ? ` - ${err.error_description}` : ""
             }`,
-            { status: err.status, cause: err },
+            status: err.status,
-          ),
+            cause: err,
+          }),
         ),
       DcrTransport: (err) =>
         Effect.fail(
-          discoveryError(`Dynamic Client Registration failed: ${err.message}`, {
+          new OAuthDiscoveryError({
+            message: "Dynamic Client Registration failed",
             status: err.status,
             cause: err.cause ?? err,
           }),
           );
     if (!authServer) {
-      return yield* Effect.fail(
+      return yield* new OAuthDiscoveryError({
-        discoveryError(
+        message: `No OAuth authorization server metadata at ${authorizationServerUrl}`,
-          `No OAuth authorization server metadata at ${authorizationServerUrl}`,
+      });
-        ),
-      );
     }
     const pkceMethods = authServer.metadata.code_challenge_methods_supported ?? [];
     if (pkceMethods.length > 0 && !pkceMethods.includes("S256")) {
-      return yield* Effect.fail(
+      return yield* new OAuthDiscoveryError({
-        discoveryError(
+        message: `Authorization server does not support PKCE S256 (advertised: ${pkceMethods.join(", ")})`,
-          `Authorization server does not support PKCE S256 (advertised: ${pkceMethods.join(", ")})`,
+      });
-        ),
-      );
     }
     const responseTypes = authServer.metadata.response_types_supported ?? [];
     if (responseTypes.length > 0 && !responseTypes.includes("code")) {
-      return yield* Effect.fail(
+      return yield* new OAuthDiscoveryError({
-        discoveryError(
+        message: `Authorization server does not support response_type=code (advertised: ${responseTypes.join(", ")})`,
-          `Authorization server does not support response_type=code (advertised: ${responseTypes.join(", ")})`,
+      });
-        ),
-      );
     }
     const baseClientMetadata: DynamicClientMetadata = {
         const reg = authServer.metadata.registration_endpoint;
         if (!reg) {
           return Effect.fail(
-            discoveryError(
+            new OAuthDiscoveryError({
-              "Authorization server does not advertise registration_endpoint — cannot auto-register a client",
+              message:
-            ),
+                "Authorization server does not advertise registration_endpoint - cannot auto-register a client",
+            }),
           );
         }
         return registerDynamicClient(

packages/core/sdk/src/oauth-helpers.test.ts+8−6

   });
   it("returns a typed OAuth2Error on transport failure", async () => {
-    globalThis.fetch = vi.fn().mockRejectedValue(new Error("boom")) as typeof fetch;
+    globalThis.fetch = vi.fn().mockRejectedValue({ message: "boom" }) as typeof fetch;
     const exit = await Effect.runPromiseExit(
       exchangeAuthorizationCode({
         tokenUrl: "https://example.com/token",
     const err = exit.cause;
     const failure = JSON.stringify(err);
     expect(failure).toContain("OAuth2Error");
-    expect(failure).toContain("boom");
+    expect(failure).toContain("OAuth token exchange failed");
   });
   it("propagates RFC 6749 error_description text in the OAuth2Error", async () => {
 describe("OAuth2Error tagging", () => {
   beforeEach(() => {
-    globalThis.fetch = vi.fn().mockRejectedValue(new Error("network down")) as typeof fetch;
+    globalThis.fetch = vi.fn().mockRejectedValue({ message: "network down" }) as typeof fetch;
   });
   afterEach(() => {
     globalThis.fetch = originalFetch;
   it("OAuth2Error is constructable directly with message and cause", () => {
     const err = new OAuth2Error({ message: "test", cause: { foo: 1 } });
-    expect(err._tag).toBe("OAuth2Error");
+    expect(err).toMatchObject({
-    expect(err.message).toBe("test");
+      _tag: "OAuth2Error",
-    expect(err.cause).toEqual({ foo: 1 });
+      message: "test",
+      cause: { foo: 1 },
+    });
   });
 });

packages/core/sdk/src/oauth-helpers.ts+16−19

@@ -134,7 +134,7 @@ const toOAuth2Error = (cause: unknown): OAuth2Error => { · 133 unmodified lines
134	});	134	});
135	}	135	}
136	return new OAuth2Error({	136	return new OAuth2Error({
137	message: `OAuth token exchange failed: ${String(cause)}`,	137	message: "OAuth token exchange failed",
138	cause,	138	cause,
139	});	139	});
140	};	140	};
@@ -173,20 +173,17 @@ const asFromTokenUrlAndIssuer = ( · 32 unmodified lines
173	};	173	};
174		174
175	const isLoopbackHttpUrl = (value: string): boolean => {	175	const isLoopbackHttpUrl = (value: string): boolean => {
176	try {	176	if (!URL.canParse(value)) return false;
177	const url = new URL(value);	177	const url = new URL(value);
178	if (url.protocol !== "http:") return false;	178	if (url.protocol !== "http:") return false;
179	const hostname = url.hostname.toLowerCase();	179	const hostname = url.hostname.toLowerCase();
180	return (	180	return (
181	hostname === "localhost" \|\|	181	hostname === "localhost" \|\|
182	hostname === "0.0.0.0" \|\|	182	hostname === "0.0.0.0" \|\|
183	hostname === "::1" \|\|	183	hostname === "::1" \|\|
184	hostname === "[::1]" \|\|	184	hostname === "[::1]" \|\|
185	hostname.startsWith("127.")	185	hostname.startsWith("127.")
186	);	186	);
187	} catch {
188	return false;
189	}
190	};	187	};
191		188
192	const oauth4webapiRequestOptions = (	189	const oauth4webapiRequestOptions = (
@@ -230,10 +227,10 @@ const tokenResponseFrom = ( · 37 unmodified lines
230	// its claims against the AS metadata and rejects mismatches we don't care	227	// its claims against the AS metadata and rejects mismatches we don't care
231	// about. Strip the field before delegation.	228	// about. Strip the field before delegation.
232	const stripIdToken = async (response: Response): Promise<Response> => {	229	const stripIdToken = async (response: Response): Promise<Response> => {
233	const body = await response	230	const body = await response.clone().json().then(
234	.clone()	231	(value: unknown) => value,
235	.json()	232	() => null,
236	.catch(() => null);	233	);
237	if (	234	if (
238	!body \|\|	235	!body \|\|
239	typeof body !== "object" \|\|	236	typeof body !== "object" \|\|