fix(webapp): redact secrets & PII from logs (worker secret, API key/JWT, Slack) (#41)

Author: d-cs

.server-changes/redact-secrets-from-logs.md

@@ -0,0 +1,6 @@
+---
+area: webapp
+type: fix
+---
+
+Improve redaction of secrets from debug logs

apps/webapp/app/models/orgIntegration.server.ts

@@ -9,6 +9,8 @@ import { z } from "zod";
 import { $transaction, prisma } from "~/db.server";
 import { env } from "~/env.server";
 import { logger } from "~/services/logger.server";
+import { slackSecretLogFields } from "./safeIntegrationLog";
+import { slackAccessResultLogFields } from "./slackOAuthResultLog";
 import { getSecretStore } from "~/services/secrets/secretStore.server";
 import { commitSession, getUserSession } from "~/services/sessionStorage.server";
 import { generateFriendlyId } from "~/v3/friendlyIdentifiers";
@@ -205,9 +207,8 @@ export class OrgIntegrationRepository {
         });
 
         if (result.ok) {
-          logger.debug("Received slack access token", {
-            result,
-          });
+          // `result` carries Slack tokens; log only non-secret diagnostics.
+          logger.debug("Received slack access token", slackAccessResultLogFields(result));
 
           if (!result.access_token) {
             throw new Error("Failed to get access token");
@@ -230,9 +231,12 @@ export class OrgIntegrationRepository {
               raw: result,
             };
 
-            logger.debug("Setting secret", {
-              secretValue,
-            });
+            // `secretValue` carries the tokens encrypted below; log only
+            // non-secret fields.
+            logger.debug(
+              "Setting secret",
+              slackSecretLogFields(integrationFriendlyId, secretValue)
+            );
 
             await secretStore.setSecret(integrationFriendlyId, secretValue);
 

apps/webapp/app/models/safeIntegrationLog.ts

@@ -0,0 +1,20 @@
+// Non-secret fields for logging a Slack integration secret: presence booleans
+// and scope arrays only, never the token values. Dependency-free so it's
+// unit-tested directly.
+export type SlackSecretLike = {
+  botAccessToken?: string;
+  userAccessToken?: string;
+  refreshToken?: string;
+  botScopes?: string[];
+  userScopes?: string[];
+};
+
+export function slackSecretLogFields(friendlyId: string, secret: SlackSecretLike) {
+  return {
+    friendlyId,
+    hasUserToken: !!secret.userAccessToken,
+    hasRefreshToken: !!secret.refreshToken,
+    botScopes: secret.botScopes,
+    userScopes: secret.userScopes,
+  };
+}

apps/webapp/app/models/slackOAuthResultLog.ts

@@ -0,0 +1,18 @@
+// Non-secret fields for logging a Slack `oauth.v2.access` response, which
+// otherwise carries bot/user/refresh tokens. Dependency-free so it's
+// unit-tested directly.
+export type SlackAccessResultLike = {
+  team?: { id?: string } | null;
+  scope?: string;
+  authed_user?: { access_token?: string } | null;
+  refresh_token?: string;
+};
+
+export function slackAccessResultLogFields(result: SlackAccessResultLike) {
+  return {
+    teamId: result.team?.id,
+    scope: result.scope,
+    hasUserToken: !!result.authed_user?.access_token,
+    hasRefreshToken: !!result.refresh_token,
+  };
+}

apps/webapp/app/services/apiAuth.server.ts

@@ -14,6 +14,8 @@ import {
 } from "~/models/runtimeEnvironment.server";
 import { type RuntimeEnvironmentForEnvRepo } from "~/v3/environmentVariables/environmentVariablesRepository.server";
 import { logger } from "./logger.server";
+import { safeEnvironmentLogFields } from "./safeEnvironmentLog";
+import { missingJwtLogContext } from "./safeRequestLogContext";
 import {
   type PersonalAccessTokenAuthenticationResult,
   authenticateApiRequestWithPersonalAccessToken,
@@ -673,9 +675,9 @@ export async function validateJWTTokenAndRenew<T extends z.ZodTypeAny>(
     const jwt = request.headers.get("x-trigger-jwt");
 
     if (!jwt) {
-      logger.debug("Missing JWT token in request", {
-        headers: Object.fromEntries(request.headers),
-      });
+      // Log a safe breadcrumb, not the raw headers (which carry the
+      // caller's Authorization credential).
+      logger.debug("Missing JWT token in request", missingJwtLogContext(request));
 
       return;
     }
@@ -735,8 +737,9 @@ export async function validateJWTTokenAndRenew<T extends z.ZodTypeAny>(
         ...payload.data,
       });
 
+      // The environment carries secret material; log only non-secret fields.
       logger.debug("Renewed JWT token from Authorization header API Key", {
-        environment: authenticatedEnv.environment,
+        environment: safeEnvironmentLogFields(authenticatedEnv.environment),
         payload: payload.data,
       });
 

apps/webapp/app/services/safeEnvironmentLog.ts

@@ -0,0 +1,19 @@
+// Non-secret subset of an AuthenticatedEnvironment for logging (the full shape
+// carries the env's apiKey). Dependency-free so it's unit-tested directly.
+export type EnvironmentForLog = {
+  id: string;
+  slug: string;
+  type: string;
+  projectId: string;
+  organizationId: string;
+};
+
+export function safeEnvironmentLogFields(environment: EnvironmentForLog) {
+  return {
+    id: environment.id,
+    slug: environment.slug,
+    type: environment.type,
+    projectId: environment.projectId,
+    organizationId: environment.organizationId,
+  };
+}

apps/webapp/app/services/safeRequestLogContext.ts

@@ -0,0 +1,15 @@
+// A safe breadcrumb for logging an inbound API request. Must never include
+// header *values*, only presence — the Authorization header carries the
+// caller's credential. Dependency-free so it's unit-tested directly.
+export function missingJwtLogContext(request: Request): {
+  method: string;
+  path: string;
+  hasAuthorization: boolean;
+} {
+  const url = new URL(request.url);
+  return {
+    method: request.method,
+    path: url.pathname,
+    hasAuthorization: request.headers.has("authorization"),
+  };
+}

apps/webapp/app/v3/services/worker/sanitizeWorkerHeaders.ts

@@ -0,0 +1,27 @@
+import { WORKER_HEADERS } from "@trigger.dev/core/v3/workers";
+
+// Secret-bearing headers to drop before logging request headers.
+// Dependency-free so the redaction is unit-tested directly.
+export const SENSITIVE_WORKER_HEADERS = new Set([
+  "authorization",
+  "cookie",
+  WORKER_HEADERS.MANAGED_SECRET.toLowerCase(),
+]);
+
+/**
+ * Copy `headers` into a plain object, dropping any header whose (lower-cased)
+ * name is in `denylist`. Used before logging request headers.
+ */
+export function sanitizeWorkerHeaders(
+  headers: Headers,
+  denylist: ReadonlySet<string> = SENSITIVE_WORKER_HEADERS
+): Partial<Record<string, string>> {
+  const skip = new Set(Array.from(denylist, (h) => h.toLowerCase()));
+  const sanitized: Partial<Record<string, string>> = {};
+  for (const [key, value] of headers.entries()) {
+    if (!skip.has(key.toLowerCase())) {
+      sanitized[key] = value;
+    }
+  }
+  return sanitized;
+}

apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts

@@ -18,6 +18,7 @@ import { fromFriendlyId } from "@trigger.dev/core/v3/isomorphic";
 import { WORKER_HEADERS, type WorkerQueueClass } from "@trigger.dev/core/v3/workers";
 import type { RuntimeEnvironment, WorkerInstanceGroup } from "@trigger.dev/database";
 import { Prisma, WorkerInstanceGroupType } from "@trigger.dev/database";
+import { SENSITIVE_WORKER_HEADERS, sanitizeWorkerHeaders } from "./sanitizeWorkerHeaders";
 import { createHash, timingSafeEqual } from "crypto";
 import { customAlphabet } from "nanoid";
 import { z } from "zod";
@@ -187,15 +188,13 @@ export class WorkerGroupTokenService extends WithRunEngine {
 
     if (a.byteLength !== b.byteLength) {
       logger.error("[WorkerGroupTokenService] Managed secret length mismatch", {
-        managedWorkerSecret,
         headers: this.sanitizeHeaders(request),
       });
       return;
     }
 
     if (!timingSafeEqual(a, b)) {
       logger.error("[WorkerGroupTokenService] Managed secret mismatch", {
-        managedWorkerSecret,
         headers: this.sanitizeHeaders(request),
       });
       return;
@@ -317,16 +316,10 @@ export class WorkerGroupTokenService extends WithRunEngine {
     }
   }
 
-  private sanitizeHeaders(request: Request, skipHeaders = ["authorization"]) {
-    const sanitizedHeaders: Partial<Record<string, string>> = {};
-
-    for (const [key, value] of request.headers.entries()) {
-      if (!skipHeaders.includes(key.toLowerCase())) {
-        sanitizedHeaders[key] = value;
-      }
-    }
-
-    return sanitizedHeaders;
+  // Strip sensitive headers before logging request headers — see
+  // `sanitizeWorkerHeaders`.
+  private sanitizeHeaders(request: Request, denylist = SENSITIVE_WORKER_HEADERS) {
+    return sanitizeWorkerHeaders(request.headers, denylist);
   }
 }
 

apps/webapp/test/safeEnvironmentLog.test.ts

@@ -0,0 +1,31 @@
+import { describe, expect, it } from "vitest";
+import { safeEnvironmentLogFields } from "../app/services/safeEnvironmentLog.js";
+
+// AuthenticatedEnvironment carries `apiKey` (and pkApiKey) — these must never
+// reach the logger when an environment is logged.
+const environment = {
+  id: "env_1",
+  slug: "prod",
+  type: "PRODUCTION",
+  projectId: "proj_1",
+  organizationId: "org_1",
+  apiKey: "tr_prod_SUPERSECRET",
+  pkApiKey: "pk_prod_SECRET",
+} as any;
+
+describe("safeEnvironmentLogFields", () => {
+  it("emits only non-secret identity fields, never the api key", () => {
+    const fields = safeEnvironmentLogFields(environment);
+    const serialized = JSON.stringify(fields);
+    expect(serialized).not.toContain("tr_prod_SUPERSECRET");
+    expect(serialized).not.toContain("pk_prod_SECRET");
+    expect(serialized).not.toContain("apiKey");
+    expect(fields).toEqual({
+      id: "env_1",
+      slug: "prod",
+      type: "PRODUCTION",
+      projectId: "proj_1",
+      organizationId: "org_1",
+    });
+  });
+});