fix(scanner): harden Scanner 2.1 integration

Signed-off-by: dongmucat <1127093059@qq.com>
This commit is contained in:
dongmucat 2026-09-18 16:13:39 +08:00
parent 78bfe10c91
commit aacf57487d
10 changed files with 200 additions and 65 deletions

View file

@ -114,11 +114,11 @@ The Server and Scanner HTTP contracts must be upgraded in this order:
3. upgrade the Scanner to 2.1.0
4. verify `/health` and an upload-mode scan before restoring normal traffic
Do not run an old Server against Scanner 2.1.0. During a mixed-version rollout in `upload` mode,
Do not run an old Server against Scanner 2.1.0. During a mixed-version rollout in either scan mode,
keep AI Defense disabled (`SKILLHUB_SCANNER_USE_AI_DEFENSE=false`, the default). If AI Defense must
remain enabled before the old Scanner is retired, configure its credential directly in the old
Scanner environment using the variable supported by that Scanner version. Never place an AI Defense
key in URL query parameters.
key in URL query parameters or request bodies.
## Verification

View file

@ -216,7 +216,7 @@ A: No. A Scanner response with `is_safe=true`, rendered in the UI as “No high-
A: First deploy the Server release that is compatible with the 2.1.0 protocol. While the old Scanner is still running, drain and remove every old Server instance and its in-flight scans. Then upgrade the Scanner and verify `/health` plus one upload-mode scan. Do not connect an old Server to Scanner 2.1.0.
During the mixed-version window, keep AI Defense disabled in upload mode (the default is `SKILLHUB_SCANNER_USE_AI_DEFENSE=false`). If AI Defense must remain enabled before the upgrade, configure its credential directly in the old Scanner environment using the variable supported by that Scanner version; never put an AI Defense key in URL query parameters.
During the mixed-version window, keep AI Defense disabled in both upload and local modes (the default is `SKILLHUB_SCANNER_USE_AI_DEFENSE=false`). If AI Defense must remain enabled before the upgrade, configure its credential directly in the old Scanner environment using the variable supported by that Scanner version; never put an AI Defense key in URL query parameters or request bodies.
## Q: How do I troubleshoot a `registry returned 400` error from `skillhub publish` (CLI)?

View file

@ -216,7 +216,7 @@ A: 不是。Scanner 返回 `is_safe=true`、UI 显示“未发现高风险问题
A: 必须先部署兼容 2.1.0 协议的 Server,在旧 Scanner 仍运行时排空并下线所有旧 Server 实例及其进行中的扫描,然后再升级 Scanner,最后验证 `/health` 和一次 upload mode 扫描。不要让旧 Server 连接 Scanner 2.1.0。
混合版本期间,upload mode 应保持 AI Defense 关闭(默认 `SKILLHUB_SCANNER_USE_AI_DEFENSE=false`)。如果升级前必须继续使用 AI Defense,应按旧 Scanner 版本支持的环境变量把凭据直接配置到旧 Scanner 环境中;不要把 AI Defense key 放入 URL query 参数。
混合版本期间,upload 和 local mode 都应保持 AI Defense 关闭(默认 `SKILLHUB_SCANNER_USE_AI_DEFENSE=false`)。如果升级前必须继续使用 AI Defense,应按旧 Scanner 版本支持的环境变量把凭据直接配置到旧 Scanner 环境中;不要把 AI Defense key 放入 URL query 参数或请求体。
## Q: 使用 CLI `skillhub publish` 报错 `registry returned 400` 怎么排查?

View file

