fix: sanitize env vars in telemetry to prevent API key leakage (#285)

* fix: sanitize env vars in telemetry to prevent API key leakage

The telemetry system was capturing full template context including
all environment variables, which leaked sensitive API keys like
GOOGLE_API_KEY, JIRA_AUTH, ZENDESK_AUTH in trace files.

Added sanitizeContextForTelemetry() that redacts env vars matching
sensitive patterns (api_key, secret, token, password, auth, etc.)
before capturing in OTEL spans or fallback NDJSON traces.

Affected files:
- src/telemetry/state-capture.ts: Added sanitization function and
  applied to captureCheckInputContext()
- src/providers/command-check-provider.ts: Applied to fallback NDJSON
- src/providers/ai-check-provider.ts: Applied to fallback NDJSON

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* fix: use sanitizedContext consistently in captureCheckInputContext

Changed individual attribute captures (pr, outputs, env_keys) to use
sanitizedContext instead of original context to avoid inconsistent
sanitization where the full context was sanitized but individual
attributes were not.

Co-Authored-By: Claude Opus 4.5 <[email protected]>

---------

Co-authored-by: Claude Opus 4.5 <[email protected]>

Author: buger

src/providers/ai-check-provider.ts

@@ -12,6 +12,7 @@ import {
   captureCheckInputContext,
   captureCheckOutput,
   captureProviderCall,
+  sanitizeContextForTelemetry,
 } from '../telemetry/state-capture';
 import { CustomToolsSSEServer } from './mcp-custom-sse-server';
 import { CustomToolDefinition } from '../types/config';
@@ -951,7 +952,8 @@ export class AICheckProvider extends CheckProvider {
     // Fallback NDJSON for input context (non-OTEL environments)
     try {
       const checkId = (config as any).checkName || (config as any).id || 'unknown';
-      const ctxJson = JSON.stringify(templateContext);
+      // Sanitize context to avoid leaking API keys in traces
+      const ctxJson = JSON.stringify(sanitizeContextForTelemetry(templateContext));
       const { emitNdjsonSpanWithEvents } = require('../telemetry/fallback-ndjson');
       emitNdjsonSpanWithEvents(
         'visor.check',

src/providers/command-check-provider.ts

@@ -18,6 +18,7 @@ import {
   captureCheckInputContext,
   captureCheckOutput,
   captureTransformJS,
+  sanitizeContextForTelemetry,
 } from '../telemetry/state-capture';
 
 /**
@@ -155,7 +156,8 @@ export class CommandCheckProvider extends CheckProvider {
     // Fallback NDJSON for input context (non-OTEL environments)
     try {
       const checkId = (config as any).checkName || (config as any).id || 'unknown';
-      const ctxJson = JSON.stringify(templateContext);
+      // Sanitize context to avoid leaking API keys in traces
+      const ctxJson = JSON.stringify(sanitizeContextForTelemetry(templateContext));
       const { emitNdjsonSpanWithEvents } = require('../telemetry/fallback-ndjson');
       // Emit both start and completion markers together for deterministic E2E assertions
       emitNdjsonSpanWithEvents(

src/telemetry/state-capture.ts

@@ -10,6 +10,53 @@ import { Span } from './lazy-otel';
 const MAX_ATTRIBUTE_LENGTH = 10000; // Truncate large values
 const MAX_ARRAY_ITEMS = 100; // Limit array size in attributes
 
+// Patterns that indicate sensitive environment variables (case-insensitive)
+const SENSITIVE_ENV_PATTERNS = [
+  /api[_-]?key/i,
+  /secret/i,
+  /token/i,
+  /password/i,
+  /auth/i,
+  /credential/i,
+  /private[_-]?key/i,
+  /^sk-/i, // OpenAI-style keys
+  /^AIza/i, // Google API keys
+];
+
+/**
+ * Check if an environment variable name is sensitive
+ */
+function isSensitiveEnvVar(name: string): boolean {
+  return SENSITIVE_ENV_PATTERNS.some(pattern => pattern.test(name));
+}
+
+/**
+ * Sanitize context for telemetry by redacting sensitive environment variables.
+ * Returns a new object with env values redacted (keys preserved).
+ */
+export function sanitizeContextForTelemetry(
+  context: Record<string, unknown>
+): Record<string, unknown> {
+  if (!context || typeof context !== 'object') return context;
+
+  const sanitized = { ...context };
+
+  // Sanitize env object if present
+  if (sanitized.env && typeof sanitized.env === 'object') {
+    const sanitizedEnv: Record<string, string> = {};
+    for (const [key, value] of Object.entries(sanitized.env as Record<string, unknown>)) {
+      if (isSensitiveEnvVar(key)) {
+        sanitizedEnv[key] = '[REDACTED]';
+      } else {
+        sanitizedEnv[key] = String(value);
+      }
+    }
+    sanitized.env = sanitizedEnv;
+  }
+
+  return sanitized;
+}
+
 /**
  * Safely serialize a value for OTEL span attributes.
  * Handles truncation, circular refs, and type conversions.
@@ -46,23 +93,30 @@ function safeSerialize(value: unknown, maxLength = MAX_ATTRIBUTE_LENGTH): string
  */
 export function captureCheckInputContext(span: Span, context: Record<string, unknown>): void {
   try {
+    // Sanitize context to redact sensitive env vars before capturing
+    const sanitizedContext = sanitizeContextForTelemetry(context);
+
     // Capture key context variables
-    const keys = Object.keys(context);
+    const keys = Object.keys(sanitizedContext);
     span.setAttribute('visor.check.input.keys', keys.join(','));
     span.setAttribute('visor.check.input.count', keys.length);
 
-    // Capture full context as JSON (with size limit)
-    span.setAttribute('visor.check.input.context', safeSerialize(context));
+    // Capture full context as JSON (with size limit) - now sanitized
+    span.setAttribute('visor.check.input.context', safeSerialize(sanitizedContext));
 
     // Capture specific important variables separately for easy querying
-    if (context.pr) {
-      span.setAttribute('visor.check.input.pr', safeSerialize(context.pr, 1000));
+    // Use sanitizedContext consistently to avoid leaking sensitive data
+    if (sanitizedContext.pr) {
+      span.setAttribute('visor.check.input.pr', safeSerialize(sanitizedContext.pr, 1000));
     }
-    if (context.outputs) {
-      span.setAttribute('visor.check.input.outputs', safeSerialize(context.outputs, 5000));
+    if (sanitizedContext.outputs) {
+      span.setAttribute('visor.check.input.outputs', safeSerialize(sanitizedContext.outputs, 5000));
     }
-    if (context.env) {
-      span.setAttribute('visor.check.input.env_keys', Object.keys(context.env as object).join(','));
+    if (sanitizedContext.env) {
+      span.setAttribute(
+        'visor.check.input.env_keys',
+        Object.keys(sanitizedContext.env as object).join(',')
+      );
     }
   } catch (err) {
     try {

tests/unit/telemetry-env-sanitization.test.ts

@@ -0,0 +1,111 @@
+import { sanitizeContextForTelemetry } from '../../src/telemetry/state-capture';
+
+describe('sanitizeContextForTelemetry', () => {
+  it('should redact sensitive env vars by pattern', () => {
+    const context = {
+      pr: { number: 1 },
+      env: {
+        PATH: '/usr/bin',
+        HOME: '/home/user',
+        GOOGLE_API_KEY: 'AIzaSyBqWp9Ent9-31GQOI8oAUPW6eGY5PMQHQM',
+        OPENAI_API_KEY: 'sk-abc123',
+        JIRA_AUTH: 'base64token',
+        ZENDESK_AUTH: 'base64token',
+        MY_SECRET: 'supersecret',
+        DB_PASSWORD: 'hunter2',
+        AWS_SECRET_ACCESS_KEY: 'aws-secret',
+        PRIVATE_KEY: 'private-key-content',
+        GITHUB_TOKEN: 'ghp_xxx',
+        API_KEY: 'some-api-key',
+        NODE_ENV: 'test',
+      },
+    };
+
+    const sanitized = sanitizeContextForTelemetry(context);
+
+    // Non-sensitive vars should be preserved
+    expect((sanitized.env as any).PATH).toBe('/usr/bin');
+    expect((sanitized.env as any).HOME).toBe('/home/user');
+    expect((sanitized.env as any).NODE_ENV).toBe('test');
+
+    // Sensitive vars should be redacted
+    expect((sanitized.env as any).GOOGLE_API_KEY).toBe('[REDACTED]');
+    expect((sanitized.env as any).OPENAI_API_KEY).toBe('[REDACTED]');
+    expect((sanitized.env as any).JIRA_AUTH).toBe('[REDACTED]');
+    expect((sanitized.env as any).ZENDESK_AUTH).toBe('[REDACTED]');
+    expect((sanitized.env as any).MY_SECRET).toBe('[REDACTED]');
+    expect((sanitized.env as any).DB_PASSWORD).toBe('[REDACTED]');
+    expect((sanitized.env as any).AWS_SECRET_ACCESS_KEY).toBe('[REDACTED]');
+    expect((sanitized.env as any).PRIVATE_KEY).toBe('[REDACTED]');
+    expect((sanitized.env as any).GITHUB_TOKEN).toBe('[REDACTED]');
+    expect((sanitized.env as any).API_KEY).toBe('[REDACTED]');
+  });
+
+  it('should preserve other context fields unchanged', () => {
+    const context = {
+      pr: { number: 123, title: 'Test PR' },
+      files: [{ name: 'test.ts' }],
+      outputs: { check1: 'result' },
+      env: {
+        PATH: '/usr/bin',
+        API_KEY: 'secret',
+      },
+    };
+
+    const sanitized = sanitizeContextForTelemetry(context);
+
+    expect(sanitized.pr).toEqual({ number: 123, title: 'Test PR' });
+    expect(sanitized.files).toEqual([{ name: 'test.ts' }]);
+    expect(sanitized.outputs).toEqual({ check1: 'result' });
+  });
+
+  it('should handle context without env', () => {
+    const context = {
+      pr: { number: 1 },
+    };
+
+    const sanitized = sanitizeContextForTelemetry(context);
+
+    expect(sanitized.pr).toEqual({ number: 1 });
+    expect(sanitized.env).toBeUndefined();
+  });
+
+  it('should handle null/undefined context', () => {
+    expect(sanitizeContextForTelemetry(null as any)).toBe(null);
+    expect(sanitizeContextForTelemetry(undefined as any)).toBe(undefined);
+  });
+
+  it('should not mutate original context', () => {
+    const context = {
+      env: {
+        API_KEY: 'secret',
+        PATH: '/usr/bin',
+      },
+    };
+
+    sanitizeContextForTelemetry(context);
+
+    // Original should be unchanged
+    expect(context.env.API_KEY).toBe('secret');
+  });
+
+  it('should detect case-insensitive patterns', () => {
+    const context = {
+      env: {
+        api_key: 'lower',
+        API_KEY: 'upper',
+        Api_Key: 'mixed',
+        apiKey: 'camel',
+        APIKEY: 'no separator',
+      },
+    };
+
+    const sanitized = sanitizeContextForTelemetry(context);
+
+    expect((sanitized.env as any).api_key).toBe('[REDACTED]');
+    expect((sanitized.env as any).API_KEY).toBe('[REDACTED]');
+    expect((sanitized.env as any).Api_Key).toBe('[REDACTED]');
+    expect((sanitized.env as any).apiKey).toBe('[REDACTED]');
+    expect((sanitized.env as any).APIKEY).toBe('[REDACTED]');
+  });
+});