Repository navigation
feat(bep): deny Minimal's infrastructure domains in the box CA and store rules (T55) - #2582
Conversation
The store-rule refusal now names the deny-set entry a host is under. The tests read bep::ca::DENY_SET from its source, the one definition.
…e-provisional-ca-deny-set-with-the-dec
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
Spec lint
Advisory. None of this blocks the merge; the review decides. Advisory, and never a merge gate. Linter: |
|
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 · 2 conditions · the full ruling
The deny-set mechanism conforms, and bep::ca::DENY_SET is the one definition of the set. The record does not conform. The doc comment and spec line 242 say that the owner's ruling on #2441 decided these three domains. The PR body says the hostnames are an inference, and that nobody read the ruling. Make C1, which states the mapping as this change's reading and keeps the #2441 tracking. Make C2, which limits the claim that the baseline set is addresses to the built-in baseline of an un-enrolled host (§5.1 line 109). The escalation asks whether a configured registry or build cache must be in the set.
Ruled at 821d346 against gominimal/arch@109541a48c31 by claude-opus-5-5.
The node-plane baseline set is addresses, which the root excludes as IP addresses. The DENY_SET entry comment and the spec note said names.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
The root certificate excludes min.internal, minimal.dev and minimal.farm, and every name under them. A [secret-store-rules] rule that names any of them is refused. The two tests show both facts against the one DENY_SET definition. Notes from the agent: Revise round 1. I removed the 'node-plane baseline names' claim from the min.internal entry comment in ca.rs. I made the same change in the spec note, so the two match. The node-plane baseline is an address (100.72.0.1 in the minimald classifier tests), and the root excludes every IP address. I still cannot read the ruling on #2441. The three domains in DENY_SET are my inference from the categories that the task names. A maintainer must confirm them before merge. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The deny set lives in the right place, and the root and the store-rule check read one definition. The spec note and the DENY_SET comment credit the owner's ruling on #2441 with three domains that the PR body calls an inference. C1 makes that record exact. Gatehouse §6.10 line 849 also refuses a configured registry or cache and the baseline members that have DNS names, which a fixed list cannot hold. C2 files that gap as a residual and limits the claim that the baseline set is addresses only.
Ruled at 0b3c082 against gominimal/arch@109541a48c31 by claude-opus-5-5.
The categories come from the task text. The domain mapping and the minimal.farm entry are this change's reading. Only the built-in baseline of an un-enrolled host is addresses.
|
Round 2 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
The root certificate excludes every name of the deny set (min.internal, minimal.dev, minimal.farm), and a store rule naming any of them is refused. The doc comment and the spec now say that the domain mapping and minimal.farm are this change's reading and wait for confirmation on #2441. Notes from the agent: This round changed only comments. The ruling on #2441 was not available to me, so I could not check the three entries against it. A person must confirm them on #2441 before merge. The DENY_SET entries are unchanged. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The deny-set mechanism conforms to Gatehouse §6.10 lines 834 and 849. The domain mapping is now labelled honestly as waiting on #2441. One gap has no owner. Line 849 also refuses a rule that names the registry, the build cache or the baseline set, and a deployment can configure these. The static DENY_SET covers only the default hosts, and the new comment states the gap without an issue. C1 files that issue and links it from ca.rs and the spec note. Do not change the names in the set.
Ruled at b7e53c7 against gominimal/arch@109541a48c31 by claude-opus-5-5.
Name NodePlaneBaseline::built_in in crates/minvmd/src/net/baseline.rs, and record the deployment-configured named baseline as a residual gap that waits on #2441.
|
Round 3 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
The root certificate excludes min.internal, minimal.dev and minimal.farm with every name under them, and every IP address. A [secret-store-rules] rule that names any of them is refused. The doc comment and the spec note say that the domain mapping is this change's reading, which waits on #2441. Notes from the agent: This round changes only comments. The DENY_SET doc comment and the spec note now limit the 'baseline is addresses' claim to NodePlaneBaseline::built_in in crates/minvmd/src/net/baseline.rs. They also record that named endpoints in a deployment-configured baseline are a residual gap. No separate issue for that gap exists that I can name, so the text points to #2441. A person can open a dedicated issue and change the link. The two HIGH/MEDIUM findings ask for the owner's confirmation of the domains on #2441. Code cannot give that confirmation, so a person must get it before merge. just lint-prose did not run: vale is not installed on this box. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The set and its placement conform. The root and the BEP-034 check read one DENY_SET. Two gaps against Gatehouse §6.10 line 849 remain. The design names the build cache cache.gominimal.dev (architecture.md line 844), and no entry covers it. C1 adds it. The minimal.dev comment claims to cover the tenant issuer, but a self-hosted Gatehouse issuer is not under it. C2 scopes the claim and names the gap as residual on #2441.
Ruled at cc83ac0 against gominimal/arch@109541a48c31 by claude-opus-5-5.
Name the configured cache (MINIMAL_REMOTE_CACHE_URL), the registry and the baseline endpoints that the deny set does not cover, and link #2441.
…e-provisional-ca-deny-set-with-the-dec
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 2 conditions · the full ruling
The deny set has one definition in bep::ca::DENY_SET. The root and the BEP-034 check both read it, and the change puts it in the correct owners. Two items stay open on #2441. C1: Gatehouse §6.10 line 849 refuses an upstream that names the registry, build cache or baseline set, and networking §5.1 line 109 lets a deployment configure those. The fixed list does not refuse those configured hosts. C2: the minimal.dev and minimal.farm mapping is an inference. Check it against the owner's ruling on #2441. Make sure that #2441 is open before merge.
Ruled at 8dc2ab9 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 4 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
The root certificate excludes every name of the deny set (min.internal, minimal.dev, minimal.farm and every name under them), and a store rule that names any of them is refused. The doc comment and the spec now name the configured cache, registry and baseline endpoints that the set does not cover. Notes from the agent: This round changed only comment and spec text. C1 asks for a new issue. I have no gh credential, so I did not file it. Both places link the existing tracking issue #2441 instead. A person must file the C1 issue and replace the link if a separate issue is wanted. The domain mapping (min.internal, minimal.dev, minimal.farm) is still this change's reading and waits for confirmation on #2441. I was not able to read the ruling on #2441 from this box. |
…e-provisional-ca-deny-set-with-the-dec
The design names the build cache cache.gominimal.dev, and no entry covered it. The minimal.dev entry covers the hosted tenant issuers only. A self-hosted issuer stays residual on #2441.
|
Round 5 of 10 on a failed check suite, pushed as
Verify Review threads the coder judged:
The root certificate lists min.internal, minimal.dev, cache.gominimal.dev and minimal.farm as excluded subtrees, so no leaf for those names or names under them validates. A [secret-store-rules] rule that names any of them is refused, and the refusal names the entry it hit. Notes from the agent: Round 5. Added cache.gominimal.dev to DENY_SET and to the two pinned lists. Scoped the minimal.dev comment to the hosted tenant issuers, and named the self-hosted issuer gap as residual on #2441 in the doc comment and in the spec note. The failed-check excerpts do not show a failing test or a failing step. The tests log shows only passing tests and a printf line from a script. The ci-linux-native-success job is the aggregator. Locally the full bep and boxes suites pass, and clippy on both crates is clean. I did not run the workspace suite. The other DENY_SET caller (crates/minimal/src/config.rs) pins no names. just lint-prose did not run, because vale is not installed on this box. I cannot read the owner's ruling on #2441, so the domain mapping still waits for that confirmation. |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
This head adds cache.gominimal.dev to DENY_SET. That is the build cache by the name in architecture.md line 844. The root and the store-rule check still read one definition, as Gatehouse §6.10 line 834 requires. A self-hosted issuer and a configured cache, registry or baseline endpoint outside the listed domains are under no entry. Line 849 requires the deny set to cover them. C1 carries that gap, and it can stay open on #2441.
Ruled at fae0425 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 6 of 10 on the runtime's review findings: the coder changed nothing. Revise round 6. This round changes no files. Both open threads ask for a change only after the owner's ruling on #2441 is confirmed, and this session cannot read #2441. The doc comment on DENY_SET already says that the category-to-domain mapping and the minimal.farm entry wait for that confirmation. It also names the self-hosted issuer and the configured cache, registry and baseline gaps as residual on #2441. A person must confirm the entries on #2441 before merge. If the ruling differs, change only DENY_SET and the two pinned lists (crates/bep/src/ca.rs root_excludes_every_deny_set_name, crates/boxes/src/core/bep.rs every_deny_set_name_is_refused_as_a_store_rule). Review threads the coder judged:
|
|
Revise round 5 ran against |
|
/revise Round 7 of #2582, on the red
The test still pins the old sentence.
One commit, Conventional Commits subject, every body line at most 72 characters. Generated by Claude Code |
…e-provisional-ca-deny-set-with-the-dec
The store rule refusal now names the deny-set entry when the host is under it. Update the config test to the sentence Display writes.
…e-provisional-ca-deny-set-with-the-dec
|
Round 7 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. The root certificate excludes every name of the decided deny set, and a store rule naming any of them is refused. The minimal config test now expects the refusal that names min.internal as the entry that host.min.internal is under, so the tests job passes again. Notes from the agent: Round 7: changed only the expected line for the rule internal in store_rules_read_through_client_config (crates/minimal/src/config.rs, accepted outside the layers by the directive). A search of crates/, scripts/ and docs/ found no other pin of the old host.min.internal sentence. The remaining 'which a secret can never be sent to' pin in crates/boxes/src/core/bep.rs is for the gh rule, whose host equals its entry, so it stays correct. |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The change still conforms to Gatehouse §6.10 lines 834 and 849. The only new PR commit updates a test to the new refusal text. The root and the store-rule check read one deny set. The hostnames for each category are the change's reading. C1 keeps them open until the owner rules on #2441.
Ruled at e0e93c8 against gominimal/arch@109541a48c31 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 20 unresolved threads — minimal-arch[bot] crates/bep/src/ca.rs, minimal-arch[bot] crates/bep/src/ca.rs, minimal-arch[bot] crates/bep/src/ca.rs:92, minimal-arch[bot] docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md, minimal-arch[bot] crates/bep/src/ca.rs, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
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 e0e93c83b96a 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 e0e93c8: every merge gate passes at this head (arch PASS at e0e93c8, zero open threads, every lane aggregator green). Squash title feat(bep): deny Minimal's infrastructure domains in the box CA and store rules (T55) and the squashed message pass commitlint. Queued for squash merge.
Generated by Claude Code
T55 — Replace the provisional CA deny set with the decided infrastructure deny set
slice: S9
kind: task
covers: BEP-013
size: S
est: 60 code, 120 tests
executor: agent
difficulty: routine
model: standard
diagnostics:
min box specshows the root's excluded subtrees, and a refused rule names the deny-set entry it hitobservability: none beyond the refusal text
layers:
- ca:
- crates/bep/src/ca.rs:
DENY_SETbecomes the decided set from one shared definition:min.internal,minimal.dev, the tenant issuer, the BEP, EGW and relay endpoints, the registry and build cache, and the node-plane baseline set;localhost,.local,home.arpaand.internalstay allowed (owner's ruling of 2026-10-24 on #2441)- crates/boxes/src/core/bep.rs: the BEP-034 check reads the same definition
- docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md: names the set
test: cargo nextest run -p bep root_excludes_every_deny_set_name
test: cargo nextest run -p boxes every_deny_set_name_is_refused_as_a_store_rule
produces: none
consumes: none
proof: The root certificate excludes every name of the decided deny set, and a store rule naming any of them is refused.
depends on:
- blocked by T3
- blocked by T50
Generated by Claude Code
Proof
The root certificate that bep issues now carries min.internal, minimal.dev and minimal.farm as excluded subtrees, and it issues no leaf for any of them or a name under them. A [secret-store-rules] rule that names one of those names, or a name under one, is refused, and the refusal names the entry it hit. printer.local, nas.home.arpa, svc.corp.internal and localhost stay allowed.
Notes from the agent: The hostnames are my inference. The task text and the spec excerpt name the decided set by category (tenant issuer, BEP/EGW/relay endpoints, registry, build cache, node-plane baseline set), not by hostname. The ruling on #2441 was not available to me. I mapped the categories to three DNS entries from what the tree uses: min.internal (box-internal names, the proxy's in-box address, the node-plane names), minimal.dev (cache.minimal.dev, go.minimal.dev, and the issuer and registry by inference) and minimal.farm (agents.minimal.farm, cache.minimal.farm, and EGW/relay by inference). The node-plane baseline set is addresses (crates/minvmd/src/net/baseline.rs), and the root already excludes every IP address. A reviewer must check this mapping against the ruling. If the ruling names other hosts, change only the DENY_SET list in ca.rs and the two pinned asserts in the tests. One shared definition: boxes does not depend on bep, and bep does not depend on boxes. Adding a dependency would change a Cargo.toml outside the layers. At runtime the BEP-034 check already reads bep::ca::DENY_SET through minimal::config::store_deny_set. The boxes tests now read the list from crates/bep/src/ca.rs with include_str!, in place of a copied literal, so a change to the set reaches them. Printed text: the refusal for a host under an entry (not the entry itself) now reads 'The secret-store rule in names , which is under . A secret can never be sent to or a name under it. Remove or change the rule.' This serves the diagnostics line. The refusal for the …[truncated]
Tests
cargo nextest run -p bep root_excludes_every_deny_set_namecargo nextest run -p boxes every_deny_set_name_is_refused_as_a_store_ruleVerify
min task run agent-verifypassed ([tasks.verify]).Base
The base branch moved while the coder worked; it was merged in before the tests ran.
Closes #2536