@ -3,7 +3,6 @@
import asyncio
import logging
import os
import re
import shutil
import tempfile
from functools import wraps
@ -11,9 +10,31 @@ from importlib import import_module
from pathlib import Path
from typing import NoReturn
_RUNTIME_TEMP_ROOT = Path(
os.getenv("SKILLHUB_SCANNER_RUNTIME_TEMP_ROOT", "/tmp/skillhub-scanner-runtime")
)
def _prepare_runtime_temp_root() -> None:
"""Recreate the scanner-owned temp root before upstream allocates request directories."""
if _RUNTIME_TEMP_ROOT.name != "skillhub-scanner-runtime" or _RUNTIME_TEMP_ROOT.is_symlink():
raise RuntimeError("Scanner runtime temp root must be a non-symlink skillhub-scanner-runtime directory")
if _RUNTIME_TEMP_ROOT.exists():
if not _RUNTIME_TEMP_ROOT.is_dir():
raise RuntimeError("Scanner runtime temp root must be a directory")
shutil.rmtree(_RUNTIME_TEMP_ROOT)
_RUNTIME_TEMP_ROOT.mkdir(parents=True, mode=0o700)
_RUNTIME_TEMP_ROOT.chmod(0o700)
tempfile.tempdir = str(_RUNTIME_TEMP_ROOT)
_prepare_runtime_temp_root()
from fastapi import Request
from fastapi.responses import JSONResponse
from skill_scanner.api.api import app
from skill_scanner.cli import cli as _upstream_cli
_upstream_router = import_module("skill_scanner.api.router")
@ -25,15 +46,27 @@ _upstream_router.MAX_UPLOAD_SIZE_BYTES = max(
_active_scans = 0
_active_scans_guard = asyncio.Lock()
_SCAN_PATHS = {"/scan", "/scan-upload"}
_SUPPORTED_TOKEN_PATTERN = re.compile(
r"\b(?:gh[pousr]_[A-Za-z0-9]{20,255}|sk-(?:proj-)?[A-Za-z0-9_-]{20,255})\b"
)
_log = logging.getLogger(__name__)
def _redact_finding_text(message: str) -> str:
"""Redact credentials without applying CLI-only truncation or control escaping."""
redacted = _upstream_cli._STATUS_PRIVATE_KEY_RE.sub("<redacted>", message)
for pattern in (
_upstream_cli._STATUS_URL_USERINFO_RE,
_upstream_cli._STATUS_URL_TOKEN_USERINFO_RE,
_upstream_cli._STATUS_QUERY_SECRET_RE,
_upstream_cli._STATUS_BEARER_SECRET_RE,
_upstream_cli._STATUS_LABELED_SECRET_RE,
):
redacted = pattern.sub(_upstream_cli._replace_status_secret, redacted)
redacted = _upstream_cli._STATUS_PROVIDER_SECRET_RE.sub("<redacted>", redacted)
return _upstream_cli._STATUS_JWT_RE.sub("<redacted>", redacted)
def _redact_supported_tokens(value):
if isinstance(value, str):
return _SUPPORTED_TOKEN_PATTERN.sub("<redacted>", value)
return _redact_finding_text(value)
if isinstance(value, list):
return [_redact_supported_tokens(item) for item in value]
if isinstance(value, dict):
@ -60,19 +93,6 @@ def _install_scan_response_redaction() -> None:
route.dependant.call = redacting_endpoint
def _cleanup_stale_scan_directories(temp_root: Path | None = None) -> None:
"""Remove incomplete upstream extraction directories left by a process restart."""
root = temp_root or Path(tempfile.gettempdir())
current_upload_root = Path(_upstream_router._API_UPLOAD_ROOT).resolve()
for candidate in root.glob("skill_scanner_*"):
if not candidate.is_dir() or candidate.resolve() == current_upload_root:
continue
try:
shutil.rmtree(candidate)
except OSError as error:
_log.warning("Could not remove stale scanner directory %s: %s", candidate, error)
def _restart_after_hard_timeout(request_path: str) -> NoReturn:
"""Terminate the single-scan worker so the container runtime can recover it."""
_log.critical(
@ -95,7 +115,6 @@ async def _await_scan_until(scan_task: asyncio.Task, deadline: float, request_pa
_install_scan_response_redaction()
app.router.add_event_handler("startup", _cleanup_stale_scan_directories)
@app.middleware("http")

View file

@ -1,6 +1,8 @@
import asyncio
import importlib.util
import os
import re
import shutil
import sys
import tempfile
import types
@ -23,6 +25,10 @@ class _FakeApp:
self.routes = [object()]
github_canary = "ghp_" + "A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8"
openai_canary = "sk-proj-" + "Z9y8X7w6V5u4T3s2R1q0" * 3
aws_canary = "AKIA1234567890ABCDEF"
jwt_canary = "eyJabcde.abcdefgh.ijklmnop"
labeled_canary = "custom-secret-1234567890"
private_key_canary = "-----BEGIN PRIVATE KEY-----\nabc123\n-----END PRIVATE KEY-----"
self.upstream_routes = [
_FakeRoute(
"/scan",
@ -30,7 +36,13 @@ class _FakeApp:
[
{
"description": f"YARA match: {github_canary}",
"metadata": {"evidence": openai_canary},
"metadata": {
"openai": openai_canary,
"aws": aws_canary,
"authorization": f"Bearer {jwt_canary}",
"labeled": f"api_key={labeled_canary}",
"private_key": private_key_canary,
},
}
]
),
@ -85,8 +97,30 @@ def _load_module(environment=None):
api.app = _FakeApp()
router = types.ModuleType("skill_scanner.api.router")
router.MAX_UPLOAD_SIZE_BYTES = -1
router._API_UPLOAD_ROOT = Path(tempfile.gettempdir()) / "skill_scanner_current"
router.router = types.SimpleNamespace(routes=api.app.upstream_routes)
cli = types.ModuleType("skill_scanner.cli.cli")
cli._STATUS_PRIVATE_KEY_RE = re.compile(
r"-----BEGIN PRIVATE KEY-----[\s\S]*?-----END PRIVATE KEY-----"
)
cli._STATUS_URL_USERINFO_RE = re.compile(r"(?!)")
cli._STATUS_URL_TOKEN_USERINFO_RE = re.compile(r"(?!)")
cli._STATUS_QUERY_SECRET_RE = re.compile(r"(?!)")
cli._STATUS_BEARER_SECRET_RE = re.compile(
r"(?i)(?P<prefix>\bBearer\s+)(?P<value>[A-Za-z0-9._-]+)"
)
cli._STATUS_LABELED_SECRET_RE = re.compile(
r"(?i)(?P<prefix>\bapi_key=)(?P<value>[^\s]+)"
)
cli._STATUS_PROVIDER_SECRET_RE = re.compile(
r"\b(?:AKIA[0-9A-Z]{16}|ghp_[A-Za-z0-9]{20,255}|"
r"github_pat_[A-Za-z0-9_]{20,255}|sk-(?:proj-)?[A-Za-z0-9_-]{20,255})\b"
)
cli._STATUS_JWT_RE = re.compile(r"\beyJ[A-Za-z0-9_-]{5,}\.[A-Za-z0-9_-]{5,}\.[A-Za-z0-9_-]{5,}\b")
def replace_status_secret(match):
return f"{match.group('prefix')}<redacted>{match.groupdict().get('suffix', '')}"
cli._replace_status_secret = replace_status_secret
stubs = {
"fastapi": fastapi,
"fastapi.responses": responses,
@ -94,13 +128,29 @@ def _load_module(environment=None):
"skill_scanner.api": types.ModuleType("skill_scanner.api"),
"skill_scanner.api.api": api,
"skill_scanner.api.router": router,
"skill_scanner.cli": types.ModuleType("skill_scanner.cli"),
"skill_scanner.cli.cli": cli,
}
with patch.dict(os.environ, environment or {}, clear=True), patch.dict(sys.modules, stubs):
test_temp_parent = Path(tempfile.mkdtemp(prefix="skillhub-scanner-wrapper-test-"))
runtime_temp_root = test_temp_parent / "skillhub-scanner-runtime"
runtime_temp_root.mkdir()
(runtime_temp_root / "stale-upload.zip").write_text("stale", encoding="utf-8")
(test_temp_parent / "outside.txt").write_text("keep", encoding="utf-8")
module_environment = {
"SKILLHUB_SCANNER_RUNTIME_TEMP_ROOT": str(runtime_temp_root),
**(environment or {}),
}
previous_tempdir = tempfile.tempdir
with patch.dict(os.environ, module_environment, clear=True), patch.dict(sys.modules, stubs):
module_path = Path(__file__).parents[1] / "skillhub_scanner_app.py"
spec = importlib.util.spec_from_file_location("skillhub_scanner_app_under_test", module_path)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
try:
spec.loader.exec_module(module)
finally:
tempfile.tempdir = previous_tempdir
module._router_stub = router
module._test_temp_parent = test_temp_parent
return module
@ -108,6 +158,9 @@ class SkillHubScannerAppTest(unittest.IsolatedAsyncioTestCase):
async def asyncSetUp(self):
self.module = _load_module()
def tearDown(self):
shutil.rmtree(self.module._test_temp_parent)
async def test_excess_scan_is_rejected(self):
self.module._active_scans = 1
@ -153,53 +206,32 @@ class SkillHubScannerAppTest(unittest.IsolatedAsyncioTestCase):
restart.assert_called_once_with("/scan-upload")
self.assertEqual(0, self.module._active_scans)
async def test_startup_cleanup_removes_only_scanner_directories(self):
with tempfile.TemporaryDirectory() as temp_root:
root = Path(temp_root)
stale = root / "skill_scanner_abcd"
unrelated = root / "skillhub-data"
stale.mkdir()
unrelated.mkdir()
self.module._cleanup_stale_scan_directories(root)
self.assertFalse(stale.exists())
self.assertTrue(unrelated.exists())
async def test_startup_cleanup_keeps_current_upstream_upload_directory(self):
with tempfile.TemporaryDirectory() as temp_root:
root = Path(temp_root)
current = root / "skill_scanner_current"
stale = root / "skill_scanner_stale"
current.mkdir()
stale.mkdir()
self.module._router_stub._API_UPLOAD_ROOT = current
self.module._cleanup_stale_scan_directories(root)
self.assertTrue(current.exists())
self.assertFalse(stale.exists())
async def test_default_upload_limit_matches_skillhub_package_limit(self):
self.assertEqual(110100480, self.module._router_stub.MAX_UPLOAD_SIZE_BYTES)
async def test_upload_limit_can_be_overridden_by_environment(self):
module = _load_module({"SKILLHUB_SCANNER_MAX_UPLOAD_SIZE_BYTES": "123456"})
self.addCleanup(shutil.rmtree, module._test_temp_parent)
self.assertEqual(123456, module._router_stub.MAX_UPLOAD_SIZE_BYTES)
async def test_upload_limit_is_at_least_one_byte(self):
module = _load_module({"SKILLHUB_SCANNER_MAX_UPLOAD_SIZE_BYTES": "0"})
self.addCleanup(shutil.rmtree, module._test_temp_parent)
self.assertEqual(1, module._router_stub.MAX_UPLOAD_SIZE_BYTES)
async def test_scan_findings_redact_supported_tokens_without_changing_response_contract(self):
async def test_scan_findings_redact_credentials_without_changing_response_contract(self):
route = next(route for route in self.module._router_stub.router.routes if route.path == "/scan")
response = await route.endpoint()
self.assertNotIn("ghp_", str(response.findings))
self.assertNotIn("sk-proj-", str(response.findings))
self.assertNotIn("AKIA1234567890ABCDEF", str(response.findings))
self.assertNotIn("eyJabcde.abcdefgh.ijklmnop", str(response.findings))
self.assertNotIn("custom-secret-1234567890", str(response.findings))
self.assertNotIn("BEGIN PRIVATE KEY", str(response.findings))
self.assertEqual(200, response.status_code)
self.assertEqual({"X-Contract": "preserved"}, response.headers)
@ -211,11 +243,32 @@ class SkillHubScannerAppTest(unittest.IsolatedAsyncioTestCase):
self.assertEqual(expected, response.findings)
async def test_startup_cleanup_is_registered_on_the_upstream_router(self):
self.assertEqual(
[("startup", self.module._cleanup_stale_scan_directories)],
self.module.app.router.handlers,
)
async def test_redaction_preserves_safe_multiline_and_long_finding_text(self):
safe_multiline = "line one\n\tline two"
safe_long = "x" * 5000
redacted = self.module._redact_supported_tokens([safe_multiline, safe_long])
self.assertEqual([safe_multiline, safe_long], redacted)
async def test_redaction_preserves_non_secret_control_characters_around_secret(self):
value = "before\napi_key=custom-secret-1234567890\tafter"
redacted = self.module._redact_supported_tokens(value)
self.assertEqual("before\napi_key=<redacted>\tafter", redacted)
async def test_startup_does_not_register_global_temp_directory_cleanup(self):
self.assertEqual([], self.module.app.router.handlers)
async def test_import_recreates_private_runtime_temp_root_without_touching_parent(self):
runtime_root = self.module._RUNTIME_TEMP_ROOT
outside = self.module._test_temp_parent / "outside.txt"
self.assertTrue(runtime_root.is_dir())
self.assertEqual(0o700, runtime_root.stat().st_mode & 0o777)
self.assertFalse((runtime_root / "stale-upload.zip").exists())
self.assertEqual("keep", outside.read_text(encoding="utf-8"))
if __name__ == "__main__":

View file

@ -186,6 +186,48 @@ MAIN_PORT="$SCANNER_PORT"
MAIN_URL="http://127.0.0.1:$MAIN_PORT"
wait_for_health "$MAIN_URL"
docker exec -i "$MAIN_CONTAINER" python - <<'PY'
from skill_scanner.core.analyzers.llm_analyzer import LLMProvider
from skillhub_scanner_app import _redact_supported_tokens
canaries = {
"aws": "AKIA1234567890ABCDEF",
"github": "github_pat_abcdefghijklmnopqrstuvwxyz",
"jwt": "eyJabcde.abcdefgh.ijklmnop",
"labeled": "custom-secret-1234567890",
"private_key": "-----BEGIN PRIVATE KEY-----\nabc123\n-----END PRIVATE KEY-----",
}
findings = {
"aws": canaries["aws"],
"github": canaries["github"],
"authorization": f"Bearer {canaries['jwt']}",
"labeled": f"api_key={canaries['labeled']}",
"private_key": canaries["private_key"],
}
redacted = str(_redact_supported_tokens(findings))
for label, canary in canaries.items():
if canary in redacted:
raise SystemExit(f"{label} canary was not redacted")
safe_values = ["line one\n\tline two", "x" * 5000]
if _redact_supported_tokens(safe_values) != safe_values:
raise SystemExit("redaction changed safe multiline or long finding text")
mixed = "before\napi_key=custom-secret-1234567890\tafter"
if _redact_supported_tokens(mixed) != "before\napi_key=<redacted>\tafter":
raise SystemExit("redaction changed non-secret characters around a credential")
if not LLMProvider.is_valid_provider("azure-openai") or LLMProvider.is_valid_provider("azure"):
raise SystemExit("unexpected Scanner 2.1 Azure provider contract")
PY
docker exec "$MAIN_CONTAINER" python -c \
'from pathlib import Path; Path("/tmp/skillhub-scanner-runtime/stale-after-timeout").write_text("stale")'
docker restart "$MAIN_CONTAINER" >/dev/null
MAIN_PORT="$(docker port "$MAIN_CONTAINER" 8000/tcp | awk -F: 'END {print $NF}')"
MAIN_URL="http://127.0.0.1:$MAIN_PORT"
wait_for_health "$MAIN_URL"
docker exec "$MAIN_CONTAINER" python -c \
'from pathlib import Path; assert not Path("/tmp/skillhub-scanner-runtime/stale-after-timeout").exists()'
post_zip "$MAIN_URL" "$TMP_DIR/safe.zip" "$TMP_DIR/safe.json"
post_zip "$MAIN_URL" "$TMP_DIR/javascript-secret.zip" "$TMP_DIR/javascript-secret.json"
post_zip "$MAIN_URL" "$TMP_DIR/dotenv-secret.zip" "$TMP_DIR/dotenv-secret.json"

View file

@ -14,6 +14,14 @@ public record ScanOptions(
) {
public ScanOptions {
if ("azure".equals(llmProvider)) {
llmProvider = "azure-openai";
}
if (!"anthropic".equals(llmProvider)
&& !"openai".equals(llmProvider)
&& !"azure-openai".equals(llmProvider)) {
throw new IllegalArgumentException("llmProvider must be anthropic, openai, or azure");
}
if (llmConsensusRuns < 1) {
throw new IllegalArgumentException("llmConsensusRuns must be at least 1");
}

View file

@ -75,9 +75,6 @@ public class SkillScannerService {
body.put("policy", options.policyPreset());
body.put("enable_meta", options.enableMeta());
body.put("use_aidefense", options.useAidefense());
if (options.useAidefense() && !options.aidefenseApiKey().isEmpty()) {
body.put("aidefense_api_key", options.aidefenseApiKey());
}
body.put("use_virustotal", options.useVirusTotal());
body.put("use_trigger", options.useTrigger());
return body;

View file

@ -30,4 +30,20 @@ class ScanOptionsTest {
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("policyPreset");
}
@Test
void rejectsUnknownLlmProvider() {
assertThatThrownBy(() -> new ScanOptions(
false, false, "openai&unexpected=value", 1, "balanced", false, false, "", false, false))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("llmProvider");
}
@Test
void acceptsDocumentedAzureLlmProvider() {
ScanOptions options = new ScanOptions(
false, true, "azure", 1, "balanced", false, false, "", false, false);
assertThat(options.llmProvider()).isEqualTo("azure-openai");
}
}

View file

@ -128,7 +128,7 @@ class SkillScannerServiceTest {
}
@Test
void scanDirectory_keepsLegacyAidefenseBodyFieldAndSendsHeader() {
void scanDirectory_sendsAidefenseApiKeyOnlyViaHeader() {
FakeHttpClient httpClient = new FakeHttpClient();
httpClient.postResponse = new SkillScannerApiResponse(
"scan-4", "test-skill", true, "LOW", 0, null, 0.5, "2026-03-22T07:00:00");
@ -141,7 +141,7 @@ class SkillScannerServiceTest {
@SuppressWarnings("unchecked")
Map<String, Object> body = (Map<String, Object>) httpClient.lastPostBody;
assertThat(body.get("aidefense_api_key")).isEqualTo("secret-key");
assertThat(body).doesNotContainKey("aidefense_api_key");
assertThat(httpClient.lastPostHeaders.getFirst("X-AIDefense-Key")).isEqualTo("secret-key");
assertThat(httpClient.lastPostUri).doesNotContain("secret-key");
assertThat(body.get("llm_consensus_runs")).isEqualTo(3);