feat: Add OpenAPI security tests and improve JWT handling in JwksService #190
Conversation
📝 WalkthroughWalkthroughAdds OpenAPI security E2E and integration tests, a SecurityTest MCP server entrypoint, JWKS weak-RSA-key detection with a Node crypto fallback and one-time warnings, Node RSA/JWT utilities, a dynamic MockAPIServer with typed handlers, plus path, config, and dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP as MCP Server (OpenAPI Adapter)
participant JWKS as JWKS Service
participant MockAPI as Mock API Server
Client->>MCP: invoke tool / request (may include Authorization)
MCP->>JWKS: verifyTransparentToken(token)
alt jose verifies successfully
JWKS-->>MCP: verification OK
else jose fails with weak-key error
JWKS->>JWKS: fetch JWKS, find matching RSA key
JWKS->>JWKS: Node crypto fallback verify
JWKS-->>MCP: verification OK or fail (emit one-time warning if weak)
end
alt token verified
MCP->>MockAPI: forward request with headers (Authorization, Accept)
MockAPI-->>MCP: response (captures headers)
MCP-->>Client: tool response
else verification failed
MCP-->>Client: error response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @libs/sdk/src/auth/jwks/jwks.service.ts:
- Around line 8-15: The module-level warnedProviders creates shared state across
all JwksService instances; move the tracking into the JwksService class by
removing the module-scope warnedProviders and adding an instance property (e.g.,
private warnedProviders = new Set<string>();) to JwksService, then update all
uses of warnedProviders inside the class (the checks that read/add provider IDs)
to reference this.warnedProviders so each service instance has its own warning
set.
In @libs/testing/src/auth/mock-api-server.ts:
- Around line 358-375: The parseBody method for IncomingMessage currently only
listens for 'data' and 'end' and can hang on stream errors; update parseBody to
also listen for the request 'error' event and reject the returned Promise with
that error (or resolve with a safe value if you prefer), and ensure you
remove/cleanup listeners on both 'end' and 'error' to avoid leaks; modify the
private async parseBody(req: IncomingMessage): Promise<unknown> implementation
to add req.on('error', ...) and appropriate cleanup logic so the Promise always
settles.
In @libs/utils/src/crypto/node.ts:
- Around line 175-185: The jwtAlgToNodeAlg function currently returns a silent
default for unknown algorithms; update jwtAlgToNodeAlg to explicitly handle
unsupported jwtAlg values by checking the mapping lookup and throwing a clear,
descriptive Error (e.g., "Unsupported JWT algorithm: <jwtAlg>") instead of
returning 'RSA-SHA256'; keep the existing mapping object (mapping) and ensure
callers receive/handle the thrown error so misconfiguration is surfaced.
🧹 Nitpick comments (7)
libs/utils/src/crypto/index.ts (1)
22-29: Remove redundant error guard.The error check at lines 26-28 is unreachable because
_provideris always assigned at line 23. This guard can never throw.♻️ Simplify by removing the unreachable error guard
export function getCrypto(): CryptoProvider { if (!_provider) { _provider = browserCrypto; } - // Provider is always initialized in the if block above - if (!_provider) { - throw new Error('Failed to initialize crypto provider'); - } return _provider; }libs/utils/src/crypto/node.ts (1)
138-158: Document the rationale for 2048-bit default RSA key size.The default 2048-bit RSA key is intentional for OAuth token verification (short-lived tokens), but consider documenting this choice. For longer-validity use cases, the
modulusLengthparameter allows callers to specify 3072+ bits. The code already properly validates and warns about weak keys (< 2048 bits) in dependent services, so this is not a security defect.The RSA utilities are intentionally isolated in
node.tsand not re-exported fromindex.ts(which avoids importingnode:cryptofor browser compatibility), so the import pattern is already clear by design.libs/testing/src/auth/mock-api-server.ts (1)
70-79: Consider validating that routes have eitherresponseorhandler.Both
responseandhandlerare optional, which means a route could be defined with neither. This would cause the route to match but return nothing (falling through to 404 behavior silently).Consider adding runtime validation in
addRoute()or documenting the expected behavior.♻️ Optional validation in addRoute
addRoute(route: MockRoute): void { + if (!route.response && !route.handler) { + throw new Error(`Route ${route.method} ${route.path} must have either 'response' or 'handler'`); + } this.routes.push(route); }apps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.ts (1)
12-13: Module-level mutable state may cause issues in parallel test execution.
lastReceivedHeadersis module-level mutable state that could cause flaky tests if Jest runs tests in parallel or if multiple test files share this module. Consider encapsulating this state within the test suite or using a more isolated approach.♻️ Alternative: Capture headers per-test via mock response
Instead of module-level state, you could return the captured headers in the mock response and verify them directly from the tool result:
-// Track received headers for verification -let lastReceivedHeaders: Record<string, string | undefined> = {}; +// Each test can verify headers via the responseThen modify the mock handler to always include received headers in the response for verification.
libs/sdk/src/index.ts (1)
3-16: Incomplete issue reference and global side effect placement.Two observations:
The GitHub issue link on line 5 is incomplete (
https://github.com/colinhacks/zod/issuesvs a specific issue number). Consider linking to the specific issue for better traceability.Placing a global
process.emitWarningoverride in a barrel export file means it executes on first import of@frontmcp/sdk. While functional, this is a non-obvious side effect. Consider documenting this behavior in the SDK's README or moving it to an explicit initialization function.libs/sdk/src/auth/jwks/jwks.service.ts (1)
2-2: Directnode:cryptousage instead of@frontmcp/utils.Per coding guidelines,
@frontmcp/utilsshould be preferred for cryptographic operations. However, looking atlibs/utils/src/crypto/node.ts, it exposesrsaSignbut not a correspondingrsaVerifyfunction that would support thecrypto.verify()call on line 196.If this direct usage is intentional, consider adding a
rsaVerifyutility to@frontmcp/utilsfor consistency, or document why directnode:cryptousage is necessary here.libs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.ts (1)
258-271: Consider adding PSS algorithm test case.The algorithm mapping tests cover RS* and PS* mappings, but there's no integration test verifying that PSS-signed tokens actually verify correctly. Given the RSA-PSS padding issue noted in
jwks.service.ts, adding a test with a PS256-signed token would help catch this.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
.github/UPDATES_FORMAT.mdapps/demo/src/main.tsapps/demo/webpack.config.jsapps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.tsapps/e2e/demo-e2e-openapi/src/security-test-main.tslibs/adapters/src/openapi/__tests__/openapi-security-integration.spec.tslibs/adapters/src/openapi/__tests__/openapi-security-unit.spec.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/sdk/src/index.tslibs/testing/src/auth/mock-api-server.tslibs/testing/src/index.tslibs/uipack/package.jsonlibs/utils/package.jsonlibs/utils/src/crypto/index.tslibs/utils/src/crypto/node.tspackage.jsonplugins/plugin-codecall/package.jsontsconfig.base.json
💤 Files with no reviewable changes (1)
- .github/UPDATES_FORMAT.md
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification
Avoid non-null assertions (!) - use proper error handling and type guards instead
Use@frontmcp/utilsfor cryptographic operations instead ofnode:cryptodirectly
Use@frontmcp/utilsfor file system operations instead offs/promisesornode:fsdirectly
Files:
libs/sdk/src/index.tslibs/adapters/src/openapi/__tests__/openapi-security-unit.spec.tslibs/adapters/src/openapi/__tests__/openapi-security-integration.spec.tsapps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.tslibs/utils/src/crypto/node.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.tsapps/e2e/demo-e2e-openapi/src/security-test-main.tslibs/testing/src/index.tslibs/utils/src/crypto/index.tslibs/testing/src/auth/mock-api-server.tsapps/demo/src/main.ts
libs/{sdk,adapters}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters}/**/*.{ts,tsx}: Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities
Files:
libs/sdk/src/index.tslibs/adapters/src/openapi/__tests__/openapi-security-unit.spec.tslibs/adapters/src/openapi/__tests__/openapi-security-integration.spec.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.ts
libs/sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/**/*.{ts,tsx}: Preferinterfacefor defining object shapes in TypeScript, avoidanytypes
Use type parameters with constraints instead of unconstrained generics withanydefaults
Validate URIs per RFC 3986 at metadata level usingisValidMcpUrirefinement
UsechangeScopeinstead ofscopein change event properties to avoid confusion with Scope class
Fail fast on invalid hook flows by validating hooks match their entry type
Centralize record types in common/records and import from there, not from module-specific files
Return strictly typed MCP protocol responses (e.g.,Promise<GetPromptResult>) instead ofPromise<unknown>for MCP entry methods
Useunknowninstead ofanyfor generic type parameter defaults (not for MCP protocol types)
Create shared base classes for common functionality like ExecutionContextBase for ToolContext and ResourceContext
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Validate inputs and outputs through parseOutput/safeParseOutput methods in validation flows
Files:
libs/sdk/src/index.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.ts
libs/*/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Export everything users need through barrel exports (index.ts), do not add backwards compatibility aliases
Files:
libs/sdk/src/index.tslibs/testing/src/index.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/index.tslibs/adapters/src/openapi/__tests__/openapi-security-unit.spec.tslibs/adapters/src/openapi/__tests__/openapi-security-integration.spec.tslibs/uipack/package.jsonlibs/utils/package.jsonlibs/utils/src/crypto/node.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/testing/src/index.tslibs/utils/src/crypto/index.tslibs/testing/src/auth/mock-api-server.ts
libs/uipack/**/{package.json,*.ts,*.tsx,*.js,*.jsx}
📄 CodeRabbit inference engine (libs/uipack/CLAUDE.md)
Do not add React dependencies to @frontmcp/uipack - it must remain React-free. Use @frontmcp/ui for React components.
Files:
libs/uipack/package.json
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Use Jest for testing with 95%+ coverage requirement and 100% test pass rate
Do not use prefixes like 'PT-001' in test names
Do not skip constructor validation tests for error classes and types
Includeinstanceofchecks in tests for error classes to verify proper error hierarchy
Files:
apps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.ts
apps/demo/**
⚙️ CodeRabbit configuration file
apps/demo/**: apps/demo directory contains a demo application for testing purposes. It can be used as a reference for SDK usage examples.
Files:
apps/demo/webpack.config.jsapps/demo/src/main.ts
🧠 Learnings (23)
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/index.{ts,js} : Export all public APIs through appropriate entry points (frontmcp/uipack, frontmcp/uipack/adapters, frontmcp/uipack/theme, etc.)
Applied to files:
tsconfig.base.jsonlibs/uipack/package.jsonlibs/testing/src/index.tsapps/demo/webpack.config.jsapps/demo/src/main.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use `frontmcp/utils` for file system operations instead of `fs/promises` or `node:fs` directly
Applied to files:
tsconfig.base.jsonlibs/sdk/src/index.tslibs/uipack/package.jsonapps/demo/src/main.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Use proper ES module imports instead of `require()` for SDK imports; avoid dynamic require of `frontmcp/sdk` modules
Applied to files:
tsconfig.base.jsonlibs/sdk/src/index.tslibs/uipack/package.jsonlibs/utils/src/crypto/index.tsapps/demo/src/main.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use `frontmcp/utils` for cryptographic operations instead of `node:crypto` directly
Applied to files:
tsconfig.base.jsonlibs/sdk/src/index.tslibs/uipack/package.jsonlibs/utils/package.jsonlibs/utils/src/crypto/node.tsapps/e2e/demo-e2e-openapi/src/security-test-main.tslibs/utils/src/crypto/index.tsapps/demo/src/main.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*plugin.ts : Use module augmentation for context properties via `declare module 'frontmcp/sdk'` combined with runtime plugin metadata `contextExtensions`, not module-level side effects
Applied to files:
tsconfig.base.jsonapps/demo/src/main.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{package.json,*.ts,*.tsx,*.js,*.jsx} : Do not add React dependencies to frontmcp/uipack - it must remain React-free. Use frontmcp/ui for React components.
Applied to files:
tsconfig.base.jsonlibs/uipack/package.jsonpackage.jsonapps/demo/webpack.config.js
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to libs/*/src/index.ts : Export everything users need through barrel exports (index.ts), do not add backwards compatibility aliases
Applied to files:
tsconfig.base.jsonapps/demo/webpack.config.js
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Organize code following the frontmcp/uipack directory structure (adapters/, bundler/, theme/, renderers/, validation/, etc.)
Applied to files:
tsconfig.base.jsonapps/demo/src/main.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/bundler/**/*.{ts,tsx} : The bundler module must re-export utilities from frontmcp/uipack/bundler and provide SSR component bundling functionality
Applied to files:
tsconfig.base.jsonlibs/uipack/package.jsonapps/demo/webpack.config.js
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/**/*.{ts,tsx} : Never import React-free utilities from frontmcp/ui; use frontmcp/uipack for bundling, build tools, platform adapters, and theme utilities
Applied to files:
tsconfig.base.jsonlibs/uipack/package.jsonapps/demo/src/main.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/package.json : Entry points must match the documented paths: frontmcp/ui/react, frontmcp/ui/renderers, frontmcp/ui/render, frontmcp/ui/universal, frontmcp/ui/bundler, frontmcp/ui/bridge, frontmcp/ui/components, frontmcp/ui/layouts, frontmcp/ui/web-components
Applied to files:
tsconfig.base.jsonlibs/uipack/package.jsonapps/demo/src/main.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Always use `frontmcp/utils` for cryptographic operations (hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode) instead of `node:crypto`
Applied to files:
tsconfig.base.jsonlibs/uipack/package.jsonlibs/utils/package.jsonlibs/utils/src/crypto/node.tslibs/utils/src/crypto/index.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*plugin.ts : Extend ExecutionContextBase with plugin-specific properties using module declaration (`declare module 'frontmcp/sdk'`) combined with `contextExtensions` in plugin metadata
Applied to files:
libs/sdk/src/index.tsapps/demo/src/main.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{theme,adapters,bundler}/**/*.{test,spec}.{ts,tsx,js,jsx} : Test behavior across all supported platform configurations (OpenAI, Claude, etc.)
Applied to files:
libs/adapters/src/openapi/__tests__/openapi-security-integration.spec.tsapps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx,js,jsx} : Do not expose internal error details in public APIs - use sanitized error messages
Applied to files:
libs/uipack/package.json
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/package.json : The frontmcp/ui package requires React as a peer dependency (^18.0.0 || ^19.0.0)
Applied to files:
libs/uipack/package.json
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Avoid using `node:crypto` directly; always use `frontmcp/utils` for cross-platform cryptographic support
Applied to files:
libs/uipack/package.jsonlibs/utils/package.jsonlibs/utils/src/crypto/node.tslibs/utils/src/crypto/index.ts
📚 Learning: 2025-11-05T15:00:47.800Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 11
File: libs/core/src/auth/jwks/jwks.service.ts:62-0
Timestamp: 2025-11-05T15:00:47.800Z
Learning: In the FrontMCP codebase (libs/core/src/auth/jwks/jwks.service.ts), the verifyGatewayToken method intentionally skips signature verification when running in development-only no-auth mode, using decodeJwtPayloadSafe instead of jwtVerify. This is a deliberate design for local development convenience and should not be flagged as a security issue when the PR or code context indicates development/no-auth mode.
Applied to files:
libs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{theme,build,bundler}/**/*.{ts,tsx,js,jsx} : Do not hard-code CDN URLs - use theme.cdn configuration instead
Applied to files:
apps/demo/webpack.config.js
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{renderers,build}/**/*.{ts,tsx} : Use mdxClientRenderer for CDN-based MDX rendering without bundled React
Applied to files:
apps/demo/webpack.config.js
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Extend tool metadata using `declare global` pattern to allow tools to specify plugin-specific options in their decorators
Applied to files:
apps/demo/webpack.config.js
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/universal/**/*.{ts,tsx} : Universal app shell (UniversalApp, FrontMCPProvider) must provide platform-agnostic React context and initialization
Applied to files:
apps/demo/src/main.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{build,bundler}/**/*.{ts,tsx,js,jsx} : For server-side MDX rendering with bundled React, use frontmcp/ui/renderers instead of frontmcp/uipack/renderers
Applied to files:
apps/demo/src/main.ts
🧬 Code graph analysis (8)
libs/adapters/src/openapi/__tests__/openapi-security-integration.spec.ts (3)
libs/sdk/src/index.ts (1)
FrontMcpContext(38-38)libs/adapters/src/openapi/openapi.security.ts (1)
resolveToolSecurity(283-326)libs/adapters/src/openapi/openapi.utils.ts (1)
buildRequest(68-177)
apps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.ts (2)
libs/testing/src/auth/mock-api-server.ts (3)
MockAPIServer(120-393)MockRequest(40-53)MockResponseHelper(58-63)libs/testing/src/index.ts (5)
MockAPIServer(84-84)TestServer(99-99)McpTestClient(46-46)MockRequest(90-90)MockResponseHelper(91-91)
libs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.ts (2)
libs/sdk/src/auth/jwks/jwks.service.ts (3)
isWeakKeyError(158-161)findMatchingKey(237-252)getNodeAlgorithm(257-267)libs/utils/src/crypto/node.ts (2)
generateRsaKeyPair(138-158)rsaSign(168-170)
libs/sdk/src/auth/jwks/jwks.service.ts (1)
libs/sdk/src/auth/jwks/jwks.types.ts (2)
ProviderVerifyRef(21-26)VerifyResult(28-36)
apps/e2e/demo-e2e-openapi/src/security-test-main.ts (1)
apps/demo/src/main.ts (1)
FrontMcp(18-33)
libs/utils/src/crypto/index.ts (1)
libs/utils/src/crypto/browser.ts (1)
browserCrypto(72-124)
libs/testing/src/auth/mock-api-server.ts (2)
libs/testing/src/index.ts (5)
MockRequest(90-90)MockResponseHelper(91-91)MockRouteHandler(92-92)MockRoute(88-88)MockResponse(89-89)libs/testing/src/interceptor/mock-registry.ts (1)
match(55-86)
apps/demo/src/main.ts (5)
apps/demo/src/apps/weather/index.ts (1)
App(13-21)apps/demo/src/apps/expenses/index.ts (1)
App(17-42)apps/demo/src/apps/employee-time/index.ts (1)
App(14-34)apps/demo/src/apps/calculator/index.ts (1)
App(19-50)libs/sdk/src/common/decorators/front-mcp.decorator.ts (1)
FrontMcp(43-104)
🪛 Gitleaks (8.30.0)
libs/adapters/src/openapi/__tests__/openapi-security-integration.spec.ts
[high] 177-177: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 134-134: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (28)
libs/utils/src/crypto/index.ts (2)
175-209: Buffer usage is appropriate for Node.js performance optimization.The encoding utilities correctly check for
Bufferavailability to use efficient Node.js APIs when available, while falling back to browser APIs (btoa/atob). This is consistent with the browser-first approach while maintaining Node.js efficiency.
5-9: Acknowledge the intentional architectural design favoring browser compatibility.The module intentionally uses
browserCrypto(backed by @noble libraries) unconditionally across all environments. This is an explicit design choice documented in the code comments (lines 18-19): the module avoids importing Node.js-only crypto modules to remain browser-compatible and prevent pulling innode:cryptoin browser builds.While this prioritizes universal compatibility over Node.js native crypto performance, the decision is reflected in the consistent use of @noble libraries across platforms. The unused
nodeCryptoimplementation suggests this approach was intentional. If Node.js-specific performance is critical for this codebase, consider exposing runtime-aware selection similar to thebase64urlEncodepattern (lines 175-182), which already optimizes encoding for Node.js when available.libs/utils/src/crypto/node.ts (2)
7-7: Good compatibility improvement.Changing from
'node:crypto'to'crypto'improves compatibility with older Node.js versions and certain bundlers while maintaining identical functionality.
196-212: JWT creation logic is correct.The implementation properly constructs a JWT with base64url encoding, includes required header fields (
alg,typ,kid), and signs using the appropriate Node.js algorithm.apps/demo/webpack.config.js (1)
19-19: LGTM! Build configuration optimization.The addition of
mergeExternals: trueand switchingbuildLibsFromSourcetofalseare appropriate optimizations for the demo app, improving build performance by using pre-built library artifacts instead of building from source.Also applies to: 25-25
libs/adapters/src/openapi/__tests__/openapi-security-unit.spec.ts (1)
1-7: Excellent documentation improvement!The added comment block clearly explains the test scope (unit tests with mocked SecurityResolver) and references the integration tests for end-to-end flows. This helps developers understand test boundaries and where to find related tests.
tsconfig.base.json (1)
28-28: OpenAPI adapter entry point is properly configured.The file
libs/adapters/src/openapi/index.tsexists and correctly exports the public API: theOpenapiAdapterclass as the default export and all type definitions fromopenapi.types.ts. This matches the documented public interface shown in the README.package.json (1)
36-36: Dependencies verified as valid with no known security advisories.Both
enclave-vm@2.5.0andvectoriadb@2.0.2exist on npm and have no known public security vulnerabilities or CVEs. The specified versions are available and the caret version constraints follow standard semver practices.libs/utils/package.json (1)
29-29: Verify the package name and source for "ast-guard".The
ast-guardpackage does not appear to exist in the public npm registry. Web searches of npm registry, Snyk, and NVD found no matching package by that name. Confirm whether this is a private/internal package, a typo (possibly "es-guard" or "ast-types"?), or if it should be sourced from a private registry configured in.npmrc.Likely an incorrect or invalid review comment.
plugins/plugin-codecall/package.json (1)
49-50: LGTM!Dependency version bumps are consistent with other package updates in this PR. The enclave-vm and vectoriadb updates align with the root package.json changes.
apps/e2e/demo-e2e-openapi/src/security-test-main.ts (1)
1-49: LGTM!The security test entry point is well-structured:
- Environment-based configuration allows flexible test setup
- Conditional
staticAuthapplication is clean- Server configuration with verbose logging aids debugging E2E tests
- Proper use of
@frontmcp/sdkand@frontmcp/adaptersimportslibs/adapters/src/openapi/__tests__/openapi-security-integration.spec.ts (4)
128-141: Test tokens are appropriate for integration tests.The static analysis flagged the JWT on line 134 and API key on line 177. These are intentional test fixtures:
- Line 134: Well-known example JWT from jwt.io documentation (safe for testing)
- Line 177: Base64-encoded "user:password" test credential
These are appropriate for security integration tests and pose no security risk.
16-25: LGTM!The
createMockContexthelper with explicit type assertion is appropriate for test mocking. The partial context object provides the minimal required properties for security testing.
102-142: Comprehensive staticAuth JWT tests.Good coverage of:
- Basic JWT header generation
- Integration with
buildRequest- Handling tokens with special characters (dots, underscores)
327-355: LGTM - Custom securityResolver tests.Tests correctly verify:
- Custom resolver is invoked with proper arguments
- Custom resolver takes precedence over staticAuth
This ensures the priority chain works as documented.
libs/testing/src/auth/mock-api-server.ts (2)
246-307: LGTM - Dynamic handler support implementation.The enhanced request handling correctly:
- Parses body only for mutating methods (POST/PUT/PATCH)
- Constructs query params from URL
- Provides a clean
MockResponseHelperinterface- Falls back to static response when no handler is defined
319-356: LGTM - Route matching with path parameter extraction.The updated
findRoutecorrectly:
- Returns both the matched route and extracted path parameters
- Handles exact matches efficiently
- Supports Express-style
:parampath parameterslibs/testing/src/index.ts (1)
85-93: LGTM!The new types (
MockRequest,MockResponseHelper,MockRouteHandler) are properly exported through the barrel file, maintaining consistency with existing export patterns and enabling consumers to use the new dynamic handler functionality.apps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.ts (4)
84-146: LGTM - Well-structured E2E test setup.The
beforeAllblock correctly:
- Initializes the mock API server with dynamic handlers
- Starts the MCP test server with appropriate environment configuration
- Creates an authenticated test client
- Uses a reasonable 60s timeout for server startup
148-157: LGTM - Proper resource cleanup.The
afterAllcorrectly cleans up resources in the right order (client → server → mockApi), andbeforeEachresets captured headers to prevent test pollution.
176-205: Comprehensive staticAuth JWT E2E tests.Good coverage of:
- Authorization header presence in API requests
- Correct Bearer token format verification
- Accept header inclusion
- Response parsing with data wrapper
These tests effectively validate end-to-end security header propagation.
207-223: Important test for public endpoint behavior.This test validates that public endpoints with
security: []correctly do NOT receive Authorization headers, ensuring the adapter respects security requirements at the endpoint level.libs/uipack/package.json (1)
62-62: No changes required — dependency update is compliant.The enclave-vm dependency update does not add React dependencies and complies with the coding guidelines. Since enclave-vm is an external private package with no public changelog available, verification of breaking changes must be handled through your internal testing process.
apps/demo/src/main.ts (1)
11-21: Demo app simplified for testing purposes.The changes look appropriate for a demo/testing scenario. A few notes:
- The empty
adapters: []on line 14 could potentially be omitted if it's the default, but keeping it explicit is fine.- The previous apps are commented out rather than removed - consider whether they should be kept as examples or removed to reduce noise.
Please confirm this simplified configuration with
mode: 'public'is intentional for the current testing phase and won't be deployed to any shared environments.libs/sdk/src/auth/jwks/jwks.service.ts (2)
202-207: Issuer validation mirrors permissive behavior from main path.The issuer validation adds
payload.isstoexpectedIssuersand then checks ifpayload.issmatches any of them—this will always succeed whenpayload.issis present. This mirrors the same pattern injwtVerify(line 123-125) with the comment about "invalid issuer" in cloud gateway.This is intentional for compatibility, but worth noting that issuer validation in the weak-key fallback is similarly permissive.
Confirm this permissive issuer validation aligns with the cloud gateway compatibility requirements mentioned in the original code.
155-161: Address weak RSA key error detection fragility and improve error typing.The
isWeakKeyErrormethod relies on string matching for jose error messages, which is fragile. While the existingverifyWithWeakKey()fallback handles detection failures gracefully, this approach is brittle if jose changes its error message format in future versions.Additionally, the
error: anyparameter violates strict TypeScript guidelines. Consider:
- Change the parameter to
error: unknownand improve type narrowing- Add a fallback detection method or attempt key validation before calling
jwtVerify()to avoid relying solely on error message parsing- Document why string matching is necessary (jose v6 doesn't expose a stable error code for this condition)
libs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.ts (2)
56-81: Good test coverage for strong key path.The test correctly validates that 2048-bit RSA keys work without triggering weak key warnings, establishing a baseline for comparison.
83-112: Solid test for weak key verification and warning emission.The test properly validates:
- Fallback verification succeeds with 1024-bit key
- Security warning is emitted
- Warning content includes expected details
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
eslint.config.mjs (1)
8-8: Consider removing the now-unreachable configuration block for.d.tsfiles.The addition of
**/*.d.tsand**/*.d.ts.mapto the ignores list makes sense for generated files. However, this change renders the configuration block at lines 58-63 unreachable, as ESLint will never process.d.tsfiles to apply those rules.♻️ Remove dead code
If the intention is to completely ignore
.d.tsfiles (rather than lint them with relaxed rules), consider removing the now-unused configuration:- { - // Allow `any` in .d.ts declaration files for compatibility with external libraries (React, Handlebars, etc.) - files: ['**/*.d.ts'], - rules: { - '@typescript-eslint/no-explicit-any': 'off', - }, - },If you prefer to lint
.d.tsfiles with relaxed rules instead, remove'**/*.d.ts'from line 8 and keep the configuration block.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
eslint.config.mjslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/utils/src/crypto/index.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- libs/utils/src/crypto/index.ts
- libs/sdk/src/auth/jwks/tests/jwks-weak-key.test.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use strict TypeScript mode with no `any` types without strong justification
Applied to files:
eslint.config.mjs
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx} : Do not use `any` type without justification in TypeScript code
Applied to files:
eslint.config.mjs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Libraries
- GitHub Check: Lint & Format Checks
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @libs/sdk/src/auth/jwks/jwks.service.ts:
- Around line 215-220: The issuer validation is flawed because it compares
normalizeIssuer(payload.iss) to each normalizeIssuer(iss) and you also push
payload.iss into expectedIssuers, creating a tautology that lets any iss pass;
fix validateIssuer logic in the function containing expectedIssuers by removing
adding payload.iss to the allowed list and instead check if
normalizeIssuer(payload.iss) matches any normalizeIssuer(provider.issuerUrl)
(and any other configured trusted issuers), or alternatively compare
normalizeIssuer(payload.iss) against the set of trusted issuers only (e.g.,
normalizeIssuer(provider.issuerUrl) and any explicitly configured issuers) using
expectedIssuers.some(iss => normalizeIssuer(payload.iss) ===
normalizeIssuer(iss)); ensure payload.iss is not added to expectedIssuers before
the comparison.
🧹 Nitpick comments (8)
libs/testing/src/auth/mock-api-server.ts (3)
70-79: Consider type-level enforcement of mutual exclusivity.While runtime validation in
validateRouteensures that exactly one ofhandlerorresponseis defined, TypeScript currently allows both to be undefined or both to be present. Consider using a discriminated union to enforce this at compile time:♻️ Proposed type-safe alternative
-export interface MockRoute { +export type MockRoute = { /** HTTP method */ method: 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'; /** Path to match (e.g., '/products', '/products/:id') */ path: string; - /** Static response to return (mutually exclusive with handler) */ - response?: MockResponse; - /** Dynamic handler function (mutually exclusive with response) */ - handler?: MockRouteHandler; -} +} & ( + | { response: MockResponse; handler?: never } + | { handler: MockRouteHandler; response?: never } +);This approach eliminates the need for runtime validation in
validateRouteand catches configuration errors at compile time.
264-306: Consider adding safeguards for body parsing.The dynamic handler implementation correctly parses request bodies for POST/PUT/PATCH methods, but lacks protection against large payloads or slow clients.
💡 Recommended safeguards
Consider adding to the
parseBodymethod:
- Size limit: Reject requests exceeding a reasonable threshold (e.g., 10MB for test scenarios)
- Timeout: Abort parsing if it takes too long (e.g., 30 seconds)
Example size limit check in
parseBody:private async parseBody(req: IncomingMessage, maxSize = 10 * 1024 * 1024): Promise<unknown> { return new Promise((resolve, reject) => { let body = ''; let size = 0; const onData = (chunk: Buffer) => { size += chunk.length; if (size > maxSize) { cleanup(); reject(new Error('Request body too large')); return; } body += chunk.toString(); }; // ... rest of implementation }); }
153-153: Non-null assertions are safe but consider alternatives.The non-null assertions on
this.serverare justified:
- Line 153: Inside the
listencallback, the server is guaranteed to exist- Line 182: Protected by the guard clause on line 177
However, for strict adherence to the coding guideline of avoiding non-null assertions, consider using a local variable:
♻️ Optional refactor to avoid non-null assertions
Line 153:
- const address = this.server!.address(); + const server = this.server; + if (!server) return reject(new Error('Server not initialized')); + const address = server.address();Line 182:
- this.server!.close((err) => { + const server = this.server; + server.close((err) => {Also applies to: 182-182
libs/utils/src/crypto/node.ts (2)
7-7: Minor: Consider keeping thenode:prefix for consistency.The import was changed from
node:cryptotocrypto. While both work in Node.js, thenode:prefix is the modern convention that makes it explicit this is a Node.js built-in module rather than an npm package.♻️ Suggested change
-import crypto from 'crypto'; +import crypto from 'node:crypto';
168-170: Consider documenting PSS limitation or adding padding support.The
rsaSignfunction doesn't support RSA-PSS padding options. If a caller uses this with a PS256/384/512 algorithm string (as mapped byjwtAlgToNodeAlg), the signature will use PKCS#1 v1.5 padding instead of PSS, which would produce invalid signatures for PS* algorithms.Note that
createSignedJwtcorrectly handles this by constructing aSignKeyObjectInputwith padding options for PS* algorithms, so this may only be an issue ifrsaSignis used directly.♻️ Option: Add PSS support to rsaSign
/** * Sign data using RSA with the specified algorithm * * @param algorithm - Node.js crypto algorithm (e.g., 'RSA-SHA256') * @param data - Data to sign * @param privateKey - Private key to sign with + * @param usePss - Whether to use RSA-PSS padding (required for PS* JWT algorithms) * @returns Signature buffer */ -export function rsaSign(algorithm: string, data: Buffer, privateKey: crypto.KeyObject): Buffer { - return crypto.sign(algorithm, data, privateKey); +export function rsaSign( + algorithm: string, + data: Buffer, + privateKey: crypto.KeyObject, + usePss = false, +): Buffer { + const signingKey = usePss + ? { + key: privateKey, + padding: crypto.constants.RSA_PKCS1_PSS_PADDING, + saltLength: crypto.constants.RSA_PSS_SALTLEN_DIGEST, + } + : privateKey; + return crypto.sign(algorithm, data, signingKey); +}libs/sdk/src/auth/jwks/jwks.service.ts (3)
137-150: Minor: Redundant JWKS fetch in error handler.The JWKS is already fetched at line 119 and could be reused in the error handler instead of calling
getJwksForProvideragain at line 140. While the cache prevents a network request, it's slightly inefficient.♻️ Reuse already-fetched JWKS
+ const jwks = await this.getJwksForProvider(p); + if (!jwks?.keys?.length) continue; try { - const jwks = await this.getJwksForProvider(p); - if (!jwks?.keys?.length) continue; const draftPayload = decodeJwtPayloadSafe(token); const JWKS = createLocalJWKSet(jwks); // ... rest of try block } catch (e: any) { // Check for weak RSA key error from jose library if (this.isWeakKeyError(e)) { - const jwks = await this.getJwksForProvider(p); if (jwks?.keys?.length) { const fallbackResult = await this.verifyWithWeakKey(token, jwks, p);
267-285: Code duplication withjwtAlgToNodeAlgin utils.This method duplicates the
jwtAlgToNodeAlgfunction fromlibs/utils/src/crypto/node.ts. Consider importing and reusing the utility function to maintain a single source of truth for the JWT algorithm mapping.♻️ Suggested refactor
+import { jwtAlgToNodeAlg } from '@frontmcp/utils'; // ... other imports // In the class, remove the private method and use the imported function: - private getNodeAlgorithm(alg: string): string { - const algorithms: Record<string, string> = { - RS256: 'RSA-SHA256', - RS384: 'RSA-SHA384', - RS512: 'RSA-SHA512', - PS256: 'RSA-SHA256', - PS384: 'RSA-SHA384', - PS512: 'RSA-SHA512', - }; - const nodeAlg = algorithms[alg]; - if (!nodeAlg) { - throw new Error(`Unsupported JWT algorithm: ${alg}`); - } - return nodeAlg; - } // Then update the usage at line 201: - const algorithm = this.getNodeAlgorithm(jwtAlg); + const algorithm = jwtAlgToNodeAlg(jwtAlg);
156-162: Consider documenting the jose error message format dependency.The weak key detection relies on string matching against jose error messages. While this approach is well-tested with documented error formats (
'RS256 requires key modulusLength to be 2048 bits or larger','modulusLength must be at least 2048'), it could become fragile if jose changes its error message in a future major version. The existing test suite provides good documentation of expected formats.If jose exposes specific error codes or classes for weak RSA key validation in future versions, consider migrating to those instead. For now, the documented test cases serve as a maintenance reference for this implementation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/sdk/src/auth/jwks/jwks.service.tslibs/testing/src/auth/mock-api-server.tslibs/utils/src/crypto/node.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification
Avoid non-null assertions (!) - use proper error handling and type guards instead
Use@frontmcp/utilsfor cryptographic operations instead ofnode:cryptodirectly
Use@frontmcp/utilsfor file system operations instead offs/promisesornode:fsdirectly
Files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/testing/src/auth/mock-api-server.tslibs/utils/src/crypto/node.ts
libs/{sdk,adapters}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters}/**/*.{ts,tsx}: Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities
Files:
libs/sdk/src/auth/jwks/jwks.service.ts
libs/sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/**/*.{ts,tsx}: Preferinterfacefor defining object shapes in TypeScript, avoidanytypes
Use type parameters with constraints instead of unconstrained generics withanydefaults
Validate URIs per RFC 3986 at metadata level usingisValidMcpUrirefinement
UsechangeScopeinstead ofscopein change event properties to avoid confusion with Scope class
Fail fast on invalid hook flows by validating hooks match their entry type
Centralize record types in common/records and import from there, not from module-specific files
Return strictly typed MCP protocol responses (e.g.,Promise<GetPromptResult>) instead ofPromise<unknown>for MCP entry methods
Useunknowninstead ofanyfor generic type parameter defaults (not for MCP protocol types)
Create shared base classes for common functionality like ExecutionContextBase for ToolContext and ResourceContext
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Validate inputs and outputs through parseOutput/safeParseOutput methods in validation flows
Files:
libs/sdk/src/auth/jwks/jwks.service.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/testing/src/auth/mock-api-server.tslibs/utils/src/crypto/node.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use `frontmcp/utils` for cryptographic operations instead of `node:crypto` directly
📚 Learning: 2025-11-05T15:00:47.800Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 11
File: libs/core/src/auth/jwks/jwks.service.ts:62-0
Timestamp: 2025-11-05T15:00:47.800Z
Learning: In the FrontMCP codebase (libs/core/src/auth/jwks/jwks.service.ts), the verifyGatewayToken method intentionally skips signature verification when running in development-only no-auth mode, using decodeJwtPayloadSafe instead of jwtVerify. This is a deliberate design for local development convenience and should not be flagged as a security issue when the PR or code context indicates development/no-auth mode.
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Avoid using `node:crypto` directly; always use `frontmcp/utils` for cross-platform cryptographic support
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use `frontmcp/utils` for cryptographic operations instead of `node:crypto` directly
Applied to files:
libs/utils/src/crypto/node.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Always use `frontmcp/utils` for cryptographic operations (hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode) instead of `node:crypto`
Applied to files:
libs/utils/src/crypto/node.ts
🧬 Code graph analysis (1)
libs/testing/src/auth/mock-api-server.ts (2)
libs/testing/src/index.ts (5)
MockRequest(90-90)MockResponseHelper(91-91)MockRouteHandler(92-92)MockRoute(88-88)MockResponse(89-89)libs/testing/src/interceptor/mock-registry.ts (1)
match(55-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Libraries
- GitHub Check: Lint & Format Checks
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
libs/testing/src/auth/mock-api-server.ts (7)
37-68: LGTM! Well-defined type interfaces for dynamic route handling.The new types provide a clean API surface for dynamic route handlers, with proper type safety using
unknownfor request bodies and clear separation of concerns between request data and response helpers.
129-131: LGTM! Proper validation on construction.Validating routes during construction ensures early detection of configuration errors, providing a better developer experience.
224-231: LGTM! Clear validation logic with descriptive error messages.The mutual exclusivity validation is correctly implemented with informative error messages that include the method and path for easy debugging.
332-369: LGTM! Robust route matching with path parameter extraction.The route matching logic correctly handles both exact paths and parameterized routes, properly extracting path parameters for handler use.
371-402: LGTM! Proper error handling and cleanup.The
parseBodymethod now correctly handles stream errors with proper listener cleanup, addressing the previous review comment. The implementation prevents memory leaks by removing all listeners in the cleanup function and gracefully handles JSON parsing failures.
404-413: LGTM! Correct header normalization.The header normalization correctly handles the case-insensitive nature of HTTP headers and appropriately handles multi-value headers by selecting the first value, which is suitable for a testing mock.
285-285: Type assertion is safe for Node.js IncomingMessage headers.The type assertion from
IncomingHttpHeaderstoRecord<string, string | string[] | undefined>is safe because Node.js headers match this structure, andnormalizeHeadersproperly handles all cases.libs/utils/src/crypto/node.ts (3)
138-158: LGTM!The RSA key pair generation is correctly implemented. Using
Date.now()andMath.random()forkidgeneration is acceptable since it's just an identifier, not a security-sensitive value.
175-190: LGTM - previous review feedback addressed.The function now correctly throws an error for unsupported JWT algorithms instead of silently falling back to a default.
201-224: LGTM!The JWT creation function correctly handles both PKCS#1 v1.5 (RS*) and RSA-PSS (PS*) algorithms with appropriate padding options. The implementation properly mirrors the verification logic in
JwksService.libs/sdk/src/auth/jwks/jwks.service.ts (3)
8-21: LGTM - previous review feedback addressed.The
warnedProvidersis now correctly instance-scoped, ensuring proper isolation betweenJwksServiceinstances.
247-265: LGTM!The key matching logic uses a sensible priority order: exact
kidmatch, then algorithm match, then fallback to the first RSA key. This aligns with standard JWKS key selection behavior.
2-2: Keepnode:cryptoimport for JWT signature verification.The @frontmcp/utils module does not expose the required APIs for this file's use case: JWT signature verification with
crypto.verify(), RSA-PSS padding constants,crypto.KeyObjecttypes, and key generation functions likecrypto.createPublicKey()andcrypto.generateKeyPairSync()are not available through utils. Thenode:cryptoimport is necessary and justified for JWKS authentication operations.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @libs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.ts:
- Line 7: Replace the direct Node import "import crypto from 'crypto';" in
jwks-weak-key.test.ts with the utilities provided by the new module in this PR:
import the RSA/crypto utilities from '@frontmcp/utils/crypto/node' (or the
appropriate exported symbol from @frontmcp/utils) and update any usages in the
test (e.g., key generation or signing calls referenced in this file) to call the
functions from that module instead of the node:crypto API so the test uses the
sanctioned crypto helpers.
In @libs/sdk/src/auth/jwks/jwks.service.ts:
- Around line 161-166: Change the isWeakKeyError signature to accept error:
unknown (private isWeakKeyError(error: unknown): boolean) and safely extract a
string message without using any; for example, compute const message = (typeof
error === 'object' && error !== null && 'message' in error && typeof (error as
any).message === 'string') ? (error as any).message : String(error); then use
message.includes(...) as before; update any related tests if they reference the
function signature.
🧹 Nitpick comments (4)
libs/sdk/src/auth/jwks/jwks.service.ts (2)
208-208: Use the existing isRsaPssAlg utility.Line 208 reimplements PSS algorithm detection with
jwtAlg.startsWith('PS'), but the codebase now hasisRsaPssAlgutility (imported on line 4) that serves this exact purpose.♻️ Use the isRsaPssAlg utility
- const verifyKey: crypto.KeyObject | crypto.VerifyKeyObjectInput = jwtAlg.startsWith('PS') + const verifyKey: crypto.KeyObject | crypto.VerifyKeyObjectInput = isRsaPssAlg(jwtAlg) ? { key: publicKey, padding: crypto.constants.RSA_PKCS1_PSS_PADDING,Don't forget to add the import at the top:
-import { jwtAlgToNodeAlg } from '@frontmcp/utils'; +import { jwtAlgToNodeAlg, isRsaPssAlg } from '@frontmcp/utils';
2-2: Consider exposing JWT verification APIs in @frontmcp/utils for consistency.The
node:cryptoimport on line 2 is used for JWT verification in the weak-key fallback handler (verifyWithWeakKey, lines 172-217). While the coding guideline recommends using@frontmcp/utilsfor cryptographic operations, this module does not currently expose the required APIs (createPublicKey,verify,generateKeyPairSync,crypto.constants). Exposing these through@frontmcp/utilswould enable full compliance with the guideline and reduce directnode:cryptodependencies across the SDK.libs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.ts (1)
25-69: Eliminate code duplication by using utilities from@frontmcp/utils.The functions
generateRsaKeyPair(lines 25-41),jwtAlgToNodeAlg(lines 43-57), andrsaSignJwt(lines 59-69) duplicate functionality now exported from@frontmcp/utils/crypto/node. Refactor to import and use:
generateRsaKeyPairfrom@frontmcp/utils/crypto/nodejwtAlgToNodeAlgfrom@frontmcp/utilsrsaSignfrom@frontmcp/utils/crypto/node(slight API adjustment needed)This eliminates duplication, validates the utilities work correctly, and follows the project's guideline to use
@frontmcp/utilsfor crypto operations.♻️ Import utilities instead of duplicating
Remove the duplicate helper functions and import from utils:
-import crypto from 'crypto'; +import { generateRsaKeyPair, rsaSign, jwtAlgToNodeAlg } from '@frontmcp/utils/crypto/node'; import { JwksService } from '../jwks.service'; - -type RsaJwk = { - kty: 'RSA'; - kid: string; - alg: string; - use: 'sig'; - n: string; - e: string; -}; - -type RsaKeyPair = { - privateKey: crypto.KeyObject; - publicKey: crypto.KeyObject; - publicJwk: RsaJwk; -}; - -function generateRsaKeyPair(modulusLength = 2048, alg = 'RS256'): RsaKeyPair { - const kid = `rsa-key-${Date.now()}-${Math.random().toString(36).slice(2, 7)}`; - const { privateKey, publicKey } = crypto.generateKeyPairSync('rsa', { modulusLength }); - const exported = publicKey.export({ format: 'jwk' }) as { n: string; e: string }; - return { - privateKey, - publicKey, - publicJwk: { - kty: 'RSA', - kid, - alg, - use: 'sig', - n: exported.n, - e: exported.e, - }, - }; -} - -function jwtAlgToNodeAlg(jwtAlg: string): string { - const mapping: Record<string, string> = { - RS256: 'RSA-SHA256', - RS384: 'RSA-SHA384', - RS512: 'RSA-SHA512', - PS256: 'RSA-SHA256', - PS384: 'RSA-SHA384', - PS512: 'RSA-SHA512', - }; - const nodeAlg = mapping[jwtAlg]; - if (!nodeAlg) { - throw new Error(`Unsupported JWT algorithm: ${jwtAlg}`); - } - return nodeAlg; -} - -function rsaSignJwt(signatureInput: string, privateKey: crypto.KeyObject, jwtAlg: string): Buffer { + +import crypto from 'crypto'; +import type { RsaKeyPair } from '@frontmcp/utils/crypto/node'; + +function rsaSignJwt(signatureInput: string, privateKey: crypto.KeyObject, jwtAlg: string): Buffer { const nodeAlgorithm = jwtAlgToNodeAlg(jwtAlg); - const signingKey: crypto.KeyObject | crypto.SignKeyObjectInput = jwtAlg.startsWith('PS') - ? { - key: privateKey, - padding: crypto.constants.RSA_PKCS1_PSS_PADDING, - saltLength: crypto.constants.RSA_PSS_SALTLEN_DIGEST, - } - : privateKey; - return crypto.sign(nodeAlgorithm, Buffer.from(signatureInput), signingKey); + const options = jwtAlg.startsWith('PS') + ? { + padding: crypto.constants.RSA_PKCS1_PSS_PADDING, + saltLength: crypto.constants.RSA_PSS_SALTLEN_DIGEST, + } + : undefined; + return rsaSign(nodeAlgorithm, Buffer.from(signatureInput), privateKey, options); }Note: You'll still need
cryptoimport for the constants used inrsaSignJwt. Alternatively, refactor to usecreateSignedJwtdirectly from utils.libs/utils/src/crypto/node.ts (1)
112-160: Well-structured RSA key pair generation utilities.The
RsaJwkandRsaKeyPairinterfaces follow standard JWK patterns, and thegenerateRsaKeyPairfunction is well-documented with appropriate defaults (2048-bit modulus for short-lived tokens).Optional: Consider using cryptographically secure random for kid generation
Line 141 uses
Math.random()for kid generation. While the kid doesn't need to be secret, usingrandomByteswould be more consistent with crypto best practices:- const kid = `rsa-key-${Date.now()}-${Math.random().toString(36).slice(2, 7)}`; + const kid = `rsa-key-${Date.now()}-${Buffer.from(nodeCrypto.randomBytes(4)).toString('hex')}`;This is a low-priority suggestion since kid values don't have security requirements beyond uniqueness.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/sdk/src/index.tslibs/testing/src/auth/mock-api-server.tslibs/utils/src/crypto/index.tslibs/utils/src/crypto/jwt-alg.tslibs/utils/src/crypto/node.tslibs/utils/src/index.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification
Avoid non-null assertions (!) - use proper error handling and type guards instead
Use@frontmcp/utilsfor cryptographic operations instead ofnode:cryptodirectly
Use@frontmcp/utilsfor file system operations instead offs/promisesornode:fsdirectly
Files:
apps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.tslibs/sdk/src/index.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/utils/src/index.tslibs/utils/src/crypto/index.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.tslibs/testing/src/auth/mock-api-server.tslibs/utils/src/crypto/jwt-alg.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Use Jest for testing with 95%+ coverage requirement and 100% test pass rate
Do not use prefixes like 'PT-001' in test names
Do not skip constructor validation tests for error classes and types
Includeinstanceofchecks in tests for error classes to verify proper error hierarchy
Files:
apps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.ts
libs/{sdk,adapters}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters}/**/*.{ts,tsx}: Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities
Files:
libs/sdk/src/index.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.ts
libs/sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/**/*.{ts,tsx}: Preferinterfacefor defining object shapes in TypeScript, avoidanytypes
Use type parameters with constraints instead of unconstrained generics withanydefaults
Validate URIs per RFC 3986 at metadata level usingisValidMcpUrirefinement
UsechangeScopeinstead ofscopein change event properties to avoid confusion with Scope class
Fail fast on invalid hook flows by validating hooks match their entry type
Centralize record types in common/records and import from there, not from module-specific files
Return strictly typed MCP protocol responses (e.g.,Promise<GetPromptResult>) instead ofPromise<unknown>for MCP entry methods
Useunknowninstead ofanyfor generic type parameter defaults (not for MCP protocol types)
Create shared base classes for common functionality like ExecutionContextBase for ToolContext and ResourceContext
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Validate inputs and outputs through parseOutput/safeParseOutput methods in validation flows
Files:
libs/sdk/src/index.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.ts
libs/*/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Export everything users need through barrel exports (index.ts), do not add backwards compatibility aliases
Files:
libs/sdk/src/index.tslibs/utils/src/index.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/index.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/utils/src/index.tslibs/utils/src/crypto/index.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.tslibs/testing/src/auth/mock-api-server.tslibs/utils/src/crypto/jwt-alg.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use `frontmcp/utils` for cryptographic operations instead of `node:crypto` directly
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{theme,adapters,bundler}/**/*.{test,spec}.{ts,tsx,js,jsx} : Test behavior across all supported platform configurations (OpenAI, Claude, etc.)
Applied to files:
apps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Use proper ES module imports instead of `require()` for SDK imports; avoid dynamic require of `frontmcp/sdk` modules
Applied to files:
libs/sdk/src/index.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use `frontmcp/utils` for file system operations instead of `fs/promises` or `node:fs` directly
Applied to files:
libs/sdk/src/index.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use `frontmcp/utils` for cryptographic operations instead of `node:crypto` directly
Applied to files:
libs/sdk/src/index.tslibs/utils/src/index.tslibs/utils/src/crypto/index.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.tslibs/utils/src/crypto/jwt-alg.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*plugin.ts : Extend ExecutionContextBase with plugin-specific properties using module declaration (`declare module 'frontmcp/sdk'`) combined with `contextExtensions` in plugin metadata
Applied to files:
libs/sdk/src/index.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/**/*.{ts,tsx} : Never import React-free utilities from frontmcp/ui; use frontmcp/uipack for bundling, build tools, platform adapters, and theme utilities
Applied to files:
libs/sdk/src/index.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*plugin.ts : Use module augmentation for context properties via `declare module 'frontmcp/sdk'` combined with runtime plugin metadata `contextExtensions`, not module-level side effects
Applied to files:
libs/sdk/src/index.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Avoid using `node:crypto` directly; always use `frontmcp/utils` for cross-platform cryptographic support
Applied to files:
libs/sdk/src/index.tslibs/utils/src/index.tslibs/utils/src/crypto/index.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.tslibs/utils/src/crypto/jwt-alg.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Always use `frontmcp/utils` for cryptographic operations (hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode) instead of `node:crypto`
Applied to files:
libs/sdk/src/index.tslibs/utils/src/index.tslibs/utils/src/crypto/index.tslibs/utils/src/crypto/node.tslibs/utils/src/crypto/jwt-alg.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/universal/**/*.{ts,tsx} : Universal app shell (UniversalApp, FrontMCPProvider) must provide platform-agnostic React context and initialization
Applied to files:
libs/sdk/src/index.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Return strictly typed MCP protocol responses (e.g., `Promise<GetPromptResult>`) instead of `Promise<unknown>` for MCP entry methods
Applied to files:
libs/sdk/src/index.ts
📚 Learning: 2025-11-05T15:00:47.800Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 11
File: libs/core/src/auth/jwks/jwks.service.ts:62-0
Timestamp: 2025-11-05T15:00:47.800Z
Learning: In the FrontMCP codebase (libs/core/src/auth/jwks/jwks.service.ts), the verifyGatewayToken method intentionally skips signature verification when running in development-only no-auth mode, using decodeJwtPayloadSafe instead of jwtVerify. This is a deliberate design for local development convenience and should not be flagged as a security issue when the PR or code context indicates development/no-auth mode.
Applied to files:
libs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to libs/*/src/index.ts : Export everything users need through barrel exports (index.ts), do not add backwards compatibility aliases
Applied to files:
libs/utils/src/crypto/index.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/index.{ts,js} : Export all public APIs through appropriate entry points (frontmcp/uipack, frontmcp/uipack/adapters, frontmcp/uipack/theme, etc.)
Applied to files:
libs/utils/src/crypto/index.ts
🧬 Code graph analysis (3)
apps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.ts (1)
libs/testing/src/auth/mock-api-server.ts (3)
MockAPIServer(135-460)MockRequest(40-53)MockResponseHelper(58-63)
libs/utils/src/crypto/index.ts (1)
libs/utils/src/index.ts (3)
CryptoProvider(86-86)getCrypto(67-67)isNode(83-83)
libs/testing/src/auth/mock-api-server.ts (2)
libs/testing/src/index.ts (6)
MockRequest(90-90)MockResponseHelper(91-91)MockRouteHandler(92-92)MockRoute(88-88)MockResponse(89-89)MockAPIServerOptions(86-86)libs/testing/src/interceptor/mock-registry.ts (1)
match(55-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint & Format Checks
- GitHub Check: Build Libraries
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
libs/utils/src/crypto/jwt-alg.ts (1)
1-28: LGTM! Clean and focused utility module.The JWT algorithm helpers are well-designed:
- Intentionally dependency-free for browser/Node compatibility
- Clear mapping of JWT algorithms to Node crypto digest names
- Proper error handling for unsupported algorithms
- Simple and testable functions
libs/sdk/src/auth/jwks/jwks.service.ts (1)
237-242: One-time warning implementation looks good.The per-provider warning mechanism using
warnedProvidersSet ensures users are notified about weak keys without spamming logs. This is a good user experience pattern.libs/testing/src/auth/mock-api-server.ts (3)
79-99: Excellent use of discriminated union types.The
MockRoutetype using mutually exclusivehandlerandresponsefields withneveris a clean TypeScript pattern that enforces correctness at compile time. This prevents the common mistake of defining both a handler and a static response.
241-253: Good runtime validation complements type safety.The
validateRoutemethod provides runtime checks that complement the compile-time type safety, which is essential for routes that might be constructed from configuration or added dynamically. The error messages are clear and actionable.
396-442: Robust body parsing with appropriate safeguards.The
parseBodymethod includes:
- Size limits to prevent DoS (maxBodyBytes)
- Timeout protection (bodyTimeoutMs)
- Proper cleanup of event listeners
- Graceful error handling
This is production-quality defensive coding.
apps/e2e/demo-e2e-openapi/e2e/openapi-security.e2e.test.ts (3)
83-145: Well-structured E2E test setup.The test setup demonstrates good practices:
- Proper resource lifecycle management with beforeAll/afterAll
- Dynamic mock handlers to capture actual request headers for verification
- Realistic OpenAPI spec with both secured and public endpoints
- Clear environment variable configuration for the test server
This provides strong evidence that the security implementation works correctly.
175-204: Comprehensive verification of authorization flow.The tests verify multiple aspects of the auth implementation:
- The tool call succeeds (functional correctness)
- The response contains the expected auth token (end-to-end verification)
- The actual HTTP headers received by the mock server are correct (implementation verification)
- Standard HTTP headers like Accept are included
This multi-layered verification catches different classes of bugs.
206-222: Good negative test for public endpoints.Testing that public endpoints (with
security: []) do NOT receive authorization headers is important to verify that the security implementation is selective and doesn't leak credentials unnecessarily. This follows the principle of least privilege.libs/utils/src/index.ts (1)
68-69: LGTM! JWT algorithm utilities properly exported.The new crypto utility exports (
jwtAlgToNodeAlgandisRsaPssAlg) are correctly added to the barrel export, making them available to consumers of@frontmcp/utils.libs/utils/src/crypto/index.ts (2)
10-10: LGTM! JWT algorithm utilities properly re-exported.The new utility functions are correctly exported from the crypto module, making them available to consumers alongside other cryptographic operations.
17-34: Documentation and implementation improvements look good.The updated documentation clarifies the lazy initialization strategy and explains why Node-only modules aren't imported at module load time. The non-null assertion on line 34 is safe because both branches (Node and browser) will initialize
_providersuccessfully or throw duringrequire().libs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.ts (1)
71-360: Comprehensive test coverage for weak RSA key handling.The test suite thoroughly covers:
- Strong vs weak key verification paths
- One-time security warnings per provider
- Invalid signature detection
- PS256 (RSA-PSS) fallback support
- Expired token rejection
- Error detection logic
- Key matching algorithms
The private method testing (lines 287, 303, 316, 334, 349) using
(service as any)is acceptable for verifying internal logic.libs/utils/src/crypto/node.ts (3)
9-10: LGTM! JWT algorithm utilities properly imported and re-exported.The import and re-export of
isRsaPssAlgandjwtAlgToNodeAlgmakes these utilities available to consumers of the Node crypto module.
167-177: LGTM! Clean RSA signing wrapper.The
rsaSignfunction provides a clean abstraction over Node's crypto.sign with proper support for RSA-PSS padding options.
188-214: LGTM! JWT creation with proper RSA-PSS support.The
createSignedJwtfunction correctly:
- Maps JWT algorithms to Node crypto algorithms using
jwtAlgToNodeAlg- Detects RSA-PSS algorithms and applies appropriate padding
- Uses base64url encoding per JWT spec (RFC 7519)
- Includes kid and alg in the JWT header
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @libs/sdk/src/auth/jwks/jwks.service.ts:
- Around line 2-4: The jwks.service.ts file currently imports node:crypto and
calls crypto.createPublicKey, crypto.verify and crypto.constants directly for
RSA-PSS verification; extract those primitives into a new rsaVerify utility in
libs/utils/src/crypto/node.ts, re-export it from libs/utils/src/crypto/index.ts,
remove the node:crypto import from jwks.service.ts and replace direct calls to
crypto.createPublicKey/crypto.verify/crypto.constants with a call to rsaVerify
(or equivalent exported helper) in jwks.service.ts (referencing where
createPublicKey and verify were used around the RSA-PSS handling and weak-key
fallback) so jwks.service.ts consumes the cryptographic abstraction from
@frontmcp/utils instead of node:crypto.
🧹 Nitpick comments (2)
libs/sdk/src/auth/jwks/jwks.service.ts (2)
178-261: Consider potential drift from jose verification logic.The manual JWT verification in the fallback path duplicates
jose's verification logic. Over time, this implementation could diverge fromjoseif they add new validation checks or update security requirements.Consider:
- Documenting which jose version this fallback is compatible with
- Adding tests that verify the fallback produces identical results to jose for valid weak keys
- Periodically auditing this code when updating jose versions
286-288: Optional: Consider inlining the wrapper.The
getNodeAlgorithmmethod is a thin wrapper aroundjwtAlgToNodeAlg. Consider callingjwtAlgToNodeAlgdirectly at line 213 to reduce indirection, unless you anticipate adding custom logic here.♻️ Simplified approach
At line 213, call directly:
- const algorithm = this.getNodeAlgorithm(jwtAlg); + const algorithm = jwtAlgToNodeAlg(jwtAlg);And remove the wrapper method (lines 286-288).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
libs/sdk/jest.config.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.tstsconfig.base.json
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/sdk/src/auth/jwks/tests/jwks-weak-key.test.ts
- tsconfig.base.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification
Avoid non-null assertions (!) - use proper error handling and type guards instead
Use@frontmcp/utilsfor cryptographic operations instead ofnode:cryptodirectly
Use@frontmcp/utilsfor file system operations instead offs/promisesornode:fsdirectly
Files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.tslibs/sdk/jest.config.ts
libs/{sdk,adapters}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters}/**/*.{ts,tsx}: Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities
Files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/sdk/jest.config.ts
libs/sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/**/*.{ts,tsx}: Preferinterfacefor defining object shapes in TypeScript, avoidanytypes
Use type parameters with constraints instead of unconstrained generics withanydefaults
Validate URIs per RFC 3986 at metadata level usingisValidMcpUrirefinement
UsechangeScopeinstead ofscopein change event properties to avoid confusion with Scope class
Fail fast on invalid hook flows by validating hooks match their entry type
Centralize record types in common/records and import from there, not from module-specific files
Return strictly typed MCP protocol responses (e.g.,Promise<GetPromptResult>) instead ofPromise<unknown>for MCP entry methods
Useunknowninstead ofanyfor generic type parameter defaults (not for MCP protocol types)
Create shared base classes for common functionality like ExecutionContextBase for ToolContext and ResourceContext
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Validate inputs and outputs through parseOutput/safeParseOutput methods in validation flows
Files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/sdk/jest.config.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.tslibs/sdk/jest.config.ts
🧠 Learnings (24)
📚 Learning: 2025-11-05T15:00:47.800Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 11
File: libs/core/src/auth/jwks/jwks.service.ts:62-0
Timestamp: 2025-11-05T15:00:47.800Z
Learning: In the FrontMCP codebase (libs/core/src/auth/jwks/jwks.service.ts), the verifyGatewayToken method intentionally skips signature verification when running in development-only no-auth mode, using decodeJwtPayloadSafe instead of jwtVerify. This is a deliberate design for local development convenience and should not be flagged as a security issue when the PR or code context indicates development/no-auth mode.
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Avoid using `node:crypto` directly; always use `frontmcp/utils` for cross-platform cryptographic support
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.tslibs/sdk/jest.config.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use `frontmcp/utils` for cryptographic operations instead of `node:crypto` directly
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.tslibs/sdk/jest.config.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Use `unknown` instead of `any` for generic type parameter defaults (not for MCP protocol types)
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Avoid using `any` type without justification in TypeScript files
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx} : Do not use `any` type without justification in TypeScript code
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Avoid non-null assertions (`!`) - use proper error handling and type guards instead
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx,js,jsx} : Do not expose internal error details in public APIs - use sanitized error messages
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to libs/{sdk,adapters}/**/*.{ts,tsx} : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Use type parameters with constraints instead of unconstrained generics with `any` defaults
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use strict TypeScript mode with no `any` types without strong justification
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Always use `frontmcp/utils` for cryptographic operations (hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode) instead of `node:crypto`
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.tslibs/sdk/jest.config.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Never hardcode encryption keys; use environment variables for key management
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Use proper ES module imports instead of `require()` for SDK imports; avoid dynamic require of `frontmcp/sdk` modules
Applied to files:
libs/sdk/jest.config.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use `frontmcp/utils` for file system operations instead of `fs/promises` or `node:fs` directly
Applied to files:
libs/sdk/jest.config.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*plugin.ts : Use module augmentation for context properties via `declare module 'frontmcp/sdk'` combined with runtime plugin metadata `contextExtensions`, not module-level side effects
Applied to files:
libs/sdk/jest.config.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*plugin.ts : Extend ExecutionContextBase with plugin-specific properties using module declaration (`declare module 'frontmcp/sdk'`) combined with `contextExtensions` in plugin metadata
Applied to files:
libs/sdk/jest.config.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/bundler/**/*.{ts,tsx} : The bundler module must re-export utilities from frontmcp/uipack/bundler and provide SSR component bundling functionality
Applied to files:
libs/sdk/jest.config.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/index.{ts,js} : Export all public APIs through appropriate entry points (frontmcp/uipack, frontmcp/uipack/adapters, frontmcp/uipack/theme, etc.)
Applied to files:
libs/sdk/jest.config.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Centralize record types in common/records and import from there, not from module-specific files
Applied to files:
libs/sdk/jest.config.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/package.json : Entry points must match the documented paths: frontmcp/ui/react, frontmcp/ui/renderers, frontmcp/ui/render, frontmcp/ui/universal, frontmcp/ui/bundler, frontmcp/ui/bridge, frontmcp/ui/components, frontmcp/ui/layouts, frontmcp/ui/web-components
Applied to files:
libs/sdk/jest.config.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/**/*.{ts,tsx} : Never import React-free utilities from frontmcp/ui; use frontmcp/uipack for bundling, build tools, platform adapters, and theme utilities
Applied to files:
libs/sdk/jest.config.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{package.json,*.ts,*.tsx,*.js,*.jsx} : Do not add React dependencies to frontmcp/uipack - it must remain React-free. Use frontmcp/ui for React components.
Applied to files:
libs/sdk/jest.config.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{build,bundler}/**/*.{ts,tsx,js,jsx} : For server-side MDX rendering with bundled React, use frontmcp/ui/renderers instead of frontmcp/uipack/renderers
Applied to files:
libs/sdk/jest.config.ts
🧬 Code graph analysis (2)
libs/sdk/src/auth/jwks/jwks.service.ts (2)
libs/sdk/src/auth/jwks/jwks.types.ts (2)
ProviderVerifyRef(21-26)VerifyResult(28-36)libs/utils/src/crypto/jwt-alg.ts (2)
isRsaPssAlg(26-28)jwtAlgToNodeAlg(18-24)
libs/utils/src/crypto/node.ts (1)
libs/utils/src/crypto/jwt-alg.ts (2)
jwtAlgToNodeAlg(18-24)isRsaPssAlg(26-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint & Format Checks
- GitHub Check: Build Libraries
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
libs/utils/src/crypto/node.ts (3)
9-10: LGTM - Clean modular design.The import and re-export of JWT algorithm helpers from a separate module promotes good separation of concerns and makes these utilities available alongside the RSA functions.
140-160: LGTM - Proper RSA key generation with appropriate defaults.The function correctly generates RSA key pairs with a 2048-bit modulus (current minimum recommended strength). The documentation appropriately advises using 3072+ bits for longer-term keys.
188-214: LGTM - Correct JWT creation with proper RSA-PSS handling.The implementation correctly follows the JWT specification and properly handles RSA-PSS algorithms by applying appropriate padding options. The use of the
rsaSignhelper and algorithm mapping promotes good code reuse.libs/sdk/jest.config.ts (1)
33-33: LGTM - Standard Jest path mapping.The moduleNameMapper entry correctly resolves the new
@frontmcp/utils/crypto/nodepath alias to its TypeScript source, consistent with the existing pattern for other@frontmcp/*packages.libs/sdk/src/auth/jwks/jwks.service.ts (6)
9-15: LGTM - Clear security warning.The warning message appropriately communicates the security risk of weak RSA keys and advises users to contact their OAuth provider. The one-time per-provider warning mechanism (tracked via
warnedProviders) balances visibility with log noise.
139-149: LGTM - Proper weak-key fallback integration.The weak-key detection and fallback are well-integrated into the existing verification flow. The implementation:
- Preserves existing behavior for non-weak-key errors
- Reuses the already-fetched JWKS when available (line 142)
- Falls through to the next provider if fallback fails
- Maintains transparent error handling
184-197: LGTM - Proper JWT parsing and key matching.The token parsing correctly:
- Validates JWT format (3 parts)
- Decodes base64url-encoded header and payload
- Finds matching key from JWKS with appropriate fallback logic
214-221: Proper RSA-PSS verification with correct padding.The signature verification correctly applies RSA-PSS padding options when needed, using the algorithm helpers from
@frontmcp/utils. The verification logic properly handles both standard RSA and RSA-PSS algorithms.
228-248: LGTM - Thorough security validation.The fallback verification properly validates:
- Issuer against trusted providers
- Token expiration
- Emits security warning once per provider
This maintains security guarantees despite accepting weak keys.
161-172: String matching for weak key detection is the standard approach for this jose library.The jose library (v6.1.x) does not provide structured error codes or error types for weak RSA key validation failures—only error message strings. Your implementation is already the standard pattern for detecting these errors. The code is well-tested (see
jwks-weak-key.test.ts), defensively parses error objects, and includes a documented fallback mechanism (verifyWithWeakKey) for weak keys. The comment noting "best-effort fallback" and specifying what to update if jose changes its message format adequately addresses the maintenance concern.No changes needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
libs/utils/src/crypto/index.ts (1)
34-35: Avoid non-null assertion – use a type guard or throw instead.The
_provider!assertion bypasses TypeScript's null safety. Since_provideris assigned immediately before this return in both branches, consider restructuring to eliminate the assertion.Suggested fix
if (!_provider) { if (isNode()) { // Node.js: keep crypto module isolated from browser builds // eslint-disable-next-line @typescript-eslint/no-require-imports _provider = require('./node').nodeCrypto as CryptoProvider; } else { // Browser: noble-based implementation // eslint-disable-next-line @typescript-eslint/no-require-imports _provider = require('./browser').browserCrypto as CryptoProvider; } } - return _provider!; + // _provider is guaranteed to be assigned after the above branches + return _provider; }Alternatively, restructure to return directly from each branch to eliminate the nullable state entirely.
As per coding guidelines, non-null assertions should be avoided in favor of proper type guards.
libs/sdk/src/auth/jwks/jwks.service.ts (4)
95-97: Useunknowninstead ofanyin catch clauses.The coding guidelines require using
unknowninstead ofanyfor generic type parameter defaults in SDK files. The catch clause should useunknownand type-narrow appropriately.Suggested fix
- } catch (err: any) { - return { ok: false, error: err?.message ?? 'verification_failed' }; + } catch (err: unknown) { + const message = err instanceof Error ? err.message : 'verification_failed'; + return { ok: false, error: message }; }As per coding guidelines, use
unknowninstead ofanyin libs/sdk.
138-151: Useunknowninstead ofanyin the catch clause.Same issue as the previous catch block – prefer
unknownfor type safety.Suggested fix
- } catch (e: any) { + } catch (e: unknown) { // Check for weak RSA key error from jose library if (this.isWeakKeyError(e)) {
217-225: Error message is misleading when issuer claim is missing.When
payload.issis absent, the error returned isissuer_mismatch, but the actual issue is a missing issuer. Consider distinguishing these cases for clearer debugging.Suggested fix
// Validate issuer const payloadIssuerRaw = typeof payload.iss === 'string' ? payload.iss : undefined; if (!payloadIssuerRaw) { - return { ok: false, error: 'issuer_mismatch' }; + return { ok: false, error: 'missing_issuer' }; } const trustedIssuers = new Set([normalizeIssuer(provider.issuerUrl)]); const payloadIssuer = normalizeIssuer(payloadIssuerRaw); if (!trustedIssuers.has(payloadIssuer)) { return { ok: false, error: 'issuer_mismatch' }; }
247-249: Useunknowninstead ofanyin catch clause.Suggested fix
- } catch (err: any) { - return { ok: false, error: `weak_key_verification_failed: ${err?.message}` }; + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + return { ok: false, error: `weak_key_verification_failed: ${message}` }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
libs/sdk/jest.config.tslibs/sdk/src/auth/jwks/__tests__/jwks-weak-key.test.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/index.tslibs/utils/src/crypto/node.tslibs/utils/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/utils/src/index.ts
- libs/sdk/jest.config.ts
- libs/sdk/src/auth/jwks/tests/jwks-weak-key.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification
Avoid non-null assertions (!) - use proper error handling and type guards instead
Use@frontmcp/utilsfor cryptographic operations instead ofnode:cryptodirectly
Use@frontmcp/utilsfor file system operations instead offs/promisesornode:fsdirectly
Files:
libs/utils/src/crypto/index.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/utils/src/crypto/index.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.ts
libs/{sdk,adapters}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters}/**/*.{ts,tsx}: Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities
Files:
libs/sdk/src/auth/jwks/jwks.service.ts
libs/sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/**/*.{ts,tsx}: Preferinterfacefor defining object shapes in TypeScript, avoidanytypes
Use type parameters with constraints instead of unconstrained generics withanydefaults
Validate URIs per RFC 3986 at metadata level usingisValidMcpUrirefinement
UsechangeScopeinstead ofscopein change event properties to avoid confusion with Scope class
Fail fast on invalid hook flows by validating hooks match their entry type
Centralize record types in common/records and import from there, not from module-specific files
Return strictly typed MCP protocol responses (e.g.,Promise<GetPromptResult>) instead ofPromise<unknown>for MCP entry methods
Useunknowninstead ofanyfor generic type parameter defaults (not for MCP protocol types)
Create shared base classes for common functionality like ExecutionContextBase for ToolContext and ResourceContext
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Validate inputs and outputs through parseOutput/safeParseOutput methods in validation flows
Files:
libs/sdk/src/auth/jwks/jwks.service.ts
🧠 Learnings (12)
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Use `frontmcp/utils` for cryptographic operations instead of `node:crypto` directly
Applied to files:
libs/utils/src/crypto/index.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Avoid using `node:crypto` directly; always use `frontmcp/utils` for cross-platform cryptographic support
Applied to files:
libs/utils/src/crypto/index.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Always use `frontmcp/utils` for cryptographic operations (hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode) instead of `node:crypto`
Applied to files:
libs/utils/src/crypto/index.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.ts
📚 Learning: 2025-11-05T15:00:47.800Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 11
File: libs/core/src/auth/jwks/jwks.service.ts:62-0
Timestamp: 2025-11-05T15:00:47.800Z
Learning: In the FrontMCP codebase (libs/core/src/auth/jwks/jwks.service.ts), the verifyGatewayToken method intentionally skips signature verification when running in development-only no-auth mode, using decodeJwtPayloadSafe instead of jwtVerify. This is a deliberate design for local development convenience and should not be flagged as a security issue when the PR or code context indicates development/no-auth mode.
Applied to files:
libs/utils/src/crypto/index.tslibs/sdk/src/auth/jwks/jwks.service.tslibs/utils/src/crypto/node.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Use `unknown` instead of `any` for generic type parameter defaults (not for MCP protocol types)
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx} : Do not use `any` type without justification in TypeScript code
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Avoid using `any` type without justification in TypeScript files
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx,js,jsx} : Do not expose internal error details in public APIs - use sanitized error messages
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to **/*.{ts,tsx} : Avoid non-null assertions (`!`) - use proper error handling and type guards instead
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to libs/{sdk,adapters}/**/*.{ts,tsx} : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T17:16:04.274Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.274Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Use type parameters with constraints instead of unconstrained generics with `any` defaults
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Never hardcode encryption keys; use environment variables for key management
Applied to files:
libs/sdk/src/auth/jwks/jwks.service.ts
🧬 Code graph analysis (3)
libs/utils/src/crypto/index.ts (1)
libs/utils/src/crypto/node.ts (1)
rsaVerify(179-190)
libs/sdk/src/auth/jwks/jwks.service.ts (3)
libs/sdk/src/auth/jwks/jwks.types.ts (3)
JwksServiceOptions(4-16)ProviderVerifyRef(21-26)VerifyResult(28-36)libs/utils/src/crypto/index.ts (3)
rsaVerify(37-41)bytesToHex(168-172)randomBytes(57-62)libs/utils/src/crypto/node.ts (2)
rsaVerify(179-190)randomBytes(37-39)
libs/utils/src/crypto/node.ts (3)
libs/utils/src/crypto/index.ts (3)
rsaVerify(37-41)jwtAlgToNodeAlg(10-10)isRsaPssAlg(10-10)libs/utils/src/index.ts (3)
rsaVerify(68-68)jwtAlgToNodeAlg(69-69)isRsaPssAlg(70-70)libs/utils/src/crypto/jwt-alg.ts (2)
jwtAlgToNodeAlg(18-24)isRsaPssAlg(26-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Libraries
- GitHub Check: Lint & Format Checks
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
libs/utils/src/crypto/index.ts (1)
37-41: LGTM – properly guarded Node-only RSA verification.The function correctly uses
assertNodeto ensure Node.js runtime before invoking the Node crypto implementation, maintaining browser build isolation.libs/sdk/src/auth/jwks/jwks.service.ts (4)
252-270: LGTM – sensible key matching fallback chain.The method correctly prioritizes exact
kidmatch, then algorithm match, then falls back to the first RSA key. This handles various JWKS configurations gracefully.
416-420: Directnode:cryptousage for key operations.The coding guidelines recommend using
@frontmcp/utilsfor cryptographic operations. However,createPrivateKeyandgenerateKeyPairSyncaren't currently exposed via utils. If these operations become common, consider adding wrappers to@frontmcp/utils/crypto/node.
8-14: LGTM – one-time security warning mechanism.The per-provider warning tracking via
warnedProvidersSet ensures users are alerted to weak RSA keys without log spam. The warning message is clear and actionable.Also applies to: 21-21, 232-237
157-171: Error detection is properly validated – The string matching approach for weak-key detection (modulusLengthand2048) is confirmed to work with jose v6.1.3 and is covered by existing tests that validate both positive and negative cases. The existing code comment already documents the fragility and maintenance requirements, making this a non-issue.libs/utils/src/crypto/node.ts (6)
9-10: LGTM – appropriate import and re-export of algorithm helpers.The import and re-export of
isRsaPssAlgandjwtAlgToNodeAlgfrom./jwt-algcentralizes algorithm mapping logic for RSA operations.
109-131: LGTM – well-defined RSA key interfaces.The
RsaJwkandRsaKeyPairinterfaces provide clear type definitions for RSA key operations. The structure aligns with JWK RFC 7517 requirements for RSA signature keys.
133-160: LGTM – well-documented RSA key generation.The function provides sensible defaults (2048-bit modulus, RS256 algorithm) with clear documentation about when larger key sizes should be used. The
kidgeneration using timestamp + random bytes provides sufficient uniqueness.
162-177: LGTM – flexible RSA signing with PSS support.The function correctly delegates RSA-PSS padding configuration to callers, with clear documentation of this responsibility.
179-190: LGTM – proper RSA verification with automatic PSS handling.The function correctly encapsulates RSA-PSS complexity by automatically applying the appropriate padding and salt length when verifying PS256/PS384/PS512 signatures.
192-227: LGTM – correct JWT construction with RSA-PSS awareness.The function properly constructs signed JWTs, automatically applying RSA-PSS padding options for PS256/PS384/PS512 algorithms. The base64url encoding uses Node's native Buffer method.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.