Repository navigation
feat(min): refuse GitHub grants at activation that the box or the host cannot honour - #2365
Conversation
Add core::bep: the [network.bep] settings (steering defaults to dns),
the [secrets] grants, and the BEP-008, 009, 010, 017 and 056 checks.
CredentialedUpstream carries the settings and an empty one still
serializes as {}. Two test literals in minimald and minvmd move to
CredentialedUpstream::default() because the struct now has fields.
Read [session.network.bep] and [session.secrets] from the project's minimal.toml onto the box's credentialed-upstream declaration, check it against the GitHub module's host set before the daemon starts, and end a refused activation with exit 3. minimal gains a path dependency on bep for the host set.
|
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 |
|
Reviewed: 1 finding, none blocking at Address the comments on the diff. Clobber: none seen. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The checks are in the correct crate, and the refusal comes before the daemon starts. BEP-008, 010, 016 and 056 conform. The change counts every [secrets] entry as a credentialed upstream, a client_env copy included. It also opens the credentialed lane for a [network.bep] table alone. Networking design §5.4 line 157 and architecture.md line 917 say that the set is derived from broker grants and never declared. Make C1 in bep.rs and C2 in session.rs. Then the change conforms.
Ruled at b01c66c against gominimal/arch@3dab1097cda1 by claude-opus-5-5.
A project file that failed to load read as no grants declared, so a broken file skipped every GitHub grant check. Only a missing file now declares nothing. Any other load error refuses before the create.
There was a problem hiding this comment.
APPROVE withheld: 3 unresolved threads — minimal-foundry[bot] crates/minimal/src/cmd/session.rs, minimal-arch[bot] crates/boxes/src/core/bep.rs:192, minimal-arch[bot] crates/minimal/src/cmd/session.rs:597.
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: CONDITIONS · 3 conditions · the full ruling
The checks are in the right place, and the host-set and installation refusals conform. Three things deviate. The change counts every [secrets] grant as a credentialed upstream, client_env included. Section 5.4 line 157 derives that set from broker grants only, so BEP-009 and BEP-017 refuse boxes the design admits (C1). A [network.bep] setting alone also opens the credentialed lane (C2). A minimal.toml that does not load exits 1, but the Exit codes table gives 3 (C3). The off plus proxy_env refusal is escalated as a design question.
Ruled at b0dcc36 against gominimal/arch@3dab1097cda1 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:
Activating a grant box whose egress omits a GitHub host exits 3 and names each missing host. A project whose minimal.toml exists but does not load is now refused before the create, so a broken file can no longer skip the grant checks. Notes from the agent: Revise round 1. The one thread is valid and is fixed in crates/minimal/src/cmd/session.rs: credentialed_upstream_for now returns a Result. Only mfile::Error::NotFound means no declaration. Any other load error refuses with "Cannot start the box: the project's minimal.toml does not load: " and one debug log line. This refusal is a plain error (exit 1), not exit 3, because the box spec was not read. I did not run the boxes test lines again this round because this round changed no file in crates/boxes. cargo clippy -p minimal --all-targets shows no warning in minimal. Its warnings are in minimald and minimald-rpc, which this round did not change. |
|
/revise minimal-arch ruled CONDITIONS on b0dcc36 (check run 118380202174). Three code conditions, all yours this round; the escalated design question (steering C1, C2, C3, same function and Then reply on the three arch threads with the commit. Generated by Claude Code |
SecretGrant.source is now a required GrantSource enum (broker or client_env). Only a broker grant opens the credentialed upstream, so a client_env grant never fires BEP-009, BEP-017, the GitHub host-set check or the CA injection.
Only the flag or a broker grant opens the lane. The activation checks the project's bep settings and grants even with no lane, so steering off with proxy_env stays refused. An unloadable minimal.toml is now a typed ProjectFileInvalid error that exits 3.
…hub-grants-at-activation-that-the-box
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 8 unresolved threads — minimal-foundry[bot] crates/minimal/src/cmd/session.rs, minimal-arch[bot] crates/boxes/src/core/bep.rs, minimal-arch[bot] crates/minimal/src/cmd/session.rs, minimal-arch[bot] crates/boxes/src/core/bep.rs, minimal-arch[bot] crates/boxes/src/core/bep.rs, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The compare fixes the baseline points. Only a broker grant now makes a credentialed upstream, and an unloadable minimal.toml exits 3. One placement deviation is still open. The code reads quic443 from [session.network.bep], but architecture.md lines 874–889 and Gatehouse §6.3.2 put it in [network]. A project that writes it there is now refused with exit 3. Make the change in C1, and correct the module doc in C2.
Ruled at 9fc1593 against gominimal/arch@3dab1097cda1 by claude-opus-5-5.
|
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:
Activating a box whose project declares a broker GitHub grant, with an egress that omits a GitHub host, exits 3 and names each missing host. A client_env grant or a [session.network.bep] setting alone no longer opens the credentialed lane. An unloadable minimal.toml now also exits 3. Notes from the agent: C1: SecretGrant.source is a required GrantSource enum (broker, client_env). The new SecretGrant::is_credentialed() is true only for broker. has_credentialed_upstream, is_github, github_grants and the BEP-009 grant list use broker grants only. SecretGrant no longer derives Default. C2: project_bep_and_grants() loads the settings, credentialed_upstream_for() declares the lane only on the flag or a broker grant, and refuse_unhonourable_grants() always runs check_activation on the project's bep and secrets. C3: ProjectFileInvalid is a typed error (re-exported as minimal::ProjectFileInvalid) and failure_exit_code() maps it to 3. failure_exit_code lives in the min binary, so the lib test of that name asserts the typed error, and a bin test of the same name asserts the exit code 3, the same split the existing grant test uses. BEP-010 refusal kept as is, per the note. Also ran: cargo nextest run -p minimal for the three new tests (bep_settings_alone_activate_without_credentialed_upstream, only_broker_grants_declare_credentialed_upstream, steering_off_with_proxy_env_is_refused_without_a_grant), all pass. just clippy-strict was not run. |
|
/revise minimal-arch ruled CONDITIONS on 9fc1593 (check run 118394712429): the three baseline conditions are met, and two new ones remain. Take both this round and nothing else. C1, move C2, Reply on the arch threads with the commit. The Generated by Claude Code |
quic443 is a [network] setting in the design. BepSettings drops the field and keeps the Quic443 enum. SessionNetwork gains quic443, so [session.network] quic443 loads and [session.network.bep] refuses it. The bep module doc now says a GitHub grant needs source = broker.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
< 10BC8 p dir="auto">The change conforms to Gatehouse §6.10 Validation and networking §5.4. The new head movesquic443 to [session.network], which matches §5.4 line 166. Only broker grants make a credentialed upstream, as §5.4 line 157 requires. Each refusal exits 3 before the daemon starts. The refusal of off with proxy_env follows §5.4 line 172. Ask arch to clarify line 173 for off. The task does not need to change.
Ruled at 19d59f1 against gominimal/arch@3dab1097cda1 by claude-opus-5-5.
|
Round 3 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:
Activating a grant box whose egress omits a GitHub host exits 3 and names each missing host. In this round, Notes from the agent: Revise round 3 addresses C1 and C2 only, in commit 19d59f1. C1: BepSettings no longer has quic443, and the Quic443 enum stays. SessionNetwork has |
There was a problem hiding this comment.
Babysitter review of 19d59f1 (T13: refuse GitHub grants at activation that the box or the un-enrolled host cannot honour). Read as a COMMENT review so it starts no coder round.
What I checked. The settings and the checks live in boxes::core::bep and take the GitHub host set as an argument, so boxes has no edge to bep. GrantSource is a closed, required enum; only a broker grant makes a credentialed upstream, a GitHub grant, a BEP-009 refusal or a CA injection. refusals collects every reason in a fixed order (BEP-009, 008, 010, 017, 056), the host comparison lowercases and strips a trailing dot, and check_activation logs one debug line per refusal with rule and fields. In min activate the project's settings and grants are read with from_dir_recursive, a file that does not load is the typed ProjectFileInvalid mapped to exit 3, the checks run whether or not a lane is declared and before ensure_daemon, and the lane is declared by the flag or a broker grant only. quic443 parses under [session.network] and is refused under [session.network.bep]. CredentialedUpstream keeps its {} wire form when the settings are empty. The four proptests pin each refusal against an oracle, including the client_env-only no-refusal cases. The out-of-layers edits are the two mechanical ::default() lines and the bep path dependency, which the epic owner accepted. No blocking finding from me.
Findings, none blocking
-
SecretGrantisdeny_unknown_fieldswith a two-variantGrantSource, so a design-validsource = "store"grant (store references, BEP-036) refuses the activation with exit 3 today. That fails closed, and the arch noted it; worth a sentence in theGrantSourcedoc so the refusal does not read as a bug when BEP-036 lands. -
ActivationPlan.inject_caandSessionNetwork.quic443have no consumer yet (the arch noted both): the plan's CA bit and the UDP 443 stance are computed and parsed but not carried intoBoxPolicy. Those are BEP-018 and §5.4 line 177, outside this task; a doc line on each saying so would stop the next reader looking for the wiring. -
The runtime's finding that an unloadable parent
minimal.tomlnow refuses an activation that used to start is correct as a description, and it is what the arch's C3 requires: a file that cannot be read cannot rule out grants. I resolved that thread with the design reason.
Generated by Claude Code
There was a problem hiding this comment.
Babysitter approval of 19d59f1 for the epic owner: every gate item passes on this head (arch PASS, runtime review concluded, every thread resolved and re-verified, all lane aggregators and the CLA status green, squash subject lints as Conventional Commits).
Generated by Claude Code
T13 — Refuse GitHub grants at activation that the box or the un-enrolled host cannot honour
slice: S0
kind: task
covers: BEP-008, BEP-009, BEP-010, BEP-016, BEP-017, BEP-056
size: L
est: 400 code, 480 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics: a refused activation exits 3 and prints, for a missing host, "This box declares a GitHub grant, but its egress does not allow: . Add them to egress.allow_dns_hosts." on stderr, pinned by prop_grant_host_set_outside_egress_is_exit_3
observability: one debug log line per activation refusal naming the rule and the fields
layers:
- spec:
- crates/boxes/src/core/bep.rs (new):
steering(defaultdns),proxy_env,no_proxy,quic443and the GitHub grants; the BEP-008, 009, 010, 017 and 056 refusals- crates/boxes/src/core/mod.rs: module line
- crates/boxes/src/lib.rs:
CredentialedUpstreamcarries the settings, defaulting so an empty declaration reads as before- crates/boxes/Cargo.toml:
proptestas a dev-dependency- Cargo.lock: the dev-dependency edge
test: cargo nextest run -p boxes prop_grant_host_set_outside_egress_is_exit_3
test: cargo nextest run -p boxes prop_none_mode_with_grant_is_exit_3
test: cargo nextest run -p boxes steering_off_with_grant_warns_and_injects_no_ca
test: cargo nextest run -p boxes steering_off_with_proxy_env_is_refused
test: cargo nextest run -p boxes steering_defaults_to_dns
test: cargo nextest run -p boxes prop_dns_steering_without_resolver_is_exit_3
test: cargo nextest run -p boxes prop_installation_mode_unenrolled_is_exit_3
- read:
- crates/mfile/src/lib.rs: reads
[session.network.bep]and[session.secrets]from the project's[session]table- crates/minimal/src/cmd/session.rs: validates before the create, passing the module's host set
- crates/minimal/src/main.rs: maps the refusal to exit 3
test: cargo nextest run -p minimal activation_refuses_grant_before_create_with_exit_3
produces:
- crates/boxes/src/core/bep.rs
consumes:
- crates/bep/src/module.rs
proof: Activating a grant box whose egress omits a GitHub host exits 3 naming each missing host.
depends on:
- blocked by #2258
Proof
A project whose minimal.toml declares a GitHub grant under [session.secrets.] (a github: scope), activated with egress that omits GitHub hosts, makes
min activatestop before the daemon starts or a box is created. It exits 3 and prints on stderr: "This box declares a GitHub grant, but its egress does not allow: uploads.github.com, codeload.github.com. Add them to egress.allow_dns_hosts." The message names each missing host. The same activation refuses network mode none with a grant, steering = "off" with proxy_env = true, dns or both steering (also the default) with no box DNS resolver, and mode = "installation" on this un-enrolled host. It warns when steering is off with a GitHub grant.Notes from the agent: Files outside the layers: (1) crates/minimal/Cargo.toml gains
bep = { path = "../bep" }. session.rs passes the GitHub module's host set, and minimal had no edge to the bep crate. bep is not in [workspace.dependencies], so I used a path dependency and did not touch the root Cargo.toml. (2) crates/minimald/src/net/switch.rs (2 lines) and crates/minvmd/src/box_registry.rs (1 line) are test code that wroteboxes::CredentialedUpstream {}. That form stops compiling once the struct has fields, so each now uses::default(). I did not compile the minimald or minvmd test targets. The change is mechanical because Default is derived.Design choices the spec excerpt does not fix: a grant is
[session.secrets.<VAR>]withsource,scopesandmode. It is a GitHub grant when a scope starts withgithub:. The settings ride CredentialedUpstream as nestedbepandsecretsfields, because serde cannot combine flatten with deny_unknown_fields. Empty settings are skipped, so{}reads and writes as before. Any grant or [session.network.bep] setting now makes the box declare a credentialed upstream, even without --credentialed-upstream. For BEP-017, "declares a credentialed upstream" means at least one grant, so a bare --credentialed-upstream flag…[truncated]Tests
cargo nextest run -p boxes prop_grant_host_set_outside_egress_is_exit_3cargo nextest run -p boxes prop_none_mode_with_grant_is_exit_3cargo nextest run -p boxes steering_off_with_grant_warns_and_injects_no_cacargo nextest run -p boxes steering_off_with_proxy_env_is_refusedcargo nextest run -p boxes steering_defaults_to_dnscargo nextest run -p boxes prop_dns_steering_without_resolver_is_exit_3cargo nextest run -p boxes prop_installation_mode_unenrolled_is_exit_3cargo nextest run -p minimal activation_refuses_grant_before_create_with_exit_3Verify
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.Out of scope
These files are outside the task's
layers:, so this PR is a draft:crates/minimal/Cargo.tomlcrates/minimald/src/net/switch.rscrates/minvmd/src/box_registry.rsCloses #2264
Layer exception
Accepted by the epic owner on 2026-10-19 under the same rule as #2361 and #2363 (required one-line changes outside the planned layers):
bep = { path = "../bep" }incrates/minimal/Cargo.toml, and theCredentialedUpstream::default()test-code updates incrates/minimald/src/net/switch.rsandcrates/minvmd/src/box_registry.rsthat the new struct fields require.Generated by Claude Code