Skip to content

fix: reject vault-escaping paths in VaultWriter - #13

Open
arimu1 wants to merge 1 commit into
obra:mainfrom
arimu1:fix/5-vault-path-traversal
Open

fix: reject vault-escaping paths in VaultWriter#13
arimu1 wants to merge 1 commit into
obra:mainfrom
arimu1:fix/5-vault-path-traversal

Conversation

@arimu1

@arimu1 arimu1 commented Jul 19, 2026

Copy link
Copy Markdown

Fixes #5

Root cause

VaultWriter.createNode, annotateNode, and addLink in src/lib/writer.ts build absolute paths with path.join(this.vaultPath, ...) from user-supplied directory/title (createNode), nodeId (annotateNode), and sourceId (addLink) with no check that the result stays inside vaultPath. path.join normalizes .. segments but does not stop them from resolving outside the vault, so a crafted MCP tool call (e.g. directory: "../../etc" or nodeId: "../../etc/passwd") could create or append to files outside the intended vault directory.

Fix

Added a private resolveInVault(relPath) helper that resolves the path with path.resolve, computes path.relative(vaultPath, absPath), and throws Path escape attempt: <path> if the relative path starts with .. or is itself absolute. Every read/write entry point in VaultWriter (createNode, annotateNode, addLink, and the internal indexFile) now routes through this helper instead of raw path.join. Normal in-vault paths are unaffected.

Testing

Added traversal-rejection tests to test/writer.test.ts:

  • createNode rejects a directory of ../../etc
  • createNode rejects a title of ../../../tmp/evil
  • annotateNode rejects a nodeId of ../../etc/passwd
  • addLink rejects a sourceId of ../../etc/passwd

All existing tests continue to pass with normal (non-escaping) paths.

$ npx vitest run test/writer.test.ts
 Test Files  1 passed (1)
      Tests  14 passed (14)

$ npx vitest run
 Test Files  12 passed (12)
      Tests  91 passed (91)

(Full-repo npm run build has pre-existing, unrelated TypeScript errors in src/lib/graph.ts, src/lib/embedder.ts, and src/mcp/index.ts on unmodified main — verified by stashing this change and re-running tsc. writer.ts itself introduces no new type errors.)


Drafted with Claude Code (model: Claude Fable 5) and human-reviewed before submission. I verified the vulnerable code paths, wrote/ran the tests above, and confirmed the pre-existing build errors are unrelated to this change.

createNode, annotateNode, and addLink joined user-supplied directory,
nodeId, and sourceId values onto vaultPath with plain path.join(),
which does not stop `../` segments or absolute overrides from
resolving outside the vault. Add resolveInVault(), which resolves the
path and rejects it if path.relative(vaultPath, absPath) escapes
(starts with '..' or is itself absolute), and route every read/write
path in VaultWriter through it.

Fixes obra#5

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Path traversal vulnerability in VaultWriter - file paths not validated against vault boundary

1 participant