fix(security): block plugin install/load on critical source scan findings by coygeek · Pull Request #11032 · openclaw/openclaw

coygeek · 2026-02-07T09:16:12Z

Fix Summary

Add source code scanning at both plugin install and load time. The existing skill-scanner.ts module already detects dangerous patterns (child_process, eval/new Function, crypto mining, obfuscated code, env harvesting) but its findings were warn-only at install time and completely absent at load time.

Changes:

install.ts: Make scanDirectoryWithSummary() blocking on critical findings (was warn-only). Add scanning to installPluginFromFile() which previously had zero checks. Add skipScan parameter for --force flag support.
loader.ts: Add pre-load scanning before jiti(candidate.source) call. Block loading when critical findings detected.

Issue Linkage

Fixes #11030

Security Snapshot

Metric	Value
Score	9.6 / 10.0
Severity	Critical
Vector	CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:C/C:H/I:H/A:H

Implementation Details

Files Changed

src/plugins/install.ts (+52/-3)
src/plugins/loader.ts (+32/-0)

Technical Analysis

Add source code scanning at both plugin install and load time. The existing skill-scanner.ts module already detects dangerous patterns (child_process, eval/new Function, crypto mining, obfuscated code, env harvesting) but its findings were warn-only at install time and completely absent at load time.

Validation Evidence

Command: install.ts
Status: passed

Risk and Compatibility

non-breaking; compatibility impact was not explicitly documented in the original PR body.

AI-Assisted Disclosure

AI-assisted: yes
Model: Claude Code

Greptile Overview

Greptile Summary

Tightens plugin security by making source scans blocking on critical findings during install (archive/dir/npm spec) with a skipScan escape hatch.
Adds scanning for single-file plugin installs before copying into the extensions directory.
Adds a pre-load source scan in the plugin loader to prevent jiti() execution when critical patterns are detected.
Overall integrates the existing skill-scanner logic into both install-time and load-time plugin workflows to reduce arbitrary code execution risk.

Confidence Score: 3/5

This PR is directionally correct but has a couple of concrete correctness issues that should be fixed before merging.
The install/load blocking behavior appears consistent with the scanner’s severity model, but existing tests now contradict the new blocking semantics, and the loader’s scan-failure catch can crash when a provided logger lacks a warn method.
src/plugins/loader.ts and src/plugins/install.test.ts

Context used:

Context from dashboard - CLAUDE.md (source)
Context from dashboard - AGENTS.md (source)

…ings The existing skill-scanner was invoked during plugin installation but operated in warn-only mode, allowing plugins with dangerous code patterns (eval, child_process exec, crypto mining, env harvesting) to install and load without restriction. This change: - Makes installPluginFromPackageDir() block installation when critical scan findings are detected (with skipScan escape hatch for --force) - Adds source scanning to installPluginFromFile() which previously had no scanning at all - Adds pre-load scanning in the plugin loader before jiti() execution, preventing already-installed malicious plugins from being loaded - Propagates skipScan parameter through all install API entry points Scan failures are non-blocking (logged as warnings) to avoid breaking legitimate plugins due to scanner errors. Co-Authored-By: Claude Opus 4.6 <[email protected]>

greptile-apps

