21 KiB
21 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | must_haves | |||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 09-botfights-platform-upgrade | 01 | execute | 1 |
|
true |
|
|
Decision IDs used in this plan map to 09-CONTEXT.md Locked Decisions:
D-01 = BOT-01 native nostr signer login, D-02 = BOT-02 unified AI bot-setup prompt,
D-03 = BOT-03 shared public match endpoint on VPS2, D-04 = BOT-04 registry/manifest + signed catalog.
Purpose: D-03 is the only genuinely new architecture in this phase and the one that can dead-end the
whole demo (SSE buffering, Host-header routing, gzip double-decode, health-check coupling). Proving
it end-to-end against a real HTTP upstream before VPS2, the image build, or the catalog publish exist
means a wrong turn costs one commit instead of ten.
Output: server/src/middleware/arena-proxy.ts, its end-to-end test, the mount in app.ts, an
SSE-through-nginx fix in routes/fights.ts, and the env var documented in docker-compose.yml.
Repo: /home/archipelago/Projects/botfight (NOT archy). Commit target: git push origin main
(= source.archipelago-foundation.org/lfg2025/botfights). The plan/SUMMARY files live in archy and
are pushed with git push gitea-ai main.
<execution_context> @$HOME/.claude/gsd-core/workflows/execute-plan.md @$HOME/.claude/gsd-core/templates/summary.md </execution_context>
@.planning/PROJECT.md @.planning/ROADMAP.md @.planning/STATE.md @.planning/phases/09-botfights-platform-upgrade/09-CONTEXT.md @.planning/phases/09-botfights-platform-upgrade/09-RESEARCH.md @.planning/phases/09-botfights-platform-upgrade/09-PATTERNS.md @.planning/phases/09-botfights-platform-upgrade/09-VALIDATION.mdArtifacts this plan produces
| Symbol | Kind | File |
|---|---|---|
arenaProxy |
exported async Hono middleware (c, next) |
server/src/middleware/arena-proxy.ts |
HOP_BY_HOP |
module-private Set<string> of headers never forwarded |
same |
arena-proxy.test.ts |
Vitest suite driving a real upstream via @hono/node-server |
server/src/middleware/arena-proxy.test.ts |
app.use('/api/*', arenaProxy) |
mount point | server/src/app.ts |
- `registers a bot upstream and reads it back through the proxy`: through the proxying app,
`POST /api/bots` with a JSON body, assert 200 and the echoed name; then `GET /api/bots`
through the proxying app and assert the bot registered a moment ago is in the list. This is
the phase's cross-node-visibility claim reduced to its smallest honest form.
- `falls through to local routers when ARENA_UPSTREAM_URL is unset`: delete the env var, mount
`arenaProxy` plus a sentinel local route, assert the sentinel answers (proving `next()` ran).
- `answers /api/health locally even in proxy mode`: point the env var at a port with nothing
listening, assert `GET /api/health` still returns 200 from the local handler.
- `forwards method, query string and JSON body unchanged`: upstream echoes back
`{method, path, query, body}`; assert each field round-trips for a `POST /api/x?a=1&b=2`.
- `does not forward the inbound Host header`: upstream echoes the `host` header it received;
assert it equals the upstream's own `127.0.0.1:<port>` authority, not the caller's.
- `strips response content-encoding and content-length`: upstream replies with those headers set;
assert neither is present on the response the proxy returns (Node's fetch already decoded the
body, so copying them corrupts the response for the browser).
Read `process.env.ARENA_UPSTREAM_URL` INSIDE the handler on every call, not at module scope — a
module-level constant is captured at import time and cannot be toggled by the tests or by a
container restart-free config change.
Behavior, in order:
1. If the env var is empty/unset, `return next()` — standalone mode, today's code path untouched.
2. If `c.req.path` is exactly `/api/health`, `return next()` — the manifest health check
(`apps/botfights/manifest.yml` `health_check.path`) must stay answerable while the arena is
unreachable, otherwise podman marks a perfectly healthy node container unhealthy and restarts
it in a loop. Keep this bypass list in one named module constant so it is greppable.
3. Build the target URL from the upstream base plus `c.req.path` plus the original query string
taken from `new URL(c.req.url).search` (do not re-encode via URLSearchParams round-trip —
that reorders and re-escapes repeated keys).
4. Copy the inbound headers into a fresh `Headers`, dropping: `host`, `connection`,
`keep-alive`, `transfer-encoding`, `upgrade`, `proxy-authorization`, `proxy-connection`,
`te`, `trailer`, and `content-length` (undici recomputes it). Define that drop list as a
module-level `HOP_BY_HOP` set. Then set `accept-encoding` to `identity` on the forwarded
request so the upstream returns an uncompressed body and no encoding bookkeeping is needed.
5. `fetch(target, { method, headers, body, redirect: 'manual', duplex: 'half' })` where `body`
is `undefined` for GET/HEAD and `c.req.raw.body` otherwise. The `duplex` option is required
by Node 22's undici whenever a streaming body is sent and needs a `@ts-expect-error` since it
is missing from the DOM `RequestInit` type.
6. Return `new Response(upstreamRes.body, { status, headers })` where `headers` is a copy of the
upstream response headers with `content-encoding`, `content-length` and the same hop-by-hop
set removed. Pass the body through as the stream it is — reading it into a string or object
first breaks the SSE fight stream (Task 2) and is the single most likely way to get this
wrong.
Mount in `app.ts`: `import { arenaProxy } from './middleware/arena-proxy.js'` and
`app.use('/api/*', arenaProxy)` placed immediately after the `/api/docs/*` cache-header
middleware (currently around line 93) and before `app.get('/api/health', ...)`. Do not move,
reorder or delete any existing middleware.
Keep the error-response convention of this codebase: `c.json({ error: '...' }, status)` returns,
never thrown exceptions inside the handler (upstream-failure handling is Task 2).
- Client IP forwarding: before fetching, append the caller's address to `x-forwarded-for`
(comma-joined with any inbound value, caller first if none exists) and set `x-real-ip` when
absent. Take the caller address from the Node socket the same way `rate-limit.ts` does —
`(c.env as Record<string, any>)?.incoming?.socket?.remoteAddress` — and skip both headers when
it cannot be determined rather than inventing a value. Without this every request from one
node arrives at the arena wearing that node's single public IP and the arena's global
`rateLimit(60_000, 300)` throttles that node's entire user base as one client.
- Upstream failure: wrap the fetch in try/catch and, on any network error or timeout, log via
the existing `logger` module and return `c.json({ error: 'Arena unreachable.' }, 502)`. Apply
an `AbortSignal.timeout(...)` of 30s for non-streaming requests; do NOT apply a timeout to the
SSE stream path (`/api/fights/` + `/stream`), which is long-lived by design.
In `routes/fights.ts`'s stream handler, set `c.header('X-Accel-Buffering', 'no')` (and keep the
existing headers) before handing off to `streamSSE`. nginx-proxy-manager fronts the canonical
arena with the stock `proxy.conf`, which does not disable proxy buffering; that header is the
documented nginx opt-out and is what makes live fight events arrive during the fight instead of
all at once when it ends. Harmless when no nginx is in the path.
In `docker-compose.yml`, document (commented, alongside the existing commented env block) the
two new vars and their split: `ARENA_UPSTREAM_URL` — set on a NODE instance to make it a thin
client of the canonical arena, left unset on the canonical arena itself; `TRUSTED_PROXY` — set
to 1 only on the canonical arena, which sits behind nginx-proxy-manager. Do not set either as an
active value in this file; the canonical arena gets its own compose file in plan 09-04.
Add tests to `arena-proxy.test.ts`:
- `streams SSE incrementally through the proxy`: the upstream test app writes three SSE frames
with a delay between them; read the proxied response body with a `ReadableStream` reader and
assert the first frame arrives before the upstream has written the last one (assert on
elapsed-time ordering, not on the total payload). A buffering proxy fails this.
- `forwards the client address in x-forwarded-for`: upstream echoes the header; assert it is
present and non-empty when the socket address is available.
- `answers 502 when the arena is unreachable`: point the env var at a closed port, assert status
502 and a JSON body with an `error` key.
<threat_model>
Trust Boundaries
| Boundary | Description |
|---|---|
| browser / bot script → node botfights instance | Untrusted client input enters the node's Hono server |
| node instance → canonical arena over the public internet | A new outbound trust hop introduced by this plan; carries NIP-98 events, JWTs and bot secrets |
| node instance → its own local SQLite DB | Bypassed entirely in proxy mode; still the live path in standalone mode |
STRIDE Threat Register
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|---|---|---|---|---|---|
| T-09-01 | Spoofing | x-forwarded-for written by the node proxy |
medium | mitigate | The arena only honours forwarding headers when TRUSTED_PROXY is set (rate-limit.ts getIp), and nginx-proxy-manager rewrites X-Real-IP to the true peer, so a client-supplied XFF cannot alone impersonate an IP for rate-limit evasion |
| T-09-02 | Information disclosure | credentials crossing node → arena | high | mitigate | ARENA_UPSTREAM_URL is an https:// origin (set in the manifest, plan 09-06); the proxy forwards Authorization unchanged over TLS and never logs header values |
| T-09-03 | Denial of service | proxy holding sockets for a hung arena | medium | mitigate | 30s AbortSignal.timeout on non-stream requests + 502 fast-fail (Task 2); SSE deliberately exempt |
| T-09-04 | Denial of service | node container flapping when the arena is down | high | mitigate | /api/health is never proxied (Task 1), so the manifest health check reflects the node container's own liveness |
| T-09-05 | Tampering | response corruption from copying stale content-encoding/content-length |
medium | mitigate | Forwarded request asks for identity; both headers are stripped from the returned response (Task 1, asserted by test) |
| T-09-06 | Repudiation | proxied requests losing the originating identity in arena logs | low | accept | XFF forwarding covers the operational need; full request attribution across nodes is out of this phase's scope |
| </threat_model> |
<success_criteria>
- A node instance in proxy mode serves
/api/*from the canonical arena and nothing else changes. - Standalone mode (env unset) is byte-for-byte the behaviour shipped today.
/api/healthis local-only; SSE streams incrementally; the client IP survives the hop; an unreachable arena degrades to a 502 JSON error.- No new npm dependency was added (native
fetchonly). </success_criteria>