Repository navigation
Preserve existing SSH connection commands - #14925
Draft
williammartin wants to merge 2 commits into
Draft
williammartin wants to merge 2 commits into
williammartin wants to merge 2 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 965836ef-19ac-4473-ba1f-e326ce0f106b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 965836ef-19ac-4473-ba1f-e326ce0f106b
2 of 6 tasks
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The SSH dispatcher can connect to the wrong destination or recursively relaunch itself, while shell validation and environment-command test coverage also need fixes.
4 open issues
What changed in this PR
This draft prototype extends GitHub CLI’s identity-based SSH routing to preserve compatible existing connection commands and literal OpenSSH options. It builds on the identity stack and remains explicitly not intended for merge.
Changes:
- Resolve underlying SSH commands from environment or Git configuration, with compatibility checks.
- Add integration coverage for persistent identities, account overrides, rollback, and cleanup.
- Add an isolated SSH scenario replay entry point.
| File | Description |
|---|---|
script/multiaccount-review/06-ssh.sh |
Adds the isolated SSH replay launcher. |
pkg/cmd/root/ssh_cooperation_test.go |
Tests command preservation across identity selection, overrides, and unset. |
pkg/cmd/auth/use/ssh_cooperation_test.go |
Tests unsupported setups and rollback on include-order conflicts. |
pkg/cmd/auth/identity/delete_test.go |
Checks that cleanup preserves the original SSH command. |
internal/identity/store_test.go |
Removes the obsolete blanket rejection test for custom SSH commands. |
internal/identity/ssh_command.go |
Resolves compatible connection commands and validates their syntax and variant. |
Review effort: Balanced
| return nil, fmt.Errorf("cannot parse SSH command: %w", err) | ||
| } | ||
| } | ||
| if isSSHDispatcher(command) { |
| return nil, errors.New("the configured SSH command has unknown argument conventions; set ssh.variant=ssh only if it accepts OpenSSH arguments and honors the supplied destination") | ||
| } | ||
| return []string{"ssh"}, nil | ||
| return command, nil |
Comment on lines
+49
to
+50
| if strings.ContainsAny(value, "\r\n$`;&|<>") { | ||
| return nil, errors.New("identity routing cannot safely preserve SSH shell expressions; your existing SSH configuration has not been changed") |
Comment on lines
+32
to
+35
| value := os.Getenv(commandEnv) | ||
| if value == "" { | ||
| var err error | ||
| value, err = s.underlyingGitValue(dir, "core.sshCommand") |
This branch has not been deployed
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.



Description
Follow on from #14924
Basic SSH routing rejects existing connection commands, including literal OpenSSH options. This PR preserves compatible connection commands while routing to the selected identity alias.
Layer 6 of an eight-part prototype review stack. This is a way to review the design and implementation in smaller pieces, not a proposal to merge the current stack as-is.
How did you test this change?
I read the original and managed Git files before and after applying the identity, then checked that unset exposed the original command. I also cloned and pushed to a private repository over real SSH while retaining the existing compression option.
A personal SSH clone for one command leaves the work identity selected
multiaccount-layer-6-02-A-personal-SSH-clone-for-one-command-leaves-the-work-identity-selected.mp4
Work SSH identity: directory inheritance preserves normal OpenSSH options
multiaccount-layer-6-03-Work-SSH-identity-directory-inheritance-preserves-normal-OpenSSH-options.mp4
Key points
The global file retains the original
core.sshCommand. Its managed include supplies the effective dispatcher andssh.variant, while the dispatcher discovers the underlying literal command. This is compatibility with existing options, not the introduction of the dispatcher.Unknown argument conventions, unsupported variants, recursive dispatch and shell expressions are refused. The recordings use temporary, independently verified keys rather than a simulated connection.
Notes for reviewers
The live recordings use
DEMO_SSH_TRACE=1, which is recording infrastructure, not a product flag. It displays selected real OpenSSH diagnostics and the effective compression setting. It does not choose the account or alter identity routing.I do not know whether this preserves a particular enterprise certificate policy. The recordings do not establish that.
The recordings use the product code at this layer, not a later stack endpoint. The included offline replay scripts are supplementary and do not reproduce the live-account recordings.
Offline replay on macOS
Build this layer, then run its synthetic scenario with an absolute executable path. This is a replay instruction, not a claim that the command below was used to make the videos.
Authorship and follow-up
Who wrote this:
Who answers review comments:
Acceptance Criteria
Prototype behaviour under review:
Given an existing SSH command uses supported literal OpenSSH arguments.
When I select a routed identity or use a one-command override.
Then the connection options are preserved while the declared alias determines the destination.
DO NOT MERGE