Repository navigation
fix: close four install-gate bypasses and five output-integrity bugs (0.15.0) - #98
Merged
Merged
Conversation
…(0.15.0) (#97) * fix: close four install-gate bypasses and five output-integrity bugs (0.15.0) Install gate: - a broken {{template}} made resolveBody throw, which skipped all four body safety lints; they now scan the raw source when resolution fails - one NUL byte marked any auxiliary file "binary" and exempted it from every scanner; binary is now control-char density, NULs are stripped and reported as hidden text - [__proto__.policy] in a skill.toml wrote onto Object.prototype and fabricated deny_remote_exec = false before loadPolicy ran; tables are null-prototype and prototype-reaching names are refused - allow_sources matched the un-normalized source, so file:/approved/../untrusted/x passed an /approved/* allowlist and was then persisted as the lockfile source; matching is canonical-only - triggers.commands was only checked for a leading slash, so a path value made compile write outside the repo; the schema's ^/[a-z][a-z0-9-]*$ is now enforced at load and compile refuses any escaping output path Output integrity: - mergeSection passed the body as a String.replace replacement, so $$, $& and $' corrupted AGENTS.md on recompile - compile hid unloadable skills and pruned their sections while exiting 0; it now reports, preserves their output, and exits non-zero - clone .git was copied into installed skills, so update/diff never converged - bare skills from a repo root were named after the mkdtemp dir, so update refused them forever; callers pass a name hint - removed files were listed but never diffed in the update review Every fix carries a regression test reproducing the original exploit. * fix: site builder had the same $-replacement bug; make the A7 test path-safe site/build.mjs spliced the generated changelog between its markers with a replacement string, so the 0.15.0 entry documenting $$ and $& duplicated content outside the markers and never converged — build --check then reported the page permanently stale. Replacer function, same as the compiler fix; the published changelog is now its own regression test. The allow_sources test interpolated a temp path into a TOML basic string, where a Windows path's backslashes are escapes: the manifest failed to parse before policy was ever consulted. Paths are JSON.stringify'd and built with join(), and the case that the allowlist still admits a legitimate source is asserted too.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
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.
Re-targets #97 at
main. #97 was opened againstfeat/update-diff(it was stacked on #96) and merged there, but #96 was squash-merged intomainseparately — so these fixes never reachedmain. This is the same commit, cherry-picked cleanly ontomain, with the full gate re-run: typecheck clean, suite green, benchmark deterministic,site/build.mjs --checkcurrent, schema parses.Everything below was reachable in published 0.13.0.
Install-gate bypasses
{{ broken }}maderesolveBody()throw, and the four "non-bypassable" safety lints only ran when it succeeded — socurl … | shplus a broken token installed with exit 0[__proto__.policy]in an untrustedskill.tomlwrote ontoObject.prototype, handing itselfdeny_remote_exec = falsebeforeloadPolicy()ran__proto__/constructor/prototyperefused as table or key namesallow_sourcesmatched the un-normalized string, sofile:/approved/../untrusted/evilpassed an/approved/*allowlist — and was persisted as the lockfile source, sodoctorreportedpolicy: okforeverPlus a path traversal:
triggers.commandswas only checked for a leading/, and the claude-code adapter builds.claude/commands/<cmd>.mdfrom it verbatim, so"/../../../../../../tmp/x"madecompilewrite six levels above the project. The schema always specified^/[a-z][a-z0-9-]*$; it is now enforced at manifest load, andcompilerefuses any output path resolving outside the project root.Output-integrity bugs
mergeSectionpassed the compiled body as aString.replacereplacement, so a skill documenting$$(Make),$&(sed) or$'corruptedAGENTS.mdon the second compile —$$HOMEsilently became$HOME, and$&spliced the old section into itself leaving doubled markers that broke pruning.compilesilently dropped skills whose manifest stopped loading and pruned theirAGENTS.mdsections, exiting 0 — the skill still on disk, still pinned, every agent quietly losing its instructions.owner/repocopied the clone's.git; git's index/reflog differ between two clones of the same commit, soupdate/diffnever converged and the review diff filled with.git/….mkdtempdirectory, so everyupdatederived a new name and refused itself as a rename, forever.update/diffshowed added files in full but listed removed ones by name only — deleting the script aSKILL.mdpoints at passed review unseen.site/build.mjshad the same$-as-replacement-pattern bug, found because the 0.15.0 changelog entry documents$$and$&: the generated HTML duplicated outside its markers and never converged, so--checkreported the page permanently stale.Verification
Findings came from a multi-agent audit — five independent review passes, every finding reproduced before it was accepted — and each fix carries a regression test that reproduces the original exploit (
A1–A10, 24 assertions). The three highest-severity exploits were also re-run by hand against the built CLI: all now exit 1 with a named lint or manifest error, and the traversal file is never created.One pre-existing fixture relied on the lax command validation; the case it covered is now asserted at manifest-load time instead, alongside a traversal case.