Skip to content

Commit 5e9c6d6

Browse files
mishushakovclaude
andauthored
feat: validate copy src paths are relative and within context directory (#1106)
Add path validation to the copy method in both JS and Python SDKs to ensure source paths are always relative and don't escape the context directory. This prevents: - Absolute paths like /absolute/whatever (Unix) or C:\whatever (Windows) - Path traversal attacks like ../whatever or ./foo/../../../bar The validation works cross-platform using Node's path.isAbsolute/normalize and Python's os.path.isabs/normpath plus PureWindowsPath for detecting Windows paths on Unix. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes behavior of `copy`/`copy_items` to throw earlier for previously-accepted absolute or escaping paths, which could break some consumers; logic is localized and well-covered by tests. > > **Overview** > Prevents path traversal in template `copy` operations by validating `src` is *relative* and does not escape the context directory (rejects absolute paths and `..`-based escapes) in both the JS and Python SDKs. > > Updates `copyItems`/`copy_items` error handling to preserve the caller’s stack trace when validation fails, adds unit coverage for the new path validator plus new stack-trace tests for absolute-path failures, and ships as patch releases via a changeset. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c1a8eb9. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 1a8fed0 commit 5e9c6d6

10 files changed

Lines changed: 519 additions & 69 deletions

File tree

.changeset/every-feet-attack.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@e2b/python-sdk': patch
3+
'e2b': patch
4+
---
5+
6+
fix: validate copy src paths are relative and within context directory

packages/js-sdk/src/template/index.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
padOctal,
4545
readDockerignore,
4646
readGCPServiceAccountJSON,
47+
validateRelativePath,
4748
} from './utils'
4849

4950
/**
@@ -520,10 +521,16 @@ export class TemplateBase
520521
}
521522

522523
const srcs = Array.isArray(src) ? src : [src]
524+
const stackTrace = getCallerFrame(STACK_TRACE_DEPTH - 1)
523525

524526
for (const src of srcs) {
527+
const srcString = src.toString()
528+
529+
// Validate that the source path is a relative path within the context directory
530+
validateRelativePath(srcString, stackTrace)
531+
525532
const args = [
526-
src.toString(),
533+
srcString,
527534
dest.toString(),
528535
options?.user ?? '',
529536
options?.mode ? padOctal(options.mode) : '',
@@ -547,14 +554,23 @@ export class TemplateBase
547554
throw new Error('Browser runtime is not supported for copyItems')
548555
}
549556

557+
// Stack trace that will be used to re-throw the error with
558+
const stackTrace = getCallerFrame(STACK_TRACE_DEPTH - 1)
559+
550560
this.runInNewStackTraceContext(() => {
551561
for (const item of items) {
552-
this.copy(item.src, item.dest, {
553-
forceUpload: item.forceUpload,
554-
user: item.user,
555-
mode: item.mode,
556-
resolveSymlinks: item.resolveSymlinks,
557-
})
562+
try {
563+
this.copy(item.src, item.dest, {
564+
forceUpload: item.forceUpload,
565+
user: item.user,
566+
mode: item.mode,
567+
resolveSymlinks: item.resolveSymlinks,
568+
})
569+
} catch (error) {
570+
const copyError = error as Error
571+
copyError.stack = stackTrace
572+
throw copyError
573+
}
558574
}
559575
})
560576

packages/js-sdk/src/template/utils.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,60 @@ import { BASE_STEP_NAME, FINALIZE_STEP_NAME } from './consts'
77
import type { Path } from 'glob'
88
import type { BuildOptions } from './types'
99

10+
/**
11+
* Validate that a source path for copy operations is a relative path that stays
12+
* within the context directory. This prevents path traversal attacks and ensures
13+
* files are copied from within the expected directory.
14+
*
15+
* @param src The source path to validate
16+
* @param stackTrace Optional stack trace for error reporting
17+
* @throws TemplateError if the path is absolute or escapes the context directory
18+
*
19+
* Invalid paths:
20+
* - Absolute paths: /absolute/path, C:\Windows\path
21+
* - Parent directory escapes: ../foo, foo/../../bar, ./foo/../../../bar
22+
*
23+
* Valid paths:
24+
* - Simple relative: foo, foo/bar
25+
* - Current directory prefix: ./foo, ./foo/bar
26+
* - Internal parent refs that don't escape: foo/../bar (stays within context)
27+
*/
28+
export function validateRelativePath(
29+
src: string,
30+
stackTrace: string | undefined
31+
): void {
32+
// Check for absolute paths using Node's cross-platform implementation
33+
if (path.isAbsolute(src)) {
34+
const error = new TemplateError(
35+
`Invalid source path "${src}": absolute paths are not allowed. Use a relative path within the context directory.`,
36+
stackTrace
37+
)
38+
throw error
39+
}
40+
41+
// Normalize the path and check if it escapes the context directory
42+
const normalized = path.normalize(src)
43+
44+
// After normalization, a path that escapes would be '..' or start with '../'
45+
// We check for '..' followed by path separator to avoid false positives on filenames like '..myconfig'
46+
// Examples:
47+
// - '../foo' -> '../foo' (escapes)
48+
// - 'foo/../../bar' -> '../bar' (escapes)
49+
// - './foo/../../../bar' -> '../../bar' (escapes)
50+
// - 'foo/../bar' -> 'bar' (doesn't escape)
51+
// - './foo/bar' -> 'foo/bar' (doesn't escape)
52+
// - '..myconfig' -> '..myconfig' (valid filename, doesn't escape)
53+
const escapes = normalized === '..' || normalized.startsWith('..' + path.sep)
54+
55+
if (escapes) {
56+
const error = new TemplateError(
57+
`Invalid source path "${src}": path escapes the context directory. The path must stay within the context directory.`,
58+
stackTrace
59+
)
60+
throw error
61+
}
62+
}
63+
1064
/**
1165
* Normalize build arguments from different overload signatures.
1266
* Handles string name or legacy options object with alias.

packages/js-sdk/tests/template/stacktrace.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { apiUrl, buildTemplateTest } from '../setup'
99
import { randomUUID } from 'node:crypto'
1010

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

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

215+
buildTemplateTest('traces on copy absolute path', async () => {
216+
await expectToThrowAndCheckTrace(async () => {
217+
Template().fromBaseImage().copy('/absolute/path', '/absolute/path')
218+
}, 'copy')
219+
})
220+
221+
buildTemplateTest('traces on copyItems absolute path', async () => {
222+
await expectToThrowAndCheckTrace(async () => {
223+
Template()
224+
.fromBaseImage()
225+
.copyItems([{ src: '/absolute/path', dest: '/absolute/path' }])
226+
}, 'copyItems')
227+
})
228+
215229
buildTemplateTest('traces on remove', async ({ buildTemplate }) => {
216230
let template = Template().fromBaseImage()
217231
template = template.skipCache().remove(nonExistentPath)
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import { describe, expect, test } from 'vitest'
2+
import { validateRelativePath } from '../../../src/template/utils'
3+
import { TemplateError } from '../../../src/errors'
4+
5+
const isWindows = process.platform === 'win32'
6+
7+
describe('validateRelativePath', () => {
8+
describe('valid paths', () => {
9+
test('accepts simple relative path', () => {
10+
expect(() => validateRelativePath('foo', undefined)).not.toThrow()
11+
})
12+
13+
test('accepts nested relative path', () => {
14+
expect(() => validateRelativePath('foo/bar', undefined)).not.toThrow()
15+
})
16+
17+
test('accepts path with ./ prefix', () => {
18+
expect(() => validateRelativePath('./foo', undefined)).not.toThrow()
19+
})
20+
21+
test('accepts nested path with ./ prefix', () => {
22+
expect(() => validateRelativePath('./foo/bar', undefined)).not.toThrow()
23+
})
24+
25+
test('accepts path with internal parent ref that stays within context', () => {
26+
expect(() => validateRelativePath('foo/../bar', undefined)).not.toThrow()
27+
})
28+
29+
test('accepts current directory', () => {
30+
expect(() => validateRelativePath('.', undefined)).not.toThrow()
31+
})
32+
33+
test('accepts glob patterns', () => {
34+
expect(() => validateRelativePath('*.txt', undefined)).not.toThrow()
35+
expect(() => validateRelativePath('**/*.ts', undefined)).not.toThrow()
36+
expect(() => validateRelativePath('src/**/*', undefined)).not.toThrow()
37+
})
38+
39+
test('accepts hidden files and directories', () => {
40+
expect(() => validateRelativePath('.hidden', undefined)).not.toThrow()
41+
expect(() =>
42+
validateRelativePath('.config/settings', undefined)
43+
).not.toThrow()
44+
})
45+
46+
test('accepts filenames starting with double dots', () => {
47+
expect(() => validateRelativePath('..myconfig', undefined)).not.toThrow()
48+
expect(() => validateRelativePath('..cache', undefined)).not.toThrow()
49+
expect(() =>
50+
validateRelativePath('...something', undefined)
51+
).not.toThrow()
52+
expect(() =>
53+
validateRelativePath('foo/..myconfig', undefined)
54+
).not.toThrow()
55+
})
56+
})
57+
58+
describe('invalid paths - absolute', () => {
59+
test('rejects Unix absolute path', () => {
60+
expect(() => validateRelativePath('/absolute/path', undefined)).toThrow(
61+
TemplateError
62+
)
63+
expect(() => validateRelativePath('/absolute/path', undefined)).toThrow(
64+
'absolute paths are not allowed'
65+
)
66+
})
67+
68+
test('rejects root path', () => {
69+
expect(() => validateRelativePath('/', undefined)).toThrow(TemplateError)
70+
})
71+
72+
// Windows path tests - only run on Windows where path.isAbsolute detects them
73+
test.skipIf(!isWindows)('rejects Windows drive letter path', () => {
74+
expect(() =>
75+
validateRelativePath('C:\\Windows\\System32', undefined)
76+
).toThrow(TemplateError)
77+
expect(() =>
78+
validateRelativePath('C:\\Windows\\System32', undefined)
79+
).toThrow('absolute paths are not allowed')
80+
})
81+
82+
test.skipIf(!isWindows)('rejects Windows UNC path', () => {
83+
expect(() =>
84+
validateRelativePath('\\\\server\\share', undefined)
85+
).toThrow(TemplateError)
86+
})
87+
})
88+
89+
describe('invalid paths - parent directory escape', () => {
90+
test('rejects simple parent directory escape', () => {
91+
expect(() => validateRelativePath('../foo', undefined)).toThrow(
92+
TemplateError
93+
)
94+
expect(() => validateRelativePath('../foo', undefined)).toThrow(
95+
'path escapes the context directory'
96+
)
97+
})
98+
99+
test('rejects parent directory escape with forward slash', () => {
100+
expect(() => validateRelativePath('../file.txt', undefined)).toThrow(
101+
TemplateError
102+
)
103+
})
104+
105+
test.skipIf(!isWindows)(
106+
'rejects parent directory escape with backslash',
107+
() => {
108+
expect(() => validateRelativePath('..\\file.txt', undefined)).toThrow(
109+
TemplateError
110+
)
111+
}
112+
)
113+
114+
test('rejects double parent directory escape', () => {
115+
expect(() => validateRelativePath('../../foo', undefined)).toThrow(
116+
TemplateError
117+
)
118+
})
119+
120+
test('rejects path that escapes via nested parent refs', () => {
121+
expect(() => validateRelativePath('foo/../../bar', undefined)).toThrow(
122+
TemplateError
123+
)
124+
})
125+
126+
test('rejects path with ./ prefix that escapes', () => {
127+
expect(() =>
128+
validateRelativePath('./foo/../../../bar', undefined)
129+
).toThrow(TemplateError)
130+
})
131+
132+
test('rejects just parent directory', () => {
133+
expect(() => validateRelativePath('..', undefined)).toThrow(TemplateError)
134+
})
135+
136+
test('rejects current directory followed by parent', () => {
137+
expect(() => validateRelativePath('./..', undefined)).toThrow(
138+
TemplateError
139+
)
140+
})
141+
142+
test('rejects deeply nested escape', () => {
143+
expect(() =>
144+
validateRelativePath('a/b/c/../../../../escape', undefined)
145+
).toThrow(TemplateError)
146+
})
147+
})
148+
149+
describe('error messages include path', () => {
150+
test('absolute path error includes the path', () => {
151+
try {
152+
validateRelativePath('/etc/passwd', undefined)
153+
expect.fail('Should have thrown')
154+
} catch (e) {
155+
expect(e.message).toContain('/etc/passwd')
156+
}
157+
})
158+
159+
test('escape path error includes the path', () => {
160+
try {
161+
validateRelativePath('../secret', undefined)
162+
expect.fail('Should have thrown')
163+
} catch (e) {
164+
expect(e.message).toContain('../secret')
165+
}
166+
})
167+
})
168+
})

0 commit comments

Comments
 (0)