Repository navigation
Fix variable interpolation in .env files loaded through envFile - #14828
Abdelhadi Salmaoui (abdelhadi703) wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Interpolation incorrectly expands quoted or escaped literals, and the tests are incomplete and environment-dependent.
2 open findings
What changed in this PR
Adds dotenv-style ${VAR} interpolation for debugger envFile parsing.
Changes:
- Resolves references from prior entries and the process environment.
- Preserves unknown references.
- Adds tests and changelog documentation.
| File | Description |
|---|---|
Extension/src/Debugger/ParsedEnvironmentFile.ts |
Implements interpolation. |
Extension/test/scenarios/SingleRootProject/tests/ParsedEnvironmentFile.test.ts |
Adds interpolation tests. |
Extension/CHANGELOG.md |
Documents the fix. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The quotes come off before the references are resolved, so VALUE='${SECRET}' was expanded and
a \${SECRET} reference kept its backslash while losing its meaning. Both are literal in dotenv,
and both were literal before the interpolation existed.
Adds the missing test for the process-environment fallback the changelog advertises.
|
Both addressed in 66ea67b. The quotes came off before the references were resolved, so you are right on both counts: The process-environment fallback has a test now as well: it sets a name of its own, parses a reference to it, and restores the previous value in a 13 passing against the first commit of this branch. The two literal tests fail there -- |
| if (!singleQuoted) { | ||
| value = value.replace(/(\\)?\$\{([\w\.\-]+)\}/g, (full: string, escape: string | undefined, name: string): string => { | ||
| if (escape) { | ||
| return full.substring(1); |
| * Reduce CPU usage when resolving headers in large Windows workspaces. | ||
|
|
||
| ### Bug Fixes | ||
| * Fix `${VAR}` interpolation in `.env` files used by `envFile`: references to variables defined earlier in the same file (or in the OS environment) are now resolved. Single-quoted values and `\${VAR}` are left as written. [#13781](https://github.com/microsoft/vscode-cpptools/issues/13781) |
.envfiles are supposed to support variable interpolation, but when such a file isloaded through
envFile, the values keep their${...}references and the variable isnever substituted.
With a file like
EMAILcame out as the literal${USER}@example.org. Later, when the debugconfiguration gets expanded,
${USER}is looked up as alaunch.jsonvariable, isn'tfound, and prints "Invalid variable reference ${USER} in string". The env var that was
meant to land in the debuggee doesn't.
I made
ParsedEnvironmentFileresolve${VAR}while it parses each line, against thevariables already read from the file plus the process environment. A reference to a name we
don't know is left untouched, so
${workspaceFolder}and the other launch.json variableskeep working exactly as before.
Only a reference the value actually asked to expand is expanded. A single-quoted value and a
\${VAR}reference are literal in dotenv, and both were literal here before this change, sothey now stay as written -- the backslash itself is dropped, leaving the text
${VAR}.Six tests were added to the existing
ParsedEnvironmentFilesuite: a reference to a variabledefined earlier in the same file, a reference coming from the initial environment, an unknown
reference staying literal, a reference resolved from the process environment, and the two
literal forms above.
Testing: ran the
ParsedEnvironmentFilesuite against the parser alone, compiled with therepo's TypeScript and a local
vscode-nlsstand-in (mocha 10 does not start on Node 26, sothe compiled file runs under a short TDD shim). 13 passing with this change.
Against the first commit of this branch the two literal tests fail --
admin@example.orgwhere
${USER}@example.orgis expected for the single-quoted one, and\admin@example.orgfor the escaped one -- while the other 11 pass either way.
Fixes #13781.