Skip to content

fix(sdk): resolve directory symlinks in template copies - #2137

Open
Elon-Jiang wants to merge 5 commits into
e2b-dev:mainfrom
Elon-Jiang:fix/resolve-directory-symlinks
Open

Elon-Jiang wants to merge 5 commits into
e2b-dev:mainfrom
Elon-Jiang:fix/resolve-directory-symlinks

Conversation

@Elon-Jiang

@Elon-Jiang Elon-Jiang commented Nov 13, 2026 •

Copy link
Copy Markdown

Problem and fix

Fixes #2136. copy('linked', '/app', resolve_symlinks=True) / resolveSymlinks: true currently uploads an empty directory when linked points to a directory. The cache hash also ignores the target's files, so editing one does not invalidate the COPY layer.

Pass the resolution setting through the shared file-discovery path in both SDKs, so hashing and tar uploads see the same resolved descendants. Keep logical link paths for archive names and ignore matching. Preserve the default symlink-copy behavior and report cyclic or out-of-context resolved paths as build errors at the user copy callsite. Python sync and async builders use the same helper.

Usage

template = Template(file_context_path=context).from_python_image("3").copy(
    "linked", "/app", resolve_symlinks=True,
)
Template.build_in_background(template, "directory-link-example")
const template = new Template({ fileContextPath: context })
  .fromPythonImage('3')
  .copy('linked', '/app', { resolveSymlinks: true })
await Template.buildInBackground(template, 'directory-link-example')

With linked -> real and real/app.py, the archive now contains linked/app.py; changing its contents changes the upload hash. Existing public options are unchanged; this restores their documented resolution behavior. Resolved paths must remain in the configured file context, including file links encountered within resolved directories. Move intended targets inside that context (or choose a context that contains them), exclude the link, or preserve links with resolution disabled. A patch changeset covers both SDKs.

Validation

