Conversation
- Add anthropic-transforms.js: injectCacheBreakpoints, upgradeEphemeralTtl, applyAnsiStrip, applyToolDrop, loadCustomTransform, makeAnthropicTransform - Add composeBodyTransforms utility to chain body-transform functions - Inject anthropic-beta: extended-cache-ttl-2025-04-11 when AWF_ANTHROPIC_AUTO_CACHE=1 - Forward AWF_ANTHROPIC_* env vars to api-proxy container in docker-manager.ts - 57 new transform tests + 7 compose tests + 6 docker-manager forwarding tests
- Extract buildToolScrubPattern helper so the regex is precompiled once in makeAnthropicTransform, not on every request - Accept optional pre-compiled scrubPattern in applyToolDrop to avoid per-request regex construction overhead - Optimize beta-header includes check: parse split result once before calling includes - Add test: stripAnsi preserves non-SGR sequences when mixed with SGR - Add tests: buildToolScrubPattern reuse safety + applyToolDrop pre-compiled path Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/dc1e196f-357e-4c3d-8099-e1ad370ab09c
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
Adds opt-in Anthropic-specific request-body transforms to the api-proxy to reduce Claude session costs via prompt-cache breakpoint injection, TTL upgrading, tool dropping, ANSI stripping, and an optional custom transform hook, integrated into the existing bodyTransform chain.
Changes:
- Introduces
anthropic-transforms.jsimplementing a composed Buffer→Buffer(/null) transform pipeline plus comprehensive unit tests. - Integrates Anthropic optimizations into
containers/api-proxy/server.jsviacomposeBodyTransformsand conditionalanthropic-betaheader merging. - Forwards Anthropic-related env vars into the api-proxy container via
generateDockerComposeand adds forwarding tests.
Show a summary per file
| File | Description |
|---|---|
| src/docker-manager.ts | Forwards selected AWF_ANTHROPIC_* env vars into the api-proxy container environment. |
| src/docker-manager.test.ts | Adds tests asserting Anthropic env var forwarding behavior. |
| containers/api-proxy/server.js | Wires in Anthropic transform pipeline, adds transform composition helper, and conditionally merges anthropic-beta. |
| containers/api-proxy/server.test.js | Adds unit tests for composeBodyTransforms. |
| containers/api-proxy/anthropic-transforms.js | New module implementing Anthropic request-body transforms (cache breakpoints, TTL upgrades, tool drop, ANSI strip, custom hook). |
| containers/api-proxy/anthropic-transforms.test.js | New test suite validating transform behavior, idempotency, and custom hook loading. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 3
…ort, function or class' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot merge main and resolve conflicts |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Security Review: PR #2360Reviewed Finding 1 — Critical: Arbitrary code execution in the API-credential-holding sidecarFile: // anthropic-transforms.js:347-348
const absolutePath = path.resolve(filePath);
const mod = require(absolutePath);
The api-proxy's log volume is mounted read-write on the host ( Suggested action: Add a strict allowlist of permitted transform file paths (e.g., only paths within the container image's Finding 2 — Moderate: Feature design creates pressure to add insecure host bind mountsFile: The PR description and code comment in However, no bind mount is added for this path in Suggested action: Either (a) document explicitly that this only works with files pre-baked in a custom container image and never forward the env var from the host (since it would resolve against paths inside the container image anyway), or (b) if host files are intended, add a narrow, validated bind mount and path-prefix check before
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This is a flaky Gemini smoke test failure unrelated to our changes. The "Execute Gemini CLI" step ran for ~2 minutes (17:13:01–17:14:59) but the Gemini agent produced no safe outputs — no Evidence that this is pre-existing flakiness:
No code fix needed; this failure is not caused by changes in this PR. |
|
@copilot merge main and address this review feedback #2360 (comment) |
…proxy container The api-proxy container holds live API credentials. Forwarding a host-provided file path to require() inside that container creates an arbitrary code execution risk (security review Finding 1 & 2). Remove the env var forwarding; users who need a custom transform must bake their hook.js into a custom container image. Add explanatory comments in docker-manager.ts and server.js. Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/7591d987-6d79-4670-8429-1e2c42f7a496 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Main was already merged (it's an ancestor of HEAD). Addressed the security review in feb1502:
The transform hook feature ( |
🔥 Smoke Test: Copilot BYOK — PASS
Running in BYOK offline mode ( PR by
|
|
Smoke test results:
Status: PASS
|
Smoke Testfeat(api-proxy): add Anthropic prompt-cache optimizations (AWF_ANTHROPIC_AUTO_CACHE) Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Chroot Version Comparison Results
Result: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
This comment has been minimized.
This comment has been minimized.
Smoke Test Results
Overall: FAIL —
|
Security Review: PR #2360Reviewed by the security guard workflow. Finding: Unrestricted
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
🧪 Smoke Test Results
PR: feat(api-proxy): Anthropic prompt-cache injection, TTL upgrade, tool drop, ANSI strip, custom transform hook Overall: PARTIAL — MCP ✅, pre-step outputs not injected into prompt.
|
AWF's api-proxy sits in the exact request path where pino operates but lacked its caching optimisations. On a typical 30-turn Claude Code session, uncached input tokens dominate cost (~$4–20/session avoidable). This PR adds five opt-in transforms on POST
/v1/messagesvia the existingbodyTransformchain.New module:
containers/api-proxy/anthropic-transforms.jsAll transforms are pure, idempotent, zero-new-dependencies functions composed into a single Buffer→Buffer pipeline.
1. Auto-inject prompt-cache breakpoints (
AWF_ANTHROPIC_AUTO_CACHE=1)Injects
cache_control: {type:"ephemeral", ttl:"1h"}at up to 4 slots (Anthropic's ceiling):toolsentrysystemblockmessages[0]tailTtlAlso adds
anthropic-beta: extended-cache-ttl-2025-04-11to upstream requests, merging with any existing client beta flags (handles both string and string-array header values safely).2. Upgrade existing ephemeral TTLs (implied by
AWF_ANTHROPIC_AUTO_CACHE)Rewrites client-injected
{type:"ephemeral"}breakpoints that lack attlto1h. The rolling tail stays atAWF_ANTHROPIC_CACHE_TAIL_TTL(default5m) to avoid the 2× write surcharge on a moving breakpoint.3. Drop unused tools (
AWF_ANTHROPIC_DROP_TOOLS=NotebookEdit,CronCreate,…)Strips named tools from
tools[]and scrubs references fromsystemtext blocks. The word-boundary regex is compiled once at startup, not per-request.4. Strip ANSI from
tool_resultblocks (AWF_ANTHROPIC_STRIP_ANSI=1)Removes SGR sequences (
\x1B[…m) from terminal output in tool results — halves token count in colour-heavy outputs and eliminates cache misses caused by colour-code variation across turns.5. Custom transform hook (
AWF_ANTHROPIC_TRANSFORM_FILE=/path/to/hook.js)Integration (
server.js)composeBodyTransforms(first, second)utility chains the existing model-alias transform with the Anthropic optimisations transform; model alias runs first.require('./anthropic-transforms')is wrapped in a try/catch so the container degrades gracefully if the module is absent.Dockerfile
containers/api-proxy/Dockerfileupdated to includeanthropic-transforms.jsin the explicit COPY list so the module is present in the container image.Env var forwarding (
src/docker-manager.ts)Four
AWF_ANTHROPIC_*env vars are forwarded from the host environment to the api-proxy container:AWF_ANTHROPIC_AUTO_CACHE,AWF_ANTHROPIC_CACHE_TAIL_TTL,AWF_ANTHROPIC_DROP_TOOLS, andAWF_ANTHROPIC_STRIP_ANSI.AWF_ANTHROPIC_TRANSFORM_FILEis intentionally not forwarded (see security note above).Tests
72 new tests: 64 in
anthropic-transforms.test.js(per-function, idempotency, mutation safety, edge cases), 7 forcomposeBodyTransforms, 6 for env-var forwarding indocker-manager.test.ts.