fix: enable Squid intercept mode for NAT-redirected traffic#520
fix: enable Squid intercept mode for NAT-redirected traffic#520
Conversation
When traffic is NAT'd (DNAT) to Squid proxy, clients send relative URLs (GET /path) because they don't know they're talking to a proxy. Squid's normal proxy mode requires absolute URLs (GET http://example.com/path), causing "Invalid URL - Missing hostname" errors. This fix: - Adds interceptPort to SquidConfig for transparent proxy traffic - Configures Squid with `http_port 3129 intercept` for NAT'd traffic - Updates iptables rules to redirect to intercept port (3129) not regular port (3128) - Keeps regular port (3128) for explicit proxy usage via HTTP_PROXY This fixes Codex/rmcp OAuth discovery timeouts - requests now reach the MCP gateway instead of being blocked by Squid. Fixes #519 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
C++ Build Test Results
Overall: PASS ✅ All C++ projects configured and built successfully.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests passed successfully.
|
Node.js Build Test Results
Overall: PASS ✅ All Node.js projects built and tested successfully.
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed their tests.
|
Smoke Test Results✅ GitHub MCP: #515 feat: add AWF agent skill for Claude Code agents, #514 fix: add auth and error handling to build-test workflows Status: PASS cc: @Mossaka
|
Build Test: Bun - FAILED ❌
Overall: FAIL Error DetailsBoth projects encountered critical runtime failures: elysia:
hono:
Root Cause: Bun runtime is crashing with core dumps on this environment (GitHub Actions runner). This appears to be an environment compatibility issue with Bun v1.3.8. Action Required: This test cannot pass until the Bun runtime stability issue is resolved.
|
|
Smoke Test Results - Claude Engine ✅ GitHub MCP - Last 2 merged PRs:
✅ Playwright - GitHub homepage title verified ✅ File Writing - Created ✅ Bash Tool - File read successful Status: PASS
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug where HTTP traffic redirected via iptables NAT (DNAT) to the Squid proxy fails with "Invalid URL - Missing hostname" errors. The root cause is that NAT-redirected clients send relative URLs (e.g., GET /path) because they don't know they're talking to a proxy, but Squid's normal proxy mode requires absolute URLs (e.g., GET http://example.com/path).
Changes:
- Added
interceptPortfield toSquidConfiginterface for transparent proxy traffic handling - Configured Squid with separate intercept port (3129) using
http_port 3129 interceptdirective - Updated iptables rules to redirect NAT traffic to the intercept port instead of the regular proxy port
- Added test coverage for the new intercept port configuration
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Added interceptPort optional field to SquidConfig interface with comprehensive documentation |
| src/squid-config.ts | Added intercept port configuration to Squid config generation with explanatory comments |
| src/squid-config.test.ts | Added tests verifying intercept port is included when specified and excluded when not |
| src/docker-manager.ts | Passed SQUID_INTERCEPT_PORT constant to Squid config generation |
| containers/agent/setup-iptables.sh | Updated NAT redirection rules to use intercept port for transparent proxy traffic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (interceptPort) { | ||
| portConfig += `\nhttp_port ${interceptPort} intercept`; | ||
| } |
There was a problem hiding this comment.
When SSL Bump is enabled, the intercept port configuration added here will be ignored. On line 436 (visible in surrounding context), portConfig is set to empty string when sslBump is enabled, and the SSL Bump section uses a hardcoded port configuration.
The generateSslBumpSection function (called on line 428-434) hardcodes "http_port 3128 ssl-bump" and doesn't receive the port or interceptPort parameters. This means NAT-redirected traffic will fail with "Invalid URL - Missing hostname" errors when SSL Bump is enabled, which is the exact issue this PR is trying to fix.
To fix this, generateSslBumpSection needs to be updated to:
- Accept port and interceptPort parameters
- Generate both "http_port [port] ssl-bump" and "http_port [interceptPort] ssl-bump intercept" configurations when interceptPort is provided
Without this fix, SSL Bump mode will continue to experience the NAT redirect failures described in issue #519.
| it('should add intercept port when specified', () => { | ||
| const config: SquidConfig = { | ||
| domains: ['github.com'], | ||
| port: 3128, | ||
| interceptPort: 3129, | ||
| }; | ||
| const result = generateSquidConfig(config); | ||
| expect(result).toContain('http_port 3128'); | ||
| expect(result).toContain('http_port 3129 intercept'); | ||
| }); | ||
|
|
||
| it('should not add intercept port when not specified', () => { | ||
| const config: SquidConfig = { | ||
| domains: ['github.com'], | ||
| port: 3128, | ||
| }; | ||
| const result = generateSquidConfig(config); | ||
| expect(result).toContain('http_port 3128'); | ||
| expect(result).not.toContain('intercept'); | ||
| }); |
There was a problem hiding this comment.
The new tests for intercept port are placed inside the "SSL Bump Mode" describe block, but they don't actually test the combination of SSL Bump with intercept port. This misses the critical bug where SSL Bump ignores the intercept port configuration.
Add a test that verifies intercept port works with SSL Bump enabled:
- Config with sslBump: true, interceptPort: 3129
- Expected: both "http_port 3128 ssl-bump" and "http_port 3129 ssl-bump intercept" in output
This test would catch the bug where generateSslBumpSection doesn't support intercept port.
❌ Build Test: Java - FAILEDStatus: ENVIRONMENT FAILURE ErrorCannot execute Java tests due to corrupted GitHub Actions runner environment. All Java binaries (across multiple installations) are being executed as bash instead of Java. Details
Overall: FAIL Diagnostic OutputAll Java installations tested:
RecommendationThis workflow needs to be re-run on a fresh GitHub Actions runner with a functional Java environment.
|
Rust Build Test Results
Overall: PASS ✅ All Rust projects built successfully and all tests passed.
|
Mossaka
left a comment
There was a problem hiding this comment.
Code Review: Add Squid Intercept Mode for NAT-Redirected Traffic
Summary
This PR adds a second port (3129) to Squid in "intercept" mode to handle NAT-redirected (transparent proxy) traffic. The change addresses the root cause of Codex smoke test failures where clients sending relative URLs (GET /path) were being rejected by Squid's normal proxy mode which expects absolute URLs (GET http://example.com/path).
Files Reviewed
src/types.ts- NewinterceptPortfield inSquidConfiginterfacesrc/squid-config.ts- Squid configuration generation with intercept portsrc/squid-config.test.ts- Unit tests for the new functionalitysrc/docker-manager.ts- Docker compose configuration updatescontainers/agent/setup-iptables.sh- iptables NAT rule updates
Correctness
The implementation follows Squid best practices. The approach of using separate ports for explicit proxy (3128) vs transparent intercept (3129) is the correct pattern. Key observations:
- The
http_port 3129 interceptdirective is the standard way to handle NAT-redirected traffic in Squid - The iptables DNAT rules correctly redirect to the intercept port
- The regular port (3128) continues to work for explicit proxy usage via
HTTP_PROXY/HTTPS_PROXYenvironment variables
Potential Issues
1. SSL Bump Mode Missing Intercept Port (Medium Severity)
When sslBump is enabled, generateSslBumpSection() generates its own port configuration with just http_port 3128 ssl-bump ..., and then portConfig is set to empty string (line 436). This means the intercept port (3129) is not added when SSL Bump is enabled.
if (sslBump && caFiles && sslDbPath) {
sslBumpSection = generateSslBumpSection(...);
// SSL Bump section includes its own port config, so use that instead
portConfig = ''; // <-- interceptPort is lost here
}If users enable SSL Bump with NAT-redirected traffic, they would still see the "Invalid URL" errors.
Suggestion: Consider passing interceptPort to generateSslBumpSection() and adding an intercept port there as well:
http_port 3129 intercept ssl-bump cert=... key=...
2. Healthcheck Only Checks Port 3128
The Docker healthcheck in docker-manager.ts (line 270) only verifies port 3128:
healthcheck: {
test: ['CMD', 'nc', '-z', 'localhost', '3128'],
...
}If the intercept port (3129) fails to start but 3128 succeeds, the container will be marked healthy even though transparent proxying won't work.
Suggestion: Consider checking both ports, e.g.:
test: ['CMD', 'sh', '-c', 'nc -z localhost 3128 && nc -z localhost 3129'],Edge Cases
Handled Well:
- Default value for
SQUID_INTERCEPT_PORT(3129) in both shell script and TypeScript - The
interceptPortis optional, so existing configurations without it continue to work - Port range handling in iptables correctly uses the intercept port
Edge Case Question:
- What happens if someone manually sets
interceptPortto the same value asport? The Squid config would have duplicate port definitions. Consider adding validation if this is a concern.
Security Considerations
No security concerns identified. The intercept mode itself is not a security issue - it's simply a different way for Squid to accept incoming connections. The domain ACLs and access controls still apply regardless of which port receives the traffic.
The port 3129 is correctly exposed only to the internal Docker network via the compose configuration, not to the host.
Test Coverage
The unit tests added are appropriate:
- Test that intercept port is added when specified
- Test that intercept port is not added when not specified
Suggestions for additional coverage:
- Consider adding an integration test that verifies NAT-redirected traffic works (e.g., a curl command without
HTTP_PROXYset that gets transparently proxied) - The integration tests currently don't have any
interceptkeyword - might be worth adding a test case
Documentation
The code is well-documented:
- Clear JSDoc comments in
types.tsexplaining when to use each port - Good inline comments in
setup-iptables.shexplaining the difference between regular and intercept ports - Comments in
squid-config.tsexplaining the two port modes
Overall Assessment
This PR correctly addresses the root cause of Codex smoke test failures. The implementation is sound and follows Squid best practices. The main concerns are:
- Must address: SSL Bump mode doesn't include the intercept port - this could cause issues for users who enable SSL Bump
- Nice to have: Healthcheck could verify both ports
- Nice to have: Consider additional integration test coverage
The PR is approvable with the SSL Bump concern addressed, or can be merged as-is if SSL Bump + transparent proxy is not a supported configuration.
1. Add intercept port (3129) to SSL Bump section: When SSL Bump was enabled, the generateSslBumpSection() function generated its own port config but did not include the intercept port needed for NAT-redirected transparent proxy traffic. 2. Update healthcheck to verify both ports: The Docker healthcheck only verified port 3128, not ensuring port 3129 was also working. Now checks both ports to ensure complete Squid proxy functionality. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
Build Test: Node.js - Results
Overall: PASS ✅ All Node.js build tests completed successfully.
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Status: PASS cc @Mossaka
|
Go Build Test Results
Overall: PASS ✅ All Go projects built and tested successfully.
|
|
Smoke Test Results (Claude) Last 2 merged PRs:
✅ GitHub MCP (list PRs) Status: PASS
|
Build Test: Bun - FAIL ❌Bun installation completed successfully (v1.3.8), but the test runner crashes in the GitHub Actions environment.
Overall: FAIL Error DetailsBoth projects failed with the same error: The Next Steps
|
Security Review: Critical HTTPS Bypass Vulnerability🔴 CRITICAL: SSL Bump Bypass on Intercept PortThis PR introduces a critical security vulnerability that allows HTTPS traffic to bypass domain filtering when SSL Bump mode is enabled. Issue DetailsFile: Problem: The intercept port (3129) lacks the Current Configuration (VULNERABLE): Attack VectorWhen SSL Bump mode is enabled:
Impact
Required FixThe intercept port MUST have Vulnerable (current): Secure (required): Testing RequiredAfter fixing, verify:
Suggested ActionDO NOT MERGE until this vulnerability is fixed. The intercept port must have identical SSL bump configuration to port 3128 when SSL Bump mode is enabled.
|
❌ Build Test: Rust - FAILEDStatus: Unable to execute tests due to critical environment issue. Error SummaryThe test environment has a fundamental problem where all command arguments are being interpreted as file paths rather than arguments. This affects all commands including Example failures:
Test Results
Overall: FAILED Environment Details
Root CauseThis appears to be a bash/shell configuration issue in the GitHub Actions runner environment where command argument parsing is fundamentally broken. The issue affects all commands, not just Rust tooling. RecommendationThe workflow environment needs investigation to determine why command arguments are being misinterpreted as file paths. This is blocking all Rust build testing.
|
After #524 removed HTTP_PROXY/HTTPS_PROXY from the agent container, the v0.13.4 images break because their setup-iptables.sh DNATs to port 3128 (explicit proxy) instead of 3129 (intercept mode). Recompile all lock files to use v0.13.5 images which have the intercept port fix (PR #520). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After #524 removed HTTP_PROXY/HTTPS_PROXY from the agent container, the v0.13.4 images break because their setup-iptables.sh DNATs to port 3128 (explicit proxy) instead of 3129 (intercept mode). Recompile all lock files to use v0.13.5 images which have the intercept port fix (PR #520). Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After #524 removed HTTP_PROXY/HTTPS_PROXY from the agent container, the v0.13.4 images break because their setup-iptables.sh DNATs to port 3128 (explicit proxy) instead of 3129 (intercept mode). Recompile all lock files to use v0.13.5 images which have the intercept port fix (PR #520). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reverts the following PRs which introduced unnecessary complexity: - #520: fix: enable Squid intercept mode for NAT-redirected traffic - v0.13.5 release: chore(release): bump version to 0.13.5 - #524: fix: remove HTTP_PROXY/HTTPS_PROXY env vars from agent container - #526: chore: recompile workflow lock files for AWF v0.13.5 - #527: fix: recompile lock files with release action mode - #522: fix: mount /etc/hosts in chroot and fix HTTP blocking test - #530: fix: restore HTTPS_PROXY, fix chroot hosts/permissions, fix Bun crash - v0.13.6 release: chore(release): bump version to 0.13.6 The intercept mode (#520) was introduced to fix Codex failing with HTTP_PROXY, but the simpler fix is to just not set HTTP_PROXY for Codex. The intercept mode introduced a cascade of breakage: - HTTPS can't be transparently intercepted (needs CONNECT method) - Image version bumps required lock file recompilation - host.docker.internal traffic crashed Squid under load - Multiple PRs needed to fix each regression This reverts to the pre-#520 explicit proxy mode (HTTP_PROXY/HTTPS_PROXY pointing to Squid port 3128) which worked for all engines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reverts the following PRs which introduced unnecessary complexity: - #520: fix: enable Squid intercept mode for NAT-redirected traffic - v0.13.5 release: chore(release): bump version to 0.13.5 - #524: fix: remove HTTP_PROXY/HTTPS_PROXY env vars from agent container - #526: chore: recompile workflow lock files for AWF v0.13.5 - #527: fix: recompile lock files with release action mode - #522: fix: mount /etc/hosts in chroot and fix HTTP blocking test - #530: fix: restore HTTPS_PROXY, fix chroot hosts/permissions, fix Bun crash - v0.13.6 release: chore(release): bump version to 0.13.6 The intercept mode (#520) was introduced to fix Codex failing with HTTP_PROXY, but the simpler fix is to just not set HTTP_PROXY for Codex. The intercept mode introduced a cascade of breakage: - HTTPS can't be transparently intercepted (needs CONNECT method) - Image version bumps required lock file recompilation - host.docker.internal traffic crashed Squid under load - Multiple PRs needed to fix each regression This reverts to the pre-#520 explicit proxy mode (HTTP_PROXY/HTTPS_PROXY pointing to Squid port 3128) which worked for all engines. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
When traffic is NAT'd (DNAT) to Squid proxy, clients send relative URLs (
GET /path) because they don't know they're talking to a proxy. Squid's normal proxy mode requires absolute URLs (GET http://example.com/path), causing "Invalid URL - Missing hostname" errors.This is the root cause of Codex smoke test failures - the rmcp client's OAuth discovery requests get NAT'd to Squid but fail because Squid can't process the relative URL.
Changes
interceptPorttoSquidConfiginterface for transparent proxy traffichttp_port 3129 interceptfor NAT'd trafficHTTP_PROXYHow it works
HTTP_PROXYenv var) - expects absolute URLsTest plan
Related
🤖 Generated with Claude Code