From a50d5a6b906d2e68670c63f0eeb7efa39b6617e0 Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Wed, 14 Oct 2026 09:48:58 +0530 Subject: [PATCH] fix: preserve Pi workspace aliases and strengthen security regressions Signed-off-by: yashrajbasav --- CHANGELOG.md | 1 + README.md | 2 +- extensions/skillspector.ts | 10 +++++--- tests/unit/test_pi_extension.mjs | 39 ++++++++++++++++++++++++++++++++ tests/unit/test_suppression.py | 2 +- 5 files changed, 49 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e517a40e3..55e6389a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ### 2.12.0 (Unreleased) ### Features/Bug Fixes +* fix(pi): accept scans from symlinked workspace roots while preserving the CLI's checks on symlinks below the workspace. Custom YARA rules loaded with `--yara-rules-dir` cannot use `include`; place each rule directly in the selected directory. * fix(taint): key TT1-TT6 taint by lexical scope and follow direct calls to functions and methods defined in the same file, so a tainted variable no longer taints same-named variables in other functions (#799). Message-glob baseline rules keep matching TT findings whose rule and message are unchanged; exact baseline fingerprints are bound to the scanner version and suppress nothing after an upgrade (see docs/SUPPRESSION.md), so they must be regenerated in any case. A TT message changes where the earlier message came from a same-named variable in another scope (it now names the variable, source and line that reach the sink); where a more severe source raises the rule (TT2 to TT3 or TT4, TT4 to TT3, TT2 to TT5 or TT6), and at such a sink, where a weaker finding is subsumed or now cites another variable; and where an identifier longer than 120 characters is now shortened. Re-accept those findings or regenerate the baseline. `match_fingerprint` changes only when the rule does. * fix(static): stop code comparisons after removal verbs from reading as unterminated tag-marker directives (#777) * fix(baseline): fingerprint every occurrence of a deduplicated finding so the next scan suppresses all of them (#633) diff --git a/README.md b/README.md index cf3eb6efa..7493a9db6 100644 --- a/README.md +++ b/README.md @@ -782,7 +782,7 @@ Options: -f, --format [terminal|json|markdown|sarif] Output format [default: terminal] -o, --output PATH Output file path --no-llm Skip LLM analysis (static only) - --yara-rules-dir PATH Extra YARA rules directory + --yara-rules-dir PATH Extra YARA rules directory (includes disabled) -b, --baseline PATH Suppress findings listed in a baseline --show-suppressed List baseline-suppressed findings -V, --verbose Show detailed progress diff --git a/extensions/skillspector.ts b/extensions/skillspector.ts index 6d081cf19..04b43be60 100644 --- a/extensions/skillspector.ts +++ b/extensions/skillspector.ts @@ -128,9 +128,13 @@ async function approveScanInputs( 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; + // Resolve only the workspace prefix; keep child symlinks for the CLI's + // no-symlink input checks. YARA directories are canonicalised by the CLI too. + prepared[path.field] = path.field === "target" + ? isWithin(resolve(ctx.cwd), path.input) + ? join(workspace, relative(resolve(ctx.cwd), path.input)) + : path.input + : path.resolved; } await approve(aliasRequests); signal?.throwIfAborted(); diff --git a/tests/unit/test_pi_extension.mjs b/tests/unit/test_pi_extension.mjs index e6c88ca2b..764dffc49 100644 --- a/tests/unit/test_pi_extension.mjs +++ b/tests/unit/test_pi_extension.mjs @@ -91,6 +91,23 @@ test("uses installed absolute executable and preserves scan arguments without ou assert.equal(ctx.calls[0].options.cwd, ctx.workspace); }); +test("accepts a symlinked workspace without resolving target symlinks below it", async (t) => { + const ctx = await setup(t); + const alias = join(ctx.root, "workspace-alias"); + symlinkSync(ctx.workspace, alias); + symlinkSync(join(ctx.workspace, "SKILL.md"), join(ctx.workspace, "linked.md")); + for (const [target, expected] of [ + ["./SKILL.md", join(ctx.workspace, "SKILL.md")], + [".", ctx.workspace], + [join(alias, "SKILL.md"), join(ctx.workspace, "SKILL.md")], + ["linked.md", join(ctx.workspace, "linked.md")], + ]) { + await ctx.scan({ target }, { cwd: alias, hasUI: false }); + assert.equal(ctx.calls.at(-1).args[1], expected); + } + assert.equal(ctx.prompts.length, 0); +}); + test("redacts long scanner output without retrying every word character", { timeout: 5000 }, async (t) => { const ctx = await setup(t, () => ({ code: 0, @@ -209,6 +226,28 @@ test("approves canonical external scope and preserves the CLI target symlink pol assert.equal(ctx.calls[0].args.at(-1), join(ctx.root, "rules")); }); +for (const approveResolved of [true, false]) { + test(`external YARA aliases require a second confirmation: approved=${approveResolved}`, async (t) => { + let confirmations = 0; + const ctx = await setup(t, undefined, async () => ++confirmations === 1 || approveResolved); + const rules = join(ctx.root, "rules"); + const alias = join(ctx.root, "rules-alias"); + mkdirSync(rules); + symlinkSync(rules, alias); + const scan = ctx.scan({ yaraRulesDir: "../rules-alias" }); + if (approveResolved) { + await scan; + assert.equal(ctx.calls[0].args.at(-1), rules); + } else { + await assert.rejects(scan, /not approved/); + assert.equal(ctx.calls.length, 0); + } + assert.equal(ctx.prompts.length, 2); + assert.ok(ctx.prompts[0].message.includes(JSON.stringify(alias))); + assert.ok(ctx.prompts[1].message.includes(JSON.stringify(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")); diff --git a/tests/unit/test_suppression.py b/tests/unit/test_suppression.py index c631c34e8..8e60331b8 100644 --- a/tests/unit/test_suppression.py +++ b/tests/unit/test_suppression.py @@ -252,7 +252,7 @@ def test_transitive_fingerprint_does_not_borrow_lookalike_local_path(delimiter: def test_root_lookup_does_not_normalize_a_transitive_content_key() -> None: from skillspector.suppression import _component_content - key = source_content_key("scope", r"..\..\..\x") + key = source_content_key(f"external/{'a' * 64}", r"..\..\..\x") assert _component_content({key: "remote content"}, './x"]') is None