Skip to content

Commit 5475caf

Browse files
CopilotCopilot
authored
Deduplicate API proxy config env lookup and auth type normalization (#4338)
* Initial plan * Deduplicate api-proxy env config helpers --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 3e9574a commit 5475caf

4 files changed

Lines changed: 117 additions & 31 deletions

File tree

‎src/env-utils.test.ts‎

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,19 @@
1-
import { pickEnvVars } from './env-utils';
1+
import * as fs from 'fs';
2+
import * as os from 'os';
3+
import * as path from 'path';
4+
import { WrapperConfig } from './types';
5+
import { getConfigEnvValue, getLowerCaseProcessEnvValue, pickEnvVars } from './env-utils';
6+
7+
function makeWrapperConfig(overrides: Partial<WrapperConfig> = {}): WrapperConfig {
8+
return {
9+
agentCommand: 'echo test',
10+
allowedDomains: [],
11+
keepContainers: false,
12+
logLevel: 'info',
13+
workDir: '/tmp/env-utils-test',
14+
...overrides,
15+
};
16+
}
217

318
describe('pickEnvVars', () => {
419
let savedEnv: Record<string, string | undefined>;
@@ -61,3 +76,76 @@ describe('pickEnvVars', () => {
6176
expect(pickEnvVars('TEST_PICK_SINGLE')).toEqual({ TEST_PICK_SINGLE: 'only-one' });
6277
});
6378
});
79+
80+
describe('getConfigEnvValue', () => {
81+
let savedEnv: string | undefined;
82+
83+
beforeEach(() => {
84+
savedEnv = process.env.TEST_CONFIG_ENV_VALUE;
85+
delete process.env.TEST_CONFIG_ENV_VALUE;
86+
});
87+
88+
afterEach(() => {
89+
if (savedEnv !== undefined) {
90+
process.env.TEST_CONFIG_ENV_VALUE = savedEnv;
91+
} else {
92+
delete process.env.TEST_CONFIG_ENV_VALUE;
93+
}
94+
});
95+
96+
it('prefers additionalEnv over envFile and process.env, trimming the result', () => {
97+
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'env-utils-'));
98+
const envFilePath = path.join(tempDir, '.env');
99+
fs.writeFileSync(envFilePath, 'TEST_CONFIG_ENV_VALUE= from-file \n');
100+
process.env.TEST_CONFIG_ENV_VALUE = ' from-process ';
101+
102+
try {
103+
const config = makeWrapperConfig({
104+
additionalEnv: { TEST_CONFIG_ENV_VALUE: ' from-additional ' },
105+
envAll: true,
106+
envFile: envFilePath,
107+
});
108+
109+
expect(getConfigEnvValue(config, 'TEST_CONFIG_ENV_VALUE')).toBe('from-additional');
110+
} finally {
111+
fs.rmSync(tempDir, { recursive: true, force: true });
112+
}
113+
});
114+
115+
it('falls back to process.env only when envAll is enabled and omits blank values', () => {
116+
process.env.TEST_CONFIG_ENV_VALUE = ' ';
117+
const config = makeWrapperConfig({ envAll: true });
118+
expect(getConfigEnvValue(config, 'TEST_CONFIG_ENV_VALUE')).toBeUndefined();
119+
120+
process.env.TEST_CONFIG_ENV_VALUE = ' from-process ';
121+
expect(getConfigEnvValue(config, 'TEST_CONFIG_ENV_VALUE')).toBe('from-process');
122+
expect(getConfigEnvValue(makeWrapperConfig({ envAll: false }), 'TEST_CONFIG_ENV_VALUE')).toBeUndefined();
123+
});
124+
});
125+
126+
describe('getLowerCaseProcessEnvValue', () => {
127+
let savedEnv: string | undefined;
128+
129+
beforeEach(() => {
130+
savedEnv = process.env.TEST_LOWERCASE_ENV_VALUE;
131+
delete process.env.TEST_LOWERCASE_ENV_VALUE;
132+
});
133+
134+
afterEach(() => {
135+
if (savedEnv !== undefined) {
136+
process.env.TEST_LOWERCASE_ENV_VALUE = savedEnv;
137+
} else {
138+
delete process.env.TEST_LOWERCASE_ENV_VALUE;
139+
}
140+
});
141+
142+
it('trims and lowercases process env values', () => {
143+
process.env.TEST_LOWERCASE_ENV_VALUE = ' GitHub-OIDC ';
144+
expect(getLowerCaseProcessEnvValue('TEST_LOWERCASE_ENV_VALUE')).toBe('github-oidc');
145+
});
146+
147+
it('returns undefined for blank process env values', () => {
148+
process.env.TEST_LOWERCASE_ENV_VALUE = ' ';
149+
expect(getLowerCaseProcessEnvValue('TEST_LOWERCASE_ENV_VALUE')).toBeUndefined();
150+
});
151+
});

