fix(training-agent): restore per-tenant /<tenant>/mcp-strict, close #3965 Class C

.changeset/3965-class-c-strict-route.md

@@ -0,0 +1,30 @@
+---
+---
+
+fix(training-agent): restore per-tenant /<tenant>/mcp-strict, close #3965 Class C
+
+The 6.0.0 multi-tenant migration (#3713) dropped the legacy `/mcp-strict` route along with `STRICT_REQUIRED_FOR` enforcement and `enforceSigningWhenWebhookAuthPresent`. The `signed_requests` conformance storyboard — gated on `request_signing.supported: true` AND `required_for: [...]` — failed at discovery on every tenant since then. The runner targets `/<tenant>/mcp-strict` (per `run-storyboards.ts:350-352`) but the multi-tenant migration only mounted `/<tenant>/mcp`.
+
+Restoring per-tenant strict routes as a sibling mount: same v5 monolith handler as the legacy `/mcp` route, but stamped with `ctx.strict = true` so `selectSigningCapability(ctx)` advertises `STRICT_REQUIRED_FOR` instead of the lax sandbox capability. Request-signing is a transport-layer property, not specialism-specific, so the strict route doesn't need v6 platform dispatch — one handler shared across all six tenants. The default `/<tenant>/mcp` continues to serve the v6 framework with sandbox signing (presence-gated, no required_for enforcement).
+
+Strict authenticator composition rebuilds two enforcement gates the migration dropped:
+
+- `requireSignatureWhenPresent` with `requiredFor: STRICT_REQUIRED_FOR` + `mcpToolNameResolver` so unsigned `create_media_buy` calls surface `request_signature_required` (vector 001) instead of admitting bearer.
+- `enforceSigningWhenWebhookAuthPresent` wrapper so unsigned webhook-registration carrying `push_notification_config.authentication` fires the same error code (vector 027) — bearer-bypass is the exact downgrade this rule prevents.
+
+Each route owns its own `InMemoryReplayStore` (#3338) — sharing one store lets a nonce consumed on `/mcp` falsely fire `request_signature_replayed` on `/mcp-strict`.
+
+Floors ratchet to capture the conformance lift:
+
+| Tenant            | Old (post-#4052) | New (with strict) | Delta |
+|-------------------|------------------|-------------------|-------|
+| /signals          | 65 / 23          | 66 / 54           | +1 / +31 |
+| /sales            | 64 / 217         | 64 / 248          | flat / +31 |
+| /governance       | 65 / 70          | 65 / 101          | flat / +31 |
+| /creative         | 66 / 83          | 66 / 114          | flat / +31 |
+| /creative-builder | 60 / 65          | 60 / 96           | flat / +31 |
+| /brand            | 66 / 14          | 66 / 45           | flat / +31 |
+
+Files: `server/src/training-agent/index.ts` (strict authenticator + per-tenant mount), `.github/workflows/training-agent-storyboards.yml`, `scripts/run-storyboards-matrix.sh`.
+
+Closes #3965 Class C. The full #3965 cluster is now closed: A (6.9.0 / adcp-client#1455), B (in-tree), C (this PR), D (in-tree), E (6.9.0 / adcp-client#1477), F (in-tree), G (phantom).

.github/workflows/training-agent-storyboards.yml

@@ -36,31 +36,36 @@ jobs:
         # SDK 6.0.0). Rising is fine; regressing fails CI.
         tenant: [signals, sales, governance, creative, creative-builder, brand]
         include:
-          # SDK 6.9.0 bump closed #3965 Class E: ComplyControllerConfig.force
-          # gained typed slots for create_media_buy_arm + task_completion
-          # (adcp-client#1472/#1477), so the directive arm now reaches the
-          # dispatcher instead of falling through to UNKNOWN_SCENARIO. 6.9.0
-          # also added a strict account.mode gate on comply_test_controller —
-          # accounts.resolve in every tenant now stamps mode: 'sandbox'.
-          # Floors capture the resulting clean baseline.
+          # #3965 Class C closed by per-tenant /<tenant>/mcp-strict route
+          # restoration. The 6.0.0 multi-tenant migration (#3713) dropped the
+          # legacy /mcp-strict; the signed_requests conformance storyboard
+          # (gated on request_signing.supported and required_for) failed at
+          # discovery on every tenant. Per-tenant strict mounts share one v5
+          # monolith handler stamped with ctx.strict — request-signing is a
+          # transport-layer property, not specialism-specific. The strict
+          # authenticator carries STRICT_REQUIRED_FOR + mcpToolNameResolver
+          # and is wrapped by enforceSigningWhenWebhookAuthPresent so vectors
+          # 001 (request_signature_required) and 027 (webhook auth unsigned)
+          # both fire correctly. Floors capture the +31 step lift per tenant
+          # from the storyboard's positive + negative conformance vectors.
           - tenant: signals
-            min_clean_storyboards: 65
-            min_passing_steps: 23
+            min_clean_storyboards: 66
+            min_passing_steps: 54
           - tenant: sales
-            min_clean_storyboards: 63
-            min_passing_steps: 217
-          - tenant: governance
             min_clean_storyboards: 64
-            min_passing_steps: 70
-          - tenant: creative
+            min_passing_steps: 248
+          - tenant: governance
             min_clean_storyboards: 65
-            min_passing_steps: 83
+            min_passing_steps: 101
+          - tenant: creative
+            min_clean_storyboards: 66
+            min_passing_steps: 114
           - tenant: creative-builder
-            min_clean_storyboards: 59
-            min_passing_steps: 65
+            min_clean_storyboards: 60
+            min_passing_steps: 96
           - tenant: brand
-            min_clean_storyboards: 65
-            min_passing_steps: 14
+            min_clean_storyboards: 66
+            min_passing_steps: 45
     steps:
       - uses: actions/checkout@v6
 

scripts/run-storyboards-matrix.sh

@@ -20,12 +20,12 @@ bash "${SCRIPT_DIR}/overlay-compliance-cache.sh" || true
 # tenant:min_clean:min_passed — kept in sync with the matrix.include block in
 # .github/workflows/training-agent-storyboards.yml.
 TENANTS=(
-  "signals:65:23"
-  "sales:63:217"
-  "governance:64:70"
-  "creative:65:83"
-  "creative-builder:59:65"
-  "brand:65:14"
+  "signals:66:54"
+  "sales:64:248"
+  "governance:65:101"
+  "creative:66:114"
+  "creative-builder:60:96"
+  "brand:66:45"
 )
 
 REGRESSED=0

server/src/training-agent/index.ts

@@ -24,6 +24,7 @@ import {
   extractBearerToken,
   respondUnauthorized,
   requireSignatureWhenPresent,
+  mcpToolNameResolver,
   signatureErrorCodeFromCause,
   AuthError,
   type Authenticator,
@@ -38,7 +39,12 @@ import type { TrainingContext } from './types.js';
 import { PUBLISHERS } from './publishers.js';
 import { SIGNAL_PROVIDERS } from './signal-providers.js';
 import { getPublicJwks } from './webhooks.js';
-import { buildRequestSigningAuthenticator } from './request-signing.js';
+import {
+  buildRequestSigningAuthenticator,
+  buildStrictRequestSigningAuthenticator,
+  enforceSigningWhenWebhookAuthPresent,
+  STRICT_REQUIRED_FOR,
+} from './request-signing.js';
 import { isWorkOSApiKeyFormat } from '../middleware/api-key-format.js';
 import { PUBLIC_TEST_AGENT } from '../config/test-agent.js';
 
@@ -113,7 +119,10 @@ function buildBearerAuthenticator(): Authenticator | null {
 
 // Lazy so the signing authenticator builds on first auth call —
 // avoids reading the compliance test JWKS at module import time, which
-// would break test setups that mock the compliance cache.
+// would break test setups that mock the compliance cache. Each route
+// owns its own InMemoryReplayStore (#3338) — sharing one store lets a
+// nonce consumed on /mcp falsely fire request_signature_replayed on
+// /mcp-strict, so the strict-route authenticator is built separately.
 let _signingAuth: Authenticator | null = null;
 function lazySigningAuth(): Authenticator {
   return (req) => {
@@ -122,6 +131,14 @@ function lazySigningAuth(): Authenticator {
   };
 }
 
+let _strictSigningAuth: Authenticator | null = null;
+function lazyStrictSigningAuth(): Authenticator {
+  return (req) => {
+    if (!_strictSigningAuth) _strictSigningAuth = buildStrictRequestSigningAuthenticator();
+    return _strictSigningAuth(req);
+  };
+}
+
 /**
  * Tenant-route authenticator: presence-gated signature composition.
  * Callers with no `Signature-Input` header fall through to bearer auth.
@@ -133,7 +150,39 @@ function buildDefaultAuthenticator(): Authenticator | null {
   return requireSignatureWhenPresent(lazySigningAuth(), bearerAuth);
 }
 
+/**
+ * Strict-route authenticator: same presence-gated composition wrapping
+ * the strict signing verifier, plus two enforcement gates:
+ *
+ *   - `requiredFor: STRICT_REQUIRED_FOR` + `mcpToolNameResolver` so
+ *     unsigned calls to `create_media_buy` (and other required-for ops)
+ *     surface `request_signature_required` (vector 001) instead of
+ *     admitting bearer.
+ *   - `enforceSigningWhenWebhookAuthPresent` wrapper so an unsigned
+ *     webhook-registration carrying `push_notification_config.authentication`
+ *     fires the same `request_signature_required` error (vector 027).
+ *     Bearer-bypass is the exact downgrade this rule prevents.
+ *
+ * Non-required ops (list tools, get_products, get_adcp_capabilities)
+ * still admit bearer so the grader can do setup probes without signing
+ * infrastructure.
+ */
+function buildStrictAuthenticator(): Authenticator | null {
+  const bearerAuth = buildBearerAuthenticator();
+  if (!bearerAuth) return null;
+  const presenceGated = requireSignatureWhenPresent(
+    lazyStrictSigningAuth(),
+    bearerAuth,
+    {
+      requiredFor: [...STRICT_REQUIRED_FOR],
+      resolveOperation: mcpToolNameResolver,
+    },
+  );
+  return enforceSigningWhenWebhookAuthPresent(presenceGated);
+}
+
 const defaultAuthenticator = buildDefaultAuthenticator();
+const strictAuthenticator = buildStrictAuthenticator();
 
 function buildRequireToken(authenticator: Authenticator | null) {
   return async function requireToken(req: Request, res: Response, next: NextFunction): Promise<void> {
@@ -175,6 +224,7 @@ function buildRequireToken(authenticator: Authenticator | null) {
 }
 
 const requireTokenDefault = buildRequireToken(defaultAuthenticator);
+const requireTokenStrict = buildRequireToken(strictAuthenticator);
 
 function getBaseUrl(req: Request): string {
   if (process.env.BASE_URL) return process.env.BASE_URL.replace(/\/$/, '');
@@ -314,6 +364,81 @@ export function createTrainingAgentRouter(): Router {
     });
   });
 
+  // Per-tenant strict MCP routes — `/<tenant>/mcp-strict` is the
+  // conformance grader target for the `signed_requests` storyboard.
+  // Same v5 monolith handler as the legacy `/mcp` mount, but stamped
+  // with `ctx.strict = true` so `get_adcp_capabilities` advertises
+  // `request_signing.required_for: STRICT_REQUIRED_FOR` and the
+  // verifier rejects unsigned mutating calls with
+  // `request_signature_required` (vector 001). One handler shared
+  // across all tenants — request-signing is a transport-layer property,
+  // not specialism-specific, so the strict route doesn't need v6
+  // platform dispatch. The default `/<tenant>/mcp` continues to serve
+  // the v6 framework with sandbox signing (presence-gated, no
+  // required_for enforcement).
+  async function strictMcpHandler(req: Request, res: Response): Promise<void> {
+    setLegacyCORS(res);
+    let server: ReturnType<typeof createTrainingAgentServer> | null = null;
+    try {
+      const principal = (res.locals.trainingPrincipal as string | undefined) ?? 'anonymous';
+      const ctx: TrainingContext = { mode: 'open', principal, strict: true };
+      server = createTrainingAgentServer(ctx);
+
+      const acceptHeader = req.headers.accept;
+      const hasJson = typeof acceptHeader === 'string' && acceptHeader.includes('application/json');
+      const hasSse = typeof acceptHeader === 'string' && acceptHeader.includes('text/event-stream');
+      if (hasJson && !hasSse) {
+        const rewritten = `${acceptHeader}, text/event-stream`;
+        req.headers.accept = rewritten;
+        const raw = (req as unknown as { rawHeaders?: string[] }).rawHeaders;
+        if (Array.isArray(raw)) {
+          for (let i = 0; i < raw.length; i += 2) {
+            if (raw[i].toLowerCase() === 'accept') raw[i + 1] = rewritten;
+          }
+        }
+      }
+
+      const transport = new StreamableHTTPServerTransport({
+        sessionIdGenerator: undefined,
+        enableJsonResponse: true,
+      });
+      await server.connect(transport);
+      logger.debug({ method: req.body?.method, route: req.originalUrl ?? req.url }, 'Training agent: strict request');
+      await runWithSessionContext(async () => {
+        await transport.handleRequest(req, res, req.body);
+        await flushDirtySessions();
+      });
+    } catch (error) {
+      logger.error({ error, route: req.originalUrl ?? req.url }, 'Training agent: strict request error');
+      if (!res.headersSent) {
+        res.status(500).json({
+          jsonrpc: '2.0',
+          id: null,
+          error: { code: -32603, message: 'Internal server error' },
+        });
+      }
+    } finally {
+      await server?.close().catch(() => {});
+    }
+  }
+
+  for (const tenantId of TENANT_IDS) {
+    router.options(`/${tenantId}/mcp-strict`, (_req: Request, res: Response) => {
+      setLegacyCORS(res);
+      res.status(204).end();
+    });
+    router.post(`/${tenantId}/mcp-strict`, mcpRateLimiter, requireTokenStrict, strictMcpHandler);
+    router.get(`/${tenantId}/mcp-strict`, (_req: Request, res: Response) => {
+      setLegacyCORS(res);
+      res.setHeader('Allow', 'POST, OPTIONS');
+      res.status(405).json({
+        jsonrpc: '2.0',
+        id: null,
+        error: { code: -32000, message: 'Method not allowed. Use POST for MCP requests.' },
+      });
+    });
+  }
+
   // Health check
   router.get('/health', (_req: Request, res: Response) => {
     res.json({ status: 'healthy', service: 'training-agent' });