diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000000..a23f313eff --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,7 @@ +## 2026-05-25 - Harden sanitizeRelativePath + +**Context**: Identified a directory traversal vulnerability in `sanitizeRelativePath` (`packages/cli-kit/src/public/node/path.ts`) where absolute paths and Windows drive letters were not stripped, potentially allowing escapes when joined. + +**Action**: Hardened `sanitizeRelativePath` to strip empty segments (leading/multiple slashes) and Windows drive letters. Updated JSDoc and the warning message to reflect these changes. + +**Learning**: When sanitizing paths intended to be relative, always strip leading slashes and platform-specific root markers (like drive letters) to prevent absolute path escapes. diff --git a/packages/cli-kit/src/public/node/path.test.ts b/packages/cli-kit/src/public/node/path.test.ts index 4972b3ac27..767ac47979 100644 --- a/packages/cli-kit/src/public/node/path.test.ts +++ b/packages/cli-kit/src/public/node/path.test.ts @@ -1,5 +1,5 @@ -import {relativizePath, normalizePath, cwd, sniffForPath, commonParentDirectory} from './path.js' -import {describe, test, expect} from 'vitest' +import {relativizePath, normalizePath, cwd, sniffForPath, commonParentDirectory, sanitizeRelativePath} from './path.js' +import {describe, test, expect, vi} from 'vitest' describe('relativize', () => { test('relativizes the path', () => { @@ -93,3 +93,46 @@ describe('sniffForPath', () => { expect(path).toStrictEqual('/path/to/project') }) }) + +describe('sanitizeRelativePath', () => { + test('strips traversal segments', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('../../etc/passwd', warn)).toBe('etc/passwd') + expect(warn).toHaveBeenCalledWith(expect.stringContaining('contains traversal or absolute segments')) + }) + + test('strips leading slashes (absolute Unix paths)', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('/etc/passwd', warn)).toBe('etc/passwd') + }) + + test('strips multiple leading slashes', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('//etc/passwd', warn)).toBe('etc/passwd') + }) + + test('strips Windows drive letters', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('C:/Windows/System32', warn)).toBe('Windows/System32') + }) + + test('handles Windows backslashes', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('..\\..\\etc\\passwd', warn)).toBe('etc/passwd') + }) + + test('collapses internal current directory markers', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('foo/./bar', warn)).toBe('foo/bar') + }) + + test('returns empty string for empty result', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('../..', warn)).toBe('') + }) + + test('returns empty string for single slash', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('/', warn)).toBe('') + }) +}) diff --git a/packages/cli-kit/src/public/node/path.ts b/packages/cli-kit/src/public/node/path.ts index f372178011..93349a5995 100644 --- a/packages/cli-kit/src/public/node/path.ts +++ b/packages/cli-kit/src/public/node/path.ts @@ -224,13 +224,20 @@ export function sanitizeRelativePath(input: string, warn: (msg: string) => void) if (seg === '..') { stripped = true stack.pop() - } else if (seg !== '.') { + } else if (seg === '.' || seg === '') { + // Skip empty segments (leading/multiple slashes) and current directory markers. + // This prevents absolute paths like '/etc/passwd' from being treated as such. + if (seg === '') stripped = true + } else if (/^[a-zA-Z]:$/.test(seg)) { + // Skip Windows drive letters (e.g. 'C:') to prevent escaping to other drives. + stripped = true + } else { stack.push(seg) } } const result = stack.join('/') if (stripped) { - warn(`Warning: path '${input}' contains '..' traversal — sanitized to '${result || '.'}'\n`) + warn(`Warning: path '${input}' contains traversal or absolute segments — sanitized to '${result || '.'}'\n`) } return result }