feat: add API proxy sidecar for secure LLM credential management #751
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Claude
AI
changed the title
|
@copilot can you make sure that api proxying is on by default? |
API proxy is now enabled by default. The The sidecar will only be deployed when Commit: 4e1c255 |
There was a problem hiding this comment.
Pull request overview
Adds an optional Envoy-based API proxy sidecar to keep LLM API credentials out of the agent container, integrating it into the AWF Docker Compose setup and documenting usage.
Changes:
- Introduces
--enable-api-proxyand config fields for enabling the sidecar and supplying OpenAI/Anthropic API keys. - Extends Docker Compose generation to optionally run an
api-proxyservice and set provider base URLs in the agent container. - Adds documentation for the sidecar feature and updates the main README to reference it.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Adds enableApiProxy, openaiApiKey, anthropicApiKey config fields + docs. |
| src/docker-manager.ts | Adds optional api-proxy service to compose output and agent env/depends_on wiring. |
| src/cli.ts | Adds --enable-api-proxy flag and reads API keys from environment into config. |
| docs/api-proxy-sidecar.md | New user-facing documentation for enabling and using the sidecar. |
| containers/envoy/entrypoint.sh | Generates Envoy config dynamically and injects auth headers. |
| containers/envoy/README.md | Container-level documentation for the Envoy sidecar. |
| containers/envoy/Dockerfile | Builds the Envoy sidecar image and installs curl for healthchecks. |
| README.md | Links to new sidecar documentation and advertises the feature. |
Comments suppressed due to low confidence (2)
src/cli.ts:1003
redactedConfigis logged at debug level with...config, butopenaiApiKey/anthropicApiKeyare included unredacted. This can leak API keys any time the user has these env vars set (even if--enable-api-proxyis false). Redact these fields explicitly (e.g., replace with '[REDACTED]') before logging, or omit them from the logged config entirely.
// Log config with redacted secrets
const redactedConfig = {
...config,
agentCommand: redactSecrets(config.agentCommand),
};
logger.debug('Configuration:', JSON.stringify(redactedConfig, null, 2));
containers/envoy/entrypoint.sh:163
- The entrypoint logs the first 8 characters of each API key. Even partial key material can be sensitive and can end up in CI logs or log aggregation. Remove these log lines (or log only that a key is present without including any characters).
echo "[INFO] Generated Envoy configuration"
if [ -n "$OPENAI_API_KEY" ]; then
echo "[INFO] OpenAI API key configured (first 8 chars: ${OPENAI_API_KEY:0:8}...)"
fi
if [ -n "$ANTHROPIC_API_KEY" ]; then
echo "[INFO] Anthropic API key configured (first 8 chars: ${ANTHROPIC_API_KEY:0:8}...)"
fi
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docs/api-proxy-sidecar.md
Outdated
| The sidecar sets these environment variables in the agent container: | ||
|
|
||
| | Variable | Value | Description | | ||
| |----------|-------|-------------| | ||
| | `OPENAI_BASE_URL` | `http://api-proxy:10000` | OpenAI API proxy endpoint | | ||
| | `ANTHROPIC_BASE_URL` | `http://api-proxy:10001` | Anthropic API proxy endpoint | | ||
|
|
There was a problem hiding this comment.
The docs present OPENAI_BASE_URL and ANTHROPIC_BASE_URL as always set when the sidecar is enabled, but the current code only sets each variable when the corresponding API key is present. Either document this as conditional behavior or set both base URLs whenever --enable-api-proxy is enabled (and enforce key presence separately).
There was a problem hiding this comment.
Fixed - documentation now clarifies that OPENAI_BASE_URL is only set when OPENAI_API_KEY is provided, and ANTHROPIC_BASE_URL is only set when ANTHROPIC_API_KEY is provided. Updated both docs/api-proxy-sidecar.md and src/types.ts. Commit: d918a43
| healthcheck: { | ||
| test: ['CMD', 'curl', '-f', 'http://localhost:10000/'], | ||
| interval: '5s', | ||
| timeout: '3s', | ||
| retries: 5, | ||
| start_period: '5s', | ||
| }, |
There was a problem hiding this comment.
The API proxy sidecar healthcheck uses curl -f http://localhost:10000/, which proxies to api.openai.com and will fail on non-2xx responses (likely 404) and also depends on external network access / domain allowlisting. This makes container readiness brittle and can block agent startup via depends_on: service_healthy. Prefer checking Envoy's admin readiness endpoint (e.g. http://localhost:9901/ready) or add a dedicated static health listener that always returns 200.
There was a problem hiding this comment.
Fixed - healthcheck now uses Envoy's admin readiness endpoint at http://localhost:9901/ready instead of proxying to external APIs. This makes container startup reliable and independent of external network access. Commit: d918a43
containers/envoy/entrypoint.sh
Outdated
| clusters: | ||
| # OpenAI API cluster | ||
| - name: openai_cluster | ||
| type: LOGICAL_DNS | ||
| dns_lookup_family: V4_ONLY | ||
| load_assignment: | ||
| cluster_name: openai_cluster | ||
| endpoints: | ||
| - lb_endpoints: | ||
| - endpoint: | ||
| address: | ||
| socket_address: | ||
| address: api.openai.com | ||
| port_value: 443 | ||
| transport_socket: | ||
| name: envoy.transport_sockets.tls | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext | ||
| sni: api.openai.com | ||
| typed_extension_protocol_options: | ||
| envoy.extensions.upstreams.http.v3.HttpProtocolOptions: | ||
| "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions | ||
| explicit_http_config: | ||
| http2_protocol_options: {} | ||
|
|
||
| # Anthropic API cluster | ||
| - name: anthropic_cluster | ||
| type: LOGICAL_DNS | ||
| dns_lookup_family: V4_ONLY | ||
| load_assignment: | ||
| cluster_name: anthropic_cluster | ||
| endpoints: | ||
| - lb_endpoints: | ||
| - endpoint: | ||
| address: | ||
| socket_address: | ||
| address: api.anthropic.com | ||
| port_value: 443 | ||
| transport_socket: | ||
| name: envoy.transport_sockets.tls | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext | ||
| sni: api.anthropic.com | ||
| typed_extension_protocol_options: | ||
| envoy.extensions.upstreams.http.v3.HttpProtocolOptions: | ||
| "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions | ||
| explicit_http_config: | ||
| http2_protocol_options: {} | ||
|
|
There was a problem hiding this comment.
The Envoy config routes directly to api.openai.com:443 / api.anthropic.com:443. In this repo, host iptables rules only allow unrestricted outbound traffic from the Squid container; other containers are only allowed to connect to Squid (plus DNS), so the sidecar won't be able to reach these upstreams. Either (1) configure Envoy to tunnel via the Squid forward proxy, or (2) update host firewall rules to explicitly allow sidecar egress in a way that preserves the domain allowlist guarantees described in the PR.
There was a problem hiding this comment.
Implemented by adding a firewall exemption for the API proxy sidecar (172.30.0.30), similar to how Squid has unrestricted egress. This is more secure and simpler than routing through Squid, as it:
- Maintains separation of concerns (Squid for agent traffic, direct connection for API proxy)
- Avoids potential proxy chaining issues
- Preserves the domain allowlist guarantees (only api.openai.com and api.anthropic.com can be reached)
The exemption is conditional - only enabled when enableApiProxy is true. Commit: d918a43
containers/envoy/entrypoint.sh
Outdated
| - match: | ||
| prefix: "/" | ||
| route: | ||
| cluster: openai_cluster | ||
| timeout: 300s |
There was a problem hiding this comment.
Envoy forwards the downstream Host/:authority header by default. Clients calling http://api-proxy:10000 will typically send Host: api-proxy, which can cause OpenAI/Anthropic to reject or misroute the request. Configure the route to rewrite the host header per upstream (e.g., host_rewrite_literal: api.openai.com / api.anthropic.com, or auto_host_rewrite).
There was a problem hiding this comment.
Fixed - added host_rewrite_literal to both routes to ensure the correct Host header is sent to upstream APIs. OpenAI route rewrites to api.openai.com and Anthropic route rewrites to api.anthropic.com. Commit: d918a43
src/docker-manager.ts
Outdated
| cap_drop: [ | ||
| 'NET_RAW', | ||
| 'NET_ADMIN', | ||
| 'SYS_ADMIN', | ||
| 'SYS_PTRACE', | ||
| 'SYS_MODULE', | ||
| 'SYS_RAWIO', | ||
| 'MKNOD', | ||
| 'AUDIT_WRITE', | ||
| 'SETFCAP', | ||
| ], |
There was a problem hiding this comment.
This sidecar drops a handful of capabilities, but Docker still grants a default capability set. For a hardened proxy that doesn't need any extra privileges, consider cap_drop: ['ALL'] (and only cap_add what is strictly required) so the comment about dropping unnecessary capabilities matches the actual runtime privileges.
| cap_drop: [ | |
| 'NET_RAW', | |
| 'NET_ADMIN', | |
| 'SYS_ADMIN', | |
| 'SYS_PTRACE', | |
| 'SYS_MODULE', | |
| 'SYS_RAWIO', | |
| 'MKNOD', | |
| 'AUDIT_WRITE', | |
| 'SETFCAP', | |
| ], | |
| cap_drop: ['ALL'], |
There was a problem hiding this comment.
Fixed - changed from dropping specific capabilities to cap_drop: ['ALL'] for maximum security hardening. The sidecar doesn't need any special privileges. Commit: d918a43
src/docker-manager.ts
Outdated
| // Add Envoy API proxy sidecar if enabled | ||
| if (config.enableApiProxy && networkConfig.proxyIp) { | ||
| const proxyService: any = { | ||
| container_name: 'awf-api-proxy', | ||
| networks: { | ||
| 'awf-net': { | ||
| ipv4_address: networkConfig.proxyIp, | ||
| }, | ||
| }, | ||
| environment: { | ||
| // Pass API keys securely to sidecar (not visible to agent) | ||
| ...(config.openaiApiKey && { OPENAI_API_KEY: config.openaiApiKey }), | ||
| ...(config.anthropicApiKey && { ANTHROPIC_API_KEY: config.anthropicApiKey }), | ||
| }, | ||
| healthcheck: { | ||
| test: ['CMD', 'curl', '-f', 'http://localhost:10000/'], | ||
| interval: '5s', | ||
| timeout: '3s', | ||
| retries: 5, | ||
| start_period: '5s', | ||
| }, | ||
| // Security hardening: Drop all unnecessary capabilities | ||
| cap_drop: [ | ||
| 'NET_RAW', | ||
| 'NET_ADMIN', | ||
| 'SYS_ADMIN', | ||
| 'SYS_PTRACE', | ||
| 'SYS_MODULE', | ||
| 'SYS_RAWIO', | ||
| 'MKNOD', | ||
| 'AUDIT_WRITE', | ||
| 'SETFCAP', | ||
| ], | ||
| security_opt: [ | ||
| 'no-new-privileges:true', | ||
| ], | ||
| // Resource limits to prevent DoS attacks | ||
| mem_limit: '512m', | ||
| memswap_limit: '512m', | ||
| pids_limit: 100, | ||
| cpu_shares: 512, | ||
| }; | ||
|
|
||
| // Use GHCR image or build locally | ||
| if (useGHCR) { | ||
| proxyService.image = `${registry}/envoy:${tag}`; | ||
| } else { | ||
| proxyService.build = { | ||
| context: path.join(projectRoot, 'containers/envoy'), | ||
| dockerfile: 'Dockerfile', | ||
| }; | ||
| } | ||
|
|
||
| services['api-proxy'] = proxyService; | ||
|
|
||
| // Update agent dependencies to wait for api-proxy | ||
| agentService.depends_on['api-proxy'] = { | ||
| condition: 'service_healthy', | ||
| }; | ||
|
|
||
| // Set environment variables in agent to use the proxy | ||
| if (config.openaiApiKey) { | ||
| environment.OPENAI_BASE_URL = `http://api-proxy:10000`; | ||
| logger.debug('OpenAI API will be proxied through sidecar at http://api-proxy:10000'); | ||
| } | ||
| if (config.anthropicApiKey) { | ||
| environment.ANTHROPIC_BASE_URL = `http://api-proxy:10001`; | ||
| logger.debug('Anthropic API will be proxied through sidecar at http://api-proxy:10001'); | ||
| } | ||
|
|
||
| logger.info('API proxy sidecar enabled - API keys will be held securely in sidecar container'); | ||
| } |
There was a problem hiding this comment.
enableApiProxy introduces new branching behavior in generateDockerCompose, but there are no unit tests covering the sidecar service generation (service presence, env injection, depends_on update, etc.). Add tests in src/docker-manager.test.ts for the enabled/disabled cases (including one-key vs two-key scenarios) to prevent regressions.
There was a problem hiding this comment.
Added 19 comprehensive unit tests for the enableApiProxy feature covering:
- Service presence/absence based on config
- Environment variable injection (one-key vs two-key scenarios)
- GHCR vs local build configurations
- Healthcheck, security hardening, and resource limits
- Agent depends_on updates
- Conditional BASE_URL environment variables
All tests pass successfully. Commit: d8de028
containers/envoy/entrypoint.sh
Outdated
| # Generate Envoy configuration from environment variables | ||
| # This allows API keys to be injected at runtime without persisting to disk | ||
|
|
There was a problem hiding this comment.
The script comment says the API keys are injected “without persisting to disk”, but the keys are written into /etc/envoy/envoy.yaml (and also end up in docker-compose.yml via the service environment). Update the comment and/or mount the config path as tmpfs and avoid writing secrets into a file if the intent is truly no-on-disk persistence.
This issue also appears on line 157 of the same file.
There was a problem hiding this comment.
Fixed - updated the entrypoint.sh comments to clarify that keys are written to /etc/envoy/envoy.yaml and are present in docker-compose.yml service environment. The comment now accurately reflects reality instead of claiming no disk persistence. Commit: d918a43
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.26% | 81.53% | 📉 -0.73% |
| Statements | 82.31% | 81.60% | 📉 -0.71% |
| Functions | 82.14% | 82.14% | ➡️ +0.00% |
| Branches | 74.70% | 73.24% | 📉 -1.46% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/docker-manager.ts |
83.5% → 81.9% (-1.62%) | 83.0% → 81.4% (-1.56%) |
src/cli.ts |
41.7% → 40.7% (-1.02%) | 41.9% → 40.9% (-1.02%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Status: PASS
|
Bun Build Test Results
Overall: PASS All Bun projects built and tested successfully.
|
Smoke Test Results (Run #21957514742)✅ GitHub MCP: Last 2 merged PRs:
✅ Playwright: GitHub.com loaded successfully (title verified) Status: PASS ✅ cc
|
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
|
Chroot tests failed Smoke Chroot was cancelled - See logs for details. |
Prevent clear-text logging of OPENAI_API_KEY and ANTHROPIC_API_KEY in the debug configuration output. CodeQL flagged this as a high severity security issue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
|
Smoke Test Results: Claude Engine ✅ GitHub MCP: Retrieved last 2 merged PRs
✅ Playwright: Navigated to https://github.com, confirmed title "GitHub · Change is constant. GitHub keeps you ahead. · GitHub" ✅ File Writing: Created test file ✅ Bash Tool: Verified file contents match expected output Overall Status: PASS ✅
|
.NET Build Test Results
Overall: PASS ✅ All .NET projects successfully restored, built, and executed.
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Status: PASS cc
|
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
Smoke Test Results - PASS ✅Last 2 Merged PRs:
Tests:
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed tests.
|
Bun Build Test Results
Overall: PASS ✅ All Bun projects installed successfully and tests passed.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
Security Review: API Proxy Sidecar Feature
|
Build Test: Node.js - Results
Overall: ✅ PASS All Node.js test projects successfully installed dependencies and passed their test suites.
|
.NET Build Test Results
Overall: PASS ✅ All .NET projects successfully restored, built, and ran with expected output.
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall: PASS
|
Build Test: Rust ✅
Overall: PASS All Rust projects built and tested successfully.
|
✅ Java Build Test ResultsAll Java projects compiled and tested successfully through the AWF firewall.
Overall: PASS All projects successfully downloaded dependencies through the Squid proxy (172.30.0.10:3128) and executed tests without errors.
|
Chroot Version Comparison Test Results ❌The chroot mode test detected version mismatches between host and chroot environments:
Overall Result: FAILED The chroot environment should use the same runtime versions as the host for transparent binary access. Python and Node.js versions differ, which may cause compatibility issues.
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
|
🔮 The ancient spirits stir... Smoke Codex awakens to divine this pull request... |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.