[Test Coverage] squid/config-generator.ts - #6679
Conversation
- 53 tests covering all branches in generateSquidConfig - Covers: domain ACLs, blocked domains, SSL bump, DLP, API proxy IP validation, DNS, upstream proxy, topology peers, host access, and static boilerplate (security rules, timeouts, logging) - Validates error path: invalid apiProxyIp throws security error Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Jest coverage for Squid configuration generation and its security-sensitive ACL ordering.
Changes:
- Tests domain, proxy, SSL, DLP, DNS, topology, and host-access configuration.
- Adds validation and static configuration assertions.
Show a summary per file
| File | Description |
|---|---|
src/squid/config-generator.test.ts |
Adds comprehensive generator tests, though several assertions are ineffective or duplicate existing suites. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
src/squid/config-generator.test.ts:183
- These assertions only prove that the blocklist ACL and deny directive exist; they do not test the claimed precedence over the allowlist rule. Moving
http_access deny blocked_domainsafter the allowlist decision would preserve both assertions while allowing a blocked subdomain.
expect(config).toContain('acl blocked_domains dstdomain .internal.example.com');
expect(config).toContain('http_access deny blocked_domains');
src/squid/config-generator.test.ts:367
- This assertion is satisfied by every generated config because the baseline port section always contains
Safe_ports;enableHostAccess: truewithoutallowHostPortsadds nothing. The test therefore cannot catch any regression in the host-access gate. Use the negative branch alongside the following positive custom-port test.
it('generates host access config when enableHostAccess is true', () => {
const config = generateSquidConfig(makeConfig({ enableHostAccess: true }));
// Host access enables additional Safe_ports entries
expect(config).toContain('Safe_ports');
});
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Medium
| expect(config).toContain('# No domains configured'); | ||
| expect(config).toContain('http_access deny all'); |
| const allowIdx = config.indexOf('172.30.0.30'); | ||
| const denyIdx = config.indexOf('http_access deny dst_ipv4'); | ||
| expect(allowIdx).toBeLessThan(denyIdx); |
| * Tests for src/squid/config-generator.ts – generateSquidConfig. | ||
| * | ||
| * Exercises the public surface of generateSquidConfig: |
|
@copilot address review feedback |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed... |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✅ Contribution Check completed successfully! PR #6679 appears to satisfy the contribution checklist from the provided context: the change adds comprehensive tests, and no documentation or file-organization issues are evident from the metadata. No comment needed. |
|
🔌 Smoke Services — All services reachable! ✅ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ Smoke Gemini completed. All facets verified. 💎 Testing safeoutputs bridge |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed... |
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ |
|
✅ Smoke Claude passed |
|
🚀 Security Guard has started processing this pull request |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Build Test Suite completed successfully! |
Smoke Test: Copilot BYOK (Direct) — PR #6679
Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com Overall: PARTIAL — BYOK inference path confirmed working; pre-fetched data unavailable for remaining checks.
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
|
EGRESS_RESULT allow=pass deny=pass ✅ Allowed domain ( Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
github-actions
Bot
added
smoke-claude
smoke-copilot-network-isolation
Smoke Test Results — Services Connectivity
Overall: FAIL
|
Smoke Test: API Proxy OpenTelemetry Tracing
Summary: All 5 scenarios pass. OTEL integration is functional — module loads, 39 tests pass, env vars forward correctly,
|
Smoke Test: Gemini Engine Validation
Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
🔬 Smoke Test Results
Overall: PASS (core connectivity verified) /cc
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
|
Smoke test results
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.
|
Chroot Version Comparison Results
Overall: ❌ FAILED — Node.js version mismatch (host: v24.18.0, chroot: v22.23.1).
|
Smoke Test Results
Overall: PASS cc
|
Adds comprehensive Jest unit tests for
src/squid/config-generator.ts, bringing it from 0% to 100% coverage (statements, branches, functions, lines).What's covered
53 tests across these areas:
cache_peer)Security coverage highlights
apiProxyIpinjection prevention: invalid IPs (non-IPv4, out-of-range octets) throwSECURITY:errorshttp_access deny dst_ipv4/dst_ipv6raw-IP block rules are presentforwarded_for deleteandvia offprivacy headersTest approach
generateSquidConfigis a pure string-generation functionmakeConfig()helper with required-field defaults