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
26 changes: 26 additions & 0 deletions docs/environment.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,32 @@ sudo -E awf --allow-domains github.com 'copilot --prompt "..."'
- Working with untrusted code
- In production/CI environments

## `COPILOT_GITHUB_TOKEN` and Classic PAT Compatibility

When `COPILOT_GITHUB_TOKEN` is set in the host environment, AWF injects it into the agent container so the Copilot CLI can authenticate against the GitHub Copilot API.

### ⚠️ Classic PAT + `COPILOT_MODEL` Incompatibility (Copilot CLI 1.0.21+)

Copilot CLI 1.0.21 introduced a startup model validation step: when `COPILOT_MODEL` is set, the CLI calls `GET /models` before executing any task. **This endpoint does not accept classic PATs** (`ghp_*` tokens), causing the agent to fail at startup with exit code 1 — before any useful work begins.

**Affected combination:**
- `COPILOT_GITHUB_TOKEN` is a classic PAT (prefixed with `ghp_`)
- `COPILOT_MODEL` is set in the agent environment (e.g., via `--env COPILOT_MODEL=...`, `--env-file`, or `--env-all`)

**Unaffected:** Workflows that do not set `COPILOT_MODEL` are not affected — the `/models` validation is only triggered when `COPILOT_MODEL` is set.

**AWF detects this combination at startup** and emits a `[WARN]` message:
```
⚠️ COPILOT_MODEL is set with a classic PAT (ghp_* token)
Copilot CLI 1.0.21+ validates COPILOT_MODEL via GET /models at startup.
Classic PATs are rejected by this endpoint — the agent will likely fail with exit code 1.
Use a fine-grained PAT or OAuth token, or unset COPILOT_MODEL to skip model validation.
```

**Remediation options:**
1. Replace the classic PAT with a **fine-grained PAT** or **OAuth token** (these are accepted by the `/models` endpoint).
2. Remove `COPILOT_MODEL` from the agent environment to skip model validation entirely.

## Internal Environment Variables

The following environment variables are set internally by the firewall and used by container scripts:
Expand Down
42 changes: 41 additions & 1 deletion src/cli.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Command } from 'commander';
import { parseEnvironmentVariables, parseDomains, parseDomainsFile, escapeShellArg, joinShellArgs, parseVolumeMounts, isValidIPv4, isValidIPv6, parseDnsServers, parseDnsOverHttps, validateAgentImage, isAgentImagePreset, AGENT_IMAGE_PRESETS, processAgentImageOption, processLocalhostKeyword, validateSkipPullWithBuildLocal, validateAllowHostPorts, validateAllowHostServicePorts, applyHostServicePortsConfig, parseMemoryLimit, validateFormat, validateApiProxyConfig, buildRateLimitConfig, validateRateLimitFlags, hasRateLimitOptions, collectRulesetFile, validateApiTargetInAllowedDomains, DEFAULT_OPENAI_API_TARGET, DEFAULT_ANTHROPIC_API_TARGET, DEFAULT_COPILOT_API_TARGET, DEFAULT_GEMINI_API_TARGET, emitApiProxyTargetWarnings, emitCliProxyStatusLogs, formatItem, program, parseAgentTimeout, applyAgentTimeout, handlePredownloadAction, resolveApiTargetsToAllowedDomains, extractGhesDomainsFromEngineApiTarget, extractGhecDomainsFromServerUrl } from './cli';
import { parseEnvironmentVariables, parseDomains, parseDomainsFile, escapeShellArg, joinShellArgs, parseVolumeMounts, isValidIPv4, isValidIPv6, parseDnsServers, parseDnsOverHttps, validateAgentImage, isAgentImagePreset, AGENT_IMAGE_PRESETS, processAgentImageOption, processLocalhostKeyword, validateSkipPullWithBuildLocal, validateAllowHostPorts, validateAllowHostServicePorts, applyHostServicePortsConfig, parseMemoryLimit, validateFormat, validateApiProxyConfig, buildRateLimitConfig, validateRateLimitFlags, hasRateLimitOptions, collectRulesetFile, validateApiTargetInAllowedDomains, DEFAULT_OPENAI_API_TARGET, DEFAULT_ANTHROPIC_API_TARGET, DEFAULT_COPILOT_API_TARGET, DEFAULT_GEMINI_API_TARGET, emitApiProxyTargetWarnings, emitCliProxyStatusLogs, warnClassicPATWithCopilotModel, formatItem, program, parseAgentTimeout, applyAgentTimeout, handlePredownloadAction, resolveApiTargetsToAllowedDomains, extractGhesDomainsFromEngineApiTarget, extractGhecDomainsFromServerUrl } from './cli';
import { redactSecrets } from './redact-secrets';
import * as fs from 'fs';
import * as path from 'path';
Expand Down Expand Up @@ -48,14 +48,14 @@

afterEach(() => {
// Clean up the test directory
if (fs.existsSync(testDir)) {

Check warning on line 51 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found existsSync from package "fs" with non literal argument at index 0

Check warning on line 51 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found existsSync from package "fs" with non literal argument at index 0

Check warning on line 51 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found existsSync from package "fs" with non literal argument at index 0
fs.rmSync(testDir, { recursive: true, force: true });
}
});

it('should parse domains from file with one domain per line', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com\napi.github.com\nnpmjs.org');

Check warning on line 58 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 58 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 58 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found writeFileSync from package "fs" with non literal argument at index 0

const result = parseDomainsFile(filePath);

Expand All @@ -64,7 +64,7 @@

it('should parse comma-separated domains from file', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com, api.github.com, npmjs.org');

Check warning on line 67 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 67 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 67 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found writeFileSync from package "fs" with non literal argument at index 0

const result = parseDomainsFile(filePath);

Expand All @@ -73,7 +73,7 @@

it('should handle mixed formats (lines and commas)', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com\napi.github.com, npmjs.org\nexample.com');

Check warning on line 76 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 76 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 76 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found writeFileSync from package "fs" with non literal argument at index 0

const result = parseDomainsFile(filePath);

Expand All @@ -82,7 +82,7 @@

it('should skip empty lines', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com\n\n\napi.github.com\n\nnpmjs.org');

Check warning on line 85 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 85 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 85 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found writeFileSync from package "fs" with non literal argument at index 0

const result = parseDomainsFile(filePath);

Expand All @@ -91,7 +91,7 @@

it('should skip lines with only whitespace', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com\n \n\t\napi.github.com');

Check warning on line 94 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 94 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 94 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found writeFileSync from package "fs" with non literal argument at index 0

const result = parseDomainsFile(filePath);

Expand All @@ -100,7 +100,7 @@

it('should skip comments starting with #', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, '# This is a comment\ngithub.com\n# Another comment\napi.github.com');

Check warning on line 103 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 103 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 103 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found writeFileSync from package "fs" with non literal argument at index 0

const result = parseDomainsFile(filePath);

Expand All @@ -109,7 +109,7 @@

it('should handle inline comments (after domain)', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com # GitHub main domain\napi.github.com # API endpoint');

Check warning on line 112 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 112 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 112 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found writeFileSync from package "fs" with non literal argument at index 0

const result = parseDomainsFile(filePath);

Expand All @@ -118,7 +118,7 @@

it('should handle domains with inline comments in comma-separated format', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com, api.github.com # GitHub domains\nnpmjs.org');

Check warning on line 121 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 121 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 121 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found writeFileSync from package "fs" with non literal argument at index 0

const result = parseDomainsFile(filePath);

Expand All @@ -133,7 +133,7 @@

it('should return empty array for file with only comments and whitespace', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, '# Comment 1\n\n# Comment 2\n \n');

Check warning on line 136 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 136 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found writeFileSync from package "fs" with non literal argument at index 0

Check warning on line 136 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found writeFileSync from package "fs" with non literal argument at index 0

const result = parseDomainsFile(filePath);

Expand Down Expand Up @@ -2112,6 +2112,46 @@
});
});

describe('warnClassicPATWithCopilotModel', () => {
it('should emit warnings when classic PAT and COPILOT_MODEL are both set', () => {
const warns: string[] = [];
warnClassicPATWithCopilotModel(true, true, (msg) => warns.push(msg));
expect(warns.length).toBeGreaterThan(0);
expect(warns[0]).toContain('COPILOT_MODEL');
expect(warns.some(w => w.includes('classic PAT'))).toBe(true);
});

it('should not warn when token is not a classic PAT', () => {
const warns: string[] = [];
warnClassicPATWithCopilotModel(false, true, (msg) => warns.push(msg));
expect(warns).toHaveLength(0);
});

it('should not warn when COPILOT_MODEL is not set', () => {
const warns: string[] = [];
warnClassicPATWithCopilotModel(true, false, (msg) => warns.push(msg));
expect(warns).toHaveLength(0);
});

it('should not warn when neither condition holds', () => {
const warns: string[] = [];
warnClassicPATWithCopilotModel(false, false, (msg) => warns.push(msg));
expect(warns).toHaveLength(0);
});

it('should mention /models endpoint in warning', () => {
const warns: string[] = [];
warnClassicPATWithCopilotModel(true, true, (msg) => warns.push(msg));
expect(warns.some(w => w.includes('/models'))).toBe(true);
});

it('should mention exit code 1 in warning', () => {
const warns: string[] = [];
warnClassicPATWithCopilotModel(true, true, (msg) => warns.push(msg));
expect(warns.some(w => w.includes('exit code 1'))).toBe(true);
});
});

describe('resolveApiTargetsToAllowedDomains', () => {
it('should add copilot-api-target option to allowed domains', () => {
const domains: string[] = ['github.com'];
Expand Down
57 changes: 57 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,30 @@ export function emitCliProxyStatusLogs(
}
}

/**
* Warns when a classic GitHub PAT (ghp_* prefix) is used alongside COPILOT_MODEL.
* Copilot CLI 1.0.21+ performs a GET /models validation at startup when COPILOT_MODEL
* is set. This endpoint rejects classic PATs, causing the agent to fail with exit code 1
* before any useful work begins.
* Accepts booleans (not actual tokens/values) to prevent sensitive data from flowing
* through to log output (CodeQL: clear-text logging of sensitive information).
* @param isClassicPAT - Whether COPILOT_GITHUB_TOKEN starts with 'ghp_' (classic PAT)
* @param hasCopilotModel - Whether COPILOT_MODEL is set in the agent environment
* @param warn - Function to emit a warning message
*/
export function warnClassicPATWithCopilotModel(
isClassicPAT: boolean,
hasCopilotModel: boolean,
warn: (msg: string) => void,
): void {
if (!isClassicPAT || !hasCopilotModel) return;

warn('⚠️ COPILOT_MODEL is set with a classic PAT (ghp_* token)');
warn(' Copilot CLI 1.0.21+ validates COPILOT_MODEL via GET /models at startup.');
warn(' Classic PATs are rejected by this endpoint — the agent will likely fail with exit code 1.');
warn(' Use a fine-grained PAT or OAuth token, or unset COPILOT_MODEL to skip model validation.');
}

/**
* Extracts GHEC domains from GITHUB_SERVER_URL and GITHUB_API_URL environment variables.
* When GITHUB_SERVER_URL points to a GHEC tenant (*.ghe.com), returns the tenant hostname,
Expand Down Expand Up @@ -1926,6 +1950,39 @@ program
// Log CLI proxy status
emitCliProxyStatusLogs(config, logger.info.bind(logger), logger.warn.bind(logger));

// Warn if a classic PAT is combined with COPILOT_MODEL (Copilot CLI 1.0.21+ incompatibility)
const hasCopilotModelInEnvFiles = (envFile: unknown): boolean => {
const envFiles = Array.isArray(envFile) ? envFile : envFile ? [envFile] : [];
for (const candidate of envFiles) {
if (typeof candidate !== 'string' || candidate.trim() === '') continue;
try {
const envFilePath = path.isAbsolute(candidate) ? candidate : path.resolve(process.cwd(), candidate);
const envFileContents = fs.readFileSync(envFilePath, 'utf8');
for (const line of envFileContents.split(/\r?\n/)) {
const trimmedLine = line.trim();
if (!trimmedLine || trimmedLine.startsWith('#')) continue;
if (/^(?:export\s+)?COPILOT_MODEL\s*=/.test(trimmedLine)) {
return true;
}
}
} catch {
// Ignore unreadable env files here; this check is only for a pre-flight warning.
}
}
return false;
};

// Warn if a classic PAT is combined with COPILOT_MODEL (Copilot CLI 1.0.21+ incompatibility)
// Check if COPILOT_MODEL is set via --env/-e flags, host env (when --env-all is active), or --env-file
const copilotModelFromFlags = !!(additionalEnv['COPILOT_MODEL']);
const copilotModelInHostEnv = !!(config.envAll && process.env.COPILOT_MODEL);
const copilotModelInEnvFile = hasCopilotModelInEnvFiles((config as { envFile?: unknown }).envFile);
warnClassicPATWithCopilotModel(
config.copilotGithubToken?.startsWith('ghp_') ?? false,
copilotModelFromFlags || copilotModelInHostEnv || copilotModelInEnvFile,
logger.warn.bind(logger)
);

// Log config with redacted secrets - remove API keys entirely
// to prevent sensitive data from flowing to logger (CodeQL sensitive data logging)
const redactedConfig: Record<string, unknown> = {};
Expand Down
Loading