Move to Go 1.26.5 and clear reachable dependency vulnerabilities - #394
Open
dpage wants to merge 1 commit into
Open
Move to Go 1.26.5 and clear reachable dependency vulnerabilities#394dpage wants to merge 1 commit into
dpage wants to merge 1 commit into
Conversation
Codacy reports a large number of SCA advisories against this repository, but most of them are not reachable from Workbench code, so I ran govulncheck across all four Go modules to find the ones that actually are. Four came back, and this change clears all of them. Three are standard library issues fixed in the 1.26.4 and 1.26.5 patch releases: a privacy leak in the crypto/tls Encrypted Client Hello handling, arbitrary input included unescaped in net/textproto error messages, and inefficient candidate hostname parsing in crypto/x509. The fourth is an infinite loop in golang.org/x/text normalisation, reached through pgxpool connection setup in all three services and directly from the server's password dictionary handling. The Go version is raised in the four module files, in the collector, server and alerter Dockerfile builder stages, and in the CI and release workflows. The release workflow is the important one, because it previously built the published binaries with the affected toolchain, so the advisories would have shipped regardless of what the module files said. Where a workflow repeats the version in an `if:` guard, the guard moved with the matrix value; leaving those behind would have silently stopped the coverage and publish steps from matching. golang.org/x/text goes to v0.40.0, which is the current release rather than the v0.39.0 floor govulncheck reports. go mod tidy also advanced golang.org/x/sync to v0.22.0 as a resolution side effect. On the client, ECharts moves to v6.1.0 to resolve a cross-site scripting advisory, and postcss, js-yaml and brace-expansion are refreshed transitively; npm audit now reports no known vulnerabilities. govulncheck reports zero affected symbols in all four modules after this change, down from three, three, four and four. The remaining advisories it lists are golang.org/x/crypto/ssh and ssh/agent issues that no Workbench code calls. No production code changes, so there is no new test coverage to add.
WalkthroughThe PR standardizes Go 1.26.5 across modules, Docker builders, CI, E2E, and release workflows. It updates indirect Go dependencies, upgrades ECharts to 6.1.0, and documents the security-related dependency changes. ChangesGo toolchain and module updates
CI and release alignment
Client dependency and changelog updates
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/go.mod`:
- Line 3: Add pkg/** to the path filters for the lint, vet, and test jobs in
ci-server.yml, ci-alerter.yml, and ci-collector.yml, matching the service image
build behavior in ci-docker.yml.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 55d8e3be-c0c3-4cf2-a662-c30fe3c6ec89
⛔ Files ignored due to path filters (4)
alerter/src/go.sumis excluded by!**/*.sumclient/package-lock.jsonis excluded by!**/package-lock.jsoncollector/go.sumis excluded by!**/*.sumserver/src/go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
.github/workflows/ci-alerter.yml.github/workflows/ci-collector.yml.github/workflows/ci-e2e.yml.github/workflows/ci-server.yml.github/workflows/release.ymlalerter/Dockerfilealerter/src/go.modclient/package.jsoncollector/Dockerfilecollector/go.moddocs/changelog.mdpkg/go.modserver/Dockerfileserver/src/go.mod
| go 1.26.1 | ||
|
|
||
| toolchain go1.26.2 | ||
| go 1.26.5 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 'paths:|pkg/\*\*|alerter/\*\*|collector/\*\*|server/\*\*' .github/workflowsRepository: pgEdge/ai-dba-workbench
Length of output: 9785
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Docker workflow ---'
cat -n .github/workflows/ci-docker.yml
printf '%s\n' '--- Go service workflows and referenced build commands ---'
for f in .github/workflows/ci-server.yml .github/workflows/ci-alerter.yml .github/workflows/ci-collector.yml; do
printf '\n### %s\n' "$f"
cat -n "$f"
done
printf '%s\n' '--- Dockerfiles and compose references ---'
git ls-files '*Dockerfile' docker-compose.yml
for f in $(git ls-files '*Dockerfile'); do
printf '\n### %s\n' "$f"
cat -n "$f"
doneRepository: pgEdge/ai-dba-workbench
Length of output: 40508
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Shared module references ---'
for f in pkg/go.mod server/src/go.mod alerter/src/go.mod collector/go.mod docker-compose.yml; do
printf '\n### %s\n' "$f"
cat -n "$f"
done
printf '%s\n' '--- Deterministic workflow coverage check ---'
python3 - <<'PY'
from pathlib import Path
import re
workflow_files = [
Path(".github/workflows/ci-server.yml"),
Path(".github/workflows/ci-alerter.yml"),
Path(".github/workflows/ci-collector.yml"),
Path(".github/workflows/ci-docker.yml"),
]
for path in workflow_files:
text = path.read_text()
paths = re.findall(r"^\s*-\s*'([^']+)'", text, re.MULTILINE)
print(f"{path}: pkg/**={'pkg/**' in paths}, push={'push:' in text}, pull_request={'pull_request:' in text}")
compose = Path("docker-compose.yml").read_text()
services = re.findall(r"^([A-Za-z0-9_-]+):\s*$", compose, re.MULTILINE)
print("compose services:", services)
for service in ("server", "alerter", "collector"):
dockerfile = Path(service) / "Dockerfile"
text = dockerfile.read_text()
print(f"{dockerfile}: copies pkg={'COPY pkg/' in text}, builds={'RUN CGO_ENABLED=0 go build' in text}")
PYRepository: pgEdge/ai-dba-workbench
Length of output: 7677
Add pkg/** to the service CI path filters.
ci-docker.yml builds all three service images for pkg/** changes, but ci-server.yml, ci-alerter.yml, and ci-collector.yml skip their lint, vet, and test jobs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/go.mod` at line 3, Add pkg/** to the path filters for the lint, vet, and
test jobs in ci-server.yml, ci-alerter.yml, and ci-collector.yml, matching the
service image build behavior in ci-docker.yml.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
Codacy currently reports a large pile of SCA advisories against this
repository, and most of them are noise in the sense that nothing in the
Workbench actually calls the vulnerable code. Rather than bump things on
the strength of the advisory list, I ran
govulncheckacross all four Gomodules to establish which vulnerabilities are genuinely reachable from
our own call graphs. Four came back, and this change clears all four.
golang.org/x/textv0.33.0pgxpoolsetup in all three services, andserver/src/internal/auth/common_passwords.go:105crypto/tls(ECH privacy leak)net/textproto(unescaped input in errors)pkg,server,alertercrypto/x509(hostname parsing DoS)What changed
The Go version moves to 1.26.5 in the four module files, the three
service
Dockerfilebuilder stages, and the CI and release workflows.The release workflow is the one that actually mattered: it built the
published binaries with
1.26.2, so the standard library advisorieswould have shipped in released artefacts no matter what the module files
said. Codacy was in fact resolving some findings against that pin rather
than against
go.mod(affectedVersion: v1.26.2).Where a workflow repeats the Go version inside an
if:guard, the guardmoved along with the matrix value. Bumping only the matrix would have
left those conditions unmatchable and silently stopped the coverage and
publish steps from running, which is a quieter failure than a red build.
golang.org/x/textgoes to v0.40.0, the current release, rather thanstopping at the v0.39.0 floor
govulncheckreports.go mod tidyalsoadvanced
golang.org/x/syncto v0.22.0 as a resolution side effect.On the client, ECharts moves to v6.1.0 for a cross-site scripting
advisory, and
postcss,js-yamlandbrace-expansionare refreshedtransitively via
npm audit fix.npm auditnow reports no knownvulnerabilities, where it previously reported one moderate and four high.
Verification
govulncheckreports zero affected symbols in all four modules,down from three, three, four and four respectively. What it still
lists are
golang.org/x/crypto/sshandssh/agentadvisories that noWorkbench code calls.
pkg,collectorandalertertestsuites pass in full.
build succeeds, and 3,488 tests across 171 files pass.
Two things a reviewer should know rather than discover:
server/internal/toolshas two failing tests,TestStoreMemoryGeneratesEmbeddingIntegrationandTestRecallMemoriesGeneratesQueryEmbeddingIntegration, both failingwith
expected 3 dimensions, not 4000. This is the pre-existingvector(3)fixture problem from Gemini provider: (1) knowledge base search silently falls back to OpenAI due to missing gemini_embedding column in search_knowledgebase.go; (2) session startup fails with 400 "empty Part" error #337 and is not caused by thischange; I confirmed it reproduces identically on unmodified
mainwith the previous toolchain. It does not surface in CI because the CI
Postgres lacks pgvector, so these tests skip there.
gofmtflags sixpkg/**_test.gofiles, but it does so identicallyon
mainand under the old toolchain: those files use the project'sfour-space indentation, which collides with gofmt's tabs. I have left
them alone rather than fold a large unrelated reformat into a security
change.
No production code is modified, so there is no new coverage to add.
Related
Separately from this PR, I triaged the 40 code-level Codacy security
findings (SQL injection, hardcoded secrets, cookie flags, timing
attacks) and ignored them with recorded justifications; all 40 were
false positives in test code, with none in production code. Open
security items went from 155 to 115, and the 115 that remain are the
dependency advisories this PR addresses.
Summary by CodeRabbit
Security
Maintenance
Documentation