acl-agent: pass Nebraska-reported hash to Trident as sha384 - #769
Open
bfjelds (bfjelds) wants to merge 2 commits into
Open
acl-agent: pass Nebraska-reported hash to Trident as sha384#769bfjelds (bfjelds) wants to merge 2 commits into
bfjelds (bfjelds) wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new hash decoding path should avoid potentially unbounded allocation on untrusted input, and the updated docs should be made internally consistent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates trident-acl-agent to forward Nebraska’s reported package hash to Trident as a hex-encoded SHA-384 (matching Trident’s image.sha384 integrity check), and makes missing/unusable hashes a hard failure rather than silently skipping integrity verification.
Changes:
- Add
PackageHash::to_cosi_sha384()to convert Nebraska’s base64 “sha1” field into the hex SHA-384 string Trident expects. - Pass the converted hash into Trident
update/update_stagecalls in both the omaha-only and annotation-orchestrator flows, failing fast when absent/invalid. - Add the
base64workspace dependency (andhexdependency fortrident-acl-agent) plus unit tests for the conversion.
File summaries
| File | Description |
|---|---|
| crates/trident-acl-agent/src/omahaonly/mod.rs | Requires a Nebraska hash and forwards it to Trident’s one-shot update RPC. |
| crates/trident-acl-agent/src/core/nebraska/client.rs | Adds base64→hex SHA-384 conversion helper + tests; documents Nebraska field repurposing. |
| crates/trident-acl-agent/src/annotations/orchestrator.rs | Requires/validates hash early and forwards it to update_stage, emitting failure status on error. |
| crates/trident-acl-agent/Cargo.toml | Adds base64 and hex dependencies via workspace. |
| Cargo.toml | Adds base64 to [workspace.dependencies]. |
| Cargo.lock | Records the new dependency edges for trident-acl-agent. |
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
trident-acl-agent previously never passed the Nebraska-reported package hash to Trident, on the theory that Trident's own COSI metadata check made it redundant. Our Nebraska deployment actually reports a base64-encoded SHA-384 of the COSI metadata section in the field Omaha calls sha1 (not a real SHA-1), which is the same value/algorithm Trident validates via image.sha384 - so it can and should be used. - Add PackageHash::to_cosi_sha384() to decode the base64 value and hex-encode it for Trident's gRPC image.sha384 field, requiring exactly 48 decoded bytes (SHA-384 digest size). - Pass the converted hash from both the omaha-only flow and the annotation orchestrator's handle_stage, instead of None. - Treat a missing or unusable hash as a hard failure: an anyhow error in the omaha-only flow, and a StatusCode::OperationFailed status write in the orchestrator, rather than silently skipping verification. - Add base64 as a new workspace dependency for the decode step. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bfjelds (bfjelds)
force-pushed
the
user/bfjelds/acl-agent-verify-nebraska-hash
branch
from
September 3, 2026 23:38
fc40064 to
22f09f8
Compare
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The functional changes are coherent and tested, with only a minor doc-comment wording issue noted.
Review details
Suppressed comments (1)
crates/trident-acl-agent/src/core/nebraska/client.rs:91
- The doc comment says the Nebraska hashes “hash the file (not its contents)”, but this is confusing/incorrect—hashes are of bytes/content, and this module now explicitly documents that our deployment repurposes
sha1for a SHA-384 of the COSI metadata section. Reword this sentence to describe upstream semantics without the misleading “not its contents” phrasing.
/// Both values are base64-encoded and hash the *file* (not its contents).
/// Nebraska reports a SHA-1; `sha256` is present only when the file was
/// registered with one.
///
/// **Our Nebraska deployment does not follow that naming.** By internal
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
- Decode the Nebraska-reported hash directly into a fixed 48-byte buffer via decode_slice instead of Engine::decode's allocating Vec<u8> output, so an oversized base64 string is rejected without an allocation proportional to (untrusted) input size. - Reword the PackageHash doc comment: "hash the file (not its contents)" was self-contradictory next to the following paragraph describing an actual content hash; state plainly that both fields are content hashes per upstream Omaha/Nebraska semantics. - Add a regression test for the oversized-input case.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The hash forwarding/conversion logic matches Trident’s image.sha384 contract, is applied consistently across flows, and is covered by targeted unit tests.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
bfjelds (bfjelds)
added a commit
that referenced
this pull request
Sep 4, 2026
Trident now requires trident-acl-agent to forward Nebraska's reported package hash (#769), so the fake Nebraska proxy must seed a package hash in its Omaha response or the stage operation fails with OperationFailed. Reproduce the real deployment's quirk: base64-encode the COSI metadata SHA-384 into the package hash field, matching what trident-acl-agent's PackageHash::to_cosi_sha384 expects to decode.
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.
Problem
trident-acl-agentnever passed the Nebraska-reported package hash to Trident (update/update_stagewere always called withhash: None), on the theory that Trident's own COSI metadata check made it redundant.Our Nebraska deployment reports a base64-encoded SHA-384 of the COSI's metadata section in the Omaha
hashattribute (the field/type Omaha callssha1, but ours does not follow that convention) - the same value/algorithm Trident validates viaimage.sha384. So it can and should be forwarded.Changes
PackageHash::to_cosi_sha384()(core/nebraska/client.rs): decodes the base64 value, requires exactly 48 bytes (SHA-384 digest size), hex-encodes it for Trident's gRPCimage.sha384field.omahaonly/mod.rsandannotations/orchestrator.rs::handle_stage: pass the converted hash instead ofNone.anyhow!error.StatusCode::OperationFailedstatus write.base64.Testing
cargo check -p trident-acl-agent --all-targetscargo clippy -p trident-acl-agent --all-targets -- -D warningscargo fmt -p trident-acl-agent -- --checkcargo test -p trident-acl-agent --lib(168 passed, incl. 3 new tests forto_cosi_sha384)