Skip to content

Embed: proxy buffers for the subscribe response headers - #1548

Merged
feruzm merged 1 commit into
developfrom
fix/embed-proxy-buffers
Aug 20, 2026
Merged

Embed: proxy buffers for the subscribe response headers#1548
feruzm merged 1 commit into
developfrom
fix/embed-proxy-buffers

Conversation

@feruzm

@feruzm feruzm commented Aug 20, 2026

Copy link
Copy Markdown
Member

Fixes #1547.

A tenant blog answers 502 to its own signup form. The routing added in #1542 is correct: host.docker.internal resolves through extra_hosts, TLS is fine and the origin answers. The read is what fails.

upstream sent too big header while reading response header from upstream,
upstream: "https://172.17.0.1:443/api/newsletter/subscribe"

Next attaches a ~3.5KB report-only CSP to every response, so even a 400 from the route carries about 4.2KB of headers, over nginx's 4k default proxy_buffer_size. Measured on alpha, which runs develop and so has the route, with an invalid body so nobody was subscribed: 400, 4250 bytes of response headers.

eu.ecency.com already carries these buffers with the same explanation. Here they go on the location rather than the server block, because the newsletter location is the only one proxying to the web tier; the others proxy to hosting_api, whose headers are small.

Verification

Run against the live origin, using the production image ecency/self-hosted:sha-4eead79 on the ecency-hosting network with the same host.docker.internal:host-gateway alias production has:

config result
committed (develop) 502
this branch 404, the origin's own answer, passed through

nginx -t passes in that image on that network.

The remaining 404 is a separate and expected thing: the production origin has no newsletter route yet. .next/server/app/api/ in the running ecency/vision-web:latest lists threespeak, hosting, oembed and the rest, with no newsletter, so the relay reaches ecency.com only with the next develop to main deploy. Both this fix and that deploy are needed before a reader on a tenant blog can subscribe.

…headers

A tenant blog answered 502 to its own signup form. The routing was right,
host.docker.internal resolved and the origin answered, and what failed was the
read: Next attaches a report-only CSP of about 3.5KB to every response, so even
a 400 carries roughly 4.2KB of headers and overflows nginx's 4k default with
"upstream sent too big header".

eu.ecency.com already carries these buffers for exactly this reason. Here they
sit on the location rather than the server block, because it is the only one
proxying to the web tier; the rest go to hosting_api, whose headers are small.

Verified against the live origin, using the production image on the compose
network with the same host-gateway alias: the committed config answers 502 and
this one passes the origin's own response through.
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

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

Next review available in: 38 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: 3975bb47-8ba4-4d5e-86e7-62bdad3b4978

📥 Commits

Reviewing files that changed from the base of the PR and between 4eead79 and 203730b.

📒 Files selected for processing (1)
  • apps/self-hosted/hosting/nginx-multi-tenant.conf

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.

@qodo-code-review

qodo-code-review Bot commented Aug 20, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Prevent tenant newsletter proxy 502s from large response headers

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Increase newsletter proxy buffers to accept Next.js responses with large CSP headers.
• Scope buffer overrides to subscription routes for wildcard and custom tenant domains.
Diagram

sequenceDiagram
    actor Reader
    participant Tenant as Tenant nginx
    participant Origin as Origin nginx
    participant Next as Next web
    Reader->>Tenant: Subscribe POST
    Tenant->>Origin: HTTPS proxy
    Origin->>Next: Forward request
    Next-->>Origin: CSP-heavy response
    Origin-->>Tenant: Large headers
    Note over Tenant: 16k header buffer
    Tenant-->>Reader: Origin response
Loading
High-Level Assessment

The location-scoped nginx buffer override is the appropriate fix because only the newsletter endpoint proxies to the web tier producing oversized CSP headers. Server-wide buffers would unnecessarily affect hosting API routes, while changing or stripping the origin's security headers would broaden the change and weaken response policy.

Files changed (1) +20 / -0

Bug fix (1) +20 / -0
nginx-multi-tenant.confIncrease newsletter proxy response buffers for tenant domains +20/-0

Increase newsletter proxy response buffers for tenant domains

• Adds 16 KB proxy header and busy-buffer settings to the newsletter subscription locations in both wildcard-subdomain and custom-domain server blocks. This prevents nginx's default 4 KB header buffer from converting CSP-heavy origin responses into 502 errors.

apps/self-hosted/hosting/nginx-multi-tenant.conf

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@feruzm
feruzm merged commit 05fa085 into develop Aug 20, 2026
12 checks passed
@feruzm
feruzm deleted the fix/embed-proxy-buffers branch August 20, 2026 07:33
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Missing proxy buffer regression test 📘 Rule violation ▣ Testability
Description
This bug fix changes the nginx response-header buffering path, but the change set adds no automated
regression test for an oversized upstream header producing a non-502 response. Manual verification
in the PR description does not satisfy the requirement for a test that fails before the fix and
passes after it.
Code

apps/self-hosted/hosting/nginx-multi-tenant.conf[R153-155]

+        proxy_buffer_size 16k;
+        proxy_buffers 8 16k;
+        proxy_busy_buffers_size 16k;
Relevance

●● Moderate

No close precedent requiring regression tests for pure nginx config changes; team verified manually
here.

PR-#1458
PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The diff adds three nginx buffering directives that alter request behavior, while the supplied
change set contains no added or modified test file. The checklist requires a regression test for
each bug fix that fails against the pre-fix configuration and passes with the fix.

Rule 2667981: Require a failing regression test alongside each bug fix
apps/self-hosted/hosting/nginx-multi-tenant.conf[153-155]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new proxy buffer configuration fixes an oversized upstream response-header failure, but no automated regression test was added.

## Issue Context
The test must reproduce an upstream response with headers larger than nginx's default buffer and assert that the request is passed through rather than returned as 502.

## Fix Focus Areas
- apps/self-hosted/hosting/nginx-multi-tenant.conf[153-155]
- apps/self-hosted/hosting

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 82 rules
✅ Skills: 6 invoked
  add-feature
  add-query
  add-sdk-mutation
  add-test
  code-review
  debug
Review mode: 🚀 Fast: This is a localized nginx configuration change with two identical, well-justified buffer settings for one proxy path, avoiding security, schema, API, and other high-risk concerns.

Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +153 to +155
proxy_buffer_size 16k;
proxy_buffers 8 16k;
proxy_busy_buffers_size 16k;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Missing proxy buffer regression test 📘 Rule violation ▣ Testability

This bug fix changes the nginx response-header buffering path, but the change set adds no automated
regression test for an oversized upstream header producing a non-502 response. Manual verification
in the PR description does not satisfy the requirement for a test that fails before the fix and
passes after it.
Agent Prompt
## Issue description
The new proxy buffer configuration fixes an oversized upstream response-header failure, but no automated regression test was added.

## Issue Context
The test must reproduce an upstream response with headers larger than nginx's default buffer and assert that the request is passed through rather than returned as 502.

## Fix Focus Areas
- apps/self-hosted/hosting/nginx-multi-tenant.conf[153-155]
- apps/self-hosted/hosting

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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.

Self-hosted embed: tenant nginx 502s on the subscribe response (4k proxy buffer)

1 participant