fix(core): clean error mapping + SSRF v6 hardening (review L1/L2/L3/L5)#118
Merged
Conversation
…SRF v6 (review L1/L2/L3/L5) Four confirmed-Low findings from the release review, all in @limner/core. L1 — ProjectStore.create (D1 + local): the handler's getByName pre-check is TOCTOU; a concurrent same-name create raced the INSERT and surfaced a raw "D1_ERROR: UNIQUE constraint failed". create() now catches the UNIQUE violation and maps it to the clean "project already exists: <name>" message (the same the pre-check produces). Protects MCP + CMA. L2 — ProjectStore.addNote (D1 + local): the F5 get()-before-INSERT guard is likewise TOCTOU against a concurrent project delete. The INSERT is now wrapped to map a FOREIGN KEY violation to the clean projectNotFound() message instead of the raw error. L3 — _http.ts httpResponseToError embedded up to 500 chars of the upstream (OpenAI/Recraft) response body in the PipelineError message, which reaches the MCP client verbatim (third-party text that can echo request fragments/ids). The body is now logged server-side (console.warn) only; the client gets a clean status-coded message. Corrected the stale server.ts comment that claimed these messages were already sanitized. L5 — _url-guard.ts isBlockedIpv6 only blocked an embedded private IPv4 in the IPv4-mapped (::ffff:) form. Extended to the deprecated IPv4-compatible (::a.b.c.d) and NAT64 (64:ff9b::/96) forms too, so a private/loopback v4 embedded in any of the three is rejected. (Low real-world impact on Workers egress, but the shared core also runs in the Node/MCPB host.) Tests: create maps duplicate-name to a clean error on both backends; NAT64 + IPv4-compatible private literals rejected by the SSRF guard. Suite 695 -> 700. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Jim Vinson <jim@vinson.org>
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.
Batch C2 of the review fixes — the four remaining confirmed-Low findings, all in
@limner/core. This closes every confirmed finding from the review.ProjectStore.create(D1 + local) now catches theUNIQUEviolation from a raced duplicate-name INSERT → cleanproject already exists: <name>(the handler'sgetByNamepre-check is TOCTOU). Protects MCP + CMA.ProjectStore.addNotewraps the INSERT to map aFOREIGN KEYviolation (concurrent project delete after the F5 guard) → cleanprojectNotFound().httpResponseToErrorno longer embeds the raw upstream (OpenAI/Recraft) body in the client-facingPipelineErrormessage — it'sconsole.warn'd server-side; the client gets a clean status-coded message. Corrected the staleserver.tscomment that claimed these were already sanitized.isBlockedIpv6now blocks an embedded private/loopback IPv4 across the IPv4-compatible (::a.b.c.d) and NAT64 (64:ff9b::/96) forms, not just IPv4-mapped (::ffff:).Tests
createmaps duplicate-name to a clean error on both backends; NAT64 + IPv4-compatible private literals rejected by the SSRF guard. Suite 695 → 700. Typecheck + lint clean.🤖 Generated with Claude Code