From 8afd386f714dd1f008bd62c858b7ca81b6a91f5e Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Wed, 7 Oct 2026 12:43:39 +0530 Subject: [PATCH 1/5] fix(pi): require approval for external scan inputs Signed-off-by: yashrajbasav --- extensions/skillspector.ts | 76 ++++++++++++++++------ tests/unit/test_pi_extension.mjs | 106 +++++++++++++++++++++++++++++-- 2 files changed, 157 insertions(+), 25 deletions(-) diff --git a/extensions/skillspector.ts b/extensions/skillspector.ts index 99a7acd40..719e7b747 100644 --- a/extensions/skillspector.ts +++ b/extensions/skillspector.ts @@ -1,4 +1,4 @@ -import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; +import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent"; import { StringEnum } from "@earendil-works/pi-ai"; import { Type, type Static } from "typebox"; import { chmodSync, constants, copyFileSync, existsSync, lstatSync, mkdtempSync, realpathSync, renameSync, rmSync } from "node:fs"; @@ -7,7 +7,7 @@ import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "nod import { fileURLToPath } from "node:url"; const scanSchema = Type.Object({ - target: Type.String({ description: "Path, URL, zip, Git repo, or SKILL.md to scan." }), + target: Type.String({ description: "Path, URL, zip, Git repo, or SKILL.md to scan. External paths and remote targets require user confirmation." }), format: Type.Optional( StringEnum(["terminal", "json", "markdown", "sarif"] as const, { description: "SkillSpector output format. Defaults to terminal.", @@ -21,22 +21,12 @@ const scanSchema = Type.Object({ }), ), model: Type.Optional(Type.String({ description: "Optional model override." })), - yaraRulesDir: Type.Optional(Type.String({ description: "Optional extra YARA rules directory." })), + yaraRulesDir: Type.Optional(Type.String({ description: "Optional extra YARA rules directory. External paths require user confirmation." })), verbose: Type.Optional(Type.Boolean({ description: "Show detailed progress." })), }); type SkillSpectorScanParams = Static; -function isLikelyUrl(value: string): boolean { - return /^[a-z][a-z0-9+.-]*:\/\//i.test(value) || /^[\w.-]+\/[\w.-]+(?:\.git)?(?:@.+)?$/i.test(value); -} - -function resolveMaybePath(ctxCwd: string, value?: string): string | undefined { - if (!value) return undefined; - if (isLikelyUrl(value)) return value; - return isAbsolute(value) ? value : resolve(ctxCwd, value); -} - function redactSecrets(value: string): string { return value .replace(/(sk-ant-[A-Za-z0-9_-]{12,})/g, "[REDACTED_ANTHROPIC_KEY]") @@ -71,6 +61,56 @@ function isWithin(root: string, path: string): boolean { return rel !== ".." && !rel.startsWith(`..${sep}`) && !isAbsolute(rel); } +async function approveScanInputs( + params: SkillSpectorScanParams, + ctx: ExtensionContext, + signal?: AbortSignal, +): Promise { + const workspace = realpathSync(ctx.cwd); + const prepared = { ...params }; + const localPaths: Array<{ input: string; resolved: string }> = []; + const requests: string[] = []; + for (const field of ["target", "yaraRulesDir"] as const) { + const value = params[field]?.trim(); + if (field === "yaraRulesDir" && !value) { + prepared[field] = undefined; + continue; + } + if (!value) throw new Error("A scan target is required."); + // Match the CLI's remote forms. A local owner/repo path is not a URL. + const remote = !isAbsolute(value) && (value.startsWith("https://") || value.startsWith("git@")); + if (remote) { + if (field !== "target") throw new Error("YARA rules must be a local directory."); + prepared[field] = value; + requests.push(`Fetch remote scan target: ${JSON.stringify(value)}`); + } else { + const input = resolve(ctx.cwd, value); + const resolved = realpathSync(input); + prepared[field] = resolved; + localPaths.push({ input, resolved }); + if (!isWithin(workspace, resolved)) { + requests.push(`Read external ${field === "target" ? "scan target" : "YARA rules"}: ${JSON.stringify(resolved)}`); + } + } + } + signal?.throwIfAborted(); + if (requests.length) { + if (!ctx.hasUI) throw new Error("External scan inputs require user confirmation in an interactive or RPC session."); + const approved = await ctx.ui.confirm( + "Allow SkillSpector external access?", + `${requests.join("\n")}\n\nScanned content and matching rule text can appear in the agent conversation.`, + ); + if (!approved) throw new Error("SkillSpector external access was not approved."); + } + signal?.throwIfAborted(); + // The dialog can wait indefinitely. Recheck aliases before launching, and + // pass canonical paths to the CLI, whose file reads use no-follow handles. + if (realpathSync(ctx.cwd) !== workspace || localPaths.some(({ input, resolved }) => realpathSync(input) !== resolved)) { + throw new Error("Scan input path changed while awaiting confirmation."); + } + return prepared; +} + function reportOutputPath(cwd: string, value?: string): string | undefined { if (!value) return undefined; const output = resolve(cwd, value); @@ -102,8 +142,8 @@ function publishReport(source: string, destination: string): void { } } -function buildScanArgs(params: SkillSpectorScanParams, cwd: string, output?: string): string[] { - const args = ["scan", resolveMaybePath(cwd, params.target) ?? params.target]; +function buildScanArgs(params: SkillSpectorScanParams, output?: string): string[] { + const args = ["scan", params.target]; args.push("--format", params.format ?? "terminal"); const noLlm = params.noLlm ?? true; @@ -111,8 +151,7 @@ function buildScanArgs(params: SkillSpectorScanParams, cwd: string, output?: str if (output) args.push("--output", output); - const yaraRulesDir = resolveMaybePath(cwd, params.yaraRulesDir); - if (yaraRulesDir) args.push("--yara-rules-dir", yaraRulesDir); + if (params.yaraRulesDir) args.push("--yara-rules-dir", params.yaraRulesDir); if (params.verbose) args.push("--verbose"); return args; @@ -131,11 +170,12 @@ export default function (pi: ExtensionAPI) { parameters: scanSchema, async execute(_toolCallId, params, signal, onUpdate, ctx) { const bin = findSkillSpectorBin(); + const prepared = await approveScanInputs(params, ctx, signal); const outputPath = reportOutputPath(ctx.cwd, params.output); const reportDir = outputPath ? mkdtempSync(join(tmpdir(), "skillspector-report-")) : undefined; const reportPath = reportDir ? join(reportDir, "report") : undefined; try { - const args = buildScanArgs(params, ctx.cwd, reportPath); + const args = buildScanArgs(prepared, reportPath); const env: Record = {}; if (params.provider) env.SKILLSPECTOR_PROVIDER = params.provider; diff --git a/tests/unit/test_pi_extension.mjs b/tests/unit/test_pi_extension.mjs index 5f349c62e..72b0bf3a7 100644 --- a/tests/unit/test_pi_extension.mjs +++ b/tests/unit/test_pi_extension.mjs @@ -26,7 +26,7 @@ registerHooks({ }, }); -async function setup(t, exec) { +async function setup(t, exec, confirm = async () => false) { const root = realpathSync(mkdtempSync(join(tmpdir(), "skillspector-pi-test-"))); const workspace = join(root, "workspace"); const install = join(root, "install"); @@ -34,6 +34,7 @@ async function setup(t, exec) { const source = process.env.SKILLSPECTOR_EXTENSION_SOURCE ?? new URL("../../extensions/skillspector.ts", import.meta.url); mkdirSync(workspace); + writeFileSync(join(workspace, "SKILL.md"), "---\nname: synthetic-skill\ndescription: Local permission test\n---\nA benign skill."); mkdirSync(join(install, "extensions"), { recursive: true }); mkdirSync(dirname(bin), { recursive: true }); writeFileSync(bin, "unused mocked executable"); @@ -49,6 +50,7 @@ async function setup(t, exec) { const { default: register } = await import(pathToFileURL(join(install, "extensions/skillspector.ts"))); let tool; const calls = []; + const prompts = []; register({ registerTool(registered) { tool = registered; }, async exec(command, args, options) { @@ -61,8 +63,13 @@ async function setup(t, exec) { }, }); return { - root, workspace, bin, calls, - scan: (params = {}) => tool.execute("scan", { target: "./SKILL.md", ...params }, undefined, undefined, { cwd: workspace }), + root, workspace, bin, calls, prompts, + scan: (params = {}, context = {}, signal) => tool.execute("scan", { target: "./SKILL.md", ...params }, signal, undefined, { + cwd: workspace, + hasUI: true, + ui: { async confirm(title, message) { prompts.push({ title, message }); return confirm({ root, workspace }); } }, + ...context, + }), }; } @@ -70,7 +77,7 @@ test("uses installed absolute executable and preserves scan arguments without ou const ctx = await setup(t); await ctx.scan({ provider: "anthropic", model: "synthetic-model", verbose: true }); assert.equal(ctx.calls[0].command, ctx.bin); - assert.deepEqual(ctx.calls[0].args, ["scan", "./SKILL.md", "--format", "terminal", "--no-llm", "--verbose"]); + assert.deepEqual(ctx.calls[0].args, ["scan", join(ctx.workspace, "SKILL.md"), "--format", "terminal", "--no-llm", "--verbose"]); assert.deepEqual(ctx.calls[0].options.env, { SKILLSPECTOR_PROVIDER: "anthropic", SKILLSPECTOR_MODEL: "synthetic-model" }); assert.equal(ctx.calls[0].options.cwd, ctx.workspace); }); @@ -132,7 +139,7 @@ for (const [noLlm, timeout] of [[undefined, 120000], [true, 120000], [false, 630 } test("uses an absolute operator override and preserves URL targets", async (t) => { - const ctx = await setup(t); + const ctx = await setup(t, undefined, async () => true); process.env.SKILLSPECTOR_BIN = join(ctx.root, "custom-cli"); writeFileSync(process.env.SKILLSPECTOR_BIN, "unused"); await ctx.scan({ target: "https://example.test/skill", noLlm: false, format: "json" }); @@ -140,6 +147,91 @@ test("uses an absolute operator override and preserves URL targets", async (t) = assert.deepEqual(ctx.calls[0].args, ["scan", "https://example.test/skill", "--format", "json"]); }); +test("keeps nested local paths local and scans workspace inputs without a prompt", async (t) => { + const ctx = await setup(t); + mkdirSync(join(ctx.workspace, "owner/repo"), { recursive: true }); + mkdirSync(join(ctx.workspace, "rules")); + for (const target of ["owner/repo", join(ctx.workspace, "SKILL.md"), " ./SKILL.md "]) { + await ctx.scan({ target, yaraRulesDir: "rules" }, { hasUI: false }); + assert.equal(ctx.calls.at(-1).args[1], realpathSync(resolve(ctx.workspace, target.trim()))); + assert.equal(ctx.calls.at(-1).args.at(-1), join(ctx.workspace, "rules")); + } + assert.equal(ctx.prompts.length, 0); +}); + +test("requires approval before exposing external targets or YARA rules to the CLI", async (t) => { + const ctx = await setup(t); + const secret = join(ctx.root, "private.md"); + const rules = join(ctx.root, "rules"); + writeFileSync(secret, "synthetic private content"); + mkdirSync(rules); + symlinkSync(secret, join(ctx.workspace, "linked.md")); + symlinkSync(rules, join(ctx.workspace, "rules")); + for (const params of [ + { target: secret }, + { target: "../private.md" }, + { target: "linked.md" }, + { yaraRulesDir: rules }, + { yaraRulesDir: "../rules" }, + { yaraRulesDir: "rules" }, + { target: "https://example.test/skill.zip" }, + { target: "git@github.com:owner/repo.git" }, + ]) { + await assert.rejects(ctx.scan(params), /not approved/); + await assert.rejects(ctx.scan(params, { hasUI: false }), /require user confirmation/); + } + assert.equal(ctx.calls.length, 0); + assert.equal(ctx.prompts.length, 8); + assert.equal(readFileSync(secret, "utf8"), "synthetic private content"); + assert.ok(ctx.prompts[2].message.includes(JSON.stringify(secret))); + assert.ok(ctx.prompts[5].message.includes(JSON.stringify(rules))); +}); + +test("approves the complete external read scope and passes only canonical paths", async (t) => { + const ctx = await setup(t, undefined, async () => true); + writeFileSync(join(ctx.root, "private.md"), "synthetic private content"); + mkdirSync(join(ctx.root, "rules")); + symlinkSync(ctx.root, join(ctx.workspace, "external")); + await ctx.scan({ target: "external/private.md", yaraRulesDir: "external/rules" }); + assert.equal(ctx.prompts.length, 1); + assert.match(ctx.prompts[0].message, /Read external scan target:/); + assert.match(ctx.prompts[0].message, /Read external YARA rules:/); + assert.equal(ctx.calls[0].args[1], join(ctx.root, "private.md")); + assert.equal(ctx.calls[0].args.at(-1), join(ctx.root, "rules")); +}); + +test("does not launch until approval arrives or after a canceled dialog", async (t) => { + let approve; + const ctx = await setup(t, undefined, () => new Promise((resolve) => { approve = resolve; })); + const controller = new AbortController(); + const scan = ctx.scan({ target: "https://example.test/skill.zip" }, {}, controller.signal); + await new Promise((resolve) => setImmediate(resolve)); + assert.equal(ctx.calls.length, 0); + controller.abort(); + approve(true); + await assert.rejects(scan, /abort/i); + assert.equal(ctx.calls.length, 0); +}); + +test("rejects input aliases retargeted while awaiting approval", async (t) => { + const ctx = await setup(t, undefined, async ({ workspace, root }) => { + renameSync(join(workspace, "SKILL.md"), join(workspace, "original.md")); + symlinkSync(join(root, "private.md"), join(workspace, "SKILL.md")); + return true; + }); + writeFileSync(join(ctx.root, "private.md"), "synthetic private content"); + mkdirSync(join(ctx.root, "rules")); + await assert.rejects(ctx.scan({ yaraRulesDir: "../rules" }), /input path changed/); + assert.equal(ctx.calls.length, 0); +}); + +test("rejects remote YARA directories instead of treating them as scan targets", async (t) => { + const ctx = await setup(t, undefined, async () => true); + await assert.rejects(ctx.scan({ yaraRulesDir: "https://example.test/rules" }), /local directory/); + assert.equal(ctx.calls.length, 0); + assert.equal(ctx.prompts.length, 0); +}); + test("never resolves a workspace executable through PATH or a relative override", async (t) => { const ctx = await setup(t); rmSync(ctx.bin); @@ -253,7 +345,7 @@ test("preserves generated reports when the scanner returns exit 1 or 2", async ( if (process.platform !== "win32") assert.equal(statSync(destination).mode & 0o777, 0o600); assert.equal(existsSync(dirname(ctx.calls.at(-1).output)), false); } - assert.deepEqual(readdirSync(ctx.workspace), ["existing.json", "new.json"]); + assert.deepEqual(readdirSync(ctx.workspace), ["SKILL.md", "existing.json", "new.json"]); }); } }); @@ -272,7 +364,7 @@ test("cleans staged reports after operational failure, cancellation, and missing await assert.rejects(ctx.scan({ output: "report.txt" })); assert.equal(readFileSync(join(ctx.workspace, "report.txt"), "utf8"), "preserve"); assert.equal(existsSync(dirname(ctx.calls[0].output)), false); - assert.deepEqual(readdirSync(ctx.workspace), ["report.txt"]); + assert.deepEqual(readdirSync(ctx.workspace), ["SKILL.md", "report.txt"]); }); } }); From f651cad51e3e3be399fdd8cf9ce77ee485063e00 Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Wed, 7 Oct 2026 13:09:11 +0530 Subject: [PATCH 2/5] fix(pi): keep git-prefixed local inputs canonical Signed-off-by: yashrajbasav --- extensions/skillspector.ts | 2 +- tests/unit/test_pi_extension.mjs | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/extensions/skillspector.ts b/extensions/skillspector.ts index 719e7b747..d3d2fb1ed 100644 --- a/extensions/skillspector.ts +++ b/extensions/skillspector.ts @@ -78,7 +78,7 @@ async function approveScanInputs( } if (!value) throw new Error("A scan target is required."); // Match the CLI's remote forms. A local owner/repo path is not a URL. - const remote = !isAbsolute(value) && (value.startsWith("https://") || value.startsWith("git@")); + const remote = !isAbsolute(value) && (value.startsWith("https://") || (value.startsWith("git@") && value.endsWith(".git"))); if (remote) { if (field !== "target") throw new Error("YARA rules must be a local directory."); prepared[field] = value; diff --git a/tests/unit/test_pi_extension.mjs b/tests/unit/test_pi_extension.mjs index 72b0bf3a7..30b0bdcbe 100644 --- a/tests/unit/test_pi_extension.mjs +++ b/tests/unit/test_pi_extension.mjs @@ -200,6 +200,22 @@ test("approves the complete external read scope and passes only canonical paths" assert.equal(ctx.calls[0].args.at(-1), join(ctx.root, "rules")); }); +test("treats local git@ paths as canonical local reads", async (t) => { + const ctx = await setup(t, undefined, async () => true); + mkdirSync(join(ctx.workspace, "git@notes")); + writeFileSync(join(ctx.workspace, "git@notes", "local.md"), "local notes"); + writeFileSync(join(ctx.root, "private.md"), "synthetic private content"); + await ctx.scan({ target: "git@notes/local.md" }, { hasUI: false }); + assert.equal(ctx.calls[0].args[1], join(ctx.workspace, "git@notes", "local.md")); + assert.equal(ctx.prompts.length, 0); + await ctx.scan({ target: "git@notes/../../private.md" }); + assert.equal(ctx.calls[1].args[1], join(ctx.root, "private.md")); + assert.equal(ctx.prompts.length, 1); + assert.match(ctx.prompts[0].message, /Read external scan target:/); + assert.ok(ctx.prompts[0].message.includes(JSON.stringify(join(ctx.root, "private.md")))); + assert.doesNotMatch(ctx.prompts[0].message, /Fetch remote/); +}); + test("does not launch until approval arrives or after a canceled dialog", async (t) => { let approve; const ctx = await setup(t, undefined, () => new Promise((resolve) => { approve = resolve; })); From fc58fbe31282db5eef6137c4b78e5f735c9b66ba Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Fri, 9 Oct 2026 11:12:11 +0530 Subject: [PATCH 3/5] fix(pi): close remaining external input permission gaps Signed-off-by: yashrajbasav --- docs/PI_EXTENSION.md | 8 +++ extensions/skillspector.ts | 63 ++++++++++++++----- .../nodes/analyzers/static_yara.py | 6 +- tests/nodes/analyzers/test_static_yara.py | 29 +++++++++ tests/unit/test_pi_extension.mjs | 58 +++++++++++++++-- 5 files changed, 138 insertions(+), 26 deletions(-) diff --git a/docs/PI_EXTENSION.md b/docs/PI_EXTENSION.md index 9b29c0682..7991f2edc 100644 --- a/docs/PI_EXTENSION.md +++ b/docs/PI_EXTENSION.md @@ -48,6 +48,14 @@ Equivalent CLI: - `yaraRulesDir`: optional directory of extra YARA rules. - `verbose`: optional detailed progress. +Inputs inside the session's working directory run without a prompt. Remote targets +and external paths require confirmation; redirected aliases show their resolved +destination too. Print and JSON sessions reject these requests because they cannot +show a dialog. Use TUI or RPC mode, move the skill into the working directory, or +run the CLI directly. Local targets retain the CLI's refusal of symlinked paths. +Missing rule directories fail before scanning, and YARA `include` directives are +disabled: put self-contained rule files in the selected directory. + ## LLM-backed analysis Static scan is default. To use semantic LLM analysis, configure provider credentials in your shell before launching Pi, then call the tool with `noLlm=false` and a provider. diff --git a/extensions/skillspector.ts b/extensions/skillspector.ts index d3d2fb1ed..6d081cf19 100644 --- a/extensions/skillspector.ts +++ b/extensions/skillspector.ts @@ -66,10 +66,25 @@ async function approveScanInputs( ctx: ExtensionContext, signal?: AbortSignal, ): Promise { + signal?.throwIfAborted(); const workspace = realpathSync(ctx.cwd); const prepared = { ...params }; - const localPaths: Array<{ input: string; resolved: string }> = []; + const localPaths: Array<{ field: "target" | "yaraRulesDir"; input: string; resolved?: string }> = []; const requests: string[] = []; + const readRequest = (field: string, path: string) => + `Read external ${field === "target" ? "scan target" : "YARA rules"}: ${JSON.stringify(path)}`; + async function approve(requests: string[]): Promise { + if (!requests.length) return; + signal?.throwIfAborted(); + if (!ctx.hasUI) throw new Error("External scan inputs require user confirmation in an interactive or RPC session."); + const approved = await ctx.ui.confirm( + "Allow SkillSpector external access?", + `${requests.join("\n")}\n\nScanned content and matching rule text can appear in the agent conversation.`, + { signal }, + ); + signal?.throwIfAborted(); + if (!approved) throw new Error("SkillSpector external access was not approved."); + } for (const field of ["target", "yaraRulesDir"] as const) { const value = params[field]?.trim(); if (field === "yaraRulesDir" && !value) { @@ -85,26 +100,40 @@ async function approveScanInputs( requests.push(`Fetch remote scan target: ${JSON.stringify(value)}`); } else { const input = resolve(ctx.cwd, value); - const resolved = realpathSync(input); - prepared[field] = resolved; - localPaths.push({ input, resolved }); - if (!isWithin(workspace, resolved)) { - requests.push(`Read external ${field === "target" ? "scan target" : "YARA rules"}: ${JSON.stringify(resolved)}`); + // Ask before resolving external paths, which can probe the host or access + // a Windows network share even when the file is never opened. + if (!isWithin(resolve(ctx.cwd), input)) { + localPaths.push({ field, input }); + requests.push(readRequest(field, input)); + } else { + let resolved: string; + try { + resolved = realpathSync(input); + } catch { + throw new Error(`Could not resolve ${field === "target" ? "scan target" : "YARA rules directory"}. Check that it exists and is accessible.`); + } + localPaths.push({ field, input, resolved }); + if (!isWithin(workspace, resolved)) requests.push(readRequest(field, resolved)); } } } - signal?.throwIfAborted(); - if (requests.length) { - if (!ctx.hasUI) throw new Error("External scan inputs require user confirmation in an interactive or RPC session."); - const approved = await ctx.ui.confirm( - "Allow SkillSpector external access?", - `${requests.join("\n")}\n\nScanned content and matching rule text can appear in the agent conversation.`, - ); - if (!approved) throw new Error("SkillSpector external access was not approved."); + await approve(requests); + const aliasRequests: string[] = []; + for (const path of localPaths) { + try { + path.resolved ??= realpathSync(path.input); + } catch { + throw new Error(`Could not resolve ${path.field === "target" ? "scan target" : "YARA rules directory"}. Check that it exists and is accessible.`); + } + if (!isWithin(resolve(ctx.cwd), path.input) && !isWithin(workspace, path.resolved) && path.resolved !== path.input) { + aliasRequests.push(readRequest(path.field, path.resolved)); + } + // Keep the original target path so the CLI can enforce its no-symlink + // input policy. YARA directories are canonicalised by the CLI too. + prepared[path.field] = path.field === "target" ? path.input : path.resolved; } + await approve(aliasRequests); signal?.throwIfAborted(); - // The dialog can wait indefinitely. Recheck aliases before launching, and - // pass canonical paths to the CLI, whose file reads use no-follow handles. if (realpathSync(ctx.cwd) !== workspace || localPaths.some(({ input, resolved }) => realpathSync(input) !== resolved)) { throw new Error("Scan input path changed while awaiting confirmation."); } @@ -170,8 +199,8 @@ export default function (pi: ExtensionAPI) { parameters: scanSchema, async execute(_toolCallId, params, signal, onUpdate, ctx) { const bin = findSkillSpectorBin(); - const prepared = await approveScanInputs(params, ctx, signal); const outputPath = reportOutputPath(ctx.cwd, params.output); + const prepared = await approveScanInputs(params, ctx, signal); const reportDir = outputPath ? mkdtempSync(join(tmpdir(), "skillspector-report-")) : undefined; const reportPath = reportDir ? join(reportDir, "report") : undefined; try { diff --git a/src/skillspector/nodes/analyzers/static_yara.py b/src/skillspector/nodes/analyzers/static_yara.py index c0a03f191..b5ad8f643 100644 --- a/src/skillspector/nodes/analyzers/static_yara.py +++ b/src/skillspector/nodes/analyzers/static_yara.py @@ -522,7 +522,7 @@ def _compile_rules( """ _enforce_rule_load_deadline() try: - compiled = yara.compile(sources=sources) + compiled = yara.compile(sources=sources, includes=False) _enforce_rule_load_deadline() return compiled, 0 except yara.SyntaxError: @@ -534,7 +534,7 @@ def _compile_rules( for ns, source in sources.items(): _enforce_rule_load_deadline() try: - yara.compile(source=source) + yara.compile(source=source, includes=False) good[ns] = source except (yara.SyntaxError, yara.Error) as exc: skipped += 1 @@ -548,7 +548,7 @@ def _compile_rules( ) _enforce_rule_load_deadline() - compiled = yara.compile(sources=good) if good else None + compiled = yara.compile(sources=good, includes=False) if good else None _enforce_rule_load_deadline() return compiled, skipped diff --git a/tests/nodes/analyzers/test_static_yara.py b/tests/nodes/analyzers/test_static_yara.py index cebfa1daf..93fdabbf2 100644 --- a/tests/nodes/analyzers/test_static_yara.py +++ b/tests/nodes/analyzers/test_static_yara.py @@ -2409,6 +2409,35 @@ def test_build_namespace_map_skips_malformed_encoded_rules(self, tmp_path): assert "invalid" not in ns_map assert skipped == 1 + @pytest.mark.parametrize("relative", [False, True]) + def test_external_includes_are_rejected_without_dropping_valid_rules( + self, tmp_path, monkeypatch, relative + ): + rules_dir = tmp_path / "rules" + rules_dir.mkdir() + outside = tmp_path / "private.yar" + outside.write_text("rule external_private_rule { condition: true }") + include = "../private.yar" if relative else str(outside) + (rules_dir / "include.yar").write_text(f'include "{include}"') + (rules_dir / "good.yar").write_text("rule approved_local_rule { condition: true }") + monkeypatch.chdir(rules_dir) + monkeypatch.setattr(static_yara, "_rule_cache", None) + monkeypatch.setattr(static_yara, "_BUILTIN_RULES_DIR", tmp_path / "empty_builtin") + result = static_yara.node( + { + "components": ["SKILL.md"], + "file_cache": {"SKILL.md": "Local sample skill."}, + "yara_rules_dir": str(rules_dir), + } + ) + assert any("approved_local_rule" in f.message for f in result["findings"]) + assert not any("external_private_rule" in f.message for f in result["findings"]) + assert result["analyzer_status_events"][0]["status"] != "completed" + assert any( + event.get("reason_code") == LedgerReason.READ_ERROR + for event in result["inspection_ledger"] + ) + def test_malformed_rule_is_reported_not_silently_dropped(self, tmp_path, monkeypatch): """A custom rule that can't compile must not report a clean, SAFE scan (#554). diff --git a/tests/unit/test_pi_extension.mjs b/tests/unit/test_pi_extension.mjs index 30b0bdcbe..e6c88ca2b 100644 --- a/tests/unit/test_pi_extension.mjs +++ b/tests/unit/test_pi_extension.mjs @@ -67,7 +67,16 @@ async function setup(t, exec, confirm = async () => false) { scan: (params = {}, context = {}, signal) => tool.execute("scan", { target: "./SKILL.md", ...params }, signal, undefined, { cwd: workspace, hasUI: true, - ui: { async confirm(title, message) { prompts.push({ title, message }); return confirm({ root, workspace }); } }, + ui: { async confirm(title, message, options) { + prompts.push({ title, message }); + return new Promise((resolve, reject) => { + const signal = options?.signal; + const abort = () => reject(signal.reason); + if (signal?.aborted) return abort(); + signal?.addEventListener("abort", abort, { once: true }); + Promise.resolve(confirm({ root, workspace })).then(resolve, reject).finally(() => signal?.removeEventListener("abort", abort)); + }); + } }, ...context, }), }; @@ -187,7 +196,7 @@ test("requires approval before exposing external targets or YARA rules to the CL assert.ok(ctx.prompts[5].message.includes(JSON.stringify(rules))); }); -test("approves the complete external read scope and passes only canonical paths", async (t) => { +test("approves canonical external scope and preserves the CLI target symlink policy", async (t) => { const ctx = await setup(t, undefined, async () => true); writeFileSync(join(ctx.root, "private.md"), "synthetic private content"); mkdirSync(join(ctx.root, "rules")); @@ -196,7 +205,7 @@ test("approves the complete external read scope and passes only canonical paths" assert.equal(ctx.prompts.length, 1); assert.match(ctx.prompts[0].message, /Read external scan target:/); assert.match(ctx.prompts[0].message, /Read external YARA rules:/); - assert.equal(ctx.calls[0].args[1], join(ctx.root, "private.md")); + assert.equal(ctx.calls[0].args[1], join(ctx.workspace, "external/private.md")); assert.equal(ctx.calls[0].args.at(-1), join(ctx.root, "rules")); }); @@ -217,14 +226,12 @@ test("treats local git@ paths as canonical local reads", async (t) => { }); test("does not launch until approval arrives or after a canceled dialog", async (t) => { - let approve; - const ctx = await setup(t, undefined, () => new Promise((resolve) => { approve = resolve; })); + const ctx = await setup(t, undefined, () => new Promise(() => {})); const controller = new AbortController(); const scan = ctx.scan({ target: "https://example.test/skill.zip" }, {}, controller.signal); await new Promise((resolve) => setImmediate(resolve)); assert.equal(ctx.calls.length, 0); controller.abort(); - approve(true); await assert.rejects(scan, /abort/i); assert.equal(ctx.calls.length, 0); }); @@ -384,3 +391,42 @@ test("cleans staged reports after operational failure, cancellation, and missing }); } }); + + +test("rejects external headless paths uniformly before checking their existence", async (t) => { + const ctx = await setup(t); + writeFileSync(join(ctx.root, "existing.md"), "private"); + for (const target of ["../existing.md", "../missing.md"]) { + await assert.rejects(ctx.scan({ target }, { hasUI: false }), /require user confirmation/); + } + assert.equal(ctx.calls.length, 0); +}); + +test("rejects invalid output and pre-canceled calls before showing a dialog", async (t) => { + const ctx = await setup(t); + await assert.rejects(ctx.scan({ target: "https://example.test/skill", output: "../report.json" }), /within the current workspace/); + const controller = new AbortController(); + controller.abort(); + await assert.rejects(ctx.scan({ target: "https://example.test/skill" }, {}, controller.signal), /abort/i); + assert.equal(ctx.prompts.length, 0); + assert.equal(ctx.calls.length, 0); +}); + +test("rejects a workspace swapped while a remote target awaits approval", async (t) => { + const ctx = await setup(t, undefined, async ({ root, workspace }) => { + renameSync(workspace, join(root, "original-workspace")); + symlinkSync(root, workspace); + return true; + }); + await assert.rejects(ctx.scan({ target: "https://example.test/skill" }), /input path changed/); + assert.equal(ctx.calls.length, 0); +}); + +test("gives a clear error for missing workspace targets or rule directories", async (t) => { + const ctx = await setup(t); + for (const params of [{ target: "missing.md" }, { yaraRulesDir: "missing-rules" }]) { + await assert.rejects(ctx.scan(params), /Could not resolve/); + } + assert.equal(ctx.calls.length, 0); + assert.equal(ctx.prompts.length, 0); +}); From d74e1a3915c7b96d49b65011579eba78389edf33 Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Fri, 9 Oct 2026 11:14:51 +0530 Subject: [PATCH 4/5] test(yara): use matching strings in include regression Signed-off-by: yashrajbasav --- tests/nodes/analyzers/test_static_yara.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/nodes/analyzers/test_static_yara.py b/tests/nodes/analyzers/test_static_yara.py index 93fdabbf2..6a046ecd8 100644 --- a/tests/nodes/analyzers/test_static_yara.py +++ b/tests/nodes/analyzers/test_static_yara.py @@ -2416,10 +2416,10 @@ def test_external_includes_are_rejected_without_dropping_valid_rules( rules_dir = tmp_path / "rules" rules_dir.mkdir() outside = tmp_path / "private.yar" - outside.write_text("rule external_private_rule { condition: true }") + outside.write_text('rule external_private_rule { strings: $a = "Local" condition: $a }') include = "../private.yar" if relative else str(outside) (rules_dir / "include.yar").write_text(f'include "{include}"') - (rules_dir / "good.yar").write_text("rule approved_local_rule { condition: true }") + (rules_dir / "good.yar").write_text('rule approved_local_rule { strings: $a = "Local" condition: $a }') monkeypatch.chdir(rules_dir) monkeypatch.setattr(static_yara, "_rule_cache", None) monkeypatch.setattr(static_yara, "_BUILTIN_RULES_DIR", tmp_path / "empty_builtin") From 12a20cd8e0b46c6a113fdc2765649a086f070116 Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Fri, 9 Oct 2026 11:41:37 +0530 Subject: [PATCH 5/5] style(test): format YARA fixture Signed-off-by: yashrajbasav --- tests/nodes/analyzers/test_static_yara.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/nodes/analyzers/test_static_yara.py b/tests/nodes/analyzers/test_static_yara.py index 6a046ecd8..8038f8e74 100644 --- a/tests/nodes/analyzers/test_static_yara.py +++ b/tests/nodes/analyzers/test_static_yara.py @@ -2419,7 +2419,9 @@ def test_external_includes_are_rejected_without_dropping_valid_rules( outside.write_text('rule external_private_rule { strings: $a = "Local" condition: $a }') include = "../private.yar" if relative else str(outside) (rules_dir / "include.yar").write_text(f'include "{include}"') - (rules_dir / "good.yar").write_text('rule approved_local_rule { strings: $a = "Local" condition: $a }') + (rules_dir / "good.yar").write_text( + 'rule approved_local_rule { strings: $a = "Local" condition: $a }' + ) monkeypatch.chdir(rules_dir) monkeypatch.setattr(static_yara, "_rule_cache", None) monkeypatch.setattr(static_yara, "_BUILTIN_RULES_DIR", tmp_path / "empty_builtin")