Skip to content

Commit 9f0b9d9

Browse files
jirispilkaclaudeCopilot
authored
fix: Sanitize all env values in evals to prevent ERR_INVALID_CHAR (#665)
* fix: Sanitize all env values in evals to prevent ERR_INVALID_CHAR CI secrets often contain trailing newlines or quotes that cause `TypeError [ERR_INVALID_CHAR]: Invalid character in header content` when passed to HTTP clients. - Sanitize OPENROUTER_CONFIG (apiKey + baseURL) at capture time in shared/config.ts - Sanitize APIFY_TOKEN in workflow evals - Sanitize both OpenRouter clients in evaluation_utils.ts (createOpenRouterTask + createClassifierEvaluator) - Rename sanitizeHeaderValue → sanitizeEnvValue (used for URLs and tokens, not just headers) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: sanitize openrouterKey and guard undefined assignment to process.env Agent-Logs-Url: https://github.com/apify/apify-mcp-server/sessions/76f2f7a9-f806-4b0b-86de-138f7be29b15 Co-authored-by: jirispilka <19406805+jirispilka@users.noreply.github.com> * fix: Remove sanitizeEnvValue usage and implement OPENROUTER_CONFIG for environment variables --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent af4d0b8 commit 9f0b9d9

9 files changed

Lines changed: 72 additions & 29 deletions

File tree

evals/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { fileURLToPath } from 'node:url';
99
import log from '@apify/log';
1010

1111
// Re-export shared config
12-
export { OPENROUTER_CONFIG, sanitizeHeaderValue, validateEnvVars, getRequiredEnvVars } from './shared/config.js';
12+
export { OPENROUTER_CONFIG, sanitizeEnvValue, validateEnvVars, getRequiredEnvVars } from './shared/config.js';
1313

1414
// Read the version from test-cases.json
1515
function getTestCasesVersion(): string {

evals/create_dataset.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { hideBin } from 'yargs/helpers';
1414

1515
import log from '@apify/log';
1616

17-
import { sanitizeHeaderValue, validatePhoenixEnvVars } from './config.js';
17+
import { sanitizeEnvValue, validatePhoenixEnvVars } from './config.js';
1818
import { loadTestCases, filterByCategory, filterById, type TestCase } from './evaluation_utils.js';
1919

2020
// Set log level to debug
@@ -98,7 +98,7 @@ async function createDatasetFromTestCases(
9898
const client = createClient({
9999
options: {
100100
baseUrl: process.env.PHOENIX_BASE_URL!,
101-
headers: { Authorization: `Bearer ${sanitizeHeaderValue(process.env.PHOENIX_API_KEY)}` },
101+
headers: { Authorization: `Bearer ${sanitizeEnvValue(process.env.PHOENIX_API_KEY)}` },
102102
},
103103
});
104104

evals/eval_single.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
loadTestCases, filterById,
1010
type TestCase
1111
} from './evaluation_utils.js';
12-
import { PASS_THRESHOLD, sanitizeHeaderValue } from './config.js';
12+
import { PASS_THRESHOLD } from './config.js';
1313

1414
dotenv.config({ path: '.env' });
1515
log.setLevel(log.LEVELS.INFO);
@@ -25,8 +25,6 @@ const EXAMPLES: TestCase[] = [
2525
EXAMPLES.push(...filterById(loadTestCases('test_cases.json').testCases, 'fetch-actor-details-1'));
2626

2727
async function main() {
28-
process.env.OPENROUTER_API_KEY = sanitizeHeaderValue(process.env.OPENROUTER_API_KEY);
29-
3028
console.log(`\nEvaluating ${EXAMPLES.length} examples\n`);
3129

3230
// 1. Load tools

evals/evaluation_utils.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
TOOL_SELECTION_EVAL_MODEL,
1919
EVALUATOR_NAMES,
2020
TEMPERATURE,
21-
sanitizeHeaderValue
21+
OPENROUTER_CONFIG,
2222
} from './config.js';
2323
import { loadTestCases as loadTestCasesShared, filterByCategory, filterById } from './shared/test_case_loader.js';
2424
import { transformToolsToOpenAIFormat } from './shared/openai_tools.js';
@@ -56,10 +56,7 @@ export function createOpenRouterTask(modelName: string, tools: ToolBase[]) {
5656
context: string;
5757
reference: string;
5858
}> => {
59-
const client = new OpenAI({
60-
baseURL: process.env.OPENROUTER_BASE_URL,
61-
apiKey: sanitizeHeaderValue(process.env.OPENROUTER_API_KEY),
62-
});
59+
const client = new OpenAI(OPENROUTER_CONFIG);
6360

6461
log.info(`Input: ${JSON.stringify(example)}`);
6562

@@ -104,10 +101,7 @@ export function createOpenRouterTask(modelName: string, tools: ToolBase[]) {
104101
}
105102

106103
export function createClassifierEvaluator() {
107-
const openai = createOpenAI({
108-
baseURL: process.env.OPENROUTER_BASE_URL,
109-
apiKey: process.env.OPENROUTER_API_KEY,
110-
});
104+
const openai = createOpenAI(OPENROUTER_CONFIG);
111105

112106
return createClassifierFn({
113107
model: openai(TOOL_SELECTION_EVAL_MODEL),

evals/run_evaluation.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
PHOENIX_RETRY_DELAY_MS,
3030
PHOENIX_MAX_RETRIES,
3131
type EvaluatorName,
32-
sanitizeHeaderValue,
32+
sanitizeEnvValue,
3333
validatePhoenixEnvVars
3434
} from './config.js';
3535

@@ -78,9 +78,6 @@ const argv = yargs(hideBin(process.argv))
7878
.epilogue(' npm run evals:run -- --dataset-name custom_v1 # Via npm script')
7979
.parseSync() as CliArgs;
8080

81-
// Sanitize secrets early to avoid invalid header characters in CI
82-
process.env.OPENROUTER_API_KEY = sanitizeHeaderValue(process.env.OPENROUTER_API_KEY);
83-
8481
// Tools match evaluator: returns score 1 if expected tool_calls match output list, 0 otherwise
8582
const toolsExactMatch = asEvaluator({
8683
name: EVALUATOR_NAMES.TOOLS_EXACT_MATCH,
@@ -197,7 +194,7 @@ async function main(datasetName: string): Promise<number> {
197194
const client = createClient({
198195
options: {
199196
baseUrl: process.env.PHOENIX_BASE_URL!,
200-
headers: { Authorization: `Bearer ${sanitizeHeaderValue(process.env.PHOENIX_API_KEY)}` },
197+
headers: { Authorization: `Bearer ${sanitizeEnvValue(process.env.PHOENIX_API_KEY)}` },
201198
},
202199
});
203200

evals/shared/config.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
* OPENROUTER_BASE_URL is optional and defaults to the standard OpenRouter API URL
99
*/
1010
export const OPENROUTER_CONFIG = {
11-
baseURL: process.env.OPENROUTER_BASE_URL || 'https://openrouter.ai/api/v1',
12-
apiKey: process.env.OPENROUTER_API_KEY || '',
11+
baseURL: sanitizeEnvValue(process.env.OPENROUTER_BASE_URL) || 'https://openrouter.ai/api/v1',
12+
apiKey: sanitizeEnvValue(process.env.OPENROUTER_API_KEY) || '',
1313
};
1414

1515
/**
@@ -23,10 +23,10 @@ export function getRequiredEnvVars(): Record<string, string | undefined> {
2323
}
2424

2525
/**
26-
* Removes newlines and trims whitespace. Useful for Authorization header values
27-
* because CI secrets sometimes include trailing newlines or quotes.
26+
* Removes newlines, trims whitespace, and strips surrounding quotes from env values.
27+
* CI secrets often include trailing newlines or quotes that break HTTP headers and URLs.
2828
*/
29-
export function sanitizeHeaderValue(value?: string): string | undefined {
29+
export function sanitizeEnvValue(value?: string): string | undefined {
3030
if (value == null) return value;
3131
return value.replace(/[\r\n]/g, '').trim().replace(/^"|"$/g, '');
3232
}

evals/workflows/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*/
77

88
// Re-export shared config for convenience
9-
export { OPENROUTER_CONFIG, sanitizeHeaderValue, validateEnvVars } from '../shared/config.js';
9+
export { OPENROUTER_CONFIG, sanitizeEnvValue, validateEnvVars } from '../shared/config.js';
1010

1111
/**
1212
* Default model configuration for agent and judge

evals/workflows/run_workflow_evals.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { hideBin } from 'yargs/helpers';
2121
import { filterByLineRanges } from '../shared/line_range_filter.js';
2222
import type { LineRange } from '../shared/line_range_parser.js';
2323
import { checkRangesOutOfBounds, parseLineRanges, validateLineRanges } from '../shared/line_range_parser.js';
24-
import { DEFAULT_TOOL_TIMEOUT_SECONDS, MODELS } from './config.js';
24+
import { DEFAULT_TOOL_TIMEOUT_SECONDS, MODELS, sanitizeEnvValue } from './config.js';
2525
import { executeConversation } from './conversation_executor.js';
2626
import { LlmClient } from './llm_client.js';
2727
import { McpClient } from './mcp_client.js';
@@ -211,8 +211,8 @@ async function main() {
211211
console.log();
212212

213213
// Check environment variables
214-
const apifyToken = process.env.APIFY_TOKEN;
215-
const openrouterKey = process.env.OPENROUTER_API_KEY;
214+
const apifyToken = sanitizeEnvValue(process.env.APIFY_TOKEN);
215+
const openrouterKey = sanitizeEnvValue(process.env.OPENROUTER_API_KEY);
216216

217217
if (!apifyToken) {
218218
console.error('❌ Error: APIFY_TOKEN environment variable is required');

tests/unit/evals.config.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { sanitizeEnvValue } from '../../evals/shared/config.js';
4+
5+
describe('sanitizeEnvValue', () => {
6+
it('returns undefined for undefined', () => {
7+
expect(sanitizeEnvValue(undefined)).toBeUndefined();
8+
});
9+
10+
it('returns null for null', () => {
11+
expect(sanitizeEnvValue(null as unknown as undefined)).toBeNull();
12+
});
13+
14+
it('strips trailing newline', () => {
15+
expect(sanitizeEnvValue('sk-abc123\n')).toBe('sk-abc123');
16+
});
17+
18+
it('strips carriage-return + newline', () => {
19+
expect(sanitizeEnvValue('sk-abc123\r\n')).toBe('sk-abc123');
20+
});
21+
22+
it('strips embedded newlines', () => {
23+
expect(sanitizeEnvValue('sk-\nabc\r\n123\n')).toBe('sk-abc123');
24+
});
25+
26+
it('trims surrounding whitespace', () => {
27+
expect(sanitizeEnvValue(' sk-abc123 ')).toBe('sk-abc123');
28+
});
29+
30+
it('strips surrounding double quotes', () => {
31+
expect(sanitizeEnvValue('"sk-abc123"')).toBe('sk-abc123');
32+
});
33+
34+
it('strips only outer quotes (not inner)', () => {
35+
expect(sanitizeEnvValue('"sk-"abc"-123"')).toBe('sk-"abc"-123');
36+
});
37+
38+
it('does not strip single quotes', () => {
39+
expect(sanitizeEnvValue("'sk-abc123'")).toBe("'sk-abc123'");
40+
});
41+
42+
it('handles combined whitespace, newlines, and quotes', () => {
43+
expect(sanitizeEnvValue(' "sk-abc123"\n')).toBe('sk-abc123');
44+
});
45+
46+
it('returns empty string for empty input', () => {
47+
expect(sanitizeEnvValue('')).toBe('');
48+
});
49+
50+
it('is idempotent', () => {
51+
const value = ' "sk-abc123"\r\n';
52+
expect(sanitizeEnvValue(sanitizeEnvValue(value))).toBe(sanitizeEnvValue(value));
53+
});
54+
});

0 commit comments

Comments
 (0)