Skip to content

Harden JWT decode path#59

Merged
mtmk merged 3 commits into
mainfrom
harden-jwt-decode
Apr 15, 2026
Merged

Harden JWT decode path#59
mtmk merged 3 commits into
mainfrom
harden-jwt-decode

Conversation

@mtmk

@mtmk mtmk commented Apr 9, 2026

Copy link
Copy Markdown
Member

Hardens the decode path and FormatUserConfig with input validation that was missing compared to the Go nats-io/jwt library.

  • Validate issuer nkey prefix on decode (matching Go's Decode at decoder.go:104-131)
  • Add MaxTokenSize 1MB cap (matching Go's MaxTokenSize added in nats-io/jwt@1e990c9)
  • Wrap framework exceptions in NatsJwtException so callers get the documented type
  • Validate seed in FormatUserConfig via KeyPair.FromSeed instead of a 2-char prefix check
  • Add Go cross-compat tests for operator, account, user claims in both directions
  • Add test confirming both .NET and Go reject wrong-prefix issuers

Add issuer prefix validation on decode to match the encode-side
check already in DoEncode, aligning with the Go nats-io/jwt
library's Decode function. Add a 1MB MaxTokenSize cap on JWT
input to prevent memory amplification. Wrap raw framework
exceptions (JsonException, FormatException, etc.) in
NatsJwtException so callers get the documented exception type.
Validate seed in FormatUserConfig via KeyPair.FromSeed instead
of a 2-char prefix check to prevent newline injection in creds
files. Add Go cross-compat tests for operator, account, and
user claims in both directions, plus a test confirming both
.NET and Go reject wrong-prefix issuers.
@codecov

codecov Bot commented Apr 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.08197% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.44%. Comparing base (e3c88c1) to head (1a3f7af).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/NATS.Jwt/NatsJwt.cs 94.73% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #59      +/-   ##
==========================================
- Coverage   85.61%   85.44%   -0.17%     
==========================================
  Files          51       51              
  Lines         723      756      +33     
  Branches       90       97       +7     
==========================================
+ Hits          619      646      +27     
- Misses         77       81       +4     
- Partials       27       29       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add tests for null/oversized JWT, malformed base64, invalid JSON,
null/invalid seed, wrong seed prefix, and malformed JWT format in
FormatUserConfig. Covers the new error paths added in the decode
hardening to restore code coverage.
@mtmk
mtmk requested a review from aricart April 9, 2026 10:03

@aricart aricart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why full validations in the end .net should be about generating.

Comment thread src/NATS.Jwt/NatsJwt.cs Outdated
throw new NatsJwtException("JWT is null");
}

if (seed == null)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about bearer tokens?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed. now seed is optional.

Make seed parameter optional so bearer token users can
format a creds file with just the JWT block, without
requiring a signing key.
@mtmk

mtmk commented Apr 9, 2026

Copy link
Copy Markdown
Member Author

@aricart here's the full list of checks added in this PR so you can flag any you think are unnecessary:

FormatUserConfig:

  1. Null JWT check
  2. JWT size limit (1 MB max)
  3. JWT must have 3 dot-separated parts
  4. Claim type must be "user"
  5. Seed parsed via KeyPair.FromSeed instead of string prefix check (validates it's a real seed, not just "SU...")
  6. Seed is now optional (null = bearer token, JWT-only output)

DecodeClaims (all Decode methods):*

  1. Null JWT check
  2. JWT size limit (1 MB max)
  3. Wraps unexpected exceptions in NatsJwtException (with inner exception preserved)
  4. Issuer key prefix validation (e.g. account claims must be issued by an operator key)

NatsJwtException:

  • Added innerException constructor overload

Most of these are basic input sanity, not full Go-style validation. Let me know if any of these are out of scope.

@mtmk
mtmk requested a review from aricart April 9, 2026 20:25

@aricart aricart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mtmk
mtmk merged commit 4f850d1 into main Apr 15, 2026
29 of 30 checks passed
@mtmk
mtmk deleted the harden-jwt-decode branch April 15, 2026 10:39
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.

2 participants