ci(lint): add golangci-lint across all modules (stacked on #20)

.github/workflows/ci.yaml

@@ -238,3 +238,25 @@ jobs:
           govulncheck ./...
           (cd tsnetid && govulncheck ./...)
           (cd gateway && govulncheck ./...)
+
+  # Static analysis across every module. The standard linter set (errcheck,
+  # govet, ineffassign, staticcheck, unused); configuration is in
+  # .golangci.yml at the repo root, which each module picks up by upward search.
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with: {go-version: '1.26'}
+      - name: Install golangci-lint
+        run: |
+          set -euo pipefail
+          curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh \
+            | sh -s -- -b "$(go env GOPATH)/bin" v2.11.4
+      - name: Lint every module
+        run: |
+          set -euo pipefail
+          export PATH="$(go env GOPATH)/bin:$PATH"
+          golangci-lint run ./...
+          (cd tsnetid && golangci-lint run ./...)
+          (cd gateway && golangci-lint run ./...)

.golangci.yml

@@ -0,0 +1,39 @@
+# golangci-lint configuration.
+#
+# The standard set (errcheck, govet, ineffassign, staticcheck, unused) is the
+# high-signal core. The tuning below keeps it green on real defects without
+# nagging on patterns this codebase deliberately accepts.
+version: "2"
+
+linters:
+  default: standard
+  settings:
+    staticcheck:
+      # The SA* bug checks plus golangci-lint's default set. The ST* doc/naming
+      # checks below are golangci-lint's own defaults (off), kept explicit here
+      # because setting `checks` replaces the default rather than extending it.
+      checks:
+        - all
+        - -ST1000
+        - -ST1003
+        - -ST1016
+        - -ST1020
+        - -ST1021
+        - -ST1022
+        # QF* are refactor "quick fixes" (tagged switch, drop embedded field),
+        # not defects. Leave them off; they are style, not correctness.
+        - -QF1002
+        - -QF1008
+  exclusions:
+    # A failed write to an HTTP response, a deferred Close, or an Fprintln to a
+    # ResponseWriter cannot be meaningfully handled; the standard preset covers
+    # exactly those and keeps errcheck focused on returns that matter.
+    presets:
+      - std-error-handling
+    rules:
+      # Unchecked error returns in test setup (e.g. Rotate, json.Unmarshal) are
+      # low value: a failure there fails the test anyway. Keep errcheck strict
+      # for non-test code.
+      - path: _test\.go
+        linters:
+          - errcheck

keys/keys.go

@@ -16,7 +16,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"math/big"
 	"sync"
 	"time"
 )
