-
Notifications
You must be signed in to change notification settings - Fork 17
feat: add --env-file support for injecting env vars from a file #1457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7262a47
52d398b
e79800e
3cc0e2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1270,6 +1270,10 @@ program | |||||||||||||||||||||||||||||||||||
| 'Pass all host environment variables to container (excludes system vars like PATH)', | ||||||||||||||||||||||||||||||||||||
| false | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| .option( | ||||||||||||||||||||||||||||||||||||
| '--env-file <path>', | ||||||||||||||||||||||||||||||||||||
| 'Read environment variables from a file (KEY=VALUE format, one per line)' | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| .option( | ||||||||||||||||||||||||||||||||||||
| '-v, --mount <host_path:container_path[:mode]>', | ||||||||||||||||||||||||||||||||||||
| 'Volume mount (repeatable). Format: host_path:container_path[:ro|rw]', | ||||||||||||||||||||||||||||||||||||
|
|
@@ -1570,6 +1574,14 @@ program | |||||||||||||||||||||||||||||||||||
| additionalEnv = parsed.env; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Validate --env-file path if provided | ||||||||||||||||||||||||||||||||||||
| if (options.envFile) { | ||||||||||||||||||||||||||||||||||||
| if (!fs.existsSync(options.envFile)) { | ||||||||||||||||||||||||||||||||||||
| logger.error(`--env-file: file not found: ${options.envFile}`); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1579
to
+1580
|
||||||||||||||||||||||||||||||||||||
| if (!fs.existsSync(options.envFile)) { | |
| logger.error(`--env-file: file not found: ${options.envFile}`); | |
| try { | |
| if (!fs.existsSync(options.envFile)) { | |
| logger.error(`--env-file: file not found: ${options.envFile}`); | |
| process.exit(1); | |
| } | |
| const envFileStat = fs.statSync(options.envFile); | |
| if (!envFileStat.isFile()) { | |
| logger.error(`--env-file: not a regular file: ${options.envFile}`); | |
| process.exit(1); | |
| } | |
| fs.accessSync(options.envFile, fs.constants.R_OK); | |
| } catch (error) { | |
| logger.error(`--env-file: cannot read file: ${options.envFile}`); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||||||||||||||||||||||||||||||||
| import { generateDockerCompose, subnetsOverlap, writeConfigs, startContainers, stopContainers, cleanup, runAgentCommand, validateIdNotInSystemRange, getSafeHostUid, getSafeHostGid, getRealUserHome, extractGhHostFromServerUrl, readGitHubPathEntries, mergeGitHubPathEntries, MIN_REGULAR_UID, ACT_PRESET_BASE_IMAGE } from './docker-manager'; | ||||||||||||||||||||||||||||||||||||||
| import { generateDockerCompose, subnetsOverlap, writeConfigs, startContainers, stopContainers, cleanup, runAgentCommand, validateIdNotInSystemRange, getSafeHostUid, getSafeHostGid, getRealUserHome, extractGhHostFromServerUrl, readGitHubPathEntries, mergeGitHubPathEntries, readEnvFile, MIN_REGULAR_UID, ACT_PRESET_BASE_IMAGE } from './docker-manager'; | ||||||||||||||||||||||||||||||||||||||
| import { WrapperConfig } from './types'; | ||||||||||||||||||||||||||||||||||||||
| import * as fs from 'fs'; | ||||||||||||||||||||||||||||||||||||||
| import * as path from 'path'; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -1334,6 +1334,78 @@ describe('docker-manager', () => { | |||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| describe('envFile option', () => { | ||||||||||||||||||||||||||||||||||||||
| let tmpDir: string; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| beforeEach(() => { | ||||||||||||||||||||||||||||||||||||||
| tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-test-envfile-')); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| afterEach(() => { | ||||||||||||||||||||||||||||||||||||||
| fs.rmSync(tmpDir, { recursive: true, force: true }); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it('should inject variables from env file into agent environment', () => { | ||||||||||||||||||||||||||||||||||||||
| const envFile = path.join(tmpDir, '.env'); | ||||||||||||||||||||||||||||||||||||||
| fs.writeFileSync(envFile, 'MY_CUSTOM_VAR=hello\nANOTHER_VAR=world\n'); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const config = { ...mockConfig, envFile }; | ||||||||||||||||||||||||||||||||||||||
| const result = generateDockerCompose(config, mockNetworkConfig); | ||||||||||||||||||||||||||||||||||||||
| const env = result.services.agent.environment as Record<string, string>; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| expect(env.MY_CUSTOM_VAR).toBe('hello'); | ||||||||||||||||||||||||||||||||||||||
| expect(env.ANOTHER_VAR).toBe('world'); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it('should allow --env flags to override env-file values', () => { | ||||||||||||||||||||||||||||||||||||||
| const envFile = path.join(tmpDir, '.env'); | ||||||||||||||||||||||||||||||||||||||
| fs.writeFileSync(envFile, 'MY_VAR=from_file\n'); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const config = { ...mockConfig, envFile, additionalEnv: { MY_VAR: 'from_flag' } }; | ||||||||||||||||||||||||||||||||||||||
| const result = generateDockerCompose(config, mockNetworkConfig); | ||||||||||||||||||||||||||||||||||||||
| const env = result.services.agent.environment as Record<string, string>; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| expect(env.MY_VAR).toBe('from_flag'); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it('should not overwrite already-set env vars with env-file values', () => { | ||||||||||||||||||||||||||||||||||||||
| const envFile = path.join(tmpDir, '.env'); | ||||||||||||||||||||||||||||||||||||||
| // AWF_DNS_SERVERS is set before envFile processing; file should not clobber it | ||||||||||||||||||||||||||||||||||||||
| fs.writeFileSync(envFile, 'AWF_DNS_SERVERS=1.1.1.1\n'); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const config = { ...mockConfig, envFile }; | ||||||||||||||||||||||||||||||||||||||
| const result = generateDockerCompose(config, mockNetworkConfig); | ||||||||||||||||||||||||||||||||||||||
| const env = result.services.agent.environment as Record<string, string>; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // AWF_DNS_SERVERS is set by the framework; file should NOT override it | ||||||||||||||||||||||||||||||||||||||
| expect(env.AWF_DNS_SERVERS).not.toBe('1.1.1.1'); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1373
to
+1381
|
||||||||||||||||||||||||||||||||||||||
| // AWF_DNS_SERVERS is set before envFile processing; file should not clobber it | |
| fs.writeFileSync(envFile, 'AWF_DNS_SERVERS=1.1.1.1\n'); | |
| const config = { ...mockConfig, envFile }; | |
| const result = generateDockerCompose(config, mockNetworkConfig); | |
| const env = result.services.agent.environment as Record<string, string>; | |
| // AWF_DNS_SERVERS is set by the framework; file should NOT override it | |
| expect(env.AWF_DNS_SERVERS).not.toBe('1.1.1.1'); | |
| // AWF_HOST_PATH is set before envFile processing; file should not clobber it | |
| fs.writeFileSync(envFile, 'AWF_HOST_PATH=/malicious/path\n'); | |
| const config = { ...mockConfig, envFile }; | |
| const result = generateDockerCompose(config, mockNetworkConfig); | |
| const env = result.services.agent.environment as Record<string, string>; | |
| // AWF_HOST_PATH is set by the framework; file should NOT override it | |
| expect(env.AWF_HOST_PATH).not.toBe('/malicious/path'); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -207,6 +207,36 @@ export function mergeGitHubPathEntries(currentPath: string, githubPathEntries: s | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return [...newEntries, ...currentEntries].join(':'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Reads environment variables from a KEY=VALUE file (like Docker's --env-file). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Rules: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Lines starting with '#' are comments and are ignored. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Empty/whitespace-only lines are ignored. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Each non-comment line must match the pattern KEY=VALUE where KEY starts with a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * letter or underscore and contains only letters, digits, or underscores. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Values may be empty (KEY=). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Values are taken literally; no quote-stripping or variable expansion is done. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param filePath - Absolute or relative path to the env file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @returns An object mapping variable names to their values | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @throws {Error} If the file cannot be read | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function readEnvFile(filePath: string): Record<string, string> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const content = fs.readFileSync(filePath, 'utf-8'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const result: Record<string, string> = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const raw of content.split('\n')) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const line = raw.trim(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Skip comments and blank lines | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (line === '' || line.startsWith('#')) continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const match = line.match(/^([A-Za-z_][A-Za-z0-9_]*)=(.*)$/); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (match) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result[match[1]] = match[2]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+223
to
+235
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @throws {Error} If the file cannot be read | |
| */ | |
| export function readEnvFile(filePath: string): Record<string, string> { | |
| const content = fs.readFileSync(filePath, 'utf-8'); | |
| const result: Record<string, string> = {}; | |
| for (const raw of content.split('\n')) { | |
| const line = raw.trim(); | |
| // Skip comments and blank lines | |
| if (line === '' || line.startsWith('#')) continue; | |
| const match = line.match(/^([A-Za-z_][A-Za-z0-9_]*)=(.*)$/); | |
| if (match) { | |
| result[match[1]] = match[2]; | |
| } | |
| * @throws {Error} If the file cannot be read or contains an invalid line | |
| */ | |
| export function readEnvFile(filePath: string): Record<string, string> { | |
| const content = fs.readFileSync(filePath, 'utf-8'); | |
| const result: Record<string, string> = {}; | |
| const lines = content.split('\n'); | |
| for (let i = 0; i < lines.length; i++) { | |
| const raw = lines[i]; | |
| const line = raw.trim(); | |
| // Skip comments and blank lines | |
| if (line === '' || line.startsWith('#')) continue; | |
| const match = line.match(/^([A-Za-z_][A-Za-z0-9_]*)=(.*)$/); | |
| if (!match) { | |
| throw new Error( | |
| `Invalid env file line in ${filePath} at line ${i + 1}: "${raw}" ` + | |
| '(expected KEY=VALUE with KEY matching /^[A-Za-z_][A-Za-z0-9_]*$/)' | |
| ); | |
| } | |
| result[match[1]] = match[2]; |
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--env-file is documented as having higher precedence than --env-all, but this block only sets values when environment does not already have the key. Since --env-all runs earlier and also populates environment, env-file entries won’t override env-all entries. Either adjust the precedence (allow env-file to override env-all while still protecting framework/excluded vars) or update the docs/comments/tests to reflect the actual “first writer wins” behavior.
| // Environment variables from --env-file (injected before --env flags so explicit flags win) | |
| if (config.envFile) { | |
| const fileEnv = readEnvFile(config.envFile); | |
| for (const [key, value] of Object.entries(fileEnv)) { | |
| if (!EXCLUDED_ENV_VARS.has(key) && !Object.prototype.hasOwnProperty.call(environment, key)) { | |
| // Environment variables from --env-file (applied after --env-all but before --env flags; | |
| // env-file entries override env-all for non-excluded vars, while explicit --env flags still win) | |
| if (config.envFile) { | |
| const fileEnv = readEnvFile(config.envFile); | |
| for (const [key, value] of Object.entries(fileEnv)) { | |
| if (!EXCLUDED_ENV_VARS.has(key)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
--env-filedocs claim (1) env-file values are “taken literally” and (2) precedence is built-in → env-all → env-file → env. In the current implementation, lines are trimmed during parsing, framework vars are protected from being overwritten by env-all/env-file, and env-file does not override env-all. Please adjust this section to match actual behavior (or update the implementation to match the documented precedence).