Fix: Replaced jsonwebtoken with jose - a modern, ESM-native JWT library#140
Fix: Replaced jsonwebtoken with jose - a modern, ESM-native JWT library#140
Conversation
…ry with no legacy Buffer dependencies.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe pull request replaces the JWT library dependency from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
This PR replaces jsonwebtoken with jose for JWT signing/verification to avoid a Node.js v22+ runtime incompatibility that was breaking installs/manifest generation, while keeping the auth:issue-jwt-token command behavior consistent.
Changes:
- Switched JWT signing in
auth:issue-jwt-tokenfromjsonwebtokentojose(SignJWT). - Updated unit tests to verify issued tokens using
jose(jwtVerify) instead ofjsonwebtoken. - Replaced dependencies accordingly (
package.json+pnpm-lock.yaml), removing@types/jsonwebtoken.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/commands/auth/issue-jwt-token.ts |
Uses jose to sign HS256 JWTs (sets kid, iat, exp, jti) and preserves existing output behavior. |
test/unit/commands/auth/issue-jwt-token.test.ts |
Verifies generated JWTs with jose and updates type assertions for decoded claims. |
package.json |
Swaps jsonwebtoken for jose and removes @types/jsonwebtoken. |
pnpm-lock.yaml |
Lockfile updates reflecting dependency replacement/removal. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/auth/issue-jwt-token.ts`:
- Around line 98-101: The JWT payload creation in the SignJWT call incorrectly
includes capabilities as an object; change the payload construction in
issue-jwt-token (the SignJWT instantiation that sets "x-ably-capability") to
include JSON.stringify(capabilities) instead of the raw object, and update any
tests that inspect decoded["x-ably-capability"] to JSON.parse that claim before
accessing properties (tests referencing decoded["x-ably-capability"] at the two
failing assertions should be modified to parse the string claim first).
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
package.jsonsrc/commands/auth/issue-jwt-token.tstest/unit/commands/auth/issue-jwt-token.test.ts
| const token = await new SignJWT({ | ||
| "x-ably-appId": appId, | ||
| "x-ably-capability": capabilities, | ||
| ...clientIdClaim, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Ably JWT authentication x-ably-capability claim format specification
💡 Result:
For an Ably JWT (JWT signed with your Ably API key secret), the x-ably-capability claim must be a string whose contents are JSON representing an Ably capability map (resource → list of operations). (ably.com)
Claim format (payload)
{
"x-ably-capability": "{\"chat:*\": [\"publish\",\"subscribe\"], \"status\": [\"subscribe\",\"history\"]}"
}x-ably-capability is not a JSON object in the JWT payload; it’s a JSON-encoded string. (ably.com)
Capability map JSON (inside the string)
Shape
{
"<resource>": ["<operation>", "..."],
"<resource2>": ["*"]
}Resources
- Channel or channel pattern (supports
:-segment wildcards likenamespace:*, and*for all channels). (ably.com) - Queue resource: starts with
"[queue]"(e.g."[queue]myapp-queue"), wildcard rules apply (e.g."[queue]*"). (ably.com) - Metachannel resource: starts with
"[meta]"(e.g."[meta]metaname"), wildcard rules apply (e.g."[meta]*"). (ably.com) - Special resource
[*]*matches all channels, all queues, and all metachannels. (ably.com)
Operations
Use any of Ably’s capability operations, e.g. subscribe, publish, presence, history, stats, push-subscribe, push-admin, channel-metadata, etc. (or "*" for all operations). (ably.com)
Validation note
An empty string for x-ably-capability can cause JWT decode/validation failure. (faqs.ably.com)
Citations:
- 1: https://ably.com/docs/auth/token?utm_source=openai
- 2: https://ably.com/docs/auth/token?utm_source=openai
- 3: https://ably.com/docs/auth/capabilities
- 4: https://ably.com/docs/auth/capabilities
- 5: https://ably.com/docs/auth/capabilities
- 6: https://ably.com/docs/auth/capabilities
- 7: https://ably.com/docs/auth/capabilities
- 8: https://faqs.ably.com/error-code-40144-unexpected-error-decoding-jwt-decode-exception?utm_source=openai
🏁 Script executed:
# Check the current implementation in the file
cat -n src/commands/auth/issue-jwt-token.ts | sed -n '90,110p'Repository: ably/ably-cli
Length of output: 956
🏁 Script executed:
# Check the test file to see how x-ably-capability is being asserted
cat -n test/unit/commands/auth/issue-jwt-token.test.ts | sed -n '60,80p'Repository: ably/ably-cli
Length of output: 847
🏁 Script executed:
# Also check lines around 167 mentioned in the review
cat -n test/unit/commands/auth/issue-jwt-token.test.ts | sed -n '160,175p'Repository: ably/ably-cli
Length of output: 717
Encode x-ably-capability as JSON string, not object.
Line 100 passes capabilities as an object. According to Ably's JWT specification, x-ably-capability must be a JSON-encoded string within the JWT payload. Passing it as an object will cause tokens to fail Ably JWT validation at runtime.
const token = await new SignJWT({
"x-ably-appId": appId,
- "x-ably-capability": capabilities,
+ "x-ably-capability": JSON.stringify(capabilities),
...clientIdClaim,
})Tests currently expect decoded["x-ably-capability"] as an object (lines 71 and 167), but this masks the bug. After fixing the code, tests must parse the claim string before accessing properties:
- expect(decoded["x-ably-capability"]).toHaveProperty("chat:*");
+ const capabilityMap = JSON.parse(decoded["x-ably-capability"] as string);
+ expect(capabilityMap).toHaveProperty("chat:*");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const token = await new SignJWT({ | |
| "x-ably-appId": appId, | |
| "x-ably-capability": capabilities, | |
| ...clientIdClaim, | |
| const token = await new SignJWT({ | |
| "x-ably-appId": appId, | |
| "x-ably-capability": JSON.stringify(capabilities), | |
| ...clientIdClaim, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/auth/issue-jwt-token.ts` around lines 98 - 101, The JWT payload
creation in the SignJWT call incorrectly includes capabilities as an object;
change the payload construction in issue-jwt-token (the SignJWT instantiation
that sets "x-ably-capability") to include JSON.stringify(capabilities) instead
of the raw object, and update any tests that inspect
decoded["x-ably-capability"] to JSON.parse that claim before accessing
properties (tests referencing decoded["x-ably-capability"] at the two failing
assertions should be modified to parse the string claim first).
Comparison Table
Summary by CodeRabbit