fix(root): stop loading .env at runtime (silent credential shadow)#204
Merged
Conversation
PersistentPreRunE called godotenv.Load() on every command to pick up VERS_URL from a local .env for dev convenience. The side effect was that any stale VERS_API_KEY in a .env file in the user's cwd would silently override their ~/.versrc — with no warning about which credential source the SDK actually used. Repro: be in a directory with a .env containing a stale VERS_API_KEY, run any 'vers' command. The CLI authenticates as the stale key's identity instead of the configured user, returning a foreign org's VMs. Took ~20 minutes of debugging (including 'vers status --verbose' to see the wire-level Authorization header) to discover the .env was the credential source. Removal rationale: - The CLI ships to end users; reading arbitrary .env from cwd is a footgun, not a feature. - Tests already load their own .env explicitly via test/testutil/helpers.go — runtime removal does not affect them. - Devs who want .env loading have direnv or 'export $(cat .env | xargs)'. If we want to restore this behavior we should: 1. Gate it behind an explicit opt-in (e.g. VERS_USE_DOTENV=1). 2. Emit a stderr line when .env contributes a credential or base URL, so the credential source is always visible.
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.
Summary
Removes the unconditional
godotenv.Load()inPersistentPreRunE. It was originally added as a dev convenience for picking upVERS_URLfrom a local.env, but it also readsVERS_API_KEY, which silently shadows the user's~/.versrcwhenever they happen to runversin a directory containing a.envfile — with no warning about which credential source the SDK actually used.Repro
~/.versrc.cdinto a directory with a.envcontaining a staleVERS_API_KEY(e.g. this repo — there's been one checked in locally for months).vers status.Expected: VMs for the org the
~/.versrckey belongs to.Actual: VMs for whatever identity the stale
.envkey happens to belong to — usually a different org. No indication anywhere that.envwon.Took ~20 minutes of debugging (including
vers status --verboseto read the wire-levelAuthorizationheader) to discover.envwas the credential source. The Verbose log shows the third, mystery key being sent:Why removal (not a flag)
.envfrom the user's cwd is a footgun, not a feature..envexplicitly viatest/testutil/helpers.go— runtime removal does not affect them..envloading havedirenvorexport $(cat .env | xargs).VERS_USE_DOTENV=1) would still leave the door open to credential shadowing for anyone who set it once and forgot.If we want this back
Make it opt-in and visible:
VERS_USE_DOTENV=1(or similar).stderrline whenever.envcontributes a credential or base URL, so the credential source is never invisible.Happy to do that as a follow-up if anyone leans on the current behavior.
Related AX gaps surfaced by this debugging session
Not fixed here, worth tracking separately:
vers whoamito show "you are user X in org Y, talking to api.vers.sh." Five seconds of identity output would have caught this immediately.VERS_ORG=hdr-is vers statusis accepted silently and does nothing when the API key isn't valid for that org. Should either scope the call or error.vers statusgives no indication of which org's VMs it's listing.