Skip to content

fix: close four install-gate bypasses and five output-integrity bugs (0.15.0) - #97

Merged
singhharsh1708 merged 2 commits into
feat/update-difffrom
fix/audit-security-integrity
Aug 7, 2026
Merged

singhharsh1708 merged 2 commits into
feat/update-difffrom
fix/audit-security-integrity

Conversation

@singhharsh1708

Copy link
Copy Markdown
Owner

Stacked on #96 (base: feat/update-diff). A multi-agent audit ran five independent review passes over the codebase and every finding was independently reproduced before being accepted — 12 confirmed, all fixed here, each with a regression test that reproduces the original exploit. Nine were reachable in published 0.13.0.

Install-gate bypasses (all four were hostile-skill paths)

Bypass Fix
1 A trailing {{ broken }} made resolveBody() throw, and the four "non-bypassable" safety lints only ran when it succeeded — so curl … | sh + a broken token installed with exit 0 scanners run on the raw source when resolution fails; only budget measurement stays gated
2 One NUL byte marked any auxiliary file "binary" and skipped every scanner on it, while the file stayed readable prose binary = control-char density over 8 KB; NULs stripped before scanning and reported as hidden text
3 [__proto__.policy] in an untrusted skill.toml wrote onto Object.prototype, handing itself deny_remote_exec = false before loadPolicy() ran null-prototype tables; __proto__/constructor/prototype refused as table or key names
4 allow_sources was matched against the un-normalized string, so file:/approved/../untrusted/evil passed an /approved/* allowlist — and was persisted as the lockfile source, so doctor said policy: ok forever canonical-only matching; the error shows both forms when they differ

Plus a path traversal: triggers.commands was only checked for a leading /, and the claude-code adapter builds .claude/commands/<cmd>.md from it verbatim, so "/../../../../../../tmp/x" made compile write six levels above the project. The schema always said ^/[a-z][a-z0-9-]*$; it is now enforced at manifest load, and compile refuses any output path resolving outside the project root.

Output-integrity bugs

  • mergeSection passed the compiled body as a String.replace replacement, so a skill documenting $$ (Make), $& (sed) or $' corrupted AGENTS.md on the second compile — $$HOME silently became $HOME, and $& spliced the old section into itself leaving doubled markers that broke pruning.
  • compile silently dropped skills whose manifest stopped loading and pruned their AGENTS.md sections, exiting 0 — the skill still on disk, still pinned, every agent quietly losing its instructions.
  • Installing owner/repo copied the clone's .git, and git's index/reflog differ between two clones of the same commit, so update/diff never converged and the review diff filled with .git/….
  • An unmanifested skill installed from a repo root was named after the random mkdtemp directory, so every update derived a new name and refused itself as a rename, forever.
  • update/diff showed added files in full but listed removed ones by name only — deleting the script a SKILL.md points at passed review unseen.

Verification

  • 21 new assertions (A1–A10), each reproducing the original exploit; full suite green on macOS, Ubuntu, Windows.
  • The three highest-severity exploits 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 test fixture relied on the lax command validation and was updated; the case it covered is now asserted at manifest-load time instead, plus a traversal case.
  • npm run check clean, npm run bench deterministic, schema parses, site/build.mjs restamped to 0.15.0.

Docs: the trust page documented the old raw-string allowlist matching and the NUL-as-binary rule; both corrected, with the .git exclusion and update's re-enforced gate documented.

…(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.
@vercel

vercel Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
kitbash Ready Ready Preview Aug 7, 2026 7:36am

@github-actions github-actions Bot added documentation Docs, spec, RFCs, README, site dependencies Dependency or action version bumps labels Aug 7, 2026
…th-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.
@singhharsh1708
singhharsh1708 merged commit d0e63ca into feat/update-diff Aug 7, 2026
8 checks passed
singhharsh1708 added a commit that referenced this pull request Aug 7, 2026
…(0.15.0) (#97) (#98)

* 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.
@singhharsh1708
singhharsh1708 deleted the fix/audit-security-integrity branch August 7, 2026 07:46

This branch was successfully deployed

1 active deployment
Preview — f3aa0a9f Deployed Aug 7, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Dependency or action version bumps documentation Docs, spec, RFCs, README, site

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant