fix: Fix error in the validation pipeline (#669)

* fix: strip all invalid HTTP header characters in sanitizeEnvValue

The previous regex only removed \r\n but Node.js rejects all control
characters outside [\t\x20-\x7e\x80-\xff] with ERR_INVALID_CHAR.
CI secrets can contain null bytes or other control chars that slipped
through.

https://claude.ai/code/session_01Kj7rEHNuhTG5U7NrM9RWaG

* fix: sanitize env vars in-place so third-party libs get clean values

The OTel OTLP exporter (used by @arizeai/phoenix-otel) reads
PHOENIX_API_KEY directly from process.env via getEnvApiKey(), bypassing
our sanitizeEnvValue() wrapper. It then passes the raw value to
node:http which throws ERR_INVALID_CHAR.

Add sanitizeProcessEnv() that rewrites sensitive env vars on
process.env itself, called right after dotenv.config(). This ensures
every reader — including third-party libraries — gets clean values.

https://claude.ai/code/session_01Kj7rEHNuhTG5U7NrM9RWaG

* fix: simplify regex, improve comments, add redacted env logging

- Replace cryptic allowlist regex with readable control-char blocklist
- Explain why in-place sanitization is needed (phoenix-otel reads env directly)
- Log redacted env var values during sanitizeProcessEnv() for CI debugging

https://claude.ai/code/session_01Kj7rEHNuhTG5U7NrM9RWaG

* fix: simplify tests for sanitizeEnvValue and sanitizeProcessEnv

* docs: update DEVELOPMENT.md to include LLM evals

* fix: improve value redaction logic for safer logging

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: jirispilka

DEVELOPMENT.md

@@ -104,6 +104,11 @@ Restart Claude Code for the change to take effect. This token is picked up by bo
 | **Unit tests** | `npm run test:unit` | Individual modules in isolation — no credentials needed |
 | **Integration tests** | `npm run test:integration` | Full server over all transports against real Apify API (requires `APIFY_TOKEN` + `npm run build`) |
 | **mcpc probing** | `mcpc @stdio tools-call ...` | Interactive end-to-end verification during development |
+| **LLM evals** | CI only — apply `validated` label | Runs `evals/run_evaluation.ts` against multiple models via OpenRouter; requires `PHOENIX_*` and `OPENROUTER_*` secrets |
+
+To trigger the eval workflow on a PR, apply the **`validated`** label.
+The workflow then runs automatically and posts results to Phoenix.
+It also runs automatically on every merge to the `master` branch.
 
 ### Live probing with mcpc
 

evals/config.ts

@@ -9,7 +9,7 @@ import { fileURLToPath } from 'node:url';
 import log from '@apify/log';
 
 // Re-export shared config
-export { OPENROUTER_CONFIG, sanitizeEnvValue, validateEnvVars, getRequiredEnvVars } from './shared/config.js';
+export { OPENROUTER_CONFIG, sanitizeEnvValue, sanitizeProcessEnv, validateEnvVars, getRequiredEnvVars } from './shared/config.js';
 
 // Read the version from test-cases.json
 function getTestCasesVersion(): string {

evals/create_dataset.ts

@@ -14,7 +14,7 @@ import { hideBin } from 'yargs/helpers';
 
 import log from '@apify/log';
 
-import { sanitizeEnvValue, validatePhoenixEnvVars } from './config.js';
+import { sanitizeEnvValue, sanitizeProcessEnv, validatePhoenixEnvVars } from './config.js';
 import { loadTestCases, filterByCategory, filterById, type TestCase } from './evaluation_utils.js';
 
 // Set log level to debug
@@ -32,6 +32,7 @@ type CliArgs = {
 
 // Load environment variables from .env file if present
 dotenv.config({ path: '.env' });
+sanitizeProcessEnv();
 
 // Parse command line arguments using yargs
 const argv = yargs(hideBin(process.argv))

evals/run_evaluation.ts

@@ -30,6 +30,7 @@ import {
     PHOENIX_MAX_RETRIES,
     type EvaluatorName,
     sanitizeEnvValue,
+    sanitizeProcessEnv,
     validatePhoenixEnvVars
 } from './config.js';
 
@@ -58,6 +59,7 @@ const RUN_LLM_EVALUATOR = true;
 const RUN_TOOLS_EXACT_MATCH_EVALUATOR = true;
 
 dotenv.config({ path: '.env' });
+sanitizeProcessEnv();
 
 // Parse command line arguments using yargs
 const argv = yargs(hideBin(process.argv))

evals/shared/config.ts

@@ -23,12 +23,60 @@ export function getRequiredEnvVars(): Record<string, string | undefined> {
 }
 
 /**
- * Removes newlines, trims whitespace, and strips surrounding quotes from env values.
- * CI secrets often include trailing newlines or quotes that break HTTP headers and URLs.
+ * Strips control characters, trims whitespace, and removes surrounding double quotes.
+ * CI secrets often contain trailing newlines or invisible control chars that break HTTP headers.
  */
 export function sanitizeEnvValue(value?: string): string | undefined {
     if (value == null) return value;
-    return value.replace(/[\r\n]/g, '').trim().replace(/^"|"$/g, '');
+    // eslint-disable-next-line no-control-regex
+    return value.replace(/[\x00-\x08\x0a-\x1f\x7f]/g, '').trim().replace(/^"|"$/g, '');
+}
+
+/**
+ * Env vars used in HTTP headers (API keys, tokens, URLs).
+ *
+ * Why in-place? The phoenix-otel OTel exporter reads PHOENIX_API_KEY directly
+ * from process.env (inside getEnvApiKey()) and passes it to node:http, which
+ * throws ERR_INVALID_CHAR on any control characters. We can't intercept that
+ * read, so we sanitize process.env itself before any library loads.
+ */
+const ENV_KEYS_TO_SANITIZE = [
+    'OPENROUTER_API_KEY',
+    'OPENROUTER_BASE_URL',
+    'PHOENIX_API_KEY',
+    'PHOENIX_BASE_URL',
+];
+
+/**
+ * Redact a value for safe logging: shows first 4 and last 4 chars, masks the rest.
+ * Fully masks short values (≤ 8 chars) to prevent reconstruction from the log line.
+ * Returns '(empty)' for empty strings, '(unset)' for undefined/null.
+ */
+function redact(value?: string | null): string {
+    if (value == null) return '(unset)';
+    if (value.length === 0) return '(empty)';
+    if (value.length <= 6) return `*** (${value.length} chars)`;
+    return `${value.slice(0, 3)}***${value.slice(-3)} (${value.length} chars)`;
+}
+
+/**
+ * Sanitize env vars in-place on process.env and log redacted values for CI debugging.
+ * Must be called before any library reads these values.
+ */
+export function sanitizeProcessEnv(): void {
+    for (const key of ENV_KEYS_TO_SANITIZE) {
+        const raw = process.env[key];
+        if (raw != null) {
+            const sanitized = sanitizeEnvValue(raw)!;
+            const changed = raw !== sanitized;
+            process.env[key] = sanitized;
+            // eslint-disable-next-line no-console
+            console.log(`env ${key}: ${redact(sanitized)}${changed ? ' (sanitized)' : ''}`);
+        } else {
+            // eslint-disable-next-line no-console
+            console.log(`env ${key}: ${redact(raw)}`);
+        }
+    }
 }
 
 /**

evals/workflows/config.ts

@@ -6,7 +6,7 @@
  */
 
 // Re-export shared config for convenience
-export { OPENROUTER_CONFIG, sanitizeEnvValue, validateEnvVars } from '../shared/config.js';
+export { OPENROUTER_CONFIG, sanitizeEnvValue, sanitizeProcessEnv, validateEnvVars } from '../shared/config.js';
 
 /**
  * Default model configuration for agent and judge

tests/unit/evals.config.test.ts

@@ -1,6 +1,6 @@
-import { describe, expect, it } from 'vitest';
+import { afterEach, describe, expect, it } from 'vitest';
 
-import { sanitizeEnvValue } from '../../evals/shared/config.js';
+import { sanitizeEnvValue, sanitizeProcessEnv } from '../../evals/shared/config.js';
 
 describe('sanitizeEnvValue', () => {
     it('returns undefined for undefined', () => {
@@ -47,8 +47,37 @@ describe('sanitizeEnvValue', () => {
         expect(sanitizeEnvValue('')).toBe('');
     });
 
+    it('strips control characters', () => {
+        expect(sanitizeEnvValue('sk-abc\x00123')).toBe('sk-abc123'); // null byte
+        expect(sanitizeEnvValue('sk-abc\x01123')).toBe('sk-abc123'); // SOH
+        expect(sanitizeEnvValue('sk-abc\x0b123')).toBe('sk-abc123'); // vertical tab
+        expect(sanitizeEnvValue('sk-abc\x0c123')).toBe('sk-abc123'); // form feed
+        expect(sanitizeEnvValue('sk-abc\x1f123')).toBe('sk-abc123'); // unit separator
+        expect(sanitizeEnvValue('sk-abc\x7f123')).toBe('sk-abc123'); // DEL
+    });
+
     it('is idempotent', () => {
         const value = '  "sk-abc123"\r\n';
         expect(sanitizeEnvValue(sanitizeEnvValue(value))).toBe(sanitizeEnvValue(value));
     });
 });
+
+describe('sanitizeProcessEnv', () => {
+    afterEach(() => {
+        delete process.env.PHOENIX_API_KEY;
+        delete process.env.OPENROUTER_API_KEY;
+    });
+
+    it('sanitizes env vars in-place', () => {
+        process.env.PHOENIX_API_KEY = 'key-with-newline\n';
+        process.env.OPENROUTER_API_KEY = '  "quoted-key"\r\n';
+        sanitizeProcessEnv();
+        expect(process.env.PHOENIX_API_KEY).toBe('key-with-newline');
+        expect(process.env.OPENROUTER_API_KEY).toBe('quoted-key');
+    });
+
+    it('leaves unset vars untouched', () => {
+        sanitizeProcessEnv();
+        expect(process.env.PHOENIX_API_KEY).toBeUndefined();
+    });
+});