SSH: fix Include directive escaping and refactor backup logic#4861
Merged
SSH: fix Include directive escaping and refactor backup logic#4861
Conversation
…ection - Extract BackupFile into fileutil package with exported suffix constants (SuffixOriginalBak, SuffixLatestBak) so callers don't hardcode strings - Replace strings.Contains with line-aware containsLine helper in sshconfig to avoid false positives from commented-out or mid-line occurrences of the Include path - Migrate unquoted Include directives written by older CLI versions to the quoted form (handles paths with spaces) Co-authored-by: Isaac
ilia-db
requested review from
andrewnester,
anton-107,
denik,
pietern,
shreyas-goenka and
simonfaltum
as code owners
Suggested reviewersBased on git history of the changed files, these people are best suited to review:
Confidence: low Eligible reviewersBased on CODEOWNERS, these people or teams could also review: @andrewnester, @anton-107, @denik, @shreyas-goenka, @simonfaltum Suggestions based on git history of 6 changed files (4 scored). See CODEOWNERS for path-specific ownership rules. |
ilia-db
changed the title
Collaborator
|
Commit: accb7dc
19 interesting tests: 10 SKIP, 6 RECOVERED, 3 flaky
Top 20 slowest tests (at least 2 minutes):
|
anton-107
reviewed
Mar 30, 2026
- BackupFile now returns an error if os.Stat on the original backup fails with anything other than IsNotExist, instead of silently falling through to latestBak - containsLine trims leading spaces/tabs so indented SSH config directives (common inside Host blocks) are matched correctly - Add replaceLine helper with the same trim logic as containsLine, replacing the substring-based strings.Replace in the migration path to prevent false matches inside comments - Extend tests: TestBackupFile_StatError, TestReplaceLine, indented/comment-substring cases for EnsureIncludeDirective, and .latest.bak coverage in TestCheckAndUpdateSettings_CreatesBackup Co-authored-by: Isaac
anton-107
approved these changes
Mar 30, 2026
Collaborator
|
Commit: c34f8d9
36 interesting tests: 17 RECOVERED, 11 KNOWN, 4 flaky, 2 FAIL, 2 SKIP
Top 50 slowest tests (at least 2 minutes):
|
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
3 participants
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.
Summary
The backup refactoring is actually secondary, the main fix is quoting paths inside "Include" statement that we add to ssh config.
Noticed that we also didn't do backups for ssh configs, so here are backup related changes:
BackupFilefromvscode/settings.gointo a sharedfileutilpackage with exported suffix constants (SuffixOriginalBak,SuffixLatestBak), eliminating hardcoded magic strings across callers and testsstrings.Containswith a line-awarecontainsLinehelper insshconfigto avoid false positives when the Include path appears in a comment or as a substring of another lineIncludedirectives (written by older CLI versions) to the quoted form, which handles paths with spacesTests
Existing, new, and manually
This pull request was AI-assisted by Isaac.