fix(api): MS22 Phase 1 audit report and fixes

apps/api/src/agent-config/agent-config.service.ts

@@ -1,6 +1,6 @@
 import { Injectable, NotFoundException } from "@nestjs/common";
 import type { LlmProvider } from "@prisma/client";
-import { timingSafeEqual } from "node:crypto";
+import { createHash, timingSafeEqual } from "node:crypto";
 import { PrismaService } from "../prisma/prisma.service";
 import { CryptoService } from "../crypto/crypto.service";
 
@@ -143,21 +143,23 @@ export class AgentConfigService {
       }),
     ]);
 
+    let match: ContainerTokenValidation | null = null;
+
     for (const container of userContainers) {
       const storedToken = this.decryptContainerToken(container.gatewayToken);
-      if (storedToken && this.tokensEqual(storedToken, token)) {
-        return { type: "user", id: container.id };
+      if (!match && storedToken && this.tokensEqual(storedToken, token)) {
+        match = { type: "user", id: container.id };
       }
     }
 
     for (const container of systemContainers) {
       const storedToken = this.decryptContainerToken(container.gatewayToken);
-      if (storedToken && this.tokensEqual(storedToken, token)) {
-        return { type: "system", id: container.id };
+      if (!match && storedToken && this.tokensEqual(storedToken, token)) {
+        match = { type: "system", id: container.id };
       }
     }
 
-    return null;
+    return match;
   }
 
   private buildOpenClawConfig(
@@ -268,14 +270,9 @@ export class AgentConfigService {
   }
 
   private tokensEqual(left: string, right: string): boolean {
-    const leftBuffer = Buffer.from(left, "utf8");
-    const rightBuffer = Buffer.from(right, "utf8");
-
-    if (leftBuffer.length !== rightBuffer.length) {
-      return false;
-    }
-
-    return timingSafeEqual(leftBuffer, rightBuffer);
+    const leftDigest = createHash("sha256").update(left, "utf8").digest();
+    const rightDigest = createHash("sha256").update(right, "utf8").digest();
+    return timingSafeEqual(leftDigest, rightDigest);
   }
 
   private hasModelId(modelEntry: unknown): modelEntry is { id: string } {

apps/api/src/chat-proxy/chat-proxy.controller.ts

@@ -1,4 +1,14 @@
-import { Body, Controller, Post, Req, Res, UnauthorizedException, UseGuards } from "@nestjs/common";
+import {
+  Body,
+  Controller,
+  HttpException,
+  Logger,
+  Post,
+  Req,
+  Res,
+  UnauthorizedException,
+  UseGuards,
+} from "@nestjs/common";
 import type { Response } from "express";
 import { AuthGuard } from "../auth/guards/auth.guard";
 import type { MaybeAuthenticatedRequest } from "../auth/types/better-auth-request.interface";
@@ -8,6 +18,8 @@ import { ChatProxyService } from "./chat-proxy.service";
 @Controller("chat")
 @UseGuards(AuthGuard)
 export class ChatProxyController {
+  private readonly logger = new Logger(ChatProxyController.name);
+
   constructor(private readonly chatProxyService: ChatProxyService) {}
 
   // POST /api/chat/stream
@@ -58,10 +70,11 @@ export class ChatProxyController {
         res.write(Buffer.from(chunk));
       }
     } catch (error: unknown) {
+      this.logStreamError(error);
+
       if (!res.writableEnded && !res.destroyed) {
-        const message = error instanceof Error ? error.message : String(error);
         res.write("event: error\n");
-        res.write(`data: ${JSON.stringify({ error: message })}\n\n`);
+        res.write(`data: ${JSON.stringify({ error: this.toSafeClientMessage(error) })}\n\n`);
       }
     } finally {
       if (!res.writableEnded && !res.destroyed) {
@@ -69,4 +82,21 @@ export class ChatProxyController {
       }
     }
   }
+
+  private toSafeClientMessage(error: unknown): string {
+    if (error instanceof HttpException && error.getStatus() < 500) {
+      return "Chat request was rejected";
+    }
+
+    return "Chat stream failed";
+  }
+
+  private logStreamError(error: unknown): void {
+    if (error instanceof Error) {
+      this.logger.warn(`Chat stream failed: ${error.message}`);
+      return;
+    }
+
+    this.logger.warn(`Chat stream failed: ${String(error)}`);
+  }
 }

apps/api/src/chat-proxy/chat-proxy.service.spec.ts

@@ -64,6 +64,7 @@ describe("ChatProxyService", () => {
         expect.objectContaining({
           method: "POST",
           headers: {
+            Authorization: "Bearer gateway-token",
             "Content-Type": "application/json",
           },
         })

apps/api/src/chat-proxy/chat-proxy.service.ts

@@ -1,12 +1,24 @@
-import { BadGatewayException, Injectable, ServiceUnavailableException } from "@nestjs/common";
+import {
+  BadGatewayException,
+  Injectable,
+  Logger,
+  ServiceUnavailableException,
+} from "@nestjs/common";
 import { ContainerLifecycleService } from "../container-lifecycle/container-lifecycle.service";
 import { PrismaService } from "../prisma/prisma.service";
 import type { ChatMessage } from "./chat-proxy.dto";
 
 const DEFAULT_OPENCLAW_MODEL = "openclaw:default";
 
+interface ContainerConnection {
+  url: string;
+  token: string;
+}
+
 @Injectable()
 export class ChatProxyService {
+  private readonly logger = new Logger(ChatProxyService.name);
+
   constructor(
     private readonly prisma: PrismaService,
     private readonly containerLifecycle: ContainerLifecycleService
@@ -14,8 +26,7 @@ export class ChatProxyService {
 
   // Get the user's OpenClaw container URL and mark it active.
   async getContainerUrl(userId: string): Promise<string> {
-    const { url } = await this.containerLifecycle.ensureRunning(userId);
-    await this.containerLifecycle.touch(userId);
+    const { url } = await this.getContainerConnection(userId);
     return url;
   }
 
@@ -25,11 +36,14 @@ export class ChatProxyService {
     messages: ChatMessage[],
     signal?: AbortSignal
   ): Promise<Response> {
-    const containerUrl = await this.getContainerUrl(userId);
+    const { url: containerUrl, token: gatewayToken } = await this.getContainerConnection(userId);
     const model = await this.getPreferredModel(userId);
     const requestInit: RequestInit = {
       method: "POST",
-      headers: { "Content-Type": "application/json" },
+      headers: {
+        "Content-Type": "application/json",
+        Authorization: `Bearer ${gatewayToken}`,
+      },
       body: JSON.stringify({
         messages,
         model,
@@ -47,10 +61,10 @@ export class ChatProxyService {
       if (!response.ok) {
         const detail = await this.readResponseText(response);
         const status = `${String(response.status)} ${response.statusText}`.trim();
-        const message = detail
-          ? `OpenClaw returned ${status}: ${detail}`
-          : `OpenClaw returned ${status}`;
-        throw new BadGatewayException(message);
+        this.logger.warn(
+          detail ? `OpenClaw returned ${status}: ${detail}` : `OpenClaw returned ${status}`
+        );
+        throw new BadGatewayException(`OpenClaw returned ${status}`);
       }
 
       return response;
@@ -60,10 +74,17 @@ export class ChatProxyService {
       }
 
       const message = error instanceof Error ? error.message : String(error);
-      throw new ServiceUnavailableException(`Failed to proxy chat to OpenClaw: ${message}`);
+      this.logger.warn(`Failed to proxy chat request: ${message}`);
+      throw new ServiceUnavailableException("Failed to proxy chat to OpenClaw");
     }
   }
 
+  private async getContainerConnection(userId: string): Promise<ContainerConnection> {
+    const connection = await this.containerLifecycle.ensureRunning(userId);
+    await this.containerLifecycle.touch(userId);
+    return connection;
+  }
+
   private async getPreferredModel(userId: string): Promise<string> {
     const config = await this.prisma.userAgentConfig.findUnique({
       where: { userId },

docs/audits/ms22-phase1-audit.md

@@ -0,0 +1,157 @@
+# MS22 Phase 1 Module Audit
+
+Date: 2026-03-01
+Branch: `fix/ms22-audit`
+Scope:
+
+- `apps/api/src/container-lifecycle/`
+- `apps/api/src/crypto/`
+- `apps/api/src/agent-config/`
+- `apps/api/src/onboarding/`
+- `apps/api/src/fleet-settings/`
+- `apps/api/src/chat-proxy/`
+
+## Summary
+
+Audit completed for module wiring, security controls, input validation, and error handling.
+
+Findings:
+
+1. `chat-proxy`: raw internal/upstream error messages were returned to clients over SSE (fixed).
+2. `chat-proxy`: proxy requests to OpenClaw did not forward the container bearer token returned by lifecycle startup (fixed).
+3. `agent-config`: token validation returned early and used length-gated compare logic, creating avoidable timing side-channel behavior (hardened).
+
+## Module Review Results
+
+### 1) `container-lifecycle`
+
+- NestJS module dependency audit:
+  - `ContainerLifecycleModule` imports `ConfigModule`, `PrismaModule`, and `CryptoModule` required by `ContainerLifecycleService`.
+  - Providers/exports are correct (`ContainerLifecycleService` provided and exported).
+- Security review:
+  - Container operations are user-scoped by `userId` and do not expose cross-user selectors in this module.
+  - AES token generation/decryption delegated to `CryptoService`.
+- Input validation:
+  - No controller endpoints in this module; no direct request DTO surface here.
+- Error handling:
+  - No direct HTTP layer here; errors flow to callers/global filter.
+- Finding status: **No issues found in this module**.
+
+### 2) `crypto`
+
+- NestJS module dependency audit:
+  - `CryptoModule` correctly imports `ConfigModule` for `ConfigService`.
+  - `CryptoService` is correctly provided/exported.
+- Security review:
+  - AES-256-GCM is implemented correctly.
+  - 96-bit IV generated via `randomBytes(12)` per encryption.
+  - Auth tag captured and verified on decrypt (`setAuthTag` + `decipher.final()`).
+  - HKDF derives a fixed 32-byte key from `MOSAIC_SECRET_KEY`.
+- Input validation:
+  - No DTO/request surface in this module.
+- Error handling:
+  - Decrypt failures are normalized to `Failed to decrypt value`.
+- Finding status: **No issues found in this module**.
+
+### 3) `agent-config`
+
+- NestJS module dependency audit:
+  - `AgentConfigModule` imports `PrismaModule` + `CryptoModule`; `AgentConfigService` and `AgentConfigGuard` are provided.
+  - Controller/guard/service wiring is correct.
+- Security review:
+  - Bearer token comparisons used `timingSafeEqual`, but returned early on first match and performed length-gated comparison.
+  - Internal route (`/api/internal/agent-config/:id`) is access-controlled by bearer token guard and container-id match (`containerAuth.id === :id`).
+- Input validation:
+  - Header token extraction and route param are manually handled (no DTO for `:id`, acceptable for current use but should remain constrained).
+- Error handling:
+  - Service throws typed Nest exceptions for not-found paths.
+- Finding status: **Issue found and fixed**.
+
+### 4) `onboarding`
+
+- NestJS module dependency audit:
+  - `OnboardingModule` imports required dependencies (`PrismaModule`, `CryptoModule`; `ConfigModule` currently unused but harmless).
+  - Providers/controllers are correctly declared.
+- Security review:
+  - `OnboardingGuard` blocks all mutating onboarding routes once `onboarding.completed=true`.
+  - Onboarding cannot be re-run via guarded endpoints after completion.
+- Input validation:
+  - DTOs use `class-validator` decorators for all request bodies.
+- Error handling:
+  - Uses typed Nest exceptions (`ConflictException`, `BadRequestException`).
+- Finding status: **No issues found in this module**.
+
+### 5) `fleet-settings`
+
+- NestJS module dependency audit:
+  - `FleetSettingsModule` imports `AuthModule`, `PrismaModule`, `CryptoModule` required by its controller/service.
+  - Provider/export wiring is correct for `FleetSettingsService`.
+- Security review:
+  - Class-level `AuthGuard` protects all routes.
+  - Admin-only routes additionally use `AdminGuard` (`oidc` and `breakglass/reset-password`).
+  - Provider list/get responses do not expose `apiKey`.
+  - OIDC read response intentionally omits `clientSecret`.
+- Input validation:
+  - DTOs are decorated with `class-validator`.
+- Error handling:
+  - Ownership/not-found conditions use typed exceptions.
+- Finding status: **No issues found in this module**.
+
+### 6) `chat-proxy`
+
+- NestJS module dependency audit:
+  - `ChatProxyModule` imports `AuthModule`, `PrismaModule`, `ContainerLifecycleModule` needed by controller/service.
+  - Provider/controller wiring is correct.
+- Security review:
+  - User identity comes from `AuthGuard`; no user-provided container selector, so no cross-user container proxy path found.
+  - **Issue fixed:** gateway bearer token was not forwarded on proxied requests.
+  - **Issue fixed:** SSE error events exposed raw internal exception messages.
+- Input validation:
+  - `ChatStreamDto` + nested `ChatMessageDto` use `class-validator` decorators.
+- Error handling:
+  - **Issue fixed:** controller now emits safe client error messages and logs details server-side.
+- Finding status: **Issues found and fixed**.
+
+## Security Checklist Outcomes
+
+- `fleet-settings`: admin-only routes are guarded; non-admin users cannot access OIDC or breakglass reset routes. Provider secrets are not returned in provider read endpoints.
+- `agent-config`: token comparison hardened; route remains gated by bearer token + container id binding.
+- `onboarding`: guarded mutating endpoints cannot run after completion.
+- `crypto`: AES-256-GCM usage is correct (random IV, auth-tag verification, fixed 32-byte key derivation).
+- `chat-proxy`: user cannot target another user’s container; proxy now authenticates to OpenClaw using per-container bearer token.
+
+## Input Validation
+
+- DTO coverage is present in onboarding, fleet-settings, and chat-proxy request bodies.
+- No critical unvalidated body inputs found in scoped modules.
+
+## Error Handling
+
+- Global API layer has a sanitizing `GlobalExceptionFilter`.
+- `chat-proxy` used manual response handling (`@Res`) and bypassed global filter; this was corrected by sending safe generic SSE errors.
+- No additional critical sensitive-data leaks found in reviewed scope.
+
+## Changes Made
+
+1. Hardened token comparison behavior in:
+   - `apps/api/src/agent-config/agent-config.service.ts`
+   - Changes:
+     - Compare SHA-256 digests with `timingSafeEqual`.
+     - Avoid early return during scan to reduce timing signal differences.
+
+2. Fixed OpenClaw auth forwarding and error leak risk in:
+   - `apps/api/src/chat-proxy/chat-proxy.service.ts`
+   - `apps/api/src/chat-proxy/chat-proxy.controller.ts`
+   - `apps/api/src/chat-proxy/chat-proxy.service.spec.ts`
+   - Changes:
+     - Forward `Authorization: Bearer <gatewayToken>` when proxying chat requests.
+     - Stop returning raw internal/upstream error text to clients over SSE.
+     - Log details server-side and return safe client-facing messages.
+
+## Validation Commands
+
+Required quality gate command run:
+
+- `pnpm turbo lint typecheck --filter=@mosaic/api`
+
+(Results captured in session logs.)