Skip to content

Triage & enforce jsonmarshalignoredeerror: 19 prod sites discard json.Marshal errors (silent corruption in code-gen) #35859

@github-actions

Description

@github-actions

Summary

The repo registered a new custom linterjsonmarshalignoredeerror (cmd/linters/main.go:26,57, the 18th registered analyzer) — that flags x, _ := json.Marshal(...) and _ = json.Unmarshal(...) where the error is discarded. It is not yet in the enforced set (.github/workflows/cgo.yml:1040 LINTER_FLAGS).

A static audit found 19 production violations across 11 files, all json.Marshal discards. When json.Marshal fails it returns nil bytes; because these sites discard the error, that nil is silently written into generated workflow YAML/JSON or returned as an empty string — producing malformed generated output that fails later at runtime and is hard to trace back.

This follows the proven pattern from R22→R23 where regexpcompileinfunction, fprintlnsprintf, and strconvparseignorederror were triaged and then enforced. Triaging these 19 sites unlocks enforcing the analyzer (zero-regression CI guard).

Note: jsonmarshalignoredeerror only matches encoding/json and only Marshal/Unmarshal (not MarshalIndent). The single _ = json.Unmarshal(...) grep hit (pkg/workflow/mcp_scripts_generator.go:334) is a string literal inside a code generator, not a real call — the AST linter correctly ignores it. So there are zero real Unmarshal violations.

Severity

Medium — silent data corruption in the compiler's code-generation path; no crash, but malformed .lock.yml output is hard to debug. One site (safe_outputs_config_generation.go:198) is sharper because the enclosing function already returns error and throws it away.

Risk-tiered findings

🔴 Tier 1 — error path already exists, discarded anyway

  • pkg/workflow/safe_outputs_config_generation.go:198configJSON, _ := json.Marshal(safeOutputsConfig) then return string(configJSON), nil. The function returns (string, error) but the marshal error is dropped; a failure yields "" with a nil error → silent empty safe-outputs config in the generated workflow.

🟠 Tier 2 — nil bytes written into generated output (strings.Builder / templates)

Marshaling any / map / struct / []string values; a failure writes nothing → malformed generated JSON/YAML:

  • pkg/workflow/compiler_experiments.go:457, :461 — manual JSON object assembled by writing json.Marshal(cfg) / json.Marshal(experiments[name]) into a strings.Builder; nil bytes produce invalid JSON ("name": followed directly by ,/})
  • pkg/workflow/compiler_pre_activation_job.go:168, :172, :230
  • pkg/workflow/compiler_yaml.go:814 (allowedDomains)
  • pkg/workflow/cache.go:636, :917 (AllowedExtensions)
  • pkg/workflow/repo_memory.go:717 (AllowedExtensions)
  • pkg/cli/mcp_config_file.go:59, :60 (existingConfig, ghAwConfig)
  • pkg/workflow/mcp_config_playwright_renderer.go:80, :87 (v is any)

🟢 Tier 3 — cannot fail / harmless (annotate with //nolint + reason)

  • json.Marshal of a string never returns an error: pkg/workflow/args.go:62 (arg), pkg/workflow/mcp_renderer_github.go:249 (arg), pkg/workflow/mcp_config_playwright_renderer.go:73 (key), pkg/workflow/compiler_experiments.go:451 (name)
  • pkg/workflow/copilot_logs.go:97json.Marshal(content.Input) used only for len(inputJSON) size metric; a failure just yields size 0

Recommendation

  1. Tier 1 — handle the error via the existing return:
configJSON, err := json.Marshal(safeOutputsConfig)
if err != nil {
    return "", fmt.Errorf("marshaling safe-outputs config: %w", err)
}
return string(configJSON), nil
  1. Tier 2 — handle the error (return/log) where a return is available, or fail compilation loudly rather than emitting partial output. Where the surrounding function cannot return an error, add //nolint:jsonmarshalignoredeerror with a justification only if the input type is provably JSON-safe.
  2. Tier 3 — add //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail (or the size-metric rationale for copilot_logs.go:97).
  3. Enforce — once all 19 sites are handled or annotated, append -jsonmarshalignoredeerror to LINTER_FLAGS in .github/workflows/cgo.yml:1040 to lock the guard in CI.

Validation

  • make golint-custom LINTER_FLAGS="-jsonmarshalignoredeerror -test=false" reports zero violations after triage
  • make test / make recompile — generated .lock.yml outputs unchanged
  • Verify no MarshalIndent or non-stdlib-json discard sites were missed

Estimated effort

Small–Medium (mechanical, ~19 sites; ~4 genuine error-handling additions, rest //nolint annotations).

Audited with Serena LSP + static analysis. Sergo run R23 (strategy: reverify-plus-new-jsonmarshal-linter-violation-audit).

Generated by 🤖 Sergo - Serena Go Expert · opus48 2.1M ·

  • expires on Jun 6, 2026, 5:07 AM UTC

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions