[sergo] Sergo Report: YAML Workflow Name Injection + Dead Parameter Audit - 2026-03-24

[github-actions[bot]]

Overview

Run 30 of Sergo analyzed the Go codebase using a dual strategy: extending the high-scoring YAML injection audit from run 26 (score 9) into the compiler_yaml.go workflow name field, combined with a new dead/unused parameter audit applied to the freshest code in the repo (commit 586e5bf, merged today). Three actionable findings were identified spanning two files introduced in the latest commit plus an existing context propagation gap.

The headline finding is that compiler_yaml.go:195 embeds the user-supplied workflow name directly inside a YAML double-quoted string (name: "...") without escaping " characters. A workflow whose H1 header or name: frontmatter field contains a double-quote character produces syntactically invalid GitHub Actions YAML that will fail at runner parse time. This is the same injection surface that produced the run 26 critical MCP tool-name findings, now extended one level up to the workflow name itself.

Serena Tools Snapshot

• Total Tools Available: 22 (unchanged from run 29)
• New Tools Since Last Run: None
• Removed Tools: None
• Tools Used Today: grep, find, bash, find_symbol, get_symbols_overview, list_dir, read_file

Strategy Selection

Cached Reuse Component (50%)

Previous Strategy Adapted: mcp-scripts-shell-injection-plus-yaml-analysis (run 26, score 9)

• Original Success Score: 9/10
• Last Used: 2026-03-19
• Why Reused: Run 26 was the highest-scoring run ever; it found CRITICAL shell injection via unsanitized toolName in mcp_setup_generator.go heredocs. The same root cause pattern (user-controlled string embedded in generated YAML without escaping) appears elsewhere in the codebase.
• Modifications: Instead of toolName in MCP setup steps, today I targeted data.Name (workflow name) in the top-level GitHub Actions name: field and GH_AW_INFO_WORKFLOW_NAME env var. I also traced provenance from ExtractWorkflowNameFromMarkdownBody (H1 header) and extractStringFromMap(frontmatter, "name") to confirm no sanitization before YAML emission.

New Exploration Component (50%)

Novel Approach: Dead/Unused Parameter Audit of today's commit (586e5bf)

• Tools Employed: git show, grep, read_file
• Hypothesis: AI-authored commits (Copilot-authored per the git log) often carry unused parameters when functions are copy-pasted from similar helpers during implementation. The latest commit introduced two new Go source files (engine_firewall_support.go, compiler_yaml_artifacts.go) — prime candidates.
• Target Areas: pkg/workflow/engine_firewall_support.go (all 189 lines, brand-new), pkg/cli/update_check.go (context propagation gap in goroutine)

Combined Strategy Rationale

The cached component ensures breadth (covering the same injection vector across a wider surface), while the new component ensures timeliness (catching bugs in code committed hours before this analysis). Together they surface issues at different lifecycle stages: a long-standing design gap (workflow name escaping) and a fresh oversight (dead parameter in today's commit).

Codebase Context

• Total Go Files (non-test): 615 (608 in pkg/)
• Files Analyzed Today: compiler_yaml.go, engine_firewall_support.go, compiler_yaml_main_job.go, compiler_orchestrator_workflow.go, compiler_orchestrator_tools.go, frontmatter_extraction_yaml.go, parser/frontmatter_content.go, update_check.go
• Latest Commit: 586e5bf — "fix: include policy-manifest.json and audit directory in firewall-audit-logs artifact" (Copilot-authored, today)

Findings Summary

#	Severity	File	Issue
1	HIGH	compiler_yaml.go:195,679	Workflow name with " produces invalid GitHub Actions YAML
2	MEDIUM	engine_firewall_support.go:143	workflowName parameter received but never used (dead param, latest commit)
3	MEDIUM	update_check.go:220	client.Get() ignores context; GetWithContext(ctx) available but unused

Detailed Findings

Finding 1 — Workflow Name Unescaped in Double-Quoted YAML (HIGH)

Issue Type: YAML Injection / Robustness

Problem: compiler_yaml.go embeds data.Name (the user workflow name) raw inside "..." YAML double-quoted scalars at two sites without escaping " characters. A workflow named My "AI" Assistant produces:

name: "My "AI" Assistant"

which is syntactically invalid YAML — the GitHub Actions runner rejects it at parse time with a YAML syntax error, preventing the workflow from ever running. Same pattern appears in the env var block:

GH_AW_INFO_WORKFLOW_NAME: "My "AI" Assistant"

Evidence of unescaped path (tracing data.Name):

• compiler_yaml.go:195 — fmt.Fprintf(yaml, "name: \"%s\"\n", data.Name) ← no escaping
• compiler_yaml.go:679 — fmt.Fprintf(yaml, " GH_AW_INFO_WORKFLOW_NAME: \"%s\"\n", data.Name) ← no escaping
• compiler_orchestrator_tools.go:264-266 — frontmatterName := extractStringFromMap(result.Frontmatter, "name", nil) → if set, overrides workflowName; no sanitization
• parser/frontmatter_content.go:158-163 — H1 header extracted as raw text; no char filtering

Contrast with correct sites that already use %q (Go-quoted string with proper backslash escaping):

• compiler_activation_job.go:229 — fmt.Sprintf(" GH_AW_WORKFLOW_NAME: %q\n", data.Name) ✅
• compiler_safe_outputs_job.go:468 — envVars["GH_AW_WORKFLOW_NAME"] = fmt.Sprintf("%q", data.Name) ✅

Impact: Any user with a workflow whose name contains " (e.g., My "Project" Workflow, "Quoted" Name) gets invalid compiled YAML that fails immediately at parse-time on the GitHub Actions runner. The " is a naturally occurring character in workflow names (especially those auto-generated from markdown H1 headers).

Location(s):

• pkg/workflow/compiler_yaml.go:195
• pkg/workflow/compiler_yaml.go:679

Recommendation: Use YAML-safe double-quote escaping. In YAML double-quoted scalars, " is escaped as \" and \ as \\:

// Before (broken):
fmt.Fprintf(yaml, "name: \"%s\"\n", data.Name)

// After (correct):
escaped := strings.ReplaceAll(strings.ReplaceAll(data.Name, `\`, `\\`), `"`, `\"`)
fmt.Fprintf(yaml, "name: \"%s\"\n", escaped)

Alternatively, extract a helper yamlDoubleQuote(s string) string used across all double-quoted scalar sites in compiler_yaml.go.

---

Finding 2 — Dead workflowName Parameter in New Firewall Function (MEDIUM)

Issue Type: Dead Code / Inconsistent API

Problem: generateFirewallLogParsingStep(workflowName string) was introduced in today's commit 586e5bf as part of the new engine_firewall_support.go file. The function signature accepts workflowName but the parameter is never referenced in the function body — the step name is hardcoded as "Print firewall logs" with no workflow identification.

// engine_firewall_support.go:143
func generateFirewallLogParsingStep(workflowName string) GitHubActionStep {
    firewallLogsDir := "/tmp/gh-aw/sandbox/firewall/logs"
    stepLines := []string{
        "      - name: Print firewall logs",   // workflowName never used here
        ...
    }
    return GitHubActionStep(stepLines)
}

Contrast with the sibling function in the same file that correctly uses the name:

// engine_firewall_support.go:98
func generateSquidLogsUploadStep(workflowName string) GitHubActionStep {
    sanitizedName := strings.ToLower(SanitizeWorkflowName(workflowName))
    artifactName := "firewall-logs-" + sanitizedName  // name IS used
    ...
}

The call site compiler_yaml_main_job.go:465 passes data.Name as a dead argument:

firewallLogParsing := generateFirewallLogParsingStep(data.Name)  // data.Name ignored

Impact: (a) The unused parameter is dead code carrying cognitive overhead. (b) In workflow_call scenarios where multiple workflows are nested, all "Print firewall logs" steps are indistinguishable in the GitHub Actions UI — users cannot tell which firewall log step belongs to which workflow. (c) If the intent was to include the workflow name in the step name (consistent with the artifact naming pattern), it was accidentally dropped during implementation.

Location(s):

• pkg/workflow/engine_firewall_support.go:143,152
• pkg/workflow/compiler_yaml_main_job.go:465

Recommendation: Either use workflowName in the step name for consistency with generateSquidLogsUploadStep, or remove the parameter entirely if intentionally unused:

// Option A: use the name (consistent with sibling function)
func generateFirewallLogParsingStep(workflowName string) GitHubActionStep {
    sanitizedName := strings.ToLower(SanitizeWorkflowName(workflowName))
    stepName := fmt.Sprintf("Print firewall logs for %s", sanitizedName)
    stepLines := []string{
        "      - name: " + stepName,
        ...
    }
}

// Option B: remove the parameter if name is intentionally omitted
func generateFirewallLogParsingStep() GitHubActionStep { ... }
// Call site: generateFirewallLogParsingStep()

---

Finding 3 — CheckForUpdatesAsync Context Not Propagated to HTTP Call (MEDIUM)

Issue Type: Context Propagation Gap

Problem: CheckForUpdatesAsync accepts a context.Context parameter with the documented intent that "the context can be used to cancel the update check if the program is shutting down" (update_check.go:231). The goroutine checks ctx.Err() before calling checkForUpdates(), but checkForUpdates() does not receive the context, and getLatestRelease() uses client.Get() instead of the available client.GetWithContext(ctx) from go-gh v2.13.0:

// update_check.go:213-220
func getLatestRelease() (string, error) {
    client, err := api.NewRESTClient(api.ClientOptions{})
    ...
    err = client.Get("repos/github/gh-aw/releases/latest", &release)  // no ctx
    ...
}

Evidence: go-gh v2 (v2.13.0 per go.mod) exposes RESTClient.GetWithContext(ctx context.Context, path string, response interface{}) error. The goroutine checks context twice (ctx.Err() at line 244), then abandons cancellation for the actual network call.

Impact: On slow or rate-limited networks, if the user Ctrl+C's the gh aw validate command, the update-check goroutine continues its GitHub API call indefinitely until the OS kills the process. This is low severity in practice (CLI exits and the OS kills all goroutines), but violates the stated contract of the function and wastes network resources.

Location(s):

• pkg/cli/update_check.go:213,220 (getLatestRelease)
• pkg/cli/update_check.go:248 (call to checkForUpdates without ctx)

Recommendation:

// Pass context through the call chain:
func getLatestRelease(ctx context.Context) (string, error) {
    client, err := api.NewRESTClient(api.ClientOptions{})
    ...
    err = client.GetWithContext(ctx, "repos/github/gh-aw/releases/latest", &release)
    ...
}

func checkForUpdates(ctx context.Context, noCheckUpdate bool, verbose bool) {
    ...
    latestVersion, err := getLatestRelease(ctx)
    ...
}

// goroutine in CheckForUpdatesAsync:
go func() {
    ...
    checkForUpdates(ctx, noCheckUpdate, verbose)  // ctx now propagated
}()

Improvement Tasks

Task 1: Fix Workflow Name Escaping in Double-Quoted YAML Scalars

Issue Type: YAML Injection / Robustness

Problem: compiler_yaml.go embeds data.Name in "..." YAML strings without escaping " → invalid compiled YAML for any workflow name containing a double-quote.

Location(s):

• pkg/workflow/compiler_yaml.go:195 — name: field
• pkg/workflow/compiler_yaml.go:679 — GH_AW_INFO_WORKFLOW_NAME: env var

Impact:

• Severity: HIGH
• Affected Users: Any user whose workflow H1 header or name: frontmatter field contains "
• Risk: Compiled workflow fails GitHub Actions YAML parse; workflow never runs

Before:

fmt.Fprintf(yaml, "name: \"%s\"\n", data.Name)
fmt.Fprintf(yaml, "          GH_AW_INFO_WORKFLOW_NAME: \"%s\"\n", data.Name)

After:

// Add helper in compiler_yaml.go or strings.go:
func yamlDoubleQuoteEscape(s string) string {
    s = strings.ReplaceAll(s, `\`, `\\`)
    s = strings.ReplaceAll(s, `"`, `\"`)
    return s
}

fmt.Fprintf(yaml, "name: \"%s\"\n", yamlDoubleQuoteEscape(data.Name))
fmt.Fprintf(yaml, "          GH_AW_INFO_WORKFLOW_NAME: \"%s\"\n", yamlDoubleQuoteEscape(data.Name))

Validation:

• Test with workflow name containing ", \, and both
• Verify compiled YAML is valid (actionlint or yq parse)
• Grep for other fmt.Fprintf(yaml, ...\"%s\"..., data.Name) patterns to find similar sites

Estimated Effort: Small

---

Task 2: Fix Dead workflowName Parameter in generateFirewallLogParsingStep

Issue Type: Dead Code / Unused Parameter

Problem: engine_firewall_support.go:143 — new function from 586e5bf has an unused workflowName parameter; step name is hardcoded, inconsistent with sibling generateSquidLogsUploadStep.

Location(s):

• pkg/workflow/engine_firewall_support.go:143,152
• pkg/workflow/compiler_yaml_main_job.go:465

Impact:

• Severity: MEDIUM
• Affected Files: 2
• Risk: Dead code in a function introduced today; step is unidentifiable per-workflow in workflow_call scenarios

Before:

func generateFirewallLogParsingStep(workflowName string) GitHubActionStep {
    firewallLogsDir := "/tmp/gh-aw/sandbox/firewall/logs"
    stepLines := []string{
        "      - name: Print firewall logs",
        // workflowName is never used
        ...
    }
}

After (Option A — use name for step identification):

func generateFirewallLogParsingStep(workflowName string) GitHubActionStep {
    firewallLogsDir := "/tmp/gh-aw/sandbox/firewall/logs"
    sanitizedName := strings.ToLower(SanitizeWorkflowName(workflowName))
    stepName := fmt.Sprintf("Print firewall logs for %s", sanitizedName)
    stepLines := []string{
        "      - name: " + stepName,
        ...
    }
}

After (Option B — remove parameter if name is intentionally omitted):

func generateFirewallLogParsingStep() GitHubActionStep { ... }
// compiler_yaml_main_job.go:465:
firewallLogParsing := generateFirewallLogParsingStep()

Validation:

• Run existing tests (go test ./pkg/workflow/...)
• Verify compiled YAML for a firewall-enabled workflow has correct step naming
• Check if generateSquidLogsUploadStep also needs alignment

Estimated Effort: Small

---

Task 3: Propagate Context Through CheckForUpdatesAsync → getLatestRelease

Issue Type: Context Propagation Gap

Problem: update_check.go:220 — client.Get() ignores the context passed to CheckForUpdatesAsync; GetWithContext(ctx) is available in go-gh v2.13 but unused, breaking the stated contract.

Location(s):

• pkg/cli/update_check.go:213,220 — getLatestRelease()
• pkg/cli/update_check.go:143,162,248 — checkForUpdates() and its caller in goroutine

Impact:

• Severity: MEDIUM
• Affected Operations: gh aw validate update check goroutine
• Risk: Ctrl+C doesn't cancel the GitHub API network call; violates documented contract

Before:

func getLatestRelease() (string, error) {
    client, err := api.NewRESTClient(api.ClientOptions{})
    err = client.Get("repos/github/gh-aw/releases/latest", &release)
    ...
}
func checkForUpdates(noCheckUpdate bool, verbose bool) { ... getLatestRelease() ... }
go func() { checkForUpdates(noCheckUpdate, verbose) }()

After:

func getLatestRelease(ctx context.Context) (string, error) {
    client, err := api.NewRESTClient(api.ClientOptions{})
    err = client.GetWithContext(ctx, "repos/github/gh-aw/releases/latest", &release)
    ...
}
func checkForUpdates(ctx context.Context, noCheckUpdate bool, verbose bool) {
    ...
    latestVersion, err := getLatestRelease(ctx)
    ...
}
go func() {
    ...
    checkForUpdates(ctx, noCheckUpdate, verbose)
}()

Validation:

• Verify api.RESTClient.GetWithContext signature in go-gh v2.13
• Test that Ctrl+C terminates the update-check goroutine promptly
• Ensure no other callers of checkForUpdates / getLatestRelease are affected

Estimated Effort: Small

Success Metrics

This Run

Metric	Value
Findings Generated	3
Tasks Created	3
Files Analyzed	8
New Commit Coverage	✅ Latest commit analyzed
Success Score	8/10

Score Reasoning:

• Findings Quality (3/4): Finding 1 is HIGH (real user-facing broken workflow); Findings 2-3 are MEDIUM actionable
• Coverage (2/3): 8 files across compiler, engine, cli packages — moderate breadth
• Task Generation (3/3): Three well-scoped, small-effort tasks with Before/After code examples

Historical Context

Strategy Performance (Last 10 Runs)

Run	Date	Strategy	Score	Findings
30	2026-03-24	yaml-workflow-name-injection-plus-dead-param-audit	8	3
29	2026-03-22	cache-id-second-propagation-plus-docker-context-audit	8	3
28	2026-03-21	cache-id-injection-plus-mcp-setup-revisit	8	3
27	2026-03-20	expression-perf-revisit-plus-http-body-drain-audit	7	3
26	2026-03-19	mcp-scripts-shell-injection-plus-yaml-analysis	9	3
25	2026-03-18	tempfile-pathtrav-plus-pr-cwd-revisit	8	3
24	2026-03-17	apm-token-lifecycle-revisit-plus-version-injection-audit	7	3
23	2026-03-16	yaml-injection-revisit-plus-new-feature-validation-gap	8	3
22	2026-03-14	yaml-step-generator-audit-plus-context-propagation-revisit	8	3
21	2026-03-13	context-propagation-revisit-plus-dead-code-audit	7	3

Cumulative Statistics

• Total Runs: 30
• Total Findings: 90
• Total Tasks Generated: 90
• Average Success Score: 7.93/10
• Most Successful Strategy: mcp-scripts-shell-injection-plus-yaml-analysis (score 9, run 26)
• Consistent 3-findings/run rate: Maintained across all 30 runs

Long-standing Unfixed Issues (Top 5 by Run Count)

1. GetSupportedEngines non-deterministic map iteration — unfixed since run 2 (28 runs)
2. getLatestWorkflowRunWithRetry missing ctx — unfixed since run 9 (21 runs)
3. mcp_setup_generator.go toolName shell injection (CRITICAL) — unfixed since run 26 (4 runs, highest severity)
4. cache.go cache.ID shell injection — unfixed since run 28 (2 runs)
5. expression_parser.go O(n²) string building — unfixed since run 12 (18 runs)

Recommendations

Immediate Actions

1. Fix workflow name YAML escaping (HIGH) — add yamlDoubleQuoteEscape helper and apply to compiler_yaml.go:195,679. Small effort, prevents invalid compiled workflows for a non-trivial class of workflow names.
2. Fix dead parameter in generateFirewallLogParsingStep (MEDIUM) — introduced today; clean up before it accumulates technical debt. Choose whether the step name should include the workflow name (consistent with generateSquidLogsUploadStep).
3. Propagate context in update check (MEDIUM) — honor the stated contract of CheckForUpdatesAsync. GetWithContext is available, change is purely mechanical.

Long-term Improvements

The YAML injection surface (Finding 1) suggests a systemic gap: data.Name is used at least 6 times in compiler_yaml.go and many more across compiler files, but only some sites use %q. A one-time audit to find all fmt.Fprintf(yaml, ..., data.Name) calls and verify each uses safe quoting would be worthwhile. The correct sites already use %q; the remaining "%s" sites should follow suit or use a YAML-safe helper.

Next Run Preview

Suggested Focus Areas

• 50% cached: Extend the workflow-name injection audit to compiler_main_job.go, compiler_activation_job.go, and compiler_safe_outputs_job.go step-name fields — any fmt.Fprintf(yaml, " - name: %s\n", data.Name) patterns where the name is embedded unquoted in YAML step names
• 50% new: Audit generateUnifiedArtifactUpload(paths []string, prefix string) in the new compiler_yaml_artifacts.go — trace where prefix comes from and whether paths slice elements are user-controlled or constant

Strategy Evolution

The dead-parameter audit of new commits proved highly efficient (found a finding in the first file examined from today's commit). Consider making "audit the most recently changed files" a standing 25% component of every run.

---

References:

• §23510452858

Generated by Sergo — The Serena Go Expert | Run ID: 23510452858 | Strategy: yaml-workflow-name-injection-plus-dead-param-audit

AI generated by Sergo - Serena Go Expert · history

• expires on Mar 25, 2026, 8:34 PM UTC

[github-actions[bot]]

This discussion has been marked as outdated by Sergo - Serena Go Expert.

A newer discussion is available at Discussion #22971.