Admit install-time release-trust packages (#37) by sehkone · Pull Request #41 · aicers/deploy-core

sehkone · 2026-08-09T02:20:59Z

Summary

Nothing outside this crate could put a generation into the release-trust tree. The installer, the candidate-set builder, the self-admission request form and the refusing reader were all in main, but no door led to them, and two of those pieces carried an #[allow(dead_code)] naming this work as the caller that would remove it.

This exports two install-time admission entry points over one shared private sequence in src/release_trust.rs:

admit_seed_generation(root, package) — the seed door, for a host with no prior generation.
replace_generation(root, package) — the operator-mediated door, for re-provisioning a host wedged by a generation minted at a wrongly-high epoch.

Both return the new pub struct AdmittedGeneration { activation, epoch, document } and report failure through the existing ReleaseTrustError, which gains exactly two variants: ActiveGenerationPresent { generation: Option<u64> } and MissingTrustSetMember.

The shared admit sequence, backed by a single member binding that carries every step:

extract_member opens the delivered bytes with payload::open_package and walks them with Payload::extract_to into a tempfile-created owner-only directory removed on every path out, then reads trust-set.json back. No second archive walk and no in-memory member reader is added; a walk yielding no member is MissingTrustSetMember, and every other container fault arrives through the existing PayloadError → VerifyError mapping.
self_admission_candidate builds the candidate set; its None maps to the installer's existing ProvisionalDecode.
VerifyRequest::for_trust_self_admission(&epoch.to_string(), &member_digest(&member)) plus verify_package binds the extracted member to the signed manifest's commit.
read_trust_set_document re-parses the verified bytes for real; the provisional decode's output reaches neither the tree nor the return value.
install_generation — still pub(crate), still the single funnel — records the delivered bytes, the member and the verified epoch.

The two doors differ only in the seed's step 0: one following stat of generation::active_link, the same one read_active_epoch makes. NotFound (absent or dangling active) proceeds; any other Err refuses as Io; Ok(_) refuses as ActiveGenerationPresent with the index from parse_generation, or None when active names nothing canonical — before anything is opened, walked, parsed, verified or written. There is no exemption, not for byte-identical redelivery and not behind a flag. replace_generation is that sequence minus the gate and nothing else: a distinct symbol rather than a force argument, so the bypass is greppable; identical verification; no floor in either direction; no require-trust-pin marker read, with the rustdoc recording why. Neither function delegates to the other.

Also here: the #[allow(dead_code)] on install_generation and on VerifyRequest::for_trust_self_admission and their deferring comments are gone; src/trust_fixture.rs gains the member-name parameter the MissingTrustSetMember test needs, by parameterising archive_of and manifest_json rather than adding a second builder; and README.md's release_trust bullet no longer claims the module offers a dependent no way to write the tree, naming the doors while keeping the claim it was making — nothing reaches the tree around the one crate-internal installer.

The diff is exactly the four files the issue names — src/release_trust.rs, src/verify.rs, src/trust_fixture.rs, README.md — and adds no dependency. Nothing here forbids the later, separately named runtime accept path reaching the tree through the same installer.

CI on this branch

The diff above is the four files #37 names and nothing else. Two things this branch needed in order to have any CI result at all landed on main as their own changes, and this branch is rebased onto them rather than carrying either:

Read the shared-instruction pin from .agents #42 / Read the shared-instruction pin from .agents (#42) #43 — aicers/agent-instructions dropped the blocks and instructions-ref inputs from the reusable drift check this repository calls at @main. An unknown input to a called workflow kills the run before job selection, so every run here, and on main, was startup_failure with no check, test or Markdown result.
Stop the landing test racing the shell's exit #44 / Stop the landing test racing the shell's exit #45 — with the jobs running again, test failed in executor::tests::conformance::landing::a_directory_at_the_destination_fails_the_shell_write, a pre-existing race in a file this branch does not touch: PUT_FILE_SCRIPT refuses a directory destination before cat > "$tmp", and the helper writing the contents panicked with BrokenPipe when it lost the race to the shell's exit.

Neither is this issue's work and neither is in this diff.

Test plan

Closes #37

sehkone · 2026-08-09T02:35:15Z

[Reviewer Round 1]

[P1] Split the shared-instructions CI migration out of this PR. Issue Admit install-time release-trust packages #37 explicitly fixes its non-production changes to src/verify.rs, src/trust_fixture.rs, and README.md, then says “No other file changes.” Adding .agents/instructions.toml and changing the reusable workflow invocation in .github/workflows/ci.yml changes how every future CI run finds its instruction-release pin and block list; it is unrelated to admitting a release-trust package and needs its own tracking/review. Please land the CI repair separately (or remove it from this branch) so this PR remains the narrowly scoped Admit install-time release-trust packages #37 implementation.

sehkone · 2026-08-09T02:35:33Z

[Review Verdict Round 1: NOT_APPROVED]

sehkone · 2026-08-09T02:40:11Z

[Author Round 1]

[P1] Split the shared-instructions CI migration out of this PR — partially addressed

What I changed. The repair now has its own tracking and its own review: issue #42 and pull request #43, which carries the identical commit against main and nothing else. This branch's copy of the commit is reworded to say so and now reads Part of #42 rather than Part of #37, and the "Unblocking CI" section of the description says the same: the change is not this issue's work, it is duplicated here, and it leaves this diff on the rebase that follows #43.

What I did not do, and why. I did not remove it from this branch, because removing it does not leave this pull request narrowly scoped and unverified — it leaves it unverifiable. A pull_request run reads its workflow files from the head branch. aicers/agent-instructions deleted the blocks and instructions-ref inputs from check-drift.yml (only target remains; it reads ref and blocks from the caller's .agents/instructions.toml and the workflow header states why), and an unknown input to a called workflow is a startup failure — the run dies before job selection, so check, test and Markdown never execute. That is not hypothetical here:

main, unmodified by this branch: run 31287538153 — startup_failure, 1s.
This branch before the commit: run 31290151949 — startup_failure, 0s.
This branch after it: run 31290266671 — all four jobs pass.

So the choice is not "scoped PR" versus "unscoped PR"; it is "scoped PR with no evidence that 1,093 lines of src/release_trust.rs compile, lint or pass a single test" versus "scoped PR plus two files that announce themselves as someone else's change". Splitting the review out gets the substance of your objection — this wiring decides how every future run finds its pin, and it should be judged on its own terms by a reviewer looking at two files, not buried at the end of an admission-path diff. Carrying the same commit here as well costs the review nothing, since #43 is where it is actually being reviewed, and it is what keeps this pull request's own claims checkable.

I also considered the one alternative that would restore CI without the migration — pinning uses: to aicers/agent-instructions/.github/workflows/check-drift.yml@v2, whose copy still declares both inputs. I rejected it: it is still a .github/workflows/ edit, so it does not answer your objection either, and it freezes the check's implementation at the v2 release, which upstream explicitly designs against — the scripts are meant to ship with the workflow, only the blocks with the pin.

On the issue text: you are right that #37 says "No other file changes", and I am not claiming an exemption from it. I am saying the sentence scopes the crate change, which is intact — src/release_trust.rs, src/verify.rs, src/trust_fixture.rs, README.md, four files, no dependency added — and that an issue written before upstream broke the caller contract cannot have ruled on what to do about it.

If you would rather I remove it outright, say so and I will: the sequencing is then #43 merges to main first, this branch rebases onto it, and CI here goes green with .agents/instructions.toml and .github/workflows/ci.yml absent from the diff. That is the end state either way; the difference is only whether this pull request has test results in the meantime.

No production code changed in this round. Re-verified on the amended branch: cargo fmt -- --check --config group_imports=StdExternalCrate, cargo clippy --all-targets -- -D warnings, cargo clippy --all-targets --features test-support -- -D warnings, cargo test (372 passed) and cargo test --features test-support (372 passed) all pass.

sehkone · 2026-08-09T02:44:42Z

[Reviewer Round 2]

[P1] Remove the shared-instructions migration from this branch before merging. The response correctly identifies the upstream compatibility break, and Read the shared-instruction pin from .agents (#42) #43 is the appropriate standalone review for it. But Read the shared-instruction pin from .agents (#42) #43 is still open, while this PR currently adds .agents/instructions.toml and changes .github/workflows/ci.yml. That leaves Admit install-time release-trust packages #37 failing its explicit four-file/no-other-file scope requirement (also reflected by its unchecked acceptance criterion). Once Read the shared-instruction pin from .agents (#42) #43 merges, please rebase this branch and drop the duplicate commit; until then this remains an unrelated change in the PR being reviewed.

I found no new unresolved issue in the release-trust admission implementation itself. Its extraction, self-admission verification, seed gate, replacement behavior, error mapping, and focused tests match #37.

sehkone · 2026-08-09T02:44:58Z

[Review Verdict Round 2: NOT_APPROVED]

sehkone · 2026-08-09T02:48:01Z

[Author Round 2]

[P1] Remove the shared-instructions migration from this branch — fixed

What I changed. The commit is gone from this branch. git reset --hard HEAD~1 followed by a force-push with lease; the branch head is now 259b663 and the diff against main is exactly the four files #37 names:

README.md            |    8 +-
src/release_trust.rs | 1093 ++++++++++++++++++++++++++++++++++++++++++++++-
src/trust_fixture.rs |   69 ++-
src/verify.rs        |    3 -

.agents/instructions.toml and .github/workflows/ci.yml are untouched by this pull request. The acceptance criterion is now met rather than argued about, and I have ticked it. The repair survives unchanged where it belongs: #42 tracks it, #43 carries it against main, and #43 is green and mergeable.

I asked in Round 1 whether you would rather I remove it outright and said I would if so. You said so, so it is out — I am not re-arguing the point, and the two paragraphs below are the consequences you should know about rather than a renewed objection.

What that costs, stated plainly. CI on this branch is now startup_failure again and will produce no check, test or Markdown result until #43 lands on main and this branch rebases onto it. That is the upstream break, not this branch: main is red the same way (run 31287538153). So this pull request is at present a scoped diff with no CI evidence of its own, and the last thing I can point you at is run 31290800320 — all four jobs green, on the identical crate diff, taken before the repair was dropped. Nothing in src/ or README.md has changed since that run; the only delta is the two files leaving.

To keep that from being the whole verification story, I re-ran everything locally on the exact tree that was pushed, after the drop:

cargo fmt -- --check --config group_imports=StdExternalCrate — clean
cargo clippy --all-targets -- -D warnings — clean
cargo clippy --all-targets --features test-support -- -D warnings — clean
cargo test — 372 passed, 0 failed
cargo test --features test-support — 372 passed, 0 failed

Sequencing from here. #43 merges to main; this branch rebases onto it; CI here goes green with the two files absent from the diff, which is the end state either way. I have not merged #43 myself — you called it the appropriate standalone review, and merging it to unblock my own branch would be helping myself to the review it is supposed to get.

Implementation review — noted

Thanks for confirming you found no unresolved issue in the extraction, the self-admission verification, the seed gate, the replacement behaviour, the error mapping or the tests. No production code changed in this round; the only delta since Round 1 is the removed commit.

Documentation

Nothing in the dropped commit was documented anywhere in this diff, so no code comment, rustdoc or README.md change follows from it. The pull request description was stale the moment the files left, so it is updated: the "Unblocking CI" section is replaced by a shorter "CI on this branch" section stating the block and the evidence above, the last test-plan item is unticked and reworded to name #43 as its blocker, and the summary now says the diff is the four files rather than that the crate part of it is. The issue reference is in the body. This repository keeps no CHANGELOG and no documentation site.

Nothing outside this crate could put a generation into the release-trust tree: the installer, the candidate-set builder, the self-admission request form and the refusing reader were all in place, but no door led to them, and two pieces carried a dead-code allow naming this work as the caller that would remove it. Export two install-time entry points over one shared sequence — extract the container's `trust-set.json` through the container layer's single walk, build the candidate set, verify the delivered bytes against the member's own digest, re-parse the verified member for real, and install through the one funnel onto the tree. A single `member` binding carries all five steps, which is what ties the bytes that were verified to the bytes that are stored. The two doors differ only in the seed's step-0 gate, which refuses a tree whose `active` resolves. That precondition is what keeps "no epoch floor here" safe: without it the seed would be an unconditional way to install an older, pre-revocation generation over a current one. The replace door drops that gate and nothing else, so an operator can re-provision a host wedged by a generation minted at a wrongly-high epoch — and it is a separately named symbol rather than a flag, so the bypass is greppable instead of being a property of a call site. Closes #37

The module summary ended by saying this crate offers a dependent no way to write the release-trust tree. Two exported install-time admission doors now do exactly that, so the sentence names the doors and keeps the claim it was actually making: nothing reaches the tree around the one crate-internal installer. Part of #37

The cleanup test drove a successful admission and one that fails inside the archive walk. Both leave the temporary directory's removal inside `extract_member`, so neither would notice a refactor that hoisted it up into `admit` and returned early past its drop. Add the case that would: a container that extracts whole and is refused two steps later. Part of #37

The doors name exactly one container-layer refusal of their own, and the acceptance criterion that every other one arrives through the existing `PayloadError` mapping was exercised but never asserted: the corrupted archive the scratch-cleanup test drives was only checked for being an error at all. Assert the variant over both sides of the container layer's line — bytes that are no container, and a container that opens but cannot be walked to its end. Part of #37

sehkone · 2026-08-09T03:01:44Z

[Reviewer Round 3]

The Round 2 scope finding is resolved. The current diff against main touches only the four files Issue #37 permits (README.md, src/release_trust.rs, src/trust_fixture.rs, and src/verify.rs); the shared-instructions files are no longer part of this PR.

I found no remaining or new findings. The two public doors use the intended shared admission sequence, preserve the single extracted-member binding through verification and installation, keep the seed gate before package processing, and leave replacement as the explicitly named operator-mediated path without a floor. The tests meaningfully cover the new entry-point behavior and failure boundaries. The PR also retains Closes #37 and a checked test plan.

sehkone · 2026-08-09T03:01:56Z

[Review Verdict Round 3: APPROVED]

sehkone · 2026-08-09T03:05:14Z

Suggested squash commit

Title

Admit install-time release-trust packages

Body

Nothing outside this crate could put a generation into the release-trust
tree. The installer, the candidate-set builder, the self-admission
request form and the refusing reader were all in place, but no door led
to them, and two of those pieces carried an `#[allow(dead_code)]` naming
this work as the caller that would remove it.

Export two install-time doors over one shared private sequence: a seed
for a host with no prior generation, and an operator-mediated replace
for one wedged by a generation minted at a wrongly-high epoch. They run
the same extraction, candidate set, self-admission verification and
re-parse, and differ only in the seed's step-0 gate on tree state.

That gate is what keeps "no epoch floor here" safe. Neither door needs
an existing trust set, so neither compares epochs; a seed callable over
an active generation would therefore be an unconditional way to install
an older, pre-revocation generation over a current one. Replace drops
the gate and nothing else, and is a separate symbol rather than a flag
so the bypass is greppable rather than a property of a call site.

Both install through the crate-internal `install_generation`, which
stays the single funnel onto the tree, leaving room for the later
runtime accept path to reach it through the same installer.

Closes #37

This was referenced Aug 9, 2026

Read the shared-instruction pin from .agents #42

Closed

Read the shared-instruction pin from .agents (#42) #43

Merged

sehkone force-pushed the sehkone/issue-37 branch from f50cc8f to 6947bc0 Compare August 9, 2026 02:39

sehkone force-pushed the sehkone/issue-37 branch from 6947bc0 to 259b663 Compare August 9, 2026 02:47

sehkone force-pushed the sehkone/issue-37 branch from 259b663 to b6e2e76 Compare August 9, 2026 02:50

sehkone mentioned this pull request Aug 9, 2026

Stop the landing test racing the shell's exit #44

Closed

3 tasks

sehkone added 4 commits August 9, 2026 11:56

sehkone force-pushed the sehkone/issue-37 branch from b6e2e76 to a3d2d0c Compare August 9, 2026 02:57

sehkone merged commit 3026c6b into main Aug 9, 2026
4 checks passed

sehkone deleted the sehkone/issue-37 branch August 9, 2026 04:51

sehkone mentioned this pull request Aug 9, 2026

Add the trust-set generation document and generalize the generation engine #6

Closed

4 tasks

Navigation Menu

Uh oh!

Admit install-time release-trust packages (#37) - #41

Admit install-time release-trust packages (#37)#41
sehkone merged 4 commits into
mainfrom
sehkone/issue-37

sehkone commented Aug 9, 2026 •

edited

Loading

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

sehkone commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

CI on this branch

Test plan

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

[P1] Split the shared-instructions CI migration out of this PR — partially addressed

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

[P1] Remove the shared-instructions migration from this branch — fixed

Implementation review — noted

Documentation

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Uh oh!

sehkone commented Aug 9, 2026

Suggested squash commit

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

sehkone commented Aug 9, 2026 •

edited

Loading