fix(ci): silence bogus SA5011, portable repo root, and an SVG path parser hang

[avdoseferovic]

Description

Three CI failures, unrelated to each other.

Lint: 23 bogus SA5011 nil dereferences. All of them are in _test.go files and all have the same shape — if x == nil { t.Fatal(...) } guarding the dereference below it. The guard is correct. SA5011 needs staticcheck's "never returns" fact for testing.common.Fatal to see it, and golangci-lint does not carry that fact into a linted package reliably. It is nondeterministic: 23 reports on the runner, none locally with a cold cache (golangci-lint cache clean, also tried under GOMAXPROCS=4 --concurrency=4 to mimic the runner), and toggling any unrelated analyzer flips the outcome. That flakiness was already documented in .golangci.yml as the reason testableexamples was held back.

There is nothing to fix in the test code, so SA5011 is excluded in tests only, with the rationale recorded next to the rule. A real nil dereference in a test fails loudly with a panic and a stack trace, so little is lost; non-test code, which never calls t.Fatal, keeps the check. Enabling the exclusion retires the reason testableexamples was off, so it is enabled here — every example already carries its // Output: marker, so it reports nothing.

Windows: TestCache_LoadImage. buildPath derived the repository root with strings.ReplaceAll(dir, "internal/cache", "") over os.Getwd(), then joined with path.Join. The working directory is backslash-separated on Windows, so the replace matched nothing and the path became D:\a\paper\paper\internal\cache/test/assets/images/biplane.jpg. It now joins relative to the package directory with filepath, the way internal/pdf/font_pdf_test.go already resolves assets. This was the only instance of that pattern in the root module.

Fuzz: FuzzRasterizeWithLimit hung on <svg><pAth d="Z0 0L10 10C1 2 3 4 5 6Z"/></svg>. Z is the only path command that consumes no tokens, and readCommand lets a number token repeat the command in effect, so a number after Z re-applied Z without ever advancing the cursor. Where the Z had a current point, the loop also appended to path.ops on every pass, so the spin came with unbounded memory growth. Path data reaches this parser from <img src> content in untrusted documents, so this is a denial of service, not a cosmetic bug.

Closepath takes no operands, so a number following one cannot be an implicit repeat of it — the path is malformed and is now rejected, which is how this parser already treats every other grammar violation. parse additionally checks that each iteration advanced, so any future zero-operand command rejects the path instead of spinning. The crashing input is committed as a seed corpus entry, so plain go test ./pkg/svg/ replays it from now on.

Where should the reviewer start

• internal/svg/pathdata.go — the readCommand guard is the fix; the progress check in parse is the structural backstop.
• internal/svg/pathdata_test.go — two new cases in the existing malformed-input table; both hang without the fix.
• .golangci.yml — the new exclusion rule and its rationale, plus testableexamples moving into the enabled list.
• internal/cache/cache_test.go — buildPath is now a one-line filepath.Join; the comment records what it used to do and why that broke.

Verification

The first two fixes are confirmed by CI on this branch: the lint and test-windows jobs both passed on the commit before this one, which is what the earlier run of this PR was for.

For the parser fix: the two new table cases time out without it and pass with it. FuzzRasterizeWithLimit then ran 12.6M executions clean over two minutes, at 130–230k execs/sec against roughly 20k/sec in the failing CI run — the stall showing up in the throughput. All five fuzz targets were then run at the same 60s budget CI uses (pkg/reader FuzzParse, pkg/svg FuzzParseBytes and FuzzRasterizeWithLimit, pkg/html/css FuzzParseValues, pkg/html FuzzFromString) and all pass. make dod passes end to end (build, test, fmt, lint — no files reformatted).

The SA5011 exclusion was verified to match rather than assumed: a throwaway test file that genuinely triggers SA5011 reported 2 issues without the rule — the primary report and its SA5011(related information) line — and 0 with it. That probe file is not part of this PR.

Notes

The Windows fix is confirmed by CI, but the job stays continue-on-error: true; making it blocking is a separate call.

Because the SA5011 flake does not reproduce locally, the exclusion is verified against a synthetic SA5011 report rather than against the CI-only ones.

Fuzzing is a search, so a clean run is evidence rather than proof — later runs may surface other findings.

Related Issue

None.

Checklist

• Wrote unit tests for new/changed features.
• Executed make dod with no issues

🤖 Generated with Claude Code