[Security Review] Daily Security Review and Threat Modeling — 2026-03-10 #1201
Replies: 2 comments
-
|
🔮 The ancient spirits stir, and the oracle records that the smoke test agent was here. The omens are cast; the circuit hums with quiet approval.
|
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
📊 Executive Summary
This is a comprehensive security review of the gh-aw-firewall codebase conducted via static analysis of ~3,500 lines of security-critical code. The architecture is generally well-designed with multiple independent defense layers. No prior "Firewall Escape Test" workflow was found in the repository; the closest workflows are
security-guard.mdandsecurity-review.md.Security Posture: GOOD with 2 Medium-severity findings and 2 Low-severity findings. Zero npm dependency vulnerabilities. No critical flaws identified.
🔍 Findings from Prior Workflow Runs
No "Firewall Escape Test" workflow exists in this repository. The
security-reviewandsecurity-guardworkflows are present but recent log downloads failed. This review is therefore based entirely on static analysis of the current codebase.🛡️ Architecture Security Analysis
Network Security Assessment — ✅ Strong
Evidence gathered:
Positive findings:
src/squid-config.ts:388-396)8.8.8.8,8.8.4.4)net.ipv6.conf.all.disable_ipv6=1) to prevent an unfiltered bypass pathsrc/host-iptables.ts:548— rule 8 REJECTs all traffic not explicitly permitted, including ICMPFinding LOW-1: Agent container does not explicitly block ICMP or non-DNS UDP:
Mitigation: The host-level DOCKER-USER chain handles this. However, if the host iptables fail to initialize (permission error, race condition), the agent-level filter becomes the only layer, and ICMP/UDP would be unfiltered.
Container Security Assessment — ✅ Good with caveats
Evidence gathered:
Positive findings:
no-new-privileges:trueapplied (src/docker-manager.ts:884)NET_RAW,SYS_PTRACE,SYS_MODULE,SYS_RAWIO,MKNODcapshdropscap_net_admin,cap_sys_chroot,cap_sys_adminbefore user code runs (entrypoint.sh:278,606)AWF_CHROOT_ENABLEDis always set to'true'bydocker-manager.ts:479, ensuring all 3 high-risk capabilities are always dropped/dev/null:/host/var/run/docker.sock:roand/dev/null:/host/run/docker.sock:ro(docker-manager.ts:683-685)docker-manager.ts:889-891)Finding MEDIUM-1: Custom seccomp profile is weaker than Docker's default
Docker's default seccomp profile uses
SCMP_ACT_ERRNO(deny-by-default) with ~300+ explicitly allowed syscalls — an allowlist approach. The custom profile usesSCMP_ACT_ALLOW(allow-by-default) with a small denylist. This means syscalls likebpf,perf_event_open,setns,unshare,clonewith namespace flags,kcmp,lookup_dcookie, and many others that Docker's default profile blocks are permitted by this custom profile.Capability drops partially compensate (e.g.,
bpfrequiresCAP_NET_ADMINorCAP_SYS_ADMIN), butbpfwithBPF_PROG_TYPE_SOCKET_FILTERrequires no capabilities for unprivileged use. The threat is reduced by theno-new-privileges:trueconstraint.Finding MEDIUM-2: AppArmor set to
unconfinedThe justification is that
SYS_ADMINis required for mounting procfs at/host/proc, and the Docker default AppArmor profile blocks mount. WhileSYS_ADMINis properly dropped before user code runs, disabling AppArmor entirely removes its file-access and network-access restrictions. Docker's default profile (docker-default) also restricts/proc/syswrites, certain/sysaccesses, and signal delivery to other containers.Domain Validation Assessment — ✅ Strong
Evidence gathered:
Positive findings:
*,*.*, and regex/^[*.]+$/patterns (domain-patterns.ts:157-165)domain-patterns.ts:170)domain-patterns.ts:183-190)DOMAIN_CHAR_PATTERN = '[a-zA-Z0-9.-]*'used instead of.*for wildcard expansion (domain-patterns.ts:79)domain-patterns.ts:232)(github.com/redacted) restricts to port 80 only (domain-patterns.ts:44-59`)Input Validation Assessment — ✅ Adequate
Evidence gathered:
Positive findings:
command:array (not shell string), prevents command injection via Docker Compose YAML$in agent command escaped as$$for Docker Compose interpolation (docker-manager.ts:896)/dev/null)Finding LOW-2:
--mountflag lacks sensitive path restrictionsDocker's default seccomp profile (moby/profiles/seccomp/default.json) blocks
bpf,perf_event_open,setns,unshare,clonewith namespace flags,kcmp, and ~44 others that are not blocked here.AppArmor setting
Comment at line 881: "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."
Agent iptables TCP-only DROP
Zero known vulnerabilities in npm dependencies.
Capability drops verified
✅ Recommendations
🟡 Medium — Should Fix Soon
M1: Migrate seccomp to deny-by-default
Replace the current allow-by-default profile with a deny-by-default allowlist (similar to Docker's default). At minimum, add explicit blocks for
bpf,perf_event_open,setns,unshare,clonewith namespace creation flags (CLONE_NEWNS,CLONE_NEWNET,CLONE_NEWPID),kcmp,open_by_handle_at, andlookup_dcookie.{ "defaultAction": "SCMP_ACT_ERRNO", "syscalls": [ { "names": ["accept", "accept4", "access", "..."], "action": "SCMP_ACT_ALLOW" }, // explicitly block especially dangerous ones with higher errnoRet priority { "names": ["bpf"], "action": "SCMP_ACT_ERRNO", "errnoRet": 1 } ] }Alternatively, remove the custom seccomp file and rely on Docker's built-in default profile (which is deny-by-default and maintained by the Docker project).
M2: Investigate AppArmor-compatible mount approach
The
apparmor:unconfinedsetting defeats a meaningful defense layer. Investigate whether:mount -t procoperation while keeping other protections🔵 Low — Plan to Address
L1: Add ICMP/UDP DROP in agent container iptables
Add explicit DROP rules in
setup-iptables.shfor defense-in-depth:This ensures the agent-level filter doesn't rely solely on the host DOCKER-USER chain.
L2: Add sensitive path blocklist to
--mountvalidationIn
parseVolumeMounts(src/cli.ts:573), add a blocklist of host paths that should not be mountable:ℹ️ Informational
I1: Document the seccomp trust model
Add a comment in
docker-manager.tsexplaining why the custom seccomp is allow-by-default (e.g., compatibility with user workloads that call unusual syscalls) so future reviewers don't mistake it for Docker's default.I2: Consider adding
--no-enable-host-accesswarningWhen
--enable-host-accessis used, the network gateway IP bypass allows direct port 80/443 access to the host without Squid domain filtering. The comment insetup-iptables.shexplains this is needed for MCP Streamable HTTP, but a visible warning to the operator would help.I3: Monitor bpf syscall use
Given the permissive seccomp profile allows
bpf, consider adding a log-and-audit rule forbpfcalls by the agent, to detect potential socket-filter insertion attempts.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions