From 58cf0c40d64307ee91958f8c70b244b00786a7d6 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 19 Mar 2026 20:07:59 +0100 Subject: [PATCH] Simplify pkg/permissions: extract helper, reduce type cases, remove redundant checks - Extract matchAny helper to DRY up the three identical loops in CheckWithArgs - Simplify argToString by removing redundant bool/int/int64 cases (handled by default) - Collapse float64 branch to a single fmt.Sprintf(%g) call, drop strconv import - Remove redundant nil args guard in matchToolPattern (nil map indexing is safe) - Fold escaped-asterisk check into ContainsAny in matchGlob Assisted-By: docker-agent --- pkg/permissions/permissions.go | 52 ++++++++++++++-------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/pkg/permissions/permissions.go b/pkg/permissions/permissions.go index 66f8eee70..5653e9b37 100644 --- a/pkg/permissions/permissions.go +++ b/pkg/permissions/permissions.go @@ -5,7 +5,6 @@ package permissions import ( "fmt" "path/filepath" - "strconv" "strings" "github.com/docker/docker-agent/pkg/config/latest" @@ -85,30 +84,34 @@ func (c *Checker) Check(toolName string) Decision { // (e.g. read-only tools). Note that --yolo mode takes precedence over ForceAsk. func (c *Checker) CheckWithArgs(toolName string, args map[string]any) Decision { // Deny patterns are checked first - they take priority - for _, pattern := range c.denyPatterns { - if matchToolPattern(pattern, toolName, args) { - return Deny - } + if matchAny(c.denyPatterns, toolName, args) { + return Deny } // Allow patterns are checked second - for _, pattern := range c.allowPatterns { - if matchToolPattern(pattern, toolName, args) { - return Allow - } + if matchAny(c.allowPatterns, toolName, args) { + return Allow } // Explicit ask patterns override auto-approval (e.g. read-only hints) - for _, pattern := range c.askPatterns { - if matchToolPattern(pattern, toolName, args) { - return ForceAsk - } + if matchAny(c.askPatterns, toolName, args) { + return ForceAsk } // Default is Ask return Ask } +// matchAny reports whether any pattern in the list matches the tool name and args. +func matchAny(patterns []string, toolName string, args map[string]any) bool { + for _, pattern := range patterns { + if matchToolPattern(pattern, toolName, args) { + return true + } + } + return false +} + // IsEmpty returns true if no permissions are configured func (c *Checker) IsEmpty() bool { return len(c.allowPatterns) == 0 && len(c.askPatterns) == 0 && len(c.denyPatterns) == 0 @@ -176,12 +179,7 @@ func matchToolPattern(pattern, toolName string, args map[string]any) bool { return true } - // If pattern has argument conditions but no args provided, no match - if args == nil { - return false - } - - // All argument patterns must match + // All argument patterns must match (indexing a nil args map is safe in Go) for argName, argPattern := range argPatterns { argValue, exists := args[argName] if !exists { @@ -203,16 +201,9 @@ func argToString(v any) string { switch val := v.(type) { case string: return val - case bool: - return strconv.FormatBool(val) case float64: - // JSON numbers are float64 - format without trailing zeros - if val == float64(int64(val)) { - return strconv.FormatInt(int64(val), 10) - } + // JSON numbers are float64 - use %g for shortest representation return fmt.Sprintf("%g", val) - case int, int64: - return fmt.Sprintf("%d", val) default: return fmt.Sprintf("%v", v) } @@ -237,10 +228,11 @@ func matchGlob(pattern, value string) bool { // Handle trailing wildcard for prefix matching // This allows "sudo*" to match "sudo rm -rf /" - if strings.HasSuffix(pattern, "*") && !strings.HasSuffix(pattern, "\\*") { + if strings.HasSuffix(pattern, "*") { prefix := pattern[:len(pattern)-1] - // If prefix contains no other glob characters, do simple prefix match - if !strings.ContainsAny(prefix, "*?[") { + // If prefix contains no other glob characters, do simple prefix match. + // Including \ catches escaped asterisks (e.g. "foo\*"). + if !strings.ContainsAny(prefix, `*?[\`) { return strings.HasPrefix(value, prefix) } }