diff --git a/graph-ui/package-lock.json b/graph-ui/package-lock.json index 000a7f0ab..8c6ae46ce 100644 --- a/graph-ui/package-lock.json +++ b/graph-ui/package-lock.json @@ -5134,9 +5134,9 @@ } }, "node_modules/nanoid": { - "version": "3.3.11", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.11.tgz", - "integrity": "sha512-N8SpfPUnUp1bK+PMYW8qSWdl9U+wwNWI4QKxOYDy9JAro3WMX7p2OeVRF9v+347pnakNevPmiHhNmZ2HbFA76w==", + "version": "3.3.16", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz", + "integrity": "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==", "dev": true, "funding": [ { @@ -5230,9 +5230,9 @@ } }, "node_modules/postcss": { - "version": "8.5.14", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.14.tgz", - "integrity": "sha512-SoSL4+OSEtR99LHFZQiJLkT59C5B1amGO1NzTwj7TT1qCUgUO6hxOvzkOYxD+vMrXBM3XJIKzokoERdqQq/Zmg==", + "version": "8.5.24", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.24.tgz", + "integrity": "sha512-8RyVklq0owXUTa4xlpzu4l9AaVKIdQvAcOHZWaMh98HgySsUtxRVf/chRe3dsSLqb6i40BzGRzEUddRaI+9TSw==", "dev": true, "funding": [ { @@ -5250,7 +5250,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.11", + "nanoid": "^3.3.16", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, diff --git a/install.ps1 b/install.ps1 index f455c983d..890dfd4f8 100644 --- a/install.ps1 +++ b/install.ps1 @@ -300,6 +300,28 @@ if ($LASTEXITCODE -ne 0) { exit 1 } +# Place the installer beside the binary so `update` points at a local file +# rather than a URL, and so the next update runs THIS release's installer. +# +# Sourced from the archive we just checksum-verified, and published by rename +# rather than written over the live path. PowerShell parses a script fully +# before executing it, so self-overwrite is less hazardous here than it is for +# bash -- but rename costs nothing and keeps both platforms on one rule. +# Best effort: a failure here still leaves a working install. +$DownloadedInstaller = Join-Path $TmpDir "install.ps1" +if (Test-Path -LiteralPath $DownloadedInstaller -PathType Leaf) { + $InstallerDest = Join-Path $InstallDir "install.ps1" + $InstallerTmp = "$InstallerDest.new" + try { + Copy-Item -LiteralPath $DownloadedInstaller -Destination $InstallerTmp -Force -ErrorAction Stop + Move-Item -LiteralPath $InstallerTmp -Destination $InstallerDest -Force -ErrorAction Stop + Write-Host "Installed updater -> $InstallerDest" + } catch { + Remove-Item -LiteralPath $InstallerTmp -Force -ErrorAction SilentlyContinue + Write-Host "note: could not place install.ps1 in $InstallDir (update will explain where to find it)" + } +} + # Verify try { $ver = & $Dest --version 2>&1 diff --git a/install.sh b/install.sh index 423464159..e58a28294 100755 --- a/install.sh +++ b/install.sh @@ -268,6 +268,32 @@ if [ "$SKIP_CONFIG" = true ]; then fi "$DLBIN" install "${INSTALL_ARGS[@]}" +# Place the installer beside the binary so `update` can point at a local file +# instead of a URL, and so the next update uses THIS release's installer. +# +# Two things make this safe. The source is the copy from the archive we just +# checksum-verified -- never "$0", which does not exist under `curl | bash` and +# would pin us to the OLD installer forever. And it is published by atomic +# rename, never by writing over the live path: bash reads a script incrementally +# by byte offset, so overwriting the file it is executing continues reading the +# NEW bytes at the OLD offset. That fails silently and bizarrely, which is worse +# than failing loudly. +# +# Best effort by design: a user who cannot write to INSTALL_DIR still gets a +# working install, and `update` falls back to explaining where to find it. +DL_INSTALLER="$DLDIR/install.sh" +if [ -f "$DL_INSTALLER" ]; then + INSTALLER_TMP="$INSTALL_DIR/.install.sh.$$" + if cp "$DL_INSTALLER" "$INSTALLER_TMP" 2>/dev/null && + chmod 755 "$INSTALLER_TMP" 2>/dev/null && + mv -f "$INSTALLER_TMP" "$INSTALL_DIR/install.sh" 2>/dev/null; then + echo "Installed updater -> $INSTALL_DIR/install.sh" + else + rm -f "$INSTALLER_TMP" 2>/dev/null || true + echo "note: could not place install.sh in $INSTALL_DIR (update will explain where to find it)" + fi +fi + # Verify VERSION=$("$DEST" --version 2>&1) || { echo "error: installed binary failed to run" >&2 diff --git a/scripts/package-release.sh b/scripts/package-release.sh index bc2faa76e..6bc98c2be 100755 --- a/scripts/package-release.sh +++ b/scripts/package-release.sh @@ -113,21 +113,31 @@ NAME="codebase-memory-mcp${SUFFIX}-${GOOS}-${GOARCH}" strip_release_binary() { local binary="$1" [ -f "$binary" ] || return 0 - # --strip-all on every format, Mach-O included. + # The right flags differ per format, and the WRONG ones fail silently in + # the dangerous direction. Measured on the flagged darwin-arm64 artifact: # - # This first shipped as `strip -x` on Mach-O out of caution that a full - # strip could leave an image dyld will not load. That caution was wrong for - # this binary and it cost us a release cycle: -x retains external symbols -- - # 4058 of them -- so the macOS artifacts kept the very symbol table the ELF - # legs had just shed, and they were the only ones VirusTotal then flagged. - # Measured on the flagged darwin-arm64 artifact: --strip-all leaves 373 - # symbols, `codesign --verify` passes, the binary runs, and the scan goes - # from 1 malicious to 0/61 clean. + # llvm-strip --strip-all 373 symbols scanned CLEAN + # strip (no flags) 378 symbols equivalent + # strip -x -S 4058 symbols the state VirusTotal FLAGGED + # strip -X / -u -r 4058 symbols likewise + # + # Apple's strip returns success for `-x -S`, so a helper that just tries + # candidates until one exits 0 would quietly reship the flagged binary. + # GNU/LLVM `--strip-all` is not even accepted by Apple's strip, which is why + # generalising it to every platform broke the macOS build -- loudly, which + # was the lucky outcome. + # + # So: --strip-all where it is understood, plain `strip` for Mach-O, and a + # hard error when no candidate can do the job. Never a weaker fallback. local stripped="" for tool in "${STRIP:-}" llvm-strip strip; do [ -n "$tool" ] || continue command -v "$tool" >/dev/null 2>&1 || continue - "$tool" --strip-all "$binary" 2>/dev/null && stripped="$tool" + if "$tool" --strip-all "$binary" 2>/dev/null; then + stripped="$tool --strip-all" + elif [ "$GOOS" = "darwin" ] && "$tool" "$binary" 2>/dev/null; then + stripped="$tool" + fi [ -n "$stripped" ] && break done if [ -z "$stripped" ]; then diff --git a/scripts/run-test-wave.py b/scripts/run-test-wave.py index 8faa6e75c..44a16c23c 100755 --- a/scripts/run-test-wave.py +++ b/scripts/run-test-wave.py @@ -123,15 +123,55 @@ def start_suite( ) +def windows_descendants(pid: int, timeout: int) -> bool: + """True if any live process still claims `pid` as its parent. + + Used only when the suite leader has already exited: `taskkill /T` cannot + walk a tree from a dead PID, so cleanup is proven by asking whether anything + is still parented to it. One level deep on purpose -- Windows does not + reparent orphans, so a grandchild keeps pointing at its own (dead) parent + and would not be found here. That is a weaker proof than taskkill /T, which + is why it is reserved for the case where the strong proof is impossible. + """ + try: + completed = subprocess.run( + [ + "powershell.exe", + "-NoProfile", + "-NonInteractive", + "-Command", + "@(Get-CimInstance Win32_Process -Filter " + f"'ParentProcessId={pid}').Count", + ], + check=False, + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + timeout=timeout, + ) + except (OSError, subprocess.TimeoutExpired): + return True # cannot prove absence -> assume the worst + if completed.returncode != 0: + return True + return (completed.stdout or "").strip() not in ("0", "") + + def terminate_process_tree(active: ActiveSuite, kill_grace: int) -> None: process = active.process leader_exited = process.poll() is not None if os.name == "nt": if leader_exited: - raise RuntimeError( - f"suite {active.name!r} leader exited before Windows tree cleanup " - "could be proven" - ) + # The leader can exit on its own between the timeout decision and + # this call. Refusing outright made the harness itself lose a race: + # a natural exit at the wrong moment failed the whole wave, which is + # how a deliberately-hanging fixture suite reddened a release run. + # taskkill /T cannot walk a tree from a dead PID, so prove cleanup + # the only way still available -- nothing is parented to it. + if windows_descendants(process.pid, kill_grace): + raise RuntimeError( + f"suite {active.name!r} leader exited leaving live descendants" + ) + return try: completed = subprocess.run( [ diff --git a/scripts/security-allowlist.txt b/scripts/security-allowlist.txt index 6c7fea9ca..efe9b37f8 100644 --- a/scripts/security-allowlist.txt +++ b/scripts/security-allowlist.txt @@ -66,7 +66,6 @@ src/ui/http_server.c:execl:exec indexing binary in child process URL:https://api.github.com/repos/DeusData/codebase-memory-mcp/releases/latest:update check URL:https://github.com/DeusData/codebase-memory-mcp/releases/latest/download:binary download + checksums URL:https://github.com/DeusData/codebase-memory-mcp/releases/latest:version check via redirect header -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) URL:http://127.0.0.1:UI server binding (localhost only) URL:https://www.sqlite.org/c3ref/c_checkpoint_full.html:sqlite WAL checkpoint API doc reference (comment only, not a network call) URL:https://github.com/DeusData/codebase-memory-mcp:project repository self-reference in update/star notice (src/mcp/mcp.c) diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index d1d5e0c06..2e7fded67 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -151,7 +151,27 @@ fi trap 'smoke_rmtree "$TMPDIR" "${DRYRUN_HOME:-}"' EXIT CLI_STDERR=$(smoke_mktemp_file) -cli() { "$BINARY" cli "$@" 2>"$CLI_STDERR"; } +# 10 of the cli call sites assign directly (VAR=$(cli ...)). Under +# `set -euo pipefail` a non-zero exit there kills the smoke with NOTHING +# printed: no FAIL line, no stderr, just an abort indistinguishable from a hang, +# a starved runner, or a real regression. One such abort cost a full Windows +# cycle just to locate, and still could not be attributed. Surface the command +# and its stderr here, while we still can. +# +# Neutral wording on purpose: one call site deliberately expects a non-zero exit +# (the unknown-function query must error loudly), so this must not read as a +# failure on its own. +cli() { + local rc=0 + "$BINARY" cli "$@" 2>"$CLI_STDERR" || rc=$? + if [ "$rc" -ne 0 ]; then + { + printf 'cli: `%s` exited %s\n' "$*" "$rc" + sed 's/^/ /' "$CLI_STDERR" 2>/dev/null + } >&2 + fi + return "$rc" +} echo "=== Phase 1: version ===" VERSION_STATUS=0 diff --git a/src/cli/cli.c b/src/cli/cli.c index 8413bad12..8d60e6994 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -11168,12 +11168,6 @@ static bool check_already_latest(void) { #endif /* CBM_CLI_ENABLE_TEST_API */ -/* One token on purpose: the security audit extracts URLs literally, so a URL - * split across string-literal continuations cannot be matched against the - * allow-list. Printed for the user to paste; never fetched by this binary. */ -#define CBM_INSTALL_SH_URL \ - "https://raw.githubusercontent.com/DeusData/codebase-memory-mcp/main/install.sh" - int cbm_cmd_update(int argc, char **argv) { parse_auto_answer(argc, argv); @@ -11253,7 +11247,9 @@ int cbm_cmd_update(int argc, char **argv) { if (have_dir) { printf(" powershell -ExecutionPolicy Bypass -File \"%s\\install.ps1\"\n\n", self_dir); } else { - printf(" powershell -ExecutionPolicy Bypass -File install.ps1\n\n"); + printf(" powershell -ExecutionPolicy Bypass -File install.ps1\n" + " (ships in the release archive, and is placed beside the\n" + " binary on install)\n\n"); } printf("It downloads the latest release, verifies its checksum, and replaces\n" "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) { if (have_dir) { printf(" bash \"%s/install.sh\"\n\n", self_dir); } else { - printf(" curl -fsSL " CBM_INSTALL_SH_URL " | bash\n\n"); + printf(" install.sh (ships in the release archive, and is placed\n" + " beside the binary on install)\n\n"); } printf("It downloads the latest release, verifies its checksum, and replaces\n" "this binary in place. install.sh is idempotent, so re-running it IS\n" diff --git a/test-infrastructure/Dockerfile.glibc22 b/test-infrastructure/Dockerfile.glibc22 index 131bbff7c..094645323 100644 --- a/test-infrastructure/Dockerfile.glibc22 +++ b/test-infrastructure/Dockerfile.glibc22 @@ -6,7 +6,11 @@ # 2. the DYNAMIC binary refuses to start (its glibc 2.38+ floor is by # design — old-glibc coverage is the portable binary's job, and a # dynamic binary that silently ran here would mask a floor regression). -FROM ubuntu:22.04 +# Pinned by digest like every other venue image: a floating tag means the +# glibc-FLOOR assertion silently drifts to whatever ubuntu:22.04 points at, and +# a floor that moves is not a floor. (Scorecard PinnedDependenciesID #76 — this +# was the one image the pinning pass missed.) +FROM ubuntu:22.04@sha256:0e0a0fc6d18feda9db1590da249ac93e8d5abfea8f4c3c0c849ce512b5ef8982 RUN apt-get update && apt-get install -y --no-install-recommends \ bash \ diff --git a/tests/test_daemon_application.c b/tests/test_daemon_application.c index cda557ea5..e81b766a3 100644 --- a/tests/test_daemon_application.c +++ b/tests/test_daemon_application.c @@ -2855,10 +2855,17 @@ TEST(daemon_application_cancels_physical_job_only_after_final_session) { started[i] = cbm_thread_create(&threads[i], 0, app_request_thread, &requests[i]) == 0; } bool subscribed = started[0] && started[1] && app_wait_for_subscribers(application, project, 2); - if (subscribed) { + /* The physical job starts asynchronously once a session subscribes, so a + * subscriber count of 2 does NOT imply it has started. Cancelling both + * sessions before that point leaves starts == 0 -- arguably the correct + * outcome, and the reason this test failed intermittently on Windows while + * passing everywhere else. Wait for the state the assertions below actually + * require; `starts` only increments, so this cannot miss the transition. */ + bool job_started = subscribed && app_wait_for_atomic_int(&fake.starts, 1); + if (job_started) { callbacks.session_cancel(callbacks.context, sessions[0]); } - bool one_left = subscribed && app_wait_for_subscribers(application, project, 1); + bool one_left = job_started && app_wait_for_subscribers(application, project, 1); int cancels_after_first = atomic_load(&fake.cancels); if (one_left) { callbacks.session_cancel(callbacks.context, sessions[1]); @@ -2874,6 +2881,7 @@ TEST(daemon_application_cancels_physical_job_only_after_final_session) { ASSERT_TRUE(setup); ASSERT_TRUE(subscribed); + ASSERT_TRUE(job_started); ASSERT_TRUE(one_left); ASSERT_EQ(atomic_load(&fake.starts), 1); ASSERT_EQ(cancels_after_first, 0); diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 93a329984..2bc3f3c88 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -3889,6 +3889,32 @@ static long extract_wide_flat_ms(int n, int *out_defs) { return (b.tv_sec - a.tv_sec) * 1000L + (b.tv_nsec - a.tv_nsec) / 1000000L; } +/* Best-of-N. Timing noise only ever ADDS time, so the minimum of a few runs is + * the cheapest good estimate of the noise-free cost. A single sample at each + * size made the RATIO carry the noise of BOTH measurements: on a loaded Windows + * VM this read 184ms -> 9387ms (51x) for code that measures ~20x unloaded, and + * tripped a bound calibrated for exactly that linear case. + * + * Deliberately NOT solved by raising WF_RATIO_MAX: the bound sits where it does + * because linear (~20x) and quadratic (~128x) are each >=2x away from it, so + * inflating it moves the test toward the very signal it exists to catch. This + * keeps the threshold and removes the variance instead. */ +static long extract_wide_flat_ms_best_of(int n, int reps, int *out_defs) { + long best = -1; + for (int i = 0; i < reps; i++) { + int defs = 0; + long ms = extract_wide_flat_ms(n, &defs); + if (ms < 0) { + return ms; + } + if (best < 0 || ms < best) { + best = ms; + *out_defs = defs; + } + } + return best; +} + TEST(extract_wide_flat_file_is_linear) { /* SCALING-RATIO guard: assert the COMPLEXITY CLASS, not a wall-clock * bound. Index-based ts_node_child(i) child loops are O(i) per call — @@ -3912,8 +3938,10 @@ TEST(extract_wide_flat_file_is_linear) { enum { WF_SMALL = 20 * 1000, WF_BIG = 400 * 1000, WF_RATIO_MAX = 40, WF_FLOOR_MS = 120 }; int defs_small = 0; int defs_big = 0; - long t_small = extract_wide_flat_ms(WF_SMALL, &defs_small); - long t_big = extract_wide_flat_ms(WF_BIG, &defs_big); + /* Small is cheap, so sample it more; big dominates runtime, so twice is the + * affordable compromise that still discards one unlucky sample. */ + long t_small = extract_wide_flat_ms_best_of(WF_SMALL, 3, &defs_small); + long t_big = extract_wide_flat_ms_best_of(WF_BIG, 2, &defs_big); ASSERT_GTE(t_small, 0); ASSERT_GTE(t_big, 0); /* Anti-vacuous guard: the breadth was actually walked at both sizes. */ diff --git a/tests/test_parallel_harness_contract.sh b/tests/test_parallel_harness_contract.sh index b97d2de7a..a658dc56a 100755 --- a/tests/test_parallel_harness_contract.sh +++ b/tests/test_parallel_harness_contract.sh @@ -373,11 +373,28 @@ try: stdout, stderr = process.communicate(timeout=8) if os.name == "nt": - if process.returncode != 2 or "tree cleanup" not in stderr: + # Assert the PROPERTY, not the wording. This used to require the phrase + # "tree cleanup" in stderr, which pinned one specific refusal message: + # rewording the guard broke the contract while the behaviour was still + # correct. What "fails closed" actually means is that the scheduler + # refused (rc=2) AND did not silently leave the descendant behind as if + # cleanup had succeeded. + descendant_pid = int(descendant_path.read_text(encoding="utf-8")) + if process.returncode != 2: raise SystemExit( f"FAIL: Windows timeout race did not fail closed " f"(rc={process.returncode}, stdout={stdout!r}, stderr={stderr!r})" ) + if process_state(descendant_pid) != "live": + raise SystemExit( + "FAIL: Windows timeout race refused without a surviving descendant " + "to refuse over -- the fixture no longer exercises the race" + ) + if "cleanup" not in stderr.lower(): + raise SystemExit( + f"FAIL: Windows timeout race refused without naming a cleanup " + f"failure (stderr={stderr!r})" + ) else: if process.returncode != 0: raise SystemExit(