fix(cli): honor global --base-url and --api-key in auth commands - #6
Merged
Conversation
Every command routes its client through get_client(ctx), which reads the
global options the root callback stashes on ctx.obj — except auth login
and auth status, which took no ctx and built a client directly.
The visible damage was auth status reporting {"valid": true} against a
host it never contacted:
$ discolike --base-url http://127.0.0.1:9 auth status
{"source": "env", "api_key": "…isco", "valid": true}
while the same override on any other command correctly refused to
connect. --api-key was ignored too, so there was no way to verify a key
before auth login wrote it to disk.
auth status gains a third source value, "option", distinguishing a key
passed on the command line from one inherited via DISCOLIKE_API_KEY.
That distinction has to come from click's parameter source, since the
global --api-key is env-bound and the value alone cannot tell them apart.
The same distinction keeps auth login's prompt intact: an ambient
DISCOLIKE_API_KEY must not silently become the saved credential, or
anyone holding a production key in their environment would persist the
wrong one while trying to store another. Only an explicit flag skips the
prompt.
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
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
auth loginandauth statusignored the global--base-urland--api-keyoptions. Every other command builds its client viaget_client(ctx), which reads what the root callback stashes onctx.obj; these two took noctxand calledbuild_client(...)directly.The visible damage —
auth statusreporting success against a host it never contacted:--api-keywas ignored too, so there was no way to verify a key beforeauth loginwrote it to disk.After
Behaviour notes for reviewers
auth statusgains a thirdsourcevalue:option. It previously emitted onlyenvorconfig. A key passed on the command line is now reported asoption. This is additive, but anything parsingsourceshould know. The distinction has to come from click's parameter source — the global--api-keyis declared withenvvar="DISCOLIKE_API_KEY", soctx.obj["api_key"]is already populated from the environment and the value alone cannot tell the two apart.auth login's prompt is deliberately preserved. An ambientDISCOLIKE_API_KEYdoes not skip the prompt — only an explicit flag does. Letting the env var through would mean anyone holding a production key in their environment silently persists it toconfig.jsonwhile trying to save a different one. Two regression tests cover this; one fails if that guard is removed.Verification
loginprompt behaviour).--base-url/--api-keytests fail against the pre-fix code.ruff checkandruff format --checkclean.Related
Found while writing the CLI reference for the docs site, which had to document this as a caution. That caution comes out once this lands.
Greptile Summary
The PR makes
auth loginandauth statushonor global API-key and base-URL options while preserving the rule that environment-provided keys do not bypass the login prompt.Confidence Score: 5/5
The PR appears safe to merge with no actionable correctness, security, or compatibility issues identified.
The changed auth paths forward both supported global client settings, select credentials according to the documented precedence, and are covered by targeted regression tests.
Important Files Changed
optionstatus source.Reviews (1): Last reviewed commit: "fix(cli): honor global --base-url and --..." | Re-trigger Greptile