[Security Review] Daily Security Review — 2026-04-09 #1839
Replies: 7 comments
-
|
🔮 The ancient spirits stir beneath the firewall.
|
Beta Was this translation helpful? Give feedback.
-
|
🔮 The ancient spirits stir in the firewall halls.
|
Beta Was this translation helpful? Give feedback.
-
|
🔮 The ancient spirits stir, and the smoke-test wanderer has passed this hall.
|
Beta Was this translation helpful? Give feedback.
-
|
🔮 The ancient spirits stir, and the smoke-test agent has walked this thread.
|
Beta Was this translation helpful? Give feedback.
-
|
🔮 The ancient spirits stir in the firewall halls.
|
Beta Was this translation helpful? Give feedback.
-
|
🔮 The ancient spirits stir in the firewall halls.
|
Beta Was this translation helpful? Give feedback.
-
|
This discussion was automatically closed because it expired on 2026-04-16T13:50:46.076Z.
|
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
📊 Executive Summary
Review date: 2026-04-09 | Reviewer: Automated Security Agent (claude-sonnet-4.6)
Codebase snapshot:
github/gh-aw-firewallmain branchSecurity-critical files analyzed: 7,736 lines across
src/cli.ts,src/docker-manager.ts,src/squid-config.ts,src/host-iptables.ts,containers/agent/entrypoint.sh,containers/agent/setup-iptables.sh,containers/agent/seccomp-profile.jsonOverall posture: STRONG — Defence-in-depth architecture with well-layered controls. Three notable gaps identified, none critical in isolation but combinable in adversarial scenarios.
🔍 Findings from Firewall Escape Test
The escape-test summary file (
/tmp/gh-aw/escape-test-summary.txt) contained CI pipeline metadata from the Secret Digger (Copilot) workflow run, not structured escape-test results. Key data points:successsuccessnoop(no secrets or vulnerabilities reported)falsefalseInterpretation: The most recent automated secret/escape scan found no exposed secrets and reported no actionable findings. This is consistent with the code-level analysis below showing the one-shot-token LD_PRELOAD library,
unset_sensitive_tokens(), and token-scrubbing controls are properly implemented.🛡️ Architecture Security Analysis
Network Security Assessment
Evidence gathered from
containers/agent/setup-iptables.sh(460 lines) andsrc/host-iptables.ts(702 lines):Strong controls:
172.30.0.10:3128) via NAT OUTPUT rules (lines 341–342 ofsetup-iptables.sh)sysctl -w net.ipv6.conf.all.disable_ipv6=1(lines 48–49) — prevents IPv6 bypassDOCKER-USERchain (src/host-iptables.ts) enforces egress at the host bridge layer — secondary control independent of in-container rulesiptables -A OUTPUT -p tcp -j DROPandiptables -A OUTPUT -p udp -j DROPmountis in the allow list (needed forprocfsmount in entrypoint), butumount/umount2are explicitly blocked — prevents an agent from unmounting security boundariesunset_sensitive_tokens(),entrypoint.sh:369–395)/usr/local/lib/one-shot-token.so) protects API keys from being read more than onceSYS_ADMINis one of the most powerful Linux capabilities (encompasses ~25 distinct privileges including mount, setns, swapon, etc.). It is granted to the agent container to allow the entrypoint to mount procfs at/host/procbefore privilege drop. The capability is dropped viacapshbefore user code runs:Risk: The window between container start and
capshexecution runs the fullentrypoint.sh(~886 lines) as root withSYS_ADMIN. If the entrypoint script has a vulnerability,SYS_ADMINcould be exploited. Additionally, the seccomp profile's allowlist must permitmountfor the procfs operation, meaning mount syscalls are available throughout container lifetime to any root process.Mitigation quality: AppArmor (Docker's default profile) restricts mount even with
SYS_ADMIN; seccomp provides a second layer. The entrypoint is well-written with proper validation. Risk is residual rather than active, but worth tracking.UID/GID validation (Strong):
Prevents root-UID injection via
AWF_USER_UIDenv var.Domain Validation Assessment
Evidence from
src/domain-patterns.ts,src/squid-config.ts:Strong controls:
SQUID_DANGEROUS_CHARS = /[\s\0"';#]/` — blocks whitespace, null bytes, comment chars, quote chars that could inject Squid directivesassertSafeForSquidConfig()called before every domain interpolation intosquid.conf[a-zA-Z0-9.-]*(not.*) — prevents ReDoS attacks against the regex engine^+$— exact domain matching, no partial-match bypassfoo.bar.comis listed andbar.comis also listed, the subdomain entry is deduplicatedBackslash
\in domain names is not blocked bySQUID_DANGEROUS_CHARS. Domain names themselves have an additional validation layer (validateDomainOrPattern()) that should reject\, but the comment creates a trust boundary that is worth auditing: confirm thatvalidateDomainOrPatternreliably rejects\in all code paths.Input Validation Assessment
Evidence from
src/cli.ts:979–1001, 1498–1522:Shell argument escaping (Strong):
Standard POSIX single-quote escaping — correct and safe.
Single-argument passthrough (Informational):
✅ Recommendations
🔴 High — SYS_ADMIN narrowing
Finding: Agent container is granted
SYS_ADMINto mount procfs, butSYS_ADMINbundles ~25 sub-capabilities.Recommendation: Evaluate whether
CAP_SYS_PTRACE(required for procfs in some contexts) or a more targeted approach (writing a small init binary that mounts procfs then immediately drops capabilities without running the full entrypoint as root) could replace theSYS_ADMINgrant. Alternatively, consider a pre-built image where/procis already mounted, eliminating the need for runtime mounting.Files:
src/docker-manager.ts:1306,containers/agent/entrypoint.sh:910–914🟡 Medium — Add explicit ICMP DROP rule
Finding: The agent container's iptables OUTPUT filter chain drops TCP and UDP but has no rule for ICMP. Tools using
IPPROTO_ICMPsockets (available withoutNET_RAW) could send/receive ICMP packets to arbitrary external hosts, creating a potential low-bandwidth covert channel.Recommendation: Add an explicit ICMP drop rule after the UDP drop:
Also add the corresponding ip6tables rule for ICMPv6 (though IPv6 is disabled via sysctl, belt-and-suspenders is appropriate here).
Files:
containers/agent/setup-iptables.sh:428–430🟡 Medium — Increase log burst limit or add per-flow rate limiting
Finding:
--limit 10/min --limit-burst 20means that after 20 rapid blocked connection attempts, subsequent attempts are silently dropped from logs. A sophisticated agent could saturate the log burst (20 attempts) then exfiltrate via blocked protocols without a log entry.Recommendation: Either increase the burst limit (e.g.,
--limit-burst 100) or add nf_conntrack-based per-connection logging using--state NEWto ensure each new connection attempt is logged regardless of rate.Files:
containers/agent/setup-iptables.sh:426–430🟡 Medium — Validate backslash in domain names across all code paths
Finding:
SQUID_DANGEROUS_CHARSintentionally excludes\to support URL regex patterns via--allow-urls. Domain names validated byvalidateDomainOrPattern()should reject\, but this creates a path-specific trust boundary.Recommendation: Add an integration test confirming that a domain containing
\(e.g.,foo\.bar.com) is rejected through the full CLI parsing path (not just the unit-test level). Also add an explicit assertion that\in a plain domain (non-URL-pattern) context triggers rejection.Files:
src/domain-patterns.ts:26–35,src/squid-config.ts:71–79🟢 Low — Upgrade execa to v9
Finding:
package.jsonlistsexeca: "^5.1.1"(CJS, 2021). v9 (ESM, 2023) includes security improvements and is the maintained branch.Note from codebase memory: The project already uses ESM-handling workarounds (
jest-resolver.js,transformIgnorePatterns) for other ESM deps. An execa v9 upgrade would require updating import syntax (import { execa } from 'execa'— already done per source).Files:
package.json,src/docker-manager.ts:5🟢 Low — Document the single-argument command trust boundary
Finding: Single-argument commands are passed as raw strings to
/bin/bash -cinside the container. This is intentional but creates a security boundary assumption that should be explicit in the threat model documentation.Recommendation: Add a note to
docs/environment.mdorAGENTS.mdclarifying thatawftreats the command string as the agent's trusted instruction and that shell injection within the agent's own command scope is an accepted risk of the architecture (the container is the boundary).Files:
src/cli.ts:1498–1522,docs/environment.md🟢 Low — Consider seccomp
SCMP_ACT_LOGfor allowed-but-sensitive syscallsFinding: The seccomp profile has no logging for high-risk allowed syscalls (e.g.,
mount,setns,unshare). If a container escape via these is attempted but fails due to capability checks, it is invisible in logs.Recommendation: For the highest-risk allowed syscalls, consider adding
SCMP_ACT_LOGrules (seccomp audit logging) to create a detection layer separate from iptables logging.Files:
containers/agent/seccomp-profile.json📈 Security Metrics
Review conducted by automated security agent. All findings include specific file:line citations and are based on static analysis of the source code. Dynamic testing was not performed in this run.
Beta Was this translation helpful? Give feedback.
All reactions