_{2 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-07T09:17:13Z

src/plugins/loader.ts

+    // Scan plugin source for dangerous code patterns before loading
+    if (isScannable(candidate.source)) {
+      try {
+        const rawSource = fs.readFileSync(candidate.source, "utf-8");
+        const scanFindings = scanSource(rawSource, candidate.source);
+        const criticalFindings = scanFindings.filter((f) => f.severity === "critical");
+        if (criticalFindings.length > 0) {
+          const details = criticalFindings
+            .map((f) => `${f.message} (line ${f.line})`)
+            .join("; ");
+          logger.error(
+            `[plugins] ${pluginId} blocked from loading: dangerous code patterns detected in ${candidate.source}: ${details}`,
+          );
+          record.status = "error";
+          record.error = `blocked: dangerous code patterns detected: ${details}`;
+          registry.plugins.push(record);
+          seenIds.set(pluginId, candidate.origin);
+          registry.diagnostics.push({
+            level: "error",
+            pluginId: record.id,
+            source: record.source,
+            message: record.error,
+          });
+          continue;
+        }
+      } catch {
+        // Scan failure should not prevent loading — log and continue
+        logger.warn(`[plugins] ${pluginId} source scan failed for ${candidate.source}`);
+      }
+    }


Crash on scan error

The new pre-load scan path uses logger.warn(...) in the catch block, but logger comes from options.logger ?? defaultLogger() where the PluginLogger type is optional methods in some call sites. Elsewhere in this file you consistently use optional chaining (e.g. logger.error?.(...)). If logger.warn is undefined, this catch block will throw and abort plugin loading whenever scanning throws.

Suggested change

// Scan plugin source for dangerous code patterns before loading

if (isScannable(candidate.source)) {

try {

const rawSource = fs.readFileSync(candidate.source, "utf-8");

const scanFindings = scanSource(rawSource, candidate.source);

const criticalFindings = scanFindings.filter((f) => f.severity === "critical");

if (criticalFindings.length > 0) {

const details = criticalFindings

.map((f) => `${f.message} (line ${f.line})`)

.join("; ");

logger.error(

`[plugins] ${pluginId} blocked from loading: dangerous code patterns detected in ${candidate.source}: ${details}`,

);

record.status = "error";

record.error = `blocked: dangerous code patterns detected: ${details}`;

registry.plugins.push(record);

seenIds.set(pluginId, candidate.origin);

registry.diagnostics.push({

level: "error",

pluginId: record.id,

source: record.source,

message: record.error,

});

continue;

}

} catch {

// Scan failure should not prevent loading — log and continue

logger.warn(`[plugins] ${pluginId} source scan failed for ${candidate.source}`);

}

}

logger.warn?.(`[plugins] ${pluginId} source scan failed for ${candidate.source}`);

Prompt To Fix With AI

This is a comment left during a code review. Path: src/plugins/loader.ts Line: 295:324 Comment: **Crash on scan error** The new pre-load scan path uses `logger.warn(...)` in the `catch` block, but `logger` comes from `options.logger ?? defaultLogger()` where the `PluginLogger` type is optional methods in some call sites. Elsewhere in this file you consistently use optional chaining (e.g. `logger.error?.(...)`). If `logger.warn` is undefined, this catch block will throw and abort plugin loading whenever scanning throws. ```suggestion logger.warn?.(`[plugins] ${pluginId} source scan failed for ${candidate.source}`); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-07T09:17:15Z

Additional Comments (2)

src/plugins/install.test.ts
Behavior change breaks test

installPluginFromPackageDir now blocks installs on critical scan findings unless skipScan is set, but this test still expects installs with child_process.exec(...) to succeed. As written, it will now fail deterministically because dangerous-exec is critical in src/security/skill-scanner.ts. Update the test to either expect a blocked install, or pass skipScan: true and assert the overridden-warning message.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/plugins/install.test.ts
Line: 373:408

Comment:
**Behavior change breaks test**

`installPluginFromPackageDir` now blocks installs on *critical* scan findings unless `skipScan` is set, but this test still expects installs with `child_process.exec(...)` to succeed. As written, it will now fail deterministically because `dangerous-exec` is `critical` in `src/security/skill-scanner.ts`. Update the test to either expect a blocked install, or pass `skipScan: true` and assert the overridden-warning message.

How can I resolve this? If you propose a fix, please make it concise.

src/plugins/install.test.ts
Critical scan now blocks

This test also expects the install to succeed while the extension entry contains child_process.exec(...), which the scanner marks as critical. After this PR, installPluginFromPackageDir returns { ok: false } unless skipScan is set, so this assertion will now fail. Adjust expectations (blocked vs overridden with skipScan: true) to match the new behavior.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/plugins/install.test.ts
Line: 410:445

Comment:
**Critical scan now blocks**

This test also expects the install to succeed while the extension entry contains `child_process.exec(...)`, which the scanner marks as `critical`. After this PR, `installPluginFromPackageDir` returns `{ ok: false }` unless `skipScan` is set, so this assertion will now fail. Adjust expectations (blocked vs overridden with `skipScan: true`) to match the new behavior.

How can I resolve this? If you propose a fix, please make it concise.

openclaw-barnacle · 2026-02-21T04:28:18Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

coygeek · 2026-02-24T01:33:43Z

Closing: linked issue #11030 was closed as not planned.

greptile-apps bot reviewed Feb 7, 2026

View reviewed changes

This was referenced Feb 7, 2026

[Bug]: oxlint preserve-caught-error violation in memory-lancedb blocks all PR CI #11060

Closed

fix(lint): preserve caught error in memory-lancedb loadLanceDB #11061

Closed

thewilloftheshadow force-pushed the main branch from bfc1ccb to f92900f Compare February 15, 2026 18:46

openclaw-barnacle bot added stale Marked as stale due to inactivity and removed stale Marked as stale due to inactivity labels Feb 21, 2026

coygeek closed this Feb 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(security): block plugin install/load on critical source scan findings#11032

fix(security): block plugin install/load on critical source scan findings#11032
coygeek wants to merge 1 commit intoopenclaw:mainfrom
coygeek:fix/sa01-plugin-scanner

coygeek commented Feb 7, 2026 •

edited

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Feb 7, 2026

Uh oh!

greptile-apps bot commented Feb 7, 2026

Uh oh!

openclaw-barnacle bot commented Feb 21, 2026

Uh oh!

coygeek commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

coygeek commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix Summary

Issue Linkage

Security Snapshot

Implementation Details

Files Changed

Technical Analysis

Validation Evidence

Risk and Compatibility

AI-Assisted Disclosure

Greptile Overview

Greptile Summary

Confidence Score: 3/5

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Feb 7, 2026

Uh oh!

openclaw-barnacle bot commented Feb 21, 2026

Uh oh!

coygeek commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coygeek commented Feb 7, 2026 •

edited

Loading