From 5f2cc6cf57b328ca19bfda003a94b504105b45d1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 May 2026 05:51:12 +0000 Subject: [PATCH 1/2] Initial plan From 19a993b2a85f6bc56c8cc7363ab95f4166b5e833 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 May 2026 06:01:59 +0000 Subject: [PATCH 2/2] fix: migrate pkg logging identifiers for pkg/logger compliance Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/mcp_server_http.go | 7 ++++--- pkg/envutil/envutil.go | 10 +++++----- pkg/fileutil/fileutil.go | 24 ++++++++++++------------ pkg/gitutil/gitutil.go | 18 +++++++++--------- pkg/repoutil/repoutil.go | 8 ++++---- pkg/semverutil/semverutil.go | 14 +++++++------- pkg/typeutil/convert.go | 8 ++++---- pkg/workflow/create_entity_helpers.go | 16 ++++++++-------- pkg/workflow/parse_helpers.go | 14 +++++++------- 9 files changed, 60 insertions(+), 59 deletions(-) diff --git a/pkg/cli/mcp_server_http.go b/pkg/cli/mcp_server_http.go index 417fc6fdba8..9f3e2b4f74d 100644 --- a/pkg/cli/mcp_server_http.go +++ b/pkg/cli/mcp_server_http.go @@ -2,7 +2,6 @@ package cli import ( "fmt" - "log" "net/http" "os" "strings" @@ -13,6 +12,8 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" ) +var mcpHTTPLog = logger.New("cli:mcp_server_http") + // sanitizeForLog removes newline and carriage return characters from user input // to prevent log injection attacks where malicious users could forge log entries. func sanitizeForLog(input string) string { @@ -39,7 +40,7 @@ func loggingHandler(handler http.Handler) http.Handler { sanitizedPath := sanitizeForLog(r.URL.Path) // Log request details. - log.Printf("[REQUEST] %s | %s | %s %s", + mcpHTTPLog.Printf("[REQUEST] %s | %s | %s %s", start.Format(time.RFC3339), r.RemoteAddr, r.Method, @@ -50,7 +51,7 @@ func loggingHandler(handler http.Handler) http.Handler { // Log response details. duration := time.Since(start) - log.Printf("[RESPONSE] %s | %s | %s %s | Status: %d | Duration: %v", + mcpHTTPLog.Printf("[RESPONSE] %s | %s | %s %s | Status: %d | Duration: %v", time.Now().Format(time.RFC3339), r.RemoteAddr, r.Method, diff --git a/pkg/envutil/envutil.go b/pkg/envutil/envutil.go index 90b9ef85573..c1d28cf9756 100644 --- a/pkg/envutil/envutil.go +++ b/pkg/envutil/envutil.go @@ -27,10 +27,10 @@ import ( // - Value is outside the [minValue, maxValue] range // // Invalid values trigger warning messages to stderr, or through the logger if provided. -func GetIntFromEnv(envVar string, defaultValue, minValue, maxValue int, log *logger.Logger) int { +func GetIntFromEnv(envVar string, defaultValue, minValue, maxValue int, debugLog *logger.Logger) int { warn := func(msg string) { - if log != nil { - log.Printf("WARNING: %s", msg) + if debugLog != nil { + debugLog.Printf("WARNING: %s", msg) } else { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg)) } @@ -52,8 +52,8 @@ func GetIntFromEnv(envVar string, defaultValue, minValue, maxValue int, log *log return defaultValue } - if log != nil { - log.Printf("Using %s=%d", envVar, val) + if debugLog != nil { + debugLog.Printf("Using %s=%d", envVar, val) } return val } diff --git a/pkg/fileutil/fileutil.go b/pkg/fileutil/fileutil.go index 34419bc6d86..3259f2b0d91 100644 --- a/pkg/fileutil/fileutil.go +++ b/pkg/fileutil/fileutil.go @@ -11,7 +11,7 @@ import ( "github.com/github/gh-aw/pkg/logger" ) -var log = logger.New("fileutil:fileutil") +var fileutilLog = logger.New("fileutil:fileutil") // ValidateAbsolutePath validates that a file path is absolute and safe to use. // It performs the following security checks: @@ -37,7 +37,7 @@ var log = logger.New("fileutil:fileutil") func ValidateAbsolutePath(path string) (string, error) { // Check for empty path if path == "" { - log.Print("ValidateAbsolutePath: rejected empty path") + fileutilLog.Print("ValidateAbsolutePath: rejected empty path") return "", errors.New("path cannot be empty") } @@ -46,11 +46,11 @@ func ValidateAbsolutePath(path string) (string, error) { // Verify the path is absolute to prevent relative path traversal if !filepath.IsAbs(cleanPath) { - log.Printf("ValidateAbsolutePath: rejected relative path: %s", path) + fileutilLog.Printf("ValidateAbsolutePath: rejected relative path: %s", path) return "", fmt.Errorf("path must be absolute, got: %s", path) } - log.Printf("ValidateAbsolutePath: validated path: %s", cleanPath) + fileutilLog.Printf("ValidateAbsolutePath: validated path: %s", cleanPath) return cleanPath, nil } @@ -63,7 +63,7 @@ func ValidateAbsolutePath(path string) (string, error) { // - Either path cannot be resolved to an absolute form. // - The resolved candidate path starts outside the resolved base directory. func ValidatePathWithinBase(base, candidate string) error { - log.Printf("ValidatePathWithinBase: checking candidate=%q within base=%q", candidate, base) + fileutilLog.Printf("ValidatePathWithinBase: checking candidate=%q within base=%q", candidate, base) // EvalSymlinks resolves both symlinks and ".." components. // Fall back to Abs when a path does not exist on disk yet. absBase, err := filepath.EvalSymlinks(base) @@ -82,10 +82,10 @@ func ValidatePathWithinBase(base, candidate string) error { } rel, err := filepath.Rel(absBase, absCand) if err != nil || !filepath.IsLocal(rel) { - log.Printf("ValidatePathWithinBase: path escape detected: candidate=%q base=%q", candidate, base) + fileutilLog.Printf("ValidatePathWithinBase: path escape detected: candidate=%q base=%q", candidate, base) return fmt.Errorf("path %q escapes base directory %q", candidate, base) } - log.Printf("ValidatePathWithinBase: path is safe: candidate=%q (rel=%s) within base=%q", candidate, rel, base) + fileutilLog.Printf("ValidatePathWithinBase: path is safe: candidate=%q (rel=%s) within base=%q", candidate, rel, base) return nil } @@ -131,7 +131,7 @@ func copyFileContents(in io.Reader, out syncWriteCloser, dst string) (err error) } if removePartial { if removeErr := os.Remove(dst); removeErr != nil { - log.Printf("Failed to remove partial destination file during cleanup: %s", removeErr) + fileutilLog.Printf("Failed to remove partial destination file during cleanup: %s", removeErr) } } }() @@ -146,17 +146,17 @@ func copyFileContents(in io.Reader, out syncWriteCloser, dst string) (err error) // CopyFile copies a file from src to dst using buffered IO. func CopyFile(src, dst string) error { - log.Printf("Copying file: src=%s, dst=%s", src, dst) + fileutilLog.Printf("Copying file: src=%s, dst=%s", src, dst) in, err := os.Open(src) if err != nil { - log.Printf("Failed to open source file: %s", err) + fileutilLog.Printf("Failed to open source file: %s", err) return err } defer in.Close() out, err := os.Create(dst) if err != nil { - log.Printf("Failed to create destination file: %s", err) + fileutilLog.Printf("Failed to create destination file: %s", err) return err } err = copyFileContents(in, out, dst) @@ -164,6 +164,6 @@ func CopyFile(src, dst string) error { return err } - log.Printf("File copied successfully: src=%s, dst=%s", src, dst) + fileutilLog.Printf("File copied successfully: src=%s, dst=%s", src, dst) return nil } diff --git a/pkg/gitutil/gitutil.go b/pkg/gitutil/gitutil.go index 4646a880c93..00b80b20014 100644 --- a/pkg/gitutil/gitutil.go +++ b/pkg/gitutil/gitutil.go @@ -12,7 +12,7 @@ import ( "github.com/github/gh-aw/pkg/logger" ) -var log = logger.New("gitutil:gitutil") +var gitutilLog = logger.New("gitutil:gitutil") var ErrNotGitRepository = errors.New("not in a git repository") var fullSHARegex = regexp.MustCompile(`^[0-9a-f]{40}$`) @@ -30,7 +30,7 @@ func IsRateLimitError(errMsg string) bool { // IsAuthError checks if an error message indicates an authentication issue. // This is used to detect when GitHub API calls fail due to missing or invalid credentials. func IsAuthError(errMsg string) bool { - log.Printf("Checking if error is auth-related: %s", errMsg) + gitutilLog.Printf("Checking if error is auth-related: %s", errMsg) lowerMsg := strings.ToLower(errMsg) isAuth := strings.Contains(lowerMsg, "gh_token") || strings.Contains(lowerMsg, "github_token") || @@ -41,7 +41,7 @@ func IsAuthError(errMsg string) bool { strings.Contains(lowerMsg, "permission denied") || strings.Contains(lowerMsg, "saml enforcement") if isAuth { - log.Print("Detected authentication error") + gitutilLog.Print("Detected authentication error") } return isAuth } @@ -83,21 +83,21 @@ func ExtractBaseRepo(repoPath string) string { // environments where git is not on PATH. // Returns an error if not in a git repository. func FindGitRoot() (string, error) { - log.Print("Finding git root directory") + gitutilLog.Print("Finding git root directory") dir, err := os.Getwd() if err != nil { - log.Printf("Failed to get current directory: %v", err) + gitutilLog.Printf("Failed to get current directory: %v", err) return "", fmt.Errorf("failed to get current directory: %w", err) } root, err := FindGitRootFrom(dir) if err != nil { - log.Printf("Failed to find git root: %v", err) + gitutilLog.Printf("Failed to find git root: %v", err) return "", err } - log.Printf("Found git root: %s", root) + gitutilLog.Printf("Found git root: %s", root) return root, nil } @@ -171,12 +171,12 @@ func ReadFileFromHEAD(filePath, gitRoot string) (string, error) { relPath = filepath.ToSlash(relPath) - log.Printf("Reading %q from git HEAD (relative path: %s)", filePath, relPath) + gitutilLog.Printf("Reading %q from git HEAD (relative path: %s)", filePath, relPath) cmd := exec.Command("git", "-C", gitRoot, "show", "HEAD:"+relPath) output, err := cmd.Output() if err != nil { - log.Printf("File %q not found in HEAD commit: %v", filePath, err) + gitutilLog.Printf("File %q not found in HEAD commit: %v", filePath, err) return "", fmt.Errorf("file %q not found in HEAD commit: %w", filePath, err) } return string(output), nil diff --git a/pkg/repoutil/repoutil.go b/pkg/repoutil/repoutil.go index ddd7ed882e0..c0a3369da4e 100644 --- a/pkg/repoutil/repoutil.go +++ b/pkg/repoutil/repoutil.go @@ -8,17 +8,17 @@ import ( "github.com/github/gh-aw/pkg/logger" ) -var log = logger.New("repoutil:repoutil") +var repoutilLog = logger.New("repoutil:repoutil") // SplitRepoSlug splits a repository slug (owner/repo) into owner and repo parts. // Returns an error if the slug format is invalid. func SplitRepoSlug(slug string) (owner, repo string, err error) { - log.Printf("Splitting repo slug: %s", slug) + repoutilLog.Printf("Splitting repo slug: %s", slug) parts := strings.Split(slug, "/") if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - log.Printf("Invalid repo slug format: %s", slug) + repoutilLog.Printf("Invalid repo slug format: %s", slug) return "", "", fmt.Errorf("invalid repo format: %s", slug) } - log.Printf("Split result: owner=%s, repo=%s", parts[0], parts[1]) + repoutilLog.Printf("Split result: owner=%s, repo=%s", parts[0], parts[1]) return parts[0], parts[1], nil } diff --git a/pkg/semverutil/semverutil.go b/pkg/semverutil/semverutil.go index b3d5fd73e7b..592dd9c0370 100644 --- a/pkg/semverutil/semverutil.go +++ b/pkg/semverutil/semverutil.go @@ -16,7 +16,7 @@ import ( "golang.org/x/mod/semver" ) -var log = logger.New("semverutil:semverutil") +var semverLog = logger.New("semverutil:semverutil") // actionVersionTagRegex matches version tags: vmajor, vmajor.minor, or vmajor.minor.patch. // It intentionally excludes prerelease and build-metadata suffixes because GitHub Actions @@ -60,11 +60,11 @@ func IsValid(ref string) bool { // ParseVersion parses v into a SemanticVersion. // It returns nil if v is not a valid semantic version string. func ParseVersion(v string) *SemanticVersion { - log.Printf("Parsing semantic version: %s", v) + semverLog.Printf("Parsing semantic version: %s", v) v = EnsureVPrefix(v) if !semver.IsValid(v) { - log.Printf("Invalid semantic version: %s", v) + semverLog.Printf("Invalid semantic version: %s", v) return nil } @@ -107,11 +107,11 @@ func Compare(v1, v2 string) int { result := semver.Compare(v1, v2) if result > 0 { - log.Printf("Version comparison result: %s > %s", v1, v2) + semverLog.Printf("Version comparison result: %s > %s", v1, v2) } else if result < 0 { - log.Printf("Version comparison result: %s < %s", v1, v2) + semverLog.Printf("Version comparison result: %s < %s", v1, v2) } else { - log.Printf("Version comparison result: %s == %s", v1, v2) + semverLog.Printf("Version comparison result: %s == %s", v1, v2) } return result @@ -146,7 +146,7 @@ func IsCompatible(pinVersion, requestedVersion string) bool { requestedMajor := semver.Major(requestedVersion) compatible := pinMajor == requestedMajor - log.Printf("Checking semver compatibility: pin=%s (major=%s), requested=%s (major=%s) -> %v", + semverLog.Printf("Checking semver compatibility: pin=%s (major=%s), requested=%s (major=%s) -> %v", pinVersion, pinMajor, requestedVersion, requestedMajor, compatible) return compatible diff --git a/pkg/typeutil/convert.go b/pkg/typeutil/convert.go index 29366c6e6d3..522c60c525e 100644 --- a/pkg/typeutil/convert.go +++ b/pkg/typeutil/convert.go @@ -33,7 +33,7 @@ import ( "github.com/github/gh-aw/pkg/logger" ) -var log = logger.New("typeutil:convert") +var typeutilLog = logger.New("typeutil:convert") // ParseIntValue strictly parses numeric types (int, int64, uint64, float64) to int, // returning (value, true) on success and (0, false) for any unrecognized or @@ -55,7 +55,7 @@ func ParseIntValue(value any) (int, bool) { // Check for overflow before converting uint64 to int const maxInt = int(^uint(0) >> 1) if v > uint64(maxInt) { - log.Printf("uint64 value %d exceeds max int value, returning 0", v) + typeutilLog.Printf("uint64 value %d exceeds max int value, returning 0", v) return 0, false } return int(v), true @@ -63,7 +63,7 @@ func ParseIntValue(value any) (int, bool) { intVal := int(v) // Warn if truncation occurs (value has fractional part) if v != float64(intVal) { - log.Printf("Float value %.2f truncated to integer %d", v, intVal) + typeutilLog.Printf("Float value %.2f truncated to integer %d", v, intVal) } return intVal, true default: @@ -102,7 +102,7 @@ func ConvertToInt(val any) int { intVal := int(v) // Warn if truncation occurs (value has fractional part) if v != float64(intVal) { - log.Printf("Float value %.2f truncated to integer %d", v, intVal) + typeutilLog.Printf("Float value %.2f truncated to integer %d", v, intVal) } return intVal case string: diff --git a/pkg/workflow/create_entity_helpers.go b/pkg/workflow/create_entity_helpers.go index b686e5f4181..5074ea32d7a 100644 --- a/pkg/workflow/create_entity_helpers.go +++ b/pkg/workflow/create_entity_helpers.go @@ -19,7 +19,7 @@ type CreateParseOptions struct { // - outputMap: full safe-output map from frontmatter parsing. // - configKey: create-* key to parse (for example "create-issue"). // - opts: shared preprocessing configuration for bool/int/expires fields. -// - log: logger used for preprocessing and parse diagnostics. +// - debugLog: logger used for preprocessing and parse diagnostics. // - onError: required error handler invoked on unmarshal failures. // // Callback lifecycle: @@ -33,7 +33,7 @@ func parseCreateEntityConfig[T any]( outputMap map[string]any, configKey string, opts CreateParseOptions, - log *logger.Logger, + debugLog *logger.Logger, onError func(error) *T, preUnmarshal func(map[string]any) bool, postUnmarshal func(map[string]any, *T, bool), @@ -53,24 +53,24 @@ func parseCreateEntityConfig[T any]( expiresDisabled := false if opts.HandleExpires { - expiresDisabled = preprocessExpiresField(configData, log) + expiresDisabled = preprocessExpiresField(configData, debugLog) } for _, field := range opts.BoolFields { - if err := preprocessBoolFieldAsString(configData, field, log); err != nil { - log.Printf("Invalid %s value: %v", field, err) + if err := preprocessBoolFieldAsString(configData, field, debugLog); err != nil { + debugLog.Printf("Invalid %s value: %v", field, err) return nil } } for _, field := range opts.IntFields { - if err := preprocessIntFieldAsString(configData, field, log); err != nil { - log.Printf("Invalid %s value: %v", field, err) + if err := preprocessIntFieldAsString(configData, field, debugLog); err != nil { + debugLog.Printf("Invalid %s value: %v", field, err) return nil } } - config := parseConfigScaffold(outputMap, configKey, log, onError) + config := parseConfigScaffold(outputMap, configKey, debugLog, onError) if config == nil { return nil } diff --git a/pkg/workflow/parse_helpers.go b/pkg/workflow/parse_helpers.go index 5def58b069c..c625eed82f3 100644 --- a/pkg/workflow/parse_helpers.go +++ b/pkg/workflow/parse_helpers.go @@ -21,8 +21,8 @@ import "github.com/github/gh-aw/pkg/logger" // so YAML unmarshaling into []string fields succeeds for single-value shorthand. // // When key is missing, nil, or already a non-string type, this function is a no-op. -// The log parameter is optional; pass nil to suppress debug output. -func coerceStringOrArrayField(configData map[string]any, key string, log *logger.Logger) { +// The debugLog parameter is optional; pass nil to suppress debug output. +func coerceStringOrArrayField(configData map[string]any, key string, debugLog *logger.Logger) { if configData == nil { return } @@ -30,17 +30,17 @@ func coerceStringOrArrayField(configData map[string]any, key string, log *logger if value, exists := configData[key]; exists { if stringValue, ok := value.(string); ok { configData[key] = []string{stringValue} - if log != nil { - log.Printf("Converted single %s string to array before unmarshaling", key) + if debugLog != nil { + debugLog.Printf("Converted single %s string to array before unmarshaling", key) } } } } // coerceStringOrArrayFields applies coerceStringOrArrayField to multiple keys. -func coerceStringOrArrayFields(configData map[string]any, keys []string, log *logger.Logger) { +func coerceStringOrArrayFields(configData map[string]any, keys []string, debugLog *logger.Logger) { for _, key := range keys { - coerceStringOrArrayField(configData, key, log) + coerceStringOrArrayField(configData, key, debugLog) } } @@ -58,7 +58,7 @@ func coerceStringOrArrayFields(configData map[string]any, keys []string, log *lo // - The extracted exclude slice is returned so callers can store it in the config struct // // When the string form is encountered the field is left unchanged and nil is returned. -// The log parameter is optional; pass nil to suppress debug output. +// The debugLog parameter is optional; pass nil to suppress debug output. func preprocessProtectedFilesField(configData map[string]any, debugLog *logger.Logger) []string { if configData == nil { return nil