Uh oh!
There was an error while loading. Please reload this page.
fix(dig-node): outcome-specific branded-error filename + diagnostic for dign open (#852) - #78
Conversation
Push-early stub to open the draft PR + claim the lane (§1.8). Co-Authored-By: Claude <noreply@anthropic.com>
…or dign open (#852) `dign open` served EVERY fail-closed resolve outcome as an opaque `http://127.0.0.1:<port>/dig-error.png`, so the opened URL revealed nothing about WHICH outcome fired — the exact symptom in #852. Each branded error is now served under an outcome-specific filename (`dig-error-<outcome>.png`, e.g. `dig-error-root_required.png`), and `open` emits a one-line stderr diagnostic naming the resolved URN + outcome (never resolved bytes), so a failing open is diagnosable. This does NOT weaken the #745 fail-closed contract: unverified bytes are still never served, the branded asset is still a static inert PNG, and the untrusted input gate is unchanged. It addresses the diagnosability + URL symptom of #852; the underlying rootless-read failure is cross-repo (see PR body). Regression tests: rootless open → root_required branded asset with its own filename; each branded outcome has a distinct filename. Refs #852 #842. SPEC §8.5 updated. Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
Correctness review — PASS (standalone diagnostic improvement; does NOT close #852/#842/#841/#747)
Reviewed as the correctness gate for the read-leg diagnostic change. This is a small additive diff to the #745 fail-closed branded-error serve path. Verdict: PASS.
#745 fail-closed contract — INTACT
open_brandedstill serves ONLY a static, bundled, inert PNG (images::png(kind)viainclude_bytes!— compiled into the binary, so no runtime 404 and no attacker-controlled bytes) with a hard-codedimage/pngcontent-type, over loopback. Unverified/attacker bytes are never served; the untrusted-input gate is untouched.- The new
branded_filename(kind)label is derived from a CLOSED set of&'static stroutcome tags:error_kind(&ResolveError)(open.rs:505) andResolveOutcome::kind()(dig-urn-resolver/src/resolver.rs:101) both return fixed string literals. The outcome→filename mapping is NOT attacker-influenced, so there is no attacker-chosen extension/type or path-traversal reintroduced. The filename is a virtual URL label; the served payload is unchanged.
stderr diagnostic — leaks nothing (§7)
log_branded (open.rs:426) logs only link.to_urn() (the user's own input URN, reconstructed from a validated DigLink) + the outcome tag. No content bytes, no key material, no secrets.
Branded assets exist
Every ErrorImage variant maps to a committed include_bytes! PNG (images.rs:99); the served filename is synthetic, so distinct filenames sharing one image (e.g. root_required and invalid_urn both render the InvalidUrn PNG) is intended, not a missing-asset bug.
Auto-close check — clean
No Closes/Fixes/Resolves #N keyword in the PR body or either commit. The body references #852 as (#852) and explicitly states #841/#747 + the user-facing rootless read are cross-repo and "NOT closed by this PR." GitHub will not auto-close #852/#842/#841/#747 — correct. Orchestrator should merge WITHOUT adding a close keyword.
§2.1 TDD / §2.5 readable
Two regression tests genuinely pin the behaviour: rootless-over-rpc → root_required distinct filename + opened-URL assertion, and distinct-filename-per-outcome across 3 outcomes. Doc-comments present on both new fns; narrative clean. SPEC §8.5 updated in the same unit. Patch bump 0.51.0→0.51.1 correct (diagnostic, no behaviour/API/format change).
Gates
fmt / clippy / Test+coverage / CodeQL (all 3 analyzers) / version-increment / commitlint all GREEN. build .msi/build .pkg still pending — unrelated packaging jobs, do not block correctness.
No findings. Merge as a standalone diagnostic improvement (do NOT let it close #852/#842/#841/#747).
Summary
Read-leg debugging batch (#836 install→connect→READ). Root-caused #852/#842/#841/#747; shipped the one dig-node-side fix that is genuinely in scope, and precisely located the remaining (cross-repo) root causes.
Root cause — #852 (
dign open"always serves dig-error.png")dign openroutes through the shareddig-urn-resolver§5.3 ladder (correct, #745/#668). Tracing the ladder:rpc.dig.netwhen there is no local node (ladder::build_plan→[Rpc(rpc.dig.net)]), andnode::fetchmaps a local-node 502 (the #747RootError) toTransport→ falls through to rpc. So root-PINNED opens degrade correctly and succeed via rpc (matches #747's "the rpc client-verify path resolves the SAME URN fine").chia://<store>/…, the common case). Over the untrustedrpc.dig.nettier,rpc::fetch→trusted_root()returnsResolveError::RootRequiredby security invariant (a rootless URN's root cannot be taken from the untrusted gateway — it could forge a root and pass verification). The resolver returns that as a hardErr(correctly does NOT fall through), andopenrenders a branded error.RootRequiredwall.Net: which fail-closed outcome fires =
root_required(→ErrorImage::InvalidUrn) for a rootless link, or a fall-throughUnreachable/NotFoundotherwise. The user reported "alwaysdig-error.png" becauseopenserved every branded outcome under the one opaque filenamedig-error.png— the URL revealed nothing about the cause.What this PR fixes (dig-node, in scope)
dig-error-<outcome>.png(e.g.dig-error-root_required.png,dig-error-unreachable.png), so the openedhttp://127.0.0.1:<port>/…URL names WHY the open failed — directly resolving the "always dig-error.png" symptom.openemits a one-line stderr diagnostic (resolved URN + outcome; never resolved bytes) so a failing open is diagnosable.root_requiredbranded asset with its own filename; each branded outcome has a distinct filename. SPEC §8.5 updated.#842 (zero-peer graceful degrade) — largely ALREADY satisfied
dig-node already degrades to the
rpc.dig.nettier-4 upstream with an empty peer set:serve_content_plaintext): tier 1 local → tier 2 peer (returnsNonewith no engine/providers) → tier 3proxy_full_content(rpc.dig.net), unconditional.dign stores pin/ sync (cache_fetch_and_cache→sync_module_from(self.upstream, …)): the §21 whole-store clone is fetched fromrpc.dig.netdirectly — peer-independent. (Gated on a loaded §21 identity, not on peers.)dign open: degrades to rpc for root-pinned URNs as analyzed above.The residual rootless-open failure is NOT a missing peer fallback — it is the
RootRequiredsecurity wall + the broken trusted-root source below.Cross-repo root cause (NOT addressable in dig-node) — #841 / #747, and the real #852 read fix
parse next store: missing child. The local node's anchored-root resolution (serve_content_plaintext→AnchoredRootResolver::anchored_state→digstore_chain::singleton::sync_datastore) walks the store's singleton lineage to the tip. That walk fails atmodules/apps/dig-store/crates/digstore-chain/src/singleton.rs:1573(also:888):DataStore::from_spend(...).map_err(|e| ChainError::Chain(format!("parse next store: {e}"))). dig-node only CONSUMES this (dig-node-core/src/lib.rs:37,1277) — it cannot fix the walk. This is the dig-store repo (active lanes #1350/#1321; #1321 is binding the lineage walk tostore_id). Needed API/behavior: either (a)sync_datastorecompletes the lineage walk without themissing childfailure for these stores, or (b) a digstore-chain API to verify a PINNED root is a real on-chain generation of a store without a full walk-to-tip — which would let dig-node HONOR a pinned root (per #747 hypothesis 1) for the local/spath. Both are digstore-chain-side./s): even the dig-node-side "honor the pinned root" requires a digstore-chain "is-this-root-a-real-generation" check (anti-rollback) that does not exist yet;decide_pincurrently requires the pinned root to equal the freshly-walked tip, so it cannot honor a valid past generation and hard-fails when the walk errors. Blocked on the same digstore-chain gap.dign opensucceed, the resolver should resolve a rootless URN's anchored root via the loopback node tier (loopback-trusted) before hitting the rpcRootRequiredwall, then pin it. That lives indig-urn-resolver(10-primitives), not dig-node, and it too depends on the node's anchored-root resolution (the digstore-chain walk) working.Disposition: #852 diagnosability + URL symptom fixed here; #842 verified already-degrading (tests/analysis); #841/#747 and the user-facing rootless-read fix are cross-repo (digstore-chain #1321 + dig-urn-resolver) — reported for the orchestrator to coordinate release-first, NOT closed by this PR.
Blast radius
open.rsonly:open_branded(+ newbranded_filename/log_branded). No callers outsiderun/run_with; no change to the resolver, the serve path, or the wire. Additive/diagnostic → patch bump0.51.0 → 0.51.1(no content-behavior/API/format change).Gates
cargo fmt --check✓ ·cargo clippy -p dig-node-service --all-targets -D warnings✓ ·cargo test -p dig-node-service --lib✓ 233 passed (incl. 2 new #852 regressions).