-
Notifications
You must be signed in to change notification settings - Fork 11
Description
π Executive Summary
This review covers a deep, evidence-based security analysis of the gh-aw-firewall codebase conducted on 2026-02-20. The system demonstrates strong defense-in-depth posture with multiple layered controls. No critical vulnerabilities were found. Two medium-severity findings were identified β one being a functional inconsistency with --dns-servers and Squid's hardcoded DNS, and another a defense-in-depth gap in the container's UDP filtering. Several low-severity observations round out the findings.
| Metric | Value |
|---|---|
| Files analyzed | src/host-iptables.ts, src/squid-config.ts, src/domain-patterns.ts, src/cli.ts, src/docker-manager.ts, src/redact-secrets.ts, containers/agent/setup-iptables.sh, containers/agent/entrypoint.sh, containers/agent/seccomp-profile.json, containers/agent/one-shot-token/one-shot-token.c, containers/agent/docker-stub.sh |
| Critical findings | 0 |
| High findings | 0 |
| Medium findings | 2 |
| Low findings | 4 |
| npm audit vulns (critical/high) | 0 (2 moderate in dev deps) |
| Escape test workflow | No dedicated workflow found; security-review workflow log unavailable |
π Firewall Escape Test Context
No dedicated "Firewall Escape Test Agent" workflow was found in the repository. The security-review workflow exists but its recent run logs were unavailable. This review is based purely on static code analysis.
π‘οΈ Architecture Security Analysis
Network Security Assessment
The network filtering architecture uses a two-layer approach:
- Container-level iptables (setup-iptables.sh) β in-container OUTPUT chain rules
- Host-level iptables (host-iptables.ts) β DOCKER-USER β FW_WRAPPER chain on the bridge interface
NAT redirection flow (agent container):
- TCP/80 and TCP/443 β DNAT to
172.30.0.10:3128(Squid) - DNS UDP/53 β allowed to whitelisted servers only
- TCP dangerous ports β NAT RETURN (bypasses DNAT) β caught by TCP DROP
- Final filter rule:
iptables -A OUTPUT -p tcp -j DROP(line 292 ofsetup-iptables.sh)
Squid (host-level) has an exemption rule (-s 172.30.0.10 -j ACCEPT) allowing it unrestricted egress, which is correct design (Squid must connect to origin servers).
The host-level DOCKER-USER chain additionally:
- Blocks multicast traffic to
224.0.0.0/4with REJECT - Blocks all UDP (except whitelisted DNS) with LOG + REJECT
- Default-rejects all other traffic
Result: β Sound architecture with proper defense-in-depth.
Container Security Assessment
Capability handling:
# Agent container adds:
cap_add: ['NET_ADMIN', 'SYS_CHROOT', 'SYS_ADMIN']
# Agent container drops:
cap_drop: ['NET_RAW', 'SYS_PTRACE', ...]
NET_ADMIN is required for setup-iptables.sh. Critically, entrypoint.sh drops it via capsh --drop=cap_net_admin before user code runs:
capsh --drop=$CAPS_TO_DROP -- -c "exec gosu awfuser $(printf '%q ' "$@")"Token protection (one-shot-token library):
LD_PRELOAD=/usr/local/lib/one-shot-token.sointerceptsgetenv()for sensitive tokens- On first read, caches value in memory and unsets from environment
- Prevents
/proc/self/environexposure after agent startup - Default token names are XOR-obfuscated (key 0x5A) in the compiled binary
Docker socket isolation: The Docker socket is masked via:
'/dev/null:/host/var/run/docker.sock:ro'
'/dev/null:/host/run/docker.sock:ro'A docker-stub.sh stub replaces /usr/bin/docker and exits 127 with a helpful error.
Credential file masking: 17+ credential files are shadowed with /dev/null mounts (SSH keys, AWS credentials, .npmrc, git credentials, etc.).
Result: β Strong container hardening with defense-in-depth.
Domain Validation Assessment
The domain-patterns.ts module provides robust input validation:
*β rejected ("Pattern '*' matches all domains and is not allowed")*.*β rejected (too broad)- Patterns with
>1wildcard segment and>= totalSegments - 1wildcards β rejected - Double dots β rejected
- Empty inputs β rejected
*in wildcard position uses[a-zA-Z0-9.-]*instead of.*to prevent ReDoS- Domain length capped at 512 chars before regex matching (
isDomainMatchedByPattern)
Result: β Well-designed, ReDoS-resistant domain validation.
Input Validation Assessment
Shell command construction (entrypoint.sh):
capsh --drop=$CAPS_TO_DROP -- -c "exec gosu awfuser $(printf '%q ' "$@")"
```
`printf '%q'` properly shell-escapes all arguments. Volume mount paths are validated to be absolute in `cli.ts`. Port numbers are validated with numeric bounds checking and against a `DANGEROUS_PORTS` list.
**`--env` passthrough:** Matches `/^([^=]+)=(.*)$/` β correct, avoids key=value injection.
**Result: β
Proper input sanitization throughout.**
---
## β οΈ Findings
### Finding M-1 β Squid DNS Hardcoded to 8.8.8.8, Ignores `--dns-servers` (Medium)
**Evidence:**
```
# src/squid-config.ts:551
dns_nameservers 8.8.8.8 8.8.4.4The generateSquidConfig() function in src/squid-config.ts hardcodes dns_nameservers 8.8.8.8 8.8.4.4 in the generated Squid configuration. Although SquidConfig.dnsServers?: string[] is defined in src/types.ts:326, and the DNS servers are dynamically configured for the agent container via environment.AWF_DNS_SERVERS = dnsServers.join(',') (docker-manager.ts:452), the generateSquidConfig() call at docker-manager.ts:1227β1243 never passes dnsServers to the Squid config β so Squid always queries Google DNS regardless of --dns-servers.
Security impact:
- Functional inconsistency: users who configure
--dns-serversfor compliance or threat-model reasons (e.g., corporate DNS, Cloudflare 1.1.1.1) expect that all DNS queries go through their specified servers. Squid silently bypasses this for its own resolution. - Squid's DNS resolution is what governs ACL matching. If an attacker could influence DNS resolution at 8.8.8.8 (unlikely, but theoretically possible via BGP hijacking), ACL decisions could be affected.
- Squid container has unrestricted egress, so it can reach 8.8.8.8 regardless of host iptables rules. The agent container's DNS restriction via iptables doesn't apply to Squid.
Recommendation: Pass dnsServers into generateSquidConfig() and generate a dynamic dns_nameservers line:
// In generateSquidConfig()
const dnsNsLine = config.dnsServers && config.dnsServers.length > 0
? `dns_nameservers \$\{config.dnsServers.filter(s => !s.includes(':')).join(' ')}`
: 'dns_nameservers 8.8.8.8 8.8.4.4';Finding M-2 β Agent Container Missing UDP DROP in OUTPUT Chain (Defense-in-Depth Gap, Medium)
Evidence:
# containers/agent/setup-iptables.sh:292 (only TCP is dropped)
iptables -A OUTPUT -p tcp -j DROP
# No equivalent: iptables -A OUTPUT -p udp -j DROPThe agent container's filter OUTPUT chain ends with a TCP-only DROP rule. Non-DNS UDP traffic is not explicitly blocked at the container level. It is blocked at the host level by the FW_WRAPPER chain in DOCKER-USER (which contains REJECT for all UDP at src/host-iptables.ts:488), but this creates a dependency on the host-level rules being in place.
Potential attack scenario: If host iptables cleanup/setup fails (race condition on startup, or if setupHostIptables() throws after the container starts), non-DNS UDP egress from the container would be unrestricted. For example:
- DNS over QUIC (UDP/443) to unauthorized resolvers
- Custom UDP exfiltration channels (e.g., NTP amplification, QUIC to attacker server)
Recommendation: Add explicit UDP DROP to the container's OUTPUT chain after DNS rules:
# In setup-iptables.sh, after DNS allow rules (around line 285):
echo "[iptables] Drop all non-DNS UDP traffic..."
iptables -A OUTPUT -p udp -j DROP
```
---
### Finding L-1 β DANGEROUS_PORTS Mismatch Between squid-config.ts and setup-iptables.sh (Low)
**Evidence:**
```
# src/squid-config.ts DANGEROUS_PORTS has 22 entries including:
5984, // CouchDB
6984, // CouchDB (SSL)
8086, // InfluxDB HTTP API
8088, // InfluxDB RPC
9200, // Elasticsearch HTTP API
9300, // Elasticsearch transport
# containers/agent/setup-iptables.sh DANGEROUS_PORTS has 15 entries β missing all 6 aboveA comment in setup-iptables.sh reads # Dangerous ports list - matches DANGEROUS_PORTS in squid-config.ts, but the lists diverge. The missing ports are not in the NAT RETURN list, meaning they don't bypass the DNAT redirect. However, they also don't appear in Safe_ports, so Squid would deny them via http_access deny !Safe_ports. And the final iptables -A OUTPUT -p tcp -j DROP catches everything else. So these ports are still blocked, just via a different layer.
Impact: No functional security gap currently, but the divergence is a maintenance hazard. If the TCP DROP rule is ever refactored, these ports would be silently unblocked at the iptables level.
Recommendation: Synchronize the DANGEROUS_PORTS lists between squid-config.ts and setup-iptables.sh, or document explicitly that the two lists serve different purposes (Squid ACL vs. iptables NAT RETURN).
Finding L-2 β Seccomp Profile Uses Default-Allow (Low)
Evidence:
// containers/agent/seccomp-profile.json
{
"defaultAction": "SCMP_ACT_ALLOW",
"syscalls": [
{ "names": ["ptrace", "process_vm_readv", "process_vm_writev"], "action": "SCMP_ACT_ERRNO" },
{ "names": ["kexec_load", "kexec_file_load", "reboot", "init_module", ...], "action": "SCMP_ACT_ERRNO" },
{ "names": ["umount", "umount2"], "action": "SCMP_ACT_ERRNO" }
]
}The seccomp profile uses SCMP_ACT_ALLOW as the default, blocking only 3 groups of syscalls. This is less restrictive than Docker's built-in default seccomp profile (which allows ~350 syscalls from the full ~400+ available). CIS Docker Benchmark recommends a restrictive seccomp profile.
Notably absent from the block list: unshare, clone (namespace creation), setuid/setgid (still restricted by capability drop), open_by_handle_at (container escape), bpf.
Recommendation: Switch to a default-deny profile (inherited from Docker's default seccomp profile or a minimal allowlist). As a quick win, add unshare and clone (with restricted flags) to the blocklist.
Finding L-3 β 5-Second Token Exposure Window (Low/Informational)
Evidence:
# containers/agent/entrypoint.sh:519,561
capsh --drop=$CAPS_TO_DROP -- -c "exec gosu awfuser $(printf '%q ' "$@")" &
AGENT_PID=$!
sleep 5 # Wait for agent to initialize and cache tokens
unset_sensitive_tokensThe one-shot-token library caches tokens in the agent process on first getenv() call. The entrypoint sleeps 5 seconds before unsetting tokens from the parent shell to give the agent time to read them. During this 5-second window, the tokens remain visible in the parent shell's /proc/$PID/environ.
Attack scenario: A malicious agent that quickly forks a subprocess before the first getenv() call could potentially read tokens from /proc/1/environ during this window β though this is already mitigated by the fact that the one-shot-token library intercepts getenv() in the forked processes too, and process introspection is blocked by the ptrace/process_vm_readv seccomp rules.
Recommendation: This is inherently a race condition mitigation tradeoff. The current approach is reasonable. Consider logging a warning if the agent doesn't call getenv() for protected tokens within the window.
Finding L-4 β --env-all May Expose Non-Standard Credential Patterns (Low/Informational)
Evidence:
// src/docker-manager.ts:317-341
const EXCLUDED_ENV_VARS = new Set([
'PATH', 'PWD', 'OLDPWD', 'SHLVL', '_',
'SUDO_COMMAND', 'SUDO_USER', 'SUDO_UID', 'SUDO_GID',
]);
// When api-proxy enabled, also excludes: OPENAI_API_KEY, ANTHROPIC_API_KEY, etc.
```
When `--env-all` is used, all host environment variables are passed through except the 9 entries in `EXCLUDED_ENV_VARS`. Non-standard credential patterns (`AWS_SESSION_TOKEN`, `AZURE_CLIENT_SECRET`, `VAULT_TOKEN`, `KUBECONFIG` inline credentials, etc.) would be passed through if present in the environment.
A warning is logged (`logger.warn('β οΈ Using --env-all...')`), but there is no block list for common credential naming patterns.
**Recommendation:** Extend the `EXCLUDED_ENV_VARS` exclusion list with common credential patterns (e.g., `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, `*_CREDENTIAL`) when `--env-all` is used, similar to how the redact-secrets module works. Add explicit documentation warning users about this risk.
---
## π― Attack Surface Map
| Surface | Location | Current Protections | Risk |
|---|---|---|---|
| Domain whitelist bypass via broad wildcard | `src/domain-patterns.ts` | `validateDomainOrPattern()` blocks `*`, `*.*`, multi-wildcards; ReDoS-safe regex | Low |
| Port bypass via non-HTTP protocols | `containers/agent/setup-iptables.sh` | TCP DROP covers all TCP; UDP blocked by host FW_WRAPPER | Low (gap: UDP not in container chain) |
| DNS exfiltration | `setup-iptables.sh`, `host-iptables.ts` | DNS restricted to whitelisted servers only; non-DNS UDP rejected at host | Low |
| iptables bypass via NET_ADMIN | `containers/agent/entrypoint.sh` | `capsh --drop=cap_net_admin` before user code; seccomp blocks `umount` | Low |
| Container escape via chroot | `containers/agent/entrypoint.sh` | `CAP_SYS_CHROOT` dropped before user code | Low |
| Token exfiltration via `/proc/environ` | `containers/agent/one-shot-token/` | one-shot-token library unsets after first read; parent unsets after 5s | Low (5s window) |
| Docker socket access | `src/docker-manager.ts:664` | Masked with `/dev/null` bind mount; stub `docker` binary | Low |
| Credential file exposure | `src/docker-manager.ts:774-806` | 17+ files masked with `/dev/null` | Low |
| Supply chain via custom agent image | `src/cli.ts:SAFE_BASE_IMAGE_PATTERNS` | Allowlist: only `ubuntu:XX.XX` and `catthehacker/ubuntu` patterns | Low |
| Command injection via user input | `containers/agent/entrypoint.sh` | `printf '%q'` for shell escaping | Low |
| Squid DNS independence | `src/squid-config.ts:551` | Hardcoded 8.8.8.8 β Squid bypasses `--dns-servers` | Medium |
---
## π Evidence Collection
<details>
<summary>Command outputs used in this analysis</summary>
**DANGEROUS_PORTS comparison:**
```
squid-config.ts: 22 entries (includes 5984, 6984, 8086, 8088, 9200, 9300)
setup-iptables.sh: 15 entries (missing CouchDB, InfluxDB, Elasticsearch ports)
```
**Seccomp profile:**
```
defaultAction: SCMP_ACT_ALLOW
Blocked syscall groups: ptrace/process_vm*, kexec/reboot/kernel modules, umountUDP in container OUTPUT chain:
# Only TCP DROP exists:
iptables -A OUTPUT -p tcp -j DROP # line 292
# No: iptables -A OUTPUT -p udp -j DROP
```
**Squid hardcoded DNS:**
```
# src/squid-config.ts:551
dns_nameservers 8.8.8.8 8.8.4.4
# generateSquidConfig() call never passes dnsServers parameter
```
**npm audit:**
```
Total vulns: 2 (both moderate, dev dependencies only)
Critical/High: 0β Recommendations (Prioritized)
π΄ Critical
None identified.
π High
None identified.
π‘ Medium
-
Fix Squid DNS hardcoding (
src/squid-config.ts:551): PassdnsServersfromWrapperConfigintogenerateSquidConfig()and use them in thedns_nameserversdirective. This ensures Squid's own resolution honors the user's--dns-serversconfiguration. -
Add UDP DROP to agent container OUTPUT chain (
containers/agent/setup-iptables.sh): Addiptables -A OUTPUT -p udp -j DROPafter the DNS allow rules (after line ~280) to make the container self-contained for UDP policy, removing reliance on host-level DOCKER-USER rules as the sole UDP blocker.
π’ Low
-
Synchronize DANGEROUS_PORTS lists between
squid-config.tsandsetup-iptables.sh: Add the 6 missing ports (5984, 6984, 8086, 8088, 9200, 9300) tosetup-iptables.sh'sDANGEROUS_PORTSarray, or add a code generation step that derives both from a single source of truth. -
Harden seccomp profile: Switch from default-allow to default-deny (or inherit Docker's default seccomp profile). At minimum, add
unshareandbpfto the blocklist to prevent namespace escape attempts. -
Extend
--env-allexclusion list: Add common credential naming patterns (*_TOKEN,*_SECRET, etc.) toEXCLUDED_ENV_VARSwhen--env-allis used, matching the redact-secrets module's patterns. -
Document the 5-second token exposure window: Add a comment in
entrypoint.shexplaining the security tradeoff and the mitigations in place (seccomp, one-shot-token, process isolation).
π Security Metrics
| Metric | Value |
|---|---|
| Security-critical TS files analyzed | 5 (host-iptables.ts, squid-config.ts, domain-patterns.ts, cli.ts, docker-manager.ts) |
| Security-critical shell scripts analyzed | 3 (setup-iptables.sh, entrypoint.sh, api-proxy-health-check.sh) |
| C source analyzed | 1 (one-shot-token.c) |
| Attack surfaces identified | 11 |
| Threat categories (STRIDE) | 6/6 addressed by existing controls |
| Defense layers for network egress | 3 (container iptables + Squid ACL + host DOCKER-USER) |
| Defense layers for token protection | 3 (one-shot-token LD_PRELOAD + parent unset + proc exclusions) |
| CVEs in production dependencies | 0 |
Note: This was intended to be a discussion, but discussions could not be created due to permissions issues. This issue was created as a fallback.
Tip: Discussion creation may fail if the specified category is not announcement-capable. Consider using the "Announcements" category or another announcement-capable category in your workflow configuration.
Generated by Daily Security Review and Threat Modeling
- expires on Feb 27, 2026, 1:51 PM UTC