fix(packages): accept absolute paths in download

What does this MR do and why?

glab packages download --path fails outright for any absolute destination:

$ glab packages download -n my-package --version 1.0.0 --filename app.zip --path /tmp/downloads/app.zip
error creating directory: mkdirat /tmp/downloads: path escapes from parent

I found this while fixing the same defect in securefile download (!3642 (merged) / #8081) and flagged it there as out of scope. The two commands carry independent copies of this logic — including their own ensureDirectoryExists — so neither fix covers the other. Filed as #8478 (closed).

The destination was reached through an os.Root anchored at the working directory, and os.Root rejects absolute names by design. This anchors the root at the destination directory instead: relative paths are still created through a working-directory root and cannot escape it, while an absolute path names a location the caller asked for explicitly. That matches what @timofurrer suggested on #8081"if you give an absolute path to download, maybe we should just use that as the root."

A second bug this uncovers

Four call sites went through that root. The first aborts the command today, so the other three were unreachable — but one of them is not just a crash:

if _, err := root.Stat(o.path); err == nil && !o.forceDownload {
    return fmt.Errorf("file %s already exists; use --force to overwrite current file", o.path)
}

On an absolute path that Stat errors rather than reporting a match, so err == nil is false and the guard silently never fires. Once the command gets far enough to reach it, an absolute download would have quietly clobbered an existing file without --force. There is now a test pinning both halves of that guard.

Testing

Test_PackagesDownload_AbsoluteDestination adds five cases: absolute --path; an absolute path naming an existing directory (the isDirectoryPath branch); the overwrite guard refusing without --force and leaving the existing file byte-intact; --force overwriting; and temp-file cleanup after a checksum failure through an absolute root.

All five fail on main with the reported path escapes from parent error, and pass with this change. Both parts of the fix are mutation-checked:

Mutation Result
tempName back to the full joined name fails — including three pre-existing relative-path cases
absolute handling removed from destinationRoot fails all five new cases

The 11 existing Test_PackagesDownload cases pass unchanged; no existing assertion was edited. ensureDirectoryExists keeps its signature and behaviour for relative paths.

Relationship to !3642 (merged)

Same shape of fix as !3642 (merged), deliberately. I kept them as separate MRs because they are separate commands with separate issues, and !3642 (merged) is already under review — I did not want to grow its diff. The destinationRoot helper is duplicated between the two packages, mirroring the ensureDirectoryExists duplication that already exists there. Happy to extract a shared helper as a follow-up once both land, if you would prefer that to two copies.

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 standard library directly rather than inferring it.

Closes #8478 (closed)

Merge request reports

Loading