From df070600bfb91d65c4f1faba771082f4332d30e1 Mon Sep 17 00:00:00 2001
From: arimu1 <af.nawfal@gmail.com>
Date: Sun, 19 Jul 2026 13:50:27 +0700
Subject: [PATCH] fix: reject vault-escaping paths in VaultWriter

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 #5

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 src/lib/writer.ts   | 33 +++++++++++++++++++++++----------
 test/writer.test.ts | 28 ++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/src/lib/writer.ts b/src/lib/writer.ts
index 481cedd..d682e6e 100644
--- a/src/lib/writer.ts
+++ b/src/lib/writer.ts
@@ -1,5 +1,5 @@
 import { existsSync, mkdirSync, readFileSync, writeFileSync, appendFileSync } from 'fs';
-import { join, basename } from 'path';
+import { basename, dirname, isAbsolute, relative, resolve } from 'path';
 import matter from 'gray-matter';
 import type { Store } from './store.js';
 
@@ -16,15 +16,28 @@ export class VaultWriter {
     private store: Store,
   ) {}
 
-  createNode(opts: CreateNodeOptions): string {
-    const dir = opts.directory
-      ? join(this.vaultPath, opts.directory)
-      : this.vaultPath;
-    mkdirSync(dir, { recursive: true });
+  /**
+   * Resolves a vault-relative path to an absolute path, guaranteeing the
+   * result stays within vaultPath. Rejects `../` segments (and absolute
+   * path overrides) that would otherwise let a caller escape the vault.
+   */
+  private resolveInVault(relPath: string): string {
+    const absPath = resolve(this.vaultPath, relPath);
+    const rel = relative(this.vaultPath, absPath);
+
+    if (rel.startsWith('..') || isAbsolute(rel)) {
+      throw new Error(`Path escape attempt: ${relPath}`);
+    }
+
+    return absPath;
+  }
 
+  createNode(opts: CreateNodeOptions): string {
     const filename = `${opts.title}.md`;
     const relPath = opts.directory ? `${opts.directory}/${filename}` : filename;
-    const absPath = join(dir, filename);
+    const absPath = this.resolveInVault(relPath);
+
+    mkdirSync(dirname(absPath), { recursive: true });
 
     if (existsSync(absPath)) {
       throw new Error(`File already exists: ${relPath}`);
@@ -41,7 +54,7 @@ export class VaultWriter {
   }
 
   annotateNode(nodeId: string, content: string): void {
-    const absPath = join(this.vaultPath, nodeId);
+    const absPath = this.resolveInVault(nodeId);
     if (!existsSync(absPath)) {
       throw new Error(`Node not found: ${nodeId}`);
     }
@@ -53,7 +66,7 @@ export class VaultWriter {
   }
 
   addLink(sourceId: string, targetRef: string, context: string): void {
-    const absPath = join(this.vaultPath, sourceId);
+    const absPath = this.resolveInVault(sourceId);
     if (!existsSync(absPath)) {
       throw new Error(`Source node not found: ${sourceId}`);
     }
@@ -74,7 +87,7 @@ export class VaultWriter {
   }
 
   private indexFile(relPath: string): void {
-    const absPath = join(this.vaultPath, relPath);
+    const absPath = this.resolveInVault(relPath);
     const raw = readFileSync(absPath, 'utf-8');
 
     let fm: Record<string, unknown>;
diff --git a/test/writer.test.ts b/test/writer.test.ts
index 164a068..6eb45a9 100644
--- a/test/writer.test.ts
+++ b/test/writer.test.ts
@@ -86,6 +86,25 @@ describe('VaultWriter', () => {
         content: 'Duplicate.',
       })).toThrow(/already exists/);
     });
+
+    it('rejects a directory that escapes the vault via ../', () => {
+      expect(() => writer.createNode({
+        title: 'Evil',
+        directory: '../../etc',
+        frontmatter: {},
+        content: 'pwned',
+      })).toThrow(/Path escape attempt/);
+
+      expect(existsSync(join(tempVault, '..', '..', 'etc', 'Evil.md'))).toBe(false);
+    });
+
+    it('rejects a title that escapes the vault via ../', () => {
+      expect(() => writer.createNode({
+        title: '../../../tmp/evil',
+        frontmatter: {},
+        content: 'pwned',
+      })).toThrow(/Path escape attempt/);
+    });
   });
 
   describe('annotateNode', () => {
@@ -115,6 +134,10 @@ describe('VaultWriter', () => {
     it('throws if the node does not exist', () => {
       expect(() => writer.annotateNode('nonexistent.md', 'stuff')).toThrow(/not found/);
     });
+
+    it('rejects a nodeId that escapes the vault via ../', () => {
+      expect(() => writer.annotateNode('../../etc/passwd', 'pwned')).toThrow(/Path escape attempt/);
+    });
   });
 
   describe('addLink', () => {
@@ -144,5 +167,10 @@ describe('VaultWriter', () => {
       const edges = store.getEdgesFrom('Source.md');
       expect(edges.some(e => e.targetId === 'People/Alice Smith.md')).toBe(true);
     });
+
+    it('rejects a sourceId that escapes the vault via ../', () => {
+      expect(() => writer.addLink('../../etc/passwd', 'People/Alice Smith', 'pwned'))
+        .toThrow(/Path escape attempt/);
+    });
   });
 });