fix(api): bound JSON and persisted input sizes - #64
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a 1 MiB JSON mutation body cap, shared field-length and collection-cardinality validation, endpoint integration, authenticated boundary tests, documentation, and version updates. ChangesBounded API input
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant JsonBodyLimitMiddleware
participant APIEndpoint
participant InputLimits
Client->>JsonBodyLimitMiddleware: Submit JSON mutation
JsonBodyLimitMiddleware->>JsonBodyLimitMiddleware: Enforce 1 MiB body cap
JsonBodyLimitMiddleware->>APIEndpoint: Forward accepted request
APIEndpoint->>InputLimits: Validate fields and collections
alt input exceeds configured limit
InputLimits-->>APIEndpoint: AppValidationException
APIEndpoint-->>Client: HTTP 400 with detail
else input is valid
APIEndpoint-->>Client: Process request
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@api/ApplyTrack.Api.Tests/InputLimitTests.cs`:
- Around line 60-222: Add endpoint-level over-limit tests to this suite for POST
/api/blacklist and the relevant /api/llm-settings operation, targeting the
validators in BlacklistEndpoints and MaterialsEndpoints. Build payloads
exceeding each endpoint’s configured limits, assert HTTP 400 responses, and
verify the response detail identifies the rejected field or limit.
In `@api/ApplyTrack.Api/Middleware/JsonBodyLimitMiddleware.cs`:
- Around line 50-51: Update the JSON content-type check in
JsonBodyLimitMiddleware to use request.HasJsonContentType() and also recognize
media types whose subtype ends with +json, ensuring structured JSON requests
remain subject to the 1 MiB limit.
In `@BACKLOG.md`:
- Line 12: Update the `#50` entry in BACKLOG.md from checked to unchecked status,
keeping the existing issue link and description unchanged until the linked issue
is closed.
In `@README.md`:
- Around line 383-386: Update the “Bounded API input” documentation in README.md
to explicitly state that requests exceeding the 1 MiB JSON body limit return
HTTP 413, while field-length and collection-cardinality validation errors return
HTTP 400.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a1903065-05ed-479b-80a7-3a33ce0426ae
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
BACKLOG.mdREADME.mdapi/ApplyTrack.Api.Tests/InputLimitTests.csapi/ApplyTrack.Api/ApplyTrack.Api.csprojapi/ApplyTrack.Api/Data/AppFields.csapi/ApplyTrack.Api/Data/ApplicationRepo.csapi/ApplyTrack.Api/Data/Criteria.csapi/ApplyTrack.Api/Data/InputLimits.csapi/ApplyTrack.Api/Data/Resume.csapi/ApplyTrack.Api/Endpoints/BlacklistEndpoints.csapi/ApplyTrack.Api/Endpoints/MaterialsEndpoints.csapi/ApplyTrack.Api/Middleware/JsonBodyLimitMiddleware.csapi/ApplyTrack.Api/Program.cspyproject.toml
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: .NET — test + audit
- GitHub Check: Web — WCAG checks
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cs,csproj,sln,py,js,html,css,sql}
📄 CodeRabbit inference engine (CLAUDE.md)
Every source file must carry
SPDX-License-Identifier: Apache-2.0andCopyright 2026 Aaron K. Clark. The project remains Apache-2.0 licensed.
Files:
api/ApplyTrack.Api/Data/Criteria.csapi/ApplyTrack.Api/Program.csapi/ApplyTrack.Api/ApplyTrack.Api.csprojapi/ApplyTrack.Api/Data/Resume.csapi/ApplyTrack.Api/Endpoints/BlacklistEndpoints.csapi/ApplyTrack.Api/Data/AppFields.csapi/ApplyTrack.Api/Data/ApplicationRepo.csapi/ApplyTrack.Api/Endpoints/MaterialsEndpoints.csapi/ApplyTrack.Api/Data/InputLimits.csapi/ApplyTrack.Api/Middleware/JsonBodyLimitMiddleware.csapi/ApplyTrack.Api.Tests/InputLimitTests.cs
api/**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
api/**/*.cs: The core API must use .NET 10 with ASP.NET Core Minimal APIs running on Kestrel.
Use Dapper and Npgsql for data access over PostgreSQL; do not use Entity Framework Core or SQLAlchemy.
Implement optimistic locking with a manualversioncolumn.
The cover-letter engine must support any OpenAI-compatible endpoint, never hard-code a vendor, require a specific model, or require a paid key.
Cover-letter drafting is opt-out per tenant viacover_letters_enabled; disabled tenants must receive no drafting UI and trigger no LLM calls.
The .NET API owns authentication, sessions, and CRUD operations.
Files:
api/ApplyTrack.Api/Data/Criteria.csapi/ApplyTrack.Api/Program.csapi/ApplyTrack.Api/Data/Resume.csapi/ApplyTrack.Api/Endpoints/BlacklistEndpoints.csapi/ApplyTrack.Api/Data/AppFields.csapi/ApplyTrack.Api/Data/ApplicationRepo.csapi/ApplyTrack.Api/Endpoints/MaterialsEndpoints.csapi/ApplyTrack.Api/Data/InputLimits.csapi/ApplyTrack.Api/Middleware/JsonBodyLimitMiddleware.csapi/ApplyTrack.Api.Tests/InputLimitTests.cs
{api/**/*.cs,src/applytrack/**/*.py}
📄 CodeRabbit inference engine (CLAUDE.md)
The .NET API and Python poller must share PostgreSQL tables without calling each other. Every query in both runtimes must unconditionally filter with
WHERE tenant_id.
Files:
api/ApplyTrack.Api/Data/Criteria.csapi/ApplyTrack.Api/Program.csapi/ApplyTrack.Api/Data/Resume.csapi/ApplyTrack.Api/Endpoints/BlacklistEndpoints.csapi/ApplyTrack.Api/Data/AppFields.csapi/ApplyTrack.Api/Data/ApplicationRepo.csapi/ApplyTrack.Api/Endpoints/MaterialsEndpoints.csapi/ApplyTrack.Api/Data/InputLimits.csapi/ApplyTrack.Api/Middleware/JsonBodyLimitMiddleware.csapi/ApplyTrack.Api.Tests/InputLimitTests.cs
🔇 Additional comments (10)
api/ApplyTrack.Api/Program.cs (1)
195-198: LGTM!api/ApplyTrack.Api/Data/InputLimits.cs (1)
1-164: LGTM!api/ApplyTrack.Api/Data/ApplicationRepo.cs (1)
236-240: LGTM!api/ApplyTrack.Api/ApplyTrack.Api.csproj (1)
8-8: LGTM!pyproject.toml (1)
7-7: LGTM!api/ApplyTrack.Api/Data/AppFields.cs (1)
42-61: LGTM!api/ApplyTrack.Api/Data/Criteria.cs (1)
64-64: LGTM!api/ApplyTrack.Api/Data/Resume.cs (1)
42-42: LGTM!api/ApplyTrack.Api/Endpoints/BlacklistEndpoints.cs (1)
28-28: LGTM!api/ApplyTrack.Api/Endpoints/MaterialsEndpoints.cs (1)
88-98: LGTM!Also applies to: 112-114
| [Fact] | ||
| public async Task Json_body_accepts_the_exact_global_limit() | ||
| { | ||
| var response = await _client.PostAsync( | ||
| "/api/poll", SizedJson((int)JsonBodyLimitMiddleware.MaxBytes)); | ||
| Assert.Equal(HttpStatusCode.OK, response.StatusCode); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task Json_body_rejects_one_byte_over_with_detail() | ||
| { | ||
| var response = await _client.PostAsync( | ||
| "/api/poll", SizedJson((int)JsonBodyLimitMiddleware.MaxBytes + 1)); | ||
|
|
||
| Assert.Equal(HttpStatusCode.RequestEntityTooLarge, response.StatusCode); | ||
| Assert.Contains("1024 KiB", await Detail(response)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task Application_accepts_field_and_notes_boundaries() | ||
| { | ||
| var response = await _client.PostAsync("/api/apps", Json(new | ||
| { | ||
| company = new string('c', InputLimits.Company), | ||
| role = "Engineer", | ||
| notes = new string('n', InputLimits.Notes), | ||
| })); | ||
|
|
||
| Assert.Equal(HttpStatusCode.Created, response.StatusCode); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData("company")] | ||
| [InlineData("notes")] | ||
| public async Task Application_rejects_overlong_fields_with_detail(string field) | ||
| { | ||
| var payload = field == "company" | ||
| ? new { company = new string('c', InputLimits.Company + 1), notes = "" } | ||
| : new { company = "Acme", notes = new string('n', InputLimits.Notes + 1) }; | ||
|
|
||
| var response = await _client.PostAsync("/api/apps", Json(payload)); | ||
|
|
||
| Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); | ||
| Assert.Contains(field, await Detail(response)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task Criteria_accepts_collection_boundaries() | ||
| { | ||
| var response = await _client.PutAsync("/api/criteria", Json(new | ||
| { | ||
| keywords = Enumerable.Range(0, InputLimits.Keywords).Select(i => $"keyword-{i}"), | ||
| exclude_locations = Enumerable.Range(0, InputLimits.ExcludedLocations) | ||
| .Select(i => $"location-{i}"), | ||
| ats_boards = Enumerable.Range(0, InputLimits.AtsBoards) | ||
| .Select(i => new { provider = "greenhouse", slug = $"company-{i}" }), | ||
| })); | ||
|
|
||
| Assert.Equal(HttpStatusCode.OK, response.StatusCode); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData("keywords")] | ||
| [InlineData("exclude_locations")] | ||
| [InlineData("ats_boards")] | ||
| public async Task Criteria_rejects_over_cardinality_with_detail(string field) | ||
| { | ||
| object payload = field switch | ||
| { | ||
| "keywords" => new | ||
| { | ||
| keywords = Enumerable.Range(0, InputLimits.Keywords + 1) | ||
| .Select(i => $"keyword-{i}"), | ||
| }, | ||
| "exclude_locations" => new | ||
| { | ||
| exclude_locations = Enumerable.Range(0, InputLimits.ExcludedLocations + 1) | ||
| .Select(i => $"location-{i}"), | ||
| }, | ||
| _ => new | ||
| { | ||
| ats_boards = Enumerable.Range(0, InputLimits.AtsBoards + 1) | ||
| .Select(i => new { provider = "lever", slug = $"company-{i}" }), | ||
| }, | ||
| }; | ||
|
|
||
| var response = await _client.PutAsync("/api/criteria", Json(payload)); | ||
|
|
||
| Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); | ||
| Assert.Contains(field, await Detail(response)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task Resume_accepts_collection_boundaries() | ||
| { | ||
| var response = await _client.PutAsync("/api/resume", Json(new | ||
| { | ||
| experience = Enumerable.Range(0, InputLimits.ResumeExperience).Select(i => new | ||
| { | ||
| company = $"Company {i}", | ||
| title = "Engineer", | ||
| highlights = Enumerable.Range(0, InputLimits.ResumeHighlights) | ||
| .Select(h => $"Highlight {h}"), | ||
| }), | ||
| skills = Enumerable.Range(0, InputLimits.ResumeSkills).Select(i => $"Skill {i}"), | ||
| certifications = Enumerable.Range(0, InputLimits.ResumeCertifications) | ||
| .Select(i => $"Certification {i}"), | ||
| links = Enumerable.Range(0, InputLimits.ResumeLinks) | ||
| .Select(i => new { label = $"Link {i}", url = $"https://example.com/{i}" }), | ||
| })); | ||
|
|
||
| Assert.Equal(HttpStatusCode.OK, response.StatusCode); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData("experience")] | ||
| [InlineData("highlights")] | ||
| [InlineData("skills")] | ||
| [InlineData("certifications")] | ||
| [InlineData("links")] | ||
| public async Task Resume_rejects_over_cardinality_with_detail(string field) | ||
| { | ||
| object payload = field switch | ||
| { | ||
| "experience" => new | ||
| { | ||
| experience = Enumerable.Range(0, InputLimits.ResumeExperience + 1) | ||
| .Select(i => new { company = $"Company {i}" }), | ||
| }, | ||
| "highlights" => new | ||
| { | ||
| experience = new[] | ||
| { | ||
| new | ||
| { | ||
| company = "Acme", | ||
| highlights = Enumerable.Range(0, InputLimits.ResumeHighlights + 1) | ||
| .Select(i => $"Highlight {i}"), | ||
| }, | ||
| }, | ||
| }, | ||
| "skills" => new | ||
| { | ||
| skills = Enumerable.Range(0, InputLimits.ResumeSkills + 1) | ||
| .Select(i => $"Skill {i}"), | ||
| }, | ||
| "certifications" => new | ||
| { | ||
| certifications = Enumerable.Range(0, InputLimits.ResumeCertifications + 1) | ||
| .Select(i => $"Certification {i}"), | ||
| }, | ||
| _ => new | ||
| { | ||
| links = Enumerable.Range(0, InputLimits.ResumeLinks + 1) | ||
| .Select(i => new { url = $"https://example.com/{i}" }), | ||
| }, | ||
| }; | ||
|
|
||
| var response = await _client.PutAsync("/api/resume", Json(payload)); | ||
|
|
||
| Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); | ||
| Assert.Contains(field, await Detail(response)); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Cover the endpoint-only limit checks.
Add over-limit HTTP tests for /api/blacklist and /api/llm-settings; this suite currently does not exercise the validators added in BlacklistEndpoints and MaterialsEndpoints, including their expected 400 detail responses.
🤖 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 `@api/ApplyTrack.Api.Tests/InputLimitTests.cs` around lines 60 - 222, Add
endpoint-level over-limit tests to this suite for POST /api/blacklist and the
relevant /api/llm-settings operation, targeting the validators in
BlacklistEndpoints and MaterialsEndpoints. Build payloads exceeding each
endpoint’s configured limits, assert HTTP 400 responses, and verify the response
detail identifies the rejected field or limit.
| && request.ContentType?.StartsWith( | ||
| "application/json", StringComparison.OrdinalIgnoreCase) == true; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For ASP.NET Core .NET 10, does HttpRequest.HasJsonContentType recognize application/vnd.example+json as JSON?
💡 Result:
Yes, in ASP.NET Core.NET 10, the HttpRequest.HasJsonContentType extension method recognizes application/vnd.example+json as a JSON content type [1][2][3]. The implementation of HasJsonContentType checks for the standard application/json media type and additionally validates if the media type suffix is "json" [2][3]. Because the suffix of application/vnd.example+json is "json", the method returns true [2][3]. Specifically, the source code performs the following check [2][3]: if (mt.Suffix.Equals("json", StringComparison.OrdinalIgnoreCase)) { return true; } This behavior ensures compatibility with RFC 6839, which defines the +json suffix as a structured-syntax marker [4].
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.httprequestjsonextensions.hasjsoncontenttype?view=aspnetcore-10.0
- 2: https://github.com/dotnet/aspnetcore/blob/a450cb69b5e4549f5515cdb057a68771f56cefd7/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs
- 3: https://github.com/dotnet/aspnetcore/blob/9b4be9e0/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs
- 4: audit: +json suffix content-type detection (RFC 6839) productdevbook/misina#58
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the middleware and any JSON content-type checks/tests.
git ls-files 'api/ApplyTrack.Api/Middleware/JsonBodyLimitMiddleware.cs' 'api/**' | sed -n '1,200p'
printf '\n--- file outline ---\n'
ast-grep outline api/ApplyTrack.Api/Middleware/JsonBodyLimitMiddleware.cs --view expanded || true
printf '\n--- search for HasJsonContentType / ContentType checks ---\n'
rg -n "HasJsonContentType|StartsWith\\(\"application/json\"|ContentType\\?" api -g '*.cs' || true
printf '\n--- relevant file excerpt ---\n'
cat -n api/ApplyTrack.Api/Middleware/JsonBodyLimitMiddleware.cs | sed -n '1,140p'Repository: CryptoJones/OSApplyTrack
Length of output: 9430
Match JSON suffix types here. StartsWith("application/json") skips structured JSON media types like application/vnd.applytrack+json, so a client can bypass the 1 MiB cap. Use request.HasJsonContentType() and add a +json test.
🤖 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 `@api/ApplyTrack.Api/Middleware/JsonBodyLimitMiddleware.cs` around lines 50 -
51, Update the JSON content-type check in JsonBodyLimitMiddleware to use
request.HasJsonContentType() and also recognize media types whose subtype ends
with +json, ensuring structured JSON requests remain subject to the 1 MiB limit.
| - [x] [#51 — Close the Python poller link-check DNS rebinding gap](https://github.com/CryptoJones/OSApplyTrack/issues/51) | ||
| - [x] [#49 — Restrict forwarded-header trust to configured proxies](https://github.com/CryptoJones/OSApplyTrack/issues/49) | ||
| - [ ] [#50 — Add global JSON body caps and per-field/cardinality limits](https://github.com/CryptoJones/OSApplyTrack/issues/50) | ||
| - [x] [#50 — Add global JSON body caps and per-field/cardinality limits](https://github.com/CryptoJones/OSApplyTrack/issues/50) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep #50 unchecked until the issue is closed.
The linked issue is currently open, but line 12 marks it complete, contrary to this file’s stated rule. (github.com)
🤖 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 `@BACKLOG.md` at line 12, Update the `#50` entry in BACKLOG.md from checked to
unchecked status, keeping the existing issue link and description unchanged
until the linked issue is closed.
| - **Bounded API input.** Ordinary JSON mutations are capped at 1 MiB, application | ||
| strings/notes have explicit length ceilings, and criteria/résumé collections | ||
| reject excessive cardinality with a clear `400` instead of growing rows or LLM | ||
| prompts without limit. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the 413 body-limit response.
The 1 MiB JSON cap returns 413, while field and collection validation returns 400. State this distinction here so API consumers can handle oversized bodies correctly.
🤖 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 `@README.md` around lines 383 - 386, Update the “Bounded API input”
documentation in README.md to explicitly state that requests exceeding the 1 MiB
JSON body limit return HTTP 413, while field-length and collection-cardinality
validation errors return HTTP 400.
Summary
Validation
Closes #50
Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/