fix: added a check for the size of JWT token

[aricart]

Note that now all JWTs are limited to 1MB - which may or not be breaking. Note that Generic JWTs could possibly have whatever size they want but this change affects all decoding.

[synadia-claude-reviewer]

Claude finished @aricart's task in 1m 32s —— View job

---

PR Review: fix: added a check for the size of JWT token

• Gather context
• Read changed files
• Analyze for correctness, security, and performance
• Post review feedback

---

Summary

The change is a straightforward and reasonable security/resource protection measure. The logic is correct and the test covers the basic case. Two issues are worth addressing:

---

Issue 1: DecodeGeneric bypasses the size check (correctness gap)

DecodeGeneric in v2/genericlaims.go has its own implementation that does not call Decode(), so it is not subject to the 1MB limit:

// v2/genericlaims.go:44 — does NOT call Decode(), no size check
func DecodeGeneric(token string) (*GenericClaims, error) {
    chunks := strings.Split(token, ".")
    ...
}

The PR description says "Generic JWTs could possibly have whatever size they want", which suggests this may be intentional. However, it means callers using DecodeGeneric directly (or FormatUserConfig which does call Decode(), so that's fine) get no size protection. If the intent is to limit all NATS JWTs uniformly, the check should also be added to DecodeGeneric. If it's intentionally exempt, it should be documented.

Fix this →

---

Issue 2: No exported sentinel error (API ergonomics)

The error is returned as a plain fmt.Errorf string. Callers who want to handle oversized tokens differently have to resort to string matching (as the test does). An exported sentinel error would be more idiomatic Go:

var ErrTokenTooLarge = errors.New("token too large")

if len(token) > MaxTokenSize {
    return nil, fmt.Errorf("token size %d exceeds maximum of %d bytes: %w", len(token), MaxTokenSize, ErrTokenTooLarge)
}

This is a non-breaking addition. Left an inline comment on the specific line.

---

Minor: Boundary test missing

The test verifies MaxTokenSize+1 is rejected but doesn't verify MaxTokenSize exactly is not rejected for size. Left an inline comment on the test file.

---

What's correct

• len(token) correctly counts bytes (not runes), appropriate for base64-encoded JWT strings
• All typed decode functions (DecodeUserClaims, DecodeAccountClaims, etc.) funnel through Decode(), so they're all covered
• FormatUserConfig calls Decode() directly, also covered
• The 1MB constant is well-chosen and clearly documented

[synadia-claude-reviewer]

Consider using a sentinel error instead of a formatted string. Callers may want to programmatically detect this specific error condition:

Suggested change

	return nil, fmt.Errorf("token size %d exceeds maximum of %d bytes", len(token), MaxTokenSize)
	if len(token) > MaxTokenSize {
	return nil, fmt.Errorf("token size %d exceeds maximum of %d bytes: %w", len(token), MaxTokenSize, ErrTokenTooLarge)
	}

Where ErrTokenTooLarge = errors.New("token too large") is exported. This allows callers to do errors.Is(err, jwt.ErrTokenTooLarge) instead of string matching.

[synadia-claude-reviewer]

Missing boundary test: the test only checks MaxTokenSize+1 (over limit) but doesn't verify that a token of exactly MaxTokenSize bytes is accepted. Consider adding:

// Exactly at the limit should not error on size (will fail on format)
token = strings.Repeat("a", MaxTokenSize)
_, err = Decode(token)
if err != nil && strings.Contains(err.Error(), "exceeds maximum") {
    t.Fatal("token at exactly MaxTokenSize should not be rejected for size")
}

[aricart]

DecodeGeneric has very specific uses that are application defined, so putting a limit is assume that we understand how it is used - this is not in any direct flow to the server etc.

[neilalexander]

LGTM