nevaforget
4306170626
Update PKGBUILD version / update-pkgver (push) Successful in 2s
The custom one-alias-per-line output was useless — no mountpoint, no source, no options. Reinventing a table format when findmnt from util-linux already produces a familiar fuse.sshfs view was the wrong call. -l now shells out to findmnt -t fuse.sshfs.
267 lines
16 KiB
Markdown
267 lines
16 KiB
Markdown
# Decisions
|
||
|
||
## 2026-05-04 – Delegate `-l` output to `findmnt -t fuse.sshfs`
|
||
- **Who**: Dom, ClaudeCode
|
||
- **Why**: The custom `-l` output ("just the alias, one per line") was
|
||
uselessly minimal — no mountpoint, no source, no options. Reinventing a
|
||
table format for a single command was the wrong call when `findmnt` from
|
||
`util-linux` already produces a familiar, well-formatted view of fuse.sshfs
|
||
mounts.
|
||
- **Tradeoffs**:
|
||
- External dependency on `findmnt` (part of `util-linux`, present on every
|
||
Arch and most other Linux systems by default — no real adoption cost).
|
||
- Output is mountpoint-first, not alias-first. The alias is only visible as
|
||
the last path segment under the SOURCE column / TARGET basename.
|
||
- Lists *all* fuse.sshfs mounts on the system, not just sshfsc-managed
|
||
ones under `$XDG_RUNTIME_DIR/sshfs/`. In practice this is what the user
|
||
wants ("show me all sshfs mounts"); a future `-r` (restrict) flag could
|
||
narrow it if needed.
|
||
- `findmnt` exits 1 when nothing matches its filter — we treat that as
|
||
success-with-empty-output, not a failure.
|
||
- **How**:
|
||
- `list_mounts` rewritten as a thin wrapper around `exec.Command("findmnt",
|
||
"-t", "fuse.sshfs")` with stdout piped to the caller's writer.
|
||
- Exit-code 1 from `findmnt` is swallowed via `errors.As(err, *exec.ExitError)`
|
||
+ `ExitCode() == 1`.
|
||
- `errors.Is(err, exec.ErrNotFound)` produces a clear "install util-linux"
|
||
message instead of a cryptic exec error.
|
||
- Tests `TestListMountsMissingBase` and `TestListMountsFiltersUnmounted`
|
||
removed: they covered the old in-Go enumeration logic that no longer
|
||
exists. Mocking `findmnt` would be alibi-testing — the wrapper is two
|
||
branches over `cmd.Run()`'s error.
|
||
|
||
## 2026-05-04 – Detect mounts via `/proc/self/mountinfo` instead of `stat`
|
||
- **Who**: Dom, ClaudeCode
|
||
- **Why**: `mountinfo.Mounted(path)` from `github.com/moby/sys/mountinfo` works
|
||
by `lstat`-ing the path. When a sshfs connection dies, the FUSE endpoint
|
||
stays in `/proc/mounts` but every `stat` on the mountpoint returns EIO.
|
||
All three sshfsc paths broke as a result:
|
||
- `-l` silently filtered the dead mount out (`Mounted` returned an error,
|
||
code skipped on `merr != nil`)
|
||
- mount path failed with `mkdir … file exists` because `MkdirAll` stats
|
||
before creating
|
||
- `-u` failed with `lstat … input/output error` before reaching
|
||
`fusermount`
|
||
Net effect: a stale mount was a dead end — couldn't be listed, couldn't be
|
||
unmounted, blocked re-mount.
|
||
- **Tradeoffs**:
|
||
- Switched mount detection to `mountinfo.GetMounts(SingleEntryFilter(path))`,
|
||
which parses `/proc/self/mountinfo` and never touches the path. Robust to
|
||
EIO on the mount target.
|
||
- `list_mounts` now uses `PrefixFilter(base)` and trims the base prefix.
|
||
Direct children only — nested mounts under an alias dir would be skipped
|
||
on purpose; sshfsc only manages one level.
|
||
- `verify_mount_dir` skips `MkdirAll` when the path is already a mountpoint.
|
||
Re-running `sshfsc <alias>` against a stale mount no longer errors on
|
||
mkdir; main's existing `is_mounted_at` check then prints "Already mounted"
|
||
and returns. The user can `sshfsc -u <alias>` to recover.
|
||
- `unmount_sshfs` falls back to `fusermount -uz` (lazy) when `-u` fails.
|
||
Lazy unmount detaches the FS from the tree immediately and lets dangling
|
||
refs settle later — exactly what stale FUSE needs. Plain `-u` first
|
||
keeps the clean path noise-free.
|
||
- **How**:
|
||
- New helper `is_mounted_at(path)`; replaces `mountinfo.Mounted` at every
|
||
call site (`main`, `verify_mount_dir`, `unmount_sshfs`).
|
||
- `list_mounts` rewritten around `PrefixFilter`. Trims `base + "/"` and
|
||
skips entries containing further separators.
|
||
- `run_fusermount(flag, mount)` extracted to keep `-u` and `-uz` calls
|
||
tidy. `unmount_sshfs` retries with `-uz` on first-call error.
|
||
- New tests: `TestIsMountedAtFalseOnPlainDir`,
|
||
`TestIsMountedAtFalseOnMissingPath`. Existing tests stay green —
|
||
`mountinfo.GetMounts` returns empty under the test tmpdirs.
|
||
|
||
## 2026-05-04 – Add `-l` (list) and `-u` (unmount) flags
|
||
- **Who**: Dom, ClaudeCode
|
||
- **Why**: Mounts under `$XDG_RUNTIME_DIR/sshfs/<alias>` had to be listed and
|
||
torn down with `ls` and `fusermount -u` by hand. A second `sshfsc <alias>`
|
||
call only prints `!!! Already mounted` and exits, so the tool gave no first-
|
||
class way to undo what it set up. `-l` and `-u` close that gap without
|
||
changing the default mount flow.
|
||
- **Tradeoffs**:
|
||
- Flag-based UX over subcommands keeps the existing `-e`/`-v`/`-r` style.
|
||
`-l` and `-u` are mutually exclusive (exit 2 when both passed). `-u` reuses
|
||
the positional `<Host>` argument; no separate value flag needed.
|
||
- `-l` only lists *real* mounts (verified via `mountinfo.Mounted`). Stale
|
||
empty dirs left behind by failed mounts are silently filtered, not
|
||
surfaced — keeps the output a true list of teardownable targets. Cleanup
|
||
of stale dirs is not implemented; tmpfs reclaims them on logout.
|
||
- `-u` shells out to `fusermount -u` and best-effort `os.Remove`s the empty
|
||
mount dir afterwards. A failed `Remove` warns but does not fail the
|
||
command — the unmount itself already succeeded.
|
||
- `mount_base(create bool)` returns `fs.ErrNotExist` when `create=false` and
|
||
the base is missing. `list_mounts` swallows that into "no mounts";
|
||
`unmount_sshfs` translates it into `not mounted: <alias>` so the caller
|
||
sees a single coherent error regardless of whether the alias-dir, the
|
||
base, or `XDG_RUNTIME_DIR` was missing.
|
||
- **How**:
|
||
- `verify_mount_dir` split into `mount_base(create bool)` (XDG resolve +
|
||
optional mkdir + EvalSymlinks) and `mount_path(base, name)` (join +
|
||
traversal + symlink guard, no mkdir). The original `verify_mount_dir`
|
||
becomes a thin wrapper that composes both and creates the mountpoint —
|
||
only the mount path needs that mkdir.
|
||
- `list_mounts(io.Writer)` reads the base dir, runs `mountinfo.Mounted` per
|
||
entry, prints alias names that pass. Takes a writer so tests can capture
|
||
output without stdout redirection.
|
||
- `unmount_sshfs(alias)` validates against `rxHostUser`, resolves the
|
||
mountpoint, checks `mountinfo.Mounted`, then `exec.Command("fusermount",
|
||
"-u", mount).Run()` and `os.Remove(mount)`.
|
||
- `main()` flow: after `flag.Parse()`, branch on `*lFlag` / `*uFlag` before
|
||
the existing mount path. Mutex check up front. Exit codes 8 (list) and 9
|
||
(unmount) extend the existing 2..7 range.
|
||
- Tests: `TestListMountsMissingBase`, `TestListMountsFiltersUnmounted`,
|
||
`TestUnmountRejectsBadAlias`, `TestUnmountNotMountedMissingBase`,
|
||
`TestUnmountNotMountedExistingDir`. The real `fusermount` path is not
|
||
exercised in unit tests — the not-mounted check returns before the exec.
|
||
|
||
## 2026-04-28 – Move mount base from `~/Servers/` to `$XDG_RUNTIME_DIR/sshfs/`
|
||
- **Who**: Dom, ClaudeCode
|
||
- **Why**: `~/Servers/` is non-standard. XDG Base Directory spec defines
|
||
`$XDG_RUNTIME_DIR` (= `/run/user/<uid>/`) for "non-essential runtime files"
|
||
— ephemeral, user-owned, session-bezogen. sshfs mounts fit that definition
|
||
exactly. systemd-logind creates the directory at login with mode 0700, so
|
||
permissions are correct by construction.
|
||
- **Tradeoffs**:
|
||
- Filemanager sidebar visibility is unchanged — gvfs surfaces FUSE mounts
|
||
via `/proc/mounts` regardless of mountpoint location.
|
||
- Tab-completion in `~/` no longer reaches mounts; users targeting the
|
||
mount via shell need the explicit `$XDG_RUNTIME_DIR/sshfs/<alias>` path.
|
||
Acceptable given Dom's primary access is the filemanager sidebar.
|
||
- tmpfs-backed → mounts and their containing dir vanish on logout/reboot.
|
||
No more orphaned empty dirs. Tradeoff: bookmarks pinned to the absolute
|
||
path stay valid because UID is stable across sessions, but the directory
|
||
is gone between logins until the next mount recreates it.
|
||
- Fallback to `/run/user/<uid>/` when `$XDG_RUNTIME_DIR` is unset (e.g.
|
||
cron, non-login shells without systemd-logind). Falls auf einem System
|
||
ohne systemd `/run/user/<uid>/` nicht existiert, schlägt `MkdirAll` mit
|
||
klarem Fehler fehl — kein Silent-Fallback auf `~/Servers/`.
|
||
- **How**:
|
||
- `verify_mount_dir` reads `$XDG_RUNTIME_DIR`, falls back to
|
||
`/run/user/<uid>/` via `os.Getuid()` + `strconv.Itoa`.
|
||
- Base = `<runtime>/sshfs`, created with `MkdirAll(0700)`.
|
||
- Existing path-traversal guard (`EvalSymlinks` + `HasPrefix`) and symlink
|
||
rejection (`Lstat`) carry over unchanged.
|
||
- Tests switched from `t.Setenv("HOME", ...)` to
|
||
`t.Setenv("XDG_RUNTIME_DIR", ...)`.
|
||
|
||
## 2026-04-28 – Use ssh_config alias as label for mountpoint and sshfs source
|
||
- **Who**: Dom, ClaudeCode
|
||
- **Why**: File managers showed the resolved `HostName` (often a raw IP) for
|
||
both the mount source (`user@10.0.0.5:`) and the mountpoint directory
|
||
(`~/Servers/10.0.0.5/`). The ssh_config alias is the human-readable label;
|
||
using it makes mounts identifiable in Nautilus/Dolphin/etc.
|
||
- **Tradeoffs**:
|
||
- sshfs now resolves the alias internally via `~/.ssh/config` instead of
|
||
receiving a pre-resolved IP. Our explicit `-o IdentityFile=` and `-p port`
|
||
still win as overrides — the resolved values were redundant but kept as a
|
||
belt-and-suspenders sanity check (early failure if config is malformed).
|
||
- The CLI argument flows into the sshfs argv, so it must be validated.
|
||
Added `validate_ssh_field("alias", args[0], rxHostUser)` immediately after
|
||
arg parsing to gate injection-shaped input.
|
||
- Existing mounts under `~/Servers/<ip>/` are now orphaned. User must
|
||
manually `fusermount -u` and `rmdir` the IP-named directories. Documented
|
||
in the README pointer.
|
||
- **How**:
|
||
- `args[0]` captured into `alias`, validated via `rxHostUser`.
|
||
- `verify_mount_dir(alias)` instead of `(hostname)`; param renamed to `name`
|
||
for clarity since it no longer represents a resolved hostname.
|
||
- `mount_sshfs` first arg renamed `hostname → alias`; argv source string
|
||
becomes `user+"@"+alias+":"+remoteDir`.
|
||
|
||
## 2026-04-28 – Add `-r` / `--remote-dir` flag for custom remote path
|
||
- **Who**: Dom, ClaudeCode
|
||
- **Why**: Mounting only the remote home was too restrictive; sometimes a
|
||
specific subdirectory (e.g. `/var/www`, `~/projects`) is the actual target.
|
||
- **Tradeoffs**:
|
||
- Two flag aliases (`-r` and `--remote-dir`) registered against the same
|
||
target via `flag.StringVar`. Doubles the `flag.PrintDefaults` output but
|
||
matches typical short/long convention.
|
||
- `rxRemoteDir` deliberately stricter than `rxIdentityFile` — no `:`, `@`,
|
||
`+`, `=`, `%` since remote paths rarely need them and looser allowlists
|
||
invite injection-shaped surprises in the `user@host:path` argv slot.
|
||
- Local mount path stays `~/Servers/<host>` regardless of remote dir — one
|
||
host = one mountpoint, even if `-r` differs between invocations. Re-mount
|
||
requires `fusermount -u` first.
|
||
- **How**:
|
||
- `rDir` package-level `string`, registered in `init()` as `r` and
|
||
`remote-dir`; validated against `rxRemoteDir = ^[A-Za-z0-9/~][A-Za-z0-9._/~-]*$`.
|
||
- `mount_sshfs` signature gains `remoteDir string`; appended to the
|
||
`user@host:` argv slot. Empty string preserves the previous home-dir
|
||
default.
|
||
- `-v` output includes `Remote:` line when `rDir` is set.
|
||
- Tests extended with eight `rxRemoteDir` boundary cases.
|
||
- **Who**: Dom, ClaudeCode
|
||
- **Why**: Audit found `kernel_cache` causes stale reads on a network FS;
|
||
`StrictHostKeyChecking` was implicit (depended on system default);
|
||
`ssh_config`-sourced strings flowed into the sshfs argv without validation,
|
||
allowing comma- or leading-dash injection if `~/.ssh/config` is attacker-influenced.
|
||
- **Tradeoffs**:
|
||
- `auto_cache` invalidates the page cache on `open(2)` when mtime/size
|
||
differ → marginal perf hit vs. correctness on a network FS.
|
||
- `accept-new` for `StrictHostKeyChecking` preserves first-connection UX
|
||
(TOFU) but locks subsequent reconnects to the recorded key.
|
||
- Allowlist regexes for HostName/User/IdentityFile are restrictive but match
|
||
real-world SSH naming. Rejected inputs surface a clear error.
|
||
- **How**:
|
||
- Replaced `-o kernel_cache` with `-o auto_cache` in `mount_sshfs`.
|
||
- Added explicit `-o StrictHostKeyChecking=accept-new`.
|
||
- Added `lookup_ssh_field` (uses `GetStrict`, surfaces parse errors) and
|
||
`validate_ssh_field` with per-field allowlists; Port parsed via `strconv.Atoi`.
|
||
- Added `main_test.go` covering `verify_mount_dir` and the allowlist regexes.
|
||
- Hardened `.gitea/workflows/update-pkgver.yaml` with an empty-PKGVER guard.
|
||
|
||
## 2026-04-26 – LOW-severity audit findings: cleanup pass + one Won't-Fix
|
||
- **Who**: Dom, ClaudeCode
|
||
- **Why**: Followup on the LOW-severity findings from the same audit.
|
||
- **How**:
|
||
- Q-L1: dead `if len(port) == 0 { port = "22" }` block already removed by Q-M1
|
||
refactor (port now flows through `lookup_ssh_field` → `strconv.Atoi`).
|
||
- Q-L2: `run_editor` returns `error` and `main` exits 7 on editor failure
|
||
instead of swallowing it.
|
||
- Q-L3: ABOUTME header added to `main.go` per global CLAUDE.md convention.
|
||
- P-L1: `golang.org/x/sys` bumped `v0.1.0 → v0.43.0` via `go get … && go mod tidy`.
|
||
`go` directive moved from 1.23.4 to 1.25.0 (required by the new dep).
|
||
- S-L2: added `-v` flag; default output now prints only the mount path.
|
||
`-v` restores the previous full HostName/User/Port/IdentityFile/Mount block.
|
||
- **Won't-Fix**:
|
||
- **S-L1** — `.gitea/workflows/update-pkgver.yaml` clones over
|
||
`http://gitea:3000/...`. This URL only resolves inside the Docker network
|
||
between the Gitea-act-runner container and the Gitea container. External
|
||
traffic still terminates TLS at the reverse proxy. An attacker on the
|
||
internal Docker bridge has already compromised the host; TLS between
|
||
containers does not help in that scenario. Documenting as accepted risk.
|
||
|
||
## 2026-04-26 – Second-round audit: defense-in-depth + UX
|
||
- **Who**: Dom, ClaudeCode
|
||
- **Why**: Re-audit (Quality + Performance + Security in parallel) surfaced five
|
||
remaining LOW findings after agent-output gegencheck. Two MEDIUM claims were
|
||
filtered out as false positives: missing `Port` already defaults to `"22"` via
|
||
`ssh_config.Default()` (verified in library source), and `lookup_ssh_field`
|
||
does not parse `~/.ssh/config` four times — `kevinburke/ssh_config` caches via
|
||
`sync.Once`, so four `GetStrict` calls = one parse + three in-memory walks.
|
||
An additional MEDIUM claim about a regex range bug (`%-` interpreted as range)
|
||
was empirically refuted (only `-` matched, not the implied range), but a
|
||
legitimate defense-in-depth gap remained: the regex still allowed leading `-`
|
||
in IdentityFile values.
|
||
- **Tradeoffs**:
|
||
- `run_editor` switched to `cmd.Start()` — drops editor exit-status
|
||
propagation in exchange for non-blocking UX on Sublime cold-start. Errors
|
||
from missing binary or fork failure are still returned.
|
||
- `verify_mount_dir` now `MkdirAll`s the base separately before
|
||
`EvalSymlinks`. `EvalSymlinks` requires the path to exist; without the
|
||
upfront `MkdirAll`, first run on a fresh system would fail.
|
||
- `rxIdentityFile` first-character class restricted to `[A-Za-z0-9/~]`. Plain
|
||
relative paths like `.ssh/id` are rejected — unusual but technically valid
|
||
in `ssh_config`. If a real config uses relative IdentityFile values, the
|
||
user gets a clear error and can switch to absolute or `~`-prefixed.
|
||
- **How**:
|
||
- **S-L1**: anchored `rxIdentityFile` to `^[A-Za-z0-9/~][A-Za-z0-9._@/:+=~%-]*$`.
|
||
- **S-L2**: `verify_mount_dir` resolves base via `filepath.EvalSymlinks` and
|
||
rejects existing mount paths that are symlinks (`os.Lstat`).
|
||
- **Q-L1**: `flag.Usage` override now prints the positional `<Host>` argument
|
||
in the help output.
|
||
- **Q-L2**: inline comment at the `run_editor` call site documents the
|
||
deliberate fall-through on already-mounted hosts.
|
||
- **P-L2**: `run_editor` uses `cmd.Start()` instead of `cmd.Run()`.
|
||
- Tests extended: rxIdentityFile boundary cases, `TestVerifyMountDirRejectsSymlink`,
|
||
existing `TestVerifyMountDir` paths now compared against `EvalSymlinks(base)`.
|