fix(skills): windows interpreter scripts outside scripts/ escape security scanning (#6321)

* fix(skills): scan Windows interpreter scripts outside scripts/

CODE_SUFFIXES is the single definition of "this file is code" that
skills/AGENTS.md requires the installer and SkillScan to import, but it
listed .ps1 while omitting .bat/.cmd/.vbs/.wsf/.psm1. Those files
outside scripts/ therefore got is_code_file() == False, so SkillScan
skipped them as binaries (zero findings, not even
package-undecodable-script) and the installer never reached the
executable=True guardrail scan.

Add the five Windows interpreter extensions to the shared table so both
surfaces see them on every platform.

Refs #6320

* fix(skills): scan Windows script host sibling formats

---------

Co-authored-by: sxh313 <sxh313@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
sxh 2026-10-06 07:17:22 +08:00 committed by GitHub
parent aa7846823d
commit a72c6489da
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 57 additions and 3 deletions

View File

@ -1296,6 +1296,7 @@ If a trusted operator manages the configured skills directory through an externa
Skill installs and agent-managed skill edits run through **SkillScan**, a native deterministic safety scanner before the LLM-based skill scanner. Phase 1 runs offline with no Semgrep/OpenGrep dependency, blocks high-confidence `CRITICAL` findings such as private keys or shell execution, and passes warning findings to the LLM scanner for contextual review. Code files (anything under `scripts/`, a script suffix such as `.py`, `.sh`, or `.js`, or an extensionless file starting with `#!`) that are not NUL-free UTF-8 text raise a warning and are still analyzed over a lossy decode, so a single stray byte cannot hide them from `CRITICAL` checks. The moderation adapter normalizes both plain-text model responses and LangChain Responses API text blocks before parsing the required JSON decision. Python instance-client exfiltration checks follow a minimal same-scope evidence chain: a simple name bound to a known client constructor, optional name-to-name aliases, and an actual outbound method or context-manager use supported by that constructor. Constructor roots must be proven imports; bare canonical-looking names are not inferred as modules. Nested scopes do not inherit client handles and inherit only constructor import aliases that are never rebound in the enclosing scope. Comprehensions, walrus-bearing statements, annotations, complex binding targets, unsupported operations, and ambiguous branch flows produce no finding from this signal; skipped constructs conservatively invalidate every name they may bind so stale client state cannot create a finding. A deterministic work budget or recursion limit reached by this best-effort analysis does not discard findings already collected for the file. Set `skill_scan.enabled: false` in `config.yaml` to disable only the deterministic analyzers; safe archive extraction and the LLM scanner still run.
Windows scripts (`.bat`, `.cmd`, `.ps1`, `.psm1`, `.js`, `.jse`, `.vbs`, `.vbe`, `.wsf`), HTML applications (`.hta`), and scriptlets (`.sct`) count as code even outside `scripts/`, regardless of filename case. They receive both SkillScan analysis and the installer's executable-code policy.
SkillScan warns about remote downloads piped into common shells, including sudo, interpreter paths, and shell line continuations. Pipes to non-shell tools such as `jq` and `tee` do not trigger this warning.
SkillScan treats HTTP hostnames case-insensitively and recognizes bracketed IPv6

View File

@ -8,7 +8,7 @@ from __future__ import annotations
from pathlib import PurePath, PurePosixPath
CODE_SUFFIXES = frozenset({".bash", ".cjs", ".js", ".mjs", ".php", ".pl", ".ps1", ".py", ".rb", ".sh", ".ts", ".zsh"})
CODE_SUFFIXES = frozenset({".bash", ".bat", ".cjs", ".cmd", ".hta", ".js", ".jse", ".mjs", ".php", ".pl", ".ps1", ".psm1", ".py", ".rb", ".sct", ".sh", ".ts", ".vbe", ".vbs", ".wsf", ".zsh"})
# Full magics per variant — a shorter shared prefix would also match
# non-executable data files.
_EXECUTABLE_MAGIC_PREFIXES = (

View File

@ -91,3 +91,38 @@ def test_skill_md_outside_eval_fixtures_is_still_reported(tmp_path: Path) -> Non
assert [(finding["rule_id"], finding["severity"]) for finding in result["findings"]] == [("package-nested-skill-md", "CRITICAL")]
assert result["blocked"] is True
_WINDOWS_SCRIPT_PATHS = (
"hooks/install.bat",
"hooks/install.cmd",
"hooks/install.vbs",
"hooks/install.wsf",
"hooks/module.psm1",
"hooks/install.jse",
"hooks/install.vbe",
"assets/setup.hta",
"policy/install.sct",
"hooks/install.JSE",
"hooks/install.VBE",
"assets/setup.HTA",
"policy/install.SCT",
)
_STRAY_BYTES = (("invalid-utf8", b"\xff"), ("nul", b"\x00"))
_SECRET_ASSIGNMENT = b'api_key = "sk-live-9f3c77a1b0e4"'
@pytest.mark.parametrize("rel_path", _WINDOWS_SCRIPT_PATHS)
@pytest.mark.parametrize(("marker", "marker_bytes"), _STRAY_BYTES)
def test_windows_script_with_one_stray_byte_is_still_analyzed(tmp_path: Path, marker: str, marker_bytes: bytes, rel_path: str) -> None:
"""Windows scripts, HTML applications and scriptlets must stay visible despite a stray byte."""
root = tmp_path / "pkg"
root.mkdir(parents=True, exist_ok=True)
(root / "SKILL.md").write_text(_MINIMAL_SKILL_MD, encoding="utf-8")
script = root / rel_path
script.parent.mkdir(parents=True, exist_ok=True)
script.write_bytes(marker_bytes + _SECRET_ASSIGNMENT + b"\r\n")
findings = sorted((finding["rule_id"], finding["severity"]) for finding in scan_skill_dir(root)["findings"] if finding["file"] == rel_path)
assert findings == [("package-undecodable-script", "HIGH"), ("secret-env-assignment", "HIGH")]

View File

@ -105,6 +105,20 @@ class TestCodeFileClassification:
("bin/notes.txt", b"#!/bin/sh\n", False),
("bin/scripts", b"echo", False),
("assets/logo.png", b"\x89PNG", False),
("hooks/install.bat", b"@echo off\r\n", True),
("hooks/install.CMD", b"@echo off\r\n", True),
("hooks/install.vbs", b'CreateObject("WScript.Shell")\r\n', True),
("hooks/install.wsf", b"<job></job>\r\n", True),
("hooks/module.psm1", b"Export-ModuleMember -Function *", True),
("hooks/install.jse", b"plain", True),
("hooks/install.JSE", b"plain", True),
("hooks/install.vbe", b"plain", True),
("hooks/install.VBE", b"plain", True),
("assets/setup.hta", b"<html></html>", True),
("assets/setup.HTA", b"<html></html>", True),
("policy/install.sct", b"<scriptlet></scriptlet>", True),
("policy/install.SCT", b"<scriptlet></scriptlet>", True),
("references/notes.txt", b"plain", False),
],
)
def test_installer_applies_the_shared_code_file_rule(self, tmp_path, rel_path, content, expected):
@ -571,17 +585,20 @@ class TestInstallSkillFromArchive:
assert sniffed == ["tool"]
def test_code_file_outside_scripts_warn_prevents_install(self, tmp_path, monkeypatch):
@pytest.mark.parametrize("rel_path", ["lib/run.py", "hooks/install.jse", "hooks/install.vbe", "assets/setup.hta", "policy/install.sct"])
def test_code_file_outside_scripts_warn_prevents_install(self, tmp_path, monkeypatch, rel_path):
zip_path = tmp_path / "test-skill.skill"
with zipfile.ZipFile(zip_path, "w") as zf:
zf.writestr("test-skill/SKILL.md", "---\nname: test-skill\ndescription: A test skill\n---\n\n# test-skill\n")
# Benign payload on purpose: the native scanner must stay quiet so the
# test exercises the LLM executable policy (warn != allow) on its own.
zf.writestr("test-skill/lib/run.py", "print('needs human review')\n")
zf.writestr(f"test-skill/{rel_path}", "print('needs human review')\n")
skills_root = tmp_path / "skills"
skills_root.mkdir()
calls = []
async def _scan(*args, executable, **kwargs):
calls.append((kwargs["location"], executable))
if executable:
return ScanResult(decision="warn", reason="code needs review")
return ScanResult(decision="allow", reason="ok")
@ -591,6 +608,7 @@ class TestInstallSkillFromArchive:
with pytest.raises(SkillSecurityScanError, match="rejected executable.*code needs review"):
get_or_new_skill_storage(skills_path=skills_root).install_skill_from_archive(zip_path)
assert (f"test-skill/{rel_path}", True) in calls
assert not (skills_root / "custom" / "test-skill").exists()
def test_executable_binary_prevents_install(self, tmp_path):