sshfs_connect/DECISIONS.md

# Decisions

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