diff --git a/.gavel.yaml b/.gavel.yaml index 772a0fba..9a7a58ef 100644 --- a/.gavel.yaml +++ b/.gavel.yaml @@ -3,6 +3,7 @@ checks: {} commit: allow: - pkg/cli/webapp/dist/.gitkeep + - pkg/cli/webapp/dist/index.html grouping: {} lint: {} message: {} diff --git a/.github/workflows/publish-image.yml b/.github/workflows/publish-image.yml new file mode 100644 index 00000000..2afb31e2 --- /dev/null +++ b/.github/workflows/publish-image.yml @@ -0,0 +1,96 @@ +name: Publish Image + +# Manual only. The agent sandbox base image (pkg/container/base) is large and +# slow to build, so it is published on demand rather than on every release. +# +# Dispatch this from the tag you want to ship — the reusable workflow builds +# whatever ref you select. Run it *after* the corresponding release exists: +# the image installs flanksource/captain through deps at 'latest', so without +# the release assets it would bake in the previous version's binary. +on: + workflow_dispatch: + inputs: + version: + description: >- + Image tag to publish, e.g. v0.0.27. Defaults to the most recent git + tag reachable from the selected ref. + required: false + type: string + latest: + description: Also publish the :latest tag. + required: false + type: boolean + default: true + platforms: + description: Platforms to build. + required: false + type: string + default: linux/amd64,linux/arm64 + build_args: + description: >- + Newline-separated Docker build args overriding the Dockerfile's pinned + defaults, e.g. CLAUDE_CODE_VERSION=2.1.227. Leave empty to publish + exactly what the Dockerfile pins — every version it installs is an ARG, + so a tag republished with no overrides reproduces its contents. + required: false + type: string + +permissions: + contents: read + +jobs: + resolve: + name: Resolve image tags + runs-on: ubuntu-latest + outputs: + image_tags: ${{ steps.tags.outputs.image_tags }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + + - id: tags + env: + INPUT_VERSION: ${{ inputs.version }} + INPUT_LATEST: ${{ inputs.latest }} + run: | + set -euo pipefail + + version=$INPUT_VERSION + if [ -z "$version" ]; then + version=$(git describe --tags --abbrev=0) + echo "No version supplied; using most recent tag: $version" + fi + case "$version" in + *[,[:space:]]*) echo "version must not contain commas or whitespace: $version" >&2; exit 1 ;; + esac + + tags="" + for repo in docker.io/flanksource/captain ghcr.io/flanksource/captain; do + tags="$tags$repo:$version," + if [ "$INPUT_LATEST" = "true" ]; then + tags="$tags$repo:latest," + fi + done + + echo "image_tags=${tags%,}" >> "$GITHUB_OUTPUT" + echo "Publishing: ${tags%,}" + + docker: + name: Build and publish + needs: resolve + permissions: + contents: read + id-token: write # keyless cosign signatures + packages: write # push to ghcr.io + uses: flanksource/action-workflows/.github/workflows/publish-multi-platform-docker-image.yml@f8512a65d38c1ea53d5c83a4e30ee2ae56acac6f # v1.2.1 + with: + dockerfile: pkg/container/base/Dockerfile + context: pkg/container/base + image_tags: ${{ needs.resolve.outputs.image_tags }} + platforms: ${{ inputs.platforms }} + build_args: ${{ inputs.build_args }} + secrets: + docker_username: ${{ secrets.DOCKER_USERNAME }} + docker_password: ${{ secrets.DOCKER_PASSWORD }} diff --git a/.gitignore b/.gitignore index 5d6c32b9..46833001 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,4 @@ hack/* .ok/ .okignore .ginkgo/ +!pkg/cli/webapp/dist/index.html diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index dc717cba..00000000 --- a/Dockerfile +++ /dev/null @@ -1,82 +0,0 @@ -FROM flanksource/base-image:latest - -ARG TZ -ENV TZ="$TZ" - -ARG CLAUDE_CODE_VERSION=latest -ARG USERNAME=claude -ARG USER_UID=501 -ARG USER_GID=20 - -# Create user/group matching host (default: moshe:501:20) -RUN if ! getent group ${USER_GID} > /dev/null 2>&1; then groupadd -g ${USER_GID} ${USERNAME}; fi && \ - useradd -u ${USER_UID} -g ${USER_GID} -m -s /bin/zsh ${USERNAME} && \ - mkdir -p /home/${USERNAME}/.claude && \ - chown -R ${USERNAME}:${USER_GID} /home/${USERNAME} - -# Install Node.js and basic development tools -RUN curl -fsSL https://deb.nodesource.com/setup_22.x | bash - && \ - apt-get update && apt-get install -y --no-install-recommends \ - nodejs \ - less \ - git \ - procps \ - sudo \ - fzf \ - zsh \ - man-db \ - unzip \ - gnupg2 \ - gh \ - iptables \ - ipset \ - iproute2 \ - dnsutils \ - aggregate \ - jq \ - nano \ - vim \ - build-essential \ - gosu \ - && apt-get clean && rm -rf /var/lib/apt/lists/* - -# Ensure user has access to /usr/local/share -RUN mkdir -p /usr/local/share/npm-global && \ - chown -R ${USERNAME} /usr/local/share - - -# Set `DEVCONTAINER` environment variable to help with orientation -ENV DEVCONTAINER=true - -# Create workspace directory -RUN mkdir -p /workspace && chown ${USERNAME}:${USER_GID} /workspace - -WORKDIR /workspace - -ARG GIT_DELTA_VERSION=0.18.2 -RUN ARCH=$(dpkg --print-architecture) && \ - wget "https://github.com/dandavison/delta/releases/download/${GIT_DELTA_VERSION}/git-delta_${GIT_DELTA_VERSION}_${ARCH}.deb" && \ - sudo dpkg -i "git-delta_${GIT_DELTA_VERSION}_${ARCH}.deb" && \ - rm "git-delta_${GIT_DELTA_VERSION}_${ARCH}.deb" - -COPY entrypoint.sh /usr/local/bin/entrypoint.sh -RUN chmod +x /usr/local/bin/entrypoint.sh - -USER ${USERNAME} - -# Install global packages -ENV NPM_CONFIG_PREFIX=/usr/local/share/npm-global -ENV PATH=$PATH:/usr/local/share/npm-global/bin - -# Set the default shell to zsh rather than sh -ENV SHELL=/bin/zsh - -# Set the default editor and visual -ENV EDITOR=nano -ENV VISUAL=nano - -# Install Claude -RUN npm install -g @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION} - -USER root -ENTRYPOINT ["/usr/local/bin/entrypoint.sh"] diff --git a/README.md b/README.md index 5d358fb4..e8fb8b91 100644 --- a/README.md +++ b/README.md @@ -188,11 +188,10 @@ captain/ ├── pkg/cmux/ # Terminal multiplexer integration (processes and screenshots) ├── pkg/collections/ # Generic collection utilities ├── pkg/container/ # Sandbox discovery, generation, build/run logic +├── pkg/container/base/ # Embedded agent base image (Dockerfile, deps.yaml, entrypoint.sh) ├── pkg/dod/ # Definition of Done persistence and execution ├── pkg/git/ # Git worktree helpers ├── pkg/sandbox/ # Token/preset/sandbox helpers -├── Dockerfile # Container image for captain/Claude tooling -├── entrypoint.sh # gosu-based user switching entrypoint ├── Makefile # Thin wrapper around Taskfile └── Taskfile.yaml # Main developer tasks ``` @@ -615,11 +614,20 @@ By default this copies the built binary to: ## Docker image -The included `Dockerfile` builds on `flanksource/base-image` and installs: - -- Node.js -- git, gh, jq, vim, nano, zsh, fzf, etc. -- Claude Code via `@anthropic-ai/claude-code` +`pkg/container/base/Dockerfile` (embedded into the binary, built as `claude-env:base` by +`captain container`) builds on `flanksource/base-image` and installs a full agent toolchain: + +- **Agent CLIs** — one per backend: `claude` (`@anthropic-ai/claude-code`), `codex` + (`@openai/codex`), `gemini` (`@google/gemini-cli`), plus `tsx` for the `claude-agent` + SDK bridge +- **Flanksource tools** — `captain`, `gavel`, `repomap` (installed via `deps` from + `pkg/container/base/deps.yaml`) +- **Go** — toolchain, `task`, `ginkgo`, `golangci-lint` +- **Node** — Node.js 22, `npm`, `pnpm`, `typescript` +- **Browser automation** — `agent-browser` and Playwright Chromium (shared at + `/ms-playwright`) +- **Shell tooling** — `rg`, `fd`, `bat`, `delta`, `gh`, `jq`, `tree`, `htop`, `lsof`, + `sqlite3`, `psql`, `tmux`, `shellcheck`, `vim`, `nano`, `zsh`, `fzf`, git The image is set up to: @@ -627,6 +635,21 @@ The image is set up to: - switch execution using `gosu` - use `/workspace` as the working directory +Building it requires BuildKit. Set `GITHUB_TOKEN` (or `GH_TOKEN`) before +`captain container build` to avoid GitHub's unauthenticated API rate limit — captain passes +it through as a BuildKit secret, so it never lands in image history. To build by hand: + +```bash +DOCKER_BUILDKIT=1 docker build -t claude-env:base \ + --secret id=GITHUB_TOKEN,env=GITHUB_TOKEN \ + pkg/container/base +``` + +Publishing to `flanksource/captain` on Docker Hub and GHCR (`linux/amd64` + +`linux/arm64`) is the **Publish Image** workflow. It is manual only — dispatch it from +the tag you want to ship, and only once that tag's release exists, since the image +installs `captain` from the latest GitHub release. + ## Dependencies and stack Primary stack: diff --git a/entrypoint.sh b/entrypoint.sh deleted file mode 100644 index 205ce484..00000000 --- a/entrypoint.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/bin/sh -set -e -EXEC_UID="${USER_UID:-501}" -EXEC_GID="${USER_GID:-20}" -exec gosu "${EXEC_UID}:${EXEC_GID}" "$@" diff --git a/go.mod b/go.mod index 82bff7e2..17d1c3a9 100644 --- a/go.mod +++ b/go.mod @@ -9,9 +9,9 @@ require ( github.com/aws/aws-sdk-go-v2/service/sts v1.42.0 github.com/charmbracelet/huh v1.0.0 github.com/firebase/genkit/go v1.11.0 - github.com/flanksource/clicky v1.21.52 - github.com/flanksource/clicky/aichat v1.21.48 - github.com/flanksource/commons v1.55.0 + github.com/flanksource/clicky v1.21.54 + github.com/flanksource/clicky/aichat v1.21.54 + github.com/flanksource/commons v1.56.0 github.com/flanksource/sandbox-runtime v1.0.2 github.com/fsnotify/fsnotify v1.9.0 github.com/google/dotprompt/go v0.0.0-20260502013637-5cd4a8405ca3 @@ -40,7 +40,7 @@ require ( ) require ( - github.com/flanksource/commons-db v0.1.26 + github.com/flanksource/commons-db v0.1.27 github.com/gliderlabs/ssh v0.3.8 github.com/pelletier/go-toml/v2 v2.4.3 ) diff --git a/go.sum b/go.sum index 74e16fbe..0c153fba 100644 --- a/go.sum +++ b/go.sum @@ -274,14 +274,14 @@ github.com/fergusstrange/embedded-postgres v1.34.0 h1:c6RKhPKFsLVU+Tdxsx8q0UxCHs github.com/fergusstrange/embedded-postgres v1.34.0/go.mod h1:w0YvnCgf19o6tskInrOOACtnqfVlOvluz3hlNLY7tRk= github.com/firebase/genkit/go v1.11.0 h1:dmf219fDGP6IcKwPkJoTnXpcTEya7o5Bi+z5iP+FuTk= github.com/firebase/genkit/go v1.11.0/go.mod h1:x9h3TsbMLiuK1hgbwnkUmKQ9bmTNPORHaqG9XQhI0K8= -github.com/flanksource/clicky v1.21.52 h1:JtcBD05mIbE0cLuhu218Z9uO6O2mFARmgrtEIBHCoVE= -github.com/flanksource/clicky v1.21.52/go.mod h1:eonv42hF6W1IPjQXOL6roBm/nHCYNomqc3Kps2E3RZE= -github.com/flanksource/clicky/aichat v1.21.48 h1:f8Kvl96Lfp1qcqPuve1zsjaYN8ZcK1/FYqnkGA7VN30= -github.com/flanksource/clicky/aichat v1.21.48/go.mod h1:PGN/lVAgxpchRctciUCpR4YIuqWoDwRxDh339A6wi3w= -github.com/flanksource/commons v1.55.0 h1:gj9zBY3V1qgAAnEiLaeGbkqCmNK0p1tJVQCDurdTZ2k= -github.com/flanksource/commons v1.55.0/go.mod h1:gupTCRqGpgD8dd2ooE7bMDJxkfcVKvkPVuHg3cWkW+Q= -github.com/flanksource/commons-db v0.1.26 h1:NXAP0WvMs4ufyDfl1L2ryRxBv5qxV67GiI1nINd4YIw= -github.com/flanksource/commons-db v0.1.26/go.mod h1:i378WIxy8g9xOeLBvhh1y3FO99oCumUqNmfhuDF79kM= +github.com/flanksource/clicky v1.21.54 h1:FZUh4GjcbnR6yh7qhdZb9Y1H2TXwlUfIfslMD+EJ14o= +github.com/flanksource/clicky v1.21.54/go.mod h1:eonv42hF6W1IPjQXOL6roBm/nHCYNomqc3Kps2E3RZE= +github.com/flanksource/clicky/aichat v1.21.54 h1:1qCcHfDcN2HrxW6/TqhfbQBNfLAXBXydIh1DkmJ1JJY= +github.com/flanksource/clicky/aichat v1.21.54/go.mod h1:JWR4cNoHQgBYiWE14cPwteWFzflPlJPStWvkY/DFA7E= +github.com/flanksource/commons v1.56.0 h1:/L1eWb3iLDM3UQEgd6vCTlG9Kbl6ssH6S0YR5f026IY= +github.com/flanksource/commons v1.56.0/go.mod h1:gupTCRqGpgD8dd2ooE7bMDJxkfcVKvkPVuHg3cWkW+Q= +github.com/flanksource/commons-db v0.1.27 h1:F6g5EY2pbK1qO/S6iewuk+VvYw7rdfp0xH9aiFP2AEU= +github.com/flanksource/commons-db v0.1.27/go.mod h1:7zw8o/HLP/sTTAzb+7NHa50Iy8FIOevUjan+900MnBc= github.com/flanksource/gomplate/v3 v3.24.84 h1:UOE0yCJsczTIKRaHUvhD6tjCYrbNvOugAizuy0FVlhE= github.com/flanksource/gomplate/v3 v3.24.84/go.mod h1:NMMZkFsjbLy/8iY8Fip5N86Y0PP6lZeq+kmPwpVVIL0= github.com/flanksource/is-healthy v1.0.88 h1:ATQuKoNdp8Qfzf41/eMFajmT0qzOmZlZNG5eLK41RFo= diff --git a/pkg/ai/agent/commit/notice_test.go b/pkg/ai/agent/commit/notice_test.go new file mode 100644 index 00000000..59c28149 --- /dev/null +++ b/pkg/ai/agent/commit/notice_test.go @@ -0,0 +1,95 @@ +package commit + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/flanksource/captain/pkg/ai" + "github.com/flanksource/captain/pkg/ai/agent" + "github.com/flanksource/captain/pkg/api" +) + +// TestNoticesReachTheRunsEventStream drives the hook through the real Runner +// rather than calling Post directly, because the link that used to be missing is +// precisely the one between a hook and the run's event stream: a hook could +// record a commit on the workspace but had no way to say so while the run was +// still going. Everything downstream — the terminal renderer, the dashboard's +// SSE frames — reads that stream, so a notice that never reaches it is invisible +// no matter how well the renderers work. +func TestNoticesReachTheRunsEventStream(t *testing.T) { + dir := newRepo(t) + target := filepath.Join(dir, "fix.go") + + var streamed []string + runner := &agent.Runner[string]{ + Provider: &writeOnceProvider{path: target}, + Repo: dir, + Cwd: dir, + Request: ai.Request{Prompt: api.Prompt{User: "fix it"}}, + Hooks: []any{New(api.Commit{ + On: api.CommitOnTurn, Mode: api.CommitModeCommit, Message: "fix: the thing", + })}, + OnEvent: func(_ int, ev ai.Event) { + if ev.Kind == ai.EventSystem && ev.Text != "" { + streamed = append(streamed, ev.Text) + } + }, + } + res, err := runner.Run(context.Background()) + if err != nil { + t.Fatalf("run: %v", err) + } + + commits := res.Response.Workspace.Commits + if len(commits) != 1 { + t.Fatalf("commits = %+v, want exactly one", commits) + } + want := "[post-turn] committed " + commits[0].SHA[:7] + ": fix: the thing" + if len(streamed) != 2 || streamed[0] != "[post-turn] committing 1 file(s)" || streamed[1] != want { + t.Errorf("streamed notices = %q, want the committing/committed pair ending in %q", streamed, want) + } + + // The same lines are also buffered for the caller to persist once the run's + // session id is known; the stream alone dies with the terminal. + var buffered []string + for _, notice := range res.Response.Workspace.Notices { + buffered = append(buffered, notice.Text) + } + if strings.Join(buffered, "\n") != strings.Join(streamed, "\n") { + t.Errorf("buffered notices = %q, want the same lines as the stream %q", buffered, streamed) + } +} + +// writeOnceProvider is an agent that edits one file on its first turn and +// nothing afterwards, so exactly one commit is cut. +type writeOnceProvider struct { + path string + runs int +} + +func (p *writeOnceProvider) GetModel() string { return "fake" } +func (p *writeOnceProvider) GetBackend() ai.Backend { return ai.Backend("fake") } +func (p *writeOnceProvider) Execute(context.Context, ai.Request) (*ai.Response, error) { + return &ai.Response{}, nil +} + +func (p *writeOnceProvider) ExecuteStream(_ context.Context, _ ai.Request) (<-chan ai.Event, error) { + first := p.runs == 0 + p.runs++ + ch := make(chan ai.Event, 4) + go func() { + defer close(ch) + if first { + if err := os.WriteFile(p.path, []byte("package main\n"), 0o600); err != nil { + ch <- ai.Event{Kind: ai.EventError, Error: err.Error()} + return + } + ch <- ai.Event{Kind: ai.EventToolUse, Tool: "Write", Input: map[string]any{"file_path": p.path}} + } + ch <- ai.Event{Kind: ai.EventResult, Success: true} + }() + return ch, nil +} diff --git a/pkg/ai/provider/claude_cli.go b/pkg/ai/provider/claude_cli.go index ffe934c3..6fba5de5 100644 --- a/pkg/ai/provider/claude_cli.go +++ b/pkg/ai/provider/claude_cli.go @@ -89,6 +89,11 @@ func (c *ClaudeCLI) ExecuteStream(ctx context.Context, req ai.Request) (<-chan a func buildClaudeCLIArgs(model string, req ai.Request) ([]string, func(), error) { args := []string{"-p", "--verbose", "--output-format", "stream-json"} cleanup := func() {} + // claude-cli advertises tool-policy support, but only for allow/deny: the + // guard still has to reject the policies no transport can express. + if err := api.RequireToolPolicySupport(api.BackendClaudeCLI, req.Permissions); err != nil { + return nil, cleanup, err + } if m := claudeCLIModel(model); m != "" { args = append(args, "--model", m) } @@ -110,11 +115,11 @@ func buildClaudeCLIArgs(model string, req ai.Request) ([]string, func(), error) if mode := cliClaudePermissionMode(req.Permissions.Mode); mode != "" { args = append(args, "--permission-mode", mode) } - if len(req.Permissions.Tools.Allow) > 0 { - args = append(args, "--allowedTools", strings.Join(req.Permissions.Tools.Allow, ",")) + if allow := req.Permissions.Tools.AllowList(); len(allow) > 0 { + args = append(args, "--allowedTools", strings.Join(allow, ",")) } - if len(req.Permissions.Tools.Deny) > 0 { - args = append(args, "--disallowedTools", strings.Join(req.Permissions.Tools.Deny, ",")) + if deny := req.Permissions.Tools.DenyList(); len(deny) > 0 { + args = append(args, "--disallowedTools", strings.Join(deny, ",")) } for _, dir := range req.Memory.Skills { if strings.TrimSpace(dir) != "" { diff --git a/pkg/ai/provider/claude_cli_test.go b/pkg/ai/provider/claude_cli_test.go index d6c94256..2c56326e 100644 --- a/pkg/ai/provider/claude_cli_test.go +++ b/pkg/ai/provider/claude_cli_test.go @@ -53,7 +53,10 @@ func TestBuildClaudeCLIArgs(t *testing.T) { requireFlagValue(t, args, "--effort", "high") requireFlagValue(t, args, "--max-budget-usd", "1.25") requireFlagValue(t, args, "--permission-mode", "acceptEdits") - requireFlagValue(t, args, "--allowedTools", "Read,Grep") + // The lists are the canonical policy map projected back out, so they are + // sorted rather than in caller order — order is not meaningful to claude, and + // a stable order keeps the command line reproducible. + requireFlagValue(t, args, "--allowedTools", "Grep,Read") requireFlagValue(t, args, "--disallowedTools", "Bash") requireFlagValue(t, args, "--plugin-dir", "/skills/a") requireFlagValue(t, args, "--mcp-config", `{"mcpServers":{}}`) @@ -166,6 +169,20 @@ func requireFlagValue(t *testing.T, args []string, flag, want string) { } } +// requireFlagPair asserts a (flag, value) pair appears anywhere in args. Use it +// for repeatable flags — codex's -c carries several unrelated overrides, so +// requireFlagValue's first-match-wins lookup answers about whichever one the +// builder happened to emit first. +func requireFlagPair(t *testing.T, args []string, flag, want string) { + t.Helper() + for i, arg := range args { + if arg == flag && i+1 < len(args) && args[i+1] == want { + return + } + } + t.Fatalf("args %v do not contain %s %q", args, flag, want) +} + func flagValue(t *testing.T, args []string, flag string) string { t.Helper() for i, arg := range args { diff --git a/pkg/ai/provider/claudeagent/agent.ts b/pkg/ai/provider/claudeagent/agent.ts index c7fd8dc2..1ad5eba1 100644 --- a/pkg/ai/provider/claudeagent/agent.ts +++ b/pkg/ai/provider/claudeagent/agent.ts @@ -64,6 +64,7 @@ interface InitializeParams { systemPrompt?: string; appendSystemPrompt?: string; allowedTools?: string[]; + disallowedTools?: string[]; maxTurns?: number; maxBudgetUsd?: number; permissionMode?: string; @@ -109,18 +110,28 @@ function buildOptions(params: InitializeParams): Options { // SDK must consult canUseTool rather than auto-approving. bypassPermissions / // allowDangerouslySkipPermissions would skip canUseTool entirely. const brokered = params.approvalMode === "ask"; + // The host always sends a resolved mode; "default" is the floor when it did + // not. Skipping permissions is gated on the caller having ASKED for bypass — + // keying it on "no broker attached" turned every unbrokered run, which is + // almost all of them, into an unconfined one. + const permissionMode = (params.permissionMode as Options["permissionMode"]) || + "default"; const options: Options = { cwd: params.cwd, model: params.model, maxTurns: params.maxTurns || undefined, maxBudgetUsd: params.maxBudgetUsd || undefined, - permissionMode: (params.permissionMode as Options["permissionMode"]) || - (brokered ? "default" : "bypassPermissions"), - allowDangerouslySkipPermissions: !brokered, + permissionMode, + allowDangerouslySkipPermissions: + !brokered && permissionMode === "bypassPermissions", allowedTools: params.allowedTools && params.allowedTools.length ? params.allowedTools : undefined, + disallowedTools: + params.disallowedTools && params.disallowedTools.length + ? params.disallowedTools + : undefined, mcpServers: params.mcpServers, stderr: (data: string) => process.stderr.write(data), hooks: { diff --git a/pkg/ai/provider/claudeagent/bridge_params.go b/pkg/ai/provider/claudeagent/bridge_params.go index 0aead56c..e37bbae1 100644 --- a/pkg/ai/provider/claudeagent/bridge_params.go +++ b/pkg/ai/provider/claudeagent/bridge_params.go @@ -8,6 +8,7 @@ type initializeParams struct { SystemPrompt string `json:"systemPrompt,omitempty"` AppendSystemPrompt string `json:"appendSystemPrompt,omitempty"` AllowedTools []string `json:"allowedTools,omitempty"` + DisallowedTools []string `json:"disallowedTools,omitempty"` MaxTurns int `json:"maxTurns,omitempty"` MaxBudgetUsd float64 `json:"maxBudgetUsd,omitempty"` PermissionMode string `json:"permissionMode,omitempty"` diff --git a/pkg/ai/provider/claudeagent/permissions_test.go b/pkg/ai/provider/claudeagent/permissions_test.go new file mode 100644 index 00000000..8f5cec95 --- /dev/null +++ b/pkg/ai/provider/claudeagent/permissions_test.go @@ -0,0 +1,125 @@ +package claudeagent + +import ( + "context" + "testing" + + "github.com/flanksource/captain/pkg/ai" + "github.com/flanksource/captain/pkg/api" + "github.com/stretchr/testify/assert" +) + +// TestInitializeParams_PermissionMode pins the posture the SDK child is started +// with. An absent permissions block must resolve to the ask/deny default, never +// to bypass: "the caller declared no policy" is not "the caller granted +// everything", and CanUseTool is nil on every path except the chat server, so +// the unbrokered branch is the common one rather than the exotic one. +func TestInitializeParams_PermissionMode(t *testing.T) { + broker := func(context.Context, ai.PermissionRequest) (ai.PermissionDecision, error) { + return ai.PermissionDecision{Allow: true}, nil + } + + tests := []struct { + name string + mode api.PermissionMode + presets []api.Preset + broker ai.PermissionFunc + wantMode string + wantApproval string + }{ + { + name: "unset and unbrokered falls back to default, not bypass", + wantMode: "default", + wantApproval: "auto", + }, + { + name: "unset and brokered defers to the broker", + broker: broker, + wantMode: "default", + wantApproval: "ask", + }, + { + name: "an explicit bypass is still honoured", + mode: api.PermissionBypass, + wantMode: "bypassPermissions", + wantApproval: "auto", + }, + { + name: "plan passes through", + mode: api.PermissionPlan, + wantMode: "plan", + wantApproval: "auto", + }, + { + name: "the edit preset still implies acceptEdits", + presets: []api.Preset{api.PresetEdit}, + wantMode: "acceptEdits", + wantApproval: "auto", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &Provider{cfg: ai.Config{CanUseTool: tt.broker}} + params := p.initializeParams(ai.Request{ + Permissions: api.Permissions{Mode: tt.mode, Presets: tt.presets}, + }) + assert.Equal(t, tt.wantMode, params.PermissionMode) + assert.Equal(t, tt.wantApproval, params.ApprovalMode) + }) + } +} + +// TestInitializeParams_EditPresetAllowlist keeps --edit's curated allowlist +// attached to the preset rather than to the absence of a permission mode. +func TestInitializeParams_EditPresetAllowlist(t *testing.T) { + p := &Provider{} + params := p.initializeParams(ai.Request{ + Permissions: api.Permissions{Presets: []api.Preset{api.PresetEdit}}, + }) + assert.Equal(t, safeEditAllowlist, params.AllowedTools) + + explicit := p.initializeParams(ai.Request{ + Permissions: api.Permissions{ + Presets: []api.Preset{api.PresetEdit}, + Tools: api.Tools{Allow: []string{"Read"}}, + }, + }) + assert.Equal(t, []string{"Read"}, explicit.AllowedTools, + "an explicit allowlist must not be replaced by the preset's") +} + +// TestInitializeParams_NormalizesToolModes pins that the SDK child is configured +// from the canonical policy map, not the raw Allow/Deny slices: `tools: {Bash: +// off}` lands in Modes only, so forwarding Tools.Deny verbatim would let a tool +// the spec turned off run. +func TestInitializeParams_NormalizesToolModes(t *testing.T) { + p := &Provider{} + params := p.initializeParams(ai.Request{ + Permissions: api.Permissions{ + Tools: api.Tools{ + Deny: []string{"WebFetch"}, + Modes: map[string]api.ToolMode{"Bash": api.ToolModeOff, "Read": api.ToolModeOn}, + }, + }, + }) + assert.Equal(t, []string{"Bash", "WebFetch"}, params.DisallowedTools, + "an off tool mode is a deny and must reach disallowedTools") + // `on` resolves to auto — the agent's normal behaviour — so it must not + // narrow the run into a one-tool allowlist. + assert.Empty(t, params.AllowedTools) +} + +// TestExecuteStream_RefusesUnenforceableAskPolicy keeps the advertised +// tool-policy support honest: claude-agent carries allow/deny lists only, so an +// `ask` policy must fail loudly rather than resolve to "allowed". +func TestExecuteStream_RefusesUnenforceableAskPolicy(t *testing.T) { + p := &Provider{} + _, err := p.ExecuteStream(context.Background(), ai.Request{ + Permissions: api.Permissions{ + Tools: api.Tools{Modes: map[string]api.ToolMode{"Bash": api.ToolModeAsk}}, + }, + }) + assert.ErrorContains(t, err, "ask") + assert.ErrorContains(t, err, "Bash") +} diff --git a/pkg/ai/provider/claudeagent/provider.go b/pkg/ai/provider/claudeagent/provider.go index 8ef7c2d7..6ef7d7bb 100644 --- a/pkg/ai/provider/claudeagent/provider.go +++ b/pkg/ai/provider/claudeagent/provider.go @@ -274,6 +274,11 @@ func (p *Provider) resultError(req ai.Request, subtype, lastErr string) error { // events back. Turns are serialized via turnMu so the single SDK session is // never driven by two prompts at once. func (p *Provider) ExecuteStream(ctx context.Context, req ai.Request) (<-chan ai.Event, error) { + // claude-agent advertises tool-policy support, but only for allow/deny: + // initializeParams has no way to express the policies the guard rejects. + if err := api.RequireToolPolicySupport(api.BackendClaudeAgent, req.Permissions); err != nil { + return nil, err + } schema, err := requestSchemaJSON(req) if err != nil { return nil, err @@ -410,7 +415,10 @@ func (p *Provider) initializeParams(req ai.Request) initializeParams { brokered := p.cfg.CanUseTool != nil mode := string(req.Permissions.Mode) - allowed := req.Permissions.Tools.Allow + // AllowList/DenyList, not the raw Allow/Deny slices: an `off` tool mode is a + // deny that only the normalized policy map reports, and forwarding the raw + // slice would let `tools: {Bash: off}` run. + allowed := req.Permissions.Tools.AllowList() if req.Permissions.HasPreset(api.PresetEdit) { if mode == "" { mode = "acceptEdits" @@ -419,12 +427,14 @@ func (p *Provider) initializeParams(req ai.Request) initializeParams { allowed = safeEditAllowlist } } + // An absent permissions block is "the caller declared no policy", never "the + // caller granted everything" — so it resolves to the ask/deny default whether + // or not a broker is attached. CanUseTool is nil on every path but the chat + // server, so the unbrokered branch is the common one: defaulting it to bypass + // meant a prompt with no `permissions:` ran unconfined here while the same + // prompt on claude-cli got the default posture. if mode == "" { - if brokered { - mode = "default" - } else { - mode = "bypassPermissions" - } + mode = "default" } approvalMode := "auto" @@ -451,6 +461,7 @@ func (p *Provider) initializeParams(req ai.Request) initializeParams { SystemPrompt: req.Prompt.System, AppendSystemPrompt: req.Prompt.AppendSystem, AllowedTools: allowed, + DisallowedTools: req.Permissions.Tools.DenyList(), MaxTurns: req.Budget.MaxTurns, MaxBudgetUsd: maxBudget, PermissionMode: mode, diff --git a/pkg/ai/provider/cmux/provider.go b/pkg/ai/provider/cmux/provider.go index 0b14ec94..dff05758 100644 --- a/pkg/ai/provider/cmux/provider.go +++ b/pkg/ai/provider/cmux/provider.go @@ -178,6 +178,11 @@ func (p *Provider) ExecuteStream(ctx context.Context, req ai.Request) (<-chan ai if req.Prompt.User == "" { return nil, fmt.Errorf("cmux provider: prompt is required") } + // AgentCommand's codex branch emits no tool flags — codex has no equivalent — + // so a policy set here would be dropped rather than applied. + if err := api.RequireToolPolicySupport(p.GetBackend(), req.Permissions); err != nil { + return nil, err + } events := make(chan ai.Event, 32) go p.drive(ctx, req, schema, events) return events, nil @@ -314,8 +319,8 @@ func (p *Provider) execute(ctx context.Context, req ai.Request, r *run) (*ai.Usa Resume: resume, Plan: req.Permissions.Mode == api.PermissionPlan, PermissionMode: req.Permissions.Mode, - AllowedTools: req.Permissions.Tools.Allow, - DisallowedTools: req.Permissions.Tools.Deny, + AllowedTools: req.Permissions.Tools.AllowList(), + DisallowedTools: req.Permissions.Tools.DenyList(), Effort: req.Effort, Memory: req.Memory, Extra: extra, diff --git a/pkg/ai/provider/codex_appserver.go b/pkg/ai/provider/codex_appserver.go index e474adaa..db70d4de 100644 --- a/pkg/ai/provider/codex_appserver.go +++ b/pkg/ai/provider/codex_appserver.go @@ -39,6 +39,10 @@ type CodexAppServer struct { rpcDone chan struct{} // closed by the rpc Run goroutine when the child exits active *turnState threadID string + // posture is the current run's approval policy. handleApproval runs on the + // rpc read loop, not the turn goroutine, so it cannot reach the request; the + // posture is recorded here when the turn starts. + posture codexPosture callerToolsMu sync.Mutex callerToolsRuntime *callertools.Runtime @@ -127,7 +131,7 @@ func (c *CodexAppServer) ExecuteStream(ctx context.Context, req ai.Request) (<-c if err := c.prepareCallerTools(req); err != nil { return nil, err } - c.turnMu.Lock() + c.beginTurn(req) if err := c.ensureStarted(ctx); err != nil { c.turnMu.Unlock() return nil, err @@ -189,7 +193,27 @@ func (c *CodexAppServer) failTurn(ts *turnState, err error) { ts.finish() } +// beginTurn takes the turn lock and records this run's approval posture under +// it. The posture must be recorded before the process can send an approval +// request — the rpc read loop is already live for a resumed provider, and a +// stale posture would answer this run's approvals from the last run's policy — +// but never before the lock: a second, more permissive ExecuteStream queued +// behind an in-flight turn would otherwise overwrite the posture that turn's +// approvals are still being judged against. driveTurn releases turnMu. +func (c *CodexAppServer) beginTurn(req ai.Request) { + c.turnMu.Lock() + c.setPosture(postureFor(req)) +} + func (c *CodexAppServer) setActive(ts *turnState) { c.mu.Lock(); c.active = ts; c.mu.Unlock() } + +func (c *CodexAppServer) setPosture(p codexPosture) { c.mu.Lock(); c.posture = p; c.mu.Unlock() } + +func (c *CodexAppServer) currentPosture() codexPosture { + c.mu.Lock() + defer c.mu.Unlock() + return c.posture +} func (c *CodexAppServer) currentTurn() *turnState { c.mu.Lock(); defer c.mu.Unlock(); return c.active } func (c *CodexAppServer) client() *jsonrpc.Client { c.mu.Lock(); defer c.mu.Unlock(); return c.rpc } diff --git a/pkg/ai/provider/codex_appserver_approval.go b/pkg/ai/provider/codex_appserver_approval.go index ce4b293d..fff1e67a 100644 --- a/pkg/ai/provider/codex_appserver_approval.go +++ b/pkg/ai/provider/codex_appserver_approval.go @@ -3,20 +3,69 @@ package provider import ( "encoding/json" + "github.com/flanksource/captain/pkg/ai" "github.com/flanksource/captain/pkg/ai/provider/jsonrpc" + "github.com/flanksource/captain/pkg/api" ) -// handleApproval auto-approves server-to-client approval requests, mirroring -// the bypass-permissions default of the exec path. +// codexPosture is the approval-relevant slice of a run's resolved permissions, +// recorded once per run so the server→client approval handler can answer from +// the policy the run declared. +type codexPosture struct { + // grantsEscalation reports that the run asked for full access. It is the only + // posture under which accepting an approval stays within what the caller + // declared: every other posture bounds the agent more tightly than the thing + // it is asking permission to do. + grantsEscalation bool + // planMode is a plan-only run, which must reach no side effect at all. + planMode bool +} + +func postureFor(req ai.Request) codexPosture { + sandbox, approval := api.CodexSafety(req.Permissions) + return codexPosture{ + grantsEscalation: sandbox == api.CodexSandboxDangerFull && approval == api.CodexApprovalNever, + planMode: req.Permissions.Mode == api.PermissionPlan, + } +} + +// allowsEscalation reports whether an approval request may be accepted. +func (p codexPosture) allowsEscalation() bool { return p.grantsEscalation && !p.planMode } + +// handleApproval answers a server→client approval request from the run's +// posture. +// +// An approval request is codex asking to exceed the sandbox it was started +// with: accepting one runs a command outside the confinement or writes a file +// the sandbox denied. Answering "accept" unconditionally therefore made +// buildThreadStartParams' sandbox and approvalPolicy advisory — the model asked, +// captain said yes — so `mode: plan` and a read-only posture gated nothing on +// this backend while gating correctly on the exec path. +// +// Only a run that declared full access has already granted the escalation; +// every other posture declines and lets the turn continue, so the agent adapts +// rather than the run dying. The decision vocabularies are codex's own: +// accept|decline (item/*, v2) and approved|denied (the legacy methods), per +// `codex app-server generate-json-schema`. func (c *CodexAppServer) handleApproval(method string, _ json.RawMessage) (any, *jsonrpc.RPCError) { + allow := c.currentPosture().allowsEscalation() switch method { case "item/commandExecution/requestApproval", "item/fileChange/requestApproval": - return map[string]string{"decision": "accept"}, nil + return map[string]string{"decision": codexDecision(allow, "accept", "decline")}, nil case "item/permissions/requestApproval": + // Granting no additional permissions is right under every posture: the + // thread already carries everything the run declared. return map[string]any{"permissions": map[string]any{}, "scope": "turn"}, nil case "item/tool/requestUserInput": return map[string]any{}, nil default: - return map[string]string{"decision": "approved"}, nil + return map[string]string{"decision": codexDecision(allow, "approved", "denied")}, nil + } +} + +func codexDecision(allow bool, accept, decline string) string { + if allow { + return accept } + return decline } diff --git a/pkg/ai/provider/codex_appserver_params_test.go b/pkg/ai/provider/codex_appserver_params_test.go index 73db9254..cae854c4 100644 --- a/pkg/ai/provider/codex_appserver_params_test.go +++ b/pkg/ai/provider/codex_appserver_params_test.go @@ -1,6 +1,7 @@ package provider import ( + "runtime" "testing" "github.com/flanksource/captain/pkg/ai" @@ -92,33 +93,115 @@ func TestBuildResumeParams(t *testing.T) { assert.Equal(t, "/repo", p["cwd"]) } -func TestHandleApproval_AutoApproves(t *testing.T) { - c, err := NewCodexAppServer(ai.Config{Model: api.Model{Name: "m"}}) - require.NoError(t, err) - tests := []struct { - method string - key string - want any +// TestHandleApproval_AnswersFromPosture pins the approval policy. An approval +// request is codex asking to exceed the sandbox it was started with, so only a +// run that declared full access may accept one — otherwise buildThreadStartParams' +// sandbox and approvalPolicy are advisory and `mode: plan` gates nothing. +// The decision vocabularies are codex's own, from `codex app-server +// generate-json-schema`: accept|decline (item/*) and approved|denied (legacy). +func TestHandleApproval_AnswersFromPosture(t *testing.T) { + methods := []struct{ method, accept, decline string }{ + {"execCommandApproval", "approved", "denied"}, + {"applyPatchApproval", "approved", "denied"}, + {"item/commandExecution/requestApproval", "accept", "decline"}, + {"item/fileChange/requestApproval", "accept", "decline"}, + {"some/unknown/approval", "approved", "denied"}, + } + postures := []struct { + name string + permissions api.Permissions + wantAccept bool }{ - {"execCommandApproval", "decision", "approved"}, - {"applyPatchApproval", "decision", "approved"}, - {"item/commandExecution/requestApproval", "decision", "accept"}, - {"item/fileChange/requestApproval", "decision", "accept"}, - {"some/unknown/approval", "decision", "approved"}, + { + name: "the default read-only posture declines", + permissions: api.Permissions{}, + }, + { + name: "plan declines", + permissions: api.Permissions{Mode: api.PermissionPlan}, + }, + { + name: "the edit preset declines escalation beyond its workspace", + permissions: api.Permissions{Presets: []api.Preset{api.PresetEdit}}, + }, + { + name: "an explicit bypass has already granted it", + permissions: api.Permissions{Mode: api.PermissionBypass}, + wantAccept: true, + }, } - for _, tc := range tests { - t.Run(tc.method, func(t *testing.T) { - res, rpcErr := c.handleApproval(tc.method, nil) + + for _, posture := range postures { + t.Run(posture.name, func(t *testing.T) { + c, err := NewCodexAppServer(ai.Config{Model: api.Model{Name: "m"}}) + require.NoError(t, err) + c.setPosture(postureFor(ai.Request{Permissions: posture.permissions})) + + for _, m := range methods { + want := m.decline + if posture.wantAccept { + want = m.accept + } + res, rpcErr := c.handleApproval(m.method, nil) + assert.Nil(t, rpcErr) + decision, ok := res.(map[string]string) + require.True(t, ok, "%s returns a string map", m.method) + assert.Equal(t, want, decision["decision"], m.method) + } + + // Additional permissions are never granted: the thread already carries + // everything the run declared. + res, rpcErr := c.handleApproval("item/permissions/requestApproval", nil) assert.Nil(t, rpcErr) - m, ok := res.(map[string]string) - require.True(t, ok, "decision approvals return a string map") - assert.Equal(t, tc.want, m[tc.key]) + perm, ok := res.(map[string]any) + require.True(t, ok) + assert.Equal(t, "turn", perm["scope"]) + assert.Empty(t, perm["permissions"]) }) } - res, rpcErr := c.handleApproval("item/permissions/requestApproval", nil) +} + +// TestBeginTurn_ConcurrentTurnCannotEscalateTheInFlightPosture pins the lock +// ordering: a bypass turn queued behind an in-flight restricted turn must not +// publish its posture until the restricted turn releases turnMu, or the +// restricted turn's approvals would be answered with the bypass policy. +func TestBeginTurn_ConcurrentTurnCannotEscalateTheInFlightPosture(t *testing.T) { + c, err := NewCodexAppServer(ai.Config{Model: api.Model{Name: "m"}}) + require.NoError(t, err) + + c.beginTurn(ai.Request{Permissions: api.Permissions{Presets: []api.Preset{api.PresetEdit}}}) + + queued := make(chan struct{}) + go func() { + defer close(queued) + c.beginTurn(ai.Request{Permissions: api.Permissions{Mode: api.PermissionBypass}}) + c.turnMu.Unlock() + }() + + // The queued turn is blocked on turnMu, so approvals raised by the still + // in-flight restricted turn keep declining. + for i := 0; i < 50; i++ { + res, rpcErr := c.handleApproval("item/commandExecution/requestApproval", nil) + assert.Nil(t, rpcErr) + require.Equal(t, "decline", res.(map[string]string)["decision"], + "the queued bypass turn overwrote the in-flight restricted posture") + runtime.Gosched() + } + + c.turnMu.Unlock() + <-queued + res, rpcErr := c.handleApproval("item/commandExecution/requestApproval", nil) + assert.Nil(t, rpcErr) + assert.Equal(t, "accept", res.(map[string]string)["decision"], + "the bypass turn's posture takes effect once it owns the turn") +} + +// TestHandleApproval_PostureDefaultsClosed guards the zero value: a provider +// that has not started a turn must not answer an approval as though it had. +func TestHandleApproval_PostureDefaultsClosed(t *testing.T) { + c, err := NewCodexAppServer(ai.Config{Model: api.Model{Name: "m"}}) + require.NoError(t, err) + res, rpcErr := c.handleApproval("item/commandExecution/requestApproval", nil) assert.Nil(t, rpcErr) - perm, ok := res.(map[string]any) - require.True(t, ok) - assert.Equal(t, "turn", perm["scope"]) - assert.NotNil(t, perm["permissions"]) + assert.Equal(t, "decline", res.(map[string]string)["decision"]) } diff --git a/pkg/ai/provider/codex_appserver_protocol.go b/pkg/ai/provider/codex_appserver_protocol.go index edb190b1..c900d306 100644 --- a/pkg/ai/provider/codex_appserver_protocol.go +++ b/pkg/ai/provider/codex_appserver_protocol.go @@ -363,6 +363,9 @@ func buildTurnStartParams(model string, req ai.Request, threadID string, outputS if err := ai.ValidateAttachmentCompatibility([]api.Model{{Name: model, Backend: api.BackendCodexAgent}}, req.Prompt.Attachments); err != nil { return nil, err } + if err := api.RequireToolPolicySupport(api.BackendCodexAgent, req.Permissions); err != nil { + return nil, err + } input := make([]map[string]any, 0, len(req.Prompt.Attachments)) if text := composePrompt(req); text != "" { input = append(input, map[string]any{"type": "text", "text": text}) diff --git a/pkg/ai/provider/codex_cli.go b/pkg/ai/provider/codex_cli.go index c27ef214..6062bb84 100644 --- a/pkg/ai/provider/codex_cli.go +++ b/pkg/ai/provider/codex_cli.go @@ -129,6 +129,9 @@ func buildCodexCLIArgs(cfg codexCLIConfig, req ai.Request) ([]string, func(), er if err := ai.ValidateAttachmentCompatibility([]api.Model{{Name: model, Backend: api.BackendCodexCLI}}, req.Prompt.Attachments); err != nil { return nil, cleanup, err } + if err := api.RequireToolPolicySupport(api.BackendCodexCLI, req.Permissions); err != nil { + return nil, cleanup, err + } if cfg.APIURL != "" { args = append(args, codexProviderOverride(cfg.APIURL)...) } @@ -148,10 +151,17 @@ func buildCodexCLIArgs(cfg codexCLIConfig, req ai.Request) ([]string, func(), er if cwd := req.Cwd(); cwd != "" { args = append(args, "-C", cwd) } - sandbox, _ := codexSafety(req) + // Both halves of the posture, so the exec path enforces what the app-server + // path enforces from the same helper. `codex exec` has no --ask-for-approval + // flag, so the approval policy rides on the config override instead; the key + // and its accepted values are validated by --strict-config. + sandbox, approval := codexSafety(req) if sandbox != "" { args = append(args, "--sandbox", sandbox) } + if approval != "" { + args = append(args, "-c", fmt.Sprintf("approval_policy=%q", approval)) + } if req.Memory.SkipMemory || req.Memory.Bare || req.Permissions.HasPreset(api.PresetBare) { args = append(args, "--ephemeral") } diff --git a/pkg/ai/provider/codex_cli_test.go b/pkg/ai/provider/codex_cli_test.go index faa67970..9007c77a 100644 --- a/pkg/ai/provider/codex_cli_test.go +++ b/pkg/ai/provider/codex_cli_test.go @@ -72,6 +72,58 @@ func TestBuildCodexCLIArgs(t *testing.T) { } } +// TestBuildCodexCLIArgsEmitsApprovalPolicy pins the other half of CodexSafety. +// `codex exec` has no --ask-for-approval flag (verified against codex-cli +// 0.147.0), so the approval policy the shared helper computes has to ride on +// -c approval_policy — dropping it left the exec path enforcing only half the +// posture the app-server path enforced from the same helper. +func TestBuildCodexCLIArgsEmitsApprovalPolicy(t *testing.T) { + tests := []struct { + name string + permissions api.Permissions + wantSandbox string + wantApproval string + }{ + { + name: "default is read-only and still asks", + wantSandbox: "read-only", + wantApproval: `approval_policy="on-request"`, + }, + { + name: "edit widens the sandbox but keeps asking", + permissions: api.Permissions{Presets: []api.Preset{api.PresetEdit}}, + wantSandbox: "workspace-write", + wantApproval: `approval_policy="on-request"`, + }, + { + name: "bypass grants full access and stops asking", + permissions: api.Permissions{Mode: api.PermissionBypass}, + wantSandbox: "danger-full-access", + wantApproval: `approval_policy="never"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args, cleanup, err := buildCodexCLIArgs( + codexCLIConfig{Model: "gpt-5.5"}, + ai.Request{Prompt: api.Prompt{User: "hi"}, Permissions: tt.permissions}, + ) + if err != nil { + t.Fatalf("buildCodexCLIArgs: %v", err) + } + defer cleanup() + requireFlagValue(t, args, "--sandbox", tt.wantSandbox) + requireFlagPair(t, args, "-c", tt.wantApproval) + for _, arg := range args { + if arg == "--ask-for-approval" { + t.Fatalf("emitted --ask-for-approval, which codex exec does not accept: %v", args) + } + } + }) + } +} + // codex ignores OPENAI_BASE_URL once an account credential is stored, so the // override has to be declared as a model provider on the command line. func TestBuildCodexCLIArgsRedirectsViaModelProvider(t *testing.T) { diff --git a/pkg/ai/provider/gemini_cli.go b/pkg/ai/provider/gemini_cli.go index ff21d44b..9d16ecd3 100644 --- a/pkg/ai/provider/gemini_cli.go +++ b/pkg/ai/provider/gemini_cli.go @@ -112,6 +112,9 @@ func buildGeminiCLIArgs(model string, req ai.Request) ([]string, error) { if err := ai.ValidateAttachmentCompatibility([]api.Model{{Name: model, Backend: api.BackendGeminiCLI}}, req.Prompt.Attachments); err != nil { return nil, err } + if err := api.RequireToolPolicySupport(api.BackendGeminiCLI, req.Permissions); err != nil { + return nil, err + } args := []string{"--output-format", "stream-json"} if m := strings.TrimSpace(model); m != "" { args = append(args, "--model", m) diff --git a/pkg/ai/provider/genkit/genkit.go b/pkg/ai/provider/genkit/genkit.go index 55616475..3e856030 100644 --- a/pkg/ai/provider/genkit/genkit.go +++ b/pkg/ai/provider/genkit/genkit.go @@ -229,6 +229,12 @@ func (p *Provider) correlatedGenerateOptions( emit func(ai.Event), correlation *toolEventCorrelation, ) ([]gkai.GenerateOption, error) { + // Caller tools are gated by ToolPreferences and CanUseTool; Permissions.Tools + // is a separate policy this backend has no seam for, so it must not be + // accepted and ignored. + if err := api.RequireToolPolicySupport(p.backend, req.Permissions); err != nil { + return nil, err + } p.toolOptionsMu.Lock() defer p.toolOptionsMu.Unlock() p.toolCorrelation = correlation diff --git a/pkg/aichat/approval_execution.go b/pkg/aichat/approval_execution.go index 578b29e2..51a15a1c 100644 --- a/pkg/aichat/approval_execution.go +++ b/pkg/aichat/approval_execution.go @@ -10,6 +10,41 @@ import ( "github.com/flanksource/captain/pkg/api" ) +const ( + suspendedSeedTimeout = 15 * time.Second + suspendedSeedInterval = 25 * time.Millisecond +) + +// awaitSuspendedSeed returns the assistant message the suspended turn ended on. +// The durable suspension (prompt run -> waiting) is committed while the same +// stream's persistence goroutine is still writing that assistant message, so an +// approval resolved the instant the run becomes resumable can observe the run +// before its transcript. Wait for that in-flight write instead of rejecting a +// legitimate approval, and fail loudly when it never lands. +func awaitSuspendedSeed(ctx context.Context, store ThreadStore, threadID, turnID string) (*UIMessage, error) { + deadline := time.Now().Add(suspendedSeedTimeout) + for { + thread, err := store.Get(ctx, threadID) + if err != nil { + return nil, err + } + if len(thread.Messages) > 0 { + seed := thread.Messages[len(thread.Messages)-1] + if strings.EqualFold(seed.Role, string(api.RoleAssistant)) && seed.TurnID == turnID { + return &seed, nil + } + } + if time.Now().After(deadline) { + return nil, fmt.Errorf("captain chat session %s does not end with the suspended turn %s", threadID, turnID) + } + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(suspendedSeedInterval): + } + } +} + func (s *Service) resumeToolApproval(ctx context.Context, threadID string, continuation *ApprovalContinuation) error { if continuation == nil || continuation.Execution == nil || continuation.Spec.ToolApproval == nil { return fmt.Errorf("tool approval continuation is incomplete") diff --git a/pkg/aichat/approval_execution_test.go b/pkg/aichat/approval_execution_test.go new file mode 100644 index 00000000..590c8279 --- /dev/null +++ b/pkg/aichat/approval_execution_test.go @@ -0,0 +1,56 @@ +package aichat + +import ( + "context" + "testing" + "time" +) + +func TestAwaitSuspendedSeedWaitsForTheInFlightAssistantMessage(t *testing.T) { + store := NewMemoryThreadStore() + thread, err := store.Create(context.Background(), "Approve") + if err != nil { + t.Fatalf("create thread: %v", err) + } + turnID := "8f4c9f2b-1f7a-4a04-9d2b-0f2b3c4d5e6f" + if err := store.AppendMessage(context.Background(), thread.ID, UIMessage{ + ID: "user-approve", Role: "user", TurnID: turnID, + Parts: []UIPart{{Type: "text", Text: "Approve the account update"}}, + }); err != nil { + t.Fatalf("append user message: %v", err) + } + // The suspension becomes resumable before the stream finishes persisting the + // assistant message it suspended on. + go func() { + time.Sleep(2 * suspendedSeedInterval) + _ = store.AppendMessage(context.Background(), thread.ID, UIMessage{ + ID: turnID + "-assistant", Role: "assistant", TurnID: turnID, + Parts: []UIPart{{Type: "dynamic-tool", ToolName: "accounts_edit", ToolCallID: "call-1", State: "approval-requested"}}, + }) + }() + + seed, err := awaitSuspendedSeed(context.Background(), store, thread.ID, turnID) + if err != nil { + t.Fatalf("awaitSuspendedSeed: %v", err) + } + if seed.ID != turnID+"-assistant" || seed.TurnID != turnID { + t.Fatalf("awaitSuspendedSeed = %q (turn %q), want the suspended assistant message", seed.ID, seed.TurnID) + } +} + +func TestAwaitSuspendedSeedStopsWithItsContext(t *testing.T) { + store := NewMemoryThreadStore() + thread, err := store.Create(context.Background(), "Approve") + if err != nil { + t.Fatalf("create thread: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(2 * suspendedSeedInterval) + cancel() + }() + + if _, err := awaitSuspendedSeed(ctx, store, thread.ID, "turn-1"); err == nil { + t.Fatal("awaitSuspendedSeed succeeded without a suspended assistant message") + } +} diff --git a/pkg/api/permissions.go b/pkg/api/permissions.go index ede976c7..a94cfccf 100644 --- a/pkg/api/permissions.go +++ b/pkg/api/permissions.go @@ -5,7 +5,9 @@ import ( "fmt" "slices" "sort" + "strings" + "github.com/flanksource/captain/pkg/api/registry" "gopkg.in/yaml.v3" ) @@ -54,6 +56,64 @@ func (p Permissions) HasPreset(x Preset) bool { return slices.Contains(p.Presets, x) } +// AllowList and DenyList project the canonical policy map onto the two lists +// every claude transport speaks (--allowedTools / --disallowedTools). They are +// the only correct source for those flags: Policies() folds an `off` tool mode +// into a deny, so reading Tools.Deny directly lets `tools: {Bash: off}` past the +// filter and the tool runs. +func (t Tools) AllowList() []string { return t.toolsWithPolicy(ToolPolicyAllow) } + +// DenyList is AllowList's counterpart; see its documentation. +func (t Tools) DenyList() []string { return t.toolsWithPolicy(ToolPolicyDeny) } + +func (t Tools) toolsWithPolicy(want ToolPolicy) []string { + var out []string + for tool, policy := range t.Policies() { + if policy == want { + out = append(out, tool) + } + } + sort.Strings(out) + return out +} + +// RequireToolPolicySupport refuses a run whose per-tool policy the backend +// cannot carry. +// +// A deny-list exists solely to forbid a tool, so dropping it silently inverts +// the caller's intent: the agent runs with strictly more authority than the spec +// granted, and nothing in the output says so. Only the claude transports reach a +// --disallowedTools equivalent today, so the rest fail loud here rather than +// proceeding as if the policy had been applied. +// +// Allow-lists are checked too: on a backend with no tool filter, an allowlist is +// equally unenforced. `ask` is refused everywhere — no transport has a per-tool +// prompt, so it would resolve to "allowed" on the backends that advertise tool +// policy support. `auto` constrains nothing, so it needs no backend support. +func RequireToolPolicySupport(backend Backend, permissions Permissions) error { + if asked := permissions.Tools.toolsWithPolicy(ToolPolicyAsk); len(asked) > 0 { + return fmt.Errorf( + "per-tool policy \"ask\" (%s) is not enforceable on any backend: transports carry allow/deny tool lists only, so the tool would run unprompted; use allow or deny", + strings.Join(asked, ", ")) + } + enforced := append(permissions.Tools.AllowList(), permissions.Tools.DenyList()...) + if len(enforced) == 0 || registry.SupportsToolPolicy(backend) { + return nil + } + sort.Strings(enforced) + return fmt.Errorf( + "backend %s cannot enforce a per-tool policy (%s), and running without it would grant more than the spec allows; remove permissions.tools or use one of: %s", + backend, strings.Join(enforced, ", "), backendListOf(registry.ToolPolicyBackends())) +} + +func backendListOf(backends []Backend) string { + out := make([]string, len(backends)) + for i, b := range backends { + out[i] = string(b) + } + return strings.Join(out, ", ") +} + // Validate checks the mode, presets, and tool modes are recognised. func (p Permissions) Validate() error { if !p.Mode.Valid() { diff --git a/pkg/api/registry/backend.go b/pkg/api/registry/backend.go index 675594dd..286e8574 100644 --- a/pkg/api/registry/backend.go +++ b/pkg/api/registry/backend.go @@ -128,6 +128,29 @@ func AuthEnvVars(b Backend) []string { return p.SupportedEnvVars() } +// SupportsToolPolicy reports whether a backend can carry Permissions.Tools to +// the agent it drives. +func SupportsToolPolicy(b Backend) bool { + p, mode, ok := ProviderFor(b) + if !ok { + return false + } + caps, ok := p.Caps(mode) + return ok && caps.ToolPolicy +} + +// ToolPolicyBackends lists the backends that can carry a per-tool policy, in +// canonical order, for help and error text. +func ToolPolicyBackends() []Backend { + var out []Backend + for _, b := range AllBackends() { + if SupportsToolPolicy(b) { + out = append(out, b) + } + } + return out +} + // BackendList renders AllBackends as a comma-separated string for help/error text. func BackendList() string { parts := make([]string, len(AllBackends())) diff --git a/pkg/api/registry/provider.go b/pkg/api/registry/provider.go index 69376eab..fe9e1ab6 100644 --- a/pkg/api/registry/provider.go +++ b/pkg/api/registry/provider.go @@ -29,6 +29,12 @@ type ModeCapabilities struct { // CallerTools reports that the adapter can expose caller-supplied // api.Config.Tools rather than only its built-in tool ecosystem. CallerTools bool + // ToolPolicy reports that the adapter can carry Permissions.Tools — the + // per-tool allow/deny policy — to the agent. It is declared rather than + // inferred because a backend that cannot carry a deny-list must refuse the + // run: silently dropping the one field whose whole purpose is to forbid a + // tool is the failure this flag exists to prevent. + ToolPolicy bool // MediaTypes is the adapter's attachment ceiling. A model's own declared // types are clamped against it — the adapter cannot carry what it cannot send. MediaTypes []string diff --git a/pkg/api/registry/providers.go b/pkg/api/registry/providers.go index 62b8aeba..fdebbe68 100644 --- a/pkg/api/registry/providers.go +++ b/pkg/api/registry/providers.go @@ -20,9 +20,9 @@ var ( EnvVars: []string{"ANTHROPIC_API_KEY"}, modes: map[RuntimeMode]ModeCapabilities{ ModeAPI: {Backend: BackendAnthropic, Streaming: true, CallerTools: true, MediaTypes: []string{"image/*"}}, - ModeCLI: {Backend: BackendClaudeCLI, Streaming: true, Resume: true}, - ModeAgent: {Backend: BackendClaudeAgent, Streaming: true, Resume: true, Interrupt: true, Steer: true, CallerTools: true, MediaTypes: []string{"image/png", "image/jpeg", "image/gif", "image/webp", "application/pdf"}}, - ModeCmux: {Backend: BackendClaudeCmux, Streaming: true, Resume: true, Keyless: true}, + ModeCLI: {Backend: BackendClaudeCLI, Streaming: true, Resume: true, ToolPolicy: true}, + ModeAgent: {Backend: BackendClaudeAgent, Streaming: true, Resume: true, Interrupt: true, Steer: true, CallerTools: true, ToolPolicy: true, MediaTypes: []string{"image/png", "image/jpeg", "image/gif", "image/webp", "application/pdf"}}, + ModeCmux: {Backend: BackendClaudeCmux, Streaming: true, Resume: true, Keyless: true, ToolPolicy: true}, }, modeTokens: sortModeTokens([]modeToken{ {prefix: "claude-agent", mode: ModeAgent}, diff --git a/pkg/api/tool_policy_support_test.go b/pkg/api/tool_policy_support_test.go new file mode 100644 index 00000000..257e4cae --- /dev/null +++ b/pkg/api/tool_policy_support_test.go @@ -0,0 +1,125 @@ +package api + +import ( + "strings" + "testing" +) + +// TestRequireToolPolicySupport enumerates which backends may carry a per-tool +// policy. A deny-list exists only to forbid a tool, so a backend that drops it +// runs with strictly more authority than the spec granted — the run must fail +// instead. The table is the contract: adding a backend forces a decision here. +func TestRequireToolPolicySupport(t *testing.T) { + supported := map[Backend]bool{ + BackendClaudeCLI: true, + BackendClaudeAgent: true, + BackendClaudeCmux: true, + } + + policy := Permissions{Tools: Tools{Deny: []string{"Bash"}}} + for _, backend := range AllBackends() { + t.Run(string(backend), func(t *testing.T) { + err := RequireToolPolicySupport(backend, policy) + if supported[backend] { + if err != nil { + t.Fatalf("%s must carry a tool policy, got %v", backend, err) + } + return + } + if err == nil { + t.Fatalf("%s silently drops the deny-list; want a loud refusal", backend) + } + // The message has to name the offending tools and a way forward, or the + // operator cannot tell which knob to remove. + for _, want := range []string{string(backend), "Bash", string(BackendClaudeCLI)} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err, want) + } + } + }) + } +} + +// TestRequireToolPolicySupport_EmptyPolicyAlwaysPasses keeps the guard scoped to +// runs that actually declared a policy: every backend must stay usable without one. +func TestRequireToolPolicySupport_EmptyPolicyAlwaysPasses(t *testing.T) { + for _, backend := range AllBackends() { + if err := RequireToolPolicySupport(backend, Permissions{}); err != nil { + t.Errorf("%s rejected an empty policy: %v", backend, err) + } + // A mode alone is not a per-tool policy. + if err := RequireToolPolicySupport(backend, Permissions{Mode: PermissionPlan}); err != nil { + t.Errorf("%s rejected a bare permission mode: %v", backend, err) + } + } +} + +// TestRequireToolPolicySupport_NormalizesToolModes pins that the guard reads the +// canonical policy map: `tools: {Bash: off}` is a deny expressed through Modes, +// so a backend with no tool filter must refuse it exactly like Tools.Deny. +func TestRequireToolPolicySupport_NormalizesToolModes(t *testing.T) { + policy := Permissions{Tools: Tools{Modes: map[string]ToolMode{"Bash": ToolModeOff}}} + err := RequireToolPolicySupport(BackendCodexCLI, policy) + if err == nil { + t.Fatal("codex-cli silently drops an off tool mode; want a loud refusal") + } + if !strings.Contains(err.Error(), "Bash") { + t.Errorf("error %q does not name the offending tool", err) + } + if err := RequireToolPolicySupport(BackendClaudeCLI, policy); err != nil { + t.Fatalf("claude-cli must carry an off tool mode, got %v", err) + } + // `on` resolves to auto, which constrains nothing and so needs no backend + // support. + auto := Permissions{Tools: Tools{Modes: map[string]ToolMode{"Read": ToolModeOn}}} + if err := RequireToolPolicySupport(BackendCodexCLI, auto); err != nil { + t.Errorf("an auto policy constrains nothing but was refused: %v", err) + } +} + +// TestRequireToolPolicySupport_AskIsRefusedEverywhere pins the gap the tool +// policy cannot express: no transport has a per-tool prompt, so an `ask` would +// resolve to "allowed" even on the backends that advertise support. +func TestRequireToolPolicySupport_AskIsRefusedEverywhere(t *testing.T) { + policy := Permissions{Tools: Tools{Modes: map[string]ToolMode{"Bash": ToolModeAsk}}} + for _, backend := range AllBackends() { + err := RequireToolPolicySupport(backend, policy) + if err == nil { + t.Errorf("%s accepted an unenforceable ask policy", backend) + continue + } + if !strings.Contains(err.Error(), "Bash") { + t.Errorf("%s: error %q does not name the offending tool", backend, err) + } + } +} + +// TestToolsAllowDenyLists pins the projection every claude transport reads: the +// raw Allow/Deny slices miss the tool modes, which is how an `off` tool escaped +// the filter. +func TestToolsAllowDenyLists(t *testing.T) { + tools := Tools{ + Allow: []string{"Read"}, + Deny: []string{"WebFetch"}, + Modes: map[string]ToolMode{"Bash": ToolModeOff, "Glob": ToolModeOn}, + } + if got := tools.DenyList(); len(got) != 2 || got[0] != "Bash" || got[1] != "WebFetch" { + t.Errorf("DenyList() = %v, want [Bash WebFetch]", got) + } + if got := tools.AllowList(); len(got) != 1 || got[0] != "Read" { + t.Errorf("AllowList() = %v, want [Read]", got) + } +} + +// TestRequireToolPolicySupport_AllowListToo pins that an allow-list is refused on +// the same backends: where there is no tool filter, an allowlist is equally +// unenforced, and silently ignoring it grants more than the spec allowed. +func TestRequireToolPolicySupport_AllowListToo(t *testing.T) { + policy := Permissions{Tools: Tools{Allow: []string{"Read"}}} + if err := RequireToolPolicySupport(BackendCodexCLI, policy); err == nil { + t.Fatal("codex-cli silently drops an allow-list; want a loud refusal") + } + if err := RequireToolPolicySupport(BackendClaudeCLI, policy); err != nil { + t.Fatalf("claude-cli must carry an allow-list, got %v", err) + } +} diff --git a/pkg/cli/ai_prompt_file.go b/pkg/cli/ai_prompt_file.go index 71d3787e..f405ea96 100644 --- a/pkg/cli/ai_prompt_file.go +++ b/pkg/cli/ai_prompt_file.go @@ -103,8 +103,10 @@ func overlayCLI(base ai.Request, baseCfg ai.Config, o AIPromptOptions) (ai.Reque return base, baseCfg, fmt.Errorf("invalid --mode %q (valid: %s)", o.Mode, registry.RuntimeModeList()) } } - // Sandbox precedence: --sandbox > frontmatter (base.Sandbox) > global default. - sandbox, err := resolveSandboxSelection(o.SandboxSelector(), base.Sandbox, loadSavedConfig().Sandbox) + // Sandbox precedence: --sandbox > frontmatter (req.Sandbox) > global default. + // Resolved here rather than at the end because the winning kind can force the + // runtime mode below; the selection is recorded onto req/cfg once cfg exists. + sandbox, err := resolveRunSandbox(&req, o.SandboxSelector()) if err != nil { return base, baseCfg, err } diff --git a/pkg/cli/ai_sandbox.go b/pkg/cli/ai_sandbox.go index f76d3492..89029dc9 100644 --- a/pkg/cli/ai_sandbox.go +++ b/pkg/cli/ai_sandbox.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/flanksource/captain/pkg/ai" "github.com/flanksource/captain/pkg/api" "github.com/flanksource/captain/pkg/api/registry" "github.com/flanksource/captain/pkg/captainconfig" @@ -35,6 +36,44 @@ func resolveSandboxSelection(flagSelector string, ref *api.SandboxRef, defaults } } +// resolveRunSandbox resolves the sandbox for one run from the request's own ref +// — the prompt's `sandbox:` frontmatter, or a spec override already layered onto +// it — plus an optional operator-supplied selector. +func resolveRunSandbox(req *ai.Request, flagSelector string) (captainconfig.SandboxSelection, error) { + return resolveSandboxSelection(flagSelector, req.Sandbox, loadSavedConfig().Sandbox) +} + +// recordSandboxSelection writes the winning selection onto the run: the config +// the exec seam reads, and — when an operator named one explicitly — the request +// ref, so the serialized spec carries the choice the run was actually made with. +func recordSandboxSelection(req *ai.Request, cfg *ai.Config, selection captainconfig.SandboxSelection, flagSelector string) { + if flagSelector != "" { + ref := api.SandboxRef{Backend: flagSelector} + if req.Sandbox != nil { + ref.Agent = req.Sandbox.Agent + ref.Policy = req.Sandbox.Policy + } + req.Sandbox = &ref + } + cfg.Sandbox = selection.Kind == registry.SandboxSRT + cfg.SandboxSelection = sandboxSelectionConfig(selection, req.Sandbox) +} + +// applyRunSandbox is the transport-neutral seam: resolve, then record. Every +// entrypoint that builds a run must reach the sandbox through it or through its +// two halves. Resolution used to live only inside overlayCLI, so a run submitted +// over HTTP carried no selection at all — and the exec seam reads a nil +// selection as "unsandboxed", silently, because both fail-loud guards only fire +// once a selection exists. +func applyRunSandbox(req *ai.Request, cfg *ai.Config, flagSelector string) error { + selection, err := resolveRunSandbox(req, flagSelector) + if err != nil { + return err + } + recordSandboxSelection(req, cfg, selection, flagSelector) + return nil +} + // sandboxForcedMode returns the single runtime mode a sandbox kind can serve, // or "" when the kind serves several (or is none). Argv-wrapping adapters are // CLI-only, so selecting one forces CLI mode the way --sandbox always has. diff --git a/pkg/cli/prompt_render.go b/pkg/cli/prompt_render.go index 0e81ba70..9e6e621f 100644 --- a/pkg/cli/prompt_render.go +++ b/pkg/cli/prompt_render.go @@ -37,6 +37,11 @@ func renderPrompt(ctx context.Context, id string, renderReq PromptRenderRequest) if renderReq.Spec != nil { overlayRuntimeSpec(&req, &cfg, *renderReq.Spec) } + // There is no --sandbox over HTTP, so the ref carries the whole selection: + // the spec override the caller sent, else the prompt's own frontmatter. + if err := applyRunSandbox(&req, &cfg, ""); err != nil { + return PromptRenderResult{}, err + } if err := applyPromptDefaults(&req, &cfg); err != nil { return PromptRenderResult{}, err } @@ -66,6 +71,9 @@ func renderEphemeralPrompt(renderReq PromptRenderRequest) (PromptRenderResult, e if req.Prompt.Source == "" { req.Prompt.Source = "" } + if err := applyRunSandbox(&req, &cfg, ""); err != nil { + return PromptRenderResult{}, err + } if err := applyPromptDefaults(&req, &cfg); err != nil { return PromptRenderResult{}, err } @@ -325,6 +333,9 @@ func overlayRuntimeSpec(req *ai.Request, cfg *ai.Config, spec api.Spec) { if spec.Setup != nil { req.Setup = spec.Setup } + if spec.Sandbox != nil { + req.Sandbox = spec.Sandbox + } if spec.SessionID != "" { req.SessionID = spec.SessionID diff --git a/pkg/cli/prompt_render_test.go b/pkg/cli/prompt_render_test.go index 7599c215..38278d2d 100644 --- a/pkg/cli/prompt_render_test.go +++ b/pkg/cli/prompt_render_test.go @@ -3,11 +3,13 @@ package cli import ( "context" "path/filepath" + "reflect" "strings" "testing" "github.com/flanksource/captain/pkg/ai" "github.com/flanksource/captain/pkg/api" + "github.com/flanksource/captain/pkg/api/registry" "github.com/flanksource/captain/pkg/captainconfig" "github.com/flanksource/commons-db/shell" ) @@ -192,6 +194,145 @@ func TestRenderPromptEphemeralSpec(t *testing.T) { } } +// TestRenderPromptResolvesSandbox pins the sandbox contract of the HTTP/Spec +// render path. Sandbox resolution used to be reachable only from overlayCLI, so +// a run submitted over HTTP dropped both the prompt's own `sandbox:` frontmatter +// and the configured sandbox.default and executed unconfined — silently, because +// the fail-loud guards only fire once a selection exists. +func TestRenderPromptResolvesSandbox(t *testing.T) { + tests := []struct { + name string + frontmatter string + globalDefault string + override *api.SandboxRef + want registry.SandboxKind + }{ + { + name: "frontmatter selects the sandbox", + frontmatter: "sandbox: srt\n", + want: registry.SandboxSRT, + }, + { + name: "global default applies when the prompt selects none", + globalDefault: "srt", + want: registry.SandboxSRT, + }, + { + name: "frontmatter beats the global default", + frontmatter: "sandbox: none\n", + globalDefault: "srt", + want: registry.SandboxNone, + }, + { + name: "request spec beats both", + frontmatter: "sandbox: none\n", + globalDefault: "none", + override: &api.SandboxRef{Backend: "srt"}, + want: registry.SandboxSRT, + }, + { + name: "nothing selects anything", + want: registry.SandboxNone, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + isolateCaptainConfig(t) + if tt.globalDefault != "" { + if err := captainconfig.Save(captainconfig.Config{ + Sandbox: captainconfig.SandboxDefaults{Default: tt.globalDefault}, + }); err != nil { + t.Fatalf("save captain config: %v", err) + } + } + + dir := t.TempDir() + t.Chdir(t.TempDir()) + ctx := ContextWithPromptDirs(context.Background(), []string{dir}) + created, err := createPrompt(ctx, map[string]any{ + "name": "Sandboxed", + "content": "---\nname: Sandboxed\nmodel: claude-code-opus\n" + tt.frontmatter + + "---\n{{role \"user\"}}\nHello\n", + }) + if err != nil { + t.Fatalf("createPrompt() err = %v", err) + } + + rendered, err := renderPrompt(ctx, created.ID, PromptRenderRequest{ + Spec: &api.Spec{Sandbox: tt.override}, + }) + if err != nil { + t.Fatalf("renderPrompt() err = %v", err) + } + if rendered.ValidationError != "" { + t.Fatalf("render validation error = %q", rendered.ValidationError) + } + + got := registry.SandboxNone + if selection := rendered.Config.ResolvedSandbox(); selection != nil { + got = selection.Kind + } + if got != tt.want { + t.Fatalf("resolved sandbox = %q, want %q", got, tt.want) + } + }) + } +} + +// TestRenderPromptPreservesSandboxMetadata pins the rest of the ref. Resolution +// only ever consumed SandboxRef.Backend, so a spec that also pinned an agent or +// a per-run policy could resolve to the right kind while silently dropping the +// two fields that bound what the dispatched run may touch. +func TestRenderPromptPreservesSandboxMetadata(t *testing.T) { + isolateCaptainConfig(t) + dir := t.TempDir() + t.Chdir(t.TempDir()) + ctx := ContextWithPromptDirs(context.Background(), []string{dir}) + created, err := createPrompt(ctx, map[string]any{ + "name": "Sandboxed", + "content": "---\nname: Sandboxed\nmodel: claude-code-opus\nsandbox: none\n---\n{{role \"user\"}}\nHello\n", + }) + if err != nil { + t.Fatalf("createPrompt() err = %v", err) + } + + policy := &api.SandboxPolicy{Paths: []string{"pkg/**"}, MaxAttempts: 3} + rendered, err := renderPrompt(ctx, created.ID, PromptRenderRequest{ + Spec: &api.Spec{Sandbox: &api.SandboxRef{Backend: "srt", Agent: "builder-1", Policy: policy}}, + }) + if err != nil { + t.Fatalf("renderPrompt() err = %v", err) + } + if rendered.ValidationError != "" { + t.Fatalf("render validation error = %q", rendered.ValidationError) + } + + if rendered.Input.Sandbox == nil { + t.Fatal("request sandbox ref = nil, want the spec's ref") + } + if rendered.Input.Sandbox.Agent != "builder-1" { + t.Errorf("request sandbox agent = %q, want builder-1", rendered.Input.Sandbox.Agent) + } + if !reflect.DeepEqual(rendered.Input.Sandbox.Policy, policy) { + t.Errorf("request sandbox policy = %+v, want %+v", rendered.Input.Sandbox.Policy, policy) + } + + selection := rendered.Config.ResolvedSandbox() + if selection == nil { + t.Fatal("resolved sandbox = nil, want the srt selection") + } + if selection.Kind != registry.SandboxSRT { + t.Errorf("resolved sandbox kind = %q, want %q", selection.Kind, registry.SandboxSRT) + } + if selection.Agent != "builder-1" { + t.Errorf("resolved sandbox agent = %q, want builder-1", selection.Agent) + } + if !reflect.DeepEqual(selection.Policy, policy) { + t.Errorf("resolved sandbox policy = %+v, want %+v", selection.Policy, policy) + } +} + func TestApplyPromptDefaultsSelectorEffortWins(t *testing.T) { isolateCaptainConfig(t) req := ai.Request{Model: api.Model{Effort: api.EffortLow}} diff --git a/pkg/container/base/Dockerfile b/pkg/container/base/Dockerfile index dc717cba..bd072d68 100644 --- a/pkg/container/base/Dockerfile +++ b/pkg/container/base/Dockerfile @@ -1,13 +1,41 @@ -FROM flanksource/base-image:latest +# syntax=docker/dockerfile:1 +# Pinned by digest, not by tag: republishing the same image tag has to produce +# the same contents or a rollback lands on something other than what it rolled +# back to. Bump the digest deliberately (docker buildx imagetools inspect +# flanksource/base-image:latest) rather than letting the tag drift underneath. +FROM flanksource/base-image:latest@sha256:f5f0945741678d7702eb13da393102dbdd10a19fb20b6e06a7efe241d5395173 ARG TZ ENV TZ="$TZ" -ARG CLAUDE_CODE_VERSION=latest ARG USERNAME=claude ARG USER_UID=501 ARG USER_GID=20 +# Every mutable input is an explicit default here and an override in +# .github/workflows/publish-image.yml, so a published tag records exactly what +# went into it. `latest` is deliberately absent: it is what made two builds of +# one tag differ. +ARG NODE_MAJOR=22 +ARG GO_VERSION=1.26.1 +# Official checksums from https://go.dev/dl/?mode=json — the archive is verified +# before extraction, so a compromised or truncated download fails the build +# instead of being unpacked into /usr/local. +ARG GO_SHA256_AMD64=031f088e5d955bab8657ede27ad4e3bc5b7c1ba281f05f245bcc304f327c987a +ARG GO_SHA256_ARM64=a290581cfe4fe28ddd737dde3095f3dbeb7f2e4065cab4eae44dfc53b760c2f7 +ARG GIT_DELTA_VERSION=0.18.2 +# Kept in step with go.mod's github.com/onsi/ginkgo/v2 so the container's runner +# matches the one the suites are compiled against. +ARG GINKGO_VERSION=v2.28.1 +ARG PLAYWRIGHT_VERSION=1.62.1 +ARG CLAUDE_CODE_VERSION=2.1.227 +ARG CODEX_VERSION=0.147.0 +ARG GEMINI_CLI_VERSION=0.54.4 +ARG TSX_VERSION=4.23.12 +ARG TYPESCRIPT_VERSION=7.0.2 +ARG PNPM_VERSION=11.21.0 +ARG AGENT_BROWSER_VERSION=0.34.0 + # Create user/group matching host (default: moshe:501:20) RUN if ! getent group ${USER_GID} > /dev/null 2>&1; then groupadd -g ${USER_GID} ${USERNAME}; fi && \ useradd -u ${USER_UID} -g ${USER_GID} -m -s /bin/zsh ${USERNAME} && \ @@ -15,7 +43,7 @@ RUN if ! getent group ${USER_GID} > /dev/null 2>&1; then groupadd -g ${USER_GID} chown -R ${USERNAME}:${USER_GID} /home/${USERNAME} # Install Node.js and basic development tools -RUN curl -fsSL https://deb.nodesource.com/setup_22.x | bash - && \ +RUN curl -fsSL https://deb.nodesource.com/setup_${NODE_MAJOR}.x | bash - && \ apt-get update && apt-get install -y --no-install-recommends \ nodejs \ less \ @@ -38,8 +66,24 @@ RUN curl -fsSL https://deb.nodesource.com/setup_22.x | bash - && \ vim \ build-essential \ gosu \ + ripgrep \ + fd-find \ + bat \ + tree \ + htop \ + lsof \ + sqlite3 \ + postgresql-client \ + openssh-client \ + rsync \ + tmux \ + shellcheck \ && apt-get clean && rm -rf /var/lib/apt/lists/* +# Debian renames these binaries to avoid clashes; agents expect the upstream names +RUN ln -sf "$(command -v fdfind)" /usr/local/bin/fd && \ + ln -sf "$(command -v batcat)" /usr/local/bin/bat + # Ensure user has access to /usr/local/share RUN mkdir -p /usr/local/share/npm-global && \ chown -R ${USERNAME} /usr/local/share @@ -53,15 +97,53 @@ RUN mkdir -p /workspace && chown ${USERNAME}:${USER_GID} /workspace WORKDIR /workspace -ARG GIT_DELTA_VERSION=0.18.2 RUN ARCH=$(dpkg --print-architecture) && \ wget "https://github.com/dandavison/delta/releases/download/${GIT_DELTA_VERSION}/git-delta_${GIT_DELTA_VERSION}_${ARCH}.deb" && \ sudo dpkg -i "git-delta_${GIT_DELTA_VERSION}_${ARCH}.deb" && \ rm "git-delta_${GIT_DELTA_VERSION}_${ARCH}.deb" +# Go toolchain — needed to build/test the Go repos agents work in. Downloaded to +# a file and checksummed before extraction: piping curl straight into tar unpacks +# whatever arrives, so a tampered or truncated archive would land in /usr/local +# with the build still reporting success. +RUN ARCH=$(dpkg --print-architecture) && \ + case "$ARCH" in \ + amd64) GO_SHA256="${GO_SHA256_AMD64}" ;; \ + arm64) GO_SHA256="${GO_SHA256_ARM64}" ;; \ + *) echo "no pinned go${GO_VERSION} checksum for architecture ${ARCH}" >&2; exit 1 ;; \ + esac && \ + curl -fsSL -o /tmp/go.tar.gz "https://go.dev/dl/go${GO_VERSION}.linux-${ARCH}.tar.gz" && \ + echo "${GO_SHA256} /tmp/go.tar.gz" | sha256sum -c - && \ + tar -C /usr/local -xzf /tmp/go.tar.gz && \ + rm /tmp/go.tar.gz +ENV GOROOT=/usr/local/go +ENV GOPATH=/home/${USERNAME}/go +ENV PATH=$PATH:/usr/local/go/bin:/home/${USERNAME}/go/bin +RUN GOBIN=/usr/local/bin go install github.com/onsi/ginkgo/v2/ginkgo@${GINKGO_VERSION} && \ + rm -rf /root/.cache/go-build /root/go && \ + mkdir -p ${GOPATH}/bin && chown -R ${USER_UID}:${USER_GID} ${GOPATH} + +# Flanksource + Go tooling via deps (already present in the base image). +# GITHUB_TOKEN arrives as a BuildKit secret so it never lands in image history. +COPY deps.yaml /tmp/deps/deps.yaml +RUN --mount=type=secret,id=GITHUB_TOKEN,env=GITHUB_TOKEN,required=false \ + cd /tmp/deps && \ + deps --no-progress install -c deps.yaml --bin-dir /usr/bin --app-dir /opt && \ + rm -rf /tmp/deps /root/.deps + COPY entrypoint.sh /usr/local/bin/entrypoint.sh RUN chmod +x /usr/local/bin/entrypoint.sh +# Browser automation. agent-browser probes the Playwright browser cache, so one +# world-readable Chromium under PLAYWRIGHT_BROWSERS_PATH serves every user in the +# container — no per-home copy. Playwright is also what installs the shared +# libraries Chromium links against; `agent-browser install --with-deps` cannot, +# because it still asks for noble's pre-t64 package names. +ENV PLAYWRIGHT_BROWSERS_PATH=/ms-playwright +RUN npx -y playwright@${PLAYWRIGHT_VERSION} install --with-deps chromium && \ + chmod -R a+rX /ms-playwright && \ + apt-get clean && rm -rf /var/lib/apt/lists/* /root/.npm + USER ${USERNAME} # Install global packages @@ -75,8 +157,17 @@ ENV SHELL=/bin/zsh ENV EDITOR=nano ENV VISUAL=nano -# Install Claude -RUN npm install -g @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION} +# Agent CLIs: one per captain backend (claude-cli, codex-cli/codex-agent, gemini-cli), +# plus tsx for the claude-agent SDK bridge and pnpm for `captain serve --dev`. +RUN npm install -g \ + @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION} \ + @openai/codex@${CODEX_VERSION} \ + @google/gemini-cli@${GEMINI_CLI_VERSION} \ + tsx@${TSX_VERSION} \ + typescript@${TYPESCRIPT_VERSION} \ + pnpm@${PNPM_VERSION} \ + agent-browser@${AGENT_BROWSER_VERSION} \ + && npm cache clean --force USER root ENTRYPOINT ["/usr/local/bin/entrypoint.sh"] diff --git a/pkg/container/base/deps.yaml b/pkg/container/base/deps.yaml new file mode 100644 index 00000000..d9bd5cad --- /dev/null +++ b/pkg/container/base/deps.yaml @@ -0,0 +1,68 @@ +# Binaries installed into the agent sandbox image via `deps` (shipped by +# flanksource/base-image). +# +# Every entry is declared explicitly rather than relying on deps' owner/repo +# heuristic: `deps install` with no arguments resolves only through the registry +# (see InstallFromConfig in flanksource/deps), and the golangci-lint release +# publishes .deb/.rpm/.tar.gz for the same platform, so a glob would be ambiguous. +registry: + gavel: + name: gavel + repo: flanksource/gavel + asset_patterns: + linux-amd64: gavel_linux_amd64.tar.gz + linux-arm64: gavel_linux_arm64.tar.gz + darwin-amd64: gavel_darwin_amd64.tar.gz + darwin-arm64: gavel_darwin_arm64.tar.gz + checksum_file: checksums.txt + version_command: version + version_regex: 'gavel v?(\d+\.\d+\.\d+)' + + repomap: + name: repomap + repo: flanksource/repomap + asset_patterns: + linux-amd64: repomap-linux-amd64 + linux-arm64: repomap-linux-arm64 + darwin-amd64: repomap-darwin-amd64 + darwin-arm64: repomap-darwin-arm64 + checksum_file: checksums.txt + version_command: version + version_regex: 'repomap v?(\d+\.\d+\.\d+)' + + captain: + name: captain + repo: flanksource/captain + asset_patterns: + linux-amd64: captain_linux_amd64.tar.gz + linux-arm64: captain_linux_arm64.tar.gz + darwin-amd64: captain_darwin_amd64.tar.gz + darwin-arm64: captain_darwin_arm64.tar.gz + checksum_file: captain_{{.version}}_checksums.txt + version_command: --version + version_regex: 'captain version v?(\d+\.\d+\.\d+)' + + golangci-lint: + name: golangci-lint + repo: golangci/golangci-lint + asset_patterns: + linux-amd64: golangci-lint-{{.version}}-linux-amd64.tar.gz + linux-arm64: golangci-lint-{{.version}}-linux-arm64.tar.gz + darwin-amd64: golangci-lint-{{.version}}-darwin-amd64.tar.gz + darwin-arm64: golangci-lint-{{.version}}-darwin-arm64.tar.gz + checksum_file: golangci-lint-{{.version}}-checksums.txt + version_command: --version + version_regex: 'golangci-lint has version v?(\d+\.\d+\.\d+)' + +# Pinned so republishing an image tag reinstalls the same binaries; bump these +# deliberately when the image is refreshed. +dependencies: + task: v3.52.0 + golangci-lint: v2.12.2 + gavel: v0.0.54 + repomap: v0.4.0 + # captain is the one deliberate exception: this image ships the release that + # triggered the publish, and the image tag records which one. Pinning it here + # would mean editing this file on every release to say what the tag already + # says. publish-image.yml refuses to run before that release exists. + captain: latest diff --git a/pkg/container/base_image.go b/pkg/container/base_image.go index 6b1bde30..188710b1 100644 --- a/pkg/container/base_image.go +++ b/pkg/container/base_image.go @@ -16,6 +16,9 @@ var baseDockerfileContent []byte //go:embed base/entrypoint.sh var baseEntrypointContent []byte +//go:embed base/deps.yaml +var baseDepsContent []byte + const baseImageTag = "claude-env:base" func EnsureBaseImage(baseImage string) error { @@ -29,9 +32,36 @@ func writeBaseContext(dir string) error { if err := os.WriteFile(filepath.Join(dir, "Dockerfile"), baseDockerfileContent, 0o644); err != nil { return err } + if err := os.WriteFile(filepath.Join(dir, "deps.yaml"), baseDepsContent, 0o644); err != nil { + return err + } return os.WriteFile(filepath.Join(dir, "entrypoint.sh"), baseEntrypointContent, 0o755) } +// githubTokenSecretArgs passes the host's GitHub token to `deps` as a BuildKit +// secret so the image can resolve release assets without hitting the 60 req/hr +// unauthenticated rate limit. The token is mounted, never baked into a layer or +// image history. The Dockerfile declares the mount as required=false, so an +// unauthenticated build still works. +func githubTokenSecretArgs() ([]string, []string) { + for _, name := range []string{"GITHUB_TOKEN", "GH_TOKEN"} { + token := os.Getenv(name) + if token == "" { + continue + } + return []string{"--secret", "id=GITHUB_TOKEN,env=GITHUB_TOKEN"}, + []string{"GITHUB_TOKEN=" + token} + } + return nil, nil +} + +// buildKitEnv returns the environment for a `docker build`, forcing BuildKit so +// secret mounts are honoured on daemons that still default to the legacy builder. +func buildKitEnv(extra []string) []string { + env := append(os.Environ(), "DOCKER_BUILDKIT=1") + return append(env, extra...) +} + func buildBaseImage() error { dir, err := os.MkdirTemp("", "captain-base-*") if err != nil { @@ -43,8 +73,12 @@ func buildBaseImage() error { return fmt.Errorf("writing base context: %w", err) } + secretArgs, secretEnv := githubTokenSecretArgs() + clicky.Printf("Building base image %s...\n", baseImageTag) - cmd := exec.Command("docker", "build", "-t", baseImageTag, dir) + args := append([]string{"build", "-t", baseImageTag}, secretArgs...) + cmd := exec.Command("docker", append(args, dir)...) + cmd.Env = buildKitEnv(secretEnv) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr return cmd.Run() diff --git a/pkg/container/base_image_test.go b/pkg/container/base_image_test.go index 84c74ac7..41a2131d 100644 --- a/pkg/container/base_image_test.go +++ b/pkg/container/base_image_test.go @@ -12,7 +12,7 @@ func TestWriteBaseContext(t *testing.T) { t.Fatalf("writeBaseContext: %v", err) } - for _, name := range []string{"Dockerfile", "entrypoint.sh"} { + for _, name := range []string{"Dockerfile", "entrypoint.sh", "deps.yaml"} { info, err := os.Stat(filepath.Join(dir, name)) if err != nil { t.Errorf("%s not written: %v", name, err) diff --git a/pkg/container/build.go b/pkg/container/build.go index 1cd3b227..18d2f7ad 100644 --- a/pkg/container/build.go +++ b/pkg/container/build.go @@ -35,6 +35,7 @@ func Build(input BuildInput) error { "-f", filepath.Join(input.ContextDir, "Dockerfile"), input.ContextDir, ) + cmd.Env = buildKitEnv(nil) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr return cmd.Run() diff --git a/pkg/gitagent/snapshot_audit_ginkgo_test.go b/pkg/gitagent/snapshot_audit_ginkgo_test.go index 237eedc7..c30d0964 100644 --- a/pkg/gitagent/snapshot_audit_ginkgo_test.go +++ b/pkg/gitagent/snapshot_audit_ginkgo_test.go @@ -11,11 +11,11 @@ import ( . "github.com/onsi/gomega" ) -// A4.3 audit of commons-db's dirty-state mechanism (shell.Checkout.Dirty → -// applyDirtyState), which issue #39 §4 names as the dispatch-snapshot +// A4.3 audit of commons-db's dirty-state mechanism (shell.Worktree.Uncommitted +// → populateWorktree), which issue #39 §4 names as the dispatch-snapshot // substrate. The dispatch snapshot does NOT use it — TakeSnapshot builds the -// commit from git plumbing directly — but Spec.Setup.Checkout.Dirty remains a -// user-facing surface, so this suite pins what round-trips and what does not. +// commit from git plumbing directly — but Spec.Setup.Checkout.Worktree remains +// a user-facing surface, so this suite pins what round-trips and what does not. // Failures here after a commons-db upgrade mean upstream behaviour changed: // re-audit before trusting it. Known gaps (filed upstream rather than forked, // A4.2): skip-worktree edits are silently dropped, CRLF normalization is not @@ -26,9 +26,12 @@ var _ = Describe("commons-db dirty-state audit (A4.3)", func() { return shell.Prepare(dbcontext.NewContext(context.Background()), &shell.Setup{ BaseDir: GinkgoT().TempDir(), Checkout: &shell.Checkout{ - Path: src, - Dirty: &shell.Dirty{Stash: shell.StashAll}, - Worktree: &shell.Worktree{Mode: shell.WorktreeNew, Prefix: "captain-audit"}, + Path: src, + Worktree: &shell.Worktree{ + Mode: shell.WorktreeNew, + Prefix: "captain-audit", + Uncommitted: shell.CloneClone, + }, }, }) } diff --git a/pkg/gitagent/workspace.go b/pkg/gitagent/workspace.go index a4b6197d..173c09b3 100644 --- a/pkg/gitagent/workspace.go +++ b/pkg/gitagent/workspace.go @@ -12,11 +12,8 @@ import ( "os" "os/exec" "path/filepath" - "strconv" "strings" "syscall" - - "golang.org/x/sys/unix" ) // NoAgentCommand opts a sidecar out of launching anything, leaving the @@ -118,23 +115,3 @@ func LaunchAgent(sidecarRepo, task, workdir, taskFile, command string) error { // reparents to init when the hook exits. return cmd.Process.Release() } - -// markInheritedDescriptorsCloseOnExec prevents receive-pack's sideband pipes -// from surviving in the detached agent and keeping the dispatch push open. -func markInheritedDescriptorsCloseOnExec() error { - closeRangeErr := unix.CloseRange(3, ^uint(0), unix.CLOSE_RANGE_CLOEXEC) - if closeRangeErr == nil { - return nil - } - entries, readErr := os.ReadDir("/proc/self/fd") - if readErr != nil { - return fmt.Errorf("marking inherited descriptors close-on-exec: close_range: %v; /proc/self/fd: %w", closeRangeErr, readErr) - } - for _, entry := range entries { - fd, parseErr := strconv.Atoi(entry.Name()) - if parseErr == nil && fd >= 3 { - syscall.CloseOnExec(fd) - } - } - return nil -} diff --git a/pkg/gitagent/workspace_descriptors.go b/pkg/gitagent/workspace_descriptors.go new file mode 100644 index 00000000..99aca495 --- /dev/null +++ b/pkg/gitagent/workspace_descriptors.go @@ -0,0 +1,27 @@ +package gitagent + +import ( + "fmt" + "os" + "strconv" + "syscall" +) + +func markOpenDescriptorsCloseOnExec(path string) error { + dir, err := os.Open(path) + if err != nil { + return fmt.Errorf("marking inherited descriptors close-on-exec via %s: %w", path, err) + } + defer dir.Close() + entries, err := dir.Readdirnames(-1) + if err != nil { + return fmt.Errorf("listing inherited descriptors via %s: %w", path, err) + } + for _, entry := range entries { + fd, parseErr := strconv.Atoi(entry) + if parseErr == nil && fd >= 3 { + syscall.CloseOnExec(fd) + } + } + return nil +} diff --git a/pkg/gitagent/workspace_descriptors_darwin.go b/pkg/gitagent/workspace_descriptors_darwin.go new file mode 100644 index 00000000..7ecda26f --- /dev/null +++ b/pkg/gitagent/workspace_descriptors_darwin.go @@ -0,0 +1,7 @@ +package gitagent + +// markInheritedDescriptorsCloseOnExec prevents receive-pack's sideband pipes +// from surviving in the detached agent and keeping the dispatch push open. +func markInheritedDescriptorsCloseOnExec() error { + return markOpenDescriptorsCloseOnExec("/dev/fd") +} diff --git a/pkg/gitagent/workspace_descriptors_linux.go b/pkg/gitagent/workspace_descriptors_linux.go new file mode 100644 index 00000000..6349a443 --- /dev/null +++ b/pkg/gitagent/workspace_descriptors_linux.go @@ -0,0 +1,20 @@ +package gitagent + +import ( + "fmt" + + "golang.org/x/sys/unix" +) + +// markInheritedDescriptorsCloseOnExec prevents receive-pack's sideband pipes +// from surviving in the detached agent and keeping the dispatch push open. +func markInheritedDescriptorsCloseOnExec() error { + closeRangeErr := unix.CloseRange(3, ^uint(0), unix.CLOSE_RANGE_CLOEXEC) + if closeRangeErr == nil { + return nil + } + if err := markOpenDescriptorsCloseOnExec("/proc/self/fd"); err != nil { + return fmt.Errorf("close_range failed: %v; %w", closeRangeErr, err) + } + return nil +} diff --git a/pkg/gitagent/workspace_ginkgo_test.go b/pkg/gitagent/workspace_ginkgo_test.go new file mode 100644 index 00000000..070b662f --- /dev/null +++ b/pkg/gitagent/workspace_ginkgo_test.go @@ -0,0 +1,35 @@ +package gitagent_test + +import ( + "fmt" + "os" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "golang.org/x/sys/unix" + + "github.com/flanksource/captain/pkg/gitagent" +) + +var _ = Describe("agent workspace", func() { + It("does not inherit unrelated file descriptors", func() { + repo := GinkgoT().TempDir() + taskDir := filepath.Join(repo, "captain", "tasks", "t-descriptors") + Expect(os.MkdirAll(taskDir, 0o755)).To(Succeed()) + + unrelated, err := os.Create(filepath.Join(repo, "unrelated")) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(unrelated.Close) + _, err = unix.FcntlInt(unrelated.Fd(), unix.F_SETFD, 0) + Expect(err).NotTo(HaveOccurred()) + + command := fmt.Sprintf("if [ -e /dev/fd/%d ]; then printf inherited; else printf closed; fi", unrelated.Fd()) + Expect(gitagent.LaunchAgent(repo, "t-descriptors", repo, "task.json", command)).To(Succeed()) + + Eventually(func() string { + output, _ := os.ReadFile(filepath.Join(taskDir, "agent.stdout.log")) + return string(output) + }).Should(Equal("closed")) + }) +})