@@ -237,9 +236,36 @@ func PublicFromXY(x, y string) (*ecdsa.PublicKey, error) {
 	if err != nil {
 		return nil, fmt.Errorf("keys: y is not base64url")
 	}
-	pub := &ecdsa.PublicKey{Curve: elliptic.P256(), X: new(big.Int).SetBytes(xb), Y: new(big.Int).SetBytes(yb)}
-	if !pub.Curve.IsOnCurve(pub.X, pub.Y) {
-		return nil, fmt.Errorf("keys: point is not on P-256")
+	xb, ok := coordBytes(xb)
+	if !ok {
+		return nil, fmt.Errorf("keys: x is not a P-256 coordinate")
+	}
+	yb, ok = coordBytes(yb)
+	if !ok {
+		return nil, fmt.Errorf("keys: y is not a P-256 coordinate")
+	}
+	// Build the SEC 1 uncompressed encoding (0x04 || x || y) and let the stdlib
+	// parse and validate it in one step. ParseUncompressedPublicKey rejects a
+	// point that is not on the curve or is the point at infinity — replacing a
+	// manual (and since Go 1.21 deprecated) Curve.IsOnCurve check.
+	uncompressed := make([]byte, 0, 1+len(xb)+len(yb))
+	uncompressed = append(append(append(uncompressed, 4), xb...), yb...)
+	pub, err := ecdsa.ParseUncompressedPublicKey(elliptic.P256(), uncompressed)
+	if err != nil {
+		return nil, fmt.Errorf("keys: invalid P-256 public key: %w", err)
 	}
 	return pub, nil
 }
+
+// coordBytes left-pads an affine coordinate to the 32 bytes P-256 requires,
+// rejecting an over-long input. A compliant JWK carries exactly 32 bytes
+// (RFC 7518 §6.2.1.2); left-padding also tolerates a producer that stripped
+// leading zeros, while a longer value is not a valid coordinate.
+func coordBytes(b []byte) ([]byte, bool) {
+	if len(b) > 32 {
+		return nil, false
+	}
+	out := make([]byte, 32)
+	copy(out[32-len(b):], b)
+	return out, true
+}

keys/keys_test.go

@@ -56,11 +56,8 @@ func TestRotateOldKeyVerifiesDuringOverlapWindow(t *testing.T) {
 
 	// Manually verify the key is in the set
 	pub1, ok := set.PublicKey(kid1)
-	if !ok {
-		t.Error("Current key not found in set after NewSet")
-	}
-	if pub1 == nil {
-		t.Error("PublicKey returned nil for current kid")
+	if !ok || pub1 == nil {
+		t.Fatal("PublicKey did not return the current key after NewSet")
 	}
 
 	// Verify it matches the private key
@@ -139,7 +136,7 @@ func TestRotateOldKeyDoesNotVerifyAfterOverlap(t *testing.T) {
 	laterSet.current = set.current
 	laterSet.retired = set.retired
 
-	pub, exists = laterSet.PublicKey(kid1)
+	_, exists = laterSet.PublicKey(kid1)
 	if exists {
 		t.Error("old key should not exist after overlap window expired")
 	}

keys/parsexy_test.go

@@ -0,0 +1,83 @@
+package keys
+
+import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"encoding/base64"
+	"math/big"
+	"strings"
+	"testing"
+)
+
+func b64(b []byte) string { return base64.RawURLEncoding.EncodeToString(b) }
+
+// pad32 mirrors the on-wire coordinate encoding (left-padded to the P-256 size).
+func pad32(n *big.Int) []byte {
+	b := make([]byte, 32)
+	n.FillBytes(b)
+	return b
+}
+
+func TestPublicFromXY_roundTripsAValidKey(t *testing.T) {
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got, err := PublicFromXY(b64(pad32(priv.X)), b64(pad32(priv.Y)))
+	if err != nil {
+		t.Fatalf("a valid P-256 point must parse: %v", err)
+	}
+	if got.X.Cmp(priv.X) != 0 || got.Y.Cmp(priv.Y) != 0 {
+		t.Fatal("round-tripped key does not match the original coordinates")
+	}
+}
+
+func TestPublicFromXY_toleratesMinimalCoordinateEncoding(t *testing.T) {
+	// big.Int.Bytes() returns the minimal big-endian form (leading zeros
+	// stripped), which a lenient producer may emit. Left-padding must accept it
+	// whatever its length.
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := PublicFromXY(b64(priv.X.Bytes()), b64(priv.Y.Bytes())); err != nil {
+		t.Fatalf("the minimal (stripped) coordinate encoding of a valid point must parse: %v", err)
+	}
+}
+
+func TestPublicFromXY_rejectsOffCurvePoint(t *testing.T) {
+	// A valid X with Y := Y+1 is (almost surely) not on the curve.
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	badY := new(big.Int).Add(priv.Y, big.NewInt(1))
+	if _, err := PublicFromXY(b64(pad32(priv.X)), b64(pad32(badY))); err == nil {
+		t.Fatal("an off-curve point must be rejected")
+	}
+}
+
+func TestPublicFromXY_rejectsPointAtInfinity(t *testing.T) {
+	zero := b64(make([]byte, 32))
+	if _, err := PublicFromXY(zero, zero); err == nil {
+		t.Fatal("the point at infinity (0,0) must be rejected")
+	}
+}
+
+func TestPublicFromXY_rejectsOverlongCoordinate(t *testing.T) {
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	overlong := b64(append([]byte{0}, pad32(priv.X)...)) // 33 bytes
+	if _, err := PublicFromXY(overlong, b64(pad32(priv.Y))); err == nil {
+		t.Fatal("an over-long (>32 byte) coordinate must be rejected")
+	}
+}
+
+func TestPublicFromXY_rejectsNonBase64(t *testing.T) {
+	if _, err := PublicFromXY("!!!", "!!!"); err == nil || !strings.Contains(err.Error(), "base64url") {
+		t.Fatalf("non-base64url coordinate must be rejected as such, got: %v", err)
+	}
+}

verifier/remote.go

@@ -3,11 +3,8 @@ package verifier
 import (
 	"context"
 	"crypto/ecdsa"
-	"crypto/elliptic"
-	"encoding/base64"
 	"encoding/json"
 	"fmt"
-	"math/big"
 	"net/http"
 	"sync"
 	"time"
@@ -201,23 +198,12 @@ func jwkToPublic(j keys.JWK) (*ecdsa.PublicKey, error) {
 	if j.Use != "" && j.Use != "sig" {
 		return nil, fmt.Errorf("key is not for signatures")
 	}
-	x, err := base64.RawURLEncoding.DecodeString(j.X)
+	// Coordinate decode and on-curve validation live in keys.PublicFromXY,
+	// which uses the stdlib's ParseUncompressedPublicKey to reject an off-curve
+	// point or the point at infinity. One source of truth, no deprecated API.
+	pub, err := keys.PublicFromXY(j.X, j.Y)
 	if err != nil {
-		return nil, fmt.Errorf("x is not base64url")
-	}
-	y, err := base64.RawURLEncoding.DecodeString(j.Y)
-	if err != nil {
-		return nil, fmt.Errorf("y is not base64url")
-	}
-	pub := &ecdsa.PublicKey{
-		Curve: elliptic.P256(),
-		X:     new(big.Int).SetBytes(x),
-		Y:     new(big.Int).SetBytes(y),
-	}
-	// Reject a point that is not on the curve. Without this check a
-	// malformed or hostile key set could supply an invalid point.
-	if !pub.Curve.IsOnCurve(pub.X, pub.Y) {
-		return nil, fmt.Errorf("point is not on P-256")
+		return nil, err
 	}
 	// The kid is defined as the RFC 7638 thumbprint of the key, so verify the
 	// declared kid actually derives from this key. Otherwise a publisher could