From a889cb5c0093ebb54ebe369eeb8df9fcd9a93a84 Mon Sep 17 00:00:00 2001 From: DongHyeonka Date: Tue, 18 Aug 2026 09:14:42 +0900 Subject: [PATCH] fix: scope the TechLog CSRF invalidation to Studio operations `contractOperations.execute` is the single executor every installed feature dispatches through, and it called `invalidateTechLogCsrfOnOutcome` for every operation. A 403 on an unrelated reference-feature request therefore threw away a perfectly good TechLog CSRF token, forcing an avoidable `getStudioSession` round trip on the next Studio operation -- and, when the session endpoint is itself unhealthy, turning someone else's authorization failure into a Studio outage. `invalidateTechLogCsrfOnOutcome` now takes the operation's auth profile and acts only on the two TechLog Studio profiles. Required, not optional, so the scoping cannot be dropped again by omission, and the predicate lives in the feature file: `bootstrap/runtime-adapters.ts` is template-synced and its change is the one added argument. The composition test now installs both contributions the way `installed-contract-contributions.ts` does, and asserts a reference-feature 403 leaves the cached token alone. Reverting the scope check fails exactly that test. Co-Authored-By: Claude Opus 5 (1M context) --- src/bootstrap/runtime-adapters.ts | 9 +- .../http/studio-session-credentials.ts | 19 ++++ .../tech-log/studio-csrf-composition.test.ts | 98 ++++++++++++++++++- 3 files changed, 121 insertions(+), 5 deletions(-) diff --git a/src/bootstrap/runtime-adapters.ts b/src/bootstrap/runtime-adapters.ts index 5726227..2b145d3 100644 --- a/src/bootstrap/runtime-adapters.ts +++ b/src/bootstrap/runtime-adapters.ts @@ -585,7 +585,14 @@ export async function createRuntimeAdapters( // 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); + // The auth profile scopes it to Studio operations: this executor serves + // every installed feature, so without it an unrelated 403 discarded the + // TechLog token. + invalidateTechLogCsrfOnOutcome( + outcome.kind, + operation.frontend.authProfileId, + techLogCsrf, + ); return outcome; }, }); diff --git a/src/features/tech-log/adapters/http/studio-session-credentials.ts b/src/features/tech-log/adapters/http/studio-session-credentials.ts index 98c7947..a06157a 100644 --- a/src/features/tech-log/adapters/http/studio-session-credentials.ts +++ b/src/features/tech-log/adapters/http/studio-session-credentials.ts @@ -70,6 +70,14 @@ export function assertExactlyOneTechLogStudioBootstrapOperation( } } +/** Every operation whose rejection can say something about the Studio token. */ +function isTechLogStudioAuthProfileId(value: string): value is TechLogStudioAuthProfileId { + return ( + value === TECH_LOG_STUDIO_BOOTSTRAP_AUTH_PROFILE_ID || + value === TECH_LOG_STUDIO_SESSION_AUTH_PROFILE_ID + ); +} + /** * Fix round 2, item 1. A CSRF rejection on a JSON operation can arrive as * either `UNAUTHENTICATED` (the Studio session itself expired) or @@ -81,11 +89,22 @@ export function assertExactlyOneTechLogStudioBootstrapOperation( * (`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. + * + * Final fix wave, item 8. `contractOperations.execute` is one executor shared + * by every installed feature, so it was calling this for reference-feature + * operations too: an unrelated 403 discarded a perfectly good Studio token + * and forced an avoidable `getStudioSession` round trip — or, when the + * session endpoint is itself unhealthy, turned someone else's authorization + * failure into a Studio outage. `authProfileId` is required rather than + * optional so the scoping cannot be dropped again by omission, and the + * decision lives here rather than at the (template-synced) call site. */ export function invalidateTechLogCsrfOnOutcome( outcomeKind: string, + authProfileId: string, csrf: CsrfTokenProvider, ): void { + if (!isTechLogStudioAuthProfileId(authProfileId)) return; if (outcomeKind === "UNAUTHENTICATED" || outcomeKind === "FORBIDDEN") { csrf.invalidate(); } diff --git a/tests/features/tech-log/studio-csrf-composition.test.ts b/tests/features/tech-log/studio-csrf-composition.test.ts index b88bf93..7904c4b 100644 --- a/tests/features/tech-log/studio-csrf-composition.test.ts +++ b/tests/features/tech-log/studio-csrf-composition.test.ts @@ -15,6 +15,7 @@ import { } from "../../../src/features/tech-log/adapters/http/studio-session-credentials.ts"; import type { StudioOperationExecutor } from "../../../src/features/tech-log/adapters/http/http-studio-gateway.ts"; import { TECH_LOG_STUDIO_CONTRIBUTION } from "../../../src/features/tech-log/contracts/tech-log-studio-contract-contribution.ts"; +import { REFERENCE_FEATURE_TEMPLATE_CONTRIBUTION } from "../../../src/features/reference-feature/contracts/reference-feature-contract-contribution.ts"; /** * I1 (Task 7 fix round 1). C1 was a self-referential CSRF bootstrap cycle @@ -61,7 +62,14 @@ function scopeSnapshot() { * afterward. */ function composeStudioRuntime() { - const composed = composeContractContributions([TECH_LOG_STUDIO_CONTRIBUTION]); + // Both contributions, exactly as `installed-contract-contributions.ts` + // composes them: `contractOperations.execute` is one executor shared by + // every installed feature, so a reference-feature operation's outcome + // travels through the same code path a Studio operation's does. + const composed = composeContractContributions([ + TECH_LOG_STUDIO_CONTRIBUTION, + REFERENCE_FEATURE_TEMPLATE_CONTRIBUTION, + ]); let contractOperations: StudioOperationExecutor = Object.freeze({ async execute() { @@ -97,7 +105,16 @@ function composeStudioRuntime() { authContext, techLogCsrf, ); - return outcome ?? Object.freeze({ kind: "UNAVAILABLE" as const }); + // `attachStudioSessionCredentials` returns null for a non-Studio + // profile so the caller falls through to its own credential logic -- + // the bearer path in the composition root, this stand-in here. + return ( + outcome ?? + Object.freeze({ + kind: "READY" as const, + headers: Object.freeze({ authorization: "Bearer reference-token" }), + }) + ); }, }); @@ -113,8 +130,14 @@ function composeStudioRuntime() { }); // Fix round 2, item 1. Same production call as // `bootstrap/runtime-adapters.ts`'s `contractOperations.execute` — not - // a reimplementation of it. - invalidateTechLogCsrfOnOutcome(outcome.kind, techLogCsrf); + // a reimplementation of it. Final fix wave item 8 added the auth + // profile: this executor serves every installed feature, so the call + // has to be told which one produced the outcome. + invalidateTechLogCsrfOnOutcome( + outcome.kind, + operation.frontend.authProfileId, + techLogCsrf, + ); return outcome; }, }); @@ -254,3 +277,70 @@ test( assert.equal(sessionCalls, 2); }, ); + +/** + * Final fix wave, item 8. `contractOperations.execute` is the single executor + * every installed feature dispatches through, and it called + * `invalidateTechLogCsrfOnOutcome` for *every* operation. A 403 on an + * unrelated reference-feature request therefore discarded the TechLog CSRF + * token, forcing an avoidable `getStudioSession` round trip on the next + * Studio operation -- and, when the session endpoint is itself unhealthy, + * turning someone else's authorization failure into a Studio outage. + */ +test( + "a 403 on a non-Studio operation leaves the TechLog CSRF token cached", + async () => { + let sessionCalls = 0; + server.use( + http.get(`${BASE}/api/v1/studio/session`, () => { + sessionCalls += 1; + return HttpResponse.json({ + authenticated: true, + displayName: "테스터", + roles: ["editor"], + csrfToken: `csrf-token-${sessionCalls}`, + csrfHeaderName: "X-CSRF-TOKEN", + }); + }), + ); + + const dashboardHeaders: (string | null)[] = []; + server.use( + http.get(`${BASE}/api/v1/studio/dashboard`, ({ request }) => { + dashboardHeaders.push(request.headers.get("x-csrf-token")); + return HttpResponse.json({ documentTotals: {}, workflowSections: [] }); + }), + ); + server.use( + http.get(`${BASE}/api/reference-resources`, () => new HttpResponse(null, { status: 403 })), + ); + + const { contractOperations } = composeStudioRuntime(); + + const first = await contractOperations.execute( + "getStudioDashboard", + {}, + { routeId: "TECH_LOG_STUDIO" }, + ); + assert.equal(first.kind, "SUCCESS"); + assert.equal(dashboardHeaders[0], "csrf-token-1"); + + // Someone else's 403, on a feature that has nothing to do with the Studio + // session. + const rejected = await contractOperations.execute( + "LIST_REFERENCE_RESOURCES", + { limit: 10 }, + { routeId: "REFERENCE_FEATURE" }, + ); + assert.equal(rejected.kind, "FORBIDDEN"); + + const second = await contractOperations.execute( + "getStudioDashboard", + {}, + { routeId: "TECH_LOG_STUDIO" }, + ); + assert.equal(second.kind, "SUCCESS"); + assert.equal(dashboardHeaders[1], "csrf-token-1"); + assert.equal(sessionCalls, 1); + }, +);