16 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | must_haves | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 01-data-integrity | 02 | execute | 2 |
|
|
true |
|
|
Bug 3 (DATA-03): WebhookHandler, TagsHandler POST branch, and TagAssignmentHandler PUT/DELETE branches decode JSON directly from r.Body with no size limit. A malicious or buggy DIUN installation could POST a multi-GB payload causing OOM. The fix applies http.MaxBytesReader before each decode and returns HTTP 413 when the limit is exceeded.
Bug 4 (DATA-04): Six test call sites use if err != nil { return } instead of t.Fatalf(...). When test setup fails (e.g., InitDB fails, UpdateEvent fails), the test silently exits with PASS, hiding the real failure from CI.
These two bugs are fixed in the same plan because they are independent of Plan 01's changes and both small enough to fit comfortably together.
Purpose: Webhook endpoint is safe from OOM attacks; test failures are always visible to the developer and CI.
Output: Updated diunwebhook.go with MaxBytesReader in three handlers; updated diunwebhook_test.go with t.Fatalf at 6 sites and 3 new 413 tests.
<execution_context> @$HOME/.claude/get-shit-done/workflows/execute-plan.md @$HOME/.claude/get-shit-done/templates/summary.md </execution_context>
@.planning/PROJECT.md @.planning/ROADMAP.md @.planning/STATE.md @.planning/phases/01-data-integrity/01-RESEARCH.md @.planning/phases/01-data-integrity/01-01-SUMMARY.md Task 1: Add request body size limits to WebhookHandler, TagsHandler, and TagAssignmentHandler pkg/diunwebhook/diunwebhook.go<read_first> - pkg/diunwebhook/diunwebhook.go — read the entire file before touching it; locate the exact lines for each handler's JSON decode call; the Plan 01 changes (UPSERT, PRAGMA) are already present — do not revert them </read_first>
- Test (new): POST /webhook with a body of 1MB + 1 byte returns HTTP 413 - Test (new): POST /api/tags with a body of 1MB + 1 byte returns HTTP 413 - Test (new): PUT /api/tag-assignments with a body of 1MB + 1 byte returns HTTP 413 - Test (existing): POST /webhook with valid JSON still returns HTTP 200 - Test (existing): POST /api/tags with valid JSON still returns HTTP 201 Make the following changes to pkg/diunwebhook/diunwebhook.go:CHANGE 1 — Add package-level constant after the import block, before the type declarations:
```go
const maxBodyBytes = 1 << 20 // 1 MB
```
CHANGE 2 — Add `"errors"` to the import block (it is not currently imported in diunwebhook.go; it is imported in the test file but not the production file).
The import block becomes:
```go
import (
"crypto/subtle"
"database/sql"
"encoding/json"
"errors"
"log"
"net/http"
"strconv"
"strings"
"sync"
"time"
_ "modernc.org/sqlite"
)
```
CHANGE 3 — In WebhookHandler, BEFORE the `var event DiunEvent` line (currently line ~177), add:
```go
r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes)
```
Then update the decode error handling block to distinguish 413 from 400:
```go
if err := json.NewDecoder(r.Body).Decode(&event); err != nil {
var maxBytesErr *http.MaxBytesError
if errors.As(err, &maxBytesErr) {
http.Error(w, "request body too large", http.StatusRequestEntityTooLarge)
return
}
log.Printf("WebhookHandler: failed to decode request: %v", err)
http.Error(w, "bad request", http.StatusBadRequest)
return
}
```
CHANGE 4 — In TagsHandler POST branch, BEFORE `var req struct { Name string }`, add:
```go
r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes)
```
Then update the decode error handling — the current code is:
```go
if err := json.NewDecoder(r.Body).Decode(&req); err != nil || req.Name == "" {
http.Error(w, "bad request: name required", http.StatusBadRequest)
return
}
```
Replace with:
```go
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
var maxBytesErr *http.MaxBytesError
if errors.As(err, &maxBytesErr) {
http.Error(w, "request body too large", http.StatusRequestEntityTooLarge)
return
}
http.Error(w, "bad request: name required", http.StatusBadRequest)
return
}
if req.Name == "" {
http.Error(w, "bad request: name required", http.StatusBadRequest)
return
}
```
(The `req.Name == ""` check must remain, now as a separate if-block after the decode succeeds.)
CHANGE 5 — In TagAssignmentHandler PUT branch, BEFORE `var req struct { Image string; TagID int }`, add:
```go
r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes)
```
Then update the decode error handling — the current code is:
```go
if err := json.NewDecoder(r.Body).Decode(&req); err != nil || req.Image == "" {
http.Error(w, "bad request", http.StatusBadRequest)
return
}
```
Replace with:
```go
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
var maxBytesErr *http.MaxBytesError
if errors.As(err, &maxBytesErr) {
http.Error(w, "request body too large", http.StatusRequestEntityTooLarge)
return
}
http.Error(w, "bad request", http.StatusBadRequest)
return
}
if req.Image == "" {
http.Error(w, "bad request", http.StatusBadRequest)
return
}
```
CHANGE 6 — In TagAssignmentHandler DELETE branch, BEFORE `var req struct { Image string }`, add:
```go
r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes)
```
Then update the decode error handling — the current code is:
```go
if err := json.NewDecoder(r.Body).Decode(&req); err != nil || req.Image == "" {
http.Error(w, "bad request", http.StatusBadRequest)
return
}
```
Replace with:
```go
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
var maxBytesErr *http.MaxBytesError
if errors.As(err, &maxBytesErr) {
http.Error(w, "request body too large", http.StatusRequestEntityTooLarge)
return
}
http.Error(w, "bad request", http.StatusBadRequest)
return
}
if req.Image == "" {
http.Error(w, "bad request", http.StatusBadRequest)
return
}
```
No other changes to diunwebhook.go in this task.
cd /home/jean-luc-makiola/Development/projects/DiunDashboard && go build ./pkg/diunwebhook/ && go test -v -run "TestWebhookHandler_BadRequest|TestCreateTagHandler_EmptyName|TestTagAssignmentHandler_Assign" ./pkg/diunwebhook/
- `grep -c "maxBodyBytes" pkg/diunwebhook/diunwebhook.go` outputs `5` (1 constant definition + 4 MaxBytesReader calls)
- `grep -c "MaxBytesReader" pkg/diunwebhook/diunwebhook.go` outputs `4`
- `grep -c "errors.As" pkg/diunwebhook/diunwebhook.go` outputs `4`
- `go build ./pkg/diunwebhook/` exits 0
- All pre-existing handler tests still pass
Task 2: Replace silent returns with t.Fatalf at 6 test setup call sites; add 3 oversized-body tests
pkg/diunwebhook/diunwebhook_test.go
<read_first>
- pkg/diunwebhook/diunwebhook_test.go — read the entire file; locate the exact 6 if err != nil { return } call sites at lines 38-40, 153-154, 228-231, 287-289, 329-331, 350-351 and verify they still exist after Plan 01 (which only appended to the file)
</read_first>
OLD (line ~38-40, in TestUpdateEventAndGetUpdates):
```go
err := diun.UpdateEvent(event)
if err != nil {
return
}
```
NEW:
```go
if err := diun.UpdateEvent(event); err != nil {
t.Fatalf("test setup: UpdateEvent failed: %v", err)
}
```
OLD (line ~153-154, in TestUpdatesHandler):
```go
err := diun.UpdateEvent(event)
if err != nil {
return
}
```
NEW:
```go
if err := diun.UpdateEvent(event); err != nil {
t.Fatalf("test setup: UpdateEvent failed: %v", err)
}
```
OLD (line ~228-231, in TestConcurrentUpdateEvent goroutine):
```go
err := diun.UpdateEvent(diun.DiunEvent{Image: fmt.Sprintf("image:%d", i)})
if err != nil {
return
}
```
NEW (note: in a goroutine, t.Fatalf is safe — testing.T.Fatalf calls runtime.Goexit which unwinds the goroutine cleanly):
```go
if err := diun.UpdateEvent(diun.DiunEvent{Image: fmt.Sprintf("image:%d", i)}); err != nil {
t.Fatalf("test setup: UpdateEvent[%d] failed: %v", i, err)
}
```
OLD (line ~287-289, in TestDismissHandler_Success):
```go
err := diun.UpdateEvent(diun.DiunEvent{Image: "nginx:latest"})
if err != nil {
return
}
```
NEW:
```go
if err := diun.UpdateEvent(diun.DiunEvent{Image: "nginx:latest"}); err != nil {
t.Fatalf("test setup: UpdateEvent failed: %v", err)
}
```
OLD (line ~329-331, in TestDismissHandler_SlashInImageName):
```go
err := diun.UpdateEvent(diun.DiunEvent{Image: "ghcr.io/user/image:tag"})
if err != nil {
return
}
```
NEW:
```go
if err := diun.UpdateEvent(diun.DiunEvent{Image: "ghcr.io/user/image:tag"}); err != nil {
t.Fatalf("test setup: UpdateEvent failed: %v", err)
}
```
OLD (line ~350-351, in TestDismissHandler_ReappearsAfterNewWebhook — note: line 350 is `diun.UpdateEvent(diun.DiunEvent{Image: "nginx:latest"})` with no error check at all):
```go
diun.UpdateEvent(diun.DiunEvent{Image: "nginx:latest"})
```
NEW:
```go
if err := diun.UpdateEvent(diun.DiunEvent{Image: "nginx:latest"}); err != nil {
t.Fatalf("test setup: UpdateEvent failed: %v", err)
}
```
CHANGE 2 — Add three new test functions after all existing tests (at the end of the file, after TestUpdateEvent_PreservesTagOnUpsert which was added in Plan 01):
```go
func TestWebhookHandler_OversizedBody(t *testing.T) {
// Generate a body that exceeds 1 MB (maxBodyBytes = 1<<20 = 1,048,576 bytes)
oversized := make([]byte, 1<<20+1)
for i := range oversized {
oversized[i] = 'x'
}
req := httptest.NewRequest(http.MethodPost, "/webhook", bytes.NewReader(oversized))
rec := httptest.NewRecorder()
diun.WebhookHandler(rec, req)
if rec.Code != http.StatusRequestEntityTooLarge {
t.Errorf("expected 413 for oversized body, got %d", rec.Code)
}
}
func TestTagsHandler_OversizedBody(t *testing.T) {
oversized := make([]byte, 1<<20+1)
for i := range oversized {
oversized[i] = 'x'
}
req := httptest.NewRequest(http.MethodPost, "/api/tags", bytes.NewReader(oversized))
rec := httptest.NewRecorder()
diun.TagsHandler(rec, req)
if rec.Code != http.StatusRequestEntityTooLarge {
t.Errorf("expected 413 for oversized body, got %d", rec.Code)
}
}
func TestTagAssignmentHandler_OversizedBody(t *testing.T) {
oversized := make([]byte, 1<<20+1)
for i := range oversized {
oversized[i] = 'x'
}
req := httptest.NewRequest(http.MethodPut, "/api/tag-assignments", bytes.NewReader(oversized))
rec := httptest.NewRecorder()
diun.TagAssignmentHandler(rec, req)
if rec.Code != http.StatusRequestEntityTooLarge {
t.Errorf("expected 413 for oversized body, got %d", rec.Code)
}
}
```
No new imports are needed — `bytes`, `net/http`, `net/http/httptest`, and `testing` are already imported.
cd /home/jean-luc-makiola/Development/projects/DiunDashboard && go test -v -run "TestWebhookHandler_OversizedBody|TestTagsHandler_OversizedBody|TestTagAssignmentHandler_OversizedBody" ./pkg/diunwebhook/
- `grep -c "if err != nil {" pkg/diunwebhook/diunwebhook_test.go` is reduced by 6 compared to before this task (the 6 setup-path returns are gone; other `if err != nil {` blocks with t.Fatal/t.Fatalf remain)
- `grep -c "return$" pkg/diunwebhook/diunwebhook_test.go` no longer contains bare `return` in error-check positions (the 6 silent returns are gone)
- TestWebhookHandler_OversizedBody passes (413)
- TestTagsHandler_OversizedBody passes (413)
- TestTagAssignmentHandler_OversizedBody passes (413)
- Full test suite passes: `go test ./pkg/diunwebhook/` exits 0
Run the full test suite after both tasks are complete:
cd /home/jean-luc-makiola/Development/projects/DiunDashboard && go test -v -coverprofile=coverage.out -coverpkg=./... ./...
Expected outcome:
- All tests pass (no regressions from Plan 01 or Plan 02)
- Three new 413 tests pass (proves DATA-03)
- Six
if err != nil { return }patterns replaced with t.Fatalf (proves DATA-04)
Spot-check the fixes:
grep -n "maxBodyBytes\|MaxBytesReader\|errors.As" pkg/diunwebhook/diunwebhook.go
grep -n "t.Fatalf" pkg/diunwebhook/diunwebhook_test.go | wc -l # should be >= 6 more than before
<success_criteria>
grep -c "MaxBytesReader" pkg/diunwebhook/diunwebhook.gooutputs4grep -c "maxBodyBytes" pkg/diunwebhook/diunwebhook.gooutputs5grep -c "StatusRequestEntityTooLarge" pkg/diunwebhook/diunwebhook.gooutputs4- TestWebhookHandler_OversizedBody, TestTagsHandler_OversizedBody, TestTagAssignmentHandler_OversizedBody all exist and pass
grep -c "if err != nil {$" pkg/diunwebhook/diunwebhook_test.gofollowed byreturnno longer appears at the 6 original sitesgo test -coverprofile=coverage.out -coverpkg=./... ./...exits 0 </success_criteria>