fix(plugins): address review feedback on trust-pinning

fede-kamel · fede-kamel · commit 9409792dbdcd · 2026-05-03T01:52:13.000-04:00
- Update install.npm-spec.test.ts: the existing it.each table installed
  bare @openclaw/* specs (with spawn payloads) and asserted ok: true via
  the trusted-source bypass. With the new pinning + integrity gate those
  fixtures must use exact-version specs and pass expectedIntegrity to
  keep proving the legitimate trust path. Add a companion negative
  it.each that asserts the scanner now blocks the bare/dist-tag/
  no-integrity variants.
- Replace the misleading provider-install-catalog.ts citation in the
  install.ts code comment; that policy is not on current main.
- Add an Unreleased Fixes changelog entry under Plugins/install.

Verification:
- pnpm test src/plugins/install.npm-spec.test.ts src/plugins/install.test.ts -&gt; 113/113 pass
- pnpm exec oxfmt --check --threads=1 src/plugins/install.ts src/plugins/install.test.ts src/plugins/install.npm-spec.test.ts CHANGELOG.md -&gt; clean
- node scripts/run-oxlint.mjs src/plugins/install.ts src/plugins/install.test.ts src/plugins/install.npm-spec.test.ts -&gt; 0 warnings 0 errors
- pnpm tsgo:core -&gt; clean
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai
 
 ### Fixes
 
+- Plugins/install: require an exact-version pin and a non-empty `expectedIntegrity` before granting the trusted-source security-scan bypass for official `@openclaw/*` npm plugin installs, so a name-only or `@latest` spec can no longer skip the install scanner if the upstream scope is ever compromised. Thanks @fede-kamel.
 - Channels/secrets: resolve SecretRef-backed channel credentials through external plugin secret contracts after the plugin split, covering runtime startup, target discovery, webhook auth, disabled-account enumeration, and late-bound web_search config. (#76449) Thanks @joshavant.
 - Docker/Gateway: pass Docker setup `.env` values into gateway and CLI containers and preserve exec SecretRef `passEnv` keys in managed service plans, so 1Password Connect-backed Discord tokens keep resolving after doctor or plugin repair. Thanks @vincentkoc.
 - Control UI/WebChat: explain compaction boundaries in chat history and link directly to session checkpoint controls so pre-compaction turns no longer look silently lost after refresh. Fixes #76415. Thanks @BunsDev.
diff --git a/src/plugins/install.npm-spec.test.ts b/src/plugins/install.npm-spec.test.ts
@@ -344,33 +344,34 @@ describe("installPluginFromNpmSpec", () => {
 
   it.each([
     {
-      spec: "@openclaw/acpx",
+      packageName: "@openclaw/acpx",
       pluginId: "acpx",
       indexJs: `import { spawn } from "node:child_process";\nspawn("codex-acp", []);`,
     },
     {
-      spec: "@openclaw/codex",
+      packageName: "@openclaw/codex",
       pluginId: "codex",
       indexJs: `import { spawn } from "node:child_process";\nspawn("codex", ["app-server"]);`,
     },
     {
-      spec: "@openclaw/google-meet",
+      packageName: "@openclaw/google-meet",
       pluginId: "google-meet",
       indexJs: `import { spawnSync } from "node:child_process";\nspawnSync("node", ["bridge.js"]);`,
     },
     {
-      spec: "@openclaw/voice-call",
+      packageName: "@openclaw/voice-call",
       pluginId: "voice-call",
       indexJs: `import { spawn } from "node:child_process";\nspawn("ngrok", ["http", "3000"]);`,
     },
   ])(
-    "allows official npm plugin $spec with reviewed launch code",
-    async ({ spec, pluginId, indexJs }) => {
+    "allows pinned official npm plugin $packageName with reviewed launch code when integrity is provided",
+    async ({ packageName, pluginId, indexJs }) => {
       const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm");
       const warnings: string[] = [];
+      const spec = `${packageName}@2026.5.2`;
       mockNpmViewAndInstall({
         spec,
-        packageName: spec,
+        packageName,
         version: "2026.5.2",
         pluginId,
         npmRoot,
@@ -380,6 +381,7 @@ describe("installPluginFromNpmSpec", () => {
       const result = await installPluginFromNpmSpec({
         spec,
         npmDir: npmRoot,
+        expectedIntegrity: "sha512-plugin-test",
         logger: {
           info: () => {},
           warn: (msg: string) => warnings.push(msg),
@@ -404,6 +406,49 @@ describe("installPluginFromNpmSpec", () => {
     },
   );
 
+  it.each([
+    {
+      label: "bare spec (no version selector)",
+      spec: "@openclaw/codex",
+      expectedIntegrity: "sha512-plugin-test" as string | undefined,
+    },
+    {
+      label: "dist-tag spec resolves to latest",
+      spec: "@openclaw/codex@latest",
+      expectedIntegrity: "sha512-plugin-test" as string | undefined,
+    },
+    {
+      label: "exact version without expectedIntegrity",
+      spec: "@openclaw/codex@2026.5.2",
+      expectedIntegrity: undefined,
+    },
+  ])(
+    "blocks official npm plugin install with reviewed launch code when trust prerequisites are missing: $label",
+    async ({ spec, expectedIntegrity }) => {
+      const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm");
+      mockNpmViewAndInstall({
+        spec,
+        packageName: "@openclaw/codex",
+        version: "2026.5.2",
+        pluginId: "codex",
+        npmRoot,
+        indexJs: `import { spawn } from "node:child_process";\nspawn("codex", ["app-server"]);`,
+      });
+
+      const result = await installPluginFromNpmSpec({
+        spec,
+        npmDir: npmRoot,
+        ...(expectedIntegrity ? { expectedIntegrity } : {}),
+        logger: { info: () => {}, warn: () => {} },
+      });
+
+      expect(result.ok).toBe(false);
+      if (!result.ok) {
+        expect(result.code).toBe(PLUGIN_INSTALL_ERROR_CODE.SECURITY_SCAN_BLOCKED);
+      }
+    },
+  );
+
   it("rejects non-registry npm specs", async () => {
     const result = await installPluginFromNpmSpec({ spec: "github:evil/evil" });
     expect(result.ok).toBe(false);
diff --git a/src/plugins/install.ts b/src/plugins/install.ts
@@ -209,11 +209,11 @@ function isTrustedOfficialNpmPluginInstall(params: {
   if (!requested) {
     return false;
   }
-  // Match the policy enforced for trusted catalog installs in
-  // src/plugins/provider-install-catalog.ts (require exact-version pin and
-  // a non-empty expectedIntegrity hash). Without these, a name-only spec
-  // would resolve to npm's "latest" tag and bypass the security scan if the
-  // upstream package is ever compromised.
+  // Trusting a name-only or dist-tag spec means npm resolves it to "latest"
+  // at install time, and the scan bypass would also cover a future malicious
+  // release if the upstream @openclaw scope is ever compromised. Require the
+  // caller to have committed to a specific resolved version + integrity hash
+  // before granting the trusted-source scan bypass.
   if (requested.selectorKind !== "exact-version") {
     return false;
   }
