fix(security): ignore scanner comment context

This commit is contained in:
Vincent Koc
2026-05-04 02:35:43 -07:00
parent 6b7f9eafed
commit 33e19fb5ae
4 changed files with 91 additions and 7 deletions

View File

@@ -66,6 +66,7 @@ Docs: https://docs.openclaw.ai
- fix: harden backend message action gateway routing [AI]. (#76374) Thanks @pgondhi987.
- Gate QQBot streaming command auth [AI]. (#76375) Thanks @pgondhi987.
- Plugins/release: make the published npm runtime verifier reject blank `openclaw.runtimeExtensions` entries instead of treating them as absent and passing via inferred outputs. Thanks @vincentkoc.
- Plugins/security: ignore inline and block comments when matching source-rule context in plugin install scans, so comment-only `fetch`/`post` references near environment defaults do not block clean plugins. Thanks @vincentkoc.
- Doctor/plugins: remove stale managed install records for bundled plugins even when the bundled plugin is not explicitly configured, so doctor cleanup cannot leave orphaned install metadata behind. Thanks @vincentkoc.
- Web fetch: scope provider fallback cache entries by the selected fetch provider so config reloads cannot reuse another provider's cached fallback payload. Thanks @vincentkoc.
- Web search: honor late-bound `tools.web.search.enabled: false` during tool execution so config reloads cannot leave an already-created `web_search` tool runnable. Thanks @vincentkoc.

View File

@@ -21,11 +21,16 @@ type PublishablePluginPackage = {
const REVIEWED_PUBLISHABLE_CRITICAL_FINDINGS = new Set([
"@openclaw/acpx:dangerous-exec:src/codex-auth-bridge.ts",
"@openclaw/acpx:dangerous-exec:src/runtime-internals/mcp-proxy.mjs",
"@openclaw/acpx:dangerous-exec:dist/mcp-proxy.mjs",
"@openclaw/acpx:dangerous-exec:dist/service-<hash>.js",
"@openclaw/codex:dangerous-exec:src/app-server/transport-stdio.ts",
"@openclaw/codex:dangerous-exec:dist/client-<hash>.js",
"@openclaw/google-meet:dangerous-exec:src/node-host.ts",
"@openclaw/google-meet:dangerous-exec:src/realtime.ts",
"@openclaw/google-meet:dangerous-exec:dist/index.js",
"@openclaw/voice-call:dangerous-exec:src/tunnel.ts",
"@openclaw/voice-call:dangerous-exec:src/webhook/tailscale.ts",
"@openclaw/voice-call:dangerous-exec:dist/runtime-entry-<hash>.js",
]);
const tempDirs: string[] = [];
@@ -72,6 +77,15 @@ function isScannerWalkedPackedPath(packedPath: string): boolean {
);
}
function normalizePackedFindingPath(packedPath: string): string {
for (const prefix of ["client", "runtime-entry", "service"]) {
if (packedPath.startsWith(`dist/${prefix}-`) && packedPath.endsWith(".js")) {
return `dist/${prefix}-<hash>.js`;
}
}
return packedPath;
}
function stageScannerRelevantPackedFiles(
packageDir: string,
packedFiles: readonly string[],
@@ -141,7 +155,9 @@ describe("publishable plugin npm package install security scan", () => {
if (finding.severity !== "critical") {
continue;
}
const packedPath = relative(stageDir, finding.file).split(sep).join("/");
const packedPath = normalizePackedFindingPath(
relative(stageDir, finding.file).split(sep).join("/"),
);
const key = `${plugin.packageName}:${finding.ruleId}:${packedPath}`;
if (REVIEWED_PUBLISHABLE_CRITICAL_FINDINGS.has(key)) {
reviewedCriticalFindings.add(key);

View File

@@ -274,6 +274,18 @@ const env = process.env;
expect(findings.some((f) => f.ruleId === "env-harvesting")).toBe(false);
});
it("does not use inline or block comments as source-rule context", () => {
const source = `
const env = process.env; // fetch("https://example.invalid")
/*
* rest.post("/channels/123/messages", {});
*/
const url = "https://example.com/path//segment";
`;
const findings = scanSource(source, "plugin.ts");
expect(findings.some((f) => f.ruleId === "env-harvesting")).toBe(false);
});
it("returns empty array for clean plugin code", () => {
const source = `
export function greet(name: string): string {

View File

@@ -236,11 +236,66 @@ function isBenignMemberExecMatch(line: string, match: RegExpExecArray): boolean
return !/\b(?:cp|childProcess|child_process)\s*\.\s*exec\s*\(/.test(line);
}
function stripFullLineCommentsForHeuristics(source: string): string {
return source
.split("\n")
.map((line) => (line.trimStart().startsWith("//") ? "" : line))
.join("\n");
function stripCommentsForHeuristics(source: string): string {
let stripped = "";
let quote: "'" | '"' | "`" | null = null;
let escaped = false;
let inBlockComment = false;
for (let i = 0; i < source.length; i++) {
const ch = source[i] ?? "";
const next = source[i + 1] ?? "";
if (inBlockComment) {
if (ch === "*" && next === "/") {
inBlockComment = false;
i++;
continue;
}
if (ch === "\n") {
stripped += "\n";
}
continue;
}
if (quote) {
stripped += ch;
if (escaped) {
escaped = false;
} else if (ch === "\\") {
escaped = true;
} else if (ch === quote) {
quote = null;
}
continue;
}
if (ch === "'" || ch === '"' || ch === "`") {
quote = ch;
stripped += ch;
continue;
}
if (ch === "/" && next === "/") {
while (i < source.length && source[i] !== "\n") {
i++;
}
if (source[i] === "\n") {
stripped += "\n";
}
continue;
}
if (ch === "/" && next === "*") {
inBlockComment = true;
i++;
continue;
}
stripped += ch;
}
return stripped;
}
function findSourceRuleMatch(params: {
@@ -282,7 +337,7 @@ function findSourceRuleMatch(params: {
export function scanSource(source: string, filePath: string): SkillScanFinding[] {
const findings: SkillScanFinding[] = [];
const lines = source.split("\n");
const heuristicSource = stripFullLineCommentsForHeuristics(source);
const heuristicSource = stripCommentsForHeuristics(source);
const heuristicLines = heuristicSource.split("\n");
const matchedLineRules = new Set<string>();