[Test Coverage] security: test coverage for compose-sanitizer, domain-validation, and domain-matchers#5554
Conversation
…in-matchers Add 81 unit tests covering three security-critical modules with no prior direct test coverage: - compose-sanitizer.ts: sanitizeDockerComposeYaml() redacts TOKEN/KEY/SECRET values from Docker Compose environments (object and array formats), including edge cases (no '=' separator, values with embedded '=', null services, invalid YAML). - domain-validation.ts: validateDomainOrPattern() covers all five security checks: empty input, dangerous characters (space, null, quotes, backtick, semicolon, hash, backslash), over-broad wildcards (*, *.*), double dots, lone dot, and excessive wildcard segments. Also tests the SQUID_DANGEROUS_CHARS regex export. - domain-matchers.ts: parseDomainList() (plain vs wildcard categorisation, protocol stripping, validation errors), isDomainMatchedByPattern() (length guard, protocol compatibility matrix, case-insensitivity), and parseUrlPatterns() (anchoring, HOST_CHAR_PATTERN vs URL_CHAR_PATTERN, .* preservation). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds new Jest unit test suites intended to increase coverage for three security-sensitive areas: Docker Compose YAML sanitization, domain/pattern validation (Squid-injection prevention), and domain/url pattern matching.
Changes:
- Adds
compose-sanitizerunit tests covering environment redaction across object/array forms plus several edge cases. - Adds
domain-validationunit tests covering dangerous character rejection, wildcard breadth checks, and error message details. - Adds
domain-matchersunit tests covering domain list parsing, protocol-aware wildcard matching, and URL pattern parsing/anchoring.
Show a summary per file
| File | Description |
|---|---|
src/compose-sanitizer.test.ts |
Adds direct unit tests for secret redaction behavior and sanitizer edge cases. |
src/domain-validation.test.ts |
Adds direct unit tests for domain/pattern validation and exported regex behavior. |
src/domain-matchers.test.ts |
Adds direct unit tests for domain list parsing, wildcard matching, and URL pattern conversion. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 4
- Review effort level: Low
| it('rejects any pattern composed only of * and .', () => { | ||
| // purely wildcard+dot patterns like *.* are caught in checkOverBroadPattern | ||
| expect(() => validateDomainOrPattern('*.*.*')).toThrow(/too broad/); | ||
| }); |
| const result = sanitizeDockerComposeYaml(raw); | ||
| // Should round-trip without throwing | ||
| expect(result).toBeTruthy(); | ||
| }); |
| /** | ||
| * Tests for domain-validation.ts | ||
| * | ||
| * validateDomainOrPattern() is the Squid-injection prevention path for | ||
| * --allow-domains and --allow-urls. Security-critical: prevents malicious | ||
| * input from escaping into generated Squid configuration. | ||
| */ |
| /** | ||
| * Tests for domain-matchers.ts | ||
| * | ||
| * Security-critical module: parses domain lists and matches domains | ||
| * against wildcard patterns with protocol-awareness. | ||
| */ | ||
|
|
||
| import { | ||
| parseDomainList, | ||
| isDomainMatchedByPattern, | ||
| parseUrlPatterns, | ||
| } from './domain-matchers'; |
|
@copilot address review feedback |
|
✅ Copilot review passed with no inline comments. @github-actions[bot] Add the |
|
✅ Smoke Claude passed |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
✅ Contribution Check completed successfully! Contribution guideline review complete: PR #5554 follows the applicable CONTRIBUTING.md requirements based on the provided context; no comment needed. |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Build Test Suite completed successfully! |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
🚀 Security Guard has started processing this pull request |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
🔌 Smoke Services — All services reachable! ✅ |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
Smoke Test: Claude Engine Validation
Overall result: PASS
|
🔬 Smoke Test: Copilot PAT Auth — PASS
Overall: PASS · Auth mode: PAT (COPILOT_GITHUB_TOKEN) PR by
|
|
Smoke test summary for PR title only:
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.
|
Smoke Test: Copilot BYOK (Direct)✅ MCP Connectivity — Successfully queried merged PRs (gh/gh-aw-firewall) Status: PASS (Direct BYOK mode active — COPILOT_PROVIDER_API_KEY routed via sidecar)
|
🔥 Smoke Test Results — PASS
Overall: PASS PR: "[Test Coverage] security: test coverage for compose-sanitizer, domain-validation, and domain-matchers" by
|
|
Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra Overall status: PASS
|
🔍 Smoke Test: API Proxy OpenTelemetry Tracing
All scenarios pass. OTEL integration is functional.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Chroot Version Comparison Results
Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environment.
|
Smoke Test Results — Service Connectivity
Overall: FAIL —
|
Smoke Test Results
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.
|
|
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall: PASS
|
lpcox
deleted the
test-coverage/security-modules-compose-domain-1688f16dff4d5c64
branch
Adds 81 unit tests covering three security-critical modules that had no direct test coverage.
What's covered
src/compose-sanitizer.tssanitizeDockerComposeYaml()redactsTOKEN/KEY/SECRETvalues before Docker Compose YAML is logged:environmentsections=separator (no-op path)=signsnull-valued services, services that are arrays, missingservicessection, invalid/non-object YAMLsrc/domain-validation.tsvalidateDomainOrPattern()is the Squid-injection prevention gate for--allow-domains:\n,\r,",',`,;,#,\, null byteU+XXXXformat*,*.*, purely wildcard+dot patternsSQUID_DANGEROUS_CHARSexported regexsrc/domain-matchers.tsparseDomainList(): plain vs wildcard categorisation, protocol stripping, validation errorsisDomainMatchedByPattern(): domain length guard (ReDoS), all protocol compatibility combinations (both/http/https), case-insensitive matchingparseUrlPatterns(): URL anchoring,HOST_CHAR_PATTERNvsURL_CHAR_PATTERNfor wildcards,.*preservation via placeholder, trailing slash strippingTest results
All 81 new tests pass. The three new test files also pass
eslint --max-warnings=0individually (the commit hook failure was due to pre-existing lint warnings in unrelated files).