mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-17 02:14:09 +00:00
fix(agents): complete shell deny groups propagation chain
ShellDenyGroups was defined in SystemPromptConfig but lacked full propagation through parser, Loop fields, context injection, and system prompt population. Per-agent overrides from other_config JSONB had zero runtime effect. Changes: - agent_store.go: Add ParseShellDenyGroups() to extract overrides from JSONB - loop_types.go: Add shellDenyGroups field to Loop and LoopConfig, wire in NewLoop - resolver.go: Wire agent-parsed shell deny groups into LoopConfig - loop.go: Inject shellDenyGroups into context via store.WithShellDenyGroups - loop_history.go: Populate ShellDenyGroups in system prompt config - message_test.go: Fix macOS symlink path normalization in test expectations Fixes test failures on macOS where /var/folders symlinks to /private/var/folders.
This commit is contained in:
1 parent
b818de6d63
commit
2504095dfe
6 files changed
+39
-7
No files matched your search
@@ -83,6 +83,9 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
if l.sandboxCfg != nil {
|
||||
ctx = tools.WithSandboxConfig(ctx, l.sandboxCfg)
|
||||
}
|
||||
if l.shellDenyGroups != nil {
|
||||
ctx = store.WithShellDenyGroups(ctx, l.shellDenyGroups)
|
||||
}
|
||||
|
||||
// Workspace scope propagation (delegation origin → workspace tools).
|
||||
if req.WorkspaceChannel != "" {
|
||||
|
||||
@@ -194,6 +194,7 @@ func (l *Loop) buildMessages(ctx context.Context, history []providers.Message, s
|
||||
SandboxEnabled: l.sandboxEnabled,
|
||||
SandboxContainerDir: l.sandboxContainerDir,
|
||||
SandboxWorkspaceAccess: l.sandboxWorkspaceAccess,
|
||||
ShellDenyGroups: l.shellDenyGroups,
|
||||
SelfEvolve: l.selfEvolve,
|
||||
CredentialCLIContext: l.buildCredentialCLIContext(ctx),
|
||||
IsBootstrap: hadBootstrap && l.agentType != "predefined",
|
||||
|
||||
@@ -91,6 +91,9 @@ type Loop struct {
|
||||
sandboxContainerDir string
|
||||
sandboxWorkspaceAccess string
|
||||
|
||||
// Shell deny group overrides from agent other_config (nil = all defaults)
|
||||
shellDenyGroups map[string]bool
|
||||
|
||||
// Event callback for broadcasting agent events (run.started, chunk, tool.call, etc.)
|
||||
onEvent func(event AgentEvent)
|
||||
|
||||
@@ -200,6 +203,9 @@ type LoopConfig struct {
|
||||
SandboxContainerDir string // e.g. "/workspace"
|
||||
SandboxWorkspaceAccess string // "none", "ro", "rw"
|
||||
|
||||
// Shell deny group overrides (nil = all defaults)
|
||||
ShellDenyGroups map[string]bool
|
||||
|
||||
// Agent UUID for context propagation to tools
|
||||
AgentUUID uuid.UUID
|
||||
AgentType string // "open" or "predefined"
|
||||
@@ -319,6 +325,7 @@ func NewLoop(cfg LoopConfig) *Loop {
|
||||
sandboxEnabled: cfg.SandboxEnabled,
|
||||
sandboxContainerDir: cfg.SandboxContainerDir,
|
||||
sandboxWorkspaceAccess: cfg.SandboxWorkspaceAccess,
|
||||
shellDenyGroups: cfg.ShellDenyGroups,
|
||||
traceCollector: cfg.TraceCollector,
|
||||
inputGuard: guard,
|
||||
injectionAction: action,
|
||||
|
||||
@@ -348,6 +348,7 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc {
|
||||
SkillEvolve: ag.AgentType == "predefined" && ag.ParseSkillEvolve(),
|
||||
SkillNudgeInterval: ag.ParseSkillNudgeInterval(),
|
||||
WorkspaceSharing: ag.ParseWorkspaceSharing(),
|
||||
ShellDenyGroups: ag.ParseShellDenyGroups(),
|
||||
GroupWriterCache: deps.GroupWriterCache,
|
||||
TeamStore: deps.TeamStore,
|
||||
SecureCLIStore: deps.SecureCLIStore,
|
||||
|
||||
@@ -232,6 +232,21 @@ func (a *AgentData) ParseWorkspaceSharing() *WorkspaceSharingConfig {
|
||||
return cfg.WS
|
||||
}
|
||||
|
||||
// ParseShellDenyGroups extracts shell_deny_groups from other_config JSONB.
|
||||
// Returns nil if not configured (all defaults apply).
|
||||
func (a *AgentData) ParseShellDenyGroups() map[string]bool {
|
||||
if len(a.OtherConfig) == 0 {
|
||||
return nil
|
||||
}
|
||||
var cfg struct {
|
||||
ShellDenyGroups map[string]bool `json:"shell_deny_groups"`
|
||||
}
|
||||
if json.Unmarshal(a.OtherConfig, &cfg) != nil || len(cfg.ShellDenyGroups) == 0 {
|
||||
return nil
|
||||
}
|
||||
return cfg.ShellDenyGroups
|
||||
}
|
||||
|
||||
// AgentShareData represents an agent share grant.
|
||||
type AgentShareData struct {
|
||||
BaseModel
|
||||
|
||||
@@ -21,8 +21,13 @@ func TestResolveMediaPath(t *testing.T) {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Normalize paths to canonical form (resolves macOS /var/folders → /private/var/folders symlink).
|
||||
// The resolvePath function uses filepath.EvalSymlinks, so test expectations must too.
|
||||
testFileCanonical, _ := filepath.EvalSymlinks(testFile)
|
||||
workspaceCanonical, _ := filepath.EvalSymlinks(workspace)
|
||||
|
||||
t.Run("restricted", func(t *testing.T) {
|
||||
tool := NewMessageTool(workspace, true)
|
||||
tool := NewMessageTool(workspaceCanonical, true)
|
||||
ctx := context.Background()
|
||||
|
||||
tests := []struct {
|
||||
@@ -36,8 +41,8 @@ func TestResolveMediaPath(t *testing.T) {
|
||||
{"valid nested temp", "MEDIA:" + filepath.Join(tmpDir, "sub", "file.txt"), filepath.Join(tmpDir, "sub", "file.txt"), true},
|
||||
|
||||
// Workspace files allowed
|
||||
{"workspace absolute", "MEDIA:" + testFile, testFile, true},
|
||||
{"workspace relative", "MEDIA:docs/report.pdf", testFile, true},
|
||||
{"workspace absolute", "MEDIA:" + testFileCanonical, testFileCanonical, true},
|
||||
{"workspace relative", "MEDIA:docs/report.pdf", testFileCanonical, true},
|
||||
|
||||
// Not a MEDIA: message
|
||||
{"no prefix", filepath.Join(tmpDir, "test.png"), "", false},
|
||||
@@ -48,7 +53,7 @@ func TestResolveMediaPath(t *testing.T) {
|
||||
|
||||
// Outside workspace + outside /tmp/ → blocked
|
||||
{"outside workspace", "MEDIA:/etc/passwd", "", false},
|
||||
{"traversal attack", "MEDIA:" + filepath.Join(workspace, "..", "etc", "passwd"), "", false},
|
||||
{"traversal attack", "MEDIA:" + filepath.Join(workspaceCanonical, "..", "etc", "passwd"), "", false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
@@ -91,14 +96,14 @@ func TestResolveMediaPath(t *testing.T) {
|
||||
t.Run("context workspace override", func(t *testing.T) {
|
||||
// Tool has no workspace, but context provides one.
|
||||
tool := NewMessageTool("", true)
|
||||
ctx := WithToolWorkspace(context.Background(), workspace)
|
||||
ctx := WithToolWorkspace(context.Background(), workspaceCanonical)
|
||||
|
||||
got, ok := tool.resolveMediaPath(ctx, "MEDIA:docs/report.pdf")
|
||||
if !ok {
|
||||
t.Fatal("expected ok=true for workspace-relative path with context workspace")
|
||||
}
|
||||
if got != testFile {
|
||||
t.Errorf("got %q, want %q", got, testFile)
|
||||
if got != testFileCanonical {
|
||||
t.Errorf("got %q, want %q", got, testFileCanonical)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user