From 673e52387965d57687d8908be288b3bfb864b529 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Wed, 12 Aug 2026 20:57:43 +0200 Subject: [PATCH] fix(security-strings): audit .mcpb manifests as structured text, not as binaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v0.10.3 release failed its `verify` gate: --- Auditing: scan-...--manifest.json --- BLOCKED: Unauthorized URL in binary: https://github.com/DeusData BLOCKED: Unauthorized URL in binary: https://deusdata.github.io/... === BINARY STRING AUDIT FAILED === The blocked URLs are OURS — the .mcpb bundle manifest's own homepage and documentation fields. security-strings.sh already knows that URL and dangerous-command auditing are "designed for compiled artifacts" and skips them for text, keeping the credential and base64 audits that are universally meaningful. Its detection just had no case for JSON: `file` reports "JSON data", which matched none of the shell-script/ASCII-text patterns, so a manifest was audited as if it were a compiled binary. An MCP manifest is exactly the shape that misfires — it carries project URLs by design, and a `command` field by specification. This is the first release to ship .mcpb bundles, which is why it surfaced now: it is the first time a manifest reached the scanned object set. The bundles were noted as end-to-end unproven until a stable release; this is that proof doing its job. Two changes: - Classify structured-text artifacts by extension FIRST (.json/.yaml/.toml/ .md/...), then by `file` type with JSON/XML added. The extension check also covers hosts without file(1), where the existing `command -v` guard silently left every artifact classified as binary. - Allow-list our own org root and documentation site, so a compiled binary embedding the docs link (the graph UI does) passes on its own merits rather than relying on the text exemption. Tests: the manifest case, plus a control proving the exemption is narrow — the credential audit still runs on structured text. The existing negative control (an unauthorized URL in a real binary is still BLOCKED) is unchanged and still passes, so the binary audit is not weakened. Signed-off-by: Martin Vogel --- scripts/security-strings.sh | 18 ++++++++-- tests/test_security_strings_allowlist.sh | 42 ++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/scripts/security-strings.sh b/scripts/security-strings.sh index 26cb4e34f..f24f4912d 100755 --- a/scripts/security-strings.sh +++ b/scripts/security-strings.sh @@ -31,10 +31,20 @@ FAIL=0 # audits which are designed for compiled artifacts. Script content is # reviewed in PRs and scanned end-to-end by VirusTotal in the same pipeline. IS_SCRIPT=false -if command -v file &>/dev/null; then +# Structured-text artifacts by EXTENSION first: `file` reports JSON as +# "JSON data" / "JSON text data", which matched none of the text patterns +# below, so an .mcpb bundle manifest was audited as if it were a compiled +# binary and its own homepage/documentation URLs were BLOCKED as unauthorized. +# The extension check also covers hosts where file(1) is absent, where the +# `command -v` guard below silently leaves every artifact classified as binary. +case "$BINARY" in + *.json|*.jsonc|*.yaml|*.yml|*.toml|*.md|*.txt) IS_SCRIPT=true ;; +esac +if [ "$IS_SCRIPT" = false ] && command -v file &>/dev/null; then FILE_TYPE=$(file -b "$BINARY" 2>/dev/null || echo "") case "$FILE_TYPE" in - *"shell script"*|*"ASCII text"*|*"UTF-8 Unicode text"*|*"Unicode text"*|*"a /usr/bin/env"*) + *"shell script"*|*"ASCII text"*|*"UTF-8 Unicode text"*|*"Unicode text"*|*"a /usr/bin/env"*|\ + *"JSON"*|*"XML"*) IS_SCRIPT=true ;; esac @@ -72,6 +82,10 @@ echo "--- URL audit ---" ALLOWED_URLS=( "https://api.github.com/repos/DeusData/codebase-memory-mcp" "https://github.com/DeusData/codebase-memory-mcp" + # Our own org root and documentation site: the .mcpb bundle manifest carries + # them as homepage/documentation fields, and the graph UI links the docs. + "https://github.com/DeusData" + "https://deusdata.github.io/codebase-memory-mcp" "http://127.0.0.1" "http://localhost" # SQLite internal URLs (part of vendored sqlite3 strings) diff --git a/tests/test_security_strings_allowlist.sh b/tests/test_security_strings_allowlist.sh index 03305ac9e..8958d3148 100755 --- a/tests/test_security_strings_allowlist.sh +++ b/tests/test_security_strings_allowlist.sh @@ -63,5 +63,47 @@ else PASS=$((PASS + 1)) fi +# ── Case 3: an .mcpb bundle manifest must not be audited as a compiled binary ── +# `file` reports JSON as "JSON data", which matched none of the text patterns, so +# the manifest was run through the URL audit and its own homepage/documentation +# fields were BLOCKED as unauthorized. This blocked the v0.10.3 release — the +# first release to ship .mcpb bundles, hence the first time a manifest reached +# the scanned object set. +MANIFEST="$TMP/manifest.json" +cat > "$MANIFEST" <<'JSON' +{ + "name": "codebase-memory-mcp", + "homepage": "https://github.com/DeusData/codebase-memory-mcp", + "documentation": "https://deusdata.github.io/codebase-memory-mcp/", + "server": { "command": "codebase-memory-mcp", "args": [] } +} +JSON +if bash "$SCRIPT" "$MANIFEST" >/dev/null 2>&1; then + echo "PASS: .mcpb manifest.json audited as structured text, not as a binary" + PASS=$((PASS + 1)) +else + echo "FAIL: manifest.json still audited as a compiled binary (regression)" + bash "$SCRIPT" "$MANIFEST" 2>&1 | grep -i "BLOCKED" || true + FAIL=$((FAIL + 1)) +fi + +# ── Case 4 (negative control for case 3): the exemption is by FILE TYPE, not a +# blanket pass. A binary carrying an unauthorized URL must still be blocked even +# though a .json carrying one would not be — that asymmetry is the design, and +# case 2 above proves the binary half still holds. Here we prove the credential +# audit still runs on structured text, so the exemption is narrow. ── +CREDS="$TMP/creds.json" +# The credential audit matches assignment syntax (api_key=, secret=, ...), which +# is what leaks look like in embedded strings — so the fixture uses that form +# inside the JSON value rather than a JSON "key": "value" pair. +printf '{"connection":"host=db user=admin password=hunter2-not-a-real-secret"}' > "$CREDS" +if bash "$SCRIPT" "$CREDS" >/dev/null 2>&1; then + echo "FAIL: credential pattern in JSON was NOT flagged (exemption too broad)" + FAIL=$((FAIL + 1)) +else + echo "PASS: credential audit still runs on structured text" + PASS=$((PASS + 1)) +fi + echo "=== security-strings allow-list test: $PASS passed, $FAIL failed ===" [ "$FAIL" -eq 0 ]