Repository navigation
feat(minimald): deliver each GitHub grant's sealed value into its variable in the box - #2491
Conversation
BEP-007: `min` now mints the GitHub grants before `CreateSession`, and the create request carries each sealed value. The daemon copies them into the box record. Every host launch places each value in its grant's environment variable, over any value already there. A grant that the mint gives no sealed value is refused by name. `BoxConfig` also gets an empty `root_certificate` field. The proxy does not publish the certificate yet (#2437). The new `Record` field also needs one line 8000 in each `Record` literal outside the task's files.
BEP-007: two GitHub grant boxes start headless. Each box's GH_TOKEN is a value of its own, and no plaintext GitHub token is in its environment. The case is NOT RUN on a lane with no Box Egress Proxy.
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
Babysitter review of T42 at 8c381bc, against the plan block on #2488.
Plan coverage. Every layer line is in the diff. Carry: BoxConfig in crates/minimald-rpc/src/lib.rs gains sealed_deliveries and root_certificate (the latter None until #2437 publishes a certificate, and read by nothing yet, as the plan allows), the constructor and the round-trip tests take them, build_record in box_manager.rs copies the deliveries into boxes::Record, which holds them under #[serde(default)] so older records read back empty. Place: HostParams carries the deliveries from the record and Host::build puts each sealed value into the connection-env layer over any earlier value, so the per-attach env files the shell re-reads, every exec and every hook see it and nothing else. The create.rs line from arch C2 on #2490 is done by moving MintGrants before CreateSession so the create request carries the sealed values; a missing value for a declared grant refuses the start by name. The e2e case starts two grant boxes headless on macOS, reads each box's GH_TOKEN, refuses any GitHub-shaped plaintext in the environment and requires the two values to differ, with the same NOT RUN gates as T14's proof. The plan's three test lines ran green in the round table; CI is in progress at this head.
Design points for arch. (1) The mint now runs before the create, so a mint failure withdraws only the row (no session exists yet), and an autogen-name collision re-mints for the new id while the replaced registration forgets the old one at the proxy. (2) The sealed value now rides the box record on disk so every relaunch places it; it is sealed to the proxy key, and Debug redacts it. (3) The shell's first environment does not hold the value, because the launcher is outside the layers; the value arrives on the per-attach re-read before the first command, and every exec and hook has it from the start.
Commits. Two, both Conventional Commits clean.
Layer exception. Eight files outside the layers, each one line sealed_deliveries: Vec::new(), in a full Record literal that stopped compiling: the same shape as the ::default() updates accepted on #2365 under the epic owner's 2026-10-19 rule. Recorded in the body; the PR is undrafted under that rule.
Size. +491/-69 against an estimate of 520 lines: at estimate.
Verdict: on track; waiting on CI, the arch ruling and the runtime review at this head.
Generated by Claude Code
|
Approved: every review thread resolved at @norrietaylor resolved the last thread, which releases the approval this review had withheld. Nothing to do. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 4 conditions · the full ruling
The change carries each sealed value to the box and places it for exec, hooks and the per-attach env files. The value is not in the shell's launch environment, and Gatehouse §6.3 line 637 requires env values before the entrypoint runs. C1 adds the deliveries to attach_env.connection in BoxActor::launch_host. C2 removes the stale #2488 interim from mint.rs. C3 makes the e2e check the compact JWE form. C4 runs the e2e leg, which is NOT RUN at this head.
Outside the diff
crates/minimal/src/mint.rs:400: C2 · This PR is the flip that this interim names, and it closes #2488. Delete the interim text and theallow(dead_code)attribute onMinted::sealed.
Ruled at 8c381bc against gominimal/arch@109541a48c31 by claude-opus-5-5.
A sealed delivery's variable is written unquoted into the file the box's shell sources. The create now refuses a name that is not a POSIX variable name, and the placement skips one as well.
A grant whose name is not a POSIX variable name is refused before anything is created, and a grant the mint left with no sealed value is refused by name. Both end the run with exit 3.
…ch-sealed-value-into-its-grant-s-envir
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The change carries each sealed value to the box with no plaintext, and the compare closes the name injection. One gap stays. The host adds the sealed values after the launcher builds the shell's environment, so the box's shell starts without its grant's variable. Gatehouse §6.3 line 637 says env values must exist before the entrypoint runs. C1 moves the placement into host.rs before the launch. C2 adds a test that reads the launch environment.
Ruled at f00a7f0 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
A grant box starts with no prompt, and its grant's variable holds only the sealed value. A grant whose name is not a variable name is refused with exit 3 before anything is created. A grant the mint gives no sealed value is also refused with exit 3, and the message names the grant. Notes from the agent: Revise round 1. I touched crates/minimal/src/main.rs outside the layers. It is a 3-line downcast in failure_exit_code, plus one test, that maps the new GrantNotDelivered error to exit 3. Without it, the exit 3 that the task's diagnostics line requires was not reachable without a new BepRefusal variant in crates/boxes/src/core/bep.rs, which is also outside the layers. I did not run ./scripts/session-e2e.sh bep_box_env_holds_sealed_value_only: this box has no /dev/kvm. The NotSealed message has no next step because no command is proven to fix it, so it states the situation and stops. The client checks the names of GitHub grants only, not of every [session.secrets] key. The other keys deliver nothing in this task. A project can still set names such as LD_PRELOAD or BASH_ENV. A project can already set those through its loadout vars, so this change does not refuse them. |
|
/revise Carry the arch-review conditions at f00a7f0 (ruling 119988501499), the one condition from the ruling at 8c381bc that no round has touched, and the review finding at f00a7f0, in one round.
Arch C4 at 8c381bc (the e2e leg NOT RUN until #2275 starts Keep every commit message Conventional Commits compliant (type(scope): subject, subject at most 92 characters, body lines at most 100), as the Generated by Claude Code |
The launch now places each grant's sealed value in attach_env.connection before session_launcher builds the shell, so the values exist before the entrypoint runs. HostParams no longer carries the deliveries, and each launch logs one debug line per variable.
The store keeps them. Only the daemon's own launch reads them.
|
Round 2 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
A second grant box starts with no prompt. Its shell's launch environment holds the grant's sealed value in GH_TOKEN before the entrypoint runs, and holds no plaintext token. The GetSessionRecord reply no longer carries any box's sealed values. Notes from the agent: Two files outside the layers, both named by the directive. (1) crates/minimald/src/rpc.rs: serve_get_session_record now builds its reply through a new session_record_response, which clears Record.sealed_deliveries. GetSessionRecordResponse is the only RPC response that carries a boxes::Record. The store keeps the field, and no #[serde(skip)] is used. (2) crates/minimal/src/mint.rs: deleted the interim doc text and the cfg_attr(dead_code) attribute on Minted::sealed. The doc now says the create request carries the value to the grant's variable. Placement now happens in launch_host through a new box_runtime::place_launch_deliveries. It sets attach_env.connection from the placed map before session_launcher runs. The HostParams::sealed_deliveries field and the second placement in Host::new are gone, so each launch logs one debug line per variable. The launcher file is not changed. I did not run ./scripts/session-e2e.sh bep_box_env_holds_sealed_value_only. The script runs the whole VM e2e and does not filter by case. On Linux the case records NOT RUN (macOS only, and it needs bep serve, see #2275). I checked only bash -n on the edited script. cargo clippy -p minimald -p minimal --all-targets shows only warnings that were there before. I did not run just clippy-strict. |
There was a problem hiding this comment.
APPROVE withheld: 8 unresolved threads — minimal-foundry[bot] crates/boxes/src/lib.rs:1288, minimal-arch[bot] crates/minimald/src/box_actor/host.rs, minimal-arch[bot] scripts/session-e2e.sh:11623, minimal-arch[bot] scripts/session-e2e.sh:11574, minimal-foundry[bot] crates/boxes/src/lib.rs:1288, ….
minimal-review — advisory review
Verdict COMMENT — No findings.
Panel: 2 of 3 personas ran (correctness, security); did not complete: supply-chain.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The sealed value is now in the shell's launch environment before the entrypoint runs, as Gatehouse §6.3 line 637 requires. GetSessionRecord no longer returns the values. BEP-007 conforms on this head. One leg is open. The e2e case records NOT RUN on every lane until #2275 starts the Box Egress Proxy on the macOS lane. Section 6.10 line 857 says sealed delivery lands with its e2e. C1 keeps that leg open as a filed residual. The change can merge with C1 open.
Ruled at 7d41640 against gominimal/arch@109541a48c31 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @norrietaylor closed the last one. The review of 7d41640e17c3 withheld this approval on the threads open then. Posted as a comment, as the review itself was; the check and the label carry it.
There was a problem hiding this comment.
Babysitter review of T42 at 7d41640, against the plan block on #2488 and the two rounds since my review at 8c381bc.
Plan coverage. Unchanged from 8c381bc: carry (BoxConfig and Record hold the deliveries, the daemon copies them at create, older records read back empty), place (now at launch time: place_launch_deliveries puts each sealed value into the launch's connection layer before the launcher builds the shell, so the shell's launch environment, every exec, every hook and the per-attach files hold it above any earlier value), the create.rs reorder of the mint before the create, and the e2e case with its NOT RUN gates, JWE-form and plaintext checks. The plan's three test lines ran green in every round table.
Rounds verified. Round 1 (f00a7f0): a grant name that is not a POSIX variable name is refused at the client before any create, at the daemon before the store, and skipped at placement, each with a test including a shell-text name; the undeliverable-grant refusal is its own error type with exit 3. Round 2 (7d41640): launch-time placement with a test through layer_session_env, the session-record reply cleared of the values with a wire test, the stale mint interim and dead-code attribute removed, the JWE-form check added. Six commits, all Conventional Commits clean.
Arch. PASS_WITH_CONDITIONS at 7d41640 with C1 only: the e2e leg NOT RUN until #2275 starts the proxy, filed as a residual on #2275 as a test line on T18's e2e layer. The review at this head has no findings.
Layer exception. Eight one-line Record literal updates and the mint.rs doc and attribute deletion arch asked for, under the recorded rule.
Size. +771/-85 against an estimate of 520 lines: 1.6x, under the 3x cap.
Verdict: ready to merge once the KVM, Kani and macOS lanes finish at this head.
Generated by Claude Code
There was a problem hiding this comment.
Approved at 7d41640 by the babysitter under the epic owner's merge-on-green delegation for #1501. All twelve gates pass at this head: own review, arch pass (C1 filed on #2275), review concluded with no findings, no open threads, every bot resolution re-verified, five lane aggregators green, not a draft, no needs-human, no round in flight, and Closes #2488. The squash message is Conventional Commits compliant (feat(minimald): deliver each GitHub grant's sealed value into its variable in the box).
Generated by Claude Code
T42 — Deliver each sealed value into its grant's environment variable in the box
slice: S0
kind: task
covers: BEP-007
size: L
est: 260 code, 260 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics:
min box spec <box>shows each grant's variable name and never its value; a box whose create request carries no sealed value for a declared grant is refused at creation with exit 3 naming the grantobservability: one debug log line per delivered variable naming the box id and the variable, never the value
layers:
- carry:
- crates/minimald-rpc/src/lib.rs:
BoxConfigcarries the sealed deliveries and the root certificate, and the shared create-request constructor takes them- crates/minimal/src/cmd/boxes/create.rs: the
MintGrantsstep hands eachMinted.sealedto the create request as its grant's delivery in place ofdrop(minted)(arch C2 on #2490, residual filed here)- crates/minimald-rpc/src/tests.rs: the new fields in its full literals
- crates/minimald/src/box_manager.rs:
record_from_configcopies the deliveries into the box record- crates/boxes/src/lib.rs:
Recordholds the deliveries- crates/minimald/src/box_manager/tests.rs: the new fields in its full literals
test: cargo nextest run -p minimald sealed_deliveries_ride_the_record_to_the_host_params
- place:
- crates/minimald/src/box_actor/host.rs: builds
HostParamswith the deliveries- crates/minimald/src/box_runtime.rs: places each sealed value in its grant's environment variable and no plaintext
- crates/minimald/src/box_runtime/tests.rs: the placement test
test: cargo nextest run -p minimald sealed_value_is_placed_in_its_grant_variable_only
- e2e:
- scripts/session-e2e.sh: adds the case
test: ./scripts/session-e2e.sh bep_box_env_holds_sealed_value_only
produces: none
consumes:
- crates/minimal/src/mint.rs
- crates/minimald-rpc/src/lib.rs
proof: A second grant box starts with no prompt and holds only a sealed value in its grant's variable.
depends on:
- blocked by T14
Proof
On a macOS host with a GitHub sign-in and
bep serverunning,min session activateof a box that declares[session.secrets.GH_TOKEN] source = "broker"starts with no prompt.min session exec <box> printenv GH_TOKENprints a sealed value for that box and no plaintext token. A second grant box does the same with a value of its own. The new e2e case checks this, but it did not run here: this box is Linux, and the case records NOT RUN. The unit tests show the daemon half: the value rides the create request into the box record, and every host launch places it in the grant's variable over any older value.Notes from the agent: Files outside the layers: adding
sealed_deliveriestoboxes::Recordbreaks every fullRecordliteral. Each of these files gets one line,sealed_deliveries: Vec::new(),: crates/boxes/src/store.rs, crates/minimal-client/src/box_registration.rs, crates/minimal/src/cmd/boxes/vm_host.rs, crates/minimal/src/dash/render.rs, crates/minimal/src/dash/rpc.rs (two literals), crates/minimal/tests/dash_snapshots.rs, crates/minimald/src/exec/tests.rs, crates/minimald/src/store.rs. The plan did not list them.Design: the mint needs the box id, and the daemon reads deliveries from the create request. So the
MintGrantsstep now runs beforeCreateSession, andActivationStep::ORDERchanges to match. An autogen-name collision re-registers under a new box id. In that case the retry mints again for the new id, and the replaced proxy registration drops and forgets the old id. A mint failure now only withdraws the row, because no box exists yet to destroy.Placement: the host puts the sealed values into its connection-env layer. That layer goes over every command the host runs in the box (exec and hooks). It is also published to the per-attach env files, which the shell re-reads before every command. The shell's first environ does not hold the value: the launcher (box_runtime/launcher.rs) is outside the layers. The re-read before every co…[truncated]
Tests
cargo nextest run -p minimald sealed_deliveries_ride_the_record_to_the_host_paramscargo nextest run -p minimald sealed_value_is_placed_in_its_grant_variable_only./scripts/session-e2e.sh bep_box_env_holds_sealed_value_onlyVerify
min task run agent-verifyexited 101 under rustc 1.98.1, which the repository does not pin (1.97.0 in rust-toolchain.toml); the PR is not a draft on that alone — CI, which honours the pin, is the gate.Layer exception
Accepted by the epic owner on 2026-10-19 under the rule recorded for #2361, #2363 and #2365 (required one-line changes outside the planned layers), applied by the babysitter on 2026-10-22:
boxes::Recordgainssealed_deliveries, so every fullRecordliteral needs the one linesealed_deliveries: Vec::new(),. The eight files arecrates/boxes/src/store.rs,crates/minimal-client/src/box_registration.rs,crates/minimal/src/cmd/boxes/vm_host.rs,crates/minimal/src/dash/render.rs,crates/minimal/src/dash/rpc.rs(two literals),crates/minimal/tests/dash_snapshots.rs,crates/minimald/src/exec/tests.rsandcrates/minimald/src/store.rs, each test or fixture code with that one line and nothing else. Alsocrates/minimal/src/mint.rs: arch-review condition C2 at 8c381bc deletes the interim doc text and theallow(dead_code)attribute onMinted::sealed, since this PR delivers the value (round 2, 73877d2).Residuals
Filed by the babysitter from the arch rulings on this PR; it may stay open at merge.
bep_box_env_holds_sealed_value_onlyrecords NOT RUN on every lane until T18 T18 — Run the proxy beside gvproxy so git and gh reach GitHub from a box #2275 startsbep serve; T18's e2e layer carries the leg to record as a pass on the macOS lane, beside T14's leg (comment 6347200074 on T18 — Run the proxy beside gvproxy so git and gh reach GitHub from a box #2275).Out of scope
These files are outside the task's
layers:, so this PR is a draft:crates/boxes/src/store.rscrates/minimal-client/src/box_registration.rscrates/minimal/src/cmd/boxes/vm_host.rscrates/minimal/src/dash/render.rscrates/minimal/src/dash/rpc.rscrates/minimal/tests/dash_snapshots.rscrates/minimald/src/exec/tests.rscrates/minimald/src/store.rsCloses #2488
Generated by Claude Code