Repository navigation
feat(bep): give bep serve the client's store rules so a store member is admitted (T83) - #2614
Conversation
bep serve reads [secret-store-rules] from the user-level config.toml, applies the BEP-034 refusals with the same check min uses, and gives the rules to BoxRedeemer::with_rules. The rules' authorities join the root's names and the admitted set, less the deny set, so a store member under a rule is admitted in the rule's injection form (BEP-032). bep now depends on boxes and paths for the config reader.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
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 · 3 conditions · the full ruling
The design allows bep serve to read the client configuration, and the new boxes dependency keeps one BEP-034 check. But bep holds a different rule set from the one min resolves. bep reads only the user file, so org rules do not apply and a user rule overrides an org deny. Gatehouse §6.10 line 849 and architecture line 1210 do not allow this. C1 fixes it. bep also reads the rules once per boot, so a narrowed rule continues to admit. Line 842 says the current rule decides each request. C2 fixes it. C3 files the boot-time refresh of the root and the admitted set as a named residual.
Ruled at 848b389 against gominimal/arch@109541a48c31 by claude-opus-5-5.
bep serve reads config.toml from the default config dir once at start. minvmd starts it and does not know a min --config-dir override. The start log line now names the file, and the doc states both limits.
|
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 box's request to a Notes from the agent: Revise round 1. The review finding holds in part. minvmd starts |
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 4 unresolved threads — minimal-foundry[bot] crates/bep/src/main.rs, minimal-arch[bot] crates/bep/src/main.rs:412, minimal-arch[bot] crates/bep/src/main.rs:765, minimal-arch[bot] crates/bep/src/main.rs:391. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
arch-review: CONDITIONS · 3 conditions · the full ruling
The admit path is correct for a rule that does not change. Gatehouse §6.10 line 842 says a rule that narrows or changes its injection form refuses the handle. wire() gives the redeemer a fixed list from start, so a narrowed rule keeps admitting until restart. C1 makes the redeemer read the current file. bep also reads only the user file, and min mints under the user rules merged over the organization rules. C2 moves that merge into boxes so both binaries use it. C3 files the restart limit for the root and the CONNECT set as a residual with an issue.
Ruled at 1788b7a against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
/revise Round 2 of #2614, on arch conditions C1, C2 and C3 at 1788b7a (CONDITIONS). No round is in flight. The arch review accepted the delivery shape (
Generated by Claude Code |
Move the store-rule merge of the user and the organization client configuration from min into boxes::client::config, so min and bep serve read the same merged rules.
bep serve now owns a rules source that stats the user and organization config.toml and reads them again when the mtime or the length changes, so a narrowed rule refuses the next request. It reads the rules through the boxes merge that min mints under, so an organization deny holds. The root's names and the admitted set still take the rules at start only, tracked in #2618.
|
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 box's request to the registered upstream of a [secret-store-rules] rule, sent through a running bep serve, is admitted and gets the stored key in the rule's injection form. The rules are the same merged user and organization rules that min mints under. The rule in force at that moment decides each request, so a narrowed rule refuses the same handle on its next request with no proxy restart. Notes from the agent: Round 2 addresses C1, C2 and C3. C2 touches crates/boxes/src/client/config.rs and crates/minimal/src/config.rs, which are outside the task's layers. The /revise note asked for that move. New in boxes::client::config: ORGANIZATION_CONFIG_PATH, read_config_parts, read_client_store_rules (returns ClientStoreRules: the config, the merged rules, the refusals and organization_error), merge_store_rules, keep_read_actions, deny_refused_items. min re-exports ORGANIZATION_CONFIG_PATH and wraps read_config_parts, so cmd/secret.rs does not change. The three merge tests moved from minimal to boxes. They now use a test deny set in place of min's refuse_store_rules. min's printed text does not change: the refusal lines and the organization-file error print as before, once per file in a process. bep: ClientRules implements bep::handle::Rules. It stats both files and reads them again on a change of mtime or length. A user or organization file that exists and does not read or parse gives no rules. The root's names and the admitted CONNECT set still take the rules at start only. The client_store_rules doc and a comment in serve name #2618 for this. The start info log line no longer says 'read once at start'. A re-read logs an info line with only the rule count. just clippy-strict was not run. |
|
The runtime will not revise this pull request again on its own: the diff is +839/-234 (1073 lines), past 3x the plan's estimate of 280 lines (est: 140 code, 140 tests, as #2599 reads now), at round 2 (the last trigger was a /revise note). A person decides now. Remove |
|
/revise Round 3 of #2614, on a CI failure at 360cc59. No round is in flight. The size cap is lifted for this epic, so the diff size is not a reason to stop.
Generated by Claude Code |
The pinned clippy refuses assert!(x.is_empty()) as assert_is_empty. The tests this pull request adds now compare against an empty value.
|
Round 1 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. A box's request to a [secret-store-rules] rule's registered upstream, sent through a running bep serve, reaches that upstream with the stored key in the rule's injection form. serve_admits_a_store_member_under_the_clients_rule still passes, and the tests this pull request adds no longer use assert!(x.is_empty()). Notes from the agent: Directive item 1: I changed all six assert!(... .is_empty()) lines that this pull request adds to assert_eq! against an empty value. Three are in crates/boxes/src/client/config.rs (lines 784, 818, 855) and three are in crates/bep/src/main.rs (store_rules_in tests). I did not change the assert!(upstream.is_empty()) lines in redeemer.rs or minimal/src/config.rs, because they were on main before this pull request and CI did not flag them. Directive item 2: I was not able to verify the workspace clippy on the pinned toolchain. This box has rustc and clippy 1.98.1 and no rustup, and rust-toolchain.toml pins 1.99.0. Clippy 1.98.1 does not have assert_is_empty. It fails first on six must_use lints in crates/boxes/src/core/compose.rs and primitives.rs, files that this pull request does not touch, so it stops before it lints bep. CI on 1.99.0 is the record for item 2. Item 3: I changed nothing else. |
|
The runtime will not revise this pull request again on its own: the diff is +848/-234 (1082 lines), past 3x the plan's estimate of 280 lines (est: 140 code, 140 tests, as #2599 reads now), at round 1 (the last trigger was a /revise note). A person decides now. Remove |
…erve-the-client-s-store-rules-so-a-sto
An edit that keeps the length and the modification time (cp -p, touch -r, rsync -t) now changes the stamp, and settled() decides on the later of the modification time and the change time. The test now asserts that such a rewrite is read. The merge of main also needed box_id on one RegisterClientKey in a test.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 2 conditions · the full ruling
The new commit closes the open point of the last ruling. The store-rule stamp now holds the change time and the inode. Thus an edit that keeps the length and the modification time is read again, and a narrowed rule refuses (Gatehouse §6.10 line 842). C1 (#2618) and C2 (#2620) stay open as filed residuals, as the ruling on 1c0609b accepted. You can merge the change with them open.
Ruled at 172c938 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 1 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
bep serve admits a CONNECT to a store member under the client's [secret-store-rules] rule. A rule file edited in a way that keeps its length and modification time (cp -p, touch -r, rsync -t) is now read again on the next redemption, so a narrowed rule refuses. Notes from the agent: Item 1: the merge of origin/main (d97ad33) was already on the branch when the session started. Cargo.lock has zstd 0.14.3, zstd-safe 8.2.0 and zstd-sys 2.1.2 from #2627. That merge broke the bep test build: T84 (#2615) added box_id to Request::RegisterClientKey, and one test in main.rs from this branch did not set it. I added |
|
The runtime will not revise this pull request again on its own: the diff is +979/-232 (1211 lines), past 3x the plan's estimate of 280 lines (est: 140 code, 140 tests, as #2599 reads now), at round 1 (the last trigger was a /revise note). A person decides now. Remove |
|
/revise Round 8 of #2614, on the security thread at
Generated by Claude Code |
…erve-the-client-s-store-rules-so-a-sto
bep holds the sealing and CA keys. It linked boxes for one reader and one check, and boxes pulls in common, nickel and google-cloud-storage. The client config, the session keys and the secret-store rules now live in a new client-config crate. boxes re-exports them under the same paths, so min keeps calling the same merge. bep depends on client-config in place of boxes.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 2 conditions · the full ruling
The new commit moves the client config and the store-rule reader from boxes into a new client-config crate. The move keeps the code and its behaviour. The design of record does not own crate dependencies, so the move is not a deviation. The merge of main is clean. The ruling on 172c9382 stands. C1 (#2618) and C2 (#2620) stay open as filed residuals.
Ruled at f8d2558 against gominimal/arch@109541a48c31 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 3 unresolved threads — minimal-foundry[bot] crates/bep/Cargo.toml, minimal-arch[bot] crates/bep/src/main.rs:427, minimal-arch[bot] crates/bep/src/main.rs:424. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
Round 1 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 box's request to a [secret-store-rules] rule's registered upstream, sent through a running bep serve, reaches that upstream with the stored key in the rule's injection form. This round, bep reads those rules through the new client-config crate, and Notes from the agent: Layer exception, as the epic owner ruled on 2026-10-27. The PR body must name these files, which are outside the T83 layers: crates/client-config/Cargo.toml (new), crates/client-config/src/lib.rs (new), crates/client-config/src/config.rs (moved from crates/boxes/src/client/config.rs), crates/client-config/src/keys.rs (moved from crates/boxes/src/keys.rs), crates/client-config/src/store_rules.rs (new, the store-rule items cut from crates/boxes/src/core/bep.rs), Cargo.toml (workspace member and [workspace.dependencies] entry), crates/boxes/Cargo.toml, crates/boxes/src/lib.rs, crates/boxes/src/core/bep.rs. What moved: the whole client config module (the reader returns the full Config, and Config denies unknown fields, so it needs the session-keys types), the keys module, and DENIED_HEADERS, StoreAction, StoreRuleConfig, StoreRules, StoreDenySet, StoreRuleRefusal, StoreRuleRefused, check_store_rules, StoreConsent and store_consent with their private helpers. boxes re-exports all of them under the old paths (boxes::keys, boxes::client::config::, boxes::core::bep::Store), so min and every other caller compile unchanged. Two methods became pub because boxes calls them across the crate line: StoreRuleConfig::upstream_hosts and StoreDenySet::is_sign_in_item. normalize_host is pub in client-config so boxes and the deny check compare hosts the same way. No cargo feature was used. The tests are unchanged: the config.rs tests moved with the file, and one test now names crate::store_rules::store_consent in place of crate::core::bep::store_consent. The store-rule tests …[truncated] |
|
The runtime will not revise this pull request again on its own: the diff is +2044/-1204 (3248 lines), past 3x the plan's estimate of 280 lines (est: 140 code, 140 tests, as #2599 reads now), at round 1 (the last trigger was a /revise note). A person decides now. Remove |
There was a problem hiding this comment.
Own review at f8d2558. bep serve reads the client's [secret-store-rules] through the new client-config crate, the same user-and-organization merge min calls, and admits a store member under the current rule. The rules are re-read when the file's stamp changes, and the stamp includes ctime and inode. bep no longer links boxes or common (cargo tree -p bep checked). The restart and --config-dir limits are plan tasks T91 (#2618) and T92 (#2620).
Generated by Claude Code
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 f8d2558e9fa5 withheld this approval on the threads open then. Posted as a comment: the App cannot approve its own pull request; the check and the label carry it.
There was a problem hiding this comment.
Babysitter approval at f8d2558: every merge gate passes at this head (arch PASS at f8d2558, zero open threads, every lane aggregator green). Squash title feat(bep): give bep serve the client's store rules so a store member is admitted (T83) and the squashed message pass commitlint. Queued for squash merge.
Generated by Claude Code
T83 — Give bep serve the client's store rules so a store member is admitted
slice: S10
kind: task
covers: BEP-032
size: M
est: 140 code, 140 tests
executor: agent
difficulty: unknown ground
model: strongest
diagnostics: a CONNECT to an authority that no module and no store rule admits stays refused
off_moduleinmin box audit, and a store handle with no matching rule stays refusedstore_handle_invalidobservability: one info log line at start naming the count of store rules
bep serveholds, never a valuelayers:
- rules:
- crates/bep/src/main.rs:
bep servegets the client's[secret-store-rules]with the BEP-034 refusals applied and gives them toBoxRedeemer::with_rules; the rules' authorities joinregistered_unionand the admitted set, less the deny set; the three interim notes that still name T25 (#2280) are removed (#2599); how the rules reachbep serve(a read of the user-levelconfig.toml, or a control-socket op thatminsends) is this task's choice, left to the implementer and the arch review by the epic owner on 2026-10-26- crates/bep/src/redeemer.rs: the
factsdoc names the rulesbep servegives in place of the interim notetest: cargo nextest run -p bep serve_admits_a_store_member_under_the_clients_rule
produces: none
consumes: none
proof: A box's request to a
[secret-store-rules]rule's registered upstream, sent through a runningbep serve, reaches that upstream with the stored key in the rule's injection form.depends on:
- blocked by #2257
- blocked by #2586
- blocked by #2280
Proof
bep servenow reads the user-levelconfig.toml[secret-store-rules], whichmin secret setwrites. It drops each rule that the BEP-034 check refuses and gives the rest to the redeemer. Each rule's authority joins the root CA's names and the admitted CONNECT set, less the deny set. A box's request to that upstream, with a store handle under the rule, is admitted as a store member in the rule's injection form. The forwarder then puts the stored key in that form, as the existingserve_injects_a_stored_key_for_an_admitted_store_membertest shows for a redeemer with these rules.Notes from the agent: Choice: bep serve reads the user-level config.toml at start. It does not use a control-socket op. To parse and check the rules with the same code that min uses (boxes::client::config::read_config_or_default and boxes::core::bep::check_store_rules), bep now depends on the boxes and paths crates. That adds lines to crates/bep/Cargo.toml and Cargo.lock, which are outside the layers. boxes pulls common (nickel, google-cloud-storage) into the bep binary's dependency graph. The other choices were a copy of the BEP-034 check in bep, or a control-socket op, which needs changes in control.rs and the minimal crate. Gaps: (1) bep reads only the user file, not the organization file /etc/minimal/config.toml that min merges. A handle that min mints under an org-only rule is refused store_handle_invalid. (2) The rules are read once at start. A changed rule needs a restart of bep serve, and the CONNECT set and the root's names need a restart in any case. (3) A rule with action = "deny" is not held, so its handle is refused. (4) A config.toml that does not read or parse gives no rules and one error log line. The proxy still starts for GitHub. Refused rules get one warn log line each, with the refusal text. The start info line logs store_rules = . The new test in main.rs checks the redeemer's Redeemed::Store decision and its injection form. It does not forward byt…[truncated]
Tests
cargo nextest run -p bep serve_admits_a_store_member_under_the_clients_ruleVerify
min task run agent-verifyexited 101 under rustc 1.98.1, which the repository does not pin (1.99.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/bep/Cargo.tomlLayer exception
The epic owner accepted these files outside T83's layers on 2026-10-27, for the security finding on
crates/bep/Cargo.toml(round 8):Cargo.toml,crates/client-config/(new crate),crates/boxes/Cargo.toml,crates/boxes/src/lib.rs,crates/boxes/src/client/config.rsandcrates/boxes/src/core/bep.rs: the client'sconfig.tomlreader and the[secret-store-rules]checks move intoclient-config, which does not depend oncommon.boxesre-exports them under their old paths, somincalls the same merge.crates/bep/Cargo.tomlandCargo.lock:bepdepends onclient-configin place ofboxes.cargo tree -p bep -e normallists noboxes,common,nickel-lang-core,google-cloud-storageorreqwest.crates/boxes/src/client/config.rsandcrates/minimal/src/config.rscarry the organization merge from round 2, as arch C2 required. The restart and--config-dirlimits are T91 (#2618) and T92 (#2620) in the plan.Residuals
bep serverestart (arch C3).bep servedoes not read rules undermin --config-dir(arch C1 at b1928ca).Closes #2599
Generated by Claude Code