Cap progress request bodies and stored module ID lists (MM-70622)

[esethna]

Summary

• Cap every plugin HTTP body at 64 KiB and return 413 when MaxBytesReader trips, so progress PUTs cannot decode an unbounded payload into memory.
• Reject completedModuleIds / moduleIds over 256 entries or IDs that fail the same character-set and length rules as guide IDs.
• Refuse to persist a merged progress record over 1024 module IDs (400) instead of growing the KV value until the ~8 MB ceiling.

Test plan

• Complete a real guide module and confirm progress still saves (normal payloads are a few hundred bytes).
• PUT a body larger than 64 KiB to /plugins/com.mattermost.academy/api/v1/progress/{guideId} and confirm 413.
• PUT more than 256 module IDs, or an ID like ../x / AI, and confirm 400.
• Confirm an existing record at 1024 IDs rejects a new unique ID with 400 and does not grow.

Made with Cursor

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 316f25c7-9e1a-4f8d-9670-15c2df85c152

📥 Commits

Reviewing files that changed from the base of the PR and between 424a2b2 and ea62161.

📒 Files selected for processing (1)

• server/progress/http.go

Limit details: You’ve used the included review currently available.

---

📝 Walkthrough

Walkthrough

The change adds bounded request-body reads and module-ID validation for progress updates. HTTP handlers return client errors for oversized or invalid payloads. Storage rejects merged records above the completed-module limit and preserves existing records.

Changes

Progress limits

Layer / File(s)	Summary
Request and module validation server/progress/ids.go, server/progress/ids_test.go	Adds request-size, module-ID count, and stored-module limits. Validates module-ID format, trimming, and count boundaries.
HTTP request enforcement server/plugin.go, server/progress/http.go, server/progress/http_test.go, server/router_test.go	Limits request bodies before routing. Progress updates return 413 for oversized bodies and 400 for read, JSON, or validation errors.
Atomic stored-module limits server/progress/store.go, server/progress/store_test.go, server/progress/http.go	Rejects merged completed-module lists above the storage limit and keeps the existing record unchanged.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ServeHTTP
  participant ProgressHTTP
  participant Store
  Client->>ServeHTTP: Send progress update
  ServeHTTP->>ProgressHTTP: Forward size-limited request body
  ProgressHTTP->>ProgressHTTP: Read and validate payload
  ProgressHTTP->>Store: Save validated progress
  Store-->>ProgressHTTP: Return persistence result
  ProgressHTTP-->>Client: Return HTTP response

Loading

Merge Risk: ⚪ Minimal · up to e9e91

Progress updates now reject oversized bodies, invalid or excessive module IDs, and merged records above the storage cap while preserving existing records on rejection. The supplied coverage supports the new client-error behavior, with no outstanding merge-readiness risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 8 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes: progress request-body limits and stored module ID limits.
Description check	✅ Passed	The description directly explains the request-body, module ID, persistence, and test changes in the pull request.
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.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch MM-70622-cap-progress-request-size

---

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

[esarafianou]

@esethna Can we please have a dev review done as well?