Skip to content

fix(cli): preserve quoted multiline .env values - #6124

Open
STiFLeR7 wants to merge 3 commits into
heygen-com:mainfrom
STiFLeR7:fix/dotenv-multiline-6075
Open

STiFLeR7 wants to merge 3 commits into
heygen-com:mainfrom
STiFLeR7:fix/dotenv-multiline-6075

Conversation

@STiFLeR7

@STiFLeR7 STiFLeR7 commented Oct 23, 2026 •

Copy link
Copy Markdown

Root cause

applyDotEnv (packages/cli/src/utils/dotEnv.ts) parsed .env content by splitting on \n and handing each physical line to entryOf/valueOf independently. For a quoted value, valueOf only ever looks for the closing quote on that same line (raw.indexOf(quote, 1)); when the closing quote is on a later line, it falls back to raw.slice(1) — just the leftover text on the opening line — and every continuation line is dropped silently, since it has no = and fails entryOf's eq < 1 check.

This truncates values like:

HYPERFRAMES_VERTEX_SERVICE_ACCOUNT='{
  "type": "service_account",
  "project_id": "synthetic-project"
}'

down to { instead of the full JSON blob, which later fails JSON.parse.

Fix

Added logicalLines, which joins physical lines into logical ones before parsing: when a line opens a quote (" or ') that isn't closed on that same line, it keeps appending the following lines until one contains the closing quote character. entryOf/valueOf then see the whole multiline value as a single string, and valueOf's existing indexOf(quote, 1) naturally finds the real closing quote across the joined lines, preserving internal newlines. This matches how Node's util.parseEnv treats quoted multiline values.

The quote-detection logic was split into a small openQuoteOf helper to keep logicalLines within the repo's complexity budget.

Test

Added a regression test in dotEnv.test.ts using the exact repro from #6075: a quoted multiline HYPERFRAMES_VERTEX_SERVICE_ACCOUNT value followed by another key, asserting the full value round-trips through JSON.parse and that the key after it still parses correctly.

Verification

  • bunx vitest run src/utils/dotEnv.test.ts (in packages/cli) — 7/7 passed, including the new test
  • node_modules/.bin/oxlint on changed files — 0 warnings/errors
  • node_modules/.bin/oxfmt --check on changed files — correctly formatted
  • bun run --filter @hyperframes/cli typecheck — passes
  • node scripts/check-comment-citations.mjs and node scripts/comment-ratchet.mjs — pass
  • Full pre-commit hook suite (lint, format, tracked-artifacts, cli-process-ownership, fallow complexity/duplication, typecheck, commitlint) — all green

Update: fixed a regression found in review

logicalLines checked openQuoteOf on every physical line before entryOf could classify it as blank or a #-comment. A commented-out example left above a real value (e.g. # OPENROUTER_API_KEY="sk-old) opens a quote that never closes, so the join ran to EOF and swallowed every real key below it. A genuinely stray unclosed quote with no later closing line hit the same failure mode, consuming the rest of the file instead of staying contained to its own line.

Fixed by:

  • Skipping blank lines and #-comments before ever checking for an open quote, mirroring the skip entryOf already does.
  • Extracting the closing-quote scan into closingLineIndex; if it reaches EOF without finding one, logicalLines now falls back to the opening line on its own and resumes from the next line, instead of consuming everything after it.
  • Stripping a trailing \r per line so a CRLF-authored .env doesn't leak \r into a joined multiline value.

Added three more regression tests covering each case, verified by running applyDotEnv directly against the exact repro shapes (not just the vitest assertions), and trimmed the added comments back down to stay within the repo's comment-ratchet budget.

Fixes #6075

applyDotEnv parsed .env content line by line: a quoted value's closing
quote was only ever looked for on the same physical line, so a value
like HYPERFRAMES_VERTEX_SERVICE_ACCOUNT='{...}' spread across several
lines got truncated to the leftover text on its opening line, and the
continuation lines were silently dropped (no "=" to parse as entries).

Join physical lines into logical ones before parsing: when a line
opens a quote that isn't closed on that same line, keep appending
following lines until one contains the closing quote character. This
matches how Node's util.parseEnv treats quoted multiline values.

Fixes heygen-com#6075

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for picking this up, and the write up of the root cause is exactly right: valueOf only ever looked for the closing quote on one physical line. The join works, but it runs before anything classifies the line, so a commented out example swallows the rest of the file. With # OPENROUTER_API_KEY="sk-... on the line above a real OPENROUTER_API_KEY=..., the comment opens a quote that never closes, every later line is absorbed into it, and the parser returns no keys at all where main returned both. A stray unquoted quote anywhere does the same thing. Node's util.parseEnv keeps the later keys in both cases, so this also moves away from the parity the description claims.

Two small changes fix both: skip blank and # lines before looking for an open quote, and if no closing quote is found before the end of the file, fall back to the single physical line and carry on. Could you also pin a CRLF case? The joined value currently keeps its \r.

One more thing: the test suite has not run on this branch yet, only the mergeability and WIP checks, so the new test has not been exercised in CI.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Requesting changes at 59cad9c, agreeing with Miguel's review at this same head. The root cause write-up is right and the single-quoted JSON case from #6075 now works, but the join runs on lines the parser would otherwise skip, and it has no way back when a quote never closes.

I ran applyDotEnv from this head directly:

  • # OPENROUTER_API_KEY="sk-old followed by OPENROUTER_API_KEY=real and GEMINI_API_KEY=g returns {}. On main it returns both keys. A commented-out example is a common thing to find in a .env, so this is a regression for people who never had a multiline value.
  • A='x followed by B=2 gives A = "x\nB=2" and drops B.
  • With CRLF line endings the joined value keeps its \r characters ({\r\n "x": 1\r\n}), which is worth a test either way.

What would fix it: only look for an open quote on lines entryOf would actually parse (skip blank and # lines), and if the end of the file comes before a closing quote, push the opening line alone and continue from the next line, so behaviour matches main for anything that is not a real multiline value. A test for each of the three cases above would pin it.

Need: yes, #6075 is real on main · Size: ok · Reuse: nothing to reuse in the repo (Node's util.parseEnv is the reference behaviour) · Simpler: already small; the fix is two guards, not more machinery

— Rames

Comment thread packages/cli/src/utils/dotEnv.ts Outdated
const lines: string[] = [];
for (let i = 0; i < rawLines.length; i++) {
const line = rawLines[i] ?? "";
const quote = openQuoteOf(line.trim());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This runs before the blank/comment check in entryOf, so # KEY="sk-... opens a quote and swallows every line after it. I got {} for a file with a commented example above two real keys; main returns both. Skipping blank and # lines here fixes it.

Comment thread packages/cli/src/utils/dotEnv.ts Outdated
while (++i < rawLines.length) {
const next = rawLines[i] ?? "";
continuation.push(next);
if (next.includes(quote)) break;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If no later line contains the quote, the loop runs to the end of the file and everything after the opening line becomes part of the value (A='x + B=2 gives A = "x\nB=2", no B). When the loop ends without a match, falling back to the opening line alone and resuming at the next line keeps main's behaviour for a stray quote.

… lines

logicalLines looked for an open quote on every physical line before
entryOf had a chance to classify it as blank or a comment, so a
commented-out example like `# KEY="sk-old` (an unclosed quote inside
a comment) opened a quote that never closed, joining every line below
it into one value and dropping the real keys that followed.

A genuinely stray unclosed quote with no later closing line had the
same failure mode: the join ran to EOF and swallowed the rest of the
file instead of leaving the malformed line on its own, so a later
file like A='x / B=2 lost B entirely.

Skip blank/comment lines before checking for an open quote (matching
entryOf's own skip), and if the closing-quote search reaches EOF
without finding one, fall back to the opening line on its own and
resume parsing from the next line. Also strip a trailing \r per line
so a CRLF multiline value doesn't carry it into the joined string.
@STiFLeR7

Copy link
Copy Markdown
Author

@miguel-heygen @jrusso1020 thanks for catching this — you were both right, and it was a real correctness regression, not a nitpick. Pushed a fix in 666bf15.

Root cause #1 (the one jrusso1020 verified by running it): logicalLines checked openQuoteOf on every physical line before the parser had a chance to classify it as blank or a #-comment. So a commented-out example like # OPENROUTER_API_KEY="sk-old opens a quote inside a comment that never closes, and the join ran to EOF, swallowing every line below it — including real keys — into one unparsed blob.

Root cause #2: a genuinely stray/malformed unclosed quote with no closing line anywhere later in the file hit the same failure mode — the join ran to EOF and ate the rest of the file instead of leaving the malformed line on its own (e.g. A='x followed by B=2 dropped B entirely).

Fix:

  • Skip blank lines and #-comment lines before ever checking for an open quote (mirrors the skip entryOf already does for real entries), so a quote opened inside a comment can't start a continuation.
  • Extracted the closing-quote scan into closingLineIndex(rawLines, from, quote). If it returns -1 (EOF reached, no closing quote found), fall back to pushing just the opening line on its own — matching main's existing behavior for a stray quote — and resume parsing from the next line instead of consuming everything after it.
  • Also stripped a trailing \r per line before parsing, so a CRLF-authored .env doesn't leak \r into a joined multiline value (per miguel's comment).

Verification: ran applyDotEnv directly against your exact repro shapes (not just the vitest assertions) — commented example above two real keys now returns both keys, the stray-quote case now preserves the key after it, and the CRLF case comes back without an embedded \r. Added regression tests for all three in dotEnv.test.ts.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the quick turnaround, all three cases from the last round are fixed. I ran applyDotEnv from this head against main and Node's util.parseEnv: a commented-out # KEY="sk-... above real keys keeps both keys, a stray unclosed quote no longer eats the rest of the file (A='x then B=2 gives both, as on main), and a CRLF multiline value comes back without \r. The three new tests fail on the previous head and pass here, so they pin the fixes.

One thing left before this can go in: the comment check will fail. node scripts/comment-ratchet.mjs reports dotEnv.ts at 13 comment lines out of 100 against 4 of 57 on main. Dropping the comments on openQuoteOf and closingLineIndex (the names already say it) and cutting the logicalLines block to two lines should clear it. While you are there, the PR description still describes only the first commit; a line on the comment skip, the end-of-file fallback and CRLF would help whoever reads it later.

The test workflows have not run on this branch yet (fork PRs need a maintainer to start them), so the new tests have not been through CI.

Verdict: COMMENT
Reasoning: All earlier correctness findings are fixed and verified, but the comment check fails locally and no required check has run at this head yet; switch to APPROVE once both are green.

…et budget

Dropped the comments on openQuoteOf and closingLineIndex (their names
already say what they do) and cut the logicalLines comment down to
two lines, bringing the file back to 6/93 comment lines against the
base's 4/57.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(cli): quoted multiline .env credentials are truncated to the first line

3 participants