[Security Review] Daily Security Review and Threat Modeling — 2026-03-07

[github-actions[bot]]

📊 Executive Summary

Review Date: 2026-03-07 | Reviewer: Security Review Agent | Version: 0.23.1

This review covers a deep analysis of the @github/agentic-workflow-firewall codebase, examining ~4,104 lines of security-critical code across the network filtering stack (host-iptables, setup-iptables, Squid config, container entrypoint, and Docker orchestration). The system's overall security posture is good, with multiple defense-in-depth layers. Three actionable findings are documented below.

Category	Count
Critical	0
High	0
Medium	2
Low	2
Informational	4

---

🔍 Findings from Firewall Escape Test

The security-review workflow was the most recent in-scope agentic workflow; no prior escape-test run logs were accessible via agenticworkflows-logs (download error). Complementary findings from previous security guards are reflected where patterns overlap.

---

🛡️ Architecture Security Analysis

Network Security Assessment — Strong

The traffic interception chain is well-designed:

1. Host-level DOCKER-USER chain (src/host-iptables.ts) creates a custom FW_WRAPPER chain that filters all egress from the bridge interface. Rules are ordered correctly: Squid exempt → ESTABLISHED/RELATED → localhost → trusted DNS → Squid port → API proxy port → multicast/link-local block → UDP block → default deny.
2. Container-level NAT (containers/agent/setup-iptables.sh) redirects HTTP/HTTPS to Squid and terminates all other TCP with a final DROP rule.
3. Squid ACL (src/squid-config.ts) enforces domain whitelisting and blocks dangerous ports via !Safe_ports.
4. DNS exfiltration prevention is implemented at both layers: only whitelisted DNS server IPs are permitted (default Google 8.8.8.8/8.8.4.4).
5. IPv6 handling: ip6tables is set up when available; when unavailable, sysctl net.ipv6.conf.all.disable_ipv6=1 prevents unfiltered IPv6 bypass.

Evidence:

# host-iptables.ts:274-288 — Correct rule ordering
await execa('iptables', ['-t', 'filter', '-A', CHAIN_NAME, '-s', squidIp, '-j', 'ACCEPT']);  // Squid free pass
// ... ESTABLISHED/RELATED ...
// ... DNS allow to whitelisted IPs only ...
await execa('iptables', ['-t', 'filter', '-A', CHAIN_NAME, '-p', 'udp', '-j', 'REJECT']);  // Block UDP exfil
await execa('iptables', ['-t', 'filter', '-A', CHAIN_NAME, '-j', 'REJECT']);  // Default deny

Container Security Assessment — Good with Notes

• NET_ADMIN is added to the agent container for iptables setup and then dropped via capsh before user code executes (entrypoint.sh:649, cap_drop: ['NET_RAW','SYS_PTRACE','SYS_MODULE','SYS_RAWIO','MKNOD'] in docker-manager.ts:872-878).
• no-new-privileges:true prevents re-acquisition of dropped privileges.
• Resource limits are applied: mem_limit: 4g, pids_limit: 1000.
• One-shot token library (LD_PRELOAD=/usr/local/lib/one-shot-token.so) protects secrets from /proc/self/environ re-reads after agent startup.
• Token unsetting (unset_sensitive_tokens) runs after a 5-second grace period for agent initialization.

Domain Validation Assessment — Strong

• ReDoS protection: wildcards become [a-zA-Z0-9.-]* (not .*) in domain-patterns.ts:77.
• Overly-broad patterns (*, *.*, *.*.com) are rejected.
• Domain length capped at 512 chars to prevent regex abuse (isDomainMatchedByPattern:279).
• Domain deduplication prevents redundant ACL entries.

Input Validation / Command Injection Assessment — Strong

• A custom ESLint rule (eslint-rules/no-unsafe-execa.js) statically enforces that execa() is only called with literal string commands — no template literals with expressions or string concatenation. Only 2 justified eslint-disable exceptions exist (log-discovery.ts:157 for container name filter, ssl-bump.ts:201 for OpenSSL cert subject — both use carefully-controlled variables, not user input).
• All commands are passed as arrays to execa, not through a shell.

---

⚠️ Threat Model

#	Threat (STRIDE)	Description	Evidence	Severity	Likelihood
T1	Elevation of Privilege	Permissive custom seccomp profile	seccomp-profile.json + docker-manager.ts:885	Medium	Low
T2	Elevation of Privilege	AppArmor disabled on agent container	docker-manager.ts:886	Medium	Low
T3	Tampering	Domain names not checked for newline characters — Squid config injection	domain-patterns.ts:139-185	Low	Very Low
T4	Spoofing	Dangerous ports list inconsistency between Squid and iptables	squid-config.ts:10-31 vs setup-iptables.sh:203-221	Low	Very Low
T5	Information Disclosure	Host-access gateway bypass bypasses domain filtering	setup-iptables.sh:156-200	Info	N/A (by design)
T6	Information Disclosure	Docker-in-Docker blocked by stub	containers/agent/docker-stub.sh	Info	Mitigated
T7	Denial of Service	Resource limits applied but no CPU quota	docker-manager.ts:888-892	Info	Low
T8	Repudiation	DNS queries logged ([FW_DNS_QUERY]) but iptables logs go to kernel dmesg only	host-iptables.ts:310-316	Info	Low

---

🎯 Attack Surface Map

Surface	Location	Protections	Risk
CLI arg parsing (--allow-domains)	src/cli.ts:870-878	validateDomainOrPattern() rejects broad patterns; no newline check	Low
CLI arg parsing (--allow-urls)	src/cli.ts:1004-1038	Checks for https:// prefix, rejects broad patterns; no newline check	Low
Squid config generation	src/squid-config.ts:119,247,256	Domains go through wildcard→regex conversion; port sanitized ([^0-9-]/g)	Low
Container command execution	src/docker-manager.ts:896	Dollar-sign escaping ($$) for YAML; passes through bash -c as intended	Info
iptables setup (host)	src/host-iptables.ts	Array args to execa (no shell injection); iptables requires root	Low
iptables setup (container)	containers/agent/setup-iptables.sh	is_valid_ipv4() validates resolved IPs before use	Low
DNS resolution in setup-iptables	setup-iptables.sh:43-48	Resolves Squid hostname and validates before use	Low
Capability drop (capsh)	entrypoint.sh:649	capsh --drop=cap_net_admin before user code; no-new-privileges:true	Low
One-shot token	entrypoint.sh:634, 647-656	LD_PRELOAD + unset_sensitive_tokens after 5s grace period	Low
API proxy placeholder key	containers/agent/get-claude-key.sh	Returns placeholder; real key held by api-proxy sidecar only	Low
Seccomp profile	containers/agent/seccomp-profile.json	Custom permissive profile (allow-default) weaker than Docker default	Medium

---

📋 Evidence Collection

Finding T1: Permissive Seccomp Profile

Command run:

cat containers/agent/seccomp-profile.json | python3 -c "
import sys, json
d = json.load(sys.stdin)
print('Default action:', d.get('defaultAction'))
syscalls = d.get('syscalls', [])
for sc in syscalls:
    print('Action:', sc.get('action'))
    print('Names:', sc.get('names'))
"
````

Output:
````
Default action: SCMP_ACT_ALLOW
Action: SCMP_ACT_ERRNO
Names: ['ptrace', 'process_vm_readv', 'process_vm_writev']
Action: SCMP_ACT_ERRNO
Names: ['kexec_load', 'kexec_file_load', 'reboot', 'init_module', 'finit_module', 'delete_module', 'acct', 'swapon', 'swapoff', 'pivot_root', 'syslog', 'add_key', 'request_key', 'keyctl', 'uselib', 'personality', 'ustat', 'sysfs', 'vhangup', 'get_kernel_syms', 'query_module', 'create_module', 'nfsservctl']
Action: SCMP_ACT_ERRNO
Names: ['umount', 'umount2']

Docker's built-in default seccomp profile uses SCMP_ACT_ERRNO as its default (deny-by-default allowlist). The AWF custom profile uses SCMP_ACT_ALLOW (allow-by-default blocklist). Syscalls not explicitly blocked include mount, unshare, and clone with CLONE_NEWUSER. These are mitigated by capability drops (cap_drop: CAP_SYS_ADMIN via capsh before user code) but the profile itself provides weaker guarantees.

Applied in docker-manager.ts:885:

security_opt: [
  'no-new-privileges:true',
  `seccomp=\$\{config.workDir}/seccomp-profile.json`,
  'apparmor:unconfined',   // <-- AppArmor also disabled (T2)
],

Finding T2: AppArmor Disabled

// docker-manager.ts:880-886
// 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',
],

The comment explains the rationale (procfs mounting). However, apparmor:unconfined removes MAC-level process confinement for the entire container lifecycle, not just during setup. Once capsh drops SYS_ADMIN, mount calls from user code would fail at the capability check — but AppArmor would no longer provide an independent audit trail or secondary enforcement for other operations (file access, network access from processes, etc.).

Finding T3: Domain Names Not Checked for Newlines (Squid Config Injection)

The validateDomainOrPattern() function in domain-patterns.ts:139-185 validates:

• Empty input
• *, *.*, [*.]+ (too broad)
• .. (double dots)
• . alone, *., .*
• Too many wildcard segments

But it does not check for newline characters (\n, \r). Domains are then embedded directly into the Squid configuration:

// squid-config.ts:247
aclLines.push(`acl allowed_domains dstdomain \$\{formatDomainForSquid(domain)}`);
// squid-config.ts:256
aclLines.push(`acl allowed_domains_regex dstdom_regex -i \$\{p.regex}`);
// squid-config.ts:119 (urlPatterns)
.map((pattern, i) => `acl allowed_url_\$\{i} url_regex \$\{pattern}`)

A domain like github.com\nhttp_access allow all would be parsed by commander as a single argument containing a literal newline (if passed via a config file or programmatic API), and when written to squid.conf, the second line would become http_access allow all, allowing unrestricted access.

Mitigating factors: Shell argument parsing strips/escapes newlines in practice when domains are passed via CLI. The parseDomains() function splits on commas but not newlines. Risk is very low in normal CLI usage but theoretically present if the AWF library is used programmatically or if the allow-domains-file feature is used with malicious input.

Finding T4: Dangerous Ports List Inconsistency

src/squid-config.ts DANGEROUS_PORTS (21 entries) includes ports not in setup-iptables.sh (15 entries):

• 5984 (CouchDB), 6984 (CouchDB SSL)
• 8086, 8088 (InfluxDB)
• 9200, 9300 (Elasticsearch)

However, in the container's OUTPUT filter chain, the final rule is:

iptables -A OUTPUT -p tcp -j DROP

This catches all TCP not explicitly redirected to Squid or allowed. Ports 5984, 9200, etc. would still be dropped. The inconsistency is a defense-in-depth gap (Squid and iptables don't give identical explicit denials) but does NOT represent an actual bypass.

Good Security Controls (evidence)

ReDoS protection:

// domain-patterns.ts:77
const DOMAIN_CHAR_PATTERN = '[a-zA-Z0-9.-]*';  // Not .*

Execa injection prevention:

# Only 2 eslint-disable instances in 4000+ lines of TS
$ grep -rn "no-unsafe-execa" src/ --include="*.ts"
src/logs/log-discovery.ts:157  # container name filter
src/ssl-bump.ts:201            # OpenSSL cert subject line

One-shot token protection:

# entrypoint.sh:634-656
export LD_PRELOAD=/usr/local/lib/one-shot-token.so
capsh --drop=$CAPS_TO_DROP -- -c "exec gosu awfuser ..." &
sleep 5   # grace period for token caching
unset_sensitive_tokens  # clear from /proc/1/environ
````

**npm audit:**
````
Total vulnerabilities: {'info': 0, 'low': 0, 'moderate': 0, 'high': 0, 'critical': 0, 'total': 0}

---

✅ Recommendations

Medium Priority

M1 — Replace permissive seccomp profile with Docker's built-in default
The custom seccomp-profile.json has defaultAction: SCMP_ACT_ALLOW (allow-by-default). Consider switching to Docker's built-in default seccomp profile (seccomp:docker-default) which uses deny-by-default and only adds specific blocks needed. If procfs mounting requires exceptions, add those specific syscalls to an allowlist-based profile rather than switching to a blocklist approach. File: containers/agent/seccomp-profile.json, src/docker-manager.ts:885.

M2 — Document or scope AppArmor unconfined
If apparmor:unconfined is only needed for the setup phase (mounting procfs), investigate whether a custom AppArmor profile can be created that allows the specific mount operations needed, then re-enables confinement. Alternatively, explicitly document in the security architecture that AppArmor is intentionally bypassed and what mitigations replace it (capability drops, seccomp, no-new-privileges). File: src/docker-manager.ts:886.

Low Priority

L1 — Add newline sanitization to domain and URL validation
Add an explicit check for control characters (including \n, \r, \t) in validateDomainOrPattern() (domain-patterns.ts:139) and in the URL pattern validation in cli.ts:1004. A one-line fix:

if (/[\n\r\t\x00-\x1f]/.test(trimmed)) {
  throw new Error(`Domain '\$\{trimmed}' contains invalid control characters`);
}

This closes the theoretical Squid config injection vector, particularly relevant for the --allow-domains-file code path.

L2 — Synchronize dangerous ports lists
Add the 6 ports present in squid-config.ts but missing from setup-iptables.sh to the iptables DANGEROUS_PORTS array: 5984, 6984, 8086, 8088, 9200, 9300. This ensures defense-in-depth is consistent at both enforcement layers.

---

📈 Security Metrics

Metric	Value
Security-critical source files analyzed	6
Lines of security-critical code reviewed	~4,104
Attack surfaces identified	11
Dependency vulnerabilities (npm audit)	0
Execa unsafe uses	2 (justified, with eslint-disable comments)
Seccomp blocked syscall groups	3
Dangerous ports (Squid)	21
Dangerous ports (iptables)	15 (6 missing)
Threat model entries	8

AI generated by Daily Security Review and Threat Modeling

• expires on Mar 14, 2026, 1:41 PM UTC