fix(policy): reject invalid custom preset YAML (Fixes #9406)

[deepujain]

Summary

Custom preset application now fails closed when the full preset document cannot be parsed into a network_policies mapping. This prevents malformed metadata above a valid policy tail from bypassing the custom-preset guards.

Related Issue

Fixes #9406

Changes

• Reject invalid or missing custom network_policies before reserved-key, allowed_ips, semantic, disclosure, or mutation work.
• Add a regression for a duplicate-key preset whose extracted policy tail remains valid.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Review pending.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit: Not applicable.
• Station profile/scenario: Not applicable.
• Result: Not applicable.
• Supporting evidence: Not applicable.

Verification

• PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project cli src/lib/policy/preset-allowed-ips.test.ts passed 14 tests.
• Applicable broad gate passed — npm run test:changed passed 139 files and 1,944 tests after the 25-test growth-guardrail lane passed.
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

Additional validation:

• npm run build:cli passed.
• npm run typecheck:cli passed.
• git diff --check passed.

---

Signed-off-by: Deepak Jain deepujain@gmail.com

Summary by CodeRabbit

• Bug Fixes
  • Custom policy presets are now rejected when policy data is missing, malformed, or invalid.
  • Presets with duplicate metadata or invalid policy definitions no longer continue through the application process.
  • Clear error reporting helps identify rejected preset configurations and prevents unintended policy changes.
  • Valid custom presets continue to be applied and recorded as provided.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3220a3e9-1d62-4284-a4f7-de9f5b28de83

📥 Commits

Reviewing files that changed from the base of the PR and between ec1d3d6 and 18e1741.

📒 Files selected for processing (2)

• src/lib/policy/index.ts
• test/policies.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/policy/index.ts

Included review availability: Your plan includes up to 12 reviews per rolling hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

Custom preset application now rejects unparseable or missing network_policies data. Tests cover invalid full-document YAML and update the valid custom preset fixture.

Changes

Custom preset validation

Layer / File(s)	Summary
Reject invalid custom presets src/lib/policy/index.ts, src/lib/policy/preset-allowed-ips.test.ts, test/policies.test.ts	The custom preset path logs an error and returns false when parsing does not produce a valid policy map. Tests verify invalid YAML rejection and update the valid preset metadata and test name.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 18e17

The change makes malformed custom preset documents fail closed before policy checks and mutation, with regression coverage and validation reported as passing. No actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

• NVIDIA/NemoClaw#9296: Both changes modify custom network policy preset handling.
• NVIDIA/NemoClaw#9346: Both changes modify custom preset validation and rejection logic.
• NVIDIA/NemoClaw#9306: Both changes modify custom preset application and validation.

Suggested labels: area: policy, security, bug-fix

Suggested reviewers: cv, prekshivyas

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes rejecting invalid custom preset YAML and references the linked issue.
Linked Issues check	✅ Passed	The changes fail closed on invalid custom preset mappings and add regression coverage for the reported whole-file parse bypass in [#9406].
Out of Scope Changes check	✅ Passed	The source changes, regression tests, and fixture updates directly support the linked issue and stated objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Model comparison: normalized findings match; normalized terminology decisions differ; normalized E2E selections differ; severity counts match.

1 additional E2E selection from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

• onboard-policy-preset-sequencing: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

Since last review: 0 prior items resolved · 0 still apply · 0 new items found

1 semantic terminology decision

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

• justified — full YAML document at src/lib/policy/preset-allowed-ips.test.ts:212: Retain “full YAML document” to distinguish complete-input validation from validation of an extracted policy tail.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: inference-routing

Manual-only E2E: security-posture, cloud-inference, network-policy
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

1 optional E2E recommendation

• ubuntu-policy-custom-missing-presets-negative

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[udsy19]

Thanks for picking this up so quickly. I filed #9406 and had a branch on the same approach, so rather than open a duplicate here are three things from my testing that may be worth folding in.

Two now-dead conditions. With the early return in place, np is non-null for the rest of the block, so the np && on the reserved-key check and the np !== null && on the allowed_ips ban can go. Leaving them suggests to a later reader that null is still reachable there, which is the assumption that caused this in the first place.

A positive regression test. The risk in turning a silent skip into a hard failure is over-rejection. I enumerated the shapes empirically against the pinned yaml@2.8.3: exactly three become newly rejected — a --- document separator above network_policies:, tab indentation above it, and the duplicate-key case from the issue. Still accepted and applied: a leading --- marker, leading comments, a BOM, a ... end marker, and unsubstituted ${...} templates. A test asserting a well-formed custom preset still applies and is still recorded in customPolicies pins that boundary.

Replay paths are safe, and it is worth stating why. registry.addCustomPolicy is the only writer of customPolicies, and both call sites store whole-file-parseable content — applyPresetContent persists content: presetContent verbatim, and the MCP bridge persists YAML.stringify output. So rebuild and snapshot restore cannot start failing for existing users. I also checked all 35 blueprint YAML files parse cleanly; the 9 without a network_policies mapping were already refused by extractPresetEntries.

Happy to send any of this as a PR against your branch, or as a follow-up after this merges — whichever you prefer.

[prekshivyas]

Reviewed commit ec1d3d6. No blocking findings.

[deepujain]

Removed both unreachable null guards and strengthened the positive path to apply a complete custom preset and record its content verbatim in customPolicies. Focused policy validation passes 121/121. Replay remains safe because the registry writers persist either accepted preset content or YAML.stringify output. Rebased on main; both PR commits are signed and GitHub Verified.