fix(policies): use structured YAML parsing for policy preset merge

[tommylin-signalpro]

Summary

Fixes #1010

• Replace text-based string manipulation in mergePresetIntoPolicy() with structured YAML parsing via the yaml package
• Merge network_policies by name: preset entries override existing on name collision (prevents duplicates on re-apply)
• Preserve all non-network sections (filesystem_policy, process, landlock, etc.)
• Falls back to text-based approach for non-standard entry formats (backward compatibility)

Problem

The previous implementation used regex and line splitting to inject preset entries into existing policy YAML. This produced invalid YAML when:

• The same preset was applied twice (duplicate entries)
• Indentation varied between the current policy and preset content
• network_policies: appeared at unexpected positions in the document

Changes

• bin/lib/policies.js — Rewrite mergePresetIntoPolicy() to parse YAML, merge objects, serialize back. Add yaml as dependency.
• test/policies.test.js — Add 3 tests with realistic preset data: structured merge, name collision dedup, non-network section preservation. Relax existing string-format assertions to check correctness instead of exact formatting.

Test plan

• All 25 policy tests pass
• Full suite: 603/605 pass (2 pre-existing failures in install-preflight.test.js)
• Tested with real presets (npm, pypi, slack) on a live sandbox

Summary by CodeRabbit

• Bug Fixes

  • Improved policy merging to use structured YAML parsing for reliable network policy merges, preserving other top-level sections and ensuring a version header when missing.
  • Added robust fallbacks to the previous text-based behavior when parsing fails.

• Tests

  • Expanded tests for structured merge scenarios, preset overrides/deduplication, and preservation of non-policy sections.

• Chores

  • Added a YAML parsing library as a runtime dependency.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 30186383-1ced-45a5-86c4-e439614fd304

📥 Commits

Reviewing files that changed from the base of the PR and between 2ce593c and 3a287af.

📒 Files selected for processing (1)

• bin/lib/policies.js

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

• bin/lib/policies.js

---

📝 Walkthrough

Walkthrough

Rewrote merge logic in bin/lib/policies.js to use structured YAML parsing (yaml lib) for merging network_policies from presets into current policies, with object-level merging, preservation of other top-level sections, and multiple fallbacks to prior text-based behavior when parsing fails.

Changes

Cohort / File(s)	Summary
Core Policy Merge Logic bin/lib/policies.js	Replaced fragile line/regex-based insertion with YAML parsing (YAML.parse/YAML.stringify) in mergePresetIntoPolicy. Wraps preset entries under a synthetic network_policies: key, merges preset and current network_policies (preset wins on key collision), preserves non-network_policies top-level sections, forces `version: current.version
Dependency Addition package.json	Added runtime dependency yaml ^2.8.3. Minor reordering of devDependencies entries.
Test Coverage Expansion test/policies.test.js	Adjusted assertions to check for version presence via toContain and added structured YAML merge tests verifying merge behavior, collision override, and preservation of unrelated top-level sections.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I nibble on keys and hop through the code,
I swapped out the regex for a YAML road.
Policies tidy, no duplicates found,
I hop home content, with output well-formed and sound. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title clearly summarizes the main change: replacing text-based string manipulation with structured YAML parsing in the policy merge function.
Linked Issues check	✅ Passed	Changes fully address issue #1010: structured YAML parsing replaces regex-based merge, network_policies are deduplicated by name, all top-level sections preserved, and YAML dependency added.
Out of Scope Changes check	✅ Passed	All changes directly align with issue #1010 objectives; no unrelated modifications detected in policies.js, package.json, or test files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@bin/lib/policies.js`:
- Around line 175-176: current.network_policies may be an array, so spreading it
into an object (const mergedNp = { ...existingNp, ...presetPolicies }) will
create numeric keys and corrupt the data; update the merge to guard for arrays:
check Array.isArray(existingNp) (and that presetPolicies is an object) and if
existingNp is an array, do not object-spread — either preserve the array
(mergedNp = existingNp) or convert the array into an object map first (e.g., by
mapping entries by a name/id field) before merging; apply this guard around the
existingNp/mergedNp logic so merges only happen when existingNp is a plain
object.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b7982780-9e9e-45a7-94bb-143e08c56f8b

📥 Commits

Reviewing files that changed from the base of the PR and between eb4ba8c and 2ce593c.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (3)

• bin/lib/policies.js
• package.json
• test/policies.test.js

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Potential issue: current.network_policies could be an array.

If the existing policy uses list-style network_policies (an array of objects) rather than named policies (an object/map), the spread operator { ...existingNp } will produce keys like "0", "1", etc., corrupting the merge.

Consider adding a guard similar to the preset check:

🛡️ Proposed defensive check

   // Structured merge: preset entries override existing on name collision.
   // This prevents duplicate policy groups that the text-based approach
   // would create when re-applying the same preset.
-  const existingNp = current.network_policies || {};
+  const existingNp = (current.network_policies && typeof current.network_policies === "object" && !Array.isArray(current.network_policies))
+    ? current.network_policies
+    : {};
   const mergedNp = { ...existingNp, ...presetPolicies };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/lib/policies.js` around lines 175 - 176, current.network_policies may be
an array, so spreading it into an object (const mergedNp = { ...existingNp,
...presetPolicies }) will create numeric keys and corrupt the data; update the
merge to guard for arrays: check Array.isArray(existingNp) (and that
presetPolicies is an object) and if existingNp is an array, do not object-spread
— either preserve the array (mergedNp = existingNp) or convert the array into an
object map first (e.g., by mapping entries by a name/id field) before merging;
apply this guard around the existingNp/mergedNp logic so merges only happen when
existingNp is a plain object.

[wscurran]

✨ Thanks for submitting this fix with a detailed summary, it identifies a bug in the policy preset merge process and proposes a solution using structured YAML parsing, which could improve the stability and reliability of NemoClaw.