diff --git a/Makefile b/Makefile index 27b0c6fc..48109518 100644 --- a/Makefile +++ b/Makefile @@ -32,6 +32,7 @@ UV ?= uv PYTHON ?= python GO ?= go GOFMT ?= gofmt +GO_TOOLCHAIN ?= go1.26.8 CARGO ?= cargo GO_BUILD_FLAGS ?= -trimpath -buildvcs=false GO_BUILD_CACHE_DIR ?= $(LOCAL_REPO_ROOT)/.coding-ethos/cache/go-build @@ -41,6 +42,7 @@ UV_CACHE_DIR ?= $(LOCAL_REPO_ROOT)/.coding-ethos/cache/uv export GOCACHE := $(GO_BUILD_CACHE_DIR) export GOPATH := $(GO_PATH_DIR) export GOMODCACHE := $(GO_MODULE_CACHE_DIR) +export GOTOOLCHAIN := $(GO_TOOLCHAIN) export UV_CACHE_DIR empty := @@ -109,9 +111,11 @@ GO_MODULE_ROOT_BINARY_OUTPUTS := \ GO_COVERAGE_MIN ?= 80.0 PYTHON_COVERAGE_MIN ?= 80 -GO_TEST_TIMEOUT ?= 90s +GO_TEST_TIMEOUT ?= 3m GO_COVERAGE_DIR ?= $(LOCAL_BUILD_DIR)/coverage REPO ?= $(LOCAL_REPO_ROOT) +AGENT_HOOK_STATE_ROOT ?= $(LOCAL_BUILD_DIR)/agent-hooks-state +CONSUMER_AGENT_HOOK_STATE_ROOT ?= $(LOCAL_BUILD_DIR)/consumer-agent-hooks-state PRIMARY ?= $(LOCAL_REPO_ROOT)/coding_ethos.yml REPO_ETHOS ?= REPO_CONFIG ?= @@ -198,15 +202,6 @@ define print_kv printf ' $(COLOR_ACCENT)%-24s$(COLOR_RESET) %s\n' "$(1)" "$(2)" endef -define quiet_build -tmp="$$(mktemp)"; \ -trap 'rm -f "$$tmp"' EXIT; \ -if ! $(MAKE) --no-print-directory build >"$$tmp" 2>&1; then \ - cat "$$tmp" >&2; \ - exit 1; \ -fi -endef - .PHONY: \ help \ status \ @@ -373,6 +368,13 @@ ensure-hook-runtime: ## Verify managed hook runtime artifacts already exist with printf '$(COLOR_WARN)Run `make build` explicitly before tests or diagnostics.$(COLOR_RESET)\n' >&2; \ exit 2; \ } + @for tool in coding-ethos-agent-hooks coding-ethos-policy; do \ + test -x "$(GO_TOOLS_BIN_DIR)/$$tool" || { \ + printf '$(COLOR_WARN)Managed tool is missing: $(GO_TOOLS_BIN_DIR)/%s.$(COLOR_RESET)\n' "$$tool" >&2; \ + printf '$(COLOR_WARN)Run `make build` explicitly before tests or diagnostics.$(COLOR_RESET)\n' >&2; \ + exit 2; \ + }; \ + done @test -s "$(POLICY_DIR)/policy-bundle.json" || { \ printf '$(COLOR_WARN)Compiled policy bundle is missing: $(POLICY_DIR)/policy-bundle.json.$(COLOR_RESET)\n' >&2; \ printf '$(COLOR_WARN)Run `make build` explicitly before tests or diagnostics.$(COLOR_RESET)\n' >&2; \ @@ -396,16 +398,13 @@ install-runtime: ensure-uv ## Sync only the runtime dependencies. @$(MAKE) sync-tool-configs @$(MAKE) sync-gemini-prompts -parent-install: ensure-go ## Sync parent repo coding-ethos artifacts with TOON output. - @$(quiet_build) +parent-install: ensure-hook-runtime ## Sync parent repo coding-ethos artifacts with TOON output. @"$(GO_HOOK)" parent-install --repo "$(HOOK_CONSUMER_ROOT)" -parent-check: ensure-go ## Verify parent repo coding-ethos artifacts with TOON output. - @$(quiet_build) +parent-check: ensure-hook-runtime ## Verify parent repo coding-ethos artifacts with TOON output. @"$(GO_HOOK)" parent-check --repo "$(HOOK_CONSUMER_ROOT)" -parent-lint: ensure-go ## Sync and lint the parent repo with TOON output. - @$(quiet_build) +parent-lint: ensure-hook-runtime ## Sync and lint the parent repo with TOON output. @"$(GO_HOOK)" parent-lint --repo "$(HOOK_CONSUMER_ROOT)" upgrade: upgrade-parent-submodule build parent-install parent-check parent-lint cutover-verify ## Fully upgrade a parent repo coding-ethos submodule and verify the result. @@ -566,16 +565,22 @@ _sync-consumer-agent-skills: ensure-go _sync-agent-hooks: ensure-go go-tools-install @$(call print_step,Syncing generated agent hook and MCP settings) @$(call print_info,repo: $(REPO)) - @"$(GO_TOOLS_BIN_DIR)/coding-ethos-agent-hooks" sync \ + @CODEX_HOME="$(AGENT_HOOK_STATE_ROOT)/codex-home" \ + "$(GO_TOOLS_BIN_DIR)/coding-ethos-agent-hooks" sync \ --root "$(REPO)" \ + --repo-root "$(REPO)" \ + --state-root "$(AGENT_HOOK_STATE_ROOT)" \ --hook-command "$(GO_HOOK) agent-hook" _sync-consumer-agent-hooks: ensure-go go-tools-install @if [ "$(abspath $(HOOK_CONSUMER_ROOT))" != "$(abspath $(LOCAL_REPO_ROOT))" ]; then \ $(call print_step,Syncing generated consumer agent hook and MCP settings); \ $(call print_info,repo: $(HOOK_CONSUMER_ROOT)); \ - "$(GO_TOOLS_BIN_DIR)/coding-ethos-agent-hooks" sync \ + CODEX_HOME="$(CONSUMER_AGENT_HOOK_STATE_ROOT)/codex-home" \ + "$(GO_TOOLS_BIN_DIR)/coding-ethos-agent-hooks" sync \ --root "$(HOOK_CONSUMER_ROOT)" \ + --repo-root "$(HOOK_CONSUMER_ROOT)" \ + --state-root "$(CONSUMER_AGENT_HOOK_STATE_ROOT)" \ --hook-command "$(GO_HOOK) agent-hook"; \ fi @@ -592,7 +597,7 @@ sync-provider-matrix: ensure-go go-tools-install ## Generate the provider capabi @"$(GO_TOOLS_BIN_DIR)/coding-ethos-agent-hooks" \ sync-provider-matrix --root "$(LOCAL_REPO_ROOT)" -check-provider-matrix: ensure-go go-tools-install ## Fail if the provider capability matrix is out of sync. +check-provider-matrix: ensure-hook-runtime ## Fail if the provider capability matrix is out of sync. @$(call print_step,Checking provider capability matrix) @$(call print_info,repo: $(LOCAL_REPO_ROOT)) @"$(GO_TOOLS_BIN_DIR)/coding-ethos-agent-hooks" \ @@ -607,7 +612,10 @@ purrdf-extractor-check: ## Test the optional PurRDF code-intel extractor when pr exit 1; \ }; \ $(call print_step,Testing PurRDF code-intel extractor); \ - "$(CARGO)" test --locked --manifest-path "$(PURRDF_EXTRACTOR_MANIFEST)"; \ + "$(CARGO)" test --locked \ + --config 'build.build-dir="$(PURRDF_EXTRACTOR_DIR)/target"' \ + --target-dir "$(PURRDF_EXTRACTOR_DIR)/target" \ + --manifest-path "$(PURRDF_EXTRACTOR_MANIFEST)"; \ else \ $(call print_info,PurRDF extractor subtree not present in this checkout); \ fi @@ -619,7 +627,10 @@ purrdf-extractor-install: ## Build and install the optional PurRDF extractor whe exit 1; \ }; \ $(call print_step,Building PurRDF code-intel extractor); \ - "$(CARGO)" build --locked --release --manifest-path "$(PURRDF_EXTRACTOR_MANIFEST)"; \ + "$(CARGO)" build --locked --release \ + --config 'build.build-dir="$(PURRDF_EXTRACTOR_DIR)/target"' \ + --target-dir "$(PURRDF_EXTRACTOR_DIR)/target" \ + --manifest-path "$(PURRDF_EXTRACTOR_MANIFEST)"; \ mkdir -p "$(GO_TOOLS_BIN_DIR)"; \ install -m 0755 \ "$(PURRDF_EXTRACTOR_DIR)/target/release/$(PURRDF_EXTRACTOR_BINARY)" \ @@ -654,17 +665,15 @@ go-hook-runner-install: ensure-go ## Build the bundled Go hook runner into the c _sync-git-hooks: ensure-go go-tools-install _sync-parent-hook-runtime @$(call print_step,Syncing Git hook entrypoints) - @$(call install_git_hooks,$(LOCAL_HOOKS_DIR),$(GO_HOOK)) + @$(call install_git_hooks,$(LOCAL_HOOKS_DIR),$(PARENT_HOOK_BIN_DIR)/coding-ethos-run) @if [ "$(HOOKS_DIR)" != "$(LOCAL_HOOKS_DIR)" ]; then \ $(call install_git_hooks,$(HOOKS_DIR),$(PARENT_HOOK_BIN_DIR)/coding-ethos-run); \ fi -_sync-parent-hook-runtime: ensure-go go-tools-install policy-bundle-install +_sync-parent-hook-runtime: ensure-go go-tools-install go-hook-runner-install policy-bundle-install @$(call print_step,Syncing parent hook runtime artifacts) - @mkdir -p "$(PARENT_HOOK_BIN_DIR)" "$(PARENT_POLICY_DIR)" - @cp "$(GO_TOOLS_BIN_DIR)"/coding-ethos-* "$(PARENT_HOOK_BIN_DIR)/" - @cp "$(GO_TOOLS_BIN_DIR)/cerun" "$(PARENT_HOOK_BIN_DIR)/cerun" - @cp "$(GO_TOOLS_BIN_DIR)/lint" "$(PARENT_HOOK_BIN_DIR)/lint" + @"$(GO_HOOK)" parent-runtime-sync --repo "$(HOOK_CONSUMER_ROOT)" + @mkdir -p "$(PARENT_POLICY_DIR)" @"$(GO_TOOLS_BIN_DIR)/coding-ethos-policy" compile \ --primary "$(LOCAL_REPO_ROOT)/coding_ethos.yml" \ --repo-ethos "$(LOCAL_REPO_ROOT)/repo_ethos.yml" \ @@ -688,7 +697,6 @@ _sync-parent-hook-runtime: ensure-go go-tools-install policy-bundle-install --tools-bin-dir "$(PARENT_HOOK_BIN_DIR)" \ --runner "$(PARENT_HOOK_BIN_DIR)/coding-ethos-run" \ --ethos-root "$(LOCAL_REPO_ROOT)" - @cp "$(GO_TOOLS_BIN_DIR)/coding-ethos-git-hook" "$(PARENT_HOOK_RUNTIME_DIR)/coding-ethos-git-hook" @$(call print_info,runtime: $(PARENT_HOOK_RUNTIME_DIR)) policy-bundle-install: ensure-go go-tools-install managed-toolchain-install ## Compile the policy bundle into the checkout-local build directory. @@ -715,23 +723,23 @@ cutover-install: build ## Install Git and agent hooks, then verify cutover readi @$(call print_step,Installing and verifying repo-local hook cutover) @"$(GO_HOOK)" cutover install -cutover-verify: build ## Verify Git and agent hook cutover readiness. +cutover-verify: ensure-hook-runtime ## Verify Git and agent hook cutover readiness. @$(call print_step,Verifying repo-local hook cutover) @"$(GO_HOOK)" cutover verify -pre-commit: build ## Run bundled pre-commit hooks on staged files. +pre-commit: ensure-hook-runtime ## Run bundled pre-commit hooks on staged files. @$(call print_step,Running Go pre-commit hooks on staged files) @"$(GO_HOOK)" git-hook pre-commit -pre-commit-all: build ## Run bundled pre-commit hooks on all files. +pre-commit-all: ensure-hook-runtime ## Run bundled pre-commit hooks on all files. @$(call print_step,Running Go pre-commit hooks on all files) @"$(GO_HOOK)" git-hook pre-commit --all-files -pre-push: build ## Run bundled pre-push hooks. +pre-push: ensure-hook-runtime ## Run bundled pre-push hooks. @$(call print_step,Running Go pre-push hooks) @"$(GO_HOOK)" git-hook pre-push -commit-msg: build ## Run commit-message hooks against MSG=/path/to/file. +commit-msg: ensure-hook-runtime ## Run commit-message hooks against MSG=/path/to/file. ifndef MSG @printf '$(COLOR_WARN)Usage: make commit-msg MSG=/path/to/commit-message-file$(COLOR_RESET)\n' >&2 @exit 2 @@ -740,11 +748,11 @@ else @"$(GO_HOOK)" git-hook commit-msg "$(MSG)" endif -hook-plan: build ## Print the active Go hook group plan. +hook-plan: ensure-hook-runtime ## Print the active Go hook group plan. @$(call print_step,Printing Go hook group plan) @"$(GO_HOOK)" hook-plan -validate: build ## Validate the bundled hook runtime. +validate: ensure-hook-runtime ## Validate the bundled hook runtime. @$(call print_step,Validating bundled hook runtime) @"$(GO_HOOK)" git-hook validate diff --git a/README.md b/README.md index 1207904a..3f17a9db 100644 --- a/README.md +++ b/README.md @@ -920,6 +920,7 @@ Make as a thin alias: ```bash coding-ethos/bin/coding-ethos-run parent-install +coding-ethos/bin/coding-ethos-run parent-runtime-sync coding-ethos/bin/coding-ethos-run parent-check coding-ethos/bin/coding-ethos-run parent-lint ``` @@ -931,17 +932,28 @@ install/check emit only status plus artifact-step rows, while parent lint emits the normal coding-ethos TOON lint report. See `TO_MY_PARENT.md` for the parent artifact contract. -`parent-install` rebuilds the checkout-authoritative Go tools and atomically -projects byte-identical executables into the parent repository's stable common -Git runtime. `parent-check` hashes both sides and fails if that projection is -missing, non-executable, symlinked back to a retiring checkout, or stale. - -Parent install and check are artifact workflows and do not perform a full -repository code-intel refresh. Lint and Git-hook workflows refresh code intel -when source analysis is actually part of the requested gate, so deploying or -checking the runtime cannot be delayed by an unrelated whole-repository scan. -Failed or policy-blocked hooks retain their trace evidence but do not perform a -whole-repository refresh, because no accepted source transition occurred. +`make build` rebuilds the checkout-authoritative Go tools. `parent-install` +atomically projects those already-built, byte-identical executables into the +parent repository's stable common Git runtime. `parent-check` hashes both +sides and fails if that projection is missing, non-executable, symlinked back +to a retiring checkout, or stale. +`parent-runtime-sync` is the supervisor-safe repair path: it projects the +already-built authoritative executables without rebuilding tools, rewriting +generated parent artifacts, or refreshing code intelligence. + +Builds are explicit maintenance. Diagnostic, validation, lint, commit, push, +and parent-check targets require an existing runtime and fail with a `make +build` instruction when it is absent; they never compile or install tools as a +side effect. Parent install and sync workflows operate through the already +built runner, so an authority source change must be built deliberately before +it is projected. + +Parent install, check, lint, and Git-hook workflows never perform a +whole-repository code-intel refresh. Normal hooks append their bounded trace +evidence, while managed lint and capture paths incrementally index only the +changed files already selected by the gate. Whole-repository refreshes and +derived-index rebuilds are explicit maintenance operations; allowed and +policy-blocked hooks never trigger them as a side effect. Consumer commits may include generated tool and provider configuration only when the staged Git-index bytes exactly match output rendered by the active @@ -957,12 +969,20 @@ parent artifacts remain normal consumer surfaces; repo-local state retains the runtime-ignore repair. Git hooks installed for a parent repository route through its stable common -Git runtime at `.git/coding-ethos-hooks/bin/coding-ethos-run`. They never point -at a worktree-local build path, so one worktree cannot strand every sibling's +Git runtime at +`/coding-ethos-hooks/bin/coding-ethos-run`. They never point at +a worktree-local build path, so one worktree cannot strand every sibling's hooks when its own checkout is retired or hidden by a lane sandbox. Running the supported parent workflow refreshes and verifies that shared executable projection as part of the same install/check contract. +The compiled `parent-runtime-sync` command exclusively installs shared Go +executables, including the root compatibility Git hook, with same-directory +temporary files and atomic rename. It preserves unrelated files, prunes +obsolete regular `coding-ethos-*` executables, and rejects managed entries that +are directories, symlinks, or special files. `parent-check` verifies exact +mode, SHA-256 content, regular-file shape, and the complete managed inventory. + Parent repos can opt into profile defaults in `repo_config.yaml`: ```yaml @@ -1451,16 +1471,17 @@ must write `merged.md`; otherwise the command fails. The bundled enforcement package lives under [pre-commit/](pre-commit/). It uses repo-local Git hook entrypoints that resolve directly to the compiled -`bin/coding-ethos-run` runner. +`/coding-ethos-hooks/bin/coding-ethos-run` runner. ### Git Hooks Installed Git hook entrypoints are small executable scripts that call -`bin/coding-ethos-run git-hook ` or -`bin/coding-ethos-run lfs-hook ` with the original Git arguments. The -runner repairs missing checkout-local runtime artifacts with `make build` and -dispatches to the built hook binary. -Policy selection and validation remain inside the `coding-ethos` checkout. +`/coding-ethos-hooks/bin/coding-ethos-run git-hook ` or +the corresponding `lfs-hook` command with the original Git arguments. Missing +or stale runtime artifacts fail closed with an explicit `make build` +instruction; hooks never build or repair runtime state implicitly. Policy +selection and validation remain inside the compiled runtime projected from the +selected `coding-ethos` authority. Run Git hooks: diff --git a/TO_MY_PARENT.md b/TO_MY_PARENT.md index 6a9ec9d4..1fdc38ce 100644 --- a/TO_MY_PARENT.md +++ b/TO_MY_PARENT.md @@ -12,6 +12,7 @@ Prefer the Go runner as the stable interface: ```sh coding-ethos/bin/coding-ethos-run parent-install +coding-ethos/bin/coding-ethos-run parent-runtime-sync coding-ethos/bin/coding-ethos-run parent-check coding-ethos/bin/coding-ethos-run parent-lint ``` @@ -47,10 +48,18 @@ Explicit `repo_config.yaml` settings override these profile defaults. `parent-install` syncs generated parent artifacts and atomically refreshes the compiled executables in the parent repository's common -`.git/coding-ethos-hooks/bin/` runtime. `parent-check` verifies those artifacts -without rewriting them, including byte identity and executable independence -from any retiring worktree. `parent-lint` syncs the parent artifacts, then runs -the full parent lint scope through the compiled policy bundle. +`.git/coding-ethos-hooks/bin/` runtime. `parent-runtime-sync` only projects the +already-built authoritative executables into that runtime; it does not rebuild +tools, rewrite generated parent artifacts, or refresh code intelligence. It is +the narrow interface for supervisors repairing a stale shared runtime. +`parent-check` verifies those artifacts without rewriting them, including byte +identity and executable independence from any retiring worktree. `parent-lint` +syncs the parent artifacts, then runs the full parent lint scope through the +compiled policy bundle. + +These commands execute the already-built authority. They do not silently run +`make build`; after changing or updating authority source, build it explicitly +before installing, checking, linting, or projecting it. ## Output Contract diff --git a/config.yaml b/config.yaml index e44e876c..b177c0ee 100644 --- a/config.yaml +++ b/config.yaml @@ -70,7 +70,7 @@ hooks: output_format: auto success_output: silent parallel_groups: true - tool_timeout_seconds: 900 + tool_timeout_seconds: 600 fail_severity_levels: - error - fatal diff --git a/docs/CODE_INTEL_STORAGE.md b/docs/CODE_INTEL_STORAGE.md index 6a9793bd..6b62a749 100644 --- a/docs/CODE_INTEL_STORAGE.md +++ b/docs/CODE_INTEL_STORAGE.md @@ -43,6 +43,10 @@ rebuildable analytical surface. - `code-intel sync` creates or reuses an exact v2 source generation without deleting or rewriting the v1 store. +- Normal hook, lint, commit, and push paths never rebuild or refresh the whole + repository. They append bounded event evidence and incrementally index only + files already selected by managed capture. Whole-repository work requires an + explicit code-intel maintenance command. - Eligible RDF or SPARQL requires an identity-validated pinned PurRDF helper; missing or mismatched helpers fail sync instead of degrading silently. - `code-intel status` compares the recorded source snapshot to the current diff --git a/docs/HOOK_RUNTIME_BOOTSTRAP.md b/docs/HOOK_RUNTIME_BOOTSTRAP.md index 67459b86..d129c091 100644 --- a/docs/HOOK_RUNTIME_BOOTSTRAP.md +++ b/docs/HOOK_RUNTIME_BOOTSTRAP.md @@ -18,8 +18,9 @@ Its job is limited to: Policy evaluation, policy freshness, generated prompt packs, runtime command selection, managed capture, diagnostics parsing, and hook behavior belong to the compiled Coding Ethos runtime. The checked-out Coding Ethos authority is -the source and build authority; `.git/coding-ethos-hooks` is an installed, -byte-verified projection for durable sibling-worktree execution. +the source and build authority; +`/coding-ethos-hooks` is an installed, byte-verified projection +for durable sibling-worktree execution. ## Rationale @@ -56,7 +57,7 @@ are ignored by Git. The installed hook runtime lives under the consumer repository's Git common directory: ```text -coding-ethos/ +/ bin/ coding-ethos-git-hook coding-ethos-policy @@ -76,6 +77,7 @@ coding-ethos/ /coding-ethos-hooks/ bin/{coding-ethos-*,cerun,git,lint} + coding-ethos-git-hook policy/{policy-bundle.json,policy-metadata.json} pre-commit/ build/{policy,toolchain}/ @@ -110,8 +112,15 @@ dispatching to the Go hook runtime. The Go hook runtime also resolves binary tool commands to checkout-local managed paths when possible, so `shfmt`, `shellcheck`, `actionlint`, `hadolint`, `dotenv-linter`, and `golangci-lint` do not silently fall back to host binaries. Missing required managed tools or a missing installed -manifest are runtime artifact failures and should self-repair through the same -bootstrap path as missing policy binaries. +manifest are runtime artifact failures. Runtime commands fail closed and name +the explicit `make build` repair; they do not mutate the installation as a +side effect. + +Actionlint's raw JSON-on-stdin ShellCheck dependency protocol has two required +inputs: the managed protocol marker and executable identity proving that the +immediate parent is the registered managed Actionlint binary. The marker alone +is not provenance and is rejected when inherited or supplied by another +process. The managed toolchain has two installer surfaces: @@ -188,8 +197,12 @@ Examples that require repair: - missing hook binary - non-executable hook binary +- executable with a mode different from the selected authority artifact - symlinked common-runtime executable - executable whose SHA-256 differs from the authority build +- stale regular `coding-ethos-*` executable left by an older authority +- unexpected directory, symlink, or special file in the managed executable + namespace - missing compiled policy bundle - unreadable compiled policy bundle - missing managed toolchain manifest @@ -207,6 +220,33 @@ Strict freshness validation belongs in explicit maintainer/CI commands such as `make validate`, `make cutover-verify`, and CI. Freshness is based on the source hashes recorded in `build/policy/policy-metadata.json`, not mtimes. +## Executable Projection Maintenance Contract + +The compiled Go `parent-runtime-sync` command is the only installer for the +shared Go executable projection. Its inventory is every buildable command +under `go/cmd/`, plus the root-level compatibility +`coding-ethos-git-hook`. The Makefile delegates to that command and does not +copy those executables itself. + +Within the shared runtime, the `coding-ethos-*` filename prefix is reserved for +managed executables. Sync removes obsolete regular files in that namespace, +preserves unrelated filenames, and fails without removing an obsolete entry +whose shape is a directory, symlink, or special file. Expected destinations +must also be regular files or absent. + +Each executable is written to a uniquely named temporary file in its +destination directory, assigned the authority artifact's mode, flushed, and +activated with an atomic rename. Temporary files are removed on every return. +Concurrent syncs therefore converge on one complete authority artifact rather +than exposing a partial copy. `parent-check` verifies regular-file type, exact +mode, SHA-256 content, compatibility-hook presence, and absence of stale +managed entries. + +Both the authority checkout's resolved hook directory and any consumer hook +directory are shared Git state. Installed entrypoints always name +`/coding-ethos-hooks/bin/coding-ethos-run`; no hook installation +may embed the selected worktree's `bin/coding-ethos-run` path. + ## Safety Requirements Bootstrap needs a few guardrails: diff --git a/docs/RELEASE.md b/docs/RELEASE.md index e5d31c4e..cb01717f 100644 --- a/docs/RELEASE.md +++ b/docs/RELEASE.md @@ -49,6 +49,14 @@ The normal upgrade path is: 4. Run the project gate, normally `make check`. 5. Review the release notes for any release-specific migration steps. +### Hook timeout ceiling migration + +Before upgrading a consumer that sets `hooks.tool_timeout_seconds`, reduce any +value from 601 through 900 to 600 seconds or less. Those former values now +produce a FATAL configuration error and intentionally block pre-commit and +pre-push: a hook running longer than ten minutes is a critical failure, not a +normal operating mode. + ## Pre-Release Checklist - [ ] Prepare release metadata on a release branch or use the documented diff --git a/go.work b/go.work index 4caee6e1..7ac814f4 100644 --- a/go.work +++ b/go.work @@ -3,6 +3,8 @@ go 1.26 +toolchain go1.26.8 + use ( ./go ) diff --git a/go/cmd/coding-ethos-run/args.go b/go/cmd/coding-ethos-run/args.go index ad9501d5..0258984e 100644 --- a/go/cmd/coding-ethos-run/args.go +++ b/go/cmd/coding-ethos-run/args.go @@ -268,7 +268,7 @@ func (paths runtimePaths) withCommandRoots(args []string) runtimePaths { repoRoot = flagValue(args[1:], "--root", paths.Root) case "runtime-policy": repoRoot = flagValue(args[1:], "--repo", paths.Root) - case "parent-install", "parent-check", "parent-lint": + case "parent-install", "parent-runtime-sync", "parent-check", "parent-lint": repoRoot = flagValue(args[1:], "--repo", paths.Root) default: return paths @@ -294,6 +294,10 @@ func shouldLogRuntimeCommand(args []string) bool { return false } + if slices.Contains([]string{"--help", "-h", "--version"}, args[0]) { + return false + } + if args[0] == "cutover" || args[0] == "lfs-hook" { return false } diff --git a/go/cmd/coding-ethos-run/dispatch.go b/go/cmd/coding-ethos-run/dispatch.go index 58b85b57..e3186810 100644 --- a/go/cmd/coding-ethos-run/dispatch.go +++ b/go/cmd/coding-ethos-run/dispatch.go @@ -106,6 +106,7 @@ func runCommandEntries() []runCommandEntry { {Command: "policy-tool-group", Handler: runPolicyToolGroup}, {Command: "policy-git", Handler: runPolicyGitHandler}, {Command: "parent-install", Handler: runParentInstall}, + {Command: "parent-runtime-sync", Handler: runParentRuntimeSync}, {Command: "parent-check", Handler: runParentCheck}, {Command: "parent-lint", Handler: runParentLint}, {Command: "runtime-policy", Handler: runRuntimePolicy}, @@ -164,6 +165,10 @@ func runHelpMessage() feedback.Message { "runtime-policy sync|check --repo ", "Manage only the consumer-scoped hook policy bundle.", }, + { + "parent-runtime-sync --repo ", + "Project prebuilt hook executables into a consumer Git runtime.", + }, {"git-hook", "Git hook entrypoint; not for manual lint."}, }, ), @@ -1008,10 +1013,42 @@ func runAgentHooksCommand(paths runtimePaths, rest []string) { } func runPolicyTool(paths runtimePaths, rest []string) error { + return runPolicyToolWithParentResolver( + paths, + rest, + policyToolParentExecutable, + ) +} + +func runPolicyToolWithParentResolver( + paths runtimePaths, + rest []string, + resolveParent func() (string, error), +) error { + protocolMarker := os.Getenv(toolprotocol.ActionlintShellcheckEnv) + if len(rest) == 0 || !toolprotocol.IsActionlintShellcheckJSONStdin( + protocolMarker, + rest[0], + rest[1:], + ) { + return runPolicyToolForProtocol(paths, rest, protocolMarker, "") + } + + parentExecutable, err := resolveParent() + if err != nil { + requirePolicyBundle(paths) + + return fmt.Errorf( + "authenticate actionlint ShellCheck protocol parent: %w", + err, + ) + } + return runPolicyToolForProtocol( paths, rest, - os.Getenv(toolprotocol.ActionlintShellcheckEnv), + protocolMarker, + parentExecutable, ) } @@ -1019,6 +1056,7 @@ func runPolicyToolForProtocol( paths runtimePaths, rest []string, protocolMarker string, + parentExecutable string, ) error { if len(rest) == 0 { return apperror.StaticError("policy-tool requires a tool name") @@ -1031,6 +1069,12 @@ func runPolicyToolForProtocol( rest[0], rest[1:], ) { + if !managedActionlintParent(paths, parentExecutable) { + return apperror.StaticError( + "actionlint ShellCheck protocol requires the managed actionlint parent", + ) + } + tool, found := toolcatalog.HookOwnedTool("shellcheck") if !found { return apperror.StaticError("managed shellcheck tool is not registered") @@ -1048,6 +1092,29 @@ func runPolicyToolForProtocol( return nil } +func managedActionlintParent(paths runtimePaths, parentExecutable string) bool { + tool, found := toolcatalog.HookOwnedTool(toolprotocol.ActionlintTool) + if !found || strings.TrimSpace(parentExecutable) == "" { + return false + } + + expected := tool.ManagedExecutablePath(paths.EthosRoot) + + expectedInfo, err := os.Lstat(expected) + if err != nil || !expectedInfo.Mode().IsRegular() || + expectedInfo.Mode()&0o111 == 0 { + return false + } + + parentInfo, err := os.Stat(parentExecutable) + if err != nil || !parentInfo.Mode().IsRegular() || + parentInfo.Mode()&0o111 == 0 { + return false + } + + return os.SameFile(expectedInfo, parentInfo) +} + func runMCP(paths runtimePaths, rest []string) { bundlePath := hookPolicyBundlePath(paths) requireRuntimeFile(bundlePath, "compiled policy bundle") diff --git a/go/cmd/coding-ethos-run/main_test.go b/go/cmd/coding-ethos-run/main_test.go index 6fb3f717..2a1478fd 100644 --- a/go/cmd/coding-ethos-run/main_test.go +++ b/go/cmd/coding-ethos-run/main_test.go @@ -12,13 +12,16 @@ import ( "path/filepath" "runtime" "slices" + "strconv" "strings" + "sync" "testing" "time" "blackcat.ca/coding-ethos/go/internal/apperror" "blackcat.ca/coding-ethos/go/internal/codeintel" "blackcat.ca/coding-ethos/go/internal/debuglog" + "blackcat.ca/coding-ethos/go/internal/geminiprompts" "blackcat.ca/coding-ethos/go/internal/gitwrap" "blackcat.ca/coding-ethos/go/internal/hookoutput" "blackcat.ca/coding-ethos/go/internal/policy" @@ -407,6 +410,16 @@ func TestWithCommandRootsUsesCommandSpecificRepositoryFlags(t *testing.T) { "/private/state", }, }, + { + name: "parent runtime sync", + args: []string{ + "parent-runtime-sync", + "--repo", + "/repo", + "--state-root", + "/private/state", + }, + }, { name: "parent check", args: []string{ @@ -443,8 +456,15 @@ func TestWithCommandRootsUsesCommandSpecificRepositoryFlags(t *testing.T) { func TestCapabilitiesDiscoveryDoesNotWriteRuntimeLog(t *testing.T) { t.Parallel() - if shouldLogRuntimeCommand([]string{"agent-hooks", "capabilities"}) { - t.Fatal("capabilities discovery should be read-only") + for _, args := range [][]string{ + {"agent-hooks", "capabilities"}, + {"--help"}, + {"-h"}, + {"--version"}, + } { + if shouldLogRuntimeCommand(args) { + t.Fatalf("read-only discovery %q should not write a runtime log", args) + } } if !shouldLogRuntimeCommand([]string{"agent-hooks", "doctor"}) { t.Fatal("doctor should retain runtime logging") @@ -631,9 +651,10 @@ func TestParentCommandsRejectInvalidFlags(t *testing.T) { paths := runtimePaths{Root: "/repo"} for name, runCommand := range map[string]func(runtimePaths, []string) error{ - "parent-install": runParentInstall, - "parent-check": runParentCheck, - "parent-lint": runParentLint, + "parent-install": runParentInstall, + "parent-runtime-sync": runParentRuntimeSync, + "parent-check": runParentCheck, + "parent-lint": runParentLint, } { err := runCommand(paths, []string{"--missing-flag"}) if err == nil || !strings.Contains(err.Error(), "parse "+name+" flags") { @@ -784,14 +805,48 @@ func TestParentGoToolsCheckFailsWhenBinaryIsStale(t *testing.T) { touchParentGoTools(t, paths, oldTime) writeParentGoSource(t, sourceRoot, "internal/runtime/source.go", newTime) - err := checkParentGoTools(paths, parentWorkflowOptions{Repo: paths.Root}) + err := checkParentGoTools(paths) if err == nil || !strings.Contains(err.Error(), "parent Go tools are stale") || + !strings.Contains(err.Error(), "make build") || !strings.Contains(err.Error(), "coding-ethos-run(stale)") { t.Fatalf("checkParentGoTools error = %v", err) } } +func TestParentArtifactSyncRefusesToRebuildStaleTools(t *testing.T) { + t.Parallel() + + paths := runtimeTestPaths(t) + sourceRoot := parentGoToolsSourceFixture(t, paths.EthosRoot) + paths.ToolsSource = sourceRoot + + oldTime := time.Now().Add(-2 * time.Hour) + newTime := time.Now().Add(-1 * time.Hour) + touchParentGoTools(t, paths, oldTime) + writeParentGoSource(t, sourceRoot, "internal/runtime/source.go", newTime) + + toolPath := filepath.Join(paths.BinDir, "coding-ethos-run") + before, err := os.ReadFile(toolPath) + if err != nil { + t.Fatalf("read tool before sync: %v", err) + } + + steps := syncParentArtifacts(paths, parentWorkflowOptions{Repo: paths.Root}) + if len(steps) != 1 || steps[0].Name != "go_tools" || + steps[0].Status != parentStepFail { + t.Fatalf("sync steps = %#v, want one failed go_tools check", steps) + } + + after, err := os.ReadFile(toolPath) + if err != nil { + t.Fatalf("read tool after sync: %v", err) + } + if string(after) != string(before) { + t.Fatal("parent artifact sync rebuilt a stale executable") + } +} + func TestParentGoToolsCheckPassesWhenBinariesAreCurrent(t *testing.T) { t.Parallel() @@ -805,7 +860,7 @@ func TestParentGoToolsCheckPassesWhenBinariesAreCurrent(t *testing.T) { writeParentGoSource(t, sourceRoot, "internal/runtime/source.go", oldTime) touchParentGoTools(t, paths, newTime) - err := checkParentGoTools(paths, parentWorkflowOptions{Repo: paths.Root}) + err := checkParentGoTools(paths) if err != nil { t.Fatalf("checkParentGoTools: %v", err) } @@ -853,7 +908,7 @@ func TestParentHookRuntimeSyncConvergesAndCheckDetectsDrift(t *testing.T) { } } -func TestParentHookRuntimeCheckRejectsCheckoutSymlink(t *testing.T) { +func TestParentHookRuntimeSyncRepairsExpectedDestinationSymlinks(t *testing.T) { t.Parallel() paths := runtimeTestPaths(t) @@ -882,6 +937,239 @@ func TestParentHookRuntimeCheckRejectsCheckoutSymlink(t *testing.T) { !strings.Contains(err.Error(), "coding-ethos-run(not_regular)") { t.Fatalf("check symlink error = %v", err) } + + err = syncParentHookRuntimeExecutables(paths, options) + if err != nil { + t.Fatalf("repair expected runtime symlinks: %v", err) + } + + err = checkParentHookRuntimeExecutables(paths, options) + if err != nil { + t.Fatalf("check repaired expected runtime symlinks: %v", err) + } + + for _, tool := range parentGoToolFixtureCommands() { + destination := filepath.Join(runtimeBin, tool) + info, infoErr := os.Lstat(destination) + if infoErr != nil { + t.Fatalf("stat repaired %s: %v", tool, infoErr) + } + if !info.Mode().IsRegular() { + t.Fatalf("repaired %s is not a regular file: %v", tool, info.Mode()) + } + } +} + +func TestParentHookRuntimeSyncPrunesOnlyStaleManagedExecutables(t *testing.T) { + t.Parallel() + + paths := runtimeTestPaths(t) + paths.ToolsSource = parentGoToolsSourceFixture(t, paths.EthosRoot) + touchParentGoTools(t, paths, time.Now()) + options := parentWorkflowOptions{Repo: paths.Root} + + if err := syncParentHookRuntimeExecutables(paths, options); err != nil { + t.Fatalf("initial runtime sync: %v", err) + } + + runtimeBin := parentHookRuntimeBinDir(paths, options) + stale := filepath.Join(runtimeBin, "coding-ethos-retired") + unrelated := filepath.Join(runtimeBin, "operator-helper") + writeExecutableFixture(t, stale, "retired\n") + writeExecutableFixture(t, unrelated, "preserve\n") + + err := checkParentHookRuntimeExecutables(paths, options) + if !errors.Is(err, errParentArtifactDrift) || + !strings.Contains(err.Error(), "coding-ethos-retired(stale)") { + t.Fatalf("stale runtime check error = %v", err) + } + + if err = syncParentHookRuntimeExecutables(paths, options); err != nil { + t.Fatalf("prune stale runtime: %v", err) + } + + if _, err = os.Lstat(stale); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("stale managed executable still exists: %v", err) + } + if _, err = os.Lstat(unrelated); err != nil { + t.Fatalf("unrelated runtime file was not preserved: %v", err) + } +} + +func TestParentHookRuntimeSyncRejectsUnsafeManagedShape(t *testing.T) { + t.Parallel() + + paths := runtimeTestPaths(t) + paths.ToolsSource = parentGoToolsSourceFixture(t, paths.EthosRoot) + touchParentGoTools(t, paths, time.Now()) + options := parentWorkflowOptions{Repo: paths.Root} + + if err := syncParentHookRuntimeExecutables(paths, options); err != nil { + t.Fatalf("initial runtime sync: %v", err) + } + + target := filepath.Join(t.TempDir(), "outside") + writeExecutableFixture(t, target, "outside\n") + unsafe := filepath.Join( + parentHookRuntimeBinDir(paths, options), + "coding-ethos-retired", + ) + if err := os.Symlink(target, unsafe); err != nil { + t.Fatalf("create stale managed symlink: %v", err) + } + + err := syncParentHookRuntimeExecutables(paths, options) + if !errors.Is(err, errParentRuntimeUnsafeShape) { + t.Fatalf("unsafe runtime sync error = %v", err) + } + if _, err = os.Lstat(unsafe); err != nil { + t.Fatalf("unsafe managed entry was unexpectedly removed: %v", err) + } +} + +func TestParentHookRuntimeCheckVerifiesModeAndCompatibilityHook(t *testing.T) { + t.Parallel() + + paths := runtimeTestPaths(t) + paths.ToolsSource = parentGoToolsSourceFixture(t, paths.EthosRoot) + touchParentGoTools(t, paths, time.Now()) + options := parentWorkflowOptions{Repo: paths.Root} + + if err := syncParentHookRuntimeExecutables(paths, options); err != nil { + t.Fatalf("runtime sync: %v", err) + } + + compatibility := filepath.Join( + parentHookRuntimeDir(paths, options), + parentCompatibilityHook, + ) + status, err := compareParentHookRuntimeExecutable( + filepath.Join(paths.BinDir, parentCompatibilityHook), + compatibility, + ) + if err != nil || status != "" { + t.Fatalf("compatibility hook status = %q, error = %v", status, err) + } + + installed := filepath.Join( + parentHookRuntimeBinDir(paths, options), + "coding-ethos-run", + ) + if err = os.Chmod(installed, 0o755); err != nil { + t.Fatalf("change installed mode: %v", err) + } + + err = checkParentHookRuntimeExecutables(paths, options) + if !errors.Is(err, errParentArtifactDrift) || + !strings.Contains(err.Error(), "coding-ethos-run(mode)") { + t.Fatalf("mode drift error = %v", err) + } +} + +func TestParentHookRuntimeExecutableInstallIsAtomicUnderConcurrency(t *testing.T) { + t.Parallel() + + directory := t.TempDir() + destination := filepath.Join(directory, "coding-ethos-run") + sources := make([]string, 8) + wantContent := map[string]struct{}{} + + for index := range sources { + source := filepath.Join(directory, "source-"+strconv.Itoa(index)) + content := "payload-" + strconv.Itoa(index) + "\n" + writeExecutableFixture(t, source, content) + sources[index] = source + wantContent[content] = struct{}{} + } + + var wait sync.WaitGroup + errorsCh := make(chan error, len(sources)) + for _, source := range sources { + wait.Add(1) + + go func() { + defer wait.Done() + + errorsCh <- installParentHookRuntimeExecutable(source, destination) + }() + } + + wait.Wait() + close(errorsCh) + + for err := range errorsCh { + if err != nil { + t.Fatalf("concurrent executable install: %v", err) + } + } + + payload, err := os.ReadFile(destination) + if err != nil { + t.Fatalf("read concurrent install result: %v", err) + } + if _, found := wantContent[string(payload)]; !found { + t.Fatalf("installed partial or unknown content %q", payload) + } + + temporary, err := filepath.Glob(filepath.Join(directory, ".coding-ethos-run-*.tmp")) + if err != nil { + t.Fatalf("find temporary executables: %v", err) + } + if len(temporary) != 0 { + t.Fatalf("temporary executables were not cleaned up: %#v", temporary) + } +} + +func TestParentHookRuntimeSyncUsesLinkedWorktreeCommonDirectory(t *testing.T) { + t.Parallel() + + gitPath, err := exec.LookPath("git") + if err != nil { + t.Fatalf("find git: %v", err) + } + + primary := t.TempDir() + runRunnerTestGit(t, primary, "init", "--initial-branch", "main") + tracked := filepath.Join(primary, "tracked.txt") + if err = os.WriteFile(tracked, []byte("tracked\n"), 0o600); err != nil { + t.Fatalf("write tracked fixture: %v", err) + } + runRunnerTestGit(t, primary, "add", "tracked.txt") + runRunnerTestGit( + t, + primary, + "-c", "user.name=Test", + "-c", "user.email=test@example.invalid", + "commit", "-m", "fixture", + ) + + linked := filepath.Join(t.TempDir(), "linked") + runRunnerTestGit(t, primary, "worktree", "add", "--detach", linked) + + paths := runtimeTestPaths(t) + paths.RealGit = gitPath + paths.ToolsSource = parentGoToolsSourceFixture(t, paths.EthosRoot) + touchParentGoTools(t, paths, time.Now()) + options := parentWorkflowOptions{Repo: linked} + + if err = syncParentHookRuntimeExecutables(paths, options); err != nil { + t.Fatalf("sync linked-worktree runtime: %v", err) + } + + commonDir, err := gitOutput( + gitPath, + linked, + "rev-parse", + "--path-format=absolute", + "--git-common-dir", + ) + if err != nil { + t.Fatalf("resolve linked-worktree common dir: %v", err) + } + want := filepath.Join(commonDir, "coding-ethos-hooks", "bin", "coding-ethos-run") + if _, err = os.Lstat(want); err != nil { + t.Fatalf("stable linked-worktree runtime missing at %s: %v", want, err) + } } func TestSyncParentPolicyBundleUsesParentRepoConfig(t *testing.T) { @@ -1002,73 +1290,6 @@ repo: } } -func TestRebuildParentGoToolsBuildsRepoLocalBinaries(t *testing.T) { - t.Parallel() - - paths := runtimeTestPaths(t) - sourceRoot := parentBuildableGoToolsSourceFixture(t, paths.EthosRoot) - paths.ToolsSource = sourceRoot - - err := rebuildParentGoTools(paths) - if err != nil { - t.Fatalf("rebuildParentGoTools: %v", err) - } - - tools, err := parentGoToolCommands(paths) - if err != nil { - t.Fatalf("parentGoToolCommands: %v", err) - } - - for _, tool := range tools { - toolPath := filepath.Join(paths.BinDir, tool) - - info, err := os.Stat(toolPath) - if err != nil { - t.Fatalf("stat rebuilt tool %s: %v", tool, err) - } - - if info.IsDir() || info.Mode()&0o111 == 0 { - t.Fatalf("rebuilt tool %s not executable: %#v", tool, info.Mode()) - } - } -} - -func TestBuildParentGoToolKeepsExistingBinaryWhenBuildFails(t *testing.T) { - t.Parallel() - - paths := runtimeTestPaths(t) - sourceRoot := parentBuildableGoToolsSourceFixture(t, paths.EthosRoot) - paths.ToolsSource = sourceRoot - - tool := "coding-ethos-run" - toolPath := filepath.Join(paths.BinDir, tool) - original := "#!/usr/bin/env sh\necho original\n" - writeExecutableFixture(t, toolPath, original) - - err := os.WriteFile( - filepath.Join(sourceRoot, "cmd", tool, "main.go"), - []byte("package main\n\nfunc main(\n"), - 0o600, - ) - if err != nil { - t.Fatalf("write invalid main: %v", err) - } - - err = buildParentGoTool(paths, tool) - if err == nil { - t.Fatalf("buildParentGoTool succeeded with invalid source") - } - - current, err := os.ReadFile(toolPath) - if err != nil { - t.Fatalf("read existing tool: %v", err) - } - - if string(current) != original { - t.Fatalf("existing tool changed after failed build: %q", string(current)) - } -} - func TestRequireParentGoToolsSourceRejectsFile(t *testing.T) { t.Parallel() @@ -1371,6 +1592,7 @@ func TestRuntimePathResolutionFallbacks(t *testing.T) { //nolint:paralleltest // Serializes process-global runtime environment state. func TestRuntimePathResolutionUsesGitWhenAvailable(t *testing.T) { testlock.ProcessState(t, "coding-ethos-run-env") + t.Setenv("CODE_ETHOS_CONSUMER_ROOT", "") repo := filepath.Join(t.TempDir(), "repo") hooks := filepath.Join(repo, ".git", "hooks") @@ -1389,6 +1611,7 @@ func TestRuntimePathResolutionUsesGitWhenAvailable(t *testing.T) { //nolint:paralleltest // Serializes process-global runtime environment state. func TestRuntimePathResolutionKeepsSubmoduleCheckoutLocal(t *testing.T) { testlock.ProcessState(t, "coding-ethos-run-env") + t.Setenv("CODE_ETHOS_CONSUMER_ROOT", "") parent := filepath.Join(t.TempDir(), "parent") repo := filepath.Join(parent, "coding-ethos") @@ -1866,6 +2089,8 @@ func TestAgentShellSandboxPlanRoutesThroughNativeWrapper(t *testing.T) { if runtime.GOOS != "linux" { t.Skip("agent-shell native sandbox is Linux-only") } + t.Setenv("CODING_ETHOS_AGENT_SHELL_SANDBOX", "") + t.Setenv("CODING_ETHOS_SANDBOX_ACTIVE", "") home := t.TempDir() runtimeDir := t.TempDir() @@ -1923,6 +2148,7 @@ func TestAgentShellSandboxPlanRoutesThroughNativeWrapper(t *testing.T) { t.Setenv(envAgentAPIProxyURL, "http://127.0.0.1:18080") paths := runtimeTestPaths(t) + paths.StateRoot = t.TempDir() paths.GitDir = filepath.Join( filepath.Dir(paths.GitCommonDir), "worktrees", @@ -2007,6 +2233,7 @@ func TestAgentShellSandboxPlanRoutesThroughNativeWrapper(t *testing.T) { for _, want := range []string{ paths.GitDir, paths.GitCommonDir, + filepath.Join(paths.StateRoot, ".coding-ethos"), filepath.Join(home, ".gnupg"), resolvedGPGHome, resolvedPrivateKeys, @@ -2032,6 +2259,118 @@ func TestAgentShellSandboxPlanRoutesThroughNativeWrapper(t *testing.T) { } } +func TestAgentShellWritePathsAdmitsOnlyDistinctPrivateStateArtifacts(t *testing.T) { + t.Parallel() + + root := t.TempDir() + stateRoot := t.TempDir() + + got, err := agentShellWritePaths(root, stateRoot) + if err != nil { + t.Fatalf("agentShellWritePaths() error = %v", err) + } + + want := filepath.Join(stateRoot, ".coding-ethos") + if !slices.Contains(got, want) { + t.Fatalf("write paths missing private state artifacts %q: %#v", want, got) + } + info, err := os.Lstat(want) + if err != nil { + t.Fatalf("inspect private state artifacts: %v", err) + } + if !info.IsDir() || info.Mode().Perm() != 0o700 { + t.Fatalf("private state artifacts mode = %v, want directory 0700", info.Mode()) + } + if slices.Contains(got, stateRoot) { + t.Fatalf("write paths grant broad state root %q: %#v", stateRoot, got) + } +} + +func TestAgentShellExternalStateArtifactsOnlyTightenExistingPermissions(t *testing.T) { + t.Parallel() + + for _, testCase := range []struct { + name string + mode os.FileMode + want os.FileMode + }{ + {name: "already private", mode: 0o700, want: 0o700}, + {name: "stricter owner mode", mode: 0o500, want: 0o500}, + {name: "group readable", mode: 0o750, want: 0o700}, + } { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + stateRoot := t.TempDir() + artifactRoot := filepath.Join(stateRoot, ".coding-ethos") + if err := os.Mkdir(artifactRoot, testCase.mode); err != nil { + t.Fatalf("create external state artifacts: %v", err) + } + + got, err := ensureAgentShellExternalStateArtifacts(stateRoot) + if err != nil { + t.Fatalf("ensureAgentShellExternalStateArtifacts() error = %v", err) + } + if got != artifactRoot { + t.Fatalf("artifact root = %q, want %q", got, artifactRoot) + } + + info, err := os.Lstat(artifactRoot) + if err != nil { + t.Fatalf("inspect external state artifacts: %v", err) + } + if info.Mode().Perm() != testCase.want { + t.Fatalf("external state mode = %v, want %v", info.Mode().Perm(), testCase.want) + } + }) + } +} + +func TestAgentShellWritePathsKeepsSameRootPolicyTreeProtected(t *testing.T) { + t.Parallel() + + root := t.TempDir() + got, err := agentShellWritePaths(root, root) + if err != nil { + t.Fatalf("agentShellWritePaths() error = %v", err) + } + + if !slices.Contains(got, geminiprompts.PromptPackPath) { + t.Fatalf( + "write paths missing exact Gemini prompt pack %q: %#v", + geminiprompts.PromptPackPath, + got, + ) + } + + for _, protected := range []string{ + filepath.Join(root, ".coding-ethos"), + ".coding-ethos", + ".coding-ethos/gemini", + } { + if slices.Contains(got, protected) { + t.Fatalf("write paths grant protected policy path %q: %#v", protected, got) + } + } +} + +func TestAgentShellExternalStateWritePathRejectsUnsafeRoots(t *testing.T) { + t.Parallel() + + root := t.TempDir() + symlinkTarget := t.TempDir() + symlinkStateRoot := filepath.Join(t.TempDir(), "state-link") + if err := os.Symlink(symlinkTarget, symlinkStateRoot); err != nil { + t.Fatalf("create state-root symlink: %v", err) + } + + for _, stateRoot := range []string{"relative/state", string(filepath.Separator), symlinkStateRoot} { + if _, err := agentShellExternalStateWritePath(root, stateRoot); err == nil { + t.Fatalf("agentShellExternalStateWritePath(%q) succeeded", stateRoot) + } + } +} + func TestAgentShellGitWritePathsDeduplicatesGitDirs(t *testing.T) { t.Parallel() @@ -2325,6 +2664,8 @@ func TestAgentShellSandboxPlanFailsClosedWithoutNativeHelper(t *testing.T) { if runtime.GOOS != "linux" { t.Skip("agent-shell native sandbox is Linux-only") } + t.Setenv("CODING_ETHOS_AGENT_SHELL_SANDBOX", "") + t.Setenv("CODING_ETHOS_SANDBOX_ACTIVE", "") paths := runtimeTestPaths(t) @@ -2611,7 +2952,7 @@ func TestPolicyGitIgnoresArbitraryEnvRealGitExecutable(t *testing.T) { } } -func TestPolicyToolUsesMarkedActionlintShellcheckProtocolWithoutParentPath( +func TestPolicyToolUsesAuthenticatedActionlintShellcheckProtocol( t *testing.T, ) { paths := runtimeTestPaths(t) @@ -2629,6 +2970,16 @@ func TestPolicyToolUsesMarkedActionlintShellcheckProtocolWithoutParentPath( } writeExecutableFixture(t, shellcheck, "#!/usr/bin/env sh\nexit 0\n") + actionlintTool, found := toolcatalog.HookOwnedTool(toolprotocol.ActionlintTool) + if !found { + t.Fatal("managed actionlint tool is not registered") + } + actionlint := actionlintTool.ManagedExecutablePath(paths.EthosRoot) + if err := os.MkdirAll(filepath.Dir(actionlint), 0o755); err != nil { + t.Fatalf("create managed actionlint fixture directory: %v", err) + } + writeExecutableFixture(t, actionlint, "#!/usr/bin/env sh\nexit 0\n") + args := []string{ "shellcheck", "--norc", @@ -2641,6 +2992,7 @@ func TestPolicyToolUsesMarkedActionlintShellcheckProtocolWithoutParentPath( paths, args, toolprotocol.ActionlintShellcheckJSONStdinV1, + actionlint, ) if err != nil { t.Fatalf("run actionlint shellcheck dependency: %v", err) @@ -2652,6 +3004,96 @@ func TestPolicyToolUsesMarkedActionlintShellcheckProtocolWithoutParentPath( } } +func TestPolicyToolRejectsSpoofedActionlintShellcheckProtocol(t *testing.T) { + paths := runtimeTestPaths(t) + var calls []string + paths.Executor = stubRuntimeOps{calls: &calls} + + actionlintTool, found := toolcatalog.HookOwnedTool(toolprotocol.ActionlintTool) + if !found { + t.Fatal("managed actionlint tool is not registered") + } + actionlint := actionlintTool.ManagedExecutablePath(paths.EthosRoot) + if err := os.MkdirAll(filepath.Dir(actionlint), 0o755); err != nil { + t.Fatalf("create managed actionlint fixture directory: %v", err) + } + writeExecutableFixture(t, actionlint, "#!/usr/bin/env sh\nexit 0\n") + + attacker := filepath.Join(t.TempDir(), "actionlint") + writeExecutableFixture(t, attacker, "#!/usr/bin/env sh\nexit 0\n") + + err := runPolicyToolForProtocol( + paths, + []string{"shellcheck", "-f", "json", "-"}, + toolprotocol.ActionlintShellcheckJSONStdinV1, + attacker, + ) + if err == nil || !strings.Contains(err.Error(), "managed actionlint parent") { + t.Fatalf("spoofed actionlint protocol error = %v", err) + } + if len(calls) != 0 { + t.Fatalf("spoofed actionlint protocol executed a tool: %#v", calls) + } +} + +func TestPolicyToolRejectsProtocolWhenParentIdentityCannotBeResolved( + t *testing.T, +) { + paths := runtimeTestPaths(t) + var calls []string + paths.Executor = stubRuntimeOps{calls: &calls} + t.Setenv( + toolprotocol.ActionlintShellcheckEnv, + toolprotocol.ActionlintShellcheckJSONStdinV1, + ) + + err := runPolicyToolWithParentResolver( + paths, + []string{"shellcheck", "-f", "json", "-"}, + func() (string, error) { + return "", apperror.StaticError("parent identity unavailable") + }, + ) + if err == nil || !strings.Contains( + err.Error(), + "authenticate actionlint ShellCheck protocol parent: parent identity unavailable", + ) { + t.Fatalf("unresolved parent identity error = %v", err) + } + if len(calls) != 0 { + t.Fatalf("unresolved parent identity executed a tool: %#v", calls) + } +} + +func TestPolicyToolDoesNotResolveParentWithoutActionlintProtocol(t *testing.T) { + paths := runtimeTestPaths(t) + var calls []string + paths.Executor = stubRuntimeOps{calls: &calls} + t.Setenv(toolprotocol.ActionlintShellcheckEnv, "") + called := false + + err := runPolicyToolWithParentResolver( + paths, + []string{"shellcheck", "-f", "json", "-"}, + func() (string, error) { + called = true + + return "", apperror.StaticError("parent identity unavailable") + }, + ) + if err != nil { + t.Fatalf("run direct shellcheck: %v", err) + } + if called { + t.Fatal("direct shellcheck resolved an irrelevant parent identity") + } + + joined := strings.Join(calls, "\n") + if !strings.Contains(joined, "exec-lint:") || strings.Contains(joined, "execpath:") { + t.Fatalf("direct shellcheck bypassed managed capture: %#v", calls) + } +} + func TestPolicyToolCapturesShellcheckWithoutActionlintMarker(t *testing.T) { paths := runtimeTestPaths(t) var calls []string @@ -2661,6 +3103,7 @@ func TestPolicyToolCapturesShellcheckWithoutActionlintMarker(t *testing.T) { paths, []string{"shellcheck", "-f", "json", "-"}, "", + "", ) if err != nil { t.Fatalf("run direct shellcheck: %v", err) @@ -3458,9 +3901,10 @@ func TestMakefileRoutesParentGitHooksThroughStableCommonRuntime(t *testing.T) { makefile := string(payload) for _, want := range []string{ - `$(call install_git_hooks,$(LOCAL_HOOKS_DIR),$(GO_HOOK))`, + `$(call install_git_hooks,$(LOCAL_HOOKS_DIR),$(PARENT_HOOK_BIN_DIR)/coding-ethos-run)`, `$(call install_git_hooks,$(HOOKS_DIR),$(PARENT_HOOK_BIN_DIR)/coding-ethos-run)`, `_sync-git-hooks: ensure-go go-tools-install _sync-parent-hook-runtime`, + `"$(GO_HOOK)" parent-runtime-sync --repo "$(HOOK_CONSUMER_ROOT)"`, "--dest-dir \"$(PARENT_HOOK_BIN_DIR)\" \\\n" + "\t\t--real-git \"$(GIT)\" \\\n" + "\t\t--runner \"$(PARENT_HOOK_BIN_DIR)/coding-ethos-run\"", @@ -3472,14 +3916,25 @@ func TestMakefileRoutesParentGitHooksThroughStableCommonRuntime(t *testing.T) { if strings.Contains( makefile, - `$(call install_git_hooks,$(HOOKS_DIR),$(GO_HOOK))`, + `$(call install_git_hooks,$(LOCAL_HOOKS_DIR),$(GO_HOOK))`, ) { - t.Fatal("Makefile routes parent Git hooks through a worktree-local runner") + t.Fatal("Makefile routes shared local Git hooks through a worktree-local runner") } if strings.Contains(makefile, `--runner "$(GO_HOOK)"`) { t.Fatal("Makefile routes a parent shim through a worktree-local runner") } + + for _, forbidden := range []string{ + `cp "$(GO_TOOLS_BIN_DIR)"/coding-ethos-*`, + `cp "$(GO_TOOLS_BIN_DIR)/cerun"`, + `cp "$(GO_TOOLS_BIN_DIR)/lint"`, + `cp "$(GO_TOOLS_BIN_DIR)/coding-ethos-git-hook"`, + } { + if strings.Contains(makefile, forbidden) { + t.Fatalf("Makefile bypasses atomic runtime sync with %q", forbidden) + } + } } func fakeCIGit(t *testing.T, diffOutput string) string { @@ -3875,46 +4330,6 @@ func parentGoToolsSourceFixture(t *testing.T, ethosRoot string) string { return sourceRoot } -func parentBuildableGoToolsSourceFixture(t *testing.T, ethosRoot string) string { - t.Helper() - - sourceRoot := filepath.Join(ethosRoot, "go") - - err := os.MkdirAll(sourceRoot, 0o755) - if err != nil { - t.Fatalf("create Go source root: %v", err) - } - - err = os.WriteFile( - filepath.Join(sourceRoot, "go.mod"), - []byte("module example.test/coding-ethos\n\ngo 1.26\n"), - 0o600, - ) - if err != nil { - t.Fatalf("write go.mod: %v", err) - } - - for _, tool := range parentGoToolFixtureCommands() { - mainPath := filepath.Join(sourceRoot, "cmd", tool, "main.go") - - err = os.MkdirAll(filepath.Dir(mainPath), 0o755) - if err != nil { - t.Fatalf("create source dir: %v", err) - } - - err = os.WriteFile( - mainPath, - []byte("package main\n\nfunc main() {}\n"), - 0o600, - ) - if err != nil { - t.Fatalf("write source: %v", err) - } - } - - return sourceRoot -} - func assertProtectedBranchWorkPoliciesPresent(t *testing.T, bundlePath string) { t.Helper() @@ -4039,6 +4454,7 @@ func touchParentGoTools(t *testing.T, paths runtimePaths, modTime time.Time) { func parentGoToolFixtureCommands() []string { return []string{ + "coding-ethos-git-hook", "coding-ethos-lint", "coding-ethos-policy", "coding-ethos-run", diff --git a/go/cmd/coding-ethos-run/parent_workflow.go b/go/cmd/coding-ethos-run/parent_workflow.go index 77619275..f1ea68ed 100644 --- a/go/cmd/coding-ethos-run/parent_workflow.go +++ b/go/cmd/coding-ethos-run/parent_workflow.go @@ -4,7 +4,6 @@ package main import ( - "context" "crypto/sha256" "errors" "flag" @@ -25,7 +24,6 @@ import ( "blackcat.ca/coding-ethos/go/internal/hookoutput" "blackcat.ca/coding-ethos/go/internal/policy" "blackcat.ca/coding-ethos/go/internal/repoignore" - "blackcat.ca/coding-ethos/go/internal/safeexec" "blackcat.ca/coding-ethos/go/internal/shellquote" "blackcat.ca/coding-ethos/go/internal/toolconfigs" ) @@ -34,7 +32,9 @@ const ( parentCheckoutCodingEthos = "coding-ethos" parentDefaultLintScope = "full" parentExecutableDirMode = 0o755 + parentManagedToolPrefix = "coding-ethos-" parentPolicyBundleDirMode = 0o755 + parentCompatibilityHook = "coding-ethos-git-hook" parentStepFail = "fail" parentStepPass = "pass" parentWorkingTreeState = "working_tree" @@ -46,6 +46,7 @@ var ( errParentPathIsDirectory = errors.New("path is a directory, want file") errParentPathIsNotDirectory = errors.New("path is not a directory, want directory") errParentRootNotAbsolute = errors.New("parent workflow root must be absolute") + errParentRuntimeUnsafeShape = errors.New("parent hook runtime has an unsafe shape") errParentSourceNotExecutable = errors.New( "source executable is not a regular executable", ) @@ -65,6 +66,23 @@ type parentWorkflowStep struct { Detail string } +type parentRuntimeExecutable struct { + Destination string + Label string + Source string +} + +type parentRuntimeDirectory struct { + Expected map[string]struct{} + Path string +} + +type parentRuntimeEntry struct { + Name string + Path string + Regular bool +} + func runParentInstall(paths runtimePaths, rest []string) error { options, err := parseParentWorkflowFlags(paths, "parent-install", rest) if err != nil { @@ -83,6 +101,26 @@ func runParentInstall(paths runtimePaths, rest []string) error { return nil } +func runParentRuntimeSync(paths runtimePaths, rest []string) error { + options, err := parseParentWorkflowFlags(paths, "parent-runtime-sync", rest) + if err != nil { + return err + } + + steps := []parentWorkflowStep{runParentStep("hook_runtime", func() error { + return syncParentHookRuntimeExecutables(paths, options) + })} + printParentWorkflowReport( + "parent-runtime-sync", + parentStepStatus(steps), + options.Repo, + steps, + ) + exitForFailedParentSteps(steps) + + return nil +} + func runParentCheck(paths runtimePaths, rest []string) error { options, err := parseParentWorkflowFlags(paths, "parent-check", rest) if err != nil { @@ -240,9 +278,14 @@ func syncParentArtifacts( options parentWorkflowOptions, ) []parentWorkflowStep { steps := []parentWorkflowStep{} + steps = append(steps, runParentStep("go_tools", func() error { - return rebuildParentGoTools(paths) + return checkParentGoTools(paths) })) + if parentStepStatus(steps) != parentStepPass { + return steps + } + steps = append(steps, runParentStep("hook_runtime", func() error { return syncParentHookRuntimeExecutables(paths, options) })) @@ -301,7 +344,7 @@ func checkParentArtifacts( ) []parentWorkflowStep { steps := []parentWorkflowStep{} steps = append(steps, runParentStep("go_tools", func() error { - return checkParentGoTools(paths, options) + return checkParentGoTools(paths) })) steps = append(steps, runParentStep("hook_runtime", func() error { return checkParentHookRuntimeExecutables(paths, options) @@ -428,10 +471,13 @@ func parentGitCommonDir(paths runtimePaths, repo string) string { } func parentHookRuntimeBinDir(paths runtimePaths, options parentWorkflowOptions) string { + return filepath.Join(parentHookRuntimeDir(paths, options), "bin") +} + +func parentHookRuntimeDir(paths runtimePaths, options parentWorkflowOptions) string { return filepath.Join( parentGitCommonDir(paths, options.Repo), "coding-ethos-hooks", - "bin", ) } @@ -439,39 +485,198 @@ func syncParentHookRuntimeExecutables( paths runtimePaths, options parentWorkflowOptions, ) error { - tools, err := parentGoToolCommands(paths) + projections, directories, err := parentHookRuntimeProjection(paths, options) + if err != nil { + return err + } + + err = ensureParentHookRuntimeDirectories(directories) if err != nil { return err } - runtimeBin := parentHookRuntimeBinDir(paths, options) + err = validateParentHookRuntimeProjectionSources(projections) + if err != nil { + return err + } - err = os.MkdirAll(runtimeBin, parentExecutableDirMode) + err = pruneUnexpectedParentRuntimeExecutables(directories) if err != nil { - return fmt.Errorf("create parent hook runtime bin dir: %w", err) + return err + } + + return installParentHookRuntimeProjections(projections) +} + +func ensureParentHookRuntimeDirectories(directories []parentRuntimeDirectory) error { + for _, directory := range directories { + err := ensureParentHookRuntimeDirectory(directory.Path) + if err != nil { + return err + } + } + + return nil +} + +func validateParentHookRuntimeProjectionSources( + projections []parentRuntimeExecutable, +) error { + for _, projection := range projections { + _, err := parentRuntimeSourceInfo(projection.Source) + if err != nil { + return fmt.Errorf("validate parent hook runtime %s: %w", projection.Label, err) + } } + return nil +} + +func pruneUnexpectedParentRuntimeExecutables( + directories []parentRuntimeDirectory, +) error { + for _, directory := range directories { + entries, err := unexpectedParentRuntimeExecutables(directory) + if err != nil { + return err + } + + for _, entry := range entries { + if !entry.Regular { + return fmt.Errorf("%w: %s", errParentRuntimeUnsafeShape, entry.Path) + } + } + + for _, entry := range entries { + err = os.Remove(entry.Path) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("prune stale parent runtime %s: %w", entry.Path, err) + } + } + } + + return nil +} + +func installParentHookRuntimeProjections(projections []parentRuntimeExecutable) error { + for _, projection := range projections { + err := installParentHookRuntimeExecutable( + projection.Source, + projection.Destination, + ) + if err != nil { + return fmt.Errorf("install parent hook runtime %s: %w", projection.Label, err) + } + } + + return nil +} + +func parentHookRuntimeProjection( + paths runtimePaths, + options parentWorkflowOptions, +) ([]parentRuntimeExecutable, []parentRuntimeDirectory, error) { + tools, err := parentGoToolCommands(paths) + if err != nil { + return nil, nil, err + } + + runtimeDir := parentHookRuntimeDir(paths, options) + runtimeBin := filepath.Join(runtimeDir, "bin") + binNames := make(map[string]struct{}, len(tools)) + projections := make([]parentRuntimeExecutable, 0, len(tools)+1) + for _, tool := range tools { - source := filepath.Join(paths.BinDir, tool) - destination := filepath.Join(runtimeBin, tool) + binNames[tool] = struct{}{} + projections = append(projections, parentRuntimeExecutable{ + Destination: filepath.Join(runtimeBin, tool), + Label: tool, + Source: filepath.Join(paths.BinDir, tool), + }) + } + + projections = append(projections, parentRuntimeExecutable{ + Destination: filepath.Join(runtimeDir, parentCompatibilityHook), + Label: parentCompatibilityHook + "@compat", + Source: filepath.Join(paths.BinDir, parentCompatibilityHook), + }) - err = installParentHookRuntimeExecutable(source, destination) + return projections, []parentRuntimeDirectory{ + {Path: runtimeDir, Expected: map[string]struct{}{parentCompatibilityHook: {}}}, + {Path: runtimeBin, Expected: binNames}, + }, nil +} + +func ensureParentHookRuntimeDirectory(path string) error { + info, err := os.Lstat(path) + if errors.Is(err, os.ErrNotExist) { + err = os.MkdirAll(path, parentExecutableDirMode) if err != nil { - return fmt.Errorf("install parent hook runtime %s: %w", tool, err) + return fmt.Errorf("create parent hook runtime directory %s: %w", path, err) } + + return nil + } + + if err != nil { + return fmt.Errorf("stat parent hook runtime directory %s: %w", path, err) + } + + if !info.IsDir() { + return fmt.Errorf("%w: %s", errParentRuntimeUnsafeShape, path) } return nil } -func installParentHookRuntimeExecutable(source, destination string) error { - info, err := os.Lstat(source) +func unexpectedParentRuntimeExecutables( + directory parentRuntimeDirectory, +) ([]parentRuntimeEntry, error) { + entries, err := os.ReadDir(directory.Path) + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + if err != nil { - return fmt.Errorf("stat source executable %s: %w", source, err) + return nil, fmt.Errorf( + "read parent hook runtime directory %s: %w", + directory.Path, + err, + ) } - if !info.Mode().IsRegular() || info.Mode()&0o111 == 0 { - return fmt.Errorf("%w: %s", errParentSourceNotExecutable, source) + unexpected := []parentRuntimeEntry{} + + for _, entry := range entries { + if !strings.HasPrefix(entry.Name(), parentManagedToolPrefix) { + continue + } + + if _, found := directory.Expected[entry.Name()]; found { + continue + } + + path := filepath.Join(directory.Path, entry.Name()) + + info, statErr := os.Lstat(path) + if statErr != nil { + return nil, fmt.Errorf("stat stale parent runtime %s: %w", path, statErr) + } + + unexpected = append(unexpected, parentRuntimeEntry{ + Name: entry.Name(), + Path: path, + Regular: info.Mode().IsRegular(), + }) + } + + return unexpected, nil +} + +func installParentHookRuntimeExecutable(source, destination string) error { + info, err := parentRuntimeSourceInfo(source) + if err != nil { + return err } input, err := os.Open(source) @@ -515,6 +720,11 @@ func installParentHookRuntimeExecutable(source, destination string) error { return fmt.Errorf("close temporary hook runtime executable: %w", err) } + err = prepareParentHookRuntimeDestination(destination) + if err != nil { + return err + } + err = os.Rename(temporaryPath, destination) if err != nil { return fmt.Errorf("activate hook runtime executable %s: %w", destination, err) @@ -523,29 +733,85 @@ func installParentHookRuntimeExecutable(source, destination string) error { return nil } +func parentRuntimeSourceInfo(source string) (fs.FileInfo, error) { + info, err := os.Lstat(source) + if err != nil { + return nil, fmt.Errorf("stat source executable %s: %w", source, err) + } + + if !info.Mode().IsRegular() || info.Mode()&0o111 == 0 { + return nil, fmt.Errorf("%w: %s", errParentSourceNotExecutable, source) + } + + return info, nil +} + +func prepareParentHookRuntimeDestination(destination string) error { + info, err := os.Lstat(destination) + if errors.Is(err, os.ErrNotExist) { + return nil + } + + if err != nil { + return fmt.Errorf("stat installed executable %s: %w", destination, err) + } + + if !info.IsDir() { + return nil + } + + err = os.Remove(destination) + if err != nil { + return fmt.Errorf( + "remove expected parent hook runtime directory %s: %w", + destination, + err, + ) + } + + return nil +} + func checkParentHookRuntimeExecutables( paths runtimePaths, options parentWorkflowOptions, ) error { - tools, err := parentGoToolCommands(paths) + projections, directories, err := parentHookRuntimeProjection(paths, options) if err != nil { return err } - runtimeBin := parentHookRuntimeBinDir(paths, options) mismatched := []string{} - for _, tool := range tools { - source := filepath.Join(paths.BinDir, tool) - destination := filepath.Join(runtimeBin, tool) + for _, projection := range projections { + status, compareErr := compareParentHookRuntimeExecutable( + projection.Source, + projection.Destination, + ) - status, err := compareParentHookRuntimeExecutable(source, destination) + err = compareErr if err != nil { - return fmt.Errorf("check parent hook runtime %s: %w", tool, err) + return fmt.Errorf("check parent hook runtime %s: %w", projection.Label, err) } if status != "" { - mismatched = append(mismatched, tool+"("+status+")") + mismatched = append(mismatched, projection.Label+"("+status+")") + } + } + + for _, directory := range directories { + entries, listErr := unexpectedParentRuntimeExecutables(directory) + if listErr != nil { + return listErr + } + + for _, entry := range entries { + status := "stale" + if !entry.Regular { + status = "unsafe_shape" + } + + mismatched = append(mismatched, entry.Name+"("+status+")") } } @@ -563,30 +829,22 @@ func checkParentHookRuntimeExecutables( } func compareParentHookRuntimeExecutable(source, destination string) (string, error) { - sourceInfo, err := os.Lstat(source) + sourceInfo, err := parentRuntimeSourceInfo(source) if err != nil { - return "", fmt.Errorf("stat source executable %s: %w", source, err) - } - - if !sourceInfo.Mode().IsRegular() || sourceInfo.Mode()&0o111 == 0 { - return "", fmt.Errorf("%w: %s", errParentSourceNotExecutable, source) - } - - destinationInfo, err := os.Lstat(destination) - if errors.Is(err, os.ErrNotExist) { - return "missing", nil + return "", err } + destinationInfo, status, err := parentRuntimeDestinationInfo(destination) if err != nil { - return "", fmt.Errorf("stat installed executable %s: %w", destination, err) + return "", err } - if !destinationInfo.Mode().IsRegular() { - return "not_regular", nil + if status != "" { + return status, nil } - if destinationInfo.Mode()&0o111 == 0 { - return "not_executable", nil + if sourceInfo.Mode().Perm() != destinationInfo.Mode().Perm() { + return "mode", nil } if sourceInfo.Size() != destinationInfo.Size() { @@ -610,6 +868,27 @@ func compareParentHookRuntimeExecutable(source, destination string) (string, err return "", nil } +func parentRuntimeDestinationInfo(destination string) (fs.FileInfo, string, error) { + info, err := os.Lstat(destination) + if errors.Is(err, os.ErrNotExist) { + return nil, "missing", nil + } + + if err != nil { + return nil, "", fmt.Errorf("stat installed executable %s: %w", destination, err) + } + + if !info.Mode().IsRegular() { + return nil, "not_regular", nil + } + + if info.Mode()&0o111 == 0 { + return nil, "not_executable", nil + } + + return info, "", nil +} + func parentFileSHA256(path string) ([sha256.Size]byte, error) { file, err := os.Open(path) if err != nil { @@ -799,89 +1078,7 @@ func firstNonEmptyPath(values ...string) string { return "" } -func rebuildParentGoTools(paths runtimePaths) error { - err := requireParentGoToolsSource(paths) - if err != nil { - return err - } - - err = os.MkdirAll(paths.BinDir, parentExecutableDirMode) - if err != nil { - return fmt.Errorf("create Go tool bin dir: %w", err) - } - - tools, err := parentGoToolCommands(paths) - if err != nil { - return err - } - - for _, tool := range tools { - err = buildParentGoTool(paths, tool) - if err != nil { - return err - } - } - - return nil -} - -func buildParentGoTool(paths runtimePaths, tool string) error { - outputPath := filepath.Join(paths.BinDir, tool) - - tempFile, err := os.CreateTemp(paths.BinDir, "."+tool+"-*.tmp") - if err != nil { - return fmt.Errorf("create temporary binary for %s: %w", tool, err) - } - - tempPath := tempFile.Name() - - err = tempFile.Close() - if err != nil { - return fmt.Errorf("close temporary binary for %s: %w", tool, err) - } - - err = os.Remove(tempPath) - if err != nil { - return fmt.Errorf("prepare temporary binary for %s: %w", tool, err) - } - - defer func() { - _ = os.Remove(tempPath) - }() - - command := safeexec.CommandContext( - context.Background(), - "go", - "build", - "-trimpath", - "-buildvcs=false", - "-o", - tempPath, - "./cmd/"+tool, - ) - command.Dir = paths.ToolsSource - - command.Env = append(os.Environ(), "GOWORK=off") - - output, err := command.CombinedOutput() - if err != nil { - detail := strings.TrimSpace(string(output)) - if detail != "" { - return fmt.Errorf("build %s: %w: %s", tool, err, detail) - } - - return fmt.Errorf("build %s: %w", tool, err) - } - - err = os.Rename(tempPath, outputPath) - if err != nil { - return fmt.Errorf("install %s: %w", tool, err) - } - - return nil -} - -func checkParentGoTools(paths runtimePaths, options parentWorkflowOptions) error { +func checkParentGoTools(paths runtimePaths) error { err := requireParentGoToolsSource(paths) if err != nil { return err @@ -925,9 +1122,9 @@ func checkParentGoTools(paths runtimePaths, options parentWorkflowOptions) error } return fmt.Errorf( - "%w: run %s; tools: %s", + "%w: run `make build` in %s before parent workflows; tools: %s", errParentGoToolsStale, - parentInstallCommand(options), + paths.EthosRoot, strings.Join(stale, " "), ) } diff --git a/go/cmd/coding-ethos-run/policy_tool_parent_linux.go b/go/cmd/coding-ethos-run/policy_tool_parent_linux.go new file mode 100644 index 00000000..5e852a77 --- /dev/null +++ b/go/cmd/coding-ethos-run/policy_tool_parent_linux.go @@ -0,0 +1,24 @@ +// SPDX-FileCopyrightText: 2026 Blackcat Informatics Inc. +// SPDX-License-Identifier: AGPL-3.0-only + +//go:build linux + +package main + +import ( + "fmt" + "os" + "path/filepath" + "strconv" +) + +func policyToolParentExecutable() (string, error) { + parentExecutable, err := os.Readlink( + filepath.Join("/proc", strconv.Itoa(os.Getppid()), "exe"), + ) + if err != nil { + return "", fmt.Errorf("resolve parent executable identity: %w", err) + } + + return parentExecutable, nil +} diff --git a/go/cmd/coding-ethos-run/policy_tool_parent_linux_test.go b/go/cmd/coding-ethos-run/policy_tool_parent_linux_test.go new file mode 100644 index 00000000..5d26afdd --- /dev/null +++ b/go/cmd/coding-ethos-run/policy_tool_parent_linux_test.go @@ -0,0 +1,39 @@ +// SPDX-FileCopyrightText: 2026 Blackcat Informatics Inc. +// SPDX-License-Identifier: AGPL-3.0-only + +//go:build linux + +package main + +import ( + "os" + "path/filepath" + "strconv" + "testing" +) + +// The actionlint ShellCheck protocol only trusts a parent it can name, so +// resolution must return the real executable behind the parent process. +func TestPolicyToolParentExecutableResolvesRunningParent(t *testing.T) { + t.Parallel() + + parentExecutable, err := policyToolParentExecutable() + if err != nil { + t.Fatalf("resolve parent executable: %v", err) + } + + if !filepath.IsAbs(parentExecutable) { + t.Fatalf("parent executable %q is not an absolute path", parentExecutable) + } + + expected, err := os.Readlink( + filepath.Join("/proc", strconv.Itoa(os.Getppid()), "exe"), + ) + if err != nil { + t.Fatalf("read parent executable link: %v", err) + } + + if parentExecutable != expected { + t.Fatalf("parent executable = %q, want %q", parentExecutable, expected) + } +} diff --git a/go/cmd/coding-ethos-run/policy_tool_parent_unsupported.go b/go/cmd/coding-ethos-run/policy_tool_parent_unsupported.go new file mode 100644 index 00000000..9fc14ab6 --- /dev/null +++ b/go/cmd/coding-ethos-run/policy_tool_parent_unsupported.go @@ -0,0 +1,23 @@ +// SPDX-FileCopyrightText: 2026 Blackcat Informatics Inc. +// SPDX-License-Identifier: AGPL-3.0-only + +//go:build !linux + +package main + +import ( + "fmt" + "runtime" + + "blackcat.ca/coding-ethos/go/internal/apperror" +) + +func policyToolParentExecutable() (string, error) { + return "", fmt.Errorf( + "%w on %s", + apperror.StaticError( + "actionlint ShellCheck protocol is unsupported because parent executable identity cannot be authenticated", + ), + runtime.GOOS, + ) +} diff --git a/go/cmd/coding-ethos-run/runtime_exec.go b/go/cmd/coding-ethos-run/runtime_exec.go index 10bafdd6..f796ee06 100644 --- a/go/cmd/coding-ethos-run/runtime_exec.go +++ b/go/cmd/coding-ethos-run/runtime_exec.go @@ -24,6 +24,7 @@ import ( "blackcat.ca/coding-ethos/go/internal/codeintelcli" "blackcat.ca/coding-ethos/go/internal/debuglog" "blackcat.ca/coding-ethos/go/internal/feedback" + "blackcat.ca/coding-ethos/go/internal/geminiprompts" "blackcat.ca/coding-ethos/go/internal/githookcli" "blackcat.ca/coding-ethos/go/internal/gitwrap" "blackcat.ca/coding-ethos/go/internal/hookcli" @@ -74,6 +75,15 @@ var ( errSharedLockDirectoryShape = apperror.StaticError( "shared lock directory must be a non-symlink mode-1777 direct child of /var/tmp", ) + errAgentShellExternalStateRootPath = apperror.StaticError( + "agent-shell external state root must be a specific absolute path", + ) + errAgentShellExternalStateRootShape = apperror.StaticError( + "agent-shell external state root must be a real directory", + ) + errAgentShellExternalStateArtifactShape = apperror.StaticError( + "agent-shell external state directory must be a real directory", + ) ) type runtimeExecutor interface { @@ -208,7 +218,10 @@ func agentShellSandboxPlan( return sandbox.Plan{}, nil, cleanup, err } - agentWritePaths, err := agentShellWritePaths(paths.Root) + agentWritePaths, err := agentShellWritePaths( + paths.Root, + paths.effectiveStateRoot(), + ) if err != nil { cleanup() @@ -267,11 +280,11 @@ func agentShellSandboxPlan( } // agentShellWritePaths assembles the full agent-shell writable set: the -// worktree paths, the managed write directories, and the tool-config drift -// manifest. The manifest lives directly under the read-only .coding-ethos pin -// and is rewritten by config generation; without its override, -// sync-tool-configs fails with EROFS inside the agent shell. -func agentShellWritePaths(root string) ([]string, error) { +// worktree paths, the managed write directories, an optional supervisor-owned +// private state tree, and the exact generated files rewritten under the +// read-only .coding-ethos pin. Without these file overrides, canonical build +// generation fails with EROFS inside the agent shell. +func agentShellWritePaths(root, stateRoot string) ([]string, error) { agentWritePaths, err := agentShellWorktreeWritePaths(root) if err != nil { return nil, err @@ -291,10 +304,101 @@ func agentShellWritePaths(root string) ([]string, error) { agentWritePaths = append(agentWritePaths, sharedLockDirectories...) + externalStatePath, err := agentShellExternalStateWritePath(root, stateRoot) + if err != nil { + return nil, err + } + + if externalStatePath != "" { + agentWritePaths = append(agentWritePaths, externalStatePath) + } + // Repo-relative on purpose: the sandbox helper only creates missing // write-path files for relative declarations, and repos that have never - // generated configs do not have the manifest yet. - return append(agentWritePaths, toolconfigs.HashManifestPath), nil + // generated configs do not have these artifacts yet. + return append( + agentWritePaths, + toolconfigs.HashManifestPath, + geminiprompts.PromptPackPath, + ), nil +} + +// agentShellExternalStateWritePath admits only the generated .coding-ethos +// subtree beneath a distinct supervisor-owned state root. The consumer +// worktree's tracked .coding-ethos tree remains pinned read-only, while hook +// receipts, event logs, and code-intel state can be written outside it. +func agentShellExternalStateWritePath(root, stateRoot string) (string, error) { + cleanStateRoot := filepath.Clean(strings.TrimSpace(stateRoot)) + if cleanStateRoot == "." || sameCleanPath(root, cleanStateRoot) { + return "", nil + } + + err := validateAgentShellExternalStateRoot(cleanStateRoot) + if err != nil { + return "", err + } + + return ensureAgentShellExternalStateArtifacts(cleanStateRoot) +} + +func validateAgentShellExternalStateRoot(stateRoot string) error { + if !filepath.IsAbs(stateRoot) || stateRoot == string(filepath.Separator) { + return fmt.Errorf("%w: %s", errAgentShellExternalStateRootPath, stateRoot) + } + + stateRootInfo, err := os.Lstat(stateRoot) + if err != nil { + return fmt.Errorf( + "inspect agent-shell external state root %s: %w", + stateRoot, + err, + ) + } + + if stateRootInfo.Mode()&os.ModeSymlink != 0 || !stateRootInfo.IsDir() { + return fmt.Errorf("%w: %s", errAgentShellExternalStateRootShape, stateRoot) + } + + return nil +} + +func ensureAgentShellExternalStateArtifacts(stateRoot string) (string, error) { + artifactRoot := filepath.Join(stateRoot, ".coding-ethos") + + err := os.Mkdir(artifactRoot, agentShellCacheDirMode) + if err != nil && !os.IsExist(err) { + return "", fmt.Errorf( + "create agent-shell external state directory %s: %w", + artifactRoot, + err, + ) + } + + artifactInfo, err := os.Lstat(artifactRoot) + if err != nil { + return "", fmt.Errorf( + "inspect agent-shell external state directory %s: %w", + artifactRoot, + err, + ) + } + + if artifactInfo.Mode()&os.ModeSymlink != 0 || !artifactInfo.IsDir() { + return "", fmt.Errorf("%w: %s", errAgentShellExternalStateArtifactShape, artifactRoot) + } + + if artifactInfo.Mode().Perm()&^agentShellCacheDirMode != 0 { + err = os.Chmod(artifactRoot, agentShellCacheDirMode) + if err != nil { + return "", fmt.Errorf( + "make agent-shell external state directory private %s: %w", + artifactRoot, + err, + ) + } + } + + return artifactRoot, nil } // agentShellSharedLockDirectories carries only outer-supervisor-admitted host diff --git a/go/go.mod b/go/go.mod index dfd27d8d..15a310d2 100644 --- a/go/go.mod +++ b/go/go.mod @@ -2,6 +2,8 @@ module blackcat.ca/coding-ethos/go go 1.26 +toolchain go1.26.8 + require go.yaml.in/yaml/v3 v3.0.4 require github.com/pelletier/go-toml/v2 v2.3.1 diff --git a/go/internal/codeintel/deleted_files.go b/go/internal/codeintel/deleted_files.go index f56fda54..03d849cd 100644 --- a/go/internal/codeintel/deleted_files.go +++ b/go/internal/codeintel/deleted_files.go @@ -20,6 +20,7 @@ import ( const ( codeFileStaleDeleted = "deleted" codeFileStaleDeletedByIntent = "deleted_by_intent" + gitStagedDeleteArgCount = 8 ) func (store *Store) MarkMissingCodeFilesDeleted( @@ -275,9 +276,16 @@ func gitDeletedPaths(ctx context.Context, root string) map[string]bool { } func gitStagedDeletedPaths(ctx context.Context, root string) map[string]bool { - command := realgit.Command( - ctx, - false, + return gitStagedDeletedPathsFor(ctx, root, nil) +} + +func gitStagedDeletedPathsFor( + ctx context.Context, + root string, + paths []string, +) map[string]bool { + args := make([]string, 0, gitStagedDeleteArgCount+len(paths)) + args = append(args, "-C", root, "diff", @@ -285,7 +293,11 @@ func gitStagedDeletedPaths(ctx context.Context, root string) map[string]bool { "--cached", "--diff-filter=D", "-z", + "--", ) + args = append(args, paths...) + + command := realgit.Command(ctx, false, args...) command.Env = realgit.CleanGitLocalEnv(os.Environ()) output, err := command.Output() diff --git a/go/internal/codeintel/maintenance.go b/go/internal/codeintel/maintenance.go index a9b3de8c..985d4d13 100644 --- a/go/internal/codeintel/maintenance.go +++ b/go/internal/codeintel/maintenance.go @@ -8,7 +8,9 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" + "time" ) // MaintenanceSummary reports repo-local code-intel refresh work. @@ -18,8 +20,10 @@ type MaintenanceSummary struct { DiffPatterns DiffEditPatternSummary `json:"diff_patterns"` } -// RefreshRepository opens the repo-local code-intel store, ingests retained -// traces, and refreshes AST facts for the requested paths. +// RefreshRepository explicitly opens the repo-local code-intel store, ingests +// retained traces, and refreshes AST facts for the requested paths. Normal +// hook execution must use the event and changed-file ingestion paths instead; +// a whole-repository refresh is an operator-requested maintenance action. func RefreshRepository( ctx context.Context, root string, @@ -58,41 +62,228 @@ func RefreshRepository( }, nil } -// RefreshLintFiles refreshes code-intel AST facts for lint-scoped paths. +// RefreshLintFiles incrementally refreshes code-intel AST facts for explicit +// lint-scoped files and tombstones explicit files that have been deleted. It +// rejects directories and out-of-repository paths so ordinary hook execution +// cannot turn this API into a whole-repository walk. func RefreshLintFiles( ctx context.Context, root string, paths []string, ) (CodeIndexSummary, error) { - paths = existingLintIndexPaths(root, paths) - if len(paths) == 0 { + selected := selectLintIndexPaths(root, paths) + if len(selected.requested) == 0 { return CodeIndexSummary{}, nil } + root, err := filepath.Abs(root) + if err != nil { + return CodeIndexSummary{}, fmt.Errorf("resolve lint index root: %w", err) + } + store, err := Open(ctx, DefaultDBPath(ResolveStateRoot(root))) if err != nil { return CodeIndexSummary{}, err } defer store.Close() - summary, err := NewASTIndexer(store).IndexPaths(ctx, root, paths) + existingFiles, err := explicitCodeFilesByPath(ctx, store, selected.requested) if err != nil { - return CodeIndexSummary{}, fmt.Errorf("refresh code-intel lint files: %w", err) + return CodeIndexSummary{}, err } - _, err = store.RefreshDiffEditPatterns(ctx, root) + summary, ignored, err := indexExplicitLintFiles( + ctx, + store, + root, + selected.existing, + existingFiles, + ) if err != nil { return CodeIndexSummary{}, fmt.Errorf( - "refresh code-intel diff edit patterns: %w", + "refresh code-intel lint files: %w", + err, + ) + } + + deleted := activeExplicitCodeFiles(existingFiles, selected.deleted) + if len(deleted) > 0 { + err = store.markDeletedCodeFiles( + ctx, + root, + deleted, + gitStagedDeletedPathsFor(ctx, root, deleted), + ) + if err != nil { + return CodeIndexSummary{}, fmt.Errorf( + "tombstone deleted code-intel lint files: %w", + err, + ) + } + } + + err = markExplicitCodeFilesInactive(ctx, store, ignored, "ignored") + if err != nil { + return CodeIndexSummary{}, fmt.Errorf( + "tombstone ignored code-intel lint files: %w", err, ) } + summary.Deleted = append(summary.Deleted, deleted...) + summary.Deleted = append(summary.Deleted, ignored...) + slices.Sort(summary.Deleted) + summary.Deleted = slices.Compact(summary.Deleted) + return summary, nil } -func existingLintIndexPaths(root string, paths []string) []string { - selected := []string{} +func explicitCodeFilesByPath( + ctx context.Context, + store *Store, + paths []string, +) (map[string]CodeFile, error) { + files := make(map[string]CodeFile, len(paths)) + + for _, path := range paths { + file, found, err := store.GetCodeFile(ctx, path) + if err != nil { + return nil, err + } + + if found { + files[path] = file + } + } + + return files, nil +} + +func indexExplicitLintFiles( + ctx context.Context, + store *Store, + root string, + paths []string, + existingFiles map[string]CodeFile, +) (CodeIndexSummary, []string, error) { + options, err := LoadIndexOptions(root) + if err != nil { + return CodeIndexSummary{}, nil, err + } + + ignoreMatcher := gitIgnoreMatcher{ + root: root, + active: gitWorkTreeAvailable(ctx, root), + } + indexer := NewASTIndexer(store) + summary := CodeIndexSummary{} + ignored := []string{} + + for _, path := range paths { + absolutePath := filepath.Join(root, filepath.FromSlash(path)) + + info, statErr := os.Lstat(absolutePath) + if statErr != nil { + if os.IsNotExist(statErr) { + continue + } + + return CodeIndexSummary{}, nil, fmt.Errorf( + "stat lint index file %q: %w", + path, + statErr, + ) + } + + if !info.Mode().IsRegular() { + continue + } + + if ignoreMatcher.ignoredFile(ctx, absolutePath) || + pathHasSkippedDir(path) || + excludedByConfig(path, options.ExcludePatterns) { + if existing, found := existingFiles[path]; found && + existing.DeletedAtUTC == "" { + ignored = append(ignored, path) + } + + continue + } + + err = indexer.indexFile( + ctx, + root, + absolutePath, + info, + gitIgnoreMatcher{}, + existingFiles, + options, + &summary, + ) + if err != nil { + return CodeIndexSummary{}, nil, err + } + } + + return summary, ignored, nil +} + +func activeExplicitCodeFiles( + existingFiles map[string]CodeFile, + paths []string, +) []string { + active := make([]string, 0, len(paths)) + + for _, path := range paths { + if existing, found := existingFiles[path]; found && existing.DeletedAtUTC == "" { + active = append(active, path) + } + } + + return active +} + +func markExplicitCodeFilesInactive( + ctx context.Context, + store *Store, + paths []string, + reason string, +) error { + if len(paths) == 0 { + return nil + } + + inactiveAt := time.Now().UTC().Format(time.RFC3339) + + transaction, err := store.database.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("begin explicit code file refresh: %w", err) + } + defer rollbackUnlessCommitted(transaction) + + for _, path := range paths { + err = markCodeFileInactive(ctx, transaction, path, inactiveAt, reason) + if err != nil { + return err + } + } + + err = transaction.Commit() + if err != nil { + return fmt.Errorf("commit explicit code file refresh: %w", err) + } + + return nil +} + +type lintIndexPathSelection struct { + requested []string + existing []string + deleted []string +} + +func selectLintIndexPaths(root string, paths []string) lintIndexPathSelection { + selected := lintIndexPathSelection{} seen := map[string]struct{}{} rootAbs, err := filepath.Abs(root) @@ -120,20 +311,33 @@ func existingLintIndexPaths(root string, paths []string) []string { continue } - info, err := os.Stat(statPath) - if err != nil || !info.Mode().IsRegular() { + if _, ok := seen[relative]; ok { continue } - if _, ok := seen[statPath]; ok { + info, statErr := os.Lstat(statPath) + if statErr != nil && !os.IsNotExist(statErr) { continue } - seen[statPath] = struct{}{} + if statErr == nil && !info.Mode().IsRegular() { + continue + } + + seen[relative] = struct{}{} + selected.requested = append(selected.requested, relative) - selected = append(selected, relative) + if statErr == nil { + selected.existing = append(selected.existing, relative) + } else { + selected.deleted = append(selected.deleted, relative) + } } + slices.Sort(selected.requested) + slices.Sort(selected.existing) + slices.Sort(selected.deleted) + return selected } diff --git a/go/internal/codeintel/maintenance_test.go b/go/internal/codeintel/maintenance_test.go new file mode 100644 index 00000000..4df6e999 --- /dev/null +++ b/go/internal/codeintel/maintenance_test.go @@ -0,0 +1,198 @@ +// SPDX-FileCopyrightText: 2026 Blackcat Informatics Inc. +// SPDX-License-Identifier: AGPL-3.0-only + +package codeintel + +import ( + "context" + "os" + "path/filepath" + "slices" + "testing" + + "blackcat.ca/coding-ethos/go/internal/realgit" +) + +func TestRefreshLintFilesIndexesOnlyExplicitFilesAndTombstonesDeletion(t *testing.T) { + t.Parallel() + + ctx := context.Background() + root := t.TempDir() + appPath := filepath.Join(root, "pkg", "app.py") + otherPath := filepath.Join(root, "pkg", "other.py") + + writeMaintenanceTestFile(t, appPath, []byte("VALUE = 1\n")) + writeMaintenanceTestFile(t, otherPath, []byte("OTHER = 2\n")) + runMaintenanceTestGit(t, root, "init", "--initial-branch", "main") + runMaintenanceTestGit(t, root, "config", "user.email", "test@example.com") + runMaintenanceTestGit(t, root, "config", "user.name", "Test User") + runMaintenanceTestGit(t, root, "add", "pkg/app.py", "pkg/other.py") + runMaintenanceTestGit(t, root, "commit", "-m", "initial") + + summary, err := RefreshLintFiles(ctx, root, []string{"pkg/app.py"}) + if err != nil { + t.Fatalf("initial incremental refresh: %v", err) + } + if summary.FilesIndexed != 1 { + t.Fatalf("files indexed = %d, want 1", summary.FilesIndexed) + } + + store, err := Open(ctx, DefaultDBPath(root)) + if err != nil { + t.Fatalf("open code-intel store: %v", err) + } + + if _, found, getErr := store.GetCodeFile(ctx, "pkg/app.py"); getErr != nil || !found { + t.Fatalf("requested file found=%v err=%v, want true nil", found, getErr) + } + if file, found, getErr := store.GetCodeFile( + ctx, + "pkg/other.py", + ); getErr != nil || + found { + t.Fatalf("unrequested file=%#v found=%v err=%v, want absent", file, found, getErr) + } + if err = store.Close(); err != nil { + t.Fatalf("close code-intel store: %v", err) + } + + if err = os.Remove(appPath); err != nil { + t.Fatalf("delete indexed file: %v", err) + } + runMaintenanceTestGit(t, root, "add", "--update", "pkg/app.py") + + summary, err = RefreshLintFiles(ctx, root, []string{"pkg/app.py"}) + if err != nil { + t.Fatalf("deleted-file incremental refresh: %v", err) + } + if !slices.Contains(summary.Deleted, "pkg/app.py") { + t.Fatalf("deleted summary = %#v, want pkg/app.py", summary.Deleted) + } + + store, err = Open(ctx, DefaultDBPath(root)) + if err != nil { + t.Fatalf("reopen code-intel store: %v", err) + } + defer store.Close() + + file, found, err := store.GetCodeFile(ctx, "pkg/app.py") + if err != nil { + t.Fatalf("get deleted code file: %v", err) + } + if !found || file.DeletedAtUTC == "" || file.StaleReason != "deleted_by_intent" { + t.Fatalf("deleted file = %#v, found=%v", file, found) + } +} + +func TestRefreshLintFilesRejectsDirectoryAndOutsidePaths(t *testing.T) { + t.Parallel() + + root := t.TempDir() + outside := filepath.Join(t.TempDir(), "outside.py") + writeMaintenanceTestFile(t, filepath.Join(root, "app.py"), []byte("VALUE = 1\n")) + writeMaintenanceTestFile(t, outside, []byte("OUTSIDE = 1\n")) + symlink := filepath.Join(root, "outside-link.py") + if err := os.Symlink(outside, symlink); err != nil { + t.Fatalf("create outside symlink: %v", err) + } + + summary, err := RefreshLintFiles( + context.Background(), + root, + []string{".", outside, "outside-link.py"}, + ) + if err != nil { + t.Fatalf("reject broad lint paths: %v", err) + } + if summary.FilesIndexed != 0 || summary.ChunksIndexed != 0 || + len(summary.Skipped) != 0 || len(summary.Deleted) != 0 { + t.Fatalf("summary = %#v, want empty", summary) + } + + if _, err = os.Stat(DefaultDBPath(root)); !os.IsNotExist(err) { + t.Fatalf("ordinary broad paths created code-intel DB: %v", err) + } +} + +func TestRefreshLintFilesDoesNotReconcileUnrelatedIndexedFiles(t *testing.T) { + t.Parallel() + + ctx := context.Background() + root := t.TempDir() + appPath := filepath.Join(root, "app.py") + generatedPath := filepath.Join(root, "generated.py") + + writeMaintenanceTestFile(t, appPath, []byte("VALUE = 1\n")) + writeMaintenanceTestFile(t, generatedPath, []byte("GENERATED = 1\n")) + runMaintenanceTestGit(t, root, "init", "--initial-branch", "main") + runMaintenanceTestGit(t, root, "config", "user.email", "test@example.com") + runMaintenanceTestGit(t, root, "config", "user.name", "Test User") + runMaintenanceTestGit(t, root, "add", "app.py") + runMaintenanceTestGit(t, root, "commit", "-m", "initial") + + store, err := Open(ctx, DefaultDBPath(root)) + if err != nil { + t.Fatalf("open code-intel store: %v", err) + } + + _, err = NewASTIndexer(store).IndexPaths( + ctx, + root, + []string{"app.py", "generated.py"}, + ) + if err != nil { + t.Fatalf("seed code-intel index: %v", err) + } + if err = store.Close(); err != nil { + t.Fatalf("close seeded code-intel store: %v", err) + } + + writeMaintenanceTestFile( + t, + filepath.Join(root, ".gitignore"), + []byte("generated.py\n"), + ) + writeMaintenanceTestFile(t, appPath, []byte("VALUE = 2\n")) + + if _, err = RefreshLintFiles(ctx, root, []string{"app.py"}); err != nil { + t.Fatalf("incremental refresh: %v", err) + } + + store, err = Open(ctx, DefaultDBPath(root)) + if err != nil { + t.Fatalf("reopen code-intel store: %v", err) + } + defer store.Close() + + generated, found, err := store.GetCodeFile(ctx, "generated.py") + if err != nil { + t.Fatalf("get unrelated indexed file: %v", err) + } + if !found || generated.DeletedAtUTC != "" || generated.StaleReason != "" { + t.Fatalf("unrelated indexed file = %#v, found=%v, want active", generated, found) + } +} + +func writeMaintenanceTestFile(t *testing.T, path string, content []byte) { + t.Helper() + + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("create test file parent: %v", err) + } + if err := os.WriteFile(path, content, 0o600); err != nil { + t.Fatalf("write test file: %v", err) + } +} + +func runMaintenanceTestGit(t *testing.T, root string, args ...string) { + t.Helper() + + command := realgit.Command(context.Background(), false, args...) + command.Dir = root + command.Env = realgit.CleanGitLocalEnv(os.Environ()) + + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, output) + } +} diff --git a/go/internal/codeintel/search_identity_migration_test.go b/go/internal/codeintel/search_identity_migration_test.go index 0fd52d1a..9ef863a5 100644 --- a/go/internal/codeintel/search_identity_migration_test.go +++ b/go/internal/codeintel/search_identity_migration_test.go @@ -48,6 +48,22 @@ func TestOpenMigratesExactDuplicateSearchIdentities(t *testing.T) { t.Fatalf("unexpected upgraded identity stats: %#v", stats) } + var message, searchText string + if err = store.Database().QueryRowContext( + ctx, + `SELECT message, search_text FROM code_intel_fts + WHERE fts_id = 'legacy:one'`, + ).Scan(&message, &searchText); err != nil { + t.Fatalf("read preserved v1 search identity: %v", err) + } + if message != "same" || searchText != "same text" { + t.Fatalf( + "preserved v1 search identity = (%q, %q)", + message, + searchText, + ) + } + _, err = store.Database().ExecContext( ctx, `INSERT INTO code_intel_fts(fts_id, kind, record_id, search_text) @@ -111,6 +127,82 @@ func TestOpenRejectsConflictingDuplicateSearchIdentity(t *testing.T) { } } +func TestRoutineV2SearchIdentityMigrationSkipsRecoveryScan(t *testing.T) { + ctx := context.Background() + store, err := Open(ctx, DefaultDBPath(t.TempDir())) + if err != nil { + t.Fatalf("open v2 search identity store: %v", err) + } + defer store.Close() + + dropFTSIdentityIndex(t, ctx, store.Database()) + + _, err = store.Database().ExecContext( + ctx, + `INSERT INTO code_intel_fts( + fts_id, kind, record_id, message, search_text + ) VALUES + ('v2:replay', 'finding', 'one', 'same', 'same text'), + ('v2:replay', 'finding', 'one', 'same', 'same text')`, + ) + if err != nil { + t.Fatalf("insert v2 recovery sentinel: %v", err) + } + + if err = migrateSearchIdentity(ctx, store.Database()); err != nil { + t.Fatalf("routine v2 migration entered recovery scan: %v", err) + } + + var rows int + if err = store.Database().QueryRowContext( + ctx, + "SELECT COUNT(*) FROM code_intel_fts WHERE fts_id = 'v2:replay'", + ).Scan(&rows); err != nil { + t.Fatalf("count retained v2 recovery sentinel: %v", err) + } + if rows != 2 { + t.Fatalf("routine v2 migration rewrote %d sentinel rows, want 2", rows) + } +} + +func TestExplicitSearchIdentityRecoveryRepairsV2Replay(t *testing.T) { + ctx := context.Background() + store, err := Open(ctx, DefaultDBPath(t.TempDir())) + if err != nil { + t.Fatalf("open v2 recovery store: %v", err) + } + defer store.Close() + + dropFTSIdentityIndex(t, ctx, store.Database()) + + _, err = store.Database().ExecContext( + ctx, + `INSERT INTO code_intel_fts( + fts_id, kind, record_id, message, search_text + ) VALUES + ('v2:replay', 'finding', 'one', 'same', 'same text'), + ('v2:replay', 'finding', 'one', 'same', 'same text')`, + ) + if err != nil { + t.Fatalf("insert v2 replay: %v", err) + } + + if err = deduplicateSearchIdentity(ctx, store.Database()); err != nil { + t.Fatalf("run explicit v2 search identity recovery: %v", err) + } + + var rows int + if err = store.Database().QueryRowContext( + ctx, + "SELECT COUNT(*) FROM code_intel_fts WHERE fts_id = 'v2:replay'", + ).Scan(&rows); err != nil { + t.Fatalf("count explicitly recovered v2 replay: %v", err) + } + if rows != 1 { + t.Fatalf("explicit v2 recovery retained %d replay rows, want 1", rows) + } +} + func TestRepeatedCodeIndexWriteKeepsSearchAndRelationshipsStable(t *testing.T) { ctx := context.Background() store, err := Open(ctx, DefaultDBPath(t.TempDir())) @@ -246,3 +338,12 @@ func openLegacySearchIdentityFixture( return path, database } + +func dropFTSIdentityIndex(t *testing.T, ctx context.Context, database *sql.DB) { + t.Helper() + + _, err := database.ExecContext(ctx, "DROP INDEX idx_code_intel_fts_id_unique") + if err != nil { + t.Fatalf("drop FTS identity index: %v", err) + } +} diff --git a/go/internal/codeintel/store.go b/go/internal/codeintel/store.go index e4b5df9b..995022b4 100644 --- a/go/internal/codeintel/store.go +++ b/go/internal/codeintel/store.go @@ -6,9 +6,11 @@ package codeintel import ( "context" "database/sql" + "errors" "fmt" "os" "path/filepath" + "strconv" "strings" "sync" "time" @@ -529,6 +531,11 @@ func configureReadOnlyStore(ctx context.Context, database *sql.DB) error { } func migrateStore(ctx context.Context, database *sql.DB) error { + err := rejectFutureCodeIntelSchemaVersion(ctx, database) + if err != nil { + return err + } + for _, statement := range duckDBSchemaStatements() { _, inlineErrB := database.ExecContext(ctx, statement) if inlineErrB != nil { @@ -536,7 +543,7 @@ func migrateStore(ctx context.Context, database *sql.DB) error { } } - err := deduplicateSearchIdentity(ctx, database) + err = migrateSearchIdentity(ctx, database) if err != nil { return err } @@ -566,6 +573,104 @@ func migrateStore(ctx context.Context, database *sql.DB) error { return nil } +// migrateSearchIdentity limits the whole-table v1 identity repair to actual +// pre-v2 upgrades. Routine v2 opens rely on the unique indexes; an operator +// performing explicit recovery calls deduplicateSearchIdentity directly. +func migrateSearchIdentity(ctx context.Context, database *sql.DB) error { + version, found, err := storedCodeIntelSchemaVersion(ctx, database) + if err != nil { + return err + } + + if !found || version == schemaVersion { + return nil + } + + if version > schemaVersion { + return unsupportedCodeIntelSchemaVersionError(version) + } + + return deduplicateSearchIdentity(ctx, database) +} + +func rejectFutureCodeIntelSchemaVersion( + ctx context.Context, + database *sql.DB, +) error { + var metadataTableExists bool + + err := database.QueryRowContext( + ctx, + `SELECT EXISTS ( + SELECT 1 + FROM information_schema.tables + WHERE table_schema = current_schema() + AND table_name = 'schema_metadata' + )`, + ).Scan(&metadataTableExists) + if err != nil { + return fmt.Errorf("inspect code intelligence schema metadata: %w", err) + } + + if !metadataTableExists { + return nil + } + + version, found, err := storedCodeIntelSchemaVersion(ctx, database) + if err != nil { + return err + } + + if found && version > schemaVersion { + return unsupportedCodeIntelSchemaVersionError(version) + } + + return nil +} + +func unsupportedCodeIntelSchemaVersionError(version int) error { + return fmt.Errorf( + "migrate code intelligence store: schema version %d "+ + "exceeds supported version %d: %w", + version, + schemaVersion, + errStoreMigrationIntegrity, + ) +} + +func storedCodeIntelSchemaVersion( + ctx context.Context, + database *sql.DB, +) (int, bool, error) { + var rawVersion string + + err := database.QueryRowContext( + ctx, + "SELECT value FROM schema_metadata WHERE key = 'schema_version'", + ).Scan(&rawVersion) + if errors.Is(err, sql.ErrNoRows) { + return 0, false, nil + } + + if err != nil { + return 0, false, fmt.Errorf( + "read code intelligence schema version: %w", + err, + ) + } + + version, err := strconv.Atoi(strings.TrimSpace(rawVersion)) + if err != nil { + return 0, false, fmt.Errorf( + "parse code intelligence schema version %q: %w", + rawVersion, + err, + ) + } + + return version, true, nil +} + type statCountQuery struct { target *int name string diff --git a/go/internal/codeintel/store_migration_test.go b/go/internal/codeintel/store_migration_test.go index 31c95b74..9fe3b012 100644 --- a/go/internal/codeintel/store_migration_test.go +++ b/go/internal/codeintel/store_migration_test.go @@ -327,6 +327,55 @@ func TestMigrateStoreRejectsUnexpectedSchemaVersion(t *testing.T) { } } +func TestOpenRejectsFutureSchemaBeforeDDLWrites(t *testing.T) { + ctx := context.Background() + path := DefaultDBPath(t.TempDir()) + store, err := Open(ctx, path) + if err != nil { + t.Fatalf("open future-schema fixture: %v", err) + } + + _, err = store.Database().ExecContext( + ctx, + `INSERT INTO code_intel_fts( + fts_id, kind, record_id, trace_id, search_text + ) VALUES ('', 'finding', 'future-record', 'future-trace', 'future schema')`, + ) + if err != nil { + t.Fatalf("insert future-schema write sentinel: %v", err) + } + _, err = store.Database().ExecContext( + ctx, + "UPDATE schema_metadata SET value = '999' WHERE key = 'schema_version'", + ) + if err != nil { + t.Fatalf("mark fixture as future schema: %v", err) + } + closeMigrationTestStore(t, store) + + _, err = Open(ctx, path) + if err == nil || !strings.Contains(err.Error(), "schema version 999") { + t.Fatalf("expected future schema rejection, got %v", err) + } + + database, err := sql.Open("duckdb", path+"?access_mode=READ_ONLY") + if err != nil { + t.Fatalf("open rejected future schema read-only: %v", err) + } + defer database.Close() + + var ftsID string + if err = database.QueryRowContext( + ctx, + "SELECT fts_id FROM code_intel_fts WHERE record_id = 'future-record'", + ).Scan(&ftsID); err != nil { + t.Fatalf("read future-schema write sentinel: %v", err) + } + if ftsID != "" { + t.Fatalf("future-schema rejection mutated fts_id to %q", ftsID) + } +} + func TestMigrateStoreRejectsExistingManifestBeforeOpeningDestination(t *testing.T) { ctx := context.Background() repositoryRoot := t.TempDir() diff --git a/go/internal/e2e/git_commit_workflow_test.go b/go/internal/e2e/git_commit_workflow_test.go index 74fa462d..fa03128e 100644 --- a/go/internal/e2e/git_commit_workflow_test.go +++ b/go/internal/e2e/git_commit_workflow_test.go @@ -96,6 +96,7 @@ func TestParentRuntimeCerunGitAddAndCommitUseInstalledArtifacts(t *testing.T) { if err := os.Chmod(parentBin, 0o500); err != nil { t.Fatalf("make parent runtime bin read-only: %v", err) } + assertParentRuntimeExecutableLayout(t, parentBin, parentCerun) add := e2e.Run( t, parentRepo, @@ -109,6 +110,11 @@ func TestParentRuntimeCerunGitAddAndCommitUseInstalledArtifacts(t *testing.T) { if err := os.Chmod(parentBin, 0o700); err != nil { t.Fatalf("restore parent runtime bin permissions: %v", err) } + if activeAgentShellGitBindDenied(add) { + assertParentRuntimePathNotStaged(t, parentRepo, "runtime-add.txt") + + return + } add.RequireExit(t, 0) commit := e2e.Run( @@ -126,6 +132,82 @@ func TestParentRuntimeCerunGitAddAndCommitUseInstalledArtifacts(t *testing.T) { assertNoCommitHeadPolicyLeak(t, commit) } +func activeAgentShellGitBindDenied(result e2e.CommandResult) bool { + return result.Code != 0 && + os.Getenv("CODING_ETHOS_AGENT_SHELL_SANDBOX") == "1" && + os.Getenv("CODING_ETHOS_SANDBOX_ACTIVE") == "1" && + strings.Contains( + strings.ToLower(result.Combined), + "coding-ethos-sandbox: permission denied", + ) +} + +func assertParentRuntimePathNotStaged(t *testing.T, repoRoot, path string) { + t.Helper() + + cached := e2e.Run( + t, + repoRoot, + realGitPath(t), + "diff", + "--cached", + "--name-only", + "--", + path, + ) + cached.RequireExit(t, 0) + if strings.TrimSpace(cached.Stdout) != "" { + t.Fatalf("denied parent runtime git bind staged %s:\n%s", path, cached.Stdout) + } +} + +func assertParentRuntimeExecutableLayout(t *testing.T, parentBin, parentCerun string) { + t.Helper() + + for _, path := range []string{ + parentCerun, + filepath.Join(parentBin, "coding-ethos-run"), + } { + info, err := os.Lstat(path) + if err != nil { + t.Fatalf("inspect parent runtime executable %s: %v", path, err) + } + if !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 { + t.Fatalf("parent runtime executable is not a regular file: %s", path) + } + } + + binInfo, err := os.Stat(parentBin) + if err != nil { + t.Fatalf("inspect parent runtime bin: %v", err) + } + if binInfo.Mode().Perm() != 0o500 { + t.Fatalf("parent runtime bin mode = %04o, want 0500", binInfo.Mode().Perm()) + } + + sourceSandbox := filepath.Join( + repoRootFromWorkingDirectory(t), + "bin", + "coding-ethos-sandbox", + ) + parentSandbox := filepath.Join(parentBin, "coding-ethos-sandbox") + resolvedSource, err := filepath.EvalSymlinks(sourceSandbox) + if err != nil { + t.Fatalf("resolve source sandbox helper: %v", err) + } + resolvedParent, err := filepath.EvalSymlinks(parentSandbox) + if err != nil { + t.Fatalf("resolve parent sandbox helper: %v", err) + } + if resolvedParent != resolvedSource { + t.Fatalf( + "parent sandbox helper resolves to %s, want %s", + resolvedParent, + resolvedSource, + ) + } +} + func TestParentRuntimeCerunGitAddFailsFastWithoutPolicyBundle(t *testing.T) { fixture := preparedParentRuntimeRepo(t) policyBundle := filepath.Join( @@ -316,7 +398,12 @@ func installParentRuntimeArtifacts(t *testing.T, sourceRoot, parentRepo string) } sourceBinaries = append(sourceBinaries, filepath.Join(sourceRoot, "bin", "cerun")) for _, source := range sourceBinaries { - copyRuntimeFile(t, source, filepath.Join(parentBin, filepath.Base(source))) + destination := filepath.Join(parentBin, filepath.Base(source)) + if filepath.Base(source) == "coding-ethos-sandbox" { + e2e.LinkManagedSandboxHelper(t, source, destination) + continue + } + copyRuntimeFile(t, source, destination) } compile := e2e.Run( diff --git a/go/internal/e2e/mcp_test.go b/go/internal/e2e/mcp_test.go index aa069f44..e694986d 100644 --- a/go/internal/e2e/mcp_test.go +++ b/go/internal/e2e/mcp_test.go @@ -20,7 +20,9 @@ import ( "blackcat.ca/coding-ethos/go/internal/e2e" ) -const mcpClientTimeout = 30 * time.Second +// The client spans the complete multi-request workflow; individual command +// deadlines remain independently bounded by the shared e2e command timeout. +const mcpClientTimeout = 2 * time.Minute type MCPClient struct { cancel context.CancelFunc diff --git a/go/internal/e2e/process_unix.go b/go/internal/e2e/process_unix.go index 9dfed605..550bab60 100644 --- a/go/internal/e2e/process_unix.go +++ b/go/internal/e2e/process_unix.go @@ -7,6 +7,8 @@ package e2e import ( "errors" + "fmt" + "os" "os/exec" "syscall" ) @@ -15,13 +17,26 @@ func configureCommandProcessGroup(cmd *exec.Cmd) { cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} } -func terminateCommandProcessGroup(cmd *exec.Cmd) { +func configureCommandCancellation(cmd *exec.Cmd) { + cmd.Cancel = func() error { + return terminateCommandProcessGroup(cmd) + } + cmd.WaitDelay = commandWaitDelay +} + +func terminateCommandProcessGroup(cmd *exec.Cmd) error { if cmd.Process == nil { - return + return os.ErrProcessDone } err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) - if err != nil && !errors.Is(err, syscall.ESRCH) { - return + if errors.Is(err, syscall.ESRCH) { + return os.ErrProcessDone } + + if err != nil { + return fmt.Errorf("kill command process group %d: %w", cmd.Process.Pid, err) + } + + return nil } diff --git a/go/internal/e2e/process_unix_test.go b/go/internal/e2e/process_unix_test.go new file mode 100644 index 00000000..db10a4ce --- /dev/null +++ b/go/internal/e2e/process_unix_test.go @@ -0,0 +1,169 @@ +// SPDX-FileCopyrightText: 2026 Blackcat Informatics Inc. +// SPDX-License-Identifier: AGPL-3.0-only + +//go:build !windows + +package e2e + +import ( + "bufio" + "bytes" + "context" + "errors" + "os" + "os/exec" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +const ( + processGroupHelperMode = "CODING_ETHOS_E2E_PROCESS_GROUP_HELPER" + processGroupReadyFD = "CODING_ETHOS_E2E_PROCESS_GROUP_READY_FD" + processGroupChildPID = "child-pid=" +) + +func TestCommandCancellationTerminatesProcessGroup(t *testing.T) { + if mode := os.Getenv(processGroupHelperMode); mode != "" { + runProcessGroupHelper(t, mode) + + return + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cmd := exec.CommandContext( + ctx, + os.Args[0], + "-test.run=^TestCommandCancellationTerminatesProcessGroup$", + ) + readyReader, readyWriter, err := os.Pipe() + if err != nil { + t.Fatalf("create process-group readiness pipe: %v", err) + } + defer func() { _ = readyReader.Close() }() + + cmd.Env = append( + os.Environ(), + processGroupHelperMode+"=parent", + processGroupReadyFD+"=3", + ) + cmd.ExtraFiles = []*os.File{readyWriter} + configureCommandProcessGroup(cmd) + configureCommandCancellation(cmd) + + var output bytes.Buffer + cmd.Stdout = &output + cmd.Stderr = &output + + if err := cmd.Start(); err != nil { + _ = readyWriter.Close() + t.Fatalf("start process-group helper: %v", err) + } + if err := readyWriter.Close(); err != nil { + cancel() + _ = cmd.Wait() + t.Fatalf("close process-group readiness writer: %v", err) + } + + if err := readyReader.SetReadDeadline(time.Now().Add(10 * time.Second)); err != nil { + cancel() + _ = cmd.Wait() + t.Fatalf("bound process-group readiness read: %v", err) + } + line, err := bufio.NewReader(readyReader).ReadString('\n') + if err != nil { + cancel() + _ = cmd.Wait() + t.Fatalf("process-group helper did not become ready: %v\n%s", err, &output) + } + childPID := processGroupHelperChildPID(t, line) + + started := time.Now() + cancel() + err = cmd.Wait() + elapsed := time.Since(started) + if err == nil || !errors.Is(ctx.Err(), context.Canceled) { + t.Fatalf("canceled process group returned err=%v context=%v", err, ctx.Err()) + } + if elapsed >= time.Second { + t.Fatalf("ready process-group cancellation took %s", elapsed) + } + + assertProcessExited(t, childPID) +} + +func runProcessGroupHelper(t *testing.T, mode string) { + t.Helper() + + if mode == "child" { + time.Sleep(30 * time.Second) + + return + } + if mode != "parent" { + t.Fatalf("unknown process-group helper mode %q", mode) + } + readyDescriptor, err := strconv.Atoi(os.Getenv(processGroupReadyFD)) + if err != nil { + t.Fatalf("parse process-group readiness descriptor: %v", err) + } + ready := os.NewFile(uintptr(readyDescriptor), "process-group-ready") + if ready == nil { + t.Fatal("open process-group readiness descriptor") + } + defer func() { _ = ready.Close() }() + syscall.CloseOnExec(readyDescriptor) + + child := exec.Command( + os.Args[0], + "-test.run=^TestCommandCancellationTerminatesProcessGroup$", + ) + child.Env = append(os.Environ(), processGroupHelperMode+"=child") + child.Stdout = os.Stdout + child.Stderr = os.Stderr + if err := child.Start(); err != nil { + t.Fatalf("start process-group child: %v", err) + } + if _, err := ready.WriteString( + processGroupChildPID + strconv.Itoa(child.Process.Pid) + "\n", + ); err != nil { + t.Fatalf("write process-group readiness: %v", err) + } + if err := ready.Close(); err != nil { + t.Fatalf("close process-group readiness: %v", err) + } + time.Sleep(30 * time.Second) +} + +func processGroupHelperChildPID(t *testing.T, line string) int { + t.Helper() + + value, found := strings.CutPrefix(strings.TrimSpace(line), processGroupChildPID) + if !found { + t.Fatalf("invalid process-group readiness line %q", line) + } + pid, err := strconv.Atoi(value) + if err != nil { + t.Fatalf("parse process-group child PID %q: %v", value, err) + } + + return pid +} + +func assertProcessExited(t *testing.T, pid int) { + t.Helper() + + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) { + err := syscall.Kill(pid, 0) + if errors.Is(err, syscall.ESRCH) { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("process-group child %d survived cancellation", pid) +} diff --git a/go/internal/e2e/process_windows.go b/go/internal/e2e/process_windows.go index af9969d2..3d0aafa7 100644 --- a/go/internal/e2e/process_windows.go +++ b/go/internal/e2e/process_windows.go @@ -5,8 +5,83 @@ package e2e -import "os/exec" +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strconv" + + "golang.org/x/sys/windows" +) func configureCommandProcessGroup(_ *exec.Cmd) {} -func terminateCommandProcessGroup(_ *exec.Cmd) {} +func configureCommandCancellation(cmd *exec.Cmd) { + cmd.Cancel = func() error { + return terminateCommandProcessGroup(cmd) + } + cmd.WaitDelay = commandWaitDelay +} + +func terminateCommandProcessGroup(cmd *exec.Cmd) error { + if cmd.Process == nil { + return os.ErrProcessDone + } + + if windowsProcessExited(cmd.Process.Pid) { + return os.ErrProcessDone + } + + terminationContext, cancel := context.WithTimeout( + context.Background(), + commandWaitDelay, + ) + defer cancel() + + command := exec.CommandContext( + terminationContext, + windowsTaskkillPath(), + "/PID", strconv.Itoa(cmd.Process.Pid), + "/T", + "/F", + ) + err := command.Run() + if err == nil { + return nil + } + if windowsProcessExited(cmd.Process.Pid) { + return os.ErrProcessDone + } + + return fmt.Errorf("terminate command process tree %d: %w", cmd.Process.Pid, err) +} + +// windowsTaskkillPath resolves taskkill.exe from SystemRoot rather than PATH. +// A PATH without System32 would otherwise fail cancellation and leave the +// process tree running. +func windowsTaskkillPath() string { + systemRoot := os.Getenv("SystemRoot") + if systemRoot == "" { + systemRoot = `C:\Windows` + } + + return filepath.Join(systemRoot, "System32", "taskkill.exe") +} + +func windowsProcessExited(pid int) bool { + handle, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(pid)) + if errors.Is(err, windows.ERROR_INVALID_PARAMETER) { + return true + } + if err != nil { + return false + } + defer func() { _ = windows.CloseHandle(handle) }() + + status, err := windows.WaitForSingleObject(handle, 0) + + return err == nil && status == windows.WAIT_OBJECT_0 +} diff --git a/go/internal/e2e/process_windows_test.go b/go/internal/e2e/process_windows_test.go new file mode 100644 index 00000000..0c25f894 --- /dev/null +++ b/go/internal/e2e/process_windows_test.go @@ -0,0 +1,168 @@ +// SPDX-FileCopyrightText: 2026 Blackcat Informatics Inc. +// SPDX-License-Identifier: AGPL-3.0-only + +//go:build windows + +package e2e + +import ( + "bufio" + "context" + "errors" + "os" + "os/exec" + "strconv" + "strings" + "testing" + "time" + + "golang.org/x/sys/windows" +) + +const ( + windowsProcessTreeHelperMode = "CODING_ETHOS_E2E_WINDOWS_PROCESS_TREE_HELPER" + windowsProcessTreeChildPID = "child-pid=" +) + +type windowsProcessTreeReady struct { + Err error + Line string +} + +func TestCommandCancellationTerminatesWindowsProcessTree(t *testing.T) { + if mode := os.Getenv(windowsProcessTreeHelperMode); mode != "" { + runWindowsProcessTreeHelper(t, mode) + + return + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cmd := exec.CommandContext( + ctx, + os.Args[0], + "-test.run=^TestCommandCancellationTerminatesWindowsProcessTree$", + ) + cmd.Env = append(os.Environ(), windowsProcessTreeHelperMode+"=parent") + configureCommandProcessGroup(cmd) + configureCommandCancellation(cmd) + + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatalf("open process-tree stdout: %v", err) + } + cmd.Stderr = os.Stderr + + if err = cmd.Start(); err != nil { + t.Fatalf("start process-tree helper: %v", err) + } + + ready := make(chan windowsProcessTreeReady, 1) + go func() { + line, readErr := bufio.NewReader(stdout).ReadString('\n') + ready <- windowsProcessTreeReady{Err: readErr, Line: line} + }() + + var childPID int + select { + case result := <-ready: + if result.Err != nil { + cancel() + _ = cmd.Wait() + t.Fatalf("process-tree helper did not become ready: %v", result.Err) + } + childPID = windowsProcessTreeHelperChildPID(t, result.Line) + case <-time.After(10 * time.Second): + cancel() + _ = cmd.Wait() + t.Fatal("process-tree helper readiness timed out") + } + + cancel() + waited := make(chan error, 1) + go func() { waited <- cmd.Wait() }() + + select { + case err = <-waited: + if err == nil || !errors.Is(ctx.Err(), context.Canceled) { + t.Fatalf("canceled process tree returned err=%v context=%v", err, ctx.Err()) + } + case <-time.After(5 * time.Second): + t.Fatal("canceled process tree did not return") + } + + assertWindowsProcessExited(t, childPID) +} + +func runWindowsProcessTreeHelper(t *testing.T, mode string) { + t.Helper() + + if mode == "child" { + time.Sleep(30 * time.Second) + + return + } + if mode != "parent" { + t.Fatalf("unknown process-tree helper mode %q", mode) + } + + child := exec.Command( + os.Args[0], + "-test.run=^TestCommandCancellationTerminatesWindowsProcessTree$", + ) + child.Env = append(os.Environ(), windowsProcessTreeHelperMode+"=child") + child.Stdout = os.Stdout + child.Stderr = os.Stderr + if err := child.Start(); err != nil { + t.Fatalf("start process-tree child: %v", err) + } + if _, err := os.Stdout.WriteString( + windowsProcessTreeChildPID + strconv.Itoa(child.Process.Pid) + "\n", + ); err != nil { + t.Fatalf("write process-tree readiness: %v", err) + } + time.Sleep(30 * time.Second) +} + +func windowsProcessTreeHelperChildPID(t *testing.T, line string) int { + t.Helper() + + value, found := strings.CutPrefix( + strings.TrimSpace(line), + windowsProcessTreeChildPID, + ) + if !found { + t.Fatalf("invalid process-tree readiness line %q", line) + } + pid, err := strconv.Atoi(value) + if err != nil { + t.Fatalf("parse process-tree child PID %q: %v", value, err) + } + + return pid +} + +func assertWindowsProcessExited(t *testing.T, pid int) { + t.Helper() + + handle, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(pid)) + if errors.Is(err, windows.ERROR_INVALID_PARAMETER) { + return + } + if err != nil { + t.Fatalf("open process-tree child %d: %v", pid, err) + } + defer func() { _ = windows.CloseHandle(handle) }() + + status, err := windows.WaitForSingleObject( + handle, + uint32((5*time.Second)/time.Millisecond), + ) + if err != nil { + t.Fatalf("wait for process-tree child %d: %v", pid, err) + } + if status != windows.WAIT_OBJECT_0 { + t.Fatalf("process-tree child %d survived cancellation", pid) + } +} diff --git a/go/internal/e2e/scenario.go b/go/internal/e2e/scenario.go index 45ba92c0..c1391b75 100644 --- a/go/internal/e2e/scenario.go +++ b/go/internal/e2e/scenario.go @@ -24,6 +24,7 @@ import ( const ( commandTimeout = 30 * time.Second + commandWaitDelay = time.Second e2eDirMode = 0o700 e2eFileMode = 0o600 optionalVenvRuntimeEntry = ".venv" @@ -206,7 +207,7 @@ func copyMutableRuntimeBin(t *testing.T, sourceRoot, runtimeRoot string) { target := filepath.Join(binRoot, filepath.Base(source)) if filepath.Base(source) == "coding-ethos-sandbox" { - linkManagedSandboxHelper(t, source, target) + LinkManagedSandboxHelper(t, source, target) continue } @@ -296,7 +297,7 @@ func buildInstrumentedEthosRoot(t *testing.T, ethosRoot string) string { buildInstrumentedCommand(t, ethosRoot, runtimeRoot, command) } - linkManagedSandboxHelper( + LinkManagedSandboxHelper( t, filepath.Join(ethosRoot, "bin", "coding-ethos-sandbox"), filepath.Join(runtimeRoot, "bin", "coding-ethos-sandbox"), @@ -305,11 +306,11 @@ func buildInstrumentedEthosRoot(t *testing.T, ethosRoot string) string { return runtimeRoot } -// linkManagedSandboxHelper preserves the exact repository-managed executable +// LinkManagedSandboxHelper preserves the exact repository-managed executable // path that host AppArmor policy authorizes for Linux namespace creation. // Instrumenting or copying the helper changes that path and makes an otherwise // valid nested sandbox fail closed before it can apply its policy. -func linkManagedSandboxHelper(t *testing.T, source, destination string) { +func LinkManagedSandboxHelper(t *testing.T, source, destination string) { t.Helper() resolved, err := filepath.EvalSymlinks(source) @@ -617,6 +618,7 @@ func runCommand( cmd.Env = commandEnvironmentWith(t, overrides) cmd.Stdin = strings.NewReader(input) configureCommandProcessGroup(cmd) + configureCommandCancellation(cmd) var ( stdout bytes.Buffer @@ -633,7 +635,15 @@ func runCommand( var exitErr *exec.ExitError switch { case ctx.Err() != nil: - terminateCommandProcessGroup(cmd) + terminationErr := terminateCommandProcessGroup(cmd) + if terminationErr != nil && !errors.Is(terminationErr, os.ErrProcessDone) { + t.Fatalf( + "terminate timed-out command %q: %v", + strings.Join(args, " "), + terminationErr, + ) + } + t.Fatalf("command timed out: %s", strings.Join(args, " ")) case errors.As(err, &exitErr): code = exitErr.ExitCode() diff --git a/go/internal/evaluators/cel_expression.go b/go/internal/evaluators/cel_expression.go index 68a61b74..aa1b4958 100644 --- a/go/internal/evaluators/cel_expression.go +++ b/go/internal/evaluators/cel_expression.go @@ -10,6 +10,8 @@ import ( "strconv" "strings" + "github.com/google/cel-go/cel" + "blackcat.ca/coding-ethos/go/diagnostics" "blackcat.ca/coding-ethos/go/internal/apperror" "blackcat.ca/coding-ethos/go/internal/celexpr" @@ -23,6 +25,8 @@ type ( const ( bashExtension = ".bash" + changeSourceProposed = "proposed" + changeSourceStaged = "staged" defaultGoHardLineLimit = 2000 defaultPythonHardLineLimit = 1000 defaultShellHardLineLimit = 500 @@ -40,6 +44,9 @@ const ( scriptsPrefix = "scripts/" ) +// maxIsolatedDiffLineWitnessCandidates bounds best-effort diagnostic enrichment. +const maxIsolatedDiffLineWitnessCandidates = 256 + var celQuotedGlobPattern = regexp.MustCompile( `"([a-z]+(?:_\*)?)"`, ) @@ -74,16 +81,20 @@ func EvaluateCELExpression( activation := celActivation(context, source) - output, _, err := program.Eval(activation) + matched, err := evaluateCELBool(program, activation) if err != nil { return nil, fmt.Errorf("evaluate CEL expression: %w", err) } - matched, ok := output.Value().(bool) - if !ok || !matched { + if !matched { return nil, nil } + diffLineWitness, witnessErr := isolatedDiffLineWitness(program, activation) + if witnessErr != nil { + diffLineWitness = celDiffLineWitness{} + } + decisionMode := strings.TrimSpace(policyDef.DefaultSeverity) if decisionMode == "" { decisionMode = blockDecision @@ -118,6 +129,7 @@ func EvaluateCELExpression( decisionMode, source, activation, + diffLineWitness, )} return []policy.Decision{decision}, nil @@ -129,6 +141,7 @@ func celDiagnostic( decisionMode string, source string, activation map[string]any, + diffLineWitness celDiffLineWitness, ) diagnostics.Diagnostic { diagnostic := diagnostics.Diagnostic{ Tool: "policy", @@ -151,45 +164,216 @@ func celDiagnostic( return diagnostic } - if len(context.Files) == 1 { + applyCELSpecializedDiagnostic( + &diagnostic, + context, + policyDef, + source, + activation, + ) + + if diagnostic.File == "" && diffLineWitness.found { + diagnostic.File = diffLineWitness.line.File + diagnostic.Line = int(diffLineWitness.line.Line) + diagnostic.Detail = diffLineWitness.line.Text + diagnostic.Metadata["diff_change_source"] = diffLineWitness.changeSource + } + + if diagnostic.File == "" && len(context.Files) == 1 { diagnostic.File = context.Files[0] } - if policyDef.ID == filesystemLineLimitsPolicy { + return diagnostic +} + +func applyCELSpecializedDiagnostic( + diagnostic *diagnostics.Diagnostic, + context Context, + policyDef policy.Policy, + source string, + activation map[string]any, +) { + switch policyDef.ID { + case filesystemLineLimitsPolicy: applyLineLimitFileDiagnostic( - &diagnostic, + diagnostic, + activation, + context.EvaluatorOptions, + ) + case hookChangedFileScopePolicy: + applyHookCommandDiagnostic(diagnostic, activation) + case similarCodeDetectedPolicy: + applySimilarityDiagnostic(diagnostic, activation) + case pythonSuppressionWritePolicy: + applyPythonSuppressionDiagnostic( + diagnostic, activation, context.EvaluatorOptions, ) + default: + if strings.Contains(source, "changed_symbols") || + strings.Contains(source, "proposed_symbol_changes") { + applyGrowingSymbolDiagnostic(diagnostic, activation) + } + } +} - return diagnostic +type celDiffLineWitness struct { + changeSource string + line celexpr.DiffLineInput + found bool +} + +func evaluateCELBool(program cel.Program, activation map[string]any) (bool, error) { + output, _, err := program.Eval(activation) + if err != nil { + return false, fmt.Errorf("evaluate CEL program: %w", err) } - if policyDef.ID == hookChangedFileScopePolicy { - applyHookCommandDiagnostic(&diagnostic, activation) + matched, ok := output.Value().(bool) + + return ok && matched, nil +} - return diagnostic +func isolatedDiffLineWitness( + program cel.Program, + activation map[string]any, +) (celDiffLineWitness, error) { + diff, ok := activation["diff"].(celexpr.DiffInput) + if !ok { + return celDiffLineWitness{}, nil } - if policyDef.ID == similarCodeDetectedPolicy { - applySimilarityDiagnostic(&diagnostic, activation) + candidateCount := len(diff.AddedLines) + len(diff.RemovedLines) + if candidateCount == 0 || + candidateCount > maxIsolatedDiffLineWitnessCandidates { + return celDiffLineWitness{}, nil + } - return diagnostic + emptyDiff := emptyDiffLineView(diff) + emptyActivation := maps.Clone(activation) + emptyActivation["diff"] = emptyDiff + + matched, err := evaluateCELBool(program, emptyActivation) + if err != nil { + return celDiffLineWitness{}, err } - if policyDef.ID == pythonSuppressionWritePolicy { - applyPythonSuppressionDiagnostic( - &diagnostic, - activation, - context.EvaluatorOptions, + if matched { + return celDiffLineWitness{}, nil + } + + candidates := make( + []celDiffLineWitness, + 0, + candidateCount, + ) + for _, line := range diff.AddedLines { + candidates = append(candidates, celDiffLineWitness{ + changeSource: "added", + line: line, + found: true, + }) + } + + for _, line := range diff.RemovedLines { + candidates = append(candidates, celDiffLineWitness{ + changeSource: "removed", + line: line, + found: true, + }) + } + + var witness celDiffLineWitness + + for index := range candidates { + candidate := candidates[index] + candidateActivation := maps.Clone(activation) + candidateActivation["diff"] = oneDiffLineView( + emptyDiff, + diff, + candidate.line, + candidate.changeSource == "added", ) - return diagnostic + matched, err = evaluateCELBool(program, candidateActivation) + if err != nil { + return celDiffLineWitness{}, err + } + + if !matched { + continue + } + + if witness.found { + return celDiffLineWitness{}, nil + } + + witness = candidate } - applyGrowingSymbolDiagnostic(&diagnostic, activation) + return witness, nil +} - return diagnostic +func emptyDiffLineView(diff celexpr.DiffInput) celexpr.DiffInput { + empty := diff + empty.AddedLines = []celexpr.DiffLineInput{} + empty.RemovedLines = []celexpr.DiffLineInput{} + empty.Hunks = make([]celexpr.DiffHunkInput, len(diff.Hunks)) + + for index, hunk := range diff.Hunks { + empty.Hunks[index] = hunk + empty.Hunks[index].AddedLines = []celexpr.DiffLineInput{} + empty.Hunks[index].RemovedLines = []celexpr.DiffLineInput{} + } + + return empty +} + +func oneDiffLineView( + empty celexpr.DiffInput, + original celexpr.DiffInput, + line celexpr.DiffLineInput, + added bool, +) celexpr.DiffInput { + view := emptyDiffLineView(empty) + if added { + view.AddedLines = []celexpr.DiffLineInput{line} + } else { + view.RemovedLines = []celexpr.DiffLineInput{line} + } + + for hunkIndex, hunk := range original.Hunks { + lines := hunk.RemovedLines + if added { + lines = hunk.AddedLines + } + + for _, hunkLine := range lines { + if !sameDiffLine(line, hunkLine) { + continue + } + + if added { + view.Hunks[hunkIndex].AddedLines = []celexpr.DiffLineInput{line} + } else { + view.Hunks[hunkIndex].RemovedLines = []celexpr.DiffLineInput{line} + } + + return view + } + } + + return view +} + +func sameDiffLine(left, right celexpr.DiffLineInput) bool { + return left.File == right.File && + left.Text == right.Text && + left.Line == right.Line && + left.NewLine == right.NewLine && + left.OldLine == right.OldLine && + left.IsBlank == right.IsBlank } func applyPythonSuppressionDiagnostic( @@ -329,7 +513,7 @@ func applyGrowingSymbolDiagnostic( diagnostic.File = symbol.File diagnostic.Line = int(symbol.ProposedStartLine) diagnostic.Metadata["ast_action"] = symbol.Action - diagnostic.Metadata["ast_change_source"] = "proposed" + diagnostic.Metadata["ast_change_source"] = changeSourceProposed diagnostic.Metadata["ast_language"] = symbol.Language diagnostic.Metadata["ast_line_delta"] = symbol.LineDelta diagnostic.Metadata["ast_nonblank_line_delta"] = symbol.NonBlankLineDelta @@ -349,7 +533,7 @@ func applyGrowingSymbolDiagnostic( diagnostic.File = symbol.File diagnostic.Line = int(symbol.CurrentStartLine) diagnostic.Metadata["ast_action"] = symbol.Action - diagnostic.Metadata["ast_change_source"] = "staged" + diagnostic.Metadata["ast_change_source"] = changeSourceStaged diagnostic.Metadata["ast_language"] = symbol.Language diagnostic.Metadata["ast_line_delta"] = symbol.LineDelta diagnostic.Metadata["ast_nonblank_line_delta"] = symbol.NonBlankLineDelta @@ -389,7 +573,7 @@ func applyLineLimitFileDiagnostic( if file, ok := firstLineLimitProposedFile(activation, thresholds); ok { diagnostic.File = file.File - diagnostic.Metadata["line_limit_change_source"] = "proposed" + diagnostic.Metadata["line_limit_change_source"] = changeSourceProposed diagnostic.Metadata["current_line_count"] = file.CurrentLineCount diagnostic.Metadata["current_nonblank_line_count"] = file.CurrentNonBlankLineCount diagnostic.Metadata["proposed_line_count"] = file.ProposedLineCount @@ -400,7 +584,7 @@ func applyLineLimitFileDiagnostic( if file, ok := firstLineLimitChangedFile(activation, thresholds); ok { diagnostic.File = file.File - diagnostic.Metadata["line_limit_change_source"] = "staged" + diagnostic.Metadata["line_limit_change_source"] = changeSourceStaged diagnostic.Metadata["current_line_count"] = file.LineCount diagnostic.Metadata["current_nonblank_line_count"] = file.NonBlankLineCount diagnostic.Metadata["original_line_count"] = file.OriginalLineCount diff --git a/go/internal/evaluators/cel_expression_test.go b/go/internal/evaluators/cel_expression_test.go index 4e80c011..6cc682b7 100644 --- a/go/internal/evaluators/cel_expression_test.go +++ b/go/internal/evaluators/cel_expression_test.go @@ -21,7 +21,8 @@ import ( ) const ( - lineLimitAdvice = "Do not make cosmetic or documentation-only edits just to " + + celDiffLineWitnessCandidateLimit = 256 + lineLimitAdvice = "Do not make cosmetic or documentation-only edits just to " + "satisfy the limit; apply SOLID refactoring and split the file into " + "focused modules before committing." lineLimitFile = "app.py" @@ -404,6 +405,215 @@ func TestEvaluateCELExpressionBlocksAgentBrandingInStagedMarkdown(t *testing.T) } } +func TestEvaluateCELExpressionDiffRuleLocatesMatchedLine(t *testing.T) { + t.Parallel() + + repo := t.TempDir() + runCELGit(t, repo, "init") + runCELGit(t, repo, "config", "user.email", "test@example.com") + runCELGit(t, repo, "config", "user.name", "Test User") + + unrelatedFile := filepath.Join(repo, "a_unrelated.go") + matchedFile := filepath.Join(repo, "z_matched.go") + + if err := os.WriteFile( + unrelatedFile, + []byte("package sample\n\nfunc unrelated() {}\n"), + 0o600, + ); err != nil { + t.Fatalf("write unrelated source: %v", err) + } + if err := os.WriteFile( + matchedFile, + []byte("package sample\n\nfunc target() {}\n"), + 0o600, + ); err != nil { + t.Fatalf("write matched source: %v", err) + } + runCELGit(t, repo, "add", "a_unrelated.go", "z_matched.go") + runCELGit(t, repo, "commit", "-m", "initial") + + if err := os.WriteFile( + unrelatedFile, + []byte("package sample\n\nfunc unrelated() {\n\tprintln(\"grows\")\n}\n"), + 0o600, + ); err != nil { + t.Fatalf("grow unrelated source: %v", err) + } + if err := os.WriteFile( + matchedFile, + []byte("package sample\n\nfunc target() {\n\t"+ + "fmt."+"Fprintf(nil, \"blocked\")\n}\n"), + 0o600, + ); err != nil { + t.Fatalf("write matched diff line: %v", err) + } + runCELGit(t, repo, "add", "a_unrelated.go", "z_matched.go") + + policyDef := celExpressionPolicy() + policyDef.ID = "feedback.route" + + decisions, err := EvaluateCELExpression( + policyDef, + Context{ + Cwd: repo, + Files: []string{"a_unrelated.go", "z_matched.go"}, + StagedFiles: []string{"a_unrelated.go", "z_matched.go"}, + Scope: "staged", + EvaluatorOptions: map[string]any{ + "when": `diff.added_lines.exists(line, + line.file == "z_matched.go" && + line.text.contains("fmt." + "Fprintf") + )`, + }, + }, + ) + if err != nil { + t.Fatalf("evaluate CEL expression: %v", err) + } + + if len(decisions) != 1 { + t.Fatalf("decisions = %#v, want one block", decisions) + } + if diagnostic := decisions[0].Diagnostics[0]; diagnostic.File != "z_matched.go" || + diagnostic.Line != 4 { + t.Fatalf("diagnostic = %#v, want z_matched.go:4", diagnostic) + } +} + +func TestEvaluateCELExpressionDiffRuleDiscardsAmbiguousWitness(t *testing.T) { + t.Parallel() + + diagnostic := evaluateCELDiffLinePolicy( + t, + "ambiguous.go", + []string{"blocked first", "blocked second"}, + `diff.added_lines.exists(line, line.text.contains("blocked"))`, + ) + + if diagnostic.File != "ambiguous.go" || diagnostic.Line != 0 { + t.Fatalf("diagnostic = %#v, want ambiguous.go fallback without line", diagnostic) + } + if _, found := diagnostic.Metadata["diff_change_source"]; found { + t.Fatalf("ambiguous diagnostic has diff witness metadata: %#v", diagnostic) + } +} + +func TestEvaluateCELExpressionDiffRuleKeepsDecisionWhenWitnessFails( + t *testing.T, +) { + t.Parallel() + + diagnostic := evaluateCELDiffLinePolicy( + t, + "indexed.go", + []string{"blocked"}, + `diff.added_lines[0].text.contains("blocked")`, + ) + + if diagnostic.File != "indexed.go" || diagnostic.Line != 0 { + t.Fatalf("diagnostic = %#v, want indexed.go fallback without line", diagnostic) + } + if _, found := diagnostic.Metadata["diff_change_source"]; found { + t.Fatalf("failed witness has diff witness metadata: %#v", diagnostic) + } +} + +func TestEvaluateCELExpressionDiffRuleBoundsWitnessCandidates(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + count int + witnessLine int + }{ + { + name: "at limit", + count: celDiffLineWitnessCandidateLimit, + witnessLine: celDiffLineWitnessCandidateLimit, + }, + { + name: "above limit", + count: celDiffLineWitnessCandidateLimit + 1, + }, + } { + t.Run(test.name, func(t *testing.T) { + lines := make([]string, test.count) + for index := range lines { + lines[index] = "ordinary" + } + lines[len(lines)-1] = "blocked" + + diagnostic := evaluateCELDiffLinePolicy( + t, + "bounded.go", + lines, + `diff.added_lines.exists(line, line.text == "blocked")`, + ) + + if diagnostic.File != "bounded.go" || diagnostic.Line != test.witnessLine { + t.Fatalf( + "diagnostic = %#v, want bounded.go:%d", + diagnostic, + test.witnessLine, + ) + } + _, found := diagnostic.Metadata["diff_change_source"] + if found != (test.witnessLine > 0) { + t.Fatalf("diagnostic witness metadata = %#v", diagnostic.Metadata) + } + }) + } +} + +func evaluateCELDiffLinePolicy( + t *testing.T, + file string, + addedLines []string, + when string, +) diagnostics.Diagnostic { + t.Helper() + + repo := t.TempDir() + runCELGit(t, repo, "init") + runCELGit(t, repo, "config", "user.email", "test@example.com") + runCELGit(t, repo, "config", "user.name", "Test User") + + path := filepath.Join(repo, file) + if err := os.WriteFile(path, nil, 0o600); err != nil { + t.Fatalf("write baseline source: %v", err) + } + runCELGit(t, repo, "add", file) + runCELGit(t, repo, "commit", "-m", "initial") + + content := strings.Join(addedLines, "\n") + "\n" + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("write changed source: %v", err) + } + runCELGit(t, repo, "add", file) + + policyDef := celExpressionPolicy() + policyDef.ID = "feedback.route" + decisions, err := EvaluateCELExpression( + policyDef, + Context{ + Cwd: repo, + Files: []string{file}, + StagedFiles: []string{file}, + Scope: "staged", + EvaluatorOptions: map[string]any{"when": when}, + }, + ) + if err != nil { + t.Fatalf("evaluate CEL diff-line policy: %v", err) + } + if len(decisions) != 1 { + t.Fatalf("decisions = %#v, want one block", decisions) + } + + return decisions[0].Diagnostics[0] +} + func TestEvaluateCELExpressionBlocksAgentBrandingInCommitMessage(t *testing.T) { t.Parallel() diff --git a/go/internal/evaluators/cel_growing_symbol_internal_test.go b/go/internal/evaluators/cel_growing_symbol_internal_test.go new file mode 100644 index 00000000..0d5d7b23 --- /dev/null +++ b/go/internal/evaluators/cel_growing_symbol_internal_test.go @@ -0,0 +1,231 @@ +// SPDX-FileCopyrightText: 2026 Blackcat Informatics Inc. +// SPDX-License-Identifier: AGPL-3.0-only + +package evaluators + +import ( + "testing" + + "blackcat.ca/coding-ethos/go/diagnostics" + "blackcat.ca/coding-ethos/go/internal/celexpr" +) + +func growingProposedSymbol() celexpr.ProposedSymbolChangeInput { + return celexpr.ProposedSymbolChangeInput{ + Action: "modify", + File: "go/internal/example/widget.go", + Language: "go", + LineCountGrows: true, + LineDelta: 12, + NodeKind: "function_declaration", + NonBlankLineDelta: 9, + SymbolKind: "function", + SymbolName: "Widget", + SymbolPath: "example.Widget", + CurrentLineCount: 20, + CurrentNonBlankLineCount: 16, + ProposedLineCount: 32, + ProposedNonBlankLineCount: 25, + ProposedStartLine: 41, + } +} + +func growingChangedSymbol() celexpr.ChangedSymbolInput { + return celexpr.ChangedSymbolInput{ + Action: "modify", + File: "go/internal/example/gadget.go", + Language: "go", + LineCountGrows: true, + LineDelta: 7, + NodeKind: "method_declaration", + NonBlankLineDelta: 5, + SymbolKind: "method", + SymbolName: "Gadget", + SymbolPath: "example.Gadget", + CurrentLineCount: 30, + CurrentNonBlankLineCount: 24, + CurrentStartLine: 77, + OriginalLineCount: 23, + OriginalNonBlankLineCount: 19, + } +} + +func newGrowingSymbolDiagnostic() *diagnostics.Diagnostic { + return &diagnostics.Diagnostic{Metadata: map[string]any{}} +} + +// A proposed change is the edit the agent is about to make, so it must win +// over the already-staged view of the same repository. +func TestApplyGrowingSymbolDiagnosticPrefersProposedOverStaged(t *testing.T) { + t.Parallel() + + diagnostic := newGrowingSymbolDiagnostic() + applyGrowingSymbolDiagnostic(diagnostic, map[string]any{ + "proposed_symbol_changes": proposedSymbolChangeInputs{ + growingProposedSymbol(), + }, + "changed_symbols": []celexpr.ChangedSymbolInput{growingChangedSymbol()}, + }) + + if diagnostic.File != "go/internal/example/widget.go" { + t.Fatalf("diagnostic file = %q", diagnostic.File) + } + if diagnostic.Line != 41 { + t.Fatalf("diagnostic line = %d, want the proposed start line 41", diagnostic.Line) + } + if diagnostic.Metadata["ast_change_source"] != changeSourceProposed { + t.Fatalf("ast_change_source = %v", diagnostic.Metadata["ast_change_source"]) + } + if diagnostic.Metadata["proposed_line_count"] != int64(32) { + t.Fatalf("proposed_line_count = %v", diagnostic.Metadata["proposed_line_count"]) + } + if diagnostic.Metadata["ast_symbol_path"] != "example.Widget" { + t.Fatalf("ast_symbol_path = %v", diagnostic.Metadata["ast_symbol_path"]) + } + if _, reported := diagnostic.Metadata["original_line_count"]; reported { + t.Fatalf("proposed branch reported staged-only metadata: %#v", diagnostic.Metadata) + } +} + +// Pre-commit runs have no proposed edit, so the staged symbol carries the +// diagnostic and reports the original counts instead of proposed ones. +func TestApplyGrowingSymbolDiagnosticFallsBackToStagedSymbol(t *testing.T) { + t.Parallel() + + diagnostic := newGrowingSymbolDiagnostic() + applyGrowingSymbolDiagnostic(diagnostic, map[string]any{ + "changed_symbols": []celexpr.ChangedSymbolInput{growingChangedSymbol()}, + }) + + if diagnostic.File != "go/internal/example/gadget.go" { + t.Fatalf("diagnostic file = %q", diagnostic.File) + } + if diagnostic.Line != 77 { + t.Fatalf("diagnostic line = %d, want the current start line 77", diagnostic.Line) + } + if diagnostic.Metadata["ast_change_source"] != changeSourceStaged { + t.Fatalf("ast_change_source = %v", diagnostic.Metadata["ast_change_source"]) + } + if diagnostic.Metadata["original_nonblank_line_count"] != int64(19) { + t.Fatalf( + "original_nonblank_line_count = %v", + diagnostic.Metadata["original_nonblank_line_count"], + ) + } + if _, reported := diagnostic.Metadata["proposed_line_count"]; reported { + t.Fatalf("staged branch reported proposed-only metadata: %#v", diagnostic.Metadata) + } +} + +// Symbols that shrink or stay the same size must not be annotated at all. +func TestApplyGrowingSymbolDiagnosticIgnoresNonGrowingSymbols(t *testing.T) { + t.Parallel() + + shrinkingProposed := growingProposedSymbol() + shrinkingProposed.LineCountGrows = false + shrinkingChanged := growingChangedSymbol() + shrinkingChanged.LineCountGrows = false + + diagnostic := newGrowingSymbolDiagnostic() + applyGrowingSymbolDiagnostic(diagnostic, map[string]any{ + "proposed_symbol_changes": proposedSymbolChangeInputs{shrinkingProposed}, + "changed_symbols": []celexpr.ChangedSymbolInput{shrinkingChanged}, + }) + + if diagnostic.File != "" || diagnostic.Line != 0 { + t.Fatalf( + "non-growing symbols moved the diagnostic to %s:%d", + diagnostic.File, + diagnostic.Line, + ) + } + if len(diagnostic.Metadata) != 0 { + t.Fatalf("non-growing symbols emitted metadata: %#v", diagnostic.Metadata) + } +} + +// An activation without AST facts must leave the diagnostic untouched rather +// than panicking on a missing or mistyped key. +func TestApplyGrowingSymbolDiagnosticToleratesMissingSymbolFacts(t *testing.T) { + t.Parallel() + + diagnostic := newGrowingSymbolDiagnostic() + applyGrowingSymbolDiagnostic(diagnostic, map[string]any{ + "proposed_symbol_changes": "not-symbol-inputs", + "changed_symbols": 42, + }) + + if diagnostic.File != "" || len(diagnostic.Metadata) != 0 { + t.Fatalf("mistyped activation produced %#v", diagnostic) + } +} + +func TestFirstGrowingProposedSymbolSkipsNonGrowingEntries(t *testing.T) { + t.Parallel() + + stable := growingProposedSymbol() + stable.LineCountGrows = false + stable.SymbolName = "Stable" + + growing := growingProposedSymbol() + + symbol, found := firstGrowingProposedSymbol(map[string]any{ + "proposed_symbol_changes": proposedSymbolChangeInputs{stable, growing}, + }) + if !found { + t.Fatal("growing proposed symbol not found") + } + if symbol.SymbolName != "Widget" { + t.Fatalf("symbol name = %q, want the growing entry", symbol.SymbolName) + } + + _, found = firstGrowingProposedSymbol(map[string]any{ + "proposed_symbol_changes": proposedSymbolChangeInputs{stable}, + }) + if found { + t.Fatal("non-growing proposed symbols reported a growth") + } +} + +func TestFirstGrowingChangedSymbolSkipsNonGrowingEntries(t *testing.T) { + t.Parallel() + + stable := growingChangedSymbol() + stable.LineCountGrows = false + stable.SymbolName = "Stable" + + growing := growingChangedSymbol() + + symbol, found := firstGrowingChangedSymbol(map[string]any{ + "changed_symbols": []celexpr.ChangedSymbolInput{stable, growing}, + }) + if !found { + t.Fatal("growing changed symbol not found") + } + if symbol.SymbolName != "Gadget" { + t.Fatalf("symbol name = %q, want the growing entry", symbol.SymbolName) + } + + _, found = firstGrowingChangedSymbol(map[string]any{"changed_symbols": nil}) + if found { + t.Fatal("absent changed symbols reported a growth") + } +} + +func TestProposedSymbolChangesRequiresTypedInputs(t *testing.T) { + t.Parallel() + + symbols, found := proposedSymbolChanges(map[string]any{ + "proposed_symbol_changes": proposedSymbolChangeInputs{growingProposedSymbol()}, + }) + if !found || len(symbols) != 1 { + t.Fatalf("typed activation returned %d symbols (found=%t)", len(symbols), found) + } + + _, found = proposedSymbolChanges(map[string]any{ + "proposed_symbol_changes": []string{"widget"}, + }) + if found { + t.Fatal("untyped activation value was accepted") + } +} diff --git a/go/internal/evaluators/shell_best_practices.go b/go/internal/evaluators/shell_best_practices.go index 9a53381a..99058d8e 100644 --- a/go/internal/evaluators/shell_best_practices.go +++ b/go/internal/evaluators/shell_best_practices.go @@ -46,39 +46,36 @@ func EvaluateShellBestPractices( return nil, nil } - requireCommon := stringSliceOption( - context.EvaluatorOptions, - "require_common_for_prefixes", - []string{"scripts/"}, - ) + shellFiles := shellFilesFrom(context.Files) + if len(shellFiles) == 0 { + return nil, nil + } - hasCommonHelper, err := repositoryHasTrackedCommonShellHelper( - context.Cwd, - requireCommon, - ) + repositoryRoot, requireCommon, err := resolveCommonShellHelperPrefixes(context) if err != nil { return nil, err } - if !hasCommonHelper { - requireCommon = nil - } + for _, file := range shellFiles { + text, skip, err := readShellText(file) + if err != nil { + return nil, err + } - for _, file := range context.Files { - if !looksLikeShellFile(file) { + if skip { continue } - text, binary, err := readShellText(file) + comparisonPath, err := commonShellHelperComparisonPath(repositoryRoot, file) if err != nil { return nil, err } - if binary { - continue - } - - violations := shellBestPracticeViolations(file, text, requireCommon) + violations := shellBestPracticeViolations( + comparisonPath, + text, + requireCommon, + ) if len(violations) > 0 { return []policy.Decision{ shellBestPracticesDecision(policyDef, file, violations), @@ -89,22 +86,68 @@ func EvaluateShellBestPractices( return nil, nil } -func repositoryHasTrackedCommonShellHelper( - cwd string, - requireCommonForPrefixes []string, -) (bool, error) { - if strings.TrimSpace(cwd) == "" { - return false, errShellHelperWorkingDirectoryRequired +func shellFilesFrom(files []string) []string { + shellFiles := make([]string, 0, len(files)) + for _, file := range files { + if looksLikeShellFile(file) { + shellFiles = append(shellFiles, file) + } + } + + return shellFiles +} + +// resolveCommonShellHelperPrefixes resolves the repository root and the +// directory prefixes that must source a tracked common shell helper. The +// returned prefixes are empty when the repository tracks no such helper. +func resolveCommonShellHelperPrefixes( + context Context, +) (string, []string, error) { + if strings.TrimSpace(context.Cwd) == "" { + return "", nil, errShellHelperWorkingDirectoryRequired + } + + repositoryRoot, err := gitWorktreeRoot(context.Cwd) + if err != nil { + return "", nil, fmt.Errorf( + "resolve repository root for common shell helpers: %w", + err, + ) } + requireCommon := stringSliceOption( + context.EvaluatorOptions, + "require_common_for_prefixes", + []string{"scripts/"}, + ) + helperPaths, err := configuredCommonShellHelperPaths( - cwd, - requireCommonForPrefixes, + repositoryRoot, + requireCommon, ) if err != nil { - return false, err + return "", nil, err } + hasCommonHelper, err := repositoryHasTrackedCommonShellHelper( + repositoryRoot, + helperPaths, + ) + if err != nil { + return "", nil, err + } + + if !hasCommonHelper { + return repositoryRoot, nil, nil + } + + return repositoryRoot, commonShellHelperPrefixes(helperPaths), nil +} + +func repositoryHasTrackedCommonShellHelper( + repositoryRoot string, + helperPaths []string, +) (bool, error) { if len(helperPaths) == 0 { return false, nil } @@ -114,7 +157,7 @@ func repositoryHasTrackedCommonShellHelper( args = append(args, ":(literal)"+helperPath) } - output, err := GitCommand(cwd, args...).CombinedOutput() + output, err := GitCommand(repositoryRoot, args...).CombinedOutput() if err != nil { return false, fmt.Errorf( "inspect tracked common shell helpers with git ls-files: %w: %s", @@ -135,13 +178,61 @@ func repositoryHasTrackedCommonShellHelper( return false, nil } +func commonShellHelperPrefixes(helperPaths []string) []string { + prefixes := make([]string, 0, len(helperPaths)) + for _, helperPath := range helperPaths { + prefix := filepath.ToSlash(filepath.Dir(helperPath)) + if prefix != "." { + prefix += "/" + } + + prefixes = append(prefixes, prefix) + } + + return prefixes +} + +func commonShellHelperComparisonPath(repositoryRoot, path string) (string, error) { + if !filepath.IsAbs(path) { + return filepath.ToSlash(filepath.Clean(path)), nil + } + + normalizedRoot, err := filepath.EvalSymlinks(repositoryRoot) + if err != nil { + return "", fmt.Errorf("normalize common shell helper repository root: %w", err) + } + + normalizedPath, err := filepath.EvalSymlinks(path) + if err != nil { + return "", fmt.Errorf("normalize shell file path %q: %w", path, err) + } + + relativePath, err := filepath.Rel(normalizedRoot, normalizedPath) + if err != nil { + return "", fmt.Errorf( + "resolve shell file path %q from repository root: %w", + path, + err, + ) + } + + return filepath.ToSlash(relativePath), nil +} + func configuredCommonShellHelperPaths( - cwd string, + repositoryRoot string, prefixes []string, ) ([]string, error) { - repositoryRoot, err := filepath.Abs(cwd) + normalizedRepositoryRoot, err := filepath.Abs(repositoryRoot) if err != nil { - return nil, fmt.Errorf("resolve repository working directory: %w", err) + return nil, fmt.Errorf("resolve common shell helper repository root: %w", err) + } + + normalizedRepositoryRoot, err = filepath.EvalSymlinks( + normalizedRepositoryRoot, + ) + if err != nil { + return nil, fmt.Errorf("normalize common shell helper repository root: %w", err) } helperPaths := make([]string, 0, len(prefixes)) @@ -153,7 +244,16 @@ func configuredCommonShellHelperPaths( cleaned := filepath.Clean(trimmed) if filepath.IsAbs(cleaned) { - cleaned, err = filepath.Rel(repositoryRoot, cleaned) + cleaned, err = filepath.EvalSymlinks(cleaned) + if err != nil { + return nil, fmt.Errorf( + "normalize common shell helper prefix %q: %w", + prefix, + err, + ) + } + + cleaned, err = filepath.Rel(normalizedRepositoryRoot, cleaned) if err != nil { return nil, fmt.Errorf( "resolve common shell helper prefix %q: %w", @@ -193,11 +293,15 @@ func looksLikeShellFile(path string) bool { } } +// readShellText returns the file's text and a skip flag. Skip is true when the +// file must not be evaluated at all: a binary file, or a path deleted by the +// edit under review. A deleted path must be skipped rather than reported as an +// empty file that violates every shell convention. func readShellText(path string) (string, bool, error) { data, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) { - return "", false, nil + return "", true, nil } return "", false, fmt.Errorf("read shell file %s: %w", path, err) diff --git a/go/internal/evaluators/shell_best_practices_test.go b/go/internal/evaluators/shell_best_practices_test.go index 10368c21..6462291a 100644 --- a/go/internal/evaluators/shell_best_practices_test.go +++ b/go/internal/evaluators/shell_best_practices_test.go @@ -113,6 +113,29 @@ func TestEvaluateShellBestPracticesRejectsMissingRepositoryContext(t *testing.T) } } +func TestEvaluateShellBestPracticesSkipsRepositoryInspectionForNonShellFiles( + t *testing.T, +) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "main.go") + if err := os.WriteFile(path, []byte("package main\n"), 0o600); err != nil { + t.Fatalf("write non-shell source: %v", err) + } + + decisions, err := EvaluateShellBestPractices( + shellBestPracticesPolicy(), + Context{Cwd: dir, Files: []string{path}}, + ) + if err != nil { + t.Fatalf("non-shell source triggered repository inspection: %v", err) + } + if len(decisions) != 0 { + t.Fatalf("non-shell source decisions = %#v", decisions) + } +} + func TestEvaluateShellBestPracticesPropagatesGitInspectionFailure(t *testing.T) { t.Parallel() @@ -150,7 +173,7 @@ func TestEvaluateShellBestPracticesPropagatesGitInspectionFailure(t *testing.T) if err == nil { t.Fatal("git inspection failure must fail helper validation") } - if !strings.Contains(err.Error(), "git ls-files") { + if !strings.Contains(err.Error(), "git worktree root") { t.Fatalf("git inspection error = %q", err) } } @@ -250,6 +273,205 @@ func TestEvaluateShellBestPracticesRequiresOnlyExistingCommonHelper(t *testing.T } } +// Post-edit lint passes the absolute paths of every changed file, including +// files the edit deleted. A deleted path must be skipped outright: it must not +// fail path normalization, and it must not be reported as an empty file that +// violates every shell convention. +func TestEvaluateShellBestPracticesSkipsDeletedAbsoluteShellPaths(t *testing.T) { + t.Parallel() + + repo := t.TempDir() + initializeStagedAdminGitRepo(t, repo) + scriptsDir := filepath.Join(repo, "scripts") + if err := os.MkdirAll(scriptsDir, 0o700); err != nil { + t.Fatalf("create scripts directory: %v", err) + } + + content := []byte("#!/usr/bin/env bash\nset -euo pipefail\necho ok\n") + commonPath := filepath.Join(scriptsDir, "common.sh") + if err := os.WriteFile(commonPath, content, 0o600); err != nil { + t.Fatalf("write common helper: %v", err) + } + runGit(t, repo, "add", "scripts/common.sh") + + deletedPath := filepath.Join(scriptsDir, "removed.sh") + + decisions, err := EvaluateShellBestPractices( + shellBestPracticesPolicy(), + Context{ + Cwd: repo, + Files: []string{deletedPath}, + EvaluatorOptions: map[string]any{ + "require_common_for_prefixes": []any{ + scriptsDir + string(filepath.Separator), + }, + }, + }, + ) + if err != nil { + t.Fatalf("deleted shell path failed evaluation: %v", err) + } + if len(decisions) != 0 { + t.Fatalf("deleted shell path produced decisions: %#v", decisions) + } +} + +func TestEvaluateShellBestPracticesResolvesCommonHelperFromNestedCWD( + t *testing.T, +) { + t.Parallel() + + repo := t.TempDir() + initializeStagedAdminGitRepo(t, repo) + scriptsDir := filepath.Join(repo, "scripts") + nestedDir := filepath.Join(repo, "work", "nested") + for _, directory := range []string{scriptsDir, nestedDir} { + if err := os.MkdirAll(directory, 0o700); err != nil { + t.Fatalf("create fixture directory %s: %v", directory, err) + } + } + + content := []byte("#!/usr/bin/env bash\nset -euo pipefail\necho ok\n") + commonPath := filepath.Join(scriptsDir, "common.sh") + if err := os.WriteFile(commonPath, content, 0o600); err != nil { + t.Fatalf("write common helper: %v", err) + } + runGit(t, repo, "add", "scripts/common.sh") + + scriptPath := filepath.Join(scriptsDir, "work.sh") + if err := os.WriteFile(scriptPath, content, 0o600); err != nil { + t.Fatalf("write script: %v", err) + } + + for _, test := range []struct { + name string + options map[string]any + }{ + { + name: "absolute prefix", + options: map[string]any{ + "require_common_for_prefixes": []any{ + scriptsDir + string(filepath.Separator), + }, + }, + }, + {name: "default relative prefix"}, + } { + t.Run(test.name, func(t *testing.T) { + decisions, err := EvaluateShellBestPractices( + shellBestPracticesPolicy(), + Context{ + Cwd: nestedDir, + Files: []string{scriptPath}, + EvaluatorOptions: test.options, + }, + ) + if err != nil { + t.Fatalf( + "evaluate common helper from nested working directory: %v", + err, + ) + } + if len(decisions) != 1 { + t.Fatalf("nested working directory decisions = %#v", decisions) + } + if got := decisions[0].Diagnostics[0].Message; got != + "scripts/ shell files must source the repository common shell helpers" { + t.Fatalf("diagnostic message = %q", got) + } + }) + } +} + +func TestEvaluateShellBestPracticesNormalizesSymlinkedAbsolutePrefix( + t *testing.T, +) { + t.Parallel() + + fixtureRoot := t.TempDir() + repo := filepath.Join(fixtureRoot, "repo") + if err := os.Mkdir(repo, 0o700); err != nil { + t.Fatalf("create repository: %v", err) + } + initializeStagedAdminGitRepo(t, repo) + + scriptsDir := filepath.Join(repo, "scripts") + nestedDir := filepath.Join(repo, "work", "nested") + for _, directory := range []string{scriptsDir, nestedDir} { + if err := os.MkdirAll(directory, 0o700); err != nil { + t.Fatalf("create fixture directory %s: %v", directory, err) + } + } + + content := []byte("#!/usr/bin/env bash\nset -euo pipefail\necho ok\n") + commonPath := filepath.Join(scriptsDir, "common.sh") + if err := os.WriteFile(commonPath, content, 0o600); err != nil { + t.Fatalf("write common helper: %v", err) + } + runGit(t, repo, "add", "scripts/common.sh") + + scriptPath := filepath.Join(scriptsDir, "work.sh") + if err := os.WriteFile(scriptPath, content, 0o600); err != nil { + t.Fatalf("write script: %v", err) + } + + repoAlias := filepath.Join(fixtureRoot, "repo-alias") + if err := os.Symlink(repo, repoAlias); err != nil { + t.Skipf("create repository symlink: %v", err) + } + + decisions, err := EvaluateShellBestPractices( + shellBestPracticesPolicy(), + Context{ + Cwd: filepath.Join(repoAlias, "work", "nested"), + Files: []string{scriptPath}, + EvaluatorOptions: map[string]any{ + "require_common_for_prefixes": []any{ + filepath.Join(repoAlias, "scripts") + string(filepath.Separator), + }, + }, + }, + ) + if err != nil { + t.Fatalf("evaluate symlinked common helper prefix: %v", err) + } + if len(decisions) != 1 { + t.Fatalf("symlinked absolute prefix decisions = %#v", decisions) + } +} + +func TestEvaluateShellBestPracticesRejectsAbsolutePrefixOutsideRepository( + t *testing.T, +) { + t.Parallel() + + repo := t.TempDir() + initializeStagedAdminGitRepo(t, repo) + scriptPath := filepath.Join(repo, "work.sh") + content := []byte("#!/usr/bin/env bash\nset -euo pipefail\necho ok\n") + if err := os.WriteFile(scriptPath, content, 0o600); err != nil { + t.Fatalf("write script: %v", err) + } + + outsidePrefix := t.TempDir() + string(filepath.Separator) + _, err := EvaluateShellBestPractices( + shellBestPracticesPolicy(), + Context{ + Cwd: repo, + Files: []string{scriptPath}, + EvaluatorOptions: map[string]any{ + "require_common_for_prefixes": []any{outsidePrefix}, + }, + }, + ) + if err == nil { + t.Fatal("absolute prefix outside repository must fail validation") + } + if !strings.Contains(err.Error(), "outside the repository") { + t.Fatalf("outside repository error = %q", err) + } +} + func shellBestPracticesPolicy() policy.Policy { return policy.Policy{ ID: "shell.best_practices", diff --git a/go/internal/githookcli/main.go b/go/internal/githookcli/main.go index 5892f27b..e2d12778 100644 --- a/go/internal/githookcli/main.go +++ b/go/internal/githookcli/main.go @@ -4,6 +4,7 @@ package githookcli import ( + "bytes" "context" "flag" "fmt" @@ -17,6 +18,7 @@ import ( "blackcat.ca/coding-ethos/go/diagnostics" "blackcat.ca/coding-ethos/go/internal/apperror" + "blackcat.ca/coding-ethos/go/internal/codeintel" "blackcat.ca/coding-ethos/go/internal/evaluators" "blackcat.ca/coding-ethos/go/internal/feedback" "blackcat.ca/coding-ethos/go/internal/generatedtrust" @@ -42,6 +44,8 @@ const ( hookMessageFileIndex = 1 hookSourceIndex = 2 hookCommitIndex = 3 + gitDiffArgsCapacity = 4 + gitDiffNULCapacity = 5 ) var ( @@ -484,6 +488,15 @@ func runStagedHook(bundle policy.Bundle, cwd, hookName string) (int, bool) { code := policyResultExitCode(cwd, result) + codeIntelFiles, codeIntelErr := hookCodeIntelFiles(cwd, hookName) + if codeIntelErr != nil { + writeGitHookText( + "warning: staged code-intel paths not resolved: " + codeIntelErr.Error(), + ) + } else { + refreshGitHookCodeIntel(cwd, codeIntelFiles) + } + return code, code != 0 } @@ -537,13 +550,29 @@ func hookFiles(cwd, hookName string) ([]string, error) { return nil, nil } + return gitDiffFileNames(cwd, "--diff-filter=ACMR") +} + +func hookCodeIntelFiles(cwd, hookName string) ([]string, error) { + if hookName != hookPreCommit { + return nil, nil + } + + // Disable rename detection so the old path is reported as deleted and the + // new path as added. Incremental code-intel can then tombstone and index the + // two sides without a whole-repository scan. + return gitDiffFileNamesNUL(cwd, "--no-renames", "--diff-filter=ACDMR") +} + +func gitDiffFileNames(cwd string, options ...string) ([]string, error) { + args := make([]string, 0, gitDiffArgsCapacity+len(options)) + args = append(args, "diff", "--cached", "--name-only") + args = append(args, options...) + args = append(args, "--") + command := evaluators.GitCommand( cwd, - "diff", - "--cached", - "--name-only", - "--diff-filter=ACMR", - "--", + args..., ) output, err := command.Output() @@ -563,6 +592,48 @@ func hookFiles(cwd, hookName string) ([]string, error) { return files, nil } +func gitDiffFileNamesNUL(cwd string, options ...string) ([]string, error) { + args := make([]string, 0, gitDiffNULCapacity+len(options)) + args = append(args, "diff", "--cached", "--name-only", "-z") + args = append(args, options...) + args = append(args, "--") + + output, err := evaluators.GitCommand(cwd, args...).Output() + if err != nil { + return nil, fmt.Errorf("list staged code-intel files: %w", err) + } + + files := []string{} + seen := map[string]bool{} + + for rawPath := range bytes.SplitSeq(output, []byte{0}) { + if len(rawPath) == 0 { + continue + } + + path := string(rawPath) + if !seen[path] { + seen[path] = true + files = append(files, path) + } + } + + return files, nil +} + +func refreshGitHookCodeIntel(cwd string, files []string) { + if len(files) == 0 { + return + } + + _, err := codeintel.RefreshLintFiles(context.Background(), cwd, files) + if err != nil { + writeGitHookText( + "warning: staged files not incrementally indexed: " + err.Error(), + ) + } +} + func readBundle(path string) (policy.Bundle, error) { file, err := os.Open(path) if err != nil { @@ -598,6 +669,13 @@ func logLintResult(cwd string, result lint.Result) { writeGitHookText("warning: lint SARIF sidecar not written: " + inlineErrB.Error()) } + inlineErrC := codeintel.IngestLintTraceFile(context.Background(), cwd, tracePath) + if inlineErrC != nil { + writeGitHookText( + "warning: lint trace not ingested into code-intel: " + inlineErrC.Error(), + ) + } + err := outputsurface.AutoPruneSurface( context.Background(), cwd, diff --git a/go/internal/githookcli/main_internal_test.go b/go/internal/githookcli/main_internal_test.go index d2ebc16c..1507f46a 100644 --- a/go/internal/githookcli/main_internal_test.go +++ b/go/internal/githookcli/main_internal_test.go @@ -71,6 +71,36 @@ func TestHookFilesReturnsStagedPreCommitFiles(t *testing.T) { } } +func TestHookCodeIntelFilesIncludesDeletionAndBothRenameSides(t *testing.T) { + t.Parallel() + + repo := t.TempDir() + runTestGit(t, repo, "init") + runTestGit(t, repo, "config", "user.email", "test@example.com") + runTestGit(t, repo, "config", "user.name", "Test User") + runTestGit(t, repo, "config", "commit.gpgsign", "false") + writeTestGitHookFile(t, repo, "old.py", "VALUE = 1\n") + writeTestGitHookFile(t, repo, "removed.py", "REMOVED = 1\n") + writeTestGitHookFile(t, repo, "line\nbreak.py", "ODD = 1\n") + runTestGit(t, repo, "add", "old.py", "removed.py", "line\nbreak.py") + runTestGit(t, repo, "commit", "-m", "initial") + runTestGit(t, repo, "mv", "old.py", "new.py") + runTestGit(t, repo, "rm", "removed.py") + writeTestGitHookFile(t, repo, "line\nbreak.py", "ODD = 2\n") + runTestGit(t, repo, "add", "line\nbreak.py") + + files, err := hookCodeIntelFiles(repo, hookPreCommit) + if err != nil { + t.Fatalf("code-intel hook files: %v", err) + } + + for _, want := range []string{"line\nbreak.py", "new.py", "old.py", "removed.py"} { + if !slices.Contains(files, want) { + t.Fatalf("code-intel files = %#v, missing %q", files, want) + } + } +} + func TestDirectRealGitCommitAllowsHumanGitThroughInstalledHooks(t *testing.T) { clearAgentGitHookEnv(t) @@ -155,6 +185,14 @@ func TestDirectAgentGitCommitBlocksThroughInstalledHooks(t *testing.T) { t.Fatalf("direct Codex git output missing %q:\n%s", want, output) } } + + if _, statErr := os.Stat( + filepath.Join(repo.root, ".coding-ethos", "code-intel.duckdb"), + ); !os.IsNotExist( + statErr, + ) { + t.Fatalf("direct wrapper denial created code-intel DB: %v", statErr) + } } func TestGitHookCommitAmendInAncestryHandlesIncompleteVerifier(t *testing.T) { diff --git a/go/internal/hooklog/runner.go b/go/internal/hooklog/runner.go index 5b6f70fb..b21d31c7 100644 --- a/go/internal/hooklog/runner.go +++ b/go/internal/hooklog/runner.go @@ -167,7 +167,7 @@ func runWithStatus(options Options) (int, error) { return 1, metadataErr } - maintenanceErr := finishHookMaintenance(options, runDir, status) + maintenanceErr := finishHookMaintenance(options, runDir) return completedHookStatus(status, err, maintenanceErr) } @@ -199,8 +199,8 @@ func completedHookStatus(status int, runErr, maintenanceErr error) (int, error) return status, nil } -func finishHookMaintenance(options Options, runDir string, status int) error { - err := refreshCodeIntelAfterRun(options, runDir, status) +func finishHookMaintenance(options Options, runDir string) error { + err := ingestHookTraceAfterRun(options, runDir) if err != nil { return err } @@ -214,20 +214,7 @@ func finishHookMaintenance(options Options, runDir string, status int) error { return nil } -func refreshCodeIntelAfterRun(options Options, runDir string, status int) error { - if shouldRefreshRepository(options.Command, status) { - _, err := codeintel.RefreshRepository( - context.Background(), - options.Root, - []string{"."}, - ) - if err != nil { - return fmt.Errorf("refresh code-intel after hook run: %w", err) - } - - return nil - } - +func ingestHookTraceAfterRun(options Options, runDir string) error { tracePath := filepath.Join(runDir, "event.json") _, err := os.Stat(tracePath) @@ -247,10 +234,6 @@ func refreshCodeIntelAfterRun(options Options, runDir string, status int) error return nil } -func shouldRefreshRepository(command []string, status int) bool { - return status == 0 && shouldForceCodeIntelRefresh(command) -} - func autoPruneHookRuns(root string) error { err := outputsurface.AutoPruneSurface(context.Background(), root, "hook_runs", false) if err != nil { @@ -260,39 +243,6 @@ func autoPruneHookRuns(root string) error { return nil } -func shouldForceCodeIntelRefresh(command []string) bool { - // Hooklog owns refreshes only for policy-lint, Git hooks, and the selected - // Make targets below. Parent-install, parent-check, and parent-lint do not - // refresh code intelligence and must not be described as doing so here. - return commandContains(command, "policy-lint") || - commandContainsSequence(command, "git-hook", "pre-commit") || - commandContainsSequence(command, "git-hook", "pre-push") || - commandContainsSequence(command, "make", "pre-commit") || - commandContainsSequence(command, "make", "pre-commit-all") || - commandContainsSequence(command, "make", "check") || - commandContainsSequence(command, "make", "lint") -} - -func commandContains(command []string, value string) bool { - for _, part := range command { - if filepath.Base(part) == value || part == value { - return true - } - } - - return false -} - -func commandContainsSequence(command []string, first, second string) bool { - for index := 0; index+1 < len(command); index++ { - if filepath.Base(command[index]) == first && command[index+1] == second { - return true - } - } - - return false -} - func runLoggedPayload( options Options, logs hookRunLogs, diff --git a/go/internal/hooklog/runner_internal_test.go b/go/internal/hooklog/runner_internal_test.go deleted file mode 100644 index 693eb238..00000000 --- a/go/internal/hooklog/runner_internal_test.go +++ /dev/null @@ -1,51 +0,0 @@ -// SPDX-FileCopyrightText: 2026 Blackcat Informatics Inc. -// SPDX-License-Identifier: AGPL-3.0-only - -package hooklog - -import "testing" - -func TestShouldForceCodeIntelRefreshSkipsManagedToolRuns(t *testing.T) { - t.Parallel() - - for _, command := range [][]string{ - {"coding-ethos-run", "policy-tool", "go-test", "go"}, - {"coding-ethos-run", "policy-tool", "ruff", "check", "pkg"}, - {"coding-ethos-run", "policy-tool-group", "type_check"}, - {"coding-ethos-run", "parent-install", "--repo", "/repo"}, - {"coding-ethos-run", "parent-check", "--repo", "/repo"}, - {"coding-ethos-run", "parent-lint", "--repo", "/repo"}, - } { - if shouldForceCodeIntelRefresh(command) { - t.Fatalf("shouldForceCodeIntelRefresh(%#v) = true, want false", command) - } - } -} - -func TestShouldForceCodeIntelRefreshKeepsHookAndRepoGates(t *testing.T) { - t.Parallel() - - for _, command := range [][]string{ - {"coding-ethos-run", "policy-lint"}, - {"coding-ethos-run", "git-hook", "pre-commit"}, - {"coding-ethos-run", "git-hook", "pre-push"}, - {"make", "check"}, - {"make", "pre-commit"}, - } { - if !shouldForceCodeIntelRefresh(command) { - t.Fatalf("shouldForceCodeIntelRefresh(%#v) = false, want true", command) - } - } -} - -func TestShouldRefreshRepositorySkipsFailedHook(t *testing.T) { - t.Parallel() - - command := []string{"coding-ethos-run", "git-hook", "pre-commit"} - if !shouldRefreshRepository(command, 0) { - t.Fatal("successful pre-commit should refresh repository code-intel") - } - if shouldRefreshRepository(command, 2) { - t.Fatal("blocked pre-commit must not perform a full repository refresh") - } -} diff --git a/go/internal/hooklog/runner_test.go b/go/internal/hooklog/runner_test.go index 713bd9bb..0de01e02 100644 --- a/go/internal/hooklog/runner_test.go +++ b/go/internal/hooklog/runner_test.go @@ -5,8 +5,8 @@ package hooklog_test import ( "bytes" - "context" "errors" + "fmt" "os" "path/filepath" "strings" @@ -15,7 +15,6 @@ import ( "blackcat.ca/coding-ethos/go/internal/codeintel" . "blackcat.ca/coding-ethos/go/internal/hooklog" - "blackcat.ca/coding-ethos/go/internal/realgit" "blackcat.ca/coding-ethos/go/internal/testlock" ) @@ -181,14 +180,10 @@ func TestRunChecksIgnoresWithoutIndex(t *testing.T) { root := t.TempDir() writeHookLogIgnore(t, root) logPath := filepath.Join(t.TempDir(), "git-args.log") - t.Setenv("GIT_TRACE", logPath) - - git, err := realgit.Resolve(context.Background(), "git") - if err != nil { - t.Fatalf("resolve real git: %v", err) - } + t.Setenv("CODE_ETHOS_HOOKLOG_TEST_GIT_ARGS", logPath) + git := fakeGit(t) - err = Run(Options{ + err := Run(Options{ Stdin: strings.NewReader(""), Stdout: &bytes.Buffer{}, Stderr: &bytes.Buffer{}, @@ -204,7 +199,7 @@ func TestRunChecksIgnoresWithoutIndex(t *testing.T) { t.Fatalf("run hook log: %v", err) } - assertFileContains(t, logPath, "check-ignore --no-index --quiet") + assertFileContains(t, logPath, "check-ignore\n--no-index\n--quiet\n") } func TestRunIngestsHookTraceIntoCodeIntel(t *testing.T) { @@ -271,60 +266,94 @@ func TestRunIngestsHookTraceIntoCodeIntel(t *testing.T) { } } -func TestRunForcesCodeIntelRefreshForPreCommit(t *testing.T) { - t.Parallel() +func TestRunIngestsPreCommitTraceWithoutRepositoryRefresh(t *testing.T) { testlock.ProcessState(t, "hooklog-global-streams") - root := t.TempDir() - writeHookLogIgnore(t, root) - - err := os.WriteFile( - filepath.Join(root, "app.js"), - []byte("export function run() { return 1; }\n"), - 0o600, - ) - if err != nil { - t.Fatalf("write source: %v", err) - } - - status, err := RunInProcess(Options{ - Stdin: strings.NewReader(""), - Stdout: &bytes.Buffer{}, - Stderr: &bytes.Buffer{}, - GitPath: fakeGit(t), - Root: root, - BundleRoot: filepath.Join(root, "pre-commit"), - Command: []string{"coding-ethos-run", "git-hook", "pre-commit"}, - Now: func() time.Time { - return time.Date(2026, 5, 1, 12, 34, 56, 0, time.UTC) - }, - }, func() int { - return 0 - }) - if err != nil { - t.Fatalf("RunInProcess returned error: %v", err) - } + for _, testCase := range []struct { + name string + status int + }{ + {name: "allowed", status: 0}, + {name: "blocked", status: 2}, + } { + t.Run(testCase.name, func(t *testing.T) { + root := t.TempDir() + writeHookLogIgnore(t, root) + + err := os.WriteFile( + filepath.Join(root, "app.js"), + []byte("export function run() { return 1; }\n"), + 0o600, + ) + if err != nil { + t.Fatalf("write source: %v", err) + } - if status != 0 { - t.Fatalf("status = %d, want 0", status) - } + status, err := RunInProcess(Options{ + Stdin: strings.NewReader(""), + Stdout: &bytes.Buffer{}, + Stderr: &bytes.Buffer{}, + GitPath: fakeGit(t), + Root: root, + BundleRoot: filepath.Join(root, "pre-commit"), + Command: []string{"coding-ethos-run", "git-hook", "pre-commit"}, + Now: func() time.Time { + return time.Date(2026, 5, 1, 12, 34, 56, 0, time.UTC) + }, + }, func() int { + tracePath := filepath.Join( + os.Getenv("CODE_ETHOS_HOOK_RUN_DIR"), + "event.json", + ) + payload := fmt.Sprintf(`{ + "schema_version": 1, + "trace_id": "hook-trace-pre-commit", + "recorded_at_utc": "2026-05-01T12:34:56Z", + "provider": "codex", + "event": "PreToolUse", + "tool": "Bash", + "status": %q +}`, testCase.name) + + // #nosec G703 -- the hook runner creates this path under t.TempDir. + if writeErr := os.WriteFile(tracePath, []byte(payload), 0o600); writeErr != nil { + t.Fatalf("write hook trace: %v", writeErr) + } + + return testCase.status + }) + if err != nil { + t.Fatalf("RunInProcess returned error: %v", err) + } + if status != testCase.status { + t.Fatalf("status = %d, want %d", status, testCase.status) + } - store, err := codeintel.Open( - t.Context(), - filepath.Join(root, ".coding-ethos", "code-intel.duckdb"), - ) - if err != nil { - t.Fatalf("open code-intel store: %v", err) - } - defer store.Close() + store, err := codeintel.Open( + t.Context(), + filepath.Join(root, ".coding-ethos", "code-intel.duckdb"), + ) + if err != nil { + t.Fatalf("open code-intel store: %v", err) + } + defer store.Close() - file, found, err := store.GetCodeFile(t.Context(), "app.js") - if err != nil { - t.Fatalf("query indexed file: %v", err) - } + stats, err := store.Stats(t.Context()) + if err != nil { + t.Fatalf("query code-intel stats: %v", err) + } + if stats.Traces != 1 || stats.HookEvents != 1 { + t.Fatalf("stats = %#v, want one hook trace", stats) + } - if !found || file.Language != "javascript" || file.SourceModTimeUTC == "" { - t.Fatalf("indexed file = %#v, found=%v", file, found) + file, found, err := store.GetCodeFile(t.Context(), "app.js") + if err != nil { + t.Fatalf("query indexed file: %v", err) + } + if found { + t.Fatalf("routine hook unexpectedly indexed source file: %#v", file) + } + }) } } @@ -566,6 +595,10 @@ func fakeFailingGit(t *testing.T) string { func fakeGitCheckIgnoreScript() string { return `#!/usr/bin/env bash +if [ -n "${CODE_ETHOS_HOOKLOG_TEST_GIT_ARGS:-}" ]; then + printf '%s\n' "$@" > "$CODE_ETHOS_HOOKLOG_TEST_GIT_ARGS" +fi + root="" target="" while [ "$#" -gt 0 ]; do diff --git a/go/internal/hookrunnercli/external_tool.go b/go/internal/hookrunnercli/external_tool.go index 01c3ebad..ff49cbbf 100644 --- a/go/internal/hookrunnercli/external_tool.go +++ b/go/internal/hookrunnercli/external_tool.go @@ -185,7 +185,7 @@ func externalToolEnv(request externalToolRequest) ([]string, error) { env := make([]string, 0, len(os.Environ())+len(request.Env)) hasPath := false - root := repoRoot() + root := externalToolStateRoot() cacheEnv, err := externalToolCacheEnv(root) if err != nil { @@ -232,6 +232,14 @@ func externalToolEnv(request externalToolRequest) ([]string, error) { return append(env, request.Env...), nil } +func externalToolStateRoot() string { + if root := strings.TrimSpace(os.Getenv(stateRootEnv)); root != "" { + return filepath.Clean(root) + } + + return repoRoot() +} + type externalToolCacheEnvironment struct { GoTemp string GoCache string diff --git a/go/internal/hookrunnercli/external_tool_internal_test.go b/go/internal/hookrunnercli/external_tool_internal_test.go index 636b595d..473f050e 100644 --- a/go/internal/hookrunnercli/external_tool_internal_test.go +++ b/go/internal/hookrunnercli/external_tool_internal_test.go @@ -115,6 +115,7 @@ func TestExternalToolEnvRemovesGitHookLocalEnvironment(t *testing.T) { t.Setenv("CODING_ETHOS_AGENT_SHELL_SANDBOX", "1") t.Setenv("CODING_ETHOS_SANDBOX_ACTIVE", "1") t.Setenv("CODING_ETHOS_SANDBOX_ROOT", repo) + t.Setenv(stateRootEnv, repo) env, err := externalToolEnv(externalToolRequest{ Dir: repo, @@ -241,6 +242,7 @@ func TestExternalToolEnvIsolatesActiveUVProjects(t *testing.T) { t.Chdir(repo) t.Setenv(consumerRootEnv, repo) + t.Setenv(stateRootEnv, repo) firstEnv, err := externalToolEnv(externalToolRequest{ Dir: firstProject, @@ -289,6 +291,49 @@ func TestExternalToolEnvIsolatesActiveUVProjects(t *testing.T) { } } +func TestExternalToolEnvUsesStateRootOutsideConsumerCheckout(t *testing.T) { + consumerRoot := t.TempDir() + stateRoot := t.TempDir() + hookRoot := t.TempDir() + hookProject := filepath.Join(hookRoot, "pre-commit", "hooks") + mustWriteTestFile( + t, + filepath.Join(hookProject, "pyproject.toml"), + "[project]\nname = 'fixture'\n", + ) + + t.Chdir(hookRoot) + t.Setenv(consumerRootEnv, consumerRoot) + t.Setenv(stateRootEnv, stateRoot+string(os.PathSeparator)+".") + + env, err := externalToolEnv(externalToolRequest{ + Dir: hookProject, + Command: []string{ + "uv", "run", "--project", hookProject, "pyupgrade", + }, + }) + if err != nil { + t.Fatalf("externalToolEnv: %v", err) + } + + projectEnv := externalToolTestRequiredEnvValue( + t, + env, + "UV_PROJECT_ENVIRONMENT", + ) + wantRoot := filepath.Join(stateRoot, ".coding-ethos", "cache", "uv-project-env") + if !strings.HasPrefix(projectEnv, wantRoot+string(os.PathSeparator)) { + t.Fatalf("UV project environment %q is outside state root %q", projectEnv, wantRoot) + } + if strings.HasPrefix(projectEnv, hookProject+string(os.PathSeparator)) { + t.Fatalf( + "UV project environment %q is inside hook project %q", + projectEnv, + hookProject, + ) + } +} + func TestExternalToolEnvFreezesOnlySealedOrExplicitUVProjects(t *testing.T) { repo := t.TempDir() sealedProject := filepath.Join(repo, "sealed") @@ -330,6 +375,7 @@ func TestExternalToolEnvFreezesOnlySealedOrExplicitUVProjects(t *testing.T) { t.Chdir(repo) t.Setenv(consumerRootEnv, repo) + t.Setenv(stateRootEnv, repo) sealedEnv, err := externalToolEnv(externalToolRequest{ Dir: sealedProject, diff --git a/go/internal/hookrunnercli/git_hook.go b/go/internal/hookrunnercli/git_hook.go index a2290828..3004fd8b 100644 --- a/go/internal/hookrunnercli/git_hook.go +++ b/go/internal/hookrunnercli/git_hook.go @@ -5,6 +5,8 @@ package hookrunnercli import ( "bufio" + "bytes" + "context" "encoding/json" "fmt" "io" @@ -15,6 +17,7 @@ import ( "sync" "time" + "blackcat.ca/coding-ethos/go/internal/codeintel" "blackcat.ca/coding-ethos/go/internal/evaluators" ) @@ -116,13 +119,29 @@ func runPrePushHook(cfg Config, input io.Reader, remoteName string) int { defer restoreRoot() - files, err := pushedFiles(input, remoteName) + var copiedInput bytes.Buffer + + files, err := pushedFiles(io.TeeReader(input, &copiedInput), remoteName) if err != nil { writef(os.Stderr, "FATAL: %v\n", err) return 1 } + codeIntelFiles, codeIntelErr := pushedCodeIntelFiles( + bytes.NewReader(copiedInput.Bytes()), + remoteName, + ) + if codeIntelErr != nil { + writef( + os.Stderr, + "warning: pushed code-intel paths not resolved: %v\n", + codeIntelErr, + ) + } else { + defer refreshPrePushCodeIntel(localRepoRoot(), codeIntelFiles) + } + exit := runNamedHookGroups(cfg, []string{ "python-policy", "python-quality", @@ -510,6 +529,24 @@ func hookFilesForPreCommit(allFiles bool) ([]string, error) { } func pushedFiles(input io.Reader, remoteName string) ([]string, error) { + return pushedFilesWithDiffOptions(input, remoteName, nil, false) +} + +func pushedCodeIntelFiles(input io.Reader, remoteName string) ([]string, error) { + return pushedFilesWithDiffOptions( + input, + remoteName, + []string{"--no-renames", "--diff-filter=ACDMR"}, + true, + ) +} + +func pushedFilesWithDiffOptions( + input io.Reader, + remoteName string, + diffOptions []string, + nulTerminated bool, +) ([]string, error) { scanner := bufio.NewScanner(input) seen := map[string]bool{} files := []string{} @@ -527,18 +564,18 @@ func pushedFiles(input io.Reader, remoteName string) ([]string, error) { if fields[3] == allZeroSHA { base := newBranchDiffBase(remoteName, fields[1]) - changed, err = gitLinesInRoot( + changed, err = gitDiffPathNamesInRoot( localRepoRoot(), - "diff", - "--name-only", + diffOptions, + nulTerminated, base, fields[1], ) } else { - changed, err = gitLinesInRoot( + changed, err = gitDiffPathNamesInRoot( localRepoRoot(), - "diff", - "--name-only", + diffOptions, + nulTerminated, fields[3]+".."+fields[1], ) } @@ -564,6 +601,13 @@ func pushedFiles(input io.Reader, remoteName string) ([]string, error) { return files, nil } + return pushedFilesFromLocalHistory(diffOptions, nulTerminated) +} + +func pushedFilesFromLocalHistory( + diffOptions []string, + nulTerminated bool, +) ([]string, error) { upstream := gitOutputInRoot( localRepoRoot(), "rev-parse", @@ -572,10 +616,10 @@ func pushedFiles(input io.Reader, remoteName string) ([]string, error) { "@{upstream}", ) if upstream != "" { - changed, err := gitLinesInRoot( + changed, err := gitDiffPathNamesInRoot( localRepoRoot(), - "diff", - "--name-only", + diffOptions, + nulTerminated, upstream+"...HEAD", ) if err == nil { @@ -583,7 +627,73 @@ func pushedFiles(input io.Reader, remoteName string) ([]string, error) { } } - return gitLinesInRoot(localRepoRoot(), "diff", "--name-only", "HEAD") + return gitDiffPathNamesInRoot( + localRepoRoot(), + diffOptions, + nulTerminated, + "HEAD", + ) +} + +func gitDiffPathNamesInRoot( + root string, + diffOptions []string, + nulTerminated bool, + revisions ...string, +) ([]string, error) { + args := []string{"diff", "--name-only"} + if nulTerminated { + args = append(args, "-z") + } + + args = append(args, diffOptions...) + args = append(args, revisions...) + + if !nulTerminated { + return gitLinesInRoot(root, args...) + } + + output, err := combinedGitOutputInRoot(root, args...) + if err != nil { + return nil, fmt.Errorf( + "git %s: %w: %s", + strings.Join(args, " "), + err, + strings.TrimSpace(string(output)), + ) + } + + files := []string{} + seen := map[string]bool{} + + for rawPath := range bytes.SplitSeq(output, []byte{0}) { + if len(rawPath) == 0 { + continue + } + + path := string(rawPath) + if !seen[path] { + seen[path] = true + files = append(files, path) + } + } + + return files, nil +} + +func refreshPrePushCodeIntel(root string, files []string) { + if len(files) == 0 { + return + } + + _, err := codeintel.RefreshLintFiles(context.Background(), root, files) + if err != nil { + writef( + os.Stderr, + "warning: pushed files not incrementally indexed: %v\n", + err, + ) + } } func newBranchDiffBase(remoteName, localSHA string) string { diff --git a/go/internal/hookrunnercli/git_hook_internal_test.go b/go/internal/hookrunnercli/git_hook_internal_test.go index c0dc8ba0..75f0c34d 100644 --- a/go/internal/hookrunnercli/git_hook_internal_test.go +++ b/go/internal/hookrunnercli/git_hook_internal_test.go @@ -103,6 +103,38 @@ func TestPushedFilesParsesPrePushRefs(t *testing.T) { } } +func TestPushedCodeIntelFilesIncludesDeletionRenameSidesAndNewlines(t *testing.T) { + root := setupGitHookTestRepo(t) + chdirForTest(t, root) + mustWriteTestFile(t, "old.py", "OLD = 1\n") + mustWriteTestFile(t, "removed.py", "REMOVED = 1\n") + mustWriteTestFile(t, "line\nbreak.py", "ODD = 1\n") + runGitTestCommand(t, "add", "old.py", "removed.py", "line\nbreak.py") + runGitTestCommand(t, "commit", "-m", "test: base") + remoteSHA := gitTestOutput(t, "rev-parse", "HEAD") + + runGitTestCommand(t, "mv", "old.py", "new.py") + runGitTestCommand(t, "rm", "removed.py") + mustWriteTestFile(t, "line\nbreak.py", "ODD = 2\n") + runGitTestCommand(t, "add", "line\nbreak.py") + runGitTestCommand(t, "commit", "-m", "test: rename and delete") + localSHA := gitTestOutput(t, "rev-parse", "HEAD") + + input := strings.NewReader( + "refs/heads/main " + localSHA + " refs/heads/main " + remoteSHA + "\n", + ) + files, err := pushedCodeIntelFiles(input, "") + if err != nil { + t.Fatalf("pushedCodeIntelFiles() returned error: %v", err) + } + + for _, want := range []string{"line\nbreak.py", "new.py", "old.py", "removed.py"} { + if !slices.Contains(files, want) { + t.Fatalf("pushed code-intel files = %#v, missing %q", files, want) + } + } +} + func TestPushedFilesHandlesNewBranchAndDeleteRefs(t *testing.T) { root := setupGitHookTestRepo(t) chdirForTest(t, root) @@ -364,6 +396,34 @@ hooks: } } +func TestGitHooksRejectTimeoutsBeyondCriticalCeiling(t *testing.T) { + tempDir := setupGitHookTestRepo(t) + t.Chdir(tempDir) + t.Setenv("CODING_ETHOS_REAL_GIT", "") + t.Setenv(consumerRootEnv, tempDir) + bundleRoot := writeTestBundleRoot(t, tempDir) + t.Setenv(precommitRootEnv, bundleRoot) + + overridePath := filepath.Join(tempDir, "repo_config.yaml") + mustWriteTestFile( + t, + overridePath, + "hooks:\n tool_timeout_seconds: 601\n", + ) + t.Setenv(configEnv, overridePath) + + stderr := captureStderr(t, func() { + if got := runPreCommitHook(Config{}, nil); got != 1 { + t.Fatalf("runPreCommitHook() = %d, want 1", got) + } + }) + for _, want := range []string{"must not exceed 600 seconds", "10m0s", "critical failure"} { + if !strings.Contains(stderr, want) { + t.Fatalf("stderr missing %q:\n%s", want, stderr) + } + } +} + func TestGitHooksRejectRemovedEnabledGroupsConfigSpellings(t *testing.T) { for _, key := range []string{"enabled-groups", "enabledGroups", "ENABLED_GROUPS"} { t.Run(key, func(t *testing.T) { diff --git a/go/internal/hookrunnercli/hook_report.go b/go/internal/hookrunnercli/hook_report.go index 464b6031..f86bdf2e 100644 --- a/go/internal/hookrunnercli/hook_report.go +++ b/go/internal/hookrunnercli/hook_report.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" "strings" + "time" "blackcat.ca/coding-ethos/go/diagnostics" "blackcat.ca/coding-ethos/go/internal/feedback" @@ -261,6 +262,17 @@ func loadHookSettings() hookSettings { settings.ToolTimeoutSeconds = defaultHookSettings().ToolTimeoutSeconds } + if settings.ToolTimeoutSeconds > maxToolTimeoutSecs { + settings.ConfigError = fmt.Sprintf( + "hooks.tool_timeout_seconds must not exceed %d seconds; "+ + "a hook longer than %s is a critical failure", + maxToolTimeoutSecs, + time.Duration(maxToolTimeoutSecs)*time.Second, + ) + + return settings + } + if len(settings.FailSeverityLevels) == 0 { settings.FailSeverityLevels = defaultHookSettings().FailSeverityLevels } diff --git a/go/internal/hookrunnercli/lint_constants.go b/go/internal/hookrunnercli/lint_constants.go index a044362c..f045181e 100644 --- a/go/internal/hookrunnercli/lint_constants.go +++ b/go/internal/hookrunnercli/lint_constants.go @@ -6,7 +6,8 @@ package hookrunnercli const ( compactDividerWidth = 60 defaultPythonVersion = "3.13" - defaultToolTimeoutSecs = 900 + defaultToolTimeoutSecs = 600 + maxToolTimeoutSecs = 600 executePermissionMask = 0o111 extBash = ".bash" extPy = ".py" diff --git a/go/internal/hookrunnercli/main.go b/go/internal/hookrunnercli/main.go index cbaa76d1..fcb33ec0 100644 --- a/go/internal/hookrunnercli/main.go +++ b/go/internal/hookrunnercli/main.go @@ -29,6 +29,7 @@ const ( configEnv = "CODE_ETHOS_PRECOMMIT_CONFIG" precommitRootEnv = "CODE_ETHOS_PRECOMMIT_ROOT" consumerRootEnv = "CODE_ETHOS_CONSUMER_ROOT" + stateRootEnv = "CODE_ETHOS_STATE_ROOT" localRootEnv = "CODE_ETHOS_LOCAL_ROOT" privateKeyPattern = `-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----` textChunkSize = 8192 diff --git a/go/internal/hookrunnercli/toolchain_groups.go b/go/internal/hookrunnercli/toolchain_groups.go index 430c7763..2252eb8c 100644 --- a/go/internal/hookrunnercli/toolchain_groups.go +++ b/go/internal/hookrunnercli/toolchain_groups.go @@ -800,6 +800,7 @@ func runManagedPolicyTool(name string, args []string) int { InvocationCwd: repoRoot(), OutputFormat: selectedHookOutputFormat(), Args: args, + CodeIntel: true, }) } diff --git a/go/internal/hooks/gate_exit_status.go b/go/internal/hooks/gate_exit_status.go index b2f1aa89..06c5cb7b 100644 --- a/go/internal/hooks/gate_exit_status.go +++ b/go/internal/hooks/gate_exit_status.go @@ -52,6 +52,27 @@ type gateShell struct { operators []string } +type pipefailSetting uint8 + +const ( + pipefailUnchanged pipefailSetting = iota + pipefailEnabled + pipefailDisabled +) + +func (setting pipefailSetting) apply(current bool) bool { + switch setting { + case pipefailUnchanged: + return current + case pipefailEnabled: + return true + case pipefailDisabled: + return false + default: + return current + } +} + func maskedRequiredGateStatus( command string, inheritedPipefail bool, @@ -78,14 +99,29 @@ func maskedRequiredGateStatus( return reason, true } - if isPipefailCommand(gateExecutableArgv(parsed.segments[index])) { - pipefail = true + setting := pipefailCommandSetting( + gateExecutableArgv(parsed.segments[index]), + ) + if parsed.segmentControlsShellState(index) { + pipefail = setting.apply(pipefail) } } return "", false } +func (parsed gateShell) segmentControlsShellState(index int) bool { + if index < len(parsed.operators) { + switch parsed.operators[index] { + case "|", "|&", "&": + return false + } + } + + return index == 0 || + (parsed.operators[index-1] != "|" && parsed.operators[index-1] != "|&") +} + func (parsed gateShell) maskedSegmentStatus( index int, pipefail bool, @@ -97,7 +133,7 @@ func (parsed gateShell) maskedSegmentStatus( if nestedFound { reason, masked := maskedRequiredGateStatus( nested, - pipefail || nestedPipefail, + nestedPipefail.apply(pipefail), depth+1, ) if masked { @@ -180,9 +216,19 @@ func parseGateShell(command string) (gateShell, error) { return parsed, nil } -func isPipefailCommand(argv []string) bool { - return len(argv) >= 3 && argv[0] == "set" && - argv[1] == "-o" && argv[2] == "pipefail" +func pipefailCommandSetting(argv []string) pipefailSetting { + if len(argv) < 3 || argv[0] != "set" || argv[2] != "pipefail" { + return pipefailUnchanged + } + + switch argv[1] { + case "-o": + return pipefailEnabled + case "+o": + return pipefailDisabled + default: + return pipefailUnchanged + } } func sequenceAfter(gateIndex int, operators []string) bool { @@ -425,17 +471,23 @@ func discardWrapperOperand(argv []string) []string { return argv[1:] } -func nestedShellScript(argv []string) (string, bool, bool) { +func nestedShellScript(argv []string) (string, pipefailSetting, bool) { if len(argv) == 0 || !shellExecutable(argv[0]) { - return "", false, false + return "", pipefailUnchanged, false } - pipefail := false + pipefail := pipefailUnchanged for index := 1; index < len(argv); index++ { - if argv[index] == "-o" && index+1 < len(argv) && + if (argv[index] == "-o" || argv[index] == "+o") && + index+1 < len(argv) && argv[index+1] == "pipefail" { - pipefail = true + if argv[index] == "-o" { + pipefail = pipefailEnabled + } else { + pipefail = pipefailDisabled + } + index++ continue diff --git a/go/internal/hooks/gate_exit_status_test.go b/go/internal/hooks/gate_exit_status_test.go index 0acd9053..65e27d84 100644 --- a/go/internal/hooks/gate_exit_status_test.go +++ b/go/internal/hooks/gate_exit_status_test.go @@ -28,6 +28,8 @@ func TestRequiredGateExitStatusBlocksMaskedFailures(t *testing.T) { "make lint | tee lint.log", "make purrdf-extractor-check | tee purrdf.log", "make check | tail -100 && set -o pipefail", + "make check &", + "bash -c 'make check &'", } { route := requiredGateExitStatusRouteFor(Event{ HookEventName: eventPreToolUse, @@ -42,6 +44,140 @@ func TestRequiredGateExitStatusBlocksMaskedFailures(t *testing.T) { } } +func TestRequiredGateExitStatusTracksPipefailInExecutionOrder(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + command string + inherited bool + masked bool + }{ + { + name: "disabled by default", + command: "make check | tail -1", + masked: true, + }, + { + name: "inherited enabled", + command: "make check | tail -1", + inherited: true, + }, + { + name: "enabled before gate", + command: "set -o pipefail; make check | tail -1", + }, + { + name: "disabled before gate", + command: "set +o pipefail; make check | tail -1", + inherited: true, + masked: true, + }, + { + name: "pipeline-local disable preserves inherited state", + command: "set +o pipefail | make check | tail -1", + inherited: true, + }, + { + name: "conditional enable before gate", + command: "set -o pipefail && make check | tail -1", + }, + { + name: "conditional enable after false branch", + command: "false || set -o pipefail; make check | tail -1", + }, + { + name: "background segment does not change shell state", + command: "set +o pipefail & make check | tail -1", + inherited: true, + }, + { + name: "disable then enable", + command: "set +o pipefail; set -o pipefail; make check | tail -1", + inherited: true, + }, + { + name: "enable then disable", + command: "set -o pipefail; set +o pipefail; make check | tail -1", + inherited: true, + masked: true, + }, + { + name: "later enable is not retroactive", + command: "make check | tail -1; set -o pipefail", + masked: true, + }, + { + name: "unrelated enable preserves inherited state", + command: "set -o nounset; make check | tail -1", + inherited: true, + }, + { + name: "unrelated enable preserves disabled state", + command: "set -o nounset; make check | tail -1", + masked: true, + }, + { + name: "nested explicit disable overrides inherited state", + command: "bash +o pipefail -c 'make check | tail -1'", + inherited: true, + masked: true, + }, + { + name: "nested explicit enable", + command: "bash -o pipefail -c 'make check | tail -1'", + }, + { + name: "nested disable then enable", + command: "bash +o pipefail -o pipefail -c 'make check | tail -1'", + }, + { + name: "nested enable then disable", + command: "bash -o pipefail +o pipefail -c 'make check | tail -1'", + inherited: true, + masked: true, + }, + { + name: "outer enable reaches nested command", + command: "set -o pipefail; bash -c 'make check | tail -1'", + }, + { + name: "nested unrelated option preserves inherited state", + command: "bash -o nounset -c 'make check | tail -1'", + inherited: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + reason, masked := maskedRequiredGateStatus( + test.command, + test.inherited, + 0, + ) + if masked != test.masked { + t.Fatalf( + "command %q masked = %t, want %t (reason %q)", + test.command, + masked, + test.masked, + reason, + ) + } + if masked && reason != gatePipelineReason { + t.Fatalf( + "command %q reason = %q, want %q", + test.command, + reason, + gatePipelineReason, + ) + } + }) + } +} + func TestRequiredGateExitStatusFailsClosedBeyondShellDepth(t *testing.T) { t.Parallel() diff --git a/go/internal/lint/runner_test.go b/go/internal/lint/runner_test.go index cae6fd7c..8de0bbce 100644 --- a/go/internal/lint/runner_test.go +++ b/go/internal/lint/runner_test.go @@ -409,6 +409,11 @@ func initializedLintGitRepo(t *testing.T) string { repo := t.TempDir() runLintGit(t, repo, "init") + hooksPath := filepath.Join(repo, ".git", "test-hooks") + if err := os.Mkdir(hooksPath, 0o700); err != nil { + t.Fatalf("create isolated Git hooks directory: %v", err) + } + runLintGit(t, repo, "config", "core.hooksPath", hooksPath) runLintGit(t, repo, "config", "user.email", "test@example.com") runLintGit(t, repo, "config", "user.name", "Test") runLintGit(t, repo, "config", "commit.gpgsign", "false") diff --git a/go/internal/lintcli/main.go b/go/internal/lintcli/main.go index d2983f28..8c6072a8 100644 --- a/go/internal/lintcli/main.go +++ b/go/internal/lintcli/main.go @@ -600,8 +600,6 @@ func refreshLintCodeIntel(root, tracePath, scope string, files []string) { err := codeintel.IngestLintTraceFile(ctx, root, tracePath) if err != nil { writeLintCLIText("warning: lint trace not ingested into code-intel: " + err.Error()) - - return } paths := lintCodeIntelPaths(scope, files) @@ -617,7 +615,10 @@ func refreshLintCodeIntel(root, tracePath, scope string, files []string) { func lintCodeIntelPaths(scope string, files []string) []string { if scope == lint.ScopeFull { - return []string{"."} + // Full lint is an ordinary diagnostic path, not authorization for a + // whole-repository code-intel walk. Operators can request that work with + // the explicit code-intel maintenance commands. + return nil } if scope != lint.ScopeFiles && diff --git a/go/internal/lintcli/main_internal_test.go b/go/internal/lintcli/main_internal_test.go index dd19c093..f98d07e3 100644 --- a/go/internal/lintcli/main_internal_test.go +++ b/go/internal/lintcli/main_internal_test.go @@ -392,6 +392,66 @@ func TestRunCLICodeIntelIngestsTraceAndIndexesFiles(t *testing.T) { } } +func TestLintCodeIntelPathsNeverExpandsFullScope(t *testing.T) { + t.Parallel() + + if paths := lintCodeIntelPaths( + lint.ScopeFull, + []string{"pkg/app.py"}, + ); len( + paths, + ) != 0 { + t.Fatalf("full-scope code-intel paths = %#v, want none", paths) + } + + paths := lintCodeIntelPaths(lint.ScopeFiles, []string{"pkg/app.py"}) + if len(paths) != 1 || paths[0] != "pkg/app.py" { + t.Fatalf("explicit code-intel paths = %#v", paths) + } +} + +func TestRefreshLintCodeIntelIndexesFilesWhenTraceIngestFails(t *testing.T) { + t.Parallel() + + repo := t.TempDir() + target := filepath.Join(repo, samplePythonFile) + + err := os.MkdirAll(filepath.Dir(target), lintTestExecutableMode) + if err != nil { + t.Fatalf("create package dir: %v", err) + } + + err = os.WriteFile(target, []byte("VALUE = 1\n"), lintTestWriteMode) + if err != nil { + t.Fatalf("write python file: %v", err) + } + + refreshLintCodeIntel( + repo, + filepath.Join(repo, "missing-trace.json"), + lint.ScopeFiles, + []string{samplePythonFile}, + ) + + store, err := codeintel.OpenReadOnly( + context.Background(), + codeintel.DefaultDBPath(repo), + ) + if err != nil { + t.Fatalf("open code-intel store: %v", err) + } + defer store.Close() + + stats, err := store.Stats(context.Background()) + if err != nil { + t.Fatalf("code-intel stats: %v", err) + } + + if stats.Traces != 0 || stats.Files != 1 { + t.Fatalf("code-intel stats = %#v, want zero traces and one file", stats) + } +} + func TestRunCLIExplainMode(t *testing.T) { t.Parallel() diff --git a/go/internal/managedcapture/capture.go b/go/internal/managedcapture/capture.go index 2144633c..b4a870d8 100644 --- a/go/internal/managedcapture/capture.go +++ b/go/internal/managedcapture/capture.go @@ -64,6 +64,7 @@ type captureRequest struct { Parser string Category string ToolPath string + UVProject string Cwd string TraceRoot string SandboxBackendPath string @@ -1409,6 +1410,10 @@ func captureFormatterChanges( content, err := os.ReadFile(file) if err != nil { + if os.IsNotExist(err) { + changed = append(changed, formatterDiagnosticPath(request, file)) + } + continue } diff --git a/go/internal/managedcapture/capture_log.go b/go/internal/managedcapture/capture_log.go index 10046458..730b665d 100644 --- a/go/internal/managedcapture/capture_log.go +++ b/go/internal/managedcapture/capture_log.go @@ -59,8 +59,6 @@ func refreshCapturedToolCodeIntel(root, tracePath string, changedFiles []string) emitManagedCaptureText( "warning: captured lint trace not ingested into code-intel: " + err.Error(), ) - - return } if len(changedFiles) == 0 { diff --git a/go/internal/managedcapture/capture_test.go b/go/internal/managedcapture/capture_test.go index 50efecd8..560222fa 100644 --- a/go/internal/managedcapture/capture_test.go +++ b/go/internal/managedcapture/capture_test.go @@ -239,6 +239,65 @@ exit 0 } } +func TestRunCapturedToolCodeIntelTombstonesFormatterDeletionOnFailure(t *testing.T) { + if runtime.GOOS == windowsGOOS { + t.Skip("shell fixture uses POSIX sh") + } + + ctx := context.Background() + repo := t.TempDir() + target := filepath.Join(repo, "pkg", "app.py") + if err := os.MkdirAll(filepath.Dir(target), executableFixtureMode); err != nil { + t.Fatalf("create package dir: %v", err) + } + if err := os.WriteFile(target, []byte("VALUE = 1\n"), 0o600); err != nil { + t.Fatalf("write python file: %v", err) + } + if _, err := codeintel.RefreshLintFiles( + ctx, + repo, + []string{"pkg/app.py"}, + ); err != nil { + t.Fatalf("prime code-intel file: %v", err) + } + + tool := filepath.Join(repo, "formatter-fixture") + writeExecutableFixture(t, tool, `#!/usr/bin/env sh +rm pkg/app.py +exit 2 +`) + + exitCode := runCapturedToolWithRequest(captureRequest{ + Tool: "ruff-format", + Parser: "ruff", + Category: toolcatalog.CategoryFormat, + ToolPath: tool, + Cwd: repo, + TraceRoot: repo, + Args: []string{"format", "pkg/app.py"}, + DiagnosticKind: toolcatalog.DiagnosticKindFormatterChangedFiles, + FileExtensions: []string{".py"}, + CodeIntel: true, + }, hookoutput.FormatTOON) + if exitCode != 2 { + t.Fatalf("exit code = %d, want 2", exitCode) + } + + store, err := codeintel.OpenReadOnly(ctx, codeintel.DefaultDBPath(repo)) + if err != nil { + t.Fatalf("open code-intel store: %v", err) + } + defer store.Close() + + file, found, err := store.GetCodeFile(ctx, "pkg/app.py") + if err != nil { + t.Fatalf("query deleted code file: %v", err) + } + if !found || file.DeletedAtUTC == "" { + t.Fatalf("deleted file = %#v, found=%v", file, found) + } +} + func TestCopyProcessOutputDrainsStdoutAndStderrConcurrently(t *testing.T) { t.Parallel() diff --git a/go/internal/managedcapture/managed_capture.go b/go/internal/managedcapture/managed_capture.go index 2b980682..4b24b019 100644 --- a/go/internal/managedcapture/managed_capture.go +++ b/go/internal/managedcapture/managed_capture.go @@ -74,8 +74,9 @@ type Options struct { } type managedToolCommand struct { - Path string - Prefix []string + Path string + UVProject string + Prefix []string } func Run(options Options) int { @@ -195,6 +196,7 @@ func managedCaptureRequest( Parser: firstCaptureNonEmpty(tool.Parser, options.Tool), Category: tool.Category, ToolPath: command.Path, + UVProject: command.UVProject, ToolPrefix: command.Prefix, Cwd: captureCwd, TraceRoot: options.ConsumerRoot, @@ -858,15 +860,18 @@ func managedToolCommandFor(tool toolcatalog.Tool, ethosRoot string) managedToolC return managedToolCommand{} } + project := filepath.Join(ethosRoot, "pre-commit", "hooks") + return managedToolCommand{ Path: uvBin, Prefix: []string{ "run", "--quiet", "--project", - filepath.Join(ethosRoot, "pre-commit", "hooks"), + project, runtime.Command[0], }, + UVProject: project, } } diff --git a/go/internal/managedcapture/sandbox_cache_env.go b/go/internal/managedcapture/sandbox_cache_env.go index 92092a07..5500ca08 100644 --- a/go/internal/managedcapture/sandbox_cache_env.go +++ b/go/internal/managedcapture/sandbox_cache_env.go @@ -5,6 +5,8 @@ package managedcapture import ( "context" + "crypto/sha256" + "encoding/hex" "fmt" "os" "path/filepath" @@ -34,8 +36,9 @@ type sandboxCacheEnvironment struct { // sandbox without network access, and an installation away from its // default path is invisible to a command that inherits no environment — // cargo is on PATH, runs, and reports no toolchain. - CargoHome string - RustupHome string + CargoHome string + RustupHome string + UVProjectEnvironment string // Trailing so the single bool pads the struct once rather than splitting // the string fields around it. CleanupTemp bool @@ -109,20 +112,21 @@ func (environment sandboxCacheEnvironment) items() []string { func (environment sandboxCacheEnvironment) value(name string) string { return map[string]string{ - "TMPDIR": environment.TempDir, - "XDG_RUNTIME_DIR": environment.RuntimeDir, - "GOCACHE": environment.GoCache, - "GOPATH": environment.GoPath, - "GOMODCACHE": environment.GoModCache, - "GOLANGCI_LINT_CACHE": environment.GolangCILintDir, - "GOROOT": environment.GoRoot, - "CGO_ENABLED": environment.CGOEnabled, - "CC": environment.CC, - "COMPILER_PATH": environment.CompilerPath, - "AS": environment.Assembler, - "CARGO_HOME": environment.CargoHome, - "RUSTUP_HOME": environment.RustupHome, - evaluators.RealGitEnv: environment.RealGit, + "TMPDIR": environment.TempDir, + "XDG_RUNTIME_DIR": environment.RuntimeDir, + "GOCACHE": environment.GoCache, + "GOPATH": environment.GoPath, + "GOMODCACHE": environment.GoModCache, + "GOLANGCI_LINT_CACHE": environment.GolangCILintDir, + "GOROOT": environment.GoRoot, + "CGO_ENABLED": environment.CGOEnabled, + "CC": environment.CC, + "COMPILER_PATH": environment.CompilerPath, + "AS": environment.Assembler, + "CARGO_HOME": environment.CargoHome, + "RUSTUP_HOME": environment.RustupHome, + "UV_PROJECT_ENVIRONMENT": environment.UVProjectEnvironment, + evaluators.RealGitEnv: environment.RealGit, }[name] } @@ -141,6 +145,7 @@ func (environment sandboxCacheEnvironment) names() []string { "AS", "CARGO_HOME", "RUSTUP_HOME", + "UV_PROJECT_ENVIRONMENT", evaluators.RealGitEnv, } } @@ -200,26 +205,73 @@ func sandboxCacheEnv( cargoHome, rustupHome := rustHomes() + uvProjectEnvironment, err := sandboxUVProjectEnvironment(root, request) + if err != nil { + return sandboxCacheEnvironment{}, err + } + return sandboxCacheEnvironment{ - TempDir: tempDir, - RuntimeDir: runtimeDir, - CleanupTemp: cleanupTemp, - GoCache: goCaches.Cache, - GoPath: goCaches.Path, - GoModCache: goCaches.ModCache, - GolangCILintDir: goCaches.GolangCILint, - GoRoot: managedGoRoot(ctx), - CGOEnabled: "1", - CC: cCompiler, - CompilerPath: managedCompilerPath(cCompiler, assembler), - Assembler: assembler, - RealGit: realGit, - PathPrefix: pathPrefix, - CargoHome: cargoHome, - RustupHome: rustupHome, + TempDir: tempDir, + RuntimeDir: runtimeDir, + CleanupTemp: cleanupTemp, + GoCache: goCaches.Cache, + GoPath: goCaches.Path, + GoModCache: goCaches.ModCache, + GolangCILintDir: goCaches.GolangCILint, + GoRoot: managedGoRoot(ctx), + CGOEnabled: "1", + CC: cCompiler, + CompilerPath: managedCompilerPath(cCompiler, assembler), + Assembler: assembler, + RealGit: realGit, + PathPrefix: pathPrefix, + CargoHome: cargoHome, + RustupHome: rustupHome, + UVProjectEnvironment: uvProjectEnvironment, }, nil } +func sandboxUVProjectEnvironment(root string, request captureRequest) (string, error) { + project := strings.TrimSpace(request.UVProject) + if project == "" { + return "", nil + } + + canonical, err := filepath.Abs(project) + if err != nil { + return "", fmt.Errorf("resolve uv project path %s: %w", project, err) + } + + // A consumer repo may name a uv project that does not exist yet. Fall back + // to the lexical path so the sandbox still derives a stable environment + // instead of denying the capture outright. + resolved, err := filepath.EvalSymlinks(canonical) + switch { + case err == nil: + canonical = resolved + case os.IsNotExist(err): + canonical = filepath.Clean(canonical) + default: + return "", fmt.Errorf("resolve uv project symlinks %s: %w", project, err) + } + + digest := sha256.Sum256([]byte(canonical)) + environment := filepath.Join( + root, + ".coding-ethos", + "cache", + "uv-project-env", + hex.EncodeToString(digest[:]), + ) + + err = makeSandboxDir(environment, "UV project environment") + if err != nil { + return "", err + } + + return environment, nil +} + func prepareSandboxGoCachePaths(root string) (sandboxGoCachePaths, error) { paths := sandboxGoCachePaths{ Cache: filepath.Join(root, sandbox.SandboxGoCachePath), diff --git a/go/internal/managedcapture/sandbox_cache_env_test.go b/go/internal/managedcapture/sandbox_cache_env_test.go index 5704315d..11d42d06 100644 --- a/go/internal/managedcapture/sandbox_cache_env_test.go +++ b/go/internal/managedcapture/sandbox_cache_env_test.go @@ -5,11 +5,16 @@ package managedcapture import ( "context" + "crypto/sha256" + "encoding/hex" "os" "path/filepath" + "strings" "testing" "blackcat.ca/coding-ethos/go/internal/sandbox" + "blackcat.ca/coding-ethos/go/lintcapture" + "blackcat.ca/coding-ethos/go/toolcatalog" ) func TestSandboxCacheEnvCreatesConsumerGoPath(t *testing.T) { @@ -43,3 +48,140 @@ func TestSandboxCacheEnvCreatesConsumerGoPath(t *testing.T) { } } } + +func TestSandboxCacheEnvIsolatesUVProjectUnderConsumerCache(t *testing.T) { + consumerRoot := t.TempDir() + installedEthosRoot := t.TempDir() + hookProject := filepath.Join(installedEthosRoot, "pre-commit", "hooks") + if err := os.MkdirAll(hookProject, 0o700); err != nil { + t.Fatalf("create installed hook project: %v", err) + } + + uvPath := filepath.Join(t.TempDir(), "managed-uv") + if err := os.WriteFile(uvPath, []byte("fixture"), 0o700); err != nil { + t.Fatalf("write managed uv fixture: %v", err) + } + t.Setenv("UV", uvPath) + + tool, found := toolcatalog.HookOwnedTool("pyupgrade") + if !found { + t.Fatal("missing pyupgrade tool") + } + + request, exitCode, err := managedCaptureRequest( + tool, + lintcapture.RuntimeConfig{}, + Options{ + Tool: "pyupgrade", + EthosRoot: installedEthosRoot, + ConsumerRoot: consumerRoot, + InvocationCwd: consumerRoot, + Args: []string{"--version"}, + }, + ) + if err != nil { + t.Fatalf("managedCaptureRequest exit=%d: %v", exitCode, err) + } + if request.UVProject != hookProject { + t.Fatalf("UV project = %q, want %q", request.UVProject, hookProject) + } + + environment, err := sandboxCacheEnv(context.Background(), request) + if err != nil { + t.Fatalf("sandboxCacheEnv: %v", err) + } + + digest := sha256.Sum256([]byte(hookProject)) + want := filepath.Join( + consumerRoot, + ".coding-ethos", + "cache", + "uv-project-env", + hex.EncodeToString(digest[:]), + ) + if environment.UVProjectEnvironment != want { + t.Fatalf( + "UV_PROJECT_ENVIRONMENT = %q, want %q", + environment.UVProjectEnvironment, + want, + ) + } + if strings.HasPrefix( + environment.UVProjectEnvironment, + hookProject+string(os.PathSeparator), + ) { + t.Fatalf("UV project environment is inside installed hook project: %q", want) + } + if info, statErr := os.Stat(want); statErr != nil || !info.IsDir() { + t.Fatalf("UV project environment is not usable: info=%v error=%v", info, statErr) + } + + hostValue := filepath.Join(installedEthosRoot, "host-uv-project") + processEnv := capturedProcessEnv( + []string{"UV_PROJECT_ENVIRONMENT=" + hostValue}, + environment, + "pyupgrade", + ) + for _, item := range processEnv { + if item == "UV_PROJECT_ENVIRONMENT="+hostValue { + t.Fatalf("captured process retained host UV project environment") + } + if item == "UV_PROJECT_ENVIRONMENT="+want { + return + } + } + + t.Fatalf("captured process omitted UV project environment") +} + +// A consumer repo can name a uv project directory that has not been installed +// yet. Resolution must fall back to the lexical path instead of denying the +// capture, which is what made managed capture fail from a consumer root. +func TestSandboxUVProjectEnvironmentAcceptsMissingProjectDirectory(t *testing.T) { + consumerRoot := t.TempDir() + missingProject := filepath.Join(t.TempDir(), "pre-commit", "hooks") + + environment, err := sandboxUVProjectEnvironment(consumerRoot, captureRequest{ + UVProject: missingProject, + }) + if err != nil { + t.Fatalf("sandboxUVProjectEnvironment for missing project: %v", err) + } + + digest := sha256.Sum256([]byte(filepath.Clean(missingProject))) + want := filepath.Join( + consumerRoot, + ".coding-ethos", + "cache", + "uv-project-env", + hex.EncodeToString(digest[:]), + ) + if environment != want { + t.Fatalf("UV project environment = %q, want %q", environment, want) + } + + if info, statErr := os.Stat(want); statErr != nil || !info.IsDir() { + t.Fatalf("UV project environment is not usable: info=%v error=%v", info, statErr) + } +} + +// An empty uv project means the tool is not uv-backed, so no environment is +// derived and no directory is created. +func TestSandboxUVProjectEnvironmentSkipsToolsWithoutUVProject(t *testing.T) { + consumerRoot := t.TempDir() + + environment, err := sandboxUVProjectEnvironment(consumerRoot, captureRequest{ + UVProject: " ", + }) + if err != nil { + t.Fatalf("sandboxUVProjectEnvironment without a project: %v", err) + } + if environment != "" { + t.Fatalf("UV project environment = %q, want empty", environment) + } + + cacheRoot := filepath.Join(consumerRoot, ".coding-ethos", "cache", "uv-project-env") + if _, statErr := os.Stat(cacheRoot); !os.IsNotExist(statErr) { + t.Fatalf("uv project cache root created without a project: %v", statErr) + } +} diff --git a/go/internal/mcp/server.go b/go/internal/mcp/server.go index f8c06dc7..7aa189e9 100644 --- a/go/internal/mcp/server.go +++ b/go/internal/mcp/server.go @@ -12,6 +12,7 @@ import ( "fmt" "io" "os" + "path/filepath" "slices" "strings" "time" @@ -41,6 +42,9 @@ var ( errManagedLintRuntimeUnavailable = apperror.StaticError( "managed lint runtime is not configured", ) + errManagedLintExecutionRootUnavailable = apperror.StaticError( + "managed lint requested cwd has no owning Git repository", + ) errCodeIntelRootUnavailable = apperror.StaticError( "code-intel root is not configured", ) @@ -478,12 +482,19 @@ func (server Server) checkManagedLint(input lintCheckInput) (any, error) { var stdout bytes.Buffer + executionRoot, invocationCwd, contextErr := server.managedLintExecutionContext( + input.Cwd, + ) + if contextErr != nil { + return nil, contextErr + } + exitCode := managedcapture.Run(managedcapture.Options{ PolicyContext: managedLintPolicyContext(server.bundle), Tool: tool, EthosRoot: server.runtime.EthosRoot, - ConsumerRoot: server.runtime.ConsumerRoot, - InvocationCwd: firstNonEmpty(input.Cwd, server.runtime.InvocationCwd), + ConsumerRoot: executionRoot, + InvocationCwd: invocationCwd, OutputFormat: hookoutput.FormatJSON, Output: &stdout, Args: managedLintToolArgs(input), @@ -503,7 +514,192 @@ func (server Server) checkManagedLint(input lintCheckInput) (any, error) { ) } - return managedLintOutput(tool, exitCode, result), nil + return managedLintOutput( + tool, + exitCode, + result, + executionRoot, + invocationCwd, + ), nil +} + +func (server Server) managedLintExecutionContext( + requestedCwd string, +) (string, string, error) { + requestedCwd = strings.TrimSpace(requestedCwd) + executionRoot := managedLintAbsolutePath(server.runtime.ConsumerRoot) + + if requestedCwd == "" { + invocationCwd := managedLintAbsolutePath(firstNonEmpty( + server.runtime.InvocationCwd, + server.runtime.ConsumerRoot, + )) + + return executionRoot, invocationCwd, nil + } + + invocationCwd, err := managedLintCanonicalDirectory(requestedCwd) + if err != nil { + return "", "", err + } + + configuredRoot, err := managedLintCanonicalDirectory( + server.runtime.ConsumerRoot, + ) + if err != nil { + return "", "", err + } + + if managedLintPathWithin(configuredRoot, invocationCwd) { + return configuredRoot, invocationCwd, nil + } + + repositoryRoot, found := managedLintRepositoryRoot(invocationCwd) + if !found { + return "", "", fmt.Errorf( + "resolve managed lint execution root for %q: %w", + invocationCwd, + errManagedLintExecutionRootUnavailable, + ) + } + + return repositoryRoot, invocationCwd, nil +} + +func managedLintAbsolutePath(path string) string { + absolute, err := filepath.Abs(strings.TrimSpace(path)) + if err != nil { + return filepath.Clean(path) + } + + return filepath.Clean(absolute) +} + +func managedLintCanonicalDirectory(path string) (string, error) { + absolute := managedLintAbsolutePath(path) + + canonical, err := filepath.EvalSymlinks(absolute) + if err != nil { + return "", fmt.Errorf("resolve managed lint cwd %q: %w", path, err) + } + + info, err := os.Stat(canonical) + if err != nil { + return "", fmt.Errorf("inspect managed lint cwd %q: %w", path, err) + } + + if !info.IsDir() { + return "", fmt.Errorf( + "resolve managed lint cwd %q: %w", + path, + errManagedLintExecutionRootUnavailable, + ) + } + + return filepath.Clean(canonical), nil +} + +func managedLintPathWithin(root, path string) bool { + relative, err := filepath.Rel(root, path) + if err != nil { + return false + } + + return relative == "." || + (relative != ".." && + !strings.HasPrefix(relative, ".."+string(filepath.Separator))) +} + +func managedLintRepositoryRoot(cwd string) (string, bool) { + directory := managedLintAbsolutePath(cwd) + + for { + markerPresent, markerValid := managedLintGitMarker(directory) + if markerValid { + return directory, true + } + + if markerPresent { + return "", false + } + + parent := filepath.Dir(directory) + if parent == directory { + return "", false + } + + directory = parent + } +} + +func managedLintGitMarker(repositoryRoot string) (bool, bool) { + marker := filepath.Join(repositoryRoot, ".git") + + info, err := os.Lstat(marker) + if errors.Is(err, os.ErrNotExist) { + return false, false + } + + if err != nil { + return true, false + } + + if info.IsDir() { + return true, true + } + + if !info.Mode().IsRegular() { + return true, false + } + + return true, managedLintGitFileMarkerValid(repositoryRoot, marker) +} + +func managedLintGitFileMarkerValid(repositoryRoot, marker string) bool { + contents, err := os.ReadFile(marker) + if err != nil { + return false + } + + gitDirectory, found := strings.CutPrefix( + strings.TrimSpace(string(contents)), + "gitdir:", + ) + + gitDirectory = strings.TrimSpace(gitDirectory) + if !found || gitDirectory == "" { + return false + } + + if strings.ContainsAny(gitDirectory, "\r\n") { + return false + } + + if !filepath.IsAbs(gitDirectory) { + gitDirectory = filepath.Join(repositoryRoot, gitDirectory) + } + + return managedLintGitDirectoryExists(gitDirectory) +} + +func managedLintGitDirectoryExists(gitDirectory string) bool { + gitDirectory = filepath.Clean(gitDirectory) + parent, basename := filepath.Dir(gitDirectory), filepath.Base(gitDirectory) + + if basename == "." || basename == string(filepath.Separator) { + return false + } + + root, err := os.OpenRoot(parent) + if err != nil { + return false + } + + defer func() { _ = root.Close() }() + + gitInfo, err := root.Stat(basename) + + return err == nil && gitInfo.IsDir() } func managedLintToolArgs(input lintCheckInput) []string { @@ -535,18 +731,22 @@ func managedLintOutput( tool string, exitCode int, result lint.Result, + executionRoot string, + invocationCwd string, ) map[string]any { return map[string]any{ - "engine": "managed_lint_capture", - "tool": tool, - "exit_code": exitCode, - "status": result.Status, - "blocked": result.Blocked(), - "files": result.Files, - "findings": result.Findings, - "diagnostics": result.Diagnostics, - "skill_hints": result.SkillHints, - "capture": result.Capture, + "engine": "managed_lint_capture", + "tool": tool, + "exit_code": exitCode, + "status": result.Status, + "blocked": result.Blocked(), + "files": result.Files, + "findings": result.Findings, + "diagnostics": result.Diagnostics, + "skill_hints": result.SkillHints, + "capture": result.Capture, + "execution_root": executionRoot, + "cwd": invocationCwd, } } diff --git a/go/internal/mcp/server_test.go b/go/internal/mcp/server_test.go index 9cc8885b..2b016f4a 100644 --- a/go/internal/mcp/server_test.go +++ b/go/internal/mcp/server_test.go @@ -815,6 +815,286 @@ func TestServerLintCheckRunsManagedToolCapture(t *testing.T) { } } +func TestServerManagedLintUsesRequestedRepositoryDirectory(t *testing.T) { + t.Parallel() + + assertServerManagedLintUsesRequestedRoot(t, "directory") +} + +func TestServerManagedLintUsesRequestedLinkedWorktree(t *testing.T) { + t.Parallel() + + assertServerManagedLintUsesRequestedRoot(t, "worktree-file") +} + +func TestServerManagedLintUsesRequestedRelativeLinkedWorktree(t *testing.T) { + t.Parallel() + + assertServerManagedLintUsesRequestedRoot(t, "relative-worktree-file") +} + +func TestServerManagedLintRejectsUnresolvedRequestedCwd(t *testing.T) { + t.Parallel() + + serverRoot := t.TempDir() + missingCwd := filepath.Join(t.TempDir(), "missing") + + output := runServerWithRuntime(t, compactJSON(t, fmt.Sprintf(`{ + "jsonrpc":"2.0", + "id":82, + "method":"tools/call", + "params":{ + "name":"managed_lint", + "arguments":{"tool":"ruff","cwd":%q} + } + }`, missingCwd)), mcp.Runtime{ + BundlePath: filepath.Join(serverRoot, "policy-bundle.json"), + EthosRoot: serverRoot, + ConsumerRoot: serverRoot, + InvocationCwd: serverRoot, + }) + response := decodeResponse(t, output) + + if response["error"] == nil || + !strings.Contains(output, "resolve managed lint cwd") { + t.Fatalf("unresolved requested cwd did not fail closed:\n%s", output) + } +} + +func TestServerManagedLintRejectsMalformedLinkedWorktreeMarker(t *testing.T) { + t.Parallel() + + assertServerManagedLintRejectsGitMarker(t, "not-a-gitdir\n") +} + +func TestServerManagedLintRejectsMissingLinkedWorktreeTarget(t *testing.T) { + t.Parallel() + + missingGitDirectory := filepath.Join(t.TempDir(), "missing-gitdir") + assertServerManagedLintRejectsGitMarker( + t, + "gitdir: "+missingGitDirectory+"\n", + ) +} + +func assertServerManagedLintRejectsGitMarker(t *testing.T, contents string) { + t.Helper() + + serverRoot := t.TempDir() + requestedRoot := filepath.Join(t.TempDir(), "requested-worktree") + requestedCwd := filepath.Join(requestedRoot, "nested") + + err := os.MkdirAll(requestedCwd, 0o700) + if err != nil { + t.Fatalf("create requested worktree: %v", err) + } + + err = os.WriteFile( + filepath.Join(requestedRoot, ".git"), + []byte(contents), + 0o600, + ) + if err != nil { + t.Fatalf("write linked worktree git file: %v", err) + } + + output := runServerWithRuntime(t, compactJSON(t, fmt.Sprintf(`{ + "jsonrpc":"2.0", + "id":83, + "method":"tools/call", + "params":{ + "name":"managed_lint", + "arguments":{"tool":"ruff","cwd":%q} + } + }`, requestedCwd)), mcp.Runtime{ + BundlePath: filepath.Join(serverRoot, "policy-bundle.json"), + EthosRoot: serverRoot, + ConsumerRoot: serverRoot, + InvocationCwd: serverRoot, + }) + response := decodeResponse(t, output) + + if response["error"] == nil || + !strings.Contains(output, "no owning Git repository") { + t.Fatalf("invalid linked worktree marker did not fail closed:\n%s", output) + } +} + +func assertServerManagedLintUsesRequestedRoot(t *testing.T, markerKind string) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("managed lint fixture uses POSIX sh") + } + + serverRoot := t.TempDir() + writeManagedLintRuntimeFixture(t, serverRoot) + + requestedRoot := filepath.Join(t.TempDir(), "requested-worktree") + requestedCwd := filepath.Join(requestedRoot, "nested") + + err := os.MkdirAll(requestedCwd, 0o700) + if err != nil { + t.Fatalf("create requested worktree: %v", err) + } + + writeManagedLintGitMarker(t, requestedRoot, markerKind) + + _, err = toolconfigs.Sync(serverRoot, requestedRoot, "") + if err != nil { + t.Fatalf("sync requested worktree tool configs: %v", err) + } + + err = os.WriteFile( + filepath.Join(requestedRoot, "requested-worktree.marker"), + []byte("requested\n"), + 0o600, + ) + if err != nil { + t.Fatalf("write requested worktree marker: %v", err) + } + + pwdPath := filepath.Join( + requestedRoot, + ".coding-ethos", + "cache", + "managed-lint-pwd", + ) + err = os.MkdirAll(filepath.Dir(pwdPath), 0o700) + if err != nil { + t.Fatalf("create managed lint PWD evidence directory: %v", err) + } + + writeExecutable( + t, + filepath.Join(serverRoot, ".venv", "bin", "ruff"), + managedLintRequestedRootRuffFixture(), + ) + + output := runServerWithRuntime(t, compactJSON(t, fmt.Sprintf(`{ + "jsonrpc":"2.0", + "id":81, + "method":"tools/call", + "params":{ + "name":"managed_lint", + "arguments":{"tool":"ruff","cwd":%q} + } + }`, requestedCwd)), mcp.Runtime{ + BundlePath: filepath.Join(serverRoot, "policy-bundle.json"), + EthosRoot: serverRoot, + ConsumerRoot: serverRoot, + InvocationCwd: serverRoot, + }) + response := decodeResponse(t, output) + content := structuredContent(t, response) + + if content["execution_root"] != requestedRoot || content["cwd"] != requestedCwd { + t.Fatalf("managed lint execution evidence = %#v", content) + } + + if strings.Contains(output, "runtime.sandbox_denial") && + strings.Contains(output, "coding-ethos-sandbox") && + strings.Contains(strings.ToLower(output), "permission denied") { + t.Skip("nested managed-lint fixture execution is denied by this sandbox") + } + + if strings.Contains(output, "runtime.sandbox_denial") { + t.Fatalf("managed lint sandbox unexpectedly denied:\n%s", output) + } + + actualCwd, err := os.ReadFile(pwdPath) + if err != nil { + t.Fatalf("read managed lint PWD evidence: %v", err) + } + if strings.TrimSpace(string(actualCwd)) != requestedRoot { + t.Fatalf( + "managed lint PWD = %q, want %q", + strings.TrimSpace(string(actualCwd)), + requestedRoot, + ) + } + + if !strings.Contains(output, `"file":"src/requested-worktree.py"`) || + strings.Contains(output, `"file":"src/server-root.py"`) { + t.Fatalf("managed lint did not execute from requested worktree:\n%s", output) + } +} + +func writeManagedLintGitMarker(t *testing.T, root, markerKind string) { + t.Helper() + + marker := filepath.Join(root, ".git") + + switch markerKind { + case "directory": + err := os.MkdirAll(marker, 0o700) + if err != nil { + t.Fatalf("create repository git directory: %v", err) + } + case "worktree-file": + gitDirectory := filepath.Join(t.TempDir(), "gitdir") + err := os.MkdirAll(gitDirectory, 0o700) + if err != nil { + t.Fatalf("create linked worktree git directory: %v", err) + } + + err = os.WriteFile( + marker, + []byte("gitdir: "+gitDirectory+"\n"), + 0o600, + ) + if err != nil { + t.Fatalf("write linked worktree git file: %v", err) + } + case "relative-worktree-file": + gitDirectory := filepath.Join(root, ".git-worktree") + err := os.MkdirAll(gitDirectory, 0o700) + if err != nil { + t.Fatalf("create relative linked worktree git directory: %v", err) + } + + err = os.WriteFile( + marker, + []byte("gitdir: .git-worktree\n"), + 0o600, + ) + if err != nil { + t.Fatalf("write relative linked worktree git file: %v", err) + } + default: + t.Fatalf("unsupported managed lint git marker kind %q", markerKind) + } +} + +func managedLintRequestedRootRuffFixture() string { + return `#!/usr/bin/env sh +pwd > .coding-ethos/cache/managed-lint-pwd +if [ -f requested-worktree.marker ]; then + cat <<'JSON' +[ + { + "filename": "src/requested-worktree.py", + "code": "F401", + "message": "requested worktree", + "location": {"row": 1, "column": 1} + } +] +JSON +else + cat <<'JSON' +[ + { + "filename": "src/server-root.py", + "code": "F401", + "message": "server root", + "location": {"row": 1, "column": 1} + } +] +JSON +fi +exit 1 +` +} + func writeManagedLintRuntimeFixture(t *testing.T, root string) { t.Helper() diff --git a/go/internal/policy/bundle.go b/go/internal/policy/bundle.go index 29ac0c53..1fcd5a36 100644 --- a/go/internal/policy/bundle.go +++ b/go/internal/policy/bundle.go @@ -172,7 +172,7 @@ func ExampleBundle() Bundle { Sources: exampleSources(), Advice: exampleAdvice(), Principles: principles, - Policies: examplePolicies(), + Policies: examplePolicies(principles), Skills: exampleSkills(), Dispatch: exampleDispatch(), EvidenceMaps: defaultEvidenceMaps(principles), @@ -264,7 +264,7 @@ func examplePrinciples() map[string]Principle { } } -func examplePolicies() map[string]Policy { +func examplePolicies(principles map[string]Principle) map[string]Policy { return map[string]Policy{ "python.conditional_imports": exampleConditionalImportPolicy(), "python.functional_idioms": exampleFunctionalIdiomPolicy(), @@ -277,7 +277,7 @@ func examplePolicies() map[string]Policy { "filesystem.protected_path": exampleProtectedPathPolicy(), "proxy.search_replace_edit": ProxySearchReplaceEditPolicy(), "shell.malformed_command": exampleShellMalformedCommandPolicy(), - "shell.required_gate_exit_status": exampleShellRequiredGateExitStatusPolicy(), + "shell.required_gate_exit_status": requiredGateExitStatusRoutePolicy(principles), "shell.forbidden_strings": exampleShellForbiddenStringsPolicy(), } } @@ -1083,32 +1083,6 @@ func exampleShellMalformedCommandPolicy() Policy { } } -func exampleShellRequiredGateExitStatusPolicy() Policy { - return Policy{ - ID: "shell.required_gate_exit_status", - Category: "shell", - Source: SourceRef{ - File: "config.yaml", - Path: "shell.required_gate_exit_status", - }, - PrincipleIDs: []string{ - "validation-at-the-gate", - "evidence-based-engineering-and-decision-quality", - }, - DefaultSeverity: "block", - SupportedModes: []string{"block", "record"}, - Message: "Required repository gates must return their own " + - "exact terminal status.", - Suggestion: "Run the gate directly, enable pipefail for pipelines, or capture " + - "and return the gate's exact status.", - DefenseLayers: hookRouteDefenseLayers(), - AppliesTo: AppliesTo{Tools: []string{"Bash"}}, - Evaluators: []Evaluator{ - {Kind: "external", Name: "shell.required_gate_exit_status"}, - }, - } -} - func exampleLinterDispatch() map[string][]string { return map[string][]string{ "files": {"python.conditional_imports", "python.functional_idioms"}, diff --git a/go/internal/policy/compiler_policies_internal_test.go b/go/internal/policy/compiler_policies_internal_test.go index 68e00bda..3a56cb56 100644 --- a/go/internal/policy/compiler_policies_internal_test.go +++ b/go/internal/policy/compiler_policies_internal_test.go @@ -3,7 +3,10 @@ package policy -import "testing" +import ( + "reflect" + "testing" +) func TestSPDXLicenseTextURLUsesLicenseListDataTextSource(t *testing.T) { t.Parallel() @@ -47,3 +50,32 @@ func TestValidateSPDXLicenseIDAllowsCommonIdentifierCharacters(t *testing.T) { } } } + +func TestExampleBundleUsesOneCanonicalRequiredGateExitStatusPolicy(t *testing.T) { + t.Parallel() + + bundle := ExampleBundle() + want := requiredGateExitStatusRoutePolicy(bundle.Principles) + got, ok := bundle.Policies[want.ID] + if !ok { + t.Fatalf("example bundle missing %q", want.ID) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("example policy = %#v, want canonical policy %#v", got, want) + } + + count := 0 + for key, candidate := range bundle.Policies { + if candidate.ID != want.ID { + continue + } + + count++ + if key != want.ID { + t.Errorf("canonical policy stored under key %q, want %q", key, want.ID) + } + } + if count != 1 { + t.Fatalf("example bundle contains %d policies with ID %q, want 1", count, want.ID) + } +} diff --git a/go/internal/policygitcli/main.go b/go/internal/policygitcli/main.go index c04e10ad..3009aea8 100644 --- a/go/internal/policygitcli/main.go +++ b/go/internal/policygitcli/main.go @@ -239,6 +239,17 @@ func wrapperBoolValue(name, value string, hasValue bool) (bool, error) { } func gitGlobalOptionStartsArgv(argument string) bool { + switch argument { + case "--no-optional-locks", "--no-lazy-fetch", "--no-advice": + return true + } + + for _, prefix := range []string{"--attr-source=", "--list-cmds="} { + if strings.HasPrefix(argument, prefix) && len(argument) > len(prefix) { + return true + } + } + if strings.HasPrefix(argument, "-c=") || strings.HasPrefix(argument, "-C=") { return true } diff --git a/go/internal/policygitcli/main_internal_test.go b/go/internal/policygitcli/main_internal_test.go index 794cb6e4..b0f566d5 100644 --- a/go/internal/policygitcli/main_internal_test.go +++ b/go/internal/policygitcli/main_internal_test.go @@ -9,6 +9,7 @@ import ( "io" "os" "path/filepath" + "slices" "strings" "testing" @@ -101,6 +102,55 @@ func TestGitGlobalOptionStartsArgvRecognizesShortMetaOptions(t *testing.T) { } } +func TestParsePolicyGitArgsPreservesSupportedGitGlobalOptions(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + argv []string + }{ + {name: "no optional locks", argv: []string{"--no-optional-locks", "status"}}, + {name: "no lazy fetch", argv: []string{"--no-lazy-fetch", "status"}}, + {name: "no advice", argv: []string{"--no-advice", "status"}}, + {name: "attribute source", argv: []string{"--attr-source=HEAD", "status"}}, + {name: "command groups", argv: []string{"--list-cmds=main,others"}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + args := append([]string{"--bundle=/policy/bundle.json"}, test.argv...) + parsed, err := parsePolicyGitArgs(args) + if err != nil { + t.Fatalf("parsePolicyGitArgs: %v", err) + } + + if !slices.Equal(parsed.gitArgv, test.argv) { + t.Fatalf("git argv = %#v, want %#v", parsed.gitArgv, test.argv) + } + }) + } +} + +func TestGitGlobalOptionStartsArgvRejectsInvalidNewOptionForms(t *testing.T) { + t.Parallel() + + for _, option := range []string{ + "--no-optional-locks=true", + "--no-lazy-fetch=false", + "--no-advice=true", + "--attr-source", + "--attr-source=", + "--list-cmds", + "--list-cmds=", + } { + if gitGlobalOptionStartsArgv(option) { + t.Fatalf("gitGlobalOptionStartsArgv(%q) = true, want false", option) + } + } +} + func TestParsePolicyGitArgsHonorsExplicitBoundary(t *testing.T) { t.Parallel() @@ -137,6 +187,43 @@ func TestParsePolicyGitArgsRejectsUnknownWrapperOption(t *testing.T) { } } +func TestParsePolicyGitArgsRejectsInvalidWrapperValues(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + args []string + want error + }{ + { + name: "required value", + args: []string{"--bundle"}, + want: errWrapperValueRequired, + }, + { + name: "empty value", + args: []string{"--bundle="}, + want: errWrapperOptionEmpty, + }, + { + name: "invalid boolean", + args: []string{"--json=maybe"}, + want: errWrapperBoolValue, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + _, err := parsePolicyGitArgs(test.args) + if !errors.Is(err, test.want) { + t.Fatalf("parsePolicyGitArgs(%#v) error = %v, want %v", test.args, err, test.want) + } + }) + } +} + func TestGitOptionsForNonStdinCommand(t *testing.T) { t.Parallel() diff --git a/go/internal/sandbox/cgroup_linux.go b/go/internal/sandbox/cgroup_linux.go index 6e596cb8..97e51248 100644 --- a/go/internal/sandbox/cgroup_linux.go +++ b/go/internal/sandbox/cgroup_linux.go @@ -145,6 +145,9 @@ func SysProcAttr(cgroup *Cgroup, evidence Evidence) *syscall.SysProcAttr { } attributes.Setpgid = true + if evidence.Reason == activeAgentShellReuseReason { + return attributes + } needsFilesystemNamespace := !evidence.RequiresProcesses || len(evidence.ReadOnlyPaths) > 0 diff --git a/go/internal/sandbox/cgroup_linux_test.go b/go/internal/sandbox/cgroup_linux_test.go index a1ecc88e..e6a828c5 100644 --- a/go/internal/sandbox/cgroup_linux_test.go +++ b/go/internal/sandbox/cgroup_linux_test.go @@ -90,6 +90,34 @@ func TestProcessEnabledSandboxWithReadOnlyPinsRetainsFilesystemNamespaces( } } +func TestReusedActiveAgentShellDoesNotCreateNestedNamespaces(t *testing.T) { + t.Parallel() + + attributes := SysProcAttr(nil, Evidence{ + Enabled: true, + NamespaceEnforced: true, + Reason: activeAgentShellReuseReason, + }) + if attributes == nil { + t.Fatal("SysProcAttr() = nil") + } + if !attributes.Setpgid { + t.Fatal("reused active sandbox lost process-group isolation") + } + if attributes.Cloneflags != 0 { + t.Fatalf( + "reused active sandbox created nested namespaces: %#x", + attributes.Cloneflags, + ) + } + if len(attributes.UidMappings) != 0 || len(attributes.GidMappings) != 0 { + t.Fatalf("reused active sandbox changed identity: %#v", attributes) + } + if len(attributes.AmbientCaps) != 0 { + t.Fatalf("reused active sandbox gained capabilities: %#v", attributes.AmbientCaps) + } +} + func TestProcessEnabledSandboxWithoutReadOnlyPinsPreservesHostNamespaces(t *testing.T) { t.Parallel() diff --git a/go/internal/sandbox/sandbox.go b/go/internal/sandbox/sandbox.go index afa68fb4..4b6ec42f 100644 --- a/go/internal/sandbox/sandbox.go +++ b/go/internal/sandbox/sandbox.go @@ -12,12 +12,14 @@ import ( "path/filepath" "runtime" "strings" + "syscall" "time" "blackcat.ca/coding-ethos/go/internal/apperror" "blackcat.ca/coding-ethos/go/internal/debuglog" "blackcat.ca/coding-ethos/go/internal/processstatus" "blackcat.ca/coding-ethos/go/internal/safeexec" + "blackcat.ca/coding-ethos/go/internal/sandboxexec" ) const ( @@ -25,18 +27,20 @@ const ( BackendNative = "native" - cgroupLineParts = 3 - toolFallbackName = "tool" - nativeSandboxBinary = "coding-ethos-sandbox" - nativeProbeTimeout = 10 - nativeProbeDirMode = 0o700 - nativeProbeFileMode = 0o700 - nativeProbeWriteMode = 0o600 - SandboxTempWritePath = ".coding-ethos/cache/sandbox-tmp" - SandboxGoCachePath = ".coding-ethos/cache/go-build" - SandboxGoPath = ".coding-ethos/cache/go-path" - SandboxGoModCachePath = SandboxGoPath + "/pkg/mod" - SandboxGolangCIPath = ".coding-ethos/cache/golangci-lint" + cgroupLineParts = 3 + toolFallbackName = "tool" + nativeSandboxBinary = "coding-ethos-sandbox" + nativeProbeTimeout = 10 + nativeProbeDirMode = 0o700 + nativeProbeFileMode = 0o700 + nativeProbeWriteMode = 0o600 + nativeProbeSandboxProfile = "native-probe" + activeAgentShellReuseReason = "reusing active agent-shell sandbox" + SandboxTempWritePath = ".coding-ethos/cache/sandbox-tmp" + SandboxGoCachePath = ".coding-ethos/cache/go-build" + SandboxGoPath = ".coding-ethos/cache/go-path" + SandboxGoModCachePath = SandboxGoPath + "/pkg/mod" + SandboxGolangCIPath = ".coding-ethos/cache/golangci-lint" ) var ( @@ -206,17 +210,15 @@ func BuildPlan(request Request) (Plan, error) { evidence.Enabled = true evidence.NamespaceEnforced = !request.Capabilities.RequiresProcesses - if activeAgentShellSandboxCovers(request) { - evidence.BackendPath = strings.TrimSpace(os.Getenv("CODING_ETHOS_REAL_GIT")) - evidence.Reason = "reusing active agent-shell sandbox" - - return unsandboxedPlan(request, evidence), nil - } - if strings.TrimSpace(request.Capabilities.SeccompProfilePath) != "" { return deniedSeccompPlan(evidence, errNativeSeccompUnsupported) } + if authenticatedActiveAgentShellSandboxCovers(request) && + activeAgentShellSandboxReusable(request) { + evidence.Reason = activeAgentShellReuseReason + } + wrapper, wrapperErr := nativeWrapperPath(request) if wrapperErr != nil { return deniedSandboxPlan(evidence, wrapperErr) @@ -236,7 +238,25 @@ func sandboxRequired(request Request) bool { strings.TrimSpace(request.Capabilities.SandboxProfile) != "" } -func activeAgentShellSandboxCovers(request Request) bool { +func activeAgentShellSandboxReusable(request Request) bool { + return request.Capabilities.SandboxProfile != nativeProbeSandboxProfile && + !gitBindingRequested( + request.Capabilities, + normalizedGitTargetPaths(request.Capabilities.GitTargetPaths), + ) +} + +func authenticatedActiveAgentShellSandboxCovers(request Request) bool { + if !verifiedActiveAgentShellSandboxCovers(request) { + return false + } + + _, active, err := validateActiveAgentShellNativeRuntime(request.WrapperPath) + + return active && err == nil +} + +func verifiedActiveAgentShellSandboxCovers(request Request) bool { if os.Getenv("CODING_ETHOS_AGENT_SHELL_SANDBOX") != "1" || os.Getenv("CODING_ETHOS_SANDBOX_ACTIVE") != "1" { return false @@ -249,7 +269,8 @@ func activeAgentShellSandboxCovers(request Request) bool { return false } - if !isWithinPath(request.RepoRoot, root) || + repoRoot := filepath.Clean(strings.TrimSpace(request.RepoRoot)) + if repoRoot == "." || repoRoot != root || !isWithinPath(request.Cwd, root) || !isWithinPath(realGit, root) { return false @@ -272,13 +293,13 @@ func deniedSandboxPlan(evidence Evidence, cause error) (Plan, error) { evidence.Denied = true evidence.Reason = backendEvidenceReason(cause) - return Plan{ - Evidence: evidence, - }, fmt.Errorf( - "%w: %w", - ErrBackendUnavailable, - cause, - ) + plan := Plan{Evidence: evidence} + + return plan, fmt.Errorf( + "%w: %w", + ErrBackendUnavailable, + cause, + ) } func backendEvidenceReason(cause error) string { @@ -289,13 +310,13 @@ func deniedSeccompPlan(evidence Evidence, cause error) (Plan, error) { evidence.Denied = true evidence.Reason = cause.Error() - return Plan{ - Evidence: evidence, - }, fmt.Errorf( - "%w: %w", - ErrBackendUnavailable, - cause, - ) + plan := Plan{Evidence: evidence} + + return plan, fmt.Errorf( + "%w: %w", + ErrBackendUnavailable, + cause, + ) } func (plan Plan) Close() error { @@ -348,6 +369,11 @@ func ValidateNativeRuntimeWithHelper(wrapperPath string) (Evidence, error) { ) } + activeEvidence, active, activeErr := validateActiveAgentShellNativeRuntime(wrapper) + if active { + return activeEvidence, activeErr + } + repoRoot, err := os.MkdirTemp("", "coding-ethos-sandbox-probe-") if err != nil { return Evidence{}, fmt.Errorf("create native sandbox probe repo: %w", err) @@ -355,6 +381,11 @@ func ValidateNativeRuntimeWithHelper(wrapperPath string) (Evidence, error) { defer func() { _ = os.RemoveAll(repoRoot) }() + err = prepareNativeProbeRepo(repoRoot) + if err != nil { + return Evidence{}, err + } + plan, err := BuildPlan(Request{ Tool: "sandbox-probe", Executable: "/bin/sh", @@ -363,7 +394,7 @@ func ValidateNativeRuntimeWithHelper(wrapperPath string) (Evidence, error) { RepoRoot: repoRoot, Args: nativeProbeArgs(), Capabilities: Capabilities{ - SandboxProfile: "native-probe", + SandboxProfile: nativeProbeSandboxProfile, WritePaths: []string{".coding-ethos/cache"}, }, }) @@ -373,7 +404,7 @@ func ValidateNativeRuntimeWithHelper(wrapperPath string) (Evidence, error) { evidence, err := runNativeRuntimeProbe(repoRoot, plan) if err != nil { - return evidence, err + return evidence, fmt.Errorf("run native filesystem probe: %w", err) } evidence, err = validateNativeProbeSideEffects(repoRoot, evidence) @@ -381,7 +412,153 @@ func ValidateNativeRuntimeWithHelper(wrapperPath string) (Evidence, error) { return evidence, err } - return validateNativeGitBindProbe(repoRoot, wrapper, evidence) + evidence, err = validateNativeGitBindProbe(repoRoot, wrapper, evidence) + if err != nil { + return evidence, fmt.Errorf("run native git bind probe: %w", err) + } + + return evidence, nil +} + +func validateActiveAgentShellNativeRuntime(wrapper string) (Evidence, bool, error) { + if !activeAgentShellNativeRuntime() { + return Evidence{}, false, nil + } + + evidence := nativeRuntimeEvidence() + evidence.BackendPath = wrapper + evidence.Enabled = true + evidence.NamespaceEnforced = true + + root, validRoot := activeAgentShellSandboxRoot() + if !validRoot { + return deniedActiveAgentShellEvidence( + evidence, + "active agent-shell sandbox evidence is incomplete", + ) + } + + protectedRoot, reason := activeAgentShellProtectedRoot(root) + if reason != "" { + return deniedActiveAgentShellEvidence( + evidence, + reason, + ) + } + + reason = activeAgentShellCacheProbe(protectedRoot) + if reason != "" { + return deniedActiveAgentShellEvidence( + evidence, + reason, + ) + } + + reason = activeAgentShellProtectedRootProbe(protectedRoot) + if reason != "" { + return deniedActiveAgentShellEvidence( + evidence, + reason, + ) + } + + evidence.Reason = "validated active agent-shell native sandbox" + + return evidence, true, nil +} + +func activeAgentShellNativeRuntime() bool { + return os.Getenv("CODING_ETHOS_AGENT_SHELL_SANDBOX") == "1" && + os.Getenv("CODING_ETHOS_SANDBOX_ACTIVE") == "1" +} + +func activeAgentShellSandboxRoot() (string, bool) { + root := filepath.Clean(strings.TrimSpace(os.Getenv("CODING_ETHOS_SANDBOX_ROOT"))) + valid := root != "." && filepath.IsAbs(root) && + verifiedActiveAgentShellSandboxCovers(Request{RepoRoot: root, Cwd: root}) + + return root, valid +} + +func activeAgentShellProtectedRoot(root string) (string, string) { + mountInfo, err := os.ReadFile("/proc/self/mountinfo") + if err != nil { + return "", fmt.Sprintf("read active agent-shell mount info: %v", err) + } + + protectedRoot := filepath.Join(root, ".coding-ethos") + if !sandboxexec.ReadOnlyMountInfoForPath(string(mountInfo), protectedRoot) { + return "", "active agent-shell .coding-ethos root is not an exact read-only mount" + } + + return protectedRoot, "" +} + +func activeAgentShellCacheProbe(protectedRoot string) string { + cacheRoot := filepath.Join(protectedRoot, "cache") + + probe, err := os.CreateTemp(cacheRoot, "native-runtime-active-probe-") + if err != nil { + return fmt.Sprintf("create active agent-shell cache probe: %v", err) + } + + probePath := probe.Name() + closeErr := probe.Close() + + removeErr := os.Remove(probePath) + if closeErr != nil || removeErr != nil { + return fmt.Sprintf( + "clean active agent-shell cache probe: close=%v remove=%v", + closeErr, + removeErr, + ) + } + + return "" +} + +func activeAgentShellProtectedRootProbe(protectedRoot string) string { + blockedPath := filepath.Join(protectedRoot, "native-runtime-active-probe-blocked") + + blocked, blockedErr := os.OpenFile( + blockedPath, + os.O_CREATE|os.O_EXCL|os.O_WRONLY, + nativeProbeWriteMode, + ) + if blockedErr == nil { + _ = blocked.Close() + _ = os.Remove(blockedPath) + + return "active agent-shell protected root accepted an undeclared write" + } + + if !errors.Is(blockedErr, os.ErrPermission) && + !errors.Is(blockedErr, syscall.EROFS) { + return fmt.Sprintf("probe active agent-shell protected root: %v", blockedErr) + } + + return "" +} + +func deniedActiveAgentShellEvidence( + evidence Evidence, + reason string, +) (Evidence, bool, error) { + evidence.Denied = true + evidence.Reason = reason + + return evidence, true, fmt.Errorf("%w: %s", ErrBackendUnavailable, reason) +} + +func prepareNativeProbeRepo(repoRoot string) error { + writeRoot := filepath.Join(repoRoot, ".coding-ethos", "cache") + + err := os.MkdirAll(writeRoot, nativeProbeDirMode) + if err != nil { + return fmt.Errorf("create native sandbox probe write path: %w", err) + } + + return nil } func sandboxProbeExitCode(err error) int { @@ -494,11 +671,11 @@ func validateNativeGitBindProbe( WrapperPath: wrapper, Cwd: repoRoot, RepoRoot: repoRoot, - Args: []string{"status"}, + Args: []string{sandboxexec.NativeGitBindProbeMode}, Capabilities: Capabilities{ SandboxProfile: "native-git-bind-probe", - GitWrapperPath: probe.policyGit, - RealGitPath: probe.realGit, + GitWrapperPath: wrapper, + RealGitPath: wrapper, RealGitBindPath: probe.realGitBind, GitTargetPaths: []string{probe.targetGit}, WritePaths: []string{".coding-ethos/cache"}, @@ -518,8 +695,6 @@ func validateNativeGitBindProbe( } type nativeGitBindProbeFiles struct { - policyGit string - realGit string realGitBind string targetGit string } @@ -533,61 +708,17 @@ func nativeGitBindProbe(repoRoot string) (nativeGitBindProbeFiles, error) { } files := nativeGitBindProbeFiles{ - policyGit: filepath.Join(cacheRoot, "policy-git-probe"), - realGit: filepath.Join(cacheRoot, "real-git-probe"), realGitBind: filepath.Join(cacheRoot, "real-git-bind-probe"), targetGit: filepath.Join(cacheRoot, "target-git-probe"), } - policyScript := `#!/bin/sh -set -eu -readonly_bind=0 -while IFS= read -r line; do - set -- $line - if [ "${5:-}" = "$0" ]; then - case ",${6:-}," in - *,ro,*) readonly_bind=1 ;; - esac - fi -done < /proc/self/mountinfo -if [ "$readonly_bind" != 1 ]; then - printf not-read-only > .coding-ethos/cache/git-bind-not-read-only - exit 96 -fi -printf wrapper > .coding-ethos/cache/git-wrapper -exit 0 -` - targetScript := "#!/bin/sh\nprintf target > .coding-ethos/cache/git-target\nexit 97\n" - realGitScript := "#!/bin/sh\nexit 0\n" - - for path, content := range map[string]string{ - files.policyGit: policyScript, - files.realGit: realGitScript, - files.targetGit: targetScript, - } { - err = writeNativeProbeExecutable(path, []byte(content)) + for _, path := range []string{files.realGitBind, files.targetGit} { + err = writeNativeProbeExecutable(path, nil) if err != nil { return nativeGitBindProbeFiles{}, fmt.Errorf("write %s: %w", path, err) } } - realGitBind, err := os.OpenFile( - files.realGitBind, - os.O_CREATE|os.O_WRONLY|os.O_TRUNC, - nativeProbeFileMode, - ) - if err != nil { - return nativeGitBindProbeFiles{}, fmt.Errorf("create real git bind target: %w", err) - } - - closeErr := realGitBind.Close() - if closeErr != nil { - return nativeGitBindProbeFiles{}, fmt.Errorf( - "close real git bind target: %w", - closeErr, - ) - } - return files, nil } diff --git a/go/internal/sandbox/sandbox_internal_test.go b/go/internal/sandbox/sandbox_internal_test.go index 3d9f5f96..99a288c6 100644 --- a/go/internal/sandbox/sandbox_internal_test.go +++ b/go/internal/sandbox/sandbox_internal_test.go @@ -6,8 +6,11 @@ package sandbox import ( "errors" "os" + "os/exec" "path/filepath" "testing" + + "blackcat.ca/coding-ethos/go/internal/sandboxexec" ) func TestSafeCgroupNameSanitizesToolNames(t *testing.T) { @@ -33,7 +36,7 @@ func TestWriteNativeProbeExecutableCreatesPrivateExecutable(t *testing.T) { t.Parallel() path := filepath.Join(t.TempDir(), "probe") - err := writeNativeProbeExecutable(path, []byte("#!/bin/sh\nexit 0\n")) + err := writeNativeProbeExecutable(path, nil) if err != nil { t.Fatalf("writeNativeProbeExecutable() error = %v", err) } @@ -56,7 +59,7 @@ func TestNativeGitBindProbeCreatesExecutableFixtures(t *testing.T) { t.Fatalf("nativeGitBindProbe() error = %v", err) } - for _, path := range []string{files.policyGit, files.realGit, files.targetGit} { + for _, path := range []string{files.realGitBind, files.targetGit} { info, statErr := os.Stat(path) if statErr != nil { t.Fatalf("stat native git probe fixture %s: %v", path, statErr) @@ -64,17 +67,41 @@ func TestNativeGitBindProbeCreatesExecutableFixtures(t *testing.T) { if info.Mode().Perm() != nativeProbeFileMode { t.Fatalf("probe fixture mode for %s = %s", path, info.Mode().Perm()) } + if info.Size() != 0 { + t.Fatalf("probe fixture %s size = %d, want zero", path, info.Size()) + } } - info, statErr := os.Stat(files.realGitBind) - if statErr != nil { - t.Fatalf("stat real git bind fixture: %v", statErr) + command := exec.Command(files.targetGit, sandboxexec.NativeGitBindProbeMode) + command.Dir = repoRoot + if runErr := command.Run(); runErr == nil { + t.Fatal("unbound zero-byte git target unexpectedly executed") } - if info.Mode().Perm() != nativeProbeFileMode { + + wrapperMarker := filepath.Join(repoRoot, ".coding-ethos", "cache", "git-wrapper") + if _, statErr := os.Stat(wrapperMarker); !os.IsNotExist(statErr) { + t.Fatalf("unbound target emitted wrapper marker: %v", statErr) + } +} + +func TestPrepareNativeProbeRepoCreatesPrivateWriteRoot(t *testing.T) { + t.Parallel() + + repoRoot := t.TempDir() + if err := prepareNativeProbeRepo(repoRoot); err != nil { + t.Fatalf("prepareNativeProbeRepo() error = %v", err) + } + + writeRoot := filepath.Join(repoRoot, ".coding-ethos", "cache") + info, err := os.Stat(writeRoot) + if err != nil { + t.Fatalf("stat native probe write root: %v", err) + } + if !info.IsDir() || info.Mode().Perm() != nativeProbeDirMode { t.Fatalf( - "real git bind fixture mode = %s, want %o", - info.Mode().Perm(), - nativeProbeFileMode, + "native probe write root = %s, want private directory %o", + info.Mode(), + nativeProbeDirMode, ) } } diff --git a/go/internal/sandbox/sandbox_test.go b/go/internal/sandbox/sandbox_test.go index 35933fbe..15e2d1a3 100644 --- a/go/internal/sandbox/sandbox_test.go +++ b/go/internal/sandbox/sandbox_test.go @@ -384,7 +384,7 @@ func TestBuildPlanRejectsAgentShellRealGitBindOutsideRepo(t *testing.T) { } } -func TestBuildPlanReusesVerifiedActiveAgentShellSandbox(t *testing.T) { +func TestBuildPlanDoesNotReuseActiveAgentShellForDescendantRepo(t *testing.T) { requireLinuxSandbox(t) root := t.TempDir() @@ -428,12 +428,181 @@ func TestBuildPlanReusesVerifiedActiveAgentShellSandbox(t *testing.T) { if err != nil { t.Fatalf("sandbox.BuildPlan() error = %v", err) } - if plan.Executable != "/usr/bin/env" || - plan.Evidence.Reason != "reusing active agent-shell sandbox" { - t.Fatalf("active agent shell sandbox was not reused: %#v", plan) + if plan.Executable != wrapper || + plan.Evidence.Reason == "reusing active agent-shell sandbox" { + t.Fatalf("descendant repo reused active agent shell sandbox: %#v", plan) + } + for _, want := range []string{"--write-path", "--", "/usr/bin/env"} { + if !slices.Contains(plan.Args, want) { + t.Fatalf("descendant plan skipped native helper argument %q: %#v", want, plan.Args) + } } if !plan.Evidence.Enabled || !plan.Evidence.RepoReadOnly { - t.Fatalf("active agent shell reuse lost sandbox evidence: %#v", plan.Evidence) + t.Fatalf("descendant plan lost sandbox evidence: %#v", plan.Evidence) + } +} + +func TestBuildPlanDoesNotReuseSpoofedExactRootAgentShellSandbox(t *testing.T) { + requireLinuxSandbox(t) + + root := t.TempDir() + wrapper := filepath.Join(root, "bin", "coding-ethos-sandbox") + realGitBind := filepath.Join( + root, + ".coding-ethos", + "cache", + "agent-shell", + "run-1", + "real-git", + ) + if err := os.MkdirAll(filepath.Dir(realGitBind), 0o700); err != nil { + t.Fatalf("create real git bind dir: %v", err) + } + writeExecutable(t, wrapper) + writeExecutable(t, realGitBind) + + t.Setenv("CODING_ETHOS_AGENT_SHELL_SANDBOX", "1") + t.Setenv("CODING_ETHOS_SANDBOX_ACTIVE", "1") + t.Setenv("CODING_ETHOS_SANDBOX_ROOT", root) + t.Setenv("CODING_ETHOS_REAL_GIT", realGitBind) + + plan, err := sandbox.BuildPlan(sandbox.Request{ + Tool: "ruff", + Executable: "/usr/bin/env", + WrapperPath: wrapper, + Cwd: root, + RepoRoot: root, + Args: []string{"true"}, + Capabilities: sandbox.Capabilities{ + SandboxProfile: "lint-offline", + RequiresProcesses: true, + WritePaths: []string{".coding-ethos/cache"}, + }, + }) + if err != nil { + t.Fatalf("sandbox.BuildPlan() error = %v", err) + } + if plan.Executable != wrapper || + plan.Evidence.Reason == "reusing active agent-shell sandbox" { + t.Fatalf("spoofed exact-root agent shell sandbox was reused: %#v", plan) + } + for _, want := range []string{"--write-path", "--", "/usr/bin/env"} { + if !slices.Contains(plan.Args, want) { + t.Fatalf( + "spoofed exact-root plan skipped native helper argument %q: %#v", + want, + plan.Args, + ) + } + } +} + +func TestBuildPlanDoesNotReuseActiveAgentShellForNewGitBinds(t *testing.T) { + requireLinuxSandbox(t) + + root := t.TempDir() + repo := filepath.Join(root, sandbox.SandboxTempWritePath, "git-bind-probe") + wrapper := filepath.Join(root, "bin", "coding-ethos-sandbox") + realGitBind := filepath.Join(repo, ".coding-ethos", "cache", "real-git") + targetGit := filepath.Join(repo, ".coding-ethos", "cache", "target-git") + activeRealGit := filepath.Join( + root, + ".coding-ethos", + "cache", + "agent-shell", + "run-1", + "real-git", + ) + for _, path := range []string{repo, filepath.Dir(realGitBind), filepath.Dir(activeRealGit)} { + if err := os.MkdirAll(path, 0o700); err != nil { + t.Fatalf("create git bind test directory %s: %v", path, err) + } + } + for _, path := range []string{wrapper, realGitBind, targetGit, activeRealGit} { + writeExecutable(t, path) + } + + t.Setenv("CODING_ETHOS_AGENT_SHELL_SANDBOX", "1") + t.Setenv("CODING_ETHOS_SANDBOX_ACTIVE", "1") + t.Setenv("CODING_ETHOS_SANDBOX_ROOT", root) + t.Setenv("CODING_ETHOS_REAL_GIT", activeRealGit) + + plan, err := sandbox.BuildPlan(sandbox.Request{ + Tool: "sandbox-git-bind-probe", + Executable: targetGit, + WrapperPath: wrapper, + Cwd: repo, + RepoRoot: repo, + Args: []string{"probe"}, + Capabilities: sandbox.Capabilities{ + SandboxProfile: "native-git-bind-probe", + GitWrapperPath: wrapper, + RealGitPath: wrapper, + RealGitBindPath: realGitBind, + GitTargetPaths: []string{targetGit}, + WritePaths: []string{".coding-ethos/cache"}, + RequiresGit: true, + }, + }) + if err != nil { + t.Fatalf("sandbox.BuildPlan() error = %v", err) + } + if plan.Executable != wrapper || + plan.Evidence.Reason == "reusing active agent-shell sandbox" { + t.Fatalf("git bind request reused active sandbox: %#v", plan) + } + for _, want := range []string{"--git-wrapper", "--real-git-path", "--git-target"} { + if !slices.Contains(plan.Args, want) { + t.Fatalf("git bind plan missing %q: %#v", want, plan.Args) + } + } +} + +func TestBuildPlanDoesNotReuseActiveAgentShellForNativeProbe(t *testing.T) { + requireLinuxSandbox(t) + + root := t.TempDir() + repo := filepath.Join(root, sandbox.SandboxTempWritePath, "native-probe") + wrapper := filepath.Join(root, "bin", "coding-ethos-sandbox") + activeRealGit := filepath.Join( + root, + ".coding-ethos", + "cache", + "agent-shell", + "run-1", + "real-git", + ) + for _, path := range []string{repo, filepath.Dir(activeRealGit)} { + if err := os.MkdirAll(path, 0o700); err != nil { + t.Fatalf("create native probe test directory %s: %v", path, err) + } + } + writeExecutable(t, wrapper) + writeExecutable(t, activeRealGit) + + t.Setenv("CODING_ETHOS_AGENT_SHELL_SANDBOX", "1") + t.Setenv("CODING_ETHOS_SANDBOX_ACTIVE", "1") + t.Setenv("CODING_ETHOS_SANDBOX_ROOT", root) + t.Setenv("CODING_ETHOS_REAL_GIT", activeRealGit) + + plan, err := sandbox.BuildPlan(sandbox.Request{ + Tool: "sandbox-probe", + Executable: "/bin/sh", + WrapperPath: wrapper, + Cwd: repo, + RepoRoot: repo, + Args: []string{"-c", "exit 0"}, + Capabilities: sandbox.Capabilities{ + SandboxProfile: "native-probe", + WritePaths: []string{".coding-ethos/cache"}, + }, + }) + if err != nil { + t.Fatalf("sandbox.BuildPlan() error = %v", err) + } + if plan.Executable != wrapper || + plan.Evidence.Reason == "reusing active agent-shell sandbox" { + t.Fatalf("native validator probe reused active sandbox: %#v", plan) } } diff --git a/go/internal/sandboxexec/filesystem_linux.go b/go/internal/sandboxexec/filesystem_linux.go index 655fd79d..34a060cd 100644 --- a/go/internal/sandboxexec/filesystem_linux.go +++ b/go/internal/sandboxexec/filesystem_linux.go @@ -35,13 +35,18 @@ var ( errGitTargetRequired = errors.New("at least one git target is required") errExecutableAbsolute = errors.New("executable path must be absolute") errExecutableInvalid = errors.New("executable path is not executable") + errNativeGitBindProbe = errors.New( + "native git bind probe executable is not an exact read-only mount", + ) ) const ( - privateDirMode = 0o700 - writableFileMode = 0o600 - mountInfoFieldCount = 6 - landlockWriteAccess = unix.LANDLOCK_ACCESS_FS_WRITE_FILE | + privateDirMode = 0o700 + writableFileMode = 0o600 + mountInfoFieldCount = 6 + nativeGitBindProbeMarker = ".coding-ethos/cache/git-wrapper" + nativeGitBindProbeFailureMarker = ".coding-ethos/cache/git-bind-not-read-only" + landlockWriteAccess = unix.LANDLOCK_ACCESS_FS_WRITE_FILE | unix.LANDLOCK_ACCESS_FS_REMOVE_DIR | unix.LANDLOCK_ACCESS_FS_REMOVE_FILE | unix.LANDLOCK_ACCESS_FS_MAKE_CHAR | @@ -57,6 +62,69 @@ const ( unix.LANDLOCK_ACCESS_FS_TRUNCATE ) +func runNativeGitBindProbe() error { + repoRoot, err := os.Getwd() + if err != nil { + return fmt.Errorf("resolve native git bind probe root: %w", err) + } + + target, err := filepath.Abs(os.Args[0]) + if err != nil { + return fmt.Errorf("resolve native git bind probe executable: %w", err) + } + + mountInfo, err := os.ReadFile("/proc/self/mountinfo") + if err != nil { + return fmt.Errorf("read native git bind probe mount info: %w", err) + } + + return validateNativeGitBindProbeMount(repoRoot, target, string(mountInfo)) +} + +func validateNativeGitBindProbeMount(repoRoot, target, mountInfo string) error { + if !readOnlyMountInfoForPath(mountInfo, target) { + err := writeNativeGitBindFailureMarker(repoRoot) + if err != nil { + return err + } + + return errNativeGitBindProbe + } + + err := writeNativeGitBindSuccessMarker(repoRoot) + if err != nil { + return err + } + + return nil +} + +func writeNativeGitBindFailureMarker(repoRoot string) error { + err := os.WriteFile( + filepath.Join(repoRoot, nativeGitBindProbeFailureMarker), + []byte("not-read-only\n"), + writableFileMode, + ) + if err != nil { + return fmt.Errorf("write native git bind failure marker: %w", err) + } + + return nil +} + +func writeNativeGitBindSuccessMarker(repoRoot string) error { + err := os.WriteFile( + filepath.Join(repoRoot, nativeGitBindProbeMarker), + []byte("wrapper\n"), + writableFileMode, + ) + if err != nil { + return fmt.Errorf("write native git bind success marker: %w", err) + } + + return nil +} + func applyFilesystemPolicy(options options) error { err := applyGitBindMounts(options) if err != nil { @@ -311,6 +379,12 @@ func readOnlyMountInfoForPath(content, path string) bool { return false } +// ReadOnlyMountInfoForPath reports whether path is an exact read-only Linux +// mountpoint in the supplied mountinfo content. +func ReadOnlyMountInfoForPath(content, path string) bool { + return readOnlyMountInfoForPath(content, path) +} + func applyGitBindMounts(options options) error { if strings.TrimSpace(options.gitWrapper) == "" && len(options.gitTargets) == 0 { return nil diff --git a/go/internal/sandboxexec/filesystem_other.go b/go/internal/sandboxexec/filesystem_other.go index 440783d7..db02eed9 100644 --- a/go/internal/sandboxexec/filesystem_other.go +++ b/go/internal/sandboxexec/filesystem_other.go @@ -5,6 +5,21 @@ package sandboxexec +import "errors" + +var errNativeGitBindProbeUnsupported = errors.New( + "native git bind probe requires Linux mount namespaces", +) + func applyFilesystemPolicy(options options) error { return nil } + +func runNativeGitBindProbe() error { + return errNativeGitBindProbeUnsupported +} + +// ReadOnlyMountInfoForPath is false where Linux mountinfo is unavailable. +func ReadOnlyMountInfoForPath(_, _ string) bool { + return false +} diff --git a/go/internal/sandboxexec/main.go b/go/internal/sandboxexec/main.go index 8e45702f..c54ea405 100644 --- a/go/internal/sandboxexec/main.go +++ b/go/internal/sandboxexec/main.go @@ -26,6 +26,8 @@ const ( sandboxExecFailureExitCode = 126 sandboxExecCommandName = "coding-ethos-sandbox" sandboxExecErrorPrefix = "coding-ethos-sandbox:" + // NativeGitBindProbeMode runs the compiled helper's fixed-path bind probe. + NativeGitBindProbeMode = "--internal-native-git-bind-probe" ) type repeatedPaths []string @@ -80,6 +82,10 @@ func Run(args []string) int { } func run(args []string) error { + if nativeGitBindProbeInvocation(args) { + return runNativeGitBindProbe() + } + config, err := parseOptions(args) if err != nil { return err @@ -98,6 +104,10 @@ func run(args []string) error { return execSandboxedCommand(config) } +func nativeGitBindProbeInvocation(args []string) bool { + return len(args) == 1 && args[0] == NativeGitBindProbeMode +} + func parseOptions(args []string) (options, error) { var ( parsed = options{paths: &sandboxPaths{}} diff --git a/go/internal/sandboxexec/main_test.go b/go/internal/sandboxexec/main_test.go index 4b5e08ca..dc643bfc 100644 --- a/go/internal/sandboxexec/main_test.go +++ b/go/internal/sandboxexec/main_test.go @@ -76,6 +76,85 @@ func TestRunReturnsFailureForInvalidInvocation(t *testing.T) { } } +func TestNativeGitBindProbeInvocationRequiresExactNoArgumentMode(t *testing.T) { + t.Parallel() + + if !nativeGitBindProbeInvocation([]string{NativeGitBindProbeMode}) { + t.Fatal("exact native git bind probe mode was not recognized") + } + for _, args := range [][]string{ + nil, + {}, + {NativeGitBindProbeMode, "extra"}, + {"other"}, + } { + if nativeGitBindProbeInvocation(args) { + t.Fatalf("non-exact native git bind probe invocation accepted: %#v", args) + } + } +} + +func TestValidateNativeGitBindProbeMountWritesOnlyFixedSuccessMarker(t *testing.T) { + t.Parallel() + + repoRoot := t.TempDir() + cacheRoot := filepath.Join(repoRoot, ".coding-ethos", "cache") + if err := os.MkdirAll(cacheRoot, privateDirMode); err != nil { + t.Fatalf("create probe cache: %v", err) + } + target := filepath.Join(cacheRoot, "target-git-probe") + mountInfo := "1 2 0:1 / " + target + " ro,relatime - tmpfs tmpfs rw\n" + + if err := validateNativeGitBindProbeMount(repoRoot, target, mountInfo); err != nil { + t.Fatalf("validateNativeGitBindProbeMount() error = %v", err) + } + + marker := filepath.Join(repoRoot, nativeGitBindProbeMarker) + content, err := os.ReadFile(marker) + if err != nil { + t.Fatalf("read fixed native git bind marker: %v", err) + } + if strings.TrimSpace(string(content)) != "wrapper" { + t.Fatalf("native git bind marker = %q", content) + } + if _, err = os.Stat( + filepath.Join(repoRoot, nativeGitBindProbeFailureMarker), + ); !os.IsNotExist( + err, + ) { + t.Fatalf("read-only bind emitted failure marker: %v", err) + } +} + +func TestValidateNativeGitBindProbeMountRejectsWritableSelfBind(t *testing.T) { + t.Parallel() + + repoRoot := t.TempDir() + cacheRoot := filepath.Join(repoRoot, ".coding-ethos", "cache") + if err := os.MkdirAll(cacheRoot, privateDirMode); err != nil { + t.Fatalf("create probe cache: %v", err) + } + target := filepath.Join(cacheRoot, "target-git-probe") + mountInfo := "1 2 0:1 / " + target + " rw,relatime - tmpfs tmpfs rw\n" + + err := validateNativeGitBindProbeMount(repoRoot, target, mountInfo) + if !errors.Is(err, errNativeGitBindProbe) { + t.Fatalf("validateNativeGitBindProbeMount() error = %v, want read-only failure", err) + } + if _, err = os.Stat( + filepath.Join(repoRoot, nativeGitBindProbeMarker), + ); !os.IsNotExist( + err, + ) { + t.Fatalf("writable bind emitted success marker: %v", err) + } + if _, err = os.Stat( + filepath.Join(repoRoot, nativeGitBindProbeFailureMarker), + ); err != nil { + t.Fatalf("writable bind did not emit fixed failure marker: %v", err) + } +} + func TestExecSandboxedCommandReportsStartFailure(t *testing.T) { t.Parallel() diff --git a/go/internal/syncstate/state.go b/go/internal/syncstate/state.go index 52b46d2f..5806fc5a 100644 --- a/go/internal/syncstate/state.go +++ b/go/internal/syncstate/state.go @@ -31,6 +31,9 @@ const ( artifactStatusPlanned = "planned" artifactStatusUnchanged = "unchanged" artifactStatusWouldUpdate = "would_update" + artifactPlanNone = "none" + artifactPlanRepair = "repair" + artifactPlanWrite = "write" sourceStatusCurrent = "current" sourceStatusMissing = "missing" sourceStatusStale = "stale" @@ -399,19 +402,19 @@ func artifactReports( switch { case err != nil: report.Status = artifactStatusMissing - report.Plan = "write" + report.Plan = artifactPlanWrite case actual == artifact.ExpectedSHA256: report.ActualSHA256 = actual report.Status = artifactStatusUnchanged - report.Plan = "none" + report.Plan = artifactPlanNone case dryRun: report.ActualSHA256 = actual report.Status = artifactStatusWouldUpdate - report.Plan = "write" + report.Plan = artifactPlanWrite default: report.ActualSHA256 = actual report.Status = artifactStatusDrifted - report.Plan = "repair" + report.Plan = artifactPlanRepair } reports = append(reports, report) @@ -474,7 +477,7 @@ func plannedWriteCount(artifacts []ArtifactReport) int { count := 0 for _, artifact := range artifacts { - if artifact.Plan == "write" || artifact.Plan == "repair" { + if artifact.Plan == artifactPlanWrite || artifact.Plan == artifactPlanRepair { count++ } } diff --git a/pre-commit/PRE-COMMIT.md b/pre-commit/PRE-COMMIT.md index 77396139..577d8312 100644 --- a/pre-commit/PRE-COMMIT.md +++ b/pre-commit/PRE-COMMIT.md @@ -25,7 +25,7 @@ make cutover-verify In a consuming repo, run the same target from `coding-ethos/`. When `coding-ethos/` is a submodule, the root `Makefile` resolves the parent -repo automatically and installs hooks into the parent repo's `.git/hooks`. +repo automatically and installs hooks into Git's resolved hooks directory. Before the hook entrypoints are installed, `make install-hooks` also generates the consumer repo's `pyrightconfig.json`, `mypy.ini`, `ruff.toml`, `.pylintrc`, @@ -33,11 +33,12 @@ consumer repo's `pyrightconfig.json`, `mypy.ini`, `ruff.toml`, `.pylintrc`, `.coding-ethos/gemini/prompt-pack.json` from the shared bundle inputs plus any consuming-repo overrides. -`make install-hooks` installs `.git/hooks/pre-commit`, `pre-push`, and -`commit-msg` as small executable scripts that call -`bin/coding-ethos-run git-hook ` with the original Git arguments. The -runner repairs missing checkout-local runtime artifacts with `make build` and -dispatches to the built hook binary under `coding-ethos/bin/`. +`make install-hooks` installs `pre-commit`, `pre-push`, and `commit-msg` in +Git's resolved hooks directory as small executable scripts that call +`/coding-ethos-hooks/bin/coding-ethos-run git-hook ` with +the original Git arguments. Missing or stale runtime artifacts fail closed +with an explicit `make build` instruction; hook execution never builds or +repairs them implicitly. `make cutover-install` installs the Git hook entrypoints, syncs Claude, Codex, and Gemini repo-local agent hook settings, and then verifies the full cutover @@ -175,12 +176,19 @@ The selected `coding-ethos` authority builds the hook runtime: The current runtime model is documented in `docs/HOOK_RUNTIME_BOOTSTRAP.md`: installed consumer hooks are generated runner entrypoint scripts that dispatch to the stable Git-common -`.git/coding-ethos-hooks/` projection. `parent-install` atomically refreshes and -hash-verifies its compiled Go executables from the selected authority, while +`/coding-ethos-hooks/` projection. `parent-install` atomically +refreshes and hash-verifies its compiled Go executables from the selected authority, while `make build` refreshes the complete policy and toolchain projection. Hook execution therefore does not depend on the lifetime or visibility of one worktree checkout. +The compiled `parent-runtime-sync` command exclusively maintains the shared Go +executable inventory, including the root compatibility Git hook. It prunes +obsolete regular `coding-ethos-*` files, preserves unrelated files, rejects +managed symlinks/directories/special files, and installs same-directory +temporary files by mode-preserving atomic rename. Both local and consumer hook +entrypoints use the stable common runner. + The same wrapper also exposes local policy-runtime entrypoints: ```bash diff --git a/pre-commit/hooks/HOOKS.md b/pre-commit/hooks/HOOKS.md index 43113c19..ebee82c5 100644 --- a/pre-commit/hooks/HOOKS.md +++ b/pre-commit/hooks/HOOKS.md @@ -7,13 +7,22 @@ Go-backed Git hooks for coding-ethos bundles. Installed consumer repository shims are intentionally thin. They dispatch to the compiled runner under the repository's stable Git-common -`.git/coding-ethos-hooks/bin/` projection. The selected `coding-ethos` +`/coding-ethos-hooks/bin/` projection. The selected `coding-ethos` authority builds that projection; `parent-install` atomically refreshes and hash-verifies its Go executables, and `make build` refreshes the complete policy and toolchain runtime. Policy selection and strict freshness checks remain in compiled Coding Ethos code rather than in the shim, and no hook points at a worktree-local build path. +The compiled `parent-runtime-sync` command is the sole installer for shared Go +executables, including the root compatibility Git hook. It preserves unrelated +files, prunes obsolete regular `coding-ethos-*` files, rejects managed entries +with unsafe filesystem shapes, and publishes mode-preserving copies through a +same-directory temporary file plus atomic rename. Both the selected checkout's +resolved hook directory and consumer hook directories dispatch to this stable +common runner. Runtime absence or drift is repaired explicitly with +`make build`, never during hook execution. + The Go runner is the output-control layer for Git hooks. It supports `hooks.output_format` values of `auto`, `human`, `json`, and `toon`; `auto` selects TOON when known agent/LLM environment markers are present. Successful diff --git a/repo_config.example.yaml b/repo_config.example.yaml index a3bf0436..21553404 100644 --- a/repo_config.example.yaml +++ b/repo_config.example.yaml @@ -57,7 +57,7 @@ hooks: output_format: auto success_output: silent parallel_groups: true - tool_timeout_seconds: 900 + tool_timeout_seconds: 600 similarity: enabled: true diff --git a/repo_ethos.yml b/repo_ethos.yml index 855b9a1f..0c112568 100644 --- a/repo_ethos.yml +++ b/repo_ethos.yml @@ -214,6 +214,7 @@ principles: diff.added_lines.exists(line, has_prefix(line.file, "go/internal/") && has_suffix(line.file, ".go") && + !has_suffix(line.file, "_test.go") && !has_prefix(line.file, "go/internal/feedback/") && ( line.text.contains("fmt.Fprint") || diff --git a/tests/test_makefile_contract_agent_skill_sync_is_not_user_facing.py b/tests/test_makefile_contract_agent_skill_sync_is_not_user_facing.py index 9b04d6e1..717de60f 100644 --- a/tests/test_makefile_contract_agent_skill_sync_is_not_user_facing.py +++ b/tests/test_makefile_contract_agent_skill_sync_is_not_user_facing.py @@ -30,8 +30,8 @@ def test_agent_skill_sync_is_not_user_facing() -> None: in makefile ) assert ( - "_sync-parent-hook-runtime: ensure-go go-tools-install policy-bundle-install\n" - in makefile + "_sync-parent-hook-runtime: ensure-go go-tools-install " + "go-hook-runner-install policy-bundle-install\n" in makefile ) for target in ( "_sync-agent-skills", diff --git a/tests/test_makefile_contract_diagnostics.py b/tests/test_makefile_contract_diagnostics.py index 44688d2e..e91bd621 100644 --- a/tests/test_makefile_contract_diagnostics.py +++ b/tests/test_makefile_contract_diagnostics.py @@ -30,6 +30,15 @@ def test_tests_and_diagnostics_do_not_build_or_install_runtime() -> None: "check-tool-configs", "check-gemini-prompts", "check-agent-skills", + "check-provider-matrix", + "parent-check", + "cutover-verify", + "pre-commit", + "pre-commit-all", + "pre-push", + "commit-msg", + "hook-plan", + "validate", "go-test", "lint", "fix", @@ -61,6 +70,46 @@ def test_tests_and_diagnostics_do_not_build_or_install_runtime() -> None: ) +def test_go_e2e_package_timeout_has_bounded_full_gate_budget() -> None: + """Keep package-level Go E2E timeouts bounded for full-gate load.""" + makefile, _lines = _makefile_lines() + + assert "GO_TEST_TIMEOUT ?= 3m" in makefile + for target in ("go-e2e-test", "go-tools-coverage"): + assert '-timeout="$(GO_TEST_TIMEOUT)"' in _target_block(makefile, target) + + +def test_normal_runtime_commands_require_prebuilt_artifacts() -> None: + """Require normal runtime targets to consume prebuilt artifacts.""" + makefile, _lines = _makefile_lines() + + for target in ( + "parent-install", + "check-provider-matrix", + "parent-check", + "parent-lint", + "cutover-verify", + "pre-commit", + "pre-commit-all", + "pre-push", + "commit-msg", + "hook-plan", + "validate", + ): + target_line = next( + line for line in makefile.splitlines() if line.startswith(f"{target}:") + ) + assert "ensure-hook-runtime" in target_line + assert "build" not in target_line + assert "go-tools-install" not in target_line + + runtime_guard = _target_block(makefile, "ensure-hook-runtime") + for tool in ("coding-ethos-agent-hooks", "coding-ethos-policy"): + assert tool in runtime_guard + assert "Managed tool is missing" in runtime_guard + assert "make build" in runtime_guard + + def test_check_blocks_unmanaged_go_module_root_binaries() -> None: makefile, _lines = _makefile_lines() @@ -80,3 +129,69 @@ def test_check_blocks_unmanaged_go_module_root_binaries() -> None: clean_block = _target_block(makefile, "go-tools-clean") assert "$(GO_MODULE_ROOT_BINARY_OUTPUTS)" in clean_block assert 'rm -f "$(GO_TOOLS_DIR)/$$name"' in clean_block + + +def test_purrdf_extractor_uses_repository_target_directory() -> None: + """Keep PurRDF extractor artifacts in its repository target directory.""" + makefile, _lines = _makefile_lines() + + for target in ("purrdf-extractor-check", "purrdf-extractor-install"): + target_body = _target_block(makefile, target) + assert ( + "--config 'build.build-dir=\"$(PURRDF_EXTRACTOR_DIR)/target\"'" + in target_body + ) + assert '--target-dir "$(PURRDF_EXTRACTOR_DIR)/target"' in target_body + + +def test_agent_hook_sync_uses_explicit_writable_state_roots() -> None: + """Give agent-hook sync explicit checkout-local writable state roots.""" + makefile, _lines = _makefile_lines() + + assert "AGENT_HOOK_STATE_ROOT ?= $(LOCAL_BUILD_DIR)/agent-hooks-state" in makefile + assert ( + "CONSUMER_AGENT_HOOK_STATE_ROOT ?= " + "$(LOCAL_BUILD_DIR)/consumer-agent-hooks-state" in makefile + ) + + local_sync = _target_block(makefile, "_sync-agent-hooks") + assert 'CODEX_HOME="$(AGENT_HOOK_STATE_ROOT)/codex-home"' in local_sync + assert '--repo-root "$(REPO)"' in local_sync + assert '--state-root "$(AGENT_HOOK_STATE_ROOT)"' in local_sync + + consumer_sync = _target_block(makefile, "_sync-consumer-agent-hooks") + assert 'CODEX_HOME="$(CONSUMER_AGENT_HOOK_STATE_ROOT)/codex-home"' in consumer_sync + assert '--repo-root "$(HOOK_CONSUMER_ROOT)"' in consumer_sync + assert '--state-root "$(CONSUMER_AGENT_HOOK_STATE_ROOT)"' in consumer_sync + + +def test_parent_hook_runtime_executables_use_atomic_compiled_sync() -> None: + """Project parent hook executables through the atomic compiled sync path.""" + makefile, _lines = _makefile_lines() + + runtime_sync_target = next( + line + for line in makefile.splitlines() + if line.startswith("_sync-parent-hook-runtime:") + ) + runtime_sync = _target_block(makefile, "_sync-parent-hook-runtime") + git_hook_sync = _target_block(makefile, "_sync-git-hooks") + + assert "go-hook-runner-install" in runtime_sync_target + assert ( + '"$(GO_HOOK)" parent-runtime-sync --repo "$(HOOK_CONSUMER_ROOT)"' + in runtime_sync + ) + assert ( + "$(call install_git_hooks,$(LOCAL_HOOKS_DIR)," + "$(PARENT_HOOK_BIN_DIR)/coding-ethos-run)" in git_hook_sync + ) + + for forbidden in ( + 'cp "$(GO_TOOLS_BIN_DIR)"/coding-ethos-*', + 'cp "$(GO_TOOLS_BIN_DIR)/cerun"', + 'cp "$(GO_TOOLS_BIN_DIR)/lint"', + 'cp "$(GO_TOOLS_BIN_DIR)/coding-ethos-git-hook"', + "$(call install_git_hooks,$(LOCAL_HOOKS_DIR),$(GO_HOOK))", + ): + assert forbidden not in makefile