Skip to content

fix(sftp): resolve relative paths to session HOME - #1177

Merged
twitchyliquid64 merged 1 commit into
mainfrom
tom/sftp
Aug 5, 2026
Merged

fix(sftp): resolve relative paths to session HOME#1177
twitchyliquid64 merged 1 commit into
mainfrom
tom/sftp

Conversation

@twitchyliquid64

@twitchyliquid64 twitchyliquid64 commented Aug 5, 2026

Copy link
Copy Markdown
Member

exec channels start in the session's home ($HOME=/home, so cd; mkdir -p .zed_server creates /home/.zed_server), while the SFTP subsystem resolves relative paths against the session's working tree (/workbench).

The spec and expected-behavior specifies that these should resolve to $HOME, so fixing this in the sftp server implementation.

Summary by CodeRabbit

  • Bug Fixes
    • Updated SFTP relative paths to resolve from the session home directory, matching standard SFTP/SCP behavior.
    • Preserved access to workspace files through explicit absolute paths.
    • Improved handling of file operations, permissions, symlinks, and directory traversal.

Note

Resolve relative SFTP paths against session HOME instead of workspace root

  • Changes SftpSession.resolve in sftp.rs to use env::HOME_ROOT as the base for relative paths, so realpath('.') now reports the home directory rather than the working tree.
  • Updates integration tests in cli.rs and git_remote.rs to use absolute /workbench/… paths for SFTP reads, since relative paths no longer resolve to the workspace.
  • Behavioral Change: SFTP clients that relied on relative paths (e.g. hello.txt) to address files in the workspace will no longer find them there; paths must now be fully qualified as /workbench/hello.txt.

Macroscope summarized b893fb1.

@twitchyliquid64
twitchyliquid64 requested a review from a team as a code owner August 5, 2026 18:19
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

SFTP relative paths now resolve from the session home directory. Workspace files use explicit /workbench paths. SFTP unit tests and CLI integration tests now verify the separate home and workspace path behavior.

Changes

SFTP path resolution

Layer / File(s) Summary
Home-based relative path resolution
crates/minimald/src/sftp.rs
SftpSession::resolve and related documentation now use the session home for relative paths and realpath(".").
Filesystem operation coverage
crates/minimald/src/sftp.rs
SFTP tests verify relative home operations and explicit /workbench paths for traversal, symlink, and chmod cases.
Integration test path updates
crates/minimal/tests/cli.rs, crates/minimal/tests/git_remote.rs
CLI and git remote tests read uploaded and pushed files through explicit /workbench paths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: bryan-minimal

Poem

A rabbit hops through /workbench bright,
While home paths stay tucked from sight.
SFTP follows the roots anew,
Tests check each path and symlink too.
“Clear trails!” the rabbit sings.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the change but omits the required Testing and Checklist sections. Add Summary, Testing, and Checklist sections, including test commands and results plus documentation and breaking-change checklist status.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the SFTP relative-path resolution change.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tom/sftp

Comment @coderabbitai help to get the list of available commands.

@twitchyliquid64
twitchyliquid64 enabled auto-merge (rebase) August 5, 2026 18:25
@twitchyliquid64
twitchyliquid64 merged commit 046d1d1 into main Aug 5, 2026
30 checks passed
@twitchyliquid64
twitchyliquid64 deleted the tom/sftp branch August 5, 2026 18:31
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.

2 participants