Repository navigation
fix(macos): never save a jump host's SSH password for the destination - #273
Merged
Merged
Conversation
SSHAttempt kept every password answer, so a ProxyJump host's password could be saved under the destination and later sent to it. Keep and offer to save only the login account's password, and count retries per account so the destination's first question after a jump host's isn't shown as a retry. Closes #270
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.
Fixes #270, found in the macos-v0.6.0 release review.
Problem
SSHAttempt.answerkept every password answer, whatever account the prompt named. The sign-in sheet offered Save in Keychain on aProxyJumphost's password too, andkeep()saved it under the destination. The next time the destination asked, Kit sent it the jump host's password. Separately, the retry counter was keyed only by "password", so the destination's first question after a jump host's said "That password didn't work."Change
SSHPromptRequest.canSave: false for another account's password. The sheet then hides Save in Keychain and says "Kit sends it to ssh and doesn't keep it."SSHAttemptkeeps a password answer only for the login account (isLogin: no account named, oruser@hostnamefromssh -G), the same rule that already governed sending a saved password.SSHPrompt.saveable, which only a test used and which now misstated the rule.docs/features/attach.mdsay only the login account's password can be saved.Tests
aJumpHostsPasswordIsNeverSavedForTheDestination: jump host, then destination; only the destination's password is saved, and neither question is a retry.aJumpHostsPasswordAloneIsNeverSaved: a jump host's password with Save chosen leaves the Keychain empty.aStaleSavedPasswordBehindAJumpHostIsDroppedNotReplacedByTheJumpHosts: the stale saved password goes only to the destination, and the user's new answer replaces it.anotherAccountsPasswordCantBeSaved: the sheet has no save toggle and the adjusted message.The first two fail with the behavior change reverted. Full
xcodebuild … test: 1000 tests pass.generate_openapi.sh --check,bash -non the scripts, andgit diff --checkpass.The code-reviewer subagent found no blocking issues. Its two low findings are pre-existing and filed as a follow-up.