fix(security): newsletter exception leak + careers upload bounds (#199) - #220

Merged
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds
Jun 14, 2026
Merged

fix(security): newsletter exception leak + careers upload bounds (#199)#220
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds

Conversation

@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
Member

Closes#199.

Problems

  1. newsletter.subscribe leaked internal detailraise HTTPException(500, detail=str(e)) echoed the raw DB/PostgREST exception text (host/table/driver) straight to the client, bypassing the sanitized 500 handler.
  2. careers.apply was an unbounded, untyped upload sink — public endpoint that read the whole resume into memory with file.file.read(), no size cap, no content-type check, and inserted full_name/email/linkedin_url unvalidated.

Fixes

  • Newsletter: drop str(e); log server-side and return a generic 500.
  • Careers:_validate_resume enforces a 5 MB cap via a bounded read (read(MAX+1) → 413 before the file is fully buffered) and restricts to PDF/DOC/DOCX (415); the text fields are stripped + validated (422 incl. an email-shape check). Mirrors the existing storage_service avatar validation (415/413).

Tests (vuln-closing) — tests/test_newsletter_careers_bounds.py

  • Newsletter DB error → response is the generic message, not the raw exception text.
  • Oversize resume → 413, storage upload never called.
  • Wrong content-type → 415; invalid email → 422; valid application → 200.

All four negative assertions fail on pre-fix code (raw text echoed; oversize/wrong-type/bad-email accepted with 200).

Scope / notes

Two route files + one test. Rate-limiting (mentioned in the issue's suggested fix) is out of scope — there's no rate-limiter infra in the repo yet; the acceptance criteria (size/type cap + basic input validation) are fully met. File-disjoint from the other security PRs.

⚠️ Per Wave-1 convention: do not merge — opened for review.

…pload (#199)
newsletter.subscribe echoed the raw DB/PostgREST exception text back to the
client (str(e)), leaking internal detail. Log the error server-side and return
a generic 500 instead.
careers.apply read the whole resume into memory with no size or content-type
bound and inserted unvalidated fields — an unauthenticated, unbounded upload +
write sink. Cap resume size at 5 MB (bounded read, 413), restrict to PDF/DOC/
DOCX (415), and validate the text fields (422), mirroring storage_service's
avatar validation.
Tests fail on pre-fix code: raw exception text echoed; oversize/wrong-type
uploads and invalid email accepted with 200.
@coderabbitai

Copy link
Copy Markdown

Warning

Review limit reached

@Jose-Gael-Cruz-Lopez, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 57 minutes and 17 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc7699b4-a68e-44e0-94ab-61af1a774a3f

📥 Commits

Reviewing files that changed from the base of the PR and between a27b688 and 5bd3cbe.

📒 Files selected for processing (3)
  • backend/routes/careers.py
  • backend/routes/newsletter.py
  • backend/tests/test_newsletter_careers_bounds.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/199-newsletter-careers-bounds

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 and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend5bd3cbeCommit Preview URL

Branch Preview URL
Jun 13 2026, 03:42 AM

@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez merged commit f39b87c into mainJun 14, 2026
6 checks passed
@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez deleted the security/199-newsletter-careers-bounds branch June 14, 2026 02:17
Jose-Gael-Cruz-Lopez added a commit that referenced this pull request Jun 14, 2026
…dpoints (#182)
/api/extract/pdf and /api/extract/image took no Request and no auth guard, ran
multi-second Docling/tesseract OCR for any anonymous caller, with no size cap
and no rate limit — an attacker could drive unbounded OCR load/cost.
- Require an authenticated session via get_session_user_id (401).
- Cap upload size at 20 MB via a bounded read (413), reusing the #220
size-bound pattern factored into a shared services/request_limits.py.
- Per-user sliding-window rate limit (429), same limiter shape as the
flashcard import service, now in the shared module.
Tests: unauthenticated PDF/image -> 401; oversize -> 413; >threshold -> 429.
All fail on pre-fix code (no auth/bounds/limiter).
Follow-ups (noted, out of scope): flashcard_import_service has a duplicate
limiter that can migrate to request_limits; main.py's HTTPException handler
drops exc.headers, so 429 conveys retry in the detail string, not Retry-After.
Sign up for freeto 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.

newsletter.subscribe leaks raw exception text; careers.apply has no size/type bounds

1 participant

@Jose-Gael-Cruz-Lopez
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(security): newsletter exception leak + careers upload bounds (#199) - #220

Merged
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds
Jun 14, 2026
Merged

fix(security): newsletter exception leak + careers upload bounds (#199)#220
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds

Conversation

@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
Member

Closes#199.

Problems

  1. newsletter.subscribe leaked internal detailraise HTTPException(500, detail=str(e)) echoed the raw DB/PostgREST exception text (host/table/driver) straight to the client, bypassing the sanitized 500 handler.
  2. careers.apply was an unbounded, untyped upload sink — public endpoint that read the whole resume into memory with file.file.read(), no size cap, no content-type check, and inserted full_name/email/linkedin_url unvalidated.

Fixes

  • Newsletter: drop str(e); log server-side and return a generic 500.
  • Careers:_validate_resume enforces a 5 MB cap via a bounded read (read(MAX+1) → 413 before the file is fully buffered) and restricts to PDF/DOC/DOCX (415); the text fields are stripped + validated (422 incl. an email-shape check). Mirrors the existing storage_service avatar validation (415/413).

Tests (vuln-closing) — tests/test_newsletter_careers_bounds.py

  • Newsletter DB error → response is the generic message, not the raw exception text.
  • Oversize resume → 413, storage upload never called.
  • Wrong content-type → 415; invalid email → 422; valid application → 200.

All four negative assertions fail on pre-fix code (raw text echoed; oversize/wrong-type/bad-email accepted with 200).

Scope / notes

Two route files + one test. Rate-limiting (mentioned in the issue's suggested fix) is out of scope — there's no rate-limiter infra in the repo yet; the acceptance criteria (size/type cap + basic input validation) are fully met. File-disjoint from the other security PRs.

⚠️ Per Wave-1 convention: do not merge — opened for review.

…pload (#199)
newsletter.subscribe echoed the raw DB/PostgREST exception text back to the
client (str(e)), leaking internal detail. Log the error server-side and return
a generic 500 instead.
careers.apply read the whole resume into memory with no size or content-type
bound and inserted unvalidated fields — an unauthenticated, unbounded upload +
write sink. Cap resume size at 5 MB (bounded read, 413), restrict to PDF/DOC/
DOCX (415), and validate the text fields (422), mirroring storage_service's
avatar validation.
Tests fail on pre-fix code: raw exception text echoed; oversize/wrong-type
uploads and invalid email accepted with 200.
@coderabbitai

Copy link
Copy Markdown

Warning

Review limit reached

@Jose-Gael-Cruz-Lopez, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 57 minutes and 17 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc7699b4-a68e-44e0-94ab-61af1a774a3f

📥 Commits

Reviewing files that changed from the base of the PR and between a27b688 and 5bd3cbe.

📒 Files selected for processing (3)
  • backend/routes/careers.py
  • backend/routes/newsletter.py
  • backend/tests/test_newsletter_careers_bounds.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/199-newsletter-careers-bounds

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 and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend5bd3cbeCommit Preview URL

Branch Preview URL
Jun 13 2026, 03:42 AM

@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez merged commit f39b87c into mainJun 14, 2026
6 checks passed
@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez deleted the security/199-newsletter-careers-bounds branch June 14, 2026 02:17
Jose-Gael-Cruz-Lopez added a commit that referenced this pull request Jun 14, 2026
…dpoints (#182)
/api/extract/pdf and /api/extract/image took no Request and no auth guard, ran
multi-second Docling/tesseract OCR for any anonymous caller, with no size cap
and no rate limit — an attacker could drive unbounded OCR load/cost.
- Require an authenticated session via get_session_user_id (401).
- Cap upload size at 20 MB via a bounded read (413), reusing the #220
size-bound pattern factored into a shared services/request_limits.py.
- Per-user sliding-window rate limit (429), same limiter shape as the
flashcard import service, now in the shared module.
Tests: unauthenticated PDF/image -> 401; oversize -> 413; >threshold -> 429.
All fail on pre-fix code (no auth/bounds/limiter).
Follow-ups (noted, out of scope): flashcard_import_service has a duplicate
limiter that can migrate to request_limits; main.py's HTTPException handler
drops exc.headers, so 429 conveys retry in the detail string, not Retry-After.
Sign up for freeto 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.

newsletter.subscribe leaks raw exception text; careers.apply has no size/type bounds

1 participant

@Jose-Gael-Cruz-Lopez
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(security): newsletter exception leak + careers upload bounds (#199) - #220

Merged
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds
Jun 14, 2026
Merged

fix(security): newsletter exception leak + careers upload bounds (#199)#220
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds

Conversation

@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
Member

Closes#199.

Problems

  1. newsletter.subscribe leaked internal detailraise HTTPException(500, detail=str(e)) echoed the raw DB/PostgREST exception text (host/table/driver) straight to the client, bypassing the sanitized 500 handler.
  2. careers.apply was an unbounded, untyped upload sink — public endpoint that read the whole resume into memory with file.file.read(), no size cap, no content-type check, and inserted full_name/email/linkedin_url unvalidated.

Fixes

  • Newsletter: drop str(e); log server-side and return a generic 500.
  • Careers:_validate_resume enforces a 5 MB cap via a bounded read (read(MAX+1) → 413 before the file is fully buffered) and restricts to PDF/DOC/DOCX (415); the text fields are stripped + validated (422 incl. an email-shape check). Mirrors the existing storage_service avatar validation (415/413).

Tests (vuln-closing) — tests/test_newsletter_careers_bounds.py

  • Newsletter DB error → response is the generic message, not the raw exception text.
  • Oversize resume → 413, storage upload never called.
  • Wrong content-type → 415; invalid email → 422; valid application → 200.

All four negative assertions fail on pre-fix code (raw text echoed; oversize/wrong-type/bad-email accepted with 200).

Scope / notes

Two route files + one test. Rate-limiting (mentioned in the issue's suggested fix) is out of scope — there's no rate-limiter infra in the repo yet; the acceptance criteria (size/type cap + basic input validation) are fully met. File-disjoint from the other security PRs.

⚠️ Per Wave-1 convention: do not merge — opened for review.

…pload (#199)
newsletter.subscribe echoed the raw DB/PostgREST exception text back to the
client (str(e)), leaking internal detail. Log the error server-side and return
a generic 500 instead.
careers.apply read the whole resume into memory with no size or content-type
bound and inserted unvalidated fields — an unauthenticated, unbounded upload +
write sink. Cap resume size at 5 MB (bounded read, 413), restrict to PDF/DOC/
DOCX (415), and validate the text fields (422), mirroring storage_service's
avatar validation.
Tests fail on pre-fix code: raw exception text echoed; oversize/wrong-type
uploads and invalid email accepted with 200.
@coderabbitai

Copy link
Copy Markdown

Warning

Review limit reached

@Jose-Gael-Cruz-Lopez, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 57 minutes and 17 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc7699b4-a68e-44e0-94ab-61af1a774a3f

📥 Commits

Reviewing files that changed from the base of the PR and between a27b688 and 5bd3cbe.

📒 Files selected for processing (3)
  • backend/routes/careers.py
  • backend/routes/newsletter.py
  • backend/tests/test_newsletter_careers_bounds.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/199-newsletter-careers-bounds

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 and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend5bd3cbeCommit Preview URL

Branch Preview URL
Jun 13 2026, 03:42 AM

@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez merged commit f39b87c into mainJun 14, 2026
6 checks passed
@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez deleted the security/199-newsletter-careers-bounds branch June 14, 2026 02:17
Jose-Gael-Cruz-Lopez added a commit that referenced this pull request Jun 14, 2026
…dpoints (#182)
/api/extract/pdf and /api/extract/image took no Request and no auth guard, ran
multi-second Docling/tesseract OCR for any anonymous caller, with no size cap
and no rate limit — an attacker could drive unbounded OCR load/cost.
- Require an authenticated session via get_session_user_id (401).
- Cap upload size at 20 MB via a bounded read (413), reusing the #220
size-bound pattern factored into a shared services/request_limits.py.
- Per-user sliding-window rate limit (429), same limiter shape as the
flashcard import service, now in the shared module.
Tests: unauthenticated PDF/image -> 401; oversize -> 413; >threshold -> 429.
All fail on pre-fix code (no auth/bounds/limiter).
Follow-ups (noted, out of scope): flashcard_import_service has a duplicate
limiter that can migrate to request_limits; main.py's HTTPException handler
drops exc.headers, so 429 conveys retry in the detail string, not Retry-After.
Sign up for freeto 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.

newsletter.subscribe leaks raw exception text; careers.apply has no size/type bounds

1 participant

@Jose-Gael-Cruz-Lopez
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(security): newsletter exception leak + careers upload bounds (#199) - #220

Merged
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds
Jun 14, 2026
Merged

fix(security): newsletter exception leak + careers upload bounds (#199)#220
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds

Conversation

@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
Member

Closes#199.

Problems

  1. newsletter.subscribe leaked internal detailraise HTTPException(500, detail=str(e)) echoed the raw DB/PostgREST exception text (host/table/driver) straight to the client, bypassing the sanitized 500 handler.
  2. careers.apply was an unbounded, untyped upload sink — public endpoint that read the whole resume into memory with file.file.read(), no size cap, no content-type check, and inserted full_name/email/linkedin_url unvalidated.

Fixes

  • Newsletter: drop str(e); log server-side and return a generic 500.
  • Careers:_validate_resume enforces a 5 MB cap via a bounded read (read(MAX+1) → 413 before the file is fully buffered) and restricts to PDF/DOC/DOCX (415); the text fields are stripped + validated (422 incl. an email-shape check). Mirrors the existing storage_service avatar validation (415/413).

Tests (vuln-closing) — tests/test_newsletter_careers_bounds.py

  • Newsletter DB error → response is the generic message, not the raw exception text.
  • Oversize resume → 413, storage upload never called.
  • Wrong content-type → 415; invalid email → 422; valid application → 200.

All four negative assertions fail on pre-fix code (raw text echoed; oversize/wrong-type/bad-email accepted with 200).

Scope / notes

Two route files + one test. Rate-limiting (mentioned in the issue's suggested fix) is out of scope — there's no rate-limiter infra in the repo yet; the acceptance criteria (size/type cap + basic input validation) are fully met. File-disjoint from the other security PRs.

⚠️ Per Wave-1 convention: do not merge — opened for review.

…pload (#199)
newsletter.subscribe echoed the raw DB/PostgREST exception text back to the
client (str(e)), leaking internal detail. Log the error server-side and return
a generic 500 instead.
careers.apply read the whole resume into memory with no size or content-type
bound and inserted unvalidated fields — an unauthenticated, unbounded upload +
write sink. Cap resume size at 5 MB (bounded read, 413), restrict to PDF/DOC/
DOCX (415), and validate the text fields (422), mirroring storage_service's
avatar validation.
Tests fail on pre-fix code: raw exception text echoed; oversize/wrong-type
uploads and invalid email accepted with 200.
@coderabbitai

Copy link
Copy Markdown

Warning

Review limit reached

@Jose-Gael-Cruz-Lopez, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 57 minutes and 17 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc7699b4-a68e-44e0-94ab-61af1a774a3f

📥 Commits

Reviewing files that changed from the base of the PR and between a27b688 and 5bd3cbe.

📒 Files selected for processing (3)
  • backend/routes/careers.py
  • backend/routes/newsletter.py
  • backend/tests/test_newsletter_careers_bounds.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/199-newsletter-careers-bounds

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 and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend5bd3cbeCommit Preview URL

Branch Preview URL
Jun 13 2026, 03:42 AM

@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez merged commit f39b87c into mainJun 14, 2026
6 checks passed
@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez deleted the security/199-newsletter-careers-bounds branch June 14, 2026 02:17
Jose-Gael-Cruz-Lopez added a commit that referenced this pull request Jun 14, 2026
…dpoints (#182)
/api/extract/pdf and /api/extract/image took no Request and no auth guard, ran
multi-second Docling/tesseract OCR for any anonymous caller, with no size cap
and no rate limit — an attacker could drive unbounded OCR load/cost.
- Require an authenticated session via get_session_user_id (401).
- Cap upload size at 20 MB via a bounded read (413), reusing the #220
size-bound pattern factored into a shared services/request_limits.py.
- Per-user sliding-window rate limit (429), same limiter shape as the
flashcard import service, now in the shared module.
Tests: unauthenticated PDF/image -> 401; oversize -> 413; >threshold -> 429.
All fail on pre-fix code (no auth/bounds/limiter).
Follow-ups (noted, out of scope): flashcard_import_service has a duplicate
limiter that can migrate to request_limits; main.py's HTTPException handler
drops exc.headers, so 429 conveys retry in the detail string, not Retry-After.
Sign up for freeto 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.

newsletter.subscribe leaks raw exception text; careers.apply has no size/type bounds

1 participant

@Jose-Gael-Cruz-Lopez
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(security): newsletter exception leak + careers upload bounds (#199) - #220

Merged
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds
Jun 14, 2026
Merged

fix(security): newsletter exception leak + careers upload bounds (#199)#220
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds

Conversation

@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
Member

Closes#199.

Problems

  1. newsletter.subscribe leaked internal detailraise HTTPException(500, detail=str(e)) echoed the raw DB/PostgREST exception text (host/table/driver) straight to the client, bypassing the sanitized 500 handler.
  2. careers.apply was an unbounded, untyped upload sink — public endpoint that read the whole resume into memory with file.file.read(), no size cap, no content-type check, and inserted full_name/email/linkedin_url unvalidated.

Fixes

  • Newsletter: drop str(e); log server-side and return a generic 500.
  • Careers:_validate_resume enforces a 5 MB cap via a bounded read (read(MAX+1) → 413 before the file is fully buffered) and restricts to PDF/DOC/DOCX (415); the text fields are stripped + validated (422 incl. an email-shape check). Mirrors the existing storage_service avatar validation (415/413).

Tests (vuln-closing) — tests/test_newsletter_careers_bounds.py

  • Newsletter DB error → response is the generic message, not the raw exception text.
  • Oversize resume → 413, storage upload never called.
  • Wrong content-type → 415; invalid email → 422; valid application → 200.

All four negative assertions fail on pre-fix code (raw text echoed; oversize/wrong-type/bad-email accepted with 200).

Scope / notes

Two route files + one test. Rate-limiting (mentioned in the issue's suggested fix) is out of scope — there's no rate-limiter infra in the repo yet; the acceptance criteria (size/type cap + basic input validation) are fully met. File-disjoint from the other security PRs.

⚠️ Per Wave-1 convention: do not merge — opened for review.

…pload (#199)
newsletter.subscribe echoed the raw DB/PostgREST exception text back to the
client (str(e)), leaking internal detail. Log the error server-side and return
a generic 500 instead.
careers.apply read the whole resume into memory with no size or content-type
bound and inserted unvalidated fields — an unauthenticated, unbounded upload +
write sink. Cap resume size at 5 MB (bounded read, 413), restrict to PDF/DOC/
DOCX (415), and validate the text fields (422), mirroring storage_service's
avatar validation.
Tests fail on pre-fix code: raw exception text echoed; oversize/wrong-type
uploads and invalid email accepted with 200.
@coderabbitai

Copy link
Copy Markdown

Warning

Review limit reached

@Jose-Gael-Cruz-Lopez, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 57 minutes and 17 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc7699b4-a68e-44e0-94ab-61af1a774a3f

📥 Commits

Reviewing files that changed from the base of the PR and between a27b688 and 5bd3cbe.

📒 Files selected for processing (3)
  • backend/routes/careers.py
  • backend/routes/newsletter.py
  • backend/tests/test_newsletter_careers_bounds.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/199-newsletter-careers-bounds

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 and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend5bd3cbeCommit Preview URL

Branch Preview URL
Jun 13 2026, 03:42 AM

@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez merged commit f39b87c into mainJun 14, 2026
6 checks passed
@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez deleted the security/199-newsletter-careers-bounds branch June 14, 2026 02:17
Jose-Gael-Cruz-Lopez added a commit that referenced this pull request Jun 14, 2026
…dpoints (#182)
/api/extract/pdf and /api/extract/image took no Request and no auth guard, ran
multi-second Docling/tesseract OCR for any anonymous caller, with no size cap
and no rate limit — an attacker could drive unbounded OCR load/cost.
- Require an authenticated session via get_session_user_id (401).
- Cap upload size at 20 MB via a bounded read (413), reusing the #220
size-bound pattern factored into a shared services/request_limits.py.
- Per-user sliding-window rate limit (429), same limiter shape as the
flashcard import service, now in the shared module.
Tests: unauthenticated PDF/image -> 401; oversize -> 413; >threshold -> 429.
All fail on pre-fix code (no auth/bounds/limiter).
Follow-ups (noted, out of scope): flashcard_import_service has a duplicate
limiter that can migrate to request_limits; main.py's HTTPException handler
drops exc.headers, so 429 conveys retry in the detail string, not Retry-After.
Sign up for freeto 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.

newsletter.subscribe leaks raw exception text; careers.apply has no size/type bounds

1 participant

@Jose-Gael-Cruz-Lopez
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(security): newsletter exception leak + careers upload bounds (#199) - #220

Merged
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds
Jun 14, 2026
Merged

fix(security): newsletter exception leak + careers upload bounds (#199)#220
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds

Conversation

@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
Member

Closes#199.

Problems

  1. newsletter.subscribe leaked internal detailraise HTTPException(500, detail=str(e)) echoed the raw DB/PostgREST exception text (host/table/driver) straight to the client, bypassing the sanitized 500 handler.
  2. careers.apply was an unbounded, untyped upload sink — public endpoint that read the whole resume into memory with file.file.read(), no size cap, no content-type check, and inserted full_name/email/linkedin_url unvalidated.

Fixes

  • Newsletter: drop str(e); log server-side and return a generic 500.
  • Careers:_validate_resume enforces a 5 MB cap via a bounded read (read(MAX+1) → 413 before the file is fully buffered) and restricts to PDF/DOC/DOCX (415); the text fields are stripped + validated (422 incl. an email-shape check). Mirrors the existing storage_service avatar validation (415/413).

Tests (vuln-closing) — tests/test_newsletter_careers_bounds.py

  • Newsletter DB error → response is the generic message, not the raw exception text.
  • Oversize resume → 413, storage upload never called.
  • Wrong content-type → 415; invalid email → 422; valid application → 200.

All four negative assertions fail on pre-fix code (raw text echoed; oversize/wrong-type/bad-email accepted with 200).

Scope / notes

Two route files + one test. Rate-limiting (mentioned in the issue's suggested fix) is out of scope — there's no rate-limiter infra in the repo yet; the acceptance criteria (size/type cap + basic input validation) are fully met. File-disjoint from the other security PRs.

⚠️ Per Wave-1 convention: do not merge — opened for review.

…pload (#199)
newsletter.subscribe echoed the raw DB/PostgREST exception text back to the
client (str(e)), leaking internal detail. Log the error server-side and return
a generic 500 instead.
careers.apply read the whole resume into memory with no size or content-type
bound and inserted unvalidated fields — an unauthenticated, unbounded upload +
write sink. Cap resume size at 5 MB (bounded read, 413), restrict to PDF/DOC/
DOCX (415), and validate the text fields (422), mirroring storage_service's
avatar validation.
Tests fail on pre-fix code: raw exception text echoed; oversize/wrong-type
uploads and invalid email accepted with 200.
@coderabbitai

Copy link
Copy Markdown

Warning

Review limit reached

@Jose-Gael-Cruz-Lopez, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 57 minutes and 17 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc7699b4-a68e-44e0-94ab-61af1a774a3f

📥 Commits

Reviewing files that changed from the base of the PR and between a27b688 and 5bd3cbe.

📒 Files selected for processing (3)
  • backend/routes/careers.py
  • backend/routes/newsletter.py
  • backend/tests/test_newsletter_careers_bounds.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/199-newsletter-careers-bounds

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 and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend5bd3cbeCommit Preview URL

Branch Preview URL
Jun 13 2026, 03:42 AM

@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez merged commit f39b87c into mainJun 14, 2026
6 checks passed
@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez deleted the security/199-newsletter-careers-bounds branch June 14, 2026 02:17
Jose-Gael-Cruz-Lopez added a commit that referenced this pull request Jun 14, 2026
…dpoints (#182)
/api/extract/pdf and /api/extract/image took no Request and no auth guard, ran
multi-second Docling/tesseract OCR for any anonymous caller, with no size cap
and no rate limit — an attacker could drive unbounded OCR load/cost.
- Require an authenticated session via get_session_user_id (401).
- Cap upload size at 20 MB via a bounded read (413), reusing the #220
size-bound pattern factored into a shared services/request_limits.py.
- Per-user sliding-window rate limit (429), same limiter shape as the
flashcard import service, now in the shared module.
Tests: unauthenticated PDF/image -> 401; oversize -> 413; >threshold -> 429.
All fail on pre-fix code (no auth/bounds/limiter).
Follow-ups (noted, out of scope): flashcard_import_service has a duplicate
limiter that can migrate to request_limits; main.py's HTTPException handler
drops exc.headers, so 429 conveys retry in the detail string, not Retry-After.
Sign up for freeto 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.

newsletter.subscribe leaks raw exception text; careers.apply has no size/type bounds

1 participant

@Jose-Gael-Cruz-Lopez
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(security): newsletter exception leak + careers upload bounds (#199) - #220

Merged
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds
Jun 14, 2026
Merged

fix(security): newsletter exception leak + careers upload bounds (#199)#220
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds

Conversation

@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
Member

Closes#199.

Problems

  1. newsletter.subscribe leaked internal detailraise HTTPException(500, detail=str(e)) echoed the raw DB/PostgREST exception text (host/table/driver) straight to the client, bypassing the sanitized 500 handler.
  2. careers.apply was an unbounded, untyped upload sink — public endpoint that read the whole resume into memory with file.file.read(), no size cap, no content-type check, and inserted full_name/email/linkedin_url unvalidated.

Fixes

  • Newsletter: drop str(e); log server-side and return a generic 500.
  • Careers:_validate_resume enforces a 5 MB cap via a bounded read (read(MAX+1) → 413 before the file is fully buffered) and restricts to PDF/DOC/DOCX (415); the text fields are stripped + validated (422 incl. an email-shape check). Mirrors the existing storage_service avatar validation (415/413).

Tests (vuln-closing) — tests/test_newsletter_careers_bounds.py

  • Newsletter DB error → response is the generic message, not the raw exception text.
  • Oversize resume → 413, storage upload never called.
  • Wrong content-type → 415; invalid email → 422; valid application → 200.

All four negative assertions fail on pre-fix code (raw text echoed; oversize/wrong-type/bad-email accepted with 200).

Scope / notes

Two route files + one test. Rate-limiting (mentioned in the issue's suggested fix) is out of scope — there's no rate-limiter infra in the repo yet; the acceptance criteria (size/type cap + basic input validation) are fully met. File-disjoint from the other security PRs.

⚠️ Per Wave-1 convention: do not merge — opened for review.

…pload (#199)
newsletter.subscribe echoed the raw DB/PostgREST exception text back to the
client (str(e)), leaking internal detail. Log the error server-side and return
a generic 500 instead.
careers.apply read the whole resume into memory with no size or content-type
bound and inserted unvalidated fields — an unauthenticated, unbounded upload +
write sink. Cap resume size at 5 MB (bounded read, 413), restrict to PDF/DOC/
DOCX (415), and validate the text fields (422), mirroring storage_service's
avatar validation.
Tests fail on pre-fix code: raw exception text echoed; oversize/wrong-type
uploads and invalid email accepted with 200.
@coderabbitai

Copy link
Copy Markdown

Warning

Review limit reached

@Jose-Gael-Cruz-Lopez, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 57 minutes and 17 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc7699b4-a68e-44e0-94ab-61af1a774a3f

📥 Commits

Reviewing files that changed from the base of the PR and between a27b688 and 5bd3cbe.

📒 Files selected for processing (3)
  • backend/routes/careers.py
  • backend/routes/newsletter.py
  • backend/tests/test_newsletter_careers_bounds.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/199-newsletter-careers-bounds

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 and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend5bd3cbeCommit Preview URL

Branch Preview URL
Jun 13 2026, 03:42 AM

@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez merged commit f39b87c into mainJun 14, 2026
6 checks passed
@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez deleted the security/199-newsletter-careers-bounds branch June 14, 2026 02:17
Jose-Gael-Cruz-Lopez added a commit that referenced this pull request Jun 14, 2026
…dpoints (#182)
/api/extract/pdf and /api/extract/image took no Request and no auth guard, ran
multi-second Docling/tesseract OCR for any anonymous caller, with no size cap
and no rate limit — an attacker could drive unbounded OCR load/cost.
- Require an authenticated session via get_session_user_id (401).
- Cap upload size at 20 MB via a bounded read (413), reusing the #220
size-bound pattern factored into a shared services/request_limits.py.
- Per-user sliding-window rate limit (429), same limiter shape as the
flashcard import service, now in the shared module.
Tests: unauthenticated PDF/image -> 401; oversize -> 413; >threshold -> 429.
All fail on pre-fix code (no auth/bounds/limiter).
Follow-ups (noted, out of scope): flashcard_import_service has a duplicate
limiter that can migrate to request_limits; main.py's HTTPException handler
drops exc.headers, so 429 conveys retry in the detail string, not Retry-After.
Sign up for freeto 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.

newsletter.subscribe leaks raw exception text; careers.apply has no size/type bounds

1 participant

@Jose-Gael-Cruz-Lopez
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(security): newsletter exception leak + careers upload bounds (#199) - #220

Merged
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds
Jun 14, 2026
Merged

fix(security): newsletter exception leak + careers upload bounds (#199)#220
Jose-Gael-Cruz-Lopez merged 1 commit into
mainfrom
security/199-newsletter-careers-bounds

Conversation

@Jose-Gael-Cruz-Lopez

Copy link
Copy Markdown
Member

Closes#199.

Problems

  1. newsletter.subscribe leaked internal detailraise HTTPException(500, detail=str(e)) echoed the raw DB/PostgREST exception text (host/table/driver) straight to the client, bypassing the sanitized 500 handler.
  2. careers.apply was an unbounded, untyped upload sink — public endpoint that read the whole resume into memory with file.file.read(), no size cap, no content-type check, and inserted full_name/email/linkedin_url unvalidated.

Fixes

  • Newsletter: drop str(e); log server-side and return a generic 500.
  • Careers:_validate_resume enforces a 5 MB cap via a bounded read (read(MAX+1) → 413 before the file is fully buffered) and restricts to PDF/DOC/DOCX (415); the text fields are stripped + validated (422 incl. an email-shape check). Mirrors the existing storage_service avatar validation (415/413).

Tests (vuln-closing) — tests/test_newsletter_careers_bounds.py

  • Newsletter DB error → response is the generic message, not the raw exception text.
  • Oversize resume → 413, storage upload never called.
  • Wrong content-type → 415; invalid email → 422; valid application → 200.

All four negative assertions fail on pre-fix code (raw text echoed; oversize/wrong-type/bad-email accepted with 200).

Scope / notes

Two route files + one test. Rate-limiting (mentioned in the issue's suggested fix) is out of scope — there's no rate-limiter infra in the repo yet; the acceptance criteria (size/type cap + basic input validation) are fully met. File-disjoint from the other security PRs.

⚠️ Per Wave-1 convention: do not merge — opened for review.

…pload (#199)
newsletter.subscribe echoed the raw DB/PostgREST exception text back to the
client (str(e)), leaking internal detail. Log the error server-side and return
a generic 500 instead.
careers.apply read the whole resume into memory with no size or content-type
bound and inserted unvalidated fields — an unauthenticated, unbounded upload +
write sink. Cap resume size at 5 MB (bounded read, 413), restrict to PDF/DOC/
DOCX (415), and validate the text fields (422), mirroring storage_service's
avatar validation.
Tests fail on pre-fix code: raw exception text echoed; oversize/wrong-type
uploads and invalid email accepted with 200.
@coderabbitai

Copy link
Copy Markdown

Warning

Review limit reached

@Jose-Gael-Cruz-Lopez, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 57 minutes and 17 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fc7699b4-a68e-44e0-94ab-61af1a774a3f

📥 Commits

Reviewing files that changed from the base of the PR and between a27b688 and 5bd3cbe.

📒 Files selected for processing (3)
  • backend/routes/careers.py
  • backend/routes/newsletter.py
  • backend/tests/test_newsletter_careers_bounds.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/199-newsletter-careers-bounds

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 and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend5bd3cbeCommit Preview URL

Branch Preview URL
Jun 13 2026, 03:42 AM

@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez merged commit f39b87c into mainJun 14, 2026
6 checks passed
@Jose-Gael-Cruz-Lopez
Jose-Gael-Cruz-Lopez deleted the security/199-newsletter-careers-bounds branch June 14, 2026 02:17
Jose-Gael-Cruz-Lopez added a commit that referenced this pull request Jun 14, 2026
…dpoints (#182)
/api/extract/pdf and /api/extract/image took no Request and no auth guard, ran
multi-second Docling/tesseract OCR for any anonymous caller, with no size cap
and no rate limit — an attacker could drive unbounded OCR load/cost.
- Require an authenticated session via get_session_user_id (401).
- Cap upload size at 20 MB via a bounded read (413), reusing the #220
size-bound pattern factored into a shared services/request_limits.py.
- Per-user sliding-window rate limit (429), same limiter shape as the
flashcard import service, now in the shared module.
Tests: unauthenticated PDF/image -> 401; oversize -> 413; >threshold -> 429.
All fail on pre-fix code (no auth/bounds/limiter).
Follow-ups (noted, out of scope): flashcard_import_service has a duplicate
limiter that can migrate to request_limits; main.py's HTTPException handler
drops exc.headers, so 429 conveys retry in the detail string, not Retry-After.
Sign up for freeto 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.

newsletter.subscribe leaks raw exception text; careers.apply has no size/type bounds

1 participant

@Jose-Gael-Cruz-Lopez