Add an upload-hop verifier and a generation-scoped trust reader (#138) - #139
Conversation
An upload hop learns a package's build and architecture only from the package, and only once its signature has verified, but every verifier entry point took them in advance. Split authentication (signature, format floor, typed parse) from the statement checks so a caller can derive the request from the authenticated manifest, and add verify_upload on top of the existing contents pipeline: one shared post-authentication body, the verifying anchor's key_id and the raw manifest digest reported, and a disk budget, upload_staging_bound, a caller can compute and reserve before the call. active_trust_set reads two files through the active link, so an activation between the reads can pair two generations. Add generation_trust_set, which reads one gen-<n> directly through the same body, and parse_generation_name, so a caller resolving active once can decode its target with the engine's own rule. Expose TrustSet::is_withdrawn and TrustSet::epoch for re-checks against the same generation. The describe() helper in tests/verify_contents.rs matches ContentError exhaustively by design, so it gains an arm for the new Upload variant. Closes #138
The README's module summaries still presented verify_contents as the package module's only full-content entry point and active_trust_set as the only way to rebuild a trust set from the tree, so a reader would not learn that an upload hop and a caller resolving `active` itself have their own entry points. Part of #138
The release-trust module doc explained the two readers that go through `active` but not the new one that never does, and the README paragraph the previous commit touched was left with a ragged wrap. Part of #138
|
[Reviewer Round 1] Review verdict: Approve, with documentation cleanup requested. The upload path authenticates before deriving identity, then shares the statement checks and content verification path with One documentation finding: TrustSet::epoch calls its value the active generation’s epoch, although The PR body also repeats |
|
[Review Verdict Round 1: APPROVED] |
Suggested squash commitTitle Body |
Summary
Adds what an upload hop, such as REView's module store, needs in order to verify a package through this crate's one verifier. Before this change, every entry point required the component, version, commit and architecture up front, but an upload hop learns those only from the package itself, after the signature has been checked. This PR adds no second verifier. It changes no verdict, order or limit of
verify_package,verify_contentsoractive_trust_set.UploadRequest(src/verify.rs): holds only the deployment namespace.UploadRequest::newrefuses an invalid namespace with the existingInputError::InvalidNamespace, using the same rule asVerifyRequest::for_namespaced_package.src/verify.rs): the new crate-privateauthenticate_manifestruns the signature check, the format floor and the typed parse. It returns anAuthenticatedholding the parsed manifest, thekey_idof the anchor that verified the signature, and the SHA-256 of the raw manifest block.verify_signaturenow returns thatkey_idinstead of(), with its order and verdicts unchanged.authenticateandverify_package_boundedare nowauthenticate_manifestfollowed by the unchangedcheck_statements.package::verify_upload(newsrc/package/upload.rs):RetentionScopewith a budget ofupload_staging_bound, takes a snapshot, and authenticates.check_statementswith the namespaced request built from them.verify_authenticated, whichverify_contentsnow shares instead of keeping a copy.VerifiedUploadholding only metadata: the manifest, the derived build and architecture,key_id,manifest_sha256,package_sha256andpackage_len. Every snapshot is released and the private directory is removed before it returns.TRUST_TARGET, is refused asContentError::Upload(UploadRefusal::NoArtifacts | ReservedTarget). Both refusals happen only after the signature verifies. A package mixing builds is stillTargetMismatch, and one mixing architectures is stillArchitectureMismatch.package::upload_staging_bound: returnsmin(RetainedDisk, 2 × package_len + OuterUncompressedTotal)with saturating arithmetic. It is pure, so a caller can compute it and reserve that much disk before the call, andverify_uploadenforces exactly this budget. The rustdoc explains where the formula comes from.release_trust::generation_trust_setandparse_generation_name:generation_trust_setreadsgen-<n>/directly and never looks atactive. It shares one body (read_generation_material) withread_active_generation. A pruned generation is reported asIowith kindNotFound.parse_generation_nameexposes the generation engine's rule for canonical generation names.TrustSet::is_withdrawnis now public, andTrustSet::epochis new. Both are#[must_use].Tests cover each item in the issue's test plan:
verify_contents;key_id, including when the container's key hint names another anchor or is unusable;staging_parentbeing left unchanged, the high-water mark staying within the bound, and the bound being enforced;TrustSetaccessors.One existing test changed:
every_error_variant_and_operation_is_nameableintests/verify_contents.rshas an exhaustive match onContentError, so it now also names the newUploadvariant.cargo fmt, both clippy runs and bothcargo testruns listed inAGENTS.mdpass. There is no new dependency and noCHANGELOG.mdentry.Closes #138
Deviations from the issue
VerifyRequest::for_namespaced_package. Step 3 of item 3 asks for the request to be built withVerifyRequest::for_namespaced_package(component, version, commit, request.namespace()). Instead,verify_uploadcalls a new crate-privateVerifyRequest::for_upload(component, version, commit, &UploadRequest), which callsVerifyRequest::for_packageand then sets the namespace from theUploadRequest.for_packagehas only one refusal,InputError::ReservedTargetforTRUST_TARGET, andfor_uploadreturnsNonefor exactly that case, whichverify_uploadreports asUploadRefusal::ReservedTarget. The namespace was already checked withis_valid_segmentinUploadRequest::new, so checking it again could never fail. Callingfor_namespaced_packagewould have left a namespace-error branch that can never run, and the reserved-target case would have had to be picked out of anInputError. The request is the same onefor_namespaced_packagewould build.verify_package_boundeddoes not go throughauthenticate. Item 2 saysauthenticatebecomesauthenticate_manifestfollowed bycheck_statements, and thatverify_package,verify_package_boundedandverify_contentskeep their behaviour through it.authenticatedoes become those two calls, andverify_packagestill uses it.verify_package_boundedis now a new crate-privateauthenticate_bounded, which does the bounded container read and thenauthenticate_manifest, followed by the samecheck_statementscall.verify_contentsreaches the same sequence throughverify_retained→verify_package_bounded. This was done becauseverify_uploadneeds the bounded read and authentication, including the archive block's offset and length, without the statement checks. Putting that step in its own function letsverify_package_boundedandverify_uploadshare one body instead of keeping two copies. The steps run in the same order and give the same verdicts.every_error_variant_and_operation_is_nameableintests/verify_contents.rsmatches exhaustively on the publicContentError, so the newContentError::Upload(UploadRefusal)variant that item 3 requires would not compile without a match arm for it. The test now namesUpload(NoArtifacts)andUpload(ReservedTarget). No existing assertion or verdict changed.Test plan
BadSignature,BadSignature,UnknownKeyIdandRevokedKey. Each is paired with a manifest that would otherwise be refused asNoArtifacts,ReservedTarget,TargetMismatchorArchitectureMismatch, and the signature verdict wins every time.verify_contentsrefusal fixtures:verify_uploadandverify_contentsgive the same variant.verify_contentsis called with the fixture's true build, the namespace, its true architecture, andRetainedDiskset toupload_staging_bound.component,version,commit,target_arch,key_id,manifest_sha256,package_sha256andpackage_lenequal the fixture's build, architecture, signing key, raw-manifest digest, package digest and length.key_id()names the anchor that verified the signature, both when the container's key hint names another anchor that does not verify and when the hint is unusable.Upload(NoArtifacts);Upload(ReservedTarget);TargetMismatch;ArchitectureMismatchwhoseexpectedis the first entry's architecture.clumit-other, verified underUploadRequest::new("clumit-security"), isImage(NamespaceMismatch), and the same package verified under its own namespace passes;UploadRequest::new("")andUploadRequest::new("a/b"), among other invalid namespaces, are refused withInputError::InvalidNamespace.staging_parentholds exactly the entries it held before;upload_staging_bound;RetainedDiskone byte below that high-water mark, the call isLimitExceedednamingRetainedDisk;staging_parentis refused asIowithInspectStagingParent.upload_staging_boundatpackage_len0,1andu64::MAX, with default limits and withOuterUncompressedTotalandRetainedDisklowered, matches the formula and saturates instead of overflowing.admit_seed_generationand extended withreplace_generation, and each generation read by number returns its own epoch, anchors and withdrawn list, whichever generationactivenames;active: a generation still reads correctly whenactiveis a dangling symlink;Iowith kindNotFound;epochortrust-set.jsongives the same refusalactive_trust_setgives.parse_generation_namereturnsSomeforgen-1,gen-42,gen-0andgen-18446744073709551615(u64::MAX). It returnsNoneforgen-01,gen-+1,gen-1.tmp,gen-,active,gen-retention.tmp, an overflowing index,gen--1, a name with a trailing space, and multi-component paths.TrustSet::is_withdrawnis true for each listed triple and false when any part differs or two listed triples are mixed.TrustSet::epochequals the constructor argument, including0andu64::MAX.ContentError::Uploadvariant added toevery_error_variant_and_operation_is_nameable, as described under Deviations.cargo fmt -- --check --config group_imports=StdExternalCratepasses.cargo clippy --all-targets -- -D warningsandcargo clippy --all-targets --features test-support -- -D warningspass.cargo testandcargo test --features test-supportpass.