feat(cli): add container hardening measures#96
Conversation
Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
Test Coverage Report
Coverage ThresholdsThe project has the following coverage thresholds configured:
Coverage report generated by `npm run test:coverage` |
There was a problem hiding this comment.
Pull request overview
This PR reimplements container security hardening measures from PR #76 on the current main branch, adding multiple layers of defense to reduce the attack surface of the firewall agent container.
Key changes:
- Drops dangerous Linux capabilities (NET_RAW, SYS_PTRACE, SYS_MODULE, SYS_RAWIO, MKNOD) to prevent container escape and firewall bypass vectors
- Implements seccomp syscall filtering to block dangerous operations like kernel module loading, mount operations, and keyring manipulation
- Adds resource limits (4GB memory, 1000 PIDs, no swap) to prevent DoS attacks via resource exhaustion
- Introduces DNS query audit logging for all DNS traffic to detect potential DNS tunneling attempts
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Adds TypeScript type definitions for new Docker Compose security and resource limit properties (cap_drop, security_opt, mem_limit, memswap_limit, pids_limit, cpu_shares) |
| containers/agent/seccomp-profile.json | Defines seccomp profile that blocks dangerous syscalls while maintaining compatibility with firewall operations |
| src/docker-manager.ts | Implements container hardening in Docker Compose generation and adds seccomp profile file copying with fallback paths for source/dist deployments |
| src/docker-manager.test.ts | Adds comprehensive test coverage for all hardening measures including capability drops, seccomp configuration, and resource limits |
| src/host-iptables.ts | Adds iptables LOG rules for DNS queries (UDP/TCP, IPv4/IPv6) before ACCEPT rules to create audit trail |
| docs/logging_quickref.md | Documents DNS query logging feature with examples for viewing, filtering, and analyzing DNS traffic logs |
Comments suppressed due to low confidence (2)
src/host-iptables.ts:303
- The DNS logging feature adds LOG rules for DNS queries but lacks test coverage. Since the host-iptables.test.ts file has comprehensive test coverage for DNS rules (verifying both UDP and TCP ACCEPT rules for DNS servers), the new LOG rules should also be tested to ensure they are created correctly before the ACCEPT rules.
Add test assertions to verify that LOG rules are created with the correct parameters including the log prefix '[FW_DNS_QUERY]' and log level '4' for both UDP and TCP DNS traffic.
// Log DNS queries first (LOG doesn't terminate processing)
await execa('iptables', [
'-t', 'filter', '-A', CHAIN_NAME,
'-p', 'udp', '-d', dnsServer, '--dport', '53',
'-j', 'LOG', '--log-prefix', '[FW_DNS_QUERY] ', '--log-level', '4',
]);
await execa('iptables', [
'-t', 'filter', '-A', CHAIN_NAME,
'-p', 'udp', '-d', dnsServer, '--dport', '53',
'-j', 'ACCEPT',
]);
await execa('iptables', [
'-t', 'filter', '-A', CHAIN_NAME,
'-p', 'tcp', '-d', dnsServer, '--dport', '53',
'-j', 'LOG', '--log-prefix', '[FW_DNS_QUERY] ', '--log-level', '4',
]);
await execa('iptables', [
'-t', 'filter', '-A', CHAIN_NAME,
'-p', 'tcp', '-d', dnsServer, '--dport', '53',
'-j', 'ACCEPT',
]);
}
src/host-iptables.ts:375
- The IPv6 DNS logging feature adds LOG rules for IPv6 DNS queries but lacks test coverage. Since the host-iptables.test.ts file has test coverage for IPv6 DNS rules (verifying ip6tables usage for IPv6 DNS servers), the new LOG rules should also be tested to ensure they are created correctly with ip6tables before the ACCEPT rules.
Add test assertions to verify that ip6tables LOG rules are created with the correct parameters including the log prefix '[FW_DNS_QUERY]' and log level '4' for both UDP and TCP DNS traffic to IPv6 servers.
// Log DNS queries first (LOG doesn't terminate processing)
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-p', 'udp', '-d', dnsServer, '--dport', '53',
'-j', 'LOG', '--log-prefix', '[FW_DNS_QUERY] ', '--log-level', '4',
]);
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-p', 'udp', '-d', dnsServer, '--dport', '53',
'-j', 'ACCEPT',
]);
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-p', 'tcp', '-d', dnsServer, '--dport', '53',
'-j', 'LOG', '--log-prefix', '[FW_DNS_QUERY] ', '--log-level', '4',
]);
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-p', 'tcp', '-d', dnsServer, '--dport', '53',
'-j', 'ACCEPT',
]);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/docker-manager.ts
Outdated
| fs.copyFileSync(altSeccompPath, seccompDestPath); | ||
| logger.debug(`Seccomp profile written to: ${seccompDestPath}`); | ||
| } else { | ||
| logger.warn(`Seccomp profile not found at ${seccompSourcePath} or ${altSeccompPath}`); |
There was a problem hiding this comment.
The seccomp profile is referenced in the Docker Compose configuration but if the file is not found, only a warning is logged and the container will fail to start. The security_opt field at line 273 will reference a non-existent file path, causing Docker Compose to fail with an error like "open /path/to/seccomp-profile.json: no such file or directory".
Consider either throwing an error if the seccomp profile cannot be found, or conditionally adding the security_opt field only when the file exists. Since this is a security hardening feature, it's better to fail explicitly rather than silently degrading security.
| logger.warn(`Seccomp profile not found at ${seccompSourcePath} or ${altSeccompPath}`); | |
| const message = `Seccomp profile not found at ${seccompSourcePath} or ${altSeccompPath}`; | |
| logger.error(message); | |
| throw new Error(message); |
| 'MKNOD', // Prevents device node creation | ||
| ], | ||
| // Apply seccomp profile to restrict dangerous syscalls | ||
| security_opt: [`seccomp=${config.workDir}/seccomp-profile.json`], |
There was a problem hiding this comment.
The seccomp profile path is hardcoded in the Docker Compose configuration, but the seccomp file copy logic allows for the file to not exist (only logging a warning). This creates a disconnect where the Docker Compose configuration will always reference the seccomp profile at line 273, even if the file wasn't successfully copied.
This needs to be coordinated with the file copy logic - either make the seccomp profile mandatory and fail if it's missing, or conditionally add the security_opt configuration only when the file exists.
- throw error instead of warning when seccomp profile is not found, preventing docker from failing at runtime with cryptic errors - add test coverage for dns query logging iptables rules (ipv4 and ipv6) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
|
💫 TO BE CONTINUED... Smoke Claude failed! Our hero faces unexpected challenges... |
Reimplements PR #76 container security hardening on latest main branch.
Security Hardening
Capability restrictions:
NET_RAW,SYS_PTRACE,SYS_MODULE,SYS_RAWIO,MKNODfrom agent containerSyscall filtering:
kexec_*,*_module,mount,reboot,pivot_root, keyring operationsResource limits:
DNS audit logging:
[FW_DNS_QUERY]prefixImplementation Notes
Seccomp profile copied to work directory at runtime with fallback paths for source/dist deployments. DNS logging implemented as pre-ACCEPT LOG rules inside existing per-DNS-server loop structure.
All 349 tests pass. No security vulnerabilities detected by CodeQL.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.