[sergo] Sergo Report: http-network-audit-plus-dispatch-context - 2026-03-12

[github-actions[bot]]

Executive Summary

Run 19 applied a two-pronged strategy: extending the recurring HTTP client/response-size audit (from runs 17–18) to two dependency-analysis files not previously examined, while simultaneously applying the context-propagation anti-pattern check to the newly-refactored dispatch.go code from PR #20715.

Three actionable findings were identified. Two are HIGH severity: deps_outdated.go creates a new http.Client per version-check call in a sequential loop (no TCP connection reuse, no response-size cap), and deps_security.go reads GitHub Advisory API responses without a size limit while also only fetching 100 advisories with no pagination loop (first reported in run 8). One is MEDIUM: the refactored fetchAndSaveRemoteDispatchWorkflows in dispatch.go calls getRepoDefaultBranch which spawns a gh api subprocess with no context.Context parameter — making the network call uncancellable during gh aw add.

---

🛠️ Serena Tools Update

Tools Snapshot

• Total Tools Available: 40 (23 active, 17 inactive)
• New Tools Since Last Run: None — same 40 tools as run 18
• Removed Tools: None
• Modified Tools: None

Tool Capabilities Used Today

• activate_project — initialized Serena LSP for the workspace
• check_onboarding_performed — verified session state
• Native Grep/Read/Bash tools — bulk pattern search and file reading (Serena search_for_pattern avoided due to known multiline timeout issues)

---

📊 Strategy Selection

Cached Reuse Component (50%)

Previous Strategy Adapted: http-response-size-audit (run 18, 2026-03-11)

• Original Success Score: 8/10
• Last Used: 2026-03-11
• Why Reused: Discovered two additional files (deps_outdated.go, deps_security.go) containing io.ReadAll without size limits + per-call http.Client construction — same pattern not yet remediated elsewhere in the codebase
• Modifications: Expanded scope from agent_download.go/mcp_registry.go/remote_fetch.go to the dependency-audit subsystem. Cross-referenced run 8's pagination finding for deps_security.go to confirm it remains unfixed and compound the issue with the new unbounded-read dimension

New Exploration Component (50%)

Novel Approach: context-propagation-dispatch — applying the "context-free subprocess" pattern from run 4 and run 15 to newly-added code in dispatch.go (refactored in the latest squash commit)

• Tools Employed: Grep, Read, Bash
• Hypothesis: New code added in a large refactor commit often misses context propagation patterns already established elsewhere
• Target Areas: pkg/cli/dispatch.go, pkg/cli/update_workflows.go (for getRepoDefaultBranch implementation)

Combined Strategy Rationale

The cached component exploits depth (re-examining the same architectural pattern in new files), while the new component exploits recency (examining code added in the latest commit). Together they provide both breadth (multiple packages) and temporal coverage (existing bugs + freshly introduced patterns).

---

🔍 Analysis Execution

Codebase Context

• Total Go Files in pkg/: 562
• Packages Analyzed: pkg/cli (deps_outdated.go, deps_security.go, dispatch.go, update_workflows.go)
• Focus Areas: Dependency audit subsystem, dispatch workflow add flow

Findings Summary

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

---

📋 Detailed Findings

High Priority Issues

Finding 1 — deps_outdated.go:161 New http.Client per call in sequential loop

CheckOutdatedDependencies iterates over every go.mod dependency in a for _, dep := range deps loop and calls getLatestVersion(dep.Path, ...) for each. Inside getLatestVersion:

// pkg/cli/deps_outdated.go:161
client := &http.Client{Timeout: 5 * time.Second}
resp, err := client.Get(url)

Each call allocates a new http.Client, establishes a fresh TCP connection to proxy.golang.org, and performs a full TLS handshake. For a project with N go.mod dependencies this means N sequential TLS round-trips with no connection pool reuse — compounding latency linearly. Additionally, line 172 reads the response body with no size cap:

body, err := io.ReadAll(resp.Body)

The Go proxy API returns a small JSON object per module, so the practical risk of the unbounded read is low; but the per-call client construction has measurable latency cost at scale.

Finding 2 — deps_security.go:135,157 Unbounded response read + pagination gap

querySecurityAdvisories fetches the GitHub Security Advisory API with a hard-coded per_page=100 and no pagination loop:

// pkg/cli/deps_security.go:135
url := "https://api.github.com/advisories?ecosystem=go&per_page=100"

// pkg/cli/deps_security.go:157
body, err := io.ReadAll(resp.Body)

The GitHub Go advisory database contains 1,600+ entries. With per_page=100 and no Link: <…>; rel="next" header handling, the function silently examines only ~6% of known Go advisories per invocation. A dependency with a CVE outside the first 100 results will never be flagged. This was first reported in run 8 (2026-02-26) and remains unfixed. The io.ReadAll without a http.MaxBytesReader or io.LimitReader adds a second dimension: a large or adversarial API response will consume unbounded memory.

The function also instantiates its own http.Client on every call (line 136), though since querySecurityAdvisories is only called once per CheckSecurityAdvisories invocation this is a style concern rather than a performance issue.

Medium Priority Issues

Finding 3 — dispatch.go:93 Context-free subprocess in new dispatch code

fetchAndSaveRemoteDispatchWorkflows has no context.Context parameter in its signature:

