mirror of
https://github.com/brunopostle/ifcurl.git
synced 2026-07-12 02:08:13 +00:00
service.py: SSRF protection — reject local transport, add --allowed-hosts
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 <noreply@anthropic.com>
This commit is contained in:
parent
8a675d667f
commit
d076196bac
5 changed files with 79 additions and 3 deletions
|
|
@ -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}
|
||||
|
|
|
|||
12
README.md
12
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`:
|
||||
|
|
|
|||
|
|
@ -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`:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue