fix(daemon): reject non-object JSON request bodies

[Torch4084]

Summary

• Validate parsed daemon HTTP request bodies are non-null JSON objects
• Return 400 for null and array payloads instead of letting null requests hang before reaching the handler
• Add regression coverage for non-object payloads

Verification

• pnpm -F @accomplish/daemon test:unit -- http-server-factory
• pnpm -F @accomplish/daemon typecheck
• pnpm exec prettier --check apps/daemon/src/http-server-factory.ts apps/daemon/__tests__/unit/http-server-factory.test.ts
• pnpm lint:eslint (passes with existing upstream warnings in apps/web/src/client/pages/execution/useExecutionEffects.ts, addressed separately in fix(web): simplify execution effect dependencies #981)
• pnpm format:check

Summary by CodeRabbit

• Bug Fixes

  • Enhanced request body validation to strictly require JSON objects, rejecting null values and arrays with appropriate error handling.

• Tests

  • Added unit tests verifying proper validation and error responses for invalid JSON request body formats.

[coderabbitai]

📝 Walkthrough

Walkthrough

The HTTP server factory's JSON parsing middleware now validates request body shape, ensuring parsed JSON is a non-null object rather than null or array. A test verifies this validation by sending invalid payloads and confirming the expected 400 error response and route handler bypass.

Changes

JSON Body Validation

Layer / File(s)	Summary
JSON body object validation apps/daemon/src/http-server-factory.ts, apps/daemon/__tests__/unit/http-server-factory.test.ts	parseJsonBody parses JSON into an intermediate unknown variable and validates the result is a non-null, non-array object before returning it as Record<string, unknown>, responding with HTTP 400 and "Request body must be a JSON object" for invalid shapes. The test covers both null and array payloads, verifies handler non-invocation, and ensures cleanup.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A humble validator hops into place,
Checking JSON objects with careful grace.
No nulls, no arrays shall pass the gate—
Just proper objects pass the test's debate! 🎯

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(daemon): reject non-object JSON request bodies' directly and accurately describes the main change: adding validation to reject non-object JSON payloads in the daemon's HTTP server.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

apps/daemon/__tests__/unit/http-server-factory.test.ts (1)

170-171: ⚡ Quick win

Consider adding a primitive test case for comprehensive coverage.

The test currently validates null and array payloads. The implementation also rejects primitives (strings, numbers, booleans) via the typeof data !== 'object' check on line 72 of http-server-factory.ts. Adding at least one primitive case would ensure all validation branches are tested.

📋 Example: Add a string primitive test

 try {
   const nullBody = await makeRequest(port, '/ping', 'POST', 'null', 'secret-token');
   const arrayBody = await makeRequest(port, '/ping', 'POST', '[]', 'secret-token');
+  const stringBody = await makeRequest(port, '/ping', 'POST', '"hello"', 'secret-token');

   expect(nullBody.statusCode).toBe(400);
   expect(JSON.parse(nullBody.body).error).toBe('Request body must be a JSON object');
   expect(arrayBody.statusCode).toBe(400);
   expect(JSON.parse(arrayBody.body).error).toBe('Request body must be a JSON object');
+  expect(stringBody.statusCode).toBe(400);
+  expect(JSON.parse(stringBody.body).error).toBe('Request body must be a JSON object');
   expect(routeHandler).not.toHaveBeenCalled();

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/daemon/__tests__/unit/http-server-factory.test.ts` around lines 170 -
171, Add a primitive payload test to cover the branch in http-server-factory.ts
that rejects non-object bodies: extend the existing test that calls makeRequest
for '/ping' to include a primitive case (e.g., send a string like '"hello"' or a
number/boolean) using makeRequest(port, '/ping', 'POST', '<primitive>',
'secret-token') and assert the expected rejection (HTTP 400 or the same response
as the null/array cases). This will exercise the typeof data !== 'object'
validation path in http-server-factory.ts and ensure primitives are tested
alongside null and arrays.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@apps/daemon/__tests__/unit/http-server-factory.test.ts`:
- Around line 170-171: Add a primitive payload test to cover the branch in
http-server-factory.ts that rejects non-object bodies: extend the existing test
that calls makeRequest for '/ping' to include a primitive case (e.g., send a
string like '"hello"' or a number/boolean) using makeRequest(port, '/ping',
'POST', '<primitive>', 'secret-token') and assert the expected rejection (HTTP
400 or the same response as the null/array cases). This will exercise the typeof
data !== 'object' validation path in http-server-factory.ts and ensure
primitives are tested alongside null and arrays.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c57d02fc-aa62-4873-9f16-bf49533a273f

📥 Commits

Reviewing files that changed from the base of the PR and between b3140a3 and 4529954.

📒 Files selected for processing (2)

• apps/daemon/__tests__/unit/http-server-factory.test.ts
• apps/daemon/src/http-server-factory.ts