fix(policies): deduplicate preset entries on re-apply to prevent inva…

bin/lib/policies.js

@@ -93,11 +93,70 @@ function buildPolicyGetCommand(sandboxName) {
   return `${getOpenshellCommand()} policy get --full ${shellQuote(sandboxName)} 2>/dev/null`;
 }
 
+/**
+ * Extract top-level mapping key names from a network_policies block.
+ * Keys are 2-space-indented identifiers followed by a colon, e.g. "  telegram_bot:".
+ */
+function extractPolicyKeyNames(text) {
+  const keys = [];
+  for (const m of text.matchAll(/^ {2}([a-zA-Z_][\w-]*):/gm)) {
+    keys.push(m[1]);
+  }
+  return keys;
+}
+
+/**
+ * Remove entries from the network_policies block whose key names appear in
+ * `keysToRemove`. Only operates within the `network_policies:` section so
+ * identically named keys in other top-level sections are not affected.
+ * Each entry starts at a 2-space-indented key line and extends to just before
+ * the next 2-space-indented key or the next top-level (non-indented) key.
+ */
+function stripPolicyKeys(policyText, keysToRemove) {
+  if (!keysToRemove.length) return policyText;
+  const removeSet = new Set(keysToRemove);
+  const lines = policyText.split("\n");
+  const result = [];
+  let inNetworkPolicies = false;
+  let skipping = false;
+
+  for (const line of lines) {
+    // Track when we enter/leave the network_policies section
+    if (/^network_policies\s*:/.test(line)) {
+      inNetworkPolicies = true;
+      result.push(line);
+      continue;
+    }
+    // Any other top-level key exits the section (and ends any skip)
+    if (/^\S/.test(line)) {
+      inNetworkPolicies = false;
+      skipping = false;
+    }
+
+    if (inNetworkPolicies) {
+      const keyMatch = line.match(/^ {2}([a-zA-Z_][\w-]*):/);
+      if (keyMatch) {
+        skipping = removeSet.has(keyMatch[1]);
+        if (skipping) continue;
+      }
+    }
+
+    if (!skipping) {
+      result.push(line);
+    }
+  }
+  return result.join("\n");
+}
+
 /**
  * Merge preset entries into existing policy YAML. Handles versionless policies
  * by ensuring the merged result has a version header when the current policy
  * has content but no version field. Pure function for testing.
  *
+ * When a preset entry's key already exists in the current policy, the old
+ * entry is replaced (update semantics) so re-applying a preset picks up
+ * any changes without creating duplicate mapping keys.
+ *
  * @param {string} currentPolicy - Existing policy YAML (may be versionless)
  * @param {string} presetEntries - Indented network_policies entries from preset
  * @returns {string} Merged YAML with version header when missing
@@ -110,9 +169,14 @@ function mergePresetIntoPolicy(currentPolicy, presetEntries) {
     return "version: 1\n\nnetwork_policies:\n" + presetEntries;
   }
 
+  // Strip existing entries whose keys overlap with the incoming preset
+  // so re-applying a preset replaces rather than duplicates.
+  const incomingKeys = extractPolicyKeyNames(presetEntries);
+  const deduped = stripPolicyKeys(currentPolicy, incomingKeys);
+
   let merged;
-  if (/^network_policies\s*:/m.test(currentPolicy)) {
-    const lines = currentPolicy.split("\n");
+  if (/^network_policies\s*:/m.test(deduped)) {
+    const lines = deduped.split("\n");
     const result = [];
     let inNetworkPolicies = false;
     let inserted = false;
@@ -220,6 +284,8 @@ module.exports = {
   loadPreset,
   getPresetEndpoints,
   extractPresetEntries,
+  extractPolicyKeyNames,
+  stripPolicyKeys,
   parseCurrentPolicy,
   buildPolicySetCommand,
   buildPolicyGetCommand,

test/policies.test.js

@@ -169,6 +169,84 @@ describe("policies", () => {
       expect(merged.startsWith("version: 1\n\nnetwork_policies:")).toBe(true);
       expect(merged).toContain("example.com");
     });
+
+    it("does not create duplicate keys when the same preset is applied twice", () => {
+      const telegramEntries = policies.extractPresetEntries(
+        policies.loadPreset("telegram"),
+      );
+      const base = "version: 1\n\nnetwork_policies:\n" + telegramEntries;
+      const merged = policies.mergePresetIntoPolicy(base, telegramEntries);
+
+      const keyCount = (merged.match(/telegram_bot:/g) || []).length;
+      expect(keyCount).toBe(1);
+    });
+
+    it("replaces stale entry when preset is re-applied (update semantics)", () => {
+      const stale =
+        "version: 1\n\nnetwork_policies:\n" +
+        "  telegram_bot:\n" +
+        "    name: telegram_bot\n" +
+        "    endpoints:\n" +
+        "      - host: old.stale.example.com\n" +
+        "        port: 443\n";
+      const telegramEntries = policies.extractPresetEntries(
+        policies.loadPreset("telegram"),
+      );
+      const merged = policies.mergePresetIntoPolicy(stale, telegramEntries);
+
+      expect(merged).not.toContain("old.stale.example.com");
+      expect(merged).toContain("api.telegram.org");
+      expect((merged.match(/telegram_bot:/g) || []).length).toBe(1);
+    });
+
+    it("preserves unrelated entries when deduplicating", () => {
+      const telegramEntries = policies.extractPresetEntries(
+        policies.loadPreset("telegram"),
+      );
+      const existing =
+        "version: 1\n\nnetwork_policies:\n" +
+        "  github:\n" +
+        "    name: github\n" +
+        "    endpoints:\n" +
+        "      - host: github.com\n" +
+        "        port: 443\n" +
+        "  telegram_bot:\n" +
+        "    name: telegram_bot\n" +
+        "    endpoints:\n" +
+        "      - host: api.telegram.org\n" +
+        "        port: 443\n";
+      const merged = policies.mergePresetIntoPolicy(existing, telegramEntries);
+
+      expect(merged).toContain("github:");
+      expect(merged).toContain("api.telegram.org");
+      expect((merged.match(/telegram_bot:/g) || []).length).toBe(1);
+    });
+
+    it("does not strip same-named keys outside network_policies", () => {
+      const telegramEntries = policies.extractPresetEntries(
+        policies.loadPreset("telegram"),
+      );
+      // Contrived policy where a non-network_policies section has a key
+      // matching the preset entry name ("telegram_bot").
+      const existing =
+        "version: 1\n\n" +
+        "other_section:\n" +
+        "  telegram_bot:\n" +
+        "    some_setting: true\n\n" +
+        "network_policies:\n" +
+        "  telegram_bot:\n" +
+        "    name: telegram_bot\n" +
+        "    endpoints:\n" +
+        "      - host: api.telegram.org\n" +
+        "        port: 443\n";
+      const merged = policies.mergePresetIntoPolicy(existing, telegramEntries);
+
+      // The key inside other_section must survive
+      expect(merged).toContain("other_section:");
+      expect(merged).toContain("some_setting: true");
+      // The network_policies entry is replaced (not duplicated)
+      expect((merged.match(/telegram_bot:/g) || []).length).toBe(2);
+    });
   });
 
   describe("preset YAML schema", () => {