feat(update): pre-flight permission check with platform-specific hint #25

Merged
David merged 1 commit from feat/update-permission-check into main 2026-05-14 17:41:09 +02:00
Owner

Summary

Direct answer: no, the original yt update did not check permissions; it would download the binary and then fail at the rename step with a generic Permission denied error. This PR adds a pre-flight probe so the user sees a clear "use sudo / Administrator" message before any HTTP call.

What changed

The flow used to be: resolve URL -> download -> chmod staging -> self-replace. The rename step is what surfaces a write-perm error, so users running yt update from /usr/local/bin/ paid for the download first.

New order: resolve URL -> current_exe() -> probe parent dir for write access -> (if OK) download -> stage -> self-replace.

The probe is OpenOptions::create_new(true).write(true) on a uniquely-named file (.yt-update-probe-<pid>-<nanos>) inside the binary's parent dir, then immediately removed. create_new means we're testing actual creation rights, not opening an existing file we happen to own.

Error message

Platform-gated via cfg:

  • unix: permission denied: cannot write to /usr/local/bin. Re-run with sudo yt update, or move the binary to a directory you own (e.g. ~/.local/bin/yt).
  • windows: permission denied: cannot write to C:\Program Files\yt. Re-run from an Administrator prompt, or move the binary to a directory you own (e.g. %USERPROFILE%\bin\yt.exe).

--dry-run also runs the probe and errors on permission denied. A planning operation should report the failure mode it'd hit at execution time.

Verification

  • cargo fmt --check clean.
  • cargo clippy --all-targets -- -D warnings clean.
  • cargo test --all-targets: 186 tests pass (5 new).

Coverage on this PR:

  • writable-tempdir success case + probe cleanup invariant
  • missing-dir surfaces the generic I/O context (NotFound, not the friendly path)
  • 0o555 read-only dir on unix asserts the message contains "sudo" + the dir path (skipped when running as root since 0o555 doesn't restrict root)
  • pure-function checks on the message wording (cfg-gated for unix and windows)
  • uniqueness check on the probe filename so two simultaneous calls don't collide

Manual verification on Linux:

$ cp target/release/yt /usr/local/bin/
$ yt update --dry-run
Error: permission denied: cannot write to /usr/local/bin. Re-run with `sudo yt update`, or move the binary to a directory you own (e.g. `~/.local/bin/yt`).

Test plan

  • After merge, on a Linux host, copy yt to /usr/local/bin/ as root and run yt update as a non-root user: should print the sudo hint and exit non-zero without making an HTTP call.
  • Run the same with --dry-run: same friendly error.
  • Re-run with sudo yt update: should succeed.
  • On Windows, run yt update from a non-Administrator command prompt: should print the Administrator hint.
## Summary Direct answer: no, the original `yt update` did not check permissions; it would download the binary and then fail at the rename step with a generic `Permission denied` error. This PR adds a pre-flight probe so the user sees a clear "use sudo / Administrator" message before any HTTP call. ## What changed The flow used to be: resolve URL -> download -> chmod staging -> self-replace. The rename step is what surfaces a write-perm error, so users running `yt update` from `/usr/local/bin/` paid for the download first. New order: resolve URL -> `current_exe()` -> probe parent dir for write access -> (if OK) download -> stage -> self-replace. The probe is `OpenOptions::create_new(true).write(true)` on a uniquely-named file (`.yt-update-probe-<pid>-<nanos>`) inside the binary's parent dir, then immediately removed. `create_new` means we're testing actual creation rights, not opening an existing file we happen to own. ## Error message Platform-gated via `cfg`: - **unix**: `permission denied: cannot write to /usr/local/bin. Re-run with `sudo yt update`, or move the binary to a directory you own (e.g. `~/.local/bin/yt`).` - **windows**: `permission denied: cannot write to C:\Program Files\yt. Re-run from an Administrator prompt, or move the binary to a directory you own (e.g. `%USERPROFILE%\bin\yt.exe`).` `--dry-run` also runs the probe and errors on permission denied. A planning operation should report the failure mode it'd hit at execution time. ## Verification - `cargo fmt --check` clean. - `cargo clippy --all-targets -- -D warnings` clean. - `cargo test --all-targets`: 186 tests pass (5 new). Coverage on this PR: - writable-tempdir success case + probe cleanup invariant - missing-dir surfaces the generic I/O context (NotFound, not the friendly path) - 0o555 read-only dir on unix asserts the message contains "sudo" + the dir path (skipped when running as root since 0o555 doesn't restrict root) - pure-function checks on the message wording (cfg-gated for unix and windows) - uniqueness check on the probe filename so two simultaneous calls don't collide Manual verification on Linux: ``` $ cp target/release/yt /usr/local/bin/ $ yt update --dry-run Error: permission denied: cannot write to /usr/local/bin. Re-run with `sudo yt update`, or move the binary to a directory you own (e.g. `~/.local/bin/yt`). ``` ## Test plan - [ ] After merge, on a Linux host, copy `yt` to `/usr/local/bin/` as root and run `yt update` as a non-root user: should print the sudo hint and exit non-zero without making an HTTP call. - [ ] Run the same with `--dry-run`: same friendly error. - [ ] Re-run with `sudo yt update`: should succeed. - [ ] On Windows, run `yt update` from a non-Administrator command prompt: should print the Administrator hint.
feat(update): pre-flight permission check with platform-specific hint
All checks were successful
Check / fmt + clippy + build + tests (pull_request) Successful in 14s
Create release / Create release from merged PR (pull_request) Has been skipped
6fcec1d926
`yt update` now probes write access to the directory containing the running binary before downloading. If the kernel returns `PermissionDenied`, the command exits with a friendly platform-specific message instead of failing mid-way with a generic "rename failed" error after the user already paid for a megabyte+ of bandwidth.

Probe: `OpenOptions::create_new(true).write(true)` on a uniquely-named file (`.yt-update-probe-<pid>-<nanos>`) inside the binary's parent dir, then remove it. `create_new` means we're testing actual creation rights, not opening an existing file we happen to own. Success cleans up; PermissionDenied surfaces the friendly message; any other I/O error bubbles with `"probe write access for <dir>"` context.

Message text is cfg-gated:

  unix:    "permission denied: cannot write to <dir>. Re-run with `sudo yt update`, or move the binary to a directory you own (e.g. `~/.local/bin/yt`)."
  windows: "permission denied: cannot write to <dir>. Re-run from an Administrator prompt, or move the binary to a directory you own (e.g. `%USERPROFILE%\\bin\\yt.exe`)."
  other:   "permission denied: cannot write to <dir>"

The probe also fires under `--dry-run`. A planning operation that says "this would succeed" only to fail at execution time is worse than one that says "this would fail with permission denied" upfront. So dry-run with no write access also errors.

Order of operations changed: `current_exe()` + dir probe happen BEFORE the HTTP request. Previously download was step 1 and the rename failure was step N. Now permission failures are detected with zero network use.

Tests: writable-tempdir success case, missing-dir error message (NotFound, not the friendly path), 0o555 read-only-dir on unix asserts the message contains "sudo" + the dir path (skipped when running as root since 0o555 doesn't restrict root), pure-function check on the message wording for both unix and windows (cfg-gated), uniqueness check on the probe filename so two simultaneous calls don't collide. The existing dry-run and 404 tests still pass: the test runner's directory under `target/debug/deps/` is writable for the test user. 186 tests; clippy/fmt clean.

Manual verification: `cp target/release/yt /usr/local/bin/ && yt update --dry-run` as a non-root user now prints "permission denied: cannot write to /usr/local/bin. Re-run with `sudo yt update`, ..." before any HTTP call.
David merged commit 7f6e70dc94 into main 2026-05-14 17:41:09 +02:00
David deleted branch feat/update-permission-check 2026-05-14 17:41:09 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
pandoras-box/youtrack-cli!25
No description provided.