FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

harden: remove capability that should never have shipped by DeusData · Pull Request #1344 · DeusData/codebase-memory-mcp · GitHub

harden: remove capability that should never have shipped - #1344

Merged
DeusData merged 2 commits into
mainfrom
harden/av-margin-rc2
Jul 29, 2026
Merged

harden: remove capability that should never have shipped#1344
DeusData merged 2 commits into
mainfrom
harden/av-margin-rc2

Conversation

Copy link
Copy Markdown
Owner

What

Removes capability that should never have shipped, and adds a release gate that proves it stays removed. Driven by a private static audit of the rc.1 artifacts after Microsoft's ML flagged them.

The framing matters, because it sets what this PR can and cannot claim. The same sha256 (9e2f0f12…, linux-amd64) went from 0/62 clean to Microsoft-detected in about an hour with no byte change — our own run-134 log versus a later rescan of the identical file. So part of that verdict lives in scanner-side state, and no code change can promise a clean scan. What code can do is stop shipping malware-shaped surface that has no business in a release artifact. Every item below stands on its own security merit; a wider classifier margin is a side effect, not the justification.

The findings

Executable stack on every Linux binary we have ever shipped. vendored/nomic/code_vectors_blob.S is the only assembly in the build and carried no .note.GNU-stack. An unannotated object makes ld assume the worst for the whole link, so every release had GNU_STACK RWE. Fixed at the cause (the note) and the outcome (ELF-only -Wl,-z,noexecstack). Verified on a real Linux ELF: RWE → RW.

A test probe that forks a SIGTERM-ignoring immortal child and writes its pid to a caller-supplied path shipped in production (src/main.c). Test seams are now opt-in (TEST_SEAMS=1 → CBM_ENABLE_TEST_SEAMS); disabled builds compile a trivial success stub so call sites are untouched and the binary holds no fork, no signal handler and no env-var string. Opt-in deliberately: forgetting the flag now yields a clean binary rather than a leaky one.

The daemon spawned curl against the GitHub releases API on the first eligible session of every run, to say "a newer version exists". That put a release URL and an outbound request in every shipped binary. Deleted. The injectable seam survives — update_ops is still honoured and the fakes in tests/test_daemon_application.c still cover notice / ownership / cancellation / generation-replay — and with no provider application_update_subscribe_locked returns early, so no generation ever starts. "No network request by default" is now structural, not a promise.

Dead capability. The tar.gz/zip extraction block (verified self-contained, zero uses of any helper outside it, only callers the already-excluded updater and tests/test_cli.c) moves under CBM_CLI_ENABLE_TEST_API. Download → decompress → pick an executable → mark it executable is the canonical dropper composite; it is now absent, not merely unreachable. SQLite is built -DSQLITE_OMIT_LOAD_EXTENSION (no caller anywhere in src/ or internal/).

Temp files and env scanning. Predictable /tmp/cbm_search_<pid>.pat-style paths opened with plain fopen → private cbm_mkdtemp directory + cbm_mkstemp exclusive creation, written through the returned descriptor and never reopened by name. pass_envscan.c no longer descends symlinked directories out of the project root (lstat on POSIX, reparse-point screening on Windows where lstat does not exist), and its fixed 512-byte buffers no longer truncate into pointer arithmetic that could land outside the buffer.

Build-time entropy. mimalloc's banner baked __DATE__/__TIME__ into every binary, so two builds of identical source seconds apart could never share a hash — and no release could ever inherit a false-positive determination made about its predecessor. Local patch removes it (marked to survive refreshes), -Wdate-time makes any future use a build error, -Wl,--no-insert-timestamp stops the PE header carrying the link clock.

The gate

scripts/ci/check-binary-composition.sh, wired into package-release.sh after strip (the last byte-changing step) so the local artifact-flow smoke enforces exactly what the release does. Asserts absences — test seams, updater URLs, SQLite load-extension, UI/HTTP in standard artifacts, non-executable stack — plus a canary string, because an absence check aimed at a compressed, stubbed or empty file would otherwise pass vacuously and read green. A missing tool is a hard error: a skipped assertion must never look like a satisfied one.

It earned its keep immediately. Both SQLITE_OMIT_LOAD_EXTENSION and the mimalloc patch silently compiled to nothing on the first rebuild, because prod_sqlite3.o / prod_mimalloc.o depended on a single named source and make never recompiled them. Source review would have called both "done" — this is very likely why earlier fixes in this saga appeared to work for one release and then stopped. Now fixed: those objects depend on Makefile.cbm (and mimalloc, an amalgamation, on its included sources), plus a .build-config stamp that removes the binary on any config change so the guarantee does not rest on mtime granularity.

Deliberately NOT done

  • Three seams stay in release artifacts: CBM_TEST_CRASH_ON, CBM_TEST_HANG_ON, CBM_TEST_WINDOWS_USER_PATH_RUN_ID. scripts/smoke-test.sh runs against the real release artifact and needs them — the run-id is what stops the PATH smoke writing the tester's actual PATH. Removing them would trade genuine release-artifact coverage for a cosmetic win. The gate treats them as an allowlist, so a novel seam still fails.
  • No-UI standard build deferred. src/ui/* is in PROD_SRCS and four files outside src/ui reference UI symbols (src/main.c, src/daemon/host.c, src/daemon/application.c, src/mcp/index_supervisor.c), including the daemon that serves the UI. That assertion reports INFO until the split lands (CBM_CHECK_UI_ABSENT=1 enforces): a gate everyone knows is red teaches people to ignore gates. All six UI needles were verified unique to src/ui/ so it will bite correctly when enabled.
  • No grammar removed. ObjectScript is ~100% of the binary growth since the last provably-clean release (+21.4MB rodata, +1.1MB text from two four-line shims), which made it the obvious ablation candidate — but a dry run performed twelve real Defender endpoint scans across standard/UI × amd64/arm64 with ObjectScript, the daemon and the expanded hooks all present, and every scan was clean. Nothing there is a deterministic trigger, so cutting a community-contributed language would spend a real feature on unproven margin. Lean is not a candidate either: at 99.6MB of source it is by far the largest grammar, but it shipped in v0.9.0 which scanned 20/20 clean, so removing it would produce a novel unscanned profile instead of restoring a known-good one.

Also in this PR

README.md was actively wrong, not merely incomplete: it documented in-place updates for macOS/Linux and promised "The MCP server also checks for updates on startup and notifies on the first tool call" — a claim that died with the MCP update thread and then the daemon's curl check. Rewritten to state the one contract that holds everywhere, why it works that way, and the affirmative property that replaced it. uninstall now reports the install script beside the binary (path + rm command, "would leave" under --dry-run) but does not delete it: that file may be the user's own copy, a symlink into a checkout, or package-manager managed, and an uninstaller must not delete what it cannot prove it owns.

Verification

  • macOS C suite: 5166 passed, 0 failed, 0 skipped.
  • Linux: build + composition gate green, 13/13 assertions on an unstripped ELF (stronger than a stripped one for the symbol-absence checks), RWE → RW confirmed against the flagged artifact.
  • Windows (real ARM64 VM): 726 passed, 0 real failures; all guards green including test_windows_update_handoff.py ("update handed off to install.ps1 without touching its own image"). One reported failure was an invalid suite name in my own runner script, re-run clean with names verified against --list-suites.
  • Full local contract set green, including venue-parity over the edited workflow-adjacent scripts.
  • New precondition in tests/test_worker_watchdog.sh: it asserts the seam is present and says how to get it, instead of dying later with an opaque Killed: 9.

Risk

Behaviour-preserving by construction for the removals (dead or test-only code). The two real behaviour changes are the daemon no longer performing a background version check (intended, documented) and uninstall printing one extra section. The gate is release-blocking, which is the point — but it fails closed on a missing tool or an unreadable file, so a broken toolchain surfaces as a red gate rather than a silent pass.

DeusData added 2 commits July 29, 2026 23:50
Microsoft's ML flagged the rc.1 release binaries. The decisive evidence is that
the SAME sha256 went from 0/62 clean to Microsoft-detected in about an hour with
no byte change, so the verdict lives partly in scanner-side state and no code
change can promise a clean result. What code CAN do is stop shipping things that
have no business in a release artifact, which is worth doing on its own merits
and incidentally widens the classifier margin. Every claim below is verified
against a built binary by the new gate, not by reading source.

Executable stack (the worst of the findings). vendored/nomic/code_vectors_blob.S
is the only assembly in the build and carried no .note.GNU-stack. An unannotated
object makes ld assume the worst for the whole link, so EVERY Linux release we
have ever shipped had GNU_STACK RWE. Adds the note (cause) plus ELF-only
-Wl,-z,noexecstack (outcome); the gate fails the release if it returns.

Test seams are now opt-in, never opt-out. TEST_SEAMS=1 defines
CBM_ENABLE_TEST_SEAMS; without it the crash-orphan probe -- which forks a child
that ignores SIGTERM and loops forever, then writes its pid to a caller-supplied
path -- and the lease-ownership marker compile to trivial stubs, so call sites are
untouched and the binary holds no fork, no signal handler and no env-var string.
Opt-IN is the point: forgetting the flag yields a clean binary rather than a leaky
one. scripts/test.sh requests it in the leg that consumes it, and
tests/test_worker_watchdog.sh now asserts the capability up front instead of
dying later with an opaque "Killed: 9".

The daemon's background version check is gone. It spawned curl against
api.github.com/repos/.../releases/latest on the first eligible session of every
run to say "a newer version exists" -- a release URL and an outbound request in
every shipped binary, for something the install scripts already report. The
INJECTABLE SEAM survives: update_ops is still honoured, the fakes in
tests/test_daemon_application.c still cover notice/ownership/cancellation/replay,
and with no provider application_update_subscribe_locked returns early so no
generation ever starts. "No network request by default" is now structural.

Dead capability out of release builds. The tar.gz/zip extraction block
(gzip_decompress through cbm_extract_binary_from_zip, plus its cli.h
declarations) moves under CBM_CLI_ENABLE_TEST_API -- verified self-contained, zero
uses of any helper outside it, only callers the excluded updater and
tests/test_cli.c. Downloading an archive, decompressing it, picking an executable
out of it and marking it executable is the canonical dropper composite; it is now
absent rather than merely unreachable. SQLite is built with
-DSQLITE_OMIT_LOAD_EXTENSION (no caller of load_extension anywhere in src/ or
internal/), removing that API surface and part of the dlopen/dlsym surface.

Temp files and environment scanning (S2/S3). Predictable paths in mcp.c,
artifact.c and diagnostics.c are created privately and exclusively and written
through the returned descriptor; pass_envscan.c no longer descends symlinked
directories out of the project root, and its fixed 512-byte path buffers no
longer truncate into pointer arithmetic that could land outside the buffer.

Build-time entropy. mimalloc's version banner baked __DATE__/__TIME__ into every
binary, so two builds of identical source seconds apart could never share a hash
and no release could inherit a false-positive determination made about its
predecessor. Local patch removes it (marked to survive refreshes), -Wdate-time
makes any future use a build error, and -Wl,--no-insert-timestamp stops the PE
header carrying the link clock.

scripts/ci/check-binary-composition.sh is the proof that each removal stays
removed, wired into package-release.sh after strip so the local artifact-flow
smoke enforces exactly what CI does. It asserts absences plus a CANARY string, so
handing it a compressed, stubbed or empty file fails instead of passing
vacuously, and a missing tool is a hard error -- a skipped assertion must never
look like a satisfied one.

Two build-system traps found by that gate, both of which had silently defeated a
fix: the product binary is compiled in one shot from sources, so a flag flip did
not rebuild it (now tracked by a .build-config stamp that also removes the
binary, making it independent of mtime granularity); and prod_sqlite3.o /
prod_mimalloc.o depended on a single named source, so SQLITE_OMIT_LOAD_EXTENSION
and the mimalloc patch BOTH compiled to nothing on the first incremental build.
Source review would have called them done.

Deliberately NOT changed. Three seams stay in release artifacts because
scripts/smoke-test.sh runs against the real artifact and needs them:
CBM_TEST_CRASH_ON and CBM_TEST_HANG_ON inject the faults that prove supervisor
recovery, and CBM_TEST_WINDOWS_USER_PATH_RUN_ID is what keeps the PATH smoke from
writing the tester's actual PATH. The gate treats those as an allowlist, so a
NOVEL seam still fails. The true no-UI standard build is deferred rather than
rushed: src/ui/* is in PROD_SRCS and four files outside src/ui reference UI
symbols, including the daemon that serves the UI, so that assertion reports
instead of failing until the split lands -- a gate everyone knows is red teaches
people to ignore gates.

No grammar is removed. ObjectScript accounts for essentially all binary growth
since the last provably-clean release (+21.4MB rodata, +1.1MB text from two
four-line shims), which made it the obvious ablation candidate, but a dry run
performed twelve real Defender endpoint scans across standard/UI and amd64/arm64
with ObjectScript, the daemon and the expanded hooks all present and every scan
was clean. Nothing there is a deterministic trigger, so cutting a
community-contributed language would spend a real feature on unproven margin.
Lean is not a candidate either: at 99.6MB of source it is by far the largest
grammar, but it shipped in v0.9.0 which scanned 20/20 clean, so removing it would
produce a novel unscanned profile instead of restoring a known-good one.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The update handoff moved out of the product on every platform, but two loose
ends were left behind.

README was actively WRONG, not merely incomplete. It still documented in-place
updates for macOS/Linux, and it promised "The MCP server also checks for updates
on startup and notifies on the first tool call" -- a claim that died with the MCP
update thread and then the daemon's curl check. The section now states the one
contract that holds everywhere (`update` validates flags and prints the install
script command), why it works that way (on Windows a running image cannot
replace itself; on POSIX a deliberate choice, because an in-process updater is
structurally a downloader), and the affirmative property that replaced it: cbm
makes no network request of its own accord and the archives carry no download
URLs at all.

`uninstall` now REPORTS the install script it finds beside the binary, printing
the path and the `rm` command, and says "would leave" under --dry-run. It does
not delete it. install.sh places itself there so `update` has something to hand
off to, but that file may equally be the user's own copy, a symlink into a
checkout, or package-manager managed -- an uninstaller must not delete a file it
cannot prove it owns, and one printed line leaves the decision where it belongs.
Windows checks install.ps1 too and tolerates either path separator.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL