Files
sshfs_connect/DECISIONS.md
T
nevaforget e6a02e5bf7
Update PKGBUILD version / update-pkgver (push) Successful in 5s
fix: detect mounts via /proc/self/mountinfo so stale FUSE works
mountinfo.Mounted lstats the path. When the sshfs link dies, every stat
on the mountpoint returns EIO, so -l filtered the dead mount out, mount
failed in MkdirAll, and -u failed before fusermount. Switch detection to
mountinfo.GetMounts (no stat) and add a fusermount -uz fallback so a
stale mount can actually be torn down.
2026-05-04 10:08:13 +02:00

14 KiB
Raw Blame History

Decisions

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.Removes 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_fieldstrconv.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 MkdirAlls 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).