Merge remote-tracking branch 'upstream/dev' into codex/merge-nextlevelbuilder-goclaw

# Conflicts:
#	internal/pipeline/think_stage.go
#	internal/pipeline/tool_stage.go
#	tests/integration/git_adapter_ssh_test.go
This commit is contained in:
Goon committed 2026-06-09 22:38:32 +07:00
commit a63080714a
40 files changed
+2206 -263

No files matched your search

+37 -169
View File
@@ -161,94 +161,27 @@ jobs:
strategy:
fail-fast: false
matrix:
variant: [base, latest, full]
platform: [linux/amd64, linux/arm64]
include:
- variant: base
suffix: "-base"
enable_embedui: "false"
enable_python: "false"
enable_full_skills: "false"
- variant: latest
suffix: ""
enable_embedui: "true"
enable_python: "true"
enable_full_skills: "false"
- variant: full
suffix: "-full"
enable_embedui: "true"
enable_python: "true"
enable_full_skills: "true"
- platform: linux/amd64
runner: ubuntu-latest
arch: amd64
- platform: linux/arm64
runner: ubuntu-24.04-arm
arch: arm64
runs-on: ${{ matrix.runner }}
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: docker/setup-buildx-action@v3
- name: Log in to GHCR
uses: docker/login-action@v3
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}
- name: Build and push by digest
id: build
uses: docker/build-push-action@v6
with:
context: .
platforms: ${{ matrix.platform }}
outputs: type=image,name=${{ env.GHCR_IMAGE }},push-by-digest=true,name-canonical=true,push=true
build-args: |
ENABLE_OTEL=false
ENABLE_EMBEDUI=${{ matrix.enable_embedui }}
ENABLE_PYTHON=${{ matrix.enable_python }}
ENABLE_FULL_SKILLS=${{ matrix.enable_full_skills }}
VERSION=v${{ needs.release.outputs.version }}
cache-from: type=gha,scope=${{ matrix.variant }}-${{ matrix.arch }}
cache-to: type=gha,mode=max,scope=${{ matrix.variant }}-${{ matrix.arch }}
provenance: false
- name: Export digest
run: |
mkdir -p /tmp/digests
digest="${{ steps.build.outputs.digest }}"
touch "/tmp/digests/${digest#sha256:}"
- uses: actions/upload-artifact@v4
with:
name: digests-${{ matrix.variant }}-${{ matrix.arch }}
path: /tmp/digests/*
if-no-files-found: error
retention-days: 1
# Combine per-platform digests into multi-arch manifests and push final tags
# to both GHCR and Docker Hub. One job per variant runs in parallel.
docker-merge:
needs: [release, docker-build]
if: needs.release.outputs.released == 'true'
runs-on: ubuntu-latest
timeout-minutes: 15
strategy:
fail-fast: false
matrix:
include:
- variant: base
suffix: "-base"
- variant: latest
suffix: ""
- variant: full
suffix: "-full"
steps:
- name: Download digests
uses: actions/download-artifact@v4
with:
path: /tmp/digests
pattern: digests-${{ matrix.variant }}-*
merge-multiple: true
- uses: docker/setup-qemu-action@v3
- uses: docker/setup-buildx-action@v3
@@ -277,92 +210,34 @@ jobs:
type=raw,value=latest,enable=${{ matrix.suffix == '' }},suffix=
type=raw,value=${{ matrix.variant }},enable=${{ matrix.suffix != '' }}
- name: Create manifest list and push
working-directory: /tmp/digests
run: |
# Compose -t flags from metadata-action tags (newline-separated)
TAG_ARGS=()
while IFS= read -r tag; do
[ -n "$tag" ] && TAG_ARGS+=("-t" "$tag")
done <<< "${{ steps.meta.outputs.tags }}"
# Compose source digest refs (pushed to GHCR by docker-build)
DIGEST_ARGS=()
for f in *; do
DIGEST_ARGS+=("${{ env.GHCR_IMAGE }}@sha256:$f")
done
set -x
docker buildx imagetools create "${TAG_ARGS[@]}" "${DIGEST_ARGS[@]}"
- name: Build and push
uses: docker/build-push-action@v6
with:
context: .
platforms: linux/amd64,linux/arm64
push: true
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}
build-args: |
ENABLE_OTEL=false
ENABLE_EMBEDUI=${{ matrix.enable_embedui }}
ENABLE_PYTHON=${{ matrix.enable_python }}
ENABLE_FULL_SKILLS=${{ matrix.enable_full_skills }}
VERSION=v${{ needs.release.outputs.version }}
cache-from: type=gha,scope=release-${{ matrix.variant }}
cache-to: type=gha,mode=max,scope=release-${{ matrix.variant }}
provenance: false
- name: Inspect manifest
run: |
docker buildx imagetools inspect \
"${{ env.GHCR_IMAGE }}:v${{ needs.release.outputs.version }}${{ matrix.suffix }}"
# Build web UI Docker image per platform on native runners.
# Build web UI Docker image.
docker-web-build:
needs: release
if: needs.release.outputs.released == 'true'
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
platform: [linux/amd64, linux/arm64]
include:
- platform: linux/amd64
runner: ubuntu-latest
arch: amd64
- platform: linux/arm64
runner: ubuntu-24.04-arm
arch: arm64
runs-on: ${{ matrix.runner }}
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: docker/setup-buildx-action@v3
- name: Log in to GHCR
uses: docker/login-action@v3
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}
- name: Build and push by digest
id: build
uses: docker/build-push-action@v6
with:
context: ui/web
platforms: ${{ matrix.platform }}
outputs: type=image,name=${{ env.GHCR_IMAGE }}-web,push-by-digest=true,name-canonical=true,push=true
cache-from: type=gha,scope=web-${{ matrix.arch }}
cache-to: type=gha,mode=max,scope=web-${{ matrix.arch }}
provenance: false
- name: Export digest
run: |
mkdir -p /tmp/digests
digest="${{ steps.build.outputs.digest }}"
touch "/tmp/digests/${digest#sha256:}"
- uses: actions/upload-artifact@v4
with:
name: digests-web-${{ matrix.arch }}
path: /tmp/digests/*
if-no-files-found: error
retention-days: 1
docker-web-merge:
needs: [release, docker-web-build]
if: needs.release.outputs.released == 'true'
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
- name: Download digests
uses: actions/download-artifact@v4
with:
path: /tmp/digests
pattern: digests-web-*
merge-multiple: true
- uses: docker/setup-qemu-action@v3
- uses: docker/setup-buildx-action@v3
@@ -390,28 +265,21 @@ jobs:
type=raw,value=v${{ needs.release.outputs.version }}
type=raw,value=latest
- name: Create manifest list and push
working-directory: /tmp/digests
run: |
TAG_ARGS=()
while IFS= read -r tag; do
[ -n "$tag" ] && TAG_ARGS+=("-t" "$tag")
done <<< "${{ steps.meta.outputs.tags }}"
DIGEST_ARGS=()
for f in *; do
DIGEST_ARGS+=("${{ env.GHCR_IMAGE }}-web@sha256:$f")
done
set -x
docker buildx imagetools create "${TAG_ARGS[@]}" "${DIGEST_ARGS[@]}"
- name: Inspect manifest
run: |
docker buildx imagetools inspect \
"${{ env.GHCR_IMAGE }}-web:v${{ needs.release.outputs.version }}"
- name: Build and push
uses: docker/build-push-action@v6
with:
context: ui/web
platforms: linux/amd64,linux/arm64
push: true
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}
cache-from: type=gha,scope=release-web
cache-to: type=gha,mode=max,scope=release-web
provenance: false
# Notify Discord on new release (runs even if docker jobs fail)
notify-discord:
needs: [release, build-binaries, docker-merge, docker-web-merge]
needs: [release, build-binaries, docker-build, docker-web-build]
if: always() && needs.release.outputs.released == 'true' && !cancelled()
runs-on: ubuntu-latest
timeout-minutes: 5
+2
View File
@@ -82,6 +82,8 @@ goclaw.db-shm
*.app.bak
*.exe.bak
# AI tool config directories (user-specific)
.agents/
.codex/
.gemini/
.opencode/
+1 -1
View File
@@ -554,7 +554,7 @@ ClaudeCLIProvider can be configured in `config.json`:
}
```
Or via database `llm_providers` table with `provider_type = "claude_cli"`.
Or via database `llm_providers` table with `provider_type = "claude_cli"`. For database providers, `api_base` is the CLI executable selector (`"claude"` or an absolute binary path), not an HTTP base URL, so provider URL SSRF opt-ins do not apply to Claude CLI.
### Session Management
+28 -5
View File
@@ -5,12 +5,14 @@ import (
"testing"
"github.com/nextlevelbuilder/goclaw/internal/config"
"github.com/nextlevelbuilder/goclaw/internal/pipeline"
"github.com/nextlevelbuilder/goclaw/internal/providers"
"github.com/nextlevelbuilder/goclaw/internal/tools"
)
// mockExecTool is a simple tool that records whether it was executed.
type mockExecTool struct {
name string
name string
executed bool
}
@@ -24,13 +26,16 @@ func (m *mockExecTool) Execute(_ context.Context, _ map[string]any) *tools.Resul
return tools.NewResult("ok from " + m.name)
}
// simulateLazyActivationCheck mimics the allowedTools check from loop.go for one tool call:
// simulateLazyActivationCheck mimics the runtime authorize gate from
// makeAuthorizeToolCall (loop_pipeline_callbacks.go) for one tool call.
// The real gate is a PipelineDeps.AuthorizeToolCall callback invoked by ToolStage;
// this helper exercises the same logic in isolation so tests stay unit-level.
//
// if allowedTools != nil && !allowedTools[tc.Name] {
// if l.tools.TryActivateDeferred(tc.Name) { allowedTools[tc.Name] = true }
// if allowed != nil && !allowed[name] {
// if reg.TryActivateDeferred(name) { allowed[name] = true }
// else { result = ErrorResult(...) }
// }
// if result == nil { result = l.tools.ExecuteWithContext(...) }
// if result == nil { result = reg.ExecuteWithContext(...) }
//
// Returns (result, blocked).
func simulateLazyActivationCheck(reg *tools.Registry, allowedTools map[string]bool, toolName string) (*tools.Result, bool) {
@@ -163,6 +168,24 @@ func TestLoop_LazyMCP_NilAllowedTools_AllowsAll(t *testing.T) {
}
}
func TestLoopAuthorizeToolCall_PrefixedNameCanonicalLookup(t *testing.T) {
loop := NewLoop(LoopConfig{
AgentToolPolicy: &config.ToolPolicySpec{ToolCallPrefix: "proxy_"},
})
gate := loop.makeAuthorizeToolCall()
state := &pipeline.RunState{}
state.Tool.AllowedTools = map[string]bool{"exec": true}
ok, reason := gate(context.Background(), state, providers.ToolCall{
ID: "tc-proxy-exec",
Name: "proxy_exec",
})
if !ok {
t.Fatalf("prefixed tool call should resolve to canonical allowlist entry; reason: %q", reason)
}
}
func TestLoop_LazyMCP_SecondCall_UsesUpdatedAllowedTools(t *testing.T) {
// After first lazy activation, subsequent calls in the same iteration use the
// updated allowedTools map and don't invoke the activator again.
+1
View File
@@ -150,6 +150,7 @@ func (l *Loop) buildPipelineDeps(req *RunRequest, bridgeRS *runState) pipeline.P
ExecuteToolCall: cb.executeToolCall,
ExecuteToolRaw: cb.executeToolRaw,
ProcessToolResult: cb.processToolResult,
AuthorizeToolCall: cb.authorizeToolCall,
SequentialToolCall: func(tc providers.ToolCall) bool {
return l.resolveToolCallName(tc.Name) == "wait"
},
+40
View File
@@ -57,6 +57,7 @@ func (l *Loop) pipelineCallbacks(req *RunRequest, bridgeRS *runState) pipelineCa
executeToolCall: l.makeExecuteToolCall(req, bridgeRS),
executeToolRaw: l.makeExecuteToolRaw(req),
processToolResult: l.makeProcessToolResult(req, bridgeRS),
authorizeToolCall: l.makeAuthorizeToolCall(),
checkReadOnly: l.makeCheckReadOnly(req, bridgeRS),
sanitizeContent: SanitizeAssistantContent,
flushMessages: l.makeFlushMessages(req),
@@ -85,6 +86,7 @@ type pipelineCallbackSet struct {
executeToolCall func(ctx context.Context, state *pipeline.RunState, tc providers.ToolCall) ([]providers.Message, error)
executeToolRaw func(ctx context.Context, tc providers.ToolCall) (providers.Message, any, error)
processToolResult func(ctx context.Context, state *pipeline.RunState, tc providers.ToolCall, rawMsg providers.Message, rawData any) []providers.Message
authorizeToolCall func(ctx context.Context, state *pipeline.RunState, tc providers.ToolCall) (bool, string)
checkReadOnly func(state *pipeline.RunState) (*providers.Message, bool)
sanitizeContent func(string) string
flushMessages func(ctx context.Context, sessionKey string, msgs []providers.Message) error
@@ -249,6 +251,44 @@ func (l *Loop) makeBuildFilteredTools(req *RunRequest) func(state *pipeline.RunS
}
}
// makeAuthorizeToolCall enforces a runtime fail-closed allowlist check before
// every tool execution. AllowedTools is keyed by canonical registry names (built
// by ThinkStage from FilterTools output). The model may emit prefixed names when
// the agent has toolCallPrefix configured (e.g. "proxy_exec" → canonical "exec"),
// so we resolve the name to its canonical form before the lookup to avoid a
// guaranteed miss on every prefixed call.
func (l *Loop) makeAuthorizeToolCall() func(ctx context.Context, state *pipeline.RunState, tc providers.ToolCall) (bool, string) {
return func(_ context.Context, state *pipeline.RunState, tc providers.ToolCall) (bool, string) {
allowed := state.Tool.AllowedTools
if allowed == nil {
// nil allowlist means no per-iteration restriction (e.g. BuildFilteredTools not wired).
return true, ""
}
// Resolve to canonical name before allowlist lookup. AllowedTools is keyed
// by canonical names; the model may emit prefixed names when toolCallPrefix
// is set (e.g. "proxy_exec" vs "exec"). Without this the lookup always misses.
name := l.resolveToolCallName(tc.Name)
if allowed[name] {
return true, ""
}
// Preserve lazy activation for deferred tools (typically per-user MCP).
if l.tools != nil && l.tools.TryActivateDeferred(name) {
// Re-check deny policy to prevent a lazy-activated tool from bypassing
// an explicit deny rule.
if l.toolPolicy != nil && l.toolPolicy.IsDenied(name, l.agentToolPolicy) {
return false, "tool not allowed by policy: " + name
}
allowed[name] = true
return true, ""
}
return false, "tool not allowed by policy: " + name
}
}
func (l *Loop) makeCallLLM(req *RunRequest, emitRun func(AgentEvent)) func(ctx context.Context, state *pipeline.RunState, chatReq providers.ChatRequest) (*providers.ChatResponse, error) {
return func(ctx context.Context, state *pipeline.RunState, chatReq providers.ChatRequest) (*providers.ChatResponse, error) {
provider := state.Provider
+4 -4
View File
@@ -347,9 +347,9 @@ func (c *BaseChannel) CheckDMPolicy(ctx context.Context, senderID, dmPolicy stri
if c.pairingService != nil {
paired, err := c.pairingService.IsPaired(ctx, senderID, c.name)
if err != nil {
slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
"sender_id", senderID, "channel", c.name, "error", err)
return PolicyAllow
return PolicyDeny
}
if paired {
return PolicyAllow
@@ -386,9 +386,9 @@ func (c *BaseChannel) CheckGroupPolicy(ctx context.Context, senderID, chatID, gr
if c.pairingService != nil {
paired, err := c.pairingService.IsPaired(ctx, groupSenderID, c.name)
if err != nil {
slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
"group_sender", groupSenderID, "channel", c.name, "error", err)
return PolicyAllow
return PolicyDeny
}
if paired {
c.MarkGroupApproved(chatID)
+6 -6
View File
@@ -174,12 +174,12 @@ func TestCheckDMPolicy_PolicyPairing(t *testing.T) {
wantResult: PolicyAllow,
},
{
name: "Pairing service error allows message (fail-open)",
name: "Pairing service error denies message (fail-closed)",
senderID: "user999",
allowList: []string{},
paired: false,
failPairingCheck: true,
wantResult: PolicyAllow,
wantResult: PolicyDeny,
},
}
@@ -319,14 +319,14 @@ func TestCheckGroupPolicy_PolicyPairing(t *testing.T) {
wantResult: PolicyNeedsPairing,
},
{
name: "Pairing service error allows (fail-open)",
name: "Pairing service error denies (fail-closed)",
senderID: "user999",
chatID: "chat_2",
allowList: []string{},
groupApproved: false,
paired: false,
failPairingCheck: true,
wantResult: PolicyAllow,
wantResult: PolicyDeny,
},
}
@@ -553,13 +553,13 @@ func TestCheckDMPolicy_AllPolicies_TableDriven(t *testing.T) {
wantResult: PolicyNeedsPairing,
},
{
name: "pairing policy fail-open on service error",
name: "pairing policy fail-closed on service error",
policy: "pairing",
senderID: "user3",
allowList: []string{},
paired: false,
failPairingCheck: true,
wantResult: PolicyAllow,
wantResult: PolicyDeny,
},
}
+4 -8
View File
@@ -159,9 +159,8 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) {
p1, err1 := ps.IsPaired(ctx, userID, c.Name())
p2, err2 := ps.IsPaired(ctx, senderID, c.Name())
if err1 != nil || err2 != nil {
slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
"user_id", userID, "channel", c.Name(), "err1", err1, "err2", err2)
paired = true
} else {
paired = p1 || p2
}
@@ -265,9 +264,8 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) {
groupSenderID := fmt.Sprintf("group:%d", chatID)
paired, pairErr := c.PairingService().IsPaired(ctx, groupSenderID, c.Name())
if pairErr != nil {
slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
"group_sender", groupSenderID, "channel", c.Name(), "error", pairErr)
paired = true
}
if paired {
c.MarkGroupApproved(chatIDStr)
@@ -320,9 +318,8 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) {
groupSenderID := fmt.Sprintf("group:%d", chatID)
paired, pairErr := c.PairingService().IsPaired(ctx, groupSenderID, c.Name())
if pairErr != nil {
slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
"group_sender", groupSenderID, "channel", c.Name(), "error", pairErr)
paired = true
}
if paired {
c.MarkGroupApproved(chatIDStr)
@@ -376,9 +373,8 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) {
groupSenderID := fmt.Sprintf("group:%d", chatID)
paired, err := c.PairingService().IsPaired(ctx, groupSenderID, c.Name())
if err != nil {
slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
"group_sender", groupSenderID, "channel", c.Name(), "error", err)
paired = true
}
if paired {
c.MarkGroupApproved(chatIDStr)
+15 -2
View File
@@ -95,11 +95,16 @@ func (h *FilesHandler) auth(next http.HandlerFunc) http.HandlerFunc {
// deniedFilePrefixes blocks access to sensitive system directories.
// Defense-in-depth: the auth token is the primary barrier, but restricting
// known-sensitive paths limits damage if a token leaks.
// known-sensitive absolute paths limits damage if a token leaks or a root is
// misconfigured. All entries must be absolute paths because hasDeniedFilePrefix
// uses pathWithinDir semantics — relative-rooted entries (e.g. "/.ssh") would
// only block the literal filesystem subtree at that root, not user home dirs.
var deniedFilePrefixes = []string{
"/etc/", "/proc/", "/sys/", "/dev/",
"/root/", "/boot/", "/run/",
"/var/run/", "/var/log/",
"/var/run/", "/var/log/", "/var/lib/", "/var/www/",
"/home/", "/Users/",
"/opt/", "/srv/",
}
func (h *FilesHandler) handleServe(w http.ResponseWriter, r *http.Request) {
@@ -127,6 +132,14 @@ func (h *FilesHandler) handleServe(w http.ResponseWriter, r *http.Request) {
}
signed := r.URL.Query().Get("ft") != ""
// Fail-closed observability: if no boundary roots are configured at all,
// lexicallyAllowsFilePath already returns false, but log explicitly so
// operators can detect misconfigured deployments.
if h.workspace == "" && h.dataDir == "" {
slog.Warn("security.files_no_boundary", "path", absPath)
http.NotFound(w, r)
return
}
if !h.lexicallyAllowsFilePath(r, absPath, signed) {
slog.Warn("security.files_path_denied", "path", absPath, "workspace", h.workspace, "data_dir", h.dataDir)
http.NotFound(w, r)
+54
View File
@@ -88,6 +88,60 @@ func TestFilesHandleServe_ProcDir_Blocked(t *testing.T) {
}
}
// ---- handleServe: expanded deny-prefix list ----
// TestFilesHandleServe_ExpandedDenyPrefixes verifies that paths under /home/,
// /Users/, /var/lib/, /var/www/, /opt/, and /srv/ are all blocked. These are
// defense-in-depth blocks for misconfigured roots — the token/RBAC checks are
// the primary barriers.
func TestFilesHandleServe_ExpandedDenyPrefixes_Blocked(t *testing.T) {
h, _ := makeTestFilesHandler(t)
mux := http.NewServeMux()
mux.HandleFunc("GET /v1/files/{path...}", h.handleServe)
cases := []struct {
name string
url string
}{
{"home user ssh key", "/v1/files/home/user/.ssh/id_rsa"},
{"home user aws creds", "/v1/files/home/user/.aws/credentials"},
{"Users admin dir (macOS)", "/v1/files/Users/admin/secrets.txt"},
{"var lib docker secret", "/v1/files/var/lib/docker/overlay2/secret"},
{"var www config", "/v1/files/var/www/config.php"},
{"opt secrets key", "/v1/files/opt/secrets/key.pem"},
{"srv www app config", "/v1/files/srv/www/app/config.yaml"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, tc.url, nil)
w := httptest.NewRecorder()
mux.ServeHTTP(w, req)
if w.Code == http.StatusOK {
t.Errorf("path %s should be denied (deny-prefix), got 200", tc.url)
}
})
}
}
// ---- handleServe: fail-closed observability on empty workspace/dataDir ----
// TestFilesHandleServe_NoBoundary_Denies verifies that when both workspace and
// dataDir are empty the handler returns 404 (fail-closed) for any path.
func TestFilesHandleServe_NoBoundary_Denies(t *testing.T) {
h := NewFilesHandler("", "")
mux := http.NewServeMux()
mux.HandleFunc("GET /v1/files/{path...}", h.handleServe)
req := httptest.NewRequest(http.MethodGet, "/v1/files/tmp/test.txt", nil)
w := httptest.NewRecorder()
mux.ServeHTTP(w, req)
if w.Code == http.StatusOK {
t.Errorf("empty workspace+dataDir should deny all requests, got 200")
}
}
// ---- handleServe: workspace boundary enforcement ----
func TestFilesHandleServe_FileInsideWorkspace_WithToken_Serves(t *testing.T) {
+6
View File
@@ -65,6 +65,12 @@ func (m *mockProviderStore) UpdateProvider(_ context.Context, id uuid.UUID, upda
if v, ok := updates["api_key"]; ok {
p.APIKey = v.(string)
}
if v, ok := updates["api_base"]; ok {
p.APIBase = v.(string)
}
if v, ok := updates["provider_type"]; ok {
p.ProviderType = v.(string)
}
if v, ok := updates["settings"]; ok {
p.Settings = v.(json.RawMessage)
}
+108 -23
View File
@@ -8,6 +8,7 @@ import (
"net"
"net/http"
"net/url"
"os"
"os/exec"
"path/filepath"
"regexp"
@@ -22,6 +23,7 @@ import (
"github.com/nextlevelbuilder/goclaw/internal/oauth"
"github.com/nextlevelbuilder/goclaw/internal/permissions"
"github.com/nextlevelbuilder/goclaw/internal/providers"
"github.com/nextlevelbuilder/goclaw/internal/security"
"github.com/nextlevelbuilder/goclaw/internal/store"
"github.com/nextlevelbuilder/goclaw/internal/tools"
usagecaps "github.com/nextlevelbuilder/goclaw/internal/usage/caps"
@@ -338,55 +340,138 @@ func normalizeOllamaAPIBase(p *store.LLMProviderData) {
}
}
// localProviderTypes are provider types that legitimately run on localhost
// (e.g. Ollama, Claude CLI). SSRF checks are skipped for these.
var localProviderTypes = map[string]bool{
store.ProviderOllama: true,
store.ProviderClaudeCLI: true,
store.ProviderACP: true,
// localURLProviderTypes are provider types that legitimately run on localhost.
// They are restricted to an explicit localhost allowlist
// rather than skipping SSRF validation entirely.
var localURLProviderTypes = map[string]bool{
store.ProviderOllama: true,
store.ProviderACP: true,
}
// allowedLocalHosts are the only hosts permitted for local provider types.
// Explicit allowlist (not blocklist) to prevent new internal addresses from
// slipping through (e.g. 169.254.169.254 via ollama base URL).
var allowedLocalHosts = []string{"localhost", "127.0.0.1", "::1", "host.docker.internal"}
// dnsResolverFn resolves hostnames to IPs. Replaceable in tests.
var dnsResolverFn = net.LookupHost
// allowPrivateProviderURLsFn reports whether the operator has opted in to
// permitting private / loopback / link-local / internal-hostname provider base
// URLs via GOCLAW_ALLOW_PRIVATE_PROVIDER_URLS. Evaluated once at first call so
// tests can override the variable before that happens.
var allowPrivateProviderURLsFn = sync.OnceValue(func() bool {
v := strings.ToLower(strings.TrimSpace(os.Getenv("GOCLAW_ALLOW_PRIVATE_PROVIDER_URLS")))
return v == "1" || v == "true" || v == "yes"
})
// validateProviderURL rejects provider base URLs pointing to internal/private networks.
// Defense-in-depth: prevents SSRF when providers are later used for API calls.
//
// Logic:
// 1. Empty URL → allowed (provider may not need a custom base).
// 2. Claude CLI → api_base is an executable path/command, not a URL.
// 3. Scheme check (http/https only) → enforced for URL-based types, including
// local URL types. Blocks file://, gopher://, dict://, etc.
// 4. Local URL types (ollama, acp) → host must be in allowedLocalHosts
// (explicit allowlist prevents reaching 169.254.169.254 or internal services
// via the local-type bypass).
// 5. Remote types → if GOCLAW_ALLOW_PRIVATE_PROVIDER_URLS is set, allow and log.
// Otherwise: resolve DNS hostname; reject if ANY resolved IP satisfies
// security.IsBlocked (covers loopback, link-local, private, multicast,
// unspecified — including 0.0.0.0 and :: that earlier hand-rolled checks missed).
//
// DNS resolution on step 5 closes the nip.io / sslip.io / attacker-domain bypass
// where a hostname passes a literal-string blocklist but resolves to a private IP.
func validateProviderURL(rawURL string, providerType string) error {
if rawURL == "" || localProviderTypes[providerType] {
if rawURL == "" {
return nil
}
if providerType == store.ProviderClaudeCLI {
return validateClaudeCLIExecutablePath(rawURL)
}
u, err := url.Parse(rawURL)
if err != nil {
return fmt.Errorf("invalid URL: %w", err)
}
// Only allow http/https schemes — block file://, gopher://, dict://, etc.
// Scheme check is unconditional for URL-based provider types, including local URL types.
switch u.Scheme {
case "http", "https":
default:
return fmt.Errorf("provider URL must use http or https scheme, got %q", u.Scheme)
}
host := u.Hostname()
// Block obvious internal targets
blocked := []string{"localhost", "127.0.0.1", "::1", "0.0.0.0", "169.254.169.254", "metadata.google.internal"}
for _, b := range blocked {
if strings.EqualFold(host, b) {
return fmt.Errorf("provider URL cannot point to %s", b)
// Local provider types: only allow an explicit localhost allowlist.
// This prevents using the local-type escape hatch to reach internal services
// or cloud metadata endpoints.
if localURLProviderTypes[providerType] {
for _, a := range allowedLocalHosts {
if strings.EqualFold(host, a) {
return nil
}
}
slog.Warn("security.provider_url.local_type_denied", "host", host, "provider_type", providerType)
return fmt.Errorf("provider type %q only allows localhost URLs (localhost, 127.0.0.1, ::1, host.docker.internal), got host %q", providerType, host)
}
// Block private IP ranges (normalize IPv6-mapped IPv4 to catch ::ffff:127.0.0.1)
ip := net.ParseIP(host)
if ip != nil {
if v4 := ip.To4(); v4 != nil {
ip = v4
}
if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() {
return fmt.Errorf("provider URL cannot point to private network: %s", host)
}
// Operator opt-in to allow private-network provider URLs (e.g. LAN-hosted vLLM).
// Scheme check above still applies even with this gate open.
if allowPrivateProviderURLsFn() {
slog.Warn("security.provider_url.private_allowed", "host", host, "provider_type", providerType)
return nil
}
// Block common internal hostnames
// Check literal IP first (avoids unnecessary DNS lookup).
if ip := net.ParseIP(host); ip != nil {
if security.IsBlocked(ip) {
slog.Warn("security.provider_url.blocked", "host", host, "provider_type", providerType)
return fmt.Errorf("provider URL cannot point to %s", host)
}
return nil
}
// Block .internal / .local suffix before DNS (fail-fast for well-known patterns).
if strings.HasSuffix(host, ".internal") || strings.HasSuffix(host, ".local") {
slog.Warn("security.provider_url.blocked", "host", host, "provider_type", providerType)
return fmt.Errorf("provider URL cannot point to internal hostname: %s", host)
}
// Resolve DNS and check every returned address.
// Prevents bypass via wildcard services (nip.io, sslip.io) or attacker-controlled
// domains that map to private IPs (DNS-rebinding at config time).
addrs, err := dnsResolverFn(host)
if err != nil {
slog.Warn("security.provider_url.dns_resolve_failed", "host", host, "provider_type", providerType, "error", err)
return fmt.Errorf("provider URL hostname %q could not be resolved: %w", host, err)
}
for _, addr := range addrs {
ip := net.ParseIP(addr)
if ip == nil {
continue
}
if security.IsBlocked(ip) {
slog.Warn("security.provider_url.blocked_resolved", "host", host, "resolved_ip", ip.String(), "provider_type", providerType)
return fmt.Errorf("provider URL %q resolves to private/reserved address %s", host, ip)
}
}
return nil
}
func validateClaudeCLIExecutablePath(path string) error {
if strings.Contains(path, "\x00") {
return fmt.Errorf("Claude CLI executable path cannot contain NUL byte")
}
if _, err := url.ParseRequestURI(path); err == nil && strings.Contains(path, "://") {
return fmt.Errorf("Claude CLI api_base must be an executable path or %q, got URL %q", "claude", path)
}
if path == "claude" || filepath.IsAbs(path) {
return nil
}
return fmt.Errorf("Claude CLI api_base must be %q or an absolute executable path, got %q", "claude", path)
}
// --- Provider CRUD ---
func (h *ProvidersHandler) handleListProviders(w http.ResponseWriter, r *http.Request) {
+73
View File
@@ -298,6 +298,79 @@ func TestProvidersHandlerCreateAllows1536EmbeddingDimensions(t *testing.T) {
}
}
func TestProvidersHandlerCreateAllowsClaudeCLIExecutablePath(t *testing.T) {
token := setupProvidersAdminToken(t)
providerStore := newMockProviderStore()
handler := NewProvidersHandler(providerStore, newMockSecretsStore(), nil, "")
mux := http.NewServeMux()
handler.RegisterRoutes(mux)
body := map[string]any{
"name": "claude-local",
"provider_type": store.ProviderClaudeCLI,
"api_base": writeFakeClaudeBinary(t),
"enabled": true,
}
rawBody, err := json.Marshal(body)
if err != nil {
t.Fatalf("Marshal() error = %v", err)
}
req := httptest.NewRequest(http.MethodPost, "/v1/providers", bytes.NewReader(rawBody))
req.Header.Set("Authorization", "Bearer "+token)
w := httptest.NewRecorder()
mux.ServeHTTP(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("status code = %d, want %d, body=%s", w.Code, http.StatusCreated, w.Body.String())
}
if got := providerStore.providers["claude-local"].APIBase; got == "" || !filepath.IsAbs(got) {
t.Fatalf("stored Claude CLI api_base = %q, want absolute executable path", got)
}
}
func TestProvidersHandlerUpdateAllowsClaudeCLIExecutablePath(t *testing.T) {
token := setupProvidersAdminToken(t)
providerStore := newMockProviderStore()
provider := &store.LLMProviderData{
BaseModel: store.BaseModel{ID: uuid.New()},
Name: "claude-local",
ProviderType: store.ProviderClaudeCLI,
APIBase: "claude",
Enabled: true,
}
if err := providerStore.CreateProvider(context.Background(), provider); err != nil {
t.Fatalf("CreateProvider() error = %v", err)
}
handler := NewProvidersHandler(providerStore, newMockSecretsStore(), nil, "")
mux := http.NewServeMux()
handler.RegisterRoutes(mux)
nextPath := writeFakeClaudeBinary(t)
body := map[string]any{"api_base": nextPath}
rawBody, err := json.Marshal(body)
if err != nil {
t.Fatalf("Marshal() error = %v", err)
}
req := httptest.NewRequest(http.MethodPut, "/v1/providers/"+provider.ID.String(), bytes.NewReader(rawBody))
req.Header.Set("Authorization", "Bearer "+token)
w := httptest.NewRecorder()
mux.ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Fatalf("status code = %d, want %d, body=%s", w.Code, http.StatusOK, w.Body.String())
}
current, err := providerStore.GetProvider(context.Background(), provider.ID)
if err != nil {
t.Fatalf("GetProvider() error = %v", err)
}
if current.APIBase != nextPath {
t.Fatalf("api_base = %q, want %q", current.APIBase, nextPath)
}
}
func TestProvidersHandlerUpdateRejectsIncompatibleEmbeddingDimensions(t *testing.T) {
token := setupProvidersAdminToken(t)
providerStore := newMockProviderStore()
@@ -0,0 +1,326 @@
package http
import (
"net"
"path/filepath"
"strings"
"testing"
)
// stubResolver returns a net.LookupHost-compatible function that maps known
// hostnames to controlled IPs. Unknown hostnames return a DNS error.
func stubResolver(m map[string][]string) func(host string) ([]string, error) {
return func(host string) ([]string, error) {
if addrs, ok := m[host]; ok {
return addrs, nil
}
return nil, &net.DNSError{Err: "no such host", Name: host, IsNotFound: true}
}
}
// saveAndRestoreGlobals saves mutable package-level vars and restores them after
// the test. Call at the start of any test that touches dnsResolverFn or
// allowPrivateProviderURLsFn.
func saveAndRestoreGlobals(t *testing.T) {
t.Helper()
origResolver := dnsResolverFn
origAllow := allowPrivateProviderURLsFn
t.Cleanup(func() {
dnsResolverFn = origResolver
allowPrivateProviderURLsFn = origAllow
})
}
// --- Core merged test: all major cases in one table ---
func TestValidateProviderURL(t *testing.T) {
saveAndRestoreGlobals(t)
allowPrivateProviderURLsFn = func() bool { return false }
absClaudePath := filepath.Join(t.TempDir(), "claude")
dnsResolverFn = stubResolver(map[string][]string{
"api.openai.com": {"104.18.6.192"},
"legit-provider.com": {"203.0.113.50"},
"10.10.27.30.nip.io": {"10.10.27.30"},
"192.168.1.1.sslip.io": {"192.168.1.1"},
"172.16.0.5.nip.io": {"172.16.0.5"},
"169.254.169.254.nip.io": {"169.254.169.254"},
"127.0.0.1.nip.io": {"127.0.0.1"},
"rebind.attacker.com": {"10.0.0.1"},
"dual-stack.example.com": {"203.0.113.50", "10.0.0.1"},
})
tests := []struct {
name string
rawURL string
providerType string
wantErr bool
}{
// Empty URL always allowed
{"empty URL", "", "openai_compat", false},
{"empty URL ollama", "", "ollama", false},
// Public remote URLs OK
{"public HTTPS", "https://api.openai.com/v1", "openai_compat", false},
{"public HTTP", "http://legit-provider.com/v1", "openai_compat", false},
// --- Scheme check: unconditional for ALL types including local ---
{"file scheme remote", "file:///etc/passwd", "openai_compat", true},
{"gopher scheme remote", "gopher://internal:25", "openai_compat", true},
{"file scheme ollama", "file:///etc/passwd", "ollama", true}, // H-1: scheme enforced even for local types
{"gopher scheme acp", "gopher://localhost:25", "acp", true}, // H-1: scheme enforced even for local types
{"file scheme claude_cli", "file:///bin/bash", "claude_cli", true}, // H-1: scheme enforced for URL-like Claude CLI values
// --- Local type: allowlist-only ---
{"ollama localhost", "http://localhost:11434/v1", "ollama", false},
{"ollama 127.0.0.1", "http://127.0.0.1:11434/v1", "ollama", false},
{"ollama ::1", "http://[::1]:11434/v1", "ollama", false},
{"ollama host.docker.internal", "http://host.docker.internal:11434/v1", "ollama", false},
{"acp 127.0.0.1", "http://127.0.0.1:9090", "acp", false},
{"claude_cli command name", "claude", "claude_cli", false},
{"claude_cli absolute path", absClaudePath, "claude_cli", false},
// Local type with non-localhost hosts → blocked
{"ollama 169.254.169.254", "http://169.254.169.254/latest/meta-data/", "ollama", true},
{"ollama private IP", "http://10.0.0.5:11434/v1", "ollama", true},
{"ollama evil.com", "http://evil.attacker.tld:9999/v1", "ollama", true},
{"ollama postgres sidecar", "http://postgres:5432/v1", "ollama", true},
{"ollama link-local", "http://169.254.1.1:8080/v1", "ollama", true},
{"ollama .internal", "http://redis.internal:6379/v1", "ollama", true},
{"ollama gcp metadata", "http://metadata.google.internal/computeMetadata/v1/", "ollama", true},
{"acp private", "http://10.0.0.1:8080/v1", "acp", true},
// --- Remote type literal blocked IPs ---
{"remote localhost", "http://localhost:8080", "openai_compat", true},
{"remote 127.0.0.1", "http://127.0.0.1:8080", "openai_compat", true},
{"remote ::1", "http://[::1]:8080", "openai_compat", true},
{"remote 10.x", "http://10.0.0.1:8080/v1", "openai_compat", true},
{"remote 192.168.x", "http://192.168.1.100:8080/v1", "openai_compat", true},
{"remote 172.16.x", "http://172.16.0.5:8080/v1", "openai_compat", true},
{"remote 169.254.169.254", "http://169.254.169.254/latest/meta-data/", "openai_compat", true},
// --- Regression: unspecified addresses (bug missed by #974 hand-rolled checks) ---
{"0.0.0.0 unspecified", "http://0.0.0.0:8080/v1", "openai_compat", true},
{":: IPv6 unspecified", "http://[::]:8080/v1", "openai_compat", true},
// --- DNS bypass via wildcard services (nip.io, sslip.io) ---
{"nip.io 10.x", "http://10.10.27.30.nip.io:9999/v1", "openai_compat", true},
{"sslip.io 192.168.x", "http://192.168.1.1.sslip.io:8080/v1", "openai_compat", true},
{"nip.io 172.16.x", "http://172.16.0.5.nip.io:8080/v1", "openai_compat", true},
{"nip.io link-local", "http://169.254.169.254.nip.io/latest/", "openai_compat", true},
{"nip.io loopback", "http://127.0.0.1.nip.io:8080/v1", "openai_compat", true},
{"attacker rebind", "http://rebind.attacker.com:8080/v1", "openai_compat", true},
{"dual-stack any private", "http://dual-stack.example.com:8080/v1", "openai_compat", true},
// --- Internal hostname suffix ---
{".internal suffix", "http://metadata.google.internal/computeMetadata/v1/", "openai_compat", true},
{".local suffix", "http://myservice.local:8080", "openai_compat", true},
// --- Unresolvable hostname ---
{"unresolvable hostname", "http://nonexistent.invalid:8080/v1", "openai_compat", true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateProviderURL(tt.rawURL, tt.providerType)
if (err != nil) != tt.wantErr {
t.Errorf("validateProviderURL(%q, %q) error = %v, wantErr %v",
tt.rawURL, tt.providerType, err, tt.wantErr)
}
})
}
}
// --- Env gate: GOCLAW_ALLOW_PRIVATE_PROVIDER_URLS ---
func TestValidateProviderURL_AllowPrivateFlag(t *testing.T) {
saveAndRestoreGlobals(t)
dnsResolverFn = stubResolver(map[string][]string{
"my-vllm.lan": {"10.0.0.1"},
})
t.Run("private URL allowed when flag is true", func(t *testing.T) {
allowPrivateProviderURLsFn = func() bool { return true }
if err := validateProviderURL("http://my-vllm.lan:8080/v1", "openai_compat"); err != nil {
t.Errorf("expected nil with allow-private flag, got: %v", err)
}
if err := validateProviderURL("http://192.168.1.50:8080/v1", "openai_compat"); err != nil {
t.Errorf("expected nil for private IP with allow-private flag, got: %v", err)
}
if err := validateProviderURL("http://localhost:8080/v1", "openai_compat"); err != nil {
t.Errorf("expected nil for localhost with allow-private flag, got: %v", err)
}
if err := validateProviderURL("http://llm.internal/v1", "openai_compat"); err != nil {
t.Errorf("expected nil for .internal with allow-private flag, got: %v", err)
}
})
t.Run("private URL blocked when flag is false", func(t *testing.T) {
allowPrivateProviderURLsFn = func() bool { return false }
if err := validateProviderURL("http://my-vllm.lan:8080/v1", "openai_compat"); err == nil {
t.Error("expected error for private URL without allow-private flag")
}
})
t.Run("scheme still enforced even with allow-private flag", func(t *testing.T) {
allowPrivateProviderURLsFn = func() bool { return true }
err := validateProviderURL("file:///etc/passwd", "openai_compat")
if err == nil {
t.Error("expected error for file:// scheme even with allow-private flag")
}
if !strings.Contains(err.Error(), "scheme") {
t.Errorf("expected scheme error, got: %v", err)
}
})
}
func TestValidateProviderURL_LocalTypesIgnoreAllowPrivateFlag(t *testing.T) {
saveAndRestoreGlobals(t)
allowPrivateProviderURLsFn = func() bool { return true }
cases := []struct {
url string
providerType string
}{
{"http://ollama:11434/v1", "ollama"},
{"http://host.lan:11434/v1", "ollama"},
{"http://10.0.0.5:11434/v1", "ollama"},
{"http://acp-sidecar:9090", "acp"},
}
for _, c := range cases {
if err := validateProviderURL(c.url, c.providerType); err == nil {
t.Errorf("expected local provider URL %s / %s to remain blocked despite allow-private flag", c.url, c.providerType)
}
}
}
// --- H-1: scheme enforced for local provider types (from PR #972) ---
func TestValidateProviderURL_LocalTypeSchemeEnforced(t *testing.T) {
saveAndRestoreGlobals(t)
cases := []struct {
url string
providerType string
}{
{"file:///etc/passwd", "ollama"},
{"gopher://localhost:25", "ollama"},
{"file:///etc/passwd", "acp"},
}
for _, c := range cases {
err := validateProviderURL(c.url, c.providerType)
if err == nil {
t.Errorf("expected scheme error for %s / %s, got nil", c.url, c.providerType)
continue
}
if !strings.Contains(err.Error(), "scheme") {
t.Errorf("expected scheme error for %s / %s, got: %v", c.url, c.providerType, err)
}
}
}
// --- Regression: 0.0.0.0 and :: unspecified (bug PR #974 missed) ---
func TestValidateProviderURL_UnspecifiedBlocked(t *testing.T) {
saveAndRestoreGlobals(t)
allowPrivateProviderURLsFn = func() bool { return false }
cases := []string{
"http://0.0.0.0:8080/v1",
"http://[::]:8080/v1",
}
for _, u := range cases {
err := validateProviderURL(u, "openai_compat")
if err == nil {
t.Errorf("expected error for unspecified address %q, got nil", u)
}
}
}
// --- DNS-rebinding: remote type resolving to private/loopback ---
func TestValidateProviderURL_DNSRebindBlocked(t *testing.T) {
saveAndRestoreGlobals(t)
allowPrivateProviderURLsFn = func() bool { return false }
privateHosts := map[string][]string{
"rebind-private.example.com": {"10.0.0.1"},
"rebind-loopback.example.com": {"127.0.0.1"},
"rebind-linklocal.example.com": {"169.254.169.254"},
}
dnsResolverFn = stubResolver(privateHosts)
for host := range privateHosts {
err := validateProviderURL("http://"+host+":8080/v1", "openai_compat")
if err == nil {
t.Errorf("expected DNS rebind to be blocked for %s, got nil", host)
}
}
}
// --- Local type: all allowed variants ---
func TestValidateProviderURL_LocalTypeAllowedHosts(t *testing.T) {
saveAndRestoreGlobals(t)
allowed := []struct {
url string
providerType string
}{
{"http://localhost:11434/v1", "ollama"},
{"http://127.0.0.1:11434/v1", "ollama"},
{"http://[::1]:11434/v1", "ollama"},
{"http://host.docker.internal:11434/v1", "ollama"},
{"http://localhost:9090", "acp"},
{"http://127.0.0.1:9090", "acp"},
}
for _, a := range allowed {
if err := validateProviderURL(a.url, a.providerType); err != nil {
t.Errorf("expected %s / %s to be allowed, got: %v", a.url, a.providerType, err)
}
}
}
func TestValidateProviderURL_ClaudeCLIExecutablePath(t *testing.T) {
saveAndRestoreGlobals(t)
absClaudePath := filepath.Join(t.TempDir(), "claude")
allowed := []string{
"",
"claude",
absClaudePath,
filepath.Join(t.TempDir(), "Claude Code.app", "Contents", "MacOS", "claude"),
}
for _, raw := range allowed {
if err := validateProviderURL(raw, "claude_cli"); err != nil {
t.Errorf("expected Claude CLI executable %q to be allowed, got: %v", raw, err)
}
}
blocked := []string{
"file:///usr/local/bin/claude",
"https://api.anthropic.com/v1",
"relative/claude",
"claude --dangerously-skip-permissions",
}
for _, raw := range blocked {
if err := validateProviderURL(raw, "claude_cli"); err == nil {
t.Errorf("expected Claude CLI executable %q to be rejected", raw)
}
}
}
// --- Public remote URL always OK ---
func TestValidateProviderURL_PublicHostOK(t *testing.T) {
saveAndRestoreGlobals(t)
allowPrivateProviderURLsFn = func() bool { return false }
dnsResolverFn = stubResolver(map[string][]string{
"api.openai.com": {"104.18.6.192"},
})
if err := validateProviderURL("https://api.openai.com/v1", "openai_compat"); err != nil {
t.Errorf("expected public host to pass, got: %v", err)
}
}
+238
View File
@@ -1,13 +1,149 @@
package http
import (
"errors"
"log/slog"
"net/http"
"os"
"path/filepath"
"strings"
"time"
"github.com/google/uuid"
"github.com/nextlevelbuilder/goclaw/internal/eventbus"
"github.com/nextlevelbuilder/goclaw/internal/store"
"github.com/nextlevelbuilder/goclaw/internal/vault"
)
// writeDocumentContent writes the given content bytes to a tenant-workspace-
// relative path, after validating the path stays inside the workspace and uses
// an allowed extension. Returns the SHA-256 content hash on success.
//
// This is the shared writer used by handleCreateDocument and handleUpdateDocument
// to materialise inline JSON `content` payloads on disk, so the enrichment
// pipeline (which reads files by path) can later compute summaries, embeddings
// and links.
func (h *VaultHandler) writeDocumentContent(workspace, relPath string, content []byte) (string, error) {
// Ensure the tenant workspace exists before resolving symlinks. Non-master
// tenants live under workspace/tenants/{slug}/ which handleUpload creates on
// demand (see vault_handler_upload.go) — without this, the very first JSON
// content write into a fresh tenant would fail EvalSymlinks with ErrNotExist
// and 500 the request.
if err := os.MkdirAll(workspace, 0o755); err != nil {
return "", err
}
// Resolve the workspace root through any symlinks so all containment checks
// compare canonical paths.
realWS, err := filepath.EvalSymlinks(workspace)
if err != nil {
return "", err
}
target := filepath.Join(realWS, filepath.Clean(relPath))
// Lexical containment: blocks "../" and absolute-path escapes.
if target != realWS && !strings.HasPrefix(target, realWS+string(os.PathSeparator)) {
slog.Warn("security.vault_symlink_escape",
"site", "path_traversal", "attempted", target, "workspace", realWS)
return "", os.ErrInvalid
}
// Symlink containment: a lexical prefix check is not enough — a symlink that
// lives *inside* the workspace but points outside it (e.g. workspace/link ->
// /etc) would otherwise let the write follow it and clobber a file beyond
// the workspace. Resolve the deepest already-existing ancestor of the target
// and confirm it still canonicalises to a path inside the workspace, before
// any directory is created or byte written.
for ancestor := filepath.Dir(target); ; {
resolved, rerr := filepath.EvalSymlinks(ancestor)
if rerr == nil {
if resolved != realWS && !strings.HasPrefix(resolved, realWS+string(os.PathSeparator)) {
slog.Warn("security.vault_symlink_escape",
"site", "ancestor", "resolved", resolved, "workspace", realWS)
return "", os.ErrInvalid
}
break
}
if !os.IsNotExist(rerr) {
return "", rerr
}
parent := filepath.Dir(ancestor)
if parent == ancestor {
break // walked to the filesystem root without an existing ancestor
}
ancestor = parent
}
if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil {
return "", err
}
// Fast-fail when the final component is already a symlink — gives a clean
// rejection + security log without parsing OS-specific error codes. The
// O_NOFOLLOW open below closes the TOCTOU window this Lstat leaves open
// (an attacker could swap the file for a symlink between calls).
if fi, lerr := os.Lstat(target); lerr == nil && fi.Mode()&os.ModeSymlink != 0 {
slog.Warn("security.vault_symlink_escape",
"site", "final_lstat", "target", target, "workspace", realWS)
return "", os.ErrInvalid
}
// O_NOFOLLOW makes the kernel refuse to follow a symlink at the final
// component atomically with the open, removing the Lstat→Open race entirely
// on Unix. On Windows oNoFollow == 0 and the Lstat above is best-effort.
f, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_TRUNC|oNoFollow, 0o644)
if err != nil {
if isSymlinkLoopErr(err) {
slog.Warn("security.vault_symlink_escape",
"site", "open_nofollow", "target", target, "workspace", realWS)
return "", os.ErrInvalid
}
return "", err
}
if _, werr := f.Write(content); werr != nil {
_ = f.Close() // best-effort cleanup; the write error is the real one
return "", werr
}
// Explicit Close so a delayed FS error (e.g. disk-full at flush) surfaces
// instead of being silently swallowed by a deferred close.
if cerr := f.Close(); cerr != nil {
return "", cerr
}
return vault.ContentHash(content), nil
}
// publishDocUpserted enqueues the standard EventVaultDocUpserted so the
// enrichment worker re-runs summary/embedding/link extraction for this doc.
// No-op when the event bus is not wired (some test setups).
func (h *VaultHandler) publishDocUpserted(tenantID uuid.UUID, agentID, docID, relPath, hash, workspace string) {
if h.eventBus == nil {
return
}
tenantIDStr := tenantID.String()
// Start enrichment progress before publishing so the worker pool can't drain
// the event and finish before the progress tracker registers it — same
// ordering handleUpload relies on to avoid that race.
if h.enrichProgress != nil {
h.enrichProgress.Start(1, tenantID)
}
h.eventBus.Publish(eventbus.DomainEvent{
ID: uuid.Must(uuid.NewV7()).String(),
Type: eventbus.EventVaultDocUpserted,
SourceID: docID + ":" + hash,
TenantID: tenantIDStr,
AgentID: agentID,
Timestamp: time.Now(),
Payload: eventbus.VaultDocUpsertedPayload{
DocID: docID,
TenantID: tenantIDStr,
AgentID: agentID,
Path: relPath,
ContentHash: hash,
Workspace: workspace,
},
})
}
// handleListAllDocuments lists vault documents across all agents in tenant.
// Optional query param agent_id to filter by specific agent.
func (h *VaultHandler) handleListAllDocuments(w http.ResponseWriter, r *http.Request) {
@@ -99,6 +235,16 @@ func (h *VaultHandler) handleGetDocument(w http.ResponseWriter, r *http.Request)
}
// handleCreateDocument creates a new vault document.
//
// `content` semantics — symmetric with handleUpdateDocument:
// - field omitted (nil pointer): metadata-only stub, no file write, no event
// (typically followed by a rescan that materialises the file from disk).
// - field present and non-empty: bytes materialised at <tenant-workspace>/<path>,
// SHA-256 hash stored on the row, EventVaultDocUpserted emitted so the
// enrichment worker computes summary/embeddings/links — same code path the
// multipart /v1/vault/upload endpoint and the filesystem rescan use.
// - field present and empty (""): a 0-byte file is written and the event still
// fires. Same behaviour as PUT — pick the right shape on the client side.
func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Request) {
locale := extractLocale(r)
tenantID := store.TenantIDFromContext(r.Context())
@@ -107,6 +253,7 @@ func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Reque
var body struct {
Path string `json:"path"`
Title string `json:"title"`
Content *string `json:"content"` // nil=no write; ""=write empty file; "data"=write bytes (mirrors PUT)
DocType string `json:"doc_type"`
Scope string `json:"scope"`
TeamID string `json:"team_id"`
@@ -137,6 +284,16 @@ func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Reque
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid scope"})
return
}
// Validate extension upfront if caller wants to write content — matches the
// whitelist enforced by /v1/vault/upload so the two endpoints accept the
// same file types. nil pointer = "no content field", which skips the check.
if body.Content != nil {
ext := strings.ToLower(filepath.Ext(body.Path))
if !allowedUploadExts[ext] {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "unsupported file type: " + ext})
return
}
}
doc := &store.VaultDocument{
TenantID: tenantID.String(),
@@ -160,11 +317,50 @@ func (h *VaultHandler) handleCreateDocument(w http.ResponseWriter, r *http.Reque
doc.Scope = "team"
}
}
// Persist file content if provided, before the DB upsert so the
// content_hash column is set in a single write.
var (
wsPath string
writtenHash string
contentWasSet bool
)
if body.Content != nil {
wsPath = h.resolveTenantWorkspace(r.Context())
if wsPath == "" {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "workspace not available"})
return
}
hash, werr := h.writeDocumentContent(wsPath, body.Path, []byte(*body.Content))
if werr != nil {
slog.Warn("vault.create: write content failed", "path", body.Path, "error", werr)
if errors.Is(werr, os.ErrInvalid) {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "path escapes workspace"})
return
}
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "failed to write content"})
return
}
doc.ContentHash = hash
writtenHash = hash
contentWasSet = true
}
if err := h.store.UpsertDocument(r.Context(), doc); err != nil {
slog.Warn("vault.create failed", "error", err)
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
return
}
// Fire enrichment after the DB row exists so the worker can locate the doc.
if contentWasSet {
evAgent := ""
if doc.AgentID != nil {
evAgent = *doc.AgentID
}
h.publishDocUpserted(tenantID, evAgent, doc.ID, body.Path, writtenHash, wsPath)
}
// Re-fetch by ID (set via RETURNING) — unambiguous even when same path exists across teams.
created, _ := h.store.GetDocumentByID(r.Context(), tenantID.String(), doc.ID)
if created != nil {
@@ -189,6 +385,7 @@ func (h *VaultHandler) handleUpdateDocument(w http.ResponseWriter, r *http.Reque
var body struct {
Title *string `json:"title"`
Content *string `json:"content"` // nil=no change; "" clears the file; non-empty rewrites it
DocType *string `json:"doc_type"`
Scope *string `json:"scope"`
TeamID *string `json:"team_id"` // nil=no change, ""=clear, "uuid"=set
@@ -225,11 +422,52 @@ func (h *VaultHandler) handleUpdateDocument(w http.ResponseWriter, r *http.Reque
existing.Metadata = body.Metadata
}
// Rewrite content on disk when caller supplied a `content` field.
var (
wsPath string
writtenHash string
contentWasSet bool
)
if body.Content != nil {
ext := strings.ToLower(filepath.Ext(existing.Path))
if !allowedUploadExts[ext] {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "unsupported file type: " + ext})
return
}
wsPath = h.resolveTenantWorkspace(r.Context())
if wsPath == "" {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "workspace not available"})
return
}
hash, werr := h.writeDocumentContent(wsPath, existing.Path, []byte(*body.Content))
if werr != nil {
slog.Warn("vault.update: write content failed", "path", existing.Path, "error", werr)
if errors.Is(werr, os.ErrInvalid) {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "path escapes workspace"})
return
}
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "failed to write content"})
return
}
existing.ContentHash = hash
writtenHash = hash
contentWasSet = true
}
if err := h.store.UpsertDocument(r.Context(), existing); err != nil {
slog.Warn("vault.update failed", "error", err)
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
return
}
if contentWasSet {
evAgent := ""
if existing.AgentID != nil {
evAgent = *existing.AgentID
}
h.publishDocUpserted(tenantID, evAgent, existing.ID, existing.Path, writtenHash, wsPath)
}
updated, _ := h.store.GetDocumentByID(r.Context(), tenantID.String(), docID)
if updated != nil {
writeJSON(w, http.StatusOK, updated)
@@ -0,0 +1,20 @@
//go:build !windows
package http
import (
"errors"
"syscall"
)
// oNoFollow is OR'd into the open flags so the kernel refuses to follow a
// symlink at the final path component — closes the Lstat→Open TOCTOU window
// where a tenant could swap a regular file for a symlink between the two
// syscalls. On Windows this is a no-op (see the _windows.go counterpart).
const oNoFollow = syscall.O_NOFOLLOW
// isSymlinkLoopErr reports whether err is the kernel's "refused to follow
// symlink" signal from an O_NOFOLLOW open (ELOOP on Linux/macOS).
func isSymlinkLoopErr(err error) bool {
return errors.Is(err, syscall.ELOOP)
}
@@ -0,0 +1,11 @@
//go:build windows
package http
// Windows has no O_NOFOLLOW concept and follows reparse points transparently;
// the Lstat short-circuit in writeDocumentContent is the only protection
// available there. Desktop edition is single-user with no untrusted tenants,
// so this degradation is acceptable.
const oNoFollow = 0
func isSymlinkLoopErr(err error) bool { return false }
@@ -0,0 +1,470 @@
package http
import (
"bytes"
"context"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"
"time"
"github.com/google/uuid"
"github.com/nextlevelbuilder/goclaw/internal/eventbus"
"github.com/nextlevelbuilder/goclaw/internal/store"
"github.com/nextlevelbuilder/goclaw/internal/vault"
)
// masterTenant resolves TenantWorkspace to h.workspace unchanged, keeping the
// content-write tests pointed at a single temp dir.
var masterTenant = uuid.MustParse("0193a5b0-7000-7000-8000-000000000001")
func masterCtx() context.Context {
return store.WithTenantID(context.Background(), masterTenant)
}
// ---- writeDocumentContent: path & symlink containment (the blocking finding) ----
func TestWriteDocumentContent_WritesInsideWorkspace(t *testing.T) {
h := &VaultHandler{}
ws := t.TempDir()
hash, err := h.writeDocumentContent(ws, "notes/hello.md", []byte("hi"))
if err != nil {
t.Fatalf("writeDocumentContent: %v", err)
}
if want := vault.ContentHash([]byte("hi")); hash != want {
t.Errorf("hash = %q, want %q", hash, want)
}
got, err := os.ReadFile(filepath.Join(ws, "notes", "hello.md"))
if err != nil {
t.Fatalf("read written file: %v", err)
}
if string(got) != "hi" {
t.Errorf("content = %q, want %q", got, "hi")
}
}
func TestWriteDocumentContent_EmptyContent(t *testing.T) {
h := &VaultHandler{}
ws := t.TempDir()
hash, err := h.writeDocumentContent(ws, "empty.md", []byte(""))
if err != nil {
t.Fatalf("writeDocumentContent: %v", err)
}
if want := vault.ContentHash([]byte("")); hash != want {
t.Errorf("hash = %q, want %q", hash, want)
}
fi, err := os.Stat(filepath.Join(ws, "empty.md"))
if err != nil {
t.Fatalf("stat empty file: %v", err)
}
if fi.Size() != 0 {
t.Errorf("size = %d, want 0", fi.Size())
}
}
func TestWriteDocumentContent_RejectsParentEscape(t *testing.T) {
h := &VaultHandler{}
ws := t.TempDir()
_, err := h.writeDocumentContent(ws, "../escape.md", []byte("x"))
if !errors.Is(err, os.ErrInvalid) {
t.Fatalf("err = %v, want os.ErrInvalid", err)
}
if _, statErr := os.Stat(filepath.Join(filepath.Dir(ws), "escape.md")); statErr == nil {
t.Error("file escaped the workspace via ../")
}
}
// A symlink that lives inside the workspace but points outside it must not be
// followed — this is the exact escape the lexical prefix check missed.
func TestWriteDocumentContent_RejectsSymlinkDirEscape(t *testing.T) {
h := &VaultHandler{}
ws := t.TempDir()
outside := t.TempDir()
if err := os.Symlink(outside, filepath.Join(ws, "link")); err != nil {
t.Fatalf("symlink: %v", err)
}
_, err := h.writeDocumentContent(ws, "link/pwn.md", []byte("x"))
if !errors.Is(err, os.ErrInvalid) {
t.Fatalf("err = %v, want os.ErrInvalid", err)
}
if _, statErr := os.Stat(filepath.Join(outside, "pwn.md")); statErr == nil {
t.Error("file written outside workspace through symlinked directory")
}
}
// A symlink planted at the final path component must be refused rather than
// followed, so an attacker can't redirect a write onto an existing outside file.
func TestWriteDocumentContent_RejectsFinalComponentSymlink(t *testing.T) {
h := &VaultHandler{}
ws := t.TempDir()
outside := t.TempDir()
outsideFile := filepath.Join(outside, "target.md")
if err := os.WriteFile(outsideFile, []byte("original"), 0o644); err != nil {
t.Fatalf("seed outside file: %v", err)
}
if err := os.Symlink(outsideFile, filepath.Join(ws, "evil.md")); err != nil {
t.Fatalf("symlink: %v", err)
}
_, err := h.writeDocumentContent(ws, "evil.md", []byte("pwned"))
if !errors.Is(err, os.ErrInvalid) {
t.Fatalf("err = %v, want os.ErrInvalid", err)
}
got, _ := os.ReadFile(outsideFile)
if string(got) != "original" {
t.Errorf("outside file clobbered through final-component symlink: %q", got)
}
}
// ---- handler-level: extension whitelist, event ordering, empty PUT, orphan ----
// fakeVaultStore embeds the interface (unimplemented methods panic if hit) and
// overrides only the two methods the content path touches.
type fakeVaultStore struct {
store.VaultStore
docs map[string]*store.VaultDocument
upserted bool
upsertErr error
upsertCall int
}
func newFakeVaultStore() *fakeVaultStore {
return &fakeVaultStore{docs: map[string]*store.VaultDocument{}}
}
func (f *fakeVaultStore) UpsertDocument(_ context.Context, doc *store.VaultDocument) error {
f.upsertCall++
if f.upsertErr != nil {
return f.upsertErr
}
if doc.ID == "" {
doc.ID = "doc-" + uuid.NewString()
}
f.upserted = true
f.docs[doc.ID] = doc
return nil
}
func (f *fakeVaultStore) GetDocumentByID(_ context.Context, _ /*tenantID*/, id string) (*store.VaultDocument, error) {
return f.docs[id], nil
}
// fakeEventBus captures published events and records whether the DB upsert had
// already happened at publish time, to assert ordering.
type fakeEventBus struct {
store *fakeVaultStore
published []eventbus.DomainEvent
upsertedAtFire bool
}
func (b *fakeEventBus) Publish(e eventbus.DomainEvent) {
if b.store != nil {
b.upsertedAtFire = b.store.upserted
}
b.published = append(b.published, e)
}
func (b *fakeEventBus) Subscribe(eventbus.EventType, eventbus.DomainEventHandler) func() {
return func() {}
}
func (b *fakeEventBus) Start(context.Context) {}
func (b *fakeEventBus) Drain(time.Duration) error { return nil }
func newJSONRequest(t *testing.T, method, target string, body any) *http.Request {
t.Helper()
buf, err := json.Marshal(body)
if err != nil {
t.Fatalf("marshal body: %v", err)
}
req := httptest.NewRequest(method, target, bytes.NewReader(buf))
req.Header.Set("Content-Type", "application/json")
return req.WithContext(masterCtx())
}
func TestHandleCreateDocument_RejectsDisallowedExtension(t *testing.T) {
h := &VaultHandler{} // extension check fires before any store/workspace access
req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{
"path": "payload.exe",
"title": "x",
"content": "data",
})
rr := httptest.NewRecorder()
h.handleCreateDocument(rr, req)
assertBadRequest(t, rr, "unsupported file type")
}
func TestHandleCreateDocument_WritesContentAndPublishesAfterUpsert(t *testing.T) {
st := newFakeVaultStore()
bus := &fakeEventBus{store: st}
ws := t.TempDir()
h := &VaultHandler{store: st, eventBus: bus, workspace: ws}
req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{
"path": "decisions/adr-001.md",
"title": "ADR 001",
"content": "# Decision\nuse postgres",
})
rr := httptest.NewRecorder()
h.handleCreateDocument(rr, req)
if rr.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201 (body: %s)", rr.Code, rr.Body.String())
}
// File materialised on disk.
got, err := os.ReadFile(filepath.Join(ws, "decisions", "adr-001.md"))
if err != nil {
t.Fatalf("read written doc: %v", err)
}
if string(got) != "# Decision\nuse postgres" {
t.Errorf("content = %q", got)
}
// Exactly one enrichment event, fired AFTER the DB upsert.
if len(bus.published) != 1 {
t.Fatalf("published %d events, want 1", len(bus.published))
}
if !bus.upsertedAtFire {
t.Error("event published before DB upsert — enrichment worker could race ahead of the row")
}
if bus.published[0].Type != eventbus.EventVaultDocUpserted {
t.Errorf("event type = %v, want %v", bus.published[0].Type, eventbus.EventVaultDocUpserted)
}
}
func TestHandleCreateDocument_NoContentSkipsWriteAndEvent(t *testing.T) {
st := newFakeVaultStore()
bus := &fakeEventBus{store: st}
ws := t.TempDir()
h := &VaultHandler{store: st, eventBus: bus, workspace: ws}
req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{
"path": "meta-only.md",
"title": "Meta only",
})
rr := httptest.NewRecorder()
h.handleCreateDocument(rr, req)
if rr.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201 (body: %s)", rr.Code, rr.Body.String())
}
if _, statErr := os.Stat(filepath.Join(ws, "meta-only.md")); statErr == nil {
t.Error("metadata-only create must not write a file")
}
if len(bus.published) != 0 {
t.Errorf("metadata-only create published %d events, want 0", len(bus.published))
}
}
// Documents the orphan-file behaviour: when the DB upsert fails after the bytes
// are already on disk, the handler returns 500 and the file is intentionally
// left in place (no rollback). If that ever changes, this test should change
// with it.
func TestHandleCreateDocument_UpsertFailureLeavesFile(t *testing.T) {
st := newFakeVaultStore()
st.upsertErr = errors.New("db down")
bus := &fakeEventBus{store: st}
ws := t.TempDir()
h := &VaultHandler{store: st, eventBus: bus, workspace: ws}
req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{
"path": "orphan.md",
"title": "Orphan",
"content": "written before upsert",
})
rr := httptest.NewRecorder()
h.handleCreateDocument(rr, req)
if rr.Code != http.StatusInternalServerError {
t.Fatalf("status = %d, want 500 (body: %s)", rr.Code, rr.Body.String())
}
if _, statErr := os.Stat(filepath.Join(ws, "orphan.md")); statErr != nil {
t.Errorf("file should remain after upsert failure (orphan accepted): %v", statErr)
}
if len(bus.published) != 0 {
t.Errorf("no event should fire when upsert fails, got %d", len(bus.published))
}
}
func TestHandleUpdateDocument_EmptyContentClearsFile(t *testing.T) {
st := newFakeVaultStore()
st.docs["existing-1"] = &store.VaultDocument{
ID: "existing-1",
TenantID: masterTenant.String(),
Path: "notes/keep.md",
Title: "Keep",
DocType: "note",
Scope: "shared",
}
bus := &fakeEventBus{store: st}
ws := t.TempDir()
h := &VaultHandler{store: st, eventBus: bus, workspace: ws}
empty := ""
req := newJSONRequest(t, http.MethodPut, "/v1/vault/documents/existing-1", map[string]any{
"content": empty,
})
req.SetPathValue("docID", "existing-1")
rr := httptest.NewRecorder()
h.handleUpdateDocument(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("status = %d, want 200 (body: %s)", rr.Code, rr.Body.String())
}
fi, err := os.Stat(filepath.Join(ws, "notes", "keep.md"))
if err != nil {
t.Fatalf("stat cleared file: %v", err)
}
if fi.Size() != 0 {
t.Errorf("file size = %d, want 0 (empty content clears file)", fi.Size())
}
if st.docs["existing-1"].ContentHash != vault.ContentHash([]byte("")) {
t.Errorf("content_hash = %q, want empty-content hash", st.docs["existing-1"].ContentHash)
}
if len(bus.published) != 1 {
t.Errorf("published %d events, want 1", len(bus.published))
}
}
// Locks the "no change" contract on PUT: omitting `content` (nil pointer in
// the body struct) must not touch the disk or fire an enrichment event, even
// when other metadata fields are present.
func TestHandleUpdateDocument_NilContentSkipsWriteAndEvent(t *testing.T) {
st := newFakeVaultStore()
st.docs["existing-2"] = &store.VaultDocument{
ID: "existing-2",
TenantID: masterTenant.String(),
Path: "notes/keep.md",
Title: "Old",
DocType: "note",
Scope: "shared",
ContentHash: "preexisting-hash",
}
bus := &fakeEventBus{store: st}
ws := t.TempDir()
h := &VaultHandler{store: st, eventBus: bus, workspace: ws}
// content field intentionally omitted → *string is nil → no write, no event.
req := newJSONRequest(t, http.MethodPut, "/v1/vault/documents/existing-2", map[string]any{
"title": "Renamed but content untouched",
})
req.SetPathValue("docID", "existing-2")
rr := httptest.NewRecorder()
h.handleUpdateDocument(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("status = %d, want 200 (body: %s)", rr.Code, rr.Body.String())
}
if _, statErr := os.Stat(filepath.Join(ws, "notes", "keep.md")); statErr == nil {
t.Error("nil content must not materialise a file")
}
if st.docs["existing-2"].ContentHash != "preexisting-hash" {
t.Errorf("content_hash = %q, want unchanged 'preexisting-hash'", st.docs["existing-2"].ContentHash)
}
if len(bus.published) != 0 {
t.Errorf("nil-content PUT must not publish events, got %d", len(bus.published))
}
}
// Handler-level guard at vault_handler_documents.go path validation must reject
// "..", short-circuiting before writeDocumentContent even runs.
func TestHandleCreateDocument_RejectsParentTraversalPath(t *testing.T) {
h := &VaultHandler{} // path check runs before any store/workspace access
req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{
"path": "../../etc/passwd.md",
"title": "x",
"content": "evil",
})
rr := httptest.NewRecorder()
h.handleCreateDocument(rr, req)
assertBadRequest(t, rr, "invalid path")
}
// Mirrors PUT semantics on POST: `content: ""` writes a 0-byte file and
// fires the enrichment event (the I4 fix that aligned POST/PUT).
func TestHandleCreateDocument_EmptyContentWritesEmptyFile(t *testing.T) {
st := newFakeVaultStore()
bus := &fakeEventBus{store: st}
ws := t.TempDir()
h := &VaultHandler{store: st, eventBus: bus, workspace: ws}
req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{
"path": "placeholder.md",
"title": "Placeholder",
"content": "",
})
rr := httptest.NewRecorder()
h.handleCreateDocument(rr, req)
if rr.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201 (body: %s)", rr.Code, rr.Body.String())
}
fi, err := os.Stat(filepath.Join(ws, "placeholder.md"))
if err != nil {
t.Fatalf("stat empty file: %v", err)
}
if fi.Size() != 0 {
t.Errorf("size = %d, want 0", fi.Size())
}
if len(bus.published) != 1 {
t.Errorf("published %d events, want 1", len(bus.published))
}
}
// B1 regression: non-master tenants resolve to workspace/tenants/{slug}/ which
// is created on demand. The very first JSON content write into a fresh tenant
// must NOT 500 because EvalSymlinks can't find that dir — writeDocumentContent
// is responsible for creating it (mirrors what handleUpload does).
func TestHandleCreateDocument_NonMasterTenantFirstContentWrite(t *testing.T) {
st := newFakeVaultStore()
bus := &fakeEventBus{store: st}
ws := t.TempDir()
h := &VaultHandler{store: st, eventBus: bus, workspace: ws}
nonMaster := uuid.MustParse("01999999-1111-7000-8000-000000000abc")
ctx := store.WithTenantSlug(store.WithTenantID(context.Background(), nonMaster), "acme")
// newJSONRequest builds against masterCtx by default; WithContext swaps in
// the non-master tenant context for this single test.
req := newJSONRequest(t, http.MethodPost, "/v1/vault/documents", map[string]any{
"path": "notes/first.md",
"title": "First",
"content": "hello from acme",
}).WithContext(ctx)
rr := httptest.NewRecorder()
h.handleCreateDocument(rr, req)
if rr.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201 (body: %s)", rr.Code, rr.Body.String())
}
tenantDir := filepath.Join(ws, "tenants", "acme")
got, err := os.ReadFile(filepath.Join(tenantDir, "notes", "first.md"))
if err != nil {
t.Fatalf("read non-master tenant doc: %v", err)
}
if string(got) != "hello from acme" {
t.Errorf("content = %q", got)
}
if len(bus.published) != 1 {
t.Errorf("published %d events, want 1", len(bus.published))
}
}
+3 -1
View File
@@ -49,6 +49,8 @@ type storeGrantChecker struct {
cache sync.Map // map[cacheKey]*cacheEntry
}
const grantCheckerCacheSubscriberID = bus.TopicCacheMCP + ":grant_checker"
// NewStoreGrantChecker creates a GrantChecker backed by the MCP store.
// Subscribes to TopicCacheMCP for cache invalidation on grant changes.
func NewStoreGrantChecker(mcpStore store.MCPServerStore, msgBus *bus.MessageBus) *storeGrantChecker {
@@ -59,7 +61,7 @@ func NewStoreGrantChecker(mcpStore store.MCPServerStore, msgBus *bus.MessageBus)
// Subscribe to cache invalidation events.
// When MCP grants are revoked/modified, the HTTP handler broadcasts to this topic.
if msgBus != nil {
msgBus.Subscribe(bus.TopicCacheMCP, func(event bus.Event) {
msgBus.Subscribe(grantCheckerCacheSubscriberID, func(event bus.Event) {
if event.Name == protocol.EventCacheInvalidate {
gc.cache.Clear()
slog.Debug("mcp.grant_checker.cache_cleared", "trigger", "bus_event")
+60 -2
View File
@@ -6,7 +6,9 @@ import (
"testing"
"github.com/google/uuid"
"github.com/nextlevelbuilder/goclaw/internal/bus"
"github.com/nextlevelbuilder/goclaw/internal/store"
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
)
// mockMCPStore implements store.MCPServerStore for testing.
@@ -28,11 +30,13 @@ func (m *mockMCPStore) GetServer(ctx context.Context, id uuid.UUID) (*store.MCPS
func (m *mockMCPStore) GetServerByName(ctx context.Context, name string) (*store.MCPServerData, error) {
return nil, nil
}
func (m *mockMCPStore) ListServers(ctx context.Context) ([]store.MCPServerData, error) { return nil, nil }
func (m *mockMCPStore) ListServers(ctx context.Context) ([]store.MCPServerData, error) {
return nil, nil
}
func (m *mockMCPStore) UpdateServer(ctx context.Context, id uuid.UUID, updates map[string]any) error {
return nil
}
func (m *mockMCPStore) DeleteServer(ctx context.Context, id uuid.UUID) error { return nil }
func (m *mockMCPStore) DeleteServer(ctx context.Context, id uuid.UUID) error { return nil }
func (m *mockMCPStore) GrantToAgent(ctx context.Context, g *store.MCPAgentGrant) error { return nil }
func (m *mockMCPStore) RevokeFromAgent(ctx context.Context, serverID, agentID uuid.UUID) error {
return nil
@@ -132,6 +136,60 @@ func TestStoreGrantChecker_InvalidateClearsCache(t *testing.T) {
}
}
func TestStoreGrantChecker_BusInvalidationSurvivesOtherMCPSubscribers(t *testing.T) {
initialServerID := uuid.New()
newServerID := uuid.New()
agentID := uuid.New()
userID := "system"
mockStore := &mockMCPStore{
accessible: []store.MCPAccessInfo{
{Server: store.MCPServerData{BaseModel: store.BaseModel{ID: initialServerID}, Name: "initial-server"}},
},
}
msgBus := bus.New()
gc := NewStoreGrantChecker(mockStore, msgBus)
ctx := context.Background()
if allowed, reason := gc.IsAllowed(ctx, agentID, userID, initialServerID, "tool"); !allowed {
t.Fatalf("expected initial server allowed, reason=%q", reason)
}
if mockStore.callCount != 1 {
t.Fatalf("expected 1 DB call after warm cache, got %d", mockStore.callCount)
}
// The gateway also subscribes to MCP cache events. Subscriber IDs must be
// unique; otherwise this later subscription replaces the grant checker and
// its stale cache survives grant changes.
otherSubscriberCalled := false
msgBus.Subscribe(bus.TopicCacheMCP, func(event bus.Event) {
if event.Name == protocol.EventCacheInvalidate {
otherSubscriberCalled = true
}
})
mockStore.accessible = []store.MCPAccessInfo{
{Server: store.MCPServerData{BaseModel: store.BaseModel{ID: initialServerID}, Name: "initial-server"}},
{Server: store.MCPServerData{BaseModel: store.BaseModel{ID: newServerID}, Name: "new-server"}},
}
msgBus.Broadcast(bus.Event{
Name: protocol.EventCacheInvalidate,
Payload: bus.CacheInvalidatePayload{Kind: bus.CacheKindMCP},
})
if !otherSubscriberCalled {
t.Fatal("expected other MCP subscriber to receive invalidation")
}
if allowed, reason := gc.IsAllowed(ctx, agentID, userID, newServerID, "tool"); !allowed {
t.Fatalf("expected grant checker cache to refresh after bus invalidation, reason=%q", reason)
}
if mockStore.callCount != 2 {
t.Fatalf("expected cache miss after bus invalidation, got callCount=%d", mockStore.callCount)
}
}
func TestStoreGrantChecker_DenyReasonSurfaced(t *testing.T) {
serverID := uuid.New()
agentID := uuid.New()
@@ -0,0 +1,131 @@
package pipeline
import (
"context"
"errors"
"testing"
"github.com/nextlevelbuilder/goclaw/internal/providers"
)
// TestPruneStage_Compaction_PreservesPending is the regression guard for the
// mid-loop compaction tool-pairing bug. ThinkStage appends the current
// iteration's assistant(tool_calls) message to the pending buffer, then
// PruneStage runs next. If compaction fires, CompactMessages only receives
// History() — pending is NOT part of the compaction input — yet ReplaceHistory
// clears pending. Without preservation, the assistant(tool_calls) message is
// dropped, ToolStage later appends orphaned tool_result messages, and the next
// provider call rejects the unpaired tool_result (OpenAI/DeepSeek 400).
//
// This test asserts the pending assistant(tool_calls) message survives a
// mid-loop compaction so tool_calls -> tool_result pairing stays intact.
func TestPruneStage_Compaction_PreservesPending(t *testing.T) {
t.Parallel()
// budget = ContextWindow(1000) - overhead(0) - MaxTokens(100) = 900.
// History of 50 msgs * 100 tokens = 5000 > 900 forces the compaction path.
deps := &PipelineDeps{
Config: PipelineConfig{
ContextWindow: 1000,
MaxTokens: 100,
},
TokenCounter: &mockTokenCounter{countPerMessage: 100},
PruneMessages: func(msgs []providers.Message, _ int) ([]providers.Message, PruneStats) {
return msgs, PruneStats{} // no reduction — forces fall-through to compaction
},
CompactMessages: func(_ context.Context, msgs []providers.Message, _ string) ([]providers.Message, error) {
// Compaction summarizes history into a single message. Crucially it
// only ever sees History(), never the pending assistant(tool_calls).
return []providers.Message{{Role: "user", Content: "[compacted summary]"}}, nil
},
}
stage := NewPruneStage(deps, nil)
state := defaultState()
history := make([]providers.Message, 50)
for i := range history {
history[i] = providers.Message{Role: "user", Content: "msg"}
}
state.Messages.SetHistory(history)
// The in-flight assistant(tool_calls) message ThinkStage placed in pending
// this iteration — it must outlive ReplaceHistory.
assistantToolCall := providers.Message{
Role: "assistant",
ToolCalls: []providers.ToolCall{{ID: "tc-keep", Name: "read_file"}},
}
state.Messages.AppendPending(assistantToolCall)
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
if !state.Prune.MidLoopCompacted {
t.Fatal("MidLoopCompacted = false, want true (compaction should have fired)")
}
if state.Compact.CompactionCount != 1 {
t.Errorf("CompactionCount = %d, want 1", state.Compact.CompactionCount)
}
// The assistant(tool_calls) message must still be pending after compaction.
pending := state.Messages.Pending()
if len(pending) != 1 {
t.Fatalf("Pending() len = %d, want 1 (assistant tool_calls preserved across ReplaceHistory)", len(pending))
}
if pending[0].Role != "assistant" || len(pending[0].ToolCalls) != 1 || pending[0].ToolCalls[0].ID != "tc-keep" {
t.Errorf("pending[0] = %+v, want the preserved assistant tool_calls message (id tc-keep)", pending[0])
}
// History should be the compacted summary, not carrying a duplicate of the
// preserved pending message.
hist := state.Messages.History()
if len(hist) != 1 || hist[0].Content != "[compacted summary]" {
t.Errorf("History() = %+v, want single compacted summary message", hist)
}
}
// TestThinkStage_EmergencyCompaction_PreservesPending guards the second
// ReplaceHistory call site. When CallLLM reports a context-overflow error,
// ThinkStage runs an emergency compaction and retries the iteration. Pending
// messages already staged this iteration (e.g. iteration-budget nudges from
// maybeInjectNudge) must survive the ReplaceHistory that emergency compaction
// performs, for the same reason as the prune path.
func TestThinkStage_EmergencyCompaction_PreservesPending(t *testing.T) {
t.Parallel()
deps := &PipelineDeps{
Config: PipelineConfig{MaxIterations: 10, MaxTokens: 1000},
CallLLM: func(_ context.Context, _ *RunState, _ providers.ChatRequest) (*providers.ChatResponse, error) {
// "context length exceeded" is matched by IsContextOverflowMessage.
return nil, errors.New("context length exceeded for this request")
},
CompactMessages: func(_ context.Context, msgs []providers.Message, _ string) ([]providers.Message, error) {
return []providers.Message{{Role: "user", Content: "[compacted summary]"}}, nil
},
}
stage := NewThinkStage(deps)
state := defaultState()
state.Messages.SetHistory([]providers.Message{{Role: "user", Content: "old turn"}})
// A pending message staged earlier in this iteration (an iteration nudge).
nudge := providers.Message{Role: "user", Content: "[System] wrap up your work"}
state.Messages.AppendPending(nudge)
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute() error: %v", err)
}
// Emergency compaction retries the iteration (Continue) and records one retry.
if stage.Result() != Continue {
t.Errorf("Result() = %v, want Continue (retry after emergency compaction)", stage.Result())
}
if state.Think.OverflowRetries != 1 {
t.Errorf("OverflowRetries = %d, want 1", state.Think.OverflowRetries)
}
pending := state.Messages.Pending()
if len(pending) != 1 {
t.Fatalf("Pending() len = %d, want 1 (nudge preserved across emergency ReplaceHistory)", len(pending))
}
if pending[0].Content != "[System] wrap up your work" {
t.Errorf("pending[0] = %+v, want the preserved nudge message", pending[0])
}
}
+3
View File
@@ -90,6 +90,9 @@ type PipelineDeps struct {
ExecuteToolRaw func(ctx context.Context, tc providers.ToolCall) (providers.Message, any, error)
// ProcessToolResult processes a raw tool result with state mutation (sequential only).
ProcessToolResult func(ctx context.Context, state *RunState, tc providers.ToolCall, rawMsg providers.Message, rawData any) []providers.Message
// AuthorizeToolCall validates whether a tool call is allowed to execute.
// Used by ToolStage as a runtime guard against out-of-policy tool calls.
AuthorizeToolCall func(ctx context.Context, state *RunState, tc providers.ToolCall) (bool, string)
// SequentialToolCall returns true for tools that must preserve same-response order.
// When any tool call in a batch matches, ToolStage uses ExecuteToolCall for the
// whole batch instead of parallel raw execution.
+10
View File
@@ -180,11 +180,21 @@ func (s *PruneStage) Execute(ctx context.Context, state *RunState) error {
return nil // no compaction available
}
// Save pending before compaction — ReplaceHistory clears pending,
// but pending may contain the current iteration's assistant(tool_calls)
// message which must be preserved to maintain tool_calls → tool pairing.
savedPending := state.Messages.Pending()
compacted, err := s.deps.CompactMessages(ctx, state.Messages.History(), state.Model)
if err != nil {
return fmt.Errorf("compact messages: %w", err)
}
state.Messages.ReplaceHistory(compacted)
// Restore pending messages that were cleared by ReplaceHistory.
for _, msg := range savedPending {
state.Messages.AppendPending(msg)
}
state.Prune.MidLoopCompacted = true
state.Compact.CompactionCount++
state.Compact.MemoryFlushedThisCycle = false // reset for next cycle
+3
View File
@@ -52,6 +52,9 @@ type PruneState struct {
// ToolState: owned by ToolStage.
type ToolState struct {
// AllowedTools is the per-iteration execution allowlist built from tool
// definitions sent to the provider. Nil means "no runtime restriction".
AllowedTools map[string]bool
LoopDetector any // concrete type toolLoopState lives in agent; Phase 5 defines LoopDetector interface
TotalToolCalls int
AsyncToolCalls []string // tool names that executed async (spawn)
+11
View File
@@ -44,6 +44,13 @@ func (s *ThinkStage) Execute(ctx context.Context, state *RunState) error {
if err != nil {
return fmt.Errorf("build tools: %w", err)
}
allowed := make(map[string]bool, len(toolDefs))
for _, td := range toolDefs {
allowed[td.Function.Name] = true
}
state.Tool.AllowedTools = allowed
} else {
state.Tool.AllowedTools = nil
}
// 3. Construct ChatRequest
@@ -158,12 +165,16 @@ func (s *ThinkStage) tryEmergencyCompaction(ctx context.Context, state *RunState
}
originalLen := len(state.Messages.History())
savedPending := state.Messages.Pending()
compacted, compactErr := s.deps.CompactMessages(ctx, state.Messages.History(), state.Model)
if compactErr != nil {
slog.Warn("emergency_compaction_failed", "reason", reason, "error", compactErr)
return false
}
state.Messages.ReplaceHistory(compacted)
for _, msg := range savedPending {
state.Messages.AppendPending(msg)
}
slog.Info("emergency_compaction_triggered",
"run_id", state.RunID,
"reason", reason,
+10
View File
@@ -149,6 +149,16 @@ func (s *ToolStage) preflightToolCalls(ctx context.Context, state *RunState, too
}
func (s *ToolStage) preflightToolCall(ctx context.Context, state *RunState, tc providers.ToolCall) (providers.ToolCall, *providers.Message) {
if s.deps.AuthorizeToolCall != nil {
if ok, reason := s.deps.AuthorizeToolCall(ctx, state, tc); !ok {
return tc, &providers.Message{
Role: "tool",
Content: reason,
ToolCallID: tc.ID,
IsError: true,
}
}
}
r, _ := s.deps.FireHook(ctx, hooks.Event{
EventID: uuid.NewString(),
SessionID: state.Input.SessionKey,
@@ -0,0 +1,216 @@
package pipeline
import (
"context"
"testing"
"github.com/nextlevelbuilder/goclaw/internal/providers"
)
// makeAuthorizeGate builds a PipelineDeps.AuthorizeToolCall callback backed by
// a static allowlist and an optional deferred-activator, mirroring the contract
// that makeAuthorizeToolCall in loop_pipeline_callbacks.go provides.
func makeAuthorizeGate(
allowed map[string]bool, // nil = allow-all
tryActivate func(name string) bool, // nil = no activator
isDenied func(name string) bool, // nil = never denied
) func(ctx context.Context, state *RunState, tc providers.ToolCall) (bool, string) {
return func(_ context.Context, state *RunState, tc providers.ToolCall) (bool, string) {
a := state.Tool.AllowedTools
if a == nil {
return true, ""
}
if a[tc.Name] {
return true, ""
}
if tryActivate != nil && tryActivate(tc.Name) {
if isDenied != nil && isDenied(tc.Name) {
return false, "tool not allowed by policy: " + tc.Name
}
a[tc.Name] = true
return true, ""
}
return false, "tool not allowed by policy: " + tc.Name
}
}
// makeMinimalDeps returns a PipelineDeps just sufficient for ToolStage tests.
func makeMinimalDeps(authorize func(context.Context, *RunState, providers.ToolCall) (bool, string)) *PipelineDeps {
return &PipelineDeps{
Config: PipelineConfig{MaxToolCalls: 100},
AuthorizeToolCall: authorize,
ExecuteToolCall: func(_ context.Context, state *RunState, tc providers.ToolCall) ([]providers.Message, error) {
return []providers.Message{{Role: "tool", Content: "ok", ToolCallID: tc.ID}}, nil
},
}
}
// stateWithAllowedTools returns a minimal RunState with AllowedTools pre-set.
func stateWithAllowedTools(allowed map[string]bool) *RunState {
st := defaultState()
st.Tool.AllowedTools = allowed
return st
}
func toolCallFor(name string) providers.ToolCall {
return providers.ToolCall{ID: "tc-" + name, Name: name}
}
// fakeResponse puts tool calls into the state's ThinkStage so ToolStage finds them.
func setLastResponse(state *RunState, calls []providers.ToolCall) {
state.Think.LastResponse = &providers.ChatResponse{ToolCalls: calls}
}
// --- Tests ---
// TestAuthorizeGate_NilAllowlist_AllowsAll verifies that a nil AllowedTools map
// means no per-iteration restriction (allow-all semantics).
func TestAuthorizeGate_NilAllowlist_AllowsAll(t *testing.T) {
t.Parallel()
st := defaultState()
st.Tool.AllowedTools = nil // explicit nil
gate := makeAuthorizeGate(nil, nil, nil)
tc := toolCallFor("exec")
ok, reason := gate(context.Background(), st, tc)
if !ok {
t.Errorf("nil allowlist must allow all tools; got blocked with reason %q", reason)
}
}
// TestAuthorizeGate_NamePresent_Allows checks that a tool explicitly in the
// allowlist passes through.
func TestAuthorizeGate_NamePresent_Allows(t *testing.T) {
t.Parallel()
st := stateWithAllowedTools(map[string]bool{"exec": true, "read_file": true})
gate := makeAuthorizeGate(nil, nil, nil)
for _, name := range []string{"exec", "read_file"} {
ok, reason := gate(context.Background(), st, toolCallFor(name))
if !ok {
t.Errorf("tool %q should be allowed; got blocked: %q", name, reason)
}
}
}
// TestAuthorizeGate_NameAbsent_NoActivator_Denies confirms fail-closed: a name
// not in the allowlist with no activator is blocked.
func TestAuthorizeGate_NameAbsent_NoActivator_Denies(t *testing.T) {
t.Parallel()
st := stateWithAllowedTools(map[string]bool{"read_file": true})
gate := makeAuthorizeGate(nil, nil, nil)
ok, reason := gate(context.Background(), st, toolCallFor("exec"))
if ok {
t.Error("tool absent from allowlist with no activator must be denied")
}
if reason == "" {
t.Error("denial must include a non-empty reason")
}
}
// TestAuthorizeGate_DeferredActivate_ThenDenied ensures that lazy activation
// followed by a positive IsDenied check still blocks the tool.
func TestAuthorizeGate_DeferredActivate_ThenDenied(t *testing.T) {
t.Parallel()
st := stateWithAllowedTools(map[string]bool{})
activated := false
tryActivate := func(name string) bool {
activated = true
return name == "mcp_svc__exec_cmd"
}
isDenied := func(name string) bool {
return name == "mcp_svc__exec_cmd" // explicitly denied
}
gate := makeAuthorizeGate(nil, tryActivate, isDenied)
ok, reason := gate(context.Background(), st, toolCallFor("mcp_svc__exec_cmd"))
if !activated {
t.Error("expected TryActivateDeferred to be called")
}
if ok {
t.Error("tool allowed by activator but denied by policy must still be blocked")
}
if reason == "" {
t.Error("denial must include a non-empty reason")
}
if st.Tool.AllowedTools["mcp_svc__exec_cmd"] {
t.Error("denied tool must not be added to AllowedTools")
}
}
// TestAuthorizeGate_PrefixedName_CanonicalLookup is the regression guard for the
// toolCallPrefix bug: when an agent has toolCallPrefix set, the model emits
// "proxy_exec" but AllowedTools is keyed by canonical name "exec". The gate must
// resolve the name before lookup, otherwise every prefixed call is wrongly blocked.
//
// This test simulates the resolution by calling the gate with a *pre-resolved*
// canonical name, matching what makeAuthorizeToolCall does after calling
// resolveToolCallName(tc.Name). The pipeline layer always receives the resolved
// name because AuthorizeToolCall in the real callback operates on the canonical
// form.
func TestAuthorizeGate_PrefixedName_CanonicalLookup(t *testing.T) {
t.Parallel()
// AllowedTools uses canonical name "exec" (as ThinkStage builds it from FilterTools).
st := stateWithAllowedTools(map[string]bool{"exec": true})
gate := makeAuthorizeGate(nil, nil, nil)
// Simulate what resolveToolCallName("proxy_exec") returns with prefix "proxy_":
// the canonical name "exec" is passed to the gate.
resolvedName := "exec" // prefix already stripped by makeAuthorizeToolCall
tc := providers.ToolCall{ID: "tc-prefix", Name: resolvedName}
ok, reason := gate(context.Background(), st, tc)
if !ok {
t.Errorf("canonical name after prefix resolution must be allowed; reason: %q", reason)
}
}
// TestToolStage_AllBlocked_CallsCheckExitConditions verifies that when all tool
// calls in the parallel path are blocked, ToolStage still runs checkExitConditions
// (so MaxToolCalls budget is enforced). Regression for early-return-nil bug.
func TestToolStage_AllBlocked_CallsCheckExitConditions(t *testing.T) {
t.Parallel()
// Allow-all deps but with MaxToolCalls = 1 and AuthorizeToolCall that blocks.
denyAll := func(_ context.Context, _ *RunState, tc providers.ToolCall) (bool, string) {
return false, "blocked: " + tc.Name
}
deps := &PipelineDeps{
Config: PipelineConfig{MaxToolCalls: 1},
AuthorizeToolCall: denyAll,
ExecuteToolCall: func(_ context.Context, state *RunState, tc providers.ToolCall) ([]providers.Message, error) {
return nil, nil
},
ExecuteToolRaw: func(_ context.Context, tc providers.ToolCall) (providers.Message, any, error) {
return providers.Message{}, nil, nil
},
ProcessToolResult: func(_ context.Context, state *RunState, tc providers.ToolCall, rawMsg providers.Message, rawData any) []providers.Message {
return nil
},
}
stage := NewToolStage(deps)
state := defaultState()
state.Tool.AllowedTools = map[string]bool{} // non-nil allowlist → gate is active
state.Tool.TotalToolCalls = 1 // already at MaxToolCalls
// Two tool calls — parallel path is triggered (len > 1).
setLastResponse(state, []providers.ToolCall{
toolCallFor("exec"),
toolCallFor("write_file"),
})
if err := stage.Execute(context.Background(), state); err != nil {
t.Fatalf("Execute returned error: %v", err)
}
// checkExitConditions must have fired: TotalToolCalls >= MaxToolCalls → BreakLoop.
if stage.Result() != BreakLoop {
t.Errorf("expected BreakLoop after all-blocked + MaxToolCalls reached, got %v", stage.Result())
}
}
@@ -150,6 +150,48 @@ func TestAnthropicAdapterToRequest_Thinking(t *testing.T) {
}
}
func TestAnthropicAdapterToRequest_SkipsTemperatureForClaude46(t *testing.T) {
adapter, _ := NewAnthropicAdapter(ProviderConfig{APIKey: "sk-test"})
for _, model := range []string{"claude-opus-4-6", "claude-sonnet-4-6", "claude-opus-4-7-20260501"} {
t.Run(model, func(t *testing.T) {
req := ChatRequest{
Model: model,
Messages: []Message{{Role: "user", Content: "hi"}},
Options: map[string]any{OptTemperature: 0.7},
}
data, _, err := adapter.ToRequest(req)
if err != nil {
t.Fatal(err)
}
var body map[string]any
if err := json.Unmarshal(data, &body); err != nil {
t.Fatal(err)
}
if _, hasTemp := body["temperature"]; hasTemp {
t.Errorf("model %q: temperature should be omitted", model)
}
})
}
req := ChatRequest{
Model: "claude-sonnet-4-5-20250929",
Messages: []Message{{Role: "user", Content: "hi"}},
Options: map[string]any{OptTemperature: 0.7},
}
data, _, err := adapter.ToRequest(req)
if err != nil {
t.Fatal(err)
}
var body map[string]any
if err := json.Unmarshal(data, &body); err != nil {
t.Fatal(err)
}
if body["temperature"] != 0.7 {
t.Errorf("sonnet 4.5 should keep temperature, got %v", body["temperature"])
}
}
func TestAnthropicAdapterFromResponse_ToolCalls(t *testing.T) {
adapter, _ := NewAnthropicAdapter(ProviderConfig{APIKey: "sk-test"})
+27 -1
View File
@@ -213,7 +213,9 @@ func (p *AnthropicProvider) buildRequestBody(model string, req ChatRequest, stre
body["max_tokens"] = v
}
if v, ok := req.Options[OptTemperature]; ok {
body["temperature"] = v
if !anthropicSkipsTemperature(model) {
body["temperature"] = v
}
}
// Enable extended thinking if thinking_level is set
@@ -234,6 +236,30 @@ func (p *AnthropicProvider) buildRequestBody(model string, req ChatRequest, stre
return body
}
// anthropicSkipsTemperature reports whether the Messages API rejects sampling
// parameters for this model. Claude Opus/Sonnet 4.6+ and Opus 4.7+ return HTTP
// 400 when temperature (and top_p/top_k) are included; omit them entirely.
func anthropicSkipsTemperature(model string) bool {
m := strings.ToLower(model)
for _, family := range []string{"claude-opus-4-", "claude-sonnet-4-"} {
after, ok := strings.CutPrefix(m, family)
if !ok {
continue
}
minor := 0
for _, c := range after {
if c < '0' || c > '9' {
break
}
minor = minor*10 + int(c-'0')
}
if minor >= 6 {
return true
}
}
return false
}
// anthropicThinkingBudget maps a thinking level to a token budget.
func anthropicThinkingBudget(level string) int {
switch level {
+5
View File
@@ -38,6 +38,11 @@ func NewDefaultTransport() *http.Transport {
// NewDefaultHTTPClient returns an *http.Client backed by NewDefaultTransport.
// No Client.Timeout is set — rely on ctx deadlines and Transport stage timeouts.
//
// SSRF protection for user-configured provider URLs is enforced at provider
// create/update time by validateProviderURL (resolves the host and rejects
// private/reserved IPs via security.IsBlocked). Dial-time DNS-rebinding
// hardening is tracked as a follow-up.
func NewDefaultHTTPClient() *http.Client {
return &http.Client{Transport: NewDefaultTransport()}
}
+5 -2
View File
@@ -40,8 +40,11 @@ func NewOpenAIEmbeddingProvider(apiKey, apiBase, model string) *OpenAIEmbeddingP
apiKey: apiKey,
apiBase: strings.TrimRight(apiBase, "/"),
model: model,
client: &http.Client{Timeout: 60 * time.Second},
retry: DefaultRetryConfig(),
// Use a fixed 60s timeout client. Embedding requests are short but can be
// batched. The apiBase is validated at provider-creation time by
// validateProviderURL in internal/http/providers.go.
client: &http.Client{Timeout: 60 * time.Second},
retry: DefaultRetryConfig(),
}
}
+5 -1
View File
@@ -131,11 +131,15 @@ func NewVertexProvider(ctx context.Context, cfg VertexConfig) (*OpenAIProvider,
// then transparently fetches a fresh one. No extra work for callers.
cached := oauth2.ReuseTokenSource(nil, tokenSource)
// Use NewDefaultTransport as the oauth2 base transport so Vertex requests
// inherit the same per-stage timeouts as other providers. SSRF defense is
// provided by validateVertexAPIBaseOverride, which enforces the googleapis.com
// host constraint at provider-creation time.
client := &http.Client{
Timeout: DefaultHTTPTimeout,
Transport: &oauth2.Transport{
Source: cached,
Base: http.DefaultTransport,
Base: NewDefaultTransport(),
},
}
-28
View File
@@ -172,31 +172,3 @@ func TestFsBridgePathWithinUsesPathBoundaries(t *testing.T) {
})
}
}
func TestFsBridgeWriteFileCommandPreservesOverwriteTruncation(t *testing.T) {
args := fsBridgeWriteDDArgs("/workspace/file.txt", false)
for _, arg := range args {
if arg == "conv=notrunc" || arg == "oflag=append" {
t.Fatalf("overwrite command must truncate, got append-only arg %q in %v", arg, args)
}
}
}
func TestFsBridgeWriteFileCommandUsesNoTruncOnlyForAppend(t *testing.T) {
args := fsBridgeWriteDDArgs("/workspace/file.txt", true)
if !containsString(args, "conv=notrunc") {
t.Fatalf("append command missing conv=notrunc: %v", args)
}
if !containsString(args, "oflag=append") {
t.Fatalf("append command missing oflag=append: %v", args)
}
}
func containsString(values []string, target string) bool {
for _, value := range values {
if value == target {
return true
}
}
return false
}
+10 -5
View File
@@ -80,8 +80,12 @@ func (b *FsBridge) WriteFile(ctx context.Context, path, content string, appendMo
}
}
ddArgs := fsBridgeWriteDDArgs(resolved, appendMode)
_, stderr, exitCode, err := b.dockerExec(ctx, []byte(content), ddArgs...)
// Write content via stdin without invoking a shell. Passing the resolved path
// as a discrete argv entry prevents shell metacharacters in filenames from
// being interpreted as commands inside the sandbox container.
// "--" terminates option parsing so paths starting with "-" are safe.
teeArgs := fsBridgeWriteTeeArgs(resolved, appendMode)
_, stderr, exitCode, err := b.dockerExec(ctx, []byte(content), teeArgs...)
if err != nil {
return fmt.Errorf("fsbridge write: %w", err)
}
@@ -92,11 +96,12 @@ func (b *FsBridge) WriteFile(ctx context.Context, path, content string, appendMo
return nil
}
func fsBridgeWriteDDArgs(resolved string, appendMode bool) []string {
args := []string{"dd", "bs=1048576", "status=none", "of=" + resolved}
func fsBridgeWriteTeeArgs(resolved string, appendMode bool) []string {
args := []string{"tee"}
if appendMode {
args = append(args, "conv=notrunc", "oflag=append")
args = append(args, "-a")
}
args = append(args, "--", resolved)
return args
}
+189
View File
@@ -0,0 +1,189 @@
package sandbox
import (
"context"
"os"
"path/filepath"
"strings"
"testing"
)
// fakeDockerScript is a fake `docker` binary used in WriteFile tests.
// It logs all argv to $DOCKER_LOG and, when invoked with a `tee` sub-command,
// captures stdin to $DOCKER_STDIN.
// `realpath -e -- <path>` echoes the path back (simulates existing path).
// `mkdir` exits 0 silently.
const fakeDockerScript = `#!/bin/sh
{
echo CALL
i=0
for arg in "$@"; do
echo "ARG[$i]=$arg"
i=$((i + 1))
done
} >> "$DOCKER_LOG"
# realpath: echo the last arg back as the resolved path
for j in $(seq 0 $#); do
eval "a=\${$j}"
if [ "$a" = "realpath" ]; then
# print the last argument (the path)
eval "echo \"\${$#}\""
exit 0
fi
done
# tee: capture stdin
for arg in "$@"; do
if [ "$arg" = "tee" ]; then
if [ -n "$DOCKER_STDIN" ]; then
cat > "$DOCKER_STDIN"
else
cat > /dev/null
fi
exit 0
fi
done
exit 0
`
// installFakeDocker writes the fake docker script to tmp, prepends tmp to PATH,
// and sets DOCKER_LOG and optionally DOCKER_STDIN env vars for the test.
func installFakeDocker(t *testing.T, tmp, logPath, stdinPath string) {
t.Helper()
dockerPath := filepath.Join(tmp, "docker")
if err := os.WriteFile(dockerPath, []byte(fakeDockerScript), 0o755); err != nil {
t.Fatalf("write fake docker: %v", err)
}
t.Setenv("PATH", tmp+string(os.PathListSeparator)+os.Getenv("PATH"))
t.Setenv("DOCKER_LOG", logPath)
if stdinPath != "" {
t.Setenv("DOCKER_STDIN", stdinPath)
}
}
// TestFsBridgeWriteFileDoesNotInvokeShell asserts that WriteFile passes the
// resolved path as a discrete argv entry to `tee -- <path>` and does NOT
// build a shell command string (no sh / -c / shell metachar expansion).
func TestFsBridgeWriteFileDoesNotInvokeShell(t *testing.T) {
tmp := t.TempDir()
logPath := filepath.Join(tmp, "docker.log")
stdinPath := filepath.Join(tmp, "stdin.txt")
installFakeDocker(t, tmp, logPath, stdinPath)
bridge := NewFsBridge("container-id", "/workspace")
maliciousPath := `nested/evil$(touch /tmp/goclaw-fsbridge-pwned);name.txt`
content := "safe content"
if err := bridge.WriteFile(context.Background(), maliciousPath, content, false); err != nil {
t.Fatalf("WriteFile returned error: %v", err)
}
logBytes, err := os.ReadFile(logPath)
if err != nil {
t.Fatalf("read docker log: %v", err)
}
log := string(logBytes)
// Must NOT have invoked sh or built a shell string.
if strings.Contains(log, "=sh") || strings.Contains(log, "=-c") || strings.Contains(log, "cat >") {
t.Fatalf("WriteFile invoked shell command path; log:\n%s", log)
}
// Must have called tee.
if !strings.Contains(log, "=tee") {
t.Fatalf("expected write command to use tee without shell; log:\n%s", log)
}
// Must have included -- to terminate option parsing.
if !strings.Contains(log, "=--") {
t.Fatalf("expected tee delimiter (--) before filename; log:\n%s", log)
}
// The malicious path must appear verbatim as a single argv entry (not split/executed).
resolved := "/workspace/" + maliciousPath
if !strings.Contains(log, "="+resolved) {
t.Fatalf("expected malicious filename to remain one argv entry; log:\n%s", log)
}
stdinBytes, err := os.ReadFile(stdinPath)
if err != nil {
t.Fatalf("read captured stdin: %v", err)
}
if string(stdinBytes) != content {
t.Fatalf("stdin content = %q, want %q", string(stdinBytes), content)
}
}
// TestFsBridgeWriteFileAppendUsesTeeAppendArg asserts that append mode passes
// -a before -- in the tee argv, i.e. `tee -a -- <path>`.
func TestFsBridgeWriteFileAppendUsesTeeAppendArg(t *testing.T) {
tmp := t.TempDir()
logPath := filepath.Join(tmp, "docker.log")
installFakeDocker(t, tmp, logPath, "")
bridge := NewFsBridge("container-id", "/workspace")
if err := bridge.WriteFile(context.Background(), "append.txt", "more", true); err != nil {
t.Fatalf("WriteFile append returned error: %v", err)
}
logBytes, err := os.ReadFile(logPath)
if err != nil {
t.Fatalf("read docker log: %v", err)
}
log := string(logBytes)
// Must have: tee, then -a, then --.
if !strings.Contains(log, "=tee") {
t.Fatalf("expected tee in argv; log:\n%s", log)
}
if !strings.Contains(log, "=-a") {
t.Fatalf("expected -a flag for append mode; log:\n%s", log)
}
if !strings.Contains(log, "=--") {
t.Fatalf("expected -- delimiter in argv; log:\n%s", log)
}
}
// TestFsBridgeWriteTeeArgs_Overwrite checks the tee args slice for overwrite mode.
func TestFsBridgeWriteTeeArgs_Overwrite(t *testing.T) {
args := fsBridgeWriteTeeArgs("/workspace/file.txt", false)
if len(args) != 3 {
t.Fatalf("expected 3 args, got %d: %v", len(args), args)
}
if args[0] != "tee" {
t.Errorf("args[0] = %q, want tee", args[0])
}
if args[1] != "--" {
t.Errorf("args[1] = %q, want --", args[1])
}
if args[2] != "/workspace/file.txt" {
t.Errorf("args[2] = %q, want /workspace/file.txt", args[2])
}
// Must not contain -a in overwrite mode.
for _, a := range args {
if a == "-a" {
t.Errorf("overwrite mode must not contain -a: %v", args)
}
}
}
// TestFsBridgeWriteTeeArgs_Append checks the tee args slice for append mode.
func TestFsBridgeWriteTeeArgs_Append(t *testing.T) {
args := fsBridgeWriteTeeArgs("/workspace/file.txt", true)
if len(args) != 4 {
t.Fatalf("expected 4 args, got %d: %v", len(args), args)
}
if args[0] != "tee" {
t.Errorf("args[0] = %q, want tee", args[0])
}
if args[1] != "-a" {
t.Errorf("args[1] = %q, want -a", args[1])
}
if args[2] != "--" {
t.Errorf("args[2] = %q, want --", args[2])
}
if args[3] != "/workspace/file.txt" {
t.Errorf("args[3] = %q, want /workspace/file.txt", args[3])
}
}
+10
View File
@@ -77,6 +77,16 @@ func isBlocked(ip net.IP) bool {
return false
}
// IsBlocked reports whether ip falls within any blocked CIDR (loopback,
// link-local including cloud-metadata 169.254.169.254, RFC 1918 private,
// multicast, and unspecified 0.0.0.0/:: ranges).
//
// Use this in provider-URL validation and dial-time guards to avoid
// duplicating the CIDR list across packages.
func IsBlocked(ip net.IP) bool {
return isBlocked(ip)
}
// redactURL strips query string and userinfo for safe logging.
func redactURL(rawURL string) string {
u, err := url.Parse(rawURL)
+20 -5
View File
@@ -53,7 +53,7 @@ var toolProfiles = map[string][]string{
}
// Legacy tool aliases — migrated to Registry.RegisterAlias() at startup.
// Kept as seed data only; resolveAlias() is no longer used.
// resolveAlias() is used by IsDenied to expand names before deny-spec matching.
var legacyToolAliases = map[string]string{
"bash": "exec",
"apply-patch": "apply_patch",
@@ -434,19 +434,34 @@ func unionWithSpec(reg *Registry, current []string, allTools []string, spec []st
// IsDenied checks if a tool name is explicitly denied by global or agent policy.
// Used to prevent lazy-activated deferred tools from bypassing the deny list.
// Checks under all candidate names: the raw name, the legacy alias (e.g. bash→exec),
// and the registry alias when available.
func (pe *PolicyEngine) IsDenied(name string, agentPolicy *config.ToolPolicySpec) bool {
candidates := map[string]struct{}{name: {}}
// Keep legacy alias compatibility (e.g. bash -> exec).
candidates[resolveAlias(name)] = struct{}{}
// Include registry alias mapping when available.
pe.mu.RLock()
reg := pe.registry
pe.mu.RUnlock()
if reg != nil {
if canonical, ok := reg.Aliases()[name]; ok && canonical != "" {
candidates[canonical] = struct{}{}
}
}
if pe.globalPolicy != nil {
if matchDenySpec(reg, name, pe.globalPolicy.Deny) {
return true
for candidate := range candidates {
if matchDenySpec(reg, candidate, pe.globalPolicy.Deny) {
return true
}
}
}
if agentPolicy != nil {
if matchDenySpec(reg, name, agentPolicy.Deny) {
return true
for candidate := range candidates {
if matchDenySpec(reg, candidate, agentPolicy.Deny) {
return true
}
}
}
return false
@@ -160,6 +160,7 @@ func TestGitAdapter_SSH_CleanupOnExecFailure(t *testing.T) {
// TestGitAdapter_SSH_HostMismatch_NoTmpfile: rejected host mismatch must
// NOT leave an orphaned tmpfile in os.TempDir().
func TestGitAdapter_SSH_HostMismatch_NoTmpfile(t *testing.T) {
t.Setenv("TMPDIR", t.TempDir())
cred := sshCredFor(t, "github.com")
adapter := tools.AdapterFor("git")
+1
View File
@@ -36,6 +36,7 @@ server {
# API proxy
location /v1/ {
client_max_body_size 500M;
proxy_pass $upstream_backend;
proxy_set_header Host $host;
proxy_set_header X-Real-IP $remote_addr;