Skip to main content
Kodelyth ECC
security

A Shared File Format Is Untrusted Input

Session bundles exist to be passed around. Two JSON fields in one built filesystem paths — one reached a recursive delete, the other planted an always-on rule file. The CLI printed success for both.

· 6 min read

A bundle.json with two attacker-controlled fields. One reaches fs.rmSync with recursive and force, the other writes a file outside the target directory.

A session bundle is a JSON file that captures a multi-agent run so it can be replayed somewhere else. Portability is the entire point. You export one, send it to a colleague, they import it.

Which means every string in it comes from whoever built it. We validated two of them like this:

if (typeof raw.session !== 'string' || !raw.session) throw new Error('missing "session"');
for (const w of raw.workers) {
  if (!w || typeof w.slug !== 'string') throw new Error('each worker needs a string "slug"');
}

Both are type checks. Neither is a containment check. And both values build filesystem paths:

targetDir = path.join(coordRoot, bundle.session)   // → fs.rmSync(recursive, force)
wdir      = path.join(targetDir, w.slug)           // → mkdirSync + 3× writeFileSync

What that gets you

A slug of ../../.claude/rules/common/injected writes task.md, handoff.md and status.md — with contents the bundle author chose — three levels outside the import target. Aimed at a real install, that directory is loaded as always-on rules, so a shared bundle plants standing instructions into every future session.

The CLI reported:

✓ imported bundle into: /…/.orchestration/looks-normal
  workers: ../../.claude/rules/common/injected
  inspect: cat /…/.claude/rules/common/injected/handoff.md

It printed success, then printed the escaped path in its own hint.

A session of ../important-work with --overwrite sent that path straight to fs.rmSync(dir, { recursive: true, force: true }). In a sandbox, an unrelated project directory — README.md, src/app.js — was gone, replaced by the bundle's own output. It printed success for that too.

Both were confirmed by running the real CLI against a victim tree, not by reading the code. That matters: reading tells you a check is missing, running tells you what the missing check costs.

The fix is smaller than the bug

What does a legitimate value look like? We went and checked rather than guessed.

On export, session is a path.basename and each slug is a readdirSync entry name, which the project's slugify constrains to [a-z0-9-]+. Both are always single path segments. Nothing legitimate has ever contained a separator.

const SAFE_SEGMENT = /^[A-Za-z0-9][A-Za-z0-9._-]*$/;

That rejects nothing a real bundle contains — confirmed by a full export/import round-trip producing identical trees — and rejects every payload above.

Two things we got wrong along the way

We added a containment check at the write site too, which is good practice. Then we wrote a test asserting it refuses a symlinked slug, and the test failed — because that state is unreachable. importBundle always starts from a directory it just created: an existing target is either refused or rmSync'd first. No planted symlink can survive into the loop.

We deleted the test rather than reword it. A test that cannot distinguish a fixed build from a broken one is worse than no test, because it reads like coverage. The containment check stayed, with a comment saying plainly that it guards no currently reachable vector and exists for a future caller that passes in a directory it did not create.

And the success message was its own bug. Printing ✓ imported bundle into: <target> while writing outside that target is a lie the user has no way to catch. If your tool reports a destination, the report should come from the path it actually wrote, not the one it intended to.

The general shape

This is the same bug as zip slip, and it shows up anywhere a file format carries names that become paths: archives, lockfiles, manifests, plugin descriptors, exported sessions.

The tell is a format whose purpose is to be shared. That purpose is exactly what makes every field in it untrusted, and it is also what makes the fields look trustworthy — they came from your own exporter, after all. Until they didn't.

If a format exists to be passed between machines, treat every string in it as hostile, and check containment at the point where a string becomes a path.

Fixed in ECC 2.24.5. The bundle reader and its tests are in scripts/replay.