-
Notifications
You must be signed in to change notification settings - Fork 48
feat: Add 1GB API request size limit via Content-Length validation #678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Implements API-level request size validation independent of model constraints. Both Run API and Manage API now enforce a configurable 1GB default limit through Content-Length header validation before body parsing. Core Changes: - Added payload_too_large error code (413) to error system - Created request-size-limit middleware with configurable maxRequestSizeBytes - Updated ServerOptions to support maxRequestSizeBytes configuration - Applied middleware to both agents-run-api and agents-manage-api - Updated OpenAPI schemas to document 413 error responses Testing: - Unit tests for middleware (18 tests) - Integration tests for Run API (9 tests) - Integration tests for Manage API (9 tests) - All tests passing Documentation: - Added comprehensive API reference docs at /docs/api-reference/request-limits - Includes configuration examples, best practices, and troubleshooting The middleware validates Content-Length headers before expensive body parsing, skips safe methods (GET/HEAD/OPTIONS), and returns proper Problem+JSON error responses when limits are exceeded. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @amikofalvy's task —— View job PR Review: Request Size Limit ImplementationReview Tasks
System Design & Architecture Review✅ Strong PointsExcellent Implementation Architecture:
Security & Performance:
🔍 Areas for Consideration1. CRITICAL: Transfer-Encoding Bypass VulnerabilityIssue: The middleware only validates // Current implementation vulnerability:
// POST /api/endpoint HTTP/1.1
// Transfer-Encoding: chunked
// (no Content-Length header)
// [unlimited chunked data can be sent]Recommendation: Add chunked transfer validation: // In request-size-limit.ts around line 62
const transferEncoding = c.req.header('transfer-encoding');
if (transferEncoding?.toLowerCase().includes('chunked')) {
// For chunked requests, we need to either:
// 1. Reject them outright for simplicity, or
// 2. Implement chunked request size tracking
logger.warn(
{ method, path: c.req.path },
'Chunked transfer encoding not supported - rejecting for size limit compliance'
);
throw createApiError({
code: 'bad_request',
message: 'Chunked transfer encoding not supported. Please use Content-Length header.',
});
}2. Configuration & Extensibility IssuesIssue: Hard-coded HTTP method skipping reduces flexibility: // Current: hard-coded method check
if (method === 'GET' || method === 'HEAD' || method === 'OPTIONS') {
return next();
}Recommendation: Make methods configurable: export interface RequestSizeLimitOptions {
maxRequestSizeBytes?: number;
skipMethods?: string[]; // Allow customization
}
const skipMethods = options.skipMethods ?? ['GET', 'HEAD', 'OPTIONS'];
if (skipMethods.includes(method)) {
return next();
}Issue: No per-route size limits capability. Recommendation: Consider future extensibility for per-endpoint limits: // Future: per-route configuration
export interface RequestSizeLimitOptions {
maxRequestSizeBytes?: number;
routeSpecificLimits?: Record<string, number>;
}3. Error Handling & MonitoringIssue: Limited observability for security monitoring. Recommendation: Enhanced logging for security analysis: // Around line 87, enhance logging:
logger.warn(
{
requestSize,
maxSize,
method,
path: c.req.path,
userAgent: c.req.header('user-agent'),
clientIp: c.req.header('x-forwarded-for') || c.req.header('x-real-ip'), // Add IP tracking
timestamp: new Date().toISOString(),
},
'Request payload size limit exceeded - potential abuse'
);4. Type Safety & Integration PatternsIssue: Minor type safety gap in ServerOptions integration. Current in Recommendation: Consider separating API-level vs model-level size constraints in the type system for clarity: export interface ServerOptions {
/** API-level request size limit (independent of model constraints) */
maxRequestSizeBytes?: number;
/** Future: model-specific constraints could go here */
// modelConstraints?: ModelSizeConstraints;
}5. Testing & Edge CasesMinor Testing Gap: Integration tests use Recommendation: Add tests against actual endpoints that consume request bodies: // Test against actual chat/agent endpoints that would process bodies
const res = await app.request('/v1/chat/completions', {
method: 'POST',
headers: {
'Content-Length': contentLength,
'Content-Type': 'application/json',
'Authorization': 'Bearer test-key'
},
body: JSON.stringify({ /* actual request body */ })
});6. Documentation & API DesignMissing: Client-side implementation guidance for handling 413 responses. Recommendation: Add to documentation: ## Client-Side Handling
```javascript
try {
const response = await fetch('/api/endpoint', {
method: 'POST',
body: largePayload
});
} catch (error) {
if (error.response?.status === 413) {
// Implement chunking, compression, or request splitting
console.log('Request too large, consider splitting into smaller requests');
}
} |
Summary
Implements API-level request size validation independent of model constraints. Both Run API and Manage API now enforce a configurable 1GB default limit through Content-Length header validation before body parsing.
Changes
Core Infrastructure
payload_too_largeerror code (413) to the core error systemmaxRequestSizeBytesmaxRequestSizeBytesconfiguration (default: 1GB)agents-run-apiandagents-manage-apiImplementation Details
Testing
Documentation
/docs/api-reference/request-limitsWhy 1GB?
The 1GB default is intentionally set high to be well above any model's context window requirements while still providing protection against abuse. This is purely an API-level limit independent of model constraints.
Configuration Example
Error Response Example
{ "title": "Payload Too Large", "status": 413, "detail": "Request payload size (1073741825 bytes) exceeds maximum allowed size of 1073741824 bytes", "code": "payload_too_large", "error": { "code": "payload_too_large", "message": "Request payload exceeds maximum allowed size" } }Related Files
packages/agents-core/src/middleware/request-size-limit.ts(new middleware)packages/agents-core/src/utils/error.ts(413 error code support)packages/agents-core/src/types/server.ts(ServerOptions configuration)agents-run-api/src/app.ts(middleware application)agents-manage-api/src/app.ts(middleware application)agents-docs/content/docs/api-reference/request-limits.mdx(documentation)🤖 Generated with Claude Code