[limen BLD-promptscope-harden] promptscope: harden — Harden the main entry points: input validation, error handli#7
Open
4444J99 wants to merge 1 commit into
Open
Conversation
… error handli limen task BLD-promptscope-harden
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideAdds structured JSON logging, consistent error handling, and stricter input validation to all primary API entrypoints, and routes non-API paths directly to static assets while enforcing size limits and safer behavior around external calls and KV operations. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
readJsonObjecthelper only enforcesMAX_BODY_BYTESwhencontent-lengthis present and trustworthy, so very large bodies without that header can still be fully read into memory; consider adding a streaming size guard (e.g., manually reading the body and aborting after the limit) to make the limit robust. - For
methodNotAllowed, you expose the allowed methods in the JSON body but don’t set anAllowresponse header; adding that header would better match HTTP semantics and make it easier for some clients/tooling to adapt.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `readJsonObject` helper only enforces `MAX_BODY_BYTES` when `content-length` is present and trustworthy, so very large bodies without that header can still be fully read into memory; consider adding a streaming size guard (e.g., manually reading the body and aborting after the limit) to make the limit robust.
- For `methodNotAllowed`, you expose the allowed methods in the JSON body but don’t set an `Allow` response header; adding that header would better match HTTP semantics and make it easier for some clients/tooling to adapt.
## Individual Comments
### Comment 1
<location path="src/index.ts" line_range="166-167" />
<code_context>
+ });
+}
+
+function methodNotAllowed(ctx: RequestContext, allow: string): Response {
+ return jsonError(ctx, 'method not allowed', 405, { allow });
+}
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The 405 helper reports allowed methods in the JSON body but does not emit an HTTP `Allow` header.
To follow HTTP semantics and support more clients, also set an `Allow` response header (e.g., by letting `jsonError` accept extra headers or by adding the header on the `Response` returned from this helper).
Suggested implementation:
```typescript
function jsonError(
ctx: RequestContext,
error: string,
status: number,
extra: Record<string, unknown> = {},
extraHeaders: Record<string, string> = {},
): Response {
// request_id is additive — existing clients read `error`; new clients can
// quote request_id when reporting problems.
return Response.json({ error, request_id: ctx.reqId, ...extra }, {
status,
headers: { 'x-request-id': ctx.reqId, ...extraHeaders },
});
}
```
```typescript
function methodNotAllowed(ctx: RequestContext, allow: string): Response {
return jsonError(
ctx,
'method not allowed',
405,
{ allow },
{ Allow: allow },
);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+166
to
+167
| function methodNotAllowed(ctx: RequestContext, allow: string): Response { | ||
| return jsonError(ctx, 'method not allowed', 405, { allow }); |
There was a problem hiding this comment.
suggestion (bug_risk): The 405 helper reports allowed methods in the JSON body but does not emit an HTTP Allow header.
To follow HTTP semantics and support more clients, also set an Allow response header (e.g., by letting jsonError accept extra headers or by adding the header on the Response returned from this helper).
Suggested implementation:
function jsonError(
ctx: RequestContext,
error: string,
status: number,
extra: Record<string, unknown> = {},
extraHeaders: Record<string, string> = {},
): Response {
// request_id is additive — existing clients read `error`; new clients can
// quote request_id when reporting problems.
return Response.json({ error, request_id: ctx.reqId, ...extra }, {
status,
headers: { 'x-request-id': ctx.reqId, ...extraHeaders },
});
}function methodNotAllowed(ctx: RequestContext, allow: string): Response {
return jsonError(
ctx,
'method not allowed',
405,
{ allow },
{ Allow: allow },
);
}![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces structured logging, centralized request context, standardized error responses with trace IDs, and robust JSON body validation (including size limits) to a Cloudflare Workers API. The review feedback suggests three key improvements: ensuring RFC 7231 compliance by adding an Allow header to 405 Method Not Allowed responses, hardening body parsing against DoS/OOM by strictly validating Content-Type and Content-Length headers, and safely handling potentially immutable response headers when appending the trace ID to prevent runtime TypeErrors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+166
to
+168
| function methodNotAllowed(ctx: RequestContext, allow: string): Response { | ||
| return jsonError(ctx, 'method not allowed', 405, { allow }); | ||
| } |
There was a problem hiding this comment.
According to the HTTP specification (RFC 7231 Section 6.5.5), a 405 Method Not Allowed response must include an Allow header containing a list of the supported methods for the target resource. Currently, methodNotAllowed returns the list of allowed methods in the JSON body but does not set the Allow HTTP header.
function methodNotAllowed(ctx: RequestContext, allow: string): Response {
const resp = jsonError(ctx, 'method not allowed', 405, { allow });
resp.headers.set('Allow', allow);
return resp;
}
Comment on lines
+176
to
+181
| async function readJsonObject(req: Request, ctx: RequestContext): Promise<BodyResult> { | ||
| const contentLength = Number(req.headers.get('content-length')); | ||
| if (Number.isFinite(contentLength) && contentLength > MAX_BODY_BYTES) { | ||
| ctx.log('warn', 'body_too_large', { content_length: contentLength, limit: MAX_BODY_BYTES }); | ||
| return { ok: false, response: jsonError(ctx, 'request body too large', 413) }; | ||
| } |
There was a problem hiding this comment.
To robustly harden the API entry points against Denial of Service (DoS) and Out-of-Memory (OOM) attacks, we should strictly validate the incoming request headers before parsing the body:
- Validate
Content-Type: Ensure it isapplication/jsonto prevent parsing unexpected media types and mitigate potential CSRF vectors. - Validate
Content-Length: IfContent-Lengthis missing,Number(null)evaluates to0, which bypasses the size check entirely. Requiring and validating a finiteContent-Lengthensures that oversized payloads are rejected before they are read into memory.
async function readJsonObject(req: Request, ctx: RequestContext): Promise<BodyResult> {
const contentType = req.headers.get('content-type');
if (!contentType || !contentType.includes('application/json')) {
return { ok: false, response: jsonError(ctx, 'content-type must be application/json', 415) };
}
const contentLengthHeader = req.headers.get('content-length');
if (!contentLengthHeader) {
return { ok: false, response: jsonError(ctx, 'content-length header is required', 411) };
}
const contentLength = Number(contentLengthHeader);
if (!Number.isFinite(contentLength)) {
return { ok: false, response: jsonError(ctx, 'invalid content-length', 400) };
}
if (contentLength > MAX_BODY_BYTES) {
ctx.log('warn', 'body_too_large', { content_length: contentLength, limit: MAX_BODY_BYTES });
return { ok: false, response: jsonError(ctx, 'request body too large', 413) };
}
Comment on lines
+614
to
+616
| // Stamp every API response with the trace id. Redirects (3xx) carry an | ||
| // immutable Headers guard, so skip them to avoid throwing. | ||
| if (resp.status < 300 || resp.status >= 400) resp.headers.set('x-request-id', ctx.reqId); |
There was a problem hiding this comment.
Stamping the response headers directly using resp.headers.set can throw a runtime TypeError if the response headers are immutable (e.g., if a future API route returns a cached response or forwards a response directly from another fetch call). To ensure maximum resilience, wrap the header mutation in a try-catch block and fall back to reconstructing the response with the new headers if they are immutable.
// Stamp every API response with the trace id. Redirects (3xx) carry an
// immutable Headers guard, so skip them to avoid throwing.
if (resp.status < 300 || resp.status >= 400) {
try {
resp.headers.set('x-request-id', ctx.reqId);
} catch {
const newHeaders = new Headers(resp.headers);
newHeaders.set('x-request-id', ctx.reqId);
resp = new Response(resp.body, {
status: resp.status,
statusText: resp.statusText,
headers: newHeaders,
});
}
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Autonomous limen dispatch of task
BLD-promptscope-harden.Harden the main entry points: input validation, error handling, and structured logging on the primary request/CLI paths.
Produced in an isolated worktree off origin — review before merge.
Summary by Sourcery
Harden the PromptScope API entrypoints with stricter validation, structured logging, and more robust error handling across all primary request paths.
Enhancements: