Skip to content

Fix deploy.jq arch filter precedence and harden registry reading helpers - #149

Open
tianon wants to merge 1 commit into
docker-library:mainfrom
infosiftr:hardening
Open

tianon wants to merge 1 commit into
docker-library:mainfrom
infosiftr:hardening

Conversation

@tianon

@tianon tianon commented Nov 3, 2026

Copy link
Copy Markdown
Member

Fix the "defense in depth" arch filter in deploy.jq: .annotations[...] // "" == $i.build.arch parses as a // ("" == arch) (// binds looser than ==), so any manifest carrying an arch annotation (even an empty one) passed the select no matter its architecture. 3dcc133 added the filter that way, so it has never actually filtered anything (the arch filtering in builds.json generation has been the only effective guard).

Also tighten and share some registry plumbing:

  • readJSONHelper now reads leftovers through decoder.Buffered(): json.Decoder reads ahead, so the old trailing-data check never saw whatever was already buffered and silently accepted trailing garbage ({"a":1} x passed)
  • add isDescriptorDataValid and descriptorDataReader, replacing two ad-hoc Data != nil && len == Size checks in SynthesizeIndex: embedded data must now also have a valid digest that matches its contents, so a descriptor with bad embedded data is ignored (and re-fetched) instead of trusted
  • add sm.Map, a typed wrapper around sync.Map, and use it in cmd/builds, cmd/deploy, and registry so the compiler checks the types and the .(T) assertions go away
  • prefix SynthesizeIndex errors with the ref and name the algorithm in validate_oci_digest messages
  • add tests for the above, including a fully synthetic .test/deploy-arch-mismatch jq test that fails against the old arch filter

Establish BASHBREW_META_SCRIPTS as the one canonical way to locate this checkout: it is already the name Jenkinsfile.build sets, helpers/*.sh require, and the commands generated by meta.jq expand, so rather than invent a second name or recompute $dir/.. in every script, .test/test.sh now computes it once and uses it everywhere, and Jenkinsfile.meta exports it so every entry point agrees on where the jq includes and helpers live. Nothing in the Jenkinsfile.meta job reads it yet: it is set ahead of code that needs to find those includes and helpers.

(This was assisted by "claude my eyes right out", but only to collect all the changes, summarize them, and write some additional tests -- all the original code was written by hand, by me, months ago as part of the signing work I have in-progress but isn't ready to merge so a lot of these fixes can/should get in in the meantime, especially the bug fixes.)

…lpers

Fix the "defense in depth" arch filter in `deploy.jq`:  `.annotations[...] // "" == $i.build.arch` parses as `a // ("" == arch)` (`//` binds looser than `==`), so any manifest carrying an arch annotation (even an empty one) passed the `select` no matter its architecture.  3dcc133 added the filter that way, so it has never actually filtered anything (the arch filtering in `builds.json` generation has been the only effective guard).

Also tighten and share some registry plumbing:

- `readJSONHelper` now reads leftovers through `decoder.Buffered()`:  `json.Decoder` reads ahead, so the old trailing-data check never saw whatever was already buffered and silently accepted trailing garbage (`{"a":1} x` passed)
- add `isDescriptorDataValid` and `descriptorDataReader`, replacing two ad-hoc `Data != nil && len == Size` checks in `SynthesizeIndex`:  embedded `data` must now also have a valid digest that matches its contents, so a descriptor with bad embedded data is ignored (and re-fetched) instead of trusted
- add `sm.Map`, a typed wrapper around `sync.Map`, and use it in `cmd/builds`, `cmd/deploy`, and `registry` so the compiler checks the types and the `.(T)` assertions go away
- prefix `SynthesizeIndex` errors with the ref and name the algorithm in `validate_oci_digest` messages
- add tests for the above, including a fully synthetic `.test/deploy-arch-mismatch` jq test that fails against the old arch filter

Establish `BASHBREW_META_SCRIPTS` as the one canonical way to locate this checkout:  it is already the name `Jenkinsfile.build` sets, `helpers/*.sh` require, and the commands generated by `meta.jq` expand, so rather than invent a second name or recompute `$dir/..` in every script, `.test/test.sh` now computes it once and uses it everywhere, and `Jenkinsfile.meta` exports it so every entry point agrees on where the jq includes and helpers live.  Nothing in the `Jenkinsfile.meta` job reads it yet:  it is set ahead of code that needs to find those includes and helpers.
@tianon
tianon requested a review from a team as a code owner November 3, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants