Repository navigation
T109 — Name the cause and the next step in the grant and proxy refusals - #2794
norrietaylor merged 11 commits into
Conversation
The exit-5 refusal now tells a missing bep program from an installed proxy that does not answer, and names the proxy's log. A refused activation reads under -o json as its defined error, with the next step as the hint.
Each refusal body now says why the proxy refused the request, and the next step where one exists, in place of one general text.
|
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. |
…ause-and-the-next-step-in-the-grant-an
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The create-time and -o json refusals conform to Gatehouse §6.2, and both defined errors keep their codes. Two proxy refusal texts give a wrong cause. no_live_box means the source is not a live box, but the text says the credential belongs to another box (§6.10 line 823). Make the change in C1. store_handle_invalid also covers bad signatures, expiry and the GitHub sign-in registration, but the text names only [secret-store-rules] (§6.10 line 842). Make the change in C2.
Ruled at 2b2345b against gominimal/arch@8640f85e193d by claude-opus-5-5.
A request from no live box does not carry another box's credential, so the refusal now says it did not come from a running box.
A relative path in MINVMD_BEP_BIN read as a bep that is not installed. The refusal now names the variable and asks for an absolute path.
The unenrolled-node refusal's fmt passed clippy's line limit once the relative-path cause was added.
…ause-and-the-next-step-in-the-grant-an
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The compare fixes the no_live_box text and the relative MINVMD_BEP_BIN cause, and BEP-003 conforms. One item from the last ruling is still open. The store_handle_invalid refusal says only that no [secret-store-rules] rule admits the secret. Gatehouse §6.10 lines 839 and 842 give that marker several causes, and for a sign-in reference the registration is the GitHub module. Make the change in C1.
Ruled at 385bae2 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 grant box refused because no proxy serves it now says why: bep is not installed at a path, bep is installed and does not answer (with its log path), or MINVMD_BEP_BIN holds a relative path. Each message gives a next step. With -o json the refusal carries its defined code and a next-step hint. Each sealed-credential refusal from the proxy names its own cause, and a request from no running box no longer reads as another box's credential. Notes from the agent: Revise round 1. The task text says no_live_box and cross_box both say the credential belongs to another box. That is false for no_live_box (BEP-020: the source resolves to no live box), so no_live_box now has its own text: "...because the request did not come from a running box on this host. Send the request from inside the running box that was given this sealed credential." This deviates from the task wording on purpose. A relative MINVMD_BEP_BIN now gives a new ProxyAbsence::RelativePath cause, and proxy_absence takes the lookup Result in place of an Option. The ProxyAbsence text moved into its own Display impl so the BepRefusal fmt stays under clippy's too_many_lines limit. just clippy-strict was not run. |
There was a problem hiding this comment.
Reviewed at 385bae2 against T109's plan block.
What holds:
- Exit codes are unchanged; exit 5 is pinned for each cause.
-o jsonmaps the refusals togithub_sign_in_requiredandgatehouse_unenrolled_node.no_live_boxhas its own text, and the plan deviation is correct perredeem.rs:448.- A relative
MINVMD_BEP_BINpath gets its own cause. min auth loginandMINVMD_BEP_BINare real.
Three defects, all in user-facing text:
crates/bep/src/forward.rs:1147-1150, pinned at:2912-2918:store_handle_invalidnames one cause, and the check covers six.redeem.rs:467-469refuses with this marker wheneverhandle.check()fails.handle.rs:373-387lists the causes:Signature,ContextMismatch,Expired,NoRule,OutsideRuleandInjectMismatch. If a handle has expired, or the key that signed it was replaced, the person is told no rule admits the secret, and goes looking for a rule problem that does not exist. This is minimal-arch's open C1 on this head (Gatehouse §6.10 line 842). The text must say what can happen, as AGENTS.md "User-facing text" requires.crates/boxes/src/core/bep.rs:520and:608(-o jsonhint): the next step names the wrong table. "Remove GH_TOKEN from the [secrets] table in minimal.toml" sends the person to a table that does not exist there. The grants are read from[session.secrets](crates/mfile/src/lib.rs:524, read byproject_bep_and_grants,create.rs:216). The only[secrets]table is in the client's config.toml. The refusals atcreate.rs:165/178/186already say[session.secrets] in minimal.toml, so one thing now has two names on the same path. The tests atcrates/minimal/src/tests.rs:4866/4886/4913/4943andmain.rs:768pin the wrong name.crates/minimal/src/cmd/boxes/create.rs:546-550, printed viacrates/boxes/src/core/bep.rs:411: the log path is stated without being checked.proxy_log_pathreturns<minvmd host dir>/bep.logwhether or not the file exists. On a host whoseminvmdhas never startedbep, the first grant box prints "The proxy's log is at " for a file that was never written. The same happens on a Linux host using the native backend, which never startsbep.
|
/revise Three user-facing text fixes from review at 385bae2. Keep exit codes and audit markers unchanged.
Run |
…ause-and-the-next-step-in-the-grant-an
The store handle refusal now lists each cause its marker covers and no single remedy. off_port says the credential is sent to that host only on other ports. The grant refusal names [session.secrets].
The not-answering refusal drops the log sentence when bep.log is absent. The grant refusal and its -o json hint name [session.secrets].
…ause-and-the-next-step-in-the-grant-an
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This head resolves the open items from the CONDITIONS on 385bae21. The store_handle_invalid text now names each check that Gatehouse §6.10 line 842 lists, and it gives no single remedy. The grant refusal and its -o json hint name [session.secrets], which is the real table. The proxy log is named only when the file exists. The defined errors github_sign_in_required and gatehouse_unenrolled_node follow §6.2 line 580, and the audit markers do not change. BEP-003 conforms, and there are no conditions.
Ruled at ac2067a against gominimal/arch@8640f85e193d by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 8 unresolved threads — minimal-foundry[bot] crates/bep/src/forward.rs:1158, minimal-arch[bot] crates/bep/src/forward.rs, minimal-arch[bot] crates/bep/src/forward.rs:1150, minimal-arch[bot] crates/bep/src/forward.rs, minimal-arch[bot] crates/bep/src/forward.rs:2925, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
Round 3 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
A grant box refused for a missing proxy tells the person to remove the grant from [session.secrets] in minimal.toml. It names the proxy log only when that file exists. The -o json hint gives the same step. A proxy refusal names its cause: store_handle_invalid lists every cause its marker covers and gives no remedy, and off_port says the credential goes to that host only on other ports. Notes from the agent: Round 3. Directive items 1-3 are done. The exit codes and audit markers did not change. The off_port refusal now has its own text, for thread PRRT_kwDOPdfcxM6yOvo5. The task grouped off_port with outside_bound_set and outside_current_set, but that text was false for off_port. The note asked for |
There was a problem hiding this comment.
Re-reviewed at ac2067a. All three are fixed and pinned.
store_handle_invalidnow names everyhandle::Invalidcause (signature, another box or host, expiry, nothing admits it, other hosts, a different header form) and gives no remedy (7fb2c70).off_portnow says the credential is sent to the host only on other ports.- The next step says
[session.secrets] in minimal.tomleverywhere, including the-o jsonhint, and the tests follow. - The proxy's log is named only when the file exists (dccac57). The test covers both cases: no log file, and a log file written.
The other changed files come from the merge of main (#2793 and #2789).
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 ac2067ad1d05 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.
T109 — Name the cause and the next step in the grant and proxy refusals
slice: S15
kind: task
covers: BEP-003
size: M
est: 180 code, 300 tests
executor: agent
difficulty: needs care
model: standard
diagnostics: each refusal below states its cause and one next step in the words the person types, pinned by the tests in its layer
observability: the existing info line of each refused creation keeps its rule, fields and code; each proxy refusal keeps its audit marker
layers:
- create:
- crates/minimal/src/cmd/boxes/create.rs: when no proxy serves boxes, the check tells apart a
bepthat is not installed (the proxy binary thatbep::store::proxy_binary_pathresolves does not exist) from an installed proxy that does not answer, and passes the cause to the refusal- crates/boxes/src/core/bep.rs: the exit-5 refusal no longer says only "Remove the grant". With no
bepinstalled it reads "Cannot create the box: it declares the GitHub grant , andbep, the Box Egress Proxy, is not installed at , so no proxy serves boxes on this machine. To create the box without the grant, remove from the [secrets] table in minimal.toml." Withbepinstalled and not answering, it says the proxy at is installed and does not answer, names the log file the host daemon writes for it, and keeps the same sentence about removing the grant- crates/minimal/src/tests.rs:
a_grant_box_with_no_proxy_is_refused_as_an_unenrolled_nodepins both texts and exit 5test: cargo nextest run -p minimal a_grant_box_with_no_proxy_is_refused_as_an_unenrolled_node
test: cargo nextest run -p boxes creation_without_sign_in_fails_with_defined_error
- json:
- crates/minimal/src/main.rs:
machine_mode_answergives a refused activation's defined error as the-o jsoncode(github_sign_in_required,gatehouse_unenrolled_node), notunspecified, with the refusal's next step as the hint: "runmin auth login, then create the box again" for the sign-intest: cargo nextest run -p minimal machine_mode_names_the_grant_refusal_code
- proxy:
- crates/bep/src/forward.rs:
refusal_bodysplits the one text "it did not accept the sealed credential in the request" by cause.github_sign_in_requiredreads "The Box Egress Proxy refused the request to , because the GitHub sign-in on the host has ended. On the host, runmin auth login, then send the request again."expired,revoked,undecryptableandunbounded_membersay the box's sealed credential no longer works on this host, and that re-creating the box gives it a new one.no_live_boxandcross_boxsay the sealed credential belongs to another box.off_port,outside_bound_setandoutside_current_setsay is not a host this credential is sent to.store_handle_invalidsays no rule in[secret-store-rules]on the host admits this stored secret fortest: cargo nextest run -p bep each_refusal_names_its_cause_and_next_step
produces: none
consumes: none
proof: A grant box refused for a missing proxy, a refusal read as -o json, and a request the proxy refuses each tell the person the cause and one next step they can take.
depends on:
- blocked by #2264
- blocked by #2542
Proof
A grant box refused for a missing proxy now says whether
bepis not installed at its path or is installed and does not answer. In the second case it also names the proxy's log. Both cases end with the step to remove the grant from [secrets] in minimal.toml. Under -o json the refusal reads as github_sign_in_required or gatehouse_unenrolled_node, with the next step as the hint. A request the proxy refuses now names its cause: an ended GitHub sign-in (runmin auth login), a sealed credential that no longer works (create the box again), one that belongs to another box, a host the credential is not sent to, or no [secret-store-rules] rule.Notes from the agent: The proxy's log path is computed in create.rs as
bep.login the minvmd host dir (minimal_client::resolve_host_dir(--minimal-dir, true)). It mirrors the privateBEP_LOG_FILEconst in crates/minvmd/src/cmd/run.rs, because minvmd is outside the layers. The proxy program path comes frombep::store::proxy_binary_on_path, the same rulemin secretuses. When no path resolves (no minvmd on PATH), the text saysbepis not installed and names no path. The not-answering text reads: "bep, the Box Egress Proxy, is installed at , but it does not answer, so no proxy serves boxes on this machine. The proxy's log is at ." The refusal fromgrant_box_facts(no box id or no stance) gets a newProxyAbsence::NotServedcause. It keeps the general 'no Box Egress Proxy serves boxes' clause and the new sentence about removing the grant. The gatehouse_unenrolled_node -o json hint is "to create the box without the grant, remove from the [secrets] table in minimal.toml". The task gave no exact hint for it. The off_port/outside_*_set and store_handle_invalid texts state only the cause, because the code cannot prove a remedy. The existing debug lines and audit markers are unchanged. Extra runs:cargo nextest run -p bep(235 passed), and the-p boxes bepand-p minimal grant/`machine_m…[truncated]Tests
cargo nextest run -p minimal a_grant_box_with_no_proxy_is_refused_as_an_unenrolled_nodecargo nextest run -p boxes creation_without_sign_in_fails_with_defined_errorcargo nextest run -p minimal machine_mode_names_the_grant_refusal_codecargo nextest run -p bep each_refusal_names_its_cause_and_next_stepVerify
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 #2783