[Security Review] Daily Security Review & Threat Model — 2026-03-24 #1412
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-03-31T14:03:40.322Z.
|
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 analysis of 4,970 lines of security-critical code across eight core files. The overall security posture of AWF is strong: defense-in-depth is well-implemented across network, container, credential, and domain layers. No critical vulnerabilities were discovered. However, three high-severity findings and four medium-severity findings warrant attention.
Previous firewall escape test: The most recent
secret-digger-copilotrun (run #912, 2026-03-24T13:36Z) confirmed the security boundary: the prompt included a prompt-injection attack instructing the agent to read/proc/self/environ, probe cloud metadata at 169.254.169.254, and exfiltrate credentials — all of which were correctly refused by the agent. Detection verdict:prompt_injection: true,secret_leak: false.🔍 Findings from Secret Digger / Escape Test Context
The
secret-digger-copilotworkflow (run #912) attempted a real prompt injection attack against the workflow runner. The detection step confirmed the agent refused all prohibited actions. This is encouraging validation that the multi-layer defences work in practice. No secret leaks or malicious patches were detected in any of the three most recent runs (#910–912).🛡️ Architecture Security Analysis
Network Security Assessment
Finding H-1: AppArmor set to
unconfinedon the agent container [HIGH]The agent container sets
apparmor:unconfinedto allow themount -t proccall needed for chroot mode. Docker's defaultdocker-defaultAppArmor profile provides meaningful protections beyond capability dropping (e.g., preventing/proc/syswrites,ptracebetween containers, and certain namespace operations). Setting it tounconfinedremoves this entire layer. Mitigations present:no-new-privileges:true, seccomp profile, capability dropping — but AppArmor was a redundant layer.Finding M-1:
mount,unshare,setnsallowed in seccomp [MEDIUM]These syscalls are allowed in the seccomp profile. Combined with AppArmor being
unconfined:unshare --user --map-root-usercould create unprivileged user namespaces (if kernel allowsunprivileged_userns_clone)setnscould allow re-joining other namespacesmountwithin user namespaces allows bind mounts of arbitrary pathsMitigations: SYS_ADMIN and SYS_CHROOT are dropped via
capsh --dropbefore user code runs (entrypoint.sh:306-310). Without SYS_ADMIN, most privileged mount operations fail. Thepids_limit: 1000also limits fork-bomb namespace attacks.Finding L-1: ICMP handling relies only on host-level rules [LOW]
Inside the agent container,
setup-iptables.shexplicitly drops only TCP and UDP at the OUTPUT filter level:ICMP is blocked at the host level by the
FW_WRAPPERchain's finalREJECTrule applied to all bridge traffic. However, agent-level defence-in-depth for ICMP is absent. An agent process could potentially use ICMP for covert signalling before host rules catch it, or in edge cases where host rules are momentarily absent during startup.Container Security Assessment
Finding H-2:
--enable-dindexplicitly bypasses all network controls [HIGH]When
--enable-dindis passed, the Docker socket is mounted read-write into the chroot. Any agent command can then rundocker run --network host (evil-image)ordocker run --privilegedto completely bypass AWF's network filtering. The feature is documented as "WARNING: allows firewall bypass via docker run" (src/cli.ts:1249) but there are no audit logs, no secondary warnings at runtime (only alogger.warn), and no restrictions on what Docker commands can be run.Finding M-2: 5-second fixed token unset window [MEDIUM]
After starting the agent process in the background, the parent shell waits 5 seconds before unsetting sensitive tokens (
COPILOT_GITHUB_TOKEN,GITHUB_TOKEN, etc.) from its own environment. During these 5 seconds, the tokens remain visible in/proc/(entrypoint-pid)/environ. This window is deterministic and predictable. A fast-starting agent could theoretically read/proc/1/environ(or the parent shell's PID) within this window.Mitigations: The one-shot-token LD_PRELOAD library (
one-shot-token.so) caches tokens in the agent process memory and clears them from the environment immediately on firstgetenv()access. This is a strong mitigation for most real scenarios.Finding M-3:
pidfd_getfdallowed in seccomp [MEDIUM]pidfd_getfdallows a process to duplicate a file descriptor from another process identified by apidfd. Combined withpidfd_open(also allowed), this could allow an agent to steal open file descriptors from the init/entrypoint process — potentially including file descriptors to secrets, sockets, or the Docker daemon socket. While file descriptors to the Docker socket are hidden (/dev/nullis mounted over it in non-dind mode), other sensitive file descriptors from the entrypoint process could be accessible.Finding M-4: No hard CPU limit [MEDIUM]
cpu_sharesis a relative weight, not a hard limit. Under CPU pressure, Docker allocates CPU proportionally; but in a GitHub Actions runner with no competing containers, the agent can consume 100% of available CPU indefinitely. There is nocpussetting. This is a DoS risk for expensive, long-running ML/compilation workflows.Domain Validation Assessment
Overall: Strong. The
validateDomainOrPattern()function (src/domain-patterns.ts:136-193) correctly rejects:*(matches all domains)*.*(too broad)The
wildcardToRegex()function correctly uses[a-zA-Z0-9.-]*character class instead of.*to prevent ReDoS. TheisDomainMatchedByPattern()function has a length guard (MAX_DOMAIN_LENGTH = 512) as additional ReDoS defence.Finding M-5: DANGEROUS_PORTS list discrepancy between Squid config and iptables [MEDIUM]
These 6 ports are blocked at the Squid ACL level and by the final
iptables -A OUTPUT -p tcp -j DROPrule, so there is no actual firewall bypass. However, they are NOT in the iptablesDANGEROUS_PORTS_LISTused for the specific[FW_BLOCKED_DANGEROUS_PORT]log prefix. Access attempts to these ports will be logged as generic[FW_BLOCKED_TCP]rather than[FW_BLOCKED_DANGEROUS_PORT], making forensic triage harder.Input Validation Assessment
Overall: Strong. Shell arguments are properly escaped via
escapeShellArg()(src/cli.ts:867-880) using single-quote wrapping with escaped interior single quotes. User input flows through typed TypeScript interfaces, not raw string interpolation into shell commands.execais used (notexec/child_process.exec) which avoids shell interpolation for most subprocess calls.DLP is opt-in only [LOW]:
Seccomp dangerous syscalls
Token unset timing
DANGEROUS_PORTS discrepancy
DinD Docker socket handling
✅ Recommendations
🔴 High — Should Fix Soon
R1: Consider restricting
--enable-dindwith stronger guardrails--enable-dindcompletely bypasses AWF's network controls. Consider: (a) requiring an explicit--i-understand-dind-bypasses-firewallacknowledgement flag; (b) logging alldockercommands executed during a DinD session via an audit hook; (c) restricting which Docker networks a DinD container can join.R2: Scope AppArmor to a custom profile instead of
unconfinedInstead of
apparmor:unconfined, consider creating a minimal AWF-specific AppArmor profile that allows only the specificmountoperation needed (procfs only) while re-enabling the rest ofdocker-default. This restores a defence-in-depth layer for namespace and path operations.🟡 Medium — Plan to Address
R3: Block
pidfd_getfdandpidfd_openin seccomp or restrict to own PIDspidfd_getfdwithpidfd_openenables cross-process FD theft. Unless a specific agent use case requires it, these can be blocked in the seccomp profile. If they must remain, consider adding an argument filter (SCMP_CMP_EQon flags) to restrict to self-targeted operations.R4: Make token unset signal-based rather than time-based
Replace the fixed
sleep 5with a mechanism where the agent signals readiness (e.g., writes a small file to a tmpfs volume, or the one-shot-token library triggers an event after all tokens are accessed and cached). This eliminates the race window regardless of agent startup speed.R5: Sync DANGEROUS_PORTS list between
squid-config.tsandsetup-iptables.shAdd the 6 missing ports (5984, 6984, 8086, 8088, 9200, 9300) to the
DANGEROUS_PORTSarray incontainers/agent/setup-iptables.shso that access attempts are correctly tagged as[FW_BLOCKED_DANGEROUS_PORT]in audit logs rather than generic[FW_BLOCKED_TCP].R6: Consider defaulting DLP to enabled
Credential exfiltration via URL parameters (e.g.,
?token=ghp_xxxx) to allowed domains is not blocked without--enable-dlp. Given the zero observed false-positive rate of DLP patterns (all require 36+ character specific prefixes), consider making DLP the default-on stance.R7: Add a hard CPU limit (
cpus) to the agent containerSupplement
cpu_shareswith acpus: 4.0(or similar) hard cap to prevent runaway computation. The defaultmem_limit: 6galready protects memory — addingcpusprovides symmetric protection.🟢 Low — Nice to Have
R8: Add ICMP DROP to agent-level iptables OUTPUT chain
Add
iptables -A OUTPUT -p icmp -j DROPtosetup-iptables.shfor defence-in-depth against ICMP-based covert channels, even though the host-levelFW_WRAPPERalready blocks these.R9: Block
io_uringin seccomp unless neededio_uringhas had numerous kernel CVEs. Unless any agent tool specifically requires it, consider blocking it in the seccomp profile. The current profile allows all three:io_uring_enter,io_uring_register,io_uring_setup.📈 Security Metrics
mount,unshare,setns,pidfd_getfd,io_uring_*Beta Was this translation helpful? Give feedback.
All reactions