From d076196bacaac95847b9365c20ea44c82dfd607b Mon Sep 17 00:00:00 2001 From: Bruno Postle Date: Fri, 24 Apr 2026 08:26:37 +0100 Subject: [PATCH] =?UTF-8?q?service.py:=20SSRF=20protection=20=E2=80=94=20r?= =?UTF-8?q?eject=20local=20transport,=20add=20--allowed-hosts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The preview endpoint is co-hosted with a specific Forgejo instance, so it should only be allowed to fetch from that host. - Local file transport (ifc:///path) is always rejected with 403 — the service must not read from the server's own filesystem - New --allowed-hosts CLI option for `ifcurl serve` takes a comma-separated list of permitted git hostnames (e.g. git.example.com or localhost:3000); any unlisted host is rejected with 403 - Defense-in-depth: literal private/loopback/link-local IP addresses are blocked even without an allowlist (169.254.x.x, 127.x, RFC1918, etc.) - README and forgejo/README.md updated to include --allowed-hosts in the standard deployment invocation Co-Authored-By: Claude Sonnet 4.6 --- .beads/issues.jsonl | 2 +- README.md | 12 ++++++++++-- forgejo/README.md | 19 +++++++++++++++++++ ifcurl/__main__.py | 13 +++++++++++++ ifcurl/service.py | 36 ++++++++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 3 deletions(-) diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 685c2b1..eed4056 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -1,4 +1,4 @@ -{"id":"ifcurl-2v9","title":"Security audit: preview endpoint SSRF risk","description":"The /preview endpoint accepts an ifc:// URL and calls fetch_ifc(), which clones or fetches from whatever git host is named in the URL. This is a potential SSRF vector: a caller could supply ifc://internal-host/... to trigger outbound connections to internal infrastructure. Assess: (1) should the service validate that the host is in an allowlist or is a public forge? (2) are there other parameters that could be abused? (3) is the intended deployment model (co-located with Forgejo, not exposed publicly) a sufficient mitigation, or should we add explicit controls regardless? Document the threat model and add any necessary validation.","status":"open","priority":2,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T07:09:33Z","created_by":"Bruno Postle","updated_at":"2026-04-24T07:09:33Z","dependency_count":0,"dependent_count":0,"comment_count":0} +{"id":"ifcurl-2v9","title":"Security audit: preview endpoint SSRF risk","description":"The /preview endpoint accepts an ifc:// URL and calls fetch_ifc(), which clones or fetches from whatever git host is named in the URL. This is a potential SSRF vector: a caller could supply ifc://internal-host/... to trigger outbound connections to internal infrastructure. Assess: (1) should the service validate that the host is in an allowlist or is a public forge? (2) are there other parameters that could be abused? (3) is the intended deployment model (co-located with Forgejo, not exposed publicly) a sufficient mitigation, or should we add explicit controls regardless? Document the threat model and add any necessary validation.","status":"closed","priority":2,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T07:09:33Z","created_by":"Bruno Postle","updated_at":"2026-04-24T07:21:07Z","started_at":"2026-04-24T07:12:29Z","closed_at":"2026-04-24T07:21:07Z","close_reason":"Implemented: reject local transport (403), --allowed-hosts allowlist for serve command, and private-IP literal blocking as defense-in-depth","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-bab","title":"Implement clip= parameter in viewer.html","description":"The ifc:// spec defines a repeatable clip= parameter (6 floats: point x,y,z + normal x,y,z in IFC world coords). The Python ifcurl renderer handles it, but viewer.html ignores it entirely — no parsing, no Three.js clipping planes, no round-trip into the URL on camera sync. Implement: parse clip= from the ifc:// URL, apply as THREE.Plane clipping planes to the renderer (with IFC→Three.js coordinate transform), and include current clip planes when rebuilding the URL.","status":"closed","priority":2,"issue_type":"feature","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T05:45:58Z","created_by":"Bruno Postle","updated_at":"2026-04-24T06:09:31Z","started_at":"2026-04-24T06:08:44Z","closed_at":"2026-04-24T06:09:31Z","close_reason":"applyClipPlanes() parses all clip= params, transforms IFC→Three.js coords via toThree(), constructs THREE.Plane objects and sets on world.renderer.three.clippingPlanes. syncCameraUrl already preserves clip= params unchanged.","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-yju","title":"viewer.html: structured form UI for ifc:// URL components","description":"Replace the single ifc:// URL text input with individual labelled fields for each component: host/repository, ref (branch/tag/commit), path (file within repo), selector (IfcOpenShell selector), camera, fov/scale. Editing any field should update the ifc:// URL and reload the model. The raw ifc:// URL should still be visible/copyable. This makes the viewer usable without needing to hand-edit a URL string.","status":"closed","priority":2,"issue_type":"feature","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-23T21:07:11Z","created_by":"Bruno Postle","updated_at":"2026-04-23T21:32:17Z","started_at":"2026-04-23T21:12:42Z","closed_at":"2026-04-23T21:32:17Z","close_reason":"Structured form UI implemented with repo/ref/path/selector fields; crash recovered, changes committed.","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-2zf","title":"viewer.html: FOV control and restore fov=/scale= from URL","description":"Two gaps: (1) No UI to change the camera FOV — add a control (e.g. a small number input or slider in the toolbar) so the user can adjust perspective FOV or orthographic scale. (2) applyCameraParam reads position/direction but ignores the fov= and scale= params written by syncCameraUrl — restore these when placing camera from URL so a shared link opens at exactly the recorded viewpoint.","status":"closed","priority":2,"issue_type":"feature","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-23T20:42:24Z","created_by":"Bruno Postle","updated_at":"2026-04-23T21:07:42Z","started_at":"2026-04-23T20:52:48Z","closed_at":"2026-04-23T21:07:42Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} diff --git a/README.md b/README.md index e51eaa2..e3bb720 100644 --- a/README.md +++ b/README.md @@ -123,10 +123,15 @@ with a Forgejo/Gitea instance. ```bash pip install "ifcurl[service]" -ifcurl serve # 127.0.0.1:8000 -ifcurl serve --host 0.0.0.0 --port 9000 +ifcurl serve --allowed-hosts git.example.com # restrict to your Forgejo host +ifcurl serve --host 0.0.0.0 --port 9000 --allowed-hosts git.example.com ``` +Pass `--allowed-hosts` as a comma-separated list of hostnames (with optional +`:port`) the service is allowed to fetch from. This prevents the preview +endpoint from being used to reach internal services. Omitting it allows all +non-private remote hosts. + ### Endpoints | Endpoint | Description | @@ -177,6 +182,9 @@ go build -tags 'sqlite sqlite_unlock_notify' \ sudo cp forgejo/custom/public/assets/viewer*.* /etc/forgejo/public/assets/ sudo cp forgejo/templates/custom/footer.tmpl /var/lib/forgejo/custom/templates/custom/ sudo systemctl restart forgejo + +# Start preview service (restrict to this Forgejo host) +ifcurl serve --allowed-hosts git.example.com ``` Add to `/etc/forgejo/conf/app.ini`: diff --git a/forgejo/README.md b/forgejo/README.md index fab80e1..045e59e 100644 --- a/forgejo/README.md +++ b/forgejo/README.md @@ -116,6 +116,25 @@ sudo systemctl restart forgejo --- +## Starting the preview service + +Run the preview service co-located with Forgejo, passing `--allowed-hosts` +set to your Forgejo hostname so the service will only fetch from that instance: + +```bash +ifcurl serve --allowed-hosts git.example.com +``` + +For a Forgejo instance on a non-standard port (e.g. 3000 for local dev): + +```bash +ifcurl serve --allowed-hosts localhost:3000 +``` + +`--allowed-hosts` accepts a comma-separated list. Omitting it allows all +non-private remote hosts, which is unsafe if the service is reachable from +untrusted clients. + ## Configuration Add to `/etc/forgejo/conf/app.ini`: diff --git a/ifcurl/__main__.py b/ifcurl/__main__.py index 6863232..cce4043 100644 --- a/ifcurl/__main__.py +++ b/ifcurl/__main__.py @@ -106,6 +106,14 @@ def main() -> None: ) serve_parser.add_argument("--host", default="127.0.0.1", help="Bind host (default: 127.0.0.1)") serve_parser.add_argument("--port", type=int, default=8000, help="Bind port (default: 8000)") + serve_parser.add_argument( + "--allowed-hosts", default="", metavar="HOSTS", + help=( + "Comma-separated list of git hostnames (with optional :port) the service " + "is permitted to fetch from, e.g. 'github.com,gitlab.example.com:3000'. " + "When omitted, all non-private remote hosts are allowed." + ), + ) # Print help when called with no arguments if len(sys.argv) == 1: @@ -178,6 +186,7 @@ def _cmd_serve(args: argparse.Namespace) -> None: try: import uvicorn + from ifcurl import service from ifcurl.service import app except ImportError: print( @@ -187,6 +196,10 @@ def _cmd_serve(args: argparse.Namespace) -> None: ) sys.exit(1) + if args.allowed_hosts: + allowed = {h.strip() for h in args.allowed_hosts.split(",") if h.strip()} + service.configure_allowed_hosts(allowed) + uvicorn.run(app, host=args.host, port=args.port) diff --git a/ifcurl/service.py b/ifcurl/service.py index fecce99..f371e6b 100644 --- a/ifcurl/service.py +++ b/ifcurl/service.py @@ -36,6 +36,7 @@ Tier 4 sha256(url) → PNG bytes from __future__ import annotations import hashlib +import ipaddress import os import tempfile import threading @@ -63,6 +64,32 @@ app = FastAPI( version="0.0.0", ) +# --------------------------------------------------------------------------- +# SSRF protection +# --------------------------------------------------------------------------- + +_allowed_hosts: set[str] | None = None + + +def configure_allowed_hosts(hosts: set[str] | None) -> None: + """Set the allowlist of git hosts the service will fetch from. + + Pass a set of hostname strings (optionally with :port) to restrict which + hosts are contacted. Pass None to allow all non-private remote hosts. + """ + global _allowed_hosts + _allowed_hosts = hosts + + +def _is_private_ip(host: str) -> bool: + """Return True if *host* is a literal private/loopback/link-local IP.""" + bare = host.split(":")[0].strip("[]") # strip port and IPv6 brackets + try: + addr = ipaddress.ip_address(bare) + return addr.is_loopback or addr.is_link_local or addr.is_private or addr.is_reserved + except ValueError: + return False + # --------------------------------------------------------------------------- # Tier 2: (commit_hexsha, path) → IFC bytes @@ -247,6 +274,15 @@ def preview(request: PreviewRequest) -> Response: if ifc_url.path is None: raise HTTPException(status_code=400, detail="URL has no 'path' parameter") + # --- SSRF protection --- + if ifc_url.transport == "local": + raise HTTPException(status_code=403, detail="Local file transport is not permitted in service mode") + if _allowed_hosts is not None: + if ifc_url.host not in _allowed_hosts: + raise HTTPException(status_code=403, detail=f"Host {ifc_url.host!r} is not in the allowed-hosts list") + elif _is_private_ip(ifc_url.host): + raise HTTPException(status_code=403, detail="Requests to private/loopback addresses are not permitted") + # --- Tier 4 / 4m: cached PNG --- if ifc_url.is_mutable_ref(): t4m_hit = _t4m_get(request.url)