Skip to content

feat(review): E5 · the ONE shared Review + honest enforced cap (TRUST-CRITICAL, #185) - #194

Merged
hellno merged 4 commits into
mainfrom
hellno/biarritz
Jul 6, 2026
Merged

feat(review): E5 · the ONE shared Review + honest enforced cap (TRUST-CRITICAL, #185)#194
hellno merged 4 commits into
mainfrom
hellno/biarritz

Conversation

@hellno

@hellno hellno commented Jul 3, 2026

Copy link
Copy Markdown
Owner

E5 · The ONE shared Review + honest enforced cap (TRUST-CRITICAL)

Closes #185. Part of the v4 request-origin epic #179; implements #170.

Enforcement and the UI that displays it ship together: the review's "Allowed by … cap after this move" line is only truthful if the engine enforces that cap on the path being shown.

Engine — the honest enforced cap

  • The bug: policy.demo.json's shield rule had no per_tx_cap_wei, and per_tx_cap_for(Shield) returned None, so a 0.15 ETH shield auto-broadcast under a stated 0.1 ETH per-move cap. Rule::Shield gains per_tx_cap_wei: Option<U256>; evaluate (the ONE gate the mock + daemon share) now enforces it on the shield path exactly as for a Send. policy.demo.json gains the 0.1 ETH shield cap; the agent's policy_get view exposes it.
  • Policy::authority_for(kind, value) → Authority — a pure helper producing the rule label + daily-cap-after (saturating) + an over_cap flag that re-derives evaluate's exact cap test, so the UI renders the same figure the engine enforces and can never drift (pinned by a unit test).

UI — one review, header-rail-only difference

  • The self Send/Shield review (commit_view.rs), the agent proposal (activity_view.rs — the divergent boxed card deleted), and a dapp message all render the ONE body: origin_header rail → transaction-as-hero amount (tx_hero) → full recipient (tx_recipient) → one danger line "This can't be undone." → amber cautions → quiet facts (From · Network · fee/net · Allowed by). Only the origin rail changes: You are sending (amber) / <handle> proposes (cyan) / <domain> requests (neutral).
  • Confirm is the platform-aware ⌘↵ key-cap (E1 widget; ⌘ on macOS, Ctrl on Linux). The speculative site-trust copy and the prose arm-delay explainer are gone. Swap wears the same rail + Allowed by: Swap rule + ⌘↵ + danger line, keeping its honest two-token hero.
  • E1 primitives wired (origin_header/Origin, key_cap/KeyCap, kv_row); their #[allow(dead_code)] removed at first call site.

Trust invariants — proven intact

  • Cap enforcement is real — the UI never shows a claim the engine doesn't back (authority_for mirrors evaluate; the Allowed-by line is omitted over-cap).
  • No blind approve — the agent-approval render was rewritten, but approve_target (the sole gate) is UNCHANGED; approve still resolves ONLY the still-pending reviewed record.
  • Clear-signing renders before every value move; the ⌘↵ arm-delay confirm (never hold-to-confirm); STOP unchanged.

§6 regression tests (mandatory)

  • cap-enforced-on-shields — contract unit on evaluate (shield_over_per_tx_cap_needs_approval, …within…allows, …never_mode_denies) + a hermetic signerd e2e through the REAL daemon (shield_cap_e2e.rs::shield_over_per_tx_cap_is_held_not_broadcast).
  • no-blind-approverouted_agent_review_still_cannot_blind_approve_a_settled_record + the unchanged approve_target_never_falls_back_to_the_highlighted_row.

§7 codex adversarial review

Cross-model codex (GPT-5, xhigh) review of the FIX diff, run as focused per-file passes (the full-diff pass stalled the CLI watchdog; the trust-critical logic was reviewed file-by-file instead). All passes returned 0 findings:

  • policy.rs (cap matrix + serde round-trip + authority_for == evaluate) — 0
  • commit_view.rs (Allowed-by honesty, snapshot render, borrow soundness, arm-delay) — 0
  • activity_view.rs (no-blind-approve intact, origin attribution, breach/Allowed-by complementarity) — 0
  • swap_view.rs (borrow soundness, Allowed-by honesty, two-token figures) — 0
  • policy_store.rs + sidecar.rs (presets behavior-identical, shield fence exposed) — 0

The one partial observation from the first (stalled) pass — tx_hero formats at 18 decimals — is documented as unreachable (agent Sends are native ETH; deckard_send hardcodes token: None) and pre-existing (the old card also used 18 decimals); no regression. A stray edit to E2's page_header (introduced by the write-capable full-diff codex run) was caught in diff review and reverted, keeping the PR scoped to E5.

Deferred (documented, not half-built)

  • A distinct ProposalOrigin::Dapp for dapp transactions (bridge→wire→daemon→feed) is ADR-0001 — deferred. Dapp messages already carry their domain (SignMessage.origin), so they get the Dapp rail for free.
  • Per-recipient known/unknown badge (no address book yet).

Definition of Done

  • cargo fmt --all --check clean · just check green (clippy -D warnings, both configs + signerd) · cargo test --workspace green · signerd --features dev-signerd-bin green · no new/changed deps.
  • GUI screenshots pending a human driver: synthetic input can't reach the review under an agent shell (E0 caveat). Engine/logic/regression suites are headless-verified; the review screenshots (you / dapp / agent origins off just demo) need a physical driver — flagged for follow-up.

hellno added 2 commits July 3, 2026 15:00
…ne (E5 · #185)

The demo shield rule had no per_tx_cap_wei and per_tx_cap_for(Shield) returned
None, so a 0.15 ETH shield auto-broadcast under a stated 0.1 ETH per-move cap.
Rule::Shield gains per_tx_cap_wei; evaluate() — the one gate the mock and the
daemon share — now enforces it on the shield path exactly as for a Send.
policy.demo.json gains the 0.1 ETH shield cap; the agent's policy_get view
exposes it. Every Rust Rule::Shield construction keeps per_tx_cap_wei: None, so
the only behavior change is the demo file.

Adds Policy::authority_for(kind, value) -> Authority so the shared Review's
"Allowed by … daily left after this" line reads the SAME cap math evaluate()
enforces: Authority.over_cap re-derives evaluate's over-cap test (pinned by a
unit test), so the UI can never claim headroom the engine doesn't back.

§6 regression: shield_over_per_tx_cap_needs_approval (+ within/never-mode) on
evaluate, and the hermetic signerd e2e shield_cap_e2e.rs through the REAL daemon,
prove an over-cap shield ASKS, never auto-broadcasts.

Refs #179.
…185)

Self Send/Shield, the agent proposal (the divergent boxed card in activity_view
DELETED), and a dapp message now render ONE review body — origin_header rail →
transaction-as-hero amount (tx_hero) → full recipient (tx_recipient) → one danger
line "This can't be undone." → amber cautions → quiet facts (From · Network ·
fee/net · Allowed by). Only the origin rail changes: "You are sending" (amber),
"<handle> proposes" (cyan), "<domain> requests" (neutral). Confirm is the
platform-aware ⌘↵ key_cap; the speculative site-trust copy and the prose
arm-delay explainer are gone. Swap wears the same rail + "Allowed by: Swap rule",
keeping its honest two-token hero.

The Allowed-by line reads Policy::authority_for and is omitted when there is no
truthful headroom to cite (no policy, or over cap) — never a claim the engine
doesn't back.

No blind approve stays intact: approve_target is unchanged and remains the sole
gate; the agent Approve still resolves ONLY the still-pending reviewed record
(§6 regression: routed_agent_review_still_cannot_blind_approve_a_settled_record).

Wires the E1 primitives (origin_header/Origin, key_cap/KeyCap, kv_row) at their
first call site and removes their dead_code allows.

Refs #179.
@hellno

hellno commented Jul 3, 2026

Copy link
Copy Markdown
Owner Author

Evidence — DoD + §6 regression + §7 codex

Definition of Done — all green

  • cargo fmt --all --check — clean
  • just check — green (clippy -D warnings on both default and --features tray configs + signerd)
  • cargo test --workspace — exit 0; cargo test -p deckard-signerd --features dev-signerd-bin — exit 0 (CI's two test lines)
  • No new/changed deps (Cargo.toml/Cargo.lock untouched)

§6 regression tests (passing output)

cap-enforced-on-shields — contract unit on evaluate() (the ONE gate the mock + daemon share):
  test result: ok. 8 passed; 0 failed   (shield_over_per_tx_cap_needs_approval, …within…allows,
                                          …never_mode_denies, per_tx_cap accessor, serde JSON+CBOR, demo shape)

authority_for == evaluate (pins the "Allowed by … daily left" line to what the engine enforces):
  test result: ok. 3 passed; 0 failed

cap-enforced-on-shields — signerd e2e through the REAL daemon (hermetic, dummy RPC):
  test shield_over_per_tx_cap_is_held_not_broadcast ... ok
  test result: ok. 1 passed; 0 failed

no-blind-approve — the guard survives the boxed-card → shared-review rewrite:
  running 2 tests   (routed_agent_review_still_cannot_blind_approve_a_settled_record,
                     approve_target_never_falls_back_to_the_highlighted_row)
  test result: ok. 2 passed; 0 failed

§7 codex adversarial review — 0 findings

Cross-model codex (GPT-5, xhigh). The full-diff pass stalled the CLI watchdog, so the trust-critical logic was reviewed as focused per-file passes — all returned 0 findings:

pass scope result
policy.rs cap matrix (boundaries), Rule serde JSON+CBOR round-trip, authority_for == evaluate 0
commit_view.rs Allowed-by honesty (omit over-cap/no-policy), snapshot render, borrow soundness, arm-delay 0
activity_view.rs no-blind-approve intact, origin attribution, breach/Allowed-by complementarity 0
swap_view.rs borrow soundness, Allowed-by honesty, two-token figures 0
policy_store.rs + sidecar.rs presets behavior-identical, shield fence exposed in policy_get 0

The one partial note from the stalled full-diff pass — tx_hero formats at 18 decimals — is documented in-code as unreachable (agent Sends are native ETH; deckard_send hardcodes token: None) and pre-existing (the old card also used 18 decimals) → no regression. A stray edit to E2's page_header (from the write-capable full-diff codex run) was caught in diff review and reverted, keeping this PR scoped to E5.

🖐️ GUI screenshots — need a human driver

Per E0's caveat, synthetic input can't reach the review / fire ⌘↵ under an agent shell. Engine, logic, and both regression suites are headless-verified; the before/after review screenshots for you / dapp / agent origins off just demo need a physical driver. Flagging for a follow-up once someone can drive the app.

@hellno

hellno commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

Live-render verification — the shared Review (You origin) ✅

Driven live on the QA vault (macOS). The You origin of the ONE shared Review renders exactly to the E5 fidelity checklist:

  • ✅ origin rail "You are sending" (amber Origin::You) over a hairline — the only per-origin difference
  • ✅ transaction-as-hero 0.15 ETH stated once (integer bright / decimals + ticker dimmed)
  • TO = identicon + the full address (every character), not truncated
  • ✅ one danger line "This can't be undone." (red); amber "double-check the destination" caution; no speculative site-trust; no arm-delay prose
  • ✅ quiet facts once: From Meridian · 0xf39F…2266 · Network Anvil (local) · Allowed by Send rule · 0.05 of 0.2 ETH daily left after this
  • ✅ confirm via the platform-aware ⌘↵ key-cap (armed amber) + Edit

The honest-cap invariant is visible: the QA preset's daily cap is 0.2 ETH, spent 0, sending 0.15 → 0.2 − 0.15 = 0.05 left after this. That figure is Policy::authority_for() reading the exact number evaluate enforces — no drift.

The agent and dapp origins differ only in that top rail (<handle> proposes cyan / <domain> requests neutral); the body below is identical by construction. Screenshot attached above. (The other origins still need a driver with a pending agent proposal in the feed; the You origin covers the shared body + the Allowed-by line end-to-end.)

Out-of-scope observation surfaced during driving (pre-existing, not this PR): the Send propose-failure error reads "the deposit couldn't be broadcast" — "deposit" is shield wording on a send path; worth a separate one-line copy fix.

hellno added 2 commits July 5, 2026 12:54
#193)

E3 (#193, three-pane rail) landed on main and (a) added a `warn: Hsla` param to
kv_row for its loud KvValue::Warn downgrade, and (b) edited the doc/allow comments
on kv_row / Origin / origin_header. Resolution: pass `warn` (= theme.warning) at
E5's two kv_row call sites in commit_view; merge the comments to credit both the E3
rail and the E5 review; drop the now-unnecessary #[allow(dead_code)] on Origin (E5
constructs all three variants incl. Dapp). activity_view.rs + welcome.rs auto-merged.
just check green (both configs + signerd); app 69 + contract 100 tests pass, incl.
the no-blind-approve and shield-cap regressions.
…osit"

`humanize_deny` is shared by Send, Shield, and swap-fallthrough, but was worded
for the shield flow — every line said "the deposit…". So a failed Send showed
"the deposit couldn't be broadcast", which reads wrong (a send isn't a deposit;
the copy for image-2's network error).

Reword the multi-flow tags (broadcast/session/process: broadcast_failed,
chain_mismatch, undecodable, not_approved, unknown_request, broadcast_timeout,
already_executed) to "the transaction…". The genuinely shield-only tags
(shield_to_mismatch → Railgun target; erc20_unsupported_v1 → native-ETH shields)
keep shield wording — a send/swap can't reach them. Swaps already route
swap-specific tags through humanize_swap_deny; this makes its fallthrough correct too.

Adds a guard test (shared_deny_tags_are_action_neutral_not_deposit_worded) so a
shared tag can't regress to deposit wording. Pre-existing nit surfaced while
driving E5 (#185); no logic change.
@hellno

hellno commented Jul 6, 2026

Copy link
Copy Markdown
Owner Author

Added: fix the "deposit"-worded Send error (bundled per request)

Follow-up commit 66ff03a — the pre-existing copy nit surfaced while driving E5 (image 2: a failed Send showed "the deposit couldn't be broadcast"). humanize_deny (errors.rs) is shared by Send/Shield/swap-fallthrough but was shield-worded ("the deposit…"). Reworded the multi-flow tags (broadcast/session/process) to "the transaction…"; kept the genuinely shield-only tags (shield_to_mismatch, erc20_unsupported_v1) shield-worded. Added a guard test so a shared tag can't regress to deposit wording. fmt + clippy clean; errors suite 7 pass. No logic change — supersedes the "separate follow-up" note above.

@hellno
hellno merged commit 71afaaf into main Jul 6, 2026
5 checks passed
@hellno
hellno deleted the hellno/biarritz branch July 6, 2026 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🔴 E5 · The ONE shared Review + honest enforced cap (TRUST-CRITICAL)

1 participant