[Security Review] Daily Security Review and Threat Modeling – 2026-03-09 #1181
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-03-16T13:53:44.288Z.
|
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 review covers a deep, evidence-based security analysis of the
gh-aw-firewallcodebase as of 2026-03-09. The overall security posture is strong: credential isolation, capability dropping, DNS restriction, and network ACL ordering are all implemented correctly. Two medium-severity configuration issues were identified — an allow-all seccomp default and a disabled AppArmor profile — along with one low-severity Squid DNS inconsistency and one note-level gap in custom mount path validation. No critical vulnerabilities were found, and no npm dependency vulnerabilities exist.🔍 Findings from Firewall Escape Test Agent
The
firewall-escape-testworkflow is not listed in the current workflow registry (agenticworkflows-statusshowed no such workflow). The closest security-adjacent workflows aresecurity-review(daily) andsecurity-guard(per-PR). No historical escape-test run data was available for cross-reference. This review is based purely on static codebase analysis.🛡️ Architecture Security Analysis
Network Security Assessment ✅ Strong
Evidence — iptables rule ordering (
containers/agent/setup-iptables.sh):The seccomp profile (
containers/agent/seccomp-profile.json:2) specifies"defaultAction": "SCMP_ACT_ALLOW". The profile only explicitly denies 28 syscalls:ptrace,process_vm_readv,process_vm_writev(anti-debugging)kexec_load,kexec_file_load,reboot,init_module,finit_module,delete_module(kernel/boot)pivot_root,syslog,add_key,request_key,keyctl(system-level operations)umount,umount2(unmounting)This allow-all-except approach leaves many potentially dangerous syscalls accessible, including
unshare(namespace manipulation),clone3with new namespace flags, andmount(if capability allows). The Docker documentation recommends a deny-by-default seccomp profile for untrusted workloads. The gap is partially mitigated by capability restrictions, but the seccomp profile represents an independent security layer that is currently weak.Mitigation: Invert the profile to use
"defaultAction": "SCMP_ACT_ERRNO"and add an explicit allowlist of required syscalls, following the Docker default seccomp profile as a baseline.Evidence (
src/docker-manager.ts:883-885):The agent container runs with
apparmor:unconfined, removing the Mandatory Access Control (MAC) layer. The code comment explains this is necessary to allow mounting procfs at/host/proc(Docker's default AppArmor profile blocks themountcall). While capability restrictions prevent user code from mounting anything (SYS_ADMIN is dropped before user command), the MAC absence represents a defense-in-depth gap during the setup phase (beforecapsh --drop).Mitigation: Write a custom AppArmor profile that permits only the specific mount operations needed during initialization, then restricts access during user command execution. Alternatively, document this explicitly as an accepted risk given the capability drop.
✅ Capability Dropping — Correctly Implemented
Evidence (
src/docker-manager.ts:870-879):Evidence (
containers/agent/entrypoint.sh):Capabilities are dropped via
capsh --drop=cap_net_admin,...before the user command executes. This is verified to occur after iptables setup and before any user-controlled code runs.NET_RAWis dropped to prevent raw socket bypass of iptables.Evidence (
src/docker-manager.ts:882):✅ Credential Isolation — Correctly Implemented
Evidence (
src/docker-manager.ts:756-784):Credentials are masked at both
$HOMEand/host$HOMEpaths. The Docker socket is hidden at/host/var/run/docker.sockand/host/run/docker.sock(line 681-685). This correctly mitigates prompt injection attacks targeting credential exfiltration.Domain Validation Assessment ✅ Strong
Evidence (
src/domain-patterns.ts:126-140):ReDoS protection (
src/domain-patterns.ts:86-95):And length validation before regex execution:
✅ Recommendations
🔴 Medium: Harden Seccomp Profile to Default-Deny
File:
containers/agent/seccomp-profile.json:2Change
"defaultAction": "SCMP_ACT_ALLOW"to"defaultAction": "SCMP_ACT_ERRNO"and add an explicit allowlist of all required syscalls. Use the Docker default seccomp allowlist as a starting point and prune syscalls not needed by the agent container. This provides the most defense against container escape via kernel exploitation.🔴 Medium: Address AppArmor Unconfined Configuration
File:
src/docker-manager.ts:884The
apparmor:unconfinedsetting removes the entire MAC layer. Either:mountat all during the capability-privileged phaseAt minimum, add a warning in documentation and CLI output when this mode is active.
🟡 Low: Propagate --dns-servers to Squid's dns_nameservers
File:
src/squid-config.ts:551Pass the configured DNS servers through
SquidConfigand write them todns_nameservers. This ensures Squid resolves domains using the same trusted servers as the rest of the container, rather than always using Google DNS regardless of user configuration.🟢 Info: Document Custom Mount Path Restrictions
File:
src/cli.ts:570-660(parseVolumeMounts)Add documentation noting that
--mountis a privileged operation. Consider adding a warning when sensitive system directories (e.g.,/proc,/sys,/dev,/etc) are included in mount paths.🟢 Info: Add Escape Test Workflow
There is no
firewall-escape-testworkflow in the current workflow registry. Adding an automated test workflow that attempts known escape techniques (DNS tunneling, raw socket creation, IPv6 bypass, ICMP tunneling) would provide continuous confidence in firewall effectiveness. This complements static analysis.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions