diff --git a/.github/actions/setup-linux-build/action.yml b/.github/actions/setup-linux-build/action.yml new file mode 100644 index 0000000..4ca379d --- /dev/null +++ b/.github/actions/setup-linux-build/action.yml @@ -0,0 +1,85 @@ +# Prepares a bare old-glibc container to build the dig-node Linux artifacts, and DECLARES the +# canonical glibc floor those artifacts must honour. +# +# WHY (#1736): a glibc-linked binary runs on its BUILD glibc and anything newer, never anything +# older — so the builder image, and nothing else, sets the supported-OS floor. dig-node v0.64.0 +# silently began requiring glibc >= 2.38 (breaking Ubuntu 22.04 LTS, Debian 12 and Amazon Linux +# 2023) purely because GitHub repointed `ubuntu-latest` at Ubuntu 24.04. Every job that produces a +# shipped Linux artifact — the release binaries AND the apt `.deb` — therefore runs inside a PINNED +# container and calls this action, so there is one floor rather than one per workflow. +# +# The job must declare `container: debian:11`; this action asserts that the container it is running +# in really provides `glibc-floor`, so the image and the number cannot drift apart. +# +# Because this is a LOCAL composite action it can only run after `actions/checkout`, and checkout in +# a bare container needs `git` + `curl` + CA certificates already present — so each calling job +# bootstraps exactly those three packages first, then checks out, then calls this action. +name: Set up an old-glibc Linux build +description: Install the Rust + C toolchains in the pinned old-glibc container and assert its glibc matches the canonical floor. + +outputs: + glibc-floor: + description: "The canonical minimum glibc version every published dig-node Linux artifact must run on." + value: ${{ steps.floor.outputs.version }} + +runs: + using: composite + steps: + - name: Declare the canonical glibc floor + id: floor + shell: bash + # THE single source of truth for the floor. 2.31 (Debian 11) clears every distro #1736 + # reported broken — Ubuntu 22.04 (2.35), Debian 12 (2.36), Amazon Linux 2023 (2.34) — plus + # Ubuntu 20.04 (2.31), Debian 11 (2.31) and RHEL 9 (2.34). Raising it is a DELIBERATE act: + # change this value, the `container:` image in every calling job, SPEC.md §11.3 and the + # published docs together — never one alone. + run: echo "version=2.31" >>"$GITHUB_OUTPUT" + + - name: Install the Rust + C toolchains + shell: bash + # The container is a bare Debian 11 with neither toolchain. The C toolchain is not optional: + # `openssl-sys` builds vendored OpenSSL from source (needs perl + make) and several other + # -sys crates in the Chia-wallet-SDK graph want cmake/clang. `binutils` provides the + # `readelf` the floor gate reads. + run: | + set -eux + apt-get update -qq + apt-get install -y --no-install-recommends \ + ca-certificates curl git build-essential pkg-config perl make cmake clang binutils + + - name: Verify the container glibc IS the declared floor + shell: bash + run: | + set -eu + # NO PIPE. `ldd --version | head -n1` is a SIGPIPE race: `head` closes the pipe after the + # first line, `ldd` dies with SIGPIPE, and under `pipefail` the step fails with exit 141 + # — intermittently, depending on whether ldd had already finished writing. It passed twice + # before failing once. Read the output into a variable and slice it with shell expansion. + ldd_output="$(ldd --version 2>&1)" + IFS= read -r first_line <<<"$ldd_output" + # glibc's banner ends with the bare version, e.g. "ldd (Debian GLIBC 2.31-13) 2.31". + actual="${first_line##* }" + declared="${{ steps.floor.outputs.version }}" + case "$actual" in + [0-9]*.[0-9]*) ;; + *) + echo "::error::could not parse a glibc version out of: $first_line" + exit 1 + ;; + esac + echo "container glibc: $actual, declared floor: $declared" + test "$actual" = "$declared" || { + echo "::error::build container glibc $actual != the declared floor $declared. Update the floor in .github/actions/setup-linux-build/action.yml, every calling job's container: image, SPEC.md §11.3 and the docs TOGETHER." + exit 1 + } + + - name: Install Rust via rustup + shell: bash + # rustup, not Debian 11's rustc, which is far older than this workspace needs. Only the Rust + # toolchain comes from rustup — the glibc it links against is the container's, which is the + # entire point of the container. + run: | + set -eu + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs \ + | sh -s -- -y --default-toolchain stable --profile minimal --no-modify-path + echo "$HOME/.cargo/bin" >>"$GITHUB_PATH" diff --git a/.github/actions/stage-binaries/action.yml b/.github/actions/stage-binaries/action.yml new file mode 100644 index 0000000..a5826f9 --- /dev/null +++ b/.github/actions/stage-binaries/action.yml @@ -0,0 +1,53 @@ +# Stages the two release binaries under their CANONICAL asset names (SPEC §11.2) and uploads them. +# +# WHY this is a composite action rather than an inline step: the binaries are produced by TWO jobs +# in build-binaries.yml — the Windows/macOS matrix on its host runners, and the Linux matrix inside +# a pinned old-glibc container (#1736) — and the ONE thing that must never diverge between them is +# the asset naming every downstream consumer resolves (the dig-installer thin-shim, apt.dig.net's +# packaging template). Naming lives here, once. +name: Stage release binaries +description: Copy the built dig-node + dign binaries to dist/ under their canonical asset names and upload them. + +inputs: + version: + description: "Version string stamped into each artifact filename (e.g. `1.2.3` or `1.2.3-nightly.20260714.abc1234`)." + required: true + target: + description: "Rust target triple the binaries were built for (selects the target//release dir)." + required: true + out-name: + description: "Canonical OS/arch asset suffix, e.g. `linux-x64`, `linux-arm64`, `windows-x64.exe`." + required: true + exe-suffix: + description: "Executable suffix of the built binary — `.exe` on Windows, empty elsewhere." + required: false + default: "" + +runs: + using: composite + steps: + - name: Stage artifact (dual-named) + shell: bash + run: | + set -euo pipefail + VER="${{ inputs.version }}" + SUFFIX="${{ inputs.exe-suffix }}" + mkdir -p dist + # `dig-node` is the service binary; `dign` is its FIRST-CLASS alias (issue #548) — a + # separate [[bin]] over the same entrypoint, published under its own stem so the + # installer can resolve it via Repo::dign(). Both are required; a missing one is a + # release-shaped failure, so fail loudly rather than publishing half the pair. + for stem in dig-node dign; do + src="target/${{ inputs.target }}/release/${stem}${SUFFIX}" + test -f "$src" || { echo "::error::binary not produced: $src"; exit 1; } + # out-name already carries the `.exe` on Windows, so it is never appended twice. + cp "$src" "dist/${stem}-${VER}-${{ inputs.out-name }}" + done + ls -la dist + + - name: Upload build artifact + uses: actions/upload-artifact@v4 + with: + name: dig-node-${{ inputs.out-name }} + path: dist/* + if-no-files-found: error diff --git a/.github/workflows/build-binaries.yml b/.github/workflows/build-binaries.yml index 5cc953f..bab639c 100644 --- a/.github/workflows/build-binaries.yml +++ b/.github/workflows/build-binaries.yml @@ -21,9 +21,21 @@ # `dign` is a FIRST-CLASS alias binary (issue #548): a SEPARATE `[[bin]]` target sharing the SAME # entrypoint, published alongside `dig-node` under its own stem `dign---[.exe]`. # -# NO linux-arm64 asset is published: the Linux build graph pulls `openssl-sys` (via the Chia wallet -# SDK), so an aarch64-Linux cross-compile would also have to cross-compile OpenSSL — fragile, and -# no consumer requests it (SPEC §11.3). +# THE LINUX TARGETS BUILD INSIDE A PINNED OLD-GLIBC CONTAINER (`debian:11`, glibc 2.31), in their +# own `build-linux` job — NOT on the runner image (#1736). A glibc-linked binary runs on the build +# glibc and anything NEWER, never anything older, so the builder image silently dictates the +# supported-OS floor. When GitHub moved `ubuntu-latest` to Ubuntu 24.04 the v0.64.0 `linux-x64` +# binary began requiring glibc >= 2.38 and stopped starting on Ubuntu 22.04 LTS (2.35), Debian 12 +# (2.36) and Amazon Linux 2023 (2.34) — the three most common server distros — with no code change +# and no signal. Pinning the container makes the floor an explicit, reviewable decision, and +# `scripts/check-glibc-floor.sh` FAILS the build if a produced binary exceeds it (#1741 covers the +# arch half of the same problem). +# +# `linux-arm64` IS published (#1741): it builds NATIVELY on the free public-repo `ubuntu-24.04-arm` +# runner, so the vendored `openssl-sys` in the Chia-wallet-SDK graph is compiled by a native +# toolchain. The old SPEC §11.3 rationale for omitting arm64 was about CROSS-compiling OpenSSL; +# a native arm64 runner removes that objection entirely. Graviton/Ampere/Pi hosts are exactly the +# always-on machines a volunteer node runs on. # # The gate runs fmt + clippy ONLY — a PR's own CI (ci.yml) already ran the full # fmt/clippy/test/coverage set (`cargo llvm-cov nextest --workspace`) against the merged tree @@ -83,6 +95,9 @@ jobs: - run: cargo fmt --all --check - run: cargo clippy --all-targets --locked -- -D warnings + # Windows + macOS build on their HOST runner images. Neither has a glibc floor to defend: the + # Windows binary targets the MSVC CRT and macOS pins its floor via the SDK/deployment target, so + # only the Linux targets need the pinned-container treatment (`build-linux` below). build: name: build (${{ matrix.out_name }}) needs: check @@ -92,23 +107,17 @@ jobs: include: - os: windows-latest target: x86_64-pc-windows-msvc - bin: dig-node.exe + exe_suffix: ".exe" out_name: windows-x64.exe - - os: ubuntu-latest - target: x86_64-unknown-linux-gnu - bin: dig-node - out_name: linux-x64 # Both macOS arches build on the fast Apple-silicon macos-14 runner; the Intel binary is # CROSS-COMPILED there (host arm64 → target x86_64-apple-darwin). The only C/asm dep in # the macOS graph is `ring` (via rustls); `openssl-sys` is cfg-gated to non-Apple targets, # so there is no vendored-openssl cross-compile to worry about. - os: macos-14 # Apple silicon (arm64) target: aarch64-apple-darwin - bin: dig-node out_name: macos-arm64 - os: macos-14 # Apple silicon — cross-compiles the Intel binary target: x86_64-apple-darwin - bin: dig-node out_name: macos-x64 runs-on: ${{ matrix.os }} steps: @@ -136,32 +145,147 @@ jobs: # Build BOTH bins — `dign` is the first-class alias (issue #548), published alongside # `dig-node` under its own asset stem below. run: cargo build --release --locked --target ${{ matrix.target }} --bin dig-node --bin dign - - name: Stage artifact (dual-named) - id: stage - # The caller's `version` is stamped into the filename verbatim: a plain `1.2.3` for a - # stable tag, or the synthesized `1.2.3-nightly.YYYYMMDD.` for a nightly. - shell: bash + # The caller's `version` is stamped into the filename verbatim: a plain `1.2.3` for a stable + # tag, or the synthesized `1.2.3-nightly.YYYYMMDD.` for a nightly. Naming lives in + # the composite action so this job and `build-linux` can never diverge on it. + - uses: ./.github/actions/stage-binaries + with: + version: ${{ inputs.version }} + target: ${{ matrix.target }} + out-name: ${{ matrix.out_name }} + exe-suffix: ${{ matrix.exe_suffix }} + + # The LINUX targets, built NATIVELY inside a pinned old-glibc container so the supported-OS floor + # is a decision rather than a side effect of whatever image `ubuntu-latest` points at this month + # (#1736 — see the header). aarch64 builds on the free public-repo arm64 runner (#1741), which + # means the vendored `openssl-sys` in the Chia-wallet-SDK graph is never cross-compiled. + build-linux: + name: build (${{ matrix.out_name }}) + needs: check + strategy: + fail-fast: false + matrix: + include: + - runner: ubuntu-latest + target: x86_64-unknown-linux-gnu + out_name: linux-x64 + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-gnu + out_name: linux-arm64 + runs-on: ${{ matrix.runner }} + # Debian 11 = glibc 2.31 = the floor declared by `.github/actions/setup-linux-build`, which + # asserts the two agree — so bumping this image without bumping the floor fails the build + # instead of silently shipping a higher floor. + container: debian:11 + steps: + # Bootstrap only what `actions/checkout` itself needs in a bare container; the full toolchain + # (and the floor assertion) comes from the local composite action, which cannot run until the + # repository is on disk. + - name: Bootstrap the container for checkout + run: | + set -eux + apt-get update -qq + apt-get install -y --no-install-recommends ca-certificates curl git + - uses: actions/checkout@v4 + with: + ref: ${{ inputs.ref }} + persist-credentials: false + - name: Trust the checkout (git runs as root in the container) + run: git config --global --add safe.directory "$GITHUB_WORKSPACE" + - id: setup + uses: ./.github/actions/setup-linux-build + - run: rustc --version && cargo --version + - uses: actions/cache@v4 + with: + path: | + ~/.cargo/registry/index + ~/.cargo/registry/cache + ~/.cargo/git/db + target + # The glibc floor is part of the cache key: artifacts built against a different glibc must + # never be restored into a build that claims a different floor. + key: linux-glibc${{ steps.setup.outputs.glibc-floor }}-build-${{ matrix.target }}-${{ hashFiles('**/Cargo.lock') }} + restore-keys: linux-glibc${{ steps.setup.outputs.glibc-floor }}-build-${{ matrix.target }}- + - name: Build release binaries (dig-node + dign alias) + # The triple is the HOST's here (native build, no cross-compilation), but it is passed + # explicitly anyway so the output lands in `target//release/` — the one path the + # shared staging action reads on every platform. + run: cargo build --release --locked --target ${{ matrix.target }} --bin dig-node --bin dign + - uses: ./.github/actions/stage-binaries + with: + version: ${{ inputs.version }} + target: ${{ matrix.target }} + out-name: ${{ matrix.out_name }} + - name: Assert the glibc floor of the produced binaries + # The gate that makes the floor durable. Both binaries are checked: `dign` is a separate + # link, so checking only `dig-node` would leave the alias unguarded. run: | - VER="${{ inputs.version }}" - mkdir -p dist - SRC="target/${{ matrix.target }}/release/${{ matrix.bin }}" - test -f "$SRC" || { echo "binary not produced: $SRC"; exit 1; } - # On Windows out_name already ends in .exe; on unix there is no extension. Publish the - # binary under the canonical dig-node-* name every consumer resolves (#585 dropped the - # duplicate legacy dig-companion-* copy). See the header comment. - cp "$SRC" "dist/dig-node-${VER}-${{ matrix.out_name }}" - # The `dign` first-class alias (issue #548): a SEPARATE binary that behaves byte-for-byte - # like `dig-node`, published under its own `dign-*` stem (same shape as `dig-node-*`) so - # the dig-installer resolves it via Repo::dign(). Its filename differs from `dig-node` - # only by the `dig-node`->`dign` substring (incl. the `.exe` suffix on Windows). - DIGN_BIN="${{ matrix.bin }}"; DIGN_BIN="${DIGN_BIN/dig-node/dign}" - DIGN_SRC="target/${{ matrix.target }}/release/${DIGN_BIN}" - test -f "$DIGN_SRC" || { echo "dign binary not produced: $DIGN_SRC"; exit 1; } - cp "$DIGN_SRC" "dist/dign-${VER}-${{ matrix.out_name }}" - ls -la dist - - name: Upload build artifact - uses: actions/upload-artifact@v4 + set -eu + for stem in dig-node dign; do + bin="dist/${stem}-${{ inputs.version }}-${{ matrix.out_name }}" + bash scripts/check-glibc-floor.sh "$bin" "${{ steps.setup.outputs.glibc-floor }}" + # Prove the gate is not a no-op against a REAL binary, not just against the unit + # tests' stub readelf: every ELF references at least the base glibc version + # (2.2.5 on x86_64, 2.17 on aarch64), so a floor of 2.0 MUST be rejected. A gate that + # cannot fail is a rubber stamp. + if bash scripts/check-glibc-floor.sh "$bin" 2.0 >/dev/null 2>&1; then + echo "::error::check-glibc-floor.sh accepted an impossible 2.0 floor for $bin — the gate is not working" + exit 1 + fi + done + + # PROOF, not inference: start each published Linux binary on the OLDEST distros it claims to + # support. `ldd --version` on the builder says what the binary was linked against; only executing + # it on the target says it STARTS there. #1736 was found by a binary that died at exec time. + # + # WHY `--version`: it is the one invocation that touches no config, no network, no filesystem + # state and no service manager, so it isolates "does the dynamic loader resolve this binary". + verify-linux-floor: + name: verify (${{ matrix.out_name }} on ${{ matrix.image }}) + needs: build-linux + strategy: + fail-fast: false + matrix: + include: + # The three distros #1736 reported broken, each on its own arch's runner so the binary + # executes natively rather than under emulation. + - runner: ubuntu-latest + out_name: linux-x64 + image: ubuntu:22.04 + - runner: ubuntu-latest + out_name: linux-x64 + image: debian:12 + - runner: ubuntu-latest + out_name: linux-x64 + image: amazonlinux:2023 + - runner: ubuntu-24.04-arm + out_name: linux-arm64 + image: ubuntu:22.04 + - runner: ubuntu-24.04-arm + out_name: linux-arm64 + image: debian:12 + - runner: ubuntu-24.04-arm + out_name: linux-arm64 + image: amazonlinux:2023 + runs-on: ${{ matrix.runner }} + steps: + - uses: actions/download-artifact@v4 with: name: dig-node-${{ matrix.out_name }} - path: dist/* - if-no-files-found: error + path: dist + - name: Run both binaries on ${{ matrix.image }} + run: | + set -eu + chmod +x dist/* + for stem in dig-node dign; do + bin="${stem}-${{ inputs.version }}-${{ matrix.out_name }}" + echo "::group::$bin on ${{ matrix.image }}" + # `ldd` FIRST and unconditionally: the dynamic loader names only the FIRST + # unsatisfied library, so a partial fix reads as a brand-new regression. Logging the + # full dependency set makes the real remaining gap visible in one run. + docker run --rm -v "$PWD/dist:/dist:ro" "${{ matrix.image }}" \ + sh -c "ldd /dist/$bin || true" + docker run --rm -v "$PWD/dist:/dist:ro" "${{ matrix.image }}" \ + /dist/$bin --version + echo "::endgroup::" + done diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dfee6bd..363e774 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,6 +30,26 @@ env: RUST_BACKTRACE: 1 jobs: + # The release path's CI-consumed shell scripts have their own tests, and they gate a real + # shipping property (#1736: the glibc floor of every published Linux artifact), so they run on + # every PR rather than only when a release is being built — a broken floor gate discovered at + # release time is discovered too late. + scripts: + name: Release-script tests + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: scripts/tests + run: | + set -eu + for t in scripts/tests/*.test.sh; do + echo "::group::$t" + bash "$t" + echo "::endgroup::" + done + fmt: name: Rustfmt runs-on: ubuntu-latest diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index c4ca4bb..c56866f 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -16,9 +16,11 @@ on: - "packaging/**" - "src/**" - "crates/**" + - "scripts/**" - "Cargo.toml" - "Cargo.lock" - ".github/workflows/package.yml" + - ".github/actions/**" push: tags: - "v*" @@ -30,18 +32,43 @@ env: CARGO_PROFILE_RELEASE_DEBUG: "0" jobs: - # ---- Ubuntu .deb ---------------------------------------------------------- + # ---- Ubuntu/Debian .deb (amd64 + arm64) ----------------------------------- + # + # Built inside the SAME pinned old-glibc container as the release binaries + # (`.github/actions/setup-linux-build`, glibc 2.31 — #1736). This .deb is what apt.dig.net serves, + # so building it on `ubuntu-latest` made `apt-get install dig-node` produce a binary that would + # not start on Ubuntu 22.04 LTS, Debian 12 or Amazon Linux 2023 — the same defect as the raw + # binary, on the path most users actually take. + # + # arm64 is built NATIVELY on the free public-repo arm64 runner (#1741): apt's arm64 index + # previously contained no `dig-node` at all, so every Graviton/Ampere/Pi host was unserved. deb: - name: build .deb (linux-x64) - runs-on: ubuntu-latest + name: build .deb (linux-${{ matrix.dpkg_arch }}) + runs-on: ${{ matrix.runner }} + container: debian:11 + strategy: + fail-fast: false + matrix: + include: + - runner: ubuntu-latest + target: x86_64-unknown-linux-gnu + dpkg_arch: amd64 + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-gnu + dpkg_arch: arm64 steps: + - name: Bootstrap the container for checkout + run: | + set -eux + apt-get update -qq + apt-get install -y --no-install-recommends ca-certificates curl git - uses: actions/checkout@v4 with: persist-credentials: false - - name: Install Rust - run: | - rustup toolchain install stable - rustup default stable + - name: Trust the checkout (git runs as root in the container) + run: git config --global --add safe.directory "$GITHUB_WORKSPACE" + - id: setup + uses: ./.github/actions/setup-linux-build - uses: actions/cache@v4 with: path: | @@ -49,8 +76,8 @@ jobs: ~/.cargo/registry/cache ~/.cargo/git/db target - key: ${{ runner.os }}-pkg-deb-${{ hashFiles('**/Cargo.lock') }} - restore-keys: ${{ runner.os }}-pkg-deb- + key: pkg-deb-glibc${{ steps.setup.outputs.glibc-floor }}-${{ matrix.target }}-${{ hashFiles('**/Cargo.lock') }} + restore-keys: pkg-deb-glibc${{ steps.setup.outputs.glibc-floor }}-${{ matrix.target }}- - name: Resolve version id: ver run: | @@ -58,16 +85,21 @@ jobs: V="$(grep -m1 '^version' Cargo.toml | sed -E 's/.*"(.*)".*/\1/')"; fi echo "v=$V" >> "$GITHUB_OUTPUT" - name: Build release binary - run: cargo build --release --locked --bin dig-node + run: cargo build --release --locked --target ${{ matrix.target }} --bin dig-node + - name: Assert the packaged binary honours the glibc floor + # Gate the .deb's payload, not just the raw release binary: apt is a separate delivery path + # and would otherwise be able to regress on its own. + run: bash scripts/check-glibc-floor.sh "target/${{ matrix.target }}/release/dig-node" "${{ steps.setup.outputs.glibc-floor }}" - name: Build .deb - run: bash packaging/linux/build-deb.sh target/release/dig-node "${{ steps.ver.outputs.v }}" amd64 dist + run: bash packaging/linux/build-deb.sh "target/${{ matrix.target }}/release/dig-node" "${{ steps.ver.outputs.v }}" "${{ matrix.dpkg_arch }}" dist - name: Validate .deb metadata run: | dpkg-deb --info dist/*.deb dpkg-deb --contents dist/*.deb - uses: actions/upload-artifact@v4 with: - name: dig-node-deb + # One artifact per arch — a shared name would make the two matrix legs collide. + name: dig-node-deb-${{ matrix.dpkg_arch }} path: dist/*.deb if-no-files-found: error @@ -175,7 +207,7 @@ jobs: # ---- Attach to the GitHub Release (tags only) ----------------------------- publish: name: Attach packages to the release - needs: [deb, pkg, msi] + needs: [deb, pkg, msi] # `deb` is a matrix; the whole matrix must be green before publishing if: github.ref_type == 'tag' runs-on: ubuntu-latest permissions: diff --git a/Cargo.lock b/Cargo.lock index dbba9bd..51e45f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2211,7 +2211,7 @@ dependencies = [ [[package]] name = "dig-node-core" -version = "0.23.0" +version = "0.24.0" dependencies = [ "async-trait", "axum", @@ -2266,7 +2266,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.66.0" +version = "0.67.0" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index 3756a40..2367846 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.66.0" +version = "0.67.0" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over diff --git a/DEVELOPMENT_LOG.md b/DEVELOPMENT_LOG.md index 964c745..3cb8c25 100644 --- a/DEVELOPMENT_LOG.md +++ b/DEVELOPMENT_LOG.md @@ -828,6 +828,15 @@ A fail-closed guard is only good UX if it fires at the EARLIEST point the bad co ## Lane anchor — dig_ecosystem#1668 / #1640 (the range-frame ceiling) +**STATUS: RESOLVED at dig-nat, and the node is now ON the fixed line.** dig-nat 0.13 shipped the +capped/paged SENDER (fallible `RangeFrame::encode` + the paged prologue) and 0.14 the receiver-side +`ChunkLensAssembler`; both fail CLOSED, so an over-ceiling frame is a clean boundary error rather than +a silent mid-read decode failure. The 0.14 line (dig-gossip 0.17, dig-dht 0.8, dig-download 0.12, +dig-peer 0.7, dig-peer-selector 0.7) landed as the single atomic cascade the note below anticipated, +so the "this node cannot reach 0.14 yet" paragraph is HISTORY, kept for the lesson it carries about +duplicate wire crates. `RANGE_WINDOW` remains the node's per-frame SPLIT size and is no longer a +KNOWN CONSTRAINT. + **One confused quantity made every DIG read above ~48 KiB impossible, network-wide.** `RANGE_WINDOW` (3 MiB) bounds how much ONE REQUEST may ask for. `dig_nat::MAX_RANGE_FRAME_PAYLOAD` (32,768 B) bounds ONE FRAME. The serve path framed on the former against the latter — 96x over — and because `bytes` diff --git a/SPEC.md b/SPEC.md index d0e44a7..ace0879 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1856,11 +1856,33 @@ a dig-node release — the installer's pre-rename fallback targets the SEPARATE `DIG-Network/dig-companion` repo's own frozen historical releases, not this asset name — so it was pure release-noise. -11.3. **Matrix.** `windows-x64` (x86_64-pc-windows-msvc), `linux-x64` (x86_64-unknown-linux-gnu), -`macos-arm64` (aarch64-apple-darwin), `macos-x64` (x86_64-apple-darwin, cross-compiled on -macos-14). No linux-arm64 asset is published (the Linux build graph pulls `openssl-sys` via the -Chia wallet SDK; no consumer requests it — apt.dig.net skips arm64 non-fatally and the installer -rejects arm64 tokens). +11.3. **Matrix + the Linux platform floor (HARD RULE).** Five assets are published: +`windows-x64` (x86_64-pc-windows-msvc), `linux-x64` (x86_64-unknown-linux-gnu), `linux-arm64` +(aarch64-unknown-linux-gnu), `macos-arm64` (aarch64-apple-darwin), and `macos-x64` +(x86_64-apple-darwin, cross-compiled on macos-14). The apt `.deb` is published for both `amd64` and +`arm64`. + +**Every published Linux artifact — the raw binaries AND the `.deb` — MUST run on glibc 2.31 or +newer.** 2.31 is the supported floor, and it is a floor on the whole Linux delivery surface, not a +per-workflow choice. It clears Ubuntu 20.04+, Debian 11+, Amazon Linux 2023 and RHEL 9. + +A glibc-linked binary runs on its BUILD glibc and anything newer, never anything older, so the +BUILDER IMAGE alone determines this floor. Every job producing a Linux artifact therefore builds +inside a pinned old-glibc container (`debian:11`) via `.github/actions/setup-linux-build`, which is +the single place the floor is declared. It is enforced in three ways, all of which MUST hold: + +- the action asserts the container's own glibc equals the declared floor, so the image and the + number cannot drift apart; +- `scripts/check-glibc-floor.sh` asserts every produced binary's highest glibc requirement is at or + below the floor, and the release job re-runs it against an impossible floor to prove the gate can + still fail; +- `verify-linux-floor` EXECUTES each published binary in `ubuntu:22.04`, `debian:12` and + `amazonlinux:2023` containers on both architectures — a link-time claim is not proof that a binary + starts. + +Raising the floor is a DELIBERATE, coordinated act: the declared value, every calling job's +`container:` image, this section, and the published docs move together. Both Linux architectures +build NATIVELY (aarch64 on the arm64 runner), so no vendored-OpenSSL cross-compile is involved. 11.4. **Release hardening.** The release profile keeps `overflow-checks = true` (the read path does offset/length arithmetic over untrusted serialized input). diff --git a/crates/dig-node-core/Cargo.toml b/crates/dig-node-core/Cargo.toml index 0262599..3f7ef15 100644 --- a/crates/dig-node-core/Cargo.toml +++ b/crates/dig-node-core/Cargo.toml @@ -1,6 +1,12 @@ [package] name = "dig-node-core" -version = "0.23.0" +# The engine library's OWN version, independent of the released binary's (`[workspace.package].version` +# in the root manifest, which only dig-node-service tracks). It moves when this crate's PUBLIC surface +# moves: 0.24.0 acknowledges that the surface now exposes dig-nat 0.14 / dig-dht 0.8 types, a +# compatibility-relevant change even though the crate is consumed in-workspace and is not published to +# crates.io. Holding it at 0.23.0 would have overstated stability to anyone reading the manifest for +# what the exported types are (dig_ecosystem#1785c). +version = "0.24.0" edition = "2021" license = "GPL-2.0-only" description = "The canonical DIG node ENGINE library (crate `dig_node_core`): the JSON-RPC dispatch (`handle_rpc`, the same contract as rpc.dig.net), local-first content serve/fetch/redirect from LOCAL .dig store modules (via digstore_host::serve_blind), chain-anchored-root resolution, chain-watch + subscriptions + generation gap-fill, the LRU cache, and the full P2P stack. Shared UNCHANGED by both host shells: the `dig-node` OS-service binary (dig-node-service) and the DIG Browser's in-process cdylib (dig-runtime). Native Rust so the compiled-module serve path works." diff --git a/crates/dig-node-core/SPEC.md b/crates/dig-node-core/SPEC.md index c8777ca..7ba0918 100644 --- a/crates/dig-node-core/SPEC.md +++ b/crates/dig-node-core/SPEC.md @@ -862,6 +862,25 @@ node state or reads local resources MUST stay off the list. - The connected pool is owned by dig-gossip: discover → dial → maintain, with churn events (`PeerAdded`/`PeerRemoved`) that feed the selector registry (§7.6) and the PEX pool feeder (§7.7). +- **`PeerAdded` is REPUBLISHED for an already-added `peer_id`, and no `PeerRemoved` precedes it.** + When a fresh verified session supersedes a stale slot for the same identity — typically a dead relay + circuit replaced by a direct dial — the pool emits another `PeerAdded` for that `peer_id` at the new + address. Every consumer MUST therefore be KEYED-IDEMPOTENT: it MUST treat a repeat `PeerAdded` as an + UPSERT of that identity's state (adding the address, preserving learned quality) and MUST NOT assume + the first `PeerAdded` for an identity is exclusive, nor that a `PeerRemoved` will arrive between two + adds. A consumer that appends per event MUST bound what it accumulates: unbounded per-identity + growth is a memory sink driven purely by churn (the node caps its per-peer address list at + `dig-dht`'s `MAX_ADDRESSES_PER_RECORD`). +- **A churn address is a HINT, not a verified destination.** A reported address MUST be rejected before + it is recorded as a peer's contact or fetch target when it is not a destination at all — an + unspecified/wildcard IP (`::` / `0.0.0.0`) or port `0`. dig-nat reports the wildcard as the remote of + an accepted RELAYED circuit when no relay endpoint is configured, and such an address both fails + every dial and consumes a scarce dial slot that a reachable address could have used. +- **The newest session's address leads the pool's per-peer order**, older addresses trailing as + fallbacks. That ordering is WITHIN the pool's list only: the dial ladder built from it prefers IPv6 + over IPv4 first (§5.2), so a freshly-adopted IPv4 address leads the IPv4 group, not the whole ladder. + An implementation MUST NOT let one address family's churn evict a peer's only address of the other + family — IPv6 is preferred, not exclusive. - On every inbound peer/DHT RPC the responder folds the mTLS-verified caller into its routing table — the caller identity MUST come from the authenticated transport, never a wire field. - A node MAY rotate its network identity (cert → `peer_id`) to reduce long-term linkability; diff --git a/crates/dig-node-core/src/download.rs b/crates/dig-node-core/src/download.rs index 064bbd2..d228563 100644 --- a/crates/dig-node-core/src/download.rs +++ b/crates/dig-node-core/src/download.rs @@ -348,6 +348,44 @@ pub(crate) fn pool_removal_reason(reason: GossipRemovalReason) -> PoolRemovalRea } } +/// The most addresses the connected pool keeps for ONE `peer_id`. +/// +/// WHY 8 (dig_ecosystem#1782): it is `dig-dht`'s own `MAX_ADDRESSES_PER_RECORD`, the limit every +/// address list this pool feeds is eventually cut to. Matching it means the pool never holds an +/// address that could not be published anyway. +/// +/// WHY a cap at all: `PeerAdded` is republished each time a fresh verified session supersedes a +/// stale slot for the same identity, and a supersede fires NO `PeerRemoved` — so without a cap the +/// list grows by one entry per distinct `SocketAddr` ever seen for that peer, forever (measured: +/// 5000 `PeerAdded` events → 5000 entries). Three independent guards downstream stop a remote peer +/// from driving that today, which makes this defence-in-depth rather than a live leak; the point of +/// the cap is that relaxing any ONE of those guards later must not turn this into a remote memory +/// sink. +pub(crate) const MAX_POOL_ADDRS_PER_PEER: usize = 8; + +/// Cut `addrs` (newest-first) down to at most `limit` entries, evicting the OLDEST address of +/// whichever address family currently has the most entries. +/// +/// WHY not a plain `truncate` (dig_ecosystem#1782, secondary): the tail is where the other address +/// FAMILY ends up. A peer reached over IPv6 that then becomes reachable over IPv4 accumulates fresh +/// IPv6 sessions; a blind truncate drops the lone IPv4 address off the end, and since the downstream +/// dial ladder is both IPv6-first and very short, losing it can make an otherwise-reachable peer +/// unreachable. Evicting from the larger family keeps at least one address of each family for as +/// long as the cap allows, which is what the IPv6-first/IPv4-FALLBACK rule (§5.2) actually requires: +/// IPv6 is preferred, not exclusive. +fn retain_newest_per_family(addrs: &mut Vec, limit: usize) { + while addrs.len() > limit { + let ipv6_count = addrs.iter().filter(|a| a.is_ipv6()).count(); + let evict_ipv6 = ipv6_count * 2 >= addrs.len(); + // Newest-first ordering means the LAST entry of a family is its oldest. + let victim = addrs + .iter() + .rposition(|a| a.is_ipv6() == evict_ipv6) + .expect("the majority family has at least one member"); + addrs.remove(victim); + } +} + /// The kind of a pool churn event, extracted from `dig_gossip::PoolEvent` at the call site so this /// module does not depend on dig-gossip's concrete type. The caller (`crate::peer`) destructures the /// gossip event into this + the raw 32-byte peer id, keeping the 1:1 map explicit and testable here. @@ -828,15 +866,29 @@ impl NodeContent { .unwrap_or_else(|poisoned| poisoned.into_inner()); match event { PoolEvent::PeerAdded { peer_id, addr } => { + // A wildcard / port-0 address is not a destination (#1784): dig-nat reports `[::]:0` + // as the remote of an accepted relayed circuit when no relay endpoint is configured. + // SKIP the event outright rather than record-then-filter, so a peer that already has + // a working address does not have it displaced from the front of the dial order by + // an address nothing can be reached at. + if !crate::seams::dig_peer::net::is_usable_contact(addr) { + return; + } // The NEWEST session's address leads the candidate's dial order, older ones trailing // as fallbacks (#1771). dig-gossip republishes `PeerAdded` when a fresh verified // session supersedes a stale slot for the same identity (#1691/#1703/#1762), which is // typically a MOVE — a dead relay circuit replaced by a direct dial. Appending would // leave the dead address first and spend a failed dial on it for every later fetch, // and dropping the older ones would discard a still-working fallback. + // + // Newest-first holds within the POOL's list; the eventual dial order additionally + // prefers IPv6 over IPv4 (dig-download's `dial_candidates` sorts by family first, + // per the ecosystem IPv6-first rule §5.2), so a freshly-adopted IPv4 address leads + // only the IPv4 group, not the whole ladder (#1785d). let addrs = pool.entry(peer_id.to_hex()).or_default(); addrs.retain(|known| known != addr); addrs.insert(0, *addr); + retain_newest_per_family(addrs, MAX_POOL_ADDRS_PER_PEER); } PoolEvent::PeerRemoved { peer_id, .. } => { pool.remove(&peer_id.to_hex()); @@ -1518,6 +1570,141 @@ pub(crate) mod tests { ContentId::resource([1; 32], root_bytes, [3; 32]) } + // -- connected-pool address bookkeeping (#1782 cap, #1784 wildcard guard) -------------------- + + /// A `NodeContent` with no real transport — enough to drive `on_pool_event` and read the pool + /// back. The download machinery is never exercised, so the mocks can be trivial. + fn pool_only_content(dir: &std::path::Path) -> Arc { + NodeContent::new( + Arc::new(MockProviderLocator::fixed(vec![])), + Arc::new(MockRangeTransport::new(MockContent::even(4, 1))), + MissMode::FetchThrough, + None, + dir, + ) + } + + /// Feed one `PeerAdded` for `peer` at `addr` through the real event path. + fn feed_added(content: &NodeContent, peer: [u8; 32], addr: &str) { + content.on_pool_event(&pool_event_to_selector( + peer, + PoolEventKind::Added { + addr: addr.parse().expect("test address parses"), + }, + )); + } + + /// The addresses the pool currently holds for `peer`, newest first. + fn pool_addrs(content: &NodeContent, peer: [u8; 32]) -> Vec { + let pool = content.connected_pool(); + let guard = pool.lock().unwrap(); + guard.get(&hex::encode(peer)).cloned().unwrap_or_default() + } + + /// #1782: a supersede republishes `PeerAdded` and fires NO `PeerRemoved`, so the address list + /// must be capped rather than growing once per distinct `SocketAddr` ever seen. 5000 is the + /// figure the security gate measured growing unbounded — well past `MAX_ADDRESSES_PER_RECORD`, + /// so the cap cannot pass by accident of a small fixture. + #[test] + fn a_peer_that_supersedes_forever_never_exceeds_the_address_cap() { + let td = tempfile::tempdir().unwrap(); + let content = pool_only_content(td.path()); + let peer = [0xA1; 32]; + + for port in 10_000..15_000u16 { + feed_added(&content, peer, &format!("203.0.113.7:{port}")); + } + + let addrs = pool_addrs(&content, peer); + assert_eq!( + addrs.len(), + MAX_POOL_ADDRS_PER_PEER, + "5000 superseding sessions must leave at most the cap, not 5000 entries" + ); + assert_eq!( + addrs[0].port(), + 14_999, + "the newest session still leads the pool's dial order" + ); + } + + /// #1782 secondary: the ONE IPv4 address of a mostly-IPv6 peer must survive continued IPv6 + /// churn. A plain `truncate` keeps the newest 8 — all IPv6 here — and silently drops the only + /// address of the fallback family, which the very short IPv6-first dial ladder downstream then + /// cannot recover. The IPv4 address is adopted BEFORE the churn precisely so a + /// newest-first-wins implementation cannot pass this by keeping it for recency. + #[test] + fn ipv6_churn_cannot_evict_a_peers_only_ipv4_address() { + let td = tempfile::tempdir().unwrap(); + let content = pool_only_content(td.path()); + let peer = [0xB2; 32]; + + feed_added(&content, peer, "203.0.113.7:9444"); + for n in 0..64 { + feed_added(&content, peer, &format!("[2001:db8::{n:x}]:9444")); + } + + let addrs = pool_addrs(&content, peer); + assert_eq!(addrs.len(), MAX_POOL_ADDRS_PER_PEER); + assert!( + addrs.iter().any(|a| a.is_ipv4()), + "the IPv4 fallback must survive IPv6 churn; got {addrs:?}" + ); + assert!( + addrs.iter().filter(|a| a.is_ipv6()).count() >= MAX_POOL_ADDRS_PER_PEER - 1, + "IPv6 still fills the rest of the list — IPv6 is PREFERRED, not evicted (§5.2)" + ); + } + + /// #1784: `[::]:0` — dig-nat's remote address for an accepted relayed circuit with no configured + /// relay endpoint — must never become a peer's contact. + /// + /// The peer is given a WORKING address first and the assertion is that the working address is + /// still there, still FIRST. That is what makes this test see the difference between skipping + /// the event and recording-then-filtering: a filter applied when the pool is READ would leave + /// the wildcard sitting at the head of the stored list, displacing the reachable address, and + /// would still satisfy a bare "the wildcard is not returned" assertion. + #[test] + fn a_wildcard_relay_address_never_displaces_a_peers_real_address() { + let td = tempfile::tempdir().unwrap(); + let content = pool_only_content(td.path()); + let peer = [0xC3; 32]; + + feed_added(&content, peer, "203.0.113.7:9444"); + feed_added(&content, peer, "[::]:0"); + + let addrs = pool_addrs(&content, peer); + assert_eq!( + addrs, + vec!["203.0.113.7:9444".parse::().unwrap()], + "the wildcard is dropped at the event, leaving the real address untouched and leading" + ); + } + + /// #1784, control: a peer whose ONLY reported address is the wildcard is absent from the pool + /// entirely — it must not appear as a fetch candidate with an unreachable target. Paired with + /// the test above so "absent" cannot be achieved by dropping every peer. + #[test] + fn a_peer_known_only_by_a_wildcard_address_is_not_a_pool_candidate() { + let td = tempfile::tempdir().unwrap(); + let content = pool_only_content(td.path()); + let wildcard_only = [0xD4; 32]; + let reachable = [0xE5; 32]; + + feed_added(&content, wildcard_only, "[::]:0"); + feed_added(&content, reachable, "203.0.113.9:9444"); + + assert!( + pool_addrs(&content, wildcard_only).is_empty(), + "an unreachable-only peer is not a candidate" + ); + assert_eq!( + pool_addrs(&content, reachable).len(), + 1, + "a reachable peer in the same pool is unaffected" + ); + } + // -- miss-mode resolution -------------------------------------------------------------------- #[test] diff --git a/crates/dig-node-core/src/peer.rs b/crates/dig-node-core/src/peer.rs index 815dd37..d15dc8e 100644 --- a/crates/dig-node-core/src/peer.rs +++ b/crates/dig-node-core/src/peer.rs @@ -114,18 +114,38 @@ pub fn dht_addr_from_gossip_addr(gossip: std::net::SocketAddr) -> std::net::Sock dht_addr } +/// A pool-reported gossip address turned into the DHT contact to store for that peer, or `None` when +/// the result would not be a usable contact. +/// +/// Composes the two things that must BOTH hold before an address enters the DHT routing table: +/// [`dht_addr_from_gossip_addr`]'s port shift, and [`crate::net::is_usable_contact`]'s "is this a +/// destination at all" check (dig_ecosystem#1784 — dig-nat reports `[::]:0` as the remote of an +/// accepted relayed circuit with no configured relay endpoint, and routing would otherwise store the +/// wildcard as that peer's contact, so every lookup seeded from it dead-ends). +/// +/// The check is applied to the MAPPED address, not the raw one: the port shift is what determines the +/// port actually stored, so a gossip port at or below the offset maps to port 0 — unusable — even +/// though the input looked fine. +pub(crate) fn dht_contact_from_pool_addr( + gossip: std::net::SocketAddr, +) -> Option { + let dht = dht_addr_from_gossip_addr(gossip); + crate::net::is_usable_contact(&dht).then_some(dht) +} + /// Per-window ciphertext cap for a `dig.fetchRange` frame (bytes) — the node window (3 MiB), the same /// cap the HTTP read path (`WINDOW`) uses. /// -/// KNOWN CONSTRAINT (dig_ecosystem#1640): this is chosen without reference to `dig-nat`'s wire framing -/// cap (`MAX_FRAMED_BODY`, 64 KiB) — a 3 MiB frame plus #1577's per-frame verification metadata -/// (`root`, the whole `chunk_lens` array, `total_length`, a base64 `inclusion_proof`) cannot actually -/// be base64-encoded and framed within that limit, so a real peer-to-peer window this large fails to -/// decode on arrival. The reshare leg's whole-module pull (`seams/dig_peer/module_serve.rs`) rides the -/// SAME constant and inherits the same defect for any module above a few tens of KB. The fix belongs -/// at `dig-nat` (publish the real payload ceiling; every framer targets it) — this constant is a -/// single source of truth for the per-frame split, so lowering it here (once the true ceiling is -/// known) requires touching only this line, not the streaming loops that consume it. +/// The framing ceiling this constant used to fight (dig_ecosystem#1640) is RESOLVED at `dig-nat`, +/// which is where it belonged: 0.13 made `RangeFrame::encode` FALLIBLE and payload-capped at the +/// sender and added the paged-prologue API (so #1577's per-frame verification metadata — `root`, the +/// `chunk_lens` array, `total_length`, the base64 `inclusion_proof` — is split across pages instead of +/// having to fit one frame), and 0.14 added the receiver-side `ChunkLensAssembler` validation. Both +/// fail CLOSED, so an over-ceiling frame is now a clean error at the boundary rather than a silent +/// decode failure mid-read. +/// +/// This value therefore remains the node's per-frame SPLIT size, and stays the single source of truth +/// for it: the streaming loops that consume it never need to change if it moves. pub const RANGE_WINDOW: usize = 3 * 1024 * 1024; /// Maximum concurrent accepted mTLS peer CONNECTIONS the listener will serve at once (audit #179 @@ -1831,7 +1851,11 @@ fn spawn_dht_routing_feed(dht: Arc, handle: dig_gossip::G for (peer_id, addr, _outbound) in handle.connected_pool_peers() { let mut bytes = [0u8; 32]; bytes.copy_from_slice(peer_id.as_ref()); - let dht_addr = dht_addr_from_gossip_addr(addr); + // Skip a peer whose pool address is not a destination (#1784) — routing must never seed a + // lookup from a wildcard contact. + let Some(dht_addr) = dht_contact_from_pool_addr(addr) else { + continue; + }; let dht = dht.clone(); tokio::spawn(async move { dht.add_peer(bytes, dht_addr).await; @@ -1852,8 +1876,11 @@ fn spawn_dht_routing_feed(dht: Arc, handle: dig_gossip::G dig_gossip::PoolEvent::PeerAdded { peer_id, addr } => { let mut bytes = [0u8; 32]; bytes.copy_from_slice(peer_id.as_ref()); - // Map the peer's gossip addr to its DHT addr before seeding routing (GAP 2). - dht.add_peer(bytes, dht_addr_from_gossip_addr(*addr)).await; + // Map the peer's gossip addr to its DHT addr before seeding routing (GAP 2), + // dropping an address that is not a destination (#1784). + if let Some(dht_addr) = dht_contact_from_pool_addr(*addr) { + dht.add_peer(bytes, dht_addr).await; + } } dig_gossip::PoolEvent::PeerRemoved { peer_id, .. } => { let mut bytes = [0u8; 32]; @@ -2738,7 +2765,7 @@ fn build_server_tls_config(node: &dig_nat::NodeCert) -> Result bool { + pub(crate) async fn host_has_ipv6_loopback() -> bool { tokio::net::TcpListener::bind("[::1]:0").await.is_ok() } @@ -3320,7 +3381,10 @@ mod tests { /// Build a real, freshly-started `GossipHandle` on the production-shaped dual-stack unspecified /// bind (`[::]:0`, §5.2) for the pool-handle tests. - async fn fresh_pool_handle(tag: &str, network: [u8; 32]) -> dig_gossip::GossipHandle { + pub(crate) async fn fresh_pool_handle( + tag: &str, + network: [u8; 32], + ) -> dig_gossip::GossipHandle { fresh_pool_handle_on(tag, network, crate::net::dual_stack_listen_addr(0)).await } @@ -3331,7 +3395,7 @@ mod tests { /// inbound loopback connections into the pool (the native-tls dual-stack accept quirk — the same /// family of `[::]`-v6only issue tracked for the extension-offline path), whereas a concrete /// loopback bind does, on every platform. Production still binds dual-stack `[::]` (`run_peer_network`). - async fn fresh_pool_handle_on( + pub(crate) async fn fresh_pool_handle_on( tag: &str, network: [u8; 32], listen_addr: std::net::SocketAddr, diff --git a/crates/dig-node-core/src/seams/dig_peer/net.rs b/crates/dig-node-core/src/seams/dig_peer/net.rs index 9af0ddb..6fc1422 100644 --- a/crates/dig-node-core/src/seams/dig_peer/net.rs +++ b/crates/dig-node-core/src/seams/dig_peer/net.rs @@ -82,6 +82,29 @@ pub fn is_advertisable_ipv4(ip: &Ipv4Addr) -> bool { !(ip.is_loopback() || ip.is_unspecified() || ip.is_link_local() || ip.is_broadcast()) } +/// Whether an address REPORTED BY A PEER OR THE POOL is a usable contact — one this node could +/// actually dial back — and so may be recorded as that peer's `fetchRange` target or DHT contact. +/// +/// WHY this exists (dig_ecosystem#1784): `dig-nat`'s accept path records `remote_addr` for an +/// accepted RELAYED circuit, and with no configured relay endpoint that address is the unspecified +/// wildcard `[::]:0`. It then flows through `PoolEvent::PeerAdded` into the connected pool as the +/// peer's fetch target AND into the DHT routing table as its contact. A wildcard address is not a +/// destination — a fetch to it can only fail — and worse, it consumes one of the very few dial +/// slots downstream, so a peer that IS reachable by another address can be rendered unreachable. +/// The root cause belongs in dig-nat; this is the node's own guard, applied where such an address +/// would enter node state. +/// +/// Rejects exactly two things, both of which mean "not a destination": +/// - an **unspecified** IP (`::` / `0.0.0.0`) — a bind wildcard, never a peer's location; +/// - port **0** — the "any port" sentinel, which nothing listens on. +/// +/// Loopback is deliberately ACCEPTED (unlike [`is_advertisable_ipv6`] / [`is_advertisable_ipv4`], +/// which decide what to advertise to the wider network): a loopback peer is genuinely dialable, and +/// single-host multi-node runs depend on it. +pub fn is_usable_contact(addr: &SocketAddr) -> bool { + !addr.ip().is_unspecified() && addr.port() != 0 +} + /// Discover a routable local IPv6 address, if the host has one. Uses the connect-a-UDP-socket trick: /// "connecting" a UDP socket to an off-host address forces the OS to select the local address it /// would route from, WITHOUT sending any packet. Returns the local IPv6 address only when it is @@ -433,6 +456,44 @@ pub fn build_node_nat_runtime( mod tests { use super::*; + /// The #1784 guard, from both sides. The rejected cases are the exact shapes dig-nat's accept + /// path can produce for a relayed circuit with no configured relay endpoint (`[::]:0`), plus the + /// two half-broken variants — a wildcard IP with a real port, and a real IP with port 0 — + /// because a guard that only recognises the fully-degenerate pair would pass either half + /// straight into the pool. + #[test] + fn a_wildcard_or_portless_address_is_not_a_usable_contact() { + for junk in [ + "[::]:0", + "0.0.0.0:0", + "[::]:9445", + "0.0.0.0:9445", + "1.2.3.4:0", + ] { + let addr: SocketAddr = junk.parse().unwrap(); + assert!( + !is_usable_contact(&addr), + "{junk} is not a destination and must never become a peer's contact" + ); + } + } + + /// The truthful control: real addresses — including LOOPBACK, which single-host multi-node runs + /// depend on — stay usable. Without this the guard could reject everything and still look + /// correct against the rejection cases alone. + #[test] + fn a_real_address_including_loopback_is_a_usable_contact() { + for good in [ + "203.0.113.7:9445", + "[2001:db8::7]:9445", + "127.0.0.1:9445", + "[::1]:9445", + ] { + let addr: SocketAddr = good.parse().unwrap(); + assert!(is_usable_contact(&addr), "{good} is dialable"); + } + } + #[test] fn dual_stack_listen_addr_is_ipv6_unspecified() { let addr = dual_stack_listen_addr(9444); diff --git a/crates/dig-node-core/src/seams/dig_peer/pex.rs b/crates/dig-node-core/src/seams/dig_peer/pex.rs index 78be192..7f8af23 100644 --- a/crates/dig-node-core/src/seams/dig_peer/pex.rs +++ b/crates/dig-node-core/src/seams/dig_peer/pex.rs @@ -626,6 +626,22 @@ impl PexPool for GossipPexPool { .await { Ok(conn) => { + // RE-CHECK pool membership now that the dial has resolved (#1783). The + // `is_pool_peer` check above happened at DECISION time; this dial ran for up + // to CANDIDATE_DIAL_TIMEOUT afterwards, and the identity may have joined the + // pool in between (an intervening PEX offer, the pool's own maintenance, or + // the peer dialing us). Adopting then is not harmless: since dig-gossip + // 0.17.12 a second connection for an identity SUPERSEDES the existing slot + // and tears down the displaced session, which kills any `fetchRange` stream + // in flight over it. Earlier versions merely returned `DuplicateConnection`, + // which is why this was previously safe to ignore. + if !should_adopt_dialed_peer(&handle, &peer_id) { + tracing::debug!( + peer = %peer_id, + "pex candidate joined the pool while the dial was in flight; dropping the redundant connection rather than superseding the live session" + ); + return; + } // Adoption dedups + caps; a duplicate/full/banned result is fine (already known). let _ = handle.adopt_nat_connection(conn).await; } @@ -652,6 +668,24 @@ impl PexPool for GossipPexPool { } } +/// Whether a PEX candidate whose dial has just SUCCEEDED should still be adopted into the pool. +/// +/// The answer is "only if the identity is still absent from the pool". The membership check that +/// authorised the dial happened before it, and a NAT dial can run for `CANDIDATE_DIAL_TIMEOUT`; if +/// the identity joined the pool in the meantime, adopting a second connection for it SUPERSEDES the +/// live slot and tears the existing session down (dig-gossip 0.17.12 onward), taking any in-flight +/// range stream with it. Dropping the redundant connection instead costs nothing — the peer is +/// already connected, which is the outcome the dial was for. +/// +/// Split out from the spawned dial task so the decision is unit-testable against a REAL pool, and so +/// there is exactly ONE place in this module that decides whether an adoption may proceed. +fn should_adopt_dialed_peer( + handle: &dig_gossip::GossipHandle, + peer_id: &dig_gossip::PeerId, +) -> bool { + !handle.is_pool_peer(peer_id) +} + /// Parse a 64-hex `peer_id` into a dig-gossip [`PeerId`](dig_gossip::PeerId) (`Bytes32`), or `None` if /// malformed. Pure so it is unit-tested without a handle. fn parse_gossip_peer_id(peer_id_hex: &str) -> Option { @@ -673,6 +707,57 @@ mod tests { format!("{b:02x}").repeat(32) } + /// #1783: once a PEX dial has resolved, an identity that is ALREADY in the pool must not be + /// adopted — adopting supersedes the live slot and tears its session (and any in-flight range + /// stream) down. + /// + /// Driven against a REAL pool holding a REAL connected peer rather than a stubbed membership + /// answer: node A dials node B over the IPv6 loopback, so A's pool genuinely contains B (the + /// DIALER's half registers synchronously on every platform, unlike the inbound half). An + /// unconnected identity in the SAME pool is the control — without it the test would also pass + /// against an implementation that refused every adoption. + #[tokio::test] + async fn a_candidate_already_in_the_pool_is_not_adopted_after_its_dial_resolves() { + if !crate::peer::tests::host_has_ipv6_loopback().await { + eprintln!("skipping: host has no usable IPv6 loopback stack"); + return; + } + // The gossip mTLS stack needs a process-global rustls provider; install it so this test is + // order-independent (production installs it during node bring-up). + let _ = rustls::crypto::ring::default_provider().install_default(); + + let network = [0x7bu8; 32]; + let node_a = crate::peer::tests::fresh_pool_handle("pex-adopt-a", network).await; + let node_b = crate::peer::tests::fresh_pool_handle_on( + "pex-adopt-b", + network, + "[::1]:0".parse().expect("parse [::1]:0"), + ) + .await; + let b_port = node_b + .__listen_bound_addr_for_tests() + .expect("node B bound listen addr") + .port(); + + let b_peer_hex = crate::peer::connect_peer(&node_a, &format!("[::1]:{b_port}")) + .await + .expect("node A dials node B over loopback mTLS"); + let b_peer_id = parse_gossip_peer_id(&b_peer_hex).expect("B's peer id is 64-hex"); + + assert!( + !should_adopt_dialed_peer(&node_a, &b_peer_id), + "B is already in A's pool — adopting a second connection would supersede the live session" + ); + + // Control: an identity A has never connected to is still adoptable, so the guard is + // membership-driven and not a blanket refusal. + let stranger = dig_gossip::PeerId::from([0x33u8; 32]); + assert!( + should_adopt_dialed_peer(&node_a, &stranger), + "an unconnected identity's verified dial is exactly what adoption is for" + ); + } + /// A capturing [`PexPool`] that records what PEX handed it, so the adapter is tested without a live /// pool or any network. #[derive(Default)] diff --git a/crates/dig-node-service/tests/release_workflow_dign_guard.rs b/crates/dig-node-service/tests/release_workflow_dign_guard.rs index cd03746..9952582 100644 --- a/crates/dig-node-service/tests/release_workflow_dign_guard.rs +++ b/crates/dig-node-service/tests/release_workflow_dign_guard.rs @@ -6,30 +6,65 @@ //! matcher (a separate follow-up, #548 step 3): here we assert the workflow actually //! EMITS the asset the installer will later resolve. The cross-OS build moved out of //! `release.yml` into the reusable `build-binaries.yml` (#592, so the stable + nightly -//! channels share ONE build); this guard follows it there. The workflow is embedded at -//! compile time so the check runs hermetically with no filesystem access. +//! channels share ONE build), and the ASSET NAMING then moved again, into the +//! `stage-binaries` composite action (dig_ecosystem#1736: the Linux targets had to move +//! into a pinned old-glibc container, giving two build jobs that must not be able to +//! diverge on the names they publish). This guard follows the naming to its current home. +//! Both files are embedded at compile time so the check runs hermetically. /// The reusable build workflow, embedded from the repo root (`crates/dig-node-service/tests` /// is three levels below it). const BUILD_YML: &str = include_str!("../../../.github/workflows/build-binaries.yml"); -/// The build step must compile the `dign` bin target beside `dig-node`; dropping +/// The composite action that OWNS the published asset names for every platform. +const STAGE_ACTION_YML: &str = include_str!("../../../.github/actions/stage-binaries/action.yml"); + +/// Every build job must compile the `dign` bin target beside `dig-node`; dropping /// `--bin dign` would silently stop shipping the alias. +/// +/// The COUNT is asserted, not just the presence: there are now TWO build jobs (the +/// Windows/macOS host-runner matrix and the containerized Linux matrix), and a guard that +/// accepted one occurrence would not notice one of them dropping the alias. +#[test] +fn every_build_job_builds_the_dign_bin() { + let jobs_building_dign = BUILD_YML.matches("--bin dig-node --bin dign").count(); + assert_eq!( + jobs_building_dign, 2, + "both build jobs (host-runner + containerized Linux) must \ + `cargo build … --bin dig-node --bin dign`; found {jobs_building_dign}" + ); +} + +/// The staging action must publish BOTH stems under `--` — the exact +/// shape the dig-installer resolves. #[test] -fn release_workflow_builds_the_dign_bin() { +fn the_staging_action_publishes_both_the_dig_node_and_dign_stems() { assert!( - BUILD_YML.contains("--bin dig-node --bin dign"), - "build-binaries.yml must `cargo build … --bin dig-node --bin dign`" + STAGE_ACTION_YML.contains("for stem in dig-node dign"), + "stage-binaries must stage BOTH the `dig-node` binary and the `dign` alias" + ); + assert!( + STAGE_ACTION_YML.contains(r#"cp "$src" "dist/${stem}-${VER}-${{ inputs.out-name }}""#), + "stage-binaries must publish each stem as `--`" ); } -/// The stage step must publish the alias under the `dign--` stem — the -/// exact shape the dig-installer resolves (matching `dig-node--`). +/// Naming lives in ONE place. If a build job ever stages an asset itself, the two jobs can +/// drift on the names downstream consumers resolve — precisely the failure the composite +/// action was extracted to prevent. #[test] -fn release_workflow_stages_the_dign_asset() { +fn no_build_job_stages_assets_outside_the_staging_action() { + assert!( + !BUILD_YML.contains("dist/dig-node-${VER}") && !BUILD_YML.contains("dist/dign-${VER}"), + "build-binaries.yml must delegate asset naming to ./.github/actions/stage-binaries, \ + never construct an asset name itself" + ); assert!( - BUILD_YML.contains("dist/dign-${VER}-${{ matrix.out_name }}"), - "build-binaries.yml must stage a `dign--` release asset" + BUILD_YML + .matches("./.github/actions/stage-binaries") + .count() + >= 2, + "every build job must stage through the shared action" ); } @@ -42,13 +77,19 @@ fn release_workflow_stages_the_dign_asset() { /// repo's own frozen historical releases (its own asset stem), not this asset name. /// /// So the duplicate is pure release-noise — the build must ship ONLY `dig-node-*` + `dign-*`. +/// Checked in BOTH files, since either could reintroduce the copy. #[test] fn release_workflow_no_longer_ships_the_legacy_dig_companion_asset() { // Scope to the STAGED asset path (`dist/dig-companion-…`), not any mention of the word — - // the header comment legitimately explains WHY the legacy copy was dropped. - assert!( - !BUILD_YML.contains("dist/dig-companion"), - "build-binaries.yml must NOT stage a duplicate legacy `dig-companion-*` asset \ - (#585) — ship only the canonical `dig-node-*` name + the `dign-*` alias" - ); + // the header comments legitimately explain WHY the legacy copy was dropped. + for (name, yml) in [ + ("build-binaries.yml", BUILD_YML), + ("stage-binaries/action.yml", STAGE_ACTION_YML), + ] { + assert!( + !yml.contains("dist/dig-companion"), + "{name} must NOT stage a duplicate legacy `dig-companion-*` asset \ + (#585) — ship only the canonical `dig-node-*` name + the `dign-*` alias" + ); + } } diff --git a/scripts/check-glibc-floor.sh b/scripts/check-glibc-floor.sh new file mode 100755 index 0000000..250e774 --- /dev/null +++ b/scripts/check-glibc-floor.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# +# Assert that a shipped Linux binary does not require a NEWER glibc than the declared floor. +# +# WHY this gate exists (#1736): dig-node v0.64.0's `linux-x64` binary silently began requiring +# glibc >= 2.38 — not from any code change, but because GitHub moved the `ubuntu-latest` runner to +# Ubuntu 24.04. That single image change broke installation on Ubuntu 22.04 LTS (2.35), Debian 12 +# (2.36) and Amazon Linux 2023 (2.34): the three most common server distros in existence. A +# supported-floor CLAIM in a doc cannot notice that happening; only a gate over the produced +# artifact can, so the release build runs this against every Linux binary it publishes. +# +# Usage: check-glibc-floor.sh +# Exit: 0 = binary's highest glibc requirement is <= +# 1 = the floor has RISEN (the regression this gate catches) +# 2 = usage error (never silently green — that is how a gate rots into a no-op) +set -uo pipefail + +# Overridable so the unit tests can feed exact adversarial symbol tables (scripts/tests/). +READELF="${READELF_BIN:-readelf}" + +binary="${1:-}" +floor="${2:-}" + +if [ -z "$binary" ] || [ -z "$floor" ]; then + echo "usage: $(basename "$0") e.g. dist/dig-node-1.2.3-linux-x64 2.31" >&2 + exit 2 +fi +if [ ! -f "$binary" ]; then + echo "check-glibc-floor: no such binary: $binary" >&2 + exit 2 +fi + +# `-V` reports the .gnu.version_r needs (the authoritative "this binary requires GLIBC_x.y" list); +# `--dyn-syms` catches per-symbol tags such as `memcpy@GLIBC_2.14`. Reading both means neither an +# unusual link layout nor a stripped version section can hide a requirement. +# +# The `[0-9]` after the underscore is load-bearing: it drops the unversioned `GLIBC_PRIVATE` tag, +# which would otherwise pollute the maximum. +requirements="$("$READELF" -V --dyn-syms --wide "$binary" 2>/dev/null | + grep -oE 'GLIBC_[0-9]+(\.[0-9]+)+' | sed 's/^GLIBC_//' | sort -Vu)" + +if [ -z "$requirements" ]; then + echo "check-glibc-floor: $binary requires no versioned glibc symbols (static or musl) — OK" + exit 0 +fi + +# `sort -V` compares each dotted component NUMERICALLY. A lexical sort would rank glibc 2.4 above +# 2.31 and reject a perfectly old binary, so the version-aware sort is required, not cosmetic. +highest="$(printf '%s\n' "$requirements" | tail -n 1)" +newest_of_pair="$(printf '%s\n%s\n' "$floor" "$highest" | sort -V | tail -n 1)" + +echo "check-glibc-floor: $binary requires glibc up to $highest (declared floor $floor)" +echo " all requirements: $(printf '%s ' $requirements)" + +if [ "$newest_of_pair" != "$floor" ]; then + cat >&2 <"$path" + chmod +x "$path" + echo "$path" +} + +# expect +expect() { + local name="$1" want="$2" stub="$3" floor="$4" + local out status + out="$(READELF_BIN="$stub" bash "$GATE" "$WORK/fake-binary" "$floor" 2>&1)" + status=$? + if [ "$status" -ne "$want" ]; then + printf 'FAIL %s: exit %s, want %s\n%s\n' "$name" "$status" "$want" "$out" + failures=$((failures + 1)) + else + printf 'ok %s\n' "$name" + fi +} + +: >"$WORK/fake-binary" + +# A binary needing exactly the floor is ACCEPTED — the at-bound side of the bound. Without this +# case a gate could reject everything and still look green on the over-bound case alone. +at_bound="$(stub_readelf at-bound \ + ' 0000: 0x0b792650 0x00 04 GLIBC_2.31' \ + ' 4: 0000000000000000 0 FUNC GLOBAL DEFAULT UND memcpy@GLIBC_2.14 (3)')" +expect 'at the floor passes' 0 "$at_bound" 2.31 + +# One minor OVER the floor is REJECTED — this is the regression the gate exists to catch: it is +# exactly what `ubuntu-latest` moving to 24.04 did to v0.64.0 (2.39 against a 2.35 target). +over_bound="$(stub_readelf over-bound \ + ' 0000: 0x0b792650 0x00 04 GLIBC_2.32' \ + ' 4: 0000000000000000 0 FUNC GLOBAL DEFAULT UND memcpy@GLIBC_2.14 (3)')" +expect 'one minor over the floor fails' 1 "$over_bound" 2.31 + +# VERSION-ORDER trap: glibc 2.4 is OLDER than 2.31, but a LEXICAL string comparison says +# "2.4" > "2.31" and would reject this binary. Only a numeric/version-aware comparison passes, +# so this fixture is what distinguishes the correct gate from the nearest wrong one. +lexical_trap="$(stub_readelf lexical-trap \ + ' 0000: 0x0b792650 0x00 04 GLIBC_2.4' \ + ' 0010: 0x09691a75 0x00 03 GLIBC_2.17')" +expect 'a 2.4 requirement is older than a 2.31 floor' 0 "$lexical_trap" 2.31 + +# GLIBC_PRIVATE carries no version number. An implementation that swept it into the requirement +# set would take it as the maximum (it sorts above any digit) and reject a binary that is in fact +# well under the floor. The other requirement here is DELIBERATELY below the floor so the expected +# result is PASS: expecting a failure here would be satisfied by the swallowing implementation too. +private_tag="$(stub_readelf private-tag \ + ' 0000: 0x09691f71 0x00 04 GLIBC_PRIVATE' \ + ' 0010: 0x09691a75 0x00 03 GLIBC_2.17')" +expect 'GLIBC_PRIVATE is not mistaken for a requirement' 0 "$private_tag" 2.31 + +# A binary with NO glibc requirement at all (a static/musl build) is accepted rather than +# crashing on an empty maximum. +no_glibc="$(stub_readelf no-glibc ' There is no dynamic section in this file.')" +expect 'no glibc requirement passes' 0 "$no_glibc" 2.31 + +# A missing floor argument is a MISUSE, not a pass — a gate invoked wrongly must never be +# silently green, which is how a CI gate rots into a no-op. +expect 'a missing floor is a usage error' 2 "$at_bound" '' + +if [ "$failures" -ne 0 ]; then + printf '\n%s test(s) failed\n' "$failures" + exit 1 +fi +printf '\nall check-glibc-floor tests passed\n'