Add the OPR packaging request schema and its validator - #292
Open
firemanxbr wants to merge 1 commit into
Open
Conversation
A packaging request is the only artifact in this system written by someone outside the project, so it is the first and cheapest place a bad one should stop. This adds the request format and a validator that runs on every PR. docs/request-schema.md and schema/request.schema.json define the format. requests/ghostty.toml and requests/google-chrome.toml are the Surface A and Surface B examples. tools/opr-request-validate implements the eleven rules in the spec's Validation section, in two stages, because the PR check and the network check are separate: --offline (default) rules 1-5 and 7-11, no network at all --resolve adds rule 6, which resolves the upstream URL The offline stage is complete on its own and cannot reach the network. That is enforced rather than asserted: an offline rule's check signature receives no HTTP handle, so the compiler rejects any attempt to add one, and jsonschema is built with default-features = false because its defaults would link a remote $ref resolver into the binary. Both directions are verified under a network-denied sandbox; see the crate README. Structural checks are delegated to the JSON Schema rather than reimplemented in Rust, so the format has a single definition. What is hand-written is the translation from a schema error into a sentence a first-time submitter can act on. The binary refuses to start if the schema it loads has stopped saying "additionalProperties": false on any object, since failing closed on unknown keys is the invariant that stops the format growing by smuggling. Every request value is treated as untrusted on the way out as well as in. Values reaching a message are escaped, length-capped and delimited, and a test asserts that no raw control, bidirectional or zero-width character can reach stdout even when the request is built to produce one. The .gitignore change is needed because its bare `src/` rule, meant for pkgbuilds/*/src/ build directories, also matches a Cargo source directory and silently untracked the whole crate. The crate README documents nine places where the implementation had to interpret the spec. Three want a decision before this merges, the largest being that requests/google-chrome.toml fails rule 6 as literally written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Adds the packaging request format and a validator for it. This is "Next work" item 1 from the OPR design: the check that runs on every request PR, before a human reads the request.
Scope is the validator only — no factory, no build pipeline, no PKGBUILD generation.
What's here
docs/request-schema.mdschema/request.schema.jsonrequests/ghostty.tomlrequests/google-chrome.tomltools/opr-request-validate/The spec files and the validator are in one PR because the validator embeds
schema/request.schema.jsonviainclude_str!and a test validates the committedrequests/*.toml— it does not build without them.Using it
Exit
0valid,1violations,2could not run. One line per violation, naming the field and the rule:Every violation carries a remedy, asserted by a test. These are read by people filing a first request, for whom
invalid upstreamis a dead end.Two stages
--offline(default)--resolveupstreamThe offline stage is complete on its own and cannot reach the network. Enforced, not asserted:
checksignature receives no HTTP handle, so the compiler rejects any attempt to add one;jsonschemais builtdefault-features = false— its defaults enablereqwest+resolve-http+resolve-file, which would link a remote$refresolver into the offline binary.Verified under a network-denied sandbox in both directions: offline passes,
--resolvefails. That pair is the evidence, and the README has the command.Design notes for review
Structural checks are not reimplemented in Rust. The JSON Schema is the single definition of the format; duplicating it would create two that drift. What is hand-written is the translation from a schema error into an actionable sentence.
The binary refuses to start if the schema stops failing closed.
"additionalProperties": falseis what stops the format growing by smuggling, it lives in one keyword per object, and a future edit could drop one silently. Startup walks the schema and aborts if any object schema has lost it.Request values are untrusted on the way out too. Nothing here builds a shell command, so there is no interpolation site; what remains is display safety. Values reaching a message are escaped, length-capped and delimited. A test asserts no raw control, bidi or zero-width character reaches stdout even when the request is built to produce one.
Decisions needed before merge
The crate README documents nine places where the implementation had to interpret the spec. Three want your call:
requests/google-chrome.tomlfails rule 6 as literally written. Rule 6 requiresupstreamto be a repository root; the Surface B example points athttps://dl.google.com/linux/chrome/deb, which is not one and cannot be. I exemptedsource.kind = "vendor"from the repository-root assertion, keeping HTTPS, resolution and the off-host check. The alternative reading is that the example is wrong and Surface B needs a different identity anchor. This is a design question, not an oversight.Rule 8's allowlist does not exist in the spec. The rule names "the redistributable allowlist"; nothing defines its contents.
data/redistributable.txtis a conservative proposal, reviewable as its own diff. Absence means unreviewed, not refused — rejected with that reason, never silently downgraded to the recipe surface.Rule 4 needs the catalogue too, not just rule 5 — it checks collision against existing OPR and Arch
core/extranames. One--catalogueserves both: distance 0 is rule 4, distance 1 is rule 5, so one mistake yields one diagnostic. If two lists were intended — everything that exists for rule 4, a top-N subset for rule 5 — that needs a second flag.One gap in the rule as specified: rule 5 is Levenshtein, so transpositions pass.
ghotstyis distance 2 fromghosttyand clears the check despite being an obvious typosquat. Damerau-Levenshtein would catch it. Implemented as specified and pinned by a test so changing it is deliberate.Also decided on fail-closed grounds: a missing
--catalogueexits 2.--no-catalogueexists for local runs and must be passed deliberately. A gate that reports success because it was misconfigured is worse than one that fails.Not included
core/extra+ current OPR). CI must pass--catalogue; generating it is not part of this crate, and rules 4 and 5 are not live until someone does.requests/*.toml— it needs that catalogue first.Incidental
.gitignore's baresrc/rule, meant forpkgbuilds/*/src/build directories, also matches a Cargo source directory and silently untracked the entire crate. Added a scoped negation. Worth knowing for any future tool in this repo.The new
request-validatorCI job runscargo fmt --check,cargo clippy -D warningsandcargo test. The suite needs no network: rule 6's redirect and host logic is tested against scripted responses through anHttptrait, so it does not depend on what a third party serves today.🤖 Generated with Claude Code