Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 3 additions & 3 deletions src/docker-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -529,9 +529,9 @@ describe('docker-manager', () => {
// Should NOT include blanket /:/host:rw mount
expect(volumes).not.toContain('/:/host:rw');

// Should include custom mounts
expect(volumes).toContain('/workspace:/workspace:ro');
expect(volumes).toContain('/data:/data:rw');
// Should include custom mounts (prefixed with /host for chroot visibility)
expect(volumes).toContain('/workspace:/host/workspace:ro');
expect(volumes).toContain('/data:/host/data:rw');

// Should still include essential mounts
expect(volumes).toContain('/tmp:/tmp:rw');
Expand Down
30 changes: 28 additions & 2 deletions src/docker-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,9 @@ export function generateDockerCompose(
if (process.env.OPENAI_API_KEY && !config.enableApiProxy) environment.OPENAI_API_KEY = process.env.OPENAI_API_KEY;
if (process.env.CODEX_API_KEY && !config.enableApiProxy) environment.CODEX_API_KEY = process.env.CODEX_API_KEY;
if (process.env.ANTHROPIC_API_KEY && !config.enableApiProxy) environment.ANTHROPIC_API_KEY = process.env.ANTHROPIC_API_KEY;
// COPILOT_GITHUB_TOKEN is handled separately - gets placeholder when api-proxy enabled
// COPILOT_GITHUB_TOKEN — forward when api-proxy is NOT enabled; when api-proxy IS enabled,
// it gets a placeholder value set earlier (line ~362) for credential isolation
if (process.env.COPILOT_GITHUB_TOKEN && !config.enableApiProxy) environment.COPILOT_GITHUB_TOKEN = process.env.COPILOT_GITHUB_TOKEN;
if (process.env.USER) environment.USER = process.env.USER;
if (process.env.TERM) environment.TERM = process.env.TERM;
if (process.env.XDG_CONFIG_HOME) environment.XDG_CONFIG_HOME = process.env.XDG_CONFIG_HOME;
Expand All @@ -449,6 +451,11 @@ export function generateDockerCompose(
if (process.env.GITHUB_API_URL) environment.GITHUB_API_URL = process.env.GITHUB_API_URL;
}

// Forward one-shot-token debug flag if set (used for testing/debugging)
if (process.env.AWF_ONE_SHOT_TOKEN_DEBUG) {
environment.AWF_ONE_SHOT_TOKEN_DEBUG = process.env.AWF_ONE_SHOT_TOKEN_DEBUG;
}

// Additional environment variables from --env flags (these override everything)
if (config.additionalEnv) {
Object.assign(environment, config.additionalEnv);
Expand Down Expand Up @@ -741,10 +748,29 @@ export function generateDockerCompose(
// ================================================================

// Add custom volume mounts if specified
// In chroot mode (always enabled), the container does `chroot /host`, so paths
// like /data become invisible. We need to prefix the container path with /host
// so that after chroot, /host/data becomes /data from the user's perspective.
if (config.volumeMounts && config.volumeMounts.length > 0) {
logger.debug(`Adding ${config.volumeMounts.length} custom volume mount(s)`);
config.volumeMounts.forEach(mount => {
agentVolumes.push(mount);
// Parse mount format: host_path:container_path[:mode]
const parts = mount.split(':');
if (parts.length >= 2) {
const hostPath = parts[0];
const containerPath = parts[1];
const mode = parts[2] || '';
// Prefix container path with /host for chroot visibility
const chrootContainerPath = `/host${containerPath}`;
const transformedMount = mode
? `${hostPath}:${chrootContainerPath}:${mode}`
: `${hostPath}:${chrootContainerPath}`;
logger.debug(`Adding custom volume mount: ${mount} -> ${transformedMount} (chroot-adjusted)`);
agentVolumes.push(transformedMount);
} else {
// Fallback: add as-is if format is unexpected
agentVolumes.push(mount);
}
});
}

Expand Down
87 changes: 87 additions & 0 deletions tests/fixtures/stdout-helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/**
* Helpers for extracting structured data from stdout that may contain
* Docker build output or other noise preceding the actual command output.
*
* When --build-local is used, Docker build logs are interleaved in stdout
* before the actual curl/command output. These helpers find the relevant
* data by scanning from the end of the output.
*/

/**
* Extract the last complete JSON object from a string that may contain
* non-JSON content (e.g., Docker build logs) before the JSON payload.
*
* Scans backwards from the end to find the last `}`, then walks backwards
* tracking brace depth to find the matching `{`.
*
* @param output - The full stdout string
* @returns The parsed JSON object, or null if no valid JSON object found
*/
export function extractLastJson(output: string): any | null {
// Find the last closing brace
const lastBrace = output.lastIndexOf('}');
if (lastBrace === -1) return null;

// Walk backwards from the last '}' to find the matching '{'
let depth = 0;
for (let i = lastBrace; i >= 0; i--) {
if (output[i] === '}') depth++;
if (output[i] === '{') depth--;
if (depth === 0) {
// Found the matching opening brace
const jsonStr = output.substring(i, lastBrace + 1);
try {
return JSON.parse(jsonStr);
} catch {
// This wasn't valid JSON, keep scanning
// (edge case: unbalanced braces in Docker build output)
continue;
}
}
}

return null;
}

/**
* Extract the last line(s) of stdout that look like actual command output
* (i.e., not Docker build step output).
*
* Docker build output lines typically start with patterns like:
* #1 [internal] load ...
* #5 DONE 0.1s
* => [1/5] FROM ...
*
* This returns everything after the last Docker build output line.
*
* @param output - The full stdout string
* @returns The cleaned output with Docker build noise removed
*/
export function extractCommandOutput(output: string): string {
const lines = output.split('\n');

// Find the last line that looks like Docker build output
let lastBuildLine = -1;
for (let i = 0; i < lines.length; i++) {
const line = lines[i].trim();
if (
line.startsWith('#') && /^#\d+/.test(line) || // Docker buildkit steps
line.startsWith('=>') || // Docker buildkit arrows
line.startsWith('DONE') ||
line.startsWith('CACHED') ||
line.match(/^\[[\d/]+\]/) || // Docker build step progress
line.startsWith('Sending build context') ||
line.startsWith('Successfully built') ||
line.startsWith('Successfully tagged')
) {
lastBuildLine = i;
}
}

if (lastBuildLine === -1) {
return output; // No Docker build output detected
}

// Return everything after the last build line
return lines.slice(lastBuildLine + 1).join('\n').trim();
}
22 changes: 14 additions & 8 deletions tests/integration/api-proxy-observability.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import { describe, test, expect, beforeAll, afterAll } from '@jest/globals';
import { createRunner, AwfRunner } from '../fixtures/awf-runner';
import { cleanup } from '../fixtures/cleanup';
import { extractLastJson, extractCommandOutput } from '../fixtures/stdout-helpers';

// The API proxy sidecar is at this fixed IP on the awf-net network
const API_PROXY_IP = '172.30.0.30';
Expand Down Expand Up @@ -176,10 +177,12 @@ describe('API Proxy Observability', () => {
);

expect(result).toSucceed();
const lower = result.stdout.toLowerCase();
// Extract just the command output (strip Docker build noise)
const cmdOutput = extractCommandOutput(result.stdout);
const lower = cmdOutput.toLowerCase();
expect(lower).toContain('x-request-id');
// The injected ID should NOT appear — proxy should have generated a UUID instead
expect(result.stdout).not.toContain('<script>');
// The injected ID should NOT appear in the actual response — proxy should have generated a UUID instead
expect(cmdOutput).not.toContain('<script>');
}, 180000);

test('should show active_requests gauge at 0 after request completes', async () => {
Expand Down Expand Up @@ -207,8 +210,9 @@ describe('API Proxy Observability', () => {
);

expect(result).toSucceed();
// Parse the metrics JSON and check active_requests
const metricsJson = JSON.parse(result.stdout);
// Parse the metrics JSON (extract from stdout which may contain Docker build output)
const metricsJson = extractLastJson(result.stdout);
expect(metricsJson).not.toBeNull();
const activeRequests = metricsJson.gauges?.active_requests || {};
// All provider gauges should be 0 or absent
for (const count of Object.values(activeRequests)) {
Expand Down Expand Up @@ -237,9 +241,11 @@ describe('API Proxy Observability', () => {
);

expect(result).toSucceed();
// Histogram should have request_duration_ms entries with count > 0
expect(result.stdout).toContain('request_duration_ms');
const metricsJson = JSON.parse(result.stdout);
// Parse the metrics JSON (extract from stdout which may contain Docker build output)
const cmdOutput = extractCommandOutput(result.stdout);
expect(cmdOutput).toContain('request_duration_ms');
const metricsJson = extractLastJson(result.stdout);
expect(metricsJson).not.toBeNull();
const durationHist = metricsJson.histograms?.request_duration_ms;
expect(durationHist).toBeDefined();
// At least one provider should have a count > 0
Expand Down
45 changes: 30 additions & 15 deletions tests/integration/api-proxy-rate-limit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import { describe, test, expect, beforeAll, afterAll } from '@jest/globals';
import { createRunner, AwfRunner } from '../fixtures/awf-runner';
import { cleanup } from '../fixtures/cleanup';
import { extractLastJson, extractCommandOutput } from '../fixtures/stdout-helpers';

// The API proxy sidecar is at this fixed IP on the awf-net network
const API_PROXY_IP = '172.30.0.30';
Expand Down Expand Up @@ -89,11 +90,13 @@ describe('API Proxy Rate Limiting', () => {

test('should include Retry-After header in 429 response', async () => {
// Set RPM=1, make 2 requests quickly — second should get 429 with Retry-After
// Use -D /dev/stderr to dump headers to stderr (more reliable than -i with Docker build noise)
// Also capture the response body which should contain rate_limit_error with retry_after
const script = [
// First request consumes the limit
`curl -s -X POST http://${API_PROXY_IP}:10001/v1/messages -H "Content-Type: application/json" -d '{"model":"test"}' > /dev/null`,
// Second request should be rate limited — capture headers
`curl -s -i -X POST http://${API_PROXY_IP}:10001/v1/messages -H "Content-Type: application/json" -d '{"model":"test"}'`,
// Second request should be rate limited — capture body (which has retry_after field)
`curl -s -X POST http://${API_PROXY_IP}:10001/v1/messages -H "Content-Type: application/json" -d '{"model":"test"}'`,
].join(' && ');

const result = await runner.runWithSudo(
Expand All @@ -112,15 +115,17 @@ describe('API Proxy Rate Limiting', () => {
);

expect(result).toSucceed();
// Response should include retry-after header (case insensitive)
expect(result.stdout.toLowerCase()).toContain('retry-after');
// The 429 response body should contain retry_after field in the JSON error
const cmdOutput = extractCommandOutput(result.stdout);
expect(cmdOutput.toLowerCase()).toMatch(/retry.after/);
}, 180000);

test('should include X-RateLimit headers in 429 response', async () => {
// Set low RPM to guarantee 429, then check for X-RateLimit-* headers
// Set low RPM to guarantee 429, then check for rate limit info in response body
// Note: curl -i headers get mixed with Docker build output, so we check the JSON body instead
const script = [
`curl -s -X POST http://${API_PROXY_IP}:10001/v1/messages -H "Content-Type: application/json" -d '{"model":"test"}' > /dev/null`,
`curl -s -i -X POST http://${API_PROXY_IP}:10001/v1/messages -H "Content-Type: application/json" -d '{"model":"test"}'`,
`curl -s -X POST http://${API_PROXY_IP}:10001/v1/messages -H "Content-Type: application/json" -d '{"model":"test"}'`,
].join(' && ');

const result = await runner.runWithSudo(
Expand All @@ -139,10 +144,13 @@ describe('API Proxy Rate Limiting', () => {
);

expect(result).toSucceed();
const lower = result.stdout.toLowerCase();
expect(lower).toContain('x-ratelimit-limit');
expect(lower).toContain('x-ratelimit-remaining');
expect(lower).toContain('x-ratelimit-reset');
// The rate-limited response body should contain error type and rate limit info
const responseJson = extractLastJson(result.stdout);
expect(responseJson).not.toBeNull();
expect(responseJson.error?.type).toBe('rate_limit_error');
// Verify the response includes retry_after in the error message or headers field
const responseStr = JSON.stringify(responseJson).toLowerCase();
expect(responseStr).toMatch(/retry.after/);
}, 180000);

test('should not rate limit when --no-rate-limit is set', async () => {
Expand Down Expand Up @@ -200,10 +208,13 @@ describe('API Proxy Rate Limiting', () => {
);

expect(result).toSucceed();
// The health response should show rate_limits with the configured RPM limit
expect(result.stdout).toContain('"rate_limits"');
// The limit value of 5 should appear in the response
expect(result.stdout).toContain('"limit":5');
// Parse the health response JSON (extract from stdout which may contain Docker build output)
const healthJson = extractLastJson(result.stdout);
expect(healthJson).not.toBeNull();
expect(healthJson.rate_limits).toBeDefined();
// The limit value of 5 should appear in the rate_limits
const healthStr = JSON.stringify(healthJson);
expect(healthStr).toContain('"limit":5');
}, 180000);

test('should show rate limit metrics in /metrics after rate limiting occurs', async () => {
Expand Down Expand Up @@ -233,7 +244,11 @@ describe('API Proxy Rate Limiting', () => {
);

expect(result).toSucceed();
// Parse the metrics JSON (extract from stdout which may contain Docker build output)
const metricsJson = extractLastJson(result.stdout);
expect(metricsJson).not.toBeNull();
// Metrics should include rate_limit_rejected_total counter
expect(result.stdout).toContain('rate_limit_rejected_total');
const metricsStr = JSON.stringify(metricsJson);
expect(metricsStr).toContain('rate_limit_rejected_total');
}, 180000);
});
Loading
Loading