fix(sdk): send the upload-request headers the API returns for template file uploads - #1876
tomassrnka wants to merge 2 commits into
Conversation
The file-upload-link response carries `headers` — request headers the upload
must send that the signed URL cannot carry itself. The API sets them when the
storage backend needs them: Azure signs layer-file uploads with a SAS and
returns `x-ms-blob-type: BlockBlob`, which a SAS cannot express.
`TemplateBase.build` destructured only `{ present, url }` and `putFileStream`
PUT with nothing but `Content-Length`, so on an Azure-backed cluster every
uncached `COPY` failed with a storage `400 MissingRequiredHeader`. GCS and S3
return no headers, which is why the live-API suites never saw it.
Thread the headers into the PUT. The map is an open `additionalProperties:
string`, so spreading it under `Content-Length` does not protect body framing:
a lowercase `content-length` from the API survives the exact-case shadowing as
a second key, and undici joins the pair into `5, 11` and rejects the request.
Drop `content-length` and `transfer-encoding` case-insensitively before
merging, then apply the archive's own `Content-Length` last. A missing or
empty map behaves exactly as before.
The upload error wrap now carries `error.cause`: fetch reports transport
failures as a bare `TypeError: fetch failed`, which told the user nothing.
Verification: `tests/template/uploadHeaders.test.ts` drives
`Template.buildInBackground` against a mocked upload link and a local PUT
server, asserting the header arrives with headers set and is absent without;
`uploadFile.test.ts` covers the `buildApi` boundary and adds a row where the
API returns `content-length: 1`, asserting the server saw the real archive
size, one `content-length` header and no chunked encoding. All fail without
this change. The msw server now errors on unhandled requests rather than
letting them escape to the real API.
The file-upload-link response carries `headers` — request headers the upload must send that the signed URL cannot carry itself. The API sets them when the storage backend needs them: Azure signs layer-file uploads with a SAS and returns `x-ms-blob-type: BlockBlob`, which a SAS cannot express. Both builders passed only `file_info.url` to `upload_file`, whose signature had no headers parameter — the sync path PUT with no headers, the async path with `Content-Length` only. On an Azure-backed cluster every uncached `COPY` failed with a storage `400 MissingRequiredHeader`. GCS and S3 return no headers, which is why the live-API suites never saw it. Thread the headers into the PUT; the generated field is `UNSET` or a `TemplateBuildFileUploadHeaders`, and both a missing and an empty map behave exactly as before. The map is an open `additionalProperties: string`, so merging it verbatim breaks body framing. httpx applies its fstat-derived `Content-Length` with a case-insensitive `setdefault`, so an API `content-length: 1` wins on the sync path and truncates the archive to one byte with a 200 back; on the async path exact-key dedupe lets it through as a second header, which hyper forwards as a duplicate plus chunked encoding — the e2b#1243 failure. `strip_framing_headers` drops `content-length` and `transfer-encoding` case-insensitively, and both paths now apply an explicit `Content-Length` from the archive's size last. Verification: `test_build_forwards_upload_link_headers` drives `_build` against a stubbed upload link and a local PUT server in both suites, parametrized over headers set and `UNSET`; `test_upload_file_sends_required_headers` covers the `build_api` boundary and `test_upload_file_drops_framing_headers_from_the_api` pins the precedence with an API `content-length: 1`. All fail without this change.
🦋 Changeset detectedLatest commit: 42102f3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
PR SummaryLow Risk Overview The builders pass those headers through Reviewed by Cursor Bugbot for commit 42102f3. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Package ArtifactsBuilt from 383a6c2. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.49.2-fix-template-upload-headers.0.tgzCLI ( npm install ./e2b-cli-2.19.1-fix-template-upload-headers.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.8.1-fix-template-upload-headers.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.4.1-fix-template-upload-headers.0.tgzPython SDK ( pip install ./e2b-2.49.1+fix.template.upload.headers-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.10.0+fix.template.upload.headers-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.5.0+fix.template.upload.headers-py3-none-any.whl |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
TASTE.md review (sdk-harness). Checked: parity (T-1/T-2), API shape (T-3/T-3a, T-11, T-18, T-20), builder (T-39), timeouts (T-46), errors (T-57/T-62), module structure (T-54).
1 violation (2 mirrored sites), both internal-only.
- T-3a — the new
headersparameter onupload_filein bothtemplate_sync/build_api.pyandtemplate_async/build_api.pyis a defaulted positional (no bare*), and it is inserted before the existingrequest_timeout, which is exactly the reorder hazard T-3a warns about. Inline suggestions attached.
Not tied to a diff line: the JS/Python helper pair is named withoutFramingHeaders ↔ strip_framing_headers — T-1 asks for mirrored names differing only by casing (stripFramingHeaders or without_framing_headers). Internal helpers, so low priority.
Everything else looks consistent with the guide: the generated TemplateBuildFileUpload.headers is unwrapped at the boundary (to_dict() / Unset check) and never reaches users (T-18), signal is still threaded through putFileStream (T-46), the option is ?: T not | null (T-20), and FileUploadError keeps the cause (T-62).
| resolve_symlinks: bool, | ||
| gzip: bool, | ||
| stack_trace: Optional[TracebackType], | ||
| headers: Optional[Dict[str, str]] = None, |
There was a problem hiding this comment.
T-3a — new optional parameters are keyword-only from day one, enforced by a bare *. As written headers is a defaulted positional inserted ahead of request_timeout, so any caller binding request_timeout by position now silently passes it as headers. Internal today, but the rule exists to make that impossible; all call sites already use kwargs so adding * is safe.
| headers: Optional[Dict[str, str]] = None, | |
| *, | |
| headers: Optional[Dict[str, str]] = None, |
| resolve_symlinks: bool, | ||
| gzip: bool, | ||
| stack_trace: Optional[TracebackType], | ||
| headers: Optional[Dict[str, str]] = None, |
There was a problem hiding this comment.
T-3a — same as the async mirror: keyword-only via bare * so the new headers cannot be bound positionally and cannot shift request_timeout.
| headers: Optional[Dict[str, str]] = None, | |
| *, | |
| headers: Optional[Dict[str, str]] = None, |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good — a small, symmetric fix (JS + both Python builders) with solid new test coverage for header forwarding and framing-header stripping.
What was reviewed: the header-threading path from getFileUploadLink/get_file_upload_link through uploadFile/upload_file to the PUT in all three implementations; the strip_framing_headers/withoutFramingHeaders filters (case-insensitive, applied before the explicit Content-Length is added last); the new sync-Python os.fstat size computation, which now matches the pre-existing async-path pattern; and the new/updated unit tests (uploadFile.test.ts, uploadHeaders.test.ts, both Python test_upload_file.py) covering headers-present, headers-absent, and content-length-collision cases.
Extended reasoning...
Overview
This PR threads an optional headers map (returned by the file-upload-link API) through the template build-context upload path in the JS SDK (buildApi.ts, index.ts) and both Python SDK implementations (template_sync/template_async build_api.py/main.py, plus a new shared strip_framing_headers helper in template/utils.py). This is needed because Azure-backed clusters sign uploads with a SAS URL that cannot carry the required x-ms-blob-type: BlockBlob header, so the API now returns it out-of-band and the SDK must forward it on the PUT.
Security risks
None identified. The headers being forwarded originate from the trusted API response (already authenticated via the SDK's own client), not from user input. The change deliberately strips content-length/transfer-encoding (case-insensitively) from the API-supplied map before layering the archive's own explicit Content-Length on top, preventing the API response from truncating or double-framing the request body — this is the main correctness-sensitive detail and it is handled consistently (and tested) in all three code paths (JS, sync Python, async Python).
Level of scrutiny
This warranted a close read since it touches the upload transport path (body framing correctness is easy to get subtly wrong, e.g. duplicate Content-Length or chunked encoding kicking in). I verified: (1) the framing-header stripping is applied uniformly across JS/sync-Python/async-Python; (2) the sync Python path's new os.fstat-based explicit Content-Length matches the existing async-Python pattern rather than diverging; (3) tar_file_stream leaves the file positioned at offset 0 before fstat/read, so the size and body are consistent; (4) the generated TemplateBuildFileUpload.headers field already exists in the codebase (this PR is stacked on a prior PR per its description), so no spec/ files were touched, consistent with the changed-file list.
Other factors
Test coverage is thorough and symmetric: new tests in both SDKs cover headers-present, headers-absent (byte-identical/no-op), and a precedence case where the API returns a colliding content-length alongside the real header, asserting the archive's real size still wins with exactly one content-length header and no chunked encoding. A changeset was added for the two affected packages. The change is additive and small in surface area (an optional parameter), with no behavior change for GCS/S3 clusters that return no headers.
|
Closing as a duplicate of #1870, which fixes the same defect (opened 2026-09-11, validated live against an Azure BYOC environment — evidence this PR could not produce). Three small deltas from this branch are offered on #1870 as a comment: dropping My miss for the record: this branch was dispatched without first checking open PRs against the same code. |
1 participant
Summary
Template file uploads now send the request headers the file-upload-link response returns.
The API's
GET /templates/{templateID}/files/{hash}response carriesheaders— "Request headersthat must be sent with the upload request" — populated when the storage backend requires a header
the signed URL cannot carry. Azure signs layer-file uploads with a SAS and returns
x-ms-blob-type: BlockBlob, because Put Blob requires that header and a SAS cannot express it.Neither SDK sent them. The JS builder destructured only
{ present, url }; both Python builderspassed only
file_info.urlto anupload_filethat had no headers parameter. On an Azure-backedcluster every uncached
COPYinTemplate.build()/AsyncTemplate.build()failed with astorage
400 MissingRequiredHeader, surfacing as a file-upload error. GCS- and S3-backedclusters return no headers, which is why this never showed up in the live-API suites.
All three upload paths now merge the returned headers into the PUT, with
Content-Lengthappliedlast so body framing still wins. A missing or empty map leaves the request byte-identical to
before.
No API surface change — the header set comes from the API, so no user code changes:
Verification
packages/js-sdk/tests/template/uploadHeaders.test.ts: mocks the upload link withheaders: {"x-ms-blob-type": "BlockBlob"}, drivesTemplate.buildInBackgroundand asserts thePUT reaching a local server carries the header plus
Content-Length; a second case with noheadersasserts the request is unchanged.uploadFile.test.tsand both Pythontest_upload_file.pysuites extended with the same twocases at the
buildApi/build_apiboundary, plusTemplate._build/AsyncTemplate._buildthreading tests parametrized over headers-set andUNSET.content-length: 1from the API alongsidex-ms-blob-typeandasserts the PUT still carried the real archive size, one
content-lengthheader and no chunkedencoding. Without the filter the sync Python path uploads a one-byte archive under a 200, the
async path emits a duplicate
content-length, and undici rejects the JS request outright.index.tsandmain.pythreading is separately forcing.pnpm run lint,pnpm run format,pnpm run typecheck,packages/js-sdkbuild;uv run make lint,uv run make typecheck,uv run pytest tests --ignore=tests/sync --ignore=tests/async(534 passed) and the two uploadsuites (18 passed).
from this environment.
Risk
Low. Additive: the only behaviour change is extra request headers on the build-context PUT, and
only when the API returns them — which today means Azure clusters only. GCS and S3 paths are
byte-identical, guarded by the no-headers test cases. The
headersmap is an openadditionalProperties: string, socontent-lengthandtransfer-encodingare dropped from itcase-insensitively and each path applies its own
Content-Lengthlast — nothing the API returns cantruncate or double-frame the body.
Reviewed adversarially by one vendor (Claude
code-review, high): five findings, all applied — the framing-header filter, the explicit syncContent-Length, the preserved error cause, the precedence test rows, and the msw unhandled-request guard. The Codex pass could not run (account out of credits), so this PR carries no cross-vendor review.Rollout
patchone2band@e2b/python-sdk. No migration, no config.Follow-ups
COPYupload against an Azure-backed cluster end-to-end; the live-APItemplate suites run only on GCS/S3 today, which is the gap that let this ship.
is in the matrix.