test_py_lsp_bench.c:262 failed the v0.10.0 release run TWICE on genuine
attempts (ubuntu-24.04-arm 3/3), while the SAME sha passed that shard in the
morning's dry run. A wall-clock bound on a shared 4-vCPU runner is not a
property of the code, and this repo's testing doctrine says exactly that:
perf tests assert a ratio; a time bound may exist only as a liveness backstop.
This test's perf content already IS ratio-form (resolution ratio with a
sanitizer-aware floor). The 150/1500 ms budget was the anti-pattern half.
It becomes a 30 s liveness backstop so a catastrophic hang still fails the
suite rather than the 900 s shard wall clock, and absolute timings stay where
they belong: scripts/benchmark-*.sh, which record rather than gate.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The CLI probe reproduces the empty pipeline (rc=1, same one-line hint), so the
remaining record is the worker's own log -- which the supervisor unlinks on a
clean exit. CBM_PROFILE=1 flips that exact branch to retention
(index.supervisor.profile_log) with no product change, and the probe now prints
every retained log tail from its isolated cache.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The retry with a fresh cache fails identically, so this is path-dependent, not
order-dependent — the daemon-lifetime theory is dead. The MCP result compresses
the pipeline's own diagnostics into one hint string; the documented CLI
entrypoint prints them. Third fresh cache, same repo, output verbatim.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The worker log shows the discriminating shape: the supervisor reaps the worker
CLEAN (exit 0, 2.3s) yet index_repository reports an empty pipeline -- and the
test daemon now serves the UI on fixed port 9749, which main's build does not
do here (no pack beside the test binary keeps its UI off, while the embedded
build auto-enables). Sequential per-case daemons therefore contend on one port,
and Cyrillic is simply the SECOND case in the run order.
An in-process retry against the same repo with a fresh cache separates the two
remaining explanations: a passing retry convicts the previous case's daemon
lifetime (order), a failing retry convicts the path itself.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The previous round narrowed Cyrillic-on-x64 to: correct project name derived,
server responsive, but the pipeline reports 'Check repo_path exists and
contains source files' and only _config.db + logs/ land in the cache. The
supervisor wrote worker logs, so the actual error is already on disk in the
failing job -- print their tails plus the repo listing instead of guessing
across another cycle.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Cyrillic fails deterministically on the x64 runner and passes on the arm64 VM
and on the main-based sibling PR at identical path depth, so neither script nor
path length explains it. The remaining unknown is what index_repository
actually said and whether anything landed in the isolated cache; print both.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Cyrillic case fails deterministically on the GitHub runner (two genuine
attempts, fresh temp roots each time) while latin1/CJK/Greek pass around it and
the SAME commit passes all cases on the real-Windows VM. The guard records an
'error' field for exactly this situation and then never prints it, so the CI log
shows nodes=None and nothing else. Surface it verbatim; the next run carries the
diagnosis instead of a guess.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The new contract hit the WSL-alias trap on its first Windows CI run: BASH is a
shell variable, NOT exported, so os.environ.get("BASH", "bash") silently fell
back — and native Windows resolved bare 'bash' to the WSL stub, which fails
every invocation with 'Windows Subsystem for Linux has no installed
distributions' before the extractor runs at all.
This is the exact pattern the venue-parity contract bans; it slipped past that
rule's regex because the literal sits inside a .get() fallback rather than a
bare argv element. Fixed the repo's documented way (df2088a7): the shell wrapper
passes "$BASH" in as argv and Python uses it verbatim.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Deleting this contract with the UI-pack architecture threw away coverage of
machinery that SURVIVED: the extractor still builds the scan-set and association
manifests that check-virustotal.sh reads. Without it the gate could report a
confident green over an incomplete set.
It immediately paid for itself. The extractor was BROKEN by my own edit:
NameError: name 'variant' is not defined (process_tar)
TypeError: add_association() missing 'variant'
Removing the variant from validate_namespace's return left the call sites still
threading it. python3 -m py_compile passes on that -- it is a runtime failure --
and the extractor runs ONLY in the release and dry-run VirusTotal jobs, so
nothing else in the tree or on the 3-OS ladder executes it. It would have
shipped and broken every release verification.
The contract is rewritten rather than adapted: the original was woven through
the pack format (16 archives, 88 associations, per-pack asset math), and a
half-adapted version that passes for the wrong reasons is worse than none. It
now asserts what still exists:
- the exact 8-archive matrix, 32 member associations, nothing dropped
- archive containers never enter the scan set as members
- deduplication is exact: 8 byte-identical LICENSE members collapse to ONE
scan object while 8 distinct binaries stay 8
- manifest metadata agrees with its own rows, and every staged object matches
its recorded SHA-256
- fail-closed on a surplus member, a missing member, and a short matrix
Registered as Step 0l so it runs in every venue, not just when someone
remembers.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
pr-smoke was green but test-windows-guards failed, and the shape of the failure
named the cause:
[PASS] non-ascii/latin1_accents nodes=12 edges=21 definitions=5
[FAIL] non-ascii/cyrillic nodes=None edges=None definitions=None
ASCII and latin1 pass, Cyrillic does not -- latin1 survives a Western ANSI code
page and Cyrillic does not. But the product code was never the problem: the
wide-path helpers (win_utf8.h, runtime.c extended paths) were KEPT. I had
reverted this test to a base that predates them, so it lost windows_extended_path()
and the explicit `executable=` argument to subprocess.run. With
lpApplicationName NULL, CreateProcess applies the legacy MAX_PATH limit while
deriving the module name from the command line, so on CI's long guard-bundle
path the process never started and every count came back None.
Same mismatched-base class as the package wrappers: production kept the fix, its
test was reverted past it.
Restored from main and adapted rather than dropped -- it still covers
extended-length publication AND non-ASCII indexing. Only the sidecar
assertions are gone (one self-contained file to publish now), and the probe
uses --version instead of the removed --verify-runtime-assets.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Deleting the archive-extractor contract took an assertion with it that had
nothing to do with the UI-pack architecture that contract tested: release.yml
and dry-run.yml must stage downloaded archives into $RUNNER_TEMP, not into the
tracked checkout, so the extractor never scans a directory that also holds repo
files.
The fix itself survived the revert; only its guard died. Moved to the
venue-parity contract, which already reads both workflows and is registered in
scripts/test.sh.
Revert-checked: reverting release.yml to ARCHIVE_DIR="assets" makes this fail
with the exact message, and restoring it passes.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Wiring this contract into scripts/test.sh Step 0l made it run on the Windows VM
for the first time, where it failed with 'cmp: command not found' -- MSYS2 does
not ship cmp, and diff is not guaranteed either. The script already requires
python3, so compare the bytes there instead.
The failure was real coverage working as intended: the contract had never run on
Windows before, because it was registered only in the test.sh version that this
branch replaces.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Its PASS line still advertised the old policy, which is exactly the string a
future reader would trust when deciding what the gate enforces.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
ui_server_root_without_ready_assets_is_retryable asserted the pack architecture's
readiness semantics: the daemon warmed an external pack asynchronously, so a
request could legitimately arrive before the assets existed, and 503 +
Retry-After told the client to come back.
With the frontend linked into the image there is no warm-up window. Whether the
UI exists is decided at build time: a binary built without --with-ui has no
index.html and never will. Promising a retry for a condition that cannot change
would make every client poll forever, so this is a 404.
The replacement also asserts Retry-After is ABSENT, so the retryable behavior
cannot quietly come back with the header still attached.
macOS suites after this: cli 261 passed; ui/httpd/mcp/daemon* 469 passed with
this as the only failure, and httpd re-runs 62 passed / 0 failed.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Caught by running the contracts rather than by inspection:
- package-release.sh built its archives from an ARCHIVE_MEMBERS array, which
only ever existed to append a UI pack conditionally. With a fixed four-file
inventory the array hides the member list from the very contract that is
supposed to pin it, so the members go back inline on the archive call.
- test_windows_bundle_contract.sh asserted the pypi wrapper exposes
_windows_binary_ready(). That function became _runtime_set_ready() when the
wrapper locks were reworked; the contract now asserts the current API, since
the property it guards is "resolves the single Windows binary", not a name.
- test_smoke_fixture_contract.sh required smoke-local.sh to emit both a
variant-suffixed archive and a standard alias. There is one archive name now,
so it asserts that instead -- and that ${SUFFIX} is gone entirely, so the
alias machinery cannot quietly return.
Verified on the built artifact: the binary runs, carries 9 shebangs, 1
node:child_process and 14 hook-augment references (the deliberately inverted
property), and contains zero references to cbm-integrations.json, CBMUIPK or
--verify-runtime-assets.
Contract status on macOS: build_dir_safety, makefile_ts_runtime_dependencies,
parallel_harness, security_fuzz_harness, security_strings_allowlist,
soak_daemon_recovery, spawn_no_window, ui_dev_proxy_security, venue_parity,
vm_worktree_manifest, windows_bundle, smoke_fixture, vt_gate_policy and
vt_release_notes all pass. hook_conflict_notice, parent_watchdog and
worker_error_response fail for the documented environmental reasons (TEST_SEAMS
build seam, and the developer's own installed daemon holding the version
cohort), not from this change.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Completes the collapse to a single shipped composition and replaces the
zero-tolerance VirusTotal gate with a narrow, disclosed policy.
Packaging and installers
- package-release.sh loses --variant; archives are codebase-memory-mcp-<os>-<arch>
with exactly four members. install.sh/install.ps1 lose --ui/--standard.
- The extractor drops CBMUIPK pack parsing and --archive-scope; its scan-set and
association manifests (which the gate depends on) are unchanged otherwise.
- npm/PyPI/Go wrappers: the runtime "set" is one file again. The Windows lock
and race fixes from #1495/#1496 are kept; only multi-file set membership goes.
This also fixes `pip install` on Windows, which rejected the fifth archive
member against a hardcoded four-name allowlist.
- The wrappers' post-download probe moves from --verify-runtime-assets (removed)
to --version, which proves the same thing: the binary executes.
VirusTotal gate
- Exactly ONE detection is tolerated, and only when the engine is Microsoft AND
the label ends in `!ml`. Two or more engines, any non-`!ml` label, any other
vendor, any suspicious verdict and every infrastructure error still block.
- A tolerated object prints TOLERATED:, never OK:, and its counts are recorded
in vt-results.tsv exactly as a blocked one would be.
- append-vt-notes.sh mirrors the policy. It previously hard-failed on any
malicious count, so loosening only the gate would have passed the scan and
then died at note publication. The notes now DISCLOSE a tolerated detection
and link to SECURITY.md rather than claiming "0 malicious" for everything.
Rationale for the tolerance is in the gate itself: the verdict is not a property
of our bytes. It inverts across architectures and link modes, moves between
sibling artifacts of one build, and lands in different variant buckets for the
same source. The same `!ml` family hits llama.cpp, GitHub's own `gh`, Microsoft's
own Go toolchain and Anthropic's Claude installer.
The zero-tolerance contract becomes test_vt_gate_policy_contract.sh, asserting
the full matrix: 1x Microsoft !ml passes and reports TOLERATED; a Microsoft
signature label, a non-Microsoft engine, two engines, a suspicious verdict and
every malformed-response case still block. Its tripwire is narrowed to the
reverted endpoint-verification mechanism rather than the words "false positive",
so it no longer fires on a deliberate in-gate policy branch.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Externalizing the integration templates (#1492/#1493) and the UI bundle
(#1501/#1503) was done to reduce the Microsoft `Wacatac.B!ml` surface. It did
not work: across dry runs the flagged artifact count stayed at ~3 and the
detections merely moved between artifacts.
Dissection of run 31286803592 shows there is no structural cause to fix. The
verdicts split across every axis at once — linux-amd64 (dynamic) flagged while
linux-amd64-portable (static) is clean, but linux-arm64 (dynamic) clean while
linux-arm64-portable (static) is flagged. The two macOS binaries have identical
segment structure and split clean/flagged. Siblings from one build landed in
different variant buckets (.B vs .C). Entropy is low everywhere
(code_vectors.bin 4.166, grammar tables 3.464 bits/byte, against 7.5-8.0 for
packed payloads), so the packed-payload hypothesis is excluded too.
So the complexity bought nothing, and installation goes back to being
self-contained: one binary that carries its own UI and agent integration
templates, with no adjacent data file that has to resolve before `install`
works. Only the UI-capable composition ships from now on, under the historical
unsuffixed archive name.
Removed: src/ui/asset_pack.{c,h}, asset_pack_stub.c, asset_manifest_stub.c,
scripts/pack-ui-assets.mjs, src/cli/integration_assets.{c,h},
assets/cbm-integrations.json, scripts/gen-integrations-hash.sh, the
--verify-runtime-assets probe (nothing adjacent left to verify), and the
composition gates A6/A7 whose property is now deliberately inverted.
Restored: scripts/embed-frontend.sh, src/ui/embedded_{assets.h,stub.c}, the
compiled-in hook/adapter template bodies, and the embed/EMBED_OBJS build path.
Kept from the reverted commits, re-applied by hand where a wholesale file
restore would have dropped them:
- cbm_module_path_utf8() in both self-path sites. GetModuleFileNameA renders
through the ANSI code page and mangles non-ASCII install paths.
- the /__cbm/ui-readiness HMAC proof, secure_random and cbm_hmac_sha256, so
`daemon start --open` still waits for a genuine CBM listener.
- X-Content-Type-Options: nosniff on served assets.
- the MinGW noexecstack gate, -lbcrypt, and the cppcheck/zip CI fixes.
Archives are now codebase-memory-mcp-<os>-<arch>[-portable] with exactly four
members (binary, LICENSE, installer, THIRD_PARTY_NOTICES.md). That restores the
names every static package manifest already points at — aur, chocolatey,
homebrew, scoop, winget and glama were all broken by the -ui- rename.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The product binary embedded nine complete shebang'd shell scripts, their
PowerShell/.cmd twins, and two node:child_process client-adapter modules as C
string literals, and wrote them into agent config dirs at 0755 on install. A
block of script-shaped bytes inside a large unsigned executable is exactly the
surface Microsoft Defender's ML scored Trojan:Script/Wacatac.B!ml — the
detection that has recurred across the last several release attempts, on the
Linux ELF and darwin-arm64 standard binaries while every UI and Windows variant
stayed clean.
This moves every integration template out of the binary into a single shipped
data file, assets/cbm-integrations.json (compact: one asset, because each
shipped file is itself scanned). The binary now carries only the templates'
identity — one embedded SHA-256, generated at build time from the JSON so it
can never drift — plus the code to load, verify, and render them.
Why a hash and not just a loose file: content compiled into the binary was
tamper-evident with the binary. Moving it to disk must not turn it into
unauthenticated code the installer blindly executes. install verifies the asset
against the embedded SHA-256 and fails closed on mismatch ("integration assets
missing or modified - reinstall from the release archive") before writing
anything. Verified end to end: a one-byte edit to the asset makes install
refuse.
Ownership, for safe upgrade/removal: the verified copy is published to
~/.cbm/assets/<version>/ — a sibling of the DB cache, never inside it, so
clearing the cache cannot strip a user's hooks. A deployed integration file is
ours to remove only if it matches a template rendered from that stored copy;
foreign files and user-edited files at the reserved paths are preserved. The
historical template bodies travel in the JSON's released[] arrays so files
written by older versions are still recognised and cleaned up on uninstall.
Removed from src/: cmm_gate/session/subagent script-prefix and suffix
constants, cbm_build_released_gate_script, the shell/PowerShell/.cmd bodies in
cli.c, and the JS/TS generators in client_adapter.c. Confirmed on the built
standard binary: 0 occurrences of '#!/usr/bin/env bash', '#!/bin/sh',
'node:child_process', and 'ExecutionPolicy' (was non-zero for each).
test_no_embedded_scripts_contract.sh makes the removal a build property, not a
reachability assumption: no production C source may contain a shebang or
node:child_process literal, and the built binary is scanned directly. It strips
comments first (a contract a comment can satisfy is a false guard), and is
revert-checked — reintroducing one shebang literal makes it fail naming the
file:line. check-binary-composition.sh (A6) enforces the same needles on every
release artifact post-strip.
This does not, by itself, prove the detection is cured: the clean UI binary is a
strict superset of the flagged standard one, so no single passage is a
deterministic trigger. It removes the most coherent malware-shaped composite
from every shipped binary, aligns with how gh/starship/zoxide externalise
integration material, and makes the templates maintainable as data rather than
escaped C literals. Causal confirmation still needs a VirusTotal ablation.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The contract asserted the literal "powershell -ExecutionPolicy Bypass -File"
appeared in cbm_cmd_update, so removing that string from the binary reddened
every test leg on every platform — the contracts run as step 0 of scripts/test.sh,
so one violation stops the matrix before a single suite executes. 13 checks, one
cause.
The contract's stated intent is that the Windows update command "must hand off to
install.ps1 and must NOT carry an in-process self-update path". The bypass flag
was never that property, only the literal form the command happened to take. The
assertion now pins the hand-off itself ("powershell -File" plus install.ps1),
which is what the comment always claimed to protect.
Adds the inverse assertion in the same place: the printed command must NOT
contain an execution-policy override at all. Without it this contract would
silently permit the pattern coming back, and the whole point of removing it was
that a shipped binary is the wrong place to carry a canonical loader signature.
Process note for myself: I ran the cli suite and lint before pushing, but not the
contracts — which is precisely the gap recorded as "scripts/test.sh runs them as
step zero of EVERY leg, so one violation reddens all 30 CI jobs". Ran the full
contract set this time.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The gate reported "1 malicious, 0 suspicious / 59 engines" and stopped there —
withholding the one fact a detection is actionable on. A false-positive
submission goes to a SPECIFIC vendor, and triage begins with "known-noisy engine
or a new family?". Neither question is answerable from a count.
It cost exactly that in the 2026-08-07 dry run: three artifacts flagged
(linux-amd64, linux-amd64-portable, darwin-arm64), one engine each, and the logs
could not say by what. The vendor had to be recovered by hand from the web UI
afterwards — and the WDSI developer false-positive path is a web form with no
public API, so guessing the wrong vendor wastes a filing round trip.
The analysis response already carries per-engine results; the gate simply threw
them away. It now prints, under the BLOCKED line, each engine that returned
malicious or suspicious with its detection label and definition date:
BLOCKED: codebase-memory-mcp-linux-amd64 flagged (1 malicious, ...)
detected by: Microsoft = Trojan:Win32/Wacatac.B!ml (engine ..., defs ...)
Engines that did not flag are not listed, or the actionable line drowns in
sixty clean rows.
Deliberately unchanged: the gate's strictness. Zero tolerance still means any
detection on any artifact on any version blocks, and an analysis WITHOUT
per-engine results still blocks on the counts alone — the contract pins both, so
naming engines cannot become a route to tolerating one. A missing engine list
prints an explicit "<engine names unavailable>" rather than nothing, so the gap
is visible instead of reading as a detection-free block.
Verified by revert: against the old gate the new assertions fail with the
counts-only output quoted above.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Every non-JSON tool result shipped its payload twice: content[0].text, and an
identical copy as structuredContent {"text": <entire payload>}. Measured on a
20k-node query_graph, the reply was 2.05x the payload it carried. Half of every
large answer was redundant bytes — half the 10 MiB transport budget, and double
the tokens billed to every LLM caller on every call.
Measured, same query, same fixture:
before 8,529,990 bytes content.text 4,154,932 structuredContent.text SAME
after 4,265,047 bytes content.text 4,154,932 structuredContent {}
Exactly 50.0% smaller with content.text byte-identical: the payload is fully
delivered, only the second copy is gone. The 10 MiB ceiling now also admits
roughly twice the rows before #1375's limit applies.
Nothing is lost. structuredContent exists to carry STRUCTURE, and a string
re-wrapped in a one-key object has none — a client reading
structuredContent.text learned exactly what content[0].text already told it. The
empty object still satisfies the declared outputSchema, which is
{"type":"object","additionalProperties":true} and never required a text field.
Two cases are deliberately NOT changed:
* JSON payloads. structuredContent stays the PARSED object. That is the
spec's structured+serialized pattern rather than waste, and it is what our
own hook-augment consumes (structuredContent.projects from list_projects).
* Errors. structuredContent.error is kept: bounded, small, and the only
machine-readable form of a failure a client gets.
Guarded at both levels, because the defect was invisible per-tool — each result
looked reasonable alone, and only measuring the wire showed half of it was
redundant:
* tests/test_mcp.c enumerates the TOOL TABLE itself, so a new tool is covered
the moment it is registered, with no test edit. A guard pinned to
query_graph would not have caught search_graph, and would not catch whatever
is added next. It fails if no tool produced a non-JSON payload, so it cannot
report a green it never earned — which it did on the first attempt, catching
that {} args make every tool error out and assert nothing.
* scripts/smoke-test.sh Phase 3z asserts the same property on the SHIPPED
binary. This is a wire-format contract: what a real client receives from the
real artifact is not something a from-source test can prove.
Both revert-checked: restoring the duplication reddens the unit guard at the
strcmp, the plain-text guard, the search_graph expectation, and the smoke phase
(which names search_graph and get_architecture).
tests/test_mcp.c expectations that pinned the old duplicating shape are updated
rather than relaxed — the format changed on purpose.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A single query_graph reply larger than one 10 MiB frame killed the MCP server:
exit code 1, no JSON-RPC error, nothing on stderr, no crash report. Under a host
that does not respawn the server, every tool from it was gone for the rest of
the session — the reporter lost their tooling mid-investigation to one dead-code
query and finished by driving the binary over stdio by hand.
Root cause is a conflation, not the limit itself. application_mcp_request
checked only response_length > UINT32_MAX and passed anything smaller on, so an
oversized reply reached the transport, which rejected the frame. The frontend
worker cannot tell that rejection from a dead socket, and for a dead socket it
deliberately _Exit()s the process — closing the kernel IPC handle is the only
portable way to cancel daemon session ownership from a thread blocked in stdio.
Correct for a broken transport; a fatal over-reaction to a large answer.
The substitution therefore has to happen in application_mcp_request, which is
the last layer still holding the request id. Replies that fit are returned
byte-identical; a reply that cannot be framed becomes a JSON-RPC -32603 naming
both numbers and what to do ("narrow the projection, add LIMIT, or paginate"),
because the failure it replaces was a silent exit and an opaque "internal error"
would be no improvement.
Reproduced first, on a locally generated 20k-node fixture rather than the
reporter's repo (external code stays unverified until reviewed):
before LIMIT 10000 -> ok, 8,529,990 bytes
LIMIT 20000 -> SERVER DIED exit=1 stderr=''
after LIMIT 20000 -> JSON-RPC -32603, and a follow-up query on the SAME
session succeeds — which is the property that actually matters
The decision and the substitution live in one function on purpose. The first
version of the test built the error object directly and PASSED with the size
check removed: it proved the error was well-formed while the server still died.
Testing application_framable_response instead binds the branch — verified by
removing the guard, which reddens it at ASSERT(replaced != big).
Not addressed here, deliberately: every reply still carries its payload twice
(content[0].text and structuredContent.text are verbatim duplicates, ~2.05x
measured), which is why the ceiling is reached at ~4 MB of real content. That is
a wire-format question affecting every tool and every caller's token bill, so it
belongs in its own change with a decision behind it, not smuggled into a crash
fix.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
#1448 set CREATE_NO_WINDOW on the subprocess spawn and on cbm_popen_isolated,
but cbm_exec_no_shell (compat_fs.c) still passed a bare 0 for dwCreationFlags.
That is the helper behind git, codesign and open, so the most frequently hit
path kept flashing a console — and under a stdio MCP session with auto_watch
those windows steal focus while the user is typing. Reported by @noctrex on
#1448 after verifying the merged fix.
Also adds the contract nobody had. The flag has to be set per call site, the
four sites do not resemble each other, and reviewing "does this PR add the
flag" says nothing about the sites a PR does not touch — which is exactly how
the third one survived a fix applied twice:
daemon/bootstrap.c had it from the start (DETACHED_PROCESS | ... | flag)
subprocess.c added by #1448 (flags variable)
compat_fs.c :313 added by #1448 (inline literal)
compat_fs.c :689 missed by #1448 (bare 0)
tests/test_spawn_no_window_contract.sh asserts the whole-tree property instead,
so a fifth spawn site cannot be added without it.
The contract strips C comments before matching, and that is load-bearing rather
than tidiness: the first version of it PASSED with the fix reverted, because the
explanatory comment above the fixed call contains the flag name and satisfied
the match. A contract that can be satisfied by prose about the contract is a
false guard. Verified in both directions afterwards — green with the fix, and
red naming compat_fs.c:696 with the fix reverted and that comment still in
place.
Co-Authored-By: noctrex <noctrex@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The production guard for #1367 — collect_fp_entries only sorting when
count > 1 — is on main, but nothing exercises it. A fix without a test is a
fix that can be silently undone, and this one is unusually easy to undo:
removing the guard produces no visible symptom on most lanes.
Adds the missing case: a graph with zero Function/Method nodes, where the
entry array is never allocated and the null base would reach qsort.
The test comment records WHERE the test binds, because that turned out to be
the interesting part. Removing the guard again:
macOS 25 passed (Apple libc does not mark qsort
nonnull, so there is no UB to see)
Linux ASan/UBSan, recovering 25 passed (prints "null pointer passed as
argument 1", then keeps going)
Linux UBSan, trapping exit 1 (process aborts — the RED)
So the assertions here are the behavioural half (empty graph → rc 0, no
SIMILAR_TO edges), and the UB half is caught by the trap-UBSan leg alone.
Both were measured on ubuntu-arm64 in each direction. Spelling that out in
the test keeps a future reader from checking it on a recovering lane, seeing
green, and concluding the null-base guard is still there when it is not.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Three language fixtures assign cbm_project_name_from_path into lp->project each
time they open an indexed store. Teardown frees the last one, so a fixture that
indexes more than once dropped every earlier heap name — 68 allocations across the
run, which the macOS leak lane reports at process exit.
Pre-existing rather than introduced here: none of these files or fqn.c are in this
branch's diff, and the leak lane passed on the previous PR. Adding a suite shifted
what the leak job sees, which is how it surfaced. Fixing it here rather than
recording it, since a gate that only stays green by composition accident is not a
gate.
Freeing before reassigning also makes the ownership obvious at the point it
matters, instead of relying on the reader noticing the teardown three functions
away.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A repository may ship .cbmpathwhitelist listing outside roots it would like
indexed alongside it. The file REQUESTS; it never GRANTS. Both attackers in this
project's threat model can write a file inside a repository — a malicious indexed
repo, and an agent with write access to the project — so a repo-local file cannot
be authoritative without handing them the boundary.
Approval is recorded user-level and keyed to a SHA-256 of the manifest bytes, so
editing the file lapses approval and it must be granted again. Without the hash a
repo approved once could widen itself forever on a later pull. This is direnv's
model, and it is the only shape that keeps a checked-in file's ergonomics without
making the file a permission slip. The hash covers the raw bytes rather than the
parsed entries: a reordering or comment change is still a change the approver has
not seen.
`allow-root --approve-manifest <project>` is the human action that grants.
Approval refuses outright if any requested entry would not stand on its own as an
indexing root, so approving cannot become a route around the breadth policy, and
entries are re-classified at use time as well so a stored approval cannot outrank
a credential list that has grown since.
Entries containing control characters are rejected — the same shape as the
newline splitting that let a crafted indexed path inject an extra entry into the
scoped file list.
Deliberately NOT wired into the root boundary. A project's manifest authorizes
outside roots FOR THAT PROJECT, so the question is "may project P pull in tree T",
not "may T be indexed standalone", and the boundary check only ever sees one path
with no project context to ask that with. A first draft did wire it there by
passing the candidate as its own project root, which read a manifest that by
definition was not the one requesting it and so authorized nothing — caught by
exercising it end to end. The consuming half belongs where project context exists,
in discovery walking a project's approved extra roots;
cbm_workspace_manifest_allows is the query it will use.
Five tests, including the one the design rests on: approval lapses when the
manifest content changes.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The dry run claimed EVERY hook group as installable because gate_ok /
session_ok / subagent_ok were simply set to `true` when dry_run was on. When
the on-disk hook script is not ours - user-modified, or written by a manual
install pointing at another binary - the real install refuses to rewrite it,
so the preview promised what the run could not deliver. That is the second
half of #1387: the reporter had no way to see the loss coming, and said so.
- New read-only predicate cbm_text_owned_document_status() mirrors
text_migrate_owned_document's decision WITHOUT writing: absent / already
current / an exact released document => a write would proceed; anything else
=> it would be refused.
- The dry run uses it per script, so the three 'hooks:' lines now reflect what
will actually happen, and each refused script is NAMED with the reason.
Silence was the original defect: an unmentioned group reads as 'nothing to
do', not 'this will be skipped'.
Test: cli_dry_run_predicts_refused_hook_script_issue1387 runs the real
dry-run path over an unowned gate script, capturing stdout, and asserts the
warning names the script. RED before, GREEN after, RED again on revert.
cli+agent_profiles+config_text_edit: 292 passed; lint-ci clean.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Two loose ends from the bounds work.
The decompression side already took size_t so a >2 GiB capacity could not wrap
through int, but compression still took int for both the source length and the
destination capacity, and the artifact export cast a size_t database size down
to reach it. A database past 2 GiB would have handed the encoder a negative
length. Both lengths and the bound helper are size_t now, and the function
returns int64_t like its decompressing counterpart.
The discovery walk now prunes the cache directory by absolute path. A custom
CBM_CACHE_DIR may sit inside a repository — tests do it routinely — and walking
into it pulls every other project's graph database into this project's file
list. This is the narrow form of a concern that was briefly implemented as
refusing any root that contained the cache; refusing a whole root was too blunt,
and not walking the cache is what the concern actually asks for.
The test fails without the prune: a .go file planted under the cache is
otherwise discovered, and "cache" is not in the built-in skip list, so the
assertion is not vacuous.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
sys/wait.h does not exist on the Windows toolchain, so the unguarded include
broke the Windows test build outright — the suites never ran. The test body is
already skipped on Windows, so the includes belong behind the same guard, as
test_cypher.c does for the same pair.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Windows bundle contract pinned the literal
remote_head="$(vm clangarm64 "cd /c/cbm && git rev-parse --verify HEAD")"
so parametrising the checkout path for per-run isolation failed it, even
though the property it guards - capture the remote HEAD into a local variable
and compare it HERE, never nesting quotes through cmd.exe - still holds.
Match the shape with the path as \S+ instead. Verified the contract still
binds: replacing the capture with a constant fails it for the right reason,
restoring it passes.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
"Users" was matched the way credential directory names are — against every
path component. On Windows every user's files live under C:\Users\<name>, so
that refused every ordinary project path, including a CI runner's own
workspace. The Windows smoke job on the pull request caught it; the unit
tests could not, because they only ever run compiled on macOS here.
System trees (Windows, ProgramData, Program Files) now match only as the
first component below the drive, which still refuses anything inside them at
any depth. "Users" is handled separately as the tree root only, mirroring
POSIX where "/Users" is refused but "/Users/dev/projects" is not.
Tests cover the regression directly: C:/Users/dev/projects/app and
C:/Users/runneradmin/work/repo are allowed, C:/Users is not, C:/Windows/System32
is refused at depth, and C:/dev/Windows-app — merely named after a system
tree — is allowed.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Routes the MCP index_repository handler and the graph UI's POST /api/index
through a single decision function. The UI route previously checked only that
root_path was a directory, so an operator's configured boundary held on one
entry point and not the other; it now canonicalizes first and applies the same
policy, answering 403 with the reason.
The decision is two-tier, because a bare default-deny would refuse every
first run and a bare opt-in leaves the default open:
- Breadth is always enforced, with nothing configured. Filesystem, drive and
share roots, top-level system trees, the home directory itself and
credential directories are refused as indexing roots out of the box.
- Containment in a declared root applies once CBM_ALLOWED_ROOT is set or a
grant exists, and is evaluated first so a path outside a configured root is
reported as exactly that.
Three things the tests caught, each a real defect rather than a test fix:
- On macOS /etc, /tmp and /var are firmlinked under /private, so
canonicalizing "/etc" yields "/private/etc" and counted two deep — sailing
past a minimum of two, missing the very path being guarded. Depth now
discounts a leading "private" component.
- An earlier draft refused any root containing the cache directory. That was
over-claimed: the indexer only parses recognised source files and a graph
database is binary SQLite it would never extract. Refusing a whole root is
also the wrong remedy where the concern does hold — not walking the cache
is. Removed, with the reasoning recorded at the site.
- Rewording the refusal to "outside every allowed root" broke an assertion
matching "outside the allowed root", and that test's early return skipped
its CBM_ALLOWED_ROOT cleanup, leaking the variable into every later test in
the suite. The original wording is kept and guidance appended instead.
Worth remembering: these contracts match strings, not properties.
Docs updated in the same change, since both env-var tables said "unset
imposes no restriction" and that is no longer true: CONFIGURATION.md and
README.md describe the two tiers, and CONFIGURATION.md lists the always-
refused roots along with the two limits that matter — this constrains scope
rather than sensitivity, and the credential list is a denylist that raises the
cost of a mistake rather than closing the class. SECURITY.md's supported-
versions table was still on 0.8.x and now reads 0.9.x.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Linux legs of the local 3-OS ladder build into build/linux-arm64 /
build/linux-amd64, not build/c, so Step 5c died with
'missing binary: /src/build/c/codebase-memory-mcp' and took the whole
leg down. Steps 5 and 5b already pass CBM_TEST_BINARY (derived from
$BUILD_DIR) to their scripts; 5c did not, and its test hardcoded the
path.
CI never caught this because every CI leg uses the default BUILD_DIR of
build/c - the container legs are the only ones that differ, which is
precisely what the local ladder is for.
- test_worker_error_response.sh honours ${CBM_TEST_BINARY:-build/c/...}
like its sibling watchdog tests; the default keeps bare manual runs working
- scripts/test.sh passes CBM_TEST_BINARY to Step 5c, matching 5 and 5b
- test_hook_conflict_notice.sh carried the identical hardcoding and is
fixed the same way (local-only today, but wrong is wrong)
Verified: with the fix the test passes against an out-of-tree binary;
reverting the fix reproduces the ladder's exact failure (rc=2,
'missing binary: <worktree>/build/c/codebase-memory-mcp').
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Adds cbm_workspace_classify_root, the breadth half of the workspace
boundary. It answers "is this path obviously too broad or too sensitive to
index as one root", not "is this caller allowed to index it" — that second
question needs the user-level grant store, since it is the only input a
caller cannot write. The header says so, because a reader who mistakes one
for the other would draw the wrong conclusion about what is enforced.
Three rules, each doing what the others cannot:
- Depth below the volume. Two components on POSIX refuses every top-level
tree in one rule with no list to maintain. Windows and UNC count
drive/share-relative and require one, because there the first component is
already user space ("D:/repos") — the system trees that sit at the same
depth ("C:/Windows") are covered by name instead, which is tractable
because that set is small and stable.
- The home directory itself, which is two deep on macOS and Linux and so is
invisible to depth.
- Credential directory names, matched against every component so both
"…/.ssh" and "…/.ssh/keys" are refused. This list is deliberately additive
so extending it needs no design authority.
A path that is or contains the cache directory is refused outright: indexing
such a tree would pull every other project's graph database into this
project's index.
Classification order is load-bearing and commented as such. The home
directory normally contains the cache directory, so testing the cache first
reported every home as "holds the cache" and made it non-overridable, which
contradicts the intent that a person may override it. Depth precedes the
cache test for the same reason: "/Users" satisfies both and "too broad" is
the reason that helps the reader.
Takes home and cache as parameters rather than reading the environment, so
the policy is a pure function and the tests need no filesystem. Twelve table
tests; three of them failed on the first implementation and drove the
ordering fix above.
Not yet wired to any caller — deliberately, so the policy can be reviewed on
its own.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A pass over places where a bound was either miscalculated or missing.
UI response builders (src/ui/http_server.c) mixed the clamping helper
http_appendf with raw cursor indexing. Route the per-line separators and
quotes through the helper too, so the cursor cannot leave the buffer once
output saturates it, and terminate explicitly afterwards — the helper pins
the cursor and then writes nothing, which would otherwise leave the "%s"
reply with no terminator in range. The log buffer was budgeted at
LOG_LINE_MAX + 10 per line while JSON escaping can double every byte, so
size it for the escaped worst case. Job status entries now escape their path
and error fields, which were interpolated raw.
Cypher (src/cypher/cypher.c): the WHERE grammar descends once per nested '('
and once per NOT, so parse depth followed the query text rather than
anything bounded. Track depth on parser_t and refuse past 256 levels with a
parse error. Bounding parse depth also bounds the resulting tree, so the
recursive evaluator inherits the limit.
Gitignore (src/discover/gitignore.c): '**' retries the remainder at every
position and consecutive groups multiply, so match cost is exponential in
the number of groups. Thread a step budget through the matcher and give up
past 20000 steps, reporting no-match so a pathological pattern fails to
ignore rather than ignoring the wrong files. glob_match keeps its name so
recursion_whitelist.h still describes the functions that recurse; the new
non-recursive wrapper that seeds the budget is glob_match_bounded.
Call extraction (internal/cbm/extract_calls.c): extract_fp_callee recursed
once per applied argument, so stack use followed the parse-tree depth of the
indexed file. Rewritten as a left-spine loop. Reassigning node/nk before
continuing leaves the fall-through cases where the recursive form left them,
so behaviour is unchanged and the extraction suite is untouched.
CALLS props (src/pipeline/pass_calls.c): `cap - pos - PAIR_LEN` wraps once
pos reaches cap - PAIR_LEN and stops bounding the copy below. Guard
additively, matching the closing write in the same function.
Shell-bound paths (src/foundation/str_util.*): the three git shell-out sites
each wrap a repo path in cmd.exe-compatible double quotes, where %VAR%,
!VAR! and ^ remain active. Two carried private copies of the check that
rejects those three on Windows and the third used the bare validator.
Promote the stricter form to cbm_validate_shell_path_arg and route all three
through it, so the copies cannot drift apart again.
Tests cover the buffer bounds, the parse-depth refusal, matcher termination
and the validator. The bounds tests run in a forked child so a violation
surfaces as a signal instead of silently.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Martin Vogel