harden: file delivery, submit metadata, client IP, status idempotency, answer validation#58
Merged
Merged
Conversation
…, answer validation Security & correctness hardening from the multi-agent review: - Serve uploaded files as `attachment` + `application/octet-stream` (never inline) so a user-uploaded HTML/SVG can't render same-origin; add explicit nosniff on the route too. - On submit, verify each file reference against object storage via a new `headObject` and record the server-stored size/MIME instead of trusting the client-supplied descriptor. - Take the LAST `X-Forwarded-For` hop (the one our trusted Apache proxy appends) so a client can't spoof its rate-limit identity; loosely validate the IP shape. - Add a shared, idempotent, race-safe `changeSubmissionStatus` helper in the db package (guarded `updateMany` + event + outbox in one transaction) and use it from both the web review route and the bot's Accept/Reject buttons. - Enforce the full `field.validation` contract server-side in `buildAnswerSchema` (min/max, lengths, pattern, email/url formats, option membership), not just `required`. Export `LAYOUT_FIELD_TYPES`/`isLayoutField`. Adds tests for the new client-IP and answer-validation behavior.
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.
Security & correctness hardening from the multi-agent code review (PR A of the follow-up cluster).
Changes
File delivery & upload metadata (High)
GET /api/files/[id]now servesattachment+application/octet-stream(+ explicitnosniff), neverinline— a user-uploaded HTML/SVG can no longer render same-origin.headObject()inlib/s3.ts. On submit, each file reference is verified to exist in object storage and the server-stored size/MIME is recorded — the client-supplied descriptor is no longer trusted.Client IP behind the proxy (Med)
clientIptakes the lastX-Forwarded-Forhop (the one our trusted Apache proxy appends) instead of the first, so a client can't spoof its rate-limit bucket. Loose IPv4/IPv6 shape check rejects junk.Idempotent status transitions (Med)
changeSubmissionStatushelper in@msk-forms/db: reads current status, writes guarded by that exact value (updateMany), and records the event + outbox notification in one transaction — race-safe and idempotent.Server-side answer validation (Med)
buildAnswerSchemanow enforces the fullfield.validationcontract: number min/max, string min/max length, regexpattern,email/urlformats, and choice-option membership — not justrequired.LAYOUT_FIELD_TYPES/isLayoutFieldfrom shared (consumed in follow-up dedup PR).Tests
client-iptests (spoofed left entry, IPv6, malformed last hop).buildAnswerSchematests covering every new constraint.Validation
pnpm typecheck,pnpm lint,pnpm test(32 tests),pnpm build— all green.