Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/every-feet-attack.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@e2b/python-sdk': patch
'e2b': patch
---

fix: validate copy src paths are relative and within context directory
30 changes: 23 additions & 7 deletions packages/js-sdk/src/template/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
padOctal,
readDockerignore,
readGCPServiceAccountJSON,
validateRelativePath,
} from './utils'

/**
Expand Down Expand Up @@ -520,10 +521,16 @@ export class TemplateBase
}

const srcs = Array.isArray(src) ? src : [src]
const stackTrace = getCallerFrame(STACK_TRACE_DEPTH - 1)
Comment thread
cursor[bot] marked this conversation as resolved.

for (const src of srcs) {
const srcString = src.toString()

// Validate that the source path is a relative path within the context directory
validateRelativePath(srcString, stackTrace)

const args = [
src.toString(),
srcString,
dest.toString(),
options?.user ?? '',
options?.mode ? padOctal(options.mode) : '',
Expand All @@ -547,14 +554,23 @@ export class TemplateBase
throw new Error('Browser runtime is not supported for copyItems')
}

// Stack trace that will be used to re-throw the error with
const stackTrace = getCallerFrame(STACK_TRACE_DEPTH - 1)

this.runInNewStackTraceContext(() => {
for (const item of items) {
this.copy(item.src, item.dest, {
forceUpload: item.forceUpload,
user: item.user,
mode: item.mode,
resolveSymlinks: item.resolveSymlinks,
})
try {
this.copy(item.src, item.dest, {
forceUpload: item.forceUpload,
user: item.user,
mode: item.mode,
resolveSymlinks: item.resolveSymlinks,
})
} catch (error) {
const copyError = error as Error
copyError.stack = stackTrace
throw copyError
}
}
})

Expand Down
54 changes: 54 additions & 0 deletions packages/js-sdk/src/template/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,60 @@ import { BASE_STEP_NAME, FINALIZE_STEP_NAME } from './consts'
import type { Path } from 'glob'
import type { BuildOptions } from './types'

/**
* Validate that a source path for copy operations is a relative path that stays
* within the context directory. This prevents path traversal attacks and ensures
* files are copied from within the expected directory.
*
* @param src The source path to validate
* @param stackTrace Optional stack trace for error reporting
* @throws TemplateError if the path is absolute or escapes the context directory
*
* Invalid paths:
* - Absolute paths: /absolute/path, C:\Windows\path
* - Parent directory escapes: ../foo, foo/../../bar, ./foo/../../../bar
*
* Valid paths:
* - Simple relative: foo, foo/bar
* - Current directory prefix: ./foo, ./foo/bar
* - Internal parent refs that don't escape: foo/../bar (stays within context)
*/
export function validateRelativePath(
src: string,
stackTrace: string | undefined
): void {
// Check for absolute paths using Node's cross-platform implementation
if (path.isAbsolute(src)) {
const error = new TemplateError(
`Invalid source path "${src}": absolute paths are not allowed. Use a relative path within the context directory.`,
stackTrace
)
throw error
}
Comment thread
mishushakov marked this conversation as resolved.

// Normalize the path and check if it escapes the context directory
const normalized = path.normalize(src)

// After normalization, a path that escapes would be '..' or start with '../'
// We check for '..' followed by path separator to avoid false positives on filenames like '..myconfig'
// Examples:
// - '../foo' -> '../foo' (escapes)
// - 'foo/../../bar' -> '../bar' (escapes)
// - './foo/../../../bar' -> '../../bar' (escapes)
// - 'foo/../bar' -> 'bar' (doesn't escape)
// - './foo/bar' -> 'foo/bar' (doesn't escape)
// - '..myconfig' -> '..myconfig' (valid filename, doesn't escape)
const escapes = normalized === '..' || normalized.startsWith('..' + path.sep)

if (escapes) {
const error = new TemplateError(
`Invalid source path "${src}": path escapes the context directory. The path must stay within the context directory.`,
stackTrace
)
throw error
}
}

/**
* Normalize build arguments from different overload signatures.
* Handles string name or legacy options object with alias.
Expand Down
16 changes: 15 additions & 1 deletion packages/js-sdk/tests/template/stacktrace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { apiUrl, buildTemplateTest } from '../setup'
import { randomUUID } from 'node:crypto'

const __fileContent = fs.readFileSync(__filename, 'utf8') // read current file content
const nonExistentPath = '/nonexistent/path'
const nonExistentPath = 'nonexistent/path'

// map template alias -> failed step index
const failureMap: Record<string, number | undefined> = {
Expand Down Expand Up @@ -212,6 +212,20 @@ buildTemplateTest('traces on copyItems', async ({ buildTemplate }) => {
}, 'copyItems')
})

buildTemplateTest('traces on copy absolute path', async () => {
await expectToThrowAndCheckTrace(async () => {
Template().fromBaseImage().copy('/absolute/path', '/absolute/path')
}, 'copy')
})

buildTemplateTest('traces on copyItems absolute path', async () => {
await expectToThrowAndCheckTrace(async () => {
Template()
.fromBaseImage()
.copyItems([{ src: '/absolute/path', dest: '/absolute/path' }])
}, 'copyItems')
})

buildTemplateTest('traces on remove', async ({ buildTemplate }) => {
let template = Template().fromBaseImage()
template = template.skipCache().remove(nonExistentPath)
Expand Down
168 changes: 168 additions & 0 deletions packages/js-sdk/tests/template/utils/validateRelativePath.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
import { describe, expect, test } from 'vitest'
import { validateRelativePath } from '../../../src/template/utils'
import { TemplateError } from '../../../src/errors'

const isWindows = process.platform === 'win32'

describe('validateRelativePath', () => {
describe('valid paths', () => {
test('accepts simple relative path', () => {
expect(() => validateRelativePath('foo', undefined)).not.toThrow()
})

test('accepts nested relative path', () => {
expect(() => validateRelativePath('foo/bar', undefined)).not.toThrow()
})

test('accepts path with ./ prefix', () => {
expect(() => validateRelativePath('./foo', undefined)).not.toThrow()
})

test('accepts nested path with ./ prefix', () => {
expect(() => validateRelativePath('./foo/bar', undefined)).not.toThrow()
})

test('accepts path with internal parent ref that stays within context', () => {
expect(() => validateRelativePath('foo/../bar', undefined)).not.toThrow()
})

test('accepts current directory', () => {
expect(() => validateRelativePath('.', undefined)).not.toThrow()
})

test('accepts glob patterns', () => {
expect(() => validateRelativePath('*.txt', undefined)).not.toThrow()
expect(() => validateRelativePath('**/*.ts', undefined)).not.toThrow()
expect(() => validateRelativePath('src/**/*', undefined)).not.toThrow()
})

test('accepts hidden files and directories', () => {
expect(() => validateRelativePath('.hidden', undefined)).not.toThrow()
expect(() =>
validateRelativePath('.config/settings', undefined)
).not.toThrow()
})

test('accepts filenames starting with double dots', () => {
expect(() => validateRelativePath('..myconfig', undefined)).not.toThrow()
expect(() => validateRelativePath('..cache', undefined)).not.toThrow()
expect(() =>
validateRelativePath('...something', undefined)
).not.toThrow()
expect(() =>
validateRelativePath('foo/..myconfig', undefined)
).not.toThrow()
})
})

describe('invalid paths - absolute', () => {
test('rejects Unix absolute path', () => {
expect(() => validateRelativePath('/absolute/path', undefined)).toThrow(
TemplateError
)
expect(() => validateRelativePath('/absolute/path', undefined)).toThrow(
'absolute paths are not allowed'
)
})

test('rejects root path', () => {
expect(() => validateRelativePath('/', undefined)).toThrow(TemplateError)
})

// Windows path tests - only run on Windows where path.isAbsolute detects them
test.skipIf(!isWindows)('rejects Windows drive letter path', () => {
expect(() =>
validateRelativePath('C:\\Windows\\System32', undefined)
).toThrow(TemplateError)
expect(() =>
validateRelativePath('C:\\Windows\\System32', undefined)
).toThrow('absolute paths are not allowed')
})

test.skipIf(!isWindows)('rejects Windows UNC path', () => {
expect(() =>
validateRelativePath('\\\\server\\share', undefined)
).toThrow(TemplateError)
})
})

describe('invalid paths - parent directory escape', () => {
test('rejects simple parent directory escape', () => {
expect(() => validateRelativePath('../foo', undefined)).toThrow(
TemplateError
)
expect(() => validateRelativePath('../foo', undefined)).toThrow(
'path escapes the context directory'
)
})

test('rejects parent directory escape with forward slash', () => {
expect(() => validateRelativePath('../file.txt', undefined)).toThrow(
TemplateError
)
})

test.skipIf(!isWindows)(
'rejects parent directory escape with backslash',
() => {
expect(() => validateRelativePath('..\\file.txt', undefined)).toThrow(
TemplateError
)
}
)

test('rejects double parent directory escape', () => {
expect(() => validateRelativePath('../../foo', undefined)).toThrow(
TemplateError
)
})

test('rejects path that escapes via nested parent refs', () => {
expect(() => validateRelativePath('foo/../../bar', undefined)).toThrow(
TemplateError
)
})

test('rejects path with ./ prefix that escapes', () => {
expect(() =>
validateRelativePath('./foo/../../../bar', undefined)
).toThrow(TemplateError)
})

test('rejects just parent directory', () => {
expect(() => validateRelativePath('..', undefined)).toThrow(TemplateError)
})

test('rejects current directory followed by parent', () => {
expect(() => validateRelativePath('./..', undefined)).toThrow(
TemplateError
)
})

test('rejects deeply nested escape', () => {
expect(() =>
validateRelativePath('a/b/c/../../../../escape', undefined)
).toThrow(TemplateError)
})
})

describe('error messages include path', () => {
test('absolute path error includes the path', () => {
try {
validateRelativePath('/etc/passwd', undefined)
expect.fail('Should have thrown')
} catch (e) {
expect(e.message).toContain('/etc/passwd')
}
})

test('escape path error includes the path', () => {
try {
validateRelativePath('../secret', undefined)
expect.fail('Should have thrown')
} catch (e) {
expect(e.message).toContain('../secret')
}
})
})
})
Loading