Skip to content

chore(deps): clear govulncheck — Go 1.26.7, excelize v2.11.0 - #8

Closed
saifulferoz wants to merge 1 commit into
xiidea:mainfrom
saifulferoz:chore/deps-govulncheck
Closed

chore(deps): clear govulncheck — Go 1.26.7, excelize v2.11.0#8
saifulferoz wants to merge 1 commit into
xiidea:mainfrom
saifulferoz:chore/deps-govulncheck

Conversation

@saifulferoz

Copy link
Copy Markdown
Contributor

Why

govulncheck started failing on every PR without any code change — both
advisories were published after main last ran CI on 2026-07-21. This is
independent of #7; it branches from main and touches no shim or Compat code.

GO-2026-6088 — encoding/xml, recursion depth during decode

Reached through excelize's row and conditional-format readers
(Rows.Columns, GetConditionalFormats, and sheetPartPaths's own
xml.Unmarshal).

The workflow asked for go-version: "1.26" and the runner's toolchain cache
served 1.26.5, which carries the vulnerability. Pinned to the 1.26.7
patch in both the workflow and the Dockerfile so the fixed stdlib is what
actually gets used.

GO-2026-5960 — excelize v2.10.1

Fixed in v2.11.0, which is a breaking upgrade: Chart.Title and
ChartAxis.Title changed from []RichTextRun to a ChartTitle struct whose
Paragraph field holds the runs. Migrated the three call sites in
compat/chart.go. The pin is presumably why this had not been taken already.

Verification

With the pinned toolchain:

$ GOTOOLCHAIN=go1.26.7 govulncheck ./registry/... ./limits/... ./exio/... ./compat/... ./core/...
No vulnerabilities found.

Was 2 before this change (1 stdlib, 1 excelize).

  • Full Go suite green under -race on go1.26.7 + excelize v2.11.0
  • go-test Docker stage pulls golang:1.26.7 and passes

New test

extension/core/titlecheck_test.go. The ChartTitle migration is the one
change here that could silently produce a wrong file rather than a build
error — assigning the wrong field would compile fine and just drop the title.
The test asserts the chart title and both axis titles actually reach the
generated chart XML. Confirmed it bites by blanking the title assignment.

Scope

No behaviour change intended beyond the dependency bump. The transitive
golang.org/x/* and mscfb bumps come from go mod tidy.

govulncheck started failing on every PR without any code change: both
advisories were published after main last ran CI on 2026-07-21.

GO-2026-6088 (encoding/xml, recursion depth during decode) — reached
through excelize's row and conditional-format readers. The workflow asked
for "1.26" and the runner's toolchain cache served 1.26.5, which carries
it. Pinned to the 1.26.7 patch in both the workflow and the Dockerfile so
the fixed stdlib is what actually gets used.

GO-2026-5960 (excelize v2.10.1) — fixed in v2.11.0, which is a breaking
upgrade: Chart.Title and ChartAxis.Title changed from []RichTextRun to a
ChartTitle struct whose Paragraph field holds the runs. Migrated the
three call sites in compat/chart.go; the pin is presumably why this was
not taken earlier.

Verified with the pinned toolchain: "No vulnerabilities found" (was 2).
Full Go suite green under -race, and the go-test Docker stage pulls
golang:1.26.7 and passes.

Adds core/titlecheck_test.go: the ChartTitle migration is the one change
here that could silently produce a wrong file rather than a build error,
so this asserts the chart and axis title text actually reaches the
generated chart XML. Confirmed it bites by blanking the title assignment.

No behaviour change intended beyond the dependency bump.
@saifulferoz

Copy link
Copy Markdown
Contributor Author

Superseded by #7, which now carries these commits directly.

d8b5089 (the dependency fix) was cherry-picked onto feature/phase-5-compat-completion as b5ea941, so the Go 1.26.7 pin, the excelize v2.11.0 upgrade with its ChartTitle migration, and titlecheck_test.go all land with #7. Verified the two branches carry identical content for every file this PR touched.

The cherry-pick did surface one thing this PR could not: wave 5.4's chart tests assert c.Title[0].Text against what v2.11.0 turns into a struct, so the test build breaks once both changes sit on the same branch. Fixed in e0f7bcc on #7. That interaction was invisible here because this branch is based on main, where those assertions do not exist yet.

Closing to avoid carrying the same commit in two open PRs. govulncheck is green on #7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant