Restore the token-target fix onto develop - #34
Merged
Conversation
SetToken reaches only the current target, so a command told to act on another one with -T had to move the current target aside and put it back, and Write persists whatever it finds in between.
logout cleared the token of the current target and then reported that it had logged out of the one -T named, so it left that Vault logged in and signed the user out of another. A logout with no target selected at all reported success too, having dropped the error that says so.
auth reached the target -T named by making it current, setting the token, and switching back, which is what SetTokenFor now does without the detour. It also dropped the error from setting the token, so a target that had gone missing looked like a success.
init put the root token of the Vault it had just initialized onto the current target, leaving the initialized one with no token at all, and read its Strongbox flag from the current target too.
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.
Restores the content of #32, which merged but never reached
develop.#32 was opened against
target-flag-selects-vaultbecause it was stacked on#31. #31 merged into
developfirst, so by the time #32 merged, its basebranch was already merged and discarded — the four commits landed on a branch
nothing reads. GitHub reports #32 as merged, and
git merge-base --is-ancestor 0211147 origin/developsays no.This is the same accident that lost #29 and needed #30 to recover.
The four commits are cherry-picked unchanged; the only difference between this
branch and
target-flag-selects-vaultis the README, whichdevelopgainedfrom #30 and the stale branch never had.
make checkandgo test -race ./...are green.
What the commits fix
Config.SetTokenonly reaches the current target, andrc.Applydoes not movethe current target — so with
-T(or$SAFE_TARGET), every command that storesa token wrote it against the wrong
~/.safercentry.safe -T beta logoutprinted "Successfully logged out of beta", left beta'stoken live in the file, and cleared alpha's instead. Verified against two live
dev Vaults.
safe -T beta initstores the new root token against the current target,leaving the Vault it just initialized with no recorded token.
safe -T beta auth ...already worked, via a hand-rolled save-and-restore ofCurrentaroundSetToken. That dance is what pointed at the defect in thefirst place; it collapses into the new call.
New
rc.SetTokenFor(alias, token)names its target without touchingCurrent— which matters because
Config.Write()persistsCurrent, so any in-memoryswitch would outlive the command that
-Twas supposed to scope.SetTokennow delegates to it. Two silently dropped errors on the auth path now surface.
Tests cover logout against a named target, logout with nothing targeted, auth
storing against a named target, and init storing the root token against a named
target, plus the
rcunit test forSetTokenFor.