The two JS discovery review findings were reproduced as two failures with six controls before the follow-up fix. The re-inclusion failure is triggered by copying the context root (.); the ordinary directory example already passed. Separate synthetic outside-target probes failed the new containment assertions before their fix in both SDKs. A second follow-up reproduces ignored self-link failures in both Python builders (two failures) and unselected-file expansion under a symlinked JS context (one failure with a resolution-disabled control); those now pass. A recursive **/* source with ignored matched directories and re-included linked children was also reproduced as one JS and two Python failures. Source matches used for traversal are now separated from archive inclusion; a literal-directory-name regression covers the added traversal path.

Original behavior was reproduced on main 3e72be3f08ec1a258ec0253c153df7ceed739f10. Final validation uses main 6dc8d5a10aa5db8dc8cf9c4d95fc769d886197fd (SDK 2.64.0), merged without rewriting published history:

  • Public sync/async Python regression and preservation controls: original source 10 failures, 2 passes with the initial expanded cases. The final public Python build suite has 36 passes.
  • Public JS build regression and preservation control: original source 1 failure, 1 pass; all 18 final public build cases pass after the fix.
  • Python: uv run pytest tests/shared/template tests/sync/template_sync/test_upload_file.py -q — 157 passed, 1 skipped.
  • JS: pnpm --dir packages/js-sdk test --project template tests/template/directorySymlinks.test.ts tests/template/utils tests/template/uploadFile.test.ts — 108 passed, 3 skipped.
  • Root pnpm run format, pnpm run lint, and pnpm run typecheck (including SDK builds) pass. Lint emits four warnings in unchanged files.

The public-build regressions run real filesystem discovery, hashing, archives and HTTP uploads against a loopback server replacing only E2B's remote build/upload endpoints. They cover nested and repeated targets, target-content cache invalidation, ignored-file stability, default symlink preservation, cycle detection (including ignored ancestors), links with literal glob characters, root-copy re-inclusion and cache invalidation, out-of-context directory/nested-file targets, actionable errors preserving the original copy frame, excluded self-referential links, and exact source selection when the context itself is a symlink. A separate native HTTP reproduction confirms the before/after archive members and hashes. No cloud image build or sandbox execution was tested. New symlink tests were run on macOS and skip on Windows where creating links requires privileges; other runtimes were not exercised.

AI assistance: OpenAI Codex helped investigate, implement, and validate this change.

The public copy()/CopyItem API documentation in both SDKs now also describes file/directory link traversal, default link preservation, and the build-time BuildError/BuildException for detected directory cycles or out-of-context targets. Repository format/lint/type checks and the same 157 Python / 108 JS passing tests were rerun after this documentation follow-up (skips unchanged: 1 / 3).

Co-authored-by: OpenAI Codex <noreply@openai.com>
@changeset-bot

changeset-bot Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: efcc2b9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
e2b Patch
@e2b/python-sdk Patch
@e2b/code-interpreter-python Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cla-bot cla-bot Bot added the cla-signed label Nov 13, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-13T10:58:47.127273Z 17e5214 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@devin-ai-integration devin-ai-integration Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

Devin Review found 2 potential issues.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

Comment thread packages/js-sdk/src/template/utils.ts Outdated
Comment on lines +279 to +280
for (const file of files.values()) {
if (!file.isSymbolicLink()) continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Reincluded files under ignored links disappear

When an ignore pattern excludes a directory link but re-includes a child, getAllFilesInPath omits the child. The expansion loop never sees the excluded link, so the build uploads no re-included content.

Learn more

File discovery uses glob's ignore matcher before expanding symlink directories. An ignored symlink is absent from files, even when childrenIgnored allows traversal for a later negated pattern. Glob does not descend into symlink directories by default, so the expansion loop cannot discover that link or its re-included children. Both calculateFilesHash and spoolTarArchive consume the incomplete list.

Example: Let real/link point to a directory containing keep.txt. Copy real with resolveSymlinks: true and ignore patterns ['real/link', '!real/link/keep.txt']. The expected archive includes real/link/keep.txt; the JS traversal omits it while the Python walker descends through the ignored link.

Recommended fix: Track directory links separately even when their own entries are ignored but mayMatchUnder is true, then traverse those links using the logical path and apply ignore patterns to their descendants. Do not add ignored link entries to the archive.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +299 to +301
const linkedFiles = await glob(
normalizePath(path.join(file.relative(), '**/*')),
{ ignore, withFileTypes: true, dot: true, cwd: contextPath }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Special-character links lose their contents

When a matched directory link has brackets in its name, getAllFilesInPath treats that name as a new glob pattern. Its target files disappear from the archive and cache hash, or files from a different directory appear instead.

Learn more

The initial glob can match a symlink with a filename containing glob metacharacters, especially when the source is a wildcard. Its relative() value is a filesystem path, not a glob-escaped prefix. Passing that value to glob again changes the meaning of characters such as [ and ]. The same construction already exists for ordinary matched directories, but this change newly uses it for matched symlink directories.

Example: With alias[1] -> real and real/app.py, copying * matches alias[1]. The generated pattern alias[1]/**/* looks under alias1 instead of alias[1], leaving the link's uploaded directory empty. An alias1 directory can contribute unrelated files.

Recommended fix: Escape glob metacharacters in the link's literal relative path before appending /**/*, or enumerate children starting at the literal directory path while preserving logical paths and applying ignore patterns.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TASTE.md compliance review (e2b-dev/sdk-harness). I checked the changed code against parity (T-1, T-2), API shape (T-3, T-3a), the template builder (T-37–T-42), errors (T-57, T-62, T-63) and docs (T-69–T-71).

2 violations, posted inline. They are the same issue in each SDK: the new cyclic-symlink error.

  • JS utils.ts and Python utils.py raise TemplateError / TemplateException instead of BuildError / BuildException. The error also loses the user's copy() stack trace (T-42, T-57), and the message doesn't say how to fix it (T-62, T-63).

