Harden attachment path handling: validate remote tree-entry names and symlink destinations in `get`, symlink sources in upload

[Hinne1]

Findings from a multi-model security audit (council audit verb e2e run, jury-verified). None are rated critical — Linux exploitability of the main sink is capped by git's tree-entry-name validation — but the download path currently trusts remote-controlled names, and two vectors are concrete.

1. get: remote tree-entry path written unvalidated (internal/cli/get.go:~200)

runGet writes each attachment to filepath.Join(*outputDir, a.Path) where a.Path is copied verbatim from the remote tree entry (internal/gh/gitdata.go:~575). Nothing enforces that it is a bare basename.

• Windows (real traversal): a Unix-uploaded name like ..\evil passes git's tree checks (backslash is a legal tree-name byte) but filepath.Join on Windows treats \ as a separator → write escapes --output. The pre-flight conflict scan at get.go:~182 uses the same unvalidated path, so --force-less protection is bypassed too.
• Linux (defense-in-depth): git fsck rejects /, ., .. in tree names and GetAttachments fetches non-recursively, skipping non-blobs — so no traversal forms today. But the safe-basename invariant lives entirely upstream in the git object format; the write site should enforce it locally.

Fix: before the Stat/WriteFile loop, reject any a.Path that is not a clean portable basename — a.Path != filepath.Base(a.Path), contains / or \, is ./.., absolute, or volume-qualified (reuse the upload-side validateName logic from internal/cli/files.go).

2. get --force: destination symlink followed (internal/cli/get.go:~201)

os.WriteFile follows an existing symlink at the destination. A checkout/output dir containing an attacker-committed symlink + --force lets a contributor controlling the upload ref overwrite a file outside the output directory. (Jury 2/2 confirmed.)

Fix: Lstat the destination and reject symlinks; create with no-follow semantics (O_NOFOLLOW) after verifying the resolved path stays beneath the output dir.

3. Upload: expandFiles follows source symlinks (internal/cli/files.go:~27)

os.Stat accepts a committed symlink as a regular file; PushAttachments then os.ReadFiles the target — uploading e.g. a predictable local secret with the operator's credentials when globbing a checkout writable by less-trusted contributors.

Fix: use os.Lstat and reject symlinks; open validated files no-follow.

Minor

• internal/gh/comment.go:~228 — findMarkerComment selects the first comment containing <!-- gh-attach --> without checking the author, so any commenter can hijack/hold the marker slot (DoS on comment upsert at worst). Fix: require marker at expected position + author == authenticated account, or persist the created comment ID.
• internal/gh/repo.go:~44 — rejected remote URL is echoed verbatim into the error; embedded credentials can reach stderr/CI logs. Fix: redact userinfo before formatting.

---

Provenance: council audit security run against this repo (9 lanes, 3 models × personas, jury verification); flagship finding converged across 5 independent lanes and was code-verified by hand.