‎src/env-utils.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,26 @@
1+
import { readEnvFile } from './github-env';
2+
import { WrapperConfig } from './types';
3+
4+
export function normalizeEnvValue(value: string | undefined): string | undefined {
5+
const normalizedValue = value?.trim();
6+
return normalizedValue || undefined;
7+
}
8+
9+
export function getConfigEnvValue(config: WrapperConfig, key: string): string | undefined {
10+
const envFileValue = config.envFile
11+
? readEnvFile(config.envFile)[key]
12+
: undefined;
13+
const value =
14+
config.additionalEnv?.[key] ??
15+
envFileValue ??
16+
(config.envAll ? process.env[key] : undefined);
17+
return normalizeEnvValue(value);
18+
}
19+
20+
export function getLowerCaseProcessEnvValue(key: string): string | undefined {
21+
return normalizeEnvValue(process.env[key])?.toLowerCase();
22+
}
23+
124
/**
225
* Returns an object containing only the specified environment variable names
326
* that are currently set (non-empty) in `process.env`.

‎src/services/api-proxy-credential-env.ts‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { logger } from '../logger';
22
import { WrapperConfig, API_PROXY_PORTS } from '../types';
3-
import { readEnvFile } from '../github-env';
43
import { COPILOT_PLACEHOLDER_TOKEN } from '../constants/placeholders';
4+
import { getConfigEnvValue, getLowerCaseProcessEnvValue } from '../env-utils';
55
import { NetworkConfig } from './squid-service';
66

77
interface ApiProxyCredentialEnvParams {
@@ -14,18 +14,6 @@ interface ApiProxyCredentialEnvParams {
1414
// are runtime-configurable and not limited to a fixed allowlist.
1515
const RESPONSES_WIRE_API_MODEL_PATTERN = /(^|[/:])(gpt-5|o3)([-_.]|$)/i;
1616

17-
function getConfigEnvValue(config: WrapperConfig, key: string): string | undefined {
18-
const envFileValue = config.envFile
19-
? readEnvFile(config.envFile)[key]
20-
: undefined;
21-
const value =
22-
config.additionalEnv?.[key] ??
23-
envFileValue ??
24-
(config.envAll ? process.env[key] : undefined);
25-
const normalizedValue = value?.trim();
26-
return normalizedValue || undefined;
27-
}
28-
2917
function requiresResponsesWireApi(copilotModel: string): boolean {
3018
return RESPONSES_WIRE_API_MODEL_PATTERN.test(copilotModel);
3119
}
@@ -35,8 +23,8 @@ export function buildAgentCredentialEnv(params: ApiProxyCredentialEnvParams): Re
3523
if (!networkConfig.proxyIp) {
3624
throw new Error('buildAgentCredentialEnv: networkConfig.proxyIp is required');
3725
}
38-
const normalizedAuthType = (process.env.AWF_AUTH_TYPE || '').trim().toLowerCase();
39-
const normalizedAuthProvider = (process.env.AWF_AUTH_PROVIDER || '').trim().toLowerCase();
26+
const normalizedAuthType = getLowerCaseProcessEnvValue('AWF_AUTH_TYPE') || '';
27+
const normalizedAuthProvider = getLowerCaseProcessEnvValue('AWF_AUTH_PROVIDER') || '';
4028
const shouldProxyAnthropic = Boolean(config.anthropicApiKey || (normalizedAuthType === 'github-oidc' && normalizedAuthProvider === 'anthropic'));
4129

4230
const agentEnvAdditions: Record<string, string> = {

‎src/services/api-proxy-service-config.ts‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@ import {
44
SQUID_PORT,
55
} from '../constants';
66
import { stripScheme } from '../host-env';
7-
import { readEnvFile } from '../github-env';
87
import { buildRuntimeImageRef } from '../image-tag';
98
import { WrapperConfig, API_PROXY_HEALTH_PORT } from '../types';
10-
import { pickEnvVars } from '../env-utils';
9+
import { getConfigEnvValue, getLowerCaseProcessEnvValue, pickEnvVars } from '../env-utils';
1110
import { NetworkConfig, ImageBuildConfig } from './squid-service';
1211
import { applyHostPathPrefixToVolumes } from './host-path-prefix';
1312
import { buildContainerSecurityHardening } from './service-security';
@@ -72,25 +71,13 @@ function resolveProviderSessionId(config: WrapperConfig): string | undefined {
7271
return normalizedValue || undefined;
7372
}
7473

75-
function getConfigEnvValue(config: WrapperConfig, key: string): string | undefined {
76-
const envFileValue = config.envFile
77-
? readEnvFile(config.envFile)[key]
78-
: undefined;
79-
const value =
80-
config.additionalEnv?.[key] ??
81-
envFileValue ??
82-
(config.envAll ? process.env[key] : undefined);
83-
const normalizedValue = value?.trim();
84-
return normalizedValue || undefined;
85-
}
86-
8774
export function buildApiProxyServiceConfig(params: ApiProxyServiceConfigParams): any {
8875
const { config, networkConfig, apiProxyLogsPath, imageConfig } = params;
8976
if (!networkConfig.proxyIp) {
9077
throw new Error('buildApiProxyServiceConfig: networkConfig.proxyIp is required');
9178
}
9279
const { useGHCR, registry, parsedTag, projectRoot } = imageConfig;
93-
const normalizedAuthType = (process.env.AWF_AUTH_TYPE || '').trim().toLowerCase();
80+
const normalizedAuthType = getLowerCaseProcessEnvValue('AWF_AUTH_TYPE') || '';
9481

9582
const proxyService: any = {
9683
container_name: API_PROXY_CONTAINER_NAME,

0 commit comments

Comments
 (0)