sshfs_connect/DECISIONS.md

# Decisions

## 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)`.