fix: prevent path traversal in FileArtifactService (CWE-22) (#210)

* fix: move test to core/tests, import helpers from service

* chore: revert package.json changes

* chore: revert package-lock.json changes

---------

Co-authored-by: g0w6y <g0w6y@users.noreply.github.com>
Co-authored-by: Alexey Kalenkevich <kalenkevich@google.com>

Author: g0w6y

core/src/artifacts/file_artifact_service.ts

@@ -393,8 +393,35 @@ export class FileArtifactService implements BaseArtifactService {
   }
 }
 
-function getUserRoot(rootDir: string, userId: string): string {
-  return path.join(rootDir, 'users', userId);
+const SAFE_SEGMENT_RE = /^[a-zA-Z0-9_@-][a-zA-Z0-9_.@-]{0,255}$/;
+
+export function assertSafeSegment(value: string, label: string): void {
+  if (!value || !SAFE_SEGMENT_RE.test(value)) {
+    throw new Error(
+      `[FileArtifactService] Invalid ${label}: value contains disallowed characters.`,
+    );
+  }
+}
+
+export function assertInsideRoot(
+  resolvedPath: string,
+  rootDir: string,
+  label: string,
+): void {
+  const root = path.resolve(rootDir);
+  const resolved = path.resolve(resolvedPath);
+  if (!resolved.startsWith(root + path.sep) && resolved !== root) {
+    throw new Error(
+      `[FileArtifactService] ${label} escapes storage root. Resolved: ${resolved}, Root: ${root}`,
+    );
+  }
+}
+
+export function getUserRoot(rootDir: string, userId: string): string {
+  assertSafeSegment(userId, 'userId');
+  const result = path.join(rootDir, 'users', userId);
+  assertInsideRoot(result, rootDir, 'userRoot');
+  return result;
 }
 
 function isUserScoped(
@@ -408,8 +435,14 @@ function getUserArtifactsDir(userRoot: string): string {
   return path.join(userRoot, 'artifacts');
 }
 
-function getSessionArtifactsDir(baseRoot: string, sessionId: string): string {
-  return path.join(baseRoot, 'sessions', sessionId, 'artifacts');
+export function getSessionArtifactsDir(
+  baseRoot: string,
+  sessionId: string,
+): string {
+  assertSafeSegment(sessionId, 'sessionId');
+  const result = path.join(baseRoot, 'sessions', sessionId, 'artifacts');
+  assertInsideRoot(result, baseRoot, 'sessionArtifactsDir');
+  return result;
 }
 
 function getVersionsDir(artifactDir: string): string {

core/tests/file_artifact_service_path_traversal_test.ts

@@ -0,0 +1,90 @@
+/**
+ * @license
+ * Copyright 2026 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {describe, expect, it} from 'vitest';
+import {
+  assertInsideRoot,
+  getSessionArtifactsDir,
+  getUserRoot,
+} from '../src/artifacts/file_artifact_service.js';
+
+const ROOT = '/tmp/adk-test-root';
+
+describe('assertSafeSegment - valid inputs', () => {
+  it('allows a plain alphanumeric userId', () => {
+    expect(() => getUserRoot(ROOT, 'alice')).not.toThrow();
+  });
+  it('allows a UUID as userId', () => {
+    expect(() =>
+      getUserRoot(ROOT, '550e8400-e29b-41d4-a716-446655440000'),
+    ).not.toThrow();
+  });
+  it('allows an email-style userId', () => {
+    expect(() => getUserRoot(ROOT, 'user.name@org')).not.toThrow();
+  });
+  it('allows a plain alphanumeric sessionId', () => {
+    expect(() =>
+      getSessionArtifactsDir(`${ROOT}/users/alice`, 'session-abc123'),
+    ).not.toThrow();
+  });
+});
+
+describe('assertSafeSegment - userId attacks', () => {
+  it('blocks dot-dot-slash traversal in userId', () => {
+    expect(() => getUserRoot(ROOT, '../../etc')).toThrow('Invalid userId');
+  });
+  it('blocks forward slash in userId', () => {
+    expect(() => getUserRoot(ROOT, 'a/b')).toThrow('Invalid userId');
+  });
+  it('blocks percent-encoded slash in userId', () => {
+    expect(() => getUserRoot(ROOT, '..%2F..%2Fetc')).toThrow('Invalid userId');
+  });
+  it('blocks null byte in userId', () => {
+    expect(() => getUserRoot(ROOT, 'alice\x00')).toThrow('Invalid userId');
+  });
+  it('blocks empty string as userId', () => {
+    expect(() => getUserRoot(ROOT, '')).toThrow('Invalid userId');
+  });
+});
+
+describe('assertSafeSegment - sessionId attacks', () => {
+  const base = `${ROOT}/users/alice`;
+  it('blocks dot-dot-slash traversal in sessionId', () => {
+    expect(() => getSessionArtifactsDir(base, '../../../secret')).toThrow(
+      'Invalid sessionId',
+    );
+  });
+  it('blocks forward slash in sessionId', () => {
+    expect(() => getSessionArtifactsDir(base, 'sess/../../etc')).toThrow(
+      'Invalid sessionId',
+    );
+  });
+  it('blocks percent-encoded slash in sessionId', () => {
+    expect(() => getSessionArtifactsDir(base, '..%2F..%2F..%2Fsecret')).toThrow(
+      'Invalid sessionId',
+    );
+  });
+  it('blocks empty string as sessionId', () => {
+    expect(() => getSessionArtifactsDir(base, '')).toThrow('Invalid sessionId');
+  });
+});
+
+describe('assertInsideRoot - defence-in-depth', () => {
+  it('throws when resolved path escapes root', () => {
+    expect(() =>
+      assertInsideRoot('/tmp/root/../outside', '/tmp/root', 'test'),
+    ).toThrow('escapes storage root');
+  });
+  it('allows a path equal to root', () => {
+    expect(() =>
+      assertInsideRoot('/tmp/root', '/tmp/root', 'test'),
+    ).not.toThrow();
+  });
+  it('allows a path nested inside root', () => {
+    expect(() =>
+      assertInsideRoot('/tmp/root/users/alice', '/tmp/root', 'test'),
+    ).not.toThrow();
+  });
+});