Skip to content

docs(server): correct the forwarded.js threat-model comment #1105

Description

@vivek7405

Problem

The module doc at packages/server/src/forwarded.js:15-21 states:

Threat model: in webjs's typical deployment topology, the container's HTTP port is only reachable through the trusted edge proxy. There's no path for an attacker to inject these headers without going through that proxy.

That is accurate for X-Forwarded-Proto but not for X-Forwarded-Host. Cloudflare (and Railway) FORWARD a client-supplied X-Forwarded-Host rather than overwriting it, so "going through the proxy" is exactly how a hostile value arrives. It is the reachable path the comment says does not exist. This is the same mechanism #1097 documents for the HTML cache.

This matters more than a normal stale comment because of how WebJs is read. The framework ships as plain JS with no build step, so packages/server/src/ IS what lands in node_modules/@webjsdev/server/src/ inside every app. An AI agent grepping the framework to reason about proxy trust reads this paragraph as the authoritative threat model and will conclude a forged X-Forwarded-Host is not reachable. It is.

Comment-only. No behaviour change, and the code's actual handling is already correct (#1091 made the origin resolution safe on both listener shells).

Design / approach

Rewrite the paragraph to separate the two headers, since their trust properties genuinely differ:

  • X-Forwarded-Proto: a proxy that terminates TLS sets this itself, so the original claim holds.
  • X-Forwarded-Host: commonly forwarded from the client rather than overwritten, so treat a hostile value as reachable in the normal topology, not just on a directly-exposed container.

Keep it short and keep the WEBJS_NO_TRUST_PROXY=1 pointer. Cross-reference #1097 (cache poisoning) and #1104 (centralizing the trust decision) so an agent reading the module lands on the open work rather than re-deriving it.

Resist the urge to widen this into a code change. The mitigations belong in #1097 and #1104; this issue is only about the comment telling the truth.

Implementation notes (for the implementing agent)

  • Where to edit: packages/server/src/forwarded.js, the module JSDoc block at L15-21 (the Threat model: paragraph). Grep confirms this is the ONLY copy of the claim in the repo, so there is no second site to sync.
  • Landmines:
    • Do not weaken or delete the csrf.js:63-68 rationale, which is a DIFFERENT and still-correct argument (a browser cannot set x-forwarded-host, and a non-browser client carries no victim cookies). The two comments are easy to conflate.
    • Do not change any code in this issue. readForwarded, resolveOrigin and applyForwarded are correct as shipped; a well-meaning "fix" here would collide with Centralize the reverse-proxy trust decision (CSRF host ignores the opt-out) #1104.
    • Invariant 11 (root AGENTS.md) applies to the comment prose: no em-dashes, no space-surrounded hyphen or semicolon as a pause, and WebJs capitalized when it names the project (the current paragraph writes lowercase "webjs's" in prose, worth fixing while editing).
  • Invariants: packages/ stays plain .js + JSDoc.
  • Tests + docs: no test change (comment-only). Check whether .agents/skills/webjs/references/runtime.md (the reverse-proxy paragraph) and docs/app/docs/deployment/page.ts:63 should carry the same distinction, and update them if they overstate it too.

Acceptance criteria

Metadata

Metadata

Assignees

Labels

documentationImprovements or additions to documentation

Type

No type

Projects

Status
Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions