fix(ssh-config): strip quotes and handle spaced = separator - #8
Merged
Merged
Conversation
`IdentityFile "~/.ssh/id_rsa"` kept its surrounding quotes, so `resolve_key()` looked for a literal path containing `"` characters, found nothing, and silently fell back to the default key paths — the host was reported as key-less by `shuttle node import`. The same `split(None, 1)` logic also mishandled a spaced `=` separator: `Port = 2222` parsed as the value `"= 2222"`, and `_build_entry` then raised `ValueError` from `int()`, aborting the whole parse so no hosts could be imported at all. Replace the ad-hoc splitting with a line regex that accepts either whitespace or an optionally-spaced `=` and captures the rest of the line, then strip a single pair of matching outer quotes. Values that are empty after unquoting are skipped. Closes #7
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.
Closes #7.
Problem
src/shuttle/core/ssh_config.pysplit each config line withline.split(None, 1), falling back toline.split("=", 1). Two bugs fall out of that:1. Quotes are not stripped (the reported issue)
IdentityFile "~/.ssh/id_rsa"parsed to the literal string'"~/.ssh/id_rsa"'.SSHConfigEntry.resolve_key()then looked for a path containing quote characters, found nothing, and silently fell back toDEFAULT_KEY_PATHS— soshuttle node importlisted the host as key-less with no error explaining why. Single quotes behave the same way.2. A spaced
=separator leaks the=into the value (found while reproducing)Port = 2222matched the whitespace split first, giving the value'= 2222'._build_entrythen doesint('= 2222')→ValueError, which propagates out ofparse_ssh_config()and aborts the whole file — no hosts at all can be imported. The existingtest_equals_separatoronly covered the unspacedPort=3022, so this was never exercised.Key = Valueis validssh_config(5)syntax.Fix
Replace the ad-hoc splitting with a single line regex plus an unquote helper:
=._unquote()strips exactly one pair of matching outer quotes ("or'), matchingssh(1).User ""no longer overwrites the default.Note on the issue text
The issue suggests
split(None, 1)"splits at the first whitespace inside the quoted value and drops the rest of the path". That part isn't quite right —maxsplit=1keeps the remainder, so"~/.ssh/my key"was already preserved in full; the only reason it failed was the unstripped quotes. The fix and the test for it are unchanged either way.Tests
Added to
tests/test_core/test_ssh_config.py, written first and confirmed failing against the old parser:test_equals_separator_with_spacesValueError: invalid literal for int(): '= 3022'test_double_quotes_stripped'"10.0.0.1"'test_single_quotes_stripped"'10.0.0.1'"test_quoted_path_with_spaces_preserved'"~/.ssh/my key"'test_quoted_equals_separator'= "~/.ssh/id_rsa"'test_key_without_value_skippeduser == '""'test_resolve_quoted_key_from_configresolve_key()→Nonetest_resolve_quoted_key_with_spaces_from_configresolve_key()→Nonetest_inner_quotes_preservedis a guard that_unquotestrips only the outer pair.Verification
uv run pytest -q— 220 passed (23 intest_ssh_config.py, up from 14)uv run ruff check src/shuttle/ tests/— cleanuv run ruff format— clean