Not tied to a changed line:

  • T-62 / T-69: the PR adds a new failure mode to the public copy(): it can now throw on a cyclic directory link when resolveSymlinks / resolve_symlinks is enabled. The public docs for that option don't mention it. In JS, copy() in template/index.ts and CopyItem.resolveSymlinks in types.ts say nothing; in Python, :param resolve_symlinks: Whether to resolve symlinks in template/main.py is all there is. Document that directory links are traversed and when the error is thrown (and which error), in both SDKs.
  • Minor, internal only (not counted): getAllFilesInPath / get_all_files_in_path now take two trailing optional booleans by position, so call sites read ..., true, resolveSymlinks). These helpers aren't public, so T-3 / T-3a don't strictly apply. Even so, a keyword-only resolve_symlinks (after a *) and an options object in JS would match the house style. The JS JSDoc also lacks @param resolveSymlinks, which the Python docstring has.

Parity is otherwise good. Both SDKs keep logical link paths for archive names and ignore matching, both detect cycles, and sync/async Python share one helper (T-1, T-2). The changeset covers both packages.

Comment thread packages/js-sdk/src/template/utils.ts Outdated
Comment on lines +290 to +292
throw new TemplateError(
`Cannot resolve cyclic directory symlink: ${file.relativePosix()}`
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T-42 / T-57 — a builder configuration failure (the copy() source contains a cyclic directory link) should be a BuildError, not a TemplateError (which subclasses SandboxError; no sandbox is involved here). T-42 also requires the stack trace to point at the user's .copy(...) call. As written, this throw skips the stackTrace that calculateFilesHash already gets, so the trace lands inside utils.ts. Pass stackTrace through (e.g. as new BuildError(msg, stackTrace)), or catch and rethrow in calculateFilesHash the same way the No files found error sets error.stack.

T-62 / T-63 — the message says what failed but not what to do. Name the fix in the option the user typed:

Suggested change
throw new TemplateError(
`Cannot resolve cyclic directory symlink: ${file.relativePosix()}`
)
throw new BuildError(
`Directory symlink '${file.relativePosix()}' points to one of its own ancestors, so it can't be resolved. Remove the link, exclude it via .dockerignore, or set \`resolveSymlinks: false\` to copy the link itself.`
)

(BuildError must be imported from ../errors.)

Comment on lines +206 to +208
raise TemplateException(
f"Cannot resolve cyclic directory symlink: {dir_relative_path}"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T-42 / T-57: a builder configuration failure should raise BuildException, not TemplateException (a SandboxException subclass). It should also carry the user's builder frame. calculate_files_hash already receives stack_trace and applies it to No files found via .with_traceback(stack_trace), but this raise bypasses it, so the traceback points into utils.py. Catch and re-raise with .with_traceback(stack_trace) in calculate_files_hash, or thread the trace down.

T-62 / T-63: make the message actionable and use the Python option name. This also keeps it in sync with the JS message (T-1):

Suggested change
raise TemplateException(
f"Cannot resolve cyclic directory symlink: {dir_relative_path}"
)
raise BuildException(
f"Directory symlink '{dir_relative_path}' points to one of its own ancestors, so it can't be resolved. "
"Remove the link, exclude it via .dockerignore, or set `resolve_symlinks=False` to copy the link itself."
)

(BuildException must be imported from e2b.exceptions.)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17e52146aa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/js-sdk/src/template/utils.ts Outdated
Comment on lines +279 to +280
for (const file of files.values()) {
if (!file.isSymbolicLink()) continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Traverse excluded symlink directories for re-included files

When resolveSymlinks is true and .dockerignore excludes a directory link but later re-includes a descendant (for example, linked then !linked/keep.txt), the initial ignore-aware glob does not follow that symlink, so it never enters files and this pass cannot expand it. JS then omits the re-included file from both the upload and cache hash, while Python's walk() follows excluded directories when may_match_under is true, leaving the SDKs with different COPY contents for the same inputs.

AGENTS.md reference: AGENTS.md:L3-L3

Useful? React with 👍 / 👎.

def walk(
dir_path: str, dir_relative_path: str, ancestors: frozenset[str] = frozenset()
) -> None:
real_path = os.path.realpath(dir_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Agentic Security Review

Severity: MEDIUM

Description: With copy(resolve_symlinks=True) / copy({resolveSymlinks:true}), a directory symlink inside the template file context is walked and its target bytes are archived under the link's logical name. realpath is used only to reject cycles. A link to any readable directory that is not a walk ancestor — a sibling checkout, ~/.aws, ~/.ssh, or another absolute directory — is listed, hashed, and uploaded. validate_relative_path only checks the COPY source string, so the context-root boundary is not applied to the resolved target.

Impact: The template context root is the documented COPY boundary that keeps path traversal out of the upload. This change lets in-context content (a repo, PR, or project tree) steer the local build into reading files the process can already open outside that root and sending them to the remote template file store, where they are baked into the image under an in-context archive path. The builder is the confused deputy: the flag defaults off, but turning it on for legitimate in-context links does not authorize that escape, and a template definition that sets the flag can ship next to the symlink. Host credentials and other projects become part of the uploaded image.

Remediation: After realpath, skip or reject a directory symlink whose resolved target is outside the resolved context root — commonpath/relative must not escape — before listing, hashing, or archiving its children. Keep the cycle check, but do not treat it as the containment gate. Apply the same root check on both the Python walk and the JS glob expansion.

Merge current main without rewriting published history and address public-build regression and review findings.

Co-authored-by: OpenAI Codex <noreply@openai.com>
devin-ai-integration[bot]

This comment was marked as resolved.

@Elon-Jiang

Copy link
Copy Markdown
Author

Addressed the visible review findings in 4b435e78e0b90c88c1c67173894194cc1a9c5e56, including the current main merge without rewriting history.

  • Reproduced the JS re-inclusion failure with copy('.') (the ordinary-directory example already passed) and the literal alias[1] failure. Both failed before the follow-up fix and now pass through the public build API. Added sync/async Python parity cases and cache checks for re-included versus ignored files.
  • Resolved directories and their nested file links must stay in the canonical template context. Synthetic outside-target tests fail before the guard and now reject before any upload; resolution-disabled controls still preserve links.
  • Cycle/context failures use BuildError/BuildException, include corrective options, and preserve the user's original copy frame. Public tests assert the error type and original callsite.

On current main 6dc8d5a: Python 147 passed / 1 skipped; JS 103 passed / 3 skipped; root format, lint, type checks and SDK builds pass (four lint warnings in untouched files). Tests replace only remote build/upload endpoints with a native loopback fixture; no cloud image execution is claimed. The PR description has the updated scope and validation.

Co-authored-by: OpenAI Codex <noreply@openai.com>
devin-ai-integration[bot]

This comment was marked as resolved.

Co-authored-by: OpenAI Codex <noreply@openai.com>
@Elon-Jiang

Copy link
Copy Markdown
Author

5d362ca9cde2bc11b855f7d3ff15f865add90924 addresses the recursive-source finding. A **/* COPY with ignored matched directories and !linked/keep.txt failed through the public API in JS and both Python builders before the fix; all three now upload exactly the re-included file.

Traversal candidates now come from actual source matches or descendants of selected directories, while archive inclusion still applies the ignore matcher. Matched directory prefixes are literal paths when resolving. This preserves the symlinked-context test that copies only app.py, and a new re-included real[1] test covers the literal-directory path.

Current validation: 157 Python passed / 1 skipped, 108 JS passed / 3 skipped, including 36 Python and 18 JS public-build cases. Root format, lint, types and SDK builds pass. The PR body includes the current scope, native loopback-fixture boundaries and all regression results.

Co-authored-by: OpenAI Codex <noreply@openai.com>
@Elon-Jiang

Copy link
Copy Markdown
Author

The public API documentation follow-up is now in efcc2b947: JS copy() and CopyItem.resolveSymlinks, plus the shared Python copy() documentation, describe directory traversal, default link preservation, and build-time error types for detected directory cycles or targets outside the file context. This addresses the public documentation note in the earlier compliance review.

Root format, lint and type/build checks pass. The same affected suites remain green: Python 157 passed / 1 skipped; JS 108 passed / 3 skipped. Runtime code is unchanged by this documentation follow-up.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Resolving directory symlinks drops COPY contents and cache inputs

1 participant