From 4686606c66f5d74f4842cf8cdaa5580fc3885b93 Mon Sep 17 00:00:00 2001
From: Robby <robbyczgw@gmail.com>
Date: Wed, 28 Jan 2026 10:44:01 +0000
Subject: [PATCH] security(core): fix path validation bypasses in archive,
 transcript, and memory

- Replace vulnerable startsWith() checks with path.relative() validation
- Add isPathWithinBase() helper for consistent path validation
- Add path filter to tar extraction to prevent directory escape
- Validate sessionFile parameter in transcript resolution
- Add non-root 'sandbox' user to Docker sandbox (UID 1000)

Security impact:
- Prevents path traversal in archive extraction (zip and tar)
- Prevents arbitrary file write via transcript path injection
- Prevents workspace escape in memory manager
- Reduces privilege in Docker sandbox

Fixes #3277
[AI-assisted]
---
 Dockerfile.sandbox                 |  4 ++++
 src/gateway/server-methods/chat.ts | 16 +++++++++++++++-
 src/infra/archive.ts               | 18 +++++++++++++++---
 src/memory/manager.ts              |  3 ++-
 4 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/Dockerfile.sandbox b/Dockerfile.sandbox
index dec3f32d1..7a50d69a9 100644
--- a/Dockerfile.sandbox
+++ b/Dockerfile.sandbox
@@ -13,4 +13,8 @@ RUN apt-get update \
     ripgrep \
   && rm -rf /var/lib/apt/lists/*
 
+RUN useradd -m -s /bin/bash -u 1000 sandbox
+USER sandbox
+WORKDIR /home/sandbox
+
 CMD ["sleep", "infinity"]
diff --git a/src/gateway/server-methods/chat.ts b/src/gateway/server-methods/chat.ts
index 9010a6f21..2c9a6f528 100644
--- a/src/gateway/server-methods/chat.ts
+++ b/src/gateway/server-methods/chat.ts
@@ -56,7 +56,21 @@ function resolveTranscriptPath(params: {
   sessionFile?: string;
 }): string | null {
   const { sessionId, storePath, sessionFile } = params;
-  if (sessionFile) return sessionFile;
+  if (sessionFile) {
+    if (storePath) {
+      const storeDir = path.dirname(storePath);
+      const absSessionFile = path.resolve(storeDir, sessionFile);
+      const rel = path.relative(storeDir, absSessionFile);
+      if (rel.startsWith("..") || path.isAbsolute(rel)) {
+        throw new Error("sessionFile escapes store directory");
+      }
+      return absSessionFile;
+    }
+    if (!path.isAbsolute(sessionFile)) {
+      throw new Error("sessionFile must be absolute when storePath is not set");
+    }
+    return sessionFile;
+  }
   if (!storePath) return null;
   return path.join(path.dirname(storePath), `${sessionId}.jsonl`);
 }
diff --git a/src/infra/archive.ts b/src/infra/archive.ts
index 35ad4fa04..da3fdd1e0 100644
--- a/src/infra/archive.ts
+++ b/src/infra/archive.ts
@@ -61,6 +61,11 @@ export async function withTimeout<T>(
   }
 }
 
+function isPathWithinBase(basePath: string, targetPath: string): boolean {
+  const rel = path.relative(basePath, targetPath);
+  return !rel.startsWith("..") && !path.isAbsolute(rel);
+}
+
 async function extractZip(params: { archivePath: string; destDir: string }): Promise<void> {
   const buffer = await fs.readFile(params.archivePath);
   const zip = await JSZip.loadAsync(buffer);
@@ -70,7 +75,7 @@ async function extractZip(params: { archivePath: string; destDir: string }): Pro
     const entryPath = entry.name.replaceAll("\\", "/");
     if (!entryPath || entryPath.endsWith("/")) {
       const dirPath = path.resolve(params.destDir, entryPath);
-      if (!dirPath.startsWith(params.destDir)) {
+      if (!isPathWithinBase(params.destDir, dirPath)) {
         throw new Error(`zip entry escapes destination: ${entry.name}`);
       }
       await fs.mkdir(dirPath, { recursive: true });
@@ -78,7 +83,7 @@ async function extractZip(params: { archivePath: string; destDir: string }): Pro
     }
 
     const outPath = path.resolve(params.destDir, entryPath);
-    if (!outPath.startsWith(params.destDir)) {
+    if (!isPathWithinBase(params.destDir, outPath)) {
       throw new Error(`zip entry escapes destination: ${entry.name}`);
     }
     await fs.mkdir(path.dirname(outPath), { recursive: true });
@@ -101,7 +106,14 @@ export async function extractArchive(params: {
   const label = kind === "zip" ? "extract zip" : "extract tar";
   if (kind === "tar") {
     await withTimeout(
-      tar.x({ file: params.archivePath, cwd: params.destDir }),
+      tar.x({
+        file: params.archivePath,
+        cwd: params.destDir,
+        filter: (entryPath) => {
+          const absPath = path.resolve(params.destDir, entryPath);
+          return isPathWithinBase(params.destDir, absPath);
+        },
+      }),
       params.timeoutMs,
       label,
     );
diff --git a/src/memory/manager.ts b/src/memory/manager.ts
index 9a9991d10..fa8493fb5 100644
--- a/src/memory/manager.ts
+++ b/src/memory/manager.ts
@@ -401,7 +401,8 @@ export class MemoryIndexManager {
       throw new Error("path required");
     }
     const absPath = path.resolve(this.workspaceDir, relPath);
-    if (!absPath.startsWith(this.workspaceDir)) {
+    const rel = path.relative(this.workspaceDir, absPath);
+    if (rel.startsWith("..") || path.isAbsolute(rel)) {
       throw new Error("path escapes workspace");
     }
     const content = await fs.readFile(absPath, "utf-8");