diff --git a/.github/actions/checkout-relay/action.yml b/.github/actions/checkout-relay/action.yml deleted file mode 100644 index 8d5de1993..000000000 --- a/.github/actions/checkout-relay/action.yml +++ /dev/null @@ -1,44 +0,0 @@ -name: Checkout ecosystem -description: Clone graycode-eco sibling repos into the workspace parent for reflect's go.work - -inputs: - ref: - description: Git ref to checkout (falls back to main) - required: false - default: main - -runs: - using: composite - steps: - - name: Clone sibling repos - shell: bash - env: - # Passed via env (not ${{ }} interpolation) so an attacker-controlled - # ref (e.g. a fork branch name) cannot inject shell. - INPUT_REF: ${{ inputs.ref }} - run: | - set -euo pipefail - # The generated workspace references sibling repos from ecosystem.yaml. - # Repositories that no longer resolve are skipped with a warning so a - # single-repo checkout still yields a working CI run (expected during - # ecosystem migration; downstream guards skip absent checkouts too). - ws_parent="$(cd "${GITHUB_WORKSPACE}/.." && pwd)" - while IFS= read -r repo; do - [ "$repo" = reflect ] && continue - dest="${ws_parent}/${repo}" - if [ -d "$dest/.git" ]; then - echo "$repo already present at $dest" - continue - fi - if ! git ls-remote "https://github.com/GrayCodeAI/${repo}.git" HEAD >/dev/null 2>&1; then - echo "WARNING: GrayCodeAI/${repo} is not reachable; skipping checkout" - continue - fi - if git ls-remote --heads "https://github.com/GrayCodeAI/${repo}.git" "$INPUT_REF" | grep -q .; then - echo "Cloning $repo at branch $INPUT_REF" - git clone --depth=1 --branch "$INPUT_REF" "https://github.com/GrayCodeAI/${repo}.git" "$dest" - else - echo "Branch '$INPUT_REF' not found on $repo, falling back to main" - git clone --depth=1 --branch main "https://github.com/GrayCodeAI/${repo}.git" "$dest" - fi - done < <(./scripts/ecosystem-manifest.sh list workspace) diff --git a/.github/actions/setup-deps/action.yml b/.github/actions/setup-deps/action.yml index bf68af73d..2221a26fe 100644 --- a/.github/actions/setup-deps/action.yml +++ b/.github/actions/setup-deps/action.yml @@ -1,52 +1,23 @@ name: Setup Dependencies -description: Clone ecosystem sibling repos with retry, setup Go, sync workspace +description: Set up Go and download the independently published module graph inputs: go-version: description: Go version to install - default: '1.27.2' + default: '1.27.3' token: - description: GitHub token for authenticated clones - required: true + description: Retained for compatibility; public modules need no clone token + required: false runs: using: composite steps: - - name: Checkout sibling dependencies - shell: bash - env: - GH_TOKEN: ${{ inputs.token }} - run: | - clone_with_retry() { - local repo=$1 dest=$2 branch=${3:-main} - for i in 1 2 3; do - git clone --depth=1 --branch "$branch" "https://x-access-token:${GH_TOKEN}@github.com/GrayCodeAI/${repo}.git" "$dest" && return 0 - echo "Retry $i for $repo..." && sleep $((i * 5)) - done - echo "Failed to clone $repo after 3 attempts" && return 1 - } - ws_parent="$(cd "${GITHUB_WORKSPACE}/.." && pwd)" - mkdir -p "$ws_parent" - while IFS= read -r repo; do - [ "$repo" = reflect ] && continue - dest="$ws_parent/$repo" - if [ -d "$dest/.git" ]; then - echo "$repo already present at $dest" - continue - fi - if ! git ls-remote "https://x-access-token:${GH_TOKEN}@github.com/GrayCodeAI/${repo}.git" HEAD >/dev/null 2>&1; then - echo "WARNING: GrayCodeAI/${repo} is not reachable; skipping checkout" - continue - fi - clone_with_retry "$repo" "$dest" main - done < <(./scripts/ecosystem-manifest.sh list workspace) - - name: Set up Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ inputs.go-version }} - - - name: Sync workspace + - name: Download published dependencies shell: bash - run: | - make workspace + env: + GOWORK: 'off' + run: go mod download diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a5e633cc..ebcfc58f8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,16 +24,8 @@ env: # published on the public proxy. Direct-VCS fallback is retained for anything # else. GOPROXY: "https://proxy.golang.org,direct" - # CI must resolve the dependency versions pinned in go.mod/go.sum, not whatever - # happens to sit in the sibling checkouts that the committed go.work points at. - # Go's reference is explicit that a committed go.work "may cause a continuous - # integration (CI) system to select and thus test the wrong versions of a - # module's dependencies" and that "CI systems should generally not be allowed to - # use the go.work file". Because ./.github/actions/checkout-relay clones the - # siblings at branch HEAD into the workspace parent, workspace mode would silently - # hide version skew (e.g. a shared pin that no longer matches). - # The `module` job opts back in (GOWORK: "auto") since `go work sync` and the - # go.work content checks genuinely require the workspace. + # Build and test the independently published dependency graph. Optional + # local Relay development uses go.local.mod, never a parent workspace. GOWORK: "off" jobs: @@ -100,55 +92,25 @@ jobs: run: bash ./scripts/check-relay-engine-boundary.sh # ------------------------------------------------------------------------- - # 2. Module hygiene — tidy, verify Reflect plus the sibling Go modules via go.work. + # 2. Module hygiene — validate the published Reflect dependency graph. # ------------------------------------------------------------------------- module: name: module hygiene runs-on: ubuntu-latest - # Workspace hygiene is this job's entire purpose (`go work sync`, the go.work - # content checks), so it deliberately re-enables the workspace that the - # workflow-level GOWORK: "off" disables. The steps here that must see the - # pinned versions already set GOWORK=off inline. - env: - GOWORK: "auto" steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - - name: Generate ecosystem workspace - run: bash ./scripts/generate-workspace.sh - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ env.GO_VERSION }} cache: true - - name: go work sync + module consistency + - name: Published module consistency run: | - # Sibling repos are independent checkouts in the workspace parent. - # go mod tidy can mis-resolve workspace modules here; go work sync is - # the supported workspace hygiene step. - go work sync + go mod tidy -diff + go mod verify go build -mod=readonly -o /dev/null ./cmd/reflect - if ! git diff --quiet -- go.mod go.sum go.work go.work.sum; then - echo "::error::go.mod / go.sum / go.work files out of date — run 'go work sync' locally and commit" - git diff -- go.mod go.sum go.work go.work.sum - exit 1 - fi - - name: go mod verify - run: go mod verify - - name: workspace references sibling checkouts - run: | - workspace_file="${GITHUB_WORKSPACE}/../go.work" - while IFS= read -r module; do - [ "$module" = reflect ] && continue - if ! grep -q "./${module}" "$workspace_file"; then - echo "::error::go.work must include ./${module}." - cat "$workspace_file" - exit 1 - fi - done < <(./scripts/ecosystem-manifest.sh list workspace) + git diff --exit-code -- go.mod go.sum - name: Validate every Go module independently run: | while IFS= read -r modfile; do @@ -166,9 +128,6 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ env.GO_VERSION }} @@ -183,15 +142,13 @@ jobs: go test ./... -count=1 -timeout=300s release-parity: + # Retain the required status-check name configured on main. name: workspace and module parity runs-on: ubuntu-latest steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ env.GO_VERSION }} @@ -208,9 +165,6 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ env.GO_VERSION }} @@ -233,9 +187,6 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ env.GO_VERSION }} @@ -262,9 +213,6 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ env.GO_VERSION }} @@ -349,9 +297,6 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ env.GO_VERSION }} @@ -438,9 +383,6 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c with: go-version: "${{ env.GO_VERSION }}" @@ -493,9 +435,6 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ env.GO_VERSION }} @@ -523,9 +462,6 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ env.GO_VERSION }} @@ -556,9 +492,6 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: ${{ env.GO_VERSION }} diff --git a/.github/workflows/compatibility-matrix.yml b/.github/workflows/compatibility-matrix.yml index 4ba106bb9..2f0843b34 100644 --- a/.github/workflows/compatibility-matrix.yml +++ b/.github/workflows/compatibility-matrix.yml @@ -17,15 +17,15 @@ on: permissions: contents: read +env: + GOWORK: "off" + jobs: validate: name: validate compatibility matrix runs-on: ubuntu-latest steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - uses: ./.github/actions/checkout-relay - with: - ref: ${{ github.head_ref || github.ref_name }} - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: "1.27.2" diff --git a/.gitignore b/.gitignore index f623ef5f6..3c73f1728 100644 --- a/.gitignore +++ b/.gitignore @@ -40,7 +40,7 @@ coverage.html CODE_REVIEW_REPORT.md review_report.md -# Workspace checksum file — regenerated by `make setup` / `go work sync`. +# Personal workspace checksums are never part of the published build. go.work.sum # Local direnv file (may hold personal keys for `make test-live`). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c9d633fd9..ac3777392 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -39,11 +39,22 @@ Repo-specific dev targets: | Target | What it does | | ------------------ | ------------------------------------------------------------------- | -| `make setup` | Set up local development environment (go.work + external repos). | +| `make setup` | Install local development tools; use published dependencies. | | `make boundaries` | Alias for all boundary guards (matches `make boundaries` in engine repos). | | `make test-10x` | Run tests 10 times to surface flakes. | | `make smoke` | Quick build + doctor + ecosystem verification. | +Reflect builds independently with the published Relay dependency in `go.mod`. +For coordinated Relay changes, create an optional local overlay: + +```bash +make workspace RELAY_DIR=/path/to/relay +GOWORK=off go test -modfile=go.local.mod ./... +``` + +The overlay stays gitignored. Run `make ci` against the published dependency +before release. Setup and CI never generate a parent `go.work`. + ## Commit message convention We use [Conventional Commits](https://www.conventionalcommits.org/). Pull diff --git a/Makefile b/Makefile index 36e7f9b43..7b49d92df 100644 --- a/Makefile +++ b/Makefile @@ -5,6 +5,8 @@ # --------------------------------------------------------------------------- # Project metadata # --------------------------------------------------------------------------- +export GOWORK := off + NAME := reflect MAIN_PKG := ./cmd/reflect @@ -170,14 +172,14 @@ security: ## Run govulncheck. @command -v $(GOVULNCHECK) >/dev/null 2>&1 || (echo "install: go install golang.org/x/vuln/cmd/govulncheck@latest" && exit 1) $(GOVULNCHECK) ./... -tidy: ## Sync workspace modules and verify checksums. - go work sync +tidy: ## Tidy the published module graph and verify checksums. + go mod tidy go mod verify # --------------------------------------------------------------------------- # Composite gate used by CI and pre-push. # --------------------------------------------------------------------------- -ci: version-check tidy fmt-check vet boundaries lint test-race security api-validate ## Run everything CI runs. +ci: version-check module-check fmt-check vet boundaries lint test-race security api-validate ## Run everything CI runs. @echo "All CI checks passed." smoke: ## Quick build + doctor + ecosystem verification. @@ -201,12 +203,12 @@ clean: ## Remove build artefacts. # --------------------------------------------------------------------------- # Setup — bootstrap local development environment. # --------------------------------------------------------------------------- -workspace: ## Regenerate the ecosystem root go.work from ecosystem.yaml. - @bash ./scripts/generate-workspace.sh +workspace: ## Create an optional local Relay overlay (RELAY_DIR=/path/to/relay). + @bash ./scripts/generate-workspace.sh "$(RELAY_DIR)" -setup: workspace ## Set up local development environment and development tools. +setup: ## Set up local development environment and development tools. @echo "=== Setting up reflect development environment ===" - @echo "✓ go.work generated and synced from ecosystem.yaml" + @echo "Using published dependencies from go.mod (no sibling checkout required)" @echo "" @echo "=== Environment check ===" @echo "Go version: $$(go version)" @@ -251,8 +253,7 @@ hooks: ## Install git hooks via lefthook (formatting, linting, conventional comm @command -v lefthook >/dev/null 2>&1 || (echo "install: go install github.com/evilmartians/lefthook@latest" && exit 1) lefthook install -sync: ## Sync the workspace go.work and verify sibling release parity. - @go work sync +sync: tidy ## Sync published modules and verify release parity. @bash ./scripts/check-module-release-parity.sh # === Cross-platform binary targets (add after existing 'build' target) === @@ -291,3 +292,8 @@ version-sync: ## Regenerate package/API metadata from VERSION. version-check: ## Reject stale generated version metadata. python3 scripts/sync-version.py --check + +.PHONY: module-check +module-check: ## Check the published module graph without rewriting it. + go mod tidy -diff + go mod verify diff --git a/README.md b/README.md index ae70e0492..6abf3e137 100644 --- a/README.md +++ b/README.md @@ -513,11 +513,13 @@ three runtime layers plus optional tooling/platform services: - **Tooling/platform:** **graycode-platform** contains the optional web/BFF/Reflect Cloud plane and is outside the Reflect Go runtime graph. -Local development uses: +Reflect builds and tests independently against the published Relay version in +`go.mod`. Setup and CI require only this repository. -- **`go.mod` modules:** pinned requirements for `relay` -- **Workspace + `go.work`:** sibling support repos are cloned in the `graycode-eco` workspace (as `../`); `go.work` resolves the module paths to those local checkouts -- **Module-mode builds:** standalone builds resolve the pinned `go.mod` versions from the module proxy (no workspace) +For coordinated Relay development, run `make workspace RELAY_DIR=/path/to/relay` +to create a gitignored `go.local.mod` with an explicit local replacement, then run +`GOWORK=off go test -modfile=go.local.mod ./...`. Existing overlays are preserved. +Verify the normal published dependency graph separately with `make ci`. Cross-repo contracts now live in `internal/contracts` (vendored from the removed `github.com/GrayCodeAI/eagle` module) so support repos do not depend @@ -535,7 +537,7 @@ Current contract packages (`internal/contracts/`): - `events` — tool, trace, and usage events - `harness` — harness evaluation reports and dimension scores -You may keep a **personal** parent **`go.work`** that lists alternate clones on disk for multi-repo development. +Reflect does not create or modify parent workspaces. | Component | Repository | Purpose | |---|---|---| diff --git a/cmd/chat_model.go b/cmd/chat_model.go index a7bb5e9b4..ac0fb6376 100644 --- a/cmd/chat_model.go +++ b/cmd/chat_model.go @@ -323,9 +323,8 @@ type chatModel struct { backgrounded bool // terminal lost focus during current turn (for completion notification) notifiedComplete bool // completion notification was sent this turn (prevent duplicates) uiFocus uiFocusArea - contentLines int // total lines in scrollback content (for footer position) - lastMouseY int // last pointer row (0-based); -1 = unknown; used when Cursor reports stale wheel Y - lastMouseX int + contentLines int // total lines in scrollback content (for footer position) + lastMouseY int // last pointer row (0-based); -1 = unknown; used when Cursor reports stale wheel Y mouseOverride *bool // runtime /mouse toggle; persisted via settings vim *VimState wal sessionWAL diff --git a/cmd/chat_update.go b/cmd/chat_update.go index 34bce0fd4..ffd3b4cf9 100644 --- a/cmd/chat_update.go +++ b/cmd/chat_update.go @@ -688,7 +688,6 @@ func (m chatModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.viewDirty = true m.updateViewportContent() return m, nil - } } } diff --git a/cmd/chat_welcome.go b/cmd/chat_welcome.go index 1ac4b8739..1791e5849 100644 --- a/cmd/chat_welcome.go +++ b/cmd/chat_welcome.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "image/color" - "os/exec" "sort" "strings" @@ -303,15 +302,6 @@ func apiKeyStatusLines() []string { return lines } -// currentGitBranch returns the current git branch name, or "" if not in a repo. -func currentGitBranch() string { - out, err := exec.CommandContext(context.Background(), "git", "rev-parse", "--abbrev-ref", "HEAD").Output() - if err != nil { - return "" - } - return strings.TrimSpace(string(out)) -} - func displayConfigValue(value string) string { if strings.TrimSpace(value) == "" { return "(empty)" diff --git a/docs/DEVELOPER-PATH.md b/docs/DEVELOPER-PATH.md index 08ac7709b..de84434db 100644 --- a/docs/DEVELOPER-PATH.md +++ b/docs/DEVELOPER-PATH.md @@ -24,26 +24,21 @@ reflect preflight ## Setup checklist -### 1. Build and workspace setup +### 1. Build setup -If you are contributing from source, clone Reflect as the main CLI. The support -repositories are independent sibling checkouts when you need the full local -workspace; they are not nested under Reflect: +Reflect is an independent repository. Clone it and use its published dependencies: ```bash -mkdir graycode-eco && cd graycode-eco git clone https://github.com/GrayCodeAI/reflect -git clone https://github.com/GrayCodeAI/relay cd reflect make setup -go build -o reflect ./cmd/reflect +make build ``` -`make setup` validates the canonical repository manifest and regenerates the -parent `../go.work` from the local Go repositories marked `workspace: true`. -Reflect can also be built as a standalone checkout with `GOWORK=off go build ./cmd/reflect`; -the sibling workspace is only required for cross-repository development and -boundary checks. +For coordinated Relay changes, use `make workspace RELAY_DIR=/path/to/relay` +and `GOWORK=off go test -modfile=go.local.mod ./...`. This creates a gitignored +local module overlay and preserves any parent workspace. Ordinary builds and +CI use the pinned `go.mod` dependency; no sibling repository is required. ### 2. Configure credentials diff --git a/docs/DYNAMIC-MODELS.md b/docs/DYNAMIC-MODELS.md index 507cf1eed..4ff4455e1 100644 --- a/docs/DYNAMIC-MODELS.md +++ b/docs/DYNAMIC-MODELS.md @@ -111,10 +111,10 @@ settings entry. 2. Add Relay tests for cache and live discovery, credential status, selection, and generation/streaming. 3. Commit and verify standalone Relay. -4. Advance Reflect's `../relay` sibling checkout to that exact commit, then update - Reflect's module version when the Relay revision is published. -5. Verify both the workspace (`go.work`) and published-module - (`GOWORK=off`) build modes. +4. For coordinated work, point Reflect's gitignored `go.local.mod` at the Relay + checkout and verify with `GOWORK=off go test -modfile=go.local.mod ./...`. +5. Publish Relay, update Reflect's module pin, and separately verify the normal + published-module build (`GOWORK=off`). Reflect changes are needed only for a new product behavior or an additive Reflect-owned presentation field—not for provider-specific mechanics. diff --git a/docs/ECOSYSTEM-WIRING.md b/docs/ECOSYSTEM-WIRING.md index 087542a7e..df69f9c6a 100644 --- a/docs/ECOSYSTEM-WIRING.md +++ b/docs/ECOSYSTEM-WIRING.md @@ -43,7 +43,7 @@ durable Queue-delivery intent. ```mermaid flowchart TB Manifest[GrayCodeAI/graycode-eco/ecosystem.yaml\ncanonical repo and contract inventory] - Manifest --> Workspace[generated root go.work] + Manifest --> Modules[go.mod pins and optional go.local.mod] Manifest --> Guards[boundary and release parity guards] Manifest --> OwlSnapshot[owl/ecosystem.json] @@ -124,17 +124,18 @@ must use `directory`/`github_repo`; UI copy may use `product_name`. ## Release and development workflow -- Run `make workspace` in Reflect to regenerate the parent `go.work`. +- Normal setup and CI use the published module graph. For coordinated Relay + work, run `make workspace RELAY_DIR=/path/to/relay` to create `go.local.mod`. - Run `make boundaries` to validate the inventory, module isolation, Eagle parity, facade locations, and support-repository coupling. -- Root `go.mod` files never contain local `replace` directives. Local sibling - substitutions exist only in the generated, uncommitted parent workspace. +- Root `go.mod` files never contain local `replace` directives. Local Relay + substitutions exist only in the gitignored `go.local.mod` overlay. - SDK CI checks its snapshot byte-for-byte against Reflect and separately checks that every OpenAPI path has a support decision. - Run `owl/scripts/sync-ecosystem.sh` after changing the canonical inventory; Owl CI rejects drift. -- Standalone/module-mode tests remain the release truth; workspace tests are an - additional integration pass, not a substitute. +- Published-module tests remain the release truth; tests with + `-modfile=go.local.mod` are an additional integration pass. ### Coordinated Go publication gate diff --git a/docs/architecture.md b/docs/architecture.md index c997587d5..dc49b37a1 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -121,5 +121,5 @@ Tool Call → " + strings.TrimPrefix(path, seg) + _, suffix, found := strings.Cut(strings.TrimPrefix(path, seg), "/") + if found { + return seg + "/" + suffix + } + return seg + "" } } return path diff --git a/internal/daemon/redaction_regression_test.go b/internal/daemon/redaction_regression_test.go new file mode 100644 index 000000000..44660c2ef --- /dev/null +++ b/internal/daemon/redaction_regression_test.go @@ -0,0 +1,15 @@ +package daemon + +import "testing" + +func TestCredentialPathRedaction(t *testing.T) { + for input, want := range map[string]string{ + "/botFAKE_AUDIT_TOKEN/sendMessage": "/bot/sendMessage", + "/botFAKE_AUDIT_TOKEN": "/bot", + "/v1/status": "/v1/status", + } { + if got := redactURLPath(input); got != want { + t.Errorf("redactURLPath(%q) = %q, want %q", input, got, want) + } + } +} diff --git a/internal/engine/search/url_scraper.go b/internal/engine/search/url_scraper.go index da2462c82..a0350e206 100644 --- a/internal/engine/search/url_scraper.go +++ b/internal/engine/search/url_scraper.go @@ -113,18 +113,16 @@ func (s *URLScraper) Fetch(ctx context.Context, rawURL string) (*ScrapeResult, e // SSRF guard: reject private/link-local targets and pin the resolved IP // to prevent DNS rebinding. Without this, a scraped URL could reach cloud // metadata endpoints or internal services. - pinnedURL, origHost, err := toolsafety.ValidateURLPublic(ctx, rawURL) + validatedURL, _, err := toolsafety.ValidateURLPublic(ctx, rawURL) if err != nil { return nil, err } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, pinnedURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, validatedURL, nil) if err != nil { return nil, fmt.Errorf("creating request: %w", err) } - if origHost != "" { - req.Host = origHost - } + req.Header.Set("User-Agent", s.UserAgent) req.Header.Set("Accept", "text/html, application/json, text/plain, */*") diff --git a/internal/engine/tool_service.go b/internal/engine/tool_service.go index a52de3bd9..528568028 100644 --- a/internal/engine/tool_service.go +++ b/internal/engine/tool_service.go @@ -400,6 +400,7 @@ func bool2tag(isErr bool) string { // retry, and raw execution. PostProcess and CompleteResult // own the remaining result lifecycle. func (s *ToolService) ExecuteOne(ctx context.Context, tc types.ToolCall, override tool.Tool, ch chan<- StreamEvent, turn int, intent string) toolExecResult { + tc.Arguments = tool.WorkspaceArguments(canonicalToolName(tc.Name), tc.Arguments, s.WorkingDir()) result := toolExecResult{tc: tc, state: ToolStateValidating} summary := toolActivitySummary(tc.Name, tc.Arguments) if s.deps.redactOutput != nil { diff --git a/internal/mcp/mcp.go b/internal/mcp/mcp.go index 84a4334e2..651a7c739 100644 --- a/internal/mcp/mcp.go +++ b/internal/mcp/mcp.go @@ -90,7 +90,7 @@ func Connect(ctx context.Context, name, command string, args ...string) (*Server } // ctx bounds the handshake, not the session. Startup cancels its discovery // context as soon as tools are loaded; Close owns the child after that. - cmd := exec.Command(command, args...) // #nosec G204 -- command comes from the configured MCP server definition + cmd := exec.CommandContext(context.WithoutCancel(ctx), command, args...) // #nosec G204 -- configured MCP executable; Close owns its lifetime after the handshake stdin, err := cmd.StdinPipe() if err != nil { return nil, fmt.Errorf("mcp: stdin pipe: %w", err) diff --git a/internal/plugin/dynamic.go b/internal/plugin/dynamic.go index 68d398964..6e7b03401 100644 --- a/internal/plugin/dynamic.go +++ b/internal/plugin/dynamic.go @@ -426,6 +426,9 @@ func (dm *DynamicPluginManager) InstallFromGitHub(repo string) error { // Uninstall deactivates a plugin and removes it from disk. func (dm *DynamicPluginManager) Uninstall(name string) error { + if !manifestNamePattern.MatchString(name) { + return fmt.Errorf("invalid plugin name %q", name) + } // Deactivate first if active dm.mu.RLock() dp, exists := dm.plugins[name] @@ -456,7 +459,7 @@ func (dm *DynamicPluginManager) Uninstall(name string) error { return fmt.Errorf("plugin %q not found on disk", name) } - if err := os.RemoveAll(pluginPath); err != nil { + if err := removePluginDir(pluginPath, dm.pluginDirs); err != nil { return fmt.Errorf("remove plugin directory: %w", err) } diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go index bb5ab8504..8f17a351d 100644 --- a/internal/plugin/plugin.go +++ b/internal/plugin/plugin.go @@ -220,8 +220,42 @@ func criticalManifestIssues(m *Manifest) []string { // Uninstall removes a plugin. func Uninstall(name string) error { + if !manifestNamePattern.MatchString(name) { + return fmt.Errorf("invalid plugin name %q", name) + } dir := filepath.Join(pluginsDir(), name) - return os.RemoveAll(dir) + return removePluginDir(dir, []string{pluginsDir()}) +} + +// removePluginDir accepts only a direct child of an installation root. Pinning +// that root also prevents removal from following a swapped parent symlink. +func removePluginDir(path string, roots []string) error { + target, err := filepath.Abs(path) + if err != nil { + return err + } + for _, dir := range roots { + base, err := filepath.Abs(dir) + if err != nil { + continue + } + rel, err := filepath.Rel(base, target) + if err != nil || rel == "." || rel == ".." || filepath.Base(rel) != rel { + continue + } + unlock, err := installtxn.Lock(base) + if err != nil { + return err + } + defer unlock() + root, err := os.OpenRoot(base) + if err != nil { + return err + } + defer func() { _ = root.Close() }() + return root.RemoveAll(rel) + } + return fmt.Errorf("plugin path %q is outside installation roots", path) } // Summary returns a formatted summary of installed plugins. diff --git a/internal/plugin/provider.go b/internal/plugin/provider.go index 627a50cde..cd817daf4 100644 --- a/internal/plugin/provider.go +++ b/internal/plugin/provider.go @@ -246,9 +246,7 @@ func SkillRoots(cwd string) []string { // Plugin skill roots follow the ordinary project/user skill locations. // InstalledComponents applies the same trust and scope rules as MCP tools. for _, components := range InstalledComponents(cwd) { - for _, dir := range components.Skills { - roots = append(roots, dir) - } + roots = append(roots, components.Skills...) } return roots } diff --git a/internal/plugin/removal_regression_test.go b/internal/plugin/removal_regression_test.go new file mode 100644 index 000000000..f330e30b8 --- /dev/null +++ b/internal/plugin/removal_regression_test.go @@ -0,0 +1,29 @@ +package plugin + +import ( + "os" + "path/filepath" + "testing" +) + +func TestUninstallRejectsTraversalAndEmptyNames(t *testing.T) { + root := t.TempDir() + plugins, other := filepath.Join(root, "plugins"), filepath.Join(root, "other") + for _, path := range []string{plugins, other} { + if err := os.MkdirAll(path, 0o700); err != nil { + t.Fatal(err) + } + } + manager := NewDynamicPluginManager([]string{plugins}, nil, nil) + for _, name := range []string{"../other", "..", ".", "", other, "..\\" + "other"} { + if err := manager.Uninstall(name); err == nil { + t.Errorf("dynamic uninstall accepted %q", name) + } + if err := Uninstall(name); err == nil { + t.Errorf("uninstall accepted %q", name) + } + } + if _, err := os.Stat(other); err != nil { + t.Fatalf("sibling was removed: %v", err) + } +} diff --git a/internal/securitylog/securitylog.go b/internal/securitylog/securitylog.go index 906379c35..f9051ef07 100644 --- a/internal/securitylog/securitylog.go +++ b/internal/securitylog/securitylog.go @@ -16,7 +16,9 @@ import ( "sync" "time" + "github.com/GrayCodeAI/reflect/internal/installtxn" "github.com/GrayCodeAI/reflect/internal/storage" + "github.com/gofrs/flock" ) // EventSeverity classifies the impact of a security event. @@ -46,6 +48,7 @@ const ( keyFileName = "sel.key" logFileName = "security_events.jsonl" headFileName = "sel.head" + lockFileName = "sel.lock" keySize = 32 genesisHash = "" // the first entry links against the empty string ) @@ -68,6 +71,7 @@ type Log struct { seq uint64 last string closed bool + lock *flock.Flock } // DefaultDir returns the default on-disk location for the security event log, @@ -91,7 +95,12 @@ func New(dir string) (*Log, error) { dir: dir, path: filepath.Join(dir, logFileName), keyPath: filepath.Join(dir, keyFileName), + lock: flock.New(filepath.Join(dir, lockFileName)), } + if err := l.lock.Lock(); err != nil { + return nil, fmt.Errorf("securitylog: lock: %w", err) + } + defer func() { _ = l.lock.Unlock() }() if err := l.loadKey(); err != nil { return nil, err @@ -111,10 +120,21 @@ func New(dir string) (*Log, error) { // loadKey reads an existing HMAC key or generates a fresh one. func (l *Log) loadKey() error { data, err := os.ReadFile(l.keyPath) - if err == nil && len(data) == keySize { + if err == nil { + if len(data) != keySize { + return fmt.Errorf("securitylog: invalid key size %d", len(data)) + } l.key = data return nil } + if !os.IsNotExist(err) { + return fmt.Errorf("securitylog: read key: %w", err) + } + if info, statErr := os.Stat(l.path); statErr == nil && info.Size() > 0 { + return fmt.Errorf("securitylog: existing log is missing its key") + } else if statErr != nil && !os.IsNotExist(statErr) { + return fmt.Errorf("securitylog: inspect log: %w", statErr) + } key := make([]byte, keySize) if _, err := rand.Read(key); err != nil { return fmt.Errorf("securitylog: generate key: %w", err) @@ -130,9 +150,13 @@ func (l *Log) loadKey() error { // tail hash so appends continue the chain without gaps. It also reads the head // pointer so a truncated tail is caught at append time, not silently extended. func (l *Log) loadChain() error { + l.seq, l.last = 0, genesisHash data, err := os.ReadFile(l.path) if err != nil { if os.IsNotExist(err) { + if head, headErr := l.readHead(); headErr == nil && head.Seq != 0 { + return fmt.Errorf("securitylog: log is missing but head pointer exists") + } return nil } return fmt.Errorf("securitylog: read log: %w", err) @@ -145,6 +169,9 @@ func (l *Log) loadChain() error { if err := json.Unmarshal(line, &ev); err != nil { return fmt.Errorf("securitylog: corrupt entry: %w", err) } + if ev.Seq != l.seq+1 || ev.PrevHash != l.last || l.computeHash(ev) != ev.Hash { + return fmt.Errorf("securitylog: corrupt chain at entry %d", l.seq) + } l.seq = ev.Seq l.last = ev.Hash } @@ -153,6 +180,8 @@ func (l *Log) loadChain() error { if head.Seq != l.seq || head.Hash != l.last { return fmt.Errorf("securitylog: log tail does not match head pointer (truncated or tampered)") } + } else if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("securitylog: read head: %w", err) } return nil } @@ -174,7 +203,7 @@ func (l *Log) writeHead() error { if err != nil { return err } - return os.WriteFile(filepath.Join(l.dir, headFileName), data, 0o600) + return installtxn.WriteFileAtomically(filepath.Join(l.dir, headFileName), data, 0o600) } // Append records a new event and returns the linked entry. It is safe for @@ -185,10 +214,18 @@ func (l *Log) Append(severity EventSeverity, eventType, detail, tool, sessionID if l.closed { return Event{}, fmt.Errorf("securitylog: log closed") } + if err := l.lock.Lock(); err != nil { + return Event{}, fmt.Errorf("securitylog: lock: %w", err) + } + defer func() { _ = l.lock.Unlock() }() + // Another handle or process may have appended since this handle opened. + // Refresh and validate under the same lock that protects the append/head. + if err := l.loadChain(); err != nil { + return Event{}, err + } - l.seq++ ev := Event{ - Seq: l.seq, + Seq: l.seq + 1, Timestamp: time.Now().UTC(), Severity: severity, Type: eventType, @@ -206,6 +243,10 @@ func (l *Log) Append(severity EventSeverity, eventType, detail, tool, sessionID if _, err := l.f.Write(append(line, '\n')); err != nil { return Event{}, fmt.Errorf("securitylog: append: %w", err) } + if err := l.f.Sync(); err != nil { + return Event{}, fmt.Errorf("securitylog: sync: %w", err) + } + l.seq = ev.Seq l.last = ev.Hash if err := l.writeHead(); err != nil { return Event{}, fmt.Errorf("securitylog: write head: %w", err) @@ -227,6 +268,18 @@ func (l *Log) computeHash(ev Event) string { // and each entry chains to the previous one. It returns the number of entries // verified and an error naming the first break in the chain. func Verify(dir string) (int, error) { + if _, err := os.Stat(dir); os.IsNotExist(err) { + return 0, nil + } + lock := flock.New(filepath.Join(dir, lockFileName)) + if err := lock.Lock(); err != nil { + return 0, fmt.Errorf("securitylog: lock verification: %w", err) + } + defer func() { _ = lock.Unlock() }() + return verifyUnlocked(dir) +} + +func verifyUnlocked(dir string) (int, error) { data, err := os.ReadFile(filepath.Join(dir, logFileName)) if err != nil { if os.IsNotExist(err) { diff --git a/internal/securitylog/writers_regression_test.go b/internal/securitylog/writers_regression_test.go new file mode 100644 index 000000000..fecfb3022 --- /dev/null +++ b/internal/securitylog/writers_regression_test.go @@ -0,0 +1,104 @@ +package securitylog + +import ( + "os" + "os/exec" + "path/filepath" + "sync" + "testing" +) + +func TestIndependentWritersShareChain(t *testing.T) { + dir := t.TempDir() + first, err := New(dir) + if err != nil { + t.Fatal(err) + } + defer func() { _ = first.Close() }() + second, err := New(dir) + if err != nil { + t.Fatal(err) + } + defer func() { _ = second.Close() }() + var wg sync.WaitGroup + for _, writer := range []*Log{first, second} { + wg.Add(1) + go func() { + defer wg.Done() + for range 10 { + if _, err := writer.Append(SeverityInfo, "test", "fixture", "", ""); err != nil { + t.Error(err) + } + } + }() + } + wg.Wait() + if count, err := Verify(dir); err != nil || count != 20 { + t.Fatalf("count=%d error=%v", count, err) + } +} + +func TestIndependentWriterProcessesShareChain(t *testing.T) { + dir := t.TempDir() + var children []*exec.Cmd + for range 3 { + child := exec.CommandContext(t.Context(), os.Args[0], "-test.run=^TestSecurityLogWriterProcess$") + child.Env = append(os.Environ(), "REFLECT_SECURITYLOG_TEST_DIR="+dir) + if err := child.Start(); err != nil { + t.Fatal(err) + } + children = append(children, child) + } + for _, child := range children { + if err := child.Wait(); err != nil { + t.Error(err) + } + } + if count, err := Verify(dir); err != nil || count != 15 { + t.Fatalf("count=%d error=%v", count, err) + } +} + +func TestSecurityLogWriterProcess(t *testing.T) { + dir := os.Getenv("REFLECT_SECURITYLOG_TEST_DIR") + if dir == "" { + t.Skip("subprocess fixture") + } + writer, err := New(dir) + if err != nil { + t.Fatal(err) + } + defer func() { _ = writer.Close() }() + for range 5 { + if _, err := writer.Append(SeverityInfo, "test", "process fixture", "", ""); err != nil { + t.Fatal(err) + } + } +} + +func TestCorruptLogIsPreservedAndNotExtended(t *testing.T) { + dir := t.TempDir() + writer, err := New(dir) + if err != nil { + t.Fatal(err) + } + defer func() { _ = writer.Close() }() + if _, err := writer.Append(SeverityInfo, "test", "fixture", "", ""); err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, logFileName) + corrupt := []byte("{invalid chain}\n") + if err := os.WriteFile(path, corrupt, 0o600); err != nil { + t.Fatal(err) + } + if _, err := writer.Append(SeverityInfo, "test", "must not append", "", ""); err == nil { + t.Fatal("append accepted a corrupt chain") + } + if reopened, err := New(dir); err == nil { + _ = reopened.Close() + t.Fatal("opening a corrupt chain succeeded") + } + if got, _ := os.ReadFile(path); string(got) != string(corrupt) { + t.Fatal("corrupt evidence was rewritten") + } +} diff --git a/internal/testaudit/standalone_development_test.go b/internal/testaudit/standalone_development_test.go new file mode 100644 index 000000000..b4c6a8359 --- /dev/null +++ b/internal/testaudit/standalone_development_test.go @@ -0,0 +1,73 @@ +package testaudit + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" +) + +func TestLocalRelayOverlayPreservesParentWorkspace(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("bash development helper is covered on Linux and macOS") + } + root := t.TempDir() + repo, relay := filepath.Join(root, "reflect"), filepath.Join(root, "relay with spaces") + scriptDir := filepath.Join(repo, "scripts") + for _, dir := range []string{scriptDir, relay} { + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + } + script, err := os.ReadFile(filepath.Join(repoRoot(t), "scripts", "generate-workspace.sh")) + if err != nil { + t.Fatal(err) + } + files := map[string]string{ + filepath.Join(scriptDir, "generate-workspace.sh"): string(script), + filepath.Join(repo, "go.mod"): "module example.com/fixture\n\ngo 1.27.2\n", + filepath.Join(repo, "go.sum"): "", + filepath.Join(relay, "go.mod"): "module github.com/GrayCodeAI/relay\n\ngo 1.27.2\n", + filepath.Join(root, "go.work"): "parent workspace sentinel\n", + filepath.Join(root, "go.work.sum"): "parent checksums sentinel\n", + } + for path, content := range files { + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + run := func(args ...string) error { + cmd := exec.CommandContext(t.Context(), "bash", append([]string{filepath.Join(scriptDir, "generate-workspace.sh")}, args...)...) + cmd.Dir = repo + output, err := cmd.CombinedOutput() + if err != nil { + t.Logf("overlay helper: %s", output) + } + return err + } + if err := run(); err == nil { + t.Fatal("optional overlay must require an explicit Relay checkout") + } + if err := run(relay); err != nil { + t.Fatal(err) + } + overlayPath := filepath.Join(repo, "go.local.mod") + overlay, err := os.ReadFile(overlayPath) + if err != nil || !strings.Contains(string(overlay), relay) { + t.Fatalf("local replacement missing: %s, %v", overlay, err) + } + if err := run(relay); err == nil { + t.Fatal("existing local overlay was overwritten") + } + if after, _ := os.ReadFile(overlayPath); string(after) != string(overlay) { + t.Fatal("existing overlay changed") + } + for _, name := range []string{"go.work", "go.work.sum"} { + path := filepath.Join(root, name) + if got, _ := os.ReadFile(path); string(got) != files[path] { + t.Fatalf("parent %s was changed", name) + } + } +} diff --git a/internal/tool/bash.go b/internal/tool/bash.go index d8a0b5556..5803191b0 100644 --- a/internal/tool/bash.go +++ b/internal/tool/bash.go @@ -363,6 +363,9 @@ func (BashTool) Execute(ctx context.Context, input json.RawMessage) (string, err if tc := GetToolContext(ctx); tc != nil && tc.ReadOnlyBash && p.RunInBackground { return "", fmt.Errorf("blocked (explore/plan read-only bash): run_in_background is not allowed") } + if tc := GetToolContext(ctx); tc != nil && tc.ReadOnlyBash { + p.Command = readOnlyGitPrelude + p.Command + } // Apply per-tool timeout from safety config, allow explicit override. timeout := time.Duration(p.Timeout) * time.Second @@ -381,7 +384,10 @@ func (BashTool) Execute(ctx context.Context, input json.RawMessage) (string, err if sbCfg.Image == "" { sbCfg.Image = sandbox.DefaultImage } - if sandbox.Enabled(sbCfg) && !p.RunInBackground { + if sandbox.Enabled(sbCfg) && p.RunInBackground { + return "", fmt.Errorf("background commands are unavailable while sandboxing is enabled; run the command in the foreground") + } + if sandbox.Enabled(sbCfg) { workdir := "" if tc := GetToolContext(ctx); tc != nil { workdir = tc.WorkingDir diff --git a/internal/tool/bash_explore.go b/internal/tool/bash_explore.go index 581ea2b4d..f8b4ab92c 100644 --- a/internal/tool/bash_explore.go +++ b/internal/tool/bash_explore.go @@ -3,56 +3,49 @@ package tool import ( "fmt" "strings" - "unicode" ) // exploreBashAllow is the first-token allowlist for explore/plan (read-only) modes. // Anything not listed is denied — fail-closed. var exploreBashAllow = map[string]bool{ - // listing / inspection "ls": true, "pwd": true, "dirname": true, "basename": true, - "cat": true, "head": true, "tail": true, "less": true, "more": true, - "wc": true, "file": true, "stat": true, "du": true, "df": true, - "which": true, "type": true, "command": true, "env": true, "printenv": true, - "echo": true, "printf": true, "true": true, "false": true, "test": true, "[": true, - // search - "grep": true, "egrep": true, "fgrep": true, "rg": true, "ag": true, "ack": true, - "find": true, "locate": true, - // text utilities (read-side) - "sort": true, "uniq": true, "cut": true, "tr": true, "awk": true, "sed": true, - "jq": true, "yq": true, "column": true, "comm": true, - "diff": true, "cmp": true, - // VCS (subcommand gated below) - "git": true, "gh": true, - // language tooling (read-ish) - "go": true, "node": true, "python": true, "python3": true, "ruby": true, - "cargo": true, "rustc": true, "javac": true, "java": true, - "npm": true, "pnpm": true, "yarn": true, "pip": true, "pip3": true, - // system info - "uname": true, "whoami": true, "id": true, "date": true, "hostname": true, - "sysctl": true, "sw_vers": true, - // network read - "curl": true, "wget": true, "dig": true, "nslookup": true, "ping": true, - // tree / pretty - "tree": true, "bat": true, "hexdump": true, "od": true, "xxd": true, + "cat": true, "head": true, "tail": true, "wc": true, + "file": true, "stat": true, "du": true, "df": true, + "which": true, "type": true, "echo": true, "printf": true, + "true": true, "false": true, "test": true, "[": true, + "grep": true, "egrep": true, "fgrep": true, "rg": true, + "find": true, "sort": true, "cut": true, + "tr": true, "column": true, "comm": true, "diff": true, "cmp": true, + "git": true, "go": true, "uname": true, "whoami": true, + "id": true, "sw_vers": true, "tree": true, + "hexdump": true, "od": true, } -// git subcommands allowed under explore/plan (no mutation). -var exploreGitAllow = map[string]bool{ - "status": true, "log": true, "show": true, "diff": true, "branch": true, - "rev-parse": true, "rev-list": true, "describe": true, "ls-files": true, - "ls-tree": true, "cat-file": true, "blame": true, "shortlog": true, - "remote": true, "tag": true, "stash": true, // stash list only enforced loosely - "config": true, "help": true, "version": true, "grep": true, - "name-rev": true, "symbolic-ref": true, "for-each-ref": true, +// Repository-configured diff/textconv/fsmonitor helpers are executable code. +// Keep ordinary git inspection available without allowing those callbacks to +// mutate files in a read-only session. The wrapper works in both bash and the +// POSIX shell used by sandbox backends. Validation allows only bare git calls. +const readOnlyGitPrelude = `git() { + case "$1" in + -C) (cd -- "$2" && shift 2 && git "$@") ;; + --no-pager) shift; git "$@" ;; + --literal-pathspecs) shift; GIT_LITERAL_PATHSPECS=1 git "$@" ;; + diff) shift; command git --no-pager -c core.fsmonitor=false diff --no-ext-diff --no-textconv "$@" ;; + show) shift; command git --no-pager -c core.fsmonitor=false show --no-ext-diff --no-textconv "$@" ;; + log) shift; command git --no-pager -c core.fsmonitor=false log --no-ext-diff --no-textconv "$@" ;; + blame) shift; command git --no-pager -c core.fsmonitor=false blame --no-textconv "$@" ;; + stash) shift; case "$1" in + show) shift; command git --no-pager -c core.fsmonitor=false stash show --no-ext-diff --no-textconv "$@" ;; + list) shift; command git --no-pager -c core.fsmonitor=false stash list --no-ext-diff --no-textconv "$@" ;; + esac ;; + *) command git --no-pager -c core.fsmonitor=false "$@" ;; + esac } +` -// go subcommands that are non-mutating (or only write caches). -var exploreGoAllow = map[string]bool{ - "version": true, "env": true, "list": true, "doc": true, "help": true, - "test": true, "vet": true, "fmt": true, // fmt rewrites — deny separately - "mod": true, "tool": true, -} +// General interpreters, package scripts, pagers, and sed/awk programs can +// execute code or write files without shell redirects. Use native tools for +// those operations in read-only modes rather than guessing their effects. // ExploreBashAllowed returns nil if command is safe for explore/plan modes. // It is segment-aware: splits on ; && || | and checks each pipeline stage's @@ -63,12 +56,14 @@ func ExploreBashAllowed(command string) error { if cmd == "" { return fmt.Errorf("empty command") } + // Expansions can execute commands or turn data into shell syntax. These + // modes accept literal inspection commands only. + if strings.ContainsAny(cmd, "$`\\()") || strings.Contains(cmd, "<<") { + return fmt.Errorf("explore bash: expansions, subshells and heredocs are not allowed") + } // Hard deny obvious mutators / redirections even before segment parse. - if strings.Contains(cmd, ">") || strings.Contains(cmd, "<<") { - // Allow `2>&1` style redirects only if no file redirect. - if hasFileRedirect(cmd) { - return fmt.Errorf("explore bash: file redirects are not allowed") - } + if strings.ContainsAny(cmd, "<>") { + return fmt.Errorf("explore bash: redirects are not allowed") } segments := splitShellSegments(cmd) if len(segments) == 0 { @@ -82,32 +77,6 @@ func ExploreBashAllowed(command string) error { return nil } -func hasFileRedirect(s string) bool { - // Detect >file or >>file or > file but not 2>&1 - runes := []rune(s) - for i := 0; i < len(runes); i++ { - if runes[i] != '>' { - continue - } - // skip >> - j := i + 1 - if j < len(runes) && runes[j] == '>' { - j++ - } - // skip optional & for 2>&1 — if next is & digit, ok - for j < len(runes) && unicode.IsSpace(runes[j]) { - j++ - } - if j < len(runes) && runes[j] == '&' { - continue // fd redirect - } - if j < len(runes) { - return true - } - } - return false -} - // splitShellSegments splits on top-level ; && || | (not inside quotes). func splitShellSegments(s string) []string { var segs []string @@ -138,7 +107,7 @@ func splitShellSegments(s string) []string { } if !inSingle && !inDouble { // operators - if ch == ';' { + if ch == ';' || ch == '\n' { if t := strings.TrimSpace(b.String()); t != "" { segs = append(segs, t) } @@ -176,27 +145,22 @@ func exploreSegmentAllowed(seg string) error { // strip env assignments: FOO=bar cmd tokens := tokenizeShellWords(seg) for len(tokens) > 0 && strings.Contains(tokens[0], "=") && !strings.HasPrefix(tokens[0], "=") { + name, _, _ := strings.Cut(tokens[0], "=") + for _, prefix := range []string{"PATH", "LD_", "DYLD_", "GIT_", "GO", "BASH", "ENV", "SHELL", "CDPATH"} { + if strings.HasPrefix(name, prefix) { + return fmt.Errorf("explore bash: execution environment override %q is not allowed", name) + } + } tokens = tokens[1:] } if len(tokens) == 0 { return fmt.Errorf("explore bash: empty segment after env strip") } - // skip common prefixes - for len(tokens) > 0 { - switch tokens[0] { - case "command", "builtin", "time", "env", "nice", "nohup": - tokens = tokens[1:] - continue - case "sudo", "doas": - return fmt.Errorf("explore bash: privilege escalation (%s) is not allowed", tokens[0]) - } - break - } if len(tokens) == 0 { return fmt.Errorf("explore bash: no command token") } base := commandBase(tokens[0]) - if !exploreBashAllow[base] { + if tokens[0] != base || !exploreBashAllow[base] { return fmt.Errorf("explore bash: command %q is not on the read-only allowlist", base) } switch base { @@ -204,119 +168,145 @@ func exploreSegmentAllowed(seg string) error { return exploreGitAllowed(tokens[1:]) case "go": return exploreGoAllowed(tokens[1:]) - case "npm", "pnpm", "yarn": - return explorePackageManagerAllowed(base, tokens[1:]) - case "sed": - // sed -i is mutating - for _, t := range tokens[1:] { - if t == "-i" || strings.HasPrefix(t, "-i") { - return fmt.Errorf("explore bash: sed -i is not allowed") - } - } case "find": for _, t := range tokens[1:] { - if t == "-delete" || t == "-exec" || t == "-execdir" || t == "-ok" { + if t == "-delete" || strings.HasPrefix(t, "-exec") || strings.HasPrefix(t, "-ok") || strings.HasPrefix(t, "-fprint") || t == "-fls" { return fmt.Errorf("explore bash: find %s is not allowed", t) } } + case "sort", "tree": + for _, arg := range tokens[1:] { + if strings.HasPrefix(arg, "--o") || (strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") && strings.Contains(arg, "o")) { + return fmt.Errorf("explore bash: file output is not allowed") + } + if base == "sort" && strings.HasPrefix(arg, "--c") { + return fmt.Errorf("explore bash: sort compression programs are not allowed") + } + } + case "printf": + for _, arg := range tokens[1:] { + if strings.HasPrefix(arg, "-v") { + return fmt.Errorf("explore bash: assigning shell variables is not allowed") + } + } + case "rg": + for _, arg := range tokens[1:] { + if strings.HasPrefix(arg, "--pre") || strings.HasPrefix(arg, "--hostname-bin") || arg == "-z" || arg == "--search-zip" { + return fmt.Errorf("explore bash: external search programs are not allowed") + } + } + case "file": + for _, arg := range tokens[1:] { + if strings.HasPrefix(arg, "--c") || (strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") && strings.Contains(arg, "C")) { + return fmt.Errorf("explore bash: compiling magic files is not allowed") + } + } } return nil } func exploreGitAllowed(args []string) error { - if len(args) == 0 { - return nil // git with no args is help-ish + // Parse the small set of read-only global options before the subcommand. + for len(args) > 0 && strings.HasPrefix(args[0], "-") { + switch args[0] { + case "-C": + if len(args) < 2 { + return fmt.Errorf("explore bash: git -C needs a directory") + } + args = args[2:] + case "--no-pager", "--literal-pathspecs": + args = args[1:] + case "--version": + if len(args) == 1 { + return nil + } + return fmt.Errorf("explore bash: unexpected git version arguments") + default: + return fmt.Errorf("explore bash: git option %q is not allowed", args[0]) + } } - sub := args[0] - if strings.HasPrefix(sub, "-") { - // git --version etc. + if len(args) == 0 { return nil } - // deny mutators even if they appear as "git -C path commit" - deny := map[string]bool{ - "add": true, "commit": true, "push": true, "pull": true, "fetch": true, - "merge": true, "rebase": true, "cherry-pick": true, "reset": true, - "checkout": true, "switch": true, "restore": true, "clean": true, - "rm": true, "mv": true, "init": true, "clone": true, "worktree": true, - "am": true, "apply": true, "revert": true, "tag": false, // tag create vs list - } - if deny[sub] { - return fmt.Errorf("explore bash: git %s is not allowed", sub) + sub, rest := args[0], args[1:] + for _, arg := range rest { + if strings.HasPrefix(arg, "--out") || strings.HasPrefix(arg, "--ext") || strings.HasPrefix(arg, "--textconv") || arg == "--exec" || strings.HasPrefix(arg, "--filt") { + return fmt.Errorf("explore bash: git option %q can mutate or execute code", arg) + } } - // stash push/pop/drop - if sub == "stash" && len(args) > 1 { - switch args[1] { - case "list", "show": + switch sub { + case "status", "log", "show", "diff", "rev-parse", "rev-list", "describe", "ls-files", "ls-tree", "cat-file", "blame", "shortlog", "name-rev", "for-each-ref", "version": + return nil + case "branch", "tag": + if len(rest) == 0 { return nil - default: - return fmt.Errorf("explore bash: git stash %s is not allowed", args[1]) } - } - // tag without args lists; with -a/-d mutates - if sub == "tag" { - for _, a := range args[1:] { - if a == "-a" || a == "-d" || a == "-m" || a == "-f" { - return fmt.Errorf("explore bash: mutating git tag flags not allowed") + if rest[0] == "--list" || rest[0] == "-l" { + for _, arg := range rest[1:] { + if strings.HasPrefix(arg, "-") { + return fmt.Errorf("explore bash: only listing patterns are allowed") + } + } + return nil + } + for _, arg := range rest { + switch arg { + case "--show-current", "--all", "--remotes", "-a", "-r", "-v", "-vv": + default: + return fmt.Errorf("explore bash: mutating git %s arguments are not allowed", sub) + } + } + return nil + case "stash": + if len(rest) > 0 && (rest[0] == "list" || rest[0] == "show") { + return nil + } + case "remote": + if len(rest) == 0 || (len(rest) == 1 && rest[0] == "-v") { + return nil + } + case "config": + // Read forms only; never allow bare key/value or edit/unset operations. + if len(rest) > 0 { + switch rest[0] { + case "--get", "--get-all", "--get-regexp", "--get-urlmatch", "--list", "-l": + for _, arg := range rest[1:] { + if strings.HasPrefix(arg, "-") { + return fmt.Errorf("explore bash: extra git config options are not allowed") + } + } + return nil } } } - if !exploreGitAllow[sub] { - return fmt.Errorf("explore bash: git %s is not on the read-only allowlist", sub) - } - return nil + return fmt.Errorf("explore bash: git %s is not a read-only operation", sub) } func exploreGoAllowed(args []string) error { if len(args) == 0 { return nil } - sub := args[0] - deny := map[string]bool{ - "get": true, "install": true, "generate": true, "work": true, - "clean": true, "mod": false, - } - if sub == "fmt" { - return fmt.Errorf("explore bash: go fmt rewrites files and is not allowed") - } - if sub == "mod" { - if len(args) > 1 { - switch args[1] { - case "download", "tidy", "vendor", "init", "edit": - return fmt.Errorf("explore bash: go mod %s is not allowed", args[1]) + switch args[0] { + case "version": + if len(args) == 1 { + return nil + } + case "env": + for _, arg := range args[1:] { + if strings.HasPrefix(arg, "-") && arg != "-json" && arg != "-changed" { + return fmt.Errorf("explore bash: go env option %q is not read-only", arg) } } return nil - } - if deny[sub] { - return fmt.Errorf("explore bash: go %s is not allowed", sub) - } - if !exploreGoAllow[sub] { - return fmt.Errorf("explore bash: go %s is not on the read-only allowlist", sub) - } - return nil -} - -func explorePackageManagerAllowed(pm string, args []string) error { - if len(args) == 0 { + case "list": + for _, arg := range args[1:] { + if strings.HasPrefix(arg, "-") && arg != "-json" && arg != "-deps" && arg != "-e" && arg != "-m" && arg != "-mod=readonly" { + return fmt.Errorf("explore bash: go list option %q is not allowed", arg) + } + } return nil } - sub := args[0] - allow := map[string]bool{ - "list": true, "ls": true, "view": true, "info": true, "outdated": true, - "why": true, "explain": true, "pack": false, "test": true, "run": false, - "help": true, "version": true, "bin": true, "root": true, "prefix": true, - } - deny := map[string]bool{ - "install": true, "i": true, "add": true, "uninstall": true, "remove": true, - "update": true, "upgrade": true, "publish": true, "link": true, "ci": true, - } - if deny[sub] { - return fmt.Errorf("explore bash: %s %s is not allowed", pm, sub) - } - if !allow[sub] { - return fmt.Errorf("explore bash: %s %s is not on the read-only allowlist", pm, sub) - } - return nil + return fmt.Errorf("explore bash: go %s is not a read-only operation", args[0]) } func commandBase(token string) string { diff --git a/internal/tool/bash_explore_test.go b/internal/tool/bash_explore_test.go index dff2be133..6e0f2d97d 100644 --- a/internal/tool/bash_explore_test.go +++ b/internal/tool/bash_explore_test.go @@ -39,6 +39,32 @@ func TestExploreBashAllowed_MutationsDenied(t *testing.T) { "sudo ls", "chmod 777 x", "mv a b", + "sed 'w output.txt' input.txt", + "awk 'BEGIN {system(\"touch output\")}'", + "python3 -c 'open(\"output\", \"w\").close()'", + "node -e 'process.exit(0)'", + "git -C /tmp commit -am msg", + "git branch new-branch", + "git tag new-tag", + "git stash", + "git config core.pager something", + "git -c core.pager=something status", + "git diff --output=output.txt", + "git diff --ext-diff", + "go env -w GOFLAGS=-x", + "go test ./...", + "go tool compile file.go", + "npm test", + "find . -fprintf output.txt '%p'", + "sort -o output.txt input.txt", + "sort --compress-program=program input.txt", + "printf -v PATH /tmp", + "rg --pre=program pattern", + "echo hello\ntouch output.txt", + "echo $(touch output.txt)", + "PATH=/tmp ls", + "./ls", + "curl -o output.txt https://example.com", } for _, c := range deny { if err := ExploreBashAllowed(c); err == nil { @@ -50,8 +76,7 @@ func TestExploreBashAllowed_MutationsDenied(t *testing.T) { func TestExploreBashAllowed_PipelineGate(t *testing.T) { // second stage mutating if err := ExploreBashAllowed("cat f | tee out"); err == nil { - // tee is not on allowlist - t.Log("tee denied as expected if not allowlisted") + t.Fatal("tee must be denied") } if err := ExploreBashAllowed("ls; rm -rf x"); err == nil { t.Fatal("expected deny for multi-segment with rm") diff --git a/internal/tool/download.go b/internal/tool/download.go index 68921093b..00ae5c4ec 100644 --- a/internal/tool/download.go +++ b/internal/tool/download.go @@ -56,26 +56,24 @@ func (DownloadTool) Execute(ctx context.Context, input json.RawMessage) (string, if p.URL == "" || p.Destination == "" { return "", fmt.Errorf("url and destination are required") } + p.Destination = resolveToolPath(ctx, p.Destination) if err := validatePathAllowed(ctx, p.Destination); err != nil { return "", err } if reason := IsSensitivePath(p.Destination); reason != "" { return "", fmt.Errorf("write blocked: %s", reason) } - pinnedURL, origHost, err := ValidateURLPublic(ctx, p.URL) + validatedURL, _, err := ValidateURLPublic(ctx, p.URL) if err != nil { return "", err } client := SSRFSafeClient(ctx, 2*time.Minute) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, pinnedURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, validatedURL, nil) if err != nil { return "", fmt.Errorf("create request: %w", err) } - // Preserve the original Host header for virtual-host routing. - if origHost != "" { - req.Host = origHost - } + resp, err := client.Do(req) if err != nil { return "", fmt.Errorf("download failed: %w", err) diff --git a/internal/tool/exec_unix.go b/internal/tool/exec_unix.go index d374e3c67..c99db5bba 100644 --- a/internal/tool/exec_unix.go +++ b/internal/tool/exec_unix.go @@ -6,12 +6,23 @@ import ( "os" "os/exec" "syscall" + "time" ) func setCmdProcessGroup(cmd *exec.Cmd) { cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + // CommandContext otherwise kills only the shell. Descendants may keep the + // output pipes open indefinitely after the shell's deadline expires. + if cmd.Cancel != nil { + cmd.Cancel = func() error { return killProcessGroup(cmd.Process) } + } + cmd.WaitDelay = time.Second } func killProcessGroup(proc *os.Process) error { - return syscall.Kill(-proc.Pid, syscall.SIGKILL) + err := syscall.Kill(-proc.Pid, syscall.SIGKILL) + if err == syscall.ESRCH { + return os.ErrProcessDone + } + return err } diff --git a/internal/tool/exec_windows.go b/internal/tool/exec_windows.go index c3b1b48dc..9401253b7 100644 --- a/internal/tool/exec_windows.go +++ b/internal/tool/exec_windows.go @@ -3,12 +3,30 @@ package tool import ( + "context" "os" "os/exec" + "path/filepath" + "strconv" + "time" ) -func setCmdProcessGroup(cmd *exec.Cmd) {} +func setCmdProcessGroup(cmd *exec.Cmd) { + if cmd.Cancel != nil { + cmd.Cancel = func() error { return killProcessGroup(cmd.Process) } + } + // Bound pipe draining even when descendants retain inherited handles. + cmd.WaitDelay = time.Second +} func killProcessGroup(proc *os.Process) error { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + // taskkill /T terminates descendants as well as the shell. Resolve the + // system executable explicitly rather than trusting a project PATH entry. + taskkill := filepath.Join(os.Getenv("SystemRoot"), "System32", "taskkill.exe") + if err := exec.CommandContext(ctx, taskkill, "/PID", strconv.Itoa(proc.Pid), "/T", "/F").Run(); err == nil { // #nosec G204 -- system taskkill with numeric PID + return nil + } return proc.Kill() } diff --git a/internal/tool/execution_safety_regression_test.go b/internal/tool/execution_safety_regression_test.go new file mode 100644 index 000000000..d454adaa8 --- /dev/null +++ b/internal/tool/execution_safety_regression_test.go @@ -0,0 +1,168 @@ +package tool + +import ( + "context" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/GrayCodeAI/reflect/internal/storage" +) + +func TestReadOnlyBashCannotWriteThroughSed(t *testing.T) { + dir := t.TempDir() + source, marker := filepath.Join(dir, "source.txt"), filepath.Join(dir, "marker.txt") + if err := os.WriteFile(source, []byte("fixture\n"), 0o600); err != nil { + t.Fatal(err) + } + ctx := WithToolContext(t.Context(), &ToolContext{ReadOnlyBash: true, WorkingDir: dir}) + input, _ := json.Marshal(map[string]interface{}{"command": "sed 'w " + marker + "' " + source}) + if _, err := (BashTool{}).Execute(ctx, input); err == nil { + t.Fatal("read-only Bash accepted a writing sed program") + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Fatalf("read-only command created marker: %v", err) + } +} + +func TestSandboxRejectsBackgroundExecution(t *testing.T) { + storage.SetTestDirs(t, t.TempDir()) + t.Setenv("REFLECT_SANDBOX", "bwrap") + ctx := WithToolContext(t.Context(), &ToolContext{WorkingDir: t.TempDir()}) + _, err := (BashTool{}).Execute(ctx, json.RawMessage(`{"command":"echo host","run_in_background":true}`)) + if err == nil || !strings.Contains(err.Error(), "sandbox") { + t.Fatalf("background execution must not bypass sandboxing: %v", err) + } +} + +func TestReadOnlyGitDisablesRepositoryPrograms(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX helper fixture") + } + storage.SetTestDirs(t, t.TempDir()) + t.Setenv("REFLECT_SANDBOX", "off") + dir := t.TempDir() + git := func(args ...string) { + t.Helper() + command := exec.CommandContext(t.Context(), "git", args...) + command.Dir = dir + if output, err := command.CombinedOutput(); err != nil { + t.Fatalf("git %v: %s, %v", args, output, err) + } + } + git("init") + for name, content := range map[string]string{ + ".gitattributes": "*.txt diff=audit\n", + "project.txt": "before\n", + "helper.sh": "#!/bin/sh\necho invoked > helper-ran\n", + } { + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o700); err != nil { + t.Fatal(err) + } + } + git("add", ".") + git("config", "diff.audit.textconv", "./helper.sh") + git("config", "core.fsmonitor", "./helper.sh") + if err := os.WriteFile(filepath.Join(dir, "project.txt"), []byte("after\n"), 0o600); err != nil { + t.Fatal(err) + } + ctx := WithToolContext(t.Context(), &ToolContext{ReadOnlyBash: true, WorkingDir: dir}) + for _, command := range []string{"git diff", "git status"} { + input, _ := json.Marshal(map[string]interface{}{"command": command}) + if output, err := (BashTool{}).Execute(ctx, input); err != nil || strings.Contains(output, "exit code:") { + t.Fatalf("%s: %s, %v", command, output, err) + } + } + if _, err := os.Stat(filepath.Join(dir, "helper-ran")); !os.IsNotExist(err) { + t.Fatalf("read-only git invoked a repository program: %v", err) + } +} + +func TestBashDeadlineTerminatesDescendants(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX shell fixture; Windows uses taskkill /T") + } + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("bash unavailable") + } + storage.SetTestDirs(t, t.TempDir()) + t.Setenv("REFLECT_SANDBOX", "off") + dir := t.TempDir() + marker := filepath.Join(dir, "survived.txt") + ctx, cancel := context.WithTimeout(t.Context(), 150*time.Millisecond) + defer cancel() + ctx = WithToolContext(ctx, &ToolContext{WorkingDir: dir}) + input := json.RawMessage(`{"command":"sleep 1; echo survived > survived.txt"}`) + started := time.Now() + output, err := (BashTool{}).Execute(ctx, input) + if err != nil || !strings.Contains(output, "timed out") { + t.Fatalf("expected deadline result, got %q, %v", output, err) + } + if elapsed := time.Since(started); elapsed > 750*time.Millisecond { + t.Fatalf("deadline waited for descendants: %v", elapsed) + } + // A child left running would create this marker after the tool returned. + time.Sleep(time.Second) + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Fatalf("descendant survived cancellation: %v", err) + } +} + +func TestFileToolsUseSessionWorkingDir(t *testing.T) { + parent, child := t.TempDir(), t.TempDir() + t.Chdir(parent) + for _, dir := range []string{parent, child} { + if err := os.WriteFile(filepath.Join(dir, "project.txt"), []byte("original"), 0o600); err != nil { + t.Fatal(err) + } + } + ctx := WithToolContext(t.Context(), &ToolContext{WorkingDir: child, AllowedDirectories: []string{child}}) + cases := []struct { + tool Tool + input string + }{ + {FileWriteTool{}, `{"path":"project.txt","content":"child change"}`}, + {FileEditTool{}, `{"file_path":"project.txt","old_str":"original","new_str":"child change"}`}, + {MultiEditTool{}, `{"file_path":"project.txt","edits":[{"old_string":"original","new_string":"child change"}]}`}, + {StructuredEditTool{}, `{"path":"project.txt","blocks":[{"search":"original","replace":"child change"}]}`}, + } + for _, tc := range cases { + t.Run(tc.tool.Name(), func(t *testing.T) { + if err := os.WriteFile(filepath.Join(child, "project.txt"), []byte("original"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := tc.tool.Execute(ctx, json.RawMessage(tc.input)); err != nil { + t.Fatal(err) + } + parentData, _ := os.ReadFile(filepath.Join(parent, "project.txt")) + childData, _ := os.ReadFile(filepath.Join(child, "project.txt")) + if string(parentData) != "original" || string(childData) != "child change" { + t.Fatalf("workspace isolation failed: parent=%q child=%q", parentData, childData) + } + }) + } + if err := validatePathAllowed(ctx, filepath.Join(parent, "project.txt")); err == nil { + t.Fatal("parent process cwd must not be implicitly allowed in a child workspace") + } + output, err := (FileReadTool{}).Execute(ctx, json.RawMessage(`{"path":"project.txt"}`)) + if err != nil || !strings.Contains(output, "child change") { + t.Fatalf("Read used the wrong workspace: %q, %v", output, err) + } +} + +func TestWorkspaceArgumentsPreserveCallerAndRemotePaths(t *testing.T) { + dir := t.TempDir() + args := map[string]interface{}{"path": "project.txt", "content": "unchanged"} + got := WorkspaceArguments("Write", args, dir) + if got["path"] != filepath.Join(dir, "project.txt") || args["path"] != "project.txt" { + t.Fatalf("path normalization mutated caller arguments: got=%v caller=%v", got, args) + } + if got := WorkspaceArguments("mcp__remote__read", args, dir); got["path"] != "project.txt" { + t.Fatalf("remote path rewritten: %v", got) + } +} diff --git a/internal/tool/file_edit.go b/internal/tool/file_edit.go index 20de6f642..3eb400d09 100644 --- a/internal/tool/file_edit.go +++ b/internal/tool/file_edit.go @@ -66,6 +66,7 @@ func (FileEditTool) Execute(ctx context.Context, input json.RawMessage) (string, if path == "" { return "", fmt.Errorf("path is required") } + path = resolveToolPath(ctx, path) if err := validatePathAllowed(ctx, path); err != nil { return "", err } diff --git a/internal/tool/file_read.go b/internal/tool/file_read.go index 30b2f7a57..dc3de7d9c 100644 --- a/internal/tool/file_read.go +++ b/internal/tool/file_read.go @@ -73,6 +73,7 @@ func (FileReadTool) Execute(ctx context.Context, input json.RawMessage) (string, // file is missing), then resolve symlinks so the checks apply to the // real target (M13): a symlink to ~/.ssh/id_rsa must be caught here, // not silently followed during the read. + path = resolveToolPath(ctx, path) if err := validatePathAllowed(ctx, path); err != nil { return "", err } diff --git a/internal/tool/file_write.go b/internal/tool/file_write.go index 023b791b5..f9b14a499 100644 --- a/internal/tool/file_write.go +++ b/internal/tool/file_write.go @@ -59,6 +59,7 @@ func (FileWriteTool) Execute(ctx context.Context, input json.RawMessage) (string if path == "" { return "", fmt.Errorf("path is required") } + path = resolveToolPath(ctx, path) if err := validatePathAllowed(ctx, path); err != nil { return "", err } diff --git a/internal/tool/glob.go b/internal/tool/glob.go index 7a92bf2f4..51dd9e744 100644 --- a/internal/tool/glob.go +++ b/internal/tool/glob.go @@ -51,6 +51,7 @@ func (GlobTool) Execute(ctx context.Context, input json.RawMessage) (string, err if root == "" { root = "." } + root = resolveToolPath(ctx, root) if err := validatePathAllowed(ctx, root); err != nil { return "", err } diff --git a/internal/tool/grep.go b/internal/tool/grep.go index 2bd60dff0..5f77ff335 100644 --- a/internal/tool/grep.go +++ b/internal/tool/grep.go @@ -59,6 +59,7 @@ func (GrepTool) Execute(ctx context.Context, input json.RawMessage) (string, err if root == "" { root = "." } + root = resolveToolPath(ctx, root) if err := validatePathAllowed(ctx, root); err != nil { return "", err } diff --git a/internal/tool/ls.go b/internal/tool/ls.go index 2e3986c5d..4dba81df3 100644 --- a/internal/tool/ls.go +++ b/internal/tool/ls.go @@ -58,6 +58,7 @@ func (LSTool) Execute(ctx context.Context, input json.RawMessage) (string, error if path == "" { path = "." } + path = resolveToolPath(ctx, path) if err := validatePathAllowed(ctx, path); err != nil { return "", err } diff --git a/internal/tool/multiedit.go b/internal/tool/multiedit.go index 68b3a32a8..f3fecfa67 100644 --- a/internal/tool/multiedit.go +++ b/internal/tool/multiedit.go @@ -68,6 +68,7 @@ func (MultiEditTool) Execute(ctx context.Context, input json.RawMessage) (string if len(p.Edits) == 0 { return "", fmt.Errorf("at least one edit is required") } + p.FilePath = resolveToolPath(ctx, p.FilePath) if err := validatePathAllowed(ctx, p.FilePath); err != nil { return "", err } diff --git a/internal/tool/patch.go b/internal/tool/patch.go index 13006026d..622a35368 100644 --- a/internal/tool/patch.go +++ b/internal/tool/patch.go @@ -246,6 +246,7 @@ func Apply(patch *FilePatch) error { func (p *PatchParser) ApplyAll(ctx context.Context) ([]string, error) { var modified []string for i := range p.patches { + p.patches[i].Path = resolveToolPath(ctx, p.patches[i].Path) if err := validatePathAllowed(ctx, p.patches[i].Path); err != nil { return modified, err } diff --git a/internal/tool/path_guard.go b/internal/tool/path_guard.go index 5a7548979..4328634b1 100644 --- a/internal/tool/path_guard.go +++ b/internal/tool/path_guard.go @@ -8,6 +8,51 @@ import ( "strings" ) +// resolveToolPath interprets relative paths in the session's workspace without +// changing the process-wide cwd, which other concurrent sessions may be using. +func resolveToolPath(ctx context.Context, path string) string { + if path == "" || filepath.IsAbs(path) { + return path + } + if tc := GetToolContext(ctx); tc != nil && tc.WorkingDir != "" { + resolved := filepath.Join(tc.WorkingDir, path) + if abs, err := filepath.Abs(resolved); err == nil { + return abs + } + return resolved + } + return path +} + +// WorkspaceArguments returns a copy with built-in file-tool paths rooted in +// workingDir. The host calls this before permission checks so the approved +// paths and the paths used by execution are identical. Remote tool arguments +// are left alone because they may refer to another machine's filesystem. +func WorkspaceArguments(name string, args map[string]interface{}, workingDir string) map[string]interface{} { + if workingDir == "" { + return args + } + if abs, err := filepath.Abs(workingDir); err == nil { + workingDir = abs + } + switch name { + case "Read", "Write", "Edit", "StructuredEdit", "MultiEdit", "LS", "Glob", "Grep", "Download", "Outline", "Refactor", "SmartCreate": + default: + return args + } + copyArgs := make(map[string]interface{}, len(args)) + for key, value := range args { + copyArgs[key] = value + switch key { + case "path", "file_path", "file", "destination": + if path, ok := value.(string); ok && path != "" && !filepath.IsAbs(path) { + copyArgs[key] = filepath.Join(workingDir, path) + } + } + } + return copyArgs +} + func validatePathAllowed(ctx context.Context, path string) error { tc := GetToolContext(ctx) if tc == nil { @@ -21,17 +66,17 @@ func validatePathAllowed(ctx context.Context, path string) error { if path == "" { return fmt.Errorf("path is required") } - absPath, err := guardedAbs(path) + absPath, err := guardedAbs(resolveToolPath(ctx, path)) if err != nil { return err } - roots := append([]string{"."}, tc.AllowedDirectories...) + roots := append([]string{resolveToolPath(ctx, ".")}, tc.AllowedDirectories...) for _, root := range roots { root = strings.TrimSpace(root) if root == "" { continue } - absRoot, err := guardedAbs(root) + absRoot, err := guardedAbs(resolveToolPath(ctx, root)) if err != nil { continue } @@ -65,7 +110,7 @@ func guardedRootPath(ctx context.Context, path string) (*os.Root, string, error) if err := validatePathAllowed(ctx, path); err != nil { return nil, "", err } - return openGuardedRoot(path) + return openGuardedRoot(resolveToolPath(ctx, path)) } // openGuardedRoot pins the canonical parent directory of a path with os.Root diff --git a/internal/tool/refactor.go b/internal/tool/refactor.go index 8c19c0e7c..60701aab0 100644 --- a/internal/tool/refactor.go +++ b/internal/tool/refactor.go @@ -834,6 +834,7 @@ func (rt RefactorTool) Execute(ctx context.Context, input json.RawMessage) (stri if p.File == "" { return "", fmt.Errorf("file is required") } + p.File = resolveToolPath(ctx, p.File) if err := validatePathAllowed(ctx, p.File); err != nil { return "", err } diff --git a/internal/tool/safety_reexports.go b/internal/tool/safety_reexports.go index 548b8b05b..cc49585ce 100644 --- a/internal/tool/safety_reexports.go +++ b/internal/tool/safety_reexports.go @@ -51,8 +51,8 @@ func SegmentCommand(cmd string) []string { return toolsafety.SegmentCommand(cmd) // WithSSRFSkip returns a context that skips SSRF URL validation. func WithSSRFSkip(ctx context.Context) context.Context { return toolsafety.WithSSRFSkip(ctx) } -// ValidateURLPublic rejects private/link-local URLs and pins the resolved IP. -func ValidateURLPublic(ctx context.Context, rawURL string) (pinnedURL, originalHost string, err error) { +// ValidateURLPublic rejects private/link-local URLs; use SSRFSafeClient to pin the dial address. +func ValidateURLPublic(ctx context.Context, rawURL string) (validatedURL, originalHost string, err error) { return toolsafety.ValidateURLPublic(ctx, rawURL) } diff --git a/internal/tool/smart_create.go b/internal/tool/smart_create.go index e7dc65d3b..24c66dff2 100644 --- a/internal/tool/smart_create.go +++ b/internal/tool/smart_create.go @@ -687,6 +687,7 @@ func (t *SmartCreateTool) Execute(ctx context.Context, input json.RawMessage) (s if params.Path == "" { return "", fmt.Errorf("path is required") } + params.Path = resolveToolPath(ctx, params.Path) if err := validatePathAllowed(ctx, params.Path); err != nil { return "", err } diff --git a/internal/tool/structured_edit.go b/internal/tool/structured_edit.go index e9a94a7ed..59e781758 100644 --- a/internal/tool/structured_edit.go +++ b/internal/tool/structured_edit.go @@ -76,6 +76,7 @@ func (s StructuredEditTool) Execute(ctx context.Context, input json.RawMessage) if p.Path == "" { return "", fmt.Errorf("path is required") } + p.Path = resolveToolPath(ctx, p.Path) if err := validatePathAllowed(ctx, p.Path); err != nil { return "", err } diff --git a/internal/tool/task_tools.go b/internal/tool/task_tools.go index d196ded8e..b201e0992 100644 --- a/internal/tool/task_tools.go +++ b/internal/tool/task_tools.go @@ -81,6 +81,9 @@ func startBackgroundBash(ctx context.Context, command string, execName string, e // Background tasks are long-lived and observable by the agent; scrub // provider API keys so the child environment cannot leak credentials. cmd.Env = env.SubprocessEnv() + if tc := GetToolContext(ctx); tc != nil { + cmd.Dir = tc.WorkingDir + } // Put the child in its own process group so we can kill the whole tree // (including grandchildren spawned by the shell) via kill(-pgid). Without // this, e.g. `bash -c 'sleep 60 &'` leaves an orphan when the parent is diff --git a/internal/tool/transaction.go b/internal/tool/transaction.go index dc5915569..d15b6667d 100644 --- a/internal/tool/transaction.go +++ b/internal/tool/transaction.go @@ -463,7 +463,10 @@ func (TransactionTool) Execute(ctx context.Context, input json.RawMessage) (stri } // Validate paths - for _, op := range p.Operations { + for i := range p.Operations { + op := &p.Operations[i] + op.Path = resolveToolPath(ctx, op.Path) + op.OldPath = resolveToolPath(ctx, op.OldPath) if err := validatePathAllowed(ctx, op.Path); err != nil { return "", err } diff --git a/internal/tool/web_fetch.go b/internal/tool/web_fetch.go index 691dc6db9..ae2c1f10e 100644 --- a/internal/tool/web_fetch.go +++ b/internal/tool/web_fetch.go @@ -61,7 +61,7 @@ func (WebFetchTool) Execute(ctx context.Context, input json.RawMessage) (string, if p.URL == "" { return "", fmt.Errorf("url is required") } - pinnedURL, origHost, err := ValidateURLPublic(ctx, p.URL) + validatedURL, _, err := ValidateURLPublic(ctx, p.URL) if err != nil { return "", err } @@ -69,18 +69,11 @@ func (WebFetchTool) Execute(ctx context.Context, input json.RawMessage) (string, ctx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() - req, err := http.NewRequestWithContext(ctx, "GET", pinnedURL, nil) + req, err := http.NewRequestWithContext(ctx, "GET", validatedURL, nil) if err != nil { return "", err } req.Header.Set("User-Agent", "reflect/0.1.0") - // Preserve the original Host header so virtual-host routing works - // correctly. validateURLPublic pins the connection to the validated - // IP (preventing DNS rebinding), but replaces the URL host with the IP. - // Setting req.Host restores the original hostname for the Host header. - if origHost != "" { - req.Host = origHost - } client := SSRFSafeClient(ctx, 30*time.Second) resp, err := client.Do(req) diff --git a/internal/toolsafety/https_regression_test.go b/internal/toolsafety/https_regression_test.go new file mode 100644 index 000000000..f02af8eb5 --- /dev/null +++ b/internal/toolsafety/https_regression_test.go @@ -0,0 +1,111 @@ +package toolsafety + +import ( + "context" + "crypto/tls" + "crypto/x509" + "fmt" + "net" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func TestSafeHTTPSRetainsHostnameAndPinsSocket(t *testing.T) { + type requestInfo struct{ host, sni string } + requests := make(chan requestInfo, 1) + dials := make(chan string, 1) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests <- requestInfo{r.Host, r.TLS.ServerName} + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + client := SSRFSafeClient(t.Context(), 5*time.Second) + transport := client.Transport.(*http.Transport) + defer transport.CloseIdleConnections() + roots := x509.NewCertPool() + roots.AddCert(server.Certificate()) // httptest's certificate includes example.com. + transport.TLSClientConfig = &tls.Config{RootCAs: roots, MinVersion: tls.VersionTLS12} + dialer := publicDialer{ + lookup: func(_ context.Context, host string) ([]net.IPAddr, error) { + if host != "example.com" { + return nil, fmt.Errorf("unexpected host %q", host) + } + return []net.IPAddr{{IP: net.ParseIP("93.184.216.34")}}, nil + }, + dial: func(ctx context.Context, network, address string) (net.Conn, error) { + dials <- address + return (&net.Dialer{}).DialContext(ctx, network, strings.TrimPrefix(server.URL, "https://")) + }, + } + transport.DialContext = dialer.DialContext + response, err := client.Get("https://example.com:8443/") + if err != nil { + t.Fatal(err) + } + _ = response.Body.Close() + got, dialAddress := <-requests, <-dials + if got.host != "example.com:8443" || got.sni != "example.com" || dialAddress != "93.184.216.34:8443" { + t.Fatalf("host=%q SNI=%q dial=%q", got.host, got.sni, dialAddress) + } + // A different hostname must still fail certificate verification. + transport.DialContext = func(ctx context.Context, network, _ string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, network, strings.TrimPrefix(server.URL, "https://")) + } + if response, err := client.Get("https://wrong.invalid:8443/"); err == nil { + _ = response.Body.Close() + t.Fatal("certificate verification was bypassed") + } +} + +func TestPublicDialRejectsReboundAndMixedAddresses(t *testing.T) { + for _, addresses := range [][]net.IPAddr{ + {{IP: net.ParseIP("127.0.0.1")}}, + {{IP: net.ParseIP("93.184.216.34")}, {IP: net.ParseIP("169.254.169.254")}}, + {{IP: net.ParseIP("::ffff:127.0.0.1")}}, + } { + dialed := false + dialer := publicDialer{ + lookup: func(context.Context, string) ([]net.IPAddr, error) { return addresses, nil }, + dial: func(context.Context, string, string) (net.Conn, error) { + dialed = true + return nil, fmt.Errorf("should not dial") + }, + } + if _, err := dialer.DialContext(t.Context(), "tcp", "example.com:443"); err == nil || dialed { + t.Fatalf("private DNS results reached socket dial: %v", addresses) + } + } +} + +func TestSafeClientBlocksPrivateRedirect(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "http://127.0.0.1/metadata", http.StatusFound) + })) + defer server.Close() + client := SSRFSafeClient(t.Context(), 5*time.Second) + transport := client.Transport.(*http.Transport) + defer transport.CloseIdleConnections() + dials := 0 + transport.DialContext = func(ctx context.Context, network, _ string) (net.Conn, error) { + dials++ + return (&net.Dialer{}).DialContext(ctx, network, strings.TrimPrefix(server.URL, "http://")) + } + if response, err := client.Get("http://93.184.216.34/start"); err == nil { + _ = response.Body.Close() + t.Fatal("private redirect was followed") + } + if dials != 1 { + t.Fatalf("redirect target was dialed: %d dials", dials) + } +} + +func TestValidateURLPreservesIPv6Authority(t *testing.T) { + input := "https://[2606:4700:4700::1111]:8443/path" + got, authority, err := ValidateURLPublic(t.Context(), input) + if err != nil || got != input || authority != "[2606:4700:4700::1111]:8443" { + t.Fatalf("URL=%q authority=%q error=%v", got, authority, err) + } +} diff --git a/internal/toolsafety/toolsafety.go b/internal/toolsafety/toolsafety.go index 375e8a725..ce82c0c0f 100644 --- a/internal/toolsafety/toolsafety.go +++ b/internal/toolsafety/toolsafety.go @@ -402,9 +402,9 @@ func WithSSRFSkip(ctx context.Context) context.Context { // ValidateURLPublic is the exported form of validateURLPublic for other // packages that perform outbound fetches (e.g. the URL scraper). It rejects -// URLs that resolve to private/link-local ranges and returns the URL with the -// resolved IP pinned to prevent DNS rebinding. -func ValidateURLPublic(ctx context.Context, rawURL string) (pinnedURL, originalHost string, err error) { +// URLs that resolve to private/link-local ranges. The hostname is retained for +// TLS verification; callers must use SSRFSafeClient to pin the dial address. +func ValidateURLPublic(ctx context.Context, rawURL string) (validatedURL, originalHost string, err error) { return validateURLPublic(ctx, rawURL) } @@ -415,10 +415,9 @@ func SSRFSafeClient(ctx context.Context, timeout time.Duration) *http.Client { // validateURLPublic rejects URLs that resolve to private/link-local IP ranges // to prevent SSRF attacks (e.g., fetching AWS metadata at 169.254.169.254). -// Returns the validated URL with the resolved IP pinned as the host (preventing -// DNS rebinding) and the original hostname (so callers can preserve the Host -// header for virtual-host routing). -func validateURLPublic(ctx context.Context, rawURL string) (pinnedURL, originalHost string, err error) { +// Returns the validated URL and its authority, including port. SSRFSafeClient +// validates DNS again at dial time and connects to the checked IP directly. +func validateURLPublic(ctx context.Context, rawURL string) (validatedURL, originalHost string, err error) { if ctx.Value(ssrfSkipKey{}) != nil { return rawURL, "", nil } @@ -435,49 +434,83 @@ func validateURLPublic(ctx context.Context, rawURL string) (pinnedURL, originalH return "", "", fmt.Errorf("blocked: URL has no host") } - // Resolve the hostname to check against private ranges. - addrs, err := net.DefaultResolver.LookupIPAddr(ctx, host) + if _, err := publicAddresses(ctx, host, net.DefaultResolver.LookupIPAddr); err != nil { + return "", "", err + } + return u.String(), u.Host, nil +} + +type ipLookup func(context.Context, string) ([]net.IPAddr, error) + +func publicAddresses(ctx context.Context, host string, lookup ipLookup) ([]net.IPAddr, error) { + addrs, err := lookup(ctx, host) if err != nil { // DNS failure — block the request rather than allowing potential SSRF bypass. - return "", "", fmt.Errorf("blocked: DNS resolution failed for %q: %w", host, err) + return nil, fmt.Errorf("blocked: DNS resolution failed for %q: %w", host, err) } - var safeIP string for _, addr := range addrs { ip := addr.IP + if !ip.IsGlobalUnicast() { + return nil, fmt.Errorf("blocked: URL host %q resolves to non-public IP %s", host, ip) + } // net.IPNet.Contains calls ip.To4() internally, so IPv4-mapped IPv6 // addresses (::ffff:a.b.c.d) are correctly checked against IPv4 CIDR // blocks — no separate handling needed. for _, block := range privateIPBlocks { if block.Contains(ip) { - return "", "", fmt.Errorf("blocked: URL %q resolves to private IP %s", rawURL, ip) + return nil, fmt.Errorf("blocked: URL host %q resolves to private IP %s", host, ip) } } - if safeIP == "" { - safeIP = ip.String() - } } - if safeIP == "" { - return "", "", fmt.Errorf("blocked: URL %q resolved to no addresses", rawURL) + if len(addrs) == 0 { + return nil, fmt.Errorf("blocked: URL host %q resolved to no addresses", host) } + return addrs, nil +} - // Pin the IP to prevent DNS rebinding: replace host with the validated IP. - // The caller should set req.Host to originalHost to preserve virtual-host - // routing (most web servers route by Host header, not by IP). - if u.Port() != "" { - u.Host = net.JoinHostPort(safeIP, u.Port()) - } else { - u.Host = safeIP +type publicDialer struct { + lookup ipLookup + dial func(context.Context, string, string) (net.Conn, error) +} + +func (d publicDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { + host, port, err := net.SplitHostPort(address) + if err != nil { + return nil, err } - return u.String(), host, nil + addrs, err := publicAddresses(ctx, host, d.lookup) + if err != nil { + return nil, err + } + var dialErr error + for _, addr := range addrs { + conn, err := d.dial(ctx, network, net.JoinHostPort(addr.String(), port)) + if err == nil { + return conn, nil + } + dialErr = err + } + return nil, dialErr } -// ssrfSafeClient returns an http.Client that validates redirect targets -// against private IP ranges, preventing SSRF via redirect chains. -// The redirect validator pins the resolved IP so that subsequent connections -// in the chain cannot be rebinding to a different (private) address. +// ssrfSafeClient keeps URL hostnames intact for TLS/SNI and HTTP routing. +// Resolve, validate and pin at the socket boundary instead. Disable proxies so +// that proxy-side DNS cannot bypass this validation. func ssrfSafeClient(ctx context.Context, timeout time.Duration) *http.Client { + transport := &http.Transport{TLSHandshakeTimeout: 10 * time.Second, IdleConnTimeout: 90 * time.Second} + if base, ok := http.DefaultTransport.(*http.Transport); ok { + transport = base.Clone() + } + transport.Proxy = nil + transport.DialTLSContext = nil + transport.DialTLS = nil + if ctx.Value(ssrfSkipKey{}) == nil { + dialer := publicDialer{lookup: net.DefaultResolver.LookupIPAddr, dial: (&net.Dialer{Timeout: timeout}).DialContext} + transport.DialContext = dialer.DialContext + } return &http.Client{ - Timeout: timeout, + Timeout: timeout, + Transport: transport, CheckRedirect: func(req *http.Request, via []*http.Request) error { if len(via) >= 5 { return fmt.Errorf("too many redirects") @@ -485,22 +518,10 @@ func ssrfSafeClient(ctx context.Context, timeout time.Duration) *http.Client { if ctx.Value(ssrfSkipKey{}) != nil { return nil } - pinned, origHost, err := validateURLPublic(ctx, req.URL.String()) + _, _, err := validateURLPublic(ctx, req.URL.String()) if err != nil { return err } - // Replace the redirect URL with the IP-pinned version so the - // transport dials the validated address instead of resolving DNS again. - parsed, parseErr := url.Parse(pinned) - if parseErr != nil { - return parseErr - } - req.URL = parsed - // Preserve the original Host header so virtual-host routing - // works correctly on the redirect target. - if origHost != "" { - req.Host = origHost - } return nil }, } diff --git a/scripts/generate-workspace.sh b/scripts/generate-workspace.sh index db96ca48a..bc370c389 100755 --- a/scripts/generate-workspace.sh +++ b/scripts/generate-workspace.sh @@ -1,57 +1,28 @@ #!/usr/bin/env bash set -euo pipefail +# Retain the old entrypoint, using an explicit, repo-local module overlay. +# Never create, remove, or rewrite a workspace in the parent directory. ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -ECO_DIR="$(cd "${ROOT_DIR}/.." && pwd)" -# The product repo may still be checked out under its pre-rename directory -# name (e.g. `hawk`) while ecosystem.yaml already lists `reflect`. Derive the -# self directory from the checkout itself so the workspace generates during -# and after the rename instead of hardcoding one name. -SELF_DIR="$(basename "${ROOT_DIR}")" -# macOS checkouts are case-insensitive, so `Reflect` and the manifest's `reflect` -# name the same directory. Compare case-insensitively to avoid adding the -# self module twice. -SELF_DIR_LC="$(printf '%s' "${SELF_DIR}" | tr '[:upper:]' '[:lower:]')" +if [[ $# -ne 1 || -z "$1" || ! -f "$1/go.mod" ]]; then + echo "usage: $0 /path/to/relay (creates optional go.local.mod; ordinary builds need no sibling checkout)" >&2 + exit 2 +fi +RELAY_DIR="$(cd "$1" && pwd)" +if [[ "$(awk '$1 == "module" {print $2; exit}' "${RELAY_DIR}/go.mod")" != github.com/GrayCodeAI/relay ]]; then + echo "the supplied checkout must declare module github.com/GrayCodeAI/relay" >&2 + exit 2 +fi +if [[ -e "${ROOT_DIR}/go.local.mod" || -e "${ROOT_DIR}/go.local.sum" ]]; then + echo "go.local.mod or go.local.sum already exists; edit the existing overlay explicitly" >&2 + exit 1 +fi -"${ROOT_DIR}/scripts/ecosystem-manifest.sh" validate - -rm -f "${ECO_DIR}/go.work" "${ECO_DIR}/go.work.sum" -( - cd "${ECO_DIR}" - go work init "./${SELF_DIR}" - go_version="$(awk '$1 == "go" { print $2; exit }' "${ROOT_DIR}/go.mod")" - go work edit -go="${go_version}" - while IFS= read -r repo; do - repo_lc="$(printf '%s' "${repo}" | tr '[:upper:]' '[:lower:]')" - [[ "${repo_lc}" == "${SELF_DIR_LC}" ]] && continue - if [[ ! -f "${repo}/go.mod" ]]; then - echo "WARNING: ${repo} is not checked out; skipping workspace entry" - continue - fi - go work use "./${repo}" - module="$(awk '$1 == "module" { print $2; exit }' "${repo}/go.mod")" - while IFS= read -r version; do - [[ -n "${version}" ]] && go work edit -replace="${module}@${version}=./${repo}" - done < <( - { - while IFS= read -r consumer; do - [[ -f "${consumer}/go.mod" ]] || continue - awk -v wanted="${module}" ' - /^(replace|exclude)[[:space:]]*\($/ { skip = 1; next } - skip && /^\)/ { skip = 0; next } - skip { next } - $1 == "replace" || $1 == "exclude" { next } - { - for (i = 1; i < NF; i++) { - if ($i == wanted && $(i + 1) ~ /^v[0-9]/) print $(i + 1) - } - } - ' "${consumer}/go.mod" - done < <("${ROOT_DIR}/scripts/ecosystem-manifest.sh" list workspace) - } | sort -u - ) - done < <("${ROOT_DIR}/scripts/ecosystem-manifest.sh" list workspace) - go work sync -) - -echo "workspace generated at ${ECO_DIR}/go.work" +stage="$(mktemp -d "${ROOT_DIR}/.relay-overlay.XXXXXX")" +trap 'rm -rf "$stage"' EXIT +cp "${ROOT_DIR}/go.mod" "${stage}/go.local.mod" +cp "${ROOT_DIR}/go.sum" "${stage}/go.local.sum" +GOWORK=off go mod edit -modfile="${stage}/go.local.mod" -replace="github.com/GrayCodeAI/relay=${RELAY_DIR}" +mv "${stage}/go.local.sum" "${ROOT_DIR}/go.local.sum" +mv "${stage}/go.local.mod" "${ROOT_DIR}/go.local.mod" +echo "Created go.local.mod; run GOWORK=off go test -modfile=go.local.mod ./..."