fix(api): restrict forwarded-header trust - #63
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe API now trusts forwarded client and protocol headers only from configured proxies or networks. Configuration parsing validates entries, rate-limit tests cover trusted and spoofed headers, and deployment documentation and defaults expose the new settings. ChangesForwarded-header trust
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Program
participant ForwardedHeadersMiddleware
participant RateLimiter
Client->>Program: Send request with X-Forwarded-For
Program->>ForwardedHeadersMiddleware: Apply configured trust policy
ForwardedHeadersMiddleware->>RateLimiter: Use trusted or direct connection address
RateLimiter-->>Client: Enforce rate-limit bucket
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ForwardedHeadersConfiguration.cs`:
- Around line 32-38: Update the KnownNetworks parsing loop in
ForwardedHeadersConfiguration to reject successfully parsed networks whose
prefix length is zero, including 0.0.0.0/0 and ::/0. Throw the same
configuration validation exception before adding such networks to
options.KnownIPNetworks, while preserving acceptance of non-wildcard valid CIDR
ranges.
🪄 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: 362d1a26-1b0d-4ffa-bb06-a385db6d3e0f
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.env.exampleBACKLOG.mdREADME.mdapi/ApplyTrack.Api.Tests/ConfigurationTests.csapi/ApplyTrack.Api.Tests/RateLimitTests.csapi/ApplyTrack.Api/ApplyTrack.Api.csprojapi/ApplyTrack.Api/ForwardedHeadersConfiguration.csapi/ApplyTrack.Api/Program.csapi/ApplyTrack.Api/appsettings.jsondocker-compose.ymlpyproject.toml
📜 Review details
🧰 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/Program.csapi/ApplyTrack.Api/ApplyTrack.Api.csprojapi/ApplyTrack.Api/ForwardedHeadersConfiguration.csapi/ApplyTrack.Api.Tests/ConfigurationTests.csapi/ApplyTrack.Api.Tests/RateLimitTests.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/Program.csapi/ApplyTrack.Api/ForwardedHeadersConfiguration.csapi/ApplyTrack.Api.Tests/ConfigurationTests.csapi/ApplyTrack.Api.Tests/RateLimitTests.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/Program.csapi/ApplyTrack.Api/ForwardedHeadersConfiguration.csapi/ApplyTrack.Api.Tests/ConfigurationTests.csapi/ApplyTrack.Api.Tests/RateLimitTests.cs
🔇 Additional comments (12)
api/ApplyTrack.Api/ForwardedHeadersConfiguration.cs (2)
16-30: LGTM!
43-47: LGTM!api/ApplyTrack.Api.Tests/ConfigurationTests.cs (1)
6-7: LGTM!Also applies to: 61-106
api/ApplyTrack.Api/Program.cs (1)
147-152: LGTM!api/ApplyTrack.Api.Tests/RateLimitTests.cs (1)
7-17: LGTM!Also applies to: 30-54, 107-159
api/ApplyTrack.Api/appsettings.json (1)
9-12: LGTM!docker-compose.yml (1)
48-51: LGTM!.env.example (1)
31-37: LGTM!README.md (1)
195-195: LGTM!Also applies to: 387-392
BACKLOG.md (1)
11-11: LGTM!api/ApplyTrack.Api/ApplyTrack.Api.csproj (1)
8-8: LGTM!pyproject.toml (1)
7-7: LGTM!
| foreach (var value in Values(configuration, "ForwardedHeaders:KnownNetworks")) | ||
| { | ||
| if (!System.Net.IPNetwork.TryParse(value, out var network)) | ||
| throw new InvalidOperationException( | ||
| $"ForwardedHeaders:KnownNetworks contains invalid CIDR network '{value}'."); | ||
| options.KnownIPNetworks.Add(network); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File outline ==\n'
ast-grep outline api/ApplyTrack.Api/ForwardedHeadersConfiguration.cs --view expanded || true
printf '\n== Relevant file contents ==\n'
cat -n api/ApplyTrack.Api/ForwardedHeadersConfiguration.cs | sed -n '1,220p'
printf '\n== Search for KnownNetworks / prefix-length handling ==\n'
rg -n "KnownNetworks|KnownIPNetworks|PrefixLength|0\.0\.0\.0/0|::/0|ForwardedHeaders" api README.md . -g '!**/bin/**' -g '!**/obj/**' || trueRepository: CryptoJones/OSApplyTrack
Length of output: 9213
Reject wildcard CIDR ranges in ForwardedHeaders:KnownNetworks. IPNetwork.TryParse accepts 0.0.0.0/0 and ::/0, which would trust every client as a proxy and reopen the spoofing hole this change is meant to close. Add an explicit zero-prefix check and fail fast.
🤖 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/ForwardedHeadersConfiguration.cs` around lines 32 - 38,
Update the KnownNetworks parsing loop in ForwardedHeadersConfiguration to reject
successfully parsed networks whose prefix length is zero, including 0.0.0.0/0
and ::/0. Throw the same configuration validation exception before adding such
networks to options.KnownIPNetworks, while preserving acceptance of non-wildcard
valid CIDR ranges.
1 participant
Summary
Validation
Closes #49
Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/