security: containment, info-disclosure, and test-isolation hardening by kerneltoast · Pull Request #716 · mksglu/context-mode

kerneltoast · 2026-05-26T00:34:59Z

Summary

Deep security audit across the in-scope codebase. Nine fixes (plus one test-only slice widening) covering exploitable supply-chain / local-trust-boundary paths, an info-disclosure DoS surface, and destructive-test hygiene.

Containment guards (the big block of work)

bf4c948 -- /ctx-upgrade swap loop in both cli.ts and server.ts (the inline-fallback variant) iterated pkg.files[] from the freshly cloned upstream package.json and handed each entry to rmSync + cpSync with no startsWith(pluginRoot) containment. A compromised upstream tag shipping files: ["../../.ssh/authorized_keys"] would rm and overwrite arbitrary files under the user's UID. Mirrors the lexical guard pattern that hooks/heal-partial-install.mjs already uses for its own files[] expansion.
e7d5599 -- cli.ts reads ~/.claude/plugins/installed_plugins.json in two hot spots (upgrade() skills cpSync and statuslineForward() dynamic-import). The same field already had a startsWith(cacheRoot + sep) guard in server.ts:790 (healCacheMidSession); this just propagates it to the more sensitive call sites. statusline re-fires several times per second, so a malicious installPath planted in the registry by a co-resident plugin or postinstall would otherwise hand the attacker durable RCE under the user account.
dff7bac -- closing adversarial sweep caught that the lexical guards above (and the swap-loop guard in bf4c948) are bypassable via a planted symlink AT cacheRoot/<owner>/<plugin>/<version> because path.resolve is purely lexical and doesn't dereference symlinks. Adds a realpathSync re-check after the lexical gate for the installPath sites and an lstatSync-backed cpSync({filter}) for the swap loop so neither a symlink anchor at the registry path nor a committed symlink inside the cloned source tree can sneak past.
b01278d -- ctx_insight accepted sessionDir / contentDir MCP overrides that flowed verbatim into INSIGHT_SESSION_DIR / INSIGHT_CONTENT_DIR on the spawned dashboard subprocess. Without containment, a prompt-injected MCP caller passing sessionDir="~/.ssh" turns the dashboard into a confused-deputy enumerator over the user's filesystem (and, for hex-named SQLite files, can delete rows via /api/content). Gates explicit overrides to the adapter config root (dirname(dirname(getSessionDir()))).
760d476 -- healClaudeJsonMcpArgs rewrote mcpServers.args[] in ~/.claude.json by slicing the arg, then resolve(newPluginRoot, suffix). The slice is a literal substring; .. characters embedded in the original arg survive into suffix, and resolve normalizes them when it builds newArg. A crafted ~/.claude.json could trick the heal into rewriting MCP spawn args at attacker-chosen paths. Adds a post-resolve startsWith guard.
25606f2 -- ctx_index's directory-dispatch branch used statSync rather than lstatSync, so a path like /tmp/link with link -> /etc was reported as a directory and walked into /etc. The deny-glob check at the head of the handler ran against /tmp/link, not the symlink target, so users whose deny globs included /etc but not /tmp would still see /etc content land in the FTS5 store. Switches to lstatSync + realpathSync + re-applied deny check.

Info-disclosure / DoS

3568d9a -- bundles two src/server.ts fixes. ctx_fetch_and_index had no size cap on either the subprocess fetch or the parent's readFileSync(outputPath), so a malicious URL returning multi-GB content could OOM the long-running MCP server. Adds a 50 MB cap on both ends. Same commit sanitizes CLAUDE_SESSION_ID before splicing it into a path.join-based stats filename, so a host-controlled CLAUDE_SESSION_ID="../../evil" can no longer write outside statsDir.

Destructive-test hygiene (matches PR #702 pattern)

c8a5728 -- tests/lifecycle.test.ts wrote _lifecycle_test_*.ts to process.cwd() (the project root, not gitignored) and tests/adapters/cursor.test.ts mkdir'd, wrote, and rmSync(force:true)'d .cursor/mcp.json against process.cwd(), silently clobbering the contributor's real .cursor/mcp.json on npm test. Both blocks now sandbox the writes inside per-test mkdtempSync directories (and chdir for the cursor case so the adapter still finds the test fixture).

Test-only follow-up

123b21b -- three pre-existing tests in tests/core/cli.test.ts sliced cli.ts source by fixed-width windows that the new containment comments + checks pushed assertions out of. Widened the slices to end-of-file; the textual landmarks (sweepStaleMcpJson, process.exit(0)) still anchor the regex assertions, so the wider slices don't weaken anything.

Out-of-scope (deferred to follow-up if you want them)

F02 SSRF defaults: working as designed (loopback / RFC1918 reachable unless CTX_FETCH_STRICT=1); could be surfaced in ctx_doctor.
F08: ...process.env spread into the npm install subprocess on /ctx-upgrade still propagates CLAUDE_* env into transitive postinstall scripts.
F27: ~20 writeFileSync call sites on critical config files don't use tmp + rename. Reliability concern, not exploitable.
F28: same-uid chmod TOCTOU in hooks/cache-heal-utils.mjs.
CONTEXT_MODE_SECURITY_BUNDLE_PATH env-controlled dynamic import in hooks/core/routing.mjs:364. Documented as a test seam; env is parent-process-controlled (same trust as the user). Fixing strictly would break legitimate test fixtures in tmpdir.

Test plan

node node_modules/.bin/tsc --noEmit is clean.
vitest run tests/core/cli.test.ts -- 173 / 173 pass.
vitest run tests/util/heal-installed-plugins.test.ts -- 33 / 33 pass.
vitest run tests/adapters/cursor.test.ts -- 39 / 39 pass.
vitest run tests/core/server.test.ts -- 463 / 463 pass when run unsandboxed (the in-sandbox EROFS failures are pre-existing in any executor / turndown / projectRoot test that needs mkdtempSync("/tmp/.ctx-mode-")).
Manual /ctx-upgrade smoke on a fresh install (legitimate path) -- the cosmetic regression for users whose ~/.claude itself is a symlink is documented in the dff7bac commit message and is not a security regression.

Co-authored-by: Claude Opus 4.7 (1M context) noreply@anthropic.com

NgoQuocViet2001 · 2026-05-26T06:37:44Z

@kerneltoast I checked this on Windows locally and found a few test-only issues, not a core security-code regression.

Findings:

tests/core/cli.test.ts and tests/core/server.test.ts create directory symlinks with symlinkSync(..., "dir"). On Windows without symlink privilege / Developer Mode this fails with EPERM. Using "junction" on Windows keeps lstatSync(...).isSymbolicLink() === true and makes the tests pass.
tests/core/server.test.ts still expects the old inline fallback snippet cpSync(from,to,{recursive:true,force:true}); this PR now correctly emits filter:noSymlink, so the assertion needs updating.
tests/lifecycle.test.ts shells out to which tsx, which fails in my Windows local run. Resolving node_modules/.bin/tsx.cmd first, then falling back to where/which, fixes it.

Validated locally on Windows:

node node_modules/typescript/bin/tsc --noEmit
node node_modules/vitest/vitest.mjs run tests/core/cli.test.ts
node node_modules/vitest/vitest.mjs run tests/core/server.test.ts -t "inline fallback copies package files to plugin root|lstatSync \\+ realpath identifies a symlinked directory"
node node_modules/vitest/vitest.mjs run tests/lifecycle.test.ts
node node_modules/vitest/vitest.mjs run tests/util/heal-installed-plugins.test.ts
node node_modules/vitest/vitest.mjs run tests/adapters/cursor.test.ts
git diff --check

I have the tiny patch locally if useful.

The /ctx-upgrade swap loop in cli.ts and the inline-fallback variant in server.ts both iterate pkg.files[] read straight out of the freshly cloned upstream package.json and hand each entry to rmSync plus cpSync without checking that the resolved destination stays inside pluginRoot. A compromised upstream tag at kerneltoast/context-mode (cli.ts) or mksglu/context-mode (server.ts inline) shipping "files": ["../../.ssh/authorized_keys", ...] would, on the next /ctx-upgrade, delete and overwrite arbitrary files under the user's UID. cli.ts is additionally vulnerable to absolute-path entries since path.resolve(P, "/abs") discards P; server.ts uses path.join, which defuses that variant but still follows relative "..". Fix it by mirroring the lexical containment pattern that hooks/heal-partial-install.mjs (mksglu#699) already uses for its own files[] expansion: resolve(rootDir) + sep, then drop items whose resolved path doesn't startsWith it. Both ends are checked; the "from" side blocks symlink-anchor variants where files[] is benign but a symlink inside the clone redirects. server.ts's inline script also switches from join() to resolve() so absolute-path entries normalize to themselves and then fail the pluginRoot prefix check. Tests in tests/core/cli.test.ts verify the guard appears at both sites and exercise the algorithm against a sandbox tmpdir tree where the malicious items target a planted victim file outside pluginRoot. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cli.ts reads `installed_plugins.json` in two hot spots: upgrade()'s skills-sync site cpSyncs the in-repo skills/ tree to `<installPath>/skills`, and statuslineForward() picks an `<installPath>/bin/statusline.mjs` candidate to dynamic-import every time the statusline re-renders (~3-5 Hz while Claude Code is open). Both sites take `installPath` verbatim from the JSON, and neither checks that it resolves under `<claudeRoot>/plugins/cache`. A co- resident plugin, a malicious dependency's postinstall script, or any other local actor who can write the registry can drop in `installPath: "/tmp/pwn"` once and convert that into a recursive write outside the cache root plus durable RCE via the statusline loop, persisting until Claude Code's next session-start sync rewrites the file. The same field already has a lexical containment guard in server.ts:790 (healCacheMidSession), which rejects entries whose `resolve()` doesn't start with `<claudeRoot>/plugins/cache + sep`. That guard was written by the same author for a less sensitive operation (creating a recovery symlink); the more sensitive cpSync and dynamic-import paths missed it. Fix it by lifting the same pattern to both call sites: resolve cacheRoot once, normalize installPath, drop anything whose normalized path doesn't start with `cacheRoot + sep`. path.resolve normalizes ".." away before the prefix check, so a "<legitCacheDir>/../../outside" installPath fails the gate the same way an outright "/etc" entry does. Tests in tests/core/cli.test.ts assert the guard appears at both sites and exercise the algorithm against a sandbox tmpdir tree where the malicious entries point outside the cache root. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

ctx_insight accepts `sessionDir` and `contentDir` MCP overrides that flow into INSIGHT_SESSION_DIR / INSIGHT_CONTENT_DIR on the spawned insight server. The server enumerates every *.db file under those roots via /api/sessions and /api/content, and its DELETE handler rewrites hex-named .db files in CONTENT_DIR. Without a containment guard, a prompt-injected MCP caller passing sessionDir="~/.ssh" or contentDir="~/.gnupg" turns the dashboard into a confused-deputy enumerator that lists, reads, and (for files whose name matches `^[a-f0-9_]+$`) deletes rows from SQLite databases the user happens to have under arbitrary directories. The 127.0.0.1 bind and the read-only opens on most routes shrink the blast radius, but the DELETE path stays write-capable. Fix it by gating explicit overrides against the adapter's config root, derived as `dirname(dirname(getSessionDir()))`. The default session dir is `<adapterConfig>/context-mode/sessions`, so the containment root resolves to e.g. `~/.claude` on Claude Code, `~/.codex` on Codex, etc. That's broad enough to cover the documented "multi-install setups or pointing at a sibling project's data" use case, narrow enough to block /etc, ~/.ssh, /tmp/<foreign-user>, and similar confused-deputy targets. The trailing `+ sep` on both sides defends against the `<root>-evil` prefix-collision case. Defaults (no explicit override) are untouched -- the gate fires only when sessionDir or contentDir is passed. Tests in tests/core/server.test.ts pin the handler source and exercise the containment algorithm against a sandbox tmpdir tree where the inputs span legitimate, /etc, traversal, and prefix- collision shapes. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

healClaudeJsonMcpArgs rewrites mcpServers args in ~/.claude.json so they point at the freshly upgraded plugin version. The rewrite reads the old arg out of the config, slices off the cacheParent prefix + the version directory, then runs the remaining `suffix` through resolve(newPluginRoot, suffix). The slice is a literal substring, so ".." characters embedded in the original arg survive into the suffix; resolve normalizes them when it builds newArg, potentially landing newArg outside newPluginRoot. A crafted ~/.claude.json with an arg like /home/user/.claude/plugins/cache/<owner>/<plugin>/1.0.0/../../../evil/start.mjs slices to suffix="../../../evil/start.mjs", and the rewrite substitutes the attacker-chosen .mjs path back into ~/.claude.json as the new MCP spawn target. The next MCP boot would then spawn node against that path. ~/.claude.json is locally user-writable, same trust boundary as installed_plugins.json (which the cli.ts fix in 1e13b37 already gates). A co-resident plugin, a malicious dependency's postinstall script, or any other process running as the same user can plant the crafted arg once and survive subsequent self-heal passes as long as the heal happily writes the traversal-derived newArg. Fix it by adding a post-resolve startsWith guard: drop the rewrite if `newArg + sep` does not start with `resolve(newPluginRoot) + sep` (with the newArg == newPluginRoot case allowed so a suffix of "" is still in-bounds). path.resolve normalizes the ".." away before the prefix check, so traversal-style suffixes fail the gate the same way absolute-path entries would. Legitimate version-rename rewrites keep working since their suffix never escapes newPluginRoot. Tests cover both the legitimate-rename happy path (preserved) and two traversal shapes (skipped; original attacker strings remain in place rather than being rewritten to attacker-chosen paths). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ymlinked dir ctx_index's directory-dispatch branch used statSync(resolvedPath) .isDirectory() to decide whether to walk a path as a tree. statSync follows symlinks, so a path like /tmp/link with link -> /etc was reported as a directory and dispatched into walkDirectoryDetailed; that helper then realpathSync'd internally and walked /etc. The deny-glob check at the head of the handler had run against the input path (/tmp/link), not the symlink target (/etc), so a user whose deny globs included /etc but not /tmp would still see /etc content land in the FTS5 store, queryable via ctx_search and exfiltratable into the model context. The default `followSymlinks: false` for the walker only blocks CHILD symlinks, not the root. Fix it by lstatSync'ing the resolved path before dispatch. When the path is a symlink, realpath it once and re-apply checkFilePathDeny Policy against the actual target so the user's deny globs see the real walk root. Symlinks whose realpath equals the input pass through unchanged (rare, but safe). Symlinks whose realpath can't be resolved (broken / dangling) get a clear refusal text rather than falling through to the file-read path. Legitimate symlink-to-dir users (e.g. ~/myproject -> /mnt/usb/...) still work since the re-deny check passes when the realpath isn't in the user's deny globs. Tests in tests/core/server.test.ts pin the handler source against the new lstat + realpath + re-deny shape and exercise the underlying fs primitives against a sandbox tmpdir tree containing a symlinked directory. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Two unrelated resource/auth concerns surfaced by the security audit sweep, both small enough to land together. ctx_fetch_and_index spawns a subprocess that buffers the entire HTTP response into resp.text(), writes it to a tmpfile, and the parent reads the tmpfile back into the long-running MCP server's heap via readFileSync. There is no size cap on either end, so a URL pointing at a server that streams gigabytes (or a slow-loris that never closes) either OOMs the subprocess or, worse, propagates a multi-GB response into parent heap and crashes the MCP server. The threat model is a prompt-injected URL in an agentic loop. Fix it by adding a 50 MB cap on both ends: a `safeText()` helper in the embedded fetch script that checks Content-Length up front and post-body size after resp.text() resolves, and a statSync gate in the parent before readFileSync so a torn write (subprocess killed mid-write, fs cache desync) can't sneak an oversized file past the subprocess cap. 50 MB is far above typical web page / API response sizes (~1-5 MB) but bounded enough to keep parent heap survivable. CLAUDE_SESSION_ID flows from the hosting process (Claude Code, pi, etc.) straight into a `path.join(statsDir, \`stats-\${sessionId}.json\`)`, and path.join collapses ".." segments. CLAUDE_SESSION_ID= "../../evil" writes "stats-evil.json" two levels above statsDir. The env var isn't under direct MCP-tool-caller control, but in CI or multi-tenant contexts where the host env is partly influenceable this is an arbitrary-write primitive within the MCP server process's filesystem permissions. Reject any session id whose characters aren't [A-Za-z0-9._-] and fall back to the pid-based id the function already uses when CLAUDE_SESSION_ID is unset. The charset covers all UUID variants the Claude Code adapter contract documents while excluding every path-separator and traversal character. Tests in tests/core/server.test.ts pin both fixes against the source and exercise the sanitizer algorithm against traversal shapes plus legitimate UUID-ish ids. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… the repo root Two destructive-test patterns surfaced by the security-audit sweep, both matching the shape PR mksglu#702 (test-home-isolation) fixed in tests/security.test.ts. Neither was exploitable, but each clobbered contributor state when the tests ran on a machine where the test layout collided with real project files. tests/lifecycle.test.ts wrote `_lifecycle_test_${exitCode}.ts` to `process.cwd()` (the project root) for every spawnGuardChild call, cleaned up only on the child's `close` event, and was not .gitignored. A hung child, a hard crash, or `vitest run` getting SIGKILL'd mid-test would orphan the temp scripts in the repo. Fix it by writing each script to a mkdtempSync directory under tmpdir(), pointing the script's lifecycle.ts import at the absolute file:// URL so the spawned ESM module still resolves correctly regardless of where the script lives, and rmSync'ing the dir on child close. tests/adapters/cursor.test.ts had two describe blocks that mkdirSync'd and writeFileSync'd `.cursor/mcp.json` against `process.cwd()`, and the afterEach rmSync(force:true) of that path would silently destroy the contributor's real `.cursor/mcp.json` if they had one. Fix it by chdir'ing into the per-test tempDir for both the write and the adapter's checkPluginRegistration call (the adapter reads .cursor/mcp.json relative to cwd), then restoring cwd before tempDir cleanup. The chdir is scoped narrow enough to not break the surrounding "validates native project hooks" test that legitimately needs the real cwd to find Claude compatibility state. Mirrors the env-override pattern PR mksglu#702 used for HOME in the security suite; just propagates the same hygiene to two more suites that missed it. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…them out of window Three tests in tests/core/cli.test.ts read src/cli.ts and slice a fixed-width window starting at "async function upgrade" or "function statuslineForward", then run regex assertions inside the window. The fixed width (16000 / 2000 chars) was sized close enough to the function body that any code addition near the start of the upgrade region pushed downstream assertions out of the slice. The "contain installed_plugins.json installPath under cache root" and "contain /ctx-upgrade swap loop to pluginRoot" security fixes added ~30 lines of containment comments + checks early in upgrade(), and the sweep-drift / statusline-exit / installPath-conjunction tests all dropped over the cliff. Fix it by slicing through end-of-file instead of a fixed width; the tests still anchor their assertions on textual landmarks ("sweepStaleMcpJson", "process.exit(0)"), so a wider slice doesn't weaken anything. Also broaden the "upgrade only syncs when installPath differs from pluginRoot" regex to accept the refactored early-continue form (`if (installPath === pluginRoot) continue;`) alongside the original conjunction form. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…tainment The lexical resolve+startsWith guards in c0088e4 (swap loop) and 1e13b37 (installPath sites) are not enough on their own. path.resolve is a pure-string normalizer; it doesn't dereference symlinks. A same-uid actor who can write into ~/.claude/plugins/cache/<owner>/<plugin>/<version> can plant a symlink AT that path pointing at an attacker-controlled directory. The lexical guard passes since the symlink itself lives under cacheRoot, but cpSync follows the link at FS-write time and the statuslineForward dynamic-import target points at attacker .mjs. The statusline forwarder re-fires several times per second while Claude Code is open, so a single planted symlink yields durable RCE under the user's UID. Fix it on three fronts: 1. /ctx-upgrade swap loop in cli.ts and server.ts inline-fallback: reject any symlink encountered in the cloned source. cpSync's default behavior preserves source symlinks as destination symlinks; a committed-symlink supply-chain variant could plant one inside pluginRoot/src that subsequent loads dereference out to /etc, ~/.ssh, etc. Use cpSync's `filter` callback with an lstat-based predicate so neither the loop's root item nor any subtree entry is allowed through if it's a symlink. 2. upgrade()'s installed_plugins.json skills sync: realpathSync the cacheRoot once and the per-entry installPath, then re-apply the startsWith gate against the canonical path. A symlink anchor at <cacheRoot>/<owner>/<plugin>/<version> -> attacker dir resolves to the attacker dir, which fails the gate. Legitimate installPath values (regular directories) resolve to themselves and pass. 3. statuslineForward() candidate selection: same realpath re-check on every registry-derived installPath before pushing bin/statusline.mjs into the candidate list. The post-realpath path is what's then dynamic-imported, so a planted symlink in the registry shape can't sneak the attacker target into the import hot loop. Tests in tests/core/cli.test.ts pin the new realpath / lstat shapes against the source and exercise the algorithm against a sandbox tmpdir tree where the planted symlink anchor's resolved target lives outside cacheRoot. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Module load shells out to `which tsx`, which doesn't resolve on Windows without Git Bash on PATH. Prefer the local devDep at node_modules/.bin/tsx[.cmd] and fall back to `where`/`which` only when the local bin isn't there. Co-authored-by: NgoQuocViet2001 <ngoquocviet2001@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

kerneltoast · 2026-05-26T18:27:22Z

@kerneltoast I checked this on Windows locally and found a few test-only issues, not a core security-code regression.

@NgoQuocViet2001 Added it as f106f56 with you as co-author, thanks!

kerneltoast and others added 10 commits May 26, 2026 11:19

kerneltoast force-pushed the security/codebase-audit-2026-05-25 branch from 296e763 to f106f56 Compare May 26, 2026 18:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

security: containment, info-disclosure, and test-isolation hardening#716

security: containment, info-disclosure, and test-isolation hardening#716
kerneltoast wants to merge 10 commits into
mksglu:nextfrom
kerneltoast:security/codebase-audit-2026-05-25

kerneltoast commented May 26, 2026

Uh oh!

NgoQuocViet2001 commented May 26, 2026

Uh oh!

kerneltoast commented May 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

kerneltoast commented May 26, 2026

Summary

Containment guards (the big block of work)

Info-disclosure / DoS

Destructive-test hygiene (matches PR #702 pattern)

Test-only follow-up

Out-of-scope (deferred to follow-up if you want them)

Test plan

Uh oh!

NgoQuocViet2001 commented May 26, 2026

Uh oh!

kerneltoast commented May 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants