Merge branch 'security_shield_v2'
This commit is contained in:
commit
b79dca6ec1
7 changed files with 194 additions and 28 deletions
10
Makefile
10
Makefile
|
|
@ -211,6 +211,13 @@ build-linux-mipsle: generate
|
||||||
build-pi-zero: build-linux-arm build-linux-arm64
|
build-pi-zero: build-linux-arm build-linux-arm64
|
||||||
@echo "Pi Zero 2 W builds: $(BUILD_DIR)/$(BINARY_NAME)-linux-arm (32-bit), $(BUILD_DIR)/$(BINARY_NAME)-linux-arm64 (64-bit)"
|
@echo "Pi Zero 2 W builds: $(BUILD_DIR)/$(BINARY_NAME)-linux-arm (32-bit), $(BUILD_DIR)/$(BINARY_NAME)-linux-arm64 (64-bit)"
|
||||||
|
|
||||||
|
## build-raspberry-pi: Build binaries and Docker image for Raspberry Pi
|
||||||
|
build-raspberry-pi: build-pi-zero docker-build-rpi
|
||||||
|
@echo "Raspberry Pi full build complete (binaries and Docker image)"
|
||||||
|
|
||||||
|
## build-rpi: Build binaries and Docker image for Raspberry Pi
|
||||||
|
build-rpi: build-raspberry-pi
|
||||||
|
|
||||||
## build-all: Build picoclaw for all platforms
|
## build-all: Build picoclaw for all platforms
|
||||||
build-all: generate
|
build-all: generate
|
||||||
@echo "Building for multiple platforms..."
|
@echo "Building for multiple platforms..."
|
||||||
|
|
@ -332,6 +339,9 @@ docker-push-rpi:
|
||||||
@echo "Pushing Raspberry Pi Docker image (ARM64)..."
|
@echo "Pushing Raspberry Pi Docker image (ARM64)..."
|
||||||
docker push $(DOCKER_USER)/picoclaw-rpi:latest
|
docker push $(DOCKER_USER)/picoclaw-rpi:latest
|
||||||
|
|
||||||
|
docker-build-raspberry-pi: docker-build-rpi
|
||||||
|
docker-push-raspberry-pi: docker-push-rpi
|
||||||
|
|
||||||
## docker-run-full: Run picoclaw gateway in Docker (full-featured)
|
## docker-run-full: Run picoclaw gateway in Docker (full-featured)
|
||||||
docker-run-full:
|
docker-run-full:
|
||||||
docker compose -f docker/docker-compose.full.yml --profile gateway up
|
docker compose -f docker/docker-compose.full.yml --profile gateway up
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
# ============================================================
|
# ============================================================
|
||||||
# Stage 1: Build the picoclaw binaries
|
# Stage 1: Build the picoclaw binaries
|
||||||
# ============================================================
|
# ============================================================
|
||||||
FROM --platform=linux/arm64 golang:1.25-alpine AS builder
|
FROM --platform=linux/arm64 golang:1.26-alpine AS builder
|
||||||
|
|
||||||
WORKDIR /app
|
WORKDIR /app
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -193,6 +193,7 @@ func registerSharedTools(
|
||||||
) {
|
) {
|
||||||
allowReadPaths := buildAllowReadPatterns(cfg)
|
allowReadPaths := buildAllowReadPatterns(cfg)
|
||||||
denyReadPaths := compilePatterns(cfg.Tools.DenyReadPaths)
|
denyReadPaths := compilePatterns(cfg.Tools.DenyReadPaths)
|
||||||
|
denyWritePaths := compilePatterns(cfg.Tools.DenyWritePaths)
|
||||||
var ttsProvider tts.TTSProvider
|
var ttsProvider tts.TTSProvider
|
||||||
if cfg.Tools.IsToolEnabled("send_tts") {
|
if cfg.Tools.IsToolEnabled("send_tts") {
|
||||||
ttsProvider = tts.DetectTTS(cfg)
|
ttsProvider = tts.DetectTTS(cfg)
|
||||||
|
|
@ -376,6 +377,7 @@ func registerSharedTools(
|
||||||
agent.Workspace,
|
agent.Workspace,
|
||||||
cfg.Tools.Skills.Whitelist,
|
cfg.Tools.Skills.Whitelist,
|
||||||
cfg.Tools.Skills.WhitelistEnabled,
|
cfg.Tools.Skills.WhitelistEnabled,
|
||||||
|
denyWritePaths,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -40,6 +40,7 @@ type ExecTool struct {
|
||||||
allowPatterns []*regexp.Regexp
|
allowPatterns []*regexp.Regexp
|
||||||
customAllowPatterns []*regexp.Regexp
|
customAllowPatterns []*regexp.Regexp
|
||||||
allowedPathPatterns []*regexp.Regexp
|
allowedPathPatterns []*regexp.Regexp
|
||||||
|
denyWritePaths []*regexp.Regexp
|
||||||
restrictToWorkspace bool
|
restrictToWorkspace bool
|
||||||
allowRemote bool
|
allowRemote bool
|
||||||
sessionManager *SessionManager
|
sessionManager *SessionManager
|
||||||
|
|
@ -120,8 +121,18 @@ func NewExecTool(workingDir string, restrict bool, allowPaths ...[]*regexp.Regex
|
||||||
func NewExecToolWithConfig(
|
func NewExecToolWithConfig(
|
||||||
workingDir string,
|
workingDir string,
|
||||||
restrict bool,
|
restrict bool,
|
||||||
config *config.Config,
|
cfg *config.Config,
|
||||||
allowPaths ...[]*regexp.Regexp,
|
allowPaths ...[]*regexp.Regexp,
|
||||||
|
) (*ExecTool, error) {
|
||||||
|
return NewExecToolWithDenyPaths(workingDir, restrict, allowPaths, nil, cfg)
|
||||||
|
}
|
||||||
|
|
||||||
|
func NewExecToolWithDenyPaths(
|
||||||
|
workingDir string,
|
||||||
|
restrict bool,
|
||||||
|
allowPaths [][]*regexp.Regexp,
|
||||||
|
denyWritePaths []*regexp.Regexp,
|
||||||
|
cfg *config.Config,
|
||||||
) (*ExecTool, error) {
|
) (*ExecTool, error) {
|
||||||
denyPatterns := make([]*regexp.Regexp, 0)
|
denyPatterns := make([]*regexp.Regexp, 0)
|
||||||
customAllowPatterns := make([]*regexp.Regexp, 0)
|
customAllowPatterns := make([]*regexp.Regexp, 0)
|
||||||
|
|
@ -131,8 +142,8 @@ func NewExecToolWithConfig(
|
||||||
allowedPathPatterns = allowPaths[0]
|
allowedPathPatterns = allowPaths[0]
|
||||||
}
|
}
|
||||||
|
|
||||||
if config != nil {
|
if cfg != nil {
|
||||||
execConfig := config.Tools.Exec
|
execConfig := cfg.Tools.Exec
|
||||||
enableDenyPatterns := execConfig.EnableDenyPatterns
|
enableDenyPatterns := execConfig.EnableDenyPatterns
|
||||||
allowRemote = execConfig.AllowRemote
|
allowRemote = execConfig.AllowRemote
|
||||||
if enableDenyPatterns {
|
if enableDenyPatterns {
|
||||||
|
|
@ -163,8 +174,8 @@ func NewExecToolWithConfig(
|
||||||
}
|
}
|
||||||
|
|
||||||
var timeout time.Duration
|
var timeout time.Duration
|
||||||
if config != nil && config.Tools.Exec.TimeoutSeconds > 0 {
|
if cfg != nil && cfg.Tools.Exec.TimeoutSeconds > 0 {
|
||||||
timeout = time.Duration(config.Tools.Exec.TimeoutSeconds) * time.Second
|
timeout = time.Duration(cfg.Tools.Exec.TimeoutSeconds) * time.Second
|
||||||
}
|
}
|
||||||
|
|
||||||
return &ExecTool{
|
return &ExecTool{
|
||||||
|
|
@ -174,6 +185,7 @@ func NewExecToolWithConfig(
|
||||||
allowPatterns: nil,
|
allowPatterns: nil,
|
||||||
customAllowPatterns: customAllowPatterns,
|
customAllowPatterns: customAllowPatterns,
|
||||||
allowedPathPatterns: allowedPathPatterns,
|
allowedPathPatterns: allowedPathPatterns,
|
||||||
|
denyWritePaths: denyWritePaths,
|
||||||
restrictToWorkspace: restrict,
|
restrictToWorkspace: restrict,
|
||||||
allowRemote: allowRemote,
|
allowRemote: allowRemote,
|
||||||
sessionManager: getSessionManager(),
|
sessionManager: getSessionManager(),
|
||||||
|
|
@ -1033,6 +1045,40 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
return "Command blocked by safety guard (dangerous pattern detected)"
|
return "Command blocked by safety guard (dangerous pattern detected)"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check deny write paths - block commands that reference protected directories or variables
|
||||||
|
// We perform a broad check on the entire command string to prevent variable bypasses.
|
||||||
|
if len(t.denyWritePaths) > 0 {
|
||||||
|
// First check: literal occurrences in the whole command
|
||||||
|
for _, pattern := range t.denyWritePaths {
|
||||||
|
if pattern.MatchString(cmd) {
|
||||||
|
return fmt.Sprintf("Command blocked: reference to restricted path detected")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Second check: check individual words/arguments for deeper validation
|
||||||
|
words := strings.Fields(cmd)
|
||||||
|
for _, word := range words {
|
||||||
|
// Clean whitespace and common shell chars from word to find actual path candidates
|
||||||
|
cleanWord := strings.Trim(word, " ;&|><\"'$()")
|
||||||
|
if cleanWord == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, pattern := range t.denyWritePaths {
|
||||||
|
if pattern.MatchString(cleanWord) {
|
||||||
|
return fmt.Sprintf("Command blocked: cannot access protected path %q", cleanWord)
|
||||||
|
}
|
||||||
|
// Also check path components (e.g. "skills" in "mkdir -p skills/foo")
|
||||||
|
pathParts := strings.Split(cleanWord, "/")
|
||||||
|
for _, part := range pathParts {
|
||||||
|
if part != "" && pattern.MatchString(part) {
|
||||||
|
return fmt.Sprintf("Command blocked: cannot access protected path component %q", part)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(t.allowPatterns) > 0 {
|
if len(t.allowPatterns) > 0 {
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,7 @@ import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"regexp"
|
||||||
"sync"
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
|
@ -20,23 +21,27 @@ type InstallSkillTool struct {
|
||||||
workspace string
|
workspace string
|
||||||
whitelist []string
|
whitelist []string
|
||||||
whitelistEnabled bool
|
whitelistEnabled bool
|
||||||
|
denyWritePaths []*regexp.Regexp
|
||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewInstallSkillTool creates a new InstallSkillTool.
|
// NewInstallSkillTool creates a new InstallSkillTool.
|
||||||
// registryMgr is the shared registry manager (same instance as FindSkillsTool).
|
// registryMgr is the shared registry manager (same instance as FindSkillsTool).
|
||||||
// workspace is the root workspace directory; skills install to {workspace}/skills/{slug}/.
|
// workspace is the root workspace directory; skills install to {workspace}/skills/{slug}/.
|
||||||
|
// denyWritePaths is a list of regex patterns to check before allowing installation.
|
||||||
func NewInstallSkillTool(
|
func NewInstallSkillTool(
|
||||||
registryMgr *skills.RegistryManager,
|
registryMgr *skills.RegistryManager,
|
||||||
workspace string,
|
workspace string,
|
||||||
whitelist []string,
|
whitelist []string,
|
||||||
whitelistEnabled bool,
|
whitelistEnabled bool,
|
||||||
|
denyWritePaths []*regexp.Regexp,
|
||||||
) *InstallSkillTool {
|
) *InstallSkillTool {
|
||||||
return &InstallSkillTool{
|
return &InstallSkillTool{
|
||||||
registryMgr: registryMgr,
|
registryMgr: registryMgr,
|
||||||
workspace: workspace,
|
workspace: workspace,
|
||||||
whitelist: whitelist,
|
whitelist: whitelist,
|
||||||
whitelistEnabled: whitelistEnabled,
|
whitelistEnabled: whitelistEnabled,
|
||||||
|
denyWritePaths: denyWritePaths,
|
||||||
mu: sync.Mutex{},
|
mu: sync.Mutex{},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -109,6 +114,18 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To
|
||||||
version, _ := args["version"].(string)
|
version, _ := args["version"].(string)
|
||||||
force, _ := args["force"].(bool)
|
force, _ := args["force"].(bool)
|
||||||
|
|
||||||
|
// Check deny write paths before proceeding with installation.
|
||||||
|
if len(t.denyWritePaths) > 0 {
|
||||||
|
pathsToCheck := []string{"skills", filepath.Join("skills", slug)}
|
||||||
|
for _, path := range pathsToCheck {
|
||||||
|
for _, pattern := range t.denyWritePaths {
|
||||||
|
if pattern.MatchString(path) {
|
||||||
|
return ErrorResult(fmt.Sprintf("access denied: cannot write to %q", path))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Check if already installed.
|
// Check if already installed.
|
||||||
skillsDir := filepath.Join(t.workspace, "skills")
|
skillsDir := filepath.Join(t.workspace, "skills")
|
||||||
targetDir := filepath.Join(skillsDir, slug)
|
targetDir := filepath.Join(skillsDir, slug)
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,7 @@ import (
|
||||||
"context"
|
"context"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"regexp"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
|
@ -13,19 +14,19 @@ import (
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestInstallSkillToolName(t *testing.T) {
|
func TestInstallSkillToolName(t *testing.T) {
|
||||||
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
|
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
|
||||||
assert.Equal(t, "install_skill", tool.Name())
|
assert.Equal(t, "install_skill", tool.Name())
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestInstallSkillToolMissingSlug(t *testing.T) {
|
func TestInstallSkillToolMissingSlug(t *testing.T) {
|
||||||
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
|
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
|
||||||
result := tool.Execute(context.Background(), map[string]any{})
|
result := tool.Execute(context.Background(), map[string]any{})
|
||||||
assert.True(t, result.IsError)
|
assert.True(t, result.IsError)
|
||||||
assert.Contains(t, result.ForLLM, "identifier is required and must be a non-empty string")
|
assert.Contains(t, result.ForLLM, "identifier is required and must be a non-empty string")
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestInstallSkillToolEmptySlug(t *testing.T) {
|
func TestInstallSkillToolEmptySlug(t *testing.T) {
|
||||||
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
|
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
|
||||||
result := tool.Execute(context.Background(), map[string]any{
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
"slug": " ",
|
"slug": " ",
|
||||||
})
|
})
|
||||||
|
|
@ -34,7 +35,7 @@ func TestInstallSkillToolEmptySlug(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestInstallSkillToolUnsafeSlug(t *testing.T) {
|
func TestInstallSkillToolUnsafeSlug(t *testing.T) {
|
||||||
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
|
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
|
||||||
|
|
||||||
cases := []string{
|
cases := []string{
|
||||||
"../etc/passwd",
|
"../etc/passwd",
|
||||||
|
|
@ -56,7 +57,7 @@ func TestInstallSkillToolAlreadyExists(t *testing.T) {
|
||||||
skillDir := filepath.Join(workspace, "skills", "existing-skill")
|
skillDir := filepath.Join(workspace, "skills", "existing-skill")
|
||||||
require.NoError(t, os.MkdirAll(skillDir, 0o755))
|
require.NoError(t, os.MkdirAll(skillDir, 0o755))
|
||||||
|
|
||||||
tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace, nil, false)
|
tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace, nil, false, nil)
|
||||||
result := tool.Execute(context.Background(), map[string]any{
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
"slug": "existing-skill",
|
"slug": "existing-skill",
|
||||||
"registry": "clawhub",
|
"registry": "clawhub",
|
||||||
|
|
@ -67,7 +68,7 @@ func TestInstallSkillToolAlreadyExists(t *testing.T) {
|
||||||
|
|
||||||
func TestInstallSkillToolRegistryNotFound(t *testing.T) {
|
func TestInstallSkillToolRegistryNotFound(t *testing.T) {
|
||||||
workspace := t.TempDir()
|
workspace := t.TempDir()
|
||||||
tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace, nil, false)
|
tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace, nil, false, nil)
|
||||||
result := tool.Execute(context.Background(), map[string]any{
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
"slug": "some-skill",
|
"slug": "some-skill",
|
||||||
"registry": "nonexistent",
|
"registry": "nonexistent",
|
||||||
|
|
@ -78,7 +79,7 @@ func TestInstallSkillToolRegistryNotFound(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestInstallSkillToolParameters(t *testing.T) {
|
func TestInstallSkillToolParameters(t *testing.T) {
|
||||||
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
|
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
|
||||||
params := tool.Parameters()
|
params := tool.Parameters()
|
||||||
|
|
||||||
props, ok := params["properties"].(map[string]any)
|
props, ok := params["properties"].(map[string]any)
|
||||||
|
|
@ -95,7 +96,7 @@ func TestInstallSkillToolParameters(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestInstallSkillToolMissingRegistry(t *testing.T) {
|
func TestInstallSkillToolMissingRegistry(t *testing.T) {
|
||||||
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
|
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
|
||||||
result := tool.Execute(context.Background(), map[string]any{
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
"slug": "some-skill",
|
"slug": "some-skill",
|
||||||
})
|
})
|
||||||
|
|
@ -108,7 +109,7 @@ func TestInstallSkillToolWhitelist(t *testing.T) {
|
||||||
rm := skills.NewRegistryManager()
|
rm := skills.NewRegistryManager()
|
||||||
|
|
||||||
t.Run("blocked-by-whitelist", func(t *testing.T) {
|
t.Run("blocked-by-whitelist", func(t *testing.T) {
|
||||||
tool := NewInstallSkillTool(rm, workspace, []string{"allowed-skill"}, true)
|
tool := NewInstallSkillTool(rm, workspace, []string{"allowed-skill"}, true, nil)
|
||||||
result := tool.Execute(context.Background(), map[string]any{
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
"slug": "blocked-skill",
|
"slug": "blocked-skill",
|
||||||
"registry": "clawhub",
|
"registry": "clawhub",
|
||||||
|
|
@ -119,7 +120,7 @@ func TestInstallSkillToolWhitelist(t *testing.T) {
|
||||||
|
|
||||||
t.Run("allowed-by-whitelist", func(t *testing.T) {
|
t.Run("allowed-by-whitelist", func(t *testing.T) {
|
||||||
// This will still fail because registry is not found, but it should pass the whitelist check
|
// This will still fail because registry is not found, but it should pass the whitelist check
|
||||||
tool := NewInstallSkillTool(rm, workspace, []string{"allowed-skill"}, true)
|
tool := NewInstallSkillTool(rm, workspace, []string{"allowed-skill"}, true, nil)
|
||||||
result := tool.Execute(context.Background(), map[string]any{
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
"slug": "allowed-skill",
|
"slug": "allowed-skill",
|
||||||
"registry": "clawhub",
|
"registry": "clawhub",
|
||||||
|
|
@ -129,7 +130,7 @@ func TestInstallSkillToolWhitelist(t *testing.T) {
|
||||||
})
|
})
|
||||||
|
|
||||||
t.Run("empty-whitelist-allows-all", func(t *testing.T) {
|
t.Run("empty-whitelist-allows-all", func(t *testing.T) {
|
||||||
tool := NewInstallSkillTool(rm, workspace, []string{}, false)
|
tool := NewInstallSkillTool(rm, workspace, []string{}, false, nil)
|
||||||
result := tool.Execute(context.Background(), map[string]any{
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
"slug": "any-skill",
|
"slug": "any-skill",
|
||||||
"registry": "clawhub",
|
"registry": "clawhub",
|
||||||
|
|
@ -139,7 +140,7 @@ func TestInstallSkillToolWhitelist(t *testing.T) {
|
||||||
})
|
})
|
||||||
|
|
||||||
t.Run("nil-whitelist-allows-all", func(t *testing.T) {
|
t.Run("nil-whitelist-allows-all", func(t *testing.T) {
|
||||||
tool := NewInstallSkillTool(rm, workspace, nil, false)
|
tool := NewInstallSkillTool(rm, workspace, nil, false, nil)
|
||||||
result := tool.Execute(context.Background(), map[string]any{
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
"slug": "any-skill",
|
"slug": "any-skill",
|
||||||
"registry": "clawhub",
|
"registry": "clawhub",
|
||||||
|
|
@ -148,3 +149,40 @@ func TestInstallSkillToolWhitelist(t *testing.T) {
|
||||||
assert.NotContains(t, result.ForLLM, "not in whitelist")
|
assert.NotContains(t, result.ForLLM, "not in whitelist")
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestInstallSkillToolDenyWritePaths(t *testing.T) {
|
||||||
|
workspace := t.TempDir()
|
||||||
|
rm := skills.NewRegistryManager()
|
||||||
|
|
||||||
|
t.Run("blocked-by-deny-write-paths", func(t *testing.T) {
|
||||||
|
denyPatterns := []*regexp.Regexp{regexp.MustCompile(`^skills(/.*)?$`)}
|
||||||
|
tool := NewInstallSkillTool(rm, workspace, nil, false, denyPatterns)
|
||||||
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
|
"slug": "some-skill",
|
||||||
|
"registry": "clawhub",
|
||||||
|
})
|
||||||
|
assert.True(t, result.IsError)
|
||||||
|
assert.Contains(t, result.ForLLM, "access denied")
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("allowed-without-deny-paths", func(t *testing.T) {
|
||||||
|
tool := NewInstallSkillTool(rm, workspace, nil, false, nil)
|
||||||
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
|
"slug": "some-skill",
|
||||||
|
"registry": "clawhub",
|
||||||
|
})
|
||||||
|
assert.True(t, result.IsError)
|
||||||
|
assert.NotContains(t, result.ForLLM, "access denied")
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("non-matching-deny-pattern-allows", func(t *testing.T) {
|
||||||
|
denyPatterns := []*regexp.Regexp{regexp.MustCompile(`^restricted(/.*)?$`)}
|
||||||
|
tool := NewInstallSkillTool(rm, workspace, nil, false, denyPatterns)
|
||||||
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
|
"slug": "some-skill",
|
||||||
|
"registry": "clawhub",
|
||||||
|
})
|
||||||
|
assert.True(t, result.IsError)
|
||||||
|
assert.NotContains(t, result.ForLLM, "access denied")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -127,9 +127,17 @@ func (h *Handler) handleListSkills(w http.ResponseWriter, r *http.Request) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Filter based on security policy
|
||||||
|
filtered := make([]skillSupportItem, 0, len(items))
|
||||||
|
for _, item := range items {
|
||||||
|
if ensureSkillRegistryToolEnabled(cfg, "", item.Name) == nil {
|
||||||
|
filtered = append(filtered, item)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
w.Header().Set("Content-Type", "application/json")
|
w.Header().Set("Content-Type", "application/json")
|
||||||
json.NewEncoder(w).Encode(skillSupportResponse{
|
json.NewEncoder(w).Encode(skillSupportResponse{
|
||||||
Skills: items,
|
Skills: filtered,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -146,6 +154,12 @@ func (h *Handler) handleGetSkill(w http.ResponseWriter, r *http.Request) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
name := r.PathValue("name")
|
name := r.PathValue("name")
|
||||||
|
|
||||||
|
if registryErr := ensureSkillRegistryToolEnabled(cfg, "", name); registryErr != nil {
|
||||||
|
http.Error(w, registryErr.Error(), http.StatusBadRequest)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
for _, skillItem := range skillItems {
|
for _, skillItem := range skillItems {
|
||||||
if skillItem.Name != name {
|
if skillItem.Name != name {
|
||||||
continue
|
continue
|
||||||
|
|
@ -174,7 +188,7 @@ func (h *Handler) handleSearchSkills(w http.ResponseWriter, r *http.Request) {
|
||||||
http.Error(w, fmt.Sprintf("Failed to load config: %v", loadErr), http.StatusInternalServerError)
|
http.Error(w, fmt.Sprintf("Failed to load config: %v", loadErr), http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if registryErr := ensureSkillRegistryToolEnabled(cfg, "find_skills"); registryErr != nil {
|
if registryErr := ensureSkillRegistryToolEnabled(cfg, "find_skills", ""); registryErr != nil {
|
||||||
http.Error(w, registryErr.Error(), http.StatusBadRequest)
|
http.Error(w, registryErr.Error(), http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
@ -278,17 +292,17 @@ func (h *Handler) handleInstallSkill(w http.ResponseWriter, r *http.Request) {
|
||||||
http.Error(w, fmt.Sprintf("Failed to load config: %v", loadErr), http.StatusInternalServerError)
|
http.Error(w, fmt.Sprintf("Failed to load config: %v", loadErr), http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if registryErr := ensureSkillRegistryToolEnabled(cfg, "install_skill"); registryErr != nil {
|
|
||||||
http.Error(w, registryErr.Error(), http.StatusBadRequest)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
var req installSkillRequest
|
var req installSkillRequest
|
||||||
if decodeErr := json.NewDecoder(r.Body).Decode(&req); decodeErr != nil {
|
if decodeErr := json.NewDecoder(r.Body).Decode(&req); decodeErr != nil {
|
||||||
http.Error(w, fmt.Sprintf("Invalid JSON: %v", decodeErr), http.StatusBadRequest)
|
http.Error(w, fmt.Sprintf("Invalid JSON: %v", decodeErr), http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if registryErr := ensureSkillRegistryToolEnabled(cfg, "install_skill", req.Slug); registryErr != nil {
|
||||||
|
http.Error(w, registryErr.Error(), http.StatusBadRequest)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
req.Slug = strings.TrimSpace(req.Slug)
|
req.Slug = strings.TrimSpace(req.Slug)
|
||||||
req.Registry = strings.TrimSpace(req.Registry)
|
req.Registry = strings.TrimSpace(req.Registry)
|
||||||
req.Version = strings.TrimSpace(req.Version)
|
req.Version = strings.TrimSpace(req.Version)
|
||||||
|
|
@ -448,6 +462,11 @@ func (h *Handler) handleImportSkill(w http.ResponseWriter, r *http.Request) {
|
||||||
}
|
}
|
||||||
defer uploadedFile.Close()
|
defer uploadedFile.Close()
|
||||||
|
|
||||||
|
if registryErr := ensureSkillRegistryToolEnabled(cfg, "write_file", fileHeader.Filename); registryErr != nil {
|
||||||
|
http.Error(w, registryErr.Error(), http.StatusBadRequest)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
content, err := io.ReadAll(io.LimitReader(uploadedFile, maxImportedSkillSize+1))
|
content, err := io.ReadAll(io.LimitReader(uploadedFile, maxImportedSkillSize+1))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
http.Error(w, fmt.Sprintf("Failed to read file: %v", err), http.StatusBadRequest)
|
http.Error(w, fmt.Sprintf("Failed to read file: %v", err), http.StatusBadRequest)
|
||||||
|
|
@ -479,6 +498,11 @@ func (h *Handler) handleDeleteSkill(w http.ResponseWriter, r *http.Request) {
|
||||||
|
|
||||||
loader := newSkillsLoader(cfg.WorkspacePath())
|
loader := newSkillsLoader(cfg.WorkspacePath())
|
||||||
name := r.PathValue("name")
|
name := r.PathValue("name")
|
||||||
|
|
||||||
|
if registryErr := ensureSkillRegistryToolEnabled(cfg, "", name); registryErr != nil {
|
||||||
|
http.Error(w, registryErr.Error(), http.StatusBadRequest)
|
||||||
|
return
|
||||||
|
}
|
||||||
workspaceSkillWriteMu.Lock()
|
workspaceSkillWriteMu.Lock()
|
||||||
defer workspaceSkillWriteMu.Unlock()
|
defer workspaceSkillWriteMu.Unlock()
|
||||||
|
|
||||||
|
|
@ -531,13 +555,42 @@ func newSkillsRegistryManager(cfg *config.Config) *skills.RegistryManager {
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
func ensureSkillRegistryToolEnabled(cfg *config.Config, toolName string) error {
|
func ensureSkillRegistryToolEnabled(cfg *config.Config, toolName string, skillName string) error {
|
||||||
if !cfg.Tools.IsToolEnabled("skills") {
|
if !cfg.Tools.IsToolEnabled("skills") {
|
||||||
return fmt.Errorf("tools.skills is disabled")
|
return fmt.Errorf("tools.skills is disabled")
|
||||||
}
|
}
|
||||||
|
if toolName != "" {
|
||||||
if !cfg.Tools.IsToolEnabled(toolName) {
|
if !cfg.Tools.IsToolEnabled(toolName) {
|
||||||
return fmt.Errorf("%s is disabled", toolName)
|
return fmt.Errorf("%s is disabled", toolName)
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check whitelist for specific skill if enabled
|
||||||
|
if cfg.Tools.Skills.WhitelistEnabled && skillName != "" {
|
||||||
|
allowed := false
|
||||||
|
for _, s := range cfg.Tools.Skills.Whitelist {
|
||||||
|
if s == skillName {
|
||||||
|
allowed = true
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !allowed {
|
||||||
|
return fmt.Errorf("skill %q is not in the whitelist", skillName)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check deny paths
|
||||||
|
if skillName != "" {
|
||||||
|
// Path would be skills/skillName
|
||||||
|
pathCandidate := filepath.Join("skills", skillName)
|
||||||
|
for _, patternStr := range cfg.Tools.DenyWritePaths {
|
||||||
|
re, err := regexp.Compile(patternStr)
|
||||||
|
if err == nil && re.MatchString(pathCandidate) {
|
||||||
|
return fmt.Errorf("access to skill %q is blocked by security policy", skillName)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue