Skip to content

Conversation

@jkyberneees
Copy link
Contributor

No description provided.

@jkyberneees jkyberneees requested a review from Copilot June 11, 2025 16:40

This comment was marked as outdated.

@jkyberneees jkyberneees requested a review from Copilot June 11, 2025 16:58

This comment was marked as outdated.

@jkyberneees jkyberneees requested a review from Copilot June 11, 2025 17:19
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a comprehensive middleware system into the project, including security tests for prototype pollution, rate limiting, logging, and CORS support while updating related documentation and configuration.

  • New security tests to prevent prototype pollution
  • New middleware modules for rate limiting, logging, and CORS
  • Documentation and dependency updates in package.json and README.md

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/security/prototype-pollution.test.js Introduces tests to validate prototype pollution protection
test/helpers/index.js Updates request helper to return a mutable proxy for testing
package.json Adds dependencies (jose, pino, typescript) for middleware support
lib/router/sequential.js Enhances parameter copy logic to filter out dangerous keys
lib/middleware/rate-limit.js Implements in-memory and sliding window rate limiting middleware
lib/middleware/logger.js Adds a logging middleware using Pino
lib/middleware/index.js Aggregates middleware modules for simple and advanced usage
lib/middleware/cors.js Creates a flexible CORS middleware
README.md Updates documentation to include middleware support

Comment on lines +202 to +206
await routerInstance.fetch(testReq)

// Should only copy own properties, not inherited ones
expect(testReq.params.safe).toBe('good')
expect(testReq.params.inherited).toBeUndefined()
Copy link

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

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

The test for handling inherited properties asserts on 'testReq.params', but since 'router.fetch' sets parameters on the request object, consider capturing and validating the response payload (e.g., via response.json()) to verify that only own properties are copied.

Suggested change
await routerInstance.fetch(testReq)
// Should only copy own properties, not inherited ones
expect(testReq.params.safe).toBe('good')
expect(testReq.params.inherited).toBeUndefined()
const response = await routerInstance.fetch(testReq)
const result = await response.json()
// Should only copy own properties, not inherited ones
expect(result.params.safe).toBe('good')
expect(result.params.inherited).toBeUndefined()

Copilot uses AI. Check for mistakes.
} catch (error) {
// If key generation fails, fallback to a default key
try {
const key = 'unknown'
Copy link

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

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

When key generation fails and a fallback key ('unknown') is used, consider logging the original error or using a more descriptive fallback, as this may lead to unintended rate limit collisions and make debugging harder.

Copilot uses AI. Check for mistakes.
@jkyberneees jkyberneees merged commit 56f14d2 into main Jun 11, 2025
5 checks passed
@jkyberneees jkyberneees deleted the introducing-middlewares branch June 11, 2025 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants