Repository navigation
Conversation
Add herdr_client::remote_file, which copies a file off an SSH host and writes it back over the endpoint's noninteractive SSH trust, for the upcoming remote file links in the GUI. POSIX hosts run /bin/sh scripts; Windows OpenSSH hosts run Windows PowerShell 5.1+ scripts through -EncodedCommand. The path travels quoted in the script; file bytes travel only on stdout and stdin. Both shells share one exit-code contract for missing, not-a-file, too-large, symlink, no-hasher, and conflict refusals. A push writes a private temporary file beside the target and renames it over the target only while the host's file still matches the expected SHA-256 (or is absent, or anything). An existing POSIX file keeps its permission bits; a new one gets the login umask's default. Tests run the POSIX scripts through a local /bin/sh and, opt-in, the PowerShell scripts through a local powershell.exe and pwsh.
A link-modifier click on a path printed in a pane on an SSH device did nothing, since this machine cannot open that host's paths. Such paths are now links by their shape: a click fetches the file over the device's SSH trust into a per-launch cache and opens the copy in the system's application, or the folder holding it when it is not a known document, under the same rule as local paths. Windows OpenSSH hosts are recognised by a drive or UNC path, or a Windows working directory for a relative path, and use PowerShell; other hosts use /bin/sh. Relative paths join the pane's cwd with the host's separator, and `~` is left for the host to expand. Hover never contacts the host. Plugin link handlers keep the first claim. WSL and cloud panes are unchanged, as are SSH panes on Windows clients, which have no SSH children. The cache is removed at quit, and folders left by earlier launches are swept on first use.
A copy fetched by clicking a path in an SSH pane is now watched for saves. The first save asks whether to upload it: Upload, Upload future saves without asking, or Not now. Later saves follow that answer, and an automatic upload reports itself in a flash. Every upload sends the hash the host's file had when it was last synced. When the file changed on the host since then, nothing is written and a dialog offers Keep both (uploads beside it as name.conflict.ext), Overwrite anyway, or Re-fetch. Copies over 64 MB are not uploaded. The client's push now carries the exact byte count, and the host refuses to replace its file with a temp file that holds fewer bytes, so a dropped connection or cancelled upload leaves it unchanged. A click that fetches a copy again keeps a save made while the fetch ran. The host, shell, and path of a transfer travel as one RemoteFile. Watching runs off the UI thread at most once a second; a change counts once two runs agree on it, which lets an editor's atomic save settle. Prompts wait for any open dialog instead of replacing it. Saving a copy back to its synced bytes drops its question and closes its dialog, and a failed Re-fetch offers the conflict again. An upload never makes the file readable by anyone who could not read it before. A POSIX host gives the temp file the target's group and mode before the rename, and writes in place instead when the group cannot be kept with the same reach or an ACL shows (`ls` `+` or `@`, `getfacl`). Windows PowerShell 5.1 and 7 create the temp file with the target's DACL. Keep both takes the original file's access. When access cannot be kept and nothing can be written safely, the host exits with its own code and the user is told so. A macOS host's inheritable folder ACL can still read the temp file during the transfer. Each window keeps its copies in its own cache folder, so a fetch in one window never replaces another's unsent edits. A click while its file is still being fetched starts no second fetch, and each fetch writes its own part file.
`herdr-gpui remote-files [list]` reports the copies of SSH hosts' files that link clicks fetched into the window, each with its host, host path, local path, sync state, and whether its saves upload without asking. It is read-only: uploads stay behind the app's dialog. The reply fits the socket's message budget and counts what it omits, as `notes list` does. Document remote file links in the Herdr UI README, the transfer API in the client README, the command in docs/cli.md, and the behaviour beside open_files_in in the example config.
The overview said SSH needs "an installed POSIX Herdr", but a macOS client connects to a Windows OpenSSH host running Herdr 0.9.4, and the Files on SSH Hosts section already describes Windows hosts.
A URL or path that ran to the pane's right edge was never a link: the client gets no soft-wrap information, so the row reader refused any token reaching the last column in case it continued on the next row. The reader now decides. `terminal/links/wrap.rs` walks the hit word's rows and joins them in two cases: a raw terminal wrap (no walls, the word ends at the pane's last column and the next row starts at column 0), and an application's word wrap as Ink/wrap-ansi lays it out (same walls, the next row starting under the text or after a bullet marker, and a word too long for the wrap width). Walls are frame characters that line up on both rows, so boxes, table cells and side-by-side panels each wrap at their own border. The existing URL and path parsers run on the joined text, and the match maps back to one segment per row, which the painter already draws. A word that reaches the edge and does not continue is a complete link. Text that runs to the end of the pane's bottom row, or ends in `…`, is still refused, since it may be cut off. A raw-wrapped URL is left to a daemon that offers `pane.link.resolve`. When the local reading strictly contains the daemon's resolved link, as at an app's word wrap, the local one is underlined and opened if the daemon declines the click. OSC 8 hyperlinks that continue on the next row are underlined on both.
Mock pages are printed as `X:\...\index.html?sheet=1` or as `file:///.../index.html?sheet=1`. The disk check looked for a file literally named `index.html?sheet=1`, so the path was never a link, and `Url::to_file_path` dropped the query from `file://` links. A path containing a separator may now end in `?query` or `#fragment`. `plain_path` splits it off at the first unescaped mark after a separator. Guards keep real names whole: no split at the start of a component, `?` only before `key=` or after an extension, `#` only after an extension and before an anchor, `L<n>[-L<m>]`, or `page=<n>` on a PDF, and never at an empty suffix. The disk check and SSH's shape check see the path part, the underline covers the whole token, and `#L10` sets the line when there is no `:line`. `file://` links keep their query and fragment, and a suffix is capped at 4096 bytes. Clicking a web page (html, htm, xhtml, svg), a PDF, or a file with no extension that has a suffix starts the default browser directly with the `file://` URL, suffix included, because the system openers drop it. This covers local panes and SSH copies (fetched, cached, and kept edits). - Windows reads the https/http UserChoice ProgId (UserChoiceLatest first) and its open command. - macOS asks NSWorkspace for the default browser and runs its bundle executable. Safari takes no URL argument, so it falls back. - Linux has no direct launch and always falls back. The URL is passed as one argument, never through a shell. If the browser cannot start, the click opens what it would open without a suffix, so a file with no extension that is executable opens its folder and is never run. Every other file opens as before, without the suffix.
…ines Three link fixes from review: - A modifier-click on an address an application wrapped across rows, made before the hover's pane.link.resolve answer arrived, opened only the first row: with no resolved link to compare, the daemon's first-row reading won. A local reading that spans more than one row now wins when nothing is resolved yet; a handler still claims the click first. - A relative Windows path such as docs\index.html?x=1 never split off its query, because without a drive a backslash read as an escape. LinkReading now carries whether the pane's host writes Windows paths, judged from the pane's working directory (a drive or UNC path means Windows, a leading / means not), else from this machine. Only the suffix split uses it; POSIX escapes such as /tmp/a\ b are unchanged. - file://...#L10 links, and schemeless hyperlinks such as src/main.rs#L10-L20, now open at the line the fragment names, as a printed src/main.rs#L10 already did.
Two fixes from review of the SSH file copies: - A repeat fetch checked the copy for saves, then renamed the downloaded file over it, so a save landing between the two was overwritten and the copy marked synced. The copy now moves aside first, the download is hard-linked into its place, which fails rather than replace a save that took the place meanwhile, and the copy moved aside comes back unless its metadata proves it is the one last synced. A filesystem without hard links falls back to a rename. The watch run no longer probes a copy a fetch is writing, and a conflict's refetch now holds its copy in the in-flight set as a click's fetch does, so only one writer handles a copy at a time. - A copy's file name was split at both separators for every host, so /srv/foo\bar.txt on a POSIX host was cached and shown as bar.txt. It is now split at the host shell's own separators, and on Windows the backslash becomes an underscore as the local naming rules intend.
The POSIX push script checked the target for a symbolic link, then reopened it by name: for its temporary file, for chgrp and chmod, and for the in-place write it falls back to when a file's ACL cannot be kept. Another user who can rename entries in the folder could swap in a link between the check and the write, so the upload landed in a different file the SSH account can write. The script now: - enters the target's folder once with cd -P and names the target relative to it, so no later step re-resolves the folder's path; - builds the copy in a private mktemp -d folder inside it, checked to be the SSH user's drwx------ folder within the anchored one, with every file in it created by mktemp, so nothing that already exists is opened or replaced, and moves it over the target with mv -T (GNU) or mv -h (BSD) where mv takes one; - writes in place only in a folder no one else can replace files in (owned by the SSH user or root, no ACL, and not writable by group or others unless sticky with the target the user's or root's), and otherwise refuses with a new exit code, shown as "Other users can replace files in that folder on the host, so it was not written in place; nothing was changed."; - never writes in place for Keep both, and re-checks that the new name is still free just before the rename. A push now also needs id, ls and rmdir on the host.
# Conflicts: # crates/herdr-gpui/src/remote_files.rs
A modifier click on a printed path opened an extensionless folder itself, but a folder whose name carried a dot (v1.2, notes.md, Tool.app) fell through to the nearest extensionless ancestor, so clicking a bundle opened the folder holding it. A directory now opens itself whatever its name. Files keep the existing rule, and a path that does not open itself still resolves to the nearest folder that is not a bundle.
A folder printed in an SSH pane cannot be opened by copying one file. Add a probe that reports the tree's size and a metadata fingerprint, so a caller can warn before a large transfer and tell whether an extracted copy is still current, and a fetch that streams the folder as an archive: a gzipped tar from a POSIX host, a zip built with .NET from a Windows OpenSSH host. Both refuse a missing path or a file. The archive fetch refuses an archive over the caller's byte limit, and the PowerShell host skips reparse points while it builds the zip, so a junction cannot recurse or leave the tree.
The folder copy flow needs two user-editable limits: a warning threshold for a large folder and a hard cap on its archive. Add a [remote_files] block with folder_warning_mb (default 64, 1..=4096) and folder_transfer_cap_mb (default 512, 1..=8192), validated at load and on every save so the warning can never sit above the cap. Both appear as typed number rows in a new Links section of Settings > General, and are reachable through the existing config get/set path. No new dependency.
A modifier click on a folder path in an SSH pane failed with "The host's path is not a regular file", so only files could be fetched. Probe the host folder, warn when its contents exceed the configured size, fetch it as a tar.gz (POSIX) or zip (PowerShell) bounded by the transfer cap, extract it into the remote file cache under the folder's own name, and open it with the system handler. A repeat click compares the tree's fingerprint: unchanged reopens the copy, changed asks Re-fetch or Open existing. Links in the archive are skipped and counted, archives are path-checked and bounds-checked, and extraction writes nothing outside its staging folder.
Local gates on 307a146 (Windows 11, x64), the head that carries the review fixes (e001da1), #627's set-user-ID fix through the merge (6946bc9), and the clippy fix for Windows:
WSL Ubuntu, Linux x86_64 (the
Windows GNU cross-check: macOS: covered by this push's |
 [High impact] The change reaches SSH hosts and local files through pane links. Fix the broken folder-copy cases and the upload permission issue before merging. Findings
SummaryAdds SSH file and folder copies, repeat-click checks, upload questions, archive extraction checks, and configurable folder limits. The stacked changes also extend wrapped links and browser query support.
Acknowledgment: Validation was limited to source inspection; no tests or native UI checks were run. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Click an SSH path] --> B[Fetch as a file]
B -->|Folder| C[Probe size and fingerprint]
C --> D{Existing copy matches?}
D -->|Yes| E[Open existing copy]
D -->|No, existing copy| F[Ask Re-fetch or Open existing]
D -->|No copy| G{Above warning size?}
G -->|Yes| H[Ask Transfer or Cancel]
G -->|No| I[Fetch archive]
F -->|Re-fetch| I
H -->|Transfer| I
I --> J[Check and extract into staging]
J --> K[Replace cached folder and open]
Reviews (1) · Last reviewed commit: "feat: open a folder printed in an SSH pa..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let extracted = archive::extract( | ||
| archive, | ||
| staging, | ||
| basename(&remote.path, remote.shell), |
There was a problem hiding this comment.
A printed path such as /home/me/v1.2/ reaches this call with its trailing slash. The host strips that slash and writes archive entries under v1.2, but basename() returns file. The extractor rejects the mismatched name, so nothing opens. Strip trailing separators consistently before choosing the expected archive name.
There was a problem hiding this comment.
🤖 Posted by Claude Code on behalf of @leftos.
Confirmed and fixed in e001da1. The printed path is stripped of trailing separators once, before both the archive and the extraction, and the probe now reports the top name it will write (top on ProbedFolder). Extraction checks the archive against that reported name instead of guessing it. The same guess broke ~ and ~/, where the host archives under the home folder's own name; that is covered too, and the local copy of a trailing-separator path is named after its last segment.
| printf '%s\n' "$size" | ||
| {hasher} | ||
| if [ -n "$hasher" ]; then | ||
| sum=$(find "$path" -exec ls -lnd -- {{}} + | LC_ALL=C sort | $hasher) || exit 1 |
There was a problem hiding this comment.
The POSIX fingerprint misses edits that keep a file's length unchanged within the same minute. ls -lnd prints recent modification times only to the minute, so those edits leave this listing unchanged. remote_folder_probed() then reopens the stale copy without offering Re-fetch. Use metadata with sufficient time precision, or hash file contents.
There was a problem hiding this comment.
🤖 Posted by Claude Code on behalf of @leftos.
Confirmed and fixed in e001da1. The fingerprint now reads times with stat instead of ls -lnd: GNU stat nanoseconds, BSD stat seconds, and ls only as the fallback when neither answers. A same-length edit within the same minute changes the fingerprint, and a folder exec test covers it.
| if (($attributes -band [System.IO.FileAttributes]::Directory) -ne 0) {{ | ||
| $directories.Push($item) | ||
| }} else {{ | ||
| $relative = [System.IO.Path]::GetFullPath($item).Substring($prefix.Length).Replace('\', '/') | ||
| [void][System.IO.Compression.ZipFileExtensions]::CreateEntryFromFile($archive, $item, $base + '/' + $relative) |
There was a problem hiding this comment.
This branch only puts directories on the stack; it never adds directory entries to the zip. A directory with no files beneath it disappears from the local copy. The probe also records only files, so adding or removing an empty directory leaves the fingerprint unchanged. Include directory paths in both the zip and the fingerprint.
There was a problem hiding this comment.
🤖 Posted by Claude Code on behalf of @leftos.
Confirmed and fixed in e001da1. The Windows probe now lists directories as well as files, and the zip carries a directory entry per directory, so an empty folder survives the archive and changes the fingerprint. The script and archive tests were updated for the directory entries.
| $directory = $directories.Pop() | ||
| foreach ($item in [System.IO.Directory]::GetFileSystemEntries($directory)) {{ | ||
| $attributes = [System.IO.File]::GetAttributes($item) | ||
| if (($attributes -band [System.IO.FileAttributes]::ReparsePoint) -ne 0) {{ continue }} |
There was a problem hiding this comment.
This continue removes symbolic links and junctions before the zip is built. The app counts skipped links only during extraction, so these entries never reach the counter and no message warns the user. Carry the skipped count back from the host and include it in the completion message.
There was a problem hiding this comment.
🤖 Posted by Claude Code on behalf of @leftos.
Confirmed and fixed in e001da1. The probe answer now carries the number of entries the host's archive leaves out, and the completion message reports it (1 link was skipped, plural above one). The Windows probe counts the skipped reparse points it does not archive, so the app's extraction counter and the host's result agree.
| if [ -n "$keep" ]; then | ||
| chmod -- "$mode" "$copy" || exit 1 |
There was a problem hiding this comment.
If the SSH account is root, uploading over an executable owned by another account with mode 4755 applies 4755 to the new root-owned file. The rename then replaces the original, allowing other users to run the program as root instead of its original owner. Preserve the owner before keeping set-user-ID bits, or refuse or clear those bits when ownership changes.
How this was verified: The upload reads the target's full mode, applies it to a newly created SSH-user-owned file, and renames that file over the target without preserving its owner.
There was a problem hiding this comment.
🤖 Posted by Claude Code on behalf of @leftos.
Confirmed, and pre-existing to this PR (#627's push script). Fixed in #627 at 6946bc9: the copy keeps the set-user-ID bit only when its owner matches the file whose access it keeps; an unreadable owner on either side drops it, and set-group-ID is dropped when chgrp fails and the copy keeps its own group. Sticky stays. This PR gets the fix through the merge of feat/remote-file-links.
|
Working on the review findings now. |
A POSIX push renames a copy owned by the SSH user over the target, so a target another account owned becomes the SSH user's. The copy kept the target's mode, set-user-ID bit included: as root, pushing over another account's 4755 file produced a root-owned setuid executable that runs as root for everyone. Keep the set-user-ID bit only when the copy's owner matches the file whose access it keeps; an unreadable owner on either side drops it. Drop the set-group-ID bit when chgrp fails and the copy keeps its own group. Sticky stays: on a regular file it names no owner or group.
The client guessed a folder archive's top name from the printed path, so a path ending in a separator, or printed as ~, was expected under one name while the host wrote another: extraction refused the archive. The probe now reports the top name it will use, and extraction checks that. The probe answer also carries the number of entries the host's archive leaves out, so a Windows host's skipped junctions are counted in the completion message. The Windows probe lists directories and the zip holds directory entries, so an empty folder survives and changes the fingerprint. A POSIX fingerprint now reads times with `stat` (GNU nanoseconds, BSD seconds, `ls` as the fallback), so a same-length edit within one minute changes it instead of reopening the stale copy. Local copies of paths with trailing separators are named after their last segment.
fill took the archive, staging and local paths and the cap one by one, eight arguments, which -D warnings rejects and the project's five-parameter standard already disallowed. Group the transfer's paths and cap into a Transfer struct; the call site and the behavior are unchanged.
|
@penso Ready for re-review. |
# Conflicts: # crates/herdr-gpui/src/cli/control/hosts.rs # crates/herdr-gpui/src/control/client/hosts.rs # crates/herdr-gpui/src/control/protocol/hosts.rs # crates/herdr-gpui/src/control/protocol/hosts/tests.rs # crates/herdr-gpui/src/control/server/hosts.rs # crates/herdr-gpui/src/control/server/hosts/tests.rs # crates/herdr-gpui/src/menu/chrome.rs # crates/herdr-gpui/src/menu/chrome/keys.rs
# Conflicts: # Cargo.toml # crates/herdr-gpui/Cargo.toml
# Conflicts: # Cargo.toml # crates/herdr-gpui/src/settings_window/pages/general.rs
Local checks for 33a997a, which adds two test fixes to 9028a7c: ce1dec1 and 33a997a. The Windows 11 x86_64:
WSL (Ubuntu 26.04, x86_64), pwsh 7.6.7: ce1dec1 fixes the Linux and macOS CI failure in 33a997a fixes the two macOS failures in Not run at 33a997a: the full WSL workspace clippy and tests, and macOS. At 6aace3c, WSL clippy and The The merge needed two fits to |
The folder probe test checked a path ending in `\`, which only Windows reads as a separator. Under pwsh on Linux and macOS the name `a tree\` does not exist, so the probe exited 66 and the CI test jobs failed. Append the platform's own separator instead.
macOS keeps temporary folders behind `/var -> /private/var`, and the folder that opens is the resolved path, so the two folder-click tests failed on the macOS CI jobs comparing it with the unresolved URL. Turn the opened URL back into a path and compare both sides canonicalized, as the file copy test already does.
This branch has not been deployed
1 participant
Stacked on #627 and #695: merge those first. Until they land this diff also contains them.
What this does
v1.2/,notes.md/); before, a dotted folder was read as a possible bundle and its parent opened.remote_files.folder_warning_mb, fetches it as a gzipped tar stream on a POSIX host or a zip on a Windows host, extracts it into the existing remote-file cache and opens the extracted folder. An archive overremote_files.folder_transfer_cap_mbis refused.[remote_files]keys, defaulting to 64 MB and 512 MB, shown in Settings > General under Links and readable through the existing config get/set path.Validation
just ciat 6aace3c (Windows): format, file size, sorted lists, lint and the workspace tests passed, 3939 passed and 6 ignored. The run stops attest-mockup-wait, the known 8 failures on upstream Windows until test: run the mockup wait tests on Windows #643 merges.tools/wsl-ci.sh): clippy with all features and with no default features, both-D warnings, passed; workspace tests passed with default and with all features.herdr-clientremote_filetests, andherdr-gpuiconfig::tests(229),settings_window::tests(85),remote_files(95) andmenu(388).QA checklist
just ciexcept the knowntest-mockup-waitfailures (test: run the mockup wait tests on Windows #643)just lint-windows(both feature sets)Screenshots: none captured yet. The dialogs need a desktop with an SSH device, so the native checks above remain for the reviewer.