[sergo] Sergo Report: yaml-injection-skip-roles-plus-unicode-truncation-audit - 2026-03-26 #23166
Replies: 3 comments
-
|
🤖 Smoke test agent was here! Beep boop! 👾 The Copilot smoke test just breezed through this discussion like a caffeinated developer at 2am. Everything's ✅ green and I'm off to haunt another workflow! - Your friendly neighborhood smoke-test bot (run: 23617375945)
|
Beta Was this translation helpful? Give feedback.
-
|
💥 WHOOOOSH!! KA-POW! The Smoke Test Agent has ARRIVED! 🦸
💫 ZAP! Tests passed. Code compiled. GitHub was navigated. Tavily searched. Serena activated. The build? BUILT. ⚡ BOOM! Run §23617381060 — NOMINAL.
|
Beta Was this translation helpful? Give feedback.
-
|
This discussion has been marked as outdated by Sergo - Serena Go Expert. A newer discussion is available at Discussion #23270. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Executive Summary
Today's run (32) extended the YAML injection audit campaign to
compiler_pre_activation_job.go, a file not previously analyzed, discovering two unquoted%sembeddings for user-controlledskip-rolesandskip-botsfrontmatter fields. These follow the exact same pattern as theGH_AW_COMMAND: %sfinding from run 30 but in the pre-activation job. The new exploration half audited manual byte-level string truncation in four CLI files handling MCP server/tool descriptions, finding that all four slice byte offsets directly into potentially Unicode/emoji-rich content from external MCP servers—a separate and more pervasive instance of thestringutil.Truncateissue first noted in run 18. A third finding identifies a single-quoted YAML scalar wrapping JSON content that may contain embedded single-quotes. Thirty-two runs now track 96 total findings with a cumulative average success score of 7.94/10.🛠️ Serena Tools Update
Tools Snapshot
Tool Capabilities Used Today
search_for_pattern— scannedcompiler_*.goforfmt.Fprintfwith unquoted%sin YAML stringsgrep+bash— verified field origin (extractStringSliceField), no validation on skip-roles/skip-bots values; located all manual byte-slice truncation sites in CLIread_file— confirmed code context incompiler_pre_activation_job.goandmcp_tool_table.go📊 Strategy Selection
Cached Reuse Component (50%)
Previous Strategy Adapted:
double-quote-injection-deepdive-plus-yaml-string-value-audit(run 31, score 8)compiler_yaml.goandengine_helpers.go; this run pivots to the pre-activation job file, which generates a distinct set of GHA YAML steps and had not been audited for injection patterns.search_for_patternglob fromcompiler_yaml.goto allcompiler_*.gofiles, then narrowed manually tocompiler_pre_activation_job.goandcompiler_yaml_main_job.goafter initial hits.New Exploration Component (50%)
Novel Approach: Unicode/byte-level truncation safety audit across CLI display code
grepfor[:N] + "..."anddescription[:patternspkg/cli/MCP list/display filesCombined Strategy Rationale
The cached component hunts security/correctness bugs in the compiled YAML output (high severity). The new component hunts a different class—data corruption at display boundaries in the CLI (medium severity). Together they cover two layers: generated CI artifacts and CLI rendering. The cached YAML injection work directly extends known patterns, maximizing new findings per query; the Unicode audit fills a gap identified in run 18 that was never fully mapped to all call sites.
🔍 Analysis Execution
Codebase Context
pkg/)pkg/workflow/(compiler chain),pkg/cli/(MCP display)compiler_pre_activation_job.go,compiler_yaml_main_job.go,mcp_list.go,mcp_tool_table.go,mcp_registry_list.go,mcp_inspect_mcp.goFindings Summary
📋 Detailed Findings
High Priority Issues
Finding 1 — Unquoted YAML injection via skip-roles/skip-bots frontmatter fields
pkg/workflow/compiler_pre_activation_job.go:194,211data.SkipRolesanddata.SkipBotsoriginate from user workflow frontmatter viaextractStringSliceField(), which performs zero content validation—any string accepted by the YAML parser is accepted. The joined value is embedded directly as an unquoted YAML scalar in the compiled GitHub Actions workflow.A
skip-rolesorskip-botsentry containing a newline followed byKEY: valueinjects an arbitrary env key into the step. A value containing:(colon-space) produces a YAML mapping node instead of a plain scalar, breaking parsing. The impact is full YAML injection into the compiled CI/CD workflow.Compare: the adjacent line 195 writes
GH_AW_WORKFLOW_NAME: %q\n—using%q(Go double-quoted string), which is safe. Lines 194 and 211 are inconsistent with that safe pattern.Severity: High — user-controlled workflow frontmatter controls compiled YAML step environment
Medium Priority Issues
Finding 2 — Byte-level string truncation corrupts UTF-8 in MCP display code
Four manual truncation sites in
pkg/cli/:mcp_list.godescription[:77] + "..."mcp_tool_table.godescription[:truncateAt] + "..."mcp_registry_list.godescription[:77] + "..."mcp_inspect_mcp.godescription[:37] + "..."All four slice the
descriptionstring at a byte offset. MCP tool and server descriptions are sourced from external MCP servers and can contain emoji (4 bytes each in UTF-8), CJK characters (3 bytes), or other multi-byte sequences. If the truncation boundary falls inside a multi-byte code point, the resulting string is invalid UTF-8, producing replacement characters (U+FFFD) or display corruption in terminals.stringutil.Truncate()exists in the codebase (pkg/stringutil/stringutil.go:16) with the same byte-level implementation. None of these four sites use it—they bypass it with direct slicing—andstringutil.Truncateitself has the same bug (noted since run 18). This is thus 5 affected locations sharing the root cause.Fix: replace
s[:n]with a rune-aware truncation, e.g.:Severity: Medium — corrupted terminal output for users of
gh aw mcp list,gh aw mcp inspect, and registry browsing commands when server descriptions contain non-ASCII textFinding 3 — Single-quoted YAML scalar wrapping JSON that may contain embedded single-quotes
pkg/workflow/compiler_yaml_main_job.go:109The JSON-serialized repository imports are wrapped in YAML single-quotes (
'...'). In YAML, the only valid escape inside a single-quoted scalar is''(doubling the single-quote). Raw JSON does not contain single-quotes in its structural syntax (it uses double-quotes), but the string values inside the JSON (e.g., ref names fromimports:frontmatter) can contain single-quote characters.If any import spec value—owner, repo, ref, or path—contains a
', the emitted single-quoted YAML scalar is invalid. For example, an import refit's-a-branchwould produce:This is a YAML parse error at runtime. Refs with apostrophes are valid git refs; git does not forbid them.
This is the same class of issue as
formatYAMLValue(runs 21–22) but in a new code path.Severity: Medium — malformed compiled workflow YAML when import ref names contain single-quotes; runtime YAML parse failure in the CI runner
✅ Improvement Tasks Generated
Task 1: Fix Unquoted YAML Env Injection in skip-roles/skip-bots Steps
Issue Type: YAML Injection / Missing Quoting
Problem:
compiler_pre_activation_job.go:194and:211embed user-controlledskip-rolesandskip-botsfrontmatter values as unquoted%sin GitHub Actions YAMLenv:blocks. A value containing:,\n, or YAML special characters produces malformed or injected compiled YAML.Location(s):
pkg/workflow/compiler_pre_activation_job.go:194—GH_AW_SKIP_ROLES: %spkg/workflow/compiler_pre_activation_job.go:211—GH_AW_SKIP_BOTS: %sImpact:
Before:
After:
Using
%q(Go'sstrconv.Quoteformat) produces a properly double-quoted Go string literal, which is a valid YAML double-quoted scalar. This matches the safe pattern already used on the adjacentGH_AW_WORKFLOW_NAME: %q\nline in the same function.Validation:
skip-roles: ["admin"]remains unchangedskip-roles: ["admin: injection"]— should produce safe quoted outputGH_AW_SKIP_*: %spatterns in other compiler filesEstimated Effort: Small
Task 2: Fix Byte-Level String Truncation to Use Rune-Aware Logic
Issue Type: Unicode / Data Corruption
Problem:
Five locations (four in
pkg/cli/plusstringutil.Truncateitself) truncate strings at byte offsets usings[:n]. External MCP server and tool descriptions can contain emoji and multi-byte Unicode characters. Byte-level slicing at a multi-byte boundary produces invalid UTF-8, rendering as replacement characters (U+FFFD) in terminal output.Location(s):
pkg/stringutil/stringutil.go:22,24—stringutil.Truncateitselfpkg/cli/mcp_list.go:244—description[:77] + "..."pkg/cli/mcp_tool_table.go:63—description[:truncateAt] + "..."pkg/cli/mcp_registry_list.go:59—description[:77] + "..."pkg/cli/mcp_inspect_mcp.go:388—description[:37] + "..."Impact:
Before (
stringutil/stringutil.go):After (
stringutil/stringutil.go):The four CLI files should then be migrated to call
stringutil.Truncate(description, N)instead of their inline byte slices, or updated with the same[]runepattern for their hardcoded limits.Validation:
Truncatewith a string starting with 4-byte emoji (e.g.,"🎉Hello") at variousmaxLenvaluesgo vet/go test ./pkg/stringutil/...Estimated Effort: Small–Medium
Task 3: Fix Single-Quoted YAML Scalar for JSON Repository Imports
Issue Type: YAML Injection / Missing Escaping
Problem:
compiler_yaml_main_job.go:109wraps JSON-marshalled repository import data in YAML single-quotes. JSON string values inside the payload can contain single-quote characters (e.g., a git ref namedit's-a-branch). In YAML, single-quoted scalars require''to represent a literal single-quote; raw'is not valid and produces a YAML parse error at CI runtime.Location(s):
pkg/workflow/compiler_yaml_main_job.go:109—GH_AW_REPOSITORY_IMPORTS: '%s'Impact:
Before:
After (option A — use
%qfor double-quoted YAML scalar):Option A is the simplest fix:
%qproduces a Go-quoted string which is also a valid YAML double-quoted scalar. JSON does not contain backslash sequences that would conflict with YAML double-quote escaping, so this is safe.Validation:
owner/repo/path@it's-weirdGH_AW_REPOSITORY_IMPORTSvalue after fixEstimated Effort: Small
📈 Success Metrics
This Run
Reasoning for Score
📊 Historical Context
Strategy Performance
stringutil.Truncatewas identified; this run maps 4 additional CLI bypass sitesCumulative Statistics
🎯 Recommendations
Immediate Actions
%s→%qforGH_AW_SKIP_ROLESandGH_AW_SKIP_BOTSincompiler_pre_activation_job.go:194,211stringutil.Truncateto use[]runeand migrate 4 inline CLI truncation sites to use it'%s'→%qforGH_AW_REPOSITORY_IMPORTSincompiler_yaml_main_job.go:109Long-term Improvements
The unquoted
%spattern in YAML env generation now has at least 6 confirmed instances across the codebase (runs 30, 31, 32). A systematic pass replacing allfmt.Fprintf(yaml, "... KEY: %s\n", ...)patterns in compiler files with either%qor JSON-marshalled output would eliminate the entire class. A linter rule or a helper functionwriteYAMLEnvVar(w, key, value string)that always uses%qwould prevent recurrence.🔄 Next Run Preview
Suggested Focus Areas
pkg/workflow/strings that are embedded into compiled stepname:orrun:fields—step names are displayed in GitHub Actions UI and could contain Unicode from user frontmatterpkg/workflow/forjson.Unmarshalcall sites that discard errors silently (expanding theimport_field_extractor.gopattern found in run 16 to other files)Strategy Evolution
The YAML injection sweep has now covered
compiler_yaml.go,engine_helpers.go,compiler_pre_activation_job.go, andcompiler_yaml_main_job.go. Remaining unaudited compiler files includecompiler_safe_outputs_job.go(already spot-checked in earlier runs but not fully swept for unquoted%s) and any new engine execution files added in recent commits.References:
Run ID: 23616076030 · Strategy: yaml-injection-skip-roles-plus-unicode-truncation-audit
Beta Was this translation helpful? Give feedback.
All reactions