Skip to content

feat(installer): embed the branded DIG icon in the Windows binary - #69

Merged
MichaelTaylor3d merged 1 commit into
mainfrom
loop/2917-windows-icon
Aug 14, 2026
Merged

feat(installer): embed the branded DIG icon in the Windows binary#69
MichaelTaylor3d merged 1 commit into
mainfrom
loop/2917-windows-icon

Conversation

@MichaelTaylor3d

@MichaelTaylor3d MichaelTaylor3d commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

DO NOT MERGE — gate round in progress. Kept as a draft until the gate verdicts return.

Closes DIG-Network/dig_ecosystem#2917 for the Windows leg of dig-installer. This lane is the
template the four sibling repos copy.

What landed

File Why
assets/dig.ico The canonical DIG icon, vendored verbatim. 10 frames (16/20/24/32/40/48/64/96/128/256), hardened alpha at <=32 so the D's counter does not bleed shut at taskbar size. sha256 2f0fb11a1254fc9275248dc340b7aa9c7236484a9531f8aaad2e4bcdf8900096, 50443 bytes.
assets/dig.rc Declares it as 1 ICON — Windows renders the lowest-ordinal icon resource as the file's icon. Deliberately declares no RT_MANIFEST.
build.rs Extended, not rewritten. New embed_icon() alongside the existing embed_manifest().
Cargo.toml embed-resource = "3" as a cfg(windows) build-dependency.
scripts/check-icon.sh The drift gate: asserts the vendored bytes still hash to the pin.
.github/workflows/ci.yml New icon asset (sha256 pin) job, runs on every PR.
SPEC.md section 5.1 Normative statement of the icon contract.

Why vendored bytes plus a pin

Decided shape: no new repo, no new crate, no shared dependency, no release-first cascade. The asset
changes approximately never, so the cost of sharing it exceeds the cost of drift — provided drift is
made loud. scripts/check-icon.sh is what makes it loud, and it is proven falsifiable below.

The manifest interaction — read this before copying into a sibling repo

The pre-existing build.rs embeds an asInvoker manifest because Windows Installer Detection
auto-elevates binaries whose name contains "install"/"setup", and that auto-elevation makes even
cargo test fail to launch the test harness (ERROR_ELEVATION_REQUIRED, os error 740). That
reasoning is load-bearing and is fully preserved.

Two facts a sibling lane needs:

  1. The manifest is NOT declared in the .rc. It reaches the binary through the linker
    (/MANIFEST:EMBED + /MANIFESTINPUT), and the icon reaches it through a linked .res. They are
    different resource types and coexist — but declaring a manifest in both places is exactly the
    combination where one silently displaces the other. Verified both are present in the linked
    binary (evidence below).
  2. embed_resource::compile links with cargo:rustc-link-arg-bins, not rustc-link-arg. That
    is the deliberate opposite of the manifest, which uses plain rustc-link-arg so it reaches the
    unit-test harness too. A test harness needs no icon, and leaving its link line untouched keeps
    this change entirely off the os-error-740 path. A sibling repo that changes this to
    compile_for_everything is taking on risk it does not need.

The result of compile is checked with .manifest_required() rather than discarded: an environment
without a usable resource compiler would otherwise silently produce an unbranded binary, which is the
exact defect this PR exists to remove.

Blast radius checked

build.rs is the only existing symbol touched. It has no callers in the source graph — Cargo invokes
it — so the blast radius is the linked artifacts of this crate: the dig-installer binary and the
unit-test harness. Both were built and exercised (below). assets/dig.rc, assets/dig.ico,
scripts/check-icon.sh and the CI job are new files with no dependents. No library code, no public
API, and no runtime behaviour changed. Independently confirmed by inspecting the linked PE's resource
directory, which is a stronger check than the call graph for a link-time change.

gitnexus was not used: this repo's worktree has no index, and the change is a build-script edit whose
true dependents are link-time artifacts that a call-graph index does not model. Blast radius was
established by reading build.rs in full plus PE resource enumeration on the built binary.

How verified

1. The drift gate is falsifiable (proven, not asserted).

control:    OK: assets/dig.ico matches the canonical DIG icon (2f0fb11a...)   exit=0

flip byte 40 (XOR 0x01):
            FAIL: assets/dig.ico has drifted from the canonical DIG icon.
              expected sha256: 2f0fb11a1254fc9275248dc340b7aa9c7236484a9531f8aaad2e4bcdf8900096
              actual   sha256: 5a1275a91aedcb1f3ac502de13352e4b51bba994d8044bcbe9ec06f202df0249
                                                                              exit=1

restored:   OK: ...                                                           exit=0

The vendored file's sha was verified against the canonical source immediately after copying, before
any other step.

2. Both resources are in the linked binary. Enumerated via EnumResourceNames on
target/debug/dig-installer.exe:

RT_GROUP_ICON(14): #1        <- ordinal 1, as required for the Explorer icon
RT_ICON(3) count:  10        <- all ten frames survived
RT_MANIFEST(24):   #1        <- the asInvoker manifest is intact

The manifest body was also confirmed present by byte-scanning the image for asInvoker and
requestedExecutionLevel.

3. The binary still runs. dig-installer.exe --version returned dig-installer 0.43.0
(pre-bump build).

4. cargo test still launches — the os-error-740 regression check. cargo test --locked:
851 passed, 1 failed. The single failure is
secure::tests::the_defect_reproduces_on_a_real_directory_and_the_repair_clears_it
(Os { code: 5, PermissionDenied }, an ACL fixture needing elevation). Confirmed pre-existing: it
fails identically on a clean origin/main worktree with none of these changes. Crucially, the harness
launched — an elevation regression would have failed to start at all, with zero tests reported.

5. The icon is visible on the shipped binary. Real Explorer windows over the built exe, captured
at two view modes; the DIG mark renders in both, and the D's counter stays open at the small size.

  • extra-large icons: C:\tmp\icon-evidence\explorer-extra-large-icons.png
  • details view (16px): C:\tmp\icon-evidence\explorer-details-view.png
  • shell-extracted frames at 16/32/48/256: C:\tmp\icon-evidence\shell-extract-*.png

These are real screen captures of Explorer, not renders of the source .ico. They are not committed
to the repo (large binaries in a source tree); paths are on the build host.

6. cargo fmt --check clean, cargo clippy --all-targets --locked -D warnings clean.

Scope — Windows only, explicitly

This PR embeds a Windows resource and nothing else. Deferred to separate tickets and not
implied as done: macOS .icns (bundle-only), Linux .desktop + hicolor PNGs (packaging), and the
icons carried by the generated .msi/.pkg/.deb packages through their own packaging toolchains.

Version

0.43.0 to 0.44.0 (minor). New user-visible capability — the shipped binary is now branded —
with no API, wire-format or behavioural break. Note the ticket brief said this repo was at 0.40.1;
main is actually at 0.43.0, so the bump is from the real tip.

Findings raised, not fixed here

build.rs gates on #[cfg(windows)], which is the host the build script runs on, not the
build target. The cfg(windows) build-dependency resolves the same way, so the new code is
internally consistent with the existing manifest logic and this PR does not change the distinction's
effect. But it means a cross-compile to Windows from a Linux host would embed neither the
manifest nor the icon, silently. Left alone deliberately rather than fixed as a drive-by; worth its
own ticket, and worth flagging to the sibling repos since some of them may cross-compile their
Windows artifacts.

Conflict note

PR #68 (loop/2784-macos-e2e-tls) is open on this repo and will conflict on the Cargo.toml version
line. Whichever merges second rebases. No other file overlaps.

Windows shows the lowest-ordinal icon resource as an executable's icon, so
assets/dig.rc declares the canonical DIG mark as ordinal 1 and build.rs
compiles it via embed-resource into the shipped binaries.

The icon is vendored verbatim rather than shared through a package: it changes
approximately never, so a vendored copy costs no release-first cascade. The
trade is that drift becomes possible, so scripts/check-icon.sh pins its sha256
and CI fails loudly on any re-save.

The existing asInvoker manifest is untouched and still reaches the unit-test
harness: embed-resource links the icon with rustc-link-arg-bins only, so the
os-error-740 elevation path is unaffected. Verified both RT_GROUP_ICON and
RT_MANIFEST are present in the linked binary.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Correctness gate — PASS

Head reviewed: 5c0063899de0db3a6c12e32b737e0dc5610cffd4 (resolved from gh pr view 69 --json headRefOid, not from the dispatch prompt).

The sha-gate non-vacuity answer (asked first, answered concretely)

Non-vacuous. The comparison is scripts/check-icon.sh:27, between:

  • LHS actualscripts/check-icon.sh:25, sha256sum assets/dig.ico (computed from the guarded file), and
  • RHS EXPECTED_SHA256scripts/check-icon.sh:18, a hardcoded literal 2f0fb11a…0096.

The warrant is NOT derived from the thing it guards, so it is not a tautology. Probed by mutation in a throwaway copy under C:\tmp (never the shared checkout):

  • control → OK, exit 0
  • one byte appended to assets/dig.icoFAIL … actual sha256: acd8c7c1…, exit 1
  • file deleted → FAIL: assets/dig.ico is missing., exit 1

The literal also matches the blob actually committed on this head: git show 5c00638:assets/dig.ico | sha256sum = 2f0fb11a1254fc9275248dc340b7aa9c7236484a9531f8aaad2e4bcdf8900096. The job is wired at .github/workflows/ci.yml:56-67, unfiltered (runs on every PR, as SPEC.md §5.1 requires), invoked via bash so a lost exec bit cannot fail it for the wrong reason. It reported icon asset (sha256 pin): SUCCESS on this head.

One MUST-DO before merge — for the orchestrator, not the implementer (no code change possible)

icon asset (sha256 pin) is not in branch protection's required contexts. The required set on main is exactly:

Lint commit messages, Check version increment, rustfmt, clippy, test + coverage (>=80% lines), gui rustfmt, gui clippy, gui build (windows-latest), gui build (macos-14), gui test (ubuntu-latest), gui test (macos-14), gui frontend build

So a future PR that swaps assets/dig.ico turns the job red without blocking merge — the check discriminates, but it is only half a gate, and SPEC.md §5.1 states the pin "MUST run on every pull request" while nothing enforces that it must PASS. Add icon asset (sha256 pin) to the required contexts (branch-protection API, orchestrator-owned) either with this merge or immediately after. Not CHANGES-REQUIRED because the fix is not in this repo's tree, and the four sibling repos copying this template inherit the same omission.

Elevation posture — verified, unchanged

build.rs:37 embeds asInvoker for the root crate's bin and its test harness (/MANIFEST:EMBED via rustc-link-arg, build.rs:51-55). The icon path uses embed_resource::compile (build.rs:73), which links via rustc-link-arg-bins and so does not touch the test harness — deliberate, documented at build.rs:59-70, and it keeps the change off the os-error-740 path. No requireAdministrator is introduced anywhere: the only requireAdministrator in the tree is the pre-existing Tauri GUI (gui/app/src-tauri/build.rs:118, #610), and this diff does not touch that file. Precision note on the PR body's "all artifacts stay asInvoker": true of everything this PR affects, but the GUI artifact was already requireAdministrator before it — worth wording as "unchanged" rather than "all asInvoker" so the template copied by sibling repos does not carry a false statement.

The manifest-vs-icon collision is handled correctly: assets/dig.rc:14-16 deliberately declares no RT_MANIFEST, and 1 ICON "dig.ico" (assets/dig.rc:18) is the sole ICON statement, so the branded mark is the lowest ordinal.

Test vacuity

No tests were added, so there is nothing vacuous to report — and there is genuine machine evidence in its place. embed_icon runs .manifest_required() and .expect(...) (build.rs:74-75), so a host with no usable resource compiler fails the build rather than emitting an unbranded binary. That path is exercised in a required check: gui/app/src-tauri/Cargo.toml:42 takes dig-installer = { path = "../../.." }, so gui build (windows-latest) compiles this build.rs on Windows — SUCCESS on this head. The unproven claim is SPEC.md §5.1's "Both MUST be present in the linked binary": nothing asserts the linked PE actually carries both RT_GROUP_ICON and RT_MANIFEST. Logged below, not gating in a phase-1 posture.

Install-flow behaviour

Packaging-only, as claimed. The diff is assets/dig.ico, assets/dig.rc, build.rs (build-time resource wiring only), Cargo.toml (version 0.43.0 → 0.44.0 + a cfg(windows) build-dependency), SPEC.md, the CI job, and two lockfiles. No install/uninstall/update code path is touched; no runtime dependency added (embed-resource is build-only). Minor is the right bump for a new user-visible capability with no API change.

Readable-code bar

Passes. Intent-revealing names, guard-clause-only control flow, no nesting beyond one level, no dead code, and the comments are genuinely WHY rather than what — build.rs:1-18 explains the auto-elevation/os-error-740 hazard, build.rs:59-70 explains why the icon and the manifest deliberately use different link-arg scopes, assets/dig.rc:3-5 explains the lowest-ordinal rule, and scripts/check-icon.sh:2-14 explains why vendoring plus a loud pin was chosen over a shared package. embed_icon and embed_manifest both carry doc-comments.

Log-and-move tickets (off-path hardening, NOT gating)

  1. Assert the shipped PE actually contains both resource kinds (RT_GROUP_ICON + RT_MANIFEST) — a Windows-only smoke check over the built exe would close the one SPEC.md MUST with no test behind it.
  2. assets/dig.rc currently has one ICON statement by convention only; a mechanical assertion (grep for a second ICON) would make the lowest-ordinal invariant self-guarding, since the sha pin covers the bytes but not the resource script.
  3. Incidental transitive re-resolution in gui/app/src-tauri/Cargo.lock: windows-sys 0.60.2 → 0.61.2 across ~10 packages and getrandom 0.3.4 → 0.4.3 under tempfile, in the dependency tree of the elevated GUI binary. All GUI checks are green and this is a side effect of cargo re-resolving after the new build-dep, not an intentional bump — but an icon PR quietly moving deps in a requireAdministrator artifact's tree is worth a deliberate bump ticket rather than a silent ride-along.

How this was verified

Read every changed blob via git show 5c00638:<path> — the shared checkout was not mutated: no checkout, no reset, no stash, no edit, and the mutation probe ran on copies in a throwaway C:\tmp\icongate-probe that was deleted afterwards. Branch-protection required contexts read from repos/DIG-Network/dig-installer/branches/main/protection/required_status_checks; check conclusions from statusCheckRollup. Elevation swept with git grep -i -E "requireAdministrator|highestAvailable|requestedExecutionLevel" 5c00638 across the whole head, not just the diff.

Verdict: PASS — mergeable once icon asset (sha256 pin) is added to the required contexts (or immediately after, at the orchestrator's call). No findings were handed to @copilot; nothing here needs an implementer.

@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review August 14, 2026 18:37
@MichaelTaylor3d
MichaelTaylor3d merged commit aba89f3 into main Aug 14, 2026
33 of 34 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the loop/2917-windows-icon branch August 14, 2026 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant