Skip to content

fix(control)!: gate chiaPeers on the master-token tier and adopt dnci 0.18 - #248

Merged
MichaelTaylor3d merged 15 commits into
mainfrom
loop/2870-chia-trusted-peers
Aug 20, 2026
Merged

fix(control)!: gate chiaPeers on the master-token tier and adopt dnci 0.18#248
MichaelTaylor3d merged 15 commits into
mainfrom
loop/2870-chia-trusted-peers

Conversation

@MichaelTaylor3d

@MichaelTaylor3dMichaelTaylor3d commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

What this does

Adopts dig-node-control-interface0.18.0 (live on crates.io) and closes
#254 — nine items, of which item 1 is a live
privilege escalation
, not a version bump.

Parent epic: https://github.com/DIG-Network/dig_ecosystem/issues/2870

DO NOT MERGE — security gate pending. Item 1 earns a real security gate; this stays DRAFT
until that verdict returns.


1. SECURITY — a paired token could grant itself unrevocable chain authority

chiaPeers.add/.remove are master-tier in the contract. This node did not honour that, because it
restated the predicate as a string list:

pubfnis_pairing_admin_method(method:&str) -> bool{matches!(method,"control.pairing.list" | "control.pairing.approve" | "control.pairing.revoke")}

chiaPeers.* is absent, so a paired token calling control.chiaPeers.add succeeded — installing a
peer with WriteAuthority::Operator, "full authority, no ceiling", over the wallet replica.

It delegated and was not revocable. After the call the attacker no longer needed the token, and
pairing::revoke_paired_token (pairing.rs) removes a token id and touches no peer row.

The fix: call the contract, do not re-state it

pubfnrequires_master_token(method:&str) -> bool{matchControlMethod::from_name(method){Some(published) => published.requires_master_token(),None => !CONTROL_METHODS.contains(&method),}}

The duplicate is gone, not corrected. A security predicate duplicated across a repo boundary as a
string match drifts silently and fails OPEN — which is exactly what happened.

is_control_method matches on the control.prefix, so an unrecognised name does reach this gate;
it fails closed. A method this node genuinely serves but the contract has not published yet
(control.peers.ping, tracked known-drift) keeps the ordinary tier, because promoting it would
silently break paired clients — a behaviour change unrelated to this escalation. That exception is
asserted explicitly so it cannot quietly widen.

Rejected: making revoke strip user-managed peers. A compromised app's cleanup would then silently
un-trust nodes the operator deliberately configured. The escalation is unrevocable by design, so it is
made unreachable instead. Recorded on revoke_paired_token so it is not "fixed" later.

After the change, a paired token

CanCannot
control.chiaPeers.list — read the trust state it is subject tocontrol.chiaPeers.add — grant trust
every ordinary control.* mutationcontrol.chiaPeers.remove — strip operator-configured trust
pairing administration (unchanged)

list stays ordinary-tier deliberately: it is a read, grants nothing that outlives the token, and
gating it would leave a paired client unable to show the operator the trust state it is subject to.


Blast radius checked (gitnexus impact, per-worktree index)

SymbolDirectionRiskRadius
is_pairing_admin_methodupstreamLOW2 direct: server.rs::rpc (HTTP gate), server.rs::ws_dispatch (WS gate); 1 transitive ws_handle_text. Both are the auth gates — exactly the intended radius.
all_peersunbanned_peersupstreamLOW4 sites, incl. sync_supervisor.rs:1822the dialler
add_peerupstreamLOWnetwork::add_peer, 3 db tests
remove_peerupstreamMEDIUMnetwork::remove_peer, 4 db tests
get_peersupstreamCRITICAL (fan-out artifact)48, almost all via the shared dispatch_inner. Real shape consumers: chia_peers_list and the Sage-parity get_peers arm — see the caveat below.

The dialler finding is why item 5 is not a one-line change.all_peers() feeds
sync_supervisor.rs:1822, which picks the full node to dial. Item 5 asks list to include banned
entries; satisfying that by relaxing that one query would have fed banned peers to the dialler. A
ban applied to a previously user_managed peer leaves that flag set, so the existing
.filter(|p| p.user_managed) would not have caught it. The reads are therefore split:
unbanned_peers() (dialling) and all_peers_including_banned() (control plane), asserted together in
one test.

Renames were done by explicit per-site edit against the enumerated radius, not find-and-replace
and a missed site is a Rust compile error, which is a stronger check than a rename tool.


The other eight items

#ChangeTest that fails without it
2removed: booloutcome: ChiaPeerRemovalOutcome, no boolean companionremoving_a_chia_peer_the_node_never_had_is_not_reported_as_a_removal
3CORROBORATION_BYPASS_NOTICE emitted into the declared notice fielda_trusted_chia_peer_can_be_added_listed_and_removed_over_the_control_plane
4corroboration_bypassed is the resulting trust state, read back from the rowadding_a_banned_peer_unbans_it_without_granting_trust
5ChiaPeerEntry.banned; list stops filtering banned rowsbanned_peers_are_listed_for_the_operator_but_never_dialled
6peak_heightOption<u32>; null unobservable, never 0add_get_remove_peer_round_trip
7Wording narrowed to "a node you run yourself"the_trust_wording_authorises_only_a_node_the_operator_runs (+ CLI help twin)
8ip canonicalised in via params::canonical_peer_ip; joined via params::chia_peer_endpointan_ipv6_chia_peer_is_stored_canonically_so_a_second_spelling_still_matches
9MAX_BANNED_CHIA_PEERS = 256, oldest evicted; remove {ban:false} unbans, grants no trustthe_ban_list_is_bounded_and_evicts_the_oldest_ban

Item 4 is the one that was reporting a falsehood about custody-grade authority: the db.rsDO UPDATE
clears banned and refreshes the port but leaves user_managed alone, so ban-then-add returned
success without granting trust
while the result claimed corroboration_bypassed: true. It now reads
the flag back and reports what actually happened, with a distinct notice.

Item 9's bound is pinned from both sides — at exactly 256 nothing is evicted, one over evicts
exactly one — and the direction is asserted: the newest ban survives, because a full list that
refused the newest would deny the ban facility exactly when an operator needs it.

Caveat worth a reviewer's eye

PeerRecord also serializes on the Sage-parityget_peers JSON surface, so items 5 and 6 change
that body too: peak_height becomes null and banned is added. The old value was always0
(nothing writes peer telemetry — SPEC §18.16), so no consumer was reading a meaningful number. Keeping
two divergent peer shapes seemed worse than one honest one, but this is a deliberate call, not an
oversight.


Verification

  • control_contract_conformanceGREEN, no exemptions. The file is byte-identical to main
    (git diff main on it is empty); KNOWN_UNPUBLISHED is still exactly ["control.peers.ping"]. It
    passes on the 0.18 bump alone, which is what it was red to force.
  • Revert proof for item 1, run with the fix reverted to the old three-string list (committed first,
    restored after):
    master_token_set_matches_the_contract ... FAILED
    left: {"control.pairing.approve", "control.pairing.list", "control.pairing.revoke"}
    right: {"control.chiaPeers.add", "control.chiaPeers.remove", "control.pairing.approve",
    "control.pairing.list", "control.pairing.revoke"}
    reading_the_trusted_peer_list_is_not_master_tier ... FAILED
    assertion failed: requires_master_token("control.chiaPeers.add")
    
    It names the two missing methods rather than merely going red.
  • cargo fmt --all --check exit 0; cargo clippy --workspace --all-targets -- -D warnings exit 0.
  • Version 0.126.20.127.0 (minor: breaking control-plane result shapes, which is minor in 0.x).
  • SPEC.md updated in the same unit — the three method rows and the CLI section now state the
    master-token tier, the outcome enum, the resulting-trust semantics, the banned enumeration, the
    null peak and the bounded ban list.

Test design notes

Two fixtures were built to avoid pinning a coincidence:

  • The paired-token e2e varies one thing across three calls on the same token and keeps a
    truthful control (list succeeds). Asserting only the two refusals would be satisfied by a gate that
    refused the whole namespace, or by a node with no Chia-peer surface at all — so it would prove
    nothing about placement.
  • master_token_set_matches_the_contract writes the expected set out by name rather than deriving
    it from the predicate under test. A derivation would agree with any implementation, including the
    one the test exists to catch.

Not fixed here — filed separately

control.config.setUpstream's tier is #255. Same
defect class as item 1 and arguably wider reach: ordinary-tier, persists a caller-chosen RPC upstream,
and survives pairing.revoke. Deliberately its own ticket and its own PR — not folded in here, so the
security gate on this diff stays scoped to the peer-trust boundary.

Closes#254

…n tier
A paired token could call control.chiaPeers.add, installing a Chia peer the wallet
replica believes WITHOUT corroboration. The entry outlives the token -- pairing.revoke
removes a token id and touches no peer row -- so the designated remedy for a compromised
paired app could not take the authority back.
The cause was a duplicated predicate: is_pairing_admin_method restated the master tier as
a match on three pairing strings, so when the contract moved chiaPeers.add/.remove onto
that tier this node kept honouring the old list, failing OPEN. The duplicate is now gone
rather than corrected -- requires_master_token delegates to
ControlMethod::requires_master_token -- and a lockstep test pins the two sets together.
Closes part of #254
… ban list
Items 2-9 of #254.
- remove reports ChiaPeerRemovalOutcome (removed|no_such_peer) with no boolean
companion, so a consumer cannot render 'nothing was there' as 'it is gone'
- add emits its notice INTO the declared notice field
- corroboration_bypassed is the RESULTING trust state, read back from the row: adding
a banned peer un-bans it WITHOUT granting the bypass, which previously reported a
falsehood about custody-grade authority
- list includes BANNED entries; it is the only enumeration of them. The dialling read
(unbanned_peers) stays separate so banned peers can never reach the dialler
- peak_height is Option<u32>: null means unobserved, never height 0
- ip is canonicalised on the way IN via params::canonical_peer_ip, and joined via
params::chia_peer_endpoint so ::1 + 8444 cannot render ::1:8444
- ban list bounded at MAX_BANNED_CHIA_PEERS, oldest evicted
- trust wording narrowed to 'a node you run yourself'
Refs #254
…as a failure to act
The add line defaulted an ABSENT corroboration_bypassed to false, which would report a
peer as untrusted against a pre-0.18 node that had in fact granted trust; absent now reads
as granted and only an explicit false says the bypass was withheld. The fallback notice for
a node too old to send one warns again -- it had been weakened to a neutral sentence.
Refs #254
@MichaelTaylor3dMichaelTaylor3d changed the title feat(cli): add, list and remove a trusted Chia peerfix(control)!: gate chiaPeers on the master-token tier and adopt dnci 0.18Aug 19, 2026
@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

loop-security — GATING finding (interim, posted as formed)

Head audited: 94f6eb63a40c67e7e2d22136446b9904fca89647 (resolved from remote).

CRITICAL — the escalation is NOT closed. A paired token still installs a trusted Chia peer, via the Sage-parity wallet plane.

The master-tier gate added here covers the control.* plane only. WalletBackend::add_peer is also reachable on the Sage-parity surface, where the policy is master-or-paired, and it lands on the identical writer.

Exploit (state -> action -> impact):

State: the attacker holds a valid paired controller token (a compromised or malicious paired app). This is exactly the principal the master tier exists to exclude.

Action: one request, no control.* involved.

POST http://127.0.0.1:<node port>/add_peer
X-Dig-Control-Token: <paired token>
Content-Type: application/json
{"ip":"203.0.113.10"}

Path, each hop verified at this head:

  1. crates/dig-node-service/src/server.rs:245.route("/:method", post(wallet_rpc).get(fallback_serve)). Only host_guard (anti-DNS-rebinding) and the CORS layer wrap it; neither excludes a paired holder — this surface is the paired extension's own node-wallet client target.
  2. crates/dig-node-service/src/server.rs:1337wallet_rpc -> wallet_authz::requires_authorization("add_peer").
  3. crates/dig-node-service/src/wallet_authz.rs:70-71add_peer and remove_peer are members of GATED_WALLET_MUTATIONS, so classify returns Mutation.
  4. crates/dig-node-service/src/wallet_authz.rs:151ct_eq(tok, master) || is_paired(tok). The paired token passes.
  5. state.wallet.dispatch("add_peer", body) -> crates/dig-wallet/src/sage/rpc.rs:4165 -> WalletBackend::add_peer (rpc.rs:3810) -> add_peer_reporting_trust (rpc.rs:3795) -> network::add_peer(&self.db, &req.ip).

That final call is byte-for-byte the writer control.chiaPeers.add uses (control.rs::chia_peers_add -> ctx.wallet.add_peer_reporting_trust). The row produced is identical: user_managed, hence PeerTrust::Operator.

Impact: precisely the escalation this PR states it closes — a peer believed without corroboration, authoritative for money-bearing chain facts, which survives pairing.revoke because revoke touches no peer row. Nothing about the outcome differs from the control.chiaPeers.add route; only the URL does.

The same hole inverts the remedy.POST /remove_peer with {"ip":"<operator's own node>","ban":true} is equally paired-reachable, so a paired token can strip and then ban the operator's deliberately trusted node. The contract's own rationale says remove is on the master tier so that "a paired token cannot strip the peers an operator deliberately trusts" — that property does not hold at this head.

Both transports, not one. The WS fall-through in ws_dispatch (server.rs, after the requires_authorization check) reaches state.wallet.dispatch(method, &body) under the same master-or-paired policy, so /ws carries it too. Fixing only POST /:method would leave the WS leg open.

Why this gates rather than being filed as pre-existing

The dispatch arms predate this PR (the diff refactors add_peer/remove_peer rather than introducing them). But the PR's deliverable is the closure of this exact escalation, and it now ships that closure as an asserted security property in three places that are false as written:

  • server.rs:1043-1048 — "so it cannot install a peer that keeps unbounded authority over the wallet replica after revocation."
  • pairing.rs:44-48 — "chiaPeers.add/.remove, so it cannot grant itself chain authority that SURVIVES the revocation below."
  • pairing.rs revoke doc — "the escalation is made UNREACHABLE at the gate instead." It is reachable at a different gate.

A false claim of unreachability is worse than the silence it replaced: it is the sentence the next reviewer will trust instead of re-deriving the reachability. The nine revert proofs are all anchored on the control.* predicate, so none of them can fail on this route.

What would close it

The tier must be a property of the capability, not of the plane the caller chose. Options, in preference order:

  1. Have the wallet-plane gate consult the same tier predicate: in wallet_authz, classify add_peer/remove_peer as a new master-only class (or short-circuit them to ct_eq(tok, master) only), so both planes derive from one rule.
  2. Map the Sage-parity names onto their ControlMethod equivalents and reuse requires_master_token.

Either way it needs a lockstep test in the shape of master_token_set_matches_the_contract asserting that every route reaching network::add_peer/network::remove_peer is master-gated — the current suite proves the property for one plane and generalises it in prose.

Remaining audit items (carve-out width, dialler split, corroboration_bypassed honesty, the peak_height wire change) follow in a second comment.

@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

loop-security — items 2 to 6, at head 94f6eb63a40c67e7e2d22136446b9904fca89647

Second of two comments. The first carries the gating finding.

Item 1 (both transports) — CLEAR, with the caveat in the first comment

Both callers of the old predicate moved: server.rs:1051 (HTTP rpc) and server.rs:1428 (ws_dispatch). There is no third call site — dispatch_control is invoked at exactly server.rs:1074 and server.rs:1440, each immediately under its own gate, and is_authorized is the only other reader of the method name. HTTP and WS agree line for line.

I checked the one route that skips the token gate entirely, is_open_control_read (control.rs:142-154): its membership is unchanged by this diff and contains no chiaPeers.* entry, so there is no anonymous route to the new methods. control.chiaPeers.list is token-gated at the ordinary tier as designed.

Within the control.* plane the escalation is genuinely closed. It is the wallet plane that is not.

Item 2 — the fail-closed carve-out: correct today, but it widens by itself. NOT gating.

I enumerated the exempted set rather than trusting the comment: CONTROL_METHODS (45 entries) minus the contract's published names at dnci 0.18.0 is exactly{control.peers.ping}. So the carve-out is one method wide right now, as claimed, and unknown names do fail closed.

But a newly-served-but-unpublished method inherits the exemption automatically, and no test catches it. Add any name to CONTROL_METHODS without publishing it in the contract and it is paired-reachable from that moment:

  • master_token_set_matches_the_contract computes actual as members of CONTROL_METHODS for which the predicate is true. A new unpublished method returns false, so it never enters actual, and actual == expected still holds. Green.
  • The second assertion compares against ControlMethod::ALL filtered by CONTROL_METHODS.contains, so an unpublished name is absent from both sides. Green.
  • an_unserved_control_method_requires_the_master_token pins two unserved names and peers.ping. Unaffected. Green.

That makes this sentence, on that test, false as written: "The served-but-unpublished diagnostic is the deliberate exception and is asserted here beside them, so the exception cannot quietly widen to cover a future method."

The exception can widen, silently, in the fail-open direction — the same failure mode that produced #254. It is latent, not live: no current method is affected, so this does not gate. But it should not ship with a comment claiming a property nothing holds.

The fix is nearly free, because the allowlist already exists.crates/dig-node-service/tests/control_contract_conformance.rs:29 already declares const KNOWN_UNPUBLISHED: &[&str] = &["control.peers.ping"];. Promote it to a pub const in control.rs, have the gate read it in the None arm instead of !CONTROL_METHODS.contains(&method), and have the conformance test read the same one. A future unpublished method then fails CLOSED by default, and granting it the ordinary tier becomes an explicit, reviewable one-line edit instead of a side effect of editing an unrelated list.

Related, pre-existing, not introduced here: the method the carve-out keeps at the ordinary tier, control.peers.ping, takes a caller-supplied params.peer as a host:port (peer_ping.rs:48), so a paired token can make the node dial an address of its choosing. Equally reachable before this PR — no regression — but it is what the exemption protects, so it is worth knowing what is being kept open.

Item 5 — the dialler split: CLEAR, and stronger than the PR claims

Verified end to end, and the design change is the right one.

  • db.rs:2372unbanned_peers() excludes bans in SQL (WHERE banned = 0), not at a caller.
  • sync_supervisor.rs:1822 — the dialler — calls unbanned_peers().
  • db.rs:2389all_peers_including_banned() has no WHERE, and its only production caller is network::get_peers (network.rs:34), the control-plane enumeration.

The concern about .filter(|p| p.user_managed) is real and correctly handled: remove_peer(ban: true) sets user_managed = 0 in its DO UPDATE (db.rs:2459-2464), but a row banned by any other route would retain the flag, so putting the exclusion in SQL is the only placement a caller-side filter cannot defeat.

No caller was left on the wrong side of the split, and this is provable rather than reviewed:all_peers() no longer exists as a symbol anywhere in the tree. At the base it had exactly two production callers (network.rs:22, sync_supervisor.rs:1822); both moved, and any missed site would be a compile error.

Item 4 — the honesty property: CLEAR

corroboration_bypassed is the resulting state, not a constant. db.rs::add_peer runs the upsert, then reads the flag back with SELECT user_managed FROM peers WHERE ip_addr = ?.

The ban-then-add case genuinely produces false: remove_peer(ban: true) sets user_managed = 0, and add_peer's DO UPDATE SET port = excluded.port, banned = 0, banned_at = NULL does not touch user_managed. So the row comes back 0, add_peer returns false, and chia_peers_add emits corroboration_bypassed: false with UNBANNED_WITHOUT_TRUST_NOTICE. The node reports less authority than was requested, which is the safe direction.

notice is emitted into the declared field, not alongside it: the contract declares ChiaPeersAddResult.corroboration_bypassed: bool and .notice: String (dnci 0.18.0 results.rs:575,582), and the handler populates both keys directly.

I also checked that the two writers in db.rs (lines 2421 and 2459) are the only INSERT INTO peers sites in the crate, so no discovery path can create a user_managed = 0 row that would make the notice's "no longer banned" wording inaccurate — a prior ban is the only way to reach that branch.

Item 6 / the Sage-parity wire change — LOW, not gating, but the coupling is avoidable

peak_height: 0 -> null and the added banned field do change the Sage-parity get_peers body, as the PR says.

The lane's safety argument holds on the facts I could measure: nothing writes peer telemetry, so the field was always literally 0, and I found no consumer of the Sage-parity get_peers anywhere in the ecosystem — not dig-app, not dig-chrome-extension, not dig-sdk. The extension's only peak_height consumer is the sync_status WS frame, a different surface, already typed number | null (dig-node-wallet-ws.ts:83).

The residual risk is a strict third-party Sage-compatible client: null where Sage sends a number is a parse-time break for anyone deserializing into a non-optional integer, even a client that never reads the value. That is a different failure from "reads a meaningless zero", and the PR's rationale only addresses the latter.

It is also avoidable, which is why I raise it. control.chiaPeers.list builds its own JSON object from PeerRecord's fields rather than serializing the struct, so the zero-to-null mapping could have lived at that boundary alone, leaving PeerRecord.peak_height: u32 intact for parity. One honest shape on the control plane and one unchanged parity shape is achievable without keeping two divergent internal types.

The follow-on point in the dispatch is the sharper one and I agree with it:peers.peak_height is NOT NULL DEFAULT 0 and the null is synthesised at the boundary by (r.peak_height > 0).then_some(...). When telemetry lands, a genuine genesis height becomes indistinguishable from unobserved. The hazard is documented at the exact site (network.rs: "the column has to become nullable"), which is the right place for it, but a documented trap is still a trap — worth a ticket so the migration is not discovered by the first person who sees a peer at height 0.

Minor: the title is fix(control)!: with a breaking marker while the bump is 0.126.2 -> 0.127.0 (minor). Defensible under 0.x and the version gate passes, but worth reconciling in the body so a consumer reading the changelog sees the wire change flagged.

Merge preconditions, asserted by name

check-merge-preconditions.sh at this head: Lint commit messages / Check version increment / Rustfmt / Clippy / Test + coverage all SUCCESS, 0 unresolved threads. It reports BLOCKED only because the PR is a draft, which is correct — leave it draft.

Revert-proof spot-checks are running in a private worktree (C:\tmp\worktrees\sec-248, detached at this head — no shared checkout touched) and will follow.

@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

loop-security — VERDICT: CHANGES-REQUIRED

Head audited: 94f6eb63a40c67e7e2d22136446b9904fca89647 (resolved from remote via gh pr view --json headRefOid; unchanged across the whole audit, re-checked at the end).

Executed probes, in a private worktree

All builds and mutations ran in C:\tmp\worktrees\sec-248, a detached worktree I created at this head. No shared checkout was mutated — the lane's own C:\tmp\worktrees\dignode-2870 was left untouched, and the primary modules/apps/dig-node checkout saw only git fetch and git show. My worktree was restored to a clean tree at this head and removed.

Baseline first: dig-wallet --lib 604 tests green, and all five tests named below pass unmodified at this head.

Revert proof 1 (item 1) — CONFIRMED. Reverted requires_master_token to the pre-fix three-string list:

control::tests::master_token_set_matches_the_contract ... FAILED
left: {"control.pairing.approve", "control.pairing.list", "control.pairing.revoke"}
right: {"control.chiaPeers.add", "control.chiaPeers.remove", "control.pairing.approve",
"control.pairing.list", "control.pairing.revoke"}
control::tests::an_unserved_control_method_requires_the_master_token ... FAILED
assertion failed: requires_master_token("control.notAThing")

It names the two missing methods, as the PR body claims. (Note the second failure differs from the body's table — reverting also breaks the fail-closed default, which the body attributes to reading_the_trusted_peer_list_is_not_master_tier. Both are real; the body's transcript is from a slightly different revert.)

Revert proof 2 (item 5) — CONFIRMED. Dropped WHERE banned = 0 from unbanned_peers(), i.e. the exact wrong fix the split exists to prevent:

sage::db::tests::banned_peers_are_listed_for_the_operator_but_never_dialled ... FAILED
assertion `left == right` failed: a banned peer reached the dialler
left: ["3.3.3.3", "4.4.4.4"]
right: ["3.3.3.3"]

Revert proof 3 (item 4) — CONFIRMED. Made db::add_peer return a constant true instead of reading the row back:

sage::db::tests::adding_a_banned_peer_unbans_it_without_granting_trust ... FAILED
add cleared the ban but left user_managed alone, so the peer is NOT trusted

The test is two-sided (a fresh add must return true, a ban-then-add must return false), so a constant in either direction fails it. Not vacuous.

Two probes of my own, both of which changed a reasoned claim into a measured one

Probe A — the carve-out DOES widen silently (finding 2). I added an unpublished control.chiaPeers.setTrustLevel to CONTROL_METHODS only, exactly as a new shell-owned method would appear, and re-ran the security lockstep tests:

control::tests::an_unserved_control_method_requires_the_master_token ... ok
control::tests::master_token_set_matches_the_contract ... ok
test result: ok. 3 passed; 0 failed

Both pass, while the None => !CONTROL_METHODS.contains(&method) arm makes that method paired-reachable. The comment claiming "the exception cannot quietly widen to cover a future method" is empirically false. Latent, not live — it needs a future code change to bite — so it does not gate, but the comment should not ship as written. Fix: have the None arm read the KNOWN_UNPUBLISHED allowlist that already exists at tests/control_contract_conformance.rs:29.

Probe B — the gating bypass is real, not a reading error. I added a temporary test asserting the control-plane and wallet-plane answers side by side for the same capability:

assert!(crate::control::requires_master_token("control.chiaPeers.add"));assert!(crate::control::requires_master_token("control.chiaPeers.remove"));assert!(authorize("add_peer",Some(PAIRED),MASTER, is_paired));assert!(authorize("remove_peer",Some(PAIRED),MASTER, is_paired));
wallet_authz::tests::audit_probe_paired_token_reaches_add_peer_on_the_wallet_plane ... ok

All four hold at once. The control plane refuses a paired token for chiaPeers.add/.remove; the wallet plane grants that same paired token the same capability, landing on the same network::add_peer / network::remove_peer writer. The probe was removed and the worktree restored clean.

Verdict by area

AreaVerdict
Secrets / credentialsCLEAR — no key, token or credential added, logged or printed; the paired/master tokens keep constant-time comparison and the empty-master fail-closed guard
Custody / privilegeCHANGES-REQUIRED — the master tier is enforced on the control.* plane only; POST /add_peer and POST /remove_peer grant the same custody-grade capability to a paired token on both HTTP and WS
Input / boundaryCLEAR on the control plane — canonical_ip refuses hostnames, bracketed forms, ip:port and blanks before storage. Note the wallet plane applies none of it, so the same bypass also skips the input validation and the ban-key bounding rationale
Crypto / protocolCLEAR — no primitive, downgrade or replay surface touched
AuthZ / exposureCHANGES-REQUIRED (as above). No new anonymous route: is_open_control_read is unchanged and excludes every chiaPeers.* method
Amplification / costCLEAR — the ban list is capped at MAX_BANNED_CHIA_PEERS = 256 with oldest-first eviction, so a caller cannot grow at-rest state without bound. Secondary note: via the wallet-plane bypass a paired token can churn junk bans and evict an operator's real ones, bounded but real
Guard specificityCLEAR on the split — the ban exclusion is in SQL, so no caller-side filter can defeat it, and all_peers() no longer exists as a symbol
Persisted state at restCLEAR — peer rows are bounded, enumerable and correctable; peak_height is read defensively (> 0), so a negative column value cannot wrap into a u32
DependenciesCLEAR — dig-node-control-interface0.17 -> 0.18 only, checksum f444925c… recorded in Cargo.lock, no pin loosened

Can a paired token still reach the escalation? YES — by one route

Plainly stated, because this is the question the gate exists to answer: a paired token can still install a Chia peer that is believed without corroboration and that survives pairing.revoke. Not through control.chiaPeers.add, which is now correctly master-gated on both transports, but through POST /add_peer (and the /ws equivalent) on the Sage-parity wallet plane, where wallet_authz grants it. The capability, the writer and the resulting row are identical; only the URL differs.

That is the finding this PR must close before merge, and it is ranked first in my earlier comment with the full call chain.

Ranked findings

  1. CRITICAL / GATINGcrates/dig-node-service/src/wallet_authz.rs:70-71 with crates/dig-node-service/src/server.rs:245,1337: add_peer / remove_peer are ordinary wallet mutations, so a paired token reaches network::add_peer and network::remove_peer on both HTTP and WS, bypassing the master tier this PR adds. Gate on it. The tier must be a property of the capability, not of the plane the caller picked, and the closing test must assert that every route reaching those two writers is master-gated.
  2. MEDIUM / defense-in-depth, do NOT gatecrates/dig-node-service/src/control.rs:312-317: the None arm exempts anything in CONTROL_METHODS, so a future served-but-unpublished method becomes paired-reachable automatically; measured above. Point it at the existing KNOWN_UNPUBLISHED allowlist and correct the comment on an_unserved_control_method_requires_the_master_token. Follow-up ticket.
  3. LOW / follow-uppeers.peak_height is NOT NULL DEFAULT 0 with the null synthesised at the boundary, so a genuine genesis height will be indistinguishable from unobserved once telemetry lands. Documented at the site; wants a ticket, not a change here.
  4. LOW / note — the Sage-parity get_peers body changes shape (peak_height null, banned added). No ecosystem consumer exists, so the practical risk is a strict third-party client failing at parse time. Avoidable by mapping zero-to-null at the control boundary only.

control.config.setUpstream (#255) was checked as instructed: this PR does not touch it and does not make it worse — CONTROL_METHODS still lists it at the ordinary tier exactly as before, and nothing in the diff changes its reachability.

State left behind

PR remains DRAFT, as instructed. Merge preconditions at this head: all five required checks SUCCESS by name, 0 unresolved threads, BLOCKED only on draft status. My worktree was verified clean at 94f6eb6 and removed; no shared checkout was mutated at any point.

MichaelTaylor3dand others added 4 commits August 19, 2026 15:56
…t plane too
The master tier was enforced on the control plane only, so a paired token
reached the identical writer through the Sage-parity POST /add_peer and its
/ws equivalent: same network::add_peer call, same user_managed row, same
corroboration-free authority surviving pairing.revoke. Only the URL differed.
The tier is a property of the capability, not of the plane it arrived on, so
the wallet gate now resolves the parity aliases to their control names and
reads the tier from the same published contract. A cross-plane test asserts
both policies for one capability in one place, which is the only shape that
can fail on a divergence a per-plane suite cannot see.
Co-Authored-By: Claude <noreply@anthropic.com>
…o null at the boundary
A strict Sage-compatible client deserializes peak_height into a non-optional
integer, so emitting null broke it at parse time regardless of whether it read
the value. The unobserved-vs-genesis distinction is the control surface's to
draw, and it has no third-party consumers, so the mapping moves there and the
parity wire returns to the shape Sage sends.
Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

Gating finding CLOSED — the tier is now a property of the capability, on both planes

Head: 141fa15. Still DRAFT; the re-gate is yours.

1. CRITICAL / GATING — the wallet plane now derives the same tier (4be4d98)

wallet_authz no longer classifies add_peer/remove_peer as ordinary mutations. A new
WalletMethodClass::MasterOnly is resolved through CONTROL_EQUIVALENT_PARITY_METHODS
(add_peercontrol.chiaPeers.add, remove_peercontrol.chiaPeers.remove), and the tier itself
is read from control::requires_master_token — your option 2, not a second string list. authorize
answers ct_eq(tok, master) alone for that class, so both POST /:method (server.rs:1337) and the
/ws fall-through (server.rs:1467) close together: they share one wallet_authz::authorize, which is
why fixing the policy rather than the call sites was sufficient.

The two names stay listed in GATED_WALLET_MUTATIONS deliberately, as a floor: if the contract ever
demotes those capabilities they fall back to master-or-paired rather than out of the gate entirely.

The cross-plane test is the one you asked for, in the shape of your probe B —
the_master_tier_is_a_property_of_the_capability_not_of_the_plane asserts, for each capability in one
loop, that the contract puts it on the master tier AND that the wallet plane refuses a paired token for
its alias. Divergence output:

test wallet_authz::tests::the_master_tier_is_a_property_of_the_capability_not_of_the_plane ... FAILED
panicked at crates\dig-node-service\src\wallet_authz.rs:414:13:
a PAIRED token reached add_peer, which is control.chiaPeers.add by another URL: the escalation is
closed on the control plane and open on the wallet plane

That is the measured output of reverting ONLY the MasterOnly arm of authorize. The fix was committed
first and the revert done by file copy, never git checkout <path>. Exactly one test fails, and it names
the divergence rather than an outcome.

every_master_tier_chia_peer_capability_has_a_gated_parity_alias covers the mapping going stale: a new
master-tier control.chiaPeers.* capability with no parity alias fails there.

2. MEDIUM — the carve-out can no longer widen by itself (4be4d98)

KNOWN_UNPUBLISHED_CONTROL_METHODS is now a pub const in control.rs, and the gate's None arm reads
it instead of !CONTROL_METHODS.contains(&method). control_contract_conformance.rs binds its
KNOWN_UNPUBLISHED to that same constant rather than keeping a copy, so tolerating drift and granting the
ordinary tier are one decision recorded once. The false sentence on
an_unserved_control_method_requires_the_master_token is gone.

A note on the fixture, because my first attempt was a false green. I initially pinned an unpublished
control.chiaPeers.setTrustLevel — but that name is not in CONTROL_METHODS, so it answers "master"
under BOTH the old and the new rule, and the test could not fail on the change. The two rules disagree
only on a name this node SERVES, and control.peers.ping is the only one that exists. So
requires_master_token delegates to requires_master_token_given(method, exempt), and the test asserts
ping twice: master-tier with an EMPTY exemption list (being served grants nothing), ordinary with the
real list. Reverting the None arm to the old form:

test control::tests::a_served_but_unpublished_method_is_not_exempt_unless_it_is_named ... FAILED
being SERVED must grant no exemption: an unpublished method fails CLOSED unless it is named in
KNOWN_UNPUBLISHED_CONTROL_METHODS ...

3. LOW — the parity wire is restored; the null lives at the control boundary (f80ac42)

PeerRecord.peak_height is u32 again, so a strict Sage-compatible client keeps parsing.
control.chiaPeers.list maps zero to null in a new pure chia_peer_wire, tested from BOTH sides
(0null AND 6_000_0006_000_000; the one-sided version would also pass a hard-wired null,
which would hide every real height once telemetry lands). Reverting the mapping:

test control::tests::the_control_list_reports_an_unpolled_peak_as_null_and_a_real_one_verbatim ... FAILED
a peer nobody has polled must not read as one stalled at genesis:
{"ip":"1.2.3.4","port":8444,"peak_height":0,"user_managed":true,"banned":false}

The residual is removed rather than argued about: no ecosystem consumer existed, and now no hypothetical
third-party one breaks either.

4. The three false assertions

All three now describe what is enforced. server.rs says the control gate covers the control plane only
and names the wallet gate carrying the same tier. pairing.rs states the scope is per capability, not per
plane. The revoke doc enumerates the planes its unreachability claim covers — and names the one it does
NOT (below), rather than leaving a sentence the next reader would trust instead of checking.

A third plane, found while verifying the claim — filed, not fixed

Answering "by any route" properly turned up a route neither of us had listed:
dig_wallet::sage::transport.rs:213 mounts POST /:method straight into WalletBackend::dispatch with
no authz gate at all, and server.rs:2032 spawns it on loopback 9257 at every node start. Every
custody, spend and master-tier method is dispatched there on possession of the shared client cert alone.

It is not reachable today: service.rs:299 generates that cert per run and never persists or exposes it,
so nothing can present it. Its own comment anticipates persisting it, and that change alone would make an
ungated wallet surface live. A paired token cannot supply that credential, so it does not reopen the
finding you gated on — which is why it is #257 and not a change in this PR.

The answer to the question the gate exists to ask

No. A paired token cannot reach network::add_peer / network::remove_peer by any route:
control.chiaPeers.add/.remove over HTTP and WS (master tier, unchanged), and add_peer/remove_peer
over HTTP and WS (now master tier). The only other route into that writer is the 9257 transport, which
takes a different credential entirely and is unreachable in the current build.

Blast radius checked

No gitnexus index existed in this worktree, and building one is the failure mode §2.0 bound (1) exists to
prevent, so the radius was taken by grep + direct read and is stated here so it can be checked:
wallet_authz::authorize has exactly 2 call sites (server.rs:1093, server.rs:1328, the latter reached
from :1344 and :1467); requires_authorization 3 (:1086, :1344, :1467); the routes into
WalletBackend::dispatch are the 4 listed above; PeerRecord.peak_height had 2 readers (network.rs
construction, control.rs serialization) plus 2 test sites; no WalletMethodClass match exists outside
the module, so the new variant is additive. add_peer/remove_peer have zero consumers in dig-app,
dig-chrome-extension or dig-sdk, so no paired client loses a capability it was using.

Gates

cargo fmt --all -- --check clean. cargo clippy -p dig-node-service -p dig-wallet --all-targets -- -D warnings clean. cargo test -p dig-node-service green across all targets (384 lib + 5 conformance +
the integration set); cargo test -p dig-wallet --lib 603 passed, 1 ignored.

Version stays 0.127.0: the added public items (KNOWN_UNPUBLISHED_CONTROL_METHODS,
CONTROL_EQUIVALENT_PARITY_METHODS, master_tier_control_equivalent, WalletMethodClass::MasterOnly)
are additive within the minor this PR already takes, and PeerRecord.peak_height returns to the type
released in 0.126.2 — so the net public change versus main is smaller than it was.

SPEC updated in the same unit: §7.12 gains the master-tier wallet-method rule (tier resolved from the
capability, on every transport, pinned by a single cross-plane assertion) and §18.17 records the integer
parity wire plus the tier of add_peer/remove_peer.

@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

loop-security re-gate — interim note 1 (audit in progress)

Head audited: 141fa153bcbddf1b4450c7fb03a7f2963e832e0d (resolved from remote via gh pr view --json headRefOid).

Scope: the escalation fix on the Sage-parity wallet plane, the carve-out fixture, and the door enumeration. Not re-auditing items already cleared in round 1.

Progress so far (no verdict yet):

  1. The MasterOnly fix reads correct.wallet_authz::authorize now branches on classify(method) == MasterOnly and answers ct_eq(tok, master) alone for it, ahead of the master || is_paired arm. The tier is not restated on the wallet plane — master_tier_control_equivalent resolves the alias through CONTROL_EQUIVALENT_PARITY_METHODS and defers to control::requires_master_token, so one rule answers for both planes.

  2. The door enumeration is off by one, in the safe direction.server.rs holds THREE wallet dispatch sites, not two:

    • the JSON-RPC handler's wallet branch (dispatch at server.rs:1117, gate at ~1090),
    • wallet_rpc / POST /{method} (dispatch at server.rs:1363, gate at ~1348),
    • ws_dispatch (dispatch at server.rs:1481, gate at ~1470).

    All three call the same wallet_authz::authorize (two of them via wallet_call_authorized), so the policy fix covers the one the table omitted. This is an enumeration inaccuracy in the PR narrative, not a gap — but the claim "exactly four doors" should read five.

  3. The serve_http mirror is NOT spawned by the node.transport.rs::build_cors_router layers allow_origin(Any) over the same ungated POST /{method} router, which would be reachable cross-origin from any web page the user visits. Grep of the whole tree shows serve_http / serve_dual / build_cors_router have no caller outside transport.rs; server.rs:2039 spawns serve_mtls only. Latent, not live.

Still open: the 9257 cert-ephemerality containment, the cross-plane revert probe, and the control.peers.ping standalone-carve-out verdict.

@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

loop-security re-gate — interim note 2 (head 141fa153bcbddf1b4450c7fb03a7f2963e832e0d)

The fifth door: I looked, and the writer's reachable set is CLOSED

The enumeration was verified by grepping every caller of WalletBackend::dispatch in the tree, not by trusting the table. The complete set:

#Call siteGate
1server.rs:1117 — JSON-RPC handler, wallet branchwallet_authz::authorize (~1090)
2server.rs:1363wallet_rpc, POST /{method}wallet_call_authorized -> authorize (~1348)
3server.rs:1481ws_dispatch, /wswallet_call_authorized -> authorize (~1470)
4transport.rs:159 — the 9257 mTLS listenerNONE (see containment below)

Plus the two control-plane doors (server.rs ~1054 HTTP, ~1430 WS) which reach the writer under the control.chiaPeers.* names via dispatch_control. That is six doors total, not four; the table folded #1 into #2. No ungated door was found.

Two candidate fifth planes were run down and are NOT doors:

  • dig_wallet::wc_dispatch / wallet_dispatch (the dapp + dig-runtime FFI plane) is a separate dispatcher with its own catalogue (chip0002_*, chia_*, dig_*). add_peer is not in it and it never calls WalletBackend::dispatch.
  • dig-node-core/src/chat.rs:281 is MessageRegistry::dispatch, an unrelated chat message-type registry.

The normalization-split bypass does not exist either. In all three server.rs sites the gate and the dispatcher read the samemethod string variable, and dispatch_inner matches with a plain exact-match match method { "add_peer" => ... } — no lowercasing, no trimming, no alias arm. So Add_Peer / add_peer classify as Other, pass the gate ungated, and then fail to match any dispatch arm. It reaches nothing.

The 9257 containment HOLDS — verified, not accepted

transport.rs:159 genuinely has no authz. Four independent checks say it is nonetheless unreachable:

  1. Per-run, in-memory only.service.rs:299 calls SharedCert::generate() (a fresh rcgen self-signed keypair) once per WalletService construction. There is no fs::write of it anywhere in the tree.
  2. No export path.SharedCert::client_identity_pem() — the method whose entire purpose is handing a client the identity — has zero callers; transport.rs:78 is the sole definition. wallet_cert appears only as a struct field (server.rs:102), its construction (:492), and two clones into the listener task (:1990, :2037). It is never serialized, never returned by any RPC.
  3. Cannot leak into a log.SharedCert derives Clone only — no Debug, no Serialize — so it cannot be emitted by a ?cert tracing field.
  4. Loopback + byte-identical client cert.server.rs binds ("127.0.0.1", 9257), and SharedCertVerifier::verify_client_cert accepts a client cert iff its DER equals the server's own.

So reaching it requires a value that exists only in this process's heap. A paired token cannot supply it; an attacker who can read that heap already holds custody. Containment confirmed — dig-node#257 correctly stays out of scope for this PR, and this is not a gating finding.

One residual for #257, not for this PR:transport.rs::build_cors_router layers allow_origin(Any) + allow_headers(Any) over the same ungated router and is served by serve_http/serve_dual. Grep confirms serve_http, serve_dual and build_cors_router have no caller anywhere outside transport.rsserver.rs:2039 spawns serve_mtls only. It is latent, not live. But if it is ever wired, it is strictly worse than the mTLS door: no client cert at all, and a wildcard CORS origin makes it reachable from any web page the user visits. The module doc's "loopback only, so a wildcard origin is safe" is not true of a browser-reachable surface. Worth recording on #257 so a future wiring does not inherit that sentence as an assurance.

@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

loop-security re-gate: PASS

Head audited: 141fa153bcbddf1b4450c7fb03a7f2963e832e0d (resolved from remote with gh pr view 248 --json headRefOid; unchanged for the whole audit).

Scoped re-gate of the round-1 gating finding plus the two secondary items. Items 1-9, the dialler split, corroboration_bypassed and the get_peers wire change were cleared in round 1 and were not re-audited.


1. The escalation is CLOSED on both planes — verified by reverting the fix

wallet_authz::authorize now answers the MasterOnly class with ct_eq(tok, master) alone, in a match arm that precedes the master || is_paired arm, so it cannot fall through. The tier is not restated on the wallet plane: master_tier_control_equivalent resolves the alias through CONTROL_EQUIVALENT_PARITY_METHODS and then filters on requires_master_token(control), so control::requires_master_token — reading the published contract — is the single rule answering for both doors.

The revert probe reproduces exactly as claimed. Deleting only the MasterOnly match arm from wallet_authz.rs and running cargo test -p dig-node-service --lib wallet_authz in a private worktree at this head:

running 12 tests
...
test wallet_authz::tests::the_master_tier_is_a_property_of_the_capability_not_of_the_plane ... FAILED
---- the_master_tier_is_a_property_of_the_capability_not_of_the_plane stdout ----
panicked at crates\dig-node-service\src\wallet_authz.rs:414:13:
a PAIRED token reached add_peer, which is control.chiaPeers.add by another URL: the
escalation is closed on the control plane and open on the wallet plane
test result: FAILED. 11 passed; 1 failed; 0 ignored; 372 filtered out

Exactly one test fails, and it is the one that would have caught the original finding. Its discrimination is real, not asserted.

Worth recording that this test also incidentally closes an ordering hazard I went looking for: classify checks the custody/auth prefixes BEFORE MasterOnly, so a future parity alias landing under wallet. or auth. would silently downgrade from master-only to master-or-paired. The test iterates the whole table asserting classify(parity) == MasterOnly, so that downgrade fails a test rather than shipping.

2. No fifth door — the writer's reachable set is closed

Enumerated by grepping every caller of WalletBackend::dispatch, not by trusting the table. Six doors, all gated:

RouteDispatchGate
control.chiaPeers.add/.remove HTTPdispatch_controlcontrol::requires_master_token (~server.rs:1054)
same, WSdispatch_controlsame (~server.rs:1430)
JSON-RPC wallet branchserver.rs:1117wallet_authz::authorize (~1090)
POST /{method}server.rs:1363wallet_call_authorized (~1348)
/ws wallet branchserver.rs:1481wallet_call_authorized (~1470)
9257 mTLS listenertransport.rs:159none — contained, see 3

The table in the PR narrative says four; it folded the JSON-RPC wallet branch into POST /add_peer. Since all three wallet doors call the same authorize, the policy fix covers the omitted one — an enumeration inaccuracy, not a gap.

Two candidate fifth planes run down and rejected: dig_wallet::wc_dispatch / wallet_dispatch (the dapp + dig-runtime FFI plane) is a separate dispatcher with its own catalogue and never calls WalletBackend::dispatch; dig-node-core/src/chat.rs:281 is MessageRegistry::dispatch, unrelated.

The normalization-split bypass does not exist. In each server.rs site the gate and the dispatcher read the same method variable, and dispatch_inner matches with a plain exact match on the literal "add_peer" — no lowercasing, no trimming, no alias arm. A variant like Add_Peer classifies as Other, passes the gate ungated, then matches no dispatch arm and reaches nothing.

3. The 9257 containment HOLDS

transport.rs:159 genuinely has no authz. Four independent checks make it unreachable:

  1. service.rs:299 calls SharedCert::generate() per WalletService construction. No fs::write of it exists anywhere in the tree.
  2. SharedCert::client_identity_pem() — the one method whose purpose is handing a client the identity — has zero callers. wallet_cert appears only as a field (server.rs:102), its construction (:492) and two clones into the listener task; never serialized, never returned by an RPC.
  3. SharedCert derives Clone only — no Debug, no Serialize — so it cannot leak through a tracing field.
  4. server.rs:2033 binds loopback 127.0.0.1:9257, and SharedCertVerifier accepts a client cert only if its DER equals the server's own.

Reaching it needs a value that exists only in this process heap. A paired token cannot supply it; anyone who can read that heap already holds custody. dig-node#257 correctly stays out of scope, and this is not gating.

One residual for #257, not for this PR:build_cors_router layers a wildcard CORS origin and wildcard headers over the SAME ungated router. serve_http, serve_dual and build_cors_router have no caller outside transport.rs — latent, not live. If ever wired it is strictly worse than the mTLS door (no client cert at all, and a wildcard origin makes it reachable from any web page the user visits), so the module doc sentence "loopback only, so a wildcard origin is safe" should not be inherited as an assurance.

4. The carve-out fixture is genuinely discriminating

requires_master_token_given(method, exempt) takes the exemption list as a parameter, and the fixture asserts control.peers.ping twice — with an empty list it is master tier, with KNOWN_UNPUBLISHED_CONTROL_METHODS it is ordinary. That is the input on which the old and new rules disagree, and the old rule (exemption DERIVED from CONTROL_METHODS membership) could not express the first assertion at all. A drift guard pins the fixture's own premise — that the name is served here and unpublished by the contract — so the test goes red rather than vacuous if the contract ever publishes peers.ping.

The carve-out can no longer widen by itself: KNOWN_UNPUBLISHED_CONTROL_METHODS is an explicit const, so adding a method to CONTROL_METHODS grants no exemption. Round-1 finding 2 is closed.

5. control.peers.ping is acceptable on the ordinary tier

It does take a caller-supplied host:port (peer_ping.rs:48) and dial it, which is a request-forgery shape. It stays ordinary-tier correctly, by this PR's own stated criterion rather than by tolerance:

  • The master tier is "an effect that OUTLIVES the token that invoked it." A ping persists nothing, installs no trust, and is gone the moment pairing.revoke runs. It fails the criterion, so promoting it would be inconsistent with the very rule the PR is enforcing.
  • The gate is on the context, not the shell.MAX_PINGS_PER_WINDOW = 6 per PING_RATE_WINDOW = 60s, plus single-flight, charged on wall time and claimed only after resolution — so a merely-eager second caller cannot burn window budget, and a pausable clock cannot hand out unlimited budget. Any future caller (CLI, app, a dign verb) inherits it.
  • No vantage escalation.server.rs:1966: no listener binds the IPv4 or IPv6 wildcard; every one is loopback. A paired-token holder is therefore a local process whose network vantage already equals the node's, and it can open sockets itself far faster than 6/min. What it gains is dialling under the node's mTLS identity — identity borrowing, bounded at 6/min, not a scanning primitive.

Recommend a follow-up (non-gating): publishing control.peers.ping in the contract would remove the carve-out entirely, since the carve-out exists only because the contract has not published it.

6. Also confirmed

  • PeerRecord.peak_height is u32 at both b92de34 and this head — unchanged. My round-1 LOW note about a null breaking strict Sage clients is moot, and get_peers_response_keeps_the_sage_integer_shape_for_peak_height pins the integer shape AND round-trips into a deliberately non-optional client type.
  • Title and bump are consistent.fix(control)!: with 0.126.2 to 0.127.0. Under 0.x a breaking change is a minor; the version-increment gate is green. Not an oversight.
  • Dependencies clean. The entire Cargo.lock delta is dig-node-control-interface 0.17.0 to 0.18.0 (first-party, crates.io registry, checksum present) plus the workspace version. No new transitive deps, no loosened pin, no git-dep substitution.

Merge preconditions

check-merge-preconditions.sh --repo DIG-Network/dig-node --pr 248, exit code read unpiped: exit 1, RESULT: BLOCKED, solely on draft=true. All five required contexts asserted BY NAME as SUCCESS (Lint commit messages, Check version increment, Rustfmt, Clippy, Test + coverage), unresolvedReviewThreads=0, mergeStateStatus=CLEAN. Left DRAFT as instructed.

Plain statement

A paired token cannot reach network::add_peer or network::remove_peer by any route. All three wallet-plane doors resolve MasterOnly through the contract and refuse it; both control-plane doors already did; the 9257 door is gated by a credential that exists only in the node's heap.

Method disclosure

Read-only against the shared checkout: git fetch origin, git show, git grep, git worktree prune, git worktree add. No checkout/reset/stash/clean, and no edit there. The mutation probe ran in my own worktree at C:\tmp\worktrees\secgate-248, since removed. Shared checkout verified after: HEAD still b92de347e22c59bd8f7f39f6afbcc7e4480c4c44, git status --porcelain shows only the pre-existing untracked .claude/loop/, and the three stash entries are pre-existing and dated (none created by me).

@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review August 20, 2026 00:18
@MichaelTaylor3d
MichaelTaylor3d merged commit 2e73fd0 into mainAug 20, 2026
15 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the loop/2870-chia-trusted-peers branch August 20, 2026 00:19
MichaelTaylor3d added a commit that referenced this pull request Aug 21, 2026
…keep the service stoppable
Three fixes that share `dig-node-service`, plus a measurement that says the fourth ticket needs no
code.
CORS (#702) is now decided per request rather than once for the router. Local web/extension origins
keep the whole surface; desktop-app origins are reflected for content reads only. The discriminator
has to be the method, not the route: `POST /` multiplexes content reads and the open wallet-read
methods onto one JSON-RPC endpoint, and `/{method}` serves the Sage-parity wallet RPC on POST and
content on GET, so a route-keyed decision must answer both traffic classes the same way. That is
what #693 deferred. Every open wallet-read method is reached by POST and every cross-origin content
read is a GET, so the split closes the wallet-read reach with #669 left intact. Preflights are
judged against `Access-Control-Request-Method` so the preflight answer matches the real one, and a
preflight declaring no method fails closed.
The DIG loopback rule (#767) gains a single source of truth in `loopback.rs` and a build-time guard.
The ephemeral content server was still on `127.0.0.1` despite the P0 #745 fix the ticket asks to
confirm; it now takes the DIG address, falling back only where that address cannot be bound at all
and logging when it does. The guard fails the build on a new literal-loopback bind, ignoring test
fixtures and dials, and it immediately found two binds a manual sweep had missed. Three sites keep a
literal address for cross-repo dial-contract or crate-layering reasons and are enumerated in the
guard with their reasons rather than left to memory.
The SCM 1061 wedge (#2880) was neither of its two usual causes: the service does reach RUNNING and
does report a STOP-accepting mask. The stop was bridged into the serve future by
`spawn_blocking(recv)`, putting it on tokio's blocking pool — the same pool the wallet replica's
synchronous database work draws from. With the pool saturated the receiving task never ran, so an
accepted stop was never observed and the service kept serving HTTP while the SCM could not stop it.
That is also why the wedge correlated with the frozen replica: one blocked resource, two symptoms.
The stop is now a watch signal delivered by the runtime itself, and graceful shutdown is bounded, so
a body that will not wind down no longer holds the service RUNNING and a forced stop is reported as
a failed run rather than a clean one.
The trusted-Chia-peer surface (#2870) is already shipped and no code was added: `control.chiaPeers.*`
(PR #248) and `dign chia-peers` (PR #45) already wire the user-facing surface to the existing
`user_managed` writer, and the help text already names the custody grant. No env var or config key
was added on purpose — that would be a second configuration path into write authority.
Closes #767
Closes #702
Closes #2880
Closes #2870
Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d added a commit that referenced this pull request Aug 21, 2026
…keep the service stoppable
Three fixes that share `dig-node-service`, plus a measurement that says the fourth ticket needs no
code.
CORS (#702) is now decided per request rather than once for the router. Local web/extension origins
keep the whole surface; desktop-app origins are reflected for content reads only. The discriminator
has to be the method, not the route: `POST /` multiplexes content reads and the open wallet-read
methods onto one JSON-RPC endpoint, and `/{method}` serves the Sage-parity wallet RPC on POST and
content on GET, so a route-keyed decision must answer both traffic classes the same way. That is
what #693 deferred. Every open wallet-read method is reached by POST and every cross-origin content
read is a GET, so the split closes the wallet-read reach with #669 left intact. Preflights are
judged against `Access-Control-Request-Method` so the preflight answer matches the real one, and a
preflight declaring no method fails closed.
The DIG loopback rule (#767) gains a single source of truth in `loopback.rs` and a build-time guard.
The ephemeral content server was still on `127.0.0.1` despite the P0 #745 fix the ticket asks to
confirm; it now takes the DIG address, falling back only where that address cannot be bound at all
and logging when it does. The guard fails the build on a new literal-loopback bind, ignoring test
fixtures and dials, and it immediately found two binds a manual sweep had missed. Three sites keep a
literal address for cross-repo dial-contract or crate-layering reasons and are enumerated in the
guard with their reasons rather than left to memory.
The SCM 1061 wedge (#2880) was neither of its two usual causes: the service does reach RUNNING and
does report a STOP-accepting mask. The stop was bridged into the serve future by
`spawn_blocking(recv)`, putting it on tokio's blocking pool — the same pool the wallet replica's
synchronous database work draws from. With the pool saturated the receiving task never ran, so an
accepted stop was never observed and the service kept serving HTTP while the SCM could not stop it.
That is also why the wedge correlated with the frozen replica: one blocked resource, two symptoms.
The stop is now a watch signal delivered by the runtime itself, and graceful shutdown is bounded, so
a body that will not wind down no longer holds the service RUNNING and a forced stop is reported as
a failed run rather than a clean one.
The trusted-Chia-peer surface (#2870) is already shipped and no code was added: `control.chiaPeers.*`
(PR #248) and `dign chia-peers` (PR #45) already wire the user-facing surface to the existing
`user_managed` writer, and the help text already names the custody grant. No env var or config key
was added on purpose — that would be a second configuration path into write authority.
Closes #767
Closes #702
Closes #2880
Closes #2870
Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d added a commit that referenced this pull request Aug 21, 2026
…keep the service stoppable
Three fixes that share `dig-node-service`, plus a measurement that says the fourth ticket needs no
code.
CORS (#702) is now decided per request rather than once for the router. Local web/extension origins
keep the whole surface; desktop-app origins are reflected for content reads only. The discriminator
has to be the method, not the route: `POST /` multiplexes content reads and the open wallet-read
methods onto one JSON-RPC endpoint, and `/{method}` serves the Sage-parity wallet RPC on POST and
content on GET, so a route-keyed decision must answer both traffic classes the same way. That is
what #693 deferred. Every open wallet-read method is reached by POST and every cross-origin content
read is a GET, so the split closes the wallet-read reach with #669 left intact. Preflights are
judged against `Access-Control-Request-Method` so the preflight answer matches the real one, and a
preflight declaring no method fails closed.
The DIG loopback rule (#767) gains a single source of truth in `loopback.rs` and a build-time guard.
The ephemeral content server was still on `127.0.0.1` despite the P0 #745 fix the ticket asks to
confirm; it now takes the DIG address, falling back only where that address cannot be bound at all
and logging when it does. The guard fails the build on a new literal-loopback bind, ignoring test
fixtures and dials, and it immediately found two binds a manual sweep had missed. Three sites keep a
literal address for cross-repo dial-contract or crate-layering reasons and are enumerated in the
guard with their reasons rather than left to memory.
The SCM 1061 wedge (#2880) was neither of its two usual causes: the service does reach RUNNING and
does report a STOP-accepting mask. The stop was bridged into the serve future by
`spawn_blocking(recv)`, putting it on tokio's blocking pool — the same pool the wallet replica's
synchronous database work draws from. With the pool saturated the receiving task never ran, so an
accepted stop was never observed and the service kept serving HTTP while the SCM could not stop it.
That is also why the wedge correlated with the frozen replica: one blocked resource, two symptoms.
The stop is now a watch signal delivered by the runtime itself, and graceful shutdown is bounded, so
a body that will not wind down no longer holds the service RUNNING and a forced stop is reported as
a failed run rather than a clean one.
The trusted-Chia-peer surface (#2870) is already shipped and no code was added: `control.chiaPeers.*`
(PR #248) and `dign chia-peers` (PR #45) already wire the user-facing surface to the existing
`user_managed` writer, and the help text already names the custody grant. No env var or config key
was added on purpose — that would be a second configuration path into write authority.
Closes #767
Closes #702
Closes #2880
Closes #2870
Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d added a commit that referenced this pull request Aug 21, 2026
…keep the service stoppable
Three fixes that share `dig-node-service`, plus a measurement that says the fourth ticket needs no
code.
CORS (#702) is now decided per request rather than once for the router. Local web/extension origins
keep the whole surface; desktop-app origins are reflected for content reads only. The discriminator
has to be the method, not the route: `POST /` multiplexes content reads and the open wallet-read
methods onto one JSON-RPC endpoint, and `/{method}` serves the Sage-parity wallet RPC on POST and
content on GET, so a route-keyed decision must answer both traffic classes the same way. That is
what #693 deferred. Every open wallet-read method is reached by POST and every cross-origin content
read is a GET, so the split closes the wallet-read reach with #669 left intact. Preflights are
judged against `Access-Control-Request-Method` so the preflight answer matches the real one, and a
preflight declaring no method fails closed.
The DIG loopback rule (#767) gains a single source of truth in `loopback.rs` and a build-time guard.
The ephemeral content server was still on `127.0.0.1` despite the P0 #745 fix the ticket asks to
confirm; it now takes the DIG address, falling back only where that address cannot be bound at all
and logging when it does. The guard fails the build on a new literal-loopback bind, ignoring test
fixtures and dials, and it immediately found two binds a manual sweep had missed. Three sites keep a
literal address for cross-repo dial-contract or crate-layering reasons and are enumerated in the
guard with their reasons rather than left to memory.
The SCM 1061 wedge (#2880) was neither of its two usual causes: the service does reach RUNNING and
does report a STOP-accepting mask. The stop was bridged into the serve future by
`spawn_blocking(recv)`, putting it on tokio's blocking pool — the same pool the wallet replica's
synchronous database work draws from. With the pool saturated the receiving task never ran, so an
accepted stop was never observed and the service kept serving HTTP while the SCM could not stop it.
That is also why the wedge correlated with the frozen replica: one blocked resource, two symptoms.
The stop is now a watch signal delivered by the runtime itself, and graceful shutdown is bounded, so
a body that will not wind down no longer holds the service RUNNING and a forced stop is reported as
a failed run rather than a clean one.
One defect found by CI and fixed at the call site, not by widening the check that caught it. The new
ephemeral bind first built its URL as `format!("http://{host}:{port}/...")`, which dig-node-core's
`banned_address_patterns` sweep correctly rejects: text concatenation loses the brackets every IPv6
literal needs. The URL is now formatted from the `SocketAddr` itself, whose Display brackets v6 and
leaves v4 alone, and the bind helper returns only the listener so the advertised address can no
longer disagree with the bound one.
The trusted-Chia-peer surface (#2870) is already shipped and no code was added: `control.chiaPeers.*`
(PR #248) and `dign chia-peers` (PR #45) already wire the user-facing surface to the existing
`user_managed` writer, and the help text already names the custody grant. No env var or config key
was added on purpose — that would be a second configuration path into write authority.
Closes #767
Closes #702
Closes #2880
Closes #2870
Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d added a commit that referenced this pull request Aug 21, 2026
…keep the service stoppable (#291)
* feat(node): scope CORS per route+method, hold the DIG loopback rule, keep the service stoppable
Three fixes that share `dig-node-service`, plus a measurement that says the fourth ticket needs no
code.
CORS (#702) is now decided per request rather than once for the router. Local web/extension origins
keep the whole surface; desktop-app origins are reflected for content reads only. The discriminator
has to be the method, not the route: `POST /` multiplexes content reads and the open wallet-read
methods onto one JSON-RPC endpoint, and `/{method}` serves the Sage-parity wallet RPC on POST and
content on GET, so a route-keyed decision must answer both traffic classes the same way. That is
what #693 deferred. Every open wallet-read method is reached by POST and every cross-origin content
read is a GET, so the split closes the wallet-read reach with #669 left intact. Preflights are
judged against `Access-Control-Request-Method` so the preflight answer matches the real one, and a
preflight declaring no method fails closed.
The DIG loopback rule (#767) gains a single source of truth in `loopback.rs` and a build-time guard.
The ephemeral content server was still on `127.0.0.1` despite the P0 #745 fix the ticket asks to
confirm; it now takes the DIG address, falling back only where that address cannot be bound at all
and logging when it does. The guard fails the build on a new literal-loopback bind, ignoring test
fixtures and dials, and it immediately found two binds a manual sweep had missed. Three sites keep a
literal address for cross-repo dial-contract or crate-layering reasons and are enumerated in the
guard with their reasons rather than left to memory.
The SCM 1061 wedge (#2880) was neither of its two usual causes: the service does reach RUNNING and
does report a STOP-accepting mask. The stop was bridged into the serve future by
`spawn_blocking(recv)`, putting it on tokio's blocking pool — the same pool the wallet replica's
synchronous database work draws from. With the pool saturated the receiving task never ran, so an
accepted stop was never observed and the service kept serving HTTP while the SCM could not stop it.
That is also why the wedge correlated with the frozen replica: one blocked resource, two symptoms.
The stop is now a watch signal delivered by the runtime itself, and graceful shutdown is bounded, so
a body that will not wind down no longer holds the service RUNNING and a forced stop is reported as
a failed run rather than a clean one.
One defect found by CI and fixed at the call site, not by widening the check that caught it. The new
ephemeral bind first built its URL as `format!("http://{host}:{port}/...")`, which dig-node-core's
`banned_address_patterns` sweep correctly rejects: text concatenation loses the brackets every IPv6
literal needs. The URL is now formatted from the `SocketAddr` itself, whose Display brackets v6 and
leaves v4 alone, and the bind helper returns only the listener so the advertised address can no
longer disagree with the bound one.
The trusted-Chia-peer surface (#2870) is already shipped and no code was added: `control.chiaPeers.*`
(PR #248) and `dign chia-peers` (PR #45) already wire the user-facing surface to the existing
`user_managed` writer, and the help text already names the custody grant. No env var or config key
was added on purpose — that would be a second configuration path into write authority.
Closes #767
Closes #702
Closes #2880
Closes #2870
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(service): exit on a forced stop instead of blocking in the runtime drop
SEC-1 (gating): the forced-stop branch reported `Stopped` to the SCM and then
returned, dropping the tokio runtime. `Runtime::drop` joins the blocking pool
with no timeout, and a forced stop is by definition the case where a blocking
task never finished, so the drop blocked forever AFTER the SCM had been told
the service was stopped. Measured on the pinned tokio 1.53.0: still blocked
after 8.03s, then 50.19ms once the wedged closure was released.
That is a privileged action reporting success without taking effect: `sc stop`
succeeds, the service is marked stopped, and the process stays alive holding
its binary image locked - the exact symptom dig_ecosystem#2880 exists to
remove. It also leaves an SCM-stopped-but-alive state from which a
`StartService` yields a second process against the same wallet replica.
A forced stop now abandons the pool (`shutdown_background`) and exits. The
graceful branch, the watch-based stop signal and the 20s deadline are
unchanged.
SEC-2 (low): `Access-Control-Allow-Methods` now mirrors the requested method.
tower-http emits it on every answered preflight independent of the origin
verdict, so a static [GET, POST, OPTIONS] answered an approved app-origin GET
preflight by also advertising POST, seeding the browser preflight cache with a
POST entry a later `POST /` could use to skip its preflight.
SEC-3: narrow two overclaiming sentences. The sandboxed-blob comment claimed
origin isolation from "anything else"; cookies ignore port, so the blob shares
the 127.0.0.2 cookie jar with the bare-IP content surface. The SPEC's
app-origin scoping sentence is true of Access-Control-Allow-Origin, not of the
whole response.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d restored the loop/2870-chia-trusted-peers branch August 22, 2026 17:11
Sign up for freeto 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.

SECURITY: adopt dnci 0.18.0 — a paired token can still call chiaPeers.add here, because the master-tier predicate is restated as a string list

1 participant

@MichaelTaylor3d