Install plugins from a branch #7

2026-09-12T00:08:34+10:00

unkin-agent commented

2026-09-12 00:08:34 +10:00

Many upstream plugins publish no tags at all, so a tag-only installer cannot manage them.

Add a branch spec field, mutually exclusive with tag and validated the same way
Build archive/refs/heads/<branch> URLs for branch specs
Refetch a branch only on :ArtiPackUpdate or a forced sync, never from cache
Record the branch, archive digest and fetch time in the lockfile
Report a branch plugin as tracked and fetched, never as up to date
Document branch tracking in the README and help file

Many upstream plugins publish no tags at all, so a tag-only installer cannot manage them. - Add a `branch` spec field, mutually exclusive with `tag` and validated the same way - Build `archive/refs/heads/<branch>` URLs for branch specs - Refetch a branch only on `:ArtiPackUpdate` or a forced sync, never from cache - Record the branch, archive digest and fetch time in the lockfile - Report a branch plugin as tracked and fetched, never as up to date - Document branch tracking in the README and help file

unkin-agent added 1 commit 2026-09-12 00:08:34 +10:00

Install plugins from a branch

ci/woodpecker/pr/test Pipeline was successful

Details

ci/woodpecker/pr/pre-commit Pipeline was successful

Details

ci/woodpecker/pr/lint Pipeline was successful

Details

460dc2c5c8

Many upstream plugins publish no tags at all, so a tag-only installer cannot
manage them.

- Add a branch spec field, mutually exclusive with tag and validated the same way
- Build archive/refs/heads/<branch> URLs for branch specs
- Refetch a branch only on :ArtiPackUpdate or a forced sync, never from cache
- Record the branch, archive digest and fetch time in the lockfile
- Report a branch plugin as tracked and fetched, never as up to date

unkin-agent commented

2026-09-12 00:16:26 +10:00

Reviewed against the real checkout (pr7, 460dc2c), ran the full suite (186 passed) and mutation-tested the update-decision (lock.needs_install) and cache-bypass (installer.download) logic — both fail correctly when broken, so the coverage bites. Mutability semantics, lockfile round-trip/back-compat, ArtiPackList honesty, and ArtiPackClean safety all check out against the stated acceptance criteria; tag-install behavior and archive_root() are byte-for-byte unchanged.

One real gap found and reproduced:

tag/branch validation accepts an embedded NUL byte, which then silently truncates the URL actually fetched. bad_name() in lua/arti-pack/spec.lua only rejects /, \, whitespace, . and .. — not \0:

spec.parse({ 'o/r', branch = 'foo\0bar' })  -- accepted, ref == "foo\0bar"

Confirmed downstream impact: installer.lua passes the built URL to curl via vim.system{...} (argv table, not a shell), and Neovim's argv marshaling truncates a Lua string at its first NUL before the child ever sees it:

vim.system({'python3','-c','import sys;print(repr(sys.argv[1]))','foo\0bar'}, {text=true}, ...)
-- stdout: 'foo'

So a ref containing \0 is accepted, stored/displayed in full by the lockfile and :ArtiPackList, but the archive actually fetched corresponds to the truncated prefix — a real mismatch between what's validated and what's fetched. Not a shell-injection vector (no shell involved), and it's pre-existing for tag too (not introduced here), but the PR's own framing — "the same rules ... apply" to branch, validated the same way as tag — implies NUL is covered and it isn't. Worth a name:find('%z') check (or #name ~= #name:gsub('%z','')) alongside the existing pattern.

Minor doc nit: the digest is sha256(base64(bytes)), clearly commented in fs.lua as "not the file's sha256sum," but README/help only say "a digest of the archive" — worth one line there too so nobody diffs it against sha256sum and reports a false mismatch.

Reviewed against the real checkout (`pr7`, 460dc2c), ran the full suite (186 passed) and mutation-tested the update-decision (`lock.needs_install`) and cache-bypass (`installer.download`) logic — both fail correctly when broken, so the coverage bites. Mutability semantics, lockfile round-trip/back-compat, `ArtiPackList` honesty, and `ArtiPackClean` safety all check out against the stated acceptance criteria; tag-install behavior and `archive_root()` are byte-for-byte unchanged. One real gap found and reproduced: **`tag`/`branch` validation accepts an embedded NUL byte, which then silently truncates the URL actually fetched.** `bad_name()` in `lua/arti-pack/spec.lua` only rejects `/`, `\`, whitespace, `.` and `..` — not `\0`: ``` spec.parse({ 'o/r', branch = 'foo\0bar' }) -- accepted, ref == "foo\0bar" ``` Confirmed downstream impact: `installer.lua` passes the built URL to `curl` via `vim.system{...}` (argv table, not a shell), and Neovim's argv marshaling truncates a Lua string at its first NUL before the child ever sees it: ```lua vim.system({'python3','-c','import sys;print(repr(sys.argv[1]))','foo\0bar'}, {text=true}, ...) -- stdout: 'foo' ``` So a ref containing `\0` is accepted, stored/displayed in full by the lockfile and `:ArtiPackList`, but the archive actually fetched corresponds to the truncated prefix — a real mismatch between what's validated and what's fetched. Not a shell-injection vector (no shell involved), and it's pre-existing for `tag` too (not introduced here), but the PR's own framing — "the same rules ... apply" to branch, validated the same way as tag — implies NUL is covered and it isn't. Worth a `name:find('%z')` check (or `#name ~= #name:gsub('%z','')`) alongside the existing pattern. Minor doc nit: the digest is `sha256(base64(bytes))`, clearly commented in `fs.lua` as "not the file's sha256sum," but README/help only say "a digest of the archive" — worth one line there too so nobody diffs it against `sha256sum` and reports a false mismatch.

unkin-agent added 2 commits 2026-09-12 00:23:14 +10:00

Reject a NUL byte in every ref that reaches curl 3c73a83602

A NUL truncates a string when vim.system marshals argv, so the URL curl
fetches diverges from the ref that was validated, locked and displayed.
The whitespace class does not cover it.

- reject a NUL in tag, branch, the "as" directory name, an explicit url
  and the registry
- cover a NUL in the middle, at the end, and alone for each

Say the recorded digest is not a file checksum

ci/woodpecker/pr/test Pipeline was successful

Details

ci/woodpecker/pr/pre-commit Pipeline was successful

Details

ci/woodpecker/pr/lint Pipeline was successful

Details

3716131e56

sha256() refuses binary input, so the archive is hashed in its base64
form. Comparing it against sha256sum reads as corruption.

benvin merged commit c06ac229af into main

2026-09-12 00:45:00 +10:00

benvin deleted branch benvin/branch-specs

2026-09-12 00:45:00 +10:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/arti-pack#7