Harden skills-get against hostile SKILL.md frontmatter and unverified JSON output - #408
Merged
Merged
Conversation
… JSON output Two findings from the v0.7.0-beta.0 release review of the new skills extension code, both triggerable by a remote MCP server: - The frontmatter key parser matched `[^:#]+?\s*:\s`, which backtracks quadratically on a long run of spaces inside a field name: 0.5 s at 20k spaces, hours within the 10 MB IPC cap, hanging the CLI in every output mode. Replaced by a single-pass split. A nesting-depth cap (32) turns a stack overflow on deeply nested collections into a parse error, and lines indented left of their block's first field are refused instead of silently dropped from the compared frontmatter. - `skills-get --json` verified only the `contents[]` item matching the target URI but emitted the whole array the server returned, so an extra item at `contents[0]` reached the caller unverified. JSON mode now emits a single item rebuilt from the verified bytes. Also records the release review in docs/security-review-2026-09.md: H5 and M9 fixed, M3 partly fixed, plus an addendum for v0.7.0-beta.0. Refs #401 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018UEz9rKjGcS6PmcKgNvT85
jancurn
added a commit
that referenced
this pull request
Sep 20, 2026
…s in hints (#410) Two small fixes from the v0.7.0-beta.1 release security review, plus the review write-up. A hostile `SKILL.md` could crash the CLI with a stack overflow despite the 32-level nesting cap, and the copy-paste hints after `skills-get`, `resources-directory-read` and `resources-read` interpolated server-supplied names unquoted, so a skill named `pdf; curl … | sh #` produced a hint that ran the injected command. - Check the nesting depth at the top of `parseMapping` and `parseSequence`, which recurse into each other directly and bypassed the check in `parseBlock`; an 8.6 MB alternating frontmatter now fails with a parse error instead of a `RangeError`. - Route the four hint sites through `quoteShellArg()` (`formatPath()` now delegates to it); unit tests cover hostile names and URIs. - Add the beta.1 addendum to `docs/security-review-2026-09.md` and correct H5 to "partially fixed": profiles created before 0.7 still refresh at whatever authorization server the MCP server points to. Everything else in the addendum is left for follow-ups. Refs #401, #408 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01J7TdVXNthTXXbmDtGJ7sYm --- _Generated by [Claude Code](https://claude.ai/code/session_01J7TdVXNthTXXbmDtGJ7sYm)_ Co-authored-by: Claude Fable 5.1 <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
3 participants
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.
Fixes two server-triggerable issues found in the v0.7.0-beta.0 release review of the skills extension (#401): a
SKILL.mdline with a long run of spaces inside a field name made the frontmatter parser backtrack quadratically and hangskills-getfor hours, andskills-get --jsonprinted everycontents[]item the server returned while verifying only one.skills-get --jsonemits one content item rebuilt from the verified bytesdocs/security-review-2026-09.mdgets the release-review status updates and a v0.7.0-beta.0 addendumRefs #401
🤖 Generated with Claude Code
https://claude.ai/code/session_018UEz9rKjGcS6PmcKgNvT85
Generated by Claude Code