Skip to content

Commit 72ff1cb

Browse files
committed
Merge branch 'main' into copilot/fix-dind-configuration-issue
2 parents 8f748ec + 218a001 commit 72ff1cb

8 files changed

Lines changed: 249 additions & 42 deletions

File tree

.github/workflows/security-guard.lock.yml

Lines changed: 27 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/security-guard.md

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ permissions:
1111
issues: read
1212
engine:
1313
id: claude
14-
max-turns: 25
14+
max-turns: 8
1515
tools:
1616
github:
1717
toolsets: [pull_requests, repos]
@@ -23,7 +23,7 @@ safe-outputs:
2323
enabled: false
2424
add-comment:
2525
max: 1
26-
timeout-minutes: 10
26+
timeout-minutes: 15
2727
steps:
2828
- name: Fetch PR changed files
2929
id: pr-diff
@@ -42,14 +42,34 @@ steps:
4242
GH_TOKEN: ${{ github.token }}
4343
PR_NUMBER: ${{ github.event.pull_request.number }}
4444
GH_REPO: ${{ github.repository }}
45+
46+
- name: Check security relevance
47+
id: security-relevance
48+
if: github.event.pull_request.number
49+
run: |
50+
SECURITY_RE="host-iptables|setup-iptables|squid-config|docker-manager|seccomp-profile|domain-patterns|entrypoint\.sh|Dockerfile|containers/"
51+
COUNT=$(gh api "repos/${GH_REPO}/pulls/${PR_NUMBER}/files" \
52+
--paginate --jq '.[].filename' \
53+
| grep -cE "$SECURITY_RE" || true)
54+
echo "security_files_changed=$COUNT" >> "$GITHUB_OUTPUT"
55+
env:
56+
GH_TOKEN: ${{ github.token }}
57+
PR_NUMBER: ${{ github.event.pull_request.number }}
58+
GH_REPO: ${{ github.repository }}
4559
---
4660

4761
# Security Guard
4862

49-
You are a security-focused AI agent that carefully reviews pull requests in this repository to identify changes that could weaken the security posture or extend the security boundaries of the Agentic Workflow Firewall (AWF).
63+
## Security Relevance Check
64+
65+
**Security-critical files changed in this PR:** ${{ steps.security-relevance.outputs.security_files_changed }}
66+
67+
> If this value is `0`, no security-critical files were modified. Use `noop` immediately without further analysis — this PR does not require a security review.
5068
5169
## Repository Context
5270

71+
You are a security-focused AI agent that carefully reviews pull requests in this repository to identify changes that could weaken the security posture or extend the security boundaries of the Agentic Workflow Firewall (AWF).
72+
5373
This repository implements a **network firewall for AI agents** that provides L7 (HTTP/HTTPS) egress control using Squid proxy and Docker containers. The firewall restricts network access to a whitelist of approved domains.
5474

5575
### Critical Security Components
@@ -134,6 +154,8 @@ Look for these types of security-weakening changes:
134154

135155
## Output Format
136156

157+
**IMPORTANT: Be concise.** Report each security finding in ≤ 150 words. Maximum 5 findings total.
158+
137159
If you find security concerns:
138160
1. Add a comment to the PR explaining each concern
139161
2. For each issue, provide:

containers/api-proxy/server.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,31 @@ if (require.main === module) {
888888
const contentLength = parseInt(req.headers['content-length'], 10) || 0;
889889
if (checkRateLimit(req, res, 'copilot', contentLength)) return;
890890

891+
// Copilot CLI 1.0.21+ calls GET /models at startup (to list or validate models).
892+
// The /models endpoint lives on the Copilot inference API (COPILOT_API_TARGET),
893+
// NOT on the GitHub REST API. Explicitly use COPILOT_GITHUB_TOKEN for this
894+
// request so the GitHub OAuth token is used even when both COPILOT_GITHUB_TOKEN
895+
// and COPILOT_API_KEY are configured (COPILOT_API_KEY alone is not accepted by
896+
// the /models endpoint).
897+
let reqPathname;
898+
try {
899+
reqPathname = new URL(req.url, 'http://localhost').pathname;
900+
} catch {
901+
logRequest('warn', 'copilot_proxy_malformed_url', {
902+
message: 'Malformed request URL in Copilot proxy — rejecting with 400',
903+
});
904+
res.writeHead(400, { 'Content-Type': 'application/json' });
905+
res.end(JSON.stringify({ error: 'Invalid request URL' }));
906+
return;
907+
}
908+
const isModelsPath = reqPathname === '/models' || reqPathname.startsWith('/models/');
909+
if (isModelsPath && req.method === 'GET' && COPILOT_GITHUB_TOKEN) {
910+
proxyRequest(req, res, COPILOT_API_TARGET, {
911+
'Authorization': `Bearer ${COPILOT_GITHUB_TOKEN}`,
912+
}, 'copilot');
913+
return;
914+
}
915+
891916
proxyRequest(req, res, COPILOT_API_TARGET, {
892917
'Authorization': `Bearer ${COPILOT_AUTH_TOKEN}`,
893918
}, 'copilot');

docs/api-proxy-sidecar.md

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ The API proxy sidecar receives **real credentials** and routing configuration:
124124
| `ANTHROPIC_API_KEY` | Real API key | `--enable-api-proxy` and env set | Anthropic API key (injected into requests) |
125125
| `COPILOT_GITHUB_TOKEN` | Real token | `--enable-api-proxy` and env set | GitHub Copilot token (injected into requests) |
126126
| `COPILOT_API_KEY` | Real API key | `--enable-api-proxy` and env set | GitHub Copilot BYOK key (injected into requests) |
127+
| `GEMINI_API_KEY` | Real API key | `--enable-api-proxy` and env set | Google Gemini API key (injected into requests) |
127128
| `HTTP_PROXY` | `http://172.30.0.10:3128` | Always | Routes through Squid for domain filtering |
128129
| `HTTPS_PROXY` | `http://172.30.0.10:3128` | Always | Routes through Squid for domain filtering |
129130

@@ -148,6 +149,8 @@ The agent container receives **redacted placeholders** and proxy URLs:
148149
| `COPILOT_OFFLINE` | `true` | `COPILOT_API_KEY` provided to host | Enables offline+BYOK mode (skips GitHub OAuth handshake) |
149150
| `COPILOT_PROVIDER_BASE_URL` | `http://172.30.0.30:10002` | `COPILOT_API_KEY` provided to host | Points Copilot CLI BYOK provider at sidecar |
150151
| `COPILOT_PROVIDER_API_KEY` | `placeholder-token-for-credential-isolation` | `COPILOT_API_KEY` provided to host | BYOK provider API key placeholder (real key in sidecar) |
152+
| `GEMINI_API_BASE_URL` | `http://172.30.0.30:10003` | `--enable-api-proxy` always | Redirects Gemini CLI to proxy (set unconditionally — see note below) |
153+
| `GEMINI_API_KEY` | `gemini-api-key-placeholder-for-credential-isolation` | `--enable-api-proxy` always | Placeholder so Gemini CLI auth check passes (real key in sidecar) |
151154
| `OPENAI_API_KEY` | Not set | `--enable-api-proxy` | Excluded from agent (held in api-proxy) |
152155
| `ANTHROPIC_API_KEY` | Not set | `--enable-api-proxy` | Excluded from agent (held in api-proxy) |
153156
| `HTTP_PROXY` | `http://172.30.0.10:3128` | Always | Routes through Squid proxy |
@@ -156,6 +159,14 @@ The agent container receives **redacted placeholders** and proxy URLs:
156159
| `AWF_API_PROXY_IP` | `172.30.0.30` | `--enable-api-proxy` | Used by iptables setup script |
157160
| `AWF_ONE_SHOT_TOKENS` | `COPILOT_GITHUB_TOKEN,GITHUB_TOKEN,...` | Always | Tokens protected by one-shot-token library |
158161

162+
:::note[Gemini always redirected to proxy]
163+
Unlike OpenAI, Anthropic, and Copilot, `GEMINI_API_BASE_URL` and the `GEMINI_API_KEY` placeholder are **always** set in the agent when `--enable-api-proxy` is active, regardless of whether `GEMINI_API_KEY` is present in the runner environment.
164+
165+
This prevents the Gemini CLI from failing with exit code 41 ("no auth method") when the real API key is only available as a GitHub Actions secret (not as a runner-level environment variable). In that case the api-proxy sidecar will return `503` for Gemini requests — a clear, actionable failure rather than a confusing missing-auth error.
166+
167+
**Important**: `GEMINI_API_KEY` must be set as a **runner-level environment variable** (e.g. `env: GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}` in the workflow step), not only as a GitHub Actions secret. The AWF process running on the runner must be able to read it so it can pass the key to the api-proxy sidecar container.
168+
:::
169+
159170
:::tip[Placeholder tokens]
160171
Token variables in the agent are set to `placeholder-token-for-credential-isolation` instead of real values. This ensures:
161172
- Agent code cannot exfiltrate credentials
@@ -262,6 +273,24 @@ sudo awf --enable-api-proxy [OPTIONS] -- COMMAND
262273
**Required environment variables** (at least one):
263274
- `OPENAI_API_KEY` — OpenAI API key
264275
- `ANTHROPIC_API_KEY` — Anthropic API key
276+
- `GEMINI_API_KEY` — Google Gemini API key
277+
- `COPILOT_GITHUB_TOKEN` — GitHub Copilot access token
278+
- `COPILOT_API_KEY` — GitHub Copilot API key (BYOK)
279+
280+
:::caution[GitHub Actions: expose keys as runner env vars]
281+
When running AWF in a GitHub Actions workflow, API keys must be available as **runner-level environment variables** — not just as GitHub Actions secrets. AWF reads the key from the environment at startup to pass it to the api-proxy sidecar container. Use `env:` in the workflow step and `sudo --preserve-env` to ensure keys pass through:
282+
283+
```yaml
284+
- name: Run agent
285+
env:
286+
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}
287+
run: sudo --preserve-env=GEMINI_API_KEY awf --enable-api-proxy ...
288+
```
289+
290+
> **Note:** `sudo` strips most environment variables by default. Use `--preserve-env=VAR` (or `sudo -E` to preserve all) to ensure API keys are visible to the AWF process.
291+
292+
If the key is present only in `secrets.*` but not exported into the step's `env:`, AWF will warn that no Gemini key was found and the api-proxy Gemini listener will return `503`.
293+
:::
265294

266295
**Recommended domain whitelist**:
267296
- `api.openai.com` — for OpenAI/Codex
@@ -283,7 +312,7 @@ The sidecar container:
283312
- **Image**: `ghcr.io/github/gh-aw-firewall/api-proxy:latest`
284313
- **Base**: `node:22-alpine`
285314
- **Network**: `awf-net` at `172.30.0.30`
286-
- **Ports**: 10000 (OpenAI), 10001 (Anthropic), 10002 (GitHub Copilot)
315+
- **Ports**: 10000 (OpenAI), 10001 (Anthropic), 10002 (GitHub Copilot), 10003 (Google Gemini)
287316
- **Proxy**: Routes via Squid at `http://172.30.0.10:3128`
288317

289318
### Health check
@@ -296,14 +325,33 @@ Docker healthcheck on the `/health` endpoint (port 10000):
296325

297326
## Troubleshooting
298327

328+
### Gemini proxy returns 503
329+
330+
When `--enable-api-proxy` is active, `GEMINI_API_BASE_URL` and a placeholder `GEMINI_API_KEY` are always injected into the agent container. If the real `GEMINI_API_KEY` was not set in the AWF runner environment, the api-proxy Gemini listener (port 10003) responds with **503** to all requests.
331+
332+
**Solution**: Export `GEMINI_API_KEY` in the runner environment before invoking AWF. In GitHub Actions, add it to the step's `env:` block and use `sudo --preserve-env`:
333+
334+
```yaml
335+
- name: Run Gemini agent
336+
env:
337+
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}
338+
run: |
339+
sudo --preserve-env=GEMINI_API_KEY \
340+
awf --enable-api-proxy \
341+
--allow-domains generativelanguage.googleapis.com \
342+
-- gemini ...
343+
```
344+
345+
> **Note:** Exit code 41 ("no auth method") should no longer occur with `--enable-api-proxy` since the placeholder key satisfies the CLI's pre-flight check. If you see exit 41, ensure `--enable-api-proxy` is active.
346+
299347
### API keys not detected
300348

301349
```
302350
⚠️ API proxy enabled but no API keys found in environment
303-
Set OPENAI_API_KEY or ANTHROPIC_API_KEY to use the proxy
351+
Set OPENAI_API_KEY, ANTHROPIC_API_KEY, GEMINI_API_KEY, COPILOT_GITHUB_TOKEN, or COPILOT_API_KEY to use the proxy
304352
```
305353
306-
**Solution**: Export API keys before running awf:
354+
**Solution**: Export API keys before running awf (use `sudo --preserve-env` in CI):
307355
308356
```bash
309357
export OPENAI_API_KEY="sk-..."
@@ -343,9 +391,8 @@ docker exec awf-squid cat /var/log/squid/access.log | grep DENIED
343391

344392
## Limitations
345393

346-
- Only supports OpenAI and Anthropic APIs
347394
- Keys must be set as environment variables (not file-based)
348-
- No support for Azure OpenAI endpoints
395+
- No support for Azure OpenAI endpoints (use `--openai-api-target` for custom endpoints)
349396
- No request/response logging (by design, for security)
350397

351398
## Related documentation

src/cli-workflow.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,4 +310,67 @@ describe('runMainWorkflow', () => {
310310

311311
await expect(runMainWorkflow(configWithDiagnostics, dependencies, { logger, performCleanup: jest.fn() })).resolves.toBe(1);
312312
});
313+
314+
it('calls collectDiagnosticLogs on startContainers failure when diagnosticLogs is enabled', async () => {
315+
const startError = new Error('Squid container is unhealthy');
316+
const collectDiagnosticLogs = jest.fn().mockResolvedValue(undefined);
317+
const configWithDiagnostics: WrapperConfig = {
318+
...baseConfig,
319+
diagnosticLogs: true,
320+
};
321+
const dependencies: WorkflowDependencies = {
322+
ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }),
323+
setupHostIptables: jest.fn().mockResolvedValue(undefined),
324+
writeConfigs: jest.fn().mockResolvedValue(undefined),
325+
startContainers: jest.fn().mockRejectedValue(startError),
326+
runAgentCommand: jest.fn(),
327+
collectDiagnosticLogs,
328+
};
329+
const logger = createLogger();
330+
331+
await expect(runMainWorkflow(configWithDiagnostics, dependencies, { logger, performCleanup: jest.fn() })).rejects.toBe(startError);
332+
333+
expect(collectDiagnosticLogs).toHaveBeenCalledWith(configWithDiagnostics.workDir);
334+
expect(dependencies.runAgentCommand).not.toHaveBeenCalled();
335+
});
336+
337+
it('does not call collectDiagnosticLogs on startContainers failure when diagnosticLogs is disabled', async () => {
338+
const startError = new Error('Squid container is unhealthy');
339+
const collectDiagnosticLogs = jest.fn().mockResolvedValue(undefined);
340+
const dependencies: WorkflowDependencies = {
341+
ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }),
342+
setupHostIptables: jest.fn().mockResolvedValue(undefined),
343+
writeConfigs: jest.fn().mockResolvedValue(undefined),
344+
startContainers: jest.fn().mockRejectedValue(startError),
345+
runAgentCommand: jest.fn(),
346+
collectDiagnosticLogs,
347+
};
348+
const logger = createLogger();
349+
350+
await expect(runMainWorkflow(baseConfig, dependencies, { logger, performCleanup: jest.fn() })).rejects.toBe(startError);
351+
352+
expect(collectDiagnosticLogs).not.toHaveBeenCalled();
353+
});
354+
355+
it('rethrows startContainers error after collecting diagnostics', async () => {
356+
const startError = new Error('docker compose failed');
357+
const configWithDiagnostics: WrapperConfig = {
358+
...baseConfig,
359+
diagnosticLogs: true,
360+
};
361+
const performCleanup = jest.fn().mockResolvedValue(undefined);
362+
const dependencies: WorkflowDependencies = {
363+
ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }),
364+
setupHostIptables: jest.fn().mockResolvedValue(undefined),
365+
writeConfigs: jest.fn().mockResolvedValue(undefined),
366+
startContainers: jest.fn().mockRejectedValue(startError),
367+
runAgentCommand: jest.fn(),
368+
collectDiagnosticLogs: jest.fn().mockResolvedValue(undefined),
369+
};
370+
const logger = createLogger();
371+
372+
await expect(runMainWorkflow(configWithDiagnostics, dependencies, { logger, performCleanup })).rejects.toBe(startError);
373+
// performCleanup should NOT be called — that is the caller's (cli.ts) responsibility
374+
expect(performCleanup).not.toHaveBeenCalled();
375+
});
313376
});

src/cli-workflow.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,25 @@ export async function runMainWorkflow(
7171
await dependencies.writeConfigs(config);
7272

7373
// Step 2: Start containers
74-
await dependencies.startContainers(config.workDir, config.allowedDomains, config.proxyLogsDir, config.skipPull);
74+
try {
75+
await dependencies.startContainers(config.workDir, config.allowedDomains, config.proxyLogsDir, config.skipPull);
76+
} catch (startError) {
77+
// Signal that containers may have been partially created so the caller's
78+
// cleanup (stopContainers / docker compose down -v) will tear them down
79+
// instead of leaving orphaned containers and networks.
80+
onContainersStarted?.();
81+
82+
// Collect diagnostics for startup failures before containers are torn down.
83+
// Must happen before performCleanup() / stopContainers() destroys them.
84+
if (config.diagnosticLogs && dependencies.collectDiagnosticLogs) {
85+
try {
86+
await dependencies.collectDiagnosticLogs(config.workDir);
87+
} catch (diagError) {
88+
logger.warn('Failed to collect diagnostic logs; continuing with cleanup.', diagError);
89+
}
90+
}
91+
throw startError;
92+
}
7593
onContainersStarted?.();
7694

7795
// Step 3: Wait for agent to complete

src/docker-manager.test.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2763,12 +2763,24 @@ describe('docker-manager', () => {
27632763
expect(env.GEMINI_API_KEY).toBe('gemini-api-key-placeholder-for-credential-isolation');
27642764
});
27652765

2766-
it('should not set GEMINI_API_BASE_URL in agent when geminiApiKey is not provided', () => {
2766+
it('should always set GEMINI_API_BASE_URL in agent when api-proxy is enabled (regardless of geminiApiKey)', () => {
27672767
const configWithProxy = { ...mockConfig, enableApiProxy: true, openaiApiKey: 'sk-test-key' };
27682768
const result = generateDockerCompose(configWithProxy, mockNetworkConfigWithProxy);
27692769
const agent = result.services.agent;
27702770
const env = agent.environment as Record<string, string>;
2771-
expect(env.GEMINI_API_BASE_URL).toBeUndefined();
2771+
// GEMINI_API_BASE_URL must be set even without a geminiApiKey so that the
2772+
// Gemini CLI does not fail with exit code 41 ("no auth method") when the
2773+
// GEMINI_API_KEY is only available as a GitHub Actions secret.
2774+
expect(env.GEMINI_API_BASE_URL).toBe('http://172.30.0.30:10003');
2775+
});
2776+
2777+
it('should set GEMINI_API_KEY placeholder in agent when api-proxy is enabled without geminiApiKey', () => {
2778+
const configWithProxy = { ...mockConfig, enableApiProxy: true, openaiApiKey: 'sk-test-key' };
2779+
const result = generateDockerCompose(configWithProxy, mockNetworkConfigWithProxy);
2780+
const agent = result.services.agent;
2781+
const env = agent.environment as Record<string, string>;
2782+
// Placeholder is required so Gemini CLI's startup auth check passes (exit code 41).
2783+
expect(env.GEMINI_API_KEY).toBe('gemini-api-key-placeholder-for-credential-isolation');
27722784
});
27732785

27742786
it('should not leak GEMINI_API_KEY to agent when api-proxy is enabled', () => {

0 commit comments

Comments
 (0)