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

.golangci.yml

@@ -38,14 +38,7 @@ linters:
     - recvcheck
     - revive
     - staticcheck
-    # Deliberately NOT enabled: testableexamples. Every example in
-    # example_test.go already carries the `// Output:` marker it would ask for,
-    # so it has nothing left to catch here. Enabling it as a fourth extra
-    # analyzer on top of modernize/usetesting/paralleltest tips golangci-lint
-    # into a state where staticcheck loses its "t.Fatal terminates" facts and
-    # reports 21 bogus SA5011 nil-dereferences in tests; dropping any one of the
-    # four clears them. Keeping SA5011 working is worth more than re-checking
-    # markers that are already present.
+    - testableexamples
     - testifylint
     - thelper
     - tparallel
@@ -211,6 +204,20 @@ linters:
         linters:
           - errcheck
         text: "^Error return value is not checked$"
+      # SA5011 needs staticcheck's "never returns" fact for testing.common.Fatal
+      # to see that `if x == nil { t.Fatal(...) }` guards the dereference below
+      # it. golangci-lint does not carry that fact into a linted package
+      # reliably: the same commit reports 23 guarded test dereferences on the CI
+      # runner and none locally with a cold cache, and toggling any unrelated
+      # analyzer flips the outcome, which is why testableexamples was held back
+      # until now. Every report so far has been one of those false positives, and
+      # a nil dereference that is real fails the test with a panic and a stack
+      # trace, so SA5011 is off in tests. Non-test code, which never calls
+      # t.Fatal, keeps it.
+      - path: _test\.go
+        linters:
+          - staticcheck
+        text: "SA5011"
       # Four tests must stay sequential, and each one says so in a comment at
       # the declaration. testing.AllocsPerRun pins GOMAXPROCS to 1 and panics
       # outright when called from a parallel test; goleak.VerifyNone inspects

internal/cache/cache_test.go

@@ -2,9 +2,7 @@ package cache_test
 
 import (
 	"fmt"
-	"os"
-	"path"
-	"strings"
+	"path/filepath"
 	"testing"
 
 	"github.com/avdoseferovic/paper/internal/cache"
@@ -94,22 +92,21 @@ func TestCache_LoadImage(t *testing.T) {
 		sut := cache.New()
 
 		// Act
-		err := sut.LoadImage(buildPath("/test/assets/images/biplane.jpg"), extension.Jpg)
+		err := sut.LoadImage(buildPath("test", "assets", "images", "biplane.jpg"), extension.Jpg)
 
 		// Assert
 		assert.Nil(t, err)
-		img, err := sut.GetImage(buildPath("/test/assets/images/biplane.jpg"), extension.Jpg)
+		img, err := sut.GetImage(buildPath("test", "assets", "images", "biplane.jpg"), extension.Jpg)
 		assert.Nil(t, err)
 		assert.NotNil(t, img)
 	})
 }
 
-func buildPath(file string) string {
-	dir, err := os.Getwd()
-	if err != nil {
-		return ""
-	}
-
-	dir = strings.ReplaceAll(dir, "internal/cache", "")
-	return path.Join(dir, file)
+// buildPath resolves a repository-relative asset path from this package's
+// directory, the way the font tests do. It used to strip the literal
+// "internal/cache" from the working directory, which matched nothing on Windows
+// and left the package directory in the path, and it joined with path.Join,
+// which builds slash-separated paths regardless of the platform.
+func buildPath(elem ...string) string {
+	return filepath.Join(append([]string{"..", ".."}, elem...)...)
 }

internal/svg/pathdata.go

@@ -25,12 +25,20 @@ func (path *svgPath) parse(data string) bool {
 		return false
 	}
 	for cursor.index < len(cursor.tokens) {
+		start := cursor.index
 		if !cursor.readCommand() {
 			return false
 		}
 		if !cursor.apply() {
 			return false
 		}
+		// Every iteration either consumes a command token or the operands of the
+		// command in effect, so the index always advances. This rejects the path
+		// rather than trusting that: a command that consumed nothing would spin
+		// here forever on input that comes from an untrusted document.
+		if cursor.index == start {
+			return false
+		}
 		// The command may have been rewritten (M implies L for the pairs that
 		// follow it), and the smooth-curve commands compare against that value.
 		cursor.previous = cursor.command
@@ -39,11 +47,16 @@ func (path *svgPath) parse(data string) bool {
 }
 
 // readCommand consumes a command token when the cursor is on one. A number token
-// repeats the command in effect, which is only valid once one has been seen.
+// repeats the command in effect, which is only valid once one has been seen and
+// only for a command that takes operands to consume.
 func (cursor *pathCursor) readCommand() bool {
 	if cursor.tokens[cursor.index].command != 0 {
 		cursor.command = cursor.tokens[cursor.index].command
 		cursor.index++
+	} else if cursor.command == 'Z' || cursor.command == 'z' {
+		// Closepath takes no operands, so a number following one is not an
+		// implicit repeat of it but a malformed path.
+		return false
 	}
 	if cursor.command == 0 {
 		return false

internal/svg/pathdata_test.go

@@ -136,6 +136,13 @@ func TestParsePathRejectsMalformedInput(t *testing.T) {
 		"vertical without point":   "V10",
 		"smooth cubic no current":  "S10 10 20 20",
 		"smooth quad no current":   "T10 10",
+		// Closepath takes no arguments, so a number after one cannot be an
+		// implicit repeat of it. Both of these used to loop forever: the second
+		// is the fuzz crasher from FuzzRasterizeWithLimit, which spun on a Z
+		// with no current point, and the first grew path.ops without bound
+		// because its close() had a current point to append.
+		"number after closepath":          "M0 0 L10 10 Z0 0",
+		"numbers after leading closepath": "Z0 0L10 10C1 2 3 4 5 6Z",
 	} {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()

pkg/svg/testdata/fuzz/FuzzRasterizeWithLimit/8e986a4cbaae1ad1

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("<svg aA0A=\"\"><pAth d=\"Z0 0L10 10C1 2 3 4 5 6Z\"/></svg>")