[Test Coverage] add branch coverage for ssl-key-storage and config-writer#5667
Conversation
ssl-key-storage.ts (65% → ~90%+ branches): - mountSslTmpfs: cover success path (return true) - secureWipeFile: zero-size file, non-regular file, ENOENT on open, suppressed close error in finally, ENOENT/non-ENOENT on post-wipe unlink (including retry path) - cleanupSslKeyMaterial: ssl_db exists but certs/ sub-dir absent config-writer.ts (78.94% → 80%+ branches): - writeAuditArtifacts: auditDir symlink guard - copySeccompProfile: dist-relative alt-path fallback - initializeSslBump: non-Error rejection string wrapping Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds targeted Jest unit tests to raise branch coverage for two security-critical modules (ssl-key-storage.ts and config-writer.ts) that were previously below the 80% threshold. The changes fit into the repo’s existing Jest-based testing strategy by adding focused test files under src/ to exercise previously untested error/edge branches.
Changes:
- Add direct unit tests for
ssl-key-storage.tsto cover tmpfs-mount success/failure and multiplesecureWipeFile+cleanupSslKeyMaterialedge branches. - Add a small branch-focused test suite for
config-writer.tscovering audit-dir symlink hardening, seccomp-profile alternate path fallback, and non-Errorrejection wrapping in SSL Bump initialization.
Show a summary per file
| File | Description |
|---|---|
| src/ssl-key-storage.test.ts | Adds direct unit tests that cover previously unexercised branches in SSL key tmpfs mounting and secure wipe / cleanup behaviors. |
| src/config-writer-branches.test.ts | Adds branch-focused tests to cover symlink hardening, seccomp-profile fallback path behavior, and SSL Bump error wrapping in config writing. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Low
|
✅ Copilot review passed with no inline comments. @github-actions[bot] Add the |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (3 files)
Coverage comparison generated by |
|
@copilot fix this failing ci check https://github.com/github/gh-aw-firewall/pull/5667/checks?check_run_id=84071218422 |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
✅ Contribution Check completed successfully! Contribution guidelines review complete for PR #5667: no important missing items found; no comment needed. |
|
🚀 Security Guard has started processing this pull request |
|
✅ Smoke Claude passed |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ Build Test Suite completed successfully! |
Smoke Test: Copilot BYOK (Direct) ModeAll tests passed ✅
Status: PASS (Direct BYOK mode via COPILOT_PROVIDER_API_KEY)
|
🔥 Smoke Test Results — Auth mode: PAT (COPILOT_GITHUB_TOKEN)
Overall: PASS PR: "[Test Coverage] add branch coverage for ssl-key-storage and config-writer" by
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
|
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: PASS
|
🔬 Smoke Test Results
PR: [Test Coverage] add branch coverage for ssl-key-storage and config-writer Overall: PASS (core engine tests pass; pre-step template variable expansion issue noted)
|
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: API Proxy OpenTelemetry Tracing
All scenarios pass. OTEL tracing integration is fully implemented and functional.
|
Chroot Version Comparison Results
Overall: ❌ Tests did not pass — Python and Node.js versions differ between host and chroot environments.
|
|
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall: PASS
|
Smoke Test: Services Connectivity — ❌ FAIL
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
|
Merged PRs:
|
Adds targeted Jest tests to improve branch coverage for two security-critical modules that were below the 80% threshold.
Changes
src/ssl-key-storage.test.ts(new — 11 tests)Targets
ssl-key-storage.tswhich had 65% branch coverage (7 uncovered branches out of 20). Covers:mountSslTmpfs— success path (return true) was never exercised; existingssl-bump.test.tsalways rejectsmountcommandssecureWipeFile— six edge-case branches:if (size > 0)false branch — no write/fsync, still deletes!stat.isFile()true — error caught and logged, no writeopenSync: earlyreturninside outercatchcatchinfinallyblockunlinkSync: earlyreturnwithout retryunlinkSyncerror: retry unlink + debug logcleanupSslKeyMaterial— two additional branch paths:ssl_db/exists butssl_db/certs/does not → cert-iteration loop skippedssl_db/does not exist → outeriffalse branchsrc/config-writer-branches.test.ts(new — 3 tests)Targets
config-writer.tswhich had 78.94% branch coverage (just under the 80% threshold). Covers:writeAuditArtifacts— symlink guard forauditDir(creates real symlink, asserts throw)copySeccompProfile— dist-relative alt-path fallback when thesrc/../containers/path is absentinitializeSslBump— wrapping of non-Errorstring rejections fromgenerateSessionCaTest results
All 14 new tests pass. No existing tests modified or removed.