📊 Executive Summary

This review covers a comprehensive evidence-based analysis of the gh-aw-firewall codebase as of 2026-03-30. The firewall implements a well-layered defence architecture (iptables + Squid proxy + container hardening + seccomp). No critical vulnerabilities were found. Two medium-severity issues were confirmed through proof-of-concept testing, and several lower-priority hardening opportunities were identified.

🔍 Findings from Firewall Escape Test

The firewall-escape-test workflow was not found under that name; security-review is the closest scheduled workflow. Logs could not be retrieved due to git not being in PATH in the runner environment. This review is therefore independent, not comparative.

🛡️ Architecture Security Analysis

Network Security Assessment

Dual-layer filtering is correctly implemented:

1. Host-level (src/host-iptables.ts) — A dedicated AWF-FIREWALL chain is inserted at position 1 in DOCKER-USER, ensuring all container egress goes through it before Docker's default rules. The chain terminates with a catch-all REJECT --reject-with icmp-port-unreachable.
2. Container-level (containers/agent/setup-iptables.sh) — The iptables init container DNATs HTTP/HTTPS to Squid; the OUTPUT filter chain has explicit DROP for TCP and UDP.
3. Domain ACL (Squid) — dstdomain and dstdom_regex ACLs enforce an allowlist; the default rule denies all unconfigured domains.

IPv6 is handled: when ip6tables is unavailable, sysctl net.ipv6.conf.all.disable_ipv6=1 disables IPv6 system-wide. When available, rules mirror IPv4.

DNS exfiltration is mitigated: DNS traffic is restricted to configured upstream servers in both the host-level chain and the container-level OUTPUT chain; Docker embedded DNS (127.0.0.11) is the only resolver inside the container.

Cloud metadata (169.254.0.0/16) is explicitly blocked by host-iptables.ts:497.

⚠️ Finding M-1: ICMP Not Blocked at Container Level

File: containers/agent/setup-iptables.sh:400-407
Confirmed: Yes (code inspection)

# setup-iptables.sh lines 404-407
iptables -A OUTPUT -p tcp -j DROP
iptables -A OUTPUT -p udp -j DROP
# ICMP is NOT dropped here

The container-level OUTPUT filter chain only drops TCP and UDP; ICMP is silently passed. The iptables default OUTPUT policy is ACCEPT, so ICMP packets egress the container unrestricted from the network namespace perspective. They are ultimately rejected by the host-level catch-all REJECT in AWF-FIREWALL, but:

• If the host-level rules fail to install (timing, error, or cleanup race), ICMP is fully unfiltered.
• ICMP tunnelling (e.g. icmptunnel, ptunnel) becomes possible if the host rules are ever absent.
• Defence-in-depth principle requires the container level to be self-sufficient.

Recommendation: Add iptables -A OUTPUT -p icmp -j DROP (and the equivalent ip6tables rule) after the UDP DROP line in setup-iptables.sh.

Container Security Assessment

Capabilities are carefully managed:

• Squid container – drops all dangerous caps (SYS_ADMIN, SYS_PTRACE, NET_RAW, SETFCAP, etc.).
• Agent container – starts with SYS_CHROOT + SYS_ADMIN (needed for procfs mount), but entrypoint.sh calls capsh --drop=cap_sys_chroot,cap_sys_admin before executing user code. NET_ADMIN is never granted to the agent.
• iptables-init container – receives NET_ADMIN + NET_RAW, cap_drop: ALL, no-new-privileges:true, and is ephemeral (exits after writing the ready signal).
• All sidecar containers use cap_drop: ALL and no-new-privileges:true.

no-new-privileges:true is applied to the agent container (src/docker-manager.ts:1183).

apparmor:unconfined is set to allow the procfs mount before capability drop (src/docker-manager.ts:1185). This is a deliberate trade-off with documented justification.

Resource limits: Memory (6g), PID limit (1000), and cpu_shares are set. Hard CPU quota (cpus) is not configured (see Low findings below).

⚠️ Finding M-2: Squid Config Injection via Newline in --allow-urls Pattern

File: src/squid-config.ts:153-156, src/cli.ts:1627-1668
Confirmed: Yes (proof-of-concept demonstrated)

URL patterns passed to --allow-urls are validated for protocol prefix, path presence, and broad-pattern blocklists, but not sanitised for embedded newlines. The patterns are inserted verbatim into the Squid config:

// src/squid-config.ts:153-155
const urlAcls = urlPatterns
  .map((pattern, i) => `acl allowed_url_\$\{i} url_regex \$\{pattern}`)
  .join('\n');
```

**Proof of concept:**
```
Input:  --allow-urls 'https://github.com/test\nhttp_access allow all'

Generated squid.conf:
  acl allowed_url_0 url_regex https://github.com/test
  http_access allow all

This injects http_access allow all into the Squid config, bypassing all domain ACL filtering.

Exploitability context: An attacker must control the --allow-urls CLI argument. In interactive use this is low-risk (self-inflicted). In agentic workflows where awf is invoked with arguments derived from untrusted inputs (issue bodies, repository names, etc.), this becomes a meaningful injection vector.

Recommendation: Sanitise URL patterns by rejecting any string containing \n, \r, or whitespace other than within the URL itself before use in generateSquidConfig. For example:

// In cli.ts validation block, add:
if (/[\r\n]/.test(url)) {
  logger.error(`URL pattern "\$\{url}" contains illegal characters (newline)`);
  process.exit(1);
}

Similarly, add sanitisation in generateSslBumpConfig / generateSquidConfig as a defence-in-depth backstop.

Domain Validation Assessment

src/domain-patterns.ts demonstrates careful security thinking:

• Overly broad wildcards (*, *.*, *.*.*) are rejected with clear error messages.
• Wildcard-to-regex conversion escapes all metacharacters except *, avoiding ReDoS via [a-zA-Z0-9.-]* instead of .*.
• Input length is limited to 512 characters before regex evaluation.
• Protocol-specific ((redacted) https://`) ACLs are correctly generated.
• Redundant subdomain entries are deduplicated to avoid ACL bloat.

No significant issues found here.

Input Validation Assessment

• All execa calls pass arguments as arrays (never as shell strings), eliminating shell injection.
• Port specs are validated with isValidPortSpec() before insertion into iptables commands.
• Domain strings go through validateDomainOrPattern() before Squid config generation.
• Docker Compose YAML is generated via js-yaml serialisation (not string templates), preventing YAML injection.
• The $ character in agentCommand is escaped to $$ for Docker Compose interpolation (src/docker-manager.ts:1209).

⚠️ Threat Model (STRIDE)

🎯 Attack Surface Map

📋 Evidence Collection

grep -n "DROP\|REJECT\|icmp" containers/agent/setup-iptables.sh
# Result: lines 405,407 only drop -p tcp and -p udp; no -p icmp rule

url = 'https://github.com/test\nhttp_access allow all'
result = [d.strip() for d in url.split(',')]
# result[0] = 'https://github.com/test\nhttp_access allow all'
# passes all validation checks (has https://, has '/', no broad pattern match)
# Generated ACL: "acl allowed_url_0 url_regex https://github.com/test\nhttp_access allow all"
# Squid interprets newline as end of directive -> injects "http_access allow all"

concerning = ['unshare','setns','mount','open_by_handle_at','clone','clone3']
# All confirmed SCMP_ACT_ALLOW
```

**AppArmor:**
```
src/docker-manager.ts:1185: 'apparmor:unconfined'
# Justified comment: needed for procfs mount before cap drop
```

**Npm audit:**
```
found 0 vulnerabilities

✅ Recommendations

🔴 Medium – Fix Soon

1. Sanitise --allow-urls patterns for embedded newlines
File: src/cli.ts (validation block around line 1634) and src/squid-config.ts:154
Add a check that rejects any URL pattern containing \r or \n, and as backstop strip newlines in generateSslBumpConfig before writing to the config file. This prevents Squid config injection from controlling the URL regex input.

2. Block ICMP at container level (defence-in-depth)
File: containers/agent/setup-iptables.sh (after line 407)
Add explicit ICMP drop rules:

iptables -A OUTPUT -p icmp -j DROP
[ "$IP6TABLES_AVAILABLE" = true ] && ip6tables -A OUTPUT -p icmpv6 -j DROP || true

This makes the container-level firewall self-sufficient, removing dependence on the host-level fallback for ICMP filtering.

🟡 Low – Plan to Address

3. Set hard CPU quota
File: src/docker-manager.ts agent service definition
Add cpus: '2' (or a configurable value) alongside cpu_shares. cpu_shares only affects scheduling weight when the system is under contention; a hard cpus limit prevents CPU-based DoS.

4. Consider removing open_by_handle_at from seccomp allowlist
File: containers/agent/seccomp-profile.json
open_by_handle_at was the foundation of the "shocker" container escape. While modern kernels and capability restrictions mitigate this, it is rarely needed by standard development toolchains. Removing it reduces the kernel attack surface with minimal compatibility impact.

5. Restore AppArmor confinement after procfs mount
Explanation: The agent container runs apparmor:unconfined to allow mount(2) for procfs before capsh drops SYS_ADMIN. Since capabilities are dropped before user code runs, AppArmor confinement could be reinstated at that point via apparmor_parser or by using a custom AWF-specific AppArmor profile that permits only the chroot and procfs mount operations while blocking everything else.

🔵 Info

6. Document --enable-dind firewall bypass prominently
The warning at src/docker-manager.ts:944 is logged at WARN level but does not appear in the help text. Consider adding a more visible WARNING to --enable-dind help text noting that this grants the agent complete network bypass capability via docker run.

7. No prior escape test results found
The firewall-escape-test workflow name was not present in the repository. If escape testing is done under a different name, consider making the naming consistent so audit tools can cross-reference results.

📈 Security Metrics

AI generated by Daily Security Review and Threat Modeling

• expires on Apr 6, 2026, 2:07 PM UTC