fix(dashboard): enforce POST body size limits

[fallintoplace]

• Add request body size limits to all dashboard POST endpoints.
• Reject payloads over 64 KiB for action endpoints and 1 MiB for /refine-comment with HTTP 413.
• Close readBody event streams safely with data length checks and error handling.
• Add regression tests for default limit and refine-comment limit.

[Copilot]

Pull request overview

This PR adds server-side request body size enforcement for dashboard POST endpoints, returning HTTP 413 with a JSON error payload when limits are exceeded. It also adds regression tests to ensure oversized requests are rejected for both the default POST limit and the larger /refine-comment limit.

Changes:

• Add per-endpoint POST body byte limits (64 KiB default; 1 MiB for /refine-comment) and a standardized 413 JSON response.
• Update POST handlers to use the new bounded body reader and short-circuit on oversized payloads.
• Add tests covering 413 responses for oversized payloads on default endpoints and /refine-comment.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
packages/apm-contributor-dashboard/.apm/extensions/issue-monitor/server-handler.mjs	Implements byte-limited request body reading and wires 413 handling into POST endpoints.
packages/apm-contributor-dashboard/tests/server.test.mjs	Adds regression tests asserting 413 behavior for oversized POST request bodies.

[sergio-sisternes-epam]

apm-review-panel synthesis for PR #2029 -- fix(dashboard): enforce POST body size limits

---

TL;DR: Well-scoped security fix that covers all 10 POST endpoints with correct HTTP 413 enforcement. One blocking stream-safety bug (req.destroy() after removing the error listener), two warnings (sentinel returns, copy-paste blocks), two infos (req mutation, test gaps).

---

Architecture lens

The implementation is pure Node.js http.IncomingMessage stream handling. The two-layer design (readBody handles streams; readBodyWithLimit handles routing and sentinel returns) is clean and testable.

---

Security / Supply-chain lens

The fix closes a denial-of-service surface: the old readBody accumulated the entire stream into memory with no bounds, so a malicious or misconfigured client could exhaust process heap. The new implementation is an improvement.

Blocking concern: req.destroy() is called AFTER cleanup() removes all event listeners. Node.js IncomingMessage.destroy() can emit error on the underlying socket. An uncaught error event in Node.js terminates the process. This is a real crash vector in production.

Fix suggestion:

cleanup();
req.once("error", () => {}); // swallow destroy-induced socket error
req.destroy();
reject(createPayloadTooLargeError());

Alternatively, prefer req.resume() over req.destroy() -- it drains the socket gracefully without emitting errors, which is safer for HTTP keep-alive connections.

---

Test coverage lens

New tests: 2 cases (oversized default, oversized refine-comment). Coverage gaps:

1. No test for the isBodyReadError (500) path.
2. No boundary test at exactly limit bytes (the off-by-one on bytes > limit vs bytes >= limit is untested).
3. All other 8 endpoints untested for 413 -- acceptable given shared readBodyWithLimit.

---

Code quality lens

Warning (DRY): The same 7-line sentinel guard block is copy-pasted 9 times. A small helper reduces maintenance surface:

function handleBodyError(raw, res) {
    if (raw?.isPayloadTooLarge) { sendPayloadTooLarge(res); return true; }
    if (raw?.isBodyReadError) { sendBodyReadError(res, raw.error); return true; }
    return false;
}
// In each handler:
if (handleBodyError(raw, res)) return;

Warning (sentinel pattern): readBodyWithLimit catches errors and returns sentinel objects instead of throwing. If any future handler omits the guard checks, JSON.parse({ isPayloadTooLarge: true }) will silently produce a 400 instead of 413. Prefer throwing and catching at each handler site.

Info: req.maxBytes mutation is a non-standard side-channel -- passing maxBytes as a direct argument to readBody(req, maxBytes) would be cleaner.

---

Panel recommendation: REQUEST CHANGES

Block on the req.destroy() / error listener ordering bug. Warn on sentinel pattern and duplication. All other findings are advisory.

[sergio-sisternes-epam]

The panel identified one blocking issue before this can merge: the req.destroy() call after cleanup() removes the error listener is a real crash vector in Node.js (unhandled error event terminates the process). Fix: add req.once('error', () => {}) before req.destroy(), or switch to req.resume() to drain gracefully. All other findings are advisory. Happy to re-review once this is addressed.

[fallintoplace]

Let me have a look and push the commit.

[sergio-sisternes-epam]

Let me have a look and push the commit.

@fallintoplace rebase also from main, as the PR is now out-of-date