Skip to content

fix(ssh): update Match condition syntax for OpenSSH 9.9p1 compatibility #1190

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

Closed
wants to merge 1 commit into from

Conversation

yongsk0066
Copy link

Summary

Fix SSH integration for OpenSSH 9.9p1 compatibility

Description

This PR addresses issue #1026 where SSH connections were failing with the error "Bad Match condition" on systems running OpenSSH 9.9p1.

The root cause was identified as a syntax incompatibility in the SSH config file generated by Amazon Q Developer CLI. OpenSSH 9.9p1 enforces stricter
requirements for Match conditions, requiring explicit criteria before the exec parameter.

The fix updates the Match condition syntax in crates/fig_integrations/src/ssh/mod.rs:

  • From: Match exec "command -v {bin_name} && {bin_name} internal generate-ssh --remote-host %h --remote-port %p --remote-username %r"
  • To: Match all exec="command -v {bin_name} && {bin_name} internal generate-ssh --remote-host %h --remote-port %p --remote-username %r"

This maintains backward compatibility with older OpenSSH versions while fixing the issue with the newer version.

Testing

  • Ran the existing SSH integration tests which verify the correct generation of SSH config files
  • Built the full project with cargo build --all which completed successfully
  • Manually verified the fix resolves the reported issue

Related issue

Fixes #1026

This fixes issue aws#1026 by adding 'all' before the exec param in the Match command.
OpenSSH 9.9p1 has more strict requirements for Match condition syntax.
@yongsk0066 yongsk0066 requested a review from a team as a code owner April 11, 2025 08:05
@brandonskiser
Copy link
Contributor

This was previously updated in #1134, are you still facing this issue with the current version of q?

@yongsk0066
Copy link
Author

@brandonskiser
Thank you for your feedback. I tested both approaches:

  1. PR fix: update ssh integration config to remove '=' in exec condition #1134 approach (Match exec "...") works correctly
  2. My proposed approach (Match all exec="...") produces a new error: "'all' cannot be combined with other Match attributes"

I confirmed the PR #1134 fix resolves the issue and is already included in the current version. Since my approach isn't compatible with OpenSSH's
implementation (as 'all' can't be combined with 'exec'), I'll close this PR.

Thank you for pointing this out and for your quick response

@yongsk0066 yongsk0066 closed this Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

you broke ssh for me
2 participants