// pkg/cli/dispatch.go:77
func fetchAndSaveRemoteDispatchWorkflows(content string, spec *WorkflowSpec, targetDir string, verbose bool, force bool, tracker *FileTracker, downloaders ...fileDownloadFn) error {

When spec.Version is empty (no pinned ref), the function resolves the default branch by calling:

// pkg/cli/dispatch.go:93
defaultBranch, err := getRepoDefaultBranch(spec.RepoSlug)

getRepoDefaultBranch (in update_workflows.go:228) spawns a gh api /repos/(slug) subprocess:

output, err := workflow.RunGH("Fetching repo info...", "api", "/repos/"+repo, "--jq", ".default_branch")

workflow.RunGH (and its underlying exec.Command) is called without a context, making the subprocess uncancellable. If the GitHub API call hangs (network issue, rate limit, etc.), the gh aw add command will block indefinitely with no Ctrl-C exit path. This is the same pattern identified in runs 8, 15 (git_helpers.go), and 17 (mcp_inspect_mcp.go), now appearing in the freshly-refactored dispatch.go code introduced in PR #20715.

---

✅ Improvement Tasks Generated

Task 1: Hoist http.Client to package-level var in deps_outdated.go

Issue Type: HTTP Client / Network Efficiency

Problem: getLatestVersion constructs a new http.Client on every call, called sequentially for each go.mod dependency.

Location(s):

• pkg/cli/deps_outdated.go:161 — client construction inside function
• pkg/cli/deps_outdated.go:172 — io.ReadAll without size cap

Impact:

• Severity: High
• Affected Files: 1
• Risk: Degraded performance for projects with many dependencies (N sequential TLS handshakes); minor memory risk from unbounded read

Recommendation: Hoist client to a package-level variable and add an io.LimitReader on the response body.

Before:

func getLatestVersion(modulePath, currentVersion string, verbose bool) (string, time.Duration, error) {
    url := fmt.Sprintf("https://proxy.golang.org/%s/`@latest`", modulePath)
    client := &http.Client{Timeout: 5 * time.Second}
    resp, err := client.Get(url)
    ...
    body, err := io.ReadAll(resp.Body)

After:

var depsHTTPClient = &http.Client{Timeout: 5 * time.Second}

func getLatestVersion(modulePath, currentVersion string, verbose bool) (string, time.Duration, error) {
    url := fmt.Sprintf("https://proxy.golang.org/%s/`@latest`", modulePath)
    resp, err := depsHTTPClient.Get(url)
    ...
    body, err := io.ReadAll(io.LimitReader(resp.Body, 64*1024))

Validation:

• Run existing tests
• Verify connection reuse with HTTP tracing under load
• Confirm size limit (64 KB) is sufficient for Go proxy JSON responses

Estimated Effort: Small

---

Task 2: Add pagination and response-size limit to querySecurityAdvisories

Issue Type: Pagination Gap + HTTP Response Safety

Problem: deps_security.go only fetches 100 of 1,600+ Go security advisories and reads the full response body without a size cap.

Location(s):

• pkg/cli/deps_security.go:135 — hard-coded per_page=100, no pagination
• pkg/cli/deps_security.go:157 — io.ReadAll(resp.Body) without size limit

Impact:

• Severity: High
• Affected Files: 1
• Risk: ~94% of known Go CVEs are never checked; a degraded GitHub API response can exhaust memory

Recommendation: Add a pagination loop following the GitHub API Link header, cap individual response reads with io.LimitReader, and add a total-advisory count ceiling to bound execution time.

Before:

url := "https://api.github.com/advisories?ecosystem=go&per_page=100"
// single request, no pagination
body, err := io.ReadAll(resp.Body)

After:

const maxAdvisories = 2000
var allAdvisories []GitHubAdvisoryResponse
url := "https://api.github.com/advisories?ecosystem=go&per_page=100"

for url != "" && len(allAdvisories) < maxAdvisories {
    resp, err := client.Do(req)
    // ...
    body, err := io.ReadAll(io.LimitReader(resp.Body, 4*1024*1024))
    // ...
    var page []GitHubAdvisoryResponse
    json.Unmarshal(body, &page)
    allAdvisories = append(allAdvisories, page...)
    url = extractNextPageURL(resp.Header.Get("Link"))
}

Validation:

• Run existing security check tests
• Verify CVEs beyond position 100 are now detected
• Add a test with mock paginated API responses
• Confirm the extractNextPageURL helper handles absent Link headers

Estimated Effort: Medium

---

Task 3: Add context.Context to fetchAndSaveRemoteDispatchWorkflows and getRepoDefaultBranch

Issue Type: Context Propagation — Uncancellable Subprocess

Problem: dispatch.go::fetchAndSaveRemoteDispatchWorkflows calls getRepoDefaultBranch which spawns an uncancellable gh api subprocess when no pinned ref is available.

Location(s):

• pkg/cli/dispatch.go:77 — function signature lacks context.Context
• pkg/cli/dispatch.go:93 — getRepoDefaultBranch(spec.RepoSlug) called without context
• pkg/cli/update_workflows.go:228 — getRepoDefaultBranch spawns subprocess without context

Impact:

• Severity: Medium
• Affected Files: 2 (dispatch.go, update_workflows.go)
• Risk: gh aw add blocks indefinitely on network failure with no Ctrl-C escape; same pattern previously found in git_helpers.go (run 15) and mcp_inspect_mcp.go (run 8)

Recommendation: Thread context.Context through the call chain.

Before:

func fetchAndSaveRemoteDispatchWorkflows(content string, spec *WorkflowSpec, ...) error {
    defaultBranch, err := getRepoDefaultBranch(spec.RepoSlug)

func getRepoDefaultBranch(repo string) (string, error) {
    output, err := workflow.RunGH("Fetching repo info...", "api", "/repos/"+repo, ...)

After:

func fetchAndSaveRemoteDispatchWorkflows(ctx context.Context, content string, spec *WorkflowSpec, ...) error {
    defaultBranch, err := getRepoDefaultBranch(ctx, spec.RepoSlug)

func getRepoDefaultBranch(ctx context.Context, repo string) (string, error) {
    output, err := workflow.RunGHContext(ctx, "Fetching repo info...", "api", "/repos/"+repo, ...)

Validation:

• Update all call sites of fetchAndSaveRemoteDispatchWorkflows to pass context
• Verify workflow.RunGHContext (or equivalent) exists or add it
• Confirm Ctrl-C cancels the gh api subprocess mid-execution
• Run dispatch workflow add integration tests

Estimated Effort: Small

---

📈 Success Metrics

This Run

• Findings Generated: 3
• Tasks Created: 3
• Files Analyzed: 4 (deps_outdated.go, deps_security.go, dispatch.go, update_workflows.go)
• Success Score: 8/10

Reasoning for Score

• Findings Quality (3/4): Two HIGH findings with clear, measurable impact; one MEDIUM with a well-established precedent in the codebase
• Coverage (2/3): Focused on 4 files in 2 subsystems; good depth but limited breadth
• Task Generation (3/3): All three tasks have clear before/after code examples and validation checklists

---

📊 Historical Context

Strategy Performance

Run	Strategy	Score	Findings
3	error-patterns-plus-field-registry-audit	9/10	3
4	dependency-graph-race-audit-plus-npm-subprocess-timeout	9/10	3
8	context-sleep-audit-plus-polling-infra	8/10	3
18	http-client-loop-plus-utf8-truncation-audit	8/10	3
19	http-network-audit-plus-dispatch-context	8/10	3

Cumulative Statistics

• Total Runs: 19
• Total Findings: 57
• Total Tasks Generated: 57
• Average Success Score: 8.1/10
• Most Successful Strategy: error-patterns-plus-field-registry-audit (score 9)

Notable Unfixed Issues (oldest first)

The following issues have appeared in multiple reports and remain unaddressed, representing the highest-leverage fix opportunities:

1. GetSupportedEngines/GetAllEngines non-deterministic map iteration — reported 11 times (runs 2–19)
2. expression_validation.go:400 regexp.MustCompile inside for loop — reported 6 times (runs 14–19)
3. DependencyGraph no mutex on maps + debounce Stop() ignored — reported in runs 11–12
4. pr_command.go transferPR() CWD not restored — reported in runs 16–19
5. mcp_registry.go io.ReadAll without size limit — reported in runs 17–19

---

🎯 Recommendations

Immediate Actions

1. Fix deps_security.go pagination (Task 2) — HIGH priority; 94% of Go CVEs currently invisible to the security check. Relatively contained change with high security value.
2. Fix deps_outdated.go http.Client reuse (Task 1) — HIGH priority; quick win (hoist one variable, add LimitReader). Improves performance for any project with many dependencies.
3. Add context to fetchAndSaveRemoteDispatchWorkflows (Task 3) — MEDIUM; newly introduced in PR [code-simplifier] refactor: simplify dispatch_workflow handler clarity (#20708 follow-up) #20715, easy to fix before the pattern propagates further.

Long-term Improvements

• Establish a http.Client singleton policy: multiple files now independently create http.Client{} in function bodies. A package-level or injected http.Client pattern would prevent this class of issue from recurring.
• Address the multi-run unfixed issues: GetSupportedEngines map iteration and expression_validation.go regexp compilation have been reported 6–11 times without remediation — these warrant explicit tracking as tech-debt items.

---

🔄 Next Run Preview

Suggested Focus Areas

• Map iteration determinism: GetSupportedEngines/GetAllEngines/GetEngineByPrefix in agentic_engine.go have been flagged 11 times. A fresh look with a fix recommendation and test coverage analysis would add value.
• pr_command.go CWD restoration: transferPR() leaves the process in a deleted directory — 4 consecutive unfixed reports; a concrete patch suggestion may accelerate remediation.
• New WASM entry point: cmd/gh-aw-wasm/main.go (added in the squash commit) hasn't been analyzed yet.

Strategy Evolution

Consider a fix-verification strategy for run 20: instead of finding new issues, cross-check the 5 most critical unfixed items to confirm their status and produce targeted, minimal code patches to unblock remediation.

---

References:

• §23026362329

AI generated by Sergo - Serena Go Expert · history

• expires on Mar 13, 2026, 10:20 PM UTC