Extract shared VMM account validation helper - #9030
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The refactor preserves existing validation behavior and includes focused coverage for the extracted logic.
Review effort: Balanced
Findings: None
What changed in this PR
Extracts duplicated, security-sensitive VMM account validation into a shared helper while preserving caller-specific checks and errors.
Changes:
- Centralizes account, group, and passwd validation.
- Updates Cloud Hypervisor and NVX identity managers to use the helper.
- Adds focused unit tests for shared and caller-specific validation.
| File | Description |
|---|---|
src/vmm-account-validation.ts |
Adds the shared validation helper and types. |
src/vmm-account-validation.test.ts |
Tests successful and rejected validation paths. |
src/nvx/runtime-lifecycle.ts |
Delegates validation while retaining the run-ID check. |
src/cloud-hypervisor/vmm-identity.ts |
Delegates account validation to the helper. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 92.65% | 92.65% | ➡️ +0.00% |
| Statements | 91.13% | 91.13% | ➡️ +0.00% |
| Functions | 89.17% | 89.17% | ➡️ +0.00% |
| Branches | 84.23% | 84.21% | 📉 -0.02% |
📁 Per-file Coverage Changes (4 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/nvx/one-shot-adapter.ts |
83.5% → 82.9% (-0.60%) | 80.3% → 79.8% (-0.56%) |
src/nvx/runtime-lifecycle.ts |
84.6% → 84.2% (-0.37%) | 79.8% → 79.4% (-0.43%) |
src/cloud-hypervisor/vmm-identity.ts |
99.0% → 99.0% (-0.04%) | 95.2% → 95.0% (-0.20%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
✨ New Files (1 files)
src/vmm-account-validation.ts: 100.0% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "o205451.ingest.us.sentry.io"See Network Configuration for more information.
|
|
🚀 Security Guard has started processing this pull request |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed...
|
|
🔌 Smoke Services — All services reachable! ✅
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤
|
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓
|
|
✅ Smoke Claude passed Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.anthropic.com"See Network Configuration for more information.
|
|
❌ Smoke Gemini reports failed. Facets need polishing... Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "play.googleapis.com"See Network Configuration for more information.
|
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"
- "msfeed25.pkgs.visualstudio.com"See Network Configuration for more information.
|
|
✅ Build Test Suite completed successfully! Warning Firewall blocked 8 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.nuget.org"
- "bun.sh"
- "dc.services.visualstudio.com"
- "deno.land"
- "dl.deno.land"
- "github.com"
- "releaseassets.githubusercontent.com"
- "repo.maven.apache.org"See Network Configuration for more information.
|
|
Smoke Cloud Hypervisor completed. Cloud Hypervisor + Copilot passed. Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"
- "github.com"See Network Configuration for more information.
|
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed...
|
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"
- "example.com"See Network Configuration for more information.
|
Smoke Test: Cloud Hypervisor + Copilot
All checks passed. Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"
- "github.com"See Network Configuration for more information.
|
Smoke Test: Claude Engine Validation
Overall result: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.anthropic.com"See Network Configuration for more information.
|
|
Smoke Test: Copilot Engine
Overall: PASS cc
|
|
Smoke Test: Copilot BYOK (Direct) Mode ✅ PASS
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) with agent → api-proxy sidecar credential injection.
|
|
EGRESS_RESULT allow=pass deny=pass ✅ Allowed domain (github.com) reachable: Overall: PASS cc Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"
- "example.com"See Network Configuration for more information.
|
github-actions
Bot
added
the
smoke-copilot-network-isolation
|
Smoke Test: Services Connectivity
Overall: PASS
|
Chroot Version Comparison Results
Overall result: FAILED — Node.js version mismatch between host and chroot environment. Python and Go versions match correctly, but Node.js differs (host Since not all tests passed, the
|
Smoke Test: API Proxy OpenTelemetry Tracing
All 5 scenarios pass — no OTEL tracing regressions detected. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "o205451.ingest.us.sentry.io"See Network Configuration for more information.
|
|
Smoke test:
Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"
- "msfeed25.pkgs.visualstudio.com"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — PASS Note: Maven's default local repository ( Warning Firewall blocked 8 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.nuget.org"
- "bun.sh"
- "dc.services.visualstudio.com"
- "deno.land"
- "dl.deno.land"
- "github.com"
- "releaseassets.githubusercontent.com"
- "repo.maven.apache.org"See Network Configuration for more information.
|
The Cloud Hypervisor VMM identity path and the NVX runtime lifecycle path each implemented their own
resolveAndValidateAccount(), duplicating theid/getentcalls, supplementary-group check, and passwd-state guard used to validate a freshly allocated system account before trusting it.Shared helper
src/vmm-account-validation.tsexportingresolveAndValidateVmmAccount(), which owns the commonid -u/-g/-G+getent passwdresolution, the supplementary-group safety check, and the base passwd-shape guard (/nonexistenthome,/usr/sbin/nologinshell, matching uid/gid/name fields).parsePositiveInteger(error wording differs slightly between the two sites) and an optionalassertPasswdStatecallback for any extra site-specific constraint.Call sites
src/cloud-hypervisor/vmm-identity.ts:resolveAndValidateAccount()now delegates to the shared helper withaccountLabel: 'Cloud Hypervisor VMM'.src/nvx/runtime-lifecycle.ts:resolveAndValidateAccount()delegates to the shared helper withaccountLabel: 'NVX VMM', passing the run-id home-directory constraint viaassertPasswdState:Error messages and validation outcomes are unchanged; both sites now share the same underlying safety checks instead of maintaining two copies.
Tests
src/vmm-account-validation.test.tscovering the shared helper directly: successful resolution, supplementary-group rejection, unsafe passwd rejection, and both outcomes of a caller-suppliedassertPasswdState.