fix: the classifier does not survive structuredClone, and the test now proves it
Since round 1 the doc claimed the `name` fallback covers "a structured clone or
a JSON hop". Verified, and it is false for half of that:
structuredClone(new InsecureTokenEndpointError(url))
-> name: "Error", tokenEndpoint: undefined, constructor: Error
structuredClone normalizes a custom Error subclass back to Error, so neither
arm has anything left to match. A caller routing this error through
structuredClone or postMessage would silently get the generic retryable
handling back — the exact defect this PR removes.
The test could not have caught it: it hand-built a look-alike plain object, so
it asserted what I believed the boundary does rather than what it does. It now
performs a real JSON round trip (asserting the prototype is gone), and a second
case pins that structuredClone is NOT recognized — turning a false claim into a
tested limitation.
The doc comment now scopes the contract to JSON and states the structuredClone
limit with the remedy: serialize the fields explicitly across such a boundary.
Also removes an orphaned JSDoc block the round-4 refactor stranded above the
report helper, and gives showInsecureTokenEndpointNotice its own doc back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018MvNbmAtQuWhDvzi4yXDju
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
c2d566ad8d
commit
a5a9d4fe15
3 changed files with 52 additions and 19 deletions
|
|
@ -31,12 +31,6 @@ import {
|
|||
insecureTokenEndpointTitle,
|
||||
} from "../utils/oauthUx";
|
||||
|
||||
/**
|
||||
* Show the terminal notice when `err` is the SDK's `InsecureTokenEndpointError`.
|
||||
*
|
||||
* @returns `true` when it was handled (the caller should stop), `false` when
|
||||
* `err` is some other failure and the caller's normal handling applies.
|
||||
*/
|
||||
/**
|
||||
* Report the refusal **and** clear the re-auth banner for that server.
|
||||
*
|
||||
|
|
@ -79,6 +73,16 @@ export function reportTerminalInsecureTokenEndpoint<
|
|||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Show the notice alone, without touching the banner.
|
||||
*
|
||||
* Prefer {@link reportTerminalInsecureTokenEndpoint} — the two are one
|
||||
* invariant. This stays exported for the one arm that has already cleared the
|
||||
* banner itself before starting.
|
||||
*
|
||||
* @returns `true` when it was handled (the caller should stop), `false` when
|
||||
* `err` is some other failure and the caller's normal handling applies.
|
||||
*/
|
||||
export function showInsecureTokenEndpointNotice(
|
||||
err: unknown,
|
||||
serverName?: string,
|
||||
|
|
|
|||
|
|
@ -43,16 +43,37 @@ describe("findInsecureTokenEndpoint", () => {
|
|||
).toMatchObject({ tokenEndpoint: ENDPOINT });
|
||||
});
|
||||
|
||||
it("recognizes a serialized copy by `name`, where the prototype is gone", () => {
|
||||
// The fallback arm: a structured clone or JSON hop drops the prototype and
|
||||
// the brand set but keeps `name`.
|
||||
expect(
|
||||
findInsecureTokenEndpoint({
|
||||
name: "InsecureTokenEndpointError",
|
||||
message: "Refusing to send credentials…",
|
||||
tokenEndpoint: ENDPOINT,
|
||||
it("recognizes a JSON round trip, where the prototype is gone", () => {
|
||||
// An ACTUAL round trip, not a hand-built look-alike: constructing the
|
||||
// object by hand asserts what I believed the boundary does rather than what
|
||||
// it does. A JSON hop drops the prototype and the brand set while keeping
|
||||
// `name` and `tokenEndpoint`, which is the case this fallback exists for.
|
||||
const err = new InsecureTokenEndpointError(ENDPOINT);
|
||||
const hopped: unknown = JSON.parse(
|
||||
JSON.stringify({
|
||||
name: err.name,
|
||||
message: err.message,
|
||||
tokenEndpoint: err.tokenEndpoint,
|
||||
}),
|
||||
).toMatchObject({ tokenEndpoint: ENDPOINT });
|
||||
);
|
||||
expect(Object.getPrototypeOf(hopped)).toBe(Object.prototype);
|
||||
expect(findInsecureTokenEndpoint(hopped)).toMatchObject({
|
||||
tokenEndpoint: ENDPOINT,
|
||||
});
|
||||
});
|
||||
|
||||
it("does NOT survive structuredClone, and this pins that limit", () => {
|
||||
// Verified, not assumed: structuredClone normalizes a custom Error subclass
|
||||
// back to `Error`, so `name` becomes "Error" and `tokenEndpoint` is dropped
|
||||
// — nothing is left for either arm to match. An earlier revision of the doc
|
||||
// comment claimed this boundary worked; it does not, and a caller relying on
|
||||
// it would silently get the generic retryable handling back.
|
||||
const cloned = structuredClone(new InsecureTokenEndpointError(ENDPOINT));
|
||||
expect(cloned.name).toBe("Error");
|
||||
expect(
|
||||
(cloned as { tokenEndpoint?: unknown }).tokenEndpoint,
|
||||
).toBeUndefined();
|
||||
expect(findInsecureTokenEndpoint(cloned)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("rejects a look-alike carrying the endpoint but not the identity", () => {
|
||||
|
|
|
|||
|
|
@ -36,10 +36,18 @@ export interface InsecureTokenEndpointShape {
|
|||
* on an instance — don't check that property.)
|
||||
*
|
||||
* The `name` comparison is the same deliberate serialization fallback
|
||||
* `isAuthorizationServerMismatchShape` carries in `issuerBinding.ts`: today the
|
||||
* web client runs `auth()` in the browser, so no boundary is crossed, but the
|
||||
* prototype and brand set are the first things a structured clone or a JSON hop
|
||||
* would drop, and `name` survives both.
|
||||
* `isAuthorizationServerMismatchShape` carries in `issuerBinding.ts`. Today the
|
||||
* web client runs `auth()` in the browser so no boundary is crossed, but a JSON
|
||||
* hop drops the prototype and the brand set while preserving `name` and
|
||||
* `tokenEndpoint`, and that is the case this arm exists for.
|
||||
*
|
||||
* ⚠️ **It does not cover `structuredClone`, and cannot.** That algorithm
|
||||
* normalizes a custom `Error` subclass back to `Error` — `name` becomes
|
||||
* `"Error"` and own properties like `tokenEndpoint` are dropped entirely — so
|
||||
* nothing survives for either arm to match on. A caller who routes this error
|
||||
* through `structuredClone` (or `postMessage`, which uses it) will silently get
|
||||
* the generic retryable handling back. Serialize the fields explicitly across
|
||||
* such a boundary rather than relying on this classifier.
|
||||
*/
|
||||
function isInsecureTokenEndpointShape(
|
||||
err: unknown,
|
||||
|
|
|
|||
Loading…
Reference in a new issue