Harden LLM prompts against scanned-skill steering; build the image from uv.lock as non-root

[oherbawi]

Summary

Changes from an external review, one commit each:

• fix(llm): frame scanned files as untrusted, nonce-delimited input. BASE_ANALYSIS_PROMPT embedded the scanned skill with no adversarial framing, followed by a precision-over-recall instruction, in the semantic analyzers whose SSD findings have no deterministic backstop. Adds an explicit untrusted-input section and wraps the file in markers carrying a nonce from the new content_nonce(). The nonce is a SHA-256 of the file's own content, so prompts stay deterministic for caching and seeded runs, while a file cannot contain its own closing marker. meta_analyzer and mcp_tool_poisoning override build_prompt and are unaffected.
• fix(meta-analyzer): frame scanned content with fixed-size nonce markers. PER_FILE_ANALYSIS_PROMPT used a fixed triple-backtick fence that a file could close and continue in its own words. It now wraps the file in the same untrusted-file markers as BASE_ANALYSIS_PROMPT, carrying content_nonce, so a file cannot forge its closing marker. The markers are constant-size and sit in the template that get_batches() already budgets. (An earlier revision sized a backtick fence to the content instead; review showed that let a long backtick run grow the prompt past the batch budget, so it was replaced.) Impact was limited to annotations, since the LLM cannot suppress deterministic findings.
• fix(input): exact or subdomain host match in _is_git_url. It tested allowed in host, so github.com.evil.example was classified as Git. Not exploitable, because _validate_url_host enforces the strict allowlist before any clone, but it misclassified input.
• fix(integrations): refuse dash-led scan targets in the OpenCode tool and the Pi extension. Both pass the target as argv after scan, so a target like --output=... was parsed as a CLI option.
• build(docker): install from uv.lock, run as non-root. The builder ran pip install ., resolving the >= ranges afresh and ignoring uv.lock; the runtime ran as root. It now syncs with uv 0.10.10 (the release workflow's version, pinned by image digest) using --frozen, and runs as uid 10001 with HOME=/tmp. Behavior change: writing reports into a bind mount now needs --user "$(id -u):$(id -g)". The README examples and tests/docker/smoke.sh pass it, and the smoke test fails if the image ever defaults to root. Also pins the commented-out codeql-action/upload-sarif step by commit, so uncommenting it is safe.

Testing

On Windows 11, Python 3.14, dependencies from uv.lock (uv sync --frozen --extra dev), default markers (no live-LLM tests):

• Full suite: 12,720 passed, 37 failed, 12 errors, 119 skipped. The failures come from 28 distinct tests, mostly symlink creation (EPERM without Windows symlink privilege), Windows upper-casing environment variable names, and release/accuracy tooling. The identical set of 28 fails on main when run with main's own src on the same machine.
• New tests pass: prompt framing, nonce determinism, a forged closing marker, hostile backtick fences staying inside the meta prompt's markers, fixed framing overhead for 300,000 consecutive backticks, git host matching, and dash-led targets.
• The review's 300,000-backtick case now builds one batch whose prompt is 301,741 characters (~75,435 tokens; base 301,685), within the 96,000-token input budget.
• OpenCode tool tests: 52 pass on this branch vs 51 on main (the new test); the 5 failing on both are Windows symlink tests.

Not tested: the Docker image build and smoke.sh (no Docker on this machine). CI's docker job covers both.

[yashrajp22]

One introduced prompt-budget regression remains in the meta analyzer; the inline comment includes a reproduction.

Validation: fresh base/HEAD wheels; selected Python tests (122 base and 127 HEAD passed in each source/wheel lane, one platform-specific skip per lane); OpenCode and Pi tests; targeted prompt/URL/integration probes; 48 offline regression scans with matching normalized results and existing partial-analysis outcomes preserved. Existing Docker build/smoke CI ran on a tree identical to HEAD. Provider context rejection was simulated; no live-model resistance claim is made.

[yashrajp22]

Rechecked 7808dc6. The fixed-size nonce framing resolves the earlier long-backtick regression: both reported cases now fit in source and fresh-wheel runs (3,478 / 4,096 and 75,478 / 96,000 estimated tokens), and the simulated budget checks complete without failed batches.

One Docker smoke-test permission issue still needs a follow-up: the report readers use a different UID from the scans. The inline comment describes the private-checkout case.

Validation: selected Python tests passed (122 at base, 128 at HEAD, each in source and wheel installs; one macOS-specific skip per run), OpenCode/Pi checks passed, and 48 offline scans across 12 samples retained normalized parity and their existing partial-analysis states. Production Docker CI also passed on an identical HEAD tree. The permission finding is based on the source; the offline checks do not establish live-provider prompt-injection resistance.

[rng1995]

[SkillSpector Review]

@oherbawi thank you for this careful hardening pass, especially the content-hash nonce framing and the quick turnaround on both review threads.

Value and readiness

Each change is sound on its own:

• A file cannot embed the 64-bit nonce of its own content, so it cannot forge its closing marker.
• The meta framing is now fixed-size, so get_batches() budgets it.
• _is_git_url now uses the same suffix rule as _validate_url_host.
• Both integrations refuse dash-led targets.
• The image builds from uv.lock with a digest-pinned uv and runs as uid 10001.

Two small items block merge, and the bundle raises scope questions for the PIC.

Material findings

1. Blocker: the branch conflicts with current main in tests/nodes/test_meta_analyzer.py, because #796 (7ee43713) changed the same imports and appended tests at the end of that file. This is a fork PR, so automation cannot update it. Please merge main and keep both sets of tests.
2. Blocker (test), extensions/skillspector.ts:175-178: the Pi extension's new dash-led refusal has no test; only the OpenCode path is covered (tests/opencode/skillspector_scan_lib.test.ts). tests/unit/test_pi_extension.mjs already mocks pi.exec. Please add a case asserting that a --output=... target throws before exec runs and that ./-skill passes.

PIC tradeoffs

• Scope: this PR bundles six unrelated changes. The Docker one breaks every existing docker run -v "$PWD:/scan" … --output … invocation: writing into the mount now needs --user, and private (0700) trees cannot be read without it. I'd recommend moving it to its own PR with a CHANGELOG/release note, so it can be announced and reverted on its own. At minimum, add that note here.
• Detection behavior: src/skillspector/llm_analyzer_base.py:886-909 does more than add framing. It also drops "It is far better to miss an edge case than to report a false positive" and tells every semantic analyzer to report steering attempts under a matching rule_id. This may raise the semantic false-positive rate, and the offline parity runs cited in review do not exercise the LLM. The PIC should either accept this or ask for a live-provider scripts/compare_scan_accuracy.py comparison first.

Verification and gaps

• Read the full diff at fdaabc00. The new smoke-test commit does pass --user "${HOST_USER}" to both report readers, as @yashrajp22 asked.
• Traced build_prompt and the get_batches budget, the content_nonce call sites, the Pi approveScanInputs → buildScanArgs flow, and the Dockerfile.
• Verified that ghcr.io/astral-sh/uv:0.10.10 resolves to index digest sha256:cbe0a44b… and that codeql-action 9f759ee6… is v3.38.3. The image still installs only core dependencies, as before.
• Both review threads are resolved, but @yashrajp22's CHANGES_REQUESTED still needs their re-review.
• No CI has run on fdaabc00 yet (7808dc65 was green). I did not build the image or run tests.

[rng1995]

Blocker (test): this refusal has no Pi-extension test; only the OpenCode path is covered. tests/unit/test_pi_extension.mjs already mocks pi.exec. Please add a case where a --output=/tmp/x target throws before exec is called, and ./-skill passes through unchanged.