Skip to content

Fix SetEnv SSH config parsing and accumulation #684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
EhabY wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix SetEnv SSH config parsing and accumulation #684

EhabY wants to merge 2 commits into from
+219 −32

Conversation

@EhabY
Copy link
Collaborator

@EhabY EhabY commented Dec 15, 2025 •
edited
Loading

  • Add parseSshConfig function to correctly parse SetEnv MY_VAR=value (previously split on first = producing incorrect key/value)
  • Modify mergeSshConfigValues to concatenate SetEnv values instead of replacing, preserving the default CODER_SSH_SESSION_TYPE
  • Ignore empty SetEnv values to prevent accidentally clearing defaults

Closes #559

deansheather
deansheather reviewed Dec 15, 2025
View reviewed changes
Copy link
Member

@deansheather deansheather left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other ssh options that can be specified multiple times? Would be nice to support those as well

EhabY added 2 commits December 16, 2025 10:58
@EhabY
Fix SetEnv SSH config parsing and accumulation
33f6238 
- Add parseSshConfig function to correctly parse "SetEnv MY_VAR=value"
  (previously split on first "=" producing incorrect key/value)
- Modify mergeSshConfigValues to concatenate SetEnv values instead of
  replacing, preserving the default CODER_SSH_SESSION_TYPE
- Ignore empty SetEnv values to prevent accidentally clearing defaults
@EhabY
Handle spaces when concatenating environment variables
e2ea071
@EhabY EhabY force-pushed the fix-setenv-ssh-config branch from d80e482 to e2ea071 Compare December 16, 2025 07:58
@EhabY
Copy link
Collaborator Author

EhabY commented Dec 16, 2025 •
edited
Loading

Are there any other ssh options that can be specified multiple times? Would be nice to support those as well

I had Claude look into the ssh_config documentation and here is the full list. We only care about multiple entries so in this case, it would be IdentityFile, CertificateFile, SetEnv, SendEnv, LocalForward, RemoteForward, DynamicForward can be accumulating. Alternatively, why not let the SSH client handle it? Like we can just pass the configs as is (override configs would be the put first since it's "first match wins"). I do not love that we have to re-implement this override behavior ourselves 🤔. If we want leaner files then we can still apply the overriding but we can always add entries from the list above as is (we do not manually combine them but pass them as is, such that override is earlier than the base config).


Claude's Analysis

SSH Client (ssh_config) Accumulating Options

Options that accumulate across multiple declarations, unlike standard "first match wins" behavior.

Option Example Notes
IdentityFile IdentityFile ~/.ssh/id_ed25519
IdentityFile ~/.ssh/id_rsa		 Tried in sequence
CertificateFile CertificateFile ~/.ssh/cert1.pub
CertificateFile ~/.ssh/cert2.pub		 Tried in sequence
SetEnv SetEnv FOO=bar
SetEnv BAZ=qux		 Both sent to server
SendEnv SendEnv LANG LC_*
SendEnv MY_VAR		 Prefix with - to clear
LocalForward LocalForward 8080 localhost:80
LocalForward 9090 localhost:90		 Multiple tunnels
RemoteForward RemoteForward 8080 localhost:80 Multiple tunnels
DynamicForward DynamicForward 1080
DynamicForward 1081		 Multiple SOCKS proxies
GlobalKnownHostsFile GlobalKnownHostsFile /etc/ssh/known_hosts /etc/ssh/known_hosts2 Space-separated
UserKnownHostsFile UserKnownHostsFile ~/.ssh/known_hosts ~/.ssh/known_hosts2 Space-separated
PermitRemoteOpen PermitRemoteOpen host1:80 host2:443 Space-separated
CanonicalDomains CanonicalDomains example.com corp.local Space-separated

Source

OpenBSD ssh_config(5) — the canonical reference (OpenSSH is developed by OpenBSD).

@EhabY EhabY requested a review from deansheather December 16, 2025 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@deansheather deansheather Awaiting requested review from deansheather

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SSH config cannot use SetEnv without multiple problems

2 participants

@EhabY @deansheather