[Security Review] Daily Security Review — gh-aw-firewall (2026-05-09) #2796
Replies: 4 comments
-
|
🔮 The ancient spirits stir... the Codex smoke-test oracle passed through this circle and left a brief shimmer in the logs. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Beta Was this translation helpful? Give feedback.
-
|
🔮 The ancient spirits stir... Codex passed through this discussion under the smoke-test moon, leaving a brief oracle mark in the runes. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Beta Was this translation helpful? Give feedback.
-
|
🔮 The ancient spirits stir: the Codex smoke test agent was here, leaving a bright ember in the oracle bowl. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Beta Was this translation helpful? Give feedback.
-
|
This discussion was automatically closed because it expired on 2026-05-16T12:38:40.742Z.
|
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
📊 Executive Summary
This review covers the gh-aw-firewall (
@github/agentic-workflow-firewall) v0.23.1 codebase, performing evidence-based analysis of network security, container hardening, domain validation, and input handling. The firewall demonstrates strong defence-in-depth across multiple layers. No critical vulnerabilities were found; however, several medium-to-high-severity design considerations and hardening gaps warrant attention.🔍 Findings from Firewall Escape Test (Secret Digger – Copilot)
The pre-fetched escape test summary (
/tmp/gh-aw/escape-test-summary.txt) contained the conclusion logs from the prior Secret Digger (Copilot) workflow run (Actions run 24273493151):GH_AW_AGENT_CONCLUSION: successGH_AW_SECRET_VERIFICATION_RESULT: successInterpretation: The agent ran successfully and produced a noop, indicating no secrets were leaked or exfiltrated through the firewall in that run. The "Secret Digger" workflow is designed to test whether an agent inside the firewall can extract secrets; a noop conclusion means no secrets were exfiltrated and no disclosure findings were reported.
🛡️ Architecture Security Analysis
Network Security Assessment
Evidence gathered:
Strengths:
containers/agent/setup-iptables.sh:204-228— When--allow-hostis enabled, all traffic tohost.docker.internalbypasses Squid entirely:Squid provides L7 domain filtering; bypassing it for the host gateway means any domain reachable via the host (if the host has broader internet access) is unfiltered. If an attacker inside the container can manipulate
host.docker.internalDNS resolution (e.g., via a compromised Docker daemon or hosts file manipulation), arbitrary IPs could be added to the bypass set.MEDIUM: iptables-save parsing for Docker DNS rules
containers/agent/setup-iptables.sh:78-93— Docker's embedded DNS DNAT rules are preserved using:The
$rulevariable is derived from parsingiptables-saveoutput and passed as shell word-split arguments toiptables. If Docker's rule format ever changes to include special characters, this could inject unexpected arguments. Risk is low in practice (Docker controls the output format) but represents a shell injection surface.Container Security Assessment
Evidence:
Strengths:
NET_ADMINis correctly isolated to the short-lived iptables-init init container, never granted to the agentno-new-privileges:trueprevents SUID escalationSYS_CHROOTandSYS_ADMINare explicitly dropped before user code runs (entrypoint.sh:355-358)no-new-privileges:true(api-proxy-service.ts:159-162)src/services/agent-service.ts:112:The agent container runs with AppArmor disabled. The comment explains this is needed to allow
mountfor procfs at/host/proc. WhileSYS_ADMINis dropped before user code runs, the absence of AppArmor confinement means there is no Mandatory Access Control (MAC) layer constraining file access, system call patterns, or socket operations during the window whenSYS_ADMINis held. AppArmor'sunconfinedmode bypasses one of Docker's default security layers entirely.MEDIUM: SYS_ADMIN Held During Startup
containers/agent/entrypoint.sh:465—SYS_ADMINis granted to the entire agent container and only dropped just before user code runs. During the entrypoint script execution window, the container holdsSYS_ADMIN, which is a very powerful capability (allows arbitrary mounts, some namespace operations). If the entrypoint script itself had a vulnerability exploitable remotely (e.g., via environment variable injection),SYS_ADMINwould be available.Domain Validation Assessment
Evidence:
Strengths:
[a-zA-Z0-9.-]*instead of.*in wildcard expansionLOW: URL Allowlist (
--allow-urls) Uses Less Strict Validationsrc/domain-patterns.ts— TheSQUID_DANGEROUS_CHARSregex (used for URLs) excludes\(backslash is allowed), while domain validation (DOMAIN_DANGEROUS_CHARS) rejects it. URLs legitimately use\for regex escaping per the comment, but this creates an asymmetry in injection protection between--allow-domainsand--allow-urls.Input Validation Assessment
Evidence:
Strengths:
joinShellArgsusesescapeShellArgwhich single-quotes arguments with special charactersMEDIUM: Single-Argument Shell Pass-Through
src/cli.ts:523-547— When exactly one argument is provided, it is passed as a raw shell command string (unescaped). This is documented as intentional — shell variables like$HOMEmust expand inside the container. However, this means the user's input is executed verbatim through/bin/sh -c, which is inherently a shell injection surface if AWF is invoked programmatically with user-controlled input. This is an acceptable design trade-off for interactive use, but dangerous if AWF is called as part of an automated pipeline where the command is constructed from untrusted input.172.30.0.0/24; Squid IP hardcoded at172.30.0.10NET_ADMIN; verified inagent-service.ts:88-96--enable-api-proxy--limit 5-10/minrate limitingno-new-privileges:trueprevents SUID escalation even in chroot🎯 Attack Surface Map
src/cli.ts:512-547escapeShellArg; domain validationsrc/domain-patterns.tssrc/squid-config.tscontainers/agent/setup-iptables.shsetup-iptables.sh:204-228src/services/agent-service.ts:96-112agent-service.ts:112containers/api-proxy/cap_drop: ALL; no-new-privileges; Squid-only egress📋 Evidence Collection
Key evidence commands and outputs
Capability configuration (src/services/agent-service.ts:88-112):
iptables-init container (agent-service.ts:255-256):
Seccomp profile default action:
IPv6 disabled in agent (setup-iptables.sh:47-49):
Host gateway bypass (setup-iptables.sh:204-211):
Domain dangerous char validation (domain-patterns.ts):
Escape test conclusion:
✅ Recommendations
🔴 High — Should Fix Soon
1. Document and restrict
--allow-hostuse casesThe host gateway bypass (
setup-iptables.sh:204-228) is currently an all-or-nothing exemption from Squid for the host gateway IP. Consider:--allow-hostis enabled--allow-hostdisables L7 filtering for host-bound traffic2. Investigate AppArmor profile for the agent
agent-service.ts:112—apparmor:unconfinedis used because procfs mount requiresSYS_ADMINand Docker's default AppArmor profile blocksmount. SinceSYS_ADMINis now the capability that gates mount (already held during startup), a custom AppArmor profile that allows only the specificprocmount and otherwise restricts the entrypoint could restore MAC coverage. Alternatively, document the explicit security trade-off in code comments and README.🟡 Medium — Plan to Address
3. Narrow the SYS_ADMIN window further
containers/agent/entrypoint.sh:355-470— The procfs mount occurs at line 465 and capability is dropped at line 358. Consider using a minimal helper binary (compiled with only mount capability viaCAP_SYS_ADMINvia file capabilitysetcap cap_sys_admin+ep) to perform the mount, allowing the main entrypoint to run withoutSYS_ADMINentirely.4. Harden iptables-save rule parsing
setup-iptables.sh:78-93— Use a whitelist regex on each rule line extracted fromiptables-saveoutput before passing toiptables -t nat. Example:5. Add per-process logging correlation
Squid logs by client IP; iptables logs by UID. Neither provides PID-level attribution. Consider logging the process name/cmdline via auditd or an eBPF-based tool on the host to correlate network events to specific agent processes.
6. Warn on programmatic use of single-argument shell pass-through
src/cli.ts:523-547— Add a lint rule or documentation note that AWF should not be called with user-controlled single-argument commands in automated pipelines. Alternatively, add a--no-shellflag that passes the command viaexec(bypassing/bin/sh -c).🟢 Low — Nice to Have
7. Align URL allowlist validation with domain allowlist
src/domain-patterns.ts— Consider whether backslash should be restricted in--allow-urlspatterns, or document why the asymmetry with--allow-domainsis intentional.8. Add explicit IPv6 DROP rules as backup
setup-iptables.sh:47-49—sysctl disable_ipv6is best-effort and may fail silently. Addip6tables -P OUTPUT DROPas a defence-in-depth fallback even if ip6tables is not always available.9. Pin dependency versions for security-sensitive packages
package.jsonuses caret ranges (^) forexeca,js-yaml,ajv. While devDependencies are not shipped, pinning production dependencies prevents unexpected version changes from introducing vulnerabilities.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions