diff --git a/CLAUDE.md b/CLAUDE.md index 22bc09446..2a8eb2d62 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -268,3 +268,4 @@ Pre-push hooks run Prettier and ESLint checks via Lefthook. These run automatica - [Development Guide](docs/Development.md) - [DevContainer Guide](docs/DevContainer.md) - [Release Process](docs/Release.md) +- [Service Proxy Guide](docs/ServiceProxy.md) diff --git a/docs/Development.md b/docs/Development.md index 117b52162..01d58e1ed 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -278,3 +278,4 @@ pixi run test-ui -- -g "the test description" ## Other documentation - [How to release a new version](Release.md) +- [HTTPS proxy for app services](ServiceProxy.md) diff --git a/docs/ServiceProxy.md b/docs/ServiceProxy.md new file mode 100644 index 000000000..941dded88 --- /dev/null +++ b/docs/ServiceProxy.md @@ -0,0 +1,131 @@ +# HTTPS Proxy for App Services + +Running app services (`type: service`) bind a port on a compute node and publish `http://:/...`. Set `apps.service_proxy_domain` to republish them over HTTPS at a per-job subdomain instead: + +``` +https://job-12-k7m2qhxr.services.example.org/lab?token=8f2c... +``` + +The `job--` label is signed with `session_secret_key`. Job ids are a small global sequence, so an unsigned `job-12` label would let anyone who can reach the proxy sweep `job-1`..`job-500` and find — and, for any service that does not enforce its own token, reach — every running service on the instance. The MAC is 8 base32 characters (40 bits); guessing it is online-only against a reverse proxy that answers 403, so a sweep of that space takes decades at 10k requests/second. + +Design rationale: `docs/superpowers/specs/2026-08-28-app-service-https-proxy-design.md`. + +## What you need + +- **A wildcard DNS record for the zone**, e.g. `*.services.example.org`, pointing at the Fileglancer host. No zone delegation is required — a wildcard may sit at any level (RFC 4592), so one record in the parent zone is enough: + + ``` + *.services IN A + ``` + + A wildcard never matches its own parent, so this does not make `services.example.org` itself resolve. Add a plain `services IN A` record if you want the bare alias to work; the proxy does not need it. + +- **A wildcard TLS certificate for that zone.** A wildcard matches exactly one label, so a certificate for `*.example.org` does **not** cover `job-1.services.example.org` — it must name the zone you actually use. If the host already serves a `*.example.org` certificate, the two coexist fine: two `server` blocks on port 443, each with its own `ssl_certificate`, selected by SNI. Leave `default_server` on the primary block. + +- **A reverse proxy with `http_auth_request_module`** compiled in (`nginx -V | grep auth_request`). + +## Fileglancer configuration + +```yaml +session_secret_key: "" +apps: + service_proxy_domain: "services.example.org" +``` + +Leave `service_proxy_domain` empty to disable; the direct `http://:` URL is then published unchanged. + +`session_secret_key` is required when the proxy domain is set, and the server refuses to start without it. An unset key is generated at random per process, so under `uvicorn --workers N` each worker would sign hostnames with a different key and most proxied requests would be refused. Rotating it invalidates live service URLs, on top of the session revocation rotation already causes. + +## Reverse proxy configuration + +Fileglancer does not proxy the traffic itself. It exposes `GET /api/apps/resolve`, which reads the `Host` header and answers `204` with `X-Fg-Upstream: :`, or `403`. The reverse proxy resolves each request through it and connects to the upstream directly, so no proxied bytes pass through the application server. + +Add a server block for the wildcard zone. This assumes a `map $http_upgrade $connection_upgrade` block already exists at the http level: + +```nginx +server { + listen 443 ssl http2; + server_name ~^job-\d+-[a-z2-7]+\.services\.example\.org$; + + ssl_certificate /etc/nginx/certs/services-wildcard.crt; + ssl_certificate_key /etc/nginx/certs/services-wildcard.key; + + location = /_fg_resolve { + internal; + proxy_pass http://127.0.0.1:8989/api/apps/resolve; + proxy_pass_request_body off; + proxy_set_header Content-Length ""; + proxy_set_header Host $host; + } + + location / { + auth_request /_fg_resolve; + auth_request_set $upstream $upstream_http_x_fg_upstream; + + # Required because proxy_pass targets a variable. Use whatever resolver the + # host actually runs; 127.0.0.53 is systemd-resolved's stub. + resolver 127.0.0.53 valid=30s; + proxy_pass http://$upstream; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto https; + + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $connection_upgrade; + + proxy_http_version 1.1; + proxy_buffering off; + proxy_read_timeout 3600s; + } + + # A refused resolution means the service is gone, not that the user did + # something wrong, so serve Fileglancer's explanation instead of nginx's + # stock 403 page. The `=` makes the response carry the page's own 503. + error_page 403 = /_fg_unavailable; + + location = /_fg_unavailable { + internal; + proxy_pass http://127.0.0.1:8989/api/apps/service-unavailable; + proxy_set_header Host $host; + } +} +``` + +Also add this to the **main** server block, so the resolve endpoint is not reachable on the primary hostname: + +```nginx + location = /api/apps/resolve { return 404; } +``` + +Six details are load-bearing: + +- **`internal;`** on the `/_fg_resolve` location makes it reachable only from nginx's own `auth_request` subrequest, never from a client. Together with the `return 404` in the main server block, it is what keeps the unauthenticated resolve endpoint off the network. Do not remove either. +- **`proxy_set_header Host $host`** passes the app subdomain through unchanged, so the app sees `Host` and `Origin` as the same value. This is what makes JupyterLab's WebSocket origin check pass without per-app configuration. +- **`resolver`** is mandatory. Without it nginx refuses to start when `proxy_pass` targets a variable. +- **`proxy_buffering off`** and the long `proxy_read_timeout` suit long-lived WebSocket and streaming sessions, such as the remote desktop app. +- **`proxy_intercept_errors` must stay off** (its default) for the `error_page 403` above to mean what it says. The 403 it catches is the one nginx generates when `auth_request` is denied; turning interception on would also catch a 403 from the app itself — a JupyterLab token rejection, say — and replace it with the "503 Service Unavailable" page. +- **`/_fg_unavailable` is a prefix location, not a named one**, because nginx refuses a `proxy_pass` with a URI part inside a named location (`proxy_pass cannot have URI part in location given by regular expression, or inside named location`). `internal;` is what keeps it out of the URL space the app sees, so a request for that path gets a 404 rather than the error page. + +The existing HTTP-to-HTTPS redirect block is typically `default_server` with `server_name _`, in which case it already covers the new subdomains. + +## Verification + +Once DNS and the certificate are in place, launch each service app and confirm it loads and stays connected. WebSocket behavior is the thing to watch: + +- JupyterLab — kernel connects, a cell executes. +- marimo — the notebook is interactive, not stuck "connecting". +- OpenVSCode — the editor loads and a terminal opens. +- Remote Desktop — the noVNC canvas renders and accepts input. +- TensorBoard — plots load. + +If an app rejects the proxied origin, fix it in that app's manifest (most servers have an allowed-origin or base-URL option); do not weaken the proxy configuration. + +## Residual risks + +- Set `apps.service_proxy_upstream_zone` to the DNS zone your compute nodes live in, e.g. `nodes.example.org`. Without it the proxy will dial any host the Fileglancer host can reach, because the upstream comes from a file the user's job wrote. Loopback, the unspecified address, link-local (including cloud instance metadata), multicast and reserved addresses are always refused, since those are what reach the Fileglancer host itself. Private and public node addresses are allowed — an address on a routable interface is already reachable directly by any cluster user. Matching is on whole DNS labels, so a leading dot is optional and a sibling zone like `evil-nodes.example.org` does not qualify. The zone applies to hostnames only: a service that publishes the node's IP instead of its name is still accepted, so setting a zone will not break one. To confine those, set `apps.service_proxy_upstream_networks` to the CIDR networks your nodes occupy, e.g. `10.20.0.0/16`. The two settings divide the space between them — the zone governs upstreams published as names, the networks govern upstreams published as addresses — and each is empty by default, meaning no restriction on that form. Bad CIDR entries are refused at startup rather than at request time, since a typo there would otherwise reject every address upstream while looking configured. +- The signed hostname is not a substitute for a service enforcing its own token. It is unguessable, but a hostname leaks where a query string does not: plaintext SNI on the wire, DNS resolvers, and the proxy's own access log. Treat it as what makes enumeration infeasible, and `${FG_SERVICE_TOKEN}` as the credential. An app with no authentication of its own (TensorBoard, for one) is protected only by the label. +- The resolve endpoint is called once per proxied HTTP request, so a single page load of an app like JupyterLab generates dozens. Successful resolutions are cached in-process for 10 seconds, which collapses that burst to roughly one database read per service per 10 seconds per worker. Refusals are deliberately not cached, so a service starts resolving the moment it publishes its URL. The endpoint is excluded from the per-request access log for the same reason and reports running totals once a minute instead — grep for `service proxy resolve totals` to see hits, misses and refusals by reason. +- That 10-second cache is also the window in which a job that has just stopped can still be proxied. Compute-node ports get recycled, so the window is kept short deliberately; if a port is reused within it, a client can briefly reach the new occupant, which will reject it for lack of that service's own token. +- A service that manages its own URL (`auto_url` unset) should write its URL file exactly once. The cached upstream is refreshed only while someone has the job's detail page open, so a URL that changes mid-run can go stale. diff --git a/docs/config.yaml.template b/docs/config.yaml.template index d5c20e746..70c0bb759 100644 --- a/docs/config.yaml.template +++ b/docs/config.yaml.template @@ -193,6 +193,33 @@ session_cookie_secure: true # unknown_timeout_hours: 24 # Give up polling a job stuck in UNKNOWN after # # this many hours and mark it FAILED (the # # scheduler can no longer report it). 0 disables. +# service_proxy_domain: services.example.org +# # Wildcard DNS zone serving per-job HTTPS +# # subdomains for running services. When set, a +# # service is published at +# # https://job--./ instead of its +# # own http://:/ URL. Needs a wildcard +# # certificate for the zone, a reverse proxy +# # resolving upstreams via /api/apps/resolve, and +# # session_secret_key (which signs the hostname); +# # see docs/ServiceProxy.md. Empty = direct URL. +# service_proxy_upstream_zone: nodes.example.org +# # DNS zone the proxy is allowed to dial, i.e. +# # where your compute nodes live. A service +# # publishes its own upstream, so this confines +# # where that can point. Matched on whole DNS +# # labels, so a leading dot is optional and +# # evil-nodes.example.org does not qualify. +# # Hostnames only: a node that publishes its IP +# # is still accepted. Empty = no restriction. +# service_proxy_upstream_networks: +# - 10.20.0.0/16 # CIDR networks the proxy is allowed to dial. +# # The companion to the zone above: that confines +# # upstreams published as hostnames, this confines +# # upstreams published as IP addresses, since a +# # bare address has no zone to match. Empty = no +# # restriction. Loopback, link-local and the other +# # host-local ranges are refused either way. # Cluster configuration # Mirrors py-cluster-api ClusterConfig - all fields are optional diff --git a/docs/superpowers/plans/2026-08-28-app-service-https-proxy.md b/docs/superpowers/plans/2026-08-28-app-service-https-proxy.md new file mode 100644 index 000000000..bbb276b07 --- /dev/null +++ b/docs/superpowers/plans/2026-08-28-app-service-https-proxy.md @@ -0,0 +1,1111 @@ +# App Service HTTPS Proxy Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Publish each running app service at a per-job HTTPS subdomain (`https://job-123.services.example.org/...`) that nginx proxies to the service's `host:port` on a compute node. + +**Architecture:** nginx terminates TLS for a wildcard zone and asks Fileglancer, via `auth_request`, which upstream a given hostname maps to. Fileglancer's contribution is one unauthenticated resolve endpoint plus a cached `service_url` column, so no proxied bytes pass through Python. The whole feature is gated on a setting that is empty by default; unset, behavior is unchanged. + +**Tech Stack:** FastAPI, SQLAlchemy (sync), Alembic, pytest, nginx (`http_auth_request_module`). + +**Spec:** `docs/superpowers/specs/2026-08-28-app-service-https-proxy-design.md` + +## Global Constraints + +- **Always use pixi.** Never call `pytest`, `npm`, or `alembic` directly. Backend tests are `pixi run -e test test-backend -- tests/` — always scope to `tests/`, or pytest will try to collect the whole pixi environment. +- **No Janelia specifics in `fileglancer` code or comments.** Setting names and docstrings stay generic (`example.org` in examples). Real hostnames, zones and certificate paths belong only in the deployment repo. +- **Alembic head is `f2a8c1d94e60`** (`add_api_tokens_table`). The new migration's `down_revision` is that value. +- **`get_db_session()` returns a plain SQLAlchemy Session and does not auto-commit.** Follow the existing convention: mutations live in a `db.*` function in `fileglancer/database.py` that calls `session.commit()` itself, as `update_job` does at `fileglancer/database.py:1194`. +- **The resolve endpoint must never echo a value that fails its netloc pattern.** nginx does `proxy_pass http://$upstream` with that header, so it is an SSRF and header-injection boundary. +- Python 3.12+, no new dependencies. + +--- + +### Task 1: Cache `service_url` in the database + +Today `service_url` exists only as a file in the user's NFS home, read through a per-user worker RPC at `fileglancer/server.py:2699`. nginx's `auth_request` fires once per HTTP request, so that path cannot be used for resolution. This task caches the value on the job row, written through at the point it is already being read. + +**Files:** +- Modify: `fileglancer/database.py` (add column to `JobDB`, add two module functions) +- Create: `fileglancer/alembic/versions/c3e9b7f41a28_add_job_service_url.py` +- Modify: `fileglancer/server.py:2694-2705` (write-through in `get_job`) +- Create: `tests/test_service_proxy.py` + +**Interfaces:** +- Consumes: nothing. +- Produces: + - `JobDB.service_url: Optional[str]` + - `db.get_job_by_id(session: Session, job_id: int) -> Optional[JobDB]` + - `db.set_job_service_url(session: Session, job_id: int, service_url: str) -> None` + +- [ ] **Step 1: Write the failing test** + +Create `tests/test_service_proxy.py`. This file will grow across Tasks 1, 3 and 4; the fixtures below are copied from `tests/test_job_endpoints.py` and are reused by every later task. + +```python +"""Tests for the app service HTTPS proxy: URL caching, rewriting, and resolution.""" + +import os +import shutil +import tempfile + +import pytest +from fastapi.testclient import TestClient + +from fileglancer.settings import Settings, AppsSettings +from fileglancer.server import create_app, get_current_user +from fileglancer.database import ( + Base, + create_engine, + dispose_engine, + get_db_session, + create_job, + get_job_by_id, + set_job_service_url, +) + +OWNER = "alice" +PROXY_DOMAIN = "services.example.org" + + +@pytest.fixture +def temp_dir(): + d = tempfile.mkdtemp() + yield d + shutil.rmtree(d) + + +@pytest.fixture +def settings_factory(temp_dir): + """Build Settings against a fresh sqlite database, with the proxy domain + configurable so the same fixtures cover both the on and off cases.""" + def _build(proxy_domain=""): + db_path = os.path.join(temp_dir, "test.db") + db_url = f"sqlite:///{db_path}" + engine = create_engine(db_url) + Base.metadata.create_all(engine) + return Settings( + db_url=db_url, + file_share_mounts=[], + cli_mode=True, + apps=AppsSettings(service_proxy_domain=proxy_domain), + ), db_url + return _build + + +@pytest.fixture +def app_factory(settings_factory): + """Build the FastAPI app. Patches fileglancer.settings.get_settings because + database and server code call it directly rather than taking it as an + argument.""" + built = [] + + def _build(proxy_domain=""): + settings, db_url = settings_factory(proxy_domain) + import fileglancer.settings + import fileglancer.database + original = fileglancer.settings.get_settings + fileglancer.settings.get_settings = lambda: settings + fileglancer.database.get_settings = lambda: settings + fileglancer.database._migrations_run = True + built.append((db_url, original)) + return create_app(settings), db_url + + yield _build + + import fileglancer.settings + import fileglancer.database + for db_url, original in built: + dispose_engine(db_url) + fileglancer.settings.get_settings = original + fileglancer.database.get_settings = original + fileglancer.database._migrations_run = False + + +def _seed_service_job(db_url, status="RUNNING", entry_point_type="service"): + session = get_db_session(db_url) + try: + job = create_job( + session, OWNER, "https://github.com/owner/repo", + "My App", "serve", "Server", {}, + entry_point_type=entry_point_type, + ) + job.status = status + session.commit() + return job.id + finally: + session.close() + + +def test_set_job_service_url_persists(app_factory): + _, db_url = app_factory() + job_id = _seed_service_job(db_url) + + session = get_db_session(db_url) + try: + assert get_job_by_id(session, job_id).service_url is None + finally: + session.close() + + session = get_db_session(db_url) + try: + set_job_service_url(session, job_id, "http://node01:41235/lab?token=abc") + finally: + session.close() + + session = get_db_session(db_url) + try: + assert get_job_by_id(session, job_id).service_url == "http://node01:41235/lab?token=abc" + finally: + session.close() + + +def test_get_job_by_id_ignores_username(app_factory): + """Resolution happens with no authenticated user, so the lookup must not + require one.""" + _, db_url = app_factory() + job_id = _seed_service_job(db_url) + session = get_db_session(db_url) + try: + assert get_job_by_id(session, job_id).id == job_id + assert get_job_by_id(session, job_id + 999) is None + finally: + session.close() +``` + +- [ ] **Step 2: Run the test to verify it fails** + +```bash +pixi run -e test test-backend -- tests/test_service_proxy.py -v +``` + +Expected: collection error — `ImportError: cannot import name 'get_job_by_id' from 'fileglancer.database'`. + +- [ ] **Step 3: Add the column to `JobDB`** + +In `fileglancer/database.py`, in `class JobDB`, immediately after the `work_dir_subpath` column (around line 197): + +```python + # Service URL as published by the job to its work directory, cached here so + # the proxy-resolve endpoint can map a hostname to an upstream with one + # indexed read, instead of a per-user worker RPC and an NFS stat on every + # proxied request. NULL until the job publishes a URL and it is first read. + service_url = Column(String, nullable=True) +``` + +- [ ] **Step 4: Add the two database functions** + +In `fileglancer/database.py`, immediately after `get_job` (which ends at line 1191): + +```python +def get_job_by_id(session: Session, job_id: int) -> Optional[JobDB]: + """Get a job by ID without an ownership check. + + The proxy-resolve endpoint runs with no authenticated user — the browser's + session cookie is scoped to the Fileglancer hostname and is never sent to an + app subdomain — so it cannot use get_job(). Callers that do have a user must + keep using get_job(), which filters by username. + """ + return session.query(JobDB).filter_by(id=job_id).first() + + +def set_job_service_url(session: Session, job_id: int, service_url: str) -> None: + """Cache a job's published service URL on its row.""" + job = session.query(JobDB).filter_by(id=job_id).first() + if job is None or job.service_url == service_url: + return + job.service_url = service_url + session.commit() +``` + +- [ ] **Step 5: Create the migration** + +Create `fileglancer/alembic/versions/c3e9b7f41a28_add_job_service_url.py`: + +```python +"""add service_url to jobs + +Revision ID: c3e9b7f41a28 +Revises: f2a8c1d94e60 +Create Date: 2026-08-28 00:00:00.000000 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'c3e9b7f41a28' +down_revision = 'f2a8c1d94e60' +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column('jobs', sa.Column('service_url', sa.String(), nullable=True)) + + +def downgrade() -> None: + op.drop_column('jobs', 'service_url') +``` + +- [ ] **Step 6: Run the tests to verify they pass** + +```bash +pixi run -e test test-backend -- tests/test_service_proxy.py -v +``` + +Expected: both tests PASS. + +- [ ] **Step 7: Verify the migration applies to a scratch database** + +`env.py` ignores `sqlalchemy.url` from the config file; it reads `FILEGLANCER_MIGRATION_DB_URL` or the app settings' `db_url`. Set the env var so this never touches a real database: + +```bash +FILEGLANCER_MIGRATION_DB_URL=sqlite:////tmp/claude-990465/-groups-scicompsoft-home-rokickik-dev-fileglancer/434f7bd3-4d05-4a80-8300-30c8cbbcd3f6/scratchpad/migrate_check.db pixi run migrate +``` + +Expected: alembic logs `Running upgrade f2a8c1d94e60 -> c3e9b7f41a28, add service_url to jobs` and exits 0. + +- [ ] **Step 8: Write the failing write-through test** + +Append to `tests/test_service_proxy.py`: + +```python +def test_get_job_caches_service_url(app_factory, monkeypatch): + """Opening the job detail page is the only way to obtain a proxied URL, so + it is where the cache gets populated — no new poll-loop work needed.""" + app, db_url = app_factory() + job_id = _seed_service_job(db_url) + + async def fake_worker_exec(username, action, **kwargs): + return {"service_url": "http://node01:41235/lab?token=abc", "phase": "running"} + + import fileglancer.server + monkeypatch.setattr(fileglancer.server, "_worker_exec", fake_worker_exec) + + app.dependency_overrides[get_current_user] = lambda: OWNER + client = TestClient(app) + resp = client.get(f"/api/jobs/{job_id}") + app.dependency_overrides.clear() + + assert resp.status_code == 200 + assert resp.json()["service_url"] == "http://node01:41235/lab?token=abc" + + session = get_db_session(db_url) + try: + assert get_job_by_id(session, job_id).service_url == "http://node01:41235/lab?token=abc" + finally: + session.close() +``` + +- [ ] **Step 9: Run it to verify it fails** + +```bash +pixi run -e test test-backend -- tests/test_service_proxy.py::test_get_job_caches_service_url -v +``` + +Expected: FAIL — the assertion on the persisted `service_url` gets `None`. + +Note: `_worker_exec` is referenced as a module-level name inside `create_app`'s closure, so `monkeypatch.setattr` on the module works. If it turns out to be a closure-local, patch `fileglancer.server._worker_exec` before calling `create_app` instead. + +- [ ] **Step 10: Add the write-through** + +In `fileglancer/server.py`, inside the `get_job` handler, replace the body of the `try` block (currently lines 2698-2703): + +```python + try: + svc_result = await _worker_exec( + username, "get_service_url", job_id=job_id, + job=serialize_job_for_worker(db_job)) + service_url = svc_result.get("service_url") + phase = svc_result.get("phase") + if service_url: + # Cache it so /api/apps/resolve can map a proxy hostname + # to an upstream without a worker RPC per request. + db.set_job_service_url(session, job_id, service_url) + except Exception: + pass +``` + +- [ ] **Step 11: Run the tests to verify they pass** + +```bash +pixi run -e test test-backend -- tests/test_service_proxy.py -v +``` + +Expected: all three tests PASS. + +- [ ] **Step 12: Run the full backend suite for regressions** + +```bash +pixi run -e test test-backend -- tests/ +``` + +Expected: no new failures. `tests/test_job_endpoints.py` and `tests/test_database.py` exercise `JobDB` and must still pass. + +- [ ] **Step 13: Commit** + +```bash +git add fileglancer/database.py fileglancer/server.py fileglancer/alembic/versions/c3e9b7f41a28_add_job_service_url.py tests/test_service_proxy.py +git commit -m "feat: cache a service job's published URL on its row + +The proxy-resolve endpoint runs per proxied HTTP request, which rules out +the per-user worker RPC and NFS read that currently fetch service_url. +Write it through where it is already read, on the job detail endpoint." +``` + +--- + +### Task 2: Service proxy URL helpers + +Three pure functions, in their own module so they can be tested without a database or an HTTP client. Everything that parses or constructs a proxy URL lives here, including the SSRF gate. + +**Files:** +- Create: `fileglancer/apps/serviceproxy.py` +- Modify: `fileglancer/apps/__init__.py` (re-export, following the existing style) +- Create: `tests/test_service_proxy_urls.py` + +**Interfaces:** +- Consumes: nothing. +- Produces: + - `build_proxied_service_url(service_url: str, job_id: int, proxy_domain: str) -> Optional[str]` + - `job_id_from_host(host: str, proxy_domain: str) -> Optional[int]` + - `upstream_from_service_url(service_url: str) -> Optional[str]` + +- [ ] **Step 1: Write the failing tests** + +Create `tests/test_service_proxy_urls.py`: + +```python +"""Tests for the pure URL helpers behind the app service HTTPS proxy.""" + +import pytest + +from fileglancer.apps.serviceproxy import ( + build_proxied_service_url, + job_id_from_host, + upstream_from_service_url, +) + +DOMAIN = "services.example.org" + + +# --- build_proxied_service_url --- + +def test_build_swaps_scheme_and_host_keeping_path_and_query(): + """The query string carries $FG_SERVICE_TOKEN, so it must survive intact.""" + assert build_proxied_service_url( + "http://node01:41235/lab?token=abc", 123, DOMAIN + ) == "https://job-123.services.example.org/lab?token=abc" + + +def test_build_preserves_fragment(): + assert build_proxied_service_url( + "http://node01:41235/vnc.html?autoconnect=true#top", 7, DOMAIN + ) == "https://job-7.services.example.org/vnc.html?autoconnect=true#top" + + +def test_build_preserves_bare_root_url(): + assert build_proxied_service_url( + "http://node01:41235", 7, DOMAIN + ) == "https://job-7.services.example.org" + + +def test_build_returns_none_without_a_proxy_domain(): + """Empty domain is the off switch: callers fall back to the raw URL.""" + assert build_proxied_service_url("http://node01:41235/", 1, "") is None + + +def test_build_returns_none_for_empty_service_url(): + assert build_proxied_service_url("", 1, DOMAIN) is None + assert build_proxied_service_url(None, 1, DOMAIN) is None + + +# --- job_id_from_host --- + +def test_job_id_from_host_extracts_id(): + assert job_id_from_host("job-123.services.example.org", DOMAIN) == 123 + + +def test_job_id_from_host_strips_port_and_case(): + assert job_id_from_host("JOB-123.Services.Example.Org:443", DOMAIN) == 123 + + +@pytest.mark.parametrize("host", [ + "services.example.org", # no job label + "job-.services.example.org", # no digits + "job-abc.services.example.org", # not numeric + "job-123.evil.example.org", # wrong zone + "job-123.services.example.org.evil", # suffix attack + "x.job-123.services.example.org", # extra label + "", +]) +def test_job_id_from_host_rejects_bad_hosts(host): + assert job_id_from_host(host, DOMAIN) is None + + +def test_job_id_from_host_rejects_everything_without_a_domain(): + """Guards against an empty domain turning the pattern into a wildcard.""" + assert job_id_from_host("job-123.services.example.org", "") is None + + +# --- upstream_from_service_url --- + +def test_upstream_extracts_host_and_port(): + assert upstream_from_service_url("http://node01:41235/lab?token=abc") == "node01:41235" + + +def test_upstream_accepts_fqdn(): + assert upstream_from_service_url("http://node01.cluster.example.org:8080/") == \ + "node01.cluster.example.org:8080" + + +@pytest.mark.parametrize("url", [ + "http://node01/lab", # no port: nothing to proxy to + "http://user:pw@node01:41235/", # userinfo + "http://node01:41235x/", # non-numeric port + "http://node01:99999/", # port out of range + "http://node01:0/", # port 0 + "http://[::1]:8080/", # bracketed IPv6, unsupported upstream form + "http://node01:8080\r\nX-Evil: 1/", # header injection attempt + "not a url", + "", +]) +def test_upstream_rejects_unproxyable_urls(url): + """This is the SSRF and header-injection gate: nginx interpolates the result + straight into proxy_pass, so anything unexpected must yield None.""" + assert upstream_from_service_url(url) is None +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +```bash +pixi run -e test test-backend -- tests/test_service_proxy_urls.py -v +``` + +Expected: collection error — `ModuleNotFoundError: No module named 'fileglancer.apps.serviceproxy'`. + +- [ ] **Step 3: Write the implementation** + +Create `fileglancer/apps/serviceproxy.py`: + +```python +"""URL helpers for serving app services behind an HTTPS reverse proxy. + +A service job publishes ``http://:`` to its work directory. +When a proxy domain is configured, Fileglancer republishes that as +``https://job-.`` and tells the reverse proxy which +upstream the hostname maps to. These functions are the whole translation layer +between the two forms, kept pure so they can be tested without a database. +""" + +import re +from typing import Optional +from urllib.parse import urlsplit, urlunsplit + +# An upstream nginx can hand to proxy_pass: hostname and explicit port, nothing +# else. Deliberately strict — the value is interpolated into a proxy_pass +# directive, so userinfo, whitespace, CR/LF and bracketed IPv6 literals are all +# rejected rather than escaped. +_UPSTREAM_RE = re.compile(r'^([A-Za-z0-9.-]+):(\d{1,5})$') + + +def build_proxied_service_url(service_url: Optional[str], job_id: int, + proxy_domain: str) -> Optional[str]: + """Rewrite a published service URL to its HTTPS proxy form. + + Path, query and fragment are carried over verbatim: the query string holds + the service's own access token, which remains the only credential. + + Returns None when there is nothing to rewrite or no proxy domain is + configured, in which case the caller should publish the URL unchanged. + """ + if not service_url or not proxy_domain: + return None + parts = urlsplit(service_url) + return urlunsplit(( + 'https', + f'job-{job_id}.{proxy_domain}', + parts.path, + parts.query, + parts.fragment, + )) + + +def job_id_from_host(host: Optional[str], proxy_domain: str) -> Optional[int]: + """Extract the job id from a proxy hostname, or None if it isn't one. + + Matches the whole hostname, so neither a longer suffix + (``job-1.services.example.org.evil``) nor an extra label + (``x.job-1.services.example.org``) is accepted. + """ + if not host or not proxy_domain: + return None + # $host in nginx normally omits the port, but a client can send one. + hostname = host.split(':', 1)[0].strip().lower() + match = re.fullmatch( + r'job-(\d+)\.' + re.escape(proxy_domain.lower()), hostname) + if match is None: + return None + return int(match.group(1)) + + +def upstream_from_service_url(service_url: Optional[str]) -> Optional[str]: + """Extract a ``host:port`` upstream from a published service URL. + + Returns None unless the authority is exactly a hostname and an in-range + port. This is a security boundary, not a convenience: the result is + interpolated into the reverse proxy's ``proxy_pass`` target, and the source + string is a file written by the user's own job. + """ + if not service_url: + return None + try: + netloc = urlsplit(service_url).netloc + except ValueError: + return None + match = _UPSTREAM_RE.fullmatch(netloc) + if match is None: + return None + port = int(match.group(2)) + if not 1 <= port <= 65535: + return None + return netloc +``` + +- [ ] **Step 4: Re-export from the apps package** + +In `fileglancer/apps/__init__.py`, after the existing `from fileglancer.apps.jobfiles import (...)` block that ends at line 47, add a matching block. The `# noqa: F401` marker is the file's convention for these re-export imports and is required to keep the linter quiet: + +```python +from fileglancer.apps.serviceproxy import ( # noqa: F401 + build_proxied_service_url, + job_id_from_host, + upstream_from_service_url, +) +``` + +The package has no `__all__` list, so nothing else needs updating. This re-export is what lets `fileglancer/server.py` call these through its existing `apps_module` alias in Tasks 3 and 4. + +- [ ] **Step 5: Run the tests to verify they pass** + +```bash +pixi run -e test test-backend -- tests/test_service_proxy_urls.py -v +``` + +Expected: all PASS. If the bracketed-IPv6 or CR/LF case fails, the regex is wrong — fix the regex, not the test. + +- [ ] **Step 6: Commit** + +```bash +git add fileglancer/apps/serviceproxy.py fileglancer/apps/__init__.py tests/test_service_proxy_urls.py +git commit -m "feat: add URL helpers for the app service HTTPS proxy + +Pure translation between a job's published http://node:port URL and its +https://job-. proxy form, plus the strict host:port validation +that guards what gets interpolated into the proxy's proxy_pass target." +``` + +--- + +### Task 3: The resolve endpoint + +The endpoint nginx calls once per proxied request. It answers "which upstream serves this hostname?" and refuses everything else. + +**Files:** +- Modify: `fileglancer/settings.py` (add `service_proxy_domain` to `AppsSettings`) +- Modify: `fileglancer/server.py` (add the endpoint after the `get_job` handler, around line 2705) +- Modify: `tests/test_service_proxy.py` (append tests) +- Modify: `docs/config.yaml.template` (document the setting) + +**Interfaces:** +- Consumes: `db.get_job_by_id`, `job_id_from_host`, `upstream_from_service_url` from Tasks 1 and 2. +- Produces: `GET /api/apps/resolve` → `204` with header `X-Fg-Upstream: :`, or `403`. + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_service_proxy.py`: + +```python +# --- /api/apps/resolve --- + +def _resolve(app, host): + return TestClient(app).get("/api/apps/resolve", headers={"Host": host}) + + +def _seed_running_service_with_url(db_url, url="http://node01:41235/lab?token=abc"): + job_id = _seed_service_job(db_url) + session = get_db_session(db_url) + try: + set_job_service_url(session, job_id, url) + finally: + session.close() + return job_id + + +def test_resolve_returns_upstream(app_factory): + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_running_service_with_url(db_url) + resp = _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}") + assert resp.status_code == 204 + assert resp.headers["x-fg-upstream"] == "node01:41235" + + +def test_resolve_rejects_finished_job(app_factory): + """Compute-node ports get recycled. A stale subdomain must not be proxied to + whatever service now holds that port on that node.""" + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_running_service_with_url(db_url) + session = get_db_session(db_url) + try: + get_job_by_id(session, job_id).status = "DONE" + session.commit() + finally: + session.close() + resp = _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}") + assert resp.status_code == 403 + assert "x-fg-upstream" not in resp.headers + + +def test_resolve_rejects_non_service_job(app_factory): + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_service_job(db_url, entry_point_type="job") + session = get_db_session(db_url) + try: + set_job_service_url(session, job_id, "http://node01:41235/") + finally: + session.close() + assert _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}").status_code == 403 + + +def test_resolve_rejects_job_without_cached_url(app_factory): + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_service_job(db_url) + assert _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}").status_code == 403 + + +def test_resolve_rejects_unknown_job(app_factory): + app, db_url = app_factory(PROXY_DOMAIN) + assert _resolve(app, f"job-999999.{PROXY_DOMAIN}").status_code == 403 + + +@pytest.mark.parametrize("host", [ + "fileglancer.example.org", + "job-1.evil.example.org", + "job-abc.services.example.org", +]) +def test_resolve_rejects_bad_hosts(app_factory, host): + app, db_url = app_factory(PROXY_DOMAIN) + _seed_running_service_with_url(db_url) + assert _resolve(app, host).status_code == 403 + + +def test_resolve_disabled_when_no_proxy_domain(app_factory): + """With the feature off, the endpoint refuses everything.""" + app, db_url = app_factory("") + job_id = _seed_running_service_with_url(db_url) + assert _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}").status_code == 403 + + +def test_resolve_rejects_malformed_cached_url(app_factory): + """SSRF regression test: the cached value comes from a file the user's own + job wrote, and the result is interpolated into the proxy's proxy_pass.""" + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_running_service_with_url( + db_url, url="http://evil.example.org:80\r\nX-Injected: 1/") + resp = _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}") + assert resp.status_code == 403 + assert "x-fg-upstream" not in resp.headers +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +```bash +pixi run -e test test-backend -- tests/test_service_proxy.py -v +``` + +Expected: the resolve tests fail. Most return `404` (no such route); `test_resolve_disabled_when_no_proxy_domain` may pass incidentally — that is fine, it is guarding the off switch. + +Note: if `Settings(apps=AppsSettings(service_proxy_domain=...))` raises before this step, add the setting from Step 3 first and re-run. + +- [ ] **Step 3: Add the setting** + +In `fileglancer/settings.py`, in `class AppsSettings`, after `unknown_timeout_hours`: + +```python + # Wildcard DNS zone serving per-job HTTPS subdomains for running services, + # e.g. "services.example.org" to publish https://job-.services.example.org/. + # Requires a matching wildcard certificate and a reverse proxy configured to + # resolve upstreams via GET /api/apps/resolve; see + # docs/superpowers/specs/2026-08-28-app-service-https-proxy-design.md. + # Empty (the default) publishes the service's own http://host:port URL + # unchanged. + service_proxy_domain: str = "" +``` + +Settable as `apps.service_proxy_domain` in `config.yaml` or `FGC_APPS__SERVICE_PROXY_DOMAIN` in the environment. + +- [ ] **Step 4: Add the endpoint** + +In `fileglancer/server.py`, immediately after the `get_job` handler ends (after the `return _convert_job(...)` at line 2705), add: + +```python + @app.get("/api/apps/resolve", include_in_schema=False) + async def resolve_service_upstream(request: Request): + """Map a service proxy hostname to its upstream, for the reverse proxy. + + Called once per proxied request via nginx's auth_request, so it must stay + a single indexed read. Deliberately unauthenticated: the session cookie + is scoped to the Fileglancer hostname and is never sent to an app + subdomain, so there is no user to check. It discloses only a host:port + that the job's detail page already shows, and the reverse proxy marks its + location `internal` so it is not reachable from outside. + + Returns 204 with X-Fg-Upstream on success and 403 for everything else, so + auth_request denies the request. + """ + # ponytail: one indexed read per proxied request. If this ever shows up + # in profiling, wrap it in a short TTL cache keyed by job id. + proxy_domain = settings.apps.service_proxy_domain + job_id = apps_module.job_id_from_host( + request.headers.get('host'), proxy_domain) + if job_id is None: + raise HTTPException(status_code=403, detail="Not a service proxy host") + + with db.get_db_session(settings.db_url) as session: + db_job = db.get_job_by_id(session, job_id) + if (db_job is None + or getattr(db_job, 'entry_point_type', 'job') != 'service' + or db_job.status != 'RUNNING'): + raise HTTPException(status_code=403, detail="No running service for this host") + upstream = apps_module.upstream_from_service_url(db_job.service_url) + + if upstream is None: + raise HTTPException(status_code=403, detail="No usable upstream for this host") + + return Response(status_code=204, headers={"X-Fg-Upstream": upstream}) +``` + +`Response`, `Request` and `HTTPException` are all already imported at `fileglancer/server.py:24-26`. No new imports are needed. + +- [ ] **Step 5: Run the tests to verify they pass** + +```bash +pixi run -e test test-backend -- tests/test_service_proxy.py -v +``` + +Expected: all PASS. + +- [ ] **Step 6: Document the setting** + +The whole `apps:` block in `docs/config.yaml.template` is commented out and uses a trailing-comment column. Append to it, directly after the `unknown_timeout_hours` entry that currently ends at line 195: + +``` +# service_proxy_domain: services.example.org +# # Wildcard DNS zone serving per-job HTTPS +# # subdomains for running services. When set, a +# # service is published at +# # https://job-./ instead of its own +# # http://:/ URL. Needs a wildcard +# # certificate for the zone and a reverse proxy +# # resolving upstreams via /api/apps/resolve; +# # see docs/ServiceProxy.md. Empty = direct URL. +``` + +- [ ] **Step 7: Check the config template test still passes** + +`tests/test_config_template.py` validates the template against `Settings`. + +```bash +pixi run -e test test-backend -- tests/test_config_template.py -v +``` + +Expected: PASS. + +- [ ] **Step 8: Commit** + +```bash +git add fileglancer/settings.py fileglancer/server.py tests/test_service_proxy.py docs/config.yaml.template +git commit -m "feat: add /api/apps/resolve for the service HTTPS proxy + +Maps a job-. hostname to the service's host:port upstream for a +reverse proxy's auth_request. Refuses anything that is not a RUNNING +service job with a well-formed cached upstream." +``` + +--- + +### Task 4: Publish the proxied URL + +The user-visible half. With a proxy domain configured, the job detail endpoint returns the HTTPS subdomain instead of the raw URL. The frontend needs no changes — `JobDetail.tsx:724` renders whatever `job.service_url` contains. + +**Files:** +- Modify: `fileglancer/server.py:2694-2705` (rewrite before returning) +- Modify: `tests/test_service_proxy.py` (append tests) + +**Interfaces:** +- Consumes: `build_proxied_service_url` from Task 2; the write-through from Task 1. +- Produces: `GET /api/jobs/{id}` returns a rewritten `service_url` when `apps.service_proxy_domain` is set. + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_service_proxy.py`: + +```python +# --- proxied URL publication --- + +def _get_job_with_worker_url(app, job_id, url, monkeypatch): + async def fake_worker_exec(username, action, **kwargs): + return {"service_url": url, "phase": "running"} + + import fileglancer.server + monkeypatch.setattr(fileglancer.server, "_worker_exec", fake_worker_exec) + app.dependency_overrides[get_current_user] = lambda: OWNER + try: + return TestClient(app).get(f"/api/jobs/{job_id}") + finally: + app.dependency_overrides.clear() + + +def test_job_detail_publishes_proxied_url(app_factory, monkeypatch): + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_service_job(db_url) + resp = _get_job_with_worker_url( + app, job_id, "http://node01:41235/lab?token=abc", monkeypatch) + assert resp.status_code == 200 + assert resp.json()["service_url"] == \ + f"https://job-{job_id}.{PROXY_DOMAIN}/lab?token=abc" + + +def test_job_detail_publishes_raw_url_when_proxy_disabled(app_factory, monkeypatch): + """The off switch: unchanged behavior, byte for byte.""" + app, db_url = app_factory("") + job_id = _seed_service_job(db_url) + resp = _get_job_with_worker_url( + app, job_id, "http://node01:41235/lab?token=abc", monkeypatch) + assert resp.json()["service_url"] == "http://node01:41235/lab?token=abc" + + +def test_job_detail_caches_the_raw_url_not_the_proxied_one(app_factory, monkeypatch): + """The cached value is the upstream, so it must stay in its raw form.""" + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_service_job(db_url) + _get_job_with_worker_url( + app, job_id, "http://node01:41235/lab?token=abc", monkeypatch) + session = get_db_session(db_url) + try: + assert get_job_by_id(session, job_id).service_url == \ + "http://node01:41235/lab?token=abc" + finally: + session.close() +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +```bash +pixi run -e test test-backend -- tests/test_service_proxy.py -v +``` + +Expected: `test_job_detail_publishes_proxied_url` FAILS (returns the raw URL). The other two PASS already — they are regression guards. + +- [ ] **Step 3: Add the rewrite** + +In `fileglancer/server.py`, in the `get_job` handler, replace the final `return` (line 2705): + +```python + # The cached value stays raw — it is the proxy's upstream. Only what + # goes back to the browser is rewritten. + proxied = apps_module.build_proxied_service_url( + service_url, job_id, settings.apps.service_proxy_domain) + return _convert_job(db_job, service_url=proxied or service_url, + files=files, phase=phase) +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +```bash +pixi run -e test test-backend -- tests/test_service_proxy.py -v +``` + +Expected: all PASS. + +- [ ] **Step 5: Run the full backend suite** + +```bash +pixi run -e test test-backend -- tests/ +``` + +Expected: no new failures. + +- [ ] **Step 6: Commit** + +```bash +git add fileglancer/server.py tests/test_service_proxy.py +git commit -m "feat: publish service jobs at their HTTPS proxy URL + +When apps.service_proxy_domain is set, the job detail endpoint returns +https://job-. with the path and query carried over verbatim, so +the service's own token still authenticates. Unset, nothing changes." +``` + +--- + +### Task 5: Deployment configuration + +nginx lives in a separate repository, so this task is a documented handoff rather than code in this one. It cannot be verified until DNS and the certificate exist. + +**Files:** +- Create: `docs/ServiceProxy.md` +- Modify: `../fileglancer-hub/nginx.conf` (separate repository — a separate commit and pull request there) + +- [ ] **Step 1: Write the operator documentation** + +Create `docs/ServiceProxy.md`: + +````markdown +# HTTPS Proxy for App Services + +Running app services (`type: service`) bind a port on a compute node and publish `http://:/...`. Set `apps.service_proxy_domain` to republish them over HTTPS at a per-job subdomain instead. + +Design rationale: `docs/superpowers/specs/2026-08-28-app-service-https-proxy-design.md`. + +## What you need + +- **A wildcard DNS record for the zone**, e.g. `*.services.example.org`, pointing at the Fileglancer host. No zone delegation is required — a wildcard may sit at any level (RFC 4592), so one record in the parent zone is enough: + + ``` + *.services IN A + ``` + + A wildcard never matches its own parent, so this does not make `services.example.org` itself resolve. Add a plain `services IN A` record if you want the bare alias to work; the proxy does not need it. + +- **A wildcard TLS certificate for that zone.** A wildcard matches exactly one label, so a certificate for `*.example.org` does **not** cover `job-1.services.example.org` — it must name the zone you actually use. If the host already serves a `*.example.org` certificate, the two coexist fine: two `server` blocks on port 443, each with its own `ssl_certificate`, selected by SNI. Leave `default_server` on the primary block. + +- **A reverse proxy with `http_auth_request_module`** compiled in (`nginx -V | grep auth_request`). + +## Fileglancer configuration + +```yaml +apps: + service_proxy_domain: "services.example.org" +``` + +Leave it empty to disable; the direct `http://:` URL is then published unchanged. + +## Reverse proxy configuration + +Fileglancer does not proxy the traffic itself. It exposes `GET /api/apps/resolve`, which reads the `Host` header and answers `204` with `X-Fg-Upstream: :`, or `403`. The reverse proxy resolves each request through it and connects to the upstream directly, so no proxied bytes pass through the application server. + +Add a server block for the wildcard zone. This assumes a `map $http_upgrade $connection_upgrade` block already exists at the http level: + +```nginx +server { + listen 443 ssl http2; + server_name ~^job-\d+\.services\.example\.org$; + + ssl_certificate /etc/nginx/certs/services-wildcard.crt; + ssl_certificate_key /etc/nginx/certs/services-wildcard.key; + + location = /_fg_resolve { + internal; + proxy_pass http://127.0.0.1:8989/api/apps/resolve; + proxy_pass_request_body off; + proxy_set_header Content-Length ""; + proxy_set_header Host $host; + } + + location / { + auth_request /_fg_resolve; + auth_request_set $upstream $upstream_http_x_fg_upstream; + + # Required because proxy_pass targets a variable. Use whatever resolver the + # host actually runs; 127.0.0.53 is systemd-resolved's stub. + resolver 127.0.0.53 valid=30s; + proxy_pass http://$upstream; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto https; + + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $connection_upgrade; + + proxy_http_version 1.1; + proxy_buffering off; + proxy_read_timeout 3600s; + } +} +``` + +Also add this to the **main** server block, so the resolve endpoint is not reachable on the primary hostname: + +```nginx + location = /api/apps/resolve { return 404; } +``` + +Three details are load-bearing: + +- **`proxy_set_header Host $host`** passes the app subdomain through unchanged, so the app sees `Host` and `Origin` as the same value. This is what makes JupyterLab's WebSocket origin check pass without per-app configuration. +- **`resolver`** is mandatory. Without it nginx refuses to start when `proxy_pass` targets a variable. +- **`proxy_buffering off`** and the long `proxy_read_timeout` suit long-lived WebSocket and streaming sessions, such as the remote desktop app. + +The existing HTTP-to-HTTPS redirect block is typically `default_server` with `server_name _`, in which case it already covers the new subdomains. + +## Verification + +Once DNS and the certificate are in place, launch each service app and confirm it loads and stays connected. WebSocket behavior is the thing to watch: + +- JupyterLab — kernel connects, a cell executes. +- marimo — the notebook is interactive, not stuck "connecting". +- OpenVSCode — the editor loads and a terminal opens. +- Remote Desktop — the noVNC canvas renders and accepts input. +- TensorBoard — plots load. + +If an app rejects the proxied origin, fix it in that app's manifest (most servers have an allowed-origin or base-URL option); do not weaken the proxy configuration. +```` + +- [ ] **Step 2: Commit the documentation** + +```bash +git add docs/ServiceProxy.md +git commit -m "docs: document the app service HTTPS proxy setup" +``` + +- [ ] **Step 3: Link it from the development guide** + +Two indexes need the entry: + +In `CLAUDE.md`, under "Related Documentation", add: + +```markdown +- [Service Proxy Guide](docs/ServiceProxy.md) +``` + +In `docs/Development.md`, under "Other documentation" (line 280, currently a single-item list), add: + +```markdown +- [HTTPS proxy for app services](ServiceProxy.md) +``` + +```bash +git add CLAUDE.md docs/Development.md +git commit -m "docs: link the service proxy guide" +``` + +- [ ] **Step 4: Prepare the deployment repository change** + +This is a separate repository (`../fileglancer-hub`) and therefore a separate commit and pull request. Do not commit it as part of this work unless explicitly asked. + +Apply the server block from `docs/ServiceProxy.md` to `nginx.conf`, substituting the real zone and certificate paths, and add the `location = /api/apps/resolve { return 404; }` line to the existing main server block. Add a comment in `nginx.conf` pointing at `docs/ServiceProxy.md` in this repository, since the two must change together. + +Validate before reloading: + +```bash +nginx -t +``` + +- [ ] **Step 5: Report what remains** + +The feature ships dark. Report to the user that the code is merged and inert, and that lighting it up requires, in order: the wildcard DNS record, the wildcard certificate, the nginx block, and finally setting `apps.service_proxy_domain`. Per-app WebSocket verification can only happen after all four. diff --git a/docs/superpowers/specs/2026-08-28-app-service-https-proxy-design.md b/docs/superpowers/specs/2026-08-28-app-service-https-proxy-design.md new file mode 100644 index 000000000..1f8ca88f2 --- /dev/null +++ b/docs/superpowers/specs/2026-08-28-app-service-https-proxy-design.md @@ -0,0 +1,246 @@ +# HTTPS Proxy for App Services + +Date: 2026-08-28 + +## Problem + +Service-type app entry points (`type: service`) run on a compute node, bind a port, and publish `http://$FG_HOSTNAME:$FG_SERVICE_PORT` to a `service_url` file in the job work directory. Fileglancer reads that file and renders it as the "Open Service" link on the job detail page. + +Two problems with handing that URL to a browser: + +1. **Mixed content.** Fileglancer is served over HTTPS. The service link is plain HTTP, so the browser flags it, and the `$FG_SERVICE_TOKEN` that authenticates the session travels in cleartext in the query string. +2. **Unstable, opaque URLs.** The hostname and port change on every launch, so the link is neither memorable nor shareable. + +Reachability is explicitly *not* a problem being solved: users can already reach compute nodes directly, and will continue to be able to. Access control is also not a goal — the service's own token remains the only authentication, exactly as today. + +## Approach + +Address each running service at a per-job subdomain of a wildcard DNS zone, terminated by the existing nginx reverse proxy: + +``` +https://job-123.services.int.janelia.org/lab?token=abc + → h11u02.int.janelia.org:41235/lab?token=abc +``` + +nginx resolves the dynamic `host:port` by asking Fileglancer, then proxies directly. **No proxied bytes pass through Python.** That matters: one of the shipped apps is a noVNC remote desktop, which is effectively a video stream. + +### Why subdomains rather than a path prefix + +A path prefix (`https://fileglancer/apps/proxy/123/...`) needs no new DNS or certificate, but requires every app to be told its base path and to behave correctly under it: + +- JupyterLab (`--ServerApp.base_url`), marimo (`--base-url`) and TensorBoard (`--path_prefix`) have flags for it. +- openvscode-server does not document a subpath configuration; the VS Code web workbench resolves assets from an absolute root. This is the app most likely to be unfixable. +- The noVNC desktop's `path=websockify?token=` handshake needs rewriting on top of that. +- Apps still emit absolute `Location:` redirects and root-scoped `Set-Cookie`, so header rewriting is needed regardless of the flags. +- Every future app — including apps users add, which we do not control — needs its own base-path story. + +The proxy core is comparably sized either way. The difference is that a path prefix buys an open-ended per-app debugging tail, while subdomains trade that for a one-time DNS and certificate request. Subdomains also give each app its own cookie jar for free. + +### Why nginx rather than a proxy inside Fileglancer + +Both are viable. nginx wins on the facts of this deployment: + +- nginx already terminates TLS and already carries a `map $http_upgrade $connection_upgrade` block, so WebSocket upgrades pass through with no new code. +- The distribution's nginx is built `--with-http_auth_request_module`, so the dynamic-upstream lookup needs no OpenResty or Lua. +- Uvicorn runs with `--workers 10` behind nginx. Routing app traffic through it would put a remote-desktop video stream on the application server's event loop for no benefit. + +Fileglancer's entire contribution is one small endpoint that answers "what is the upstream for this hostname?". + +### DNS and certificate constraints + +A wildcard certificate matches exactly one label, in the leftmost position only (RFC 6125), and browsers enforce this strictly. The existing `*.int.janelia.org` certificate therefore covers `foo.int.janelia.org` but **not** `job-123.services.int.janelia.org`. + +Two shapes are possible, each needing one thing from operations: + +| Hostname shape | Certificate | DNS | +| --- | --- | --- | +| `fgapp-123.int.janelia.org` | existing wildcard works | needs a wildcard A record at the `int.janelia.org` apex, which would swallow every mistyped internal hostname — not viable | +| `job-123.services.int.janelia.org` | needs a new `*.services.int.janelia.org` certificate | one wildcard record, scoped to that name | + +The second shape is the viable one, and the certificate request is the same process already completed once for a different name. + +The DNS side needs no zone delegation. A wildcard may sit at any level (RFC 4592), so one record in the existing `int.janelia.org` zone is sufficient: + +``` +*.services IN A +``` + +Note that a wildcard never matches its own parent, so `services.int.janelia.org` itself is not covered by that record. Add a plain `services IN A` record if the bare alias should resolve; nothing in this design requires it, and the existing `*.int.janelia.org` certificate already covers that name. + +Two certificates coexist on one nginx without difficulty: two `server` blocks on port 443, each with its own `ssl_certificate`, selected by SNI. The names cannot collide — `job-123.services.int.janelia.org` is two labels deep and so cannot match `*.int.janelia.org`. `default_server` stays on the primary block. + +One thing to confirm before relying on this: a nested wildcard (`*.services.int.janelia.org` issued alongside an existing `*.int.janelia.org`) is routine for public CAs, but some internal PKI policies restrict wildcard depth. + +### Shipping dark + +The feature is gated on a single setting that is empty by default. With it unset, the published URL is unchanged byte for byte: the raw `http://host:port` URL is published exactly as it is today. Database traffic is not quite as inert, though — the cache write happens whenever a service job publishes a URL, regardless of the setting, so a `RUNNING` service job's detail fetch now also caches `service_url` to the row. The frontend polls the job detail endpoint every 5 seconds while a job is active, so this adds one write per poll. This lets the code merge and be tested before DNS and the certificate exist, and lights up when they land. + +## Design + +### 1. Persist `service_url` to the database + +`auth_request` fires once per HTTP request, and a single JupyterLab page load is dozens of requests. Today `service_url` lives only as a file in the user's NFS home, read through a per-user worker RPC (`fileglancer/server.py:2699`) because root cannot read user files under NFS root squash. A worker round trip plus an NFS read per proxied request is not viable. + +Add one nullable column: + +```python +# fileglancer/database.py, class JobDB +# Service URL as published by the job to its work directory, cached here so the +# proxy-resolve endpoint can map a hostname to an upstream with a single indexed +# read instead of a per-user worker RPC and an NFS stat per proxied request. +service_url = Column(String, nullable=True) +``` + +One Alembic migration adds it. Chain it off the current head computed from the revision graph, not a remembered value. + +It is written through at the existing read site in `get_job`, where the URL is already being fetched in user context: + +```python +service_url = svc_result.get("service_url") +if service_url and service_url != db_job.service_url: + db_job.service_url = service_url +``` + +No poll-loop change and no new worker action are needed. The job detail page is where the "Open Service" link lives, so a user cannot obtain the proxied URL without first triggering the write. The stored value survives a server restart, so a bookmarked subdomain keeps working. + +The column is never cleared. Staleness is handled at resolve time by the status check, which is a stronger guarantee than clearing. + +### 2. Resolve endpoint + +``` +GET /api/apps/resolve +``` + +Behavior: + +1. Read the `Host` header and match it against `^job-(\d+)\.$`. No match → `403`. +2. Load the job by id. Missing → `403`. +3. Require `entry_point_type == 'service'`, `status == 'RUNNING'`, and a non-null `service_url`. Otherwise → `403`. +4. Parse the stored URL and extract its netloc. +5. Validate the netloc against `^[A-Za-z0-9.-]+:\d{1,5}$`. No match → `403`. +6. Return `204 No Content` with header `X-Fg-Upstream: `. + +Three points carry weight: + +**The `RUNNING` check is a security control, not a nicety.** Compute-node ports are recycled. Without it, a stale subdomain from a finished job would proxy to whatever service now occupies that port on that node — potentially another user's. + +**The netloc regex is the header-injection gate.** nginx performs `proxy_pass http://$upstream` using this header's value, so nothing that fails the pattern is ever echoed. It constrains the authority's *shape* only — hostname and in-range port — not where it points. The value originates from a file the user's own job wrote, so it is untrusted input; the destination is bounded separately. The privilege being withheld is narrow: reaching a service that is reachable *only* from the Fileglancer host. So loopback, the unspecified address and link-local are refused (the app server itself listens on 127.0.0.1:8989, and link-local covers instance metadata), while private and public node addresses are allowed — an address bound to a routable interface is already reachable directly by any cluster user, so proxying to it grants nothing new. The optional `service_proxy_upstream_zone` then confines hostnames to one DNS zone, matched on whole labels; IP literals are exempt from it, since some clusters publish a node's address rather than its name. + +**The endpoint is deliberately unauthenticated.** nginx marks its location `internal`, and the main server block returns `404` for the path so it is not reachable from outside. Even if it were reached, it discloses only a `host:port` that the job detail page already displays to the job's owner. + +Note that resolve does not check that the requesting user owns the job. It cannot: the browser's session cookie is scoped to the Fileglancer hostname and is not sent to the app subdomain. This is intentional and matches the current security model, where possession of the service token is what grants access. It is also why the token stays in the URL (see below). + +### 3. URL rewrite for display + +New setting on `AppsSettings` in `fileglancer/settings.py`: + +```python +# Wildcard DNS zone serving per-job HTTPS subdomains for running services, e.g. +# "services.example.org" to publish https://job-.services.example.org/. Requires a +# matching wildcard certificate and a reverse proxy configured to resolve +# upstreams via /api/apps/resolve. Empty (the default) publishes the service's +# own http://host:port URL unchanged. +service_proxy_domain: str = "" +``` + +Settable as `apps.service_proxy_domain` in `config.yaml` or `FGC_APPS__SERVICE_PROXY_DOMAIN` in the environment. + +When set, `get_job` swaps scheme and netloc while preserving path, query and fragment verbatim — the query string is what carries the token: + +``` +http://h11u02:41235/lab?token=abc → https://job-123.services.int.janelia.org/lab?token=abc +``` + +The stored value stays raw, since it is the upstream the proxy needs. Only the value returned to the client is rewritten. + +The frontend requires no changes: `JobDetail.tsx` renders whatever `job.service_url` contains. + +The setting name and value are deployment configuration. No Janelia hostname, zone, or certificate path appears anywhere in `fileglancer` code or comments; the examples in this document are illustrative of one deployment. + +### 4. nginx configuration + +Lives in the separate deployment repository, alongside the existing `nginx.conf`. It reuses the `$connection_upgrade` map already defined there. + +```nginx +server { + listen 443 ssl http2; + server_name ~^job-(?\d+)\.services\.int\.janelia\.org$; + + ssl_certificate /etc/nginx/certs/services-wildcard.crt; + ssl_certificate_key /etc/nginx/certs/services-wildcard.key; + + location = /_fg_resolve { + internal; + proxy_pass http://127.0.0.1:8989/api/apps/resolve; + proxy_pass_request_body off; + proxy_set_header Content-Length ""; + proxy_set_header Host $host; + } + + location / { + auth_request /_fg_resolve; + auth_request_set $upstream $upstream_http_x_fg_upstream; + + resolver 127.0.0.53 valid=30s; + proxy_pass http://$upstream; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto https; + + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $connection_upgrade; + + proxy_http_version 1.1; + proxy_buffering off; + proxy_read_timeout 3600s; + } +} +``` + +Details that are load-bearing: + +- **`Host $host`** passes the app subdomain through unchanged, so the app sees `Host` and `Origin` as the same value. This is what makes JupyterLab's WebSocket origin check pass without any per-app configuration. +- **`resolver`** is required because `proxy_pass` targets a variable; without it nginx refuses to start. The address shown is systemd-resolved's stub; use whatever the host actually runs. +- **`proxy_buffering off`** and a long `proxy_read_timeout` suit long-lived streaming and WebSocket sessions. +- The main server block gains `location = /api/apps/resolve { return 404; }` so the endpoint is not reachable on the primary hostname. + +The HTTP-to-HTTPS redirect server block is already `default_server` with `server_name _`, so it covers the new subdomains without modification. + +## Testing + +Backend tests in `tests/`, run with `pixi run -e test test-backend`: + +Resolve endpoint: +- Happy path: running service job with a stored URL returns `204` and the correct `X-Fg-Upstream`. +- Job is not a service entry point → `403`. +- Job is not `RUNNING` → `403`. This is the port-recycling case and deserves an explicit test. +- Job has no stored `service_url` → `403`. +- Host does not match the expected pattern, or `service_proxy_domain` is unset → `403`. +- Stored `service_url` has a malformed netloc → `403`, and the response carries no `X-Fg-Upstream` header. This is the SSRF regression test. + +URL rewrite: +- Query string and fragment survive the rewrite intact. +- With `service_proxy_domain` empty, `get_job` returns the raw URL unchanged. + +Write-through: +- Fetching a running service job persists `service_url` to the row. + +nginx cannot be tested here. Verify it by hand once DNS and the certificate exist, against each of the five shipped apps. + +## Risks + +**Per-app WebSocket origin validation.** Passing `Host $host` should satisfy JupyterLab and marimo, whose checks compare `Origin` against `Host`. openvscode-server and the noVNC desktop cannot be proven until there is real DNS to test against. Verification is cheap once the zone exists and impossible before it. If an app does reject the proxied origin, the fallback is per-app configuration in that app's manifest, not a change to this design. + +**One indexed database read per proxied request, cached.** A page load is dozens of resolves, so successful resolutions are held in a bounded TTL cache (10 seconds, 1024 entries) and the endpoint is excluded from the per-request access log, reporting aggregate totals once a minute instead. Refusals are not cached, so a starting service resolves as soon as it publishes. The TTL is the window in which a stopped job can still be proxied, which is why it is seconds rather than minutes. Each uvicorn worker holds its own cache, so expect up to one miss per worker per TTL. + +**Configuration split across two repositories.** The setting lives in `fileglancer`, the nginx block in the deployment repository. They must be changed together. Both sides should carry a comment pointing at the other. + +## Out of scope + +- Authentication at the proxy. The service token remains the only credential. +- Hiding the token from the URL. Doing so requires a subdomain-scoped cookie handshake and proxy-side token injection; it is a real feature with real value, but it is a separate piece of work. +- Sharing a running service with users other than the owner. +- A path-prefix fallback for deployments without wildcard DNS. +- Removing the direct `http://host:port` URL. It remains reachable, and the job's `service_url` file remains visible in the job files panel as a fallback when the proxy misbehaves. diff --git a/fileglancer/alembic/versions/c3e9b7f41a28_add_job_service_url.py b/fileglancer/alembic/versions/c3e9b7f41a28_add_job_service_url.py new file mode 100644 index 000000000..848e4f9af --- /dev/null +++ b/fileglancer/alembic/versions/c3e9b7f41a28_add_job_service_url.py @@ -0,0 +1,24 @@ +"""add service_url to jobs + +Revision ID: c3e9b7f41a28 +Revises: f2a8c1d94e60 +Create Date: 2026-08-28 00:00:00.000000 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'c3e9b7f41a28' +down_revision = 'f2a8c1d94e60' +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column('jobs', sa.Column('service_url', sa.String(), nullable=True)) + + +def downgrade() -> None: + op.drop_column('jobs', 'service_url') diff --git a/fileglancer/apps/__init__.py b/fileglancer/apps/__init__.py index 45f0ce0b1..581f608e1 100644 --- a/fileglancer/apps/__init__.py +++ b/fileglancer/apps/__init__.py @@ -45,3 +45,14 @@ get_service_url, get_service_phase, ) +from fileglancer.apps.serviceproxy import ( # noqa: F401 + build_proxied_service_url, + cache_upstream, + cached_upstream, + job_id_from_host, + record_resolve, + reset_resolve_metrics, + resolve_counts, + service_host_label, + upstream_from_service_url, +) diff --git a/fileglancer/apps/serviceproxy.py b/fileglancer/apps/serviceproxy.py new file mode 100644 index 000000000..7496cb2d7 --- /dev/null +++ b/fileglancer/apps/serviceproxy.py @@ -0,0 +1,329 @@ +"""URL helpers for serving app services behind an HTTPS reverse proxy. + +A service job publishes ``http://:`` to its work directory. +When a proxy domain is configured, Fileglancer republishes that as +``https://job--.`` and tells the reverse proxy +which upstream the hostname maps to. These functions are the whole translation +layer between the two forms, kept pure so they can be tested without a database. +""" + +import base64 +import hashlib +import hmac +import ipaddress +import re +import time +from collections import Counter +from functools import lru_cache +from typing import Iterable, Optional +from urllib.parse import urlsplit, urlunsplit + +from cachetools import TTLCache +from loguru import logger + +# An upstream nginx can hand to proxy_pass: hostname and explicit port, nothing +# else. Deliberately strict — the value is interpolated into a proxy_pass +# directive, so userinfo, whitespace, CR/LF and bracketed IPv6 literals are all +# rejected rather than escaped. +_UPSTREAM_RE = re.compile(r'^([A-Za-z0-9.-]+):(\d{1,5})$') + +# Hostnames that always name the local machine, so an upstream naming one would +# aim the proxy at the web host itself rather than at a compute node. +_LOCAL_NAMES = frozenset({'localhost'}) + +# Job ids are a small global sequence, so a bare job- label would let anyone +# who can reach the proxy sweep job-1..job-500 and find every running service on +# the instance — and reach any of them that does not enforce its own token. The +# label therefore carries a MAC over the id, keyed by the server's session +# secret. Signing rather than storing a random key keeps the id in the hostname, +# so resolution stays one indexed read with no extra column, and the MAC is +# checked before the database is touched at all. +# +# Eight base32 characters is 40 bits. Guessing is online-only — a wrong label is +# a 403 from the reverse proxy, and there is nothing to attack offline — so at +# 10k requests/second a sweep of the space takes decades. Rotating +# session_secret_key invalidates live service URLs, which is the same blast +# radius as the session revocation that rotation already causes. +_MAC_CHARS = 8 + + +def _service_mac(job_id: int, secret: str) -> str: + """The MAC for a job's proxy hostname label.""" + digest = hmac.new(secret.encode('utf-8'), + f'job:{job_id}'.encode('utf-8'), + hashlib.sha256).digest() + # base32 rather than hex: same DNS-safe alphabet cost, 25% fewer characters + # per bit. Lowercased because hostnames are compared case-insensitively. + return base64.b32encode(digest).decode('ascii').lower()[:_MAC_CHARS] + + +def service_host_label(job_id: int, secret: str) -> str: + """The leftmost hostname label for a job's proxy URL, e.g. ``job-12-k7m2qhxr``.""" + return f'job-{job_id}-{_service_mac(job_id, secret)}' + + +def build_proxied_service_url(service_url: Optional[str], job_id: int, + proxy_domain: str, secret: str) -> Optional[str]: + """Rewrite a published service URL to its HTTPS proxy form. + + Path, query and fragment are carried over verbatim: the query string holds + the service's own access token, which remains the only credential. + + Returns None when there is nothing to rewrite, no proxy domain is + configured, or no secret is available to sign the hostname with, in which + case the caller should publish the URL unchanged. + """ + if not service_url or not proxy_domain or not secret: + return None + parts = urlsplit(service_url) + return urlunsplit(( + 'https', + f'{service_host_label(job_id, secret)}.{proxy_domain}', + parts.path, + parts.query, + parts.fragment, + )) + + +def job_id_from_host(host: Optional[str], proxy_domain: str, + secret: str) -> Optional[int]: + """Extract the job id from a proxy hostname, or None if it isn't one. + + Matches the whole hostname, so neither a longer suffix + (``job-1-k7m2qhxr.services.example.org.evil``) nor an extra label + (``x.job-1-k7m2qhxr.services.example.org``) is accepted, and the label's MAC + must verify against the id it carries. + """ + if not host or not proxy_domain or not secret: + return None + # $host in nginx normally omits the port, but a client can send one. + hostname = host.split(':', 1)[0].strip().lower() + match = re.fullmatch( + r'job-(\d{1,9})-([a-z2-7]{%d})\.' % _MAC_CHARS + + re.escape(proxy_domain.lower()), hostname) + if match is None: + return None + job_id = int(match.group(1)) + if not hmac.compare_digest(match.group(2), _service_mac(job_id, secret)): + return None + return job_id + + +def _is_ip_literal(host: str) -> bool: + """Whether an upstream host is an IP address rather than a DNS name.""" + try: + ipaddress.ip_address(host.strip().rstrip('.')) + return True + except ValueError: + return False + + +def _host_matches_zone(host: str, allowed_zone: str) -> bool: + """Whether an upstream host sits inside the configured allowed zone. + + Matching is on whole DNS labels, not raw text: a plain ``endswith`` would + let ``evil-nodes.example.org`` satisfy a suffix of ``nodes.example.org``, + which turns the allowlist into a prefix-guessing game the moment an + operator omits the leading dot. Writing the suffix with or without that dot + therefore means the same thing, and the zone's own name is allowed to be + the upstream. A trailing root dot on either side is insignificant in DNS + and is ignored here too. + + An IP literal is exempt rather than refused: it has no DNS zone to compare + against, and some clusters publish a node's address instead of its name, so + applying a zone here would silently break them. Literals are bounded by + _is_safe_upstream_host instead. + """ + if _is_ip_literal(host): + return True + zone = allowed_zone.lower().strip().rstrip('.').lstrip('.') + if not zone: + return True + name = host.lower().rstrip('.') + return name == zone or name.endswith('.' + zone) + + +@lru_cache(maxsize=8) +def _parse_networks(networks: tuple) -> tuple: + """Parse CIDR strings once per distinct configuration, not per request. + + Entries are validated at startup, so anything unparseable here is dropped + rather than raised: this runs on every proxied request, and a config error + should have already stopped the server. + """ + parsed = [] + for entry in networks: + try: + parsed.append(ipaddress.ip_network(entry.strip(), strict=False)) + except ValueError: + continue + return tuple(parsed) + + +def _host_in_networks(host: str, networks: Iterable[str]) -> bool: + """Whether an upstream address falls inside one of the allowed networks. + + A hostname is exempt: there is no address to compare without a DNS lookup, + which this path must not do. Names are governed by the zone check instead, + so the two settings divide the space between them rather than overlapping. + """ + allowed = _parse_networks(tuple(networks)) + if not allowed: + return True + try: + addr = ipaddress.ip_address(host.strip().rstrip('.')) + except ValueError: + return True + # Comparing across families raises, so pair each address with its own. + return any(addr in net for net in allowed if net.version == addr.version) + + +def _is_safe_upstream_host(host: str) -> bool: + """Reject upstreams that would aim the proxy at the Fileglancer host itself. + + The upstream comes from a file the user's own job wrote, and the reverse + proxy dials it from the Fileglancer host rather than from the compute node. + The privilege that must not be handed out is therefore narrow and specific: + reaching a service that is *only* reachable from that host. + + In practice that means loopback and the unspecified address, which catch + anything bound to localhost — the app server itself listens on + 127.0.0.1:8989 — plus link-local, which covers cloud instance-metadata + endpoints. Multicast and reserved ranges are refused as well; they are not a + threat, they are simply never a service. + + Private and public addresses are deliberately allowed. Some clusters publish + a node's IP rather than its hostname, and an address bound to a routable + interface is already reachable directly by any cluster user, so proxying to + it grants nothing new. Blocking RFC 1918 would break real deployments to + prevent nothing. + + Checks are textual and literal-only. Resolving a name here would mean a + blocking DNS lookup on every proxied request; confining names to one zone is + the job of service_proxy_upstream_zone. + """ + name = host.lower().strip().rstrip('.') + if name in _LOCAL_NAMES or name.endswith('.localhost'): + return False + try: + addr = ipaddress.ip_address(name) + except ValueError: + return True # A DNS name; bounded by the zone check when one is set. + return not ( + addr.is_loopback or addr.is_unspecified or addr.is_link_local + or addr.is_multicast or addr.is_reserved + ) + + +def upstream_from_service_url(service_url: Optional[str], + allowed_zone: str = "", + allowed_networks: Iterable[str] = ()) -> Optional[str]: + """Extract a ``host:port`` upstream from a published service URL. + + Returns None unless the authority is exactly a hostname and an in-range + port. The netloc regex is a header-injection gate — it constrains the + authority's shape only, since the result is interpolated into the reverse + proxy's ``proxy_pass`` target. The destination itself (where the proxy + actually dials) is bounded separately: hosts that reach the Fileglancer host + itself are rejected; ``allowed_zone``, when set, confines upstreams + published as hostnames to one DNS zone; and ``allowed_networks``, when + set, confines upstreams published as addresses to those CIDRs. The two + allowlists divide the space rather than overlapping, since a bare address + has no zone and a name has no address without a DNS lookup. All of this + matters because the source string is a file written by the user's own job. + """ + if not service_url: + return None + try: + netloc = urlsplit(service_url).netloc + except ValueError: + return None + match = _UPSTREAM_RE.fullmatch(netloc) + if match is None: + return None + port = int(match.group(2)) + if not 1 <= port <= 65535: + return None + host = match.group(1) + if not _is_safe_upstream_host(host): + return None + if allowed_zone and not _host_matches_zone(host, allowed_zone): + return None + if not _host_in_networks(host, allowed_networks): + return None + return netloc + + +# --- Resolution cache and counters --- +# +# The reverse proxy calls the resolve endpoint once per proxied HTTP request, so +# one JupyterLab page load is dozens of calls and a long WebSocket session keeps +# adding them. Without a cache each of those is a database round trip, and each +# one would also emit an access-log line carrying no information. +# +# Only successful resolutions are cached. A miss for a service that has not +# published its URL yet must stay a miss, or clicking "Open Service" the moment a +# service comes up would fail for the whole TTL. +# +# The TTL is deliberately short. It is the window during which a job that has +# stopped can still be proxied, and the RUNNING check it bypasses exists because +# compute-node ports get recycled. A page load's burst is sub-second, so a few +# seconds captures nearly all of the benefit while keeping that window small. +# Under `uvicorn --workers N` each worker keeps its own cache, so expect up to N +# misses per TTL rather than one; that is still bounded and small. +_RESOLVE_CACHE_TTL_SECONDS = 10 +_RESOLVE_CACHE_MAX_ENTRIES = 1024 +_RESOLVE_LOG_INTERVAL_SECONDS = 60 + +_resolve_cache = TTLCache(maxsize=_RESOLVE_CACHE_MAX_ENTRIES, + ttl=_RESOLVE_CACHE_TTL_SECONDS) +_resolve_counts = Counter() +_resolve_last_logged = 0.0 + + +def cached_upstream(job_id: int) -> Optional[str]: + """Return a recently resolved upstream for a job, or None to consult the DB.""" + return _resolve_cache.get(job_id) + + +def cache_upstream(job_id: int, upstream: str) -> None: + """Remember a successful resolution for the cache's short TTL.""" + _resolve_cache[job_id] = upstream + + +def record_resolve(outcome: str) -> None: + """Count one resolve outcome, and periodically log the running totals. + + Counting replaces per-request access logging for this endpoint: an aggregate + every minute says the same thing as hundreds of individual lines, and says it + in a form an operator can actually read. Outcomes are coarse on purpose — + 'hit', 'miss', and a refusal reason — so the line stays useful without + naming any specific job. + """ + global _resolve_last_logged + _resolve_counts[outcome] += 1 + now = time.monotonic() + if _resolve_last_logged == 0.0: + # First call: start the clock rather than logging a one-request summary. + _resolve_last_logged = now + return + if now - _resolve_last_logged < _RESOLVE_LOG_INTERVAL_SECONDS: + return + _resolve_last_logged = now + totals = ' '.join(f'{name}={count}' + for name, count in sorted(_resolve_counts.items())) + logger.info(f"service proxy resolve totals: {totals} " + f"cached={len(_resolve_cache)}") + + +def reset_resolve_metrics() -> None: + """Clear the cache and counters. For tests, and for nothing else.""" + global _resolve_last_logged + _resolve_cache.clear() + _resolve_counts.clear() + _resolve_last_logged = 0.0 + + +def resolve_counts() -> dict: + """Snapshot of the outcome counters, for tests and diagnostics.""" + return dict(_resolve_counts) diff --git a/fileglancer/database.py b/fileglancer/database.py index 1f1403704..9e4f56ff0 100644 --- a/fileglancer/database.py +++ b/fileglancer/database.py @@ -196,6 +196,11 @@ class JobDB(Base): # detail endpoint build browse links without realpath'ing mounts per read. work_dir_fsp_name = Column(String, nullable=True) work_dir_subpath = Column(String, nullable=True) + # Service URL as published by the job to its work directory, cached here so + # the proxy-resolve endpoint can map a hostname to an upstream with one + # indexed read, instead of a per-user worker RPC and an NFS stat on every + # proxied request. NULL until the job publishes a URL and it is first read. + service_url = Column(String, nullable=True) # Commit whose code this job executed (the code repo's SHA when the # manifest declares a separate repo_url, else the app repo's SHA). NULL for # jobs submitted before commit pinning existed. @@ -1191,6 +1196,26 @@ def get_job(session: Session, job_id: int, username: str) -> Optional[JobDB]: return session.query(JobDB).filter_by(id=job_id, username=username).first() +def get_job_by_id(session: Session, job_id: int) -> Optional[JobDB]: + """Get a job by ID without an ownership check. + + The proxy-resolve endpoint runs with no authenticated user — the browser's + session cookie is scoped to the Fileglancer hostname and is never sent to an + app subdomain — so it cannot use get_job(). Callers that do have a user must + keep using get_job(), which filters by username. + """ + return session.query(JobDB).filter_by(id=job_id).first() + + +def set_job_service_url(session: Session, job_id: int, service_url: str) -> None: + """Cache a job's published service URL on its row.""" + job = session.query(JobDB).filter_by(id=job_id).first() + if job is None or job.service_url == service_url: + return + job.service_url = service_url + session.commit() + + def update_job(session: Session, job_id: int, username: str, name: str) -> Optional[JobDB]: """Rename a job. Returns the job, or None if it doesn't exist or isn't diff --git a/fileglancer/log.py b/fileglancer/log.py index 08d2d0929..81d7be86a 100644 --- a/fileglancer/log.py +++ b/fileglancer/log.py @@ -50,8 +50,22 @@ def __init__(self, app: ASGIApp, settings: Settings): super().__init__(app) self.settings = settings + # Paths that are logged in aggregate elsewhere rather than per request. The + # service proxy's resolve endpoint is called once per proxied HTTP request, + # so a single app page load would emit dozens of identical 204 lines that + # carry no user, no useful path and no useful duration — and would skew the + # duration percentiles for the endpoints that do. It reports running totals + # once a minute instead; see fileglancer/apps/serviceproxy.py. + _AGGREGATED_PATHS = frozenset({"/api/apps/resolve"}) + async def dispatch(self, request: Request, call_next: Callable) -> Response: """Process request and log access information""" + # Checked before anything else so an aggregated path also skips the + # session-cookie lookup below — that is signature work per request, on a + # cookie a proxied app subdomain never sends in the first place. + if request.url.path in self._AGGREGATED_PATHS: + return await call_next(request) + start_time = time.perf_counter() # Extract username from session (if authenticated) diff --git a/fileglancer/server.py b/fileglancer/server.py index 62c6b2bee..46e5fac98 100644 --- a/fileglancer/server.py +++ b/fileglancer/server.py @@ -23,7 +23,7 @@ from contextlib import asynccontextmanager from fastapi import FastAPI, HTTPException, Request, Query, Path, Body, Depends from fastapi.middleware.cors import CORSMiddleware -from fastapi.responses import RedirectResponse, Response, JSONResponse, PlainTextResponse, StreamingResponse, FileResponse +from fastapi.responses import RedirectResponse, Response, JSONResponse, PlainTextResponse, StreamingResponse, FileResponse, HTMLResponse from fastapi.exceptions import RequestValidationError, StarletteHTTPException from fastapi.staticfiles import StaticFiles from starlette.middleware.sessions import SessionMiddleware @@ -361,6 +361,54 @@ def _validate_short_name(short_name: str) -> None: raise HTTPException(status_code=400, detail="short_name can only contain letters, numbers, hyphens, and underscores") +# Shown at the URL the user clicked when the service proxy refuses to resolve a +# hostname, which is almost always a service whose job has since stopped. The +# reverse proxy maps its own 403 here (see docs/ServiceProxy.md), so this ships +# with Fileglancer rather than living in each deployment's nginx html directory. +# +# Self-contained on purpose: the proxy's server block routes everything through +# auth_request, so a stylesheet or image referenced from here would be refused +# by the same check that produced this page. No link back to Fileglancer either +# — the app has no configured public base URL, and this page is served on a +# subdomain that cannot be assumed to share one with it. +_SERVICE_UNAVAILABLE_HTML = """ + + + + +Service not running + + + +
+

