fix: invalidate the TechLog CSRF token on 403 and harden the bootstrap profile wiring
Item 1 (real bug): contractOperations.execute only invalidated the cached CSRF token on UNAUTHENTICATED (401). A CSRF-specific rejection normally arrives as FORBIDDEN (403) -- the platform classifies any 403 response as FORBIDDEN unconditionally -- so a token rejected during an ordinary document save left the stale token cached and every subsequent Studio mutation kept failing until reload. Extracted invalidateTechLogCsrfOnOutcome() so production and the composition test call the identical function; it now invalidates on both UNAUTHENTICATED and FORBIDDEN. Item 2: the upload transport's uncontracted-status fallback hardcoded status 503, so an uncontracted 401/403 body never reached the gateway's error.status === 401 || 403 invalidation check. Passes the real response.status through. Item 3: safeOperation()'s auth-profile parameter is now typed as a union of the two valid profile constants instead of a bare string, and assertExactlyOneTechLogStudioBootstrapOperation() fails composition closed if getStudioSession stops being the sole caller of the credential-free bootstrap profile. Item 4: corrected two stale operation counts in the adapter review doc. Both new tests for items 1 and 2 were run and shown failing before their fix, per this task's TDD standard for error-path changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
2cab4974b7
commit
35cc5c868a
@@ -28,7 +28,10 @@ import { createBrowserStorageAdapter } from "../adapters/storage/browser-storage
|
||||
import { createBrowserMutationIntentFactory } from "../adapters/platform/browser-mutation-intent-factory.ts";
|
||||
import { createTelemetryAdapter } from "../adapters/telemetry/best-effort-telemetry.ts";
|
||||
import { createCsrfTokenProvider } from "../features/tech-log/adapters/http/studio-session-csrf.ts";
|
||||
import { attachStudioSessionCredentials } from "../features/tech-log/adapters/http/studio-session-credentials.ts";
|
||||
import {
|
||||
attachStudioSessionCredentials,
|
||||
invalidateTechLogCsrfOnOutcome,
|
||||
} from "../features/tech-log/adapters/http/studio-session-credentials.ts";
|
||||
import type { StudioOperationExecutor as TechLogStudioOperationExecutor } from "../features/tech-log/adapters/http/http-studio-gateway.ts";
|
||||
import type { AuthSessionPort } from "../application/ports/auth-session-port.ts";
|
||||
import type { ReleaseInfo } from "../application/ports/release-info-port.ts";
|
||||
@@ -573,12 +576,16 @@ export async function createRuntimeAdapters(
|
||||
});
|
||||
if (outcome.kind === "UNAUTHENTICATED") {
|
||||
authSession.onUnauthenticated();
|
||||
// The Studio session (and the CSRF token it issued) expired. The
|
||||
// cache owner discards it here, not the gateway — the gateway has no
|
||||
// way to know a 401 on one operation invalidates a token shared by
|
||||
// every other in-flight and future Studio request.
|
||||
techLogCsrf.invalidate();
|
||||
}
|
||||
// Fix round 2, item 1. `UNAUTHENTICATED` (401) and `FORBIDDEN` (403 —
|
||||
// the shape a CSRF-specific rejection normally takes) both leave a
|
||||
// stale token cached for every other in-flight and future Studio
|
||||
// request if nothing discards it. The cache owner discards it here,
|
||||
// not the gateway — the gateway has no way to know a rejection on one
|
||||
// operation invalidates a token shared across all of them. Same call
|
||||
// the composition test drives
|
||||
// (`tests/features/tech-log/studio-csrf-composition.test.ts`).
|
||||
invalidateTechLogCsrfOnOutcome(outcome.kind, techLogCsrf);
|
||||
return outcome;
|
||||
},
|
||||
});
|
||||
|
||||
@@ -102,7 +102,13 @@ export function createAssetUploadTransport(
|
||||
if (problem && typeof problem.code === "string" && CODES.has(problem.code)) {
|
||||
throw new StudioGatewayError(problem);
|
||||
}
|
||||
throw unavailable(`Upload returned an uncontracted status ${response.status}.`);
|
||||
// Fix round 2, item 2. The real status is passed through (not the
|
||||
// hardcoded 503 default) so `http-studio-asset-gateway.ts`'s
|
||||
// `error.status === 401 || error.status === 403` check can still act on
|
||||
// an uncontracted 401/403 body and invalidate the cached CSRF token.
|
||||
throw unavailable(`Upload returned an uncontracted status ${response.status}.`, {
|
||||
status: response.status,
|
||||
});
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
@@ -19,10 +19,78 @@ export const TECH_LOG_STUDIO_BOOTSTRAP_AUTH_PROFILE_ID =
|
||||
/** The seventeen other Studio operations — everything but `getStudioSession`. */
|
||||
export const TECH_LOG_STUDIO_SESSION_AUTH_PROFILE_ID = "TECH_LOG_STUDIO_SESSION";
|
||||
|
||||
/**
|
||||
* Fix round 2, item 3. `tech-log-studio-contract-contribution.ts`'s
|
||||
* `safeOperation()` used to accept a bare `string` for its auth-profile
|
||||
* override, so a typo (or a copy-pasted wrong constant) would silently
|
||||
* compile. Closing the type to this union turns that class of mistake into a
|
||||
* compile error instead of a runtime footgun — a future *mutating* operation
|
||||
* stamped with the bootstrap profile by mistake would otherwise dispatch
|
||||
* with no CSRF header at all and nothing would object, because
|
||||
* `resolveRestSecurityProfiles`'s "unsafe method + cookie ⇒ CSRF" rule only
|
||||
* applies at `contractVersion === 2`, which this feature's operations are
|
||||
* not. {@link assertExactlyOneTechLogStudioBootstrapOperation} closes the
|
||||
* remaining gap the type alone cannot: two operations (or zero) correctly
|
||||
* typed but wrongly assigned.
|
||||
*/
|
||||
export type TechLogStudioAuthProfileId =
|
||||
| typeof TECH_LOG_STUDIO_BOOTSTRAP_AUTH_PROFILE_ID
|
||||
| typeof TECH_LOG_STUDIO_SESSION_AUTH_PROFILE_ID;
|
||||
|
||||
function isCredentialHeaderName(value: string): value is CredentialHeaderName {
|
||||
return (CREDENTIAL_HEADER_NAMES as readonly string[]).includes(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Fix round 2, item 3. Fails composition closed if `getStudioSession` stops
|
||||
* being the sole caller of the bootstrap profile — whether because it was
|
||||
* removed from every operation (reopening the C1 cycle) or because a second
|
||||
* operation picked it up (dispatching with no CSRF header, silently, since
|
||||
* the bootstrap profile allows none). Called once at module load from
|
||||
* `tech-log-studio-contract-contribution.ts`, after `HTTP_CONTRACTS` is
|
||||
* built — a composition failure, never a runtime downgrade, matching
|
||||
* `installRestAuthProfileRegistry`'s own fail-fast pattern.
|
||||
*/
|
||||
export function assertExactlyOneTechLogStudioBootstrapOperation(
|
||||
operations: readonly Readonly<{ operationId: string; authProfileId: string }>[],
|
||||
): void {
|
||||
const bootstrapOperationIds = operations
|
||||
.filter(
|
||||
(operation) =>
|
||||
operation.authProfileId === TECH_LOG_STUDIO_BOOTSTRAP_AUTH_PROFILE_ID,
|
||||
)
|
||||
.map((operation) => operation.operationId);
|
||||
if (bootstrapOperationIds.length !== 1) {
|
||||
throw new Error(
|
||||
`${TECH_LOG_STUDIO_BOOTSTRAP_AUTH_PROFILE_ID} must be used by exactly one ` +
|
||||
`operation (the one that issues the CSRF token this profile exists to ` +
|
||||
`avoid requiring). Found ${bootstrapOperationIds.length}: ` +
|
||||
`[${bootstrapOperationIds.join(", ")}].`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Fix round 2, item 1. A CSRF rejection on a JSON operation can arrive as
|
||||
* either `UNAUTHENTICATED` (the Studio session itself expired) or
|
||||
* `FORBIDDEN` — the platform classifies any HTTP 403 as `FORBIDDEN`
|
||||
* regardless of the response body (`http-execution-v3.ts:1050`), and 403 is
|
||||
* the shape a CSRF-specific rejection normally takes. Both leave a stale
|
||||
* token cached for every subsequent Studio operation if nothing discards it.
|
||||
* The multipart upload path already invalidates on both statuses
|
||||
* (`http-studio-asset-gateway.ts`); this keeps the JSON path in agreement.
|
||||
* Only `UNAUTHENTICATED` also tears down the auth session itself — that stays
|
||||
* the caller's responsibility, not this function's.
|
||||
*/
|
||||
export function invalidateTechLogCsrfOnOutcome(
|
||||
outcomeKind: string,
|
||||
csrf: CsrfTokenProvider,
|
||||
): void {
|
||||
if (outcomeKind === "UNAUTHENTICATED" || outcomeKind === "FORBIDDEN") {
|
||||
csrf.invalidate();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The Studio branch of the platform's `attachCredentials` collaborator,
|
||||
* extracted so the composition root (`bootstrap/runtime-adapters.ts`) and its
|
||||
|
||||
@@ -10,8 +10,10 @@ import { TECH_LOG_FEATURE_ID } from "../application/tech-log-feature-input.ts";
|
||||
import canonicalSource from "./studio/canonical-source.json" with { type: "json" };
|
||||
import { STUDIO_ERROR_CODES } from "../adapters/http/studio-error-mapping.ts";
|
||||
import {
|
||||
assertExactlyOneTechLogStudioBootstrapOperation,
|
||||
TECH_LOG_STUDIO_BOOTSTRAP_AUTH_PROFILE_ID,
|
||||
TECH_LOG_STUDIO_SESSION_AUTH_PROFILE_ID,
|
||||
type TechLogStudioAuthProfileId,
|
||||
} from "../adapters/http/studio-session-credentials.ts";
|
||||
|
||||
function zodValidator<T>(schemaId: string, schema: z.ZodType<T>): RuntimeValidator<T> {
|
||||
@@ -83,7 +85,7 @@ function safeOperation(
|
||||
// is the one operation that must NOT require one — it is the only caller
|
||||
// of `TECH_LOG_STUDIO_BOOTSTRAP`. Every other safe/keyed operation keeps
|
||||
// the default `TECH_LOG_STUDIO_SESSION`.
|
||||
authProfileId: string = TECH_LOG_STUDIO_SESSION_AUTH_PROFILE_ID,
|
||||
authProfileId: TechLogStudioAuthProfileId = TECH_LOG_STUDIO_SESSION_AUTH_PROFILE_ID,
|
||||
): InstalledHttpContract<unknown, unknown, unknown> {
|
||||
return Object.freeze({
|
||||
contract: Object.freeze({
|
||||
@@ -558,6 +560,15 @@ const HTTP_CONTRACTS = Object.freeze([
|
||||
DELETE_STUDIO_ASSET,
|
||||
]);
|
||||
|
||||
// Fix round 2, item 3. Fails composition closed rather than letting a wrong
|
||||
// assignment (two operations, or zero) reach production undetected.
|
||||
assertExactlyOneTechLogStudioBootstrapOperation(
|
||||
HTTP_CONTRACTS.map((entry) => ({
|
||||
operationId: entry.contract.operationId,
|
||||
authProfileId: entry.frontend.authProfileId,
|
||||
})),
|
||||
);
|
||||
|
||||
export const TECH_LOG_STUDIO_OPERATION_IDS = Object.freeze(
|
||||
HTTP_CONTRACTS.map((entry) => entry.contract.operationId),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user