fix(cli): propagate TLS env vars from .gemini/.env in parent process#26011
fix(cli): propagate TLS env vars from .gemini/.env in parent process#26011gaurav0107 wants to merge 2 commits intogoogle-gemini:mainfrom
Conversation
NODE_EXTRA_CA_CERTS and other TLS-initialization env vars defined in .gemini/.env were ignored after PR google-gemini#24667 introduced the lightweight parent / heavy child process model. Node reads these vars once at process start (before any JS runs), so by the time the child's loadEnvironment() parses .env, it is too late to influence the TLS trust store. This change keeps the parent lightweight (no dotenv import, no heavy module loads) by hand-parsing .gemini/.env for a small allowlist of env vars that Node and libcurl require at init time: - NODE_EXTRA_CA_CERTS - NODE_TLS_REJECT_UNAUTHORIZED - SSL_CERT_FILE, SSL_CERT_DIR - HTTPS_PROXY, HTTP_PROXY, NO_PROXY Values are injected into the child's spawn env only when the key is not already set in the shell environment, preserving the existing "shell env wins" rule from loadEnvironment(). Non-TLS vars continue to flow through the child's full loadEnvironment() path with its trust checks unchanged. Also adds an is-main-module guard around run() so that the helpers can be imported from unit tests without triggering a child spawn. Fixes google-gemini#25987
Drop project-local (cwd-walking) .gemini/.env lookup from the parent helper. The existing child-side loadEnvironment() already gates project files behind the full workspace-trust model; reproducing that logic in the lightweight parent would require either duplicating the longest-match + TRUST_PARENT resolution or importing heavy modules (defeating PR google-gemini#24667's goal). Scoping this first fix to HOME-only: - Resolves the reporter's own use case on google-gemini#25987 (enterprise CA cert placed in ~/.gemini/.env for the whole machine). - Closes a small security-surface widening (untrusted project .gemini/.env could have forced NODE_EXTRA_CA_CERTS into the parent env without a trust check). - Shrinks the diff and removes one exported helper (findProjectGeminiEnvFile) that is no longer needed. Project-local support remains a clean follow-up once the trust check is factored into a shared standalone helper.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a regression where TLS-initialization environment variables defined in .gemini/.env were ignored due to the recent introduction of a parent/child process model. By hand-parsing a specific allowlist of network-critical variables in the lightweight parent process and injecting them into the child's spawn environment, the fix ensures that Node.js correctly picks up these settings before the TLS layer initializes, while preserving existing precedence rules. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Marking this PR ready for review. Quick summary of what's in the patch:
Happy to iterate on scope, naming, or trust model if reviewers prefer a different cut. |
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to propagate TLS and proxy-related environment variables from a user's $HOME/.gemini/.env file to the child process. It includes a lightweight .env parser and a helper function to load allowlisted variables, ensuring that critical settings like NODE_EXTRA_CA_CERTS are available during process initialization. Additionally, the entry point was refactored to support unit testing of these new utilities. I have no feedback to provide.
Summary
Fixes #25987.
NODE_EXTRA_CA_CERTSand other TLS-initialization envvars defined in
.gemini/.envwere ignored after PR #24667 introducedthe lightweight parent / heavy child process model, because the
child's Node TLS layer initializes from
process.envbefore itsloadEnvironment()call ever runs.Details
Node reads
NODE_EXTRA_CA_CERTS,SSL_CERT_FILE,HTTPS_PROXY, etc.once at process start, in C++ before any user JS runs. Pre-#24667 the
CLI ran as a single process, so there was no observable ordering
problem — no outbound TLS happened until after
.envwas loaded.After #24667 the child is spawned with a pre-built env, and by the
time the child's JS gets around to parsing
.envthe TLS layer isalready initialized without the cert.
This PR keeps the parent lightweight (no
dotenvimport, no heavymodule loads) by hand-parsing
.gemini/.envfor a small allowlist ofenv vars that Node and libcurl require at init time:
Values from `.gemini/.env` are injected into the child's spawn env
only when the key is not already set in the shell env, preserving
the existing "shell env wins" rule from `loadEnvironment()`.
Lookup order (matches `findEnvFile` in `settings.ts`):
Scope is deliberately narrow: seven well-known network/TLS vars, and
only the `.gemini/.env` path (not arbitrary `.env`). Non-TLS vars
continue to flow through the child's full `loadEnvironment()` path
with its trust checks unchanged.
The PR also adds a small is-main-module guard around `run()` so the
new helpers (`parseSimpleEnv`, `findProjectGeminiEnvFile`,
`loadTlsEnvFromGemini`) can be imported from unit tests without
triggering a child spawn.
Related Issues
Fixes #25987
Related to #24667 (introduced the regression)
How to Validate
`NODE_EXTRA_CA_CERTS=/path/to/cert.pem`.
that cert (e.g. `gemini mcp list` against an internal MCP server).
Also verified:
failures on my machine (`extension_integrity.json` local-state
issue) also fail on stock `main`, so they are not regressions
from this PR.
Pre-Merge Checklist