fix(js-sdk): parse Dockerfile ENV/ARG values with spaces - #1789
fix(js-sdk): parse Dockerfile ENV/ARG values with spaces#1789eeshsaxena wants to merge 1 commit into
Conversation
handleEnvInstruction parsed each dockerfile-ast argument token in isolation, which splits on whitespace (including inside quotes). ENV NAME="John Doe" and ENV KEY=hello world fell into the "key value" branch and produced a malformed key like NAME="John. Rejoin the tokens and parse the raw value the same way as the Python SDK: a key=value value runs across whitespace until the next key= token, and surrounding quotes are stripped. Multiple pairs on one line stay separated.
|
We require contributors to sign our Contributor License Agreement, and we don't have @eeshsaxena on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
🦋 Changeset detectedLatest commit: c17ca80 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c17ca80d87
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const value = argumentsData.map((arg) => arg.getValue()).join(' ') | ||
| const envVars: Record<string, string> = {} | ||
|
|
||
| if (value.includes('=')) { |
There was a problem hiding this comment.
Preserve legacy ENV values containing equals signs
Docker's legacy ENV <key> <value> form permits = inside the value, but selecting the assignment form whenever the whole line contains = misparses ENV KEY value=foo as { value: "foo" } and drops KEY; the parent implementation's two-token branch correctly produced KEY=value=foo. Determine the syntax from the leading token rather than from any equals sign in the joined value, and make the equivalent correction in the shared Python parser, which currently uses the same faulty test.
AGENTS.md reference: AGENTS.md:L3-L3
Useful? React with 👍 / 👎.
| const envVars: Record<string, string> = {} | ||
|
|
||
| if (value.includes('=')) { | ||
| const pairRegex = /(\w+)=([^\s]*(?:\s+(?!\w+=)[^\s]*)*)/g |
There was a problem hiding this comment.
Preserve punctuation in ENV variable names
Docker ENV keys are not restricted to JavaScript's ASCII \w class, so a valid instruction such as ENV com.example.setting=on is now parsed as { setting: "on" }; similarly, ENV MY-VAR=value becomes VAR=value. The previous substring(0, equalIndex) logic preserved these names, so the matcher should capture the complete non-whitespace key before = and the equivalent Python regex should be corrected as well.
AGENTS.md reference: AGENTS.md:L3-L3
Useful? React with 👍 / 👎.
Problem
The JS SDK's Dockerfile parser (
Template.fromDockerfile) parsed eachENV/ARGargument token in isolation.dockerfile-astsplits arguments on whitespace — including inside quotes — so a value with a space landed in the "traditionalkey value" branch and produced a malformed key.This breaks the standard, documented way to put a space in an env value.
ENV KEY=hello worldwas affected too. The Python SDK already parses these correctly (regex on the raw string + quote strip), so the same Dockerfile produced different env vars depending on which SDK you used.Fix
Rejoin the tokens and parse the raw value the same way as the Python SDK:
key=valueform: a value runs across whitespace until the nextkey=token; surrounding quotes are stripped.key valueform unchanged; multiplekey=valuepairs on one line stay separated.Notes
fromDockerfiletest asserting the quoted, spaced, and multi-pair cases, plus a changeset. I verified the new parser againstdockerfile-ast's real tokenization for all the cases above; the monorepo install was too slow to run the fullpnpm testlocally, so CI runs the suite.