fix(ssh): prevent tar from overwriting remote home dir permissions (#17767) - #17898
Merged
Conversation
tar xf - -C / extracts the staging directory tree to the remote root. GNU tar default behavior overwrites metadata (including mode) of existing directories. When the local umask is 002 (Ubuntu default), the staging dirs are 0775, and tar chmod's /home/<user> to 0775 — breaking sshd StrictModes which requires 0755 or stricter for home dirs. Add --no-overwrite-dir to the remote tar command so existing directory metadata is preserved. Fixes #17767
Existing test_tar_pipe_commands asserted the literal substring 'tar xf - -C /' in ssh_str, which is no longer present after the #17767 fix adds --no-overwrite-dir between 'tar xf -' and '-C /'. Split the one substring check into three independent assertions for the tar stdin mode, the new --no-overwrite-dir flag (regression guard for #17767), and the extract target.
alt-glitch
added
type/bug
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.
Salvages #17867 by @vominh1919 onto current
main. Closes #17767.Problem
The SSH terminal backend's
_ssh_bulk_upload()runstar xf - -C /on the remote, extracting a staging directory tree to the remote root. GNU tar's default is to overwrite metadata (including mode) of existing directories. When the local umask is002(Ubuntu default), staging dirs are0775, and tar chmods/home/<user>to0775on the remote.That violates sshd
StrictModes yes(which requires non-group-writable home dirs), so subsequent SSH connections fail withPermission denied (publickey). Recovery requires out-of-band console access to chmod the home dir back. Reporter (@luismartinezs) hit this on a Hetzner VPS and couldn't reconnect.Confirmed on current main —
tools/environments/ssh.py:185still has the baretar xf - -C /.Fix (author: @vominh1919, 1 file, +5/-1)
Add
--no-overwrite-dirto the remote tar command. Tar skips updating attributes of already-existing directories, so new entries (~/.hermes/,~/.hermes/skills/) are still created with reasonable modes via remote umask, but/home/<user>keeps its original0755.Follow-up test update
tests/tools/test_ssh_bulk_upload.py::test_tar_pipe_commandsasserted the literal substring"tar xf - -C /"in the ssh command — which is no longer present with--no-overwrite-dirbetweentar xf -and-C /. Split into three separate assertions; added an explicit check for the new flag as a regression guard.Validation
Authorship preserved for @vominh1919 via plain cherry-pick.