[Security Review] Daily Security Review — 2026-04-21 #2135
Replies: 2 comments
-
|
🔮 The ancient spirits stir, and the smoke-test agent has walked this thread. Warning
|
Beta Was this translation helpful? Give feedback.
-
|
🔮 The ancient spirits stir, and the firewall runes confirm this path was walked. Warning
|
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
📊 Executive Summary
The
gh-aw-firewallcodebase demonstrates a mature, defense-in-depth security posture. The multi-layer architecture (host iptables → container DNAT → Squid ACL) is sound, and recent additions — IPv6 disablement, one-shot-token LD_PRELOAD, capability dropping, and DNS restriction — significantly raise the bar for egress escape. No critical vulnerabilities were identified. Five medium-priority findings and three low-priority hardening opportunities are documented below.🔍 Findings from Firewall Escape Test
The escape test summary file (
/tmp/gh-aw/escape-test-summary.txt) contained GitHub Actions CI framework metadata from a "Secret Digger (Copilot)" workflow run (April 11, 2026), not structured escape-test results. Key observations from that metadata:GH_AW_SECRET_VERIFICATION_RESULT: success— secret isolation verification passedGH_AW_AGENT_CONCLUSION: success— agent completed without errorsGH_AW_INFERENCE_ACCESS_ERROR: false— no inference service access failuresnoopsafe-outputs, indicating no credentials or sensitive data were exfiltrated in that runInterpretation: The Secret Digger workflow is a continuous red-team probe designed to test whether a compromised agent inside the AWF sandbox can exfiltrate GitHub tokens or API keys. The result being
successwith anoopoutput means the firewall successfully blocked exfiltration — the agent found nothing to report. This is the expected behavior and validates the credential isolation layer.🛡️ Architecture Security Analysis
Network Security Assessment
Evidence gathered:
Assessment: The layered approach is correct and well-implemented. IPv6 is disabled at the kernel level inside the container — a critical hardening step that prevents AAAA-record Happy Eyeballs bypasses. The DNAT approach intercepts proxy-unaware tools. The default-deny TCP/UDP policy is strong. DNS exfiltration is addressed by restricting DNS to configured servers only.
Gap identified (Medium — M1): When
AWF_ENABLE_HOST_ACCESSis active, bothhost.docker.internaland the container's default gateway (172.30.0.1) are given unconditional NAT RETURN (bypass Squid) and filter ACCEPT rules on ports 80/443. This creates a partial egress path to the Docker host that sidesteps L7 domain filtering:An attacker who controls code running in the agent container can send HTTP/HTTPS requests directly to the Docker host without Squid domain filtering when host access is enabled. Mitigation: document this as an explicit trust boundary in
--enable-host-accessdocumentation, and consider whether an optional host-side nginx/squid reverse proxy could enforce domain policy even for host-bound traffic.Container Privilege Model
Evidence gathered:
Gap identified (Medium — M2): In non-chroot mode (
AWF_CHROOT_ENABLEDunset or not "true"), the entrypoint setsCAPS_TO_DROP=""and no capabilities are explicitly stripped before running the user command. The agent container does not receiveNET_ADMIN(handled by the init container), but it retains all other default Docker capabilities — includingDAC_OVERRIDE,CHOWN,FOWNER,SETUID,SETGID,AUDIT_WRITE, andMKNOD. In the CIS Docker Benchmark (check 5.3),cap_drop: ALLwith explicit allowlist is recommended. The non-chroot path should drop unnecessary capabilities explicitly.Strength: The iptables init container pattern is well-designed —
NET_ADMINis never granted to the agent container; instead, a dedicatedawf-iptables-initcontainer shares the network namespace and runs setup. This is the correct architecture for network namespace manipulation without grantingNET_ADMINto user code.Gap identified (Low — L1): The ready-file handshake at
entrypoint.sh:133polls/tmp/awf-init/readywith a 30-second timeout:If the init container crashes silently (exit 0 but no ready file), the entrypoint waits 30 seconds then exits — which is the correct safe-fail behavior. However, if the init container partially applies rules before crashing, the agent could start with incomplete iptables rules. Consider adding an integrity hash or rule count checksum to the ready file.
Domain Validation Assessment
Evidence gathered:
Assessment: The domain injection protection is well-implemented.
assertSafeForSquidConfigvalidates every domain and regex before interpolation intosquid.conf. TheSQUID_DANGEROUS_CHARSset correctly covers the main injection primitives. The wildcard-to-regex conversion uses[a-zA-Z0-9.-]*(not.*) to prevent ReDoS. Port specs are sanitized withreplace(/[^0-9-]/g, '')(squid-config.ts:567) before insertion.Note (Low — L2): The
SQUID_DANGEROUS_CHARSregex excludes backslash (intentionally documented for URL regex patterns). The code comment states domain names are additionally validated invalidateDomainOrPattern()to reject\. Ensure this secondary validation is always enforced before any domain string reachesassertSafeForSquidConfigto maintain the defense-in-depth guarantee.Input Validation Assessment
Evidence gathered:
Assessment: Input validation is thorough across all three layers (TypeScript, bash entrypoint, bash iptables setup). UID/GID validation correctly rejects non-numeric values and root (0). IPv4 addresses fetched from Docker APIs are validated before use in iptables commands. Port specs are validated against leading-zero patterns that could be interpreted ambiguously.
Gap identified (Medium — M3): At
entrypoint.sh:347:Command: IPv6 disablement (setup-iptables.sh:36-49)
Command: Default deny rules (setup-iptables.sh:426-430)
Command: Capability dropping (entrypoint.sh:356-365)
Command: Squid config injection guard (squid-config.ts:98-108)
Command: UID/GID root prevention (entrypoint.sh:26-33)
Secret Digger (Copilot) workflow — agent found no exfiltration paths.
✅ Recommendations
🔴 Critical
None identified.
🟠 High
None identified.
🟡 Medium
M1 — Document and constrain
--enable-host-accesstrust boundarysetup-iptables.sh:207-211When host access is enabled, HTTP/HTTPS traffic to
host.docker.internaland the network gateway bypasses Squid domain filtering. Add explicit documentation in help text and README that--enable-host-accesscreates an L7-unfiltered path to the Docker host. Consider adding an optional host-side proxy for this path, or restricting to a named allowlist of host ports only (not ports 80/443 open-ended).M2 — Drop unnecessary capabilities in non-chroot mode
entrypoint.sh:360-364When
AWF_CHROOT_ENABLEDis not set, no capabilities are dropped before user code runs. Add a baselinecap_dropin the Docker Compose agent service definition coveringDAC_OVERRIDE,CHOWN,FOWNER,SETUID,SETGID,AUDIT_WRITE,MKNODper CIS Docker Benchmark 5.3. Alternatively, add explicitcap_drop: ALLwith a minimal allowlist indocker-manager.tsfor both modes.M3 — Scope
git safe.directoryto workspace onlyentrypoint.sh:347Replace:
git config --global --add safe.directory '*'With:
git config --global --add safe.directory "\$\{AWF_WORKDIR:-/workspace}"This restores CVE-2022-24765 protections for all paths outside the designated workspace, preventing malicious git hooks from executing if the agent traverses an attacker-controlled directory.
🔵 Low
L1 — Add integrity check to init container ready signal
entrypoint.sh:133,setup-iptables.sh:444-450The current ready-file protocol is binary (exists/not-exists). Consider writing a rule count or SHA of the expected iptables state to the ready file and having the entrypoint validate it. This prevents partial rule sets from silently going undetected if the init container crashes mid-script.
L2 — Verify
validateDomainOrPattern()always precedesassertSafeForSquidConfig()src/domain-patterns.ts,src/squid-config.ts:102The backslash exclusion from
SQUID_DANGEROUS_CHARSrelies onvalidateDomainOrPattern()rejecting\in domain names. Add a test case that passes a domain containing\directly toassertSafeForSquidConfig()to confirm the secondary check would catch it if validation is bypassed.L3 — Reduce permissions on copied config files
entrypoint.sh:193,217,229,239.claude.jsonis created/updated with mode0666(world-readable and world-writable) and the.claude/directory with0777. In a shared runner environment, these permissions allow any process running on the host to read session tokens cached in these files. Prefer0600(owner read/write only) for the files and0700for the directory.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions