[Security Review] Daily Security Review and Threat Modeling — 2026-03-18

[github-actions[bot]]

📊 Executive Summary

A comprehensive security review of the gh-aw-firewall codebase was performed on 2026-03-18, covering 6,092 lines of security-critical code across the network firewall, container orchestration, and domain validation layers. The overall security posture is strong, with defence-in-depth implemented at multiple layers (host iptables → container NAT → Squid L7 proxy). Four focused findings are documented below, ranging from medium to low severity.

Metric	Value
Lines of security-critical code reviewed	6,092
Attack surfaces identified	8
Critical findings	0
High findings	0
Medium findings	2
Low findings	2
Informational notes	3
npm moderate vulnerabilities	4 (dev/doc tooling)

---

🔍 Findings from Escape Test Workflow

The firewall-escape-test workflow was not found by its shorthand name in the agenticworkflows registry (workflows visible: security-review, secret-digger-*, security-guard). The daily security-review log retrieval was also blocked due to missing git binary in the runner path. This review therefore proceeds entirely on static codebase analysis.

---

🛡️ Architecture Security Analysis

Network Security Assessment — Good

The firewall implements two independent filtering layers:

Host layer (src/host-iptables.ts) — A dedicated FW_WRAPPER chain is inserted at position 1 in DOCKER-USER, ensuring all container-origin traffic is filtered before Docker's own rules. Rules are well-ordered: Squid IP is explicitly allowed first, then DNS upstreams, then established/related state, then REJECT for everything else (including multicast and link-local 169.254.0.0/16). IPv6 is handled via ip6tables when available; if not, IPv6 is disabled system-wide via sysctl.

Container NAT layer (containers/agent/setup-iptables.sh) — All port-80/443 TCP is DNAT'd to Squid. Dangerous ports (SSH:22, SMTP:25, databases 1433/3306/5432/6379/27017, RDP:3389) are explicitly RETURN'd from NAT and then DROP'd by filter chain. Final rules drop all TCP and UDP not previously allowed.

Finding M-1 (Medium): ICMP not blocked in agent container OUTPUT chain

# Evidence: setup-iptables.sh final drop rules
$ grep "iptables.*DROP\|iptables.*REJECT" containers/agent/setup-iptables.sh | tail -5
319: iptables -A OUTPUT -p tcp -j DROP
320: iptables -A OUTPUT -p udp -j DROP
# Note: no -p icmp DROP rule exists
````

The agent container's `OUTPUT` filter chain drops TCP and UDP, but does not explicitly DROP or REJECT ICMP (`-p icmp`). This means an agent can send arbitrary ICMP traffic (echo requests, timestamp requests, etc.) to any destination, which:
- Allows basic ICMP ping reachability probing
- Creates a low-bandwidth covert channel via ICMP payload (ping tunneling)
- Bypasses the "deny all non-Squid" intent for raw packet channels

The host-level `FW_WRAPPER` chain (in `src/host-iptables.ts`) does block most outbound traffic with a final `REJECT`, but only after specific ACCEPT rules. The REJECT rule at the end of `FW_WRAPPER` uses `-j REJECT --reject-with icmp-port-unreachable` and is the default-deny for unmatched traffic. However, the container-level filter chain explicitly only drops TCP and UDP, leaving ICMP in a ACCEPT-by-default state at the container level.

**Recommendation:** Add `iptables -A OUTPUT -p icmp -j DROP` to `setup-iptables.sh` after the TCP/UDP DROP rules. Allow only necessary ICMP types (e.g., type 0 echo-reply for responses) if needed.

---

### Container Security Assessment — **Good with noted trade-offs**

**Capability management is well-designed:**
- `iptables-init` container: `NET_ADMIN` + `NET_RAW`, drops all others (`cap_drop: ALL`)
- Agent container (chroot mode): starts with `SYS_CHROOT` + `SYS_ADMIN`, drops `NET_ADMIN`, `NET_RAW`, `SYS_PTRACE`, `SYS_MODULE`, `SYS_RAWIO`, `MKNOD`; then `capsh` drops `SYS_CHROOT` + `SYS_ADMIN` before user code runs
- API proxy sidecar: `cap_drop: ALL`
- This eliminates the dangerous pattern of giving `NET_ADMIN` to the agent directly

**Seccomp profile blocks 28 high-risk syscalls** including `ptrace`, `process_vm_readv/writev`, `kexec_load`, `reboot`, `init_module`, `pivot_root`, `add_key`/`request_key`/`keyctl`, and `personality`. The profile uses `SCMP_ACT_ERRNO` (default deny block for the listed syscalls), leaving other syscalls allowed by the Docker default.

**Finding M-2 (Medium): AppArmor is set to `unconfined` for agent container**

````
# Evidence: src/docker-manager.ts:1025-1028
security_opt: [
  'no-new-privileges:true',
  `seccomp=\$\{config.workDir}/seccomp-profile.json`,
  'apparmor:unconfined',   // ← applied to agent container only
],

The comment explains this is required to allow mount(2) for procfs inside the chroot (Docker's default AppArmor profile docker-default blocks the mount syscall). While this is mitigated by:

1. SYS_ADMIN being dropped via capsh before user code runs
2. no-new-privileges:true being set
3. The seccomp profile blocking pivot_root and kexec_load

...disabling AppArmor removes a kernel-enforced security layer. If a future refactor inadvertently fails to drop SYS_ADMIN via capsh, or if capsh itself is exploited, there is no AppArmor backstop preventing filesystem manipulations.

Recommendation: Investigate creating a custom AppArmor profile that allows only the specific mount operation needed for procfs (e.g., mount fstype=proc), rather than going fully unconfined. This would restore AppArmor protection while allowing the necessary mounts.

---

Domain Validation Assessment — Strong

// Evidence: src/domain-patterns.ts
// Over-broad patterns rejected:
if (trimmed === '*') throw new Error("Pattern '*' matches all domains...");
if (trimmed === '*.*') throw new Error("Pattern '*.*' is too broad...");
if (/^[*.]+$/.test(trimmed) && trimmed.includes('*')) throw new Error("too broad");

// ReDoS prevention in wildcard regex:
const DOMAIN_CHAR_PATTERN = '[a-zA-Z0-9.-]*';  // safe character class vs .*

// Length guard before regex matching:
const MAX_DOMAIN_LENGTH = 512;
if (domainEntry.domain.length > MAX_DOMAIN_LENGTH) return false;

// Squid blocks direct-IP CONNECT:
acl dst_ipv4 dstdom_regex ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$
http_access deny dst_ipv4

The domain validation is carefully implemented. Wildcard patterns use a character-class based regex ([a-zA-Z0-9.-]*) instead of .* to prevent ReDoS. Over-broad patterns are rejected with meaningful errors. Squid independently blocks direct IP connections, providing defence-in-depth against domain filter bypass via numeric IPs.

---

Input Validation Assessment — Good

All execa calls use array-based arguments (no shell: true), preventing command injection:

// Evidence: src/docker-manager.ts
await execa('docker', ['network', 'inspect', NETWORK_NAME]);  // array args, no shell
await execa('iptables', ['-t', 'filter', '-N', CHAIN_NAME]);  // same pattern throughout

The escapeShellArg function in src/cli.ts:864-876 correctly uses single-quote wrapping with proper '\\'' escaping for single quotes within arguments. UID/GID values passed via environment are validated as numeric and non-zero in entrypoint.sh.

Finding L-1 (Low): disableIpv6ViaSysctl failure is only a warning, not a hard error

// Evidence: src/host-iptables.ts:69-78
async function disableIpv6ViaSysctl(): Promise(void) {
  try {
    await execa('sysctl', ['-w', 'net.ipv6.conf.all.disable_ipv6=1']);
    await execa('sysctl', ['-w', 'net.ipv6.conf.default.disable_ipv6=1']);
    ipv6DisabledViaSysctl = true;
    logger.info('IPv6 disabled via sysctl (ip6tables unavailable)');
  } catch (error) {
    logger.warn('Failed to disable IPv6 via sysctl:', error);  // ← only a warning
  }
}

When ip6tables is unavailable, the fallback is to disable IPv6 via sysctl. If sysctl fails (e.g., kernel restrictions, read-only /proc/sys), the firewall continues operating but IPv6 remains active and unfiltered. An agent could exfiltrate data or reach blocked services over IPv6.

Recommendation: Treat a sysctl failure here as a hard error — either abort with a clear message or at minimum escalate to logger.error and expose a --allow-ipv6-unfiltered opt-in flag.

---

Dependency Vulnerability Assessment

# Evidence: npm audit output
Vulns: {'info': 0, 'low': 0, 'moderate': 4, 'high': 0, 'critical': 0}
- js-yaml: moderate - prototype pollution in merge (<<)
- markdown-it: moderate - ReDoS
- markdownlint: moderate - (via markdown-it)
- markdownlint-cli2: moderate - (via js-yaml)

Finding L-2 (Low): js-yaml prototype pollution in markdownlint-cli2

The js-yaml vulnerability (prototype pollution via YAML << merge key) affects markdownlint-cli2, which is used in the documentation/linting pipeline. The js-yaml library is also used at runtime (src/docker-manager.ts generates Docker Compose YAML via js-yaml). However, js-yaml in the main dependency tree should be separately checked:

npm ls js-yaml
# If the runtime version is ≥ 4.0.0, it is NOT affected (prototype pollution was fixed in v4)
# The vulnerability affects 3.x

The markdown-it ReDoS only applies to documentation tooling, not runtime code.

Recommendation: Run npm ls js-yaml to confirm the runtime dependency resolves to js-yaml@4.x. Update markdownlint-cli2 to resolve the moderate advisories.

---

⚠️ Threat Model (STRIDE)

#	Category	Threat	Evidence	Likelihood	Impact	Mitigated?
T1	Info Disclosure	ICMP covert channel from agent to external host	setup-iptables.sh L319-320: only TCP/UDP dropped	Low	Low	❌ Partial
T2	Elevation of Privilege	AppArmor unconfined allows mount if capsh drop fails	docker-manager.ts:1028	Very Low	High	⚠️ Partial
T3	Spoofing / Bypass	--enable-dind mounts real Docker socket, agent creates unfiltered containers	docker-manager.ts:788-793 + log warning	Medium (when flag used)	Critical	⚠️ By design, warned
T4	Info Disclosure	IPv6 unfiltered if ip6tables unavailable AND sysctl fails	host-iptables.ts:69-78	Very Low	High	⚠️ Partial
T5	Repudiation	ICMP traffic not logged (not captured by Squid or iptables LOG rules)	No ICMP LOG rule in setup-iptables.sh or host-iptables.ts	Low	Low	❌ No
T6	DoS	pids_limit=1000 for agent; memory limited to 2g	docker-manager.ts:1031-1033	Low	Low	✅ Yes
T7	Tampering	DinD agent spawns new Docker container without awf-net restrictions	docker-manager.ts log warning on enableDind	Medium (when DinD used)	High	⚠️ Documented risk
T8	Spoofing	Direct IP-based bypass of domain ACL	Squid dst_ipv4/dst_ipv6 ACLs block this	Very Low	High	✅ Yes

---

🎯 Attack Surface Map

Surface	Location	Current Protection	Risk
--allow-domains input	src/cli.ts:1117, src/domain-patterns.ts	validateDomainOrPattern(), ReDoS-safe regex	Low
--enable-dind Docker socket	src/docker-manager.ts:788-793	Warning log; no network restriction on spawned containers	High (opt-in)
--enable-host-access bypass	src/docker-manager.ts:1062, setup-iptables.sh:175-214	Restricted to ports 80/443 + --allow-host-ports	Medium
Agent container ICMP output	containers/agent/setup-iptables.sh:319-320	TCP/UDP blocked; ICMP not blocked	Medium
IPv6 when ip6tables absent	src/host-iptables.ts:69-78	Sysctl disable (may fail silently)	Low–Medium
AppArmor bypass window	src/docker-manager.ts:1028, entrypoint.sh capsh drop	Seccomp + no-new-privileges backstop	Low
API proxy credential injection	containers/api-proxy/server.js	Credentials never reach agent env; sidecar routes through Squid	Low
UID/GID 0 injection	containers/agent/entrypoint.sh:20-30	Numeric validation + explicit 0-check	Low

---

📋 Evidence Collection

Commands run and key outputs

# 1. Agent container iptables — ICMP gap
$ grep "iptables.*DROP\|iptables.*REJECT" containers/agent/setup-iptables.sh | tail -5
iptables -A OUTPUT -p tcp -j DROP
iptables -A OUTPUT -p udp -j DROP
# Result: No ICMP DROP rule

# 2. AppArmor configuration
$ grep -n "apparmor" src/docker-manager.ts
1028:      'apparmor:unconfined',
# Only appears once — agent container

# 3. Seccomp blocked syscalls (28 total)
# Profile: SCMP_ACT_ERRNO (default-allow-with-blocklist)
# Blocked: ptrace, process_vm_readv/writev, kexec_load, reboot, init_module,
#          finit_module, delete_module, acct, swapon, swapoff, pivot_root,
#          syslog, add_key, request_key, keyctl, uselib, personality,
#          ustat, sysfs, vhangup, get_kernel_syms, query_module, create_module,
#          nfsservctl, umount, umount2

# 4. Resource limits
$ grep -n "mem_limit\|pids_limit\|memswap" src/docker-manager.ts
1031:  mem_limit: config.memoryLimit || '2g',
1032:  memswap_limit: config.memoryLimit || '2g',
1033:  pids_limit: 1000,

# 5. Capability management
$ grep -n "cap_add\|cap_drop" src/docker-manager.ts
319:  cap_drop: [NET_RAW, SYS_PTRACE, SYS_MODULE, SYS_RAWIO, MKNOD]  # non-chroot
1012: cap_add: [SYS_CHROOT, SYS_ADMIN]   # chroot mode, dropped by capsh
1014: cap_drop: [NET_ADMIN, NET_RAW, SYS_PTRACE, ...]
1154: cap_add: [NET_ADMIN, NET_RAW]  # iptables-init only
1155: cap_drop: ['ALL']

# 6. IPv6 sysctl failure handling
$ sed -n '69,78p' src/host-iptables.ts
  } catch (error) {
    logger.warn('Failed to disable IPv6 via sysctl:', error);  # only a warning
  }

# 7. npm audit
Vulns: {'moderate': 4, 'high': 0, 'critical': 0}
js-yaml: moderate - prototype pollution in merge (<<)
markdown-it: moderate - ReDoS

---

✅ Recommendations

🔴 Medium — Should fix soon

[M-1] Block ICMP in agent container OUTPUT chain

• File: containers/agent/setup-iptables.sh, after line 320
• Fix: Add iptables -A OUTPUT -p icmp -j DROP
• Rationale: Closes ICMP-based covert channel and ping tunneling bypass; consistent with "deny all non-Squid" intent

[M-2] Replace apparmor:unconfined with a minimal custom AppArmor profile

• File: src/docker-manager.ts:1028, new file containers/agent/apparmor-profile
• Fix: Create an AppArmor profile that allows only mount fstype=proc and denies everything else the default profile would deny
• Rationale: Restores kernel-level MAC enforcement; reduces blast radius if capsh drop fails

🟡 Low — Plan to address

[L-1] Treat IPv6 sysctl failure as hard error

• File: src/host-iptables.ts:69-78
• Fix: Re-throw the error after logging, or add a --allow-ipv6-unfiltered opt-in flag
• Rationale: Prevents silent security regression when IPv6 cannot be disabled

[L-2] Update dev dependencies to resolve moderate npm advisories

• File: package.json
• Fix: npm update markdownlint-cli2; confirm runtime js-yaml is v4.x via npm ls js-yaml
• Rationale: Eliminates moderate vulnerability surface, especially if js-yaml@3.x is transitively present in runtime path

ℹ️ Informational

[I-1] --enable-dind is an inherent firewall bypass — documented with a warning log, but should also appear prominently in user-facing help text and README. Spawned containers will not be subject to awf-net ACLs.

[I-2] Agent-level iptables LOG for ICMP — even if ICMP is eventually blocked, adding a LOG rule before the DROP (iptables -A OUTPUT -p icmp -j LOG --log-prefix "[FW_BLOCKED_ICMP] ") improves forensic visibility, consistent with how UDP and TCP are already logged.

[I-3] sysctl IPv6 disable also applies in container-level setup-iptables.sh — the same silent-failure risk exists there (sysctl ... || echo "[iptables] WARNING:..." rather than exit 1). Consider failing hard when the IPv6 disable is required but fails.

---

📈 Security Metrics

Metric	Value
Total lines of security-critical code reviewed	6,092
Files reviewed	6 (host-iptables.ts, docker-manager.ts, squid-config.ts, cli.ts, setup-iptables.sh, entrypoint.sh)
Attack surfaces mapped	8
Findings: Critical / High / Medium / Low	0 / 0 / 2 / 2
Syscalls blocked by seccomp	28
Dangerous ports blocked	15 (SSH, Telnet, SMTP, POP3, IMAP, SMB, MSSQL, OracleDB, MySQL, RDP, PostgreSQL, Redis, MongoDB×3)
Defence-in-depth layers	3 (host iptables → container NAT → Squid L7)
npm moderate vulnerabilities	4 (dev/doc tooling only)

AI generated by Daily Security Review and Threat Modeling

• expires on Mar 25, 2026, 2:03 PM UTC