fix: KEEP-1049 compare CLI version against server floor, stop showing API keys as identities - #84
Merged
Conversation
kh doctor's CLI Version check only ever printed the local version and unconditionally passed - the server now advertises a floor via KH-Minimum-CLI-Version (KEEP-1047), but nothing compared against it. checkCLIVersion probes /api/health for the header and reuses khhttp.SemverLessThan to warn, naming the remedy, when the local build is behind.
fetchAPIKeyInfo has no real identity for an API key, so it sets Email to a truncated key prefix (kh_EU7Fc1Xi...). kh auth status and kh auth login printed that prefix under "User" / "logged in as", which reads as though the key were the account. Both now name it as a credential for API-key auth while leaving session auth unchanged.
1 task
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
Closes out the remaining scope from KEEP-1049 after #75 fixed the two
headline bugs (kh doctor false-positive auth, kh auth status accepting any
kh_... string). Two things were still open:
1.
kh doctor's CLI Version check never compared anything.The server has advertised a floor via
KH-Minimum-CLI-Versionsince KEEP-1047(keeperhub#1839), and
internal/http/version.goalready warned on it forevery request made through the retryable client - but
checkCLIVersion(
cmd/doctor/doctor.go) only ever printed the local version string andunconditionally passed. It now probes
/api/health(the same endpointcheckAPIalready hits), reads the header, and reuses the already-exportedkhhttp.SemverLessThanto warn with the same remedy text:2.
kh auth status/kh auth loginshowed the API-key prefix as an identity.fetchAPIKeyInfohas no real identity for an API key, so it setsEmailto atruncated prefix (
kh_EU7Fc1Xi...). Both commands printed that under"User" / "logged in as", which reads as though the key were the account:
Now labeled/worded as a credential for API-key auth; session auth is
unchanged.
Test plan
go build ./...,go vet ./...,go test -race ./...(full suite)go generate ./docs/produces no diffcmd/doctor/doctor_version_test.go(no header / below floor /at floor / dev build),
cmd/auth/status_test.goandcmd/auth/login_apikey_test.go(API-key vs session wording)Companion doc fix in keeperhub, documenting the
KH_MINIMUM_CLI_VERSIONoverride this relies on: KeeperHub/keeperhub#1861