feat: add exec admin support for unrestricted shell command execution
- Add exec_admins config option to specify admin identities - Add ExecuteUnrestricted method to ExecTool for admin override - Update agent loop to check sender identity for exec admin permission - Exec admins bypass all command guards including dangerous command checks
This commit is contained in:
parent
1f2802906f
commit
e3416662dd
5 changed files with 68 additions and 24 deletions
|
|
@ -424,8 +424,9 @@
|
|||
"^unzip\\b",
|
||||
"^tar\\b",
|
||||
"^git\\b",
|
||||
"^rm\\s+(?!.*-[rR])\\S"
|
||||
]
|
||||
"^rm\\s+[^-]"
|
||||
],
|
||||
"exec_admins": []
|
||||
},
|
||||
"skills": {
|
||||
"enabled": true,
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ import (
|
|||
"github.com/sipeed/picoclaw/pkg/commands"
|
||||
"github.com/sipeed/picoclaw/pkg/config"
|
||||
"github.com/sipeed/picoclaw/pkg/constants"
|
||||
"github.com/sipeed/picoclaw/pkg/identity"
|
||||
"github.com/sipeed/picoclaw/pkg/logger"
|
||||
"github.com/sipeed/picoclaw/pkg/mcp"
|
||||
"github.com/sipeed/picoclaw/pkg/media"
|
||||
|
|
@ -64,16 +65,17 @@ type AgentLoop struct {
|
|||
|
||||
// processOptions configures how a message is processed
|
||||
type processOptions struct {
|
||||
SessionKey string // Session identifier for history/context
|
||||
Channel string // Target channel for tool execution
|
||||
ChatID string // Target chat ID for tool execution
|
||||
UserMessage string // User message content (may include prefix)
|
||||
Media []string // media:// refs from inbound message
|
||||
DefaultResponse string // Response when LLM returns empty
|
||||
EnableSummary bool // Whether to trigger summarization
|
||||
SendResponse bool // Whether to send response via bus
|
||||
NoHistory bool // If true, don't load session history (for heartbeat)
|
||||
WorkingDir string // Current working directory override (for hipico from cmd mode)
|
||||
SessionKey string // Session identifier for history/context
|
||||
Channel string // Target channel for tool execution
|
||||
ChatID string // Target chat ID for tool execution
|
||||
UserMessage string // User message content (may include prefix)
|
||||
Media []string // media:// refs from inbound message
|
||||
DefaultResponse string // Response when LLM returns empty
|
||||
EnableSummary bool // Whether to trigger summarization
|
||||
SendResponse bool // Whether to send response via bus
|
||||
NoHistory bool // If true, don't load session history (for heartbeat)
|
||||
WorkingDir string // Current working directory override (for hipico from cmd mode)
|
||||
Sender bus.SenderInfo // Sender identity for per-user permission checks
|
||||
}
|
||||
|
||||
const (
|
||||
|
|
@ -722,6 +724,7 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage)
|
|||
EnableSummary: true,
|
||||
SendResponse: false,
|
||||
WorkingDir: msg.Metadata["work_dir"],
|
||||
Sender: msg.Sender,
|
||||
}
|
||||
|
||||
// In cmd mode, rewrite bare text as /exec so all shell execution flows
|
||||
|
|
@ -1962,7 +1965,8 @@ func (al *AgentLoop) buildCommandsRuntime(agent *AgentInstance, opts *processOpt
|
|||
if agent == nil {
|
||||
return "", fmt.Errorf("no agent available")
|
||||
}
|
||||
return al.executeCmdMode(ctx, agent, command, opts.SessionKey, opts.Channel, opts.ChatID)
|
||||
isAdmin := al.isExecAdmin(opts.Sender)
|
||||
return al.executeCmdMode(ctx, agent, command, opts.SessionKey, opts.Channel, opts.ChatID, isAdmin)
|
||||
},
|
||||
|
||||
// One-shot AI query for /hipico (stays in modeCmd, separate session key)
|
||||
|
|
@ -2025,12 +2029,26 @@ func mapCommandError(result commands.ExecuteResult) string {
|
|||
return fmt.Sprintf("Failed to execute /%s: %v", result.Command, result.Err)
|
||||
}
|
||||
|
||||
// isExecAdmin returns true if the sender is listed in tools.exec.exec_admins,
|
||||
// using the same identity matching as channel allow_from.
|
||||
func (al *AgentLoop) isExecAdmin(sender bus.SenderInfo) bool {
|
||||
admins := al.cfg.Tools.Exec.ExecAdmins
|
||||
for _, admin := range admins {
|
||||
if identity.MatchAllowed(sender, admin) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// executeCmdMode executes a shell command in command mode via ExecTool.
|
||||
// Output is formatted as a console code block for channel display.
|
||||
// isAdmin bypasses all command guards for verified exec admins.
|
||||
func (al *AgentLoop) executeCmdMode(
|
||||
ctx context.Context,
|
||||
agent *AgentInstance,
|
||||
content, sessionKey, channel, chatID string,
|
||||
isAdmin bool,
|
||||
) (string, error) {
|
||||
content = strings.TrimSpace(content)
|
||||
if content == "" {
|
||||
|
|
@ -2053,11 +2071,23 @@ func (al *AgentLoop) executeCmdMode(
|
|||
workDir = agent.Workspace
|
||||
}
|
||||
|
||||
// Execute via ExecTool
|
||||
result := agent.Tools.ExecuteWithContext(ctx, "exec", map[string]any{
|
||||
execArgs := map[string]any{
|
||||
"command": content,
|
||||
"working_dir": workDir,
|
||||
}, channel, chatID, nil)
|
||||
}
|
||||
|
||||
// Execute via ExecTool — admins bypass command guards
|
||||
var result *tools.ToolResult
|
||||
if isAdmin {
|
||||
if t, ok := agent.Tools.Get("exec"); ok {
|
||||
if et, ok := t.(*tools.ExecTool); ok {
|
||||
result = et.ExecuteUnrestricted(ctx, execArgs)
|
||||
}
|
||||
}
|
||||
}
|
||||
if result == nil {
|
||||
result = agent.Tools.ExecuteWithContext(ctx, "exec", execArgs, channel, chatID, nil)
|
||||
}
|
||||
|
||||
displayDir := shortenHomePath(workDir)
|
||||
output := result.ForLLM
|
||||
|
|
|
|||
|
|
@ -678,6 +678,9 @@ type ExecConfig struct {
|
|||
// When non-empty and DevMode is false, only commands matching at least one pattern are permitted;
|
||||
// deny patterns are bypassed — the whitelist is the sole access control.
|
||||
AllowedCommands []string `env:"PICOCLAW_TOOLS_EXEC_ALLOWED_COMMANDS" json:"allowed_commands"`
|
||||
// ExecAdmins lists sender identities (same format as channel allow_from) that are granted
|
||||
// unrestricted shell execution regardless of DevMode or AllowedCommands.
|
||||
ExecAdmins []string `env:"PICOCLAW_TOOLS_EXEC_ADMINS" json:"exec_admins"`
|
||||
}
|
||||
|
||||
type SkillsToolsConfig struct {
|
||||
|
|
|
|||
|
|
@ -179,7 +179,17 @@ func (t *ExecTool) Parameters() map[string]any {
|
|||
}
|
||||
}
|
||||
|
||||
// ExecuteUnrestricted executes a command bypassing all command guards.
|
||||
// Use only after the caller has verified the sender is an exec admin.
|
||||
func (t *ExecTool) ExecuteUnrestricted(ctx context.Context, args map[string]any) *ToolResult {
|
||||
return t.execute(ctx, args, true)
|
||||
}
|
||||
|
||||
func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult {
|
||||
return t.execute(ctx, args, false)
|
||||
}
|
||||
|
||||
func (t *ExecTool) execute(ctx context.Context, args map[string]any, adminOverride bool) *ToolResult {
|
||||
command, ok := args["command"].(string)
|
||||
if !ok {
|
||||
return ErrorResult("command is required")
|
||||
|
|
@ -205,7 +215,7 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
|
|||
}
|
||||
}
|
||||
|
||||
if guardError := t.guardCommand(command, cwd); guardError != "" {
|
||||
if guardError := t.guardCommand(command, cwd, adminOverride); guardError != "" {
|
||||
return ErrorResult(guardError)
|
||||
}
|
||||
|
||||
|
|
@ -310,9 +320,9 @@ func sanitizeCommand(cmd string) string {
|
|||
return cmd
|
||||
}
|
||||
|
||||
func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||
// Dev mode: no restrictions at all.
|
||||
if t.devMode {
|
||||
func (t *ExecTool) guardCommand(command, cwd string, adminOverride bool) string {
|
||||
// Dev mode or exec admin: no restrictions at all.
|
||||
if t.devMode || adminOverride {
|
||||
return ""
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -369,7 +369,7 @@ func TestGuardCommand_URLEncodedTraversal(t *testing.T) {
|
|||
}
|
||||
tool.SetRestrictToWorkspace(true)
|
||||
|
||||
msg := tool.guardCommand("cat %2e%2e%2f%2e%2e%2fetc/passwd", tmpDir)
|
||||
msg := tool.guardCommand("cat %2e%2e%2f%2e%2e%2fetc/passwd", tmpDir, false)
|
||||
if msg == "" {
|
||||
t.Error("Expected URL-encoded path traversal to be blocked")
|
||||
}
|
||||
|
|
@ -385,7 +385,7 @@ func TestGuardCommand_NullByte(t *testing.T) {
|
|||
}
|
||||
tool.SetRestrictToWorkspace(true)
|
||||
|
||||
msg := tool.guardCommand("cat foo\x00../../etc/passwd", tmpDir)
|
||||
msg := tool.guardCommand("cat foo\x00../../etc/passwd", tmpDir, false)
|
||||
if msg == "" {
|
||||
t.Error("Expected null-byte traversal to be blocked")
|
||||
}
|
||||
|
|
@ -402,7 +402,7 @@ func TestGuardCommand_SuBlocked(t *testing.T) {
|
|||
|
||||
cases := []string{"su", "su -", "su root", "doas ls", "pkexec /bin/bash"}
|
||||
for _, cmd := range cases {
|
||||
msg := tool.guardCommand(cmd, tmpDir)
|
||||
msg := tool.guardCommand(cmd, tmpDir, false)
|
||||
if msg == "" {
|
||||
t.Errorf("Expected %q to be blocked", cmd)
|
||||
}
|
||||
|
|
@ -420,7 +420,7 @@ func TestGuardCommand_SuNoFalsePositive(t *testing.T) {
|
|||
|
||||
cases := []string{"echo surplus", "cat summary.txt", "ls result/"}
|
||||
for _, cmd := range cases {
|
||||
msg := tool.guardCommand(cmd, tmpDir)
|
||||
msg := tool.guardCommand(cmd, tmpDir, false)
|
||||
if msg != "" {
|
||||
t.Errorf("Expected %q to NOT be blocked, got: %s", cmd, msg)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue