Repository navigation
T97 — Accept source = "store" with store and id in a box spec's grant - #2774
norrietaylor merged 9 commits into
Conversation
A store reference is now a grant written source = "store" with store and id. store_references() reads only that form, not broker scopes. A store grant missing store or id is refused with text that names the missing field. The e2e store-reference seed writes the new form.
min box spec lists each store row from store_references(), so a row names the item by its id, or <store>:<id> outside the keychain. Box creation names items the same way, and a store grant declares a credentialed upstream as a broker grant does. The tests build store references as source = "store" with store and id.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Reviewed: 2 findings, none blocking at Address the comments on the diff. Clobber: none seen. |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The new grant form matches the design. Gatehouse §6.10 line 842 writes a store reference as { source = "store", store, id }, and the parse refuses every malformed form. The task's proof also says the reference mints and redeems. The e2e leg did not run, and the store member mint waits on #2525. C1 tells you to state that leg as open in the PR body and to link #2525 as the residual.
Ruled at e427134 against gominimal/arch@8640f85e193d by claude-opus-5-5.
The old store-reference spelling, source = "broker" with a scope such as "claude", parsed and counted as credentialed but skipped the store rules. The parser now refuses it and points to source = "store".
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The store-reference grammar conforms to Gatehouse §6.10 line 842. The new commit refuses the old broker-scope spelling, so no reference skips the store rules (architecture line 129, AT25). The mint and redeem half of BEP-032 is not in this head. mint_store_handle has no caller, and create.rs line 1182 says that work waits on #2525. Edit the PR body as C1 says. It must name #2525 and record steps 3 to 5 of proof_bep_store_reference_end_to_end as open.
Ruled at ef2936b against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
A box spec can declare a store reference as source = "store" with store and id. Box creation checks it against the store rules and mints it, and Notes from the agent: This round adds one commit. The grant parser now refuses a source = "broker" grant that has a scope without the github: prefix. The refusal names the scope and tells the user to write source = "store", store = "keychain" and id = "". A broker grant with no scopes still parses, because the e2e GitHub seeds use that form. The test a_store_grant_parses_store_and_id now asserts the refusal, in place of the old check that store_references skipped such a grant. The proptest generator still builds Broker grants with "keychain:npm" directly, not through the parser, so those properties are unchanged. All 26 core::bep tests in boxes pass, and clippy -p boxes --all-targets is clean. I did not run just clippy-strict. |
|
/revise Findings from review at ef2936b:
The broker-scope refusal at |
A store grant with store = "" or a blank id now gets the same missing-field refusal. Move the store_grant doc to its function.
There was a prob 10000 lem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The grant form conforms to Gatehouse §6.10 line 842 and architecture.md line 759. The parse refuses every other shape, and the compare makes a blank store or id a refusal too. One half of the task proof is still open. No box creation mints a store handle until #2525, so steps 3 to 5 of proof_bep_store_reference_end_to_end cannot pass on this head. Declare that half as a residual on #2525 in the PR body, as C1 says. Keep the e2e steps hard.
Ruled at 6df2ad3 against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
Round 2 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
A box spec can declare a store reference as source = "store" with store and id, and Notes from the agent: Round 2. Item 1: an empty or whitespace-only store or id now reads as absent and gets the same 'has no store field' / 'has no id field' refusal, with two new cases in a_store_grant_parses_store_and_id. Item 2: removed the stale broker-grant doc line above store_reference and put a corrected one ('One store grant OPENAI_KEY ...') on store_grant. Item 3 (PR body): please record in the body that proof_bep_store_reference_end_to_end was not run at this head. Steps 3 to 5 of that leg stay open because mint_store_handle has no caller, so no store member is sealed into the box. #2525 is the residual. The coder cannot edit the PR body, so this is stated here and in proof. Change nothing else, per the directive. |
|
/revise Two items remain at 6df2ad3:
I re-checked and resolved the empty-field and broker-scope threads. Change nothing else. |
A padded id such as " claude " parsed but never matched its rule. The grant now keeps the trimmed values.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 4 unresolved threads — minimal-arch[bot] scripts/session-e2e.sh:12535, minimal-arch[bot] scripts/session-e2e.sh:12535, minimal-foundry[bot] crates/boxes/src/core/bep.rs, minimal-arch[bot] scripts/session-e2e.sh:12535. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The source = "store" grant now parses with store and id, as Gatehouse §6.10 line 842 says. min box spec and box creation read references only from that form. One item is open. The PR body says proof_bep_store_reference_end_to_end did not run, so the mint and redeem half of the proof is not shown. Run it at this head and record the result, as C1 says.
Ruled at 0d5b06f against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
Round 3 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
A box spec can declare a store reference as source = "store" with store and id. The grant parses, and padding around store or id is trimmed, so id = " claude " reads as claude. Notes from the agent: For the PR body Tests section: proof_bep_store_reference_end_to_end (scripts/session-e2e.sh) was not run at this head. This box has no KVM or microVM lane. Steps 3 to 5 of that leg (sealed value, injection, redemption) stay hard in the script, and the script does not soften them. They cannot pass until #2525 mints a store handle at box creation, because mint_store_handle has no caller at this head. Residual: #2525. This round: SecretGrant::try_from now keeps the trimmed store and id. The new test a_store_grant_parses_store_and_id_trimmed checks that id = " claude " parses as claude. It is a separate test so the existing test stays under clippy's too_many_lines limit, and the task's test: line still selects it by substring. No other change. |
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 0d5b06f909c2 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.
Reviewed at 0d5b06f against T97's plan block. The grant parse, store_references(), min box spec and the creation fixtures match the plan. The refusal text gives the field, the reason and the edit to make. No assertion was loosened.
Two defects:
-
crates/boxes/src/core/bep.rs:273-283: box records saved before this PR stop loading. The new refusal ("source = broker takes only github: scopes") sits inSecretGrant'sDeserialize(try_from = "GrantFields", line 184). That is also the deserializer for stored box records:Record→BoxPolicy.credentialed_upstream→CredentialedUpstream.secrets(crates/boxes/src/lib.rs:350).- A box created with the interim spelling (a broker grant with a
keychain:scope, shipped since #2514) has that grant in itsrecord.json. - After an upgrade, that record fails to parse.
classify_record_for_reconcile(crates/boxes/src/store.rs:590-593) treats any parse error as corruption and returnsDropMissing. - The box disappears from
min ls, cannot be destroyed, and its directory stays on disk. - Fix: refuse the old spelling where
minimal.tomlis read, not in the shared deserializer, or read stored records leniently. Add a test that loads a record containing the interim spelling.
- A box created with the interim spelling (a broker grant with a
-
crates/boxes/src/core/bep.rs:1213-1246and:1576: the property tests never generate a store grant.any_source()yields onlyBrokerandClientEnv.any_grant()still buildsBrokergrants with"keychain:npm", a grant the parser now refuses. Sostore_references(any_grants())is always empty. The quic443 oracle at:1576counts onlyBrokergrants as credentialed, whileis_credentialednow countsStoreas well.- Fix: add
GrantSource::Storetoany_source(), drop"keychain:npm"from the broker scope pool, and make the oracle useis_credentialed().
- Fix: add
|
/revise Two defects from review at 0d5b06f. Leave the PR body alone. Leave the e2e steps 3-5 as they are; they stay open on #2525.
Run |
|
That Nothing to type again: the command gets a rocket when its round starts, or a refusal that names the reason. No round was counted for it yet. |
A box record saved before BEP-032 can carry the old store spelling, a broker grant with a keychain: scope. The shared grant deserializer refused it, so such a record stopped loading and reconcile dropped the box. The parse accepts it again, and refusals() refuses it when min creates a box. The property tests now generate store grants, and their oracles count credentialed grants through is_credentialed().
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The change conforms to Gatehouse §6.10 line 842. A store reference is source = "store" with store and id, and no broker scope is a store reference. The new compare moves the refusal of the old spelling to box creation. Creation still refuses it before anything is created, so the change fails closed. One item stays open. The mint and redeem steps of bep_store_reference_end_to_end did not run, and they wait on #2525. C1 keeps that leg hard until it runs green.
Ruled at fe0de0e against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
Round 4 of 10 on a review requesting changes, pushed as
Verify A box spec that declares a store reference as source = "store" with store and id mints and redeems it, and Notes from the agent: Revise round 4. Defect 1: the refusal of a broker grant with a non-github: scope moved out of SecretGrant's Deserialize, which stored records share, and into refusals() as BepRefusal::BrokerScopeNotGithub (BEP-032). Only min box creation calls refusals() through check_activation. New test a_store_grant_parses_store_and_id_in_a_saved_record loads a Record JSON with {source = broker, scopes = [keychain:npm]}. The old-spelling check moved to a_store_grant_parses_store_and_id_not_a_broker_scope, because the original test went over clippy's too_many_lines limit. Defect 2: any_source() now yields a Store grant, keychain:npm left the broker scope pool, and a generated store grant has no scopes and no mode. The quic443 oracle uses is_credentialed(). The BEP-009 and BEP-017 property oracles also counted only Broker grants, so they now use is_credentialed() too, and their doc comments say broker or store. All 28 core::bep tests pass, and clippy -p boxes --all-targets is clean. The new refusal text is new printed text that no plan fixed: "The secret sets source = "broker" with the scope "". A grant with source = "broker" takes only github: scopes. For a secret you stored with |
…rce-store-with-store-and-id-in-a-box-s
The preview kept only the narrow-scope refusal, so a broker grant with the old store spelling previewed clean and min box create refused it. render now keeps BrokerScopeNotGithub too.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The change conforms to Gatehouse §6.10 line 842 and to the Store Reference glossary at architecture.md line 129. The new commit makes min box spec refuse the old broker-scope spelling, as box creation does. That is the correct fail-closed behaviour. The e2e leg bep_store_reference_end_to_end did not run, so the mint and redeem half of the proof is open. C1 keeps that leg hard until #2525 mints the store member. The change can merge with C1 open on #2525.
Ruled at d9ec77d against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
Round 5 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
A box spec that declares a store reference as source = "store" with store and id parses and is credentialed. Notes from the agent: Both items of the round-5 note were already on the branch in fe0de0e. The refusal moved from the grant deserializer into refusals(), which box creation runs on the minimal.toml [secrets]. a_store_grant_parses_store_and_id_in_a_saved_record deserializes a Record with a broker grant scoped keychain:npm. The proptests generate GrantSource::Store, the broker scope pool no longer has keychain:npm, and prop_quic443_auto_resolves_on_grants_and_steering uses is_credentialed(). This round changed only box_spec.rs, for thread PRRT_kwDOPdfcxM6yOp72. The e2e steps 3-5 are unchanged, as the note asked. clippy -p minimal --all-targets reported no warnings. just clippy-strict was not run. |
There was a problem hiding this comment.
Re-reviewed at d9ec77d. Both defects are fixed.
- The grant parse accepts the old spelling again, so a saved box record keeps loading.
a_store_grant_parses_store_and_id_in_a_saved_recorddeserializes aRecordthat holds{"source": "broker", "scopes": ["keychain:npm"]}. Box creation refuses that spelling throughBepRefusal::BrokerScopeNotGithub(exit 3), and so doesmin box spec(d9ec77d).a_store_grant_parses_store_and_id_not_a_broker_scopepins both, forclaudeandkeychain:claude. any_source()now yieldsStoregrants, with no scopes and no mode.keychain:npmis gone from the broker pool, and both quic443 oracles useis_credentialed().
The other changed files come from the merge of main.
T97 — Accept source = "store" with store and id in a box spec's grant
slice: S13
kind: task
covers: BEP-032
size: M
est: 160 code, 240 tests
executor: agent
difficulty: needs care
model: standard
diagnostics: a store reference spelled with
source = "store"and nostoreoridis refused with text that names the missing fieldobservability: none: the parse runs inside
minbox creation, which logs no line for a grantlayers:
- grant:
- crates/boxes/src/core/bep.rs:
GrantSourceacceptssource = "store"withstoreandid, andstore_references()reads store references from that form, not from brokerscopes(#2656)- scripts/session-e2e.sh: the seed of
proof_bep_store_reference_end_to_endwritessource = "store",store = "keychain"andid, and the# Interim spelling:comment goestest: cargo nextest run -p boxes a_store_grant_parses_store_and_id
- review:
- crates/minimal/src/cmd/box_spec.rs:
renderreads each store row fromboxes::core::bep::store_references()of thesource = "store"form, not fromgrant.scopes; its test builds the reference in that form (widened 2026-10-31, owner, #2656 noop)test: cargo nextest run -p minimal box_spec_lists_store_references_with_their_authorities
- create:
- crates/minimal/src/cmd/boxes/create.rs: the tests of
accept_store_referencesbuild store references assource = "store"withstoreandid(widened 2026-10-31, owner, #2656 noop)test: cargo nextest run -p minimal store_reference_without_rule_is_refused_at_creation
test: cargo nextest run -p minimal store_rule_outside_egress_exits_3
produces: none
consumes: none
proof: A box spec that declares a store reference as source = "store" with store and id mints and redeems it, and
min box speclists it with its authorities.depends on:
- blocked by #2294
- blocked by #2257
Generated by Claude Code
Proof
A box spec can declare a store reference as [session.secrets.] source = "store", store = "keychain", id = "". The grant parses, box creation checks it against its [secret-store-rules] rule, and
min box specprintsstore <id> <VAR> upstream <hosts>for it. A grant with source = "store" and no store or no id is refused with text that names the missing field. The e2e proof_bep_store_reference_end_to_end now seeds this form. I did not run that e2e here, so the mint and redeem half of the proof is not shown on this box.Notes from the agent: Design: GrantSource gets a Store { store, id } variant, and SecretGrant reads and writes through a private GrantFields struct (serde try_from/into). The struct literals in crates/minimald/src/net/switch.rs and crates/minimal/src/tests.rs compile unchanged, so no file outside the layers changed. GrantSource is no longer Copy or Serialize on its own. Behaviour changes: a broker scope that is not github: is no longer a store reference. is_credentialed() is true for a store grant, so a store grant declares a credentialed upstream, and credentialed_upstream_for in create.rs now uses is_credentialed() (a non-test change in create.rs, beyond the layer note, which names only its tests). store and id on a broker or client_env grant, and scopes or mode on a store grant, are refused. Rule matching: a [secret-store-rules] rule with no store matches a reference with store = "keychain". boxes now has NATIVE_STORE = "keychain", and a test reads crates/bep/src/store.rs to keep it equal to bep::store::NATIVE_STORE. Printed text:
min box specand the creation refusals name a keychain item by its id alone, and an item in any other store as : (before: keychain:). The StoreReferenceToSignIn refusal now says to set source = "broker" and scopes = ["github:user-token"] and remove store and id. New parse refusals: 'A grant with source = "store" has no store field. Add store = "keychain".' and the id or both-field…[truncated]Tests
cargo nextest run -p boxes a_store_grant_parses_store_and_idcargo nextest run -p minimal box_spec_lists_store_references_with_their_authoritiescargo nextest run -p minimal store_reference_without_rule_is_refused_at_creationcargo nextest run -p minimal store_rule_outside_egress_exits_3./scripts/session-e2e.sh bep_store_reference_end_to_endsource = "store"form andmin box specshows it, but steps 3 to 5 need a sealedstoremember in the box, and no box creation mints a store handle until #2525 (T98). Those steps stay hard and are open on #2525.Verify
min task run agent-verifyexited 100 under rustc 1.99.0, which the repository does not pin (1.99.1 in rust-toolchain.toml); the PR is not a draft on that alone — CI, which honours the pin, is the gate.Closes #2656