[sergo] Sergo Report: YAML Step Generator Audit + Context-Propagation Revisit - 2026-03-14 #20993
Closed
Replies: 2 comments
-
|
This discussion has been marked as outdated by Sergo - Serena Go Expert. A newer discussion is available at Discussion #21125. |
Beta Was this translation helpful? Give feedback.
0 replies
-
|
/plan |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Date: 2026-03-14
Strategy:
yaml-step-generator-audit-plus-context-propagation-revisitSuccess Score: 8/10
Run: §23097433230
Executive Summary
Today's analysis combined a revisit of the persistent context-propagation gap in
dispatch.go(unfixed since run 19) with a new deep audit of the YAML step-generator layer — specifically how user-supplied values are serialized into GitHub Actions YAML steps. The audit surfaced two co-located bugs inruntime_step_generator.goand one in the brand-new Copilot pre-flight diagnostic code introduced in PR #20975.The core new finding is a single-quote escaping defect in
formatYAMLValue: every string is wrapped in YAML single quotes usingfmt.Sprintf("'%s'", v)without ever doubling embedded'characters as''. Since YAML single-quoted scalars represent a literal'only via'', any user-suppliedExtraFieldsvalue or engine version string containing a single quote will produce syntactically invalid workflow YAML. The related PR #20975 independently introduced a separate unquoted YAML injection point in the Copilot pre-flight diagnostic step for theCOPILOT_API_TARGETenv variable.Three findings were discovered and three improvement tasks are proposed.
🛠️ Serena Tools Update
Tools Snapshot
Tool Capabilities Used Today
get_symbols_overviewcopilot_engine_execution.goandruntime_step_generator.gofind_symbolgenerateCopilotPreflightDiagnosticStepandGetExecutionStepsbodiesgrep(native)formatYAMLValuecallers,ExtraFieldssources,APITargetusage,append(step,patternsRead(native)formatYAMLValueimplementation and step-generation contextbashgo.modGo version (1.25.0)📊 Strategy Selection
Cached Reuse Component (50%)
Previous Strategy Adapted:
context-propagation-dispatch-audit(run 19, score 8) +context-propagation-revisit(run 20, score 7)dispatch.go fetchAndSaveRemoteDispatchWorkflowsmissingcontext.Contexthas been confirmed unfixed for three consecutive runs. Re-validating keeps it visible. Also used to anchor the audit on the latest commit (PR Add Copilot pre-flight diagnostic for GHES environments #20975: Copilot pre-flight GHES diagnostic) for any new context issues.generateCopilotPreflightDiagnosticStepfunction introduced in the latest commit; audited new code for YAML and context issues simultaneously.New Exploration Component (50%)
Novel Approach:
yaml-step-generator-audit— systematic review of how user-controlled values are embedded into generated GitHub Actions YAML strings.grepforfmt.Sprintf("'%s'",append(step,patterns;ReadforformatYAMLValueand callers; Serenafind_symbolfor body inspection', user-configured values with single quotes would silently produce broken workflow YAML.pkg/workflow/runtime_step_generator.go,pkg/workflow/copilot_engine_execution.go, allappend(step,patterns acrosspkg/workflow/Combined Strategy Rationale
The two components complement each other because the latest commit (PR #20975) introduced new step-generation code that follows the same structural pattern as
runtime_step_generator.go. Reviewing new code through the lens of an existing known anti-pattern (direct YAML concatenation) immediately revealed the newCOPILOT_API_TARGETinjection point. The cached context-propagation angle caught the pre-flight step's missing context propagation opportunity while inspecting the new code.🔍 Analysis Execution
Codebase Context
pkg/: 577pkg/workflow/(runtime step generators, Copilot engine, dispatch)go.mod)Findings Summary
📋 Detailed Findings
High Priority Issues
Finding 1 —
formatYAMLValuesingle-quote escaping bugpkg/workflow/runtime_step_generator.go:162–208The function
formatYAMLValueis the central utility for serializing userExtraFieldsinto YAMLwith:blocks of generated runtime setup steps. It wraps all string values in YAML single quotes:In YAML, single-quoted scalars cannot contain a bare
'character — it must be doubled as''(the only escape mechanism in single-quoted strings). The current implementation never applies this escaping. If a user configures anExtraFieldsvalue containing a single quote — for example, acache-dependency-pathexpression, anauthtoken description, or a customversionstring — the generated step YAML will be syntactically invalid.Additional instance:
runtime_step_generator.go:125applies the same unescaped single-quoting directly for the version field:If a user's engine version string (e.g.
node-version: "20.0.0's LTS") contained a', the same breakage occurs.Evidence path:
ExtraFieldsis populated from user-suppliedwith:blocks parsed from.github/workflows/YAML files atpkg/workflow/runtime_deduplication.go:168(req.ExtraFields[key] = value), making this fully user-controlled.Impact: Users who include single-quote characters in any
runtime.setup.with.*field or engine version string will silently receive a broken generated workflow that fails to parse — with no error atgh awcompile time and a confusing YAML parse error at runtime.Finding 2 —
COPILOT_API_TARGETunquoted YAML injection (new in PR #20975)pkg/workflow/copilot_engine_execution.go:498The new
generateCopilotPreflightDiagnosticStepfunction (introduced in the latest commit, PR #20975) directly concatenatesEngineConfig.APITargetinto a YAMLenv:block without any quoting:APITargetis a user-configured hostname string (e.g.api.acme.ghe.com). While typical hostname values are safe, this pattern:formatYAMLValueor are pre-validated$\{\{ }}expressions:(colon-space),#, or a newline would produce invalid or misleading YAML in the generated workflow fileThe three places in the codebase using unquoted
append(step, "...: "+var)are:runtime_step_generator.go:98—IfCondition(GitHub Actions expression, controlled syntax)runtime_step_generator.go:104—GoModFile(filesystem path, limited charset)copilot_engine_execution.go:498—APITarget(user hostname, new)The first two have constrained input formats by convention;
APITargetdoes not.Medium Priority Issues
Finding 3 —
fetchAndSaveRemoteDispatchWorkflowsmissing context.Context (unfixed since run 19)pkg/cli/dispatch.go:77The function
fetchAndSaveRemoteDispatchWorkflowshas nocontext.Contextparameter. ThegetRepoDefaultBranch(spec.RepoSlug)call at line 93 performs an uncancellable network request — spawned duringgh aw addto resolve a remote workflow's default branch when no explicitversion:is set.This has been confirmed unfixed for three consecutive Sergo runs (19, 20, 21). Ctrl+C during
gh aw addcannot cancel this network call; the process must wait for the TCP timeout.✅ Improvement Tasks Generated
Task 1: Fix
formatYAMLValuesingle-quote escaping in YAML step generatorIssue Type: YAML Generation Correctness
Problem:
formatYAMLValue(pkg/workflow/runtime_step_generator.go:162) wraps all string values in YAML single quotes viafmt.Sprintf("'%s'", v)without escaping embedded'as''. Per the YAML specification, a single-quoted scalar's only escape is''for a literal single quote. Any user-configuredExtraFieldsvalue or engine version string containing'will produce syntactically invalid workflow YAML.The same bug appears on line 125 where the version field is formatted directly:
Locations:
pkg/workflow/runtime_step_generator.go:162–174—formatYAMLValuestring casepkg/workflow/runtime_step_generator.go:125— version field formattingpkg/workflow/runtime_step_generator.go:206— default case also unescaped:fmt.Sprintf("'%v'", v)Impact:
runtime_step_generator.go)ExtraFieldsor engine version strings; fails at GitHub Actions parse time, not atgh awcompile timeRecommendation:
Add a helper that properly escapes single-quoted YAML scalars, and apply it consistently:
Before:
After:
Validation:
formatYAMLValue("it's")→"'it''s'",formatYAMLValue("O'Brien")→"'O''Brien'"gopkg.in/yaml.v3runtime_step_generator_test.gofor existing round-trip testsEstimated Effort: Small
Task 2: Quote
COPILOT_API_TARGETvalue in pre-flight diagnostic step generatorIssue Type: YAML Generation Consistency / Injection Risk (new code, PR #20975)
Problem:
generateCopilotPreflightDiagnosticStepatpkg/workflow/copilot_engine_execution.go:498directly concatenatesEngineConfig.APITargetinto a YAML env block without quoting:This is inconsistent with how other user-supplied values are handled throughout the step-generation layer. While a well-formed hostname (
api.acme.ghe.com) is safe, the pattern is fragile: a value containing:,#, or a newline would silently corrupt the generated workflow YAML.Location:
pkg/workflow/copilot_engine_execution.go:498Impact:
copilot_engine_execution.go)api-targetconfig value contains YAML-special characters; inconsistent with codebase patternsRecommendation:
Apply
singleQuoteYAML(from Task 1) orformatYAMLValueto theAPITargetvalue. SinceAPITargetis a plain hostname string, single-quoting it is the safest approach:Before:
After:
Validation:
APITargetvalues containing:,#,', and newlinegopkg.in/yaml.v3Estimated Effort: Small (fix is one line; test coverage is the bulk of effort)
Task 3: Add
context.ContexttofetchAndSaveRemoteDispatchWorkflowsIssue Type: Context Propagation / Cancellation
Problem:
fetchAndSaveRemoteDispatchWorkflows(pkg/cli/dispatch.go:77) has nocontext.Contextparameter. ThegetRepoDefaultBranch(spec.RepoSlug)call at line 93 is an uncancellable network request triggered duringgh aw add. If the network is slow or the remote is unreachable, the process cannot be interrupted by Ctrl+C. This has been unfixed since run 19 (2026-03-11).Location:
pkg/cli/dispatch.go:77— function signature missingctx context.Contextpkg/cli/dispatch.go:93—getRepoDefaultBranch(spec.RepoSlug)→ should becomegetRepoDefaultBranch(ctx, spec.RepoSlug)Impact:
dispatch.go, callers that pass context)gh aw addif remote branch resolution stallsRecommendation:
Before:
After:
Validation:
getRepoDefaultBranchsignature to acceptctxgrep -rn fetchAndSaveRemoteDispatchWorkflows)gh aw addwith a slow remote cancels cleanlyEstimated Effort: Small–Medium (propagation through call chain)
📈 Success Metrics
This Run
copilot_engine_execution.go,runtime_step_generator.go,runtime_deduplication.go,dispatch.go,engine_helpers.go,engine.go,unified_prompt_step.go,awf_helpers.go)Reasoning for Score
COPILOT_API_TARGET) depends on Task 1'ssingleQuoteYAMLhelper, making the tasks sequentially coupled📊 Historical Context
Strategy Performance (all 21 runs)
Cumulative Statistics
error-patterns-plus-field-registry-audit(run 3, score 9)🎯 Recommendations
Immediate Actions
formatYAMLValuesingle-quote escaping (runtime_step_generator.go:162,125,206) — introducesingleQuoteYAML(s string) stringhelper usingstrings.ReplaceAll(s, "'", "''")and apply throughout the step generator. Small effort, prevents silent broken YAML for users with'in config values.COPILOT_API_TARGETin pre-flight step (copilot_engine_execution.go:498) — applysingleQuoteYAML(orformatYAMLValue) toEngineConfig.APITarget. One-line fix in new code before it ships widely to GHES users.context.ContexttofetchAndSaveRemoteDispatchWorkflows(dispatch.go:77) — allows Ctrl+C cancellation of thegetRepoDefaultBranchnetwork call duringgh aw add. Has been flagged for 3 consecutive runs.Long-term Improvements
getLatestWorkflowRunWithRetrymissing context) have been flagged for 10+ consecutive runs. A dedicated debt-reduction sprint targeting the top 5 by severity would have significant impact.🔄 Next Run Preview
Suggested Focus Areas
copilot_preflight_diagnostic.shscript is referenced atcopilot_engine_execution.go:501. Auditing the shell script generation pattern for shell injection vulnerabilities (env variables expanded inrun:steps).IfConditionandGoModFileunquoted injection: The two pre-existing unquoted YAML concatenations inruntime_step_generator.go:98,104were left un-analyzed today. A follow-up audit of what inputs flow intoIfConditionand whether they can contain YAML-special sequences is warranted.Strategy Evolution
pkg/workflow/step-generator files focusing onIfConditionsourcing and shell injection inrun:step content would be a natural continuation.References:
Beta Was this translation helpful? Give feedback.
All reactions