Carry OAuth token material as Redacted

Rhys Sullivan committed 3 months ago

Commit 9fca2e61 parent 55b7bea8 files · +643 −92Browse files

Unified · Split

8 files

.changeset/quiet-moons-shave.md +23 −0
packages/core/sdk/src/executor.ts +11 −12
packages/core/sdk/src/oauth-client.ts +30 −14
packages/core/sdk/src/oauth-discovery.ts +41 −7
packages/core/sdk/src/oauth-flow.test.ts +149 −0
packages/core/sdk/src/oauth-helpers.test.ts +229 −15
packages/core/sdk/src/oauth-helpers.ts +109 −25
packages/core/sdk/src/oauth-service.ts +51 −19

@@ -0,0 +1,23 @@
	1	+---
	2	+"executor": patch
	3	+---
	4	+
	5	+OAuth token material is now carried as `Redacted<string>`. `OAuth2TokenResponse`
	6	+returns `access_token` and `refresh_token` wrapped, the PKCE code verifier and
	7	+the RFC 6749 `state` are wrapped for their in-memory life, and a DCR response's
	8	+`client_secret` / `registration_access_token` are wrapped as they are decoded.
	9	+The unwraps are confined to the oauth4webapi call boundary, the credential
	10	+provider write, and the `oauth_session.pkce_verifier` column.
	11	+
	12	+Inputs are widened, not narrowed: `clientSecret`, `code`, `codeVerifier`,
	13	+`refreshToken`, and the DCR `initialAccessToken` all accept
	14	+`string \| Redacted<string>`, so existing callers are unaffected. `OAuthClient.
	15	+clientSecret` accepts either as well; `null` remains the only spelling of
	16	+"public / PKCE client", and `oauthClientSecretFromInput` now tests emptiness on
	17	+the unwrapped value, since every `Redacted` is truthy.
	18	+
	19	+Also fixes a leak this surfaced: `OAuth2Error.cause` carried the token
	20	+endpoint's parsed error body verbatim, so a rejected grant republished the
	21	+credentials the endpoint echoed back into anything that serialized the failure
	22	+(a log line, an error payload). The cause is now redacted structurally, keeping
	23	+the RFC 6749 `error` / `error_description` diagnostics.

packages/core/sdk/src/executor.ts+11−12

@@ -1531,17 +1531,16 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 1530 unmodified lines
1531	1531	// Null means "public client, send no client_secret" — both for a row
1532	1532	// that never had one and for a confidential row whose vault item is
1533	1533	// gone, which the AS then rejects loudly instead of us guessing.
1534		− // Boundary: the OAuth 2.1 helpers take a bare `client_secret` (it is
1535		− // posted to the token endpoint by oauth4webapi). Unwrap BEFORE
1536		− // presence-normalizing — `Redacted.make("")` is truthy, so an emptiness
1537		− // test on the wrapper would read a public client as confidential.
	1534	+ // It stays wrapped all the way to the OAuth 2.1 helpers, which unwrap
	1535	+ // at the oauth4webapi call. Presence is normalized on the UNWRAPPED
	1536	+ // value — `Redacted.make("")` is truthy, so an emptiness test on the
	1537	+ // wrapper would read a public client as confidential.
1538	1538	const storedClientSecret =
1539	1539	clientRow.client_secret_item_id == null
1540	1540	? null
1541	1541	: yield* provider.get(ProviderItemId.make(String(clientRow.client_secret_item_id)));
1542		− const clientSecret = oauthClientSecretFromInput(
1543		− storedClientSecret === null ? null : Redacted.value(storedClientSecret),
1544		− );
	1542	+ const clientSecret =
	1543	+ storedClientSecret === null ? null : oauthClientSecretFromInput(storedClientSecret);
1545	1544	// Re-request the scopes this connection was GRANTED (RFC 6749 §6: a
1546	1545	// refresh must not exceed the originally-granted scope). Empty → omit
1547	1546	// the param, which the AS treats as "same scopes as granted".
@@ -1600,9 +1599,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 52 unmodified lines
1600	1599	tokenUrl,
1601	1600	clientId: String(clientRow.client_id),
1602	1601	clientSecret,
1603		− // Boundary: the refresh-token grant posts the token as a form
1604		− // field, so the helper takes it bare.
1605		− refreshToken: Redacted.value(refreshToken),
	1602	+ refreshToken,
1606	1603	scopes: grantedScopes,
1607	1604	// RFC 8707: keep the re-minted token bound to the same resource
1608	1605	// (MCP servers require this on refresh).
@@ -1647,8 +1644,10 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 38 unmodified lines
1647	1644	const tokenItemId =
1648	1645	connectionItemIds(row)[PRIMARY_INPUT_VARIABLE] ??
1649	1646	`connection:${row.owner}:${row.integration}:${row.name}:${PRIMARY_INPUT_VARIABLE}`;
	1647	+ // `provider.set` accepts `Redacted` and unwraps at its own write line.
1650	1648	yield* provider.set(ProviderItemId.make(tokenItemId), token.access_token);
1651		− if (token.refresh_token && row.refresh_item_id) {
	1649	+ // Presence, not truthiness — every `Redacted` is truthy.
	1650	+ if (token.refresh_token !== undefined && row.refresh_item_id) {
1652	1651	yield* provider.set(ProviderItemId.make(row.refresh_item_id), token.refresh_token);
1653	1652	}
1654	1653	}
@@ -1670,7 +1669,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 15 unmodified lines
1670	1669	set,
1671	1670	});
1672	1671
1673		− return Redacted.make(token.access_token);
	1672	+ return token.access_token;
1674	1673	});
1675	1674
1676	1675	const refreshConnectionToken = (

packages/core/sdk/src/oauth-client.ts+30−14

@@ -1,5 +1,5 @@
1	1	import type { Effect } from "effect";
2		−import { Schema } from "effect";
	2	+import { Redacted, Schema } from "effect";
3	3
4	4	import type { Connection } from "./connection";
5	5	import type { UserActionableError } from "./errors";
@@ -56,24 +56,40 @@ export interface OAuthClient { · 50 unmodified lines
56	56	readonly tokenUrl: string;
57	57	readonly grant: OAuthGrant;
58	58	readonly clientId: string;
59		− /** The literal client secret, or `null` for a public / PKCE client. Stored
60		− * out-of-band in the credential provider (vault item id), never inline.
61		− * Presence is `null`, never `""`: an empty string is a value, so any
62		− * falsiness or emptiness test on it is a bug waiting to happen. Convert
63		− * form/wire input through `clientSecretFromInput` at the boundary. */
64		− readonly clientSecret: string \| null;
	59	+ /** The client secret, or `null` for a public / PKCE client. Stored out-of-band
	60	+ * in the credential provider (vault item id), never inline. Accepts a bare
	61	+ * string as well as `Redacted` so a pasted form field needs no wrapping;
	62	+ * everything downstream of `createClient` carries it wrapped.
	63	+ * Presence is `null`, never `""` and never `Redacted.make("")`: an empty
	64	+ * string is a value and every `Redacted` is truthy, so any falsiness or
	65	+ * emptiness test on it is a bug waiting to happen. Convert form/wire input
	66	+ * through `oauthClientSecretFromInput` at the boundary. */
	67	+ readonly clientSecret: string \| Redacted.Redacted<string> \| null;
65	68	/** RFC 8707 Resource Indicator (MCP). Carried so the refresh request can keep
66	69	* the re-minted token bound to the same resource. Null/omitted otherwise. */
67	70	readonly resource?: string \| null;
68	71	}
69	72
70		−/** Presence-normalize a client secret arriving from a form field or the wire,
71		− * where "absent" can only be spelled as the empty string. Deliberately does NOT
72		− * trim: a whitespace-bearing secret is a value, and trimming it here would
73		− * silently downgrade a confidential client to a public one. Callers that want
74		− * the user's leading/trailing whitespace dropped trim before calling. */
75		−export const oauthClientSecretFromInput = (value: string \| null \| undefined): string \| null =>
76		− value == null \|\| value.length === 0 ? null : value;
	73	+/** Presence-normalize a client secret arriving from a form field, the wire, or a
	74	+ * DCR response, where "absent" can only be spelled as the empty string.
	75	+ * Emptiness is tested on the UNWRAPPED value: every `Redacted` is truthy, so a
	76	+ * wrapped `""` would otherwise register a confidential client with an empty
	77	+ * secret. The wrapper is preserved — normalizing does not unwrap.
	78	+ * Deliberately does NOT trim: a whitespace-bearing secret is a value, and
	79	+ * trimming it here would silently downgrade a confidential client to a public
	80	+ * one. Callers that want the user's leading/trailing whitespace dropped trim
	81	+ * before calling. */
	82	+export function oauthClientSecretFromInput(value: string \| null \| undefined): string \| null;
	83	+export function oauthClientSecretFromInput<A extends string \| Redacted.Redacted<string>>(
	84	+ value: A \| null \| undefined,
	85	+): A \| null;
	86	+export function oauthClientSecretFromInput(
	87	+ value: string \| Redacted.Redacted<string> \| null \| undefined,
	88	+): string \| Redacted.Redacted<string> \| null {
	89	+ if (value == null) return null;
	90	+ const literal = Redacted.isRedacted(value) ? Redacted.value(value) : value;
	91	+ return literal.length === 0 ? null : value;
	92	+}
77	93
78	94	export type OAuthClientOrigin =
79	95	\| {

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

@@ -18,7 +18,18 @@ · 17 unmodified lines
18	18	// call callers actually need.
19	19	// ---------------------------------------------------------------------------
20	20
21		−import { Data, Duration, Effect, Layer, Option, Predicate, Result, Schema } from "effect";
	21	+import {
	22	+ Data,
	23	+ Duration,
	24	+ Effect,
	25	+ Layer,
	26	+ Option,
	27	+ Predicate,
	28	+ Redacted,
	29	+ Result,
	30	+ Schema,
	31	+ SchemaGetter,
	32	+} from "effect";
22	33	import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http";
23	34
24	35	import {
@@ -27,6 +38,7 @@ import { · 2 unmodified lines
27	38	buildAuthorizationUrl,
28	39	createPkceCodeChallenge,
29	40	createPkceCodeVerifier,
	41	+ type OAuth2SecretInput,
30	42	type OAuthEndpointUrlPolicy,
31	43	} from "./oauth-helpers";
32	44
@@ -106,12 +118,28 @@ export type DynamicClientMetadata = { · 73 unmodified lines
106	118	readonly extra?: Readonly<Record<string, unknown>>;
107	119	};
108	120
	121	+/** A bidirectional string ⇄ `Redacted<string>` codec. `Schema.Redacted` and
	122	+ * `Schema.RedactedFromValue` forbid encoding, which would make any struct
	123	+ * carrying one decode-only; DCR client information round-trips (it is cached
	124	+ * and replayed through `previousState`), so the encode side is required. */
	125	+const RedactedString = Schema.String.pipe(
	126	+ Schema.decodeTo(Schema.Redacted(Schema.String), {
	127	+ decode: SchemaGetter.transform(Redacted.make<string>),
	128	+ encode: SchemaGetter.transform(Redacted.value<string>),
	129	+ }),
	130	+);
	131	+
109	132	export const OAuthClientInformationSchema = Schema.Struct({
110	133	client_id: Schema.String,
111		− client_secret: Schema.optional(Schema.String),
	134	+ /** Minted by the AS for a confidential DCR client. Redacted from the moment
	135	+ * the registration response is decoded — it is posted to the token endpoint
	136	+ * and stored in the credential provider, and reaches nothing else. */
	137	+ client_secret: Schema.optional(RedactedString),
112	138	client_id_issued_at: Schema.optional(Schema.Number),
113	139	client_secret_expires_at: Schema.optional(Schema.Number),
114		− registration_access_token: Schema.optional(Schema.String),
	140	+ /** RFC 7592 bearer token for the client's own registration resource — as
	141	+ * privileged as the client secret. */
	142	+ registration_access_token: Schema.optional(RedactedString),
115	143	registration_client_uri: Schema.optional(Schema.String),
116	144	token_endpoint_auth_method: Schema.optional(Schema.String),
117	145	grant_types: Schema.optional(StringArray),
@@ -413,7 +441,7 @@ export const discoverAuthorizationServerMetadata = ( · 295 unmodified lines
413	441	export interface RegisterDynamicClientInput {
414	442	readonly registrationEndpoint: string;
415	443	readonly metadata: DynamicClientMetadata;
416		− readonly initialAccessToken?: string \| null;
	444	+ readonly initialAccessToken?: OAuth2SecretInput \| null;
417	445	}
418	446
419	447	// Internal failure modes — collapsed into `OAuthDiscoveryError` at the
@@ -513,8 +541,12 @@ export const registerDynamicClient = ( · 93 unmodified lines
513	541	"content-type": "application/json",
514	542	accept: "application/json",
515	543	};
516		− if (input.initialAccessToken) {
517		− headers.authorization = `Bearer ${input.initialAccessToken}`;
	544	+ if (input.initialAccessToken != null) {
	545	+ // Boundary: the RFC 7591 initial access token goes out as a bearer header.
	546	+ const token = Redacted.isRedacted(input.initialAccessToken)
	547	+ ? Redacted.value(input.initialAccessToken)
	548	+ : input.initialAccessToken;
	549	+ if (token.length > 0) headers.authorization = `Bearer ${token}`;
518	550	}
519	551
520	552	let request = HttpClientRequest.post(input.registrationEndpoint).pipe(
@@ -661,7 +693,9 @@ export interface DynamicAuthorizationState { · 140 unmodified lines
661	693
662	694	export interface DynamicAuthorizationStartResult {
663	695	readonly authorizationUrl: string;
664		− readonly codeVerifier: string;
	696	+ /** Grant material — the caller holds it until the matching token exchange.
	697	+ * Unwrap only at that exchange or at the line that persists it. */
	698	+ readonly codeVerifier: Redacted.Redacted<string>;
665	699	readonly state: DynamicAuthorizationState;
666	700	}
667	701

packages/core/sdk/src/oauth-flow.test.ts+149−0

@@ -1331,6 +1331,155 @@ describe("oauth client secret presence", () => { · 1330 unmodified lines
1331	1331	);
1332	1332	});
1333	1333
	1334	+// ---------------------------------------------------------------------------
	1335	+// Persisted bytes.
	1336	+//
	1337	+// A missed unwrap on a WRITE path does not throw: `Redacted`'s toString/toJSON
	1338	+// render "<redacted>", so the literal is stored and the failure only surfaces
	1339	+// later, as an authorization error against a provider. These assert the stored
	1340	+// value, not the in-memory one — the in-memory half is covered in
	1341	+// `oauth-helpers.test.ts`.
	1342	+// ---------------------------------------------------------------------------
	1343	+
	1344	+describe("OAuth material persists as its value, never as the redaction marker", () => {
	1345	+ it.effect("the minted access + refresh tokens and the PKCE verifier store literally", () =>
	1346	+ Effect.scoped(
	1347	+ Effect.gen(function* () {
	1348	+ const server = yield* serveOAuthTestServer({ scopes: ["read"] });
	1349	+ const { config, executor } = yield* makeTestWorkspaceHarness({ plugins });
	1350	+ yield* executor.acme.seed(["read"]);
	1351	+
	1352	+ yield* executor.oauth.createClient({
	1353	+ owner: "org",
	1354	+ slug: CLIENT,
	1355	+ authorizationUrl: server.authorizationEndpoint,
	1356	+ tokenUrl: server.tokenEndpoint,
	1357	+ grant: "authorization_code",
	1358	+ clientId: "test-client",
	1359	+ clientSecret: "test-secret",
	1360	+ });
	1361	+
	1362	+ const started = yield* executor.oauth.start({
	1363	+ owner: "org",
	1364	+ client: CLIENT,
	1365	+ clientOwner: "org",
	1366	+ name: ConnectionName.make("main"),
	1367	+ integration: INTEG,
	1368	+ template: TEMPLATE,
	1369	+ });
	1370	+ expect(started.status).toBe("redirect");
	1371	+ if (started.status !== "redirect") return;
	1372	+
	1373	+ // The session's PKCE verifier is written by `start`. If it stored
	1374	+ // "<redacted>" the exchange below would fail RFC 7636 validation — but
	1375	+ // assert the column too, so the reason is legible when it does.
	1376	+ const sessionRow = yield* Effect.promise(() =>
	1377	+ config.db.findFirst("oauth_session", {
	1378	+ where: (b) => b("state", "=", String(started.state)),
	1379	+ }),
	1380	+ );
	1381	+ const storedVerifier = String(sessionRow?.pkce_verifier);
	1382	+ expect(storedVerifier).not.toBe("<redacted>");
	1383	+ expect(storedVerifier).toMatch(/^[A-Za-z0-9_-]{43,128}$/);
	1384	+
	1385	+ const callback = yield* server.completeAuthorizationCodeFlow({
	1386	+ authorizationUrl: started.authorizationUrl,
	1387	+ });
	1388	+ yield* executor.oauth.complete({ state: started.state, code: callback.code });
	1389	+
	1390	+ // The stored access token is the one the AS issued: the tool echoes the
	1391	+ // resolved credential, and the server still honours it.
	1392	+ const echoed = (yield* executor.execute(
	1393	+ ToolAddress.make("tools.acme.org.main.whoami"),
	1394	+ {},
	1395	+ )) as { token: string };
	1396	+ expect(echoed.token).not.toBe("<redacted>");
	1397	+ expect(echoed.token).toMatch(/^at_/);
	1398	+ expect(yield* server.acceptsAccessToken(echoed.token)).toBe(true);
	1399	+
	1400	+ // The refresh token is stored under its own item id and must be equally
	1401	+ // literal — nothing reads it until a refresh, so a "<redacted>" write
	1402	+ // here would lie dormant until the access token first expired.
	1403	+ const connectionRow = yield* Effect.promise(() =>
	1404	+ config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }),
	1405	+ );
	1406	+ expect(connectionRow?.refresh_item_id ?? null).not.toBeNull();
	1407	+
	1408	+ // Force expiry so the refresh grant runs against the stored refresh
	1409	+ // token. The AS matches it by exact string, so a grant that succeeds is
	1410	+ // proof the token round-tripped through the provider byte for byte.
	1411	+ yield* Effect.promise(() =>
	1412	+ config.db.updateMany("connection", {
	1413	+ where: (b) => b("name", "=", "main"),
	1414	+ set: { expires_at: Date.now() - 60_000 },
	1415	+ }),
	1416	+ );
	1417	+ const refreshed = (yield* executor.execute(
	1418	+ ToolAddress.make("tools.acme.org.main.whoami"),
	1419	+ {},
	1420	+ )) as { token: string };
	1421	+ expect(refreshed.token).not.toBe("<redacted>");
	1422	+ expect(refreshed.token).toMatch(/^at_/);
	1423	+ expect(refreshed.token).not.toBe(echoed.token);
	1424	+ expect(yield* server.acceptsAccessToken(refreshed.token)).toBe(true);
	1425	+ }),
	1426	+ ),
	1427	+ );
	1428	+
	1429	+ it.effect("a DCR-minted client secret authenticates instead of storing <redacted>", () =>
	1430	+ // The DCR secret is decoded straight out of the registration response into
	1431	+ // a `Redacted` field, then written to the provider and later posted to the
	1432	+ // token endpoint. A missed unwrap anywhere on that path stores the marker
	1433	+ // and the AS rejects the grant with `invalid_client`.
	1434	+ Effect.scoped(
	1435	+ Effect.gen(function* () {
	1436	+ const server = yield* serveOAuthTestServer({ scopes: ["read"] });
	1437	+ const { config, executor } = yield* makeTestWorkspaceHarness({ plugins });
	1438	+ yield* executor.acme.seed(["read"]);
	1439	+
	1440	+ const slug = yield* executor.oauth.registerDynamicClient({
	1441	+ owner: "org",
	1442	+ slug: CLIENT,
	1443	+ issuer: server.issuerUrl,
	1444	+ registrationEndpoint: server.registrationEndpoint,
	1445	+ authorizationUrl: server.authorizationEndpoint,
	1446	+ tokenUrl: server.tokenEndpoint,
	1447	+ scopes: ["read"],
	1448	+ // Advertising no `none` makes DCR register a CONFIDENTIAL client, so
	1449	+ // the server mints a secret we must carry back to /token.
	1450	+ tokenEndpointAuthMethodsSupported: ["client_secret_post"],
	1451	+ });
	1452	+
	1453	+ const row = yield* Effect.promise(() =>
	1454	+ config.db.findFirst("oauth_client", { where: (b) => b("slug", "=", String(slug)) }),
	1455	+ );
	1456	+ expect(row?.client_secret_item_id ?? null).not.toBeNull();
	1457	+
	1458	+ const started = yield* executor.oauth.start({
	1459	+ owner: "org",
	1460	+ client: slug,
	1461	+ clientOwner: "org",
	1462	+ name: ConnectionName.make("main"),
	1463	+ integration: INTEG,
	1464	+ template: TEMPLATE,
	1465	+ });
	1466	+ expect(started.status).toBe("redirect");
	1467	+ if (started.status !== "redirect") return;
	1468	+ const callback = yield* server.completeAuthorizationCodeFlow({
	1469	+ authorizationUrl: started.authorizationUrl,
	1470	+ });
	1471	+ // The exchange succeeding IS the assertion: the test server rejects a
	1472	+ // mismatched client_secret with invalid_client.
	1473	+ yield* executor.oauth.complete({ state: started.state, code: callback.code });
	1474	+
	1475	+ const grant = tokenGrantBodies(yield* server.requests).at(0);
	1476	+ expect(grant?.get("client_secret")).not.toBe("<redacted>");
	1477	+ expect(grant?.get("client_secret") ?? "").not.toBe("");
	1478	+ }),
	1479	+ ),
	1480	+ );
	1481	+});
	1482	+
1334	1483	describe("oauth.probe error message sanitization", () => {
1335	1484	it.effect("echoes the probed URL without its query string or userinfo", () =>
1336	1485	// The probe URL is a raw user paste and a supported credential carrier (an

packages/core/sdk/src/oauth-helpers.test.ts+229−15

@@ -6,7 +6,7 @@ · 5 unmodified lines
6	6	// ---------------------------------------------------------------------------
7	7
8	8	import { describe, expect, it } from "@effect/vitest";
9		−import { Effect, Exit, Ref } from "effect";
	9	+import { Effect, Exit, Logger, Redacted, Ref } from "effect";
10	10	import { HttpServerResponse } from "effect/unstable/http";
11	11
12	12	import {
@@ -15,6 +15,7 @@ import { · 2 unmodified lines
15	15	OAuth2Error,
16	16	buildAuthorizationUrl,
17	17	providerAuthorizeExtras,
	18	+ createOAuthState,
18	19	createPkceCodeChallenge,
19	20	createPkceCodeVerifier,
20	21	exchangeAuthorizationCode,
@@ -99,6 +100,11 @@ const tokenResponse = · 78 unmodified lines
99	100	() =>
100	101	json(200, body);
101	102
	103	+/** Assert on the token VALUE, not on `Redacted`'s "<redacted>" rendering — a
	104	+ * `toBe` against the wrapper would pass for any implementation. */
	105	+const secretOf = (value: Redacted.Redacted<string> \| undefined): string \| undefined =>
	106	+ value === undefined ? undefined : Redacted.value(value);
	107	+
102	108	// ---------------------------------------------------------------------------
103	109	// PKCE
104	110	// ---------------------------------------------------------------------------
@@ -106,7 +112,7 @@ const tokenResponse = · 1 unmodified line
106	112	describe("PKCE", () => {
107	113	it("createPkceCodeVerifier returns a base64url string in the RFC 7636 length range", () => {
108	114	for (let i = 0; i < 25; i++) {
109		− const verifier = createPkceCodeVerifier();
	115	+ const verifier = Redacted.value(createPkceCodeVerifier());
110	116	expect(verifier).toMatch(/^[A-Za-z0-9_-]+$/);
111	117	expect(verifier.length).toBeGreaterThanOrEqual(43);
112	118	expect(verifier.length).toBeLessThanOrEqual(128);
@@ -117,13 +123,26 @@ describe("PKCE", () => { · 4 unmodified lines
117	123	const verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk";
118	124	const expected = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM";
119	125	expect(await createPkceCodeChallenge(verifier)).toBe(expected);
	126	+ // A wrapped verifier hashes identically — the challenge is computed from
	127	+ // the value, not from `Redacted`'s "<redacted>" rendering.
	128	+ expect(await createPkceCodeChallenge(Redacted.make(verifier))).toBe(expected);
120	129	});
121	130
122	131	it("createPkceCodeVerifier produces unique values", () => {
123	132	const seen = new Set<string>();
124		− for (let i = 0; i < 50; i++) seen.add(createPkceCodeVerifier());
	133	+ for (let i = 0; i < 50; i++) seen.add(Redacted.value(createPkceCodeVerifier()));
125	134	expect(seen.size).toBe(50);
126	135	});
	136	+
	137	+ it("PKCE and state material serializes as <redacted> rather than its value", () => {
	138	+ // These are minted long before they reach a DB column or an authorize URL,
	139	+ // so anything that stringifies them in between (a log line, a span
	140	+ // attribute, a session dump) must not print the grant material.
	141	+ for (const secret of [createPkceCodeVerifier(), createOAuthState()]) {
	142	+ expect(JSON.stringify({ secret })).toBe('{"secret":"<redacted>"}');
	143	+ expect(String(secret)).not.toContain(Redacted.value(secret));
	144	+ }
	145	+ });
127	146	});
128	147
129	148	// ---------------------------------------------------------------------------
@@ -259,7 +278,7 @@ describe("exchangeAuthorizationCode", () => { · 129 unmodified lines
259	278	codeVerifier: "verifier",
260	279	code: "abc",
261	280	});
262		− expect(result.access_token).toBe("tok");
	281	+ expect(secretOf(result.access_token)).toBe("tok");
263	282	const call = (yield* calls)[0]!;
264	283	expect(call.method).toBe("POST");
265	284	expect(call.headers["content-type"]).toMatch(/^application\/x-www-form-urlencoded/);
@@ -398,8 +417,8 @@ describe("exchangeAuthorizationCode", () => { · 132 unmodified lines
398	417	codeVerifier: "verifier",
399	418	code: "abc",
400	419	});
401		− expect(result.access_token).toBe("tok");
402		− expect(result.refresh_token).toBe("rtok");
	420	+ expect(secretOf(result.access_token)).toBe("tok");
	421	+ expect(secretOf(result.refresh_token)).toBe("rtok");
403	422	expect(result.idTokenIdentityLabel).toBe("user-1");
404	423	}),
405	424	),
@@ -473,7 +492,7 @@ describe("exchangeAuthorizationCode", () => { · 67 unmodified lines
473	492	codeVerifier: "verifier",
474	493	code: "abc",
475	494	});
476		− expect(result.access_token).toBe("tok");
	495	+ expect(secretOf(result.access_token)).toBe("tok");
477	496	expect(result.idTokenIdentityLabel).toBeUndefined();
478	497	}),
479	498	),
@@ -501,7 +520,7 @@ describe("exchangeAuthorizationCode", () => { · 21 unmodified lines
501	520	codeVerifier: "verifier",
502	521	code: "abc",
503	522	});
504		− expect(result.access_token).toBe("tok");
	523	+ expect(secretOf(result.access_token)).toBe("tok");
505	524	}),
506	525	),
507	526	);
@@ -516,8 +535,8 @@ describe("exchangeAuthorizationCode", () => { · 8 unmodified lines
516	535	codeVerifier: "verifier",
517	536	code: "abc",
518	537	});
519		− expect(result.access_token).toBe("tok");
520		− expect(result.refresh_token).toBe("rtok");
	538	+ expect(secretOf(result.access_token)).toBe("tok");
	539	+ expect(secretOf(result.refresh_token)).toBe("rtok");
521	540	expect(result.expires_in).toBe(3600);
522	541	expect(result.idTokenIdentityLabel).toBeUndefined();
523	542	}),
@@ -577,8 +596,8 @@ describe("exchangeAuthorizationCode", () => { · 53 unmodified lines
577	596	codeVerifier: "verifier",
578	597	code: "abc",
579	598	});
580		− expect(result.access_token).toBe("tok");
581		− expect(result.refresh_token).toBe("rtok");
	599	+ expect(secretOf(result.access_token)).toBe("tok");
	600	+ expect(secretOf(result.refresh_token)).toBe("rtok");
582	601	}),
583	602	),
584	603	);
@@ -976,7 +995,7 @@ describe("refreshAccessToken", () => { · 391 unmodified lines
976	995	clientId: "cid",
977	996	refreshToken: "old",
978	997	});
979		− expect(result.access_token).toBe("tok2");
	998	+ expect(secretOf(result.access_token)).toBe("tok2");
980	999	}),
981	1000	),
982	1001	);
@@ -1004,7 +1023,7 @@ describe("refreshAccessToken", () => { · 21 unmodified lines
1004	1023	clientId: "cid",
1005	1024	refreshToken: "old",
1006	1025	});
1007		− expect(result.access_token).toBe("tok2");
	1026	+ expect(secretOf(result.access_token)).toBe("tok2");
1008	1027	}),
1009	1028	),
1010	1029	);
@@ -1017,13 +1036,208 @@ describe("refreshAccessToken", () => { · 6 unmodified lines
1017	1036	clientId: "cid",
1018	1037	refreshToken: "old",
1019	1038	});
1020		− expect(result.access_token).toBe("tok2");
	1039	+ expect(secretOf(result.access_token)).toBe("tok2");
1021	1040	expect(result.expires_in).toBe(3600);
1022	1041	}),
1023	1042	),
1024	1043	);
1025	1044	});
1026	1045
	1046	+// ---------------------------------------------------------------------------
	1047	+// Token material is Redacted end to end.
	1048	+//
	1049	+// Two halves, both required: the WIRE half (the secret still reaches the token
	1050	+// endpoint byte for byte, so the wrapping is not merely decorative) and the
	1051	+// LEAK half (a failed exchange's error and log output quotes neither the tokens
	1052	+// the caller sent nor the ones the AS echoed back). Either half alone passes
	1053	+// trivially for a broken implementation.
	1054	+// ---------------------------------------------------------------------------
	1055	+
	1056	+// Synthetic — not shaped like any real provider's token.
	1057	+const SENT_REFRESH = "synthetic-sent-refresh-value";
	1058	+const SENT_SECRET = "synthetic-sent-client-secret";
	1059	+const ECHOED_ACCESS = "synthetic-echoed-access-value";
	1060	+const ECHOED_REFRESH = "synthetic-echoed-refresh-value";
	1061	+
	1062	+describe("OAuth token material as Redacted", () => {
	1063	+ it.effect("a wrapped refresh token and client secret reach the token endpoint intact", () =>
	1064	+ withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) =>
	1065	+ Effect.gen(function* () {
	1066	+ yield* refreshAccessToken({
	1067	+ tokenUrl,
	1068	+ clientId: "cid",
	1069	+ clientSecret: Redacted.make(SENT_SECRET),
	1070	+ refreshToken: Redacted.make(SENT_REFRESH),
	1071	+ });
	1072	+ const body = (yield* calls)[0]!.body;
	1073	+ expect(body.get("refresh_token")).toBe(SENT_REFRESH);
	1074	+ expect(body.get("client_secret")).toBe(SENT_SECRET);
	1075	+ }),
	1076	+ ),
	1077	+ );
	1078	+
	1079	+ it.effect("a wrapped code and PKCE verifier reach the token endpoint intact", () =>
	1080	+ withTokenEndpoint(tokenResponse(validCodeBody), ({ tokenUrl, calls }) =>
	1081	+ Effect.gen(function* () {
	1082	+ yield* exchangeAuthorizationCode({
	1083	+ tokenUrl,
	1084	+ clientId: "cid",
	1085	+ clientSecret: Redacted.make(SENT_SECRET),
	1086	+ redirectUrl: "https://app.example.com/cb",
	1087	+ codeVerifier: Redacted.make("synthetic-verifier-value"),
	1088	+ code: Redacted.make("synthetic-code-value"),
	1089	+ });
	1090	+ const body = (yield* calls)[0]!.body;
	1091	+ expect(body.get("code")).toBe("synthetic-code-value");
	1092	+ expect(body.get("code_verifier")).toBe("synthetic-verifier-value");
	1093	+ expect(body.get("client_secret")).toBe(SENT_SECRET);
	1094	+ }),
	1095	+ ),
	1096	+ );
	1097	+
	1098	+ it.effect("a successful exchange returns tokens that serialize as <redacted>", () =>
	1099	+ // Distinct synthetic values rather than the shared `validCodeBody` fixture:
	1100	+ // its "tok" is a substring of the `token_type` key, so a not-to-contain
	1101	+ // assertion against it could never be trusted.
	1102	+ withTokenEndpoint(
	1103	+ tokenResponse({
	1104	+ access_token: ECHOED_ACCESS,
	1105	+ token_type: "Bearer",
	1106	+ refresh_token: ECHOED_REFRESH,
	1107	+ expires_in: 3600,
	1108	+ }),
	1109	+ ({ tokenUrl }) =>
	1110	+ Effect.gen(function* () {
	1111	+ const result = yield* exchangeAuthorizationCode({
	1112	+ tokenUrl,
	1113	+ clientId: "cid",
	1114	+ redirectUrl: "https://app.example.com/cb",
	1115	+ codeVerifier: "v",
	1116	+ code: "c",
	1117	+ });
	1118	+ // The values survive an explicit unwrap …
	1119	+ expect(secretOf(result.access_token)).toBe(ECHOED_ACCESS);
	1120	+ expect(secretOf(result.refresh_token)).toBe(ECHOED_REFRESH);
	1121	+ // … but a caller that forgets one serializes the marker, not the token.
	1122	+ const serialized = JSON.stringify(result);
	1123	+ expect(serialized).toContain('"access_token":"<redacted>"');
	1124	+ expect(serialized).toContain('"refresh_token":"<redacted>"');
	1125	+ expect(serialized).not.toContain(ECHOED_ACCESS);
	1126	+ expect(serialized).not.toContain(ECHOED_REFRESH);
	1127	+ }),
	1128	+ ),
	1129	+ );
	1130	+
	1131	+ it.effect("an AS that returns no refresh_token yields undefined, not a wrapped empty", () =>
	1132	+ // Presence must survive the wrapping: `Redacted.make("")` is truthy, so a
	1133	+ // downstream truthiness test on the field would report a refresh token that
	1134	+ // does not exist and mark the connection refreshable when it is not.
	1135	+ withTokenEndpoint(
	1136	+ tokenResponse({ access_token: "tok", token_type: "Bearer", expires_in: 3600 }),
	1137	+ ({ tokenUrl }) =>
	1138	+ Effect.gen(function* () {
	1139	+ const result = yield* exchangeAuthorizationCode({
	1140	+ tokenUrl,
	1141	+ clientId: "cid",
	1142	+ redirectUrl: "https://app.example.com/cb",
	1143	+ codeVerifier: "v",
	1144	+ code: "c",
	1145	+ });
	1146	+ expect(result.refresh_token).toBeUndefined();
	1147	+ expect(Redacted.isRedacted(result.access_token)).toBe(true);
	1148	+ }),
	1149	+ ),
	1150	+ );
	1151	+
	1152	+ it.effect("a failed exchange leaks neither the sent nor the echoed token material", () =>
	1153	+ // The token endpoint echoes the grant it rejected, and the failure summary
	1154	+ // quotes that body. This is the silent-failure shape the whole change
	1155	+ // exists for: the error reaches a user-visible message AND the log, so
	1156	+ // assert against both renderings of the failure.
	1157	+ withTokenEndpoint(
	1158	+ () =>
	1159	+ json(400, {
	1160	+ error: "invalid_grant",
	1161	+ error_description: "Refresh token is no longer valid",
	1162	+ refresh_token: ECHOED_REFRESH,
	1163	+ access_token: ECHOED_ACCESS,
	1164	+ }),
	1165	+ ({ tokenUrl }) =>
	1166	+ Effect.gen(function* () {
	1167	+ // `Effect.flip` keeps the failure typed as `OAuth2Error` — the tagged
	1168	+ // error carries `message` and `error` as declared fields.
	1169	+ const error = yield* Effect.flip(
	1170	+ refreshAccessToken({
	1171	+ tokenUrl,
	1172	+ clientId: "cid",
	1173	+ clientSecret: Redacted.make(SENT_SECRET),
	1174	+ refreshToken: Redacted.make(SENT_REFRESH),
	1175	+ }),
	1176	+ );
	1177	+ // The diagnostic that makes the failure worth surfacing survives.
	1178	+ expect(error.error).toBe("invalid_grant");
	1179	+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message`; the leak test asserts on its rendering
	1180	+ expect(error.message).toContain("Refresh token is no longer valid");
	1181	+
	1182	+ // The rendering a log actually emits: the error goes through the
	1183	+ // logger's own cause serialization, not a hand-rolled stringify.
	1184	+ const logged = yield* Effect.gen(function* () {
	1185	+ const lines = yield* Ref.make<readonly string[]>([]);
	1186	+ yield* Effect.logError("OAuth token refresh failed", error).pipe(
	1187	+ Effect.provide(
	1188	+ Logger.layer([
	1189	+ Logger.make((options) =>
	1190	+ Effect.runSync(
	1191	+ Ref.update(lines, (all) => [
	1192	+ ...all,
	1193	+ `${options.message} ${String(options.cause)}`,
	1194	+ ]),
	1195	+ ),
	1196	+ ),
	1197	+ ]),
	1198	+ ),
	1199	+ );
	1200	+ return (yield* Ref.get(lines)).join("\n");
	1201	+ });
	1202	+
	1203	+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuth2Error carries a typed `message`; the leak test asserts on its rendering
	1204	+ for (const rendering of [error.message, JSON.stringify(error), logged]) {
	1205	+ for (const secret of [
	1206	+ SENT_REFRESH,
	1207	+ SENT_SECRET,
	1208	+ ECHOED_ACCESS,
	1209	+ ECHOED_REFRESH,
	1210	+ ] as const) {
	1211	+ expect(rendering).not.toContain(secret);
	1212	+ }
	1213	+ }
	1214	+ }),
	1215	+ ),
	1216	+ );
	1217	+
	1218	+ it.effect("redacting the cause keeps a transport failure's own diagnostics", () =>
	1219	+ // A native fetch failure has NO enumerable own property — its name,
	1220	+ // message, and stack are all non-enumerable — so projecting the cause
	1221	+ // through a plain object walk silently reduces it to `{}`. Redaction must
	1222	+ // not cost the only diagnostic a connection-refused failure has.
	1223	+ Effect.gen(function* () {
	1224	+ const error = yield* Effect.flip(
	1225	+ exchangeAuthorizationCode({
	1226	+ tokenUrl: "http://127.0.0.1:1/token",
	1227	+ clientId: "cid",
	1228	+ redirectUrl: "https://cb",
	1229	+ codeVerifier: "v",
	1230	+ code: "c",
	1231	+ timeoutMs: 100,
	1232	+ }),
	1233	+ );
	1234	+ expect(Object.keys(error.cause as object)).toEqual(
	1235	+ expect.arrayContaining(["name", "message"]),
	1236	+ );
	1237	+ }),
	1238	+ );
	1239	+});
	1240	+
1027	1241	describe("shouldRefreshToken", () => {
1028	1242	it("never refreshes when expiresAt is null", () => {
1029	1243	expect(shouldRefreshToken({ expiresAt: null })).toBe(false);

packages/core/sdk/src/oauth-helpers.ts+109−25

@@ -16,7 +16,7 @@ · 15 unmodified lines
16	16	// construction keeps the call sync and lets callers opt out of PAR
17	17	// ---------------------------------------------------------------------------
18	18
19		−import { Data, Effect, Option, Predicate, Schema } from "effect";
	19	+import { Data, Effect, Option, Predicate, Redacted, Schema } from "effect";
20	20	import * as oauth from "oauth4webapi";
21	21
22	22	import { endpointForTelemetry } from "./telemetry-endpoint";
@@ -41,10 +41,25 @@ export class OAuth2Error extends Data.TaggedError("OAuth2Error")<{ · 18 unmodified lines
41	41	// Token response shape (RFC 6749 §5.1)
42	42	// ---------------------------------------------------------------------------
43	43
	44	+/** Token material a caller hands one of these helpers. Widened to accept a bare
	45	+ * string as well as `Redacted` so a value that never left the process (a code
	46	+ * straight off the callback query string, a pasted secret) does not have to be
	47	+ * wrapped first. Outputs are always `Redacted` — the guarantee lives there. */
	48	+export type OAuth2SecretInput = string \| Redacted.Redacted<string>;
	49	+
	50	+/** Unwrap at an allowlisted boundary: `oauth4webapi` takes bare strings and puts
	51	+ * them on the wire. Every call site is a form field or an HTTP header. */
	52	+const secretToSend = (value: OAuth2SecretInput): string =>
	53	+ Redacted.isRedacted(value) ? Redacted.value(value) : value;
	54	+
44	55	export type OAuth2TokenResponse = {
45		− readonly access_token: string;
	56	+ /** Wrapped at construction (`tokenResponseFrom`) so the minted token cannot
	57	+ * reach a log, a span attribute, or an error payload without an explicit
	58	+ * `Redacted.value`. Unwrap only at a persistence or wire line — a missed
	59	+ * unwrap does not throw, it writes the literal "<redacted>". */
	60	+ readonly access_token: Redacted.Redacted<string>;
46	61	readonly token_type?: string;
47		− readonly refresh_token?: string;
	62	+ readonly refresh_token?: Redacted.Redacted<string>;
48	63	readonly expires_in?: number;
49	64	readonly scope?: string;
50	65	readonly idTokenIdentityLabel?: string;
@@ -105,14 +120,22 @@ export const assertSupportedOAuthEndpointUrl = ( · 54 unmodified lines
105	120	// PKCE (RFC 7636) — straight delegation to `oauth4webapi`
106	121	// ---------------------------------------------------------------------------
107	122
108		−export const createPkceCodeVerifier = (): string => oauth.generateRandomCodeVerifier();
	123	+/** The PKCE verifier is grant material: whoever holds it plus the authorization
	124	+ * code can redeem the token. Wrapped for its whole in-memory life; the only
	125	+ * unwraps are the challenge computation below and the session-row write. */
	126	+export const createPkceCodeVerifier = (): Redacted.Redacted<string> =>
	127	+ Redacted.make(oauth.generateRandomCodeVerifier());
109	128
110		−export const createPkceCodeChallenge = (verifier: string): Promise<string> =>
111		− oauth.calculatePKCECodeChallenge(verifier);
	129	+/** The challenge is a public S256 hash of the verifier (it travels in the
	130	+ * authorize URL), so it comes back bare. */
	131	+export const createPkceCodeChallenge = (verifier: OAuth2SecretInput): Promise<string> =>
	132	+ oauth.calculatePKCECodeChallenge(secretToSend(verifier));
112	133
113	134	/** RFC 6749 `state` — an unguessable correlation token minted by `oauth.start`
114		− * and redeemed by `oauth.complete`. */
115		−export const createOAuthState = (): string => oauth.generateRandomState();
	135	+ * and redeemed by `oauth.complete`. Wrapped like the verifier: it is the
	136	+ * session's bearer key, and it is persisted and echoed through the provider. */
	137	+export const createOAuthState = (): Redacted.Redacted<string> =>
	138	+ Redacted.make(oauth.generateRandomState());
116	139
117	140	// ---------------------------------------------------------------------------
118	141	// Authorization URL builder
@@ -316,6 +339,62 @@ const redactTokenEndpointBody = (body: string): string => · 197 unmodified lines
316	339	)
317	340	.replaceAll(new RegExp(`((?:${CREDENTIAL_BODY_KEYS}\|code)=)[^&\\s]*`, "gi"), "$1[redacted]");
318	341
	342	+const CREDENTIAL_KEY_PATTERN = new RegExp(`^(?:${CREDENTIAL_BODY_KEYS})$`, "i");
	343	+
	344	+/** The cause an `OAuth2Error` carries is the raw failure oauth4webapi threw, and
	345	+ * for an RFC 6749 §5.2 rejection that object holds the endpoint's PARSED error
	346	+ * body — which echoes the grant it rejected. The `message` is summarized
	347	+ * through `redactTokenEndpointBody`, but the cause is not, and anything that
	348	+ * serializes the failure (a log line, an error payload, `JSON.stringify` of the
	349	+ * Effect cause) republishes those credentials verbatim.
	350	+ *
	351	+ * So the cause is projected into a plain object with every credential-named
	352	+ * leaf replaced, keeping the diagnostics (`error`, `error_description`,
	353	+ * `status`, `name`) that make it worth attaching. `Response` instances pass
	354	+ * through untouched: their body is not enumerable, and the summary already
	355	+ * quoted a redacted preview of it. */
	356	+const redactErrorCause = (
	357	+ cause: unknown,
	358	+ /** True when `cause` is the value of an `error` key, where a `code` entry is
	359	+ * the provider's error LABEL (`{"error":{"code":"invalid_client_id"}}`) and
	360	+ * not an authorization code. Same exemption `redactTokenEndpointBody` makes
	361	+ * for the text form, spelled structurally. */
	362	+ insideErrorObject = false,
	363	+ seen: ReadonlySet<object> = new Set(),
	364	+): unknown => {
	365	+ if (cause instanceof Response) return cause;
	366	+ if (seen.has(cause as object)) return "[circular]";
	367	+ if (Array.isArray(cause)) {
	368	+ const next = new Set([...seen, cause]);
	369	+ return cause.map((entry) => redactErrorCause(entry, false, next));
	370	+ }
	371	+ if (!Predicate.isObject(cause)) return cause;
	372	+ const next = new Set([...seen, cause]);
	373	+ const redacted = Object.fromEntries(
	374	+ Object.entries(cause).map(([key, value]) => [
	375	+ key,
	376	+ CREDENTIAL_KEY_PATTERN.test(key) \|\| (key === "code" && !insideErrorObject)
	377	+ ? "[redacted]"
	378	+ : redactErrorCause(value, key === "error", next),
	379	+ ]),
	380	+ );
	381	+ // oxlint-disable-next-line executor/no-instanceof-error -- boundary: `cause` is the untyped value oauth4webapi threw; narrowing it is how the diagnostics below are recovered
	382	+ if (!(cause instanceof Error)) return redacted;
	383	+ // `name`, `message`, and `stack` are non-enumerable on an Error, so the
	384	+ // projection above drops them — and for a transport failure they are the ONLY
	385	+ // diagnostic (a native fetch TypeError has no enumerable own property at
	386	+ // all). Carry them explicitly, with the message scrubbed the same way the
	387	+ // HTTP summary scrubs a body: a library that quotes a rejected field in its
	388	+ // message must not smuggle the value back in through the cause.
	389	+ return {
	390	+ ...redacted,
	391	+ name: cause.name,
	392	+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: narrowed to Error above; the message is scrubbed, not interpreted
	393	+ message: redactTokenEndpointBody(cause.message),
	394	+ ...(cause.stack === undefined ? {} : { stack: cause.stack }),
	395	+ };
	396	+};
	397	+
319	398	const tokenEndpointHttpSummary = async (response: Response): Promise<string> => {
320	399	const status = `HTTP ${response.status}${response.statusText ? ` ${response.statusText}` : ""}`;
321	400	const contentType = response.headers.get("content-type");
@@ -359,7 +438,7 @@ const toOAuth2Error = (cause: unknown): OAuth2Error => { · 37 unmodified lines
359	438	return new OAuth2Error({
360	439	message: `OAuth token exchange failed: ${description ?? code ?? "unknown error"}`,
361	440	error: code,
362		− cause,
	441	+ cause: redactErrorCause(cause),
363	442	});
364	443	}
365	444	return new OAuth2Error({
@@ -379,7 +458,7 @@ const toOAuth2ErrorWithHttpSummary = (cause: unknown): Effect.Effect<OAuth2Error · 13 unmodified lines
379	458	new OAuth2Error({
380	459	message: `${base.message} (${summary})`,
381	460	error: base.error,
382		− cause,
	461	+ cause: base.cause,
383	462	}),
384	463	),
385	464	);
@@ -467,19 +546,22 @@ const oauth4webapiRequestOptions = ( · 81 unmodified lines
467	546	// public rather than failing on the malformed row. The `method` only chooses
468	547	// HOW a present secret is sent (post vs basic).
469	548	const pickClientAuth = (
470		− clientSecret: string \| null \| undefined,
	549	+ clientSecret: OAuth2SecretInput \| null \| undefined,
471	550	method: ClientAuthMethod,
472	551	): oauth.ClientAuth => {
473	552	if (clientSecret == null) return oauth.None();
474		− return method === "basic"
475		− ? oauth.ClientSecretBasic(clientSecret)
476		− : oauth.ClientSecretPost(clientSecret);
	553	+ // Boundary: oauth4webapi puts the secret in the form body / Basic header.
	554	+ const secret = secretToSend(clientSecret);
	555	+ return method === "basic" ? oauth.ClientSecretBasic(secret) : oauth.ClientSecretPost(secret);
477	556	};
478	557
479	558	const tokenResponseFrom = (r: oauth.TokenEndpointResponse): OAuth2TokenResponse => ({
480		− access_token: r.access_token,
	559	+ access_token: Redacted.make(r.access_token),
481	560	token_type: r.token_type,
482		− refresh_token: r.refresh_token,
	561	+ // Presence is `undefined`, never falsiness: `Redacted.make("")` is truthy, so
	562	+ // every downstream test asks whether the field is present. oauth4webapi has
	563	+ // already rejected an empty `refresh_token` by this point.
	564	+ refresh_token: r.refresh_token === undefined ? undefined : Redacted.make(r.refresh_token),
483	565	expires_in: typeof r.expires_in === "number" ? r.expires_in : undefined,
484	566	scope: r.scope,
485	567	});
@@ -578,10 +660,10 @@ export type ExchangeAuthorizationCodeInput = { · 92 unmodified lines
578	660	readonly tokenUrl: string;
579	661	readonly issuerUrl?: string \| null;
580	662	readonly clientId: string;
581		− readonly clientSecret?: string \| null;
	663	+ readonly clientSecret?: OAuth2SecretInput \| null;
582	664	readonly redirectUrl: string;
583		− readonly codeVerifier: string;
584		− readonly code: string;
	665	+ readonly codeVerifier: OAuth2SecretInput;
	666	+ readonly code: OAuth2SecretInput;
585	667	readonly clientAuth?: ClientAuthMethod;
586	668	readonly idTokenSigningAlgValuesSupported?: readonly string[];
587	669	/** RFC 8707 Resource Indicator. MCP Auth spec MUST-requires this on
@@ -612,10 +694,11 @@ export const exchangeAuthorizationCode = ( · 24 unmodified lines
612	694	// takes the `code` directly (the UI already validated `state` by
613	695	// looking up the session), so skip the library's state-validation
614	696	// rail and go through the generic grant request instead.
	697	+ // Boundary: both go out as RFC 6749 form fields.
615	698	const params = new URLSearchParams({
616		− code: input.code,
	699	+ code: secretToSend(input.code),
617	700	redirect_uri: input.redirectUrl,
618		− code_verifier: input.codeVerifier,
	701	+ code_verifier: secretToSend(input.codeVerifier),
619	702	});
620	703	if (input.resource) {
621	704	params.set("resource", input.resource);
@@ -647,7 +730,7 @@ export type ExchangeClientCredentialsInput = { · 25 unmodified lines
647	730	readonly clientId: string;
648	731	/** Null for a client the AS registered without a secret; the grant then goes
649	732	* out unauthenticated and the AS decides. Never `""` — see `OAuthClient`. */
650		− readonly clientSecret: string \| null;
	733	+ readonly clientSecret: OAuth2SecretInput \| null;
651	734	readonly scopes?: readonly string[];
652	735	readonly scopeSeparator?: string;
653	736	readonly clientAuth?: ClientAuthMethod;
@@ -703,8 +786,8 @@ export type RefreshAccessTokenInput = { · 49 unmodified lines
703	786	readonly tokenUrl: string;
704	787	readonly issuerUrl?: string \| null;
705	788	readonly clientId: string;
706		− readonly clientSecret?: string \| null;
707		− readonly refreshToken: string;
	789	+ readonly clientSecret?: OAuth2SecretInput \| null;
	790	+ readonly refreshToken: OAuth2SecretInput;
708	791	readonly scopes?: readonly string[];
709	792	readonly scopeSeparator?: string;
710	793	readonly clientAuth?: ClientAuthMethod;
@@ -745,7 +828,8 @@ export const refreshAccessToken = ( · 34 unmodified lines
745	828	as,
746	829	client,
747	830	clientAuth,
748		− input.refreshToken,
	831	+ // Boundary: goes out as the `refresh_token` form field.
	832	+ secretToSend(input.refreshToken),
749	833	{
750	834	...oauth4webapiRequestOptions(
751	835	input.tokenUrl,

packages/core/sdk/src/oauth-service.ts+51−19

@@ -201,8 +201,12 @@ const recordedOAuthScope = ( · 200 unmodified lines
201	201	if (token.scope == null) return requestedScopes.join(" ") \|\| null;
202	202
203	203	const granted = token.scope.split(/\s+/).filter(Boolean);
	204	+ // Presence, not truthiness: the refresh token is a `Redacted`, and every
	205	+ // `Redacted` is truthy.
204	206	const coveredByRefreshToken =
205		− token.refresh_token && requestedScopes.includes("offline_access") ? ["offline_access"] : [];
	207	+ token.refresh_token !== undefined && requestedScopes.includes("offline_access")
	208	+ ? ["offline_access"]
	209	+ : [];
206	210	const recorded = dedupeScopes([...granted, ...coveredByRefreshToken]);
207	211	return recorded.join(" ") \|\| null;
208	212	};
@@ -388,9 +392,11 @@ interface LoadedOAuthClient { · 179 unmodified lines
388	392	readonly tokenUrl: string;
389	393	readonly grant: OAuthGrant;
390	394	readonly clientId: string;
391		− /** Resolved literal secret (read from the provider via the stored item id),
392		− * or null for a public / PKCE client that stored none. */
393		− readonly clientSecret: string \| null;
	395	+ /** Resolved secret (read from the provider via the stored item id), or null
	396	+ * for a public / PKCE client that stored none. Presence stays `null` — an
	397	+ * emptiness test on the wrapper would read a public client as confidential,
	398	+ * since `Redacted.make("")` is truthy. */
	399	+ readonly clientSecret: Redacted.Redacted<string> \| null;
394	400	readonly resource: string \| null;
395	401	}
396	402
@@ -603,8 +609,14 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 206 unmodified lines
603	609	// secret is empty (the token request would then send `client_secret=`),
604	610	// so reject it and make the caller state which it meant. Boundaries that
605	611	// can only produce "" (HTTP payloads, form fields) normalize through
606		− // `oauthClientSecretFromInput` before calling.
607		− if (input.clientSecret === "") {
	612	+ // `oauthClientSecretFromInput` before calling. Tested on the unwrapped
	613	+ // value: a wrapped "" is just as empty and just as wrong.
	614	+ if (
	615	+ input.clientSecret != null &&
	616	+ (Redacted.isRedacted(input.clientSecret)
	617	+ ? Redacted.value(input.clientSecret)
	618	+ : input.clientSecret) === ""
	619	+ ) {
608	620	return yield* new StorageError({
609	621	message:
610	622	"Invalid OAuth client secret: pass null for a public/PKCE client, or a non-empty secret for a confidential one.",
@@ -1025,21 +1037,22 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 414 unmodified lines
1025	1037	// goes out unauthenticated and the AS rejects it, which is the visible
1026	1038	// failure a re-registration prompt hangs off.
1027	1039	return Effect.gen(function* () {
1028		− let clientSecret: string \| null = null;
	1040	+ let clientSecret: Redacted.Redacted<string> \| null = null;
1029	1041	if (row.client_secret_item_id != null) {
1030	1042	const provider = deps.defaultWritableProvider();
1031	1043	if (provider) {
1032		− // Boundary: the OAuth 2.1 helpers take a bare `client_secret`
1033		− // (it is posted to the token endpoint). Unwrap BEFORE
1034		− // presence-normalizing — `Redacted.make("")` is truthy, so an
1035		− // emptiness test on the wrapper would read a public client as
1036		− // confidential.
	1044	+ // The secret stays wrapped from here to the token endpoint; the
	1045	+ // helpers unwrap it at the oauth4webapi call. Presence is
	1046	+ // normalized against the UNWRAPPED value — `Redacted.make("")`
	1047	+ // is truthy, so testing the wrapper would read a public client
	1048	+ // as confidential.
1037	1049	const stored = yield* provider.get(
1038	1050	ProviderItemId.make(String(row.client_secret_item_id)),
1039	1051	);
1040		− clientSecret = oauthClientSecretFromInput(
1041		− stored === null ? null : Redacted.value(stored),
1042		− );
	1052	+ clientSecret =
	1053	+ stored !== null && oauthClientSecretFromInput(Redacted.value(stored)) !== null
	1054	+ ? stored
	1055	+ : null;
1043	1056	}
1044	1057	}
1045	1058	return {
@@ -1174,9 +1187,16 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 128 unmodified lines
1174	1187	: yield* filterAuthorizationCodeScopes(client, requestedScopes);
1175	1188
1176	1189	// authorization_code: persist a session + build the authorize URL.
	1190	+ // The verifier stays wrapped across the challenge computation and down to
	1191	+ // the session write below; the challenge itself is a public S256 hash.
1177	1192	const verifier = createPkceCodeVerifier();
1178	1193	const challenge = yield* Effect.promise(() => createPkceCodeChallenge(verifier));
1179		− const state = OAuthState.make(createOAuthState());
	1194	+ // Boundary: `OAuthState` is a correlation KEY, not just grant material —
	1195	+ // it is matched by equality in `oauth_session`, echoed to the provider in
	1196	+ // the authorize URL, and returned to the caller so `complete` can quote
	1197	+ // it. It cannot stay wrapped, so the unwrap happens here at the branded-id
	1198	+ // constructor rather than being spread across those call sites.
	1199	+ const state = OAuthState.make(Redacted.value(createOAuthState()));
1180	1200	const providerState = encodeOAuthCallbackState({
1181	1201	state: String(state),
1182	1202	orgSlug: deps.callbackStateOrgSlug,
@@ -1195,7 +1215,11 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 12 unmodified lines
1195	1215	name: String(input.name),
1196	1216	template: String(input.template),
1197	1217	redirect_url: flowRedirectUri,
1198		− pkce_verifier: verifier,
	1218	+ // Boundary: the DB persistence line. `Redacted`'s toJSON renders
	1219	+ // "<redacted>", so a missed unwrap here would silently store that
	1220	+ // literal and every later `complete` would fail PKCE validation.
	1221	+ // (The column itself is plaintext; encrypting it is a separate change.)
	1222	+ pkce_verifier: Redacted.value(verifier),
1199	1223	identity_label: input.identityLabel ?? null,
1200	1224	// Persist the requested scope set (declared ∪ client, filtered to the
1201	1225	// authorization-code flow) so `complete`'s recorded-scope fallback
@@ -1259,7 +1283,10 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 57 unmodified lines
1259	1283	name: ConnectionName.make(String(sessionRow.name)),
1260	1284	template: AuthTemplateSlug.make(String(sessionRow.template)),
1261	1285	redirectUrl: String(sessionRow.redirect_url),
1262		− pkceVerifier: sessionRow.pkce_verifier == null ? null : String(sessionRow.pkce_verifier),
	1286	+ // Re-wrapped on the way out of storage: everything downstream treats the
	1287	+ // verifier as grant material again, and the exchange unwraps it.
	1288	+ pkceVerifier:
	1289	+ sessionRow.pkce_verifier == null ? null : Redacted.make(String(sessionRow.pkce_verifier)),
1263	1290	identityLabel: sessionRow.identity_label == null ? null : String(sessionRow.identity_label),
1264	1291	expiresAt: Number(sessionRow.expires_at),
1265	1292	// The scope set `start` requested (the integration's declared or
@@ -1397,11 +1424,16 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 131 unmodified lines
1397	1424	cause: undefined,
1398	1425	});
1399	1426	}
	1427	+ // `provider.set` accepts `Redacted` and unwraps at its own write line, so
	1428	+ // the token never becomes a bare string here.
1400	1429	const itemId = accessItemId(target.owner, target.integration, target.name);
1401	1430	yield* provider.set(ProviderItemId.make(itemId), token.access_token);
1402	1431
1403	1432	let refreshItemId: string \| null = null;
1404		− if (token.refresh_token) {
	1433	+ // Absence is `undefined` — never falsiness. Every `Redacted` is truthy, so
	1434	+ // a truthiness test would stop distinguishing "no refresh token" the day
	1435	+ // the field's presence model changes.
	1436	+ if (token.refresh_token !== undefined) {
1405	1437	refreshItemId = refreshItemIdFor(itemId);
1406	1438	yield* provider.set(ProviderItemId.make(refreshItemId), token.refresh_token);
1407	1439	}