[sergo] Sergo Report: HTTP Client Loop + UTF-8 Truncation Audit - 2026-03-11 #20599
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-03-12T22:24:47.239Z.
|
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.
-
Overview
Today's run (18th) audited the newly refactored codebase from commit
60651a8— which splitcommands.gointoversion.go,agent_download.go, andworkflows.go, introduced shareddeps_helpers.go, and eliminated semantic duplicates includingtruncateSnippet. Two distinct analyses confirmed concrete improvements in the refactor while uncovering 3 actionable issues that require follow-up.Key findings: The refactoring added a proper HTTP client timeout to
agent_download.go(a partial fix of run-17's issue), but introduced a byte-vs-rune truncation hazard in the newstringutil.Truncateaffecting 23 call sites. Separately,deps_outdated.gocreates a freshhttp.Clienton every dependency check, defeating Go's TCP connection pool.Serena Tools Snapshot
Newly Discovered Inactive Tools
execute_shell_commandswitch_modesprepare_for_new_conversationsummarize_changesread_file,create_text_file,delete_lines,insert_at_line,replace_content,replace_linesrestart_language_serveropen_dashboardjet_brains_find_symbol,jet_brains_find_referencing_symbols,jet_brains_get_symbols_overview,jet_brains_type_hierarchyremove_projectThese tools are present in the Serena server but excluded from the current
codexcontext/editingmode configuration. The JetBrains tools andexecute_shell_commandare particularly noteworthy new capabilities.Strategy Selection
Cached Reuse Component (50%)
Previous strategy adapted:
http-response-size-audit(run 17, score 8)commands.go:75,100usedclient.Get(url)without context andio.ReadAllwithout size limitcommands.gointoagent_download.go— a perfect re-examination targetagent_download.goand expanded todeps_outdated.gowhich appeared in the same HTTP client audit sweepagent_download.go), butio.ReadAllsize cap still missingNew Exploration Component (50%)
Novel approach: HTTP client per-call-in-loop + UTF-8 byte-truncation audit
grep,read_file(fallback pattern — Serena used for project activation and config inspection)deps_helpers.goshared parser + newstringutil.Truncateshared utility could introduce subtle correctness issues at their many call sitespkg/cli/deps_outdated.go,pkg/stringutil/stringutil.go,pkg/workflow/markdown_security_scanner.goCombined Strategy Rationale
The HTTP response size audit (cached) naturally led to examining all HTTP clients in the
pkg/cli/layer. Oncedeps_outdated.gowas opened for the LimitReader audit, the sequential client-per-call loop became immediately apparent. The new exploration of thestringutil.Truncatefunction emerged directly from the commit'struncateSnippetremoval — examining what the replacement function actually does revealed the byte-slicing behavior.Codebase Context
60651a8— refactor: eliminate semantic duplicates, delete stub files, split commands.goagent_download.go,version.go,workflows.go,deps_helpers.gocompiler_safe_outputs_shared.go,compiler_safe_outputs_discussions.goFindings Summary
Detailed Findings
Finding 1 (High):
deps_outdated.go— Newhttp.ClientCreated Per Dependency in Sequential LoopLocation:
pkg/cli/deps_outdated.go:161(insidegetLatestVersion), called from loop atpkg/cli/deps_outdated.go:66Problem:
getLatestVersion()creates a fresh&http.Client{Timeout: 5 * time.Second}on every invocation. This function is called in a sequentialfor _, dep := range depsloop insideCheckOutdatedDependencies. With N direct dependencies (a typical Go project may have 20–50), this means:proxy.golang.org— Go's built-in HTTP connection pooling (insidehttp.DefaultTransport) is completely bypassed because each new client has its own transport with an empty poolImpact:
5stimeout applies per request; total command time can reach5s × Nin the worst case (all timeouts)deps outdatedcommands unnecessarilyRecommendation: Create a single
*http.ClientinCheckOutdatedDependenciesand pass it togetLatestVersion. Optionally add bounded parallel execution withgolang.org/x/sync/errgroup.Validation:
go vetand existing tests after refactorgetLatestVersionsignature change propagates to all callershttp.Transport{MaxIdleConns: 10, IdleConnTimeout: 30 * time.Second}for explicit pool configurationEstimated Effort: Small
Finding 2 (Medium):
stringutil.TruncateByte-Level Slicing Corrupts UTF-8Location:
pkg/stringutil/stringutil.go:16-23(introduced in commit60651a8), called from 23+ sites inpkg/workflow/markdown_security_scanner.goProblem: The
stringutil.Truncatefunction introduced in the latest refactoring commit uses byte-level operations —len(s)counts bytes ands[:maxLen-3]byte-slices the string. When the truncation point falls inside a multi-byte UTF-8 codepoint (e.g., an emoji🌍or non-ASCII comment text), the result is an invalid UTF-8 sequence:Affected call sites (23 in markdown_security_scanner.go):
Workflow
.mdfiles increasingly contain non-ASCII content (Unicode identifiers, emoji in comments, multilingual step names). When a security scan finds a match near byte 77–80 in such a line, the snippet field contains an invalid UTF-8 byte sequence, causing rendering issues in terminals and potential JSON marshaling failures (Go'sencoding/jsonmarshals invalid UTF-8 as replacement characters or errors withjson.Marshal).Additionally,
workflow_errors.go:50,152performs the same manual byte-level truncation without using the new shared function — a further inconsistency.Recommendation: Fix
Truncateto count and slice by rune:Also update
workflow_errors.go:49-51andworkflow_errors.go:149-151to usestringutil.Truncatefor consistency.Validation:
Truncatewith multi-byte characters (emoji, CJK, accented Latin)json.Marshalof aSecurityFindingcontaining a truncated UTF-8 snippetEstimated Effort: Small
Finding 3 (Medium):
io.ReadAllWithout Size Limit — Unbounded Memory ConsumptionLocations:
pkg/cli/agent_download.go:74(refactored fromcommands.goin this commit)pkg/cli/deps_outdated.go:172pkg/cli/mcp_registry.go:95,112(unfixed from run 17)pkg/parser/remote_fetch.go:466(unfixed from run 17)Problem: Multiple HTTP response bodies are read with
io.ReadAll(resp.Body)without a size cap. A malicious server, a misconfiguration, or a CDN returning an error page could cause unbounded memory allocation:Note on partial improvement: The latest commit refactored
commands.gointoagent_download.goand added a proper HTTP client withTimeout: 30 * time.Second. This is a genuine improvement over run 17's finding where the originalcommands.go:75usedclient.Get(url)with the default (no-timeout) client. However, the body size cap was not added.Impact: A server sending a multi-megabyte response body (e.g., a misconfigured proxy returning an error page, or a GitHub API pagination response for advisories at
mcp_registry.go) can exhaust process memory. Theagent_download.gocase downloads user-controlled content (agentic-workflows.agent.md) from a GitHub raw URL — a compromised CDN or DNS hijack could serve arbitrarily large content.Recommendation: Apply
io.LimitReaderat each site:Use appropriate limits per call site:
agent_download.go: 10 MiB (markdown file)deps_outdated.go: 64 KiB (JSON object with version info)mcp_registry.goerror body: 4 KiBmcp_registry.gosuccess body: 1 MiB (server list)Validation:
Estimated Effort: Small
Improvement Tasks Generated
Task 1: Lift
http.ClientOut ofgetLatestVersionLoop indeps_outdated.goIssue Type: Performance / Resource Management
Problem:
getLatestVersioncreates a newhttp.Clientper call, bypassing TCP connection pool reuse. Called sequentially per dependency.Location:
pkg/cli/deps_outdated.go:161(client creation),pkg/cli/deps_outdated.go:66(call site loop)Impact:
deps_outdated.go)deps outdatedcommand; worst-case N×5s timeout if proxy is unreachableEstimated Effort: Small
Task 2: Fix
stringutil.Truncateto Use Rune-Level SlicingIssue Type: Correctness / UTF-8 Safety
Problem:
Truncateintroduced in this commit uses byte-levellen()ands[:n], producing invalid UTF-8 when truncating strings with multi-byte characters. Affects 23 security scanner call sites.Location:
pkg/stringutil/stringutil.go:16-23,pkg/workflow/markdown_security_scanner.go(allTruncatecalls)Impact:
workflow_errors.gofor consistency fixEstimated Effort: Small
Task 3: Add
io.LimitReaderto All External HTTP Response ReadsIssue Type: Security / Memory Safety
Problem:
io.ReadAll(resp.Body)without size cap on external HTTP responses allows unbounded memory consumption. Theagent_download.gopartial fix (added client timeout) demonstrates the pattern — the size cap is the remaining half.Locations:
agent_download.go:74,deps_outdated.go:172,mcp_registry.go:95,112,remote_fetch.go:466Impact:
agent_download.gowhich downloads user-referenced content from GitHubEstimated Effort: Small
Success Metrics
This Run
agent_download.go,deps_outdated.go,deps_helpers.go,workflows.go,version.go,stringutil.go,markdown_security_scanner.go)Score Reasoning
Historical Context
Strategy Performance History
Cumulative Statistics
Top Recurring Unfixed Issues
The following issues have been reported in multiple runs and remain outstanding:
GetSupportedEngines/GetAllEngines/GetEngineByPrefixnon-deterministic map iteration — 10 runsexpression_validation.go:400regexp inside for loop — 5 runspr_command.go:transferPR()os.Chdir without CWD restore — 3 runsgateway_logs.go:336,777missingscanner.Buffer()— 2 runsDependencyGraphno mutex on shared maps — 2 runsRecommendations
Immediate Actions
http.Clientout ofgetLatestVersionloop; add optional bounded parallelismTruncateto use rune-level slicing; add UTF-8 test coverageio.LimitReaderto allio.ReadAll(resp.Body)call sitesLong-term Improvements
var httpClient = &http.Client{...}constants inpkg/cli/for HTTP-heavy functions rather than per-call constructionstringutilthat string manipulation functions must preserve valid UTF-8; add to code review checklistagentic_engine.gohas been reported 10 times; it's a genuine data race that could cause flaky CI failuresNext Run Preview
Suggested Focus Areas
findAvailablePortTOCTOU:mcp_inspect_mcp_scripts_server.go:27-40— port is found, listener closed, then port returned; another process could steal the port between Close() and the Node.js server bindingExecuteWithRepeatsignal handling:retry.go:70-82usesselect { case <-sigChan: ... default: }(non-blocking check) inside a for loop; signal is only checked between iterations, not during executionjet_brains_type_hierarchyorexecute_shell_commandif they become active to get deeper type analysisStrategy Evolution
execute_shell_commandinactive Serena tool, if activated, would enable runninggo build ./...andgo vet ./...directly from within Serena for deeper diagnostic coverageGenerated by Sergo — The Serena Go Expert
Run ID: §22976946291
Strategy: http-client-loop-plus-utf8-truncation-audit
References:
Beta Was this translation helpful? Give feedback.
All reactions