Pin cross-repo type-registry fetch; harden the MCP plugin#96
Merged
Conversation
Three hygiene fixes from the SecID audit: F-12-05 — both registry validators (validate-subtypes.py, validate-type-list.py) defaulted to fetching SecID-Service's type-registry.ts from the moving `main` branch. That is a cross-repo TOCTOU: an unrelated SecID-Service merge silently changes the canonical type/subtype set that gates every SecID PR here. Pinned to a reviewed commit SHA (2477ec7, the commit that last changed the file), with a fail-closed guard if the SHA is ever blanked. --type-registry-url/-path overrides preserved. Bump the SHA via a reviewed PR here when the upstream file changes (keep both validators in lockstep). F-13-03 — the MCP plugin used parse_known_args(), so a typo'd flag (--base-ur1) was silently dropped and the server fell back to the PUBLIC resolver, leaking internal queries. Moved argparse into main() with parse_args() (hard error on unknown flags) plus a stderr echo of the effective base URL. Side benefit: the module now imports cleanly (no argparse at import), so it is testable. F-13-01 — the plugin returned raw resolver JSON to the LLM. Registry free-text is third-party contributor-authored; a crafted description/scope/contacts field could carry injected instructions. Added an output-boundary sanitizer: strip C0/C1 + zero-width/bidi chars, cap length, relocate contributor prose under a labeled `registry_text_untrusted` envelope, and rebuild the response from an allowlist (drops unexpected top-level keys). Tool docstrings note the data is not instructions. +6 plugin unit tests. Validators verified against the pinned content (identical to the live file). The sibling F-04-01 fix for SecID-Service's Worker MCP is a separate PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three hygiene fixes from the SecID audit (SecID-2026-06-14-claude-skill): F-12-05, F-13-03, F-13-01.
F-12-05 — pin the cross-repo
type-registry.tsfetchBoth registry validators (
validate-subtypes.py,validate-type-list.py) defaulted to fetching SecID-Service'stype-registry.tsfrom the movingmainbranch. That's a cross-repo TOCTOU: an unrelated SecID-Service merge silently changes the canonical type/subtype set that gates every SecID PR here, with no review on this side.Pinned to a reviewed commit SHA (
2477ec7— the commit that last modified the file), with a fail-closed guard if the SHA is ever blanked.--type-registry-url/--type-registry-pathoverrides are preserved for local/feature-branch testing. Verified the pinned raw URL returns 200, byte-for-byte identical to the live file, so CI's network fetch still passes.F-13-03 — MCP plugin silently fell back to the public resolver
server.pyusedparse_known_args(), so a typo'd flag (--base-ur1) was silently dropped and the server kept the public default — leaking what were meant to be internal-resolver queries to the public endpoint. Moved argparse intomain()withparse_args()(hard error on unknown flags) plus a stderr echo of the effective base URL. Side benefit: the module now imports cleanly (no argparse at import), making it testable.F-13-01 — MCP plugin returned raw resolver JSON to the LLM
Registry free-text is third-party contributor-authored and flows straight into an LLM-read channel (the disclosure tooling even tells the agent to act on
contacts/scope). Added an output-boundary sanitizer: strip C0/C1 + zero-width/bidi chars, cap field/array length, relocate contributor prose under a clearly-labeledregistry_text_untrustedenvelope (_warning: data, not instructions), and rebuild the response from an allowlist so an unexpected upstream response can't inject arbitrary top-level keys. Tool docstrings carry the same note.The control-char class is built from integer code points via
chr()so the source stays pure ASCII (no literal invisible bytes).Tests
+6 plugin unit tests (
pytest plugins/secid/test_server.py, 6 pass): control/bidi stripping, length cap, prose relocation+labeling, envelope rebuild dropping unknown keys, non-dict input. Existingtest_validate_urls.pystill green; both validators pass against the pinned content.Notes for review
parse_args()may break a launcher passing unknown extra flags — the shipped.mcp.jsonpasses none (and--base-urlis a defined flag), so the happy path is unaffected._UNTRUSTED_TEXT_KEYS/_RESULT_STRUCTURAL_KEYSif the response schema gains new fields.🤖 Generated with Claude Code