fix(integrations): close the two holes the blocked-container guard left open

A re-review of yesterday's fix found it incomplete in one direction and
crashing in another.

`typeof null === "object"`, so a document that is literally `null` walked past
the guard: `blockedContainerPath` treated it as an absent prefix, apply
reported success, and the user's file was replaced wholesale. Only `undefined`
means absent — a missing file parses as `{}`, while a parsed `null` is a value
the file actually contains.

And `disableIntegration` still assumed everything past the conflict branch has
an ownership record, which a blocked container never does. Direct disable threw
a TypeError and the management route answered 500. The GUI locks the switch for
`unsafe`, but `ocx integration client disable` and API callers do not — the
surface that hides a defect is not the surface that has to be correct.

Tests added for both, plus OpenClaw's nested `models.providers` collision,
which a one-level check would miss, and the IPv6/wildcard base URLs asserted
against the emitted bytes rather than the helper — bypassing the shared
composer is what caused that defect in the first place, so a helper-level test
would not have caught it.

Both new guards driven red before restoring.

Author: lidge-jun

src/integrations/state.ts

@@ -73,8 +73,19 @@ export function blockedContainerPath(
     let cursor: unknown = doc;
     for (let depth = 0; depth < fragment.path.length - 1; depth += 1) {
       const key = fragment.path[depth]!;
-      if (cursor === undefined || cursor === null) break;
-      if (typeof cursor !== "object" || Array.isArray(cursor)) return fragment.path.slice(0, depth);
+      /*
+       * ONLY `undefined` means absent. A missing file parses as `{}`, so an
+       * absent prefix reads `undefined` — but a parsed `null` is a value the
+       * user's file actually contains, and treating it as absent let a
+       * document that is literally `null` be replaced wholesale by a
+       * "successful" apply.
+       */
+      if (cursor === undefined) break;
+      // `typeof null === "object"`, so null has to be named explicitly or it
+      // walks straight into the dereference below.
+      if (cursor === null || typeof cursor !== "object" || Array.isArray(cursor)) {
+        return fragment.path.slice(0, depth);
+      }
       const next = (cursor as Record<string, unknown>)[key];
       if (next === undefined) break;
       if (typeof next !== "object" || next === null || Array.isArray(next)) {

src/integrations/writer.ts

@@ -304,6 +304,19 @@ export function disableIntegration(input: IntegrationWriteInput): WriteOutcome {
         ? `${configPath} changed after opencodex wrote it; disabling would discard that edit`
         : `${configPath} contains an opencodex block we did not write`);
   }
+  /*
+   * `unsafe` reaches here the same way it reaches apply, and the code below
+   * dereferences `record` on the assumption that anything past this point is
+   * `current` or `stale`. A blocked container has no record, so disable threw
+   * a TypeError and the route answered 500 — the GUI locks the switch, but the
+   * CLI and direct API callers do not.
+   */
+  if (classified.state === "unsafe") {
+    return refuse(clientId, "unsafe", "unsafe",
+      classified.reason === "blocked-container"
+        ? `${configPath} holds a value where opencodex would have to read a section, so nothing can be removed safely`
+        : `${configPath} cannot be changed safely`);
+  }
 
   // current | stale only: the file fingerprint still matches our record, so the
   // recorded paths are exactly what we put there.

tests/integrations-invariants.test.ts

@@ -248,6 +248,87 @@ describe("a container we would have to replace is refused, not overwritten", ()
       expect(store.listOperations()).toHaveLength(0);
     });
   }
+
+  test("openclaw: a collision in the NESTED container is refused too", () => {
+    // OpenClaw's fragment path is two segments (`models.providers`), so a
+    // one-level check would miss a collision at the inner container.
+    const configPath = installClient("openclaw");
+    const seed = '{\n  models: {\n    providers: ["user-value"],\n  },\n}\n';
+    writeFileSync(configPath, seed);
+
+    const result = applyIntegration({
+      clientId: "openclaw", models: MODELS, config: CONFIG, port: 10100,
+      env: TEST_ENV, home, store,
+    });
+    expect(result.ok).toBe(false);
+    if (!result.ok) expect(result.reason).toBe("unsafe");
+    expect(readFileSync(configPath, "utf8")).toBe(seed);
+  });
+
+  test("a document that is literally null is not treated as absent", () => {
+    /*
+     * A missing file parses as `{}`, so an absent prefix reads `undefined`.
+     * A parsed `null` is a value the file actually contains — treating it as
+     * absent let apply replace the whole document and report success.
+     */
+    const configPath = installClient("pi");
+    writeFileSync(configPath, "null\n");
+
+    const result = applyIntegration({
+      clientId: "pi", models: MODELS, config: CONFIG, port: 10100,
+      env: TEST_ENV, home, store,
+    });
+    expect(result.ok).toBe(false);
+    expect(readFileSync(configPath, "utf8")).toBe("null\n");
+  });
+
+  test("disable refuses a blocked container instead of throwing", () => {
+    /*
+     * The GUI locks the switch for `unsafe`, but `ocx integration client
+     * disable` and direct API callers do not — and the removal path
+     * dereferences a record that a blocked container never has, so this threw
+     * a TypeError and surfaced as a 500.
+     */
+    const configPath = installClient("pi");
+    const seed = '{\n  "providers": ["user-value"]\n}\n';
+    writeFileSync(configPath, seed);
+
+    const result = disableIntegration({
+      clientId: "pi", models: MODELS, config: CONFIG, port: 10100,
+      env: TEST_ENV, home, store,
+    });
+    expect(result.ok).toBe(false);
+    if (!result.ok) expect(result.reason).toBe("unsafe");
+    expect(readFileSync(configPath, "utf8")).toBe(seed);
+  });
+});
+
+describe("the base URL is composed, never interpolated", () => {
+  test("IPv6 and wildcard binds produce a URL a client can actually dial", () => {
+    /*
+     * The defect was bypassing the shared composer, so this asserts the
+     * emitted bytes rather than the helper — bypassing it again would pass a
+     * helper-level test.
+     */
+    const cases: [string, string][] = [
+      ["::1", "http://[::1]:10100/v1"],
+      ["::", "http://127.0.0.1:10100/v1"],
+      ["0.0.0.0", "http://127.0.0.1:10100/v1"],
+    ];
+    for (const [hostname, expected] of cases) {
+      const configPath = installClient("hermes");
+      writeFileSync(configPath, "providers: {}\n");
+      const result = applyIntegration({
+        clientId: "hermes", models: MODELS, port: 10100,
+        config: { ...CONFIG, hostname } as OcxConfig,
+        env: TEST_ENV, home, store,
+      });
+      expect(result.ok).toBe(true);
+      expect(readFileSync(configPath, "utf8")).toContain(expected);
+      rmSync(configPath, { force: true });
+      store.dropRecord("hermes");
+    }
+  });
 });
 
 describe("a restore never launders a foreign edit into owned content", () => {