[Security Review] Daily Security Review and Threat Modeling — 2026-03-25 #1424
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-04-01T13:59:34.103Z.
|
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
📊 Executive Summary
This review covers a deep, evidence-based analysis of the
gh-aw-firewallcodebase focusing on the network security architecture, container hardening, domain validation, input sanitization, and token protection mechanisms. No npm dependency vulnerabilities were found (Critical: 0, High: 0, Moderate: 0). Six distinct security issues were identified spanning High, Medium, and Low severity. No active firewall-escape-test workflow logs were retrievable in this run (git not in PATH for the log tool), so findings are based entirely on static codebase analysis.🛡️ Architecture Security Analysis
Network Security Assessment — GOOD with gaps
Evidence gathered:
The defense-in-depth model is well-designed:
DOCKER-USER → FW_WRAPPERchain blocks all container egress except Squid (172.30.0.10) and whitelisted DNSGap found: The
FW_WRAPPERchain is installed with-I DOCKER-USER 1, which ensures it is evaluated first. However, there is a brief startup window betweendocker network createand the call tosetupHostIptables()where containers onawf-netcould make unfiltered outbound connections. This is structurally inherent to the Docker DOCKER-USER approach.Container Security Assessment — GOOD with one key finding
Evidence gathered:
SYS_ADMIN+SYS_CHROOTare granted to the container for theentrypoint.shprocfs mount, and both are dropped at runtime viacapshbefore user code runs. However,apparmor:unconfinedremoves the AppArmor layer, andmount,unshare, andsetnsare all in the seccomp ALLOW list.Domain Validation Assessment — STRONG
*converts to[a-zA-Z0-9.-]*(not.*)*,*.*, patterns where wildcards ≥ TLD-1 segments) are rejected(redacted) vshttps://`) is cleanly implementedInput Validation Assessment — GOOD
execa(array-form, no shell interpolation)escapeShellArg/joinShellArgsinsrc/cli.ts:867-885AWF_USER_UID/AWF_USER_GIDvalidated as numeric and non-zero inentrypoint.sh:10-30docker-stub.shblocks Docker-in-Docker completely with a hard errormount/unshare/setnsallowed in seccomp +apparmor:unconfined🎯 Detailed Findings with Evidence
F1 — HIGH: API Proxy Iptables Port 10003 Gap
File:
src/types.ts:16-41,src/host-iptables.ts:365-372Evidence:
Verification:
Issue: The
min:maxiptables port range syntax (10000:10004) allows ALL ports from 10000 to 10004 inclusive, including port 10003. If the API proxy sidecar (172.30.0.30) were compromised or a service were running on port 10003, the agent container could reach it despite the firewall intending to only allow the four defined ports.Recommendation: Replace the
min:maxrange with an explicit multiport rule listing only the four defined ports:F2 — HIGH: Dangerous Port List Divergence Between Squid and iptables
Files:
src/squid-config.ts:14-31,containers/agent/setup-iptables.sh:DANGEROUS_PORTSEvidence:
Issue: Six database/time-series ports are blocked in the Squid config's "deny" ACL but are not listed in
setup-iptables.sh'sDANGEROUS_PORTSarray. Since the agent's OUTPUT filter chain drops all TCP not explicitly allowed, this is currently mitigated by the default-deny rule — connections to these ports would still be blocked.However, if any of these ports were reachable via DNAT redirect through Squid (which would normally allow HTTP/HTTPS ports), they could be accessed. More importantly, if Squid's
http_access deny dangerous_portsACL is the only enforcement, it is subject to future Squid misconfig. The single source-of-truth principle is violated: the two lists MUST stay in sync.Recommendation: Define
DANGEROUS_PORTSin one place (e.g.,src/types.ts) and generate both the Squid ACL config andsetup-iptables.shfrom it, or at minimum add a CI test that asserts both lists are identical.F3 — MEDIUM:
mount,unshare,setnsin Seccomp ALLOW + AppArmor UnconfinedFiles:
containers/agent/seccomp-profile.json,src/docker-manager.ts:1083-1099Evidence:
Analysis:
mountis allowed by seccomp and needed forentrypoint.sh's procfs mount.SYS_ADMINis dropped viacapshbefore user code runs, which should prevent successfulmountcalls from user code. However, user-namespace-based mounts (unshare -m) on Linux kernels withkernel.unprivileged_userns_clone=1(common on Ubuntu) do not requireSYS_ADMIN.unshare(allowed by seccomp) +clonewithCLONE_NEWUSERcan create user namespaces as an unprivileged user, which can then be used to mount bind mounts within the new namespace. This is a known container escape vector when combined withsetns.apparmor:unconfinedremoves the AppArmor policy which would normally block these operations independently.no-new-privileges:trueprevents privilege escalation via setuid binaries, but does not block user namespaces.Recommendation: Add
unshareandsetnsto the seccomp ERRNO list. Alternatively, consider re-enabling AppArmor with a custom profile that allows only the neededmountoperations during container startup (beforecapsh), then restricts it after.{ "action": "SCMP_ACT_ERRNO", "names": ["unshare", "setns"] }F4 — MEDIUM: 5-Second Token Unset Race Window
File:
containers/agent/entrypoint.sh:644-660, 691-707Evidence:
Issue: For 5 seconds after the agent process starts, all sensitive tokens (
GITHUB_TOKEN,ANTHROPIC_API_KEY,OPENAI_API_KEY, etc.) remain visible in/proc/[PARENT_PID]/environand/proc/[AGENT_PID]/environ. A malicious subprocess spawned immediately at agent startup could read these beforeunset_sensitive_tokensexecutes.The
one-shot-token.soLD_PRELOAD library partially mitigates this by interceptinggetenv()calls, but:key=0x5A) — not cryptographic/proc/self/environdirectly (bypassinggetenv) is not protectedRecommendation: Instead of a fixed-time sleep, use a pipe/socket-based synchronization where the agent signals back to the parent shell when tokens have been cached (reading them exactly once), then the parent unsets them. This eliminates the race entirely.
F5 — MEDIUM: IPv6 Bypass if Sysctl Disable Fails Silently
Files:
src/host-iptables.ts:66-78,containers/agent/setup-iptables.sh:16-28Evidence:
Issue: If
ip6tablesis unavailable ANDsysctlfails to disable IPv6 (e.g., in environments where the sysctl is not writable), IPv6 traffic from the agent container is completely unfiltered. An agent tool that uses IPv6 (or resolves a hostname to an IPv6 address) would bypass all L7 filtering.Recommendation: Make failure to restrict IPv6 a hard error:
F6 — LOW: DLP Scanning Bypassed via HTTPS (Without SSL Bump)
Files:
src/dlp.ts,src/squid-config.ts:134-176Evidence:
Issue: DLP patterns (detecting GitHub PATs, Anthropic keys, OpenAI keys in URLs) only apply to HTTP traffic. For HTTPS (
CONNECTmethod), Squid only sees theCONNECT hostname:443line — the full URL with any embedded tokens is invisible without SSL Bump. An agent that exfiltrates credentials via HTTPS URL query parameters would bypass DLP scanning.Recommendation: Document this limitation clearly in
docs/and recommend--enable-ssl-bumpfor deployments where DLP coverage of HTTPS is required. Alternatively, note that DLP's primary value is for HTTP APIs (e.g., package registries on port 80).📋 Evidence Collection Summary
cat src/host-iptables.tscat containers/agent/setup-iptables.shcat src/squid-config.ts | grep DANGEROUS_PORTS -A 30cat containers/agent/seccomp-profile.json | python3 -c "..."cat src/docker-manager.ts | grep cap_add -A 10cat src/domain-patterns.tsnpm audit --json | python3 -c "..."cat containers/agent/entrypoint.shpython3 -c "ports = ...✅ Recommendations
🔴 High — Fix promptly
[F1] Fix API proxy iptables port range — Replace
\$\{minPort}:\$\{maxPort}with an explicitmultiport --dportsrule insrc/host-iptables.ts:371. This closes the port 10003 gap with a one-line change.[F2] Synchronize dangerous port lists — Add CouchDB (5984/6984), InfluxDB (8086/8088), Elasticsearch (9200/9300) to
setup-iptables.sh'sDANGEROUS_PORTSarray. Consider a unit test asserting parity between the two lists.🟠 Medium — Address in next sprint
[F3] Restrict
unshare/setnsin seccomp — Add both to theSCMP_ACT_ERRNOlist incontainers/agent/seccomp-profile.json. These are not needed for any documented functionality and create user-namespace escape opportunities.[F4] Replace 5-second sleep with synchronization primitive — Use a pipe or signal file to synchronize token-unset with agent token caching, rather than relying on a fixed
sleep 5.[F5] Fail hard when IPv6 cannot be restricted — Treat sysctl failure + ip6tables unavailability as a fatal error to prevent unfiltered IPv6 egress.
🟡 Low — Document or track for future work
docs/that DLP scanning requires--enable-ssl-bumpfor full HTTPS coverage.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions