Don't inject empty strings into process.env for unset schema items - #1038
Merged
Conversation
Auto-load injected '' for items resolving to undefined while varlock run skipped them. Unify on skipping everywhere (runtime injection, run, shell format), matching the documented VAR= semantics, and add the @injectUndefinedAsEmpty root decorator to opt back into dotenv-style empty-string injection. Blob provenance/drift checks are now mode-aware.
Contributor
|
The changes in this PR will be included in the next version bump.
|
Contributor
📦 Bundle size
dist/ only; native binaries are versioned separately and not counted here. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
varlock-website | 7da34fb | Commit Preview URL Branch Preview URL |
Aug 27 2026, 04:18 AM |
varlock
@varlock/native-helper-darwin
@varlock/native-helper-linux-arm64
@varlock/native-helper-linux-x64
@varlock/native-helper-win32-x64
commit: |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Important
The generated ImportMetaEnv contract becomes unsound when @injectUndefinedAsEmpty is enabled. This should be corrected before merge.
Reviewed changes in 823e159 across runtime injection, CLI output, blob provenance and reuse, generated TypeScript declarations, documentation, and focused coverage.
- Undefined injection: Unset schema items are omitted from
process.env, child environments, and shell output by default while explicit empty strings remain distinct. - Compatibility decorator:
@injectUndefinedAsEmptyrestores empty-string injection and is serialized for runtime reuse and provenance checks. - Type generation: String-form declarations preserve optional literal unions and attempt to reflect the selected injection mode.
- Coverage and docs: Runtime and smoke tests exercise both modes, with guides and reference pages documenting the behavior.
azure/gpt-5.6-sol ｜ 𝕏
…tUndefinedAsEmpty
The empty-string injection guarantee only covers process.env; frameworks expose only prefixed keys through import.meta.env, so its augmentation must keep optional keys. Split the string-form type: ProcessEnv extends a required variant with '' unions, ImportMetaEnv keeps the optional one.
Contributor
There was a problem hiding this comment.
Important
The generated ProcessEnv type is still unsound for consumers that disable strictNullChecks. This should be corrected before merge.
Reviewed changes since the prior Pullfrog review, covering the target-specific TypeScript alias fix and its documentation and test updates.
- Clarified loader scope: Documented that blob-based non-TypeScript loaders retain their language-specific absent representation regardless of
@injectUndefinedAsEmpty. - Separated environment declarations: Kept
ImportMetaEnvon the optional string schema while applying required empty-string-aware fields only toProcessEnv. - Expanded source assertions: Checked that generated declarations select distinct aliases for
ProcessEnvandImportMetaEnv.
azure/gpt-5.6-sol ｜ 𝕏
…SSR runtime Smoke tests now typecheck the generated env.d.ts in both modes with tsc --strict (ts-expect-error assertions pin process.env vs import.meta.env optionality). Vite framework tests build an SSR entry with resolved-env inject mode and execute it with plain node, asserting unset items are skipped from process.env by default, become '' under @injectUndefinedAsEmpty, and never appear on import.meta.env.
Contributor
There was a problem hiding this comment.
ℹ️ No new issues found in the added coverage. The existing open type-soundness thread still applies.
Reviewed changes since the prior Pullfrog review, covering the new generated-type smoke checks and Vite SSR runtime coverage.
- Added compiled type checks: Generated declarations for default and empty-string injection modes are now checked with
tsc --strict, including the distinctprocess.env,import.meta.env, andENVcontracts. - Added Vite SSR runtime checks: Vite 5 through 8 now build and execute both injection modes to verify unset values across
process.env,import.meta.env, and theENVproxy.
azure/gpt-5.6-sol ｜ 𝕏
'undefined extends T' is evaluated under the consumer's compiler settings,
so with strictNullChecks disabled it is true for every type and required
literal unions would gain ''. Use '{} extends Pick<...>' instead, and add
a non-strict tsc pass to the smoke type tests covering required/optional
enum and boolean fields.
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, covering the structural optionality fix and its compiler-level regression coverage.
- Corrected non-strict generated types: Replaced the
strictNullChecks-dependent conditional with structural optional-key detection, preserving required literal unions while adding''only to optional fields. - Added non-strict compiler coverage: Extended the smoke schema with required and optional literal fields and verified the generated declaration without
strictNullChecks.
azure/gpt-5.6-sol ｜ 𝕏
The type-test fixtures previously shadowed the process global with a local declaration, bypassing the merge between the generated ProcessEnv augmentation and @types/node's own ProcessEnv. Use the real global instead and assert that named schema props win over the index signature (definite string in empty mode) while non-schema keys keep string | undefined.
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, covering the generated-type fixtures' switch to the real Node environment declarations.
- Exercised declaration merging: Replaced local
processstand-ins with@types/node, so schema-key guarantees are now checked against Node's actualProcessEnvindex signature. - Pinned unaffected keys: Added compiler assertions that ordinary Node and unknown environment keys remain
string | undefinedwhile named schema keys retain their mode-specific types.
azure/gpt-5.6-sol ｜ 𝕏
This was referenced Aug 31, 2026
theoephraim
added a commit
that referenced
this pull request
Sep 1, 2026
…napshot (#1055) When a server boots from the env blob baked into build output (e.g. a Next.js standalone container where the varlock CLI is unreachable), initVarlockEnv treated the blob like a fresh resolution. Items that resolved to undefined at build time triggered the stale-echo cleanup from #1038, DELETING the corresponding runtime-provided values from process.env. A schema item like `REDIS_URL=` would actively clear a `docker run -e REDIS_URL=...` value at boot, silently breaking the service. Shipped only in varlock@1.17.1. That cleanup assumes a resolution happened in this process (a genuine ambient value would have acted as an override and resolved to it), which is false for a blob resolved on a build machine. The injection preludes (nextjs webpack + turbopack, vite resolved-env SSR entry) now bake an `injectedAtBuild: true` flag INSIDE the serialized payload, before encryption where applicable, and initVarlockEnv skips the cleanup for a flagged blob. Provenance lives in the payload so it travels with the blob to child processes and through encryption round-trips, and can never outlive it: any fresh resolution produces an unflagged blob, so no marker clearing is needed anywhere. Otherwise unchanged: baked values stay authoritative for ENV, and runtime values are still not applied to a baked snapshot (they cannot be validated or coerced against it). Making that contract explicit and enforceable belongs with `varlock freeze` (#1049).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A user migrating a Next.js app to varlock hit regressions because auto-load injected
''intoprocess.envfor schema items that resolved to undefined, breakingprocess.env.SOMEVAR ?? 'default'patterns. Meanwhilevarlock runskipped those items entirely, so the two injection paths disagreed, and the docs promise thatVAR=(undefined) andVAR=""are distinct.This unifies on not injecting unset items, everywhere:
initVarlockEnv(): items resolving to undefined are no longer injected as''; a stale parent-injected echo for such a key is cleared (matching the maskingvarlock runalready does). Explicit""values are still injected as empty strings.varlock load --format shell: skips unset items (export KEY=would set''in the shell, misrepresenting undefined).--format envkeeps itsKEY=lines since those round-trip to undefined in the varlock dialect.varlock run: unchanged (already skipped), but now honors the new decorator below.@injectUndefinedAsEmptyNew root decorator that restores dotenv-style empty-string injection across all three surfaces, for code that relies on unset vars being
''. It travels in the__VARLOCK_ENVblob settings, and the blob provenance/drift checks are mode-aware: in skip mode an explicitly-set ambient''now correctly reads as an override rather than a parent echo.Generated TS types follow the setting: the
process.envaugmentation drops optionality when it is enabled, and any key that can be unset gets''added to its union, so an optionalenum(alpha, beta)types as"alpha" | "beta" | "". Theimport.meta.envaugmentation keeps its optional keys: frameworks only expose prefixed keys through it, so the injection guarantee does not cover it. TheENVproxy typing keeps optional keys optional either way, sinceENV.Xstill returns the real resolved value. As a side effect of the same fix (strippingundefinedbefore the conditional withNonNullable), optional enums/booleans in default mode now keep their literal unions onprocess.envinstead of widening tostring. Because codegen reads the decorator, its value must be a static boolean (same rule as@disableProcessEnvInjection).Testing
varlock run, and both load formats in both modestsc --stricttype tests in both modes (@ts-expect-errorassertions pin process.env vs import.meta.env optionality)ssrInjectMode: 'resolved-env'and execute it with plain node, asserting unset items are skipped from process.env by default, become''under the decorator, and never appear onimport.meta.envin either modeDocs updated: root decorator reference, schema guide, dotenv migration guide, and load/run CLI reference.