Repository navigation
Conversation
…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.
yosifkit
approved these changes
Nov 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix the "defense in depth" arch filter in
deploy.jq:.annotations[...] // "" == $i.build.archparses asa // ("" == arch)(//binds looser than==), so any manifest carrying an arch annotation (even an empty one) passed theselectno matter its architecture. 3dcc133 added the filter that way, so it has never actually filtered anything (the arch filtering inbuilds.jsongeneration has been the only effective guard).Also tighten and share some registry plumbing:
readJSONHelpernow reads leftovers throughdecoder.Buffered():json.Decoderreads ahead, so the old trailing-data check never saw whatever was already buffered and silently accepted trailing garbage ({"a":1} xpassed)isDescriptorDataValidanddescriptorDataReader, replacing two ad-hocData != nil && len == Sizechecks inSynthesizeIndex: embeddeddatamust now also have a valid digest that matches its contents, so a descriptor with bad embedded data is ignored (and re-fetched) instead of trustedsm.Map, a typed wrapper aroundsync.Map, and use it incmd/builds,cmd/deploy, andregistryso the compiler checks the types and the.(T)assertions go awaySynthesizeIndexerrors with the ref and name the algorithm invalidate_oci_digestmessages.test/deploy-arch-mismatchjq test that fails against the old arch filterEstablish
BASHBREW_META_SCRIPTSas the one canonical way to locate this checkout: it is already the nameJenkinsfile.buildsets,helpers/*.shrequire, and the commands generated bymeta.jqexpand, so rather than invent a second name or recompute$dir/..in every script,.test/test.shnow computes it once and uses it everywhere, andJenkinsfile.metaexports it so every entry point agrees on where the jq includes and helpers live. Nothing in theJenkinsfile.metajob 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.)