Skip to content

fix(job-analysis): bound pre-authentication request metadata - #88

Open
seonghobae wants to merge 6 commits into
developfrom
fix/job-analysis-http-request-budgets
Open

fix(job-analysis): bound pre-authentication request metadata#88
seonghobae wants to merge 6 commits into
developfrom
fix/job-analysis-http-request-budgets

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Verified protected-main defect

Protected develop@9e3e4847510e1e612b48474ba42b177b8ed824df parsed attacker-controlled Job Analysis API request metadata before bearer authentication without application-owned budgets. _typed_headers(...) iterated/lower-cased/stored an unbounded ASGI header sequence, and route recognition split a route-shaped path without an application path bound.

This is an Orgmetra-owned transport availability boundary. No Keyverse or other dedicated-writer repository is changed.

RED first

Exact RED head 48d938d2c751f7dddafdcb6d4dd10fbfa57d874d added only the header-budget regression. Job-Analysis API Quality run 32601527276, job 97100416229, checked out that exact SHA and failed only the two new budget tests: excessive header count and excessive aggregate header bytes were both accepted. The other 69 tests passed and existing owned production remained at 100% statement/branch coverage, isolating the causal boundary to missing request-metadata budgets.

The regression was then strengthened for an oversized route-shaped path before the final repair.

Root-cause repair

Current exact head: 0dc4f09cc3c87829ea1e3a0e3dc0188df07ad8cd.

  • reject paths longer than 256 characters before route splitting/UUID parsing;
  • reject more than 64 ASGI header frames before normalization;
  • reject more than 16 KiB aggregate header-name/value bytes before lower-casing/dictionary allocation;
  • preserve existing duplicate/type/header authentication behavior, 1 MiB body bound, authorization, tenant isolation, and persistence contracts;
  • correct buyer-facing README truth: GET uses Authorization + X-Purpose-Code, accepts no query parameters, and now documents request-metadata budgets.

These limits align with Orgmetra's already-reviewed People HTTP transport budgets rather than weakening a gateway or relying on a foreign dependency.

Exact-current-head GREEN evidence

All applicable hosted workflows are terminal GREEN on unchanged head 0dc4f09cc3c87829ea1e3a0e3dc0188df07ad8cd:

  • Job-Analysis API Quality 32601723497 — unit job checks out the exact head; 72 tests passed; owned production totals 603 statements / 214 branches at exact 100% statement and branch coverage; PostgreSQL integration also passed.
  • Foundation CI 32601723498 — terminal success, including repository validation and PostgreSQL integrity matrix.
  • Security Scan 32601723512 — terminal success.
  • SAST Semgrep 32601723524 — terminal success.
  • Recovery Rehearsal Quality 32601723530 — terminal success.

Fresh Devin review is COMMENTED only. Its sole unresolved thread is explicitly informational: _typed_headers currently performs the same already-bounded header normalization twice per accepted request. With the new 64-frame/16 KiB caps this is bounded redundant work, not an authentication/authorization/privacy/data-integrity bypass or a gate failure, so it is intentionally not marked resolved and does not justify another behavior-changing repair in this security fix. No qualifying independent non-author APPROVE or CHANGES_REQUESTED exists.

The PR is ready for independent review but intentionally unmerged until the qualifying independent-review gate is satisfied on this unchanged head.

@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 16 minutes

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

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2233fcf2-d903-4693-a5f3-d90beea6fdc3

📥 Commits

Reviewing files that changed from the base of the PR and between 9e3e484 and 0dc4f09.

📒 Files selected for processing (3)
  • services/job-analysis-api/README.md
  • services/job-analysis-api/src/orgmetra_job_analysis_api/http.py
  • services/job-analysis-api/tests/test_http_request_budgets.py

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

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

@seonghobae seonghobae changed the title fix(job-analysis): bound pre-authentication request headers fix(job-analysis): bound pre-authentication request metadata Aug 22, 2026
@seonghobae
seonghobae marked this pull request as ready for review August 22, 2026 22:12

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Header parsing runs twice per request

_typed_headers runs once via _authorization_header (http.py:148) and again directly at http.py:173, so the header sequence is validated and lower-cased twice per request. Bounded and correct, just redundant work.

(Refers to this code)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent Please review the current unchanged head against protected develop. Local exact-head verification: all owned package suites pass at 100% statement/branch coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant