ci: fail the build when an executable embeds a standard library header path

[UdjinM6]

Issue being fixed or feature implemented

A source location captured inside a standard library header is wrong twice over: gsl::details::terminate() prints it, so a failed precondition names a standard library internal instead of the code that broke it, and the recorded path belongs to the toolchain, so on darwin it comes from the macOS SDK - whose location differs between builders and makes the release binaries irreproducible.

Nothing catches that today. 966f532 introduced exactly this bug in evo/creditpool.cpp and it went unnoticed until guix attestation for v24.0.0-rc.1 disagreed on the macOS hashes: just over seven days after the commit landed, and only because one builder happened to keep their SDK somewhere other than the rest. Comparing hashes across builders is a poor detector. It needs a second builder, a full release build, and a difference in their setups before it fires at all, and it fires at the worst possible moment.

What was done?

A check that fires on a single build instead. contrib/guix/stdlib-path-check.py parses each executable with LIEF and fails if one embeds a C++ standard library header path, wired as a check-stdlib-paths make target beside the existing check-symbols and check-security, and invoked from the guix build.

It also runs in regular CI, behind RUN_STDLIB_PATH_CHECK, enabled for the linux64 and mac targets. check-symbols could not be run there - it caps GLIBC at 2.31 (contrib/guix/symbol-check.py:31-38) while a native CI build links against the container's far newer glibc, so it asserts a property of release artifacts that a native build legitimately violates - and check-security is simply not wired into regular CI today. This check has no such dependence: outside the instrumented builds noted below, an embedded standard library header path is wrong regardless of host or toolchain. Regular CI builds from a make distdir tree (ci/dash/build_src.sh:29-37), so the script is listed in BIN_CHECKS/EXTRA_DIST.

Both targets matter, and neither is redundant. On libc++ the relocation form of this bug is invisible, because clang proves the moved-from pointer non-null and drops the branch that keeps the string alive; gcc does not. On libstdc++ it is visible. Conversely the original rc.1 form - a raw pointer forwarded into the standard library - is visible on both. So darwin alone cannot cover this class, and linux64 should not be dropped on the assumption that it does.

Only sections that hold string literals are examined: .rodata, .rdata, __cstring, __const, and anything with those prefixes, so ELF variants such as .rodata.str1.1 are included. DWARF legitimately names standard library headers for inlined template code, and skipping debug sections by name is not portable - PE stores a long section name as an offset into the string table, so they read as /81 rather than .debug_*. A first version denied those by name and drowned the mingw build in thousands of DWARF false positives. Naming the sections we want cannot pick up debug data by accident. The trade is deliberate: a literal in an unusual section would be missed, which for a gate that blocks builds is the safer direction.

Each hit reports its section, how many times the path occurs, and the first few virtual addresses, so a real literal is distinguishable at a glance from debug data, and can be attributed to the referencing code with objdump without a rebuild. A file LIEF cannot parse is also a failure rather than a silent pass.

RUN_STDLIB_PATH_CHECK defaults to false, and only linux64 and mac opt in. That is a default rather than a hard exclusion - a target that inherited the variable as true would run the check - but no target sets it today. Sanitizer and fuzz builds are deliberately not opted in: their instrumentation records source locations for its own diagnostics and is expected to name standard library headers legitimately, so enabling it there would report instrumentation as a defect. They can be revisited once we know what they actually embed.

How Has This Been Tested?

The check found a real second bug on its first run, which is the strongest evidence for it. After #7760 the linux release binaries still embedded bits/stl_construct.h; that turned out to be a separate defect in gsl::not_null, fixed in #7772, which source review had missed.

Against real artifacts:

• the pre-fix guix build of 0f87636 fails on dashd, dash-qt and test_dash - precisely the three binaries that differed between builders on rc.1 - and passes on dash-cli, dash-tx, dash-util and dash-wallet, precisely the four that were byte-identical
• discrimination was checked directly: an object file containing twelve standard library paths in total reports exactly one, the string literal in __TEXT,__cstring
• no false positives on an ordinary local non-guix build of dashd, test_dash, dash-qt and dash-cli

The two halves were separated across branches so each can be observed on its own, since this PR on its own is expected to fail until #7772 merges:

• check-stdlib-paths - this PR, the check without the fix. Regular CI linux64 and the guix libstdc++ hosts fail on /usr/include/c++/13/bits/stl_construct.h; mac and both guix darwin hosts pass. That asymmetry is the coverage point above: clang drops the branch that keeps the string alive, gcc does not.
• check-stdlib-paths-with-fix - this PR rebased on top of fix: don't re-capture a source_location when a not_null is relocated #7772, kept purely as evidence and not proposed for merge. Every guix host passes - x86_64-linux-gnu, riscv64, aarch64, powerpc64, x86_64-w64-mingw32 and both darwin - along with linux64 and mac in regular CI.

Taken together: the check fails on a tree with the bug, passes on a tree with the fix, and produces no false positives across four object-format and standard-library combinations.

Breaking Changes

None at runtime. This adds a build-time check; a build that would have produced a binary embedding such a path now fails instead.

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

[thepastaclaw]

✅ Final review complete — no blockers (commit b6b9b75) · triage: normal

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 13c0d9a1-44c4-4d8a-8c8a-e8cada4a22e4

📥 Commits

Reviewing files that changed from the base of the PR and between 7c15a39 and b6b9b75.

📒 Files selected for processing (8)

• Makefile.am
• ci/dash/build_src.sh
• ci/test/00_setup_env.sh
• ci/test/00_setup_env_mac.sh
• ci/test/00_setup_env_native_qt5.sh
• contrib/guix/libexec/build.sh
• contrib/guix/stdlib-path-check.py
• src/Makefile.am

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

Walkthrough

The change adds a Python checker that scans selected binary sections for printable paths containing include/c++/. A new Make target runs the checker on built programs. Distribution checks include the script. CI runs the target when enabled, and the Guix build runs it after security and symbol checks. Two environment configurations enable the check; the default is disabled.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CI as ci/dash/build_src.sh
  participant Guix as contrib/guix/libexec/build.sh
  participant Make as src Makefile
  participant Checker as stdlib-path-check.py
  participant Binaries as bin_PROGRAMS
  CI->>Make: Run check-stdlib-paths when RUN_STDLIB_PATH_CHECK is true
  Guix->>Make: Run check-stdlib-paths after security and symbol checks
  Make->>Checker: Pass bin_PROGRAMS
  Checker->>Binaries: Scan selected literal sections
  Checker-->>Make: Return status for matches or parse errors

Loading

Merge Risk: ⚪ Minimal · up to b6b9b

This adds a build-time check for embedded C++ standard-library header paths. It does not change runtime behavior. No merge-blocking risk was found in the supplied context.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b6b9b

The new check is confined to build-time validation and does not modify the executables it examines. Failures stop the inspected build scripts. No introduced security defect was established, but build-runner privileges, isolation, and external publication behavior remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated attackable surface is executable content supplied to the build-job parser, including content influenced by source changes. Any parser compromise would inherit the build process's authority; tenant, credential, host, and environment exposure cannot be bounded further from the inspected callers.

Trust Boundaries and Controls

• observed — Repository callers supply built executable paths rather than runtime requests. Guix already parses the same executable set through security and symbol checks immediately before the new check. Regular CI gains this parsing step, so existing Guix parser exposure does not establish equivalent CI isolation.

Resilience and Maintainability Implications

• observed — Findings and parse failures retain a failing exit status even when scanning continues. Shell fail-stop behavior prevents a failed check from reaching later Guix packaging and output replacement. Repeated checks use fresh local state and do not mutate artifacts. Pre-existing final output files are removed only at publication, so their treatment after a failed rerun remains an external-consumer coverage gap, not an established PR-introduced defect.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 6 files. (2 skipped: 2 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main change: failing builds when executables embed standard-library header paths.
Description check	✅ Passed	The description is directly related to the changeset. It explains the new checker, build integration, CI configuration, scope, rationale, and testing results.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 6 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🧪 Generate unit tests (beta)

• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[thepastaclaw]

Final validation — Phase 1 + Phase 2

The checker, source-distribution wiring, and Guix/CI invocation are consistent with the PR's stated goal. One non-blocking coverage gap is confirmed: the regex misses Debian/Ubuntu multiarch standard-library header paths; no supplied finding establishes a blocking defect.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: dash-core-commit-history); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — The change introduces executable section parsing and path detection across Guix and native CI build gates, requiring ordinary tooling review but changing no critical runtime surface.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — dash-core-commit-history (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 13% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort high); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `contrib/guix/stdlib-path-check.py`:
- [SUGGESTION] contrib/guix/stdlib-path-check.py:40-42: Regex misses Debian multiarch stdlib header paths
  The pattern requires the contiguous substring `include/c++/`, so it misses libstdc++ headers installed under Debian/Ubuntu's `include/<triplet>/c++/<version>/` layout. Testing the exact regex from this file confirms that `/usr/include/c++/13/bits/stl_construct.h` matches, while `/usr/include/x86_64-linux-gnu/c++/13/bits/c++config.h` does not. An embedded literal naming a header in that directory would therefore pass the new gate, despite being a standard-library header path. This does not undermine detection of the forwarding-wrapper regressions described in the PR, whose headers use the main include directory, so this is non-blocking. Allow intervening directory components while retaining the existing printable-byte restriction.

[thepastaclaw]

🟡 Suggestion: Regex misses Debian multiarch stdlib header paths

The pattern requires the contiguous substring include/c++/, so it misses libstdc++ headers installed under Debian/Ubuntu's include/<triplet>/c++/<version>/ layout. Testing the exact regex from this file confirms that /usr/include/c++/13/bits/stl_construct.h matches, while /usr/include/x86_64-linux-gnu/c++/13/bits/c++config.h does not. An embedded literal naming a header in that directory would therefore pass the new gate, despite being a standard-library header path. This does not undermine detection of the forwarding-wrapper regressions described in the PR, whose headers use the main include directory, so this is non-blocking. Allow intervening directory components while retaining the existing printable-byte restriction.

Suggested change

	# Matches the include directory of both libc++ (include/c++/v1) and libstdc++
	# (include/c++/<version>), wherever the toolchain or sysroot places it.
	STDLIB_INCLUDE = re.compile(rb'[\x20-\x7e]*include/c\+\+/[\x20-\x7e]*')
	# Matches the include directory of both libc++ (include/c++/v1) and libstdc++
	# (include/c++/<version>), including multiarch layouts such as
	# include/<triplet>/c++/<version>, wherever the toolchain or sysroot places it.
	STDLIB_INCLUDE = re.compile(rb'[\x20-\x7e]*include/(?:[-\w.]+/)*c\+\+/[\x20-\x7e]*')

_{source: muse-spark-1.3-contributor (phase1-reviewer: general)}