Skip to content

fix: Windows ssh command quoting #1863

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.

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

Merged
merged 1 commit into from
Sep 30, 2023

Conversation

pendo324
Copy link
Contributor

I thought I fixed this issue in #1825, but I must have tested it incorrectly, as the issue was still occurring in my latest test

Before:

{"level":"info","msg":"Waiting for the essential requirement 1 of 3: \"ssh\"","time":"2023-09-29T18:46:02Z"}
{"level":"debug","msg":"executing script \"ssh\"","time":"2023-09-29T18:46:02Z"}
{"level":"debug","msg":"executing ssh for script \"ssh\": C:\\Program Files\\Git\\usr\\bin\\ssh.exe [ssh -F /dev/null -o IdentityFile=\"C:/Users/Administrator/Code/finch space/_output/lima/data/_config/user\" -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o NoHostAuthenticationForLocalhost=yes -o GSSAPIAuthentication=no -o PreferredAuthentications=publickey -o Compression=no -o BatchMode=yes -o IdentitiesOnly=yes -o Ciphers=^aes128-gcm@openssh.com,aes256-gcm@openssh.com -o User=lima -o ControlMaster=auto -o ControlPath=\"C:/Users/Administrator/Code/finch space/_output/lima/data/finch/ssh.sock\" -o ControlPersist=yes -p 22 192.168.169.245 -- /bin/bash]","time":"2023-09-29T18:46:02Z"}
{"level":"debug","msg":"stdout=\"\", stderr=\"command-line line 0: invalid quotes\\r\\n\", err=failed to execute script \"ssh\": stdout=\"\", stderr=\"command-line line 0: invalid quotes\\r\\n\": exit status 255","time":"2023-09-29T18:46:02Z"}

After:

{"level":"info","msg":"The essential requirement 1 of 3 is satisfied","time":"2023-09-29T18:51:52Z"}
{"level":"info","msg":"Waiting for the essential requirement 2 of 3: \"user session is ready for ssh\"","time":"2023-09-29T18:51:52Z"}
{"level":"debug","msg":"executing script \"user session is ready for ssh\"","time":"2023-09-29T18:51:53Z"}
{"level":"debug","msg":"executing ssh for script \"user session is ready for ssh\": C:\\Program Files\\Git\\usr\\bin\\ssh.exe [ssh -F /dev/null -o IdentityFile='C:/Users/Administrator/Code/finch space/_output/lima/data/_config/user' -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o NoHostAuthenticationForLocalhost=yes -o GSSAPIAuthentication=no -o PreferredAuthentications=publickey -o Compression=no -o BatchMode=yes -o IdentitiesOnly=yes -o Ciphers='^aes128-gcm@openssh.com,aes256-gcm@openssh.com' -o User=lima -o ControlMaster=auto -o ControlPath='C:/Users/Administrator/Code/finch space/_output/lima/data/finch/ssh.sock' -o ControlPersist=yes -p 22 192.168.169.245 -- /bin/bash]","time":"2023-09-29T18:51:53Z"}

I tried out the changes on macOS as well, and I didn't see any regressions. I'm not quite sure why Windows doesn't like the double quotes. Instead of figuring out the root cause, this is more of a workaround, but it doesn't seem to cause any issues

@pendo324 pendo324 force-pushed the windows-ssh-quoting branch 2 times, most recently from ed81aa2 to 5a57e51 Compare September 29, 2023 21:10
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
@AkihiroSuda AkihiroSuda added this to the v0.18.0 milestone Sep 30, 2023
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

}
opts = []string{fmt.Sprintf(`IdentityFile="%s"`, privateKeyPath)}
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we can use the "windows" form for non-windows too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that at first, but some of the e2e tests were failing :(. Example: https://github.com/lima-vm/lima/actions/runs/6355908876

@AkihiroSuda AkihiroSuda merged commit e7f8ffb into lima-vm:master Sep 30, 2023
@AkihiroSuda
Copy link
Member

Merged, but still doesn't work on GHA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants