diff --git a/cmd/picoclaw/main_test.go b/cmd/picoclaw/main_test.go index cb221dece..f18013371 100644 --- a/cmd/picoclaw/main_test.go +++ b/cmd/picoclaw/main_test.go @@ -9,7 +9,9 @@ import ( "github.com/stretchr/testify/require" "github.com/sipeed/picoclaw/cmd/picoclaw/internal" + "github.com/sipeed/picoclaw/pkg/agent" "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/security" ) func TestNewPicoclawCommand(t *testing.T) { @@ -57,3 +59,24 @@ func TestNewPicoclawCommand(t *testing.T) { assert.False(t, subcmd.Hidden) } } + +func TestSecurityShieldRegistration_Regression(t *testing.T) { + // Initialize security hooks (registers them in pkg/agent) + security.Init() + + expectedHooks := []string{ + "security_policy", + "security_canary", + "security_behavior", + "security_pii", + "security_ipia", + } + + for _, name := range expectedHooks { + t.Run(name, func(t *testing.T) { + assert.True(t, agent.IsBuiltinHookRegistered(name), + "Builtin hook %q is not registered. This usually means pkg/security was deleted "+ + "or security.Init() is no longer called in main().", name) + }) + } +} diff --git a/pkg/agent/hook_mount.go b/pkg/agent/hook_mount.go index c92145f1f..aa2e38d60 100644 --- a/pkg/agent/hook_mount.go +++ b/pkg/agent/hook_mount.go @@ -8,8 +8,17 @@ import ( "time" "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/logger" ) +// IsBuiltinHookRegistered returns true if a builtin hook factory is registered with the given name. +func IsBuiltinHookRegistered(name string) bool { + builtinHookRegistryMu.RLock() + defer builtinHookRegistryMu.RUnlock() + _, exists := builtinHookRegistry[name] + return exists +} + type hookRuntime struct { initOnce sync.Once mu sync.Mutex @@ -143,19 +152,25 @@ func (al *AgentLoop) loadConfiguredHooks(ctx context.Context) (err error) { spec := al.cfg.Hooks.Builtins[name] factory, ok := lookupBuiltinHook(name) if !ok { + logger.WarnCF("agent", "Builtin hook not registered", map[string]any{"hook": name}) return fmt.Errorf("builtin hook %q is not registered", name) } + logger.DebugCF("agent", "Executing builtin hook factory", map[string]any{"hook": name}) hook, factoryErr := factory(ctx, spec) if factoryErr != nil { + logger.ErrorCF("agent", "Builtin hook factory failed", map[string]any{"hook": name, "error": factoryErr.Error()}) return fmt.Errorf("build builtin hook %q: %w", name, factoryErr) } + logger.DebugCF("agent", "Builtin hook factory finished", map[string]any{"hook": name}) + if err := al.MountHook(HookRegistration{ Name: name, Priority: spec.Priority, Source: HookSourceInProcess, Hook: hook, }); err != nil { + logger.ErrorCF("agent", "Failed to mount builtin hook", map[string]any{"hook": name, "error": err.Error()}) return fmt.Errorf("mount builtin hook %q: %w", name, err) } mounted = append(mounted, name) diff --git a/pkg/security/policy/checker.go b/pkg/security/policy/checker.go index f4b5e13b7..eb51ea467 100644 --- a/pkg/security/policy/checker.go +++ b/pkg/security/policy/checker.go @@ -55,7 +55,7 @@ func (c *Checker) ApproveTool(ctx context.Context, req *agent.ToolApprovalReques if c.Config.AllowedTools[req.Tool] { allowed = true } else { - // Check for prefix matches (e.g. "monday" matches "mcp_monday_...") + // Check for prefix matches (e.g. "github" matches "mcp_github_...") // Match logic consistent with ToolRegistry.Filter for w, ok := range c.Config.AllowedTools { if !ok {