Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
922 changes: 415 additions & 507 deletions .github/workflows/smoke-codex.lock.yml

Large diffs are not rendered by default.

3 changes: 0 additions & 3 deletions .github/workflows/smoke-codex.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ tools:
edit:
bash:
- "*"
sandbox:
mcp:
container: "ghcr.io/github/gh-aw-mcpg"
safe-outputs:
add-comment:
hide-older-comments: true
Expand Down
1 change: 1 addition & 0 deletions docs-site/src/content/docs/reference/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ awf [options] -- <command>
| `--skip-pull` | flag | `false` | Use local images without pulling from registry |
| `-e, --env <KEY=VALUE>` | string | `[]` | Environment variable (repeatable) |
| `--env-all` | flag | `false` | Pass all host environment variables |
| `--exclude-env <name>` | string | `[]` | Exclude a variable from `--env-all` passthrough (repeatable) |
| `-v, --mount <host:container[:mode]>` | string | `[]` | Volume mount (repeatable) |
| `--container-workdir <dir>` | string | User home | Working directory inside container |
| `--dns-servers <servers>` | string | `8.8.8.8,8.8.4.4` | Trusted DNS servers (comma-separated) |
Expand Down
10 changes: 2 additions & 8 deletions scripts/ci/postprocess-smoke-workflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const workflowPaths = [
// - "Install awf binary" or "Install AWF binary" step at any indent level
// - run command invoking install_awf_binary.sh with a version
const installStepRegex =
/^(\s*)- name: Install [Aa][Ww][Ff] binary\n\1\s*run: bash \/opt\/gh-aw\/actions\/install_awf_binary\.sh v[0-9.]+\n/m;
/^(\s*)- name: Install [Aa][Ww][Ff] binary\n\1\s*run: bash (?:\/opt\/gh-aw|\$\{RUNNER_TEMP\}\/gh-aw)\/actions\/install_awf_binary\.sh v[0-9.]+\n/m;
const installStepRegexGlobal = new RegExp(installStepRegex.source, 'gm');

function buildLocalInstallSteps(indent: string): string {
Expand Down Expand Up @@ -102,18 +102,12 @@ for (const workflowPath of workflowPaths) {
// Replace "Install awf binary" step with local build steps
const matches = content.match(installStepRegexGlobal);
if (matches) {
if (matches.length !== 1) {
throw new Error(
`Expected exactly one awf install step in ${workflowPath}, found ${matches.length}. ` +
'Ensure the workflow has a single "Install awf binary" step in the agent job.'
);
}
content = content.replace(
installStepRegexGlobal,
(_match, indent: string) => buildLocalInstallSteps(indent)
);
modified = true;
console.log(` Replaced awf install step with local build`);
console.log(` Replaced ${matches.length} awf install step(s) with local build`);
}

// Remove sparse-checkout from agent job checkout (need full repo for npm build)
Expand Down
7 changes: 7 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,12 @@ program
'Pass all host environment variables to container (excludes system vars like PATH)',
false
)
.option(
'--exclude-env <name>',
'Exclude a specific environment variable from --env-all passthrough (repeatable)',
(value: string, previous: string[] = []) => [...previous, value],
[]
)
.option(
'--env-file <path>',
'Read environment variables from a file (KEY=VALUE format, one per line)'
Expand Down Expand Up @@ -1706,6 +1712,7 @@ program
imageTag: options.imageTag,
additionalEnv: Object.keys(additionalEnv).length > 0 ? additionalEnv : undefined,
envAll: options.envAll,
excludeEnv: options.excludeEnv && options.excludeEnv.length > 0 ? options.excludeEnv : undefined,
envFile: options.envFile,
volumeMounts,
containerWorkDir: options.containerWorkdir,
Expand Down
71 changes: 71 additions & 0 deletions src/docker-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,77 @@ describe('docker-manager', () => {
}
});

it('should exclude specified variables when excludeEnv is set with envAll', () => {
process.env.CUSTOM_HOST_VAR = 'test_value';
process.env.SECRET_TOKEN = 'super-secret';

try {
const configWithExcludeEnv = { ...mockConfig, envAll: true, excludeEnv: ['SECRET_TOKEN'] };
const result = generateDockerCompose(configWithExcludeEnv, mockNetworkConfig);
const env = result.services.agent.environment as Record<string, string>;

// Should pass through non-excluded vars
expect(env.CUSTOM_HOST_VAR).toBe('test_value');
// Should NOT pass through excluded var
expect(env.SECRET_TOKEN).toBeUndefined();
} finally {
delete process.env.CUSTOM_HOST_VAR;
delete process.env.SECRET_TOKEN;
}
});

it('should exclude multiple variables when excludeEnv contains multiple names', () => {
process.env.TOKEN_A = 'value-a';
process.env.TOKEN_B = 'value-b';
process.env.SAFE_VAR = 'safe';

try {
const configWithExcludeEnv = { ...mockConfig, envAll: true, excludeEnv: ['TOKEN_A', 'TOKEN_B'] };
const result = generateDockerCompose(configWithExcludeEnv, mockNetworkConfig);
const env = result.services.agent.environment as Record<string, string>;

expect(env.TOKEN_A).toBeUndefined();
expect(env.TOKEN_B).toBeUndefined();
expect(env.SAFE_VAR).toBe('safe');
} finally {
delete process.env.TOKEN_A;
delete process.env.TOKEN_B;
delete process.env.SAFE_VAR;
}
});

it('should have no effect when excludeEnv is set but envAll is false', () => {
process.env.SECRET_TOKEN = 'super-secret';

try {
const configWithExcludeEnv = { ...mockConfig, envAll: false, excludeEnv: ['SECRET_TOKEN'] };
const result = generateDockerCompose(configWithExcludeEnv, mockNetworkConfig);
const env = result.services.agent.environment as Record<string, string>;

// envAll is false so SECRET_TOKEN was never going to be injected anyway
expect(env.SECRET_TOKEN).toBeUndefined();
} finally {
delete process.env.SECRET_TOKEN;
}
});

it('should exclude GITHUB_TOKEN from env-all passthrough when specified in excludeEnv', () => {
const prevToken = process.env.GITHUB_TOKEN;
process.env.GITHUB_TOKEN = 'ghp_test_token';

try {
const configWithExcludeEnv = { ...mockConfig, envAll: true, excludeEnv: ['GITHUB_TOKEN'] };
const result = generateDockerCompose(configWithExcludeEnv, mockNetworkConfig);
const env = result.services.agent.environment as Record<string, string>;

// GITHUB_TOKEN should be excluded from the env-all passthrough
expect(env.GITHUB_TOKEN).toBeUndefined();
} finally {
if (prevToken !== undefined) process.env.GITHUB_TOKEN = prevToken;
else delete process.env.GITHUB_TOKEN;
}
});

it('should auto-inject GH_HOST from GITHUB_SERVER_URL when envAll is true', () => {
const prevServerUrl = process.env.GITHUB_SERVER_URL;
const prevGhHost = process.env.GH_HOST;
Expand Down
7 changes: 7 additions & 0 deletions src/docker-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,13 @@ export function generateDockerCompose(
environment.AWF_BUN_INSTALL = process.env.BUN_INSTALL;
}

// If --exclude-env names were specified, add them to the excluded set
if (config.excludeEnv && config.excludeEnv.length > 0) {
for (const name of config.excludeEnv) {
EXCLUDED_ENV_VARS.add(name);
}
}
Comment on lines +596 to +601
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excludeEnv is added to EXCLUDED_ENV_VARS unconditionally, which means it also affects --env-file injection (and can have effects even when envAll is false), despite the WrapperConfig.excludeEnv docs stating it only applies to --env-all. Consider scoping excludeEnv to the envAll passthrough only (e.g., use a separate exclusion set for the env-all loop, or only add these names inside the if (config.envAll) block).

See below for a potential fix:

  // If --env-all is specified, pass through all host environment variables (except excluded ones)
  if (config.envAll) {
    // Build a local exclusion set for env-all passthrough:
    // start with the global excluded vars, then apply any --exclude-env names.
    const envAllExcludedVars = new Set(EXCLUDED_ENV_VARS);
    if (config.excludeEnv && config.excludeEnv.length > 0) {
      for (const name of config.excludeEnv) {
        envAllExcludedVars.add(name);
      }
    }

    for (const [key, value] of Object.entries(process.env)) {
      if (
        value !== undefined &&
        !envAllExcludedVars.has(key) &&
        !Object.prototype.hasOwnProperty.call(environment, key)
      ) {

Copilot uses AI. Check for mistakes.

// If --env-all is specified, pass through all host environment variables (except excluded ones)
if (config.envAll) {
for (const [key, value] of Object.entries(process.env)) {
Expand Down
11 changes: 11 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,17 @@ export interface WrapperConfig {
*/
envAll?: boolean;

/**
* Additional environment variable names to exclude when using --env-all
*
* When `envAll` is true, these variable names are excluded from the host environment
* passthrough in addition to the built-in exclusion list (PATH, HOME, etc.).
* Has no effect when `envAll` is false.
*
* @example ['GITHUB_MCP_SERVER_TOKEN', 'GH_AW_GITHUB_TOKEN']
*/
excludeEnv?: string[];

/**
* Path to a file containing environment variables to inject into the container
*
Expand Down
Loading