fix(hook): stop stdin choosing which directory is a project's transcript store - #370
Open
fstubner wants to merge 2 commits into
Open
fix(hook): stop stdin choosing which directory is a project's transcript store#370fstubner wants to merge 2 commits into
fstubner wants to merge 2 commits into
Conversation
…nd keep design drafts out of search CHANGELOG.md listed 112 "release xtctx X.Y.Z" entries running up to 0.74.0 under the 0.20.0 heading. No tag exists above 0.21.8 and npm has never published past 0.19.0, so a reader scanning the file for what shipped was being shown versions that do not exist. They are real commits — an automated pipeline cut them in a few hours on 2026-08-29 before the version was reset to 0.20.0 and the pipeline replaced by a manually-triggered release. So this summarises what happened in one entry rather than deleting the history: the churn is worth recording, the 112 individual bullets are not. Separately, `landing/src/pages/v9.astro` is a design draft that builds to a public URL and carried no `noindex`, so a near-duplicate of the landing page was indexable alongside the real one — and its hand-copied FAQ has already drifted from the live copy, describing four MCP tools where the real page lists five. The `concepts/` drafts already carried the tag; v9 did not, and nothing carried a robots.txt. Verified against a real build: `landing/dist/robots.txt` is emitted, the built v9 page carries the meta tag, and the real landing page does NOT — a noindex on the published page is the failure this could most easily have introduced. The robots.txt deliberately names no sitemap. There is no sitemap.xml in the build, and pointing crawlers at a 404 is its own small false claim.
…ipt store The session-start hook reads `transcript_path` from its stdin and recorded `dirname()` of it into `.xtctx/state/store-dirs.json` as this project's claude-code store, to be read on every later scan. Nothing checked where it pointed. Reproduced end to end before the fix: a payload naming this project in `cwd` and `<project>/../../../../../../evil/x.jsonl` in `transcript_path` recorded the escaped directory verbatim. The scraper reads a recorded store dir with `exactDirectory: true` — the mode that lets a record carrying no `cwd` past the ownership check — so an attacker-chosen directory's contents came back as this project's own history, to the next agent, with no provenance check. The `cwd` guard already in the hook does not cover this. It validates a different field, and a payload can name the project correctly there while pointing `transcript_path` anywhere on the machine; that is the shape used to reproduce it. A legitimate value always sits under the tool's own store root as `<store>/<encoded-project>/<id>.jsonl`, so containment against that root is the check that fits. The root comes from the tool definition rather than being assumed, because `CLAUDE_CONFIG_DIR` moving the tree is the case this payload exists to handle in the first place. `resolve` collapses `..` before the comparison, so the decision is made on the destination rather than the string. A path outside the root is dropped rather than raised: the hook runs inside another agent's startup and must not fail a session over a payload it merely mistrusts. Dropping it costs only the reconstruction this was optimising away. Verified by running the built CLI: traversal rejected, an unrelated absolute path rejected, a genuine `~/.claude/projects/...` path still recorded. Removing the containment check turns two of the three tests red. Found by an audit agent fixturing hostile input against the real hook.
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
1 participant
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.
The session-start hook reads
transcript_pathfrom stdin and recordeddirname()of it into.xtctx/state/store-dirs.jsonas this project's claude-code store, read on every later scan. Nothing checked where it pointed.Reproduced before the fix
A payload naming this project correctly in
cwd, and pointing elsewhere intranscript_path:{"cwd":"<project>","transcript_path":"<project>/../../../../../../evil/x.jsonl"}recorded the escaped directory verbatim. The scraper reads a recorded store dir with
exactDirectory: true— the mode that lets a record carrying nocwdpast the ownership check — so an attacker-chosen directory's contents came back as this project's own history, to the next agent, with no provenance check.The
cwdguard already in the hook doesn't cover this: it validates a different field, and a payload can name the project correctly there while pointingtranscript_pathanywhere on the machine. That's the shape used to reproduce it.The fix
A legitimate value always sits under the tool's own store root as
<store>/<encoded-project>/<id>.jsonl, so containment against that root is the check that fits.CLAUDE_CONFIG_DIRmoving the tree is the case this payload exists to handle in the first place.resolvecollapses..before comparing, so the decision is made on the destination rather than the string.Verification, running the built CLI
<project>/../../../../../../evil/x.jsonlC:/Windows/Temp/x.jsonl~/.claude/projects/h--some-project/abc.jsonlThat last row matters: rejecting it too would be a "fix" that silently turns the optimisation off for everyone.
709 tests pass, typecheck and lint clean.
Found by an audit agent fixturing hostile input against the real hook.