You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This review covers a comprehensive, evidence-based security analysis of the gh-aw-firewall repository as of 2026-04-14. The codebase implements a layered defense strategy: host-level iptables via a DOCKER-USER chain, an in-container DNAT/filter stack, domain ACL enforcement through Squid, capability drops, seccomp, and one-shot token protection. The overall posture is strong with several well-designed defence-in-depth mechanisms. Six findings are reported, two rated medium severity and four low.
The most recent Secret Digger (Copilot) run (workflow run 24273493151, completed 2026-04-11) concluded with GH_AW_SECRET_VERIFICATION_RESULT: success and only noop outputs. The agent found no accessible secrets, confirming that credential-hiding via /dev/null overlays and one-shot token protection are working as intended. No novel escape vectors were discovered in that run.
lines 526–563: REJECT multicast (224.0.0.0/4), link-local (169.254.0.0/16)
line 539: LOG + REJECT all UDP (DNS exfiltration prevention)
line 552: LOG + REJECT everything else (default deny)
The FW_WRAPPER chain correctly allows only:
Squid source IP (unrestricted outbound as the proxy)
Established/related return traffic
Localhost and DNS to configured upstream servers
Squid's proxy port, API proxy sidecar ports, optional host gateway
Container-level (setup-iptables.sh)
IPv6 is explicitly disabled via sysctl at line 46:
sysctl -w net.ipv6.conf.all.disable_ipv6=1
```This prevents Happy Eyeballs from bypassing IPv4 DNAT rules.Docker embedded DNS DNAT rules are preserved before NAT flush (lines 80–100) — a subtle but important correctness fix.Dangerous ports are blocked via NAT RETURN + filter DROP with audit logging.**Finding M-1 — DANGEROUS_PORTS mismatch between iptables and Squid (Medium)**`src/squid-config.ts` blocks 21 ports;`containers/agent/setup-iptables.sh` blocks only 15. The following 6 ports have Squid ACL protection but **no iptables block**:| Port | Service ||------|---------|| 5984 | CouchDB HTTP || 6984 | CouchDB SSL || 8086 | InfluxDB HTTP API || 8088 | InfluxDB RPC || 9200 | Elasticsearch HTTP || 9300 | Elasticsearch transport |Evidence:```# squid-config.ts lines 15–36 (21 ports)
5984, 6984, 8086, 8088, 9200, 9300, ...
# setup-iptables.sh lines 312–328 (15 ports)
22, 23, 25, 110, 143, 445, 1433, 1521, 3306, 3389, 5432, 6379, 27017, 27018, 28017
The default DROP at the end of the container filter chain and the host-level default REJECT still block these ports for all non-HTTP/HTTPS traffic. The practical risk is low because only ports 80 and 443 (and any explicitly allowed ports) are ever DNAT'd to Squid. However, the inconsistency could introduce a gap if the overall default-deny rules are ever changed.
Finding L-1 — ICMP not explicitly blocked at container iptables level (Low)
setup-iptables.sh only adds DROP rules for TCP and UDP. ICMP is not explicitly handled in the container's OUTPUT chain. The host-level FW_WRAPPER default REJECT catches all other traffic (including ICMP), so external ICMP exfiltration is blocked. Within the AWF subnet (e.g., pinging Squid at 172.30.0.10), ICMP is reachable from the agent. This is a very limited channel with no practical exfiltration value but deviates from the principle of explicit deny.
This is a well-designed split: NET_ADMIN never enters the agent container. SYS_CHROOT and SYS_ADMIN are granted to the agent for the procfs mount and chroot, then dropped via capsh --drop before user code runs (entrypoint.sh lines 806–840).
Finding M-2 — AppArmor set to unconfined on agent container (Medium)
// src/docker-manager.ts line 1409'apparmor:unconfined',```Justification in the codebase is that Docker's default AppArmor profile blocks the `mount` syscall needed for procfs. While SYS_ADMIN is dropped before user code runs, the window between container start and `capsh--drop` executes without AppArmor protection. A custom AppArmor profile permitting only the needed `mount-tproc` transition would close this window without sacrificing security intent. CIS Docker Benchmark 5.1 explicitly warns against setting containers to unconfined.**Seccomp profile (`containers/agent/seccomp-profile.json`)**```
mount: SCMP_ACT_ALLOW←neededforprocfs
unshare: SCMP_ACT_ALLOW←neededfornamespaceisolation
setns: SCMP_ACT_ALLOW←neededfornamespaceops
clone: SCMP_ACT_ALLOW
ptrace: SCMP_ACT_ERRNO✓blockedkexec_load,init_module,pivot_root,umount: SCMP_ACT_ERRNO✓blockedprocess_vm_readv/writev: SCMP_ACT_ERRNO✓blocked
ptrace, process_vm_readv, process_vm_writev, and kexec_load are correctly blocked. mount, unshare, setns, and clone are allowed because they are required. Without CAP_SYS_ADMIN (dropped before user code) and without CAP_NET_ADMIN (never granted), the attack surface from these syscalls is materially limited.
Finding L-2 — unshare/setns allowed in seccomp + user namespace consideration (Low)
unshare without CLONE_NEWUSER requires capabilities that are dropped before user code runs. However, CLONE_NEWUSER (user namespace creation) does not require any capability in the kernel default configuration. An attacker could use unshare --user to create a new user namespace with a UID 0 mapping, then attempt further privilege escalation. Docker's default --userns-remap is not in use here. The seccomp profile does not restrict clone flags. The practical risk is mitigated by no-new-privileges:true and the seccomp profile, but a dedicated seccomp block on CLONE_NEWUSER would be a stronger control.
Domain Validation Assessment
src/domain-patterns.ts lines 153–210
exportconstSQUID_DANGEROUS_CHARS=/[\s\0"'`;#]/;// line 27constDOMAIN_DANGEROUS_CHARS=/[\s\0"'`;#\\]/;// line 176
Backslashes (domain validation) — the SQUID_DANGEROUS_CHARS regex does NOT include \ because URL patterns legitimately use it, but the domain validator adds it
Overly broad patterns: *, *.*, patterns matching only [*.]+
Double dots, bare dots
The assertSafeForSquidConfig() in squid-config.ts provides a defence-in-depth check at the point of interpolation. This two-layer validation is well-designed.
Finding L-3 — Unicode/IDN domain normalization not validated (Low)
The validators check ASCII special characters but do not normalise or reject Unicode/IDN domains. An attacker with control of --allow-domains values could supply an IDN domain (e.g., xn--githu-yta.com) that looks similar to a legitimate domain but reaches a different host. This is low risk because --allow-domains is a trusted CLI input (operator-controlled), but an IDN spoofing check would improve supply-chain trust.
// Single argument → treated as a complete shell command string (no escaping)// Multiple arguments → each is shell-escaped via escapeShellArg()
The comment at line 1597 explains the design choice clearly. Single-argument mode passes the command string verbatim to bash -c, which is correct for shell variable expansion. The trust assumption is documented. No injection vectors from user-controlled external sources (e.g., issue body, PR title) were found in the input path.
execa usage: All subprocess calls in src/docker-manager.ts use execa with argument arrays (not shell interpolation), preventing shell injection.
Finding L-4 — AWF_USER_UID/AWF_USER_GID validated only for format, not minimum value (Low)
# entrypoint.sh lines 12–36if! [[ "$HOST_UID"=~ ^[0-9]+$ ]];thenexit 1;fiif [ "$HOST_UID"-eq 0 ];then ... exit 1;fi```Root (UID 0) is blocked. System UIDs 1-999 are NOT blocked in the shell entrypoint (only in the TypeScript CLI at `docker-manager.ts` line 109). If the Docker image is used directly (not via the CLI), UIDs 1-999 (e.g., `daemon=1`, `bin=2`, `sys=3`) are accepted. While system UIDs rarely have meaningful privileges inside the container, defence-in-depth suggests blocking them in the entrypoint as well.---## ⚠️ Threat Model (STRIDE)| Category | Threat | Evidence | Likelihood | Impact | Severity ||----------|--------|----------|-----------|--------|----------||**Spoofing**| Attacker-controlled domain passes validation via IDN homograph | L-3 | Low | Medium | Low ||**Spoofing**| Container identity spoofing via UID conflict |`entrypoint.sh:50-60` WARN path | Very Low | Low | Low ||**Tampering**| Squid config injection via `--allow-domains`| Blocked by `SQUID_DANGEROUS_CHARS` check | Very Low | High | Low (mitigated) ||**Tampering**| iptables rules modified by agent after init | NET_ADMIN never granted to agent | Very Low | Critical | Low (mitigated) ||**Tampering**| Procfs-based container info leakage |`ptrace` blocked in seccomp | Very Low | Medium | Low (mitigated) ||**Repudiation**| Agent traffic not attributed to specific process | UID logged via `--log-uid`, no PID | Medium | Low | Low ||**Information Disclosure**| Token leakage via `/proc/self/environ`| Protected by one-shot-token LD_PRELOAD | Very Low | Critical | Low (mitigated) ||**Information Disclosure**| Credential files readable via credential mounts | Blocked by `/dev/null` overlays | Very Low | High | Low (mitigated) ||**Denial of Service**| Container resource exhaustion |`mem_limit=6g`, `pids_limit=1000`| Medium | Medium | Low (mitigated) ||**Denial of Service**| Docker network pool exhaustion from cleanup failures | Pre-test cleanup in CI scripts | Medium | Medium | Low (mitigated) ||**Elevation of Privilege**| SYS_ADMIN window before capsh drop |`apparmor:unconfined` — M-2 | Low | High | Medium ||**Elevation of Privilege**| User namespace creation via `unshare --user`| Seccomp allows clone/unshare — L-2 | Low | Medium | Low ||**Elevation of Privilege**| API key theft via api-proxy | Sidecar strips inbound auth headers (line 43–47 `server.js`) | Very Low | High | Low (mitigated) |---## 🎯 Attack Surface Map| Surface | Entry Point | Current Protections | Residual Risk ||---------|-------------|--------------------|----||**Domain whitelist**|`src/domain-patterns.ts:153`| Dangerous char rejection, broad-pattern checks | IDN not normalised (L-3) ||**Squid config generation**|`src/squid-config.ts:assertSafeForSquidConfig`| Double validation + escape | None identified ||**Container network egress**|`containers/agent/setup-iptables.sh`| DNAT→Squid, DANGEROUS_PORTS NAT RETURN, default DROP | 6-port gap vs Squid (M-1) ||**Host network egress**|`src/host-iptables.ts:FW_WRAPPER`| Default REJECT, DNS whitelist, multicast block | ICMP intra-subnet (L-1) ||**Container capabilities**|`src/docker-manager.ts:1393`| cap_drop, seccomp, no-new-privileges | AppArmor unconfined (M-2); user ns (L-2) ||**Container filesystem**|`src/docker-manager.ts:1220-1302`| Selective mounts, /dev/null overlays on creds | None identified ||**API key exposure**|`containers/agent/one-shot-token/`| LD_PRELOAD clears /proc/self/environ | Heuristic 1s wait||**API proxy auth**|`containers/api-proxy/server.js:43`| Strips inbound auth headers | Port 10003 gap (L-5*) ||**Docker socket**|`src/docker-manager.ts:1170-1180`| Exposed only via explicit opt-in `--expose-docker`; default `/dev/null`| Operator trust required |*L-5: The host-level iptables rule forthe API proxy uses a contiguous port range `10000:10004` (lines 5bin host-iptables), but port 10003 is now assigned to Gemini. This was correct as of AGENTS.md saying only 10000, 10001, 10002, 10004, but 10003 is now in use (`API_PROXY_PORTS.GEMINI = 10003`). The AGENTS.md comment appears outdated — the implementation is correct.---## 📋 Evidence Collection<details><summary>DANGEROUS_PORTS comparison</summary>**squid-config.ts** (21 ports): 22, 23, 25, 110, 143, 445, 1433, 1521, 3306, 3389, 5432, **5984**, 6379, **6984**, **8086**, **8088**, **9200**, **9300**, 27017, 27018, 28017**setup-iptables.sh** (15 ports): 22, 23, 25, 110, 143, 445, 1433, 1521, 3306, 3389, 5432, 6379, 27017, 27018, 28017Missing from iptables: **5984, 6984, 8086, 8088, 9200, 9300**</details><details><summary>seccomp profile dangerous syscall actions</summary>```mount: SCMP_ACT_ALLOW (needed for procfs)unshare: SCMP_ACT_ALLOW (needed for namespace isolation)setns: SCMP_ACT_ALLOWclone: SCMP_ACT_ALLOWptrace: SCMP_ACT_ERRNO ✓kexec_load: SCMP_ACT_ERRNO ✓init_module: SCMP_ACT_ERRNO ✓pivot_root: SCMP_ACT_ERRNO ✓process_vm_readv: SCMP_ACT_ERRNO ✓process_vm_writev: SCMP_ACT_ERRNO ✓
AppArmor unconfined rationale
// src/docker-manager.ts:1402-1410// Apply seccomp profile and no-new-privileges to restrict dangerous syscalls// AppArmor is set to unconfined to allow mounting procfs at /host/proc// (Docker's default AppArmor profile blocks mount). This is safe because SYS_ADMIN is// dropped via capsh before user code runs, so user code cannot mount anything.security_opt: ['no-new-privileges:true',`seccomp=\$\{config.workDir}/seccomp-profile.json`,'apparmor:unconfined',],
Escape test run context
From /tmp/gh-aw/escape-test-summary.txt (workflow run 24273493151):
Workflow: Secret Digger (Copilot), run 2026-04-11
GH_AW_AGENT_CONCLUSION: success
GH_AW_SECRET_VERIFICATION_RESULT: success
Agent produced only noop outputs — no secrets discovered or leaked
✅ Recommendations
🔴 Critical
None identified.
🟠 High
None identified.
🟡 Medium
M-1 — Sync DANGEROUS_PORTS between setup-iptables.sh and squid-config.ts
Add the 6 missing ports (5984, 6984, 8086, 8088, 9200, 9300) to the DANGEROUS_PORTS array in containers/agent/setup-iptables.sh. Consider extracting a single source-of-truth and auto-generating both lists from it (e.g., a JSON file read at build time).
M-2 — Replace apparmor:unconfined with a minimal custom AppArmor profile
Create a custom AppArmor profile that permits mount only for proc filesystem types and only before capsh --drop executes. This eliminates the unprotected window during container startup without impacting functionality.
🟢 Low
L-1 — Add explicit ICMP DROP to container iptables
Add iptables -A OUTPUT -p icmp -j DROP to setup-iptables.sh for completeness and defence-in-depth, even though the host-level FW_WRAPPER already catches this.
L-2 — Restrict CLONE_NEWUSER in seccomp profile
Consider using a seccomp args filter on clone and unshare to deny CLONE_NEWUSER flag (value 0x10000000) while still permitting other clone/unshare operations. This closes the unprivileged user namespace creation vector.
L-3 — Add IDN/Punycode normalization to domain validation
In validateDomainOrPattern(), run inputs through an IDN normalization step (or reject xn-- prefixes from user-supplied plain domains) to prevent IDN homograph attacks in the domain whitelist.
L-4 — Block system UIDs (1-999) in entrypoint.sh
Mirror the TypeScript validation at docker-manager.ts:109 in containers/agent/entrypoint.sh to reject HOST_UID < 1000 when the image is used directly.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
-
📊 Executive Summary
This review covers a comprehensive, evidence-based security analysis of the
gh-aw-firewallrepository as of 2026-04-14. The codebase implements a layered defense strategy: host-level iptables via aDOCKER-USERchain, an in-container DNAT/filter stack, domain ACL enforcement through Squid, capability drops, seccomp, and one-shot token protection. The overall posture is strong with several well-designed defence-in-depth mechanisms. Six findings are reported, two rated medium severity and four low.Security lines analysed: 4,773 across seven critical files
Attack surfaces identified: 9
Findings: 2 Medium · 4 Low · 0 High/Critical
🔍 Findings from Firewall Escape Test
The most recent
Secret Digger (Copilot)run (workflow run24273493151, completed 2026-04-11) concluded withGH_AW_SECRET_VERIFICATION_RESULT: successand only noop outputs. The agent found no accessible secrets, confirming that credential-hiding via/dev/nulloverlays and one-shot token protection are working as intended. No novel escape vectors were discovered in that run.🛡️ Architecture Security Analysis
Network Security Assessment
Host-level (DOCKER-USER chain —
src/host-iptables.ts)The
FW_WRAPPERchain correctly allows only:Container-level (setup-iptables.sh)
IPv6 is explicitly disabled via sysctl at line 46:
The default DROP at the end of the container filter chain and the host-level default REJECT still block these ports for all non-HTTP/HTTPS traffic. The practical risk is low because only ports 80 and 443 (and any explicitly allowed ports) are ever DNAT'd to Squid. However, the inconsistency could introduce a gap if the overall default-deny rules are ever changed.
Finding L-1 — ICMP not explicitly blocked at container iptables level (Low)
setup-iptables.shonly addsDROPrules for TCP and UDP. ICMP is not explicitly handled in the container's OUTPUT chain. The host-levelFW_WRAPPERdefault REJECT catches all other traffic (including ICMP), so external ICMP exfiltration is blocked. Within the AWF subnet (e.g., pinging Squid at 172.30.0.10), ICMP is reachable from the agent. This is a very limited channel with no practical exfiltration value but deviates from the principle of explicit deny.Container Security Assessment
Capability management (
src/docker-manager.tslines 1385–1410)This is a well-designed split: NET_ADMIN never enters the agent container. SYS_CHROOT and SYS_ADMIN are granted to the agent for the procfs mount and chroot, then dropped via
capsh --dropbefore user code runs (entrypoint.sh lines 806–840).Finding M-2 — AppArmor set to
unconfinedon agent container (Medium)ptrace,process_vm_readv,process_vm_writev, andkexec_loadare correctly blocked.mount,unshare,setns, andcloneare allowed because they are required. Without CAP_SYS_ADMIN (dropped before user code) and without CAP_NET_ADMIN (never granted), the attack surface from these syscalls is materially limited.Finding L-2 —
unshare/setnsallowed in seccomp + user namespace consideration (Low)unsharewithoutCLONE_NEWUSERrequires capabilities that are dropped before user code runs. However,CLONE_NEWUSER(user namespace creation) does not require any capability in the kernel default configuration. An attacker could useunshare --userto create a new user namespace with a UID 0 mapping, then attempt further privilege escalation. Docker's default--userns-remapis not in use here. The seccomp profile does not restrictcloneflags. The practical risk is mitigated byno-new-privileges:trueand the seccomp profile, but a dedicated seccomp block onCLONE_NEWUSERwould be a stronger control.Domain Validation Assessment
src/domain-patterns.tslines 153–210The
validateDomainOrPattern()function rejects:\because URL patterns legitimately use it, but the domain validator adds it*,*.*, patterns matching only[*.]+The
assertSafeForSquidConfig()insquid-config.tsprovides a defence-in-depth check at the point of interpolation. This two-layer validation is well-designed.Finding L-3 — Unicode/IDN domain normalization not validated (Low)
The validators check ASCII special characters but do not normalise or reject Unicode/IDN domains. An attacker with control of
--allow-domainsvalues could supply an IDN domain (e.g.,xn--githu-yta.com) that looks similar to a legitimate domain but reaches a different host. This is low risk because--allow-domainsis a trusted CLI input (operator-controlled), but an IDN spoofing check would improve supply-chain trust.Input Validation Assessment
Shell argument handling (
src/cli.tslines 1578–1601)The comment at line 1597 explains the design choice clearly. Single-argument mode passes the command string verbatim to
bash -c, which is correct for shell variable expansion. The trust assumption is documented. No injection vectors from user-controlled external sources (e.g., issue body, PR title) were found in the input path.execausage: All subprocess calls insrc/docker-manager.tsuseexecawith argument arrays (not shell interpolation), preventing shell injection.Finding L-4 —
AWF_USER_UID/AWF_USER_GIDvalidated only for format, not minimum value (Low)AppArmor unconfined rationale
Escape test run context
From
/tmp/gh-aw/escape-test-summary.txt(workflow run 24273493151):Secret Digger (Copilot), run 2026-04-11GH_AW_AGENT_CONCLUSION: successGH_AW_SECRET_VERIFICATION_RESULT: success✅ Recommendations
🔴 Critical
None identified.
🟠 High
None identified.
🟡 Medium
M-1 — Sync DANGEROUS_PORTS between
setup-iptables.shandsquid-config.tsAdd the 6 missing ports (5984, 6984, 8086, 8088, 9200, 9300) to the
DANGEROUS_PORTSarray incontainers/agent/setup-iptables.sh. Consider extracting a single source-of-truth and auto-generating both lists from it (e.g., a JSON file read at build time).M-2 — Replace
apparmor:unconfinedwith a minimal custom AppArmor profileCreate a custom AppArmor profile that permits
mountonly forprocfilesystem types and only beforecapsh --dropexecutes. This eliminates the unprotected window during container startup without impacting functionality.🟢 Low
L-1 — Add explicit ICMP DROP to container iptables
Add
iptables -A OUTPUT -p icmp -j DROPtosetup-iptables.shfor completeness and defence-in-depth, even though the host-level FW_WRAPPER already catches this.L-2 — Restrict
CLONE_NEWUSERin seccomp profileConsider using a seccomp
argsfilter oncloneandunshareto denyCLONE_NEWUSERflag (value0x10000000) while still permitting other clone/unshare operations. This closes the unprivileged user namespace creation vector.L-3 — Add IDN/Punycode normalization to domain validation
In
validateDomainOrPattern(), run inputs through an IDN normalization step (or rejectxn--prefixes from user-supplied plain domains) to prevent IDN homograph attacks in the domain whitelist.L-4 — Block system UIDs (1-999) in entrypoint.sh
Mirror the TypeScript validation at
docker-manager.ts:109incontainers/agent/entrypoint.shto rejectHOST_UID < 1000when the image is used directly.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions