docs(01-02): complete body-size-limits and test-hardening plan
- 01-02-SUMMARY.md: plan completion summary with deviations - STATE.md: advanced plan position, added decisions, updated metrics - ROADMAP.md: phase 01 marked complete (2/2 plans) - REQUIREMENTS.md: DATA-03 and DATA-04 marked complete
This commit is contained in:
@@ -11,8 +11,8 @@ Requirements for this milestone. Each maps to roadmap phases.
|
|||||||
|
|
||||||
- [x] **DATA-01**: Webhook events use proper UPSERT (ON CONFLICT DO UPDATE) instead of INSERT OR REPLACE, preserving tag assignments when an image receives a new event
|
- [x] **DATA-01**: Webhook events use proper UPSERT (ON CONFLICT DO UPDATE) instead of INSERT OR REPLACE, preserving tag assignments when an image receives a new event
|
||||||
- [x] **DATA-02**: SQLite foreign key enforcement is enabled (PRAGMA foreign_keys = ON) so tag deletion properly cascades to tag assignments
|
- [x] **DATA-02**: SQLite foreign key enforcement is enabled (PRAGMA foreign_keys = ON) so tag deletion properly cascades to tag assignments
|
||||||
- [ ] **DATA-03**: Webhook and API endpoints enforce request body size limits (e.g., 1MB) to prevent OOM from oversized payloads
|
- [x] **DATA-03**: Webhook and API endpoints enforce request body size limits (e.g., 1MB) to prevent OOM from oversized payloads
|
||||||
- [ ] **DATA-04**: Test error handling uses t.Fatal instead of silent returns, so test failures are never swallowed
|
- [x] **DATA-04**: Test error handling uses t.Fatal instead of silent returns, so test failures are never swallowed
|
||||||
|
|
||||||
### Backend Refactor
|
### Backend Refactor
|
||||||
|
|
||||||
@@ -93,8 +93,8 @@ Which phases cover which requirements. Updated during roadmap creation.
|
|||||||
|-------------|-------|--------|
|
|-------------|-------|--------|
|
||||||
| DATA-01 | Phase 1 | Complete |
|
| DATA-01 | Phase 1 | Complete |
|
||||||
| DATA-02 | Phase 1 | Complete |
|
| DATA-02 | Phase 1 | Complete |
|
||||||
| DATA-03 | Phase 1 | Pending |
|
| DATA-03 | Phase 1 | Complete |
|
||||||
| DATA-04 | Phase 1 | Pending |
|
| DATA-04 | Phase 1 | Complete |
|
||||||
| REFAC-01 | Phase 2 | Pending |
|
| REFAC-01 | Phase 2 | Pending |
|
||||||
| REFAC-02 | Phase 2 | Pending |
|
| REFAC-02 | Phase 2 | Pending |
|
||||||
| REFAC-03 | Phase 2 | Pending |
|
| REFAC-03 | Phase 2 | Pending |
|
||||||
|
|||||||
@@ -32,7 +32,7 @@ Decimal phases appear between their surrounding integers in numeric order.
|
|||||||
|
|
||||||
Plans:
|
Plans:
|
||||||
- [x] 01-01-PLAN.md — Fix INSERT OR REPLACE → UPSERT in UpdateEvent(); enable PRAGMA foreign_keys = ON in InitDB(); add regression test
|
- [x] 01-01-PLAN.md — Fix INSERT OR REPLACE → UPSERT in UpdateEvent(); enable PRAGMA foreign_keys = ON in InitDB(); add regression test
|
||||||
- [ ] 01-02-PLAN.md — Add http.MaxBytesReader body limits to 3 handlers (413 on oversized); replace 6 silent test returns with t.Fatalf
|
- [x] 01-02-PLAN.md — Add http.MaxBytesReader body limits to 3 handlers (413 on oversized); replace 6 silent test returns with t.Fatalf
|
||||||
|
|
||||||
### Phase 2: Backend Refactor
|
### Phase 2: Backend Refactor
|
||||||
**Goal**: The codebase has a clean Store interface and Server struct so the SQLite implementation can be swapped without touching HTTP handlers, enabling parallel test execution and PostgreSQL support
|
**Goal**: The codebase has a clean Store interface and Server struct so the SQLite implementation can be swapped without touching HTTP handlers, enabling parallel test execution and PostgreSQL support
|
||||||
|
|||||||
@@ -2,14 +2,14 @@
|
|||||||
gsd_state_version: 1.0
|
gsd_state_version: 1.0
|
||||||
milestone: v1.0
|
milestone: v1.0
|
||||||
milestone_name: milestone
|
milestone_name: milestone
|
||||||
status: Ready to execute
|
status: Phase complete — ready for verification
|
||||||
stopped_at: Completed 01-01-PLAN.md (UPSERT + FK enforcement)
|
stopped_at: Completed 01-02-PLAN.md (body size limits + test hardening)
|
||||||
last_updated: "2026-03-23T20:16:27.362Z"
|
last_updated: "2026-03-23T20:25:50.296Z"
|
||||||
progress:
|
progress:
|
||||||
total_phases: 4
|
total_phases: 4
|
||||||
completed_phases: 0
|
completed_phases: 1
|
||||||
total_plans: 2
|
total_plans: 2
|
||||||
completed_plans: 1
|
completed_plans: 2
|
||||||
---
|
---
|
||||||
|
|
||||||
# Project State
|
# Project State
|
||||||
@@ -47,6 +47,7 @@ Plan: 2 of 2
|
|||||||
|
|
||||||
*Updated after each plan completion*
|
*Updated after each plan completion*
|
||||||
| Phase 01 P01 | 2 | 2 tasks | 2 files |
|
| Phase 01 P01 | 2 | 2 tasks | 2 files |
|
||||||
|
| Phase 01-data-integrity P02 | 7 | 2 tasks | 2 files |
|
||||||
|
|
||||||
## Accumulated Context
|
## Accumulated Context
|
||||||
|
|
||||||
@@ -61,6 +62,8 @@ Recent decisions affecting current work:
|
|||||||
- `DATABASE_URL` present activates PostgreSQL; absent falls back to SQLite with `DB_PATH` — no separate `DB_DRIVER` variable
|
- `DATABASE_URL` present activates PostgreSQL; absent falls back to SQLite with `DB_PATH` — no separate `DB_DRIVER` variable
|
||||||
- [Phase 01]: Use named-column UPSERT (ON CONFLICT DO UPDATE) to preserve tag_assignments child rows on re-insert
|
- [Phase 01]: Use named-column UPSERT (ON CONFLICT DO UPDATE) to preserve tag_assignments child rows on re-insert
|
||||||
- [Phase 01]: Enable PRAGMA foreign_keys = ON in InitDB() before DDL to activate ON DELETE CASCADE for tag deletion
|
- [Phase 01]: Enable PRAGMA foreign_keys = ON in InitDB() before DDL to activate ON DELETE CASCADE for tag deletion
|
||||||
|
- [Phase 01-data-integrity]: Use MaxBytesReader + errors.As(*http.MaxBytesError) per-handler (not middleware) for request body size limiting — consistent with no-middleware architecture
|
||||||
|
- [Phase 01-data-integrity]: Oversized body tests need valid JSON prefix so decoder reads past 1MB limit; all-x bytes fail at byte 1 before MaxBytesReader triggers
|
||||||
|
|
||||||
### Pending Todos
|
### Pending Todos
|
||||||
|
|
||||||
@@ -73,6 +76,6 @@ None yet.
|
|||||||
|
|
||||||
## Session Continuity
|
## Session Continuity
|
||||||
|
|
||||||
Last session: 2026-03-23T20:16:27.360Z
|
Last session: 2026-03-23T20:25:50.294Z
|
||||||
Stopped at: Completed 01-01-PLAN.md (UPSERT + FK enforcement)
|
Stopped at: Completed 01-02-PLAN.md (body size limits + test hardening)
|
||||||
Resume file: None
|
Resume file: None
|
||||||
|
|||||||
127
.planning/phases/01-data-integrity/01-02-SUMMARY.md
Normal file
127
.planning/phases/01-data-integrity/01-02-SUMMARY.md
Normal file
@@ -0,0 +1,127 @@
|
|||||||
|
---
|
||||||
|
phase: 01-data-integrity
|
||||||
|
plan: 02
|
||||||
|
subsystem: api
|
||||||
|
tags: [go, http, security, testing, maxbytesreader, body-size-limit]
|
||||||
|
|
||||||
|
# Dependency graph
|
||||||
|
requires:
|
||||||
|
- phase: 01-data-integrity plan 01
|
||||||
|
provides: UPSERT fix and FK enforcement already applied; test file structure established
|
||||||
|
|
||||||
|
provides:
|
||||||
|
- HTTP 413 response for oversized request bodies (>1MB) on WebhookHandler, TagsHandler POST, TagAssignmentHandler PUT/DELETE
|
||||||
|
- maxBodyBytes constant (1 << 20) and MaxBytesReader + errors.As pattern
|
||||||
|
- t.Fatalf at all 6 test setup call sites (no more silent test pass-on-setup-failure)
|
||||||
|
- 3 new oversized-body tests proving DATA-03 fixed
|
||||||
|
|
||||||
|
affects:
|
||||||
|
- phase-02 (database refactor — handlers are now correct and hardened, test suite is reliable)
|
||||||
|
- any future handler additions that accept a body (pattern established)
|
||||||
|
|
||||||
|
# Tech tracking
|
||||||
|
tech-stack:
|
||||||
|
added: []
|
||||||
|
patterns:
|
||||||
|
- "MaxBytesReader + errors.As(*http.MaxBytesError) pattern for request body size limiting in handlers"
|
||||||
|
- "JSON-prefix oversized body test: use valid JSON opening so decoder reads past limit before MaxBytesReader triggers"
|
||||||
|
|
||||||
|
key-files:
|
||||||
|
created: []
|
||||||
|
modified:
|
||||||
|
- pkg/diunwebhook/diunwebhook.go
|
||||||
|
- pkg/diunwebhook/diunwebhook_test.go
|
||||||
|
|
||||||
|
key-decisions:
|
||||||
|
- "Use MaxBytesReader wrapping r.Body before each JSON decode; distinguish 413 from 400 via errors.As on *http.MaxBytesError"
|
||||||
|
- "Oversized body test bodies must use valid JSON prefix (e.g. {\"image\":\") + padding — all-x bodies trigger JSON parse error before MaxBytesReader limit is reached"
|
||||||
|
|
||||||
|
patterns-established:
|
||||||
|
- "MaxBytesReader body guard pattern: r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes) before decode, errors.As for 413 vs 400"
|
||||||
|
- "Test setup errors must use t.Fatalf, never silent return"
|
||||||
|
|
||||||
|
requirements-completed:
|
||||||
|
- DATA-03
|
||||||
|
- DATA-04
|
||||||
|
|
||||||
|
# Metrics
|
||||||
|
duration: 7min
|
||||||
|
completed: 2026-03-23
|
||||||
|
---
|
||||||
|
|
||||||
|
# Phase 01 Plan 02: Body Size Limits and Test Setup Hardening Summary
|
||||||
|
|
||||||
|
**Request body size limits (1MB cap, HTTP 413) added to four handler paths; six silent test-setup returns replaced with t.Fatalf to surface setup failures in CI**
|
||||||
|
|
||||||
|
## Performance
|
||||||
|
|
||||||
|
- **Duration:** 7 min
|
||||||
|
- **Started:** 2026-03-23T20:17:30Z
|
||||||
|
- **Completed:** 2026-03-23T20:24:37Z
|
||||||
|
- **Tasks:** 2
|
||||||
|
- **Files modified:** 2
|
||||||
|
|
||||||
|
## Accomplishments
|
||||||
|
|
||||||
|
- Added `maxBodyBytes` constant and `errors` import to `diunwebhook.go`; applied `http.MaxBytesReader` + `errors.As(*http.MaxBytesError)` guard before JSON decode in WebhookHandler, TagsHandler POST, TagAssignmentHandler PUT and DELETE — returns HTTP 413 on body > 1MB
|
||||||
|
- Replaced 6 silent `if err != nil { return }` test setup patterns with `t.Fatalf(...)` so failing setup always fails the test, not silently passes
|
||||||
|
- Added 3 new oversized-body tests (TestWebhookHandler_OversizedBody, TestTagsHandler_OversizedBody, TestTagAssignmentHandler_OversizedBody); all pass with 413
|
||||||
|
|
||||||
|
## Task Commits
|
||||||
|
|
||||||
|
Each task was committed atomically:
|
||||||
|
|
||||||
|
1. **RED: add failing 413 tests** - `311e91d` (test)
|
||||||
|
2. **Task 1: MaxBytesReader in handlers + GREEN test fix** - `98dfd76` (feat)
|
||||||
|
3. **Task 2: Replace silent returns with t.Fatalf** - `7bdfc5f` (fix)
|
||||||
|
|
||||||
|
**Plan metadata:** (docs commit — see below)
|
||||||
|
|
||||||
|
_Note: TDD task 1 has a RED commit followed by a combined feat commit covering the implementation and the test body correction._
|
||||||
|
|
||||||
|
## Files Created/Modified
|
||||||
|
|
||||||
|
- `pkg/diunwebhook/diunwebhook.go` — added `errors` import, `maxBodyBytes` constant, MaxBytesReader guards in 4 handler paths
|
||||||
|
- `pkg/diunwebhook/diunwebhook_test.go` — 3 new oversized-body tests; 6 t.Fatalf replacements
|
||||||
|
|
||||||
|
## Decisions Made
|
||||||
|
|
||||||
|
- `http.MaxBytesReader` is applied per-handler (not via middleware) to match the existing no-middleware architecture
|
||||||
|
- Body limit set at 1MB (`1 << 20`) matching the plan spec
|
||||||
|
- Oversized body test bodies use a valid JSON prefix (`{"image":"` + padding) rather than all-`x` bytes — the JSON decoder reads only 1 byte of invalid content before failing, so all-`x` never triggers MaxBytesReader; a JSON string value causes the decoder to read the full field before the limit fires
|
||||||
|
|
||||||
|
## Deviations from Plan
|
||||||
|
|
||||||
|
### Auto-fixed Issues
|
||||||
|
|
||||||
|
**1. [Rule 1 - Bug] Oversized body tests used all-x bytes; fixed to use valid JSON prefix**
|
||||||
|
- **Found during:** Task 1 GREEN phase
|
||||||
|
- **Issue:** Test body `make([]byte, 1<<20+1)` filled with `'x'` causes JSON decoder to fail at byte 1 with "invalid character" — MaxBytesReader never triggers because the read count never reaches the limit
|
||||||
|
- **Fix:** Changed test bodies to `{"image":"` (or `{"name":"`) + `bytes.Repeat([]byte("x"), 1<<20+1)` so the decoder reads past 1MB before encountering an unterminated string
|
||||||
|
- **Files modified:** pkg/diunwebhook/diunwebhook_test.go
|
||||||
|
- **Verification:** All 3 oversized-body tests now pass with HTTP 413
|
||||||
|
- **Committed in:** 98dfd76 (Task 1 feat commit)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
**Total deviations:** 1 auto-fixed (Rule 1 - test bug)
|
||||||
|
**Impact on plan:** The fix is necessary for tests to validate what they claim. No scope creep; the handler implementation is exactly as specified.
|
||||||
|
|
||||||
|
## Issues Encountered
|
||||||
|
|
||||||
|
None beyond the test body deviation documented above.
|
||||||
|
|
||||||
|
## User Setup Required
|
||||||
|
|
||||||
|
None - no external service configuration required.
|
||||||
|
|
||||||
|
## Next Phase Readiness
|
||||||
|
|
||||||
|
- DATA-03 and DATA-04 fixed; all Phase 01 plans complete
|
||||||
|
- Full test suite passes with 0 failures
|
||||||
|
- Handler hardening pattern (MaxBytesReader + errors.As) established for future handlers
|
||||||
|
- Ready to transition to Phase 02 (database refactor / PostgreSQL support)
|
||||||
|
|
||||||
|
---
|
||||||
|
*Phase: 01-data-integrity*
|
||||||
|
*Completed: 2026-03-23*
|
||||||
Reference in New Issue
Block a user