Honor populated variables before dotenv defaults in ConfigProvider

[fubhy]

Summary

With variable expansion enabled, ${SET:-fallback} resolves to fallback even when SET contains actual.

Important

This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.

Dotenv default overrides a populated variable

Module: ConfigProvider
Audit ID: core-a-f-config-provider-dotenv-default-precedence
Severity / confidence: medium / high

What happens

With variable expansion enabled, ${SET:-fallback} resolves to fallback even when SET contains actual.

Why it happens

interpolate replaces the match with defaultValue || parsed[variableName], so every non-empty default wins before the referenced variable is checked.

Expected behavior

Dotenv expansion follows dotenv and dotenv-expand semantics: ${NAME:-fallback} uses NAME when populated and fallback only when the variable is unset or empty.

Relevant implementation

These links and excerpts are pinned to audit base c9b56ab507f224426ee8388dc450da447ec4715f.

• packages/effect/src/ConfigProvider.ts:1071-1103
• packages/effect/src/ConfigProvider.ts:1101

View problematic code at packages/effect/src/ConfigProvider.ts:1071-1103

function interpolate(envValue: string, parsed: Record<string, string>): string {
  // find the last unescaped dollar sign in the
  // value so that we can evaluate it
  const lastUnescapedDollarSignIndex = searchLast(envValue, /(?!(?<=\\))\$/g)

  // If we couldn't match any unescaped dollar sign
  // let's return the string as is
  if (lastUnescapedDollarSignIndex === -1) return envValue

  // This is the right-most group of variables in the string
  const rightMostGroup = envValue.slice(lastUnescapedDollarSignIndex)

  /**
   * This finds the inner most variable/group divided
   * by variable name and default value (if present)
   * (
   *   (?!(?<=\\))\$        // only match dollar signs that are not escaped
   *   {?                   // optional opening curly brace
   *     ([\w]+)            // match the variable name
   *     (?::-([^}\\]*))?   // match an optional default value
   *   }?                   // optional closing curly brace
   * )
   */
  const matchGroup = /((?!(?<=\\))\${?([\w]+)(?::-([^}\\]*))?}?)/
  const match = rightMostGroup.match(matchGroup)

  if (match !== null) {
    const [_, group, variableName, defaultValue] = match

    return interpolate(
      envValue.replace(group, defaultValue || (Object.hasOwn(parsed, variableName) ? parsed[variableName] : "")),
      parsed
    )

View exact lines on GitHub

View problematic code at packages/effect/src/ConfigProvider.ts:1101

      envValue.replace(group, defaultValue || (Object.hasOwn(parsed, variableName) ? parsed[variableName] : "")),

View exact lines on GitHub

Reproduction

pnpm test --run packages/effect/test/ConfigProviderDotEnvDefault.test.ts

Observed failure: The provider loaded fallback instead of actual.

Implementation handoff

The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.

1. Start with the pinned implementation excerpts and the Why it happens analysis above.
2. Change the implementation so it satisfies the stated Expected behavior; do not weaken or remove the reproduction assertions.
3. Run the focused reproduction command(s) and confirm the observed failures become passing tests:

pnpm test --run packages/effect/test/ConfigProviderDotEnvDefault.test.ts

4. Run the affected package's existing tests, then the repository lint and type checks before requesting review.

Audit provenance

• Audit base: c9b56ab507f224426ee8388dc450da447ec4715f
• Reproduction base: c9b56ab507f224426ee8388dc450da447ec4715f
• Findings: core-a-f-config-provider-dotenv-default-precedence
• Initial patch: focused reproduction tests; implementation fix pending

[changeset-bot]

⚠️ No Changeset found

Latest commit: 0169e67

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[pullfrog]

Important

The reproduction test is accurate, but the implementation fix described in the PR body is missing, so this PR cannot merge as-is.

Reviewed changes

This PR adds a new test file, packages/effect/test/ConfigProviderDotEnvDefault.test.ts, containing one regression test that demonstrates the dotenv expansion precedence bug: ${SET:-fallback} resolves to fallback when SET is populated.

• Verified the new test fails against main with the expected error (value: 'fallback' received, value: 'actual' expected).
• The implementation change shown in the PR body as required has not been committed yet.

⚠️ Implementation fix is missing

The bug is in packages/effect/src/ConfigProvider.ts:1101. The current replacement:

envValue.replace(group, defaultValue || (Object.hasOwn(parsed, variableName) ? parsed[variableName] : ""))

evaluates defaultValue first, so any non-empty default wins before the variable is consulted. Per dotenv/dotenv-expand semantics, ${NAME:-fallback} should use NAME when populated and fallback only when NAME is unset or empty.

Please add the implementation change. A minimal fix is:

const value = Object.hasOwn(parsed, variableName) && parsed[variableName] !== ""
  ? parsed[variableName]
  : defaultValue ?? ""
envValue.replace(group, value)

ℹ️ Nitpicks

• Consider adding two more cases to the new test file: an unset variable (${UNSET:-fallback}) and an empty variable (${EMPTY:-fallback}). This would fully specify the fallback behavior and guard against a partial implementation.
• A dedicated test file is fine, but these assertions could also live in the existing ConfigProvider dotenv expansion describe block; either location is acceptable.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Consider adding cases for an unset variable (${UNSET:-fallback}) and an empty variable (${EMPTY:-fallback}) so the fallback side of the ${...:-...} semantics is also specified.