Skip to content

feat(pkgdef): add package definition library - #3001

Open
mikeland73 wants to merge 2 commits into
mikeland73/pkgdef-specfrom
mikeland73/pkgdef-lib
Open

mikeland73 wants to merge 2 commits into
mikeland73/pkgdef-specfrom
mikeland73/pkgdef-lib

Conversation

@mikeland73

Copy link
Copy Markdown
Collaborator

Summary

Second PR of the package definitions stack (spec: specs/package-definitions.md, added in the previous PR). Adds internal/pkgdef, a leaf package with no dependency on devpkg, lock or plugin:

  • Parse/Validate for *.pkg.json (hujson), with every error reported at once.
  • ParseRef for pkg:<ref>@<version> (version split at the last @).
  • Client.Load for path, https:// and file:// refs. Remote definitions are cached by content hash and can be pinned to a hash from the lockfile (ErrDefinitionChanged when the server now serves something else).
  • Version sources: static, http_json (jq via gojq), github_releases (drafts and pre-releases skipped, token only ever sent to the GitHub API host).
  • Client.Resolve/Lock: resolve latest, a prefix or an exact version, then render the artifact URL and fetch its sha256 (http_json or sha256_url, which handles bare digests and SHASUMS-style lists) for every supported system.
  • Client.Install: idempotent install into $XDG_CACHE_HOME/devbox/pkgs/<name>/<version>/<system>, serialized across processes with a file lock, verified against the lockfile digest, extracted with path and symlink sanitization (tar.{gz,xz,bz2,zst} and zip via mholt/archives, which the repo already uses), post_install hooks with their output kept for error messages.
  • manager and script strategies return ErrNotImplemented for now, as the spec marks them planned.

New direct dependency: github.com/itchyny/gojq (vendor-hash refreshed).

Test plan

  • go test ./internal/pkgdef/ (unit tests against an httptest "vendor": resolve, lock, archive and binary installs, concurrent installs, checksum mismatch, trust on first use, GitHub releases, remote pinning, extraction safety)
  • golangci-lint and gofumpt clean

🤖 Generated with Claude Code

Add internal/pkgdef, the library behind package definitions (see
specs/package-definitions.md): parsing and validation of *.pkg.json
files, pkg:<ref>@<version> reference parsing, loading local and remote
definitions (remote ones cached and pinned by content hash), version
resolution from static lists, JSON indexes (via gojq) and GitHub
releases, per-system artifact locking with checksums, and idempotent
installs into $XDG_CACHE_HOME/devbox/pkgs/<name>/<version>/<system>.

The package is a leaf with no dependency on devpkg, lock or plugin so
the wiring in a follow-up PR can build on it. The manager and script
strategies defined by the spec return ErrNotImplemented for now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@mikeland73
mikeland73 force-pushed the mikeland73/pkgdef-lib branch from 63a0202 to e28622f Compare October 26, 2026 00:57
# Conflicts:
#	go.mod
#	go.sum
#	vendor-hash
Comment on lines +101 to +103
resolved := filepath.Join(filepath.Dir(abs), filepath.FromSlash(target))
if !isInside(dest, resolved) {
return fmt.Errorf("archive entry %q: symlink target %q escapes the destination", name, target)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symlink containment is checked lexically, so a chain of symlinks can still write outside dest:

  1. x -> .. The target resolves to dest, so it is accepted.
  2. x/y -> ... abs is dest/x/y, so the lexical target dest/x/.. is dest and is accepted. On disk, x is dest, so y actually points at the parent of dest.
  3. A regular file x/y/evil. isInside passes lexically, MkdirAll and OpenFile follow the planted symlinks, and the file is written to <parent of dest>/evil.

The doc comment on extractArchive says symlinks whose target escapes dest are rejected, but this sequence gets past that. The Lstat check in writeRegular only looks at the final path component.

A fix is to resolve the real parent directory with filepath.EvalSymlinks before every write or symlink creation, and re-check it with isInside. Alternatively, use os.Root (Go 1.24+) for all of the extraction I/O.

Code

Comment on lines +102 to +104
if !isInside(dest, resolved) {
return fmt.Errorf("archive entry %q: symlink target %q escapes the destination", name, target)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symlink containment check is lexical, so a crafted archive can still write outside dest.

filepath.Join cleans .. textually, but the OS resolves it through real symlinks. Example archive entries:

  1. d/up -> .. (resolves to dest, so it passes the check)
  2. d/up2 -> up/.. (the join cleans it to dest/d, so it passes the check, but the OS resolves it to the parent of dest)
  3. regular file d/up2/evil

isInside and the Lstat check in writeRegular only look at the final component. The file is therefore written to <dest>/../evil, outside the install dir. This defeats the "symlinks whose target escapes dest are rejected" guarantee documented on extractArchive.

Suggested fix: resolve each parent directory with filepath.EvalSymlinks before writing files or symlinks and re-check containment. Alternatively, use os.Root (Go 1.24+) to scope all writes to dest.

extract.go L102-104

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant