Skip to content

Pin trust_proxy to a hop count so a forged X-Forwarded-For can't impersonate the allowlisted crawler #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jpr5 wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Pin trust_proxy to a hop count so a forged X-Forwarded-For can't impersonate the allowlisted crawler #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions deploy/aimock-docs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,23 @@ server:
max_sessions: 1000
session_ttl_minutes: 30
session_unused_ttl_minutes: 15
# Anthropic crawler — bypasses per-IP session limit (requires trust_proxy: true to match the forwarded IP)
# Anthropic crawler — bypasses per-IP session limit (requires trust_proxy to be
# set so the allowlist matches the forwarded client IP, not the proxy's)
allowlist: ["160.79.106.35"]
# Safe to enable: Railway's edge discards any client-supplied X-Forwarded-For
# and sets its own trusted value, so the leftmost XFF entry is the real peer.
trust_proxy: true
# Numeric hop count, NOT `true`. Exactly one proxy sits in front of this
# container: Railway's edge terminates TLS and forwards to the Node process
# (no CDN, no in-container reverse proxy). `trust proxy: 1` therefore tells
# Express to trust that single hop and resolve req.ip to the address the
# edge itself appended.
#
# `true` would be a vulnerability here: it trusts EVERY hop and resolves
# req.ip to the LEFTMOST X-Forwarded-For entry, which is client-supplied.
# Any caller could send `X-Forwarded-For: 160.79.106.35` and be attributed
# to the allowlisted crawler above — bypassing the per-IP session limit and
# poisoning query_log IP attribution. With a hop count, a forged prefix is
# ignored because the edge appends the real peer to the right of it.
# See src/ip-util.ts for the full rationale.
trust_proxy: 1

sources:
- name: docs
Expand Down
20 changes: 16 additions & 4 deletions deploy/copilotkit-docs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,23 @@ server:
max_sessions: 1000
session_ttl_minutes: 30
session_unused_ttl_minutes: 15
# Anthropic crawler — bypasses per-IP session limit (requires trust_proxy: true to match the forwarded IP)
# Anthropic crawler — bypasses per-IP session limit (requires trust_proxy to be
# set so the allowlist matches the forwarded client IP, not the proxy's)
allowlist: ["160.79.106.35"]
# Safe to enable: Railway's edge discards any client-supplied X-Forwarded-For
# and sets its own trusted value, so the leftmost XFF entry is the real peer.
trust_proxy: true
# Numeric hop count, NOT `true`. Exactly one proxy sits in front of this
# container: Railway's edge terminates TLS and forwards to the Node process
# (no CDN, no in-container reverse proxy). `trust proxy: 1` therefore tells
# Express to trust that single hop and resolve req.ip to the address the
# edge itself appended.
#
# `true` would be a vulnerability here: it trusts EVERY hop and resolves
# req.ip to the LEFTMOST X-Forwarded-For entry, which is client-supplied.
# Any caller could send `X-Forwarded-For: 160.79.106.35` and be attributed
# to the allowlisted crawler above — bypassing the per-IP session limit and
# poisoning query_log IP attribution. With a hop count, a forged prefix is
# ignored because the edge appends the real peer to the right of it.
# See src/ip-util.ts for the full rationale.
trust_proxy: 1

sources:
- name: docs
Expand Down
20 changes: 16 additions & 4 deletions deploy/pathfinder-docs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,23 @@ server:
max_sessions: 1000
session_ttl_minutes: 30
session_unused_ttl_minutes: 15
# Anthropic crawler — bypasses per-IP session limit (requires trust_proxy: true to match the forwarded IP)
# Anthropic crawler — bypasses per-IP session limit (requires trust_proxy to be
# set so the allowlist matches the forwarded client IP, not the proxy's)
allowlist: ["160.79.106.35"]
# Safe to enable: Railway's edge discards any client-supplied X-Forwarded-For
# and sets its own trusted value, so the leftmost XFF entry is the real peer.
trust_proxy: true
# Numeric hop count, NOT `true`. Exactly one proxy sits in front of this
# container: Railway's edge terminates TLS and forwards to the Node process
# (no CDN, no in-container reverse proxy). `trust proxy: 1` therefore tells
# Express to trust that single hop and resolve req.ip to the address the
# edge itself appended.
#
# `true` would be a vulnerability here: it trusts EVERY hop and resolves
# req.ip to the LEFTMOST X-Forwarded-For entry, which is client-supplied.
# Any caller could send `X-Forwarded-For: 160.79.106.35` and be attributed
# to the allowlisted crawler above — bypassing the per-IP session limit and
# poisoning query_log IP attribution. With a hop count, a forged prefix is
# ignored because the edge appends the real peer to the right of it.
# See src/ip-util.ts for the full rationale.
trust_proxy: 1

sources:
- name: pathfinder-docs
Expand Down
5 changes: 4 additions & 1 deletion pathfinder.example.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ server:
# - "160.79.106.35" # Example: Anthropic Assistant crawler
# - "10.0.0.0/8" # Example: internal health-probe CIDR
# NOTE: behind a reverse proxy (Railway, Fly, etc.), allowlist entries only match
# when trust_proxy: true. Otherwise the server sees the proxy IP for every request.
# when trust_proxy is set. Otherwise the server sees the proxy IP for every
# request. Prefer a hop count (e.g. 1) over `true` — with `true`, a client can
# forge X-Forwarded-For and be attributed to an allowlisted IP. See the
# SECURITY WARNING below.
#
# ⚠️ SECURITY WARNING — trust_proxy ⚠️
# When true, the server honors X-Forwarded-For and populates req.ip from
Expand Down
36 changes: 18 additions & 18 deletions scripts/atlas-harvest/SANDBOX.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ migration step.
(`deploy/copilotkit-docs.yaml`), not a from-scratch config. Start from the
prod file and apply these deltas:

| # | Prod | Sandbox | Why |
| --- | ----------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- |
| 1 | `server.name`, `allowlist`, `trust_proxy: true` | name suffixed `-sandbox`; allowlist and trust_proxy **dropped** | Crawler-IP allowlist and proxy trust are Railway-edge concerns |
| 2 | 4 file sources over real docs/code repos | same 4 source names/types/chunking, but `repo` → `https://github.com/CopilotKit/pathfinder.git`, `path: "."`, and `file_patterns` deliberately **non-matching** (`**/*.sandbox-none.*`) | Startup `checkAndIndex` clones one small repo and indexes **0 chunks per source** — zero embedding calls, so the dummy key is never exercised |
| 3 | no `type: atlas` source | `type: atlas` source block **added** | Step-6 prerequisite — `POST /admin/reindex` 400s `unknown_source` for `--source atlas` without it |
| 4 | `indexing.auto_reindex: true` | `false` | No nightly reindex in the sandbox |
| 5 | `webhook:` block (repo_sources/path_triggers) | **removed** | No webhook surface locally |
| 6 | tools (4 search + 2 bash + 1 collect) | identical structure, descriptions shortened | Keep the tool surface real |
| # | Prod | Sandbox | Why |
| --- | --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- |
| 1 | `server.name`, `allowlist`, `trust_proxy: 1` | name suffixed `-sandbox`; allowlist and trust_proxy **dropped** | Crawler-IP allowlist and proxy trust are Railway-edge concerns |
| 2 | 4 file sources over real docs/code repos | same 4 source names/types/chunking, but `repo` → `https://github.com/CopilotKit/pathfinder.git`, `path: "."`, and `file_patterns` deliberately **non-matching** (`**/*.sandbox-none.*`) | Startup `checkAndIndex` clones one small repo and indexes **0 chunks per source** — zero embedding calls, so the dummy key is never exercised |
| 3 | no `type: atlas` source | `type: atlas` source block **added** | Step-6 prerequisite — `POST /admin/reindex` 400s `unknown_source` for `--source atlas` without it |
| 4 | `indexing.auto_reindex: true` | `false` | No nightly reindex in the sandbox |
| 5 | `webhook:` block (repo_sources/path_triggers) | **removed** | No webhook surface locally |
| 6 | tools (4 search + 2 bash + 1 collect) | identical structure, descriptions shortened | Keep the tool surface real |

The two snippets that matter. Each file source keeps its prod shape but
matches nothing:
Expand Down Expand Up @@ -96,16 +96,16 @@ source at boot — that is the design, not a failure.

Run `node dist/index.js` from the built checkout with this environment:

| Variable | Value | Notes |
| ------------------- | ---------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- |
| `DATABASE_URL` | `postgresql://mcp:mcp_local@localhost:5433/mcp_docs` | The Step-1 container |
| `PATHFINDER_CONFIG` | `<sandbox>/sandbox.yaml` | Absolute path |
| `ANALYTICS_TOKEN` | `sandbox-smoke` | The bearer for every API call below; any string works |
| `OPENAI_API_KEY` | `sandbox-dummy` | Must be SET (search tools + `embedding.provider: openai` exist in the config) but is never called over an empty corpus |
| `PORT` | `3001` | |
| `NODE_ENV` | `production` | **Required to exercise the 401 path** — in dev mode `bearerTokenAuth` bypasses the token check for localhost requests |
| `MCP_JWT_SECRET` | random (`openssl rand -hex 32`) | **Required whenever `NODE_ENV=production`** — startup is fatal without it. The two settings travel together |
| `PATHFINDER_CONSENT_HMAC_KEY` | random (`openssl rand -hex 32`) | **Also required whenever `NODE_ENV=production`** — signs the OAuth consent-nonce; startup is fatal without it. Comma-separate values to rotate (all accepted on verify) |
| Variable | Value | Notes |
| ----------------------------- | ---------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `DATABASE_URL` | `postgresql://mcp:mcp_local@localhost:5433/mcp_docs` | The Step-1 container |
| `PATHFINDER_CONFIG` | `<sandbox>/sandbox.yaml` | Absolute path |
| `ANALYTICS_TOKEN` | `sandbox-smoke` | The bearer for every API call below; any string works |
| `OPENAI_API_KEY` | `sandbox-dummy` | Must be SET (search tools + `embedding.provider: openai` exist in the config) but is never called over an empty corpus |
| `PORT` | `3001` | |
| `NODE_ENV` | `production` | **Required to exercise the 401 path** — in dev mode `bearerTokenAuth` bypasses the token check for localhost requests |
| `MCP_JWT_SECRET` | random (`openssl rand -hex 32`) | **Required whenever `NODE_ENV=production`** — startup is fatal without it. The two settings travel together |
| `PATHFINDER_CONSENT_HMAC_KEY` | random (`openssl rand -hex 32`) | **Also required whenever `NODE_ENV=production`** — signs the OAuth consent-nonce; startup is fatal without it. Comma-separate values to rotate (all accepted on verify) |

On macOS, daemonize via Python — `nohup` + `disown` dies with a spawning
subagent shell, and **`env=env` must be passed explicitly** (omitting it
Expand Down
Loading