posix_directory_parent_secure(current_fd) validates the directory we are ALREADY
IN, but the refusal message printed `component` - the child about to be entered.
Every reporter has therefore been sent to inspect the wrong directory:
#1537 read "ancestor '.cache'" when /Users/<user> was refusing
#1621 read "ancestor 'cbm-daemon-501'" when /private/tmp was refusing
Both inspected the named directory, found it clean, and said so. They were
right. #1537 has been open for weeks with the reporter repeatedly confirming a
correct `.cache` - including on v0.10.4 today - because we kept pointing at it.
The message now names the CONTAINING directory and says explicitly not to check
the component itself.
This fixes no permission logic. It changes weeks of talking past each other into
a report someone can act on in a minute, which for this class of bug is the
whole game: the refusal is invisible from outside, so the message IS the
diagnosis.
Found while reviewing the CBM_RUNTIME_DIR work; the original detail was added in
the earlier half of #1537 and named the wrong variable from the start.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`cbm_spawn_backoff` clamped its shift with
attempt < CBM_SPAWN_RETRY_ATTEMPTS ? attempt : CBM_SPAWN_RETRY_ATTEMPTS
and its comment said "the budget is the only bound needed — callers never
exceed it, and a second clamp would be dead code". The clamp that stayed is the
dead one: both retry loops return before `attempt` reaches the budget
(cbm_fork_with_retry, and the posix_spawn loop on Apple), so it can never fire.
What it leaves unbounded is the case it was written for. Nothing stops the
doubling, so the schedule explodes with the budget rather than flattening:
budget 6 10 20 40 80 160 320 630ms
budget 9 10 20 40 80 160 320 640 1280 2560 5110ms
budget 12 10 20 40 80 160 320 640 1280 2560 5120 10240 20480 40950ms
40s of waiting is exactly the "hang instead of fail fast" this retry exists to
avoid, and it is one edit to the budget away.
Bound a single wait instead, at 2560ms — the largest wait either shipping
budget already produces. Measured old vs new across both:
budget 6 old 630ms new 630ms identical
budget 9 old 5110ms new 5110ms identical
budget 12 old 40950ms new 12790ms flattens at the ceiling
So nothing changes today; a future budget increase now costs 2560ms per extra
attempt instead of doubling. The ceiling is spelled as a shift so the value
cannot overflow `long` on the way to being clamped, and the doubling comment
above now names both bounds instead of only the budget.
clang-format clean; subprocess suite green (14 passed, 17 skipped — the POSIX
retry probes skip on this Windows box, so the loop bounds themselves are CI's to
confirm).
Signed-off-by: Mauricio Offermann <mauricio.offermann@gocode.cl>
CI lint flagged two continuation lines from the previous commit. Formatting
collapsed the runtime-directory call onto one line, so the contract pin that
asserted ancestor=false there stopped matching; the pin now keys on the
formatted text. Both revert directions re-verified: capability-only fails, and
relaxing the runtime directory to ancestor=true fails.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Six independent machines in #1533 and #1574 cannot run cbm at all, and none of
them is exotic: a domain-joined UAC-filtered admin, a secondary volume, orphaned
ACEs from an uninstalled application, and an AppContainer package SID belonging
to a shipping desktop application.
The ancestor walk was identity-blind to all of it. win_directory_component_secure
demanded that no ancestor DACL entry grant any private-mutation bit to any SID
outside the trusted set, so a single ACE anywhere up %LOCALAPPDATA% refused the
endpoint before logging started. Every mode failed, config list included, so the
product could not even be reconfigured out of it, and CBM_CACHE_DIR does not help
because the runtime directory is %LOCALAPPDATA%\cbm-daemon-<hash>, whose ancestor
chain relocating the cache never touches.
Ancestor components now also tolerate AppContainer identities: package SIDs
(S-1-15-2-*) and capability SIDs (S-1-15-3-*), under identifier authority 15.
The boundary is ancestor-only. The private runtime directory is validated with
ancestor=false and keeps demanding the exact current user with a protected DACL;
the flag is threaded explicitly through win_file_security_secure and
win_file_acl_secure rather than inferred, so the strict path cannot acquire the
tolerance by accident.
Why these identities are admissible on an ancestor: a process cannot choose which
AppContainer it runs in. The identity is stamped by the OS at process creation
from the package it was launched from, so such an ACE cannot be exercised by
arbitrary local code the way a live local group can. What it does permit is the
packaged application itself — that is the residual risk this accepts, and it is
the same trust already extended to whoever installed that package.
BOTH forms are covered deliberately. The most common real ACE of this shape is
S-1-15-2-*, a package SID; on reported machines it resolves through the registry
AppContainer mappings to Anthropic Claude Desktop, which many of our users run and
cannot be asked to uninstall. Covering only capability SIDs leaves exactly that
case failing.
This narrows a deliberate policy: the strict gate was chosen on purpose and a
middle ground was previously declined. It is reopened here by explicit maintainer
decision, and narrowed as far as the evidence allows rather than relaxed wholesale
— a live local group, Authenticated Users on a secondary volume, and orphaned
unresolvable SIDs all still refuse. Those need CBM_RUNTIME_DIR or a separate
change; orphan tolerance in particular needs LookupAccountSid bound first and has
an offline-domain-controller caveat, so it is not bundled in here.
Approach and the ancestor-only boundary follow @mlandolfi90's PR #1447, extended
from capability SIDs to package SIDs.
Co-Authored-By: mlandolfi90 <mlandolfi90@users.noreply.github.com>
Contract-pinned in tests/test_windows_bundle_contract.sh and revert-checked in
both directions: reducing it to capability-only fails, and passing ancestor=true
for the runtime directory fails.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Release notes now carry one row per product — the bytes actually published, with
their VirusTotal verdict — instead of a column per candidate.
Several candidates are scanned so the selector has an alternative when an opaque
classifier flags one, but a reader installing cbm cares about what they receive,
not about what we discarded. The rejected candidates' verdicts remain in the
published evidence TSVs for anyone auditing the selection, and they stay useful
to US as a development signal; in a changelog they are noise, and listing a
"microsoft-ml" verdict next to a binary nobody can download invites exactly the
wrong conclusion.
Also removes a claim that had become FALSE: the section asserted that every scan
had a minimum of 50 decisive engines. That floor was removed deliberately (it is
VirusTotal fleet availability, not a property of our binary) and the last release
observed 31. Published notes would have stated something untrue. The observed
range is still reported, now over the shipped binaries.
The disclosure of a tolerated Microsoft `!ml` is preserved and narrowed to the
binaries that ship. The contract test now pins both directions: a flagged binary
that SHIPS must be disclosed, and flagged candidates that were rejected must not
appear at all. Its fixture gained a target that draws the tolerated verdict on
all three candidates, so the disclosure branch is actually exercised — previously
every flagged candidate was rejected in favour of a clean sibling, leaving that
path untested.
README and SECURITY.md updated: they promised notes "link every candidate result".
All five release/VT contract tests pass.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Four places ask "is this binary instrumented?" and each asked it differently.
One of them, the C# LSP bench, only recognised ASan, so TSan and MSan measured
an instrumented parse against the NATIVE 200ms budget. The other three carried
a hand-copied list of `__SANITIZE_*__` macros that nobody kept in sync.
That drift is what the TSan gap was made of. CFLAGS_TSAN never passed
SANITIZED_DEFINE, and the per-site conditions could not cover for it: they test
`__SANITIZE_THREAD__`, which is GCC's spelling. Clang — the compiler that leg
uses — announces thread instrumentation through `__has_feature(thread_sanitizer)`
only, and no site consulted it. The claim in 0a163d4f that compiler probes could
not have helped is true of the probes we had, not of the one clang actually
offers.
src/foundation/sanitized.h now answers the question once, as CBM_SANITIZED,
from two sources with distinct jobs:
- CBM_SANITIZED_BUILD from the build system stays the source of truth, and is
the ONLY thing that can answer for UBSan and trap-UBSan: undefined-behaviour
instrumentation leaves no macro and no __has_feature bit to probe.
- The clang and GCC probes are the backstop for the three sanitizers that do
announce themselves, so a lane that forgets the define still gets correct
budgets instead of native ones on an instrumented binary.
Deliberately no #error when a probe fires without the define: promoting leaves
the binary correct while the lane gets fixed, and it does not break an
out-of-tree `make CFLAGS_EXTRA=-fsanitize=address` that never went near
Makefile.cbm.
__has_feature is defined away where it does not exist rather than guarded with
`#elif defined(__has_feature)`. The guarded form compiles everywhere but fails
cppcheck, which walks every configuration and rejects the file with "failed to
evaluate #if condition, undefined function-like macro invocation". The
define-away idiom is what clang documents and what tests/test_mem.c already
uses; `defined(...) && __has_feature(...)` is not an option at all, since && does
not spare a preprocessor without the builtin from parsing `0 (0)`.
Verified the resolution rather than assuming it (clang 22, -dM -E):
native CBM_SANITIZED 0
-DCBM_SANITIZED_BUILD=1 CBM_SANITIZED 1
-fsanitize=address CBM_SANITIZED 1
-fsanitize=thread (linux target) CBM_SANITIZED 1
-fsanitize=memory (linux target) CBM_SANITIZED 1
-fsanitize=undefined CBM_SANITIZED 0 <- define-only, as designed
The third row is the one that matters: the TSan failure this header is named
after would have self-healed.
Also wired the define into the instrumented flag sets of our own code that
still lacked it — CXXFLAGS_TSAN (preprocessor.cpp is ours, and CXXFLAGS_TEST
already had it), GRAMMAR_CFLAGS_TEST and GRAMMAR_CFLAGS_TSAN. Neither tree can
include the header today (no -Isrc), so this is the build system keeping its
own promise rather than a behaviour change. Vendored flag sets are untouched:
mimalloc, sqlite3, tre, zstd, lz4 and tree-sitter read no macro of ours.
Behaviour change worth naming: test_cs_lsp_bench now allows 2000ms on the TSan
and MSan lanes instead of 200ms. It loosens a bound that was being applied to
an instrumented binary by accident; it never tightens one.
Not verified locally: this machine has no POSIX-target compiler, so the POSIX
half of subprocess.c was not compiled here. The full Windows test-runner builds
clean with -Werror and the subprocess suite is green (14 passed, 17 skipped);
clang-format clean; macro matrix as above.
Signed-off-by: Mauricio Offermann <mauricio.offermann@gocode.cl>
The v0.10.4 release (run 31755436916) failed at the very last step of `verify`,
after all 69 other jobs had passed — full test matrix, all eight builds, smoke
and soak:
publish-vt-evidence: wrong evidence marker in binaries/vt-results.tsv
The results format was bumped to `cbm-virustotal-results-v2` in #1596, but
publish-vt-evidence.sh still demanded `-v1`. It was the only straggler: the
gate, the selector, the notes renderer and all three contract fixtures were
already on v2.
It survived because that PR had removed the script's only caller, leaving it
dead code that nothing exercised. Restoring the full-surface scan restored the
caller too, and the stale expectation surfaced in the worst possible place — at
the end of a real release rather than in a dry-run, since `verify` does not run
in dry-runs.
Also adds the guard that would have caught it: every marker publish-vt-evidence.sh
validates must be one the gate actually writes. Revert-checked — reintroducing
v1 fails with "publisher expects marker never written by the gate:
cbm-virustotal-results-v1".
The scan itself passed. This is purely the evidence-publishing step.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Two things, both found by the dry-run and by sweeping what it exposed.
1. The three-candidate change was incomplete. The dry-run failed at
stage-release-candidates.py, which carries its OWN copy of the transform
validation that the previous commit only fixed in the selector:
stage-release-candidates: candidate linux-amd64/debug-stripped has
invalid transform: 'strip-debug'
Sweeping for that assumption found it in six places, not two:
stage-release-candidates.py, select-release-candidates.py,
verify-release-selection.py, append-vt-notes.sh and two contract-test
fixtures — as hardcoded 16s, `len(TARGETS) * 2`, two-entry VARIANTS tuples
and two-key truth tables. All are now derived from len(VARIANTS).
Field prefixes needed care: the variant NAME keeps its hyphen because it is
the on-disk directory, while the evidence columns use underscores, so
`debug-stripped` reads `debug_stripped_sha256`. Every lookup now goes through
an explicit FIELD_KEY map instead of interpolating the variant directly.
The selection contract test now covers the truth table EXHAUSTIVELY: three
variants x two tolerated classifications is exactly eight combinations, and
there are exactly eight targets, so every case is exercised once.
2. Full-surface VirusTotal scanning is restored. This PR had moved scanning
upstream to the candidates and deleted the post-package pass, which silently
narrowed coverage from everything we ship to executables only. The 42 runtime
files across the 14 containers — install.sh, install.ps1, LICENSE,
THIRD_PARTY_NOTICES.md, the MCPB manifest.json and the unpacked UI assets —
were still extracted, structurally verified and strings-audited, but no
longer scanned at all. install.sh and install.ps1 are the highest-consequence
non-executable bytes we publish; users pipe them straight into a shell.
The verify job scans every extracted object again, under the same policy.
Re-submitting the selected executables alongside them is close to free
because VirusTotal is content-addressed and answers for identical bytes from
its own record — the same property that made the analysis-id equality check
untenable two commits ago.
The gate-chain contract asserted the opposite ("duplicate post-package
VirusTotal path remains"). That assertion is inverted: the pass is required,
and it is not a duplicate, since it covers a strictly larger set. README and
SECURITY.md updated from "archive containers are checksummed rather than
redundantly rescanned" to state the full covered surface.
All five release/VT contract tests pass.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Release evidence from run 31744302624 shows the tolerated Microsoft `!ml`
verdict is close to a coin flip per byte image rather than a property of the
code. Across the eight targets the stripped and unstripped candidates of the
SAME linker output disagreed on four, and in both directions:
linux-amd64 stripped microsoft-ml unstripped clean
darwin-arm64 stripped clean unstripped microsoft-ml
linux-arm64 stripped clean unstripped microsoft-ml
If the classifier were keying on something intrinsic to our code the siblings
would agree; they do not. So each variant is close to an independent draw, and
5 of 16 candidates drew the flag.
Two draws is not always enough. On that run linux-amd64-portable came back
microsoft-ml on BOTH candidates, leaving no clean binary to ship for that
target. A third independent draw at a ~31% observed per-candidate hit rate takes
the both/all-flagged case from roughly 1-in-10 per target to roughly 1-in-30.
The third candidate is `--strip-debug` (Apple: `-S`): debug information removed,
symbol table kept. Behaviourally identical to the other two — same linker
output, only metadata differs — but a distinct byte image, which is all
VirusTotal needs to scan it as its own file. Verified on the real v0.10.4
candidates: linux-amd64 gives three distinct hashes (294,634,656 /
294,623,208 / 293,746,096 bytes) and darwin-arm64 likewise, with the ad-hoc
signature verifying after strip.
Selection is unchanged in spirit and now ordered: smallest artifact first
(stripped, debug-stripped, unstripped), take the first CLEAN one, and only if
every candidate drew the tolerated verdict ship the smallest flagged one. A
hard verdict on any candidate still blocks the release before selection.
Cost is 24 objects per release instead of 16.
Derivation enforces that all three hashes differ — identical candidates would be
one draw wearing three hats, and the selector would believe it had alternatives
it does not have. Public claims in README, SECURITY.md and docs/index.html
updated from "both stripped and unstripped" to the three candidates.
All three release contract tests pass, including the native derivation test
which exercises the real strip and codesign path on this host.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The build+VT dry-run (run 31744302624) blocked all eight targets on:
BLOCKED: objects/3eb486a4... completed with only 48/75 decisive engines (< 50)
That object is the windows-arm64 STRIPPED candidate with ZERO detections. It
was classified "hard" — the same bucket as a real malware verdict — purely
because 48 engines answered instead of 50. Its own sibling scanned clean with
66.
How many engines return a decisive result is VirusTotal fleet availability on
the day, not a property of our binary. These artifacts are ~300 MB and many
engines skip or time out at that size, so the count sits near the floor and
moves run to run: on this run one object landed at 48 and the other fifteen
spread 59-68. A floor there makes shipping a lottery decided by someone else's
infrastructure, which is exactly what our CI-determinism rule forbids, and it is
not something we can influence by fixing anything on our side.
Classification now depends only on what engines FOUND:
hard - two or more engines, any non-Microsoft engine, any label that is
not `!ml`, or anything suspicious. Still blocks the release.
microsoft-ml- exactly one Microsoft `!ml`. Tolerated and disclosed.
clean - nothing flagged.
The decisive-engine count is still recorded per candidate in the results
manifest and printed as a NOTE when it is below the reference value, so the
evidence is unchanged; only its power to block is gone. The selector keeps
rejecting an incoherent response (total < completed).
Public claims updated to match in README.md, SECURITY.md and docs/index.html —
all three promised "at least 50 decisive engines" as a publication requirement,
which would no longer have been true.
Contract test updated: a clean verdict now stands regardless of engine count,
and the below-reference NOTE must still be reported. All three release contract
tests pass (vt-gate-policy, release-gate-chain, candidate-derivation).
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The previous commit framed accepting VirusTotal's own analysis id as giving up
freshness. That was the wrong way round.
VirusTotal recognising our bytes is desirable: the evidence this pipeline
publishes is the hash-keyed file report — append-vt-notes.sh asserts the URL is
exactly .../gui/file/<sha256>/detection — so the report we gate on is the same
one a reader gets by looking that SHA-256 up themselves. Requiring a freshly
minted analysis id would have contradicted the evidence we publish next to it,
and made our verdict unreproducible by anyone outside the release job.
Comment-only; no behaviour change. The build+VT dry-run on 094929fd exercises
identical logic.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The dry-run for this branch (run 31729540899) failed in `select-package`, the
new job, after every build had completed and before smoke — the exact shape of
failure that would have taken a release down:
BLOCKED: VirusTotal response analysis id does not match the submitted
analysis: objects/70fbbc37...
That object is the linux-arm64 UNSTRIPPED candidate, 295,476,856 bytes.
VirusTotal is content-addressed, and for bytes it already holds it may answer
with its own canonical analysis rather than the one this upload created. A large
release binary is exactly the artifact most likely to be already on file, so
requiring the ids to be equal would have recurred on most releases rather than
being a one-off.
The equality check was guarding the right idea at the wrong level. What has to
hold is that a verdict describes THESE bytes, and parse_completed already
enforces that a few lines below against meta.file_info.sha256 and
file_info.size — a strictly stronger binding than the id. It is also what keeps
a tuple's two candidates apart: stripped and unstripped differ in hash by
construction (70fbbc37... vs 28b3890f... on linux-arm64), so neither can be read
as the other whatever ids VirusTotal hands out. The wrong-hash and wrong-size
contract cases pin that and still block.
The property deliberately given up is freshness: an older cached analysis of
identical bytes is now accepted. For identical bytes that is the same content
risk, and it buys a gate that is deterministic instead of one that fails
whenever VirusTotal recognises our own binary. The submitted id is still
recorded in the results manifest as evidence.
The selection policy is unchanged and needs no change: stripped clean ships
stripped; stripped microsoft-ml with a clean unstripped ships the clean one;
both microsoft-ml ships stripped; a hard verdict on either sibling still blocks
the release for analysis.
Contract test updated to assert the passing direction, and revert-checked: with
the fix reverted it fails with the production message, with it applied it passes.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The CLI resolves each client's config home from its own environment
variable before falling back to $HOME — cbm_codex_config_dir() honours
$CODEX_HOME and ignores the home_dir it is handed. A test that redirects
only HOME therefore does not isolate those clients: it resolves, reads
and WRITES the developer's real configuration.
On a machine with $CODEX_HOME exported (any Codex or Orca user) 27 of
269 cli tests failed for that reason alone. Every one of them asserted a
successful rc while the CLI was reporting genuine failures against the
ambient config: an unbalanced managed marker left in the real
config.toml, an AGENTS.md that is a symlink and is refused by the
O_NOFOLLOW writer, and user-modified agent profiles that are correctly
preserved. The behaviour under test passed in all of them; only the rc
was poisoned. Worse than the red: the suite mutated that real config,
installing and removing SKILL.md under the user's Codex home.
The individual tests that already neutralize CODEX_HOME are exactly the
ones that pass, and the file carries a comment admitting one suite was
"green only via env leaked from earlier suites". Rather than extend those
ad-hoc per-test lists again, neutralize the whole inventory once in the
runner, and reuse the inventory the shell fixtures are already required
to satisfy.
tests/test_smoke_fixture_contract.sh grows one check so the two cannot
drift: a client isolated in the smoke scripts but forgotten in the C
runner now fails the contract.
Verified on macOS arm64 with $CODEX_HOME and $OPENCODE_CONFIG_DIR
exported: cli went from 241 passed / 28 failed to 268 passed / 0 failed,
with no change outside tests/.
Signed-off-by: Gabriel Gonzalez Pineda <64bits@gmail.com>
Three comments still described the original design: "Three attempts over ~30ms
total" (it has been six attempts over ~630ms since the backoff landed), and a
hard-coded list of waits plus a "~0.6s ceiling" rationale that is now only the
unsanitized half of the story.
Rewritten so the prose derives from CBM_SPAWN_RETRY_ATTEMPTS instead of
restating it, which is what let the first one go stale. Comment-only; no
behaviour change. subprocess suite 31/0/0, clang-format clean.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The previous commit widened the spawn-retry budget for sanitized builds, and it
did nothing on TSan — the leg it was written for. `subprocess_run_spawn_failure`
failed again on the very PR that was meant to fix it.
CBM_SANITIZED_BUILD comes from SANITIZED_DEFINE, which keys off $(SANITIZE).
TSan does not use that variable — it has its own TSAN_SANITIZE — and CFLAGS_TSAN
never included SANITIZED_DEFINE. So the macro was undefined on that leg and
every sanitized-budget branch compiled to its NATIVE value while running an
instrumented, several-times-slower binary.
The comment above SANITIZED_DEFINE already describes this exact failure for
trap-UBSan: "the build system is the single source of truth for is this binary
instrumented; compiler-specific probes miss clang's feature-check spelling and
every non-ASan sanitizer". That lesson was recorded and the TSan leg was never
wired up to it. Nor would compiler probes have saved this: clang spells thread
instrumentation __has_feature(thread_sanitizer), not __SANITIZE_THREAD__.
CFLAGS_TSAN now defines it unconditionally, which is honest — that flag set
exists solely to build an instrumented binary.
Checked the neighbours: MSan (scripts/msan.sh passes SANITIZE=) and the diag
lane (passes SANITIZE= too) both already get the define. TSan was the only gap.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The exponential backoff shipped in v0.10.3 (10/20/40/80/160/320ms, ~0.6s) fixed
the ordinary case. `subprocess_run_spawn_failure` then kept failing on
`test-tsan (macos-14)` — twice on the same SHA, on a PR whose entire diff was a
shell contract plus one line in test.sh, so causation was impossible.
ThreadSanitizer runs several times slower and holds far more process state, so
the pressure window it creates is simply longer than 0.6s. The budget was right
for a normal build and short for a sanitized one.
Raising it for everyone would be the wrong fix: an unsanitized machine that is
genuinely out of capacity should fail fast rather than hang for seconds. So the
extra patience is scoped to the builds that need it — three more doublings,
about 5s, under CBM_SANITIZED_BUILD or __SANITIZE_*__ only. Same shape as the
daemon announce backstop in test_daemon_frontend.c, and for the same reason.
Also fixes a bug in the backoff itself: the shift was capped at a hard-coded 6,
so with a budget of 9 the final three waits would have flattened to 320ms each
instead of continuing to double — the extra attempts would have bought a third
of the intended time. The cap now follows the budget.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A duplicated install left a Codex config.toml carrying a CLOSING
`# <<< codebase-memory-mcp SessionStart <<<` with no opener. Every later
install then failed that client with op=legacy_hook_cleanup and aborted the
whole activation, so one bad file made the installer permanently unusable for
that client. Reported by @PsyTech-Rob.
We are the only writer of these markers, which makes the imbalance our own
residue. Refusing to touch a file we cannot parse is the right default in
general — it is the wrong default for a mess we made.
Removal now strips the stray line. The bounds come from the marker scan that
already ran, so nothing is guessed and no surrounding content is touched; the
user's own keys either side survive verbatim.
A WRITE still refuses on an imbalance, deliberately: with only one marker there
is no defensible region to replace, and guessing could destroy user content.
That asymmetry is the whole design — heal where the answer is unambiguous,
refuse where it is not.
Both orientations are tested (orphan opener, orphan closer) and revert-checked:
without the fix each returns the same -1 that aborted the reporter's install.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`ui_enabled` governs a loopback HTTP listener on 127.0.0.1:9749, and the only
way to turn it off was hand-editing ~/.cache/codebase-memory-mcp/config.json.
The key was absent from CONFIG_KEYS, so `config list` could not show it and
`config set` rejected it outright. A reporter spent two debugging sessions
finding the switch, and described the surface as looking impossible to turn off.
A network surface a user cannot discover how to disable is not acceptable
whatever its default, and "documented in the source" is not discoverable.
ui_enabled and ui_port now appear in `config list`, and get/set/reset work on
them. They live in a separate file (cbm_ui_config_load/save) rather than the
key-value store the other keys use, so this routes rather than merely listing
them — set validates (true/false; a port in 1..65535) and reports that the
daemon must restart for the change to take effect.
The test asserts DISCOVERABILITY rather than the current values: both keys must
be present in the table that `config list` and `config set` walk, because a key
missing there is a key the user can neither find nor change.
Reported by @PsyTech-Rob.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The previous commit pinned the linux/amd64 asset hash directly, which
verified the asset this job needs but hard-coded the platform: the
uname-based selection was replaced by a fixed filename, so moving the job
to another runner OS or architecture would have needed a code change and
a second pinned hash.
Pin the SHA-256 of the release's own checksums file instead, and verify
whichever asset the runner selects against it. Upstream publishes that
file for the whole release, covering linux, darwin and windows on both
amd64 and arm64, so one pinned value now covers every platform and the
uname-based selection is restored.
Match the asset by exact filename, since a substring match would also
accept the .sbom.json and .sigstore.json lines for the same asset, and
fail closed when an asset is absent from the checksums file rather than
installing it unverified.
Verified end to end against the pinned release: the checksums file
matches its pinned hash; asset selection resolves to a listed asset for
linux/amd64, linux/arm64, darwin/amd64 and darwin/arm64; the linux/amd64
asset verifies and extracts to a statically linked x86-64 ELF; and
appending one byte to the downloaded asset makes verification FAIL, so
the gate binds rather than passing vacuously.
Co-Authored-By: Andrew Hundt <ATHundt@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The publish-mcp-registry job fetched mcp-publisher from the `latest`
release and piped curl directly into tar. Whatever upstream published at
that moment therefore executed inside the job that holds the MCP Registry
publish credential, with no opportunity to verify it first.
Pin the release to v1.8.1, download to a file, verify its SHA-256 against
the checksum published in registry_1.8.1_checksums.txt for that same
release, and only extract once the hash matches. The job runs on
ubuntu-latest, so the linux/amd64 asset replaces the uname-derived
selection.
Reported by Andrew Hundt in #1245.
Verified: fetched the pinned asset (7,339,841 bytes, matching the release
asset size), confirmed its SHA-256 against upstream's checksums file, and
extracted a valid statically linked x86-64 ELF.
Co-Authored-By: Andrew Hundt <ATHundt@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@gotspatel's Claude Desktop log showed the entire failure as:
Server transport closed unexpectedly, this is likely due to the process
exiting early
for what was a specific, nameable refusal. v0.10.3 added a JSON-RPC error for
bootstrap failures (#1539), but every EARLIER exit on the client path still
wrote to stderr only — and no MCP client surfaces stderr. So the guarantee "a
server that cannot start always says why" held for exactly one failure mode.
The worst of them was the endpoint-creation exit, which is precisely where an
ownership or ancestry refusal lands: the very failure most likely to stop a
fresh install was the one that said nothing at all.
Client-path exits now report through the same path as bootstrap failures, and
the endpoint failure carries the validation detail — so instead of a closed
transport the client receives the directory and the rule that refused it.
The reporter was extracted rather than duplicated (main_report_client_failure),
so a future exit added on this path gets the behaviour by calling one function
instead of remembering to hand-roll a JSON-RPC frame.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
On 0.9.0 -> 0.10.2, `update -y` auto-confirmed deleting other projects' indexes,
removed two .db files (~59 MB), and THEN aborted on an interactive prompt it
could not answer in a non-interactive shell. The run failed; the data was
already gone. Reported by @PsyTech-Rob.
The current code is already correct: both activation flows prompt early but
delete late, after the binary swap and agent configuration, and both return
before the deletion when configuration fails. #1538 also removed the specific
prompt that aborted. So this commit fixes nothing — it stops the fix from being
undone, which is the part that was missing. The reporter made exactly this
point: removing one bad prompt addressed the instance, not the rule.
Contract: in each flow that deletes indexes, cbm_remove_indexes() must appear
after the agent-configuration call. It keys on the CALL rather than a local
result variable, because the two flows spell that result differently, and it
fails loudly if a flow is renamed rather than passing vacuously.
Verified by reverting: moving the deletion ahead of configuration reproduces
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
#1558's shape and the contract goes red.
v0.10.3 split the activation refusal into two messages so a reservation failure
would stop blaming "active CBM sessions" for something no session caused. That
half worked. The other half did not: the new message tells the reader to "check
the errors above", and nothing is above.
The detail that names the failing component is recorded on the daemon side
(ipc_validation_detail) and was only ever read by `daemon status`. So the CLI
refusal replaced a message that blamed the WRONG thing with one that blamed
NOTHING, and two reporters — #1537 on macOS, #1416 on Linux — were left with a
refusal, a reboot that changed nothing, and no way forward.
The refusal now prints the check that actually refused:
error: activation could not reserve exclusive access; no activation was
committed.
error: this is NOT a running-session problem — nothing needs to be closed.
The check that refused was: <path>: ancestor '.cache' is not a usable
private-directory parent
The test asserts the PROPERTY rather than the wording: the message must never
point at evidence it does not show. A test pinned to the current text would
have passed happily while the dangling pointer was live, which is exactly how
this shipped.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`install` configured EVERY detected client. Someone who wanted Claude and Codex
had to revert the OpenCode and Cursor integrations by hand — and the next
`install`/`update` silently recreated them, so the choice could not be made to
stick. Reported by @PsyTech-Rob.
`--clients=claude,codex` restricts it. Omitting the flag keeps today's
behaviour, so nothing changes for anyone not asking for this.
The vocabulary is what makes the flag usable rather than decorative. 26 clients
ship here, several with tokens nobody would guess — factory-droid, mistral-vibe,
copilot-cli, kilocode. So:
- `install --clients` prints every token with its display name and an example;
- an unknown token FAILS and prints that same list. Silently treating a typo as
"that client was not detected" would configure nothing and report success,
which is the failure mode this project keeps having to fix;
- validation runs before anything is written, so a typo cannot strand a
half-configured install.
The test pins the whole table rather than a sample: every token must resolve, so
a client added to detection but forgotten in the selector's list is caught here
instead of by a user who cannot select it.
Per the maintainer's decision `install` keeps its name — splitting it into a
separate `setup` command (#1566) stays a deliberate future decision rather than
something folded into a fix batch.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Martin Vogel