diff --git a/scripts/test.sh b/scripts/test.sh index 0aafe6c20..43c140fee 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -346,6 +346,12 @@ CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_worker_error_response. echo "=== Step 5e: watcher_enabled kill-switch regression (#335) ===" CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_watcher_disabled.sh" +# Step 5f: a supervised worker is scoped to the request the daemon admitted, +# never to the CBM_ALLOWED_ROOT it inherited from the daemon starter's +# environment. Reuses the prod binary built in Step 5. +echo "=== Step 5f: worker request-scope regression ===" +CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_worker_session_scope.sh" + # Step 6: security-strings URL allow-list regression. The MSYS2 CLANG64 toolchain # bakes its package-tracker URL into the static Windows .exe; the binary string # audit must allow-list it (Windows-only — Linux smoke never saw it). diff --git a/src/main.c b/src/main.c index 8a575c44f..a836b7ac0 100644 --- a/src/main.c +++ b/src/main.c @@ -676,6 +676,36 @@ static bool cli_first_nonspace_is_brace(const char *s) { static char *main_local_cli_daemon_execute(const char *tool_name, const char *args_json, bool quiet_requested); +/* A supervised worker runs in the DAEMON's environment, not the requesting + * client's. The daemon admitted this request under the client's session policy + * and re-executed us with the canonical repo_path in the args (both daemon + * spawn paths rewrite it), so the request itself is the worker's workspace + * scope: repository == session root == allowed root. Without this, a fresh + * server fell back to the process-wide CBM_ALLOWED_ROOT inherited from whoever + * started the daemon and refused every admitted session outside it. Returns + * NULL once scoped, otherwise the reason to fail closed: a worker never indexes + * under an ambient policy. + * + * A repository that cannot be canonicalized (it does not exist) can only have + * been admitted by a session with no declared boundary, because containment + * needs a real path. The worker mirrors that with an explicit unrestricted + * policy - still never the environment fallback - so the pipeline reports the + * missing repository as the tool error it always was, instead of the + * supervisor misreading a refused worker as a crash. */ +static const char *main_index_worker_scope_request(cbm_mcp_server_t *srv, const char *args_json) { + char *repo_path = cbm_mcp_get_string_arg(args_json, "repo_path"); + if (!repo_path || !repo_path[0]) { + free(repo_path); + return "request names no repo_path"; + } + char canonical[MAIN_PATH_CAP]; + bool exists = cbm_canonical_path(repo_path, canonical, sizeof(canonical)) != 0; + const char *scope = exists ? canonical : repo_path; + bool scoped = cbm_mcp_server_set_session_context(srv, scope, exists ? scope : NULL); + free(repo_path); + return scoped ? NULL : "session context could not be installed"; +} + static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_locks, main_local_maintenance_context_t *maintenance_context) { cbm_cli_output_flags_t output_flags; @@ -817,11 +847,15 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo }; bool maintenance_binding_failed = false; bool maintenance_cancelled = false; + const char *worker_scope_refused = NULL; if (!index_worker) { result = main_local_cli_daemon_execute(tool_name, args_json, output_flags.quiet_requested); } else { srv = cbm_mcp_server_new(NULL); if (srv) { + worker_scope_refused = main_index_worker_scope_request(srv, args_json); + } + if (srv && !worker_scope_refused) { /* The in-process worker is a standalone instance: it may not * launch MCP-session background tasks. It receives project_locks * from its own process-level coordination setup and therefore @@ -834,8 +868,8 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo main_local_cli_mutation_try_begin); } } - maintenance_binding_failed = srv && !maintenance_context; - if (srv && maintenance_context) { + maintenance_binding_failed = srv && !worker_scope_refused && !maintenance_context; + if (srv && !worker_scope_refused && maintenance_context) { main_local_maintenance_server_bind(maintenance_context, srv); result = cbm_mcp_handle_tool(srv, tool_name, args_json); /* Unbind under the same mutex used by cancellation before any @@ -847,7 +881,10 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo } } if (!result) { - if (maintenance_binding_failed) { + if (worker_scope_refused) { + (void)fprintf(stderr, "error: request workspace scope invalid: %s\n", + worker_scope_refused); + } else if (maintenance_binding_failed) { (void)fprintf(stderr, "error: local %s maintenance cancellation could not bind safely\n", index_worker ? "worker" : "CLI"); diff --git a/tests/test_worker_session_scope.sh b/tests/test_worker_session_scope.sh new file mode 100644 index 000000000..fed32e97e --- /dev/null +++ b/tests/test_worker_session_scope.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# A supervised index worker (`cli --index-worker index_repository ...`) runs in +# the DAEMON's environment, not the requesting client's. The daemon has already +# admitted the request under the client's session policy and re-executes the +# worker with the canonical repo_path in its args, so the worker must scope its +# own workspace boundary to that request. Before the fix it built an unscoped +# server, fell back to the process-wide CBM_ALLOWED_ROOT it inherited from the +# daemon starter, and refused every admitted session outside that root with +# "... is outside the allowed root". A missing repo_path must fail closed: a +# worker never indexes under an ambient policy. +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +BINARY="${CBM_TEST_BINARY:-${ROOT}/build/c/codebase-memory-mcp}" +if [[ ! -x "${BINARY}" && -x "${BINARY}.exe" ]]; then + BINARY="${BINARY}.exe" +fi + +if [[ ! -x "${BINARY}" ]]; then + echo "missing binary: ${BINARY}" >&2 + exit 2 +fi + +if command -v shasum >/dev/null 2>&1; then + BUILD_FINGERPRINT="$(shasum -a 256 "${BINARY}" | awk '{print $1}')" +elif command -v sha256sum >/dev/null 2>&1; then + BUILD_FINGERPRINT="$(sha256sum "${BINARY}" | awk '{print $1}')" +elif command -v openssl >/dev/null 2>&1; then + BUILD_FINGERPRINT="$(openssl dgst -sha256 "${BINARY}" | awk '{print $NF}')" +else + echo "no SHA-256 command available for worker build binding" >&2 + exit 2 +fi +if [[ ! "${BUILD_FINGERPRINT}" =~ ^[0-9a-f]{64}$ ]]; then + echo "invalid worker build fingerprint: ${BUILD_FINGERPRINT}" >&2 + exit 2 +fi + +# shellcheck source=../scripts/test-runtime.sh +source "${ROOT}/scripts/test-runtime.sh" +cbm_test_runtime_init +tmpdir="${CBM_TEST_RUNTIME_ROOT}" +cleanup() { + cbm_test_runtime_cleanup "${BINARY}" +} +trap cleanup EXIT + +# Product-facing path, the way the daemon hands it to the worker: canonical on +# POSIX, mixed-style on a native Windows binary (test-runtime.sh's convention). +product_path() { + case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) cygpath -m "$1" ;; + *) (cd "$1" && pwd -P) ;; + esac +} + +# Root A is what the daemon's starter allowed; the admitted session lives under +# root B. The two share no prefix. +mkdir -p "${tmpdir}/root-a" "${tmpdir}/root-b/tiny" +printf 'int tiny_main(void) { return 0; }\n' >"${tmpdir}/root-b/tiny/tiny.c" +root_a="$(product_path "${tmpdir}/root-a")" +repo="$(product_path "${tmpdir}/root-b/tiny")" + +run_worker() { + local args="$1" response="$2" out="$3" err="$4" + CBM_ALLOWED_ROOT="${root_a}" \ + "${BINARY}" cli --index-worker \ + --index-worker-build "${BUILD_FINGERPRINT}" \ + index_repository "${args}" \ + --response-out "${response}" >"${out}" 2>"${err}" +} + +response="${tmpdir}/scoped.response" +if ! run_worker "{\"repo_path\":\"${repo}\",\"mode\":\"fast\"}" "${response}" \ + "${tmpdir}/scoped.out" "${tmpdir}/scoped.err"; then + echo "worker exited nonzero for an admitted request" >&2 + cat "${tmpdir}/scoped.err" >&2 + exit 1 +fi +if [[ ! -s "${response}" ]]; then + echo "worker delivered no response" >&2 + exit 1 +fi +if grep -q 'outside the allowed root' "${response}"; then + echo "worker re-decided the workspace boundary from the daemon environment" >&2 + cat "${response}" >&2 + exit 1 +fi +if ! grep -q '"status":"indexed"' "${response}"; then + echo "worker did not index the admitted repository" >&2 + cat "${response}" >&2 + exit 1 +fi + +# Fail closed: no repo_path means no request scope, so no indexing at all. +unscoped="${tmpdir}/unscoped.response" +if run_worker '{"mode":"fast"}' "${unscoped}" "${tmpdir}/unscoped.out" "${tmpdir}/unscoped.err"; then + echo "worker ran without a request workspace scope" >&2 + exit 1 +fi +if [[ -s "${unscoped}" ]] || ! grep -q 'request workspace scope invalid' "${tmpdir}/unscoped.err"; then + echo "worker without repo_path did not fail closed on scope" >&2 + cat "${tmpdir}/unscoped.err" >&2 + exit 1 +fi + +echo "ok: index worker is scoped to the admitted request, not the daemon environment"