fix(securefile): accept absolute paths in download

What does this MR do and why?

glab securefile download --path /tmp/file.txt fails outright:

$ glab securefile download 1 --path /tmp/secure/file.txt
error creating directory: mkdirat /tmp/secure: path escapes from parent

--all --output-dir fails the same way for an absolute directory. Any absolute destination is rejected, which is what #8081 reports.

Both the destination directory and the temporary file were created through an os.Root anchored at the working directory, and os.Root rejects absolute names by design. There were three separate points that broke: ensureDirectoryExists at the CWD root, utils.CreateTemp on a joined path, and the final root.Rename.

This anchors the root at the destination directory instead:

  • A relative path is still created through a working-directory root, so it cannot escape the directory glab was run from.
  • An absolute path names a location the caller asked for explicitly, so it is created directly — which is what @timofurrer suggested on the issue: "if you give an absolute path to download, maybe we should just use that as the root."

Everything after that works on a plain file name inside the root, so saveFile has one path rather than one per destination kind.

Confining names that come from the API

--all builds each destination from file.Name, which the server supplies. Previously the working-directory root was what stopped a name like ../../elsewhere from escaping --output-dir. An absolute --output-dir has no such root left, so the name is now anchored to the root of --output-dir before being joined. Behaviour for a relative --output-dir is unchanged — the existing Handle secure file with path traversal case still produces the same error, byte for byte.

How to set up and validate locally

# Before: fails. After: writes the file, creating /tmp/secure as needed.
glab securefile download 1 --path /tmp/secure/file.txt

# Also fixed
glab securefile download --all --output-dir /tmp/secure

# Still rejected, as before
glab securefile download 1 --path ../../outside/file.txt

Testing

Test_SecurefileDownload_AbsoluteDestination adds five cases: absolute --path, absolute --path with --name, absolute --output-dir with --all, temp-file cleanup after a checksum failure through an absolute root, and confinement of a traversing server-supplied name. They live in their own function because the existing tables carry static CLI strings and these need a t.TempDir() interpolated.

All five fail on main with the reported path escapes from parent error, and pass with this change. I also mutation-tested each part of the fix:

Mutation Result
tempName back to the full joined name fails — including two pre-existing relative-subdirectory cases
server name no longer anchored to --output-dir fails the confinement case
absolute paths back through the working-directory root fails all five

The existing 27 cases pass unchanged; no existing assertion was edited. ensureDirectoryExists keeps its signature, so TestEnsureDirectoryExists_InvalidDirectory is untouched.

One implementation detail worth a reviewer's eye: os.Root.OpenFile reports Name() joined onto the root, and root.Stat/root.Rename then reject that name as escaping when the root is absolute. The temporary file is always a direct child of the root, so its base name addresses it. I verified this against the stdlib directly rather than inferring it — it is the specific trap that the previous attempt at this fix (!2761 (closed)) hit, where cleanup had to branch on absolute vs relative.

Out of scope

internal/commands/packages/download/download.go:192-198 has the same structure — a working-directory os.Root plus ensureDirectoryExists — so glab package download --path /abs fails identically. I left it out to keep this MR to the reported issue; happy to follow up if useful.

Relates to #8081

Merge request reports

Loading