503 Service Unavailable

+

The app that was served at this address is no longer available. Services + are reachable only while their job is running.

+
    +
  • The job finished, was cancelled, or hit its time limit.
    • +
  • The job is still starting up and hasn't published its address yet — + wait a moment and reload.
    • +
  • The link is old. Each launch gets its own address, so a bookmark from a + previous run will not work.
    • +
+

Open the job in Fileglancer to check its status or launch the + app again.

+
+ + +""" + + def create_app(settings): # Initialize OAuth client for OKTA @@ -2700,9 +2748,92 @@ async def get_job(job_id: int, job=serialize_job_for_worker(db_job)) service_url = svc_result.get("service_url") phase = svc_result.get("phase") + if service_url: + # Cache it so /api/apps/resolve can map a proxy hostname + # to an upstream without a worker RPC per request. + db.set_job_service_url(session, job_id, service_url) except Exception: - pass - return _convert_job(db_job, service_url=service_url, files=files, phase=phase) + logger.debug( + f"Could not resolve or cache the service URL for job {job_id}", + exc_info=True) + # The cached value stays raw — it is the proxy's upstream. Only what + # goes back to the browser is rewritten. + proxied = apps_module.build_proxied_service_url( + service_url, job_id, settings.apps.service_proxy_domain, + settings.session_secret_key) + return _convert_job(db_job, service_url=proxied or service_url, + files=files, phase=phase) + + @app.get("/api/apps/resolve", include_in_schema=False) + async def resolve_service_upstream(request: Request): + """Map a service proxy hostname to its upstream, for the reverse proxy. + + Called once per proxied request via nginx's auth_request, so it must stay + a single indexed read. Deliberately unauthenticated: the session cookie + is scoped to the Fileglancer hostname and is never sent to an app + subdomain, so there is no user to check. It discloses only a host:port + that the job's detail page already shows, and the reverse proxy marks its + location `internal` so it is not reachable from outside. + + Returns 204 with X-Fg-Upstream on success and 403 for everything else, so + auth_request denies the request. + + Successful resolutions are cached for a few seconds, which is also the + window in which a job that has just stopped can still be proxied. See + the cache notes in fileglancer/apps/serviceproxy.py for why that window + is deliberately short. + """ + proxy_domain = settings.apps.service_proxy_domain + job_id = apps_module.job_id_from_host( + request.headers.get('host'), proxy_domain, + settings.session_secret_key) + if job_id is None: + apps_module.record_resolve("refused_bad_host") + raise HTTPException(status_code=403, detail="Not a service proxy host") + + # A page load is dozens of these, so serve repeats from the short-lived + # cache and keep the database out of the hot path. Only hits are cached; + # a service that has not published its URL yet must be able to start + # resolving the moment it does. + upstream = apps_module.cached_upstream(job_id) + if upstream is not None: + apps_module.record_resolve("hit") + return Response(status_code=204, + headers={"X-Fg-Upstream": upstream}) + + with db.get_db_session(settings.db_url) as session: + db_job = db.get_job_by_id(session, job_id) + if (db_job is None + or getattr(db_job, 'entry_point_type', 'job') != 'service' + or db_job.status != 'RUNNING'): + apps_module.record_resolve("refused_not_running") + raise HTTPException(status_code=403, detail="No running service for this host") + upstream = apps_module.upstream_from_service_url( + db_job.service_url, + allowed_zone=settings.apps.service_proxy_upstream_zone, + allowed_networks=tuple(settings.apps.service_proxy_upstream_networks)) + + if upstream is None: + apps_module.record_resolve("refused_no_upstream") + raise HTTPException(status_code=403, detail="No usable upstream for this host") + + apps_module.cache_upstream(job_id, upstream) + apps_module.record_resolve("miss") + return Response(status_code=204, headers={"X-Fg-Upstream": upstream}) + + @app.get("/api/apps/service-unavailable", include_in_schema=False) + async def service_unavailable_page(): + """The page a user sees when a service proxy hostname no longer resolves. + + The reverse proxy renders this in place of its own 403, at the URL the + user clicked. 503 rather than 403 because "not running" is what actually + happened in every case a real user reaches: a refusal from + /api/apps/resolve means the job is gone, still starting, or was never + theirs to reach, and none of those is worth distinguishing to a browser. + Unauthenticated for the same reason the resolve endpoint is — the session + cookie is scoped to the Fileglancer hostname and is never sent here. + """ + return HTMLResponse(_SERVICE_UNAVAILABLE_HTML, status_code=503) @app.patch("/api/jobs/{job_id}", response_model=Job, description="Rename a job") diff --git a/fileglancer/settings.py b/fileglancer/settings.py index 9a6237172..bff67106d 100644 --- a/fileglancer/settings.py +++ b/fileglancer/settings.py @@ -1,5 +1,6 @@ from typing import List, Optional from functools import cache +import ipaddress import sys from pydantic import BaseModel, HttpUrl, ValidationError, field_validator, model_validator @@ -51,6 +52,55 @@ class AppsSettings(BaseModel): # it. This is Fileglancer poll policy, not py-cluster-api config, so it # lives here rather than under `cluster`. Set to 0 to disable the cutoff. unknown_timeout_hours: float = 24.0 + # Wildcard DNS zone serving per-job HTTPS subdomains for running services, + # e.g. "services.example.org" to publish + # https://job-.services.example.org/. Requires a matching wildcard + # certificate and a reverse proxy configured to resolve upstreams via + # GET /api/apps/resolve. Empty (the default) publishes the service's own + # http://host:port URL unchanged. + service_proxy_domain: str = "" + # Restricts which hosts the service proxy will dial. When set, a service's + # published upstream hostname must sit inside this DNS zone, e.g. + # "nodes.example.org" to allow only cluster nodes. Matching is on whole DNS + # labels, so a leading dot is optional and "evil-nodes.example.org" does not + # satisfy a zone of "nodes.example.org". Applies to hostnames only: a service + # that publishes the node's IP address is still accepted, since some clusters + # do that and a literal has no zone to match. Recommended wherever + # service_proxy_domain is set: the upstream comes from a file the user's job + # wrote, so without a zone the proxy will dial any host the Fileglancer host + # can reach. Empty disables the check. + service_proxy_upstream_zone: str = "" + # Restricts which IP addresses the service proxy will dial, as a list of + # CIDR networks, e.g. ["10.20.0.0/16"] to allow only the cluster's node + # subnet. The companion to service_proxy_upstream_zone: that one confines + # upstreams published as hostnames, this one confines upstreams published as + # addresses, since a bare address has no DNS zone to match. IPv6 entries + # parse but cannot match anything today: the upstream authority pattern + # forbids colons in the host, so no IPv6 literal ever reaches this check. + # Empty disables the check, in which case any + # address is allowed except those that reach the Fileglancer host itself + # (loopback, unspecified, link-local, multicast and reserved are always + # refused regardless of this setting). + service_proxy_upstream_networks: List[str] = [] + + @field_validator('service_proxy_upstream_networks') + @classmethod + def validate_service_proxy_upstream_networks(cls, v: List[str]) -> List[str]: + """Reject unparseable CIDR entries at startup. + + A typo here fails closed — every address upstream stops resolving — so + it is far better to refuse to start than to let the proxy look + configured while silently rejecting every service. + """ + for entry in v: + try: + ipaddress.ip_network(entry.strip(), strict=False) + except ValueError as exc: + raise ValueError( + f"apps.service_proxy_upstream_networks contains an invalid " + f"network {entry!r}: {exc}" + ) from exc + return v class Settings(BaseSettings): @@ -235,6 +285,22 @@ def settings_customise_sources( # noqa: PLR0913 file_secret_settings, ) + @model_validator(mode='after') + def validate_service_proxy_needs_a_session_secret(self): + """The service proxy signs its hostnames with session_secret_key. + + An unset key is generated at random per process, so under + `uvicorn --workers N` each worker would sign with a different key and + most proxied requests would be refused. Fail at startup instead, where + the cause is obvious. + """ + if self.apps.service_proxy_domain and not self.session_secret_key: + raise ValueError( + "apps.service_proxy_domain requires session_secret_key to be " + "set: service proxy hostnames are signed with it, and a " + "per-process random key breaks resolution across workers") + return self + @model_validator(mode='after') def set_jira_browse_url(self): if self.jira_browse_url is None: diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 70fcadf92..1ebd287cf 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1,12 +1,12 @@ { "name": "fileglancer", - "version": "3.1.0", + "version": "3.2.0-a1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "fileglancer", - "version": "3.1.0", + "version": "3.2.0-a1", "license": "BSD-3-Clause", "dependencies": { "@bioimagetools/capability-manifest": "0.7.1", diff --git a/frontend/package.json b/frontend/package.json index 1967f3b2f..3db051d95 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "fileglancer", "type": "module", - "version": "3.1.0", + "version": "3.2.0-a1", "description": "Browse, share, and publish files on the Janelia file system", "keywords": [ "ngff", diff --git a/pixi.lock b/pixi.lock index 84132dda3..75dddeab1 100644 --- a/pixi.lock +++ b/pixi.lock @@ -866,7 +866,7 @@ environments: - conda: https://conda.anaconda.org/conda-forge/noarch/tomlkit-0.15.1-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/trove-classifiers-2026.6.1.19-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/truststore-0.10.4-pyhcf101f3_0.conda - - conda: https://conda.anaconda.org/conda-forge/noarch/twine-6.2.0-pyhcf101f3_0.conda + - conda: https://conda.anaconda.org/conda-forge/noarch/twine-7.0.0-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typer-0.27.0-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typing-extensions-4.16.0-h69aa097_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typing-inspection-0.4.2-pyhcf101f3_2.conda @@ -1081,7 +1081,7 @@ environments: - conda: https://conda.anaconda.org/conda-forge/noarch/tomlkit-0.15.1-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/trove-classifiers-2026.6.1.19-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/truststore-0.10.4-pyhcf101f3_0.conda - - conda: https://conda.anaconda.org/conda-forge/noarch/twine-6.2.0-pyhcf101f3_0.conda + - conda: https://conda.anaconda.org/conda-forge/noarch/twine-7.0.0-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typer-0.27.0-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typing-extensions-4.16.0-h69aa097_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typing-inspection-0.4.2-pyhcf101f3_2.conda @@ -1222,7 +1222,7 @@ environments: - conda: https://conda.anaconda.org/conda-forge/noarch/tomlkit-0.15.1-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/trove-classifiers-2026.6.1.19-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/truststore-0.10.4-pyhcf101f3_0.conda - - conda: https://conda.anaconda.org/conda-forge/noarch/twine-6.2.0-pyhcf101f3_0.conda + - conda: https://conda.anaconda.org/conda-forge/noarch/twine-7.0.0-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typer-0.27.0-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typing-extensions-4.16.0-h69aa097_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typing-inspection-0.4.2-pyhcf101f3_2.conda @@ -1429,7 +1429,7 @@ environments: - conda: https://conda.anaconda.org/conda-forge/noarch/tomlkit-0.15.1-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/trove-classifiers-2026.6.1.19-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/truststore-0.10.4-pyhcf101f3_0.conda - - conda: https://conda.anaconda.org/conda-forge/noarch/twine-6.2.0-pyhcf101f3_0.conda + - conda: https://conda.anaconda.org/conda-forge/noarch/twine-7.0.0-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typer-0.27.0-pyhcf101f3_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typing-extensions-4.16.0-h69aa097_0.conda - conda: https://conda.anaconda.org/conda-forge/noarch/typing-inspection-0.4.2-pyhcf101f3_2.conda @@ -9551,28 +9551,28 @@ packages: - pkg:pypi/truststore?source=hash-mapping size: 24279 timestamp: 1766494826559 -- conda: https://conda.anaconda.org/conda-forge/noarch/twine-6.2.0-pyhcf101f3_0.conda - sha256: 0370098cab22773e33755026bf78539c2f05645fce7dcc9713d01e21950756bb - md5: 901a86453fa6183e914b937643619a03 +- conda: https://conda.anaconda.org/conda-forge/noarch/twine-7.0.0-pyhcf101f3_0.conda + sha256: 9abcd51d09a4d2da757861ad4684b02ae82ff721db6644e6fe477c5fd7e9278c + md5: e1d7495efcd3d0bc79bdc6cc5a34b8ba depends: - id - - importlib-metadata >=3.6 - keyring >=21.2.0 - - packaging >=24.0 + - packaging >=26.1 - python >=3.10 - readme_renderer >=35.0 - requests >=2.20 - requests-toolbelt >=0.8.0,!=0.9.0 - rfc3986 >=1.4.0 - - rich >=12.0.0 + - rich >=14.3.3 - urllib3 >=1.26.0 - python license: Apache-2.0 license_family: APACHE purls: - pkg:pypi/twine?source=hash-mapping - size: 42488 - timestamp: 1757013705407 + run_exports: {} + size: 42936 + timestamp: 1785252141368 - conda: https://conda.anaconda.org/conda-forge/noarch/typer-0.27.0-pyhcf101f3_0.conda sha256: f44984374b248c45f7e3d43a2cb0bf73d1441ecea07fb73198e89f643f6c5824 md5: aa0ef00569728be93656de5f097fc81d @@ -13062,7 +13062,7 @@ packages: - uvicorn==0.38.0 - x2s3==1.5.2 - hatch==1.17.1 ; extra == 'release' - - twine==6.2.0 ; extra == 'release' + - twine==7.0.0 ; extra == 'release' - coverage==7.15.1 ; python_full_version == '3.12.*' and platform_machine == 'x86_64' and sys_platform == 'darwin' and extra == 'test' - coverage==7.15.2 ; (python_full_version != '3.12.*' and extra == 'test') or (platform_machine != 'x86_64' and extra == 'test') or (sys_platform != 'darwin' and extra == 'test') - httpx2==2.9.1 ; extra == 'test' diff --git a/pyproject.toml b/pyproject.toml index 8740f63ba..a97e79edf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,7 +59,7 @@ test = [ "httpx2 ==2.9.1" ] release = [ - "twine ==6.2.0", + "twine ==7.0.0", "hatch ==1.17.1" ] @@ -121,7 +121,7 @@ pypi-build = "pip install build && python -m build" pypi-upload = "twine upload dist/*" [tool.pixi.feature.release.dependencies] -twine = ">=6.2.0,<7" +twine = ">=7.0.0,<8" hatch = ">=1.14.1,<2" [tool.pixi.feature.release.pypi-dependencies] diff --git a/tests/conftest.py b/tests/conftest.py index 8ceb2409e..5edf458e5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -30,6 +30,19 @@ def _can_create_symlink(): ) +@pytest.fixture(autouse=True) +def _clear_service_proxy_cache(): + """Drop the service-proxy resolution cache between tests. + + It is keyed by job id only — the proxy resolves before any user is known — + so two tests that reuse an id would otherwise read each other's upstream. + """ + from fileglancer.apps import serviceproxy + serviceproxy.reset_resolve_metrics() + yield + serviceproxy.reset_resolve_metrics() + + @pytest.fixture(autouse=True) def _clear_fsp_caches(): """Drop the file-share-path caches between tests. diff --git a/tests/test_log.py b/tests/test_log.py index 7a75c816d..92b392ffa 100644 --- a/tests/test_log.py +++ b/tests/test_log.py @@ -5,7 +5,7 @@ import logging import pytest -from fastapi import FastAPI, Request +from fastapi import FastAPI, Request, Response from fastapi.testclient import TestClient from loguru import logger @@ -219,3 +219,33 @@ def test_text_mode_is_the_default_and_stays_human_readable(capsys): assert '"GET /api/content/share/file.txt HTTP/1.1" 200 - ' in lines[0] assert "{" not in lines[0], lines[0] + + +def test_the_resolve_endpoint_is_not_logged_per_request(): + """The service proxy calls it once per proxied HTTP request, so one app page + load would bury the log in identical 204 lines and skew the duration + percentiles for endpoints that carry real work. It reports totals instead.""" + app = FastAPI() + app.add_middleware(AccessLogMiddleware, settings=Settings()) + + @app.get("/api/apps/resolve") + async def resolve(): + return Response(status_code=204) + + @app.get("/api/content/{rest:path}") + async def content(rest: str): + return {"ok": True} + + lines = [] + sink_id = logger.add(lambda msg: lines.append(msg), format="{message}") + try: + with TestClient(app) as client: + for _ in range(5): + client.get("/api/apps/resolve") + client.get("/api/content/share/file.zarr") + finally: + logger.remove(sink_id) + + assert len(lines) == 1, lines + assert "file.zarr" in lines[0], lines[0] + assert "resolve" not in lines[0], lines[0] diff --git a/tests/test_service_proxy.py b/tests/test_service_proxy.py new file mode 100644 index 000000000..b6c270cae --- /dev/null +++ b/tests/test_service_proxy.py @@ -0,0 +1,432 @@ +"""Tests for the app service HTTPS proxy: URL caching, rewriting, and resolution.""" + +import os +import shutil +import tempfile + +import pytest +from fastapi.testclient import TestClient +from urllib.parse import urlsplit + +from fileglancer import apps +from fileglancer.settings import Settings, AppsSettings +from fileglancer.server import create_app, get_current_user +from fileglancer.database import ( + Base, + create_engine, + dispose_engine, + get_db_session, + create_job, + get_job_by_id, + set_job_service_url, +) + +OWNER = "alice" +PROXY_DOMAIN = "services.example.org" +SECRET = "test-session-secret" + + +def _host(job_id): + """The signed proxy hostname for a job, as the API would publish it.""" + return f"{apps.service_host_label(job_id, SECRET)}.{PROXY_DOMAIN}" + + +@pytest.fixture +def temp_dir(): + d = tempfile.mkdtemp() + yield d + shutil.rmtree(d) + + +@pytest.fixture +def settings_factory(temp_dir): + """Build Settings against a fresh sqlite database, with the proxy domain + configurable so the same fixtures cover both the on and off cases. + + Engines are tracked and disposed on teardown. Windows keeps a lock on an + open sqlite file, so leaving one open makes the temp_dir fixture's rmtree + fail with WinError 32. Teardown runs in reverse dependency order, which puts + this disposal before that rmtree. + """ + engines = [] + + def _build(proxy_domain="", upstream_zone=""): + db_path = os.path.join(temp_dir, "test.db") + db_url = f"sqlite:///{db_path}" + engine = create_engine(db_url) + engines.append(engine) + Base.metadata.create_all(engine) + return Settings( + db_url=db_url, + file_share_mounts=[], + cli_mode=True, + session_secret_key=SECRET, + apps=AppsSettings(service_proxy_domain=proxy_domain, + service_proxy_upstream_zone=upstream_zone), + ), db_url + + yield _build + + for engine in engines: + engine.dispose() + + +@pytest.fixture +def app_factory(settings_factory): + """Build the FastAPI app. Patches fileglancer.settings.get_settings because + database and server code call it directly rather than taking it as an + argument.""" + built = [] + + def _build(proxy_domain="", upstream_zone=""): + settings, db_url = settings_factory(proxy_domain, upstream_zone) + import fileglancer.settings + import fileglancer.database + original = fileglancer.settings.get_settings + fileglancer.settings.get_settings = lambda: settings + fileglancer.database.get_settings = lambda: settings + fileglancer.database._migrations_run = True + built.append((db_url, original)) + return create_app(settings), db_url + + yield _build + + import fileglancer.settings + import fileglancer.database + for db_url, original in built: + dispose_engine(db_url) + fileglancer.settings.get_settings = original + fileglancer.database.get_settings = original + fileglancer.database._migrations_run = False + + +def _seed_service_job(db_url, status="RUNNING", entry_point_type="service"): + session = get_db_session(db_url) + try: + job = create_job( + session, OWNER, "https://github.com/owner/repo", + "My App", "serve", "Server", {}, + entry_point_type=entry_point_type, + ) + job.status = status + session.commit() + return job.id + finally: + session.close() + + +def test_set_job_service_url_persists(app_factory): + _, db_url = app_factory() + job_id = _seed_service_job(db_url) + + session = get_db_session(db_url) + try: + assert get_job_by_id(session, job_id).service_url is None + finally: + session.close() + + session = get_db_session(db_url) + try: + set_job_service_url(session, job_id, "http://node01:41235/lab?token=abc") + finally: + session.close() + + session = get_db_session(db_url) + try: + assert get_job_by_id(session, job_id).service_url == "http://node01:41235/lab?token=abc" + finally: + session.close() + + +def test_get_job_by_id_ignores_username(app_factory): + """Resolution happens with no authenticated user, so the lookup must not + require one.""" + _, db_url = app_factory() + job_id = _seed_service_job(db_url) + session = get_db_session(db_url) + try: + assert get_job_by_id(session, job_id).id == job_id + assert get_job_by_id(session, job_id + 999) is None + finally: + session.close() + + +def test_get_job_caches_service_url(app_factory, monkeypatch): + """Opening the job detail page is the only way to obtain a proxied URL, so + it is where the cache gets populated — no new poll-loop work needed. + + `_worker_exec` is a closure local of `create_app` (not a module attribute), + so it can't be monkeypatched from outside. In CLI mode (used by these + fixtures) it dispatches in-process to the `get_service_url` action, which + calls fileglancer.apps.jobfiles.get_service_url/get_service_phase — patch + those instead, which also exercises the real CLI-mode dispatch path. + """ + app, db_url = app_factory() + job_id = _seed_service_job(db_url) + + import fileglancer.apps.jobfiles as jobfiles + monkeypatch.setattr(jobfiles, "get_service_url", + lambda db_job: "http://node01:41235/lab?token=abc") + monkeypatch.setattr(jobfiles, "get_service_phase", lambda db_job: "running") + + app.dependency_overrides[get_current_user] = lambda: OWNER + client = TestClient(app) + resp = client.get(f"/api/jobs/{job_id}") + app.dependency_overrides.clear() + + assert resp.status_code == 200 + assert resp.json()["service_url"] == "http://node01:41235/lab?token=abc" + + session = get_db_session(db_url) + try: + assert get_job_by_id(session, job_id).service_url == "http://node01:41235/lab?token=abc" + finally: + session.close() + + +# --- /api/apps/resolve --- + +def _resolve(app, host): + return TestClient(app).get("/api/apps/resolve", headers={"Host": host}) + + +def _seed_running_service_with_url(db_url, url="http://node01:41235/lab?token=abc"): + job_id = _seed_service_job(db_url) + session = get_db_session(db_url) + try: + set_job_service_url(session, job_id, url) + finally: + session.close() + return job_id + + +def test_resolve_returns_upstream(app_factory): + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_running_service_with_url(db_url) + resp = _resolve(app, _host(job_id)) + assert resp.status_code == 204 + assert resp.headers["x-fg-upstream"] == "node01:41235" + + +def test_resolve_rejects_finished_job(app_factory): + """Compute-node ports get recycled. A stale subdomain must not be proxied to + whatever service now holds that port on that node.""" + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_running_service_with_url(db_url) + session = get_db_session(db_url) + try: + get_job_by_id(session, job_id).status = "DONE" + session.commit() + finally: + session.close() + resp = _resolve(app, _host(job_id)) + assert resp.status_code == 403 + assert "x-fg-upstream" not in resp.headers + + +def test_resolve_rejects_non_service_job(app_factory): + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_service_job(db_url, entry_point_type="job") + session = get_db_session(db_url) + try: + set_job_service_url(session, job_id, "http://node01:41235/") + finally: + session.close() + assert _resolve(app, _host(job_id)).status_code == 403 + + +def test_resolve_rejects_job_without_cached_url(app_factory): + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_service_job(db_url) + assert _resolve(app, _host(job_id)).status_code == 403 + + +def test_resolve_rejects_unknown_job(app_factory): + app, db_url = app_factory(PROXY_DOMAIN) + assert _resolve(app, _host(999999)).status_code == 403 + + +@pytest.mark.parametrize("host", [ + "fileglancer.example.org", + "job-1.evil.example.org", + "job-abc.services.example.org", + "job-1.services.example.org", # unsigned: the old, guessable form + "job-1-aaaaaaaa.services.example.org", # wrong MAC +]) +def test_resolve_rejects_bad_hosts(app_factory, host): + app, db_url = app_factory(PROXY_DOMAIN) + _seed_running_service_with_url(db_url) + assert _resolve(app, host).status_code == 403 + + +def test_resolve_disabled_when_no_proxy_domain(app_factory): + """With the feature off, the endpoint refuses everything.""" + app, db_url = app_factory("") + job_id = _seed_running_service_with_url(db_url) + assert _resolve(app, _host(job_id)).status_code == 403 + + +def test_service_unavailable_page_is_servable_without_auth(app_factory): + """The reverse proxy renders this in place of its 403, on a subdomain the + session cookie is never sent to, so it must need no user and no assets.""" + app, _ = app_factory(PROXY_DOMAIN) + resp = TestClient(app).get("/api/apps/service-unavailable") + assert resp.status_code == 503 + assert resp.headers["content-type"].startswith("text/html") + assert "503 Service Unavailable" in resp.text + # Self-contained: the proxy would refuse a subresource fetched from here. + assert "src=" not in resp.text and "