Skip to main content
Kodelyth ECC
security

Enumerate, Don't Construct: The Path Traversal We Shipped

Five loaders resolved a name to a file. Four enumerated a directory and matched; one joined the name onto a path. Only one of them had a path traversal, and it was reachable by prompt injection.

· 6 min read

Two ways to resolve a name to a file. Enumerating a directory can only return a listed file. Joining the name onto a path lets ../.. escape and return 51,297 bytes of the wrong file.

Our MCP server exposes five read tools: get_agent, get_skill, get_command, get_bundle, get_rule. Same shape, same contract — take a name, return that item's markdown.

Four of them reject ../../README. One returned 51,297 bytes of the project's README.

The difference is one line

The four safe ones all look like this, give or take:

function loadAgents() {
  for (const file of listMarkdownFiles(PATHS.agents)) { /* … */ }
  // later: list.find(a => a.name === name)
}

They enumerate a directory, build a list, then match the requested name against that list. A caller can only ever select something already in the list. ../../README is not in the list, so it is not found. There is nothing to get right — the structure does the work.

The fifth did this:

function loadRule(name) {
  const file = path.join(PATHS.rules, `${name}.md`);
  const raw = safeReadFile(file);
  // …
}

It constructs a path from the argument. rules/common/../../README.md resolves to ROOT/README.md, and readFileSync is perfectly happy to read it.

That is the entire bug. Not a missing check — a different shape.

What it could reach

The .md suffix bounds it, which sounds reassuring and is not. /etc/passwd is safe because /etc/passwd.md does not exist. But every markdown file on the machine is in scope, with arbitrary traversal depth:

get_rule("../../README")             → 51,297 bytes
get_rule("../../CHANGELOG")          → 188,170 bytes
get_rule("testing/../../../README")  → 51,297 bytes

Nested traversal works too, which rules out the "just block a leading .." fix.

Why this is not a local-only problem

The instinct is: it's an MCP server running on my machine, under my user, reading files I can already read. Who cares?

The caller is not always the user.

An MCP tool argument can originate in text the agent merely read — a dependency's README, an issue comment, a page it fetched. That is the standard indirect prompt-injection shape. So this turns a read-scoped tool into an arbitrary-markdown-read primitive that untrusted text can aim.

Which is exactly the class our own prompt-injection-hunter agent exists to catch. The call was coming from inside the house.

The fix, in two layers

const RULE_NAME = /^[A-Za-z0-9][A-Za-z0-9._-]*$/;
 
function loadRule(name) {
  if (typeof name !== 'string' || !RULE_NAME.test(name)) return null;
  const file = resolveContained(path.join(PATHS.rules, `${name}.md`), PATHS.rules);
  if (file === null) return null;
  // …
}

Two guards, because they fail differently.

The pattern rejects anything that is a path rather than a name, before touching the disk. Rule names are flat basenames — the directory has no subdirectories — so this costs nothing.

resolveContained then proves the target really sits under rules/common once symlinks are resolved. The pattern cannot know that: escape-probe is a perfectly valid name, and a symlink under that name reads 51KB. We tested exactly that — planted the symlink, confirmed fs.readFileSync would follow it, confirmed loadRule refuses.

The part that actually let it through

loadRule had no test at all. Not a weak test — none.

The other four loaders were covered. This one looked trivially correct, so nobody wrote one, and the shape that made it different from its four siblings was never examined. It now has four tests: the exact payloads that leaked, non-string input (a crash there would kill the stdio server rather than return one tool error), a round-trip over all 15 real rules, and the symlink case.

The rule worth keeping

When resolving a caller-supplied name to a file, enumerate and match. Only construct a path when you cannot, and then prove containment.

Enumeration is safe by structure. Construction is safe by vigilance, and vigilance does not survive six months and three contributors.

If you have a loader that takes a name, go look at it right now. If it calls path.join with that name, you already know what to test.

Fixed in ECC 2.24.2. The containment helper is in scripts/lib/safe-fs.js — it realpaths both sides, so a symlink is judged by where it points rather than where it sits.