[Security Review] Daily Security Review — 2026-05-13 #3086
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-05-20T05:07:10.141Z.
|
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
Overall Security Posture: GOOD with a small number of medium-severity design trade-offs that are already documented.
The firewall follows strong defense-in-depth principles: iptables default-deny → Squid domain ACL → seccomp → capability drop → no-new-privileges. The architecture is well-reasoned and documented.
🔍 Findings from Previous Firewall Escape Test
The pre-fetched file at
/tmp/gh-aw/escape-test-summary.txtcontained GitHub Actions CI log metadata from a "Secret Digger (Copilot)" workflow run (GH_AW_SECRET_VERIFICATION_RESULT: success,GH_AW_AGENT_CONCLUSION: success, concluded withnoopoutput). This indicates:GH_AW_INFERENCE_ACCESS_ERROR: false— inference firewall functioned correctlyGH_AW_LOCKDOWN_CHECK_FAILED: false— no lockdown bypassInterpretation: The secret-digging agent successfully ran under firewall restrictions and could not exfiltrate credentials, validating the credential-isolation design.
🛡️ Architecture Security Analysis
Network Security
Evidence (
containers/agent/setup-iptables.sh):The iptables ruleset is sound. IPv6 is disabled at the kernel level inside the container namespace to prevent proxy bypass via IPv6 dual-stack paths. The dangerous-ports list is maintained in sync with
DANGEROUS_PORTSinsrc/squid/policy-manifest.ts. HTTP (port 80) and HTTPS (port 443) are DNAT'd to Squid, providing defense-in-depth for proxy-unaware tools.Observation: The
--log-uidflag on LOG rules enables agent UID tracking, but PID is not available in iptables logs — a known limitation documented in LOGGING.md.Container Security
Evidence (
src/services/agent-service.ts:88–115):Evidence (
containers/agent/entrypoint.sh:909):SYS_CHROOTandSYS_ADMINare dropped viacapshfrom the bounding set before user code runs, so user code cannot regain them even with a setuid binary. The iptables-init container holdsNET_ADMIN/NET_RAWonly; the agent container never hasNET_ADMIN.The
pids_limit: 1000andmem_limit: 6gprevent DoS via fork bombs or memory exhaustion.Domain Validation
Evidence (
src/domain-patterns.ts:25, 153):Domains are validated before being interpolated into Squid config. The
formatDomainForSquid()function normalizes todstdomain .domain.comformat. Wildcard patterns are converted to anchored regexes usingDOMAIN_CHAR_PATTERN = '[a-zA-Z0-9.-]*'— a character class that prevents ReDoS via catastrophic backtracking.The
apiProxyIpvalue insrc/squid-config.ts:37is validated with a strict per-octet IPv4 regex before being embedded in Squid config, preventing config injection via that field.Input Validation
Evidence (
src/option-parsers.ts:447):All subprocess calls use
execawith array arguments (never string interpolation into shell commands), which prevents shell injection. This applies to docker, iptables, openssl, mount, umount, ip6tables, and sysctl invocations throughoutsrc/host-iptables-rules.ts,src/ssl-bump.ts, etc.apparmor:unconfinedremoves MAC protection; ifcapshdrop fails, SYS_ADMIN is available to user codeagent-service.ts:112docker-compose.ymlcontains secrets in plaintext; tmpfs overlay (workDir) mitigates this within containeragent-service.ts:55–76agent-volumes.tsmemswap_limit: -1(unlimited swap) when no user memory limit; a swap-based side-channel could persist beyond container lifecycleagent-service.ts:119pids_limit: 1000set,mem_limit: 6gset; fork bomb or memory exhaustion containedagent-service.ts127.0.0.11forward-only resolversetup-iptables.shagent-logs/squid-config.ts:40AGENTS.md🎯 Attack Surface Map
--allow-domainsargvalidateDomainOrPattern(),SQUID_DANGEROUS_CHARSapparmor:unconfined; capsh drops before user code/workspaceRW mount--enable-api-proxysidecar172.30.0.30📋 Evidence Collection
npm audit — 0 vulnerabilities
Container capability configuration (agent-service.ts)
iptables default-deny + dangerous ports (setup-iptables.sh)
capsh privilege drop (entrypoint.sh:909)
Domain injection prevention (domain-patterns.ts)
✅ Recommendations
🔴 Critical
None identified.
🟠 High
H-1: AppArmor unconfined weakens MAC layer
src/services/agent-service.ts:112setsapparmor:unconfinedto allowmount(2)for procfs. Ifcapshfails to dropSYS_ADMIN(e.g.,capshbinary missing or corrupted on the host), user code would have bothSYS_ADMINand no AppArmor profile — a significant privilege.seccompprofile (containers/agent/seccomp-profile.json) still constrains syscalls, and the entrypoint already validates capsh availability (entrypoint.sh:592). Consider writing a targeted AppArmor profile that allows onlymountfor procfs and blocks other privileged paths, rather than fullunconfined.no-new-privileges:true+ seccomp + capsh validation provide compensating controls.🟡 Medium
M-1: Secret exposure window in docker-compose.yml
The tmpfs overlay on
workDirprevents the agent from readingdocker-compose.yml(which contains API keys as environment variables). This is well-mitigated, but the secret is still in plaintext in memory on the host filesystem during the run. Consider using Docker secrets or environment-file injection instead of compose YAML for long-lived runs.M-2: Workspace write access is unrestricted
The agent has read-write access to the full workspace. A compromised or prompt-injected agent can overwrite any file in the workspace (including
.github/workflows/). This is by design for coding agents, but operators deploying untrusted code should be aware.--block-domainsfor untrusted code scenarios. Consider a future--workspace-readonlyflag for read-only analysis tasks.M-3: Unlimited swap when no memory limit specified
memswap_limit: -1(unlimited) is set when the user doesn't provide--memory-limit. This prevents OOM kills (intentional) but means sensitive data could be paged to disk swap. On hosts with encrypted swap this is acceptable; on bare-metal runners without swap encryption it's a minor concern.docs/environment.md.🟢 Low
L-1: Dangerous-port list maintained in two places
DANGEROUS_PORTSis defined in bothsrc/squid/policy-manifest.tsandcontainers/agent/setup-iptables.sh(as a bash array comment says "matches DANGEROUS_PORTS in squid-config.ts"). A drift could leave a port blocked at iptables but allowed at Squid (or vice versa). Consider auto-generating the shell array from the TypeScript source at build time.L-2: Log injection via User-Agent
src/squid-config.ts:377notes that Squid's logformat does not JSON-escape strings, so a maliciousUser-Agentheader can inject log lines. This is a cosmetic/audit concern, not a security bypass.L-3: Proxy-unaware HTTPS tools get TLS error, not 403
Tools that ignore proxy env vars and connect directly to port 443 are DNAT'd to Squid, which rejects the raw TLS ClientHello with a connection error rather than a clean 403. The connection is still blocked, but error messages in agent output may be confusing. No code change needed, but document in troubleshooting.
L-4: Escape test produced noop (no findings)
The previous Secret Digger agent concluded with
noopand no secrets exfiltrated — a positive result. Continued periodic testing is recommended to catch regressions.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions