Skip to content

Commit e9ce367

Browse files
authored
Merge pull request DeusData#1338 from DeusData/fix/update-handoff-and-determinism
fix: install-script update handoff, Mach-O strip, security alerts, Windows determinism
2 parents 7dd8d22 + ae52db3 commit e9ce367

12 files changed

Lines changed: 208 additions & 37 deletions

‎graph-ui/package-lock.json‎

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎install.ps1‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,28 @@ if ($LASTEXITCODE -ne 0) {
300300
exit 1
301301
}
302302

303+
# Place the installer beside the binary so `update` points at a local file
304+
# rather than a URL, and so the next update runs THIS release's installer.
305+
#
306+
# Sourced from the archive we just checksum-verified, and published by rename
307+
# rather than written over the live path. PowerShell parses a script fully
308+
# before executing it, so self-overwrite is less hazardous here than it is for
309+
# bash -- but rename costs nothing and keeps both platforms on one rule.
310+
# Best effort: a failure here still leaves a working install.
311+
$DownloadedInstaller = Join-Path $TmpDir "install.ps1"
312+
if (Test-Path -LiteralPath $DownloadedInstaller -PathType Leaf) {
313+
$InstallerDest = Join-Path $InstallDir "install.ps1"
314+
$InstallerTmp = "$InstallerDest.new"
315+
try {
316+
Copy-Item -LiteralPath $DownloadedInstaller -Destination $InstallerTmp -Force -ErrorAction Stop
317+
Move-Item -LiteralPath $InstallerTmp -Destination $InstallerDest -Force -ErrorAction Stop
318+
Write-Host "Installed updater -> $InstallerDest"
319+
} catch {
320+
Remove-Item -LiteralPath $InstallerTmp -Force -ErrorAction SilentlyContinue
321+
Write-Host "note: could not place install.ps1 in $InstallDir (update will explain where to find it)"
322+
}
323+
}
324+
303325
# Verify
304326
try {
305327
$ver = & $Dest --version 2>&1

‎install.sh‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,32 @@ if [ "$SKIP_CONFIG" = true ]; then
268268
fi
269269
"$DLBIN" install "${INSTALL_ARGS[@]}"
270270

271+
# Place the installer beside the binary so `update` can point at a local file
272+
# instead of a URL, and so the next update uses THIS release's installer.
273+
#
274+
# Two things make this safe. The source is the copy from the archive we just
275+
# checksum-verified -- never "$0", which does not exist under `curl | bash` and
276+
# would pin us to the OLD installer forever. And it is published by atomic
277+
# rename, never by writing over the live path: bash reads a script incrementally
278+
# by byte offset, so overwriting the file it is executing continues reading the
279+
# NEW bytes at the OLD offset. That fails silently and bizarrely, which is worse
280+
# than failing loudly.
281+
#
282+
# Best effort by design: a user who cannot write to INSTALL_DIR still gets a
283+
# working install, and `update` falls back to explaining where to find it.
284+
DL_INSTALLER="$DLDIR/install.sh"
285+
if [ -f "$DL_INSTALLER" ]; then
286+
INSTALLER_TMP="$INSTALL_DIR/.install.sh.$$"
287+
if cp "$DL_INSTALLER" "$INSTALLER_TMP" 2>/dev/null &&
288+
chmod 755 "$INSTALLER_TMP" 2>/dev/null &&
289+
mv -f "$INSTALLER_TMP" "$INSTALL_DIR/install.sh" 2>/dev/null; then
290+
echo "Installed updater -> $INSTALL_DIR/install.sh"
291+
else
292+
rm -f "$INSTALLER_TMP" 2>/dev/null || true
293+
echo "note: could not place install.sh in $INSTALL_DIR (update will explain where to find it)"
294+
fi
295+
fi
296+
271297
# Verify
272298
VERSION=$("$DEST" --version 2>&1) || {
273299
echo "error: installed binary failed to run" >&2

‎scripts/package-release.sh‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -113,21 +113,31 @@ NAME="codebase-memory-mcp${SUFFIX}-${GOOS}-${GOARCH}"
113113
strip_release_binary() {
114114
local binary="$1"
115115
[ -f "$binary" ] || return 0
116-
# --strip-all on every format, Mach-O included.
116+
# The right flags differ per format, and the WRONG ones fail silently in
117+
# the dangerous direction. Measured on the flagged darwin-arm64 artifact:
117118
#
118-
# This first shipped as `strip -x` on Mach-O out of caution that a full
119-
# strip could leave an image dyld will not load. That caution was wrong for
120-
# this binary and it cost us a release cycle: -x retains external symbols --
121-
# 4058 of them -- so the macOS artifacts kept the very symbol table the ELF
122-
# legs had just shed, and they were the only ones VirusTotal then flagged.
123-
# Measured on the flagged darwin-arm64 artifact: --strip-all leaves 373
124-
# symbols, `codesign --verify` passes, the binary runs, and the scan goes
125-
# from 1 malicious to 0/61 clean.
119+
# llvm-strip --strip-all 373 symbols scanned CLEAN
120+
# strip (no flags) 378 symbols equivalent
121+
# strip -x -S 4058 symbols the state VirusTotal FLAGGED
122+
# strip -X / -u -r 4058 symbols likewise
123+
#
124+
# Apple's strip returns success for `-x -S`, so a helper that just tries
125+
# candidates until one exits 0 would quietly reship the flagged binary.
126+
# GNU/LLVM `--strip-all` is not even accepted by Apple's strip, which is why
127+
# generalising it to every platform broke the macOS build -- loudly, which
128+
# was the lucky outcome.
129+
#
130+
# So: --strip-all where it is understood, plain `strip` for Mach-O, and a
131+
# hard error when no candidate can do the job. Never a weaker fallback.
126132
local stripped=""
127133
for tool in "${STRIP:-}" llvm-strip strip; do
128134
[ -n "$tool" ] || continue
129135
command -v "$tool" >/dev/null 2>&1 || continue
130-
"$tool" --strip-all "$binary" 2>/dev/null && stripped="$tool"
136+
if "$tool" --strip-all "$binary" 2>/dev/null; then
137+
stripped="$tool --strip-all"
138+
elif [ "$GOOS" = "darwin" ] && "$tool" "$binary" 2>/dev/null; then
139+
stripped="$tool"
140+
fi
131141
[ -n "$stripped" ] && break
132142
done
133143
if [ -z "$stripped" ]; then

‎scripts/run-test-wave.py‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,15 +123,55 @@ def start_suite(
123123
)
124124

125125

126+
def windows_descendants(pid: int, timeout: int) -> bool:
127+
"""True if any live process still claims `pid` as its parent.
128+
129+
Used only when the suite leader has already exited: `taskkill /T` cannot
130+
walk a tree from a dead PID, so cleanup is proven by asking whether anything
131+
is still parented to it. One level deep on purpose -- Windows does not
132+
reparent orphans, so a grandchild keeps pointing at its own (dead) parent
133+
and would not be found here. That is a weaker proof than taskkill /T, which
134+
is why it is reserved for the case where the strong proof is impossible.
135+
"""
136+
try:
137+
completed = subprocess.run(
138+
[
139+
"powershell.exe",
140+
"-NoProfile",
141+
"-NonInteractive",
142+
"-Command",
143+
"@(Get-CimInstance Win32_Process -Filter "
144+
f"'ParentProcessId={pid}').Count",
145+
],
146+
check=False,
147+
stdin=subprocess.DEVNULL,
148+
capture_output=True,
149+
text=True,
150+
timeout=timeout,
151+
)
152+
except (OSError, subprocess.TimeoutExpired):
153+
return True # cannot prove absence -> assume the worst
154+
if completed.returncode != 0:
155+
return True
156+
return (completed.stdout or "").strip() not in ("0", "")
157+
158+
126159
def terminate_process_tree(active: ActiveSuite, kill_grace: int) -> None:
127160
process = active.process
128161
leader_exited = process.poll() is not None
129162
if os.name == "nt":
130163
if leader_exited:
131-
raise RuntimeError(
132-
f"suite {active.name!r} leader exited before Windows tree cleanup "
133-
"could be proven"
134-
)
164+
# The leader can exit on its own between the timeout decision and
165+
# this call. Refusing outright made the harness itself lose a race:
166+
# a natural exit at the wrong moment failed the whole wave, which is
167+
# how a deliberately-hanging fixture suite reddened a release run.
168+
# taskkill /T cannot walk a tree from a dead PID, so prove cleanup
169+
# the only way still available -- nothing is parented to it.
170+
if windows_descendants(process.pid, kill_grace):
171+
raise RuntimeError(
172+
f"suite {active.name!r} leader exited leaving live descendants"
173+
)
174+
return
135175
try:
136176
completed = subprocess.run(
137177
[

‎scripts/security-allowlist.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ src/ui/http_server.c:execl:exec indexing binary in child process
6666
URL:https://api.github.com/repos/DeusData/codebase-memory-mcp/releases/latest:update check
6767
URL:https://github.com/DeusData/codebase-memory-mcp/releases/latest/download:binary download + checksums
6868
URL:https://github.com/DeusData/codebase-memory-mcp/releases/latest:version check via redirect header
69-
URL:https://raw.githubusercontent.com/DeusData/codebase-memory-mcp/main/install.sh:printed by `update` as the install-script command when install.sh is not beside the binary (a string in the message, not a network call - the binary no longer downloads anything)
7069
URL:http://127.0.0.1:UI server binding (localhost only)
7170
URL:https://www.sqlite.org/c3ref/c_checkpoint_full.html:sqlite WAL checkpoint API doc reference (comment only, not a network call)
7271
URL:https://github.com/DeusData/codebase-memory-mcp:project repository self-reference in update/star notice (src/mcp/mcp.c)

‎scripts/smoke-test.sh‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,27 @@ fi
151151
trap 'smoke_rmtree "$TMPDIR" "${DRYRUN_HOME:-}"' EXIT
152152

153153
CLI_STDERR=$(smoke_mktemp_file)
154-
cli() { "$BINARY" cli "$@" 2>"$CLI_STDERR"; }
154+
# 10 of the cli call sites assign directly (VAR=$(cli ...)). Under
155+
# `set -euo pipefail` a non-zero exit there kills the smoke with NOTHING
156+
# printed: no FAIL line, no stderr, just an abort indistinguishable from a hang,
157+
# a starved runner, or a real regression. One such abort cost a full Windows
158+
# cycle just to locate, and still could not be attributed. Surface the command
159+
# and its stderr here, while we still can.
160+
#
161+
# Neutral wording on purpose: one call site deliberately expects a non-zero exit
162+
# (the unknown-function query must error loudly), so this must not read as a
163+
# failure on its own.
164+
cli() {
165+
local rc=0
166+
"$BINARY" cli "$@" 2>"$CLI_STDERR" || rc=$?
167+
if [ "$rc" -ne 0 ]; then
168+
{
169+
printf 'cli: `%s` exited %s\n' "$*" "$rc"
170+
sed 's/^/ /' "$CLI_STDERR" 2>/dev/null
171+
} >&2
172+
fi
173+
return "$rc"
174+
}
155175

156176
echo "=== Phase 1: version ==="
157177
VERSION_STATUS=0

‎src/cli/cli.c‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11168,12 +11168,6 @@ static bool check_already_latest(void) {
1116811168

1116911169
#endif /* CBM_CLI_ENABLE_TEST_API */
1117011170

11171-
/* One token on purpose: the security audit extracts URLs literally, so a URL
11172-
* split across string-literal continuations cannot be matched against the
11173-
* allow-list. Printed for the user to paste; never fetched by this binary. */
11174-
#define CBM_INSTALL_SH_URL \
11175-
"https://raw.githubusercontent.com/DeusData/codebase-memory-mcp/main/install.sh"
11176-
1117711171
int cbm_cmd_update(int argc, char **argv) {
1117811172
parse_auto_answer(argc, argv);
1117911173

@@ -11253,7 +11247,9 @@ int cbm_cmd_update(int argc, char **argv) {
1125311247
if (have_dir) {
1125411248
printf(" powershell -ExecutionPolicy Bypass -File \"%s\\install.ps1\"\n\n", self_dir);
1125511249
} else {
11256-
printf(" powershell -ExecutionPolicy Bypass -File install.ps1\n\n");
11250+
printf(" powershell -ExecutionPolicy Bypass -File install.ps1\n"
11251+
" (ships in the release archive, and is placed beside the\n"
11252+
" binary on install)\n\n");
1125711253
}
1125811254
printf("It downloads the latest release, verifies its checksum, and replaces\n"
1125911255
"this binary in place. If PowerShell refuses to run the script because\n"
@@ -11263,7 +11259,8 @@ int cbm_cmd_update(int argc, char **argv) {
1126311259
if (have_dir) {
1126411260
printf(" bash \"%s/install.sh\"\n\n", self_dir);
1126511261
} else {
11266-
printf(" curl -fsSL " CBM_INSTALL_SH_URL " | bash\n\n");
11262+
printf(" install.sh (ships in the release archive, and is placed\n"
11263+
" beside the binary on install)\n\n");
1126711264
}
1126811265
printf("It downloads the latest release, verifies its checksum, and replaces\n"
1126911266
"this binary in place. install.sh is idempotent, so re-running it IS\n"

‎test-infrastructure/Dockerfile.glibc22‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66
# 2. the DYNAMIC binary refuses to start (its glibc 2.38+ floor is by
77
# design — old-glibc coverage is the portable binary's job, and a
88
# dynamic binary that silently ran here would mask a floor regression).
9-
FROM ubuntu:22.04
9+
# Pinned by digest like every other venue image: a floating tag means the
10+
# glibc-FLOOR assertion silently drifts to whatever ubuntu:22.04 points at, and
11+
# a floor that moves is not a floor. (Scorecard PinnedDependenciesID #76 — this
12+
# was the one image the pinning pass missed.)
13+
FROM ubuntu:22.04@sha256:0e0a0fc6d18feda9db1590da249ac93e8d5abfea8f4c3c0c849ce512b5ef8982
1014

1115
RUN apt-get update && apt-get install -y --no-install-recommends \
1216
bash \

‎tests/test_daemon_application.c‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2855,10 +2855,17 @@ TEST(daemon_application_cancels_physical_job_only_after_final_session) {
28552855
started[i] = cbm_thread_create(&threads[i], 0, app_request_thread, &requests[i]) == 0;
28562856
}
28572857
bool subscribed = started[0] && started[1] && app_wait_for_subscribers(application, project, 2);
2858-
if (subscribed) {
2858+
/* The physical job starts asynchronously once a session subscribes, so a
2859+
* subscriber count of 2 does NOT imply it has started. Cancelling both
2860+
* sessions before that point leaves starts == 0 -- arguably the correct
2861+
* outcome, and the reason this test failed intermittently on Windows while
2862+
* passing everywhere else. Wait for the state the assertions below actually
2863+
* require; `starts` only increments, so this cannot miss the transition. */
2864+
bool job_started = subscribed && app_wait_for_atomic_int(&fake.starts, 1);
2865+
if (job_started) {
28592866
callbacks.session_cancel(callbacks.context, sessions[0]);
28602867
}
2861-
bool one_left = subscribed && app_wait_for_subscribers(application, project, 1);
2868+
bool one_left = job_started && app_wait_for_subscribers(application, project, 1);
28622869
int cancels_after_first = atomic_load(&fake.cancels);
28632870
if (one_left) {
28642871
callbacks.session_cancel(callbacks.context, sessions[1]);
@@ -2874,6 +2881,7 @@ TEST(daemon_application_cancels_physical_job_only_after_final_session) {
28742881

28752882
ASSERT_TRUE(setup);
28762883
ASSERT_TRUE(subscribed);
2884+
ASSERT_TRUE(job_started);
28772885
ASSERT_TRUE(one_left);
28782886
ASSERT_EQ(atomic_load(&fake.starts), 1);
28792887
ASSERT_EQ(cancels_after_first, 0);

0 commit comments

Comments
 (0)