fix: deduplicate validation logic and guard JSON parse #27

Merged

forgejo_admin merged 1 commit from 5-fix-duplicated-validation-unguarded-json into main 2026-03-27 22:00:16 +00:00

Conversation 1 Commits 1 Files changed 2 +23 -15

forgejo_admin commented 2026-03-27 21:57:21 +00:00

Owner

Copy link

Summary

• Removes duplicated inline validation from the sign endpoint, replacing it with the tested validateSignRequest from validation.ts
• Wraps request.json() in try/catch so malformed JSON returns 400 instead of 500
• Adds 2 new test cases for malformed/non-object request bodies

Changes

• src/routes/contract/[token]/sign/+server.ts: Removed 12 lines of inline validation (required-field check, type guard, PNG prefix check, size check) and replaced with single validateSignRequest() import from $lib/validation. Wrapped request.json() in try/catch to return 400 on malformed JSON.
• tests/validation.test.ts: Added test for undefined body (malformed JSON parse result) and non-object primitives (string, number, boolean). Total tests: 14.

Test Plan

• Tests pass locally (npm test -- 14 passed)
• Manual verification: validateSignRequest(undefined) returns { valid: false, error: 'Invalid request body' }
• No regressions -- all 12 existing tests still pass

Review Checklist

• Passed automated review-fix loop
• No secrets committed
• No unnecessary file changes
• Commit messages are descriptive

Related Notes

• Closes #5
• project-westside-basketball -- the project this work belongs to

## Summary - Removes duplicated inline validation from the sign endpoint, replacing it with the tested `validateSignRequest` from `validation.ts` - Wraps `request.json()` in try/catch so malformed JSON returns 400 instead of 500 - Adds 2 new test cases for malformed/non-object request bodies ## Changes - `src/routes/contract/[token]/sign/+server.ts`: Removed 12 lines of inline validation (required-field check, type guard, PNG prefix check, size check) and replaced with single `validateSignRequest()` import from `$lib/validation`. Wrapped `request.json()` in try/catch to return 400 on malformed JSON. - `tests/validation.test.ts`: Added test for `undefined` body (malformed JSON parse result) and non-object primitives (string, number, boolean). Total tests: 14. ## Test Plan - [x] Tests pass locally (`npm test` -- 14 passed) - [x] Manual verification: `validateSignRequest(undefined)` returns `{ valid: false, error: 'Invalid request body' }` - [x] No regressions -- all 12 existing tests still pass ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - Closes #5 - `project-westside-basketball` -- the project this work belongs to

forgejo_admin added 1 commit 2026-03-27 21:57:21 +00:00

fix: deduplicate validation logic and guard JSON parse in sign endpoint ... 32b80e2638

Replace inline validation in +server.ts with imported validateSignRequest
from validation.ts (single source of truth). Wrap request.json() in
try/catch so malformed JSON returns 400 instead of 500. Add tests for
undefined and non-object bodies.

Closes #5

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-03-27 21:58:48 +00:00

Author

Owner

Copy link

QA Review -- PR #27

Scope Check

All 5 acceptance criteria from issue #5 are addressed:

• Sign endpoint imports validateSignRequest from validation.ts
• Inline validation removed from +server.ts (12 lines removed)
• Malformed JSON returns 400 (try/catch around request.json())
• New tests added for malformed JSON body returning 400 (2 new test cases)
• Existing 12 tests pass (14 total now)

Code Review

+server.ts changes

• validateSignRequest import added correctly from $lib/validation
• try/catch around request.json() is correct -- SvelteKit's error() returns never, so TypeScript knows body is always assigned after the try/catch block
• The validation.error ?? 'Invalid request' fallback is defensive but fine -- validateSignRequest always sets error when valid is false
• No other code in the file was touched -- minimal diff, surgical change

Test additions

• undefined body test correctly validates the malformed JSON parse path
• Non-object primitives (string, number, boolean) cover edge cases where JSON.parse succeeds but returns a non-object
• Both tests assert on the correct error message

Potential Concerns (non-blocking)

• The SignRequest type import is still used for the let body: SignRequest declaration, which is fine -- it provides type narrowing after validation, even though validateSignRequest accepts unknown
• Pre-existing svelte-check errors (12 total, all @types/pg / @types/node missing) are unrelated to this PR

Test Results

• 14/14 tests pass
• No new TypeScript errors introduced

VERDICT: APPROVE

## QA Review -- PR #27 ### Scope Check All 5 acceptance criteria from issue #5 are addressed: - [x] Sign endpoint imports `validateSignRequest` from validation.ts - [x] Inline validation removed from +server.ts (12 lines removed) - [x] Malformed JSON returns 400 (try/catch around `request.json()`) - [x] New tests added for malformed JSON body returning 400 (2 new test cases) - [x] Existing 12 tests pass (14 total now) ### Code Review **+server.ts changes** - `validateSignRequest` import added correctly from `$lib/validation` - try/catch around `request.json()` is correct -- SvelteKit's `error()` returns `never`, so TypeScript knows `body` is always assigned after the try/catch block - The `validation.error ?? 'Invalid request'` fallback is defensive but fine -- `validateSignRequest` always sets `error` when `valid` is false - No other code in the file was touched -- minimal diff, surgical change **Test additions** - `undefined` body test correctly validates the malformed JSON parse path - Non-object primitives (string, number, boolean) cover edge cases where `JSON.parse` succeeds but returns a non-object - Both tests assert on the correct error message ### Potential Concerns (non-blocking) - The `SignRequest` type import is still used for the `let body: SignRequest` declaration, which is fine -- it provides type narrowing after validation, even though `validateSignRequest` accepts `unknown` - Pre-existing svelte-check errors (12 total, all `@types/pg` / `@types/node` missing) are unrelated to this PR ### Test Results - 14/14 tests pass - No new TypeScript errors introduced **VERDICT: APPROVE**

forgejo_admin merged commit adae3247bf into main 2026-03-27 22:00:16 +00:00

forgejo_admin deleted branch 5-fix-duplicated-validation-unguarded-json 2026-03-27 22:00:16 +00:00

forgejo_admin referenced this pull request from a commit 2026-03-27 22:00:17 +00:00

fix: deduplicate validation logic and guard JSON parse (#27)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

forgejo_admin/westside-contracts!27

No description provided.