From 0a56316c4cfefecfcbd423281fc1725101552c09 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 01:24:53 +0000 Subject: [PATCH 1/3] Initial plan From 7604ca0e39ec3bda49c248e7f9139104d6adac4e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 01:32:22 +0000 Subject: [PATCH 2/3] chore: initial plan for jsonmarshalignoredeerror triage Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .github/workflows/daily-model-inventory.lock.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/daily-model-inventory.lock.yml b/.github/workflows/daily-model-inventory.lock.yml index 4ad7596a490..89bcc656530 100644 --- a/.github/workflows/daily-model-inventory.lock.yml +++ b/.github/workflows/daily-model-inventory.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"941bf2df6cf246f352cfb0607cd13f711a6fa33bb44bfd133f7ff0ccd4f3aa33","body_hash":"eddd84cb14b52557f5fd4337e9e7d5fcf36fa38a1bbe0e74d773293aa86b64b6","strict":true,"agent_id":"copilot"} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"941bf2df6cf246f352cfb0607cd13f711a6fa33bb44bfd133f7ff0ccd4f3aa33","body_hash":"1411aa03bf28a3c98eff9d54cdee03ed6e87584a3b11c9f6dd740cb2b466ea00","strict":true,"agent_id":"copilot"} # gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GEMINI_API_KEY","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN","OPENAI_API_KEY"],"actions":[{"repo":"actions/checkout","sha":"de0fac2e4500dabe0009e67214ff5f5447ce83dd","version":"v6.0.2"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e","version":"v6.4.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.25.58"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.25.58"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.25.58"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.3.22"},{"image":"ghcr.io/github/github-mcp-server:v1.1.0"},{"image":"node:lts-alpine","digest":"sha256:2bdb65ed1dab192432bc31c95f94155ca5ad7fc1392fb7eb7526ab682fa5bf14","pinned_image":"node:lts-alpine@sha256:2bdb65ed1dab192432bc31c95f94155ca5ad7fc1392fb7eb7526ab682fa5bf14"}]} # ___ _ _ # / _ \ | | (_) From 6a9c222adcd77c021aa25521187336f93eb3b187 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 01:36:01 +0000 Subject: [PATCH 3/3] fix: triage jsonmarshalignoredeerror violations and enforce linter in CI - Tier 1: properly handle json.Marshal error in safe_outputs_config_generation.go - Add //nolint:jsonmarshalignoredeerror to all provably-safe sites (string, []string, VSCodeMCPServer, ExperimentConfig marshal calls) - Add nolint directive support to jsonmarshalignoredeerror linter (matching the errstringmatch pattern using internal/nolint package) - Add Suppressed() test case to linter testdata to cover nolint behavior - Enforce linter by adding -jsonmarshalignoredeerror to LINTER_FLAGS in cgo.yml Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .github/workflows/cgo.yml | 2 +- pkg/cli/mcp_config_file.go | 4 ++-- .../jsonmarshalignoredeerror.go | 11 +++++++++++ .../jsonmarshalignoredeerror.go | 6 ++++++ pkg/workflow/args.go | 2 +- pkg/workflow/cache.go | 4 ++-- pkg/workflow/compiler_experiments.go | 6 +++--- pkg/workflow/compiler_pre_activation_job.go | 6 +++--- pkg/workflow/compiler_yaml.go | 2 +- pkg/workflow/copilot_logs.go | 2 +- pkg/workflow/mcp_config_playwright_renderer.go | 6 +++--- pkg/workflow/mcp_renderer_github.go | 2 +- pkg/workflow/repo_memory.go | 2 +- pkg/workflow/safe_outputs_config_generation.go | 5 ++++- 14 files changed, 40 insertions(+), 20 deletions(-) diff --git a/.github/workflows/cgo.yml b/.github/workflows/cgo.yml index d48a49c6c2e..20d86fcd9b9 100644 --- a/.github/workflows/cgo.yml +++ b/.github/workflows/cgo.yml @@ -1037,7 +1037,7 @@ jobs: # Enforce selected production custom analyzers without blocking on unrelated # legacy custom analyzer findings in tests or other analyzer families. - name: Run custom linters - run: make golint-custom LINTER_FLAGS="-errstringmatch -panicinlibrarycode -manualmutexunlock -osexitinlibrary -rawloginlib -regexpcompileinfunction -fprintlnsprintf -strconvparseignorederror -test=false" + run: make golint-custom LINTER_FLAGS="-errstringmatch -panicinlibrarycode -manualmutexunlock -osexitinlibrary -rawloginlib -regexpcompileinfunction -fprintlnsprintf -strconvparseignorederror -jsonmarshalignoredeerror -test=false" actions-build: runs-on: ubuntu-latest diff --git a/pkg/cli/mcp_config_file.go b/pkg/cli/mcp_config_file.go index f072ecb10f4..a104cb7124f 100644 --- a/pkg/cli/mcp_config_file.go +++ b/pkg/cli/mcp_config_file.go @@ -56,8 +56,8 @@ func ensureMCPConfig(verbose bool) error { // Check if the server is already configured correctly if existingConfig, exists := config.MCPServers[ghAwServerName]; exists { - existingJSON, _ := json.Marshal(existingConfig) - newJSON, _ := json.Marshal(ghAwConfig) + existingJSON, _ := json.Marshal(existingConfig) //nolint:jsonmarshalignoredeerror // VSCodeMCPServer contains only JSON-safe types (string, []string) + newJSON, _ := json.Marshal(ghAwConfig) //nolint:jsonmarshalignoredeerror // VSCodeMCPServer contains only JSON-safe types (string, []string) if string(existingJSON) == string(newJSON) { mcpConfigLog.Print("Configuration is identical, skipping") if verbose { diff --git a/pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go b/pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go index 7ae82078b13..1ebe516e438 100644 --- a/pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go +++ b/pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go @@ -9,6 +9,8 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" "golang.org/x/tools/go/ast/inspector" + + "github.com/github/gh-aw/pkg/linters/internal/nolint" ) // Analyzer is the json-marshal-ignored-error analysis pass. @@ -22,6 +24,7 @@ var Analyzer = &analysis.Analyzer{ func run(pass *analysis.Pass) (any, error) { insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + noLintLinesByFile := nolint.BuildLineIndex(pass, "jsonmarshalignoredeerror") nodeFilter := []ast.Node{(*ast.AssignStmt)(nil)} insp.Preorder(nodeFilter, func(n ast.Node) { assign, ok := n.(*ast.AssignStmt) @@ -36,6 +39,10 @@ func run(pass *analysis.Pass) (any, error) { call, ok := assign.Rhs[0].(*ast.CallExpr) if ok { if isJSONFunc(pass, call, "Marshal") { + position := pass.Fset.PositionFor(call.Pos(), false) + if nolint.HasDirective(position, noLintLinesByFile) { + return + } pass.ReportRangef(call, "error return from json.Marshal is discarded; marshal failures produce nil bytes silently") } } @@ -49,6 +56,10 @@ func run(pass *analysis.Pass) (any, error) { call, ok := assign.Rhs[0].(*ast.CallExpr) if ok { if isJSONFunc(pass, call, "Unmarshal") { + position := pass.Fset.PositionFor(call.Pos(), false) + if nolint.HasDirective(position, noLintLinesByFile) { + return + } pass.ReportRangef(call, "error return from json.Unmarshal is discarded; unmarshal failures leave the target value in a partial state") } } diff --git a/pkg/linters/jsonmarshalignoredeerror/testdata/src/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go b/pkg/linters/jsonmarshalignoredeerror/testdata/src/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go index 0d8532b5da4..8498ca89455 100644 --- a/pkg/linters/jsonmarshalignoredeerror/testdata/src/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go +++ b/pkg/linters/jsonmarshalignoredeerror/testdata/src/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go @@ -27,3 +27,9 @@ func Good() error { } return nil } + +func Suppressed() { + f := Foo{X: 1} + val, _ := json.Marshal(f) //nolint:jsonmarshalignoredeerror // Foo contains only JSON-safe types + _ = val +} diff --git a/pkg/workflow/args.go b/pkg/workflow/args.go index d8c1dee143e..b2d2e9b5f7b 100644 --- a/pkg/workflow/args.go +++ b/pkg/workflow/args.go @@ -59,7 +59,7 @@ func writeArgsToYAML(yaml *strings.Builder, args []string, indent string) { for _, arg := range args { yaml.WriteString(",\n") // Use json.Marshal to properly quote and escape the argument - quotedArg, _ := json.Marshal(arg) + quotedArg, _ := json.Marshal(arg) //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail yaml.WriteString(indent + string(quotedArg)) } } diff --git a/pkg/workflow/cache.go b/pkg/workflow/cache.go index bae1ff157bf..201531442fd 100644 --- a/pkg/workflow/cache.go +++ b/pkg/workflow/cache.go @@ -633,7 +633,7 @@ func generateCacheMemoryValidation(builder *strings.Builder, data *WorkflowData) cacheDir := cacheMemoryDirFor(cache.ID) // Prepare allowed extensions array for JavaScript - allowedExtsJSON, _ := json.Marshal(cache.AllowedExtensions) + allowedExtsJSON, _ := json.Marshal(cache.AllowedExtensions) //nolint:jsonmarshalignoredeerror // marshaling a string slice cannot fail // Build validation script var validationScript strings.Builder @@ -914,7 +914,7 @@ func (c *Compiler) buildUpdateCacheMemoryJob(data *WorkflowData, threatDetection cacheLog.Printf("Skipping validation step for cache %s in update job (empty allowed-extensions means all files are allowed)", cache.ID) } else { // Prepare allowed extensions array for JavaScript - allowedExtsJSON, _ := json.Marshal(cache.AllowedExtensions) + allowedExtsJSON, _ := json.Marshal(cache.AllowedExtensions) //nolint:jsonmarshalignoredeerror // marshaling a string slice cannot fail // Build validation script var validationScript strings.Builder diff --git a/pkg/workflow/compiler_experiments.go b/pkg/workflow/compiler_experiments.go index 0f0e319936a..2a1a940a470 100644 --- a/pkg/workflow/compiler_experiments.go +++ b/pkg/workflow/compiler_experiments.go @@ -448,17 +448,17 @@ func buildExperimentSpecJSON(experiments map[string][]string, configs map[string if i > 0 { sb.WriteString(",") } - keyBytes, _ := json.Marshal(name) + keyBytes, _ := json.Marshal(name) //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail sb.Write(keyBytes) sb.WriteString(":") // Use the full config when available so the JS can consume metadata. if cfg, ok := configs[name]; ok && cfg != nil { - cfgBytes, _ := json.Marshal(cfg) + cfgBytes, _ := json.Marshal(cfg) //nolint:jsonmarshalignoredeerror // ExperimentConfig contains only JSON-safe types (strings, ints, []string) sb.Write(cfgBytes) } else { // Fallback: bare variants array (legacy behaviour). - varBytes, _ := json.Marshal(experiments[name]) + varBytes, _ := json.Marshal(experiments[name]) //nolint:jsonmarshalignoredeerror // marshaling a string slice cannot fail sb.Write(varBytes) } } diff --git a/pkg/workflow/compiler_pre_activation_job.go b/pkg/workflow/compiler_pre_activation_job.go index a1df44efaab..f153e20e22b 100644 --- a/pkg/workflow/compiler_pre_activation_job.go +++ b/pkg/workflow/compiler_pre_activation_job.go @@ -165,11 +165,11 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec if len(data.SkipIfCheckFailing.Include) > 0 || len(data.SkipIfCheckFailing.Exclude) > 0 || data.SkipIfCheckFailing.Branch != "" || data.SkipIfCheckFailing.AllowPending { steps = append(steps, " env:\n") if len(data.SkipIfCheckFailing.Include) > 0 { - includeJSON, _ := json.Marshal(data.SkipIfCheckFailing.Include) + includeJSON, _ := json.Marshal(data.SkipIfCheckFailing.Include) //nolint:jsonmarshalignoredeerror // marshaling a string slice cannot fail steps = append(steps, fmt.Sprintf(" GH_AW_SKIP_CHECK_INCLUDE: %q\n", string(includeJSON))) } if len(data.SkipIfCheckFailing.Exclude) > 0 { - excludeJSON, _ := json.Marshal(data.SkipIfCheckFailing.Exclude) + excludeJSON, _ := json.Marshal(data.SkipIfCheckFailing.Exclude) //nolint:jsonmarshalignoredeerror // marshaling a string slice cannot fail steps = append(steps, fmt.Sprintf(" GH_AW_SKIP_CHECK_EXCLUDE: %q\n", string(excludeJSON))) } if data.SkipIfCheckFailing.Branch != "" { @@ -227,7 +227,7 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec steps = append(steps, fmt.Sprintf(" uses: %s\n", getCachedActionPin("actions/github-script", data))) steps = append(steps, " env:\n") // Pass commands as JSON array - commandsJSON, _ := json.Marshal(data.Command) + commandsJSON, _ := json.Marshal(data.Command) //nolint:jsonmarshalignoredeerror // marshaling a string slice cannot fail steps = append(steps, fmt.Sprintf(" GH_AW_COMMANDS: %q\n", string(commandsJSON))) if data.CommandPlaceholder != "" { steps = append(steps, fmt.Sprintf(" GH_AW_COMMAND_PLACEHOLDER: %q\n", data.CommandPlaceholder)) diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index 98f57c8d37f..5de876aa11c 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -811,7 +811,7 @@ func (c *Compiler) generateCreateAwInfo(yaml *strings.Builder, data *WorkflowDat // Allowed domains as JSON array string domainsJSON := "[]" if len(allowedDomains) > 0 { - b, _ := json.Marshal(allowedDomains) + b, _ := json.Marshal(allowedDomains) //nolint:jsonmarshalignoredeerror // marshaling a string slice cannot fail domainsJSON = string(b) } diff --git a/pkg/workflow/copilot_logs.go b/pkg/workflow/copilot_logs.go index bec44286c51..5d57b489f27 100644 --- a/pkg/workflow/copilot_logs.go +++ b/pkg/workflow/copilot_logs.go @@ -94,7 +94,7 @@ func (e *CopilotEngine) parseSessionJSONL(logContent string, verbose bool) (LogM // Calculate input size inputSize := 0 if content.Input != nil { - inputJSON, _ := json.Marshal(content.Input) + inputJSON, _ := json.Marshal(content.Input) //nolint:jsonmarshalignoredeerror // used only for len() size metric; failure yields len(nil)==0 which is acceptable inputSize = len(inputJSON) } diff --git a/pkg/workflow/mcp_config_playwright_renderer.go b/pkg/workflow/mcp_config_playwright_renderer.go index 56419f6749b..b68d5189303 100644 --- a/pkg/workflow/mcp_config_playwright_renderer.go +++ b/pkg/workflow/mcp_config_playwright_renderer.go @@ -70,21 +70,21 @@ var mcpPlaywrightLog = logger.New("workflow:mcp_config_playwright_renderer") // If inline is true, it writes on a single line; otherwise uses multi-line formatting. // Always appends a trailing comma after the closing bracket. func writeJSONStringArray(b *strings.Builder, indent, key string, values []string, inline bool) { - jsonKey, _ := json.Marshal(key) + jsonKey, _ := json.Marshal(key) //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail if inline { b.WriteString(indent + string(jsonKey) + ": [") for i, v := range values { if i > 0 { b.WriteString(", ") } - jsonVal, _ := json.Marshal(v) + jsonVal, _ := json.Marshal(v) //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail b.Write(jsonVal) } b.WriteString("],\n") } else { b.WriteString(indent + string(jsonKey) + ": [\n") for i, v := range values { - jsonVal, _ := json.Marshal(v) + jsonVal, _ := json.Marshal(v) //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail b.WriteString(indent + " " + string(jsonVal)) if i < len(values)-1 { b.WriteString(",") diff --git a/pkg/workflow/mcp_renderer_github.go b/pkg/workflow/mcp_renderer_github.go index 2c8028ac5de..d99a81734eb 100644 --- a/pkg/workflow/mcp_renderer_github.go +++ b/pkg/workflow/mcp_renderer_github.go @@ -246,7 +246,7 @@ func RenderGitHubMCPDockerConfig(yaml *strings.Builder, options GitHubMCPDockerO if len(options.CustomArgs) > 0 { yaml.WriteString(" \"args\": [\n") for _, arg := range options.CustomArgs { - quotedArg, _ := json.Marshal(arg) + quotedArg, _ := json.Marshal(arg) //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail yaml.WriteString(" " + string(quotedArg) + ",\n") } yaml.WriteString(" ],\n") diff --git a/pkg/workflow/repo_memory.go b/pkg/workflow/repo_memory.go index ac04a733e3a..c25d6eb5cdd 100644 --- a/pkg/workflow/repo_memory.go +++ b/pkg/workflow/repo_memory.go @@ -714,7 +714,7 @@ func (c *Compiler) buildPushRepoMemoryJob(data *WorkflowData, threatDetectionEna fmt.Fprintf(&step, " MAX_FILE_COUNT: %d\n", memory.MaxFileCount) fmt.Fprintf(&step, " MAX_PATCH_SIZE: %d\n", memory.MaxPatchSize) // Pass allowed extensions as JSON array - allowedExtsJSON, _ := json.Marshal(memory.AllowedExtensions) + allowedExtsJSON, _ := json.Marshal(memory.AllowedExtensions) //nolint:jsonmarshalignoredeerror // marshaling a string slice cannot fail fmt.Fprintf(&step, " ALLOWED_EXTENSIONS: '%s'\n", allowedExtsJSON) if fileGlobFilter != "" { // Quote the value to prevent YAML alias interpretation of patterns like *.md diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index dcb3e27b154..3d1427395c9 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -195,7 +195,10 @@ func generateSafeOutputsConfig(data *WorkflowData) (string, error) { if len(safeOutputsConfig) == 0 { return "", nil } - configJSON, _ := json.Marshal(safeOutputsConfig) + configJSON, err := json.Marshal(safeOutputsConfig) + if err != nil { + return "", fmt.Errorf("marshaling safe-outputs config: %w", err) + } safeOutputsConfigLog.Printf("Safe outputs config generation complete: %d tool types configured", len(safeOutputsConfig)) return string(configJSON), nil }