test(server): the three remaining round-13 gaps (BUG-2803)

Codex round 13's other three findings, all of the same shape: a test
that would stay green with the production change reverted.

1. THE MANIFEST CHECK WAS UNTESTED. The bundle test built archives
containing only pad-export.json, so disabling the INDEPENDENT attachment-
manifest check left the suite green. The new test builds a bundle with
both entries, differing only in the manifest, so a refusal cannot come
from the export half. Verified by disabling each check separately: only
the matching test fails, so the two are independently covered.

2. THE TEST-EMAIL CHANGE HAD NO HANDLER-LEVEL TEST. Every existing leg
exercised decodeJSON, so reverting handlers_admin.go to default EVERY
decode failure to the admin's own address passed them all. The new test
drives the real endpoint with a wired mock sender and pins the
distinction that used to collapse: an ABSENT body still means "send it
to me" (control), an ordinary body still sends (control), and a body that
is present and refused answers 400 rather than being reinterpreted as
the default recipient.

3. THE MULTIPART LEG CHECKED ONE BYTE CLASS. A filter rejecting NULs
while letting malformed UTF-8 through would have passed it. It now drives
both, which matters because invalid UTF-8 is the class that reaches
Postgres as 22021 on a UTF8 database.

Round 13 was asked whether the new TESTS are sound — deterministic,
order-independent, and failing on broken code. It reported the fixtures
isolated and found five ways they were not discriminating. Two were
fixed in the previous commit; these are the rest.

Claude-Session: https://claude.ai/code/session_011T365kP1N9V88y15HxL4YN
This commit is contained in:
xarmian
2026-08-28 18:50:55 +00:00
parent 799a8bfd9e
commit 83831acb66
3 changed files with 126 additions and 0 deletions
+47
View File
@@ -850,3 +850,50 @@ func TestArtifactBindableTextAcceptsLiteralEscapeText(t *testing.T) {
}
}
}
// TestTestEmailRefusesUnusableBody is the handler-level leg codex round 13
// found missing. Reverting handlers_admin.go to `err != nil || input.To == ""`
// — which defaulted EVERY decode failure to the admin's own address — passed
// every other test in this file, because they only exercise decodeJSON.
//
// The distinction being pinned is between two things that used to collapse
// into one: an ABSENT body legitimately means "send it to me", while a body
// that is present and REFUSED must not be reinterpreted as that default.
func TestTestEmailRefusesUnusableBody(t *testing.T) {
srv := testServer(t)
token := bootstrapFirstUser(t, srv, "admin-mail@example.com", "Admin")
mock := mockMailerooEndpoint(t, http.StatusOK, true)
configureEmailForTest(srv, mock.URL, "http://localhost:7777")
post := func(t *testing.T, body string) *httptest.ResponseRecorder {
t.Helper()
var r io.Reader
if body != "" {
r = strings.NewReader(body)
}
req := httptest.NewRequest("POST", "/api/v1/admin/test-email", r)
if body != "" {
req.Header.Set("Content-Type", "application/json")
}
req.Header.Set("Authorization", "Bearer "+token)
req.RemoteAddr = "127.0.0.1:1234"
rr := httptest.NewRecorder()
srv.ServeHTTP(rr, req)
return rr
}
// Control 1: no body at all still means "send it to me".
if rr := post(t, ""); rr.Code != http.StatusOK {
t.Fatalf("absent body must still send, got %d: %s", rr.Code, rr.Body.String())
}
// Control 2: an ordinary body still works.
if rr := post(t, `{"to":"someone@example.com"}`); rr.Code != http.StatusOK {
t.Fatalf("ordinary body must send, got %d: %s", rr.Code, rr.Body.String())
}
// The case: a body that is valid JSON but carries an unusable value.
rr := post(t, `{"to":"a`+escNULLiteral+`b@example.com"}`)
if rr.Code != http.StatusBadRequest {
t.Errorf("a refused body must answer 400, not be reinterpreted as the default recipient; got %d: %s",
rr.Code, rr.Body.String())
}
}
@@ -992,6 +992,17 @@ func TestUpload_MultipartTextFieldsAreBindableText(t *testing.T) {
if control.Code != http.StatusCreated && control.Code != http.StatusOK {
t.Fatalf("control (no item_id) must succeed, got %d: %s", control.Code, control.Body.String())
}
// Both byte classes, not just the NUL: a filter that rejected NULs
// while letting malformed UTF-8 through would have passed the
// earlier version of this leg (codex round 13). Invalid UTF-8 is the
// class that reaches Postgres as 22021 on a UTF8 database.
for _, bad := range []string{"TASK-1\x00", "TASK-1\xff"} {
rr := upload(t, "clean.png", &bad)
if rr.Code != control.Code {
t.Errorf("an unusable item_id %q must be treated as no value (like the control, %d), got %d: %s",
bad, control.Code, rr.Code, rr.Body.String())
}
}
bad := "TASK-1\x00"
rr := upload(t, "clean.png", &bad)
if rr.Code != control.Code {
@@ -697,3 +697,71 @@ func TestImportBundle_RefusesNULInExport(t *testing.T) {
t.Errorf("the 400 should name the cause, got body=%s", got.Body.String())
}
}
// TestImportBundle_RefusesNULInManifest is the leg codex round 13 found
// missing: TestImportBundle_RefusesNULInExport builds bundles containing only
// pad-export.json, so removing the INDEPENDENT manifest check left the suite
// green. The manifest is a second JSON document parsed the same way and needs
// its own coverage.
//
// Both bundles carry a valid export; they differ only in the manifest, so a
// refusal cannot come from the export half.
func TestImportBundle_RefusesNULInManifest(t *testing.T) {
src, srcSlug := testServerWithAttachments(t)
rr := doRequest(src, "GET", "/api/v1/workspaces/"+srcSlug+"/export", nil)
if rr.Code != http.StatusOK {
t.Fatalf("export src: %d %s", rr.Code, rr.Body.String())
}
exportJSON := rr.Body.Bytes()
esc := string([]byte{'\\', 'u', '0', '0', '0', '0'})
cleanManifest := `{"version":1,"entries":[]}`
nulManifest := `{"version":1,"entries":[{"id":"00000000-0000-0000-0000-000000000001",` +
`"filename":"a` + esc + `b.png","mime":"image/png","size_bytes":1,"content_hash":"deadbeef"}]}`
bundle := func(t *testing.T, manifest string) []byte {
t.Helper()
var buf bytes.Buffer
gzw := gzip.NewWriter(&buf)
tw := tar.NewWriter(gzw)
for _, e := range []struct {
name string
body []byte
}{
{"pad-export.json", exportJSON},
{"attachments/manifest.json", []byte(manifest)},
} {
if err := tw.WriteHeader(&tar.Header{Name: e.name, Mode: 0o644, Size: int64(len(e.body))}); err != nil {
t.Fatalf("write header %s: %v", e.name, err)
}
if _, err := tw.Write(e.body); err != nil {
t.Fatalf("write %s: %v", e.name, err)
}
}
tw.Close()
gzw.Close()
return buf.Bytes()
}
post := func(t *testing.T, name string, body []byte) *httptest.ResponseRecorder {
t.Helper()
dest, _ := testServerWithAttachments(t)
req := httptest.NewRequest("POST", "/api/v1/workspaces/import?name="+name, bytes.NewReader(body))
req.Header.Set("Content-Type", "application/gzip")
req.RemoteAddr = "127.0.0.1:1234"
rec := httptest.NewRecorder()
dest.ServeHTTP(rec, req)
return rec
}
if got := post(t, "CleanManifest", bundle(t, cleanManifest)); got.Code != http.StatusOK && got.Code != http.StatusCreated {
t.Fatalf("control bundle must import, got %d: %s", got.Code, got.Body.String())
}
got := post(t, "NULManifest", bundle(t, nulManifest))
if got.Code < 400 {
t.Errorf("a manifest carrying a NUL escape must be refused, got %d: %s", got.Code, got.Body.String())
}
if !strings.Contains(got.Body.String(), "NUL") {
t.Errorf("the refusal should name the cause, got body=%s", got.Body.String())
}
}