test(selfupdate): pin the archive path guards, reject zero-length binaries, add fuzzing

CHANGELOG.md

@@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Security
+
+- `grant update` now refuses to install a zero-length binary from a release archive
+- `grant update` now rejects non-regular zip entries, matching the existing tar behaviour
+
 ### Fixed
 
 - `grant favorites add` now fails immediately without a terminal instead of authenticating first

CLAUDE.md

@@ -108,7 +108,14 @@ Custom `SCAAccessService` follows SDK conventions:
   - Asset selection: `grant-cli_<version>_<goos>_<goarch>.tar.gz` (`.zip` on windows) — must stay in sync with `.goreleaser.yaml`
   - Integrity: SHA-256 of the archive checked against the release's `checksums.txt` (GNU `*filename` binary marker tolerated). **Trust model:** `checksums.txt` comes from the same origin as the archive, so it defends against corrupted/tampered downloads in transit, **not** against a compromised GitHub account or release pipeline. Signature verification would be needed for that. Note the checksum covers the *archive*, not the extracted binary — hence the independent size checks below
   - Extraction: `archive/tar`+`compress/gzip` / `archive/zip`. Rejects absolute, drive-absolute (`C:\`), UNC and `..` paths (gosec G305); accepts only a single `grant`/`grant.exe` at the archive root (nested entries and duplicate candidates are errors). Size cap is 128 MiB (`maxDownloadBytes`), enforced by `readCapped`, which probes one byte past the cap — a bare `io.LimitReader` reports a *successful* short read and would silently install a truncated binary (gosec G110). `maxDownloadBytes` is a var only so tests can shrink it — mutate it exclusively through the `withMaxDownloadBytes(t, n)` helper (restores via `t.Cleanup`), and never call `t.Parallel()` in a test that does
-  - Apply: `github.com/minio/selfupdate` v0.6.0 owns the staged-file write, the two-rename swap including the Windows path, and rollback — do not hand-roll this. grant adds the `fsync` of the staged file (minio does not sync) plus a best-effort directory sync. Seams: `applyWithOptions`, `prepareFn`, `commitFn` in `internal/selfupdate/apply.go`
+  - **Zero-length binaries are refused twice**, in `extractBinary` and again at the apply boundary (`applyBinaryTo`). The checksum covers the *archive*, not the extracted bytes, and `applyBinary` hashes whatever it is handed — so an empty payload verifies against itself and would replace a working binary with nothing. The extractor's type/size guards are the real fix; these two checks are backstops with a **known gap**: they do not cover a non-regular entry that carries *non-empty* bytes. Go's `tar.Reader` forces a zero-length body only for header-only types (symlink, hardlink, dir, char, block, fifo); `tar.TypeCont` and the vendor types `'A'..'Z'` have readable bodies, so with the type guard removed they would extract attacker-chosen bytes and the zero-length backstop would never fire. That is precisely why the type guard must never be removed, and why `TestExtractBinaryRejectsNonRegularEntries` includes content-carrying cases (`tar.TypeCont`, vendor `'Z'`) that fail on the *bytes returned* rather than on the error wording — the header-only fixtures alone would pin only the message
+  - **Path-guard order in `checkArchivePath` is load-bearing**: the UNC (`//`) arm must precede `path.IsAbs`, because `path.Clean` collapses `//host/share/x` to `/host/share/x` and the absolute arm would otherwise make the UNC arm dead code. Specificity before generality; the rejection is identical either way, only the diagnostic differs
+  - **Security fixtures**: `internal/selfupdate/archive_security_test.go` builds deliberately malformed archives (absolute, drive-absolute, UNC, `..`, empty-name, non-regular and oversized entries) entirely in memory and asserts REJECTION — nothing is written, linked, followed or executed. `buildTarGzEntries`/`buildZipEntries` are the full-control builders (`buildHostileTarGz`/`buildHostileZip` are the same thing under an intent-revealing name); `buildTarGz`/`buildZip` are the well-formed-file wrappers. Hostile literals live in `hostile*` constants. `TestExtractBinary` uses `wantErrContains`, not `wantErr bool`, and every hostile entry sits beside a valid `grant` — otherwise the "no binary" fallback, not the guard under test, is what produces the error
+  - `internal/selfupdate/fuzz_test.go` has native fuzz targets for `checkArchivePath` and both extractors, asserting contracts (nothing accepted escapes; a successful extraction is never empty). Their seed corpora run under a normal `go test`; extended fuzzing is run by hand (`-fuzz=FuzzCheckArchivePath -fuzztime=30s`), never in CI. Both archive targets shrink `maxDownloadBytes` to `fuzzMaxDownloadBytes` (64 KiB) through `withMaxDownloadBytes` — with the production 128 MiB cap, `readCapped` can `io.ReadAll` that much per exec and `FuzzExtractFromZip` collapses to **0 exec/sec while still reporting `PASS`**, making its exec count worthless. The cap keeps the fuzzer on archive *shape* rather than *size*. `withMaxDownloadBytes` takes `testing.TB` for this reason
+  - **Non-regular entries are rejected symmetrically**: tar filters on `hdr.Typeflag != tar.TypeReg`, zip on `f.Mode()&fs.ModeType != 0`. `IsDir()` alone was not enough — a zip entry with `fs.ModeSymlink` named `grant.exe` was accepted, extracting the link-target string as the binary. Never exploitable (extraction is in-memory and never follows a link, and the bytes are attacker-chosen either way), but the two formats must reject the same shapes
+  - The tar/zip declared-size asymmetry is **intentional**: tar checks every entry because reaching the next header inflates the current one, while `zip.NewReader` reads only the central directory and never opens a skipped entry. Pinned by `TestExtractFromZipIgnoresOversizeDecoy`
+  - The `len(data) != hdr.Size` / `!= f.UncompressedSize64` cross-checks are **unreachable** (a successful `readCapped` returns exactly the declared size; early exhaustion fails as `io.ErrUnexpectedEOF`). Kept as defense in depth with **no coverage claimed** — `TestExtractBinaryRejectsTruncatedArchive` pins gzip-stream truncation, not those branches
+  - Apply: `github.com/minio/selfupdate` v0.6.0 owns the staged-file write, the two-rename swap including the Windows path, and rollback — do not hand-roll this. grant adds the `fsync` of the staged file (minio does not sync) plus a best-effort directory sync. Seams: `applyWithOptions`, `prepareFn`, `commitFn`, `syncStagedFileFn` in `internal/selfupdate/apply.go`. `syncStagedFileFn` is a **test seam only** — `syncStagedFile` already returns `f.Sync()` errors and `applyWithOptions` already aborts before commit on them; the seam exists to assert the call *order* and the abort. The real `f.Sync()` failure is exercised on Unix only, via a FIFO staged path (`fsync` on a FIFO returns `EINVAL`); there is no portable equivalent, so that file is **build-excluded** on Windows (`//go:build !windows`, not `t.Skip` — the `t.Skipf` inside fires only when `mkfifo` itself is unavailable). The test asserts the error *is* `EINVAL`/`ENOTSUP`, not merely non-nil: without that, a Unix where `os.OpenFile(fifo, O_RDWR)` fails would return the **open** error, the test would still pass, and the mutation would silently survive
   - **Atomicity, precisely:** each rename is atomic, so the installed binary is never partially written. The *pair* is not: a kill between the two renames, or a failed second rename whose rollback also fails, leaves the binary path absent with `.grant.old`/`.grant.new` beside it. `InterruptedUpdate()` detects that state and `recoveryHint()` prints the `mv` command that fixes it. Do not describe this as fully atomic
   - `selfUpdater` in `cmd/interfaces.go` is defined over grant-owned types: `UpdateSelf(ctx, current string) (newVersion string, updated bool, err error)`
   - **End-to-end apply test:** `internal/selfupdate/e2e_test.go`, build tag `selfupdate_e2e`, run with `go test -tags=selfupdate_e2e ./internal/selfupdate/`. Everything else in `apply_test.go` swaps inert byte blobs; this compiles two real fixture binaries (a dependency-free module built with `-ldflags -X main.version=...`, so no network), executes one, and replaces it through `applyBinaryTo`/`applyWithOptions` **while a process is still running from that image**. That running child is what makes the Windows path real — a running `.exe` cannot be deleted, only renamed. `heldProcess.alive()` asserts the child survived the swap so the held cases cannot silently degrade into the idle cases. Covers success, rollback after a failed second rename (the restored file must still *execute*), and debris