feat: embed branded DIG icon in dig-node/dign/dig-wallet (#2917)

[MichaelTaylor3d]

DO NOT MERGE — gate round in progress. Refs #2917.

Work in progress; will update with full verifier output before marking ready.

[MichaelTaylor3d]

gate-sec-dig-node — IN PROGRESS finding (not the verdict)

Item 3 (build-graph confinement) partially fails.

New packages ARE confined to exactly 5 (embed-resource, toml 1.1.5+spec-1.1.0, vswhom,
vswhom-sys, winreg), and a reverse-dependency scan of the full lockfile confirms nothing
outside dig-node-service/dig-wallet's new build-dependencies reaches them.

But diffing the resolved Cargo.lock between the true merge-base (b27180b6, == current
develop tip) and head (6c1a3287) via a fresh clone (not cached API state) shows 14
pre-existing, unrelated packages had their transitive dependency edges silently repointed:

• windows-sys 0.48.0/0.52.0/0.59.0/0.60.2 -> 0.61.2 for: dirs-sys, tempfile, winapi-util,
  rustix, anstyle-query, errno, anstyle-wincon, nu-ansi-term, colored, is-terminal, socket2
• socket2 0.5.10 -> 0.6.5 for: hyper-util, quinn, quinn-udp

This is the exact "unrelated windows-sys/socket2 major bump" class the PR history describes
as already identified and rejected — but it is present in the current head. It reaches
quinn/quinn-udp/hyper-util, which are part of dig-node's actual networking stack
(QUIC/reqwest), not confined to Windows build tooling. Not a demonstrated exploit — plausibly
an artifact of Cargo's resolver unifying semver-compatible ranges once the new
build-dependency subtree entered the graph — but it contradicts the PR's own stated/claimed
scope ("Not a dig-/chia- crate, so no cross-repo version lockstep applies here") and widens
what compiles into dig-node's transport layer without review.

[MichaelTaylor3d]

gate-sec-dig-node — VERDICT: CHANGES-REQUIRED

Head SHA audited: 6c1a3287d1ee6ddd5790f0a551cfb5e6c4d236bb (base develop @ b27180b6,
confirmed via gh api compare and independently via a fresh git clone + git diff).

1. Elevation posture — CLEAR. No RT_MANIFEST/requestedExecutionLevel/asInvoker
string exists anywhere in this repo before or after the diff (repo-wide git grep at head).
assets/dig.rc declares only 1 ICON "dig.ico", no manifest, with an explicit comment
against ever adding one. Verified against upstream embed-resource v3.0.11 source
(nabijaczleweli/rust-embed-resource, tag v3.0.11): .manifest_required() only gates
whether resource compilation must succeed (NotAttempted/Failed -> Err); it has
nothing to do with emitting a Windows application manifest. Icon linking is scoped:
dig-node-service/build.rs calls embed_resource::compile_for(.., &["dig-node","dign"], ..)
(emits cargo:rustc-link-arg-bin=<name>=.. per name — confirmed in upstream source),
correctly excluding the fake_beacon_cli test fixture (3 [[bin]]s total, verified).
dig-wallet/build.rs calls the unscoped compile() (emits rustc-link-arg-bins, reaching
every bin) but dig-wallet has exactly one [[bin]], so it is equivalently scoped. No
elevation-behaviour change is possible from this diff.

2. Dependency-classification repair — CLEAR. Fetched crates/dig-node-service/Cargo.toml
and crates/dig-wallet/Cargo.toml at develop and at head directly (not via the squashed
PR diff) and diffed them: in dig-node-service, the new
[target.'cfg(windows)'.build-dependencies] block sits at true EOF, after [dev-dependencies]
— not mid-table. In dig-wallet it is self-contained (immediately followed by a new
[dependencies] header). Diffed the resolved Cargo.lock dependency-edge lists for both
dig-node-service and dig-wallet packages between develop and head: the only change in
either list is the addition of embed-resource; windows-sys/windows-service remain
exactly where they were (still under [target.'cfg(windows)'.dependencies], confirmed by
file offset). No runtime dependency was reclassified — the repair is complete and correct,
and the same hazard does not recur in dig-wallet.

3. Build-graph confinement — CHANGES-REQUIRED (posted above). New package additions are
confined to exactly 5 (embed-resource, toml 1.1.5+spec-1.1.0, vswhom, vswhom-sys,
winreg), all reachable only from the two new build-dependencies entries (reverse-dependency
scan of the whole lockfile, zero other reachers). But the resolved lockfile also silently
re-points 14 pre-existing packages' windows-sys/socket2 edges to newer major versions
(0.48/0.52/0.59/0.60 -> 0.61.2, 0.5.10 -> 0.6.5), reaching quinn/quinn-udp/hyper-util —
dig-node's actual QUIC/HTTP transport stack, not just Windows build tooling. This is the exact
class of unrelated major-version drag-in the PR's own history describes as already rejected;
it is present in the audited head. Not a demonstrated exploit (both crates are
well-maintained, and this is plausibly a Cargo resolver artifact from adding a build-dependency
subtree with overlapping semver ranges), but it is a live, unacknowledged divergence between
the PR's claimed scope ("no cross-repo version lockstep applies here") and what actually
compiles into the binary that handles untrusted peer connections. Needs either an explicit
re-pin (cargo update -p <pkg> --precise <old-version> for the 14 packages, restoring
develop's selections) or an explicit, reviewed acknowledgment that this bump is intended and
in scope — a human/orchestrator decision, not a rubber stamp.

4. Vendored icon blob — CLEAR. assets/dig.ico: 50443 bytes, sha256
2f0fb11a...8900096, matches scripts/check-icon.sh's pin exactly. Parsed the ICO directory
(10 entries, sizes/dimensions match the .rc comment: 16/20/24/32/40/48/64/96/128/256) and
every one of the 10 frames: each is a valid PNG whose IEND chunk ends exactly at the frame's
declared size (zero trailing bytes per frame), zero gaps between consecutive frames, and zero
trailing bytes after the last frame (28654 + 21789 = 50443, exact). Enumerated every PNG chunk
type across all 10 frames: only IHDR/IDAT/IEND appear — no ancillary/metadata chunks, no
polyglot tail, no slack space for a hidden payload.

Scope audited: all 9 changed files in PR #589 (.github/workflows/ci.yml, Cargo.lock,
assets/dig.ico, assets/dig.rc, crates/dig-node-service/{Cargo.toml,build.rs},
crates/dig-wallet/{Cargo.toml,build.rs}, scripts/check-icon.sh); the new CI icon job
(no pull_request_target, no secrets, persist-credentials: false); upstream embed-resource
v3.0.11 source for compile/compile_for/manifest_required semantics.

Not covered: an actual Windows build/link to inspect requestedExecutionLevel on the
produced PE binary directly (no Windows build environment available in this read-only audit;
posture was instead established by proving no manifest resource exists or can be silently
added anywhere in the diff/repo, which is the load-bearing property). dig-installer's own
elevation manifest (separate repo, out of scope for this PR).

Filesystem note: Read-only throughout. Did not touch D:\worktrees\dig-node-2917. Used a
disposable blobless clone under %TEMP%\...\scratchpad\verify-clone (this session's own
scratchpad, not a shared worktree) purely to get independent git diff/git grep evidence
against the actual commit objects rather than trusting cached API/diff output; nothing was
pushed or mutated.

[MichaelTaylor3d]

Orchestrator decision on the security gate's CHANGES-REQUIRED: the widened lockfile is ACCEPTED, explicitly, and the dependency change is being filed separately

The security gate was right to stop this, and the check it used is now the standard for the rest of the rollout. Recording the decision and the evidence it rests on, because "we looked and it was fine" is not something a future reader can audit.

The finding stands

Adding embed-resource rewrites 15 dependency edges on pre-existing packages:

-  "socket2 0.5.10",      x3      +  "socket2 0.6.5",        x3
-  "windows-sys 0.48.0",  x2      +  "windows-sys 0.59.0",   x1
-  "windows-sys 0.59.0",  x7      +  "windows-sys 0.61.2",  x12
-  "windows-sys 0.60.2",  x3

reaching quinn, quinn-udp and hyper-util — this repo's transport stack. No [[package]] block is added or removed for any of them, which is why the grep '^[+-]name' check everyone (including me) was using reported "5 added packages, clean". That check is blind to edge rewrites by construction and has been retired.

Why it is being accepted rather than re-pinned

A lane was sent to determine whether a minimal lock was achievable, with instructions to report honestly if it was not rather than force it. The result is decisive:

• Cargo.lock reset to pristine origin/develop, then the textbook-minimal cargo add embed-resource@3 --build --target 'cfg(windows)' — the output is byte-for-byte identical to the committed lock, zero lines different.
• cargo generate-lockfile was strictly worse (387-line churn), ruling out "the lane regenerated the whole lock" as the mechanism.
• The confirming detail: the newly added winreg keeps its own windows-sys 0.59.0 edge unrewritten. Only pre-existing consumers with wider version requirements were unified onto a version already present in the lockfile.

So this PR did not drag anything in. develop's lockfile was already not a minimal resolution for that family, and any legitimate resolve touching the region refreshes those edges. The narrower diff is reachable only by hand-pinning the lockfile, and a hand-pinned lock is worse than an honest wide one: it misrepresents what the resolver actually produces, and it decays silently the next time anyone adds a dependency.

Note also that both windows-sys 0.61.2 and socket2 0.6.5 were already present in develop's lockfile before this PR. Nothing new is being vendored; what changes is which of the already-vendored versions the transport crates link against. All 14 checks are green at this SHA, including every platform's build and test job and build .msi (windows-x64).

What is NOT being swept under the icon

The consolidation is a real change to what the transport stack compiles against, and it does not belong to a packaging ticket even when it is unavoidable inside one. It is being filed as its own ticket so it is visible, attributable, and re-checkable — rather than discovered a month from now by someone bisecting a connectivity regression into a PR titled "embed the branded DIG icon".

The gate's other three areas were CLEAR: elevation posture (no manifest anywhere, before or after; compile_for(["dig-node","dign"]) correctly excludes the fake_beacon_cli test fixture), the TOML mid-table repair, and the vendored icon blob (50443 bytes, 10 valid PNG frames, zero gaps, zero trailing bytes).

[MichaelTaylor3d]

Gate review — PASS (with one note) at head 6c1a3287d1ee6ddd5790f0a551cfb5e6c4d236bb

Fresh-context static review. No cargo build performed here — the orchestrator is
supplying the PE-resource-verifier output (verify_pe_icon.py against real
dig-node/dign/dig-wallet binaries, including proof the fake_beacon_cli
exclusion holds) separately on this PR. That artifact-level evidence is not
yet posted as of this review; treat this PASS as conditional on it landing
before merge.

Checked, one by one

1. Non-vacuous gate — mutation-proved off-PR: fetched scripts/check-icon.sh
   and the committed assets/dig.ico at this head SHA into a scratch dir.
   Baseline exit 0; append one byte to dig.ico -> exit 1 (hash mismatch printed);
   restore -> exit 0; rename the file away -> exit 1 ("is missing"). Non-vacuous.
2. Canonical bytes — fetched assets/dig.ico from the repo at this head SHA:
   50443 bytes, sha256 2f0fb11a1254fc9275248dc340b7aa9c7236484a9531f8aaad2e4bcdf8900096.
   Matches the literal exactly and matches the reference file byte-for-byte.
3. assets/dig.rc — exactly one ICON statement (1 ICON "dig.ico", ordinal
   1), no RT_MANIFEST declared. Confirmed by direct fetch + grep, not trust.
4. Scoping — dig-node-service/build.rs uses
   embed_resource::compile_for("../../assets/dig.rc", &["dig-node", "dign"], ...),
   scoping the link line to those two bins. Cargo.toml lists a third bin,
   fake_beacon_cli, which is not in that list — excluded as claimed.
   dig-wallet/build.rs uses the unscoped embed_resource::compile, correct
   because dig-wallet's Cargo.toml declares exactly one [[bin]].
5. No behaviour change — dig-node-service/build.rs's pre-existing
   DIG_NODE_GIT_SHA emission (cargo:rustc-env=DIG_NODE_GIT_SHA={sha}, read by
   GET /version) is byte-identical to before; the new code is additive
   (#[cfg(windows)] embed_icon()) and gated to Windows only. ci.yml's diff is
   purely additive (new icon: job appended, nothing else touched).
6. CI job name — name: icon asset (sha256 pin) in ci.yml, and gh pr checks shows the live job as exactly icon asset (sha256 pin) — pass, 5s.
   Exact string match, no near-miss.
7. Cargo.lock — diff is embed-resource 3.0.11 + its own direct deps
   (toml 1.1.5+spec-1.1.0, vswhom, vswhom-sys, winreg) plus the
   windows-sys/socket2 edge-consolidation on pre-existing quinn/
   hyper-util packages already settled and filed separately per the brief —
   not re-litigated here. No unrelated package appears in the lock diff.
8. Doc-truth — PR body is currently DO NOT MERGE — gate round in progress
   (still draft, correctly not claiming completion). No SPEC/README files are
   touched by this PR. New code comments describe only this repo's own
   scoping/exclusion decisions; the "every sibling repo that vendors the same
   bytes" language in dig.rc/check-icon.sh describes the shared shape
   (already proven end-to-end by dig-installer#69), not a false present-tense
   claim that sibling repos have landed. No finding.

TOML mid-splice repair (prior finding) — verified complete

crates/dig-node-service/Cargo.toml: fetched this repo's [target.'cfg(windows)'.dependencies]
table content at this head SHA and diffed it byte-for-byte against the same
table on origin/develop — identical, zero diff. The new
[target.'cfg(windows)'.build-dependencies] block sits at end-of-file (line
400), after [dev-dependencies], not spliced into the existing table.
crates/dig-wallet/Cargo.toml checked for the same hazard: the new
[target.'cfg(windows)'.build-dependencies] block is inserted between [[bin]]
(ends at path = "src/main.rs") and [dependencies] — a clean table boundary,
not mid-table. Its own pre-existing [target.'cfg(windows)'.dependencies] table
is also byte-identical vs develop. No splice hazard in either file.

Lockfile consolidation (prior finding) — not re-litigated

Per the brief, this was proven reproducible from a pristine develop lock with
the minimal cargo add, and filed as its own ticket. Accepted as-is.

Not run

• No cargo build in this review (explicitly out of scope per this round's
  instructions) — the PE-resource verification is being supplied separately by
  the orchestrator and must land on this PR before merge.
• No live Windows MSI artifact inspection beyond the green build .msi (windows-x64) check already on the PR.

Threads

Zero open review threads existed before this review; zero findings raised, so
zero threads opened. Nothing to resolve.

Verdict: PASS at 6c1a3287d1ee6ddd5790f0a551cfb5e6c4d236bb, conditional on
the orchestrator's PE-resource-verifier output landing on this PR before it
leaves draft / merges.

[MichaelTaylor3d]

PASS
head SHA: 6c1a3287d1ee6ddd5790f0a551cfb5e6c4d236bb (base develop)

Re-audited fresh, read-only, no build (artifact/PE-resource evidence supplied separately by the orchestrator).

Lockfile-widening finding (previously CHANGES-REQUIRED) — independently re-verified, verdict changes to accepted:

• Diffed pr589 vs merge-base (b27180b6) directly: 12x windows-sys edges rewritten (2x 0.48.0, 7x 0.59.0, 3x 0.60.2 -> 0.61.2) and 3x socket2 edges rewritten (0.5.10 -> 0.6.5, reaching quinn/quinn-udp/hyper-util) — exact match to the prior audit's count.
• Confirmed both windows-sys 0.61.2 and socket2 0.6.5 package blocks already existed in develop's lock (checked at merge-base b27180b6, Cargo.lock/Cargo.toml unchanged between merge-base and current develop tip) — nothing new is vendored by version, only re-pointing of existing edges.
• Zero [[package]] blocks removed. Only 5 new packages added: embed-resource, toml 1.1.5+spec-1.1.0, vswhom, vswhom-sys, winreg — all embed-resource's own transitive build-time deps.
• Confirmed windows-sys 0.48.0/0.52.0/0.59.0/0.60.2 blocks are all still present post-change (still used elsewhere, e.g. digstore-remote, rustix stay on 0.59.0) — this is a targeted consolidation of specific consumers, not a version floor bump.
• Confirmed the new winreg dependency keeps its own unrewritten windows-sys 0.59.0 edge, matching the debugger's finding that only pre-existing wide-range consumers were unified.
• Did not re-run the cargo add embed-resource@3 --build --target 'cfg(windows)' resolver reproduction myself — my scratch worktree's git metadata hit a busy index.lock under contention from other lanes' live cargo/git processes on this shared machine, and I chose not to force past it. I did not need it: the diff-based check above independently establishes the same facts the reproduction was offered to support (no new vendored versions, only edge consolidation, winreg unrewritten), so I'm not relying on the debugger's number alone.
• Orchestrator's decision (accept + file consolidation as its own ticket) is reasonable and matches what I can verify; endorsing it.

Elevation posture — CLEAR (re-confirmed fresh).
git grep -n "rustc-link-arg|MANIFEST|requestedExecutionLevel|winres|.manifest" across the whole repo at pr589 finds no manifest resource, no RT_MANIFEST, no /MANIFEST linker arg anywhere in the tree, before or after this PR — assets/dig.rc explicitly documents why (avoiding CVT1100/LNK1123 dual-declaration). crates/dig-node-service/build.rs:33-38 uses embed_resource::compile_for("../../assets/dig.rc", &["dig-node","dign"], embed_resource::NONE) — scoped link line, fake_beacon_cli excluded from the file list entirely (not present in the PR's changed-files list, confirmed via gh pr view --json files). crates/dig-wallet/build.rs:24-27 uses unscoped embed_resource::compile, which is safe because that crate has exactly one bin. embed_resource::NONE in both call sites means no manifest is generated by the icon path either.

TOML mid-table repair — CLEAR.
crates/dig-node-service/Cargo.toml:396-399 and crates/dig-wallet/Cargo.toml:16-19 both add [target.'cfg(windows)'.build-dependencies] after the existing [dependencies]/[[bin]] tables with a clear preceding comment; both are syntactically valid table headers, not appended keys inside a wrong table (visually confirmed against the raw diff).

Vendored icon blob — CLEAR (independently re-parsed, not trusted from the PR body).
assets/dig.ico at pr589: 50443 bytes, sha256 2f0fb1...0096 — exact match to the pin in scripts/check-icon.sh and to the ticket's reference hash. Parsed the ICONDIR + 10 ICONDIRENTRY records myself: entries cover offsets 166..50443 contiguously (zero gaps, zero overlaps), directory header ends exactly where frame 0 starts, last frame ends exactly at EOF (0 trailing bytes). All 10 frames are valid PNGs with 0 bytes after their IEND chunk. No slack, no polyglot tail.

CI job — CLEAR.
.github/workflows/ci.yml: new icon job's only step is run: bash scripts/check-icon.sh (no ${{ }} interpolation inside the run block), actions/checkout@v4 with persist-credentials: false, and the job inherits the workflow-level permissions: contents: read (no broader override). scripts/check-icon.sh itself only reads a hardcoded path and computes a hash — no injectable input.

Pin-strength honesty — CLEAR.
scripts/check-icon.sh comments describe it accurately as making "drift LOUD" against accidental re-save/re-export, and the brief's framing (accidental-drift detector, not a tamper-integrity anchor against a malicious committer) is not contradicted anywhere in the code or its comments — no overclaim to flag.

Scope audited: full PR diff (.github/workflows/ci.yml, Cargo.lock, assets/dig.ico, assets/dig.rc, crates/dig-node-service/{Cargo.toml,build.rs}, crates/dig-wallet/{Cargo.toml,build.rs}, scripts/check-icon.sh) at head 6c1a3287d1ee6ddd5790f0a551cfb5e6c4d236bb, diffed against merge-base b27180b6185729809323433517fff13806de4ce4.
Not covered: the cargo add resolver reproduction (blocked by a busy lock under machine load, not forced); any compiled-binary verification (deferred to the orchestrator's PE-resource check per brief).

Worktrees created (mine, not the live build): D:\worktrees\audit-2917-dig-node (clean) and D:\worktrees\audit-2917-repro (a git worktree add there is stuck on a busy index.lock held by another process on this shared machine — I did not force-remove it or otherwise touch D:\worktrees\dig-node-2917).