mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 08:18:19 +00:00
Merge upstream dev fixes
Merged upstream nextlevelbuilder/goclaw dev fixes plus review fix enforcing team access on vault document updates.
This commit is contained in:
40 files changed
+2277
-263
No files matched your search
+37
-169
@@ -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
|
||||
|
||||
@@ -82,6 +82,8 @@ goclaw.db-shm
|
||||
*.app.bak
|
||||
*.exe.bak
|
||||
# AI tool config directories (user-specific)
|
||||
.agents/
|
||||
.codex/
|
||||
.gemini/
|
||||
.opencode/
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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"
|
||||
},
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -353,9 +353,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
|
||||
@@ -392,9 +392,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)
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@@ -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
@@ -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)
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
@@ -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) {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
@@ -186,9 +382,17 @@ func (h *VaultHandler) handleUpdateDocument(w http.ResponseWriter, r *http.Reque
|
||||
writeJSON(w, http.StatusNotFound, map[string]string{"error": "document not found"})
|
||||
return
|
||||
}
|
||||
// Verify team boundary before any metadata or content rewrite. PUT can
|
||||
// materialize bytes on disk, so it must match GET/DELETE team access rules.
|
||||
if existing.TeamID != nil && *existing.TeamID != "" && !store.IsOwnerRole(r.Context()) {
|
||||
if !h.validateTeamMembership(r.Context(), w, *existing.TeamID) {
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
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 +429,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,534 @@
|
||||
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 }
|
||||
|
||||
type fakeTeamAccessStore struct {
|
||||
allowed bool
|
||||
}
|
||||
|
||||
func (f *fakeTeamAccessStore) GrantTeamAccess(context.Context, uuid.UUID, string, string, string) error {
|
||||
return nil
|
||||
}
|
||||
func (f *fakeTeamAccessStore) RevokeTeamAccess(context.Context, uuid.UUID, string) error {
|
||||
return nil
|
||||
}
|
||||
func (f *fakeTeamAccessStore) ListTeamGrants(context.Context, uuid.UUID) ([]store.TeamUserGrant, error) {
|
||||
return nil, nil
|
||||
}
|
||||
func (f *fakeTeamAccessStore) ListUserTeams(context.Context, string) ([]store.TeamData, error) {
|
||||
return nil, nil
|
||||
}
|
||||
func (f *fakeTeamAccessStore) HasTeamAccess(context.Context, uuid.UUID, string) (bool, error) {
|
||||
return f.allowed, 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))
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleUpdateDocument_TeamDocRequiresMembershipBeforeContentWrite(t *testing.T) {
|
||||
teamID := uuid.NewString()
|
||||
st := newFakeVaultStore()
|
||||
st.docs["team-doc"] = &store.VaultDocument{
|
||||
ID: "team-doc",
|
||||
TenantID: masterTenant.String(),
|
||||
TeamID: &teamID,
|
||||
Path: "teams/" + teamID + "/private.md",
|
||||
Title: "Private",
|
||||
DocType: "note",
|
||||
Scope: "team",
|
||||
}
|
||||
bus := &fakeEventBus{store: st}
|
||||
ws := t.TempDir()
|
||||
h := &VaultHandler{
|
||||
store: st,
|
||||
teamAccess: &fakeTeamAccessStore{allowed: false},
|
||||
eventBus: bus,
|
||||
workspace: ws,
|
||||
}
|
||||
|
||||
ctx := store.WithUserID(masterCtx(), "non-member")
|
||||
req := newJSONRequest(t, http.MethodPut, "/v1/vault/documents/team-doc", map[string]any{
|
||||
"content": "should not be written",
|
||||
}).WithContext(ctx)
|
||||
req.SetPathValue("docID", "team-doc")
|
||||
rr := httptest.NewRecorder()
|
||||
|
||||
h.handleUpdateDocument(rr, req)
|
||||
|
||||
if rr.Code != http.StatusForbidden {
|
||||
t.Fatalf("status = %d, want 403 (body: %s)", rr.Code, rr.Body.String())
|
||||
}
|
||||
if _, statErr := os.Stat(filepath.Join(ws, "teams", teamID, "private.md")); statErr == nil {
|
||||
t.Fatal("non-member content update wrote file before team access check")
|
||||
}
|
||||
if st.upsertCall != 0 {
|
||||
t.Fatalf("upsertCall = %d, want 0", st.upsertCall)
|
||||
}
|
||||
if len(bus.published) != 0 {
|
||||
t.Fatalf("published %d events, want 0", 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))
|
||||
}
|
||||
}
|
||||
@@ -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")
|
||||
|
||||
@@ -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])
|
||||
}
|
||||
}
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -42,6 +42,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
|
||||
@@ -156,12 +163,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,
|
||||
|
||||
@@ -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"})
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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()}
|
||||
}
|
||||
@@ -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(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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(),
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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])
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in new issue
Block a user