Skip to content

Commit 8b1381e

Browse files
authored
Merge pull request #11 from ImagingDataCommons/rest/uvicorn-proxy-headers
Harden the REST container behind the hosted LB (proxy-headers, exec, configurable trusted proxies)
2 parents d6723a4 + e688f72 commit 8b1381e

2 files changed

Lines changed: 30 additions & 11 deletions

File tree

Dockerfile

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,15 @@ EXPOSE 8080
3434

3535
# Default: REST API. For the remote MCP server instead, override the command with:
3636
# idc-mcp --http --host 0.0.0.0 --port 8080
37-
CMD ["sh", "-c", "uvicorn idc_api.rest.app:app --host 0.0.0.0 --port ${PORT:-8080}"]
37+
#
38+
# --proxy-headers: Cloud Run (and any HTTPS load balancer) terminates TLS at the edge and forwards
39+
# to the container over http with `X-Forwarded-Proto: https`. uvicorn enables --proxy-headers by
40+
# default but only trusts forwarded headers from peers in FORWARDED_ALLOW_IPS (default 127.0.0.1);
41+
# the managed proxy isn't loopback, so without a wider allow-list the app treats requests as http
42+
# and emits http:// redirect Location headers (a protocol downgrade — e.g. the /v3/ -> /v3 307).
43+
# We default FORWARDED_ALLOW_IPS to '*' (safe here: the managed front end is the only ingress and
44+
# always sets the header) but leave it overridable — run the image standalone with a trusted proxy
45+
# restriction via e.g. `docker run -e FORWARDED_ALLOW_IPS=127.0.0.1 ...`.
46+
# `exec` makes uvicorn PID 1 so Cloud Run's SIGTERM reaches it directly for a graceful shutdown.
47+
ENV FORWARDED_ALLOW_IPS="*"
48+
CMD ["sh", "-c", "exec uvicorn idc_api.rest.app:app --host 0.0.0.0 --port ${PORT:-8080} --proxy-headers"]

dev/deployment.md

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -480,16 +480,24 @@ curl -sI https://dev-api.canceridc.dev/v3/version | grep -i server # want: Goo
480480
> exist" while `/v3/*` worked (the half-fixed state seen 2026-07: `/health` and `/docs` 404'd,
481481
> `/v3/version` didn't).
482482
>
483-
> **Resolved (2026-07): every REST route now lives under `/v3`**`/v3` (landing), `/v3/health`,
484-
> `/v3/version`, `/v3/docs`, `/v3/redoc`, `/v3/openapi.json`, and nothing at the bare root
485-
> (`FastAPI(docs_url=…, redoc_url=…, openapi_url=…)` set to `/v3/…` + landing route at `/v3`; see
486-
> [rest/app.py](../src/idc_api/rest/app.py)). The previously-broken paths moved *into* the range the
487-
> `/v3/*` glob already routes, so **redeploying the app is enough — no further URL-map change is
488-
> needed**; the bare-root paths stay 404, which is now the intended behavior. MCP remains a sibling
489-
> at `/mcp`. **If you add a REST route, keep it under `/v3`** or this glob will strand it. (No Cloud
490-
> Run health-check impact: the deploys above set no HTTP probe, so the default TCP startup probe is
491-
> unaffected. Belt-and-suspenders option: also point the URL map's *default* backend at the
492-
> `idc-api-v3` NEG, so any stray non-`/v3` path still resolves instead of hitting the ESP.)
483+
> **Mostly resolved (2026-07): every REST route now lives under `/v3`**`/v3` (landing),
484+
> `/v3/health`, `/v3/version`, `/v3/docs`, `/v3/redoc`, `/v3/openapi.json`, and nothing at the bare
485+
> root (`FastAPI(docs_url=…, redoc_url=…, openapi_url=…)` set to `/v3/…` + landing route at `/v3`;
486+
> see [rest/app.py](../src/idc_api/rest/app.py)). The previously-broken paths moved *into* the range
487+
> the `/v3/*` glob already routes, so a **redeploy fixed every functional endpoint under `/v3/…`
488+
> with no URL-map change**. The bare non-`/v3` paths (`/`, `/health`, …) stay 404 — intended. MCP
489+
> remains a sibling at `/mcp`. **If you add a REST route, keep it under `/v3/`** or this glob will
490+
> strand it. (No Cloud Run health-check impact: the deploys above set no HTTP probe, so the default
491+
> TCP startup probe is unaffected.)
492+
>
493+
> **⚠️ Known limitation (deferred) — the bare `/v3` landing is unreachable on the hosted domain.**
494+
> `/v3/*` matches only paths *under* `/v3/`; the admin confirmed the LB, as configured, **does not
495+
> count a bare (trailing-slash-less) path in the match**, so `/v3` (no trailing slash) falls through
496+
> to the legacy-ESP default backend and 404s (`Server: nginx`), and `/v3/` 307-redirects onto it.
497+
> This affects only the landing page — every real endpoint is under `/v3/…` and works; use
498+
> `/v3/docs` or `/v3/version` as the entry points. Making bare `/v3` resolve needs an LB-side change
499+
> (an exact/prefix match on `/v3`, or pointing the URL map's *default* backend at the `idc-api-v3`
500+
> NEG); **left for later investigation.**
493501
494502
#### Remote MCP over the shared domain — verified behavior
495503

0 commit comments

Comments
 (0)