mirror of
https://github.com/brunopostle/ifcurl.git
synced 2026-07-12 02:08:13 +00:00
tests: extend coverage — SSRF protection, GET endpoint, LRU eviction, branch ref
- SSRF: test local transport rejection, allowlist enforcement, private/loopback/ link-local IP blocking, allowlist with port, public host permitted without list - GET /preview: happy path, invalid scheme, local transport rejection - Tier 2 LRU: eviction at capacity, LRU promotion, cache miss - git: heads/<branch> ref form (was only tested via HEAD/tag/hash) - clear_caches fixture now also resets _allowed_hosts between tests 103 → 118 passing tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
7b95551cb8
commit
5d5b8c6516
3 changed files with 121 additions and 3 deletions
|
|
@ -25,7 +25,7 @@
|
|||
{"id":"ifcurl-07a","title":"Cache size control for bare repo clones","description":"git.py clones remote repos as bare repos under ~/.cache/ifcurl/\u003chash\u003e/repo.git with no size limit or eviction policy. Large repos accumulate indefinitely. Add a configurable max cache size (e.g. via env var IFCURL_CACHE_MAX_GB or a config file) with an LRU eviction policy based on last-access time of each repo directory. Should also expose a CLI command (ifcurl cache --list, --prune, --clear) to inspect and manage the cache manually.","status":"open","priority":3,"issue_type":"feature","owner":"bruno@postle.net","created_at":"2026-04-24T05:45:59Z","created_by":"Bruno Postle","updated_at":"2026-04-24T05:45:59Z","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||
{"id":"ifcurl-ts5","title":"Assess Go test strategy for Forgejo plugin patch","description":"The Forgejo integration is a low-maintenance source patch (footer.tmpl, viewer.html, /viewer and /proxy HTTP handlers). Evaluate whether adding Go tests is feasible without coupling to Forgejo's full test suite: can the handler logic be extracted into a small testable package, or do tests have to live inside the Forgejo tree? If the cost of keeping tests green across Forgejo upstream upgrades is too high, document the decision and close. Outcome: either a small test file or a documented rationale for no Go tests.","status":"closed","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T05:45:57Z","created_by":"Bruno Postle","updated_at":"2026-04-24T06:51:21Z","started_at":"2026-04-24T06:36:35Z","closed_at":"2026-04-24T06:51:21Z","close_reason":"Go tests are feasible and low-maintenance. 7 tests in ifc_url_test.go live in the same package (markdown_test), use goldmark.New() directly (no Forgejo test infrastructure needed beyond TestMain which is already there). Only deps: setting.IfcURL.PreviewServiceURL (ours), goldmark (stable), testify. Test file copied to forgejo/modules/markup/markdown/ in ifcurl repo as patch artifact.","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||
{"id":"ifcurl-hvw","title":"Add JavaScript tests for viewer.html URL logic","description":"viewer.html contains pure-JS functions (parseIfcUrl, buildIfcUrl, toRawUrl, syncCameraUrl) with no test coverage. Add a lightweight JS test harness (e.g. a standalone test.html or Node-runnable test.js using the native test runner) that unit-tests these functions: round-trip URL parse/build, toRawUrl for each host type (GitHub, GitLab, Forgejo), and camera parameter serialisation. No browser automation needed — pure function tests.","status":"closed","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T05:45:56Z","created_by":"Bruno Postle","updated_at":"2026-04-24T06:30:47Z","started_at":"2026-04-24T06:26:01Z","closed_at":"2026-04-24T06:30:47Z","close_reason":"32 Node tests in tests/test_viewer_url.mjs covering parseIfcUrl, buildIfcUrl, toRawUrl. Pure functions extracted to viewer-url.js; viewer.html imports from it. Also caught + not being decoded in URL building.","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||
{"id":"ifcurl-b9e","title":"Audit and extend Python test coverage","description":"103 tests exist across test_url.py, test_git.py, test_auth.py, test_render.py, and test_service.py. Audit what is missing: check branch/edge coverage in git.py (offline fallback, mutable ref fetch, auth injection), service.py (tier 2 cache eviction, mutable ref bypass), and the CLI entry points in __main__.py. Add tests for any significant gaps found.","status":"open","priority":3,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T05:45:40Z","created_by":"Bruno Postle","updated_at":"2026-04-24T05:45:40Z","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||
{"id":"ifcurl-b9e","title":"Audit and extend Python test coverage","description":"103 tests exist across test_url.py, test_git.py, test_auth.py, test_render.py, and test_service.py. Audit what is missing: check branch/edge coverage in git.py (offline fallback, mutable ref fetch, auth injection), service.py (tier 2 cache eviction, mutable ref bypass), and the CLI entry points in __main__.py. Add tests for any significant gaps found.","status":"closed","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T05:45:40Z","created_by":"Bruno Postle","updated_at":"2026-04-24T07:37:10Z","started_at":"2026-04-24T07:33:43Z","closed_at":"2026-04-24T07:37:10Z","close_reason":"Added 15 tests: SSRF protection (8), GET /preview endpoint (3), tier-2 LRU eviction (3), heads/\u003cbranch\u003e ref form (1)","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||
{"id":"ifcurl-bqt","title":"Phase 4: Bonsai IFCDOCUMENTREFERENCE federation via ifc://","description":"When opening a model via ifc://, resolve IFCDOCUMENTREFERENCE locations using the repo and ref from the root file's URL as context. Resolve ifc:// locations in linked model references recursively. Requires the Phase 4 protocol handler (ifcurl-0oj).","status":"deferred","priority":3,"issue_type":"feature","owner":"bruno@postle.net","created_at":"2026-04-23T05:48:56Z","created_by":"Bruno Postle","updated_at":"2026-04-23T06:15:11Z","defer_until":"2026-12-01T00:00:00Z","dependencies":[{"issue_id":"ifcurl-bqt","depends_on_id":"ifcurl-0oj","type":"blocks","created_at":"2026-04-23T06:49:02Z","created_by":"Bruno Postle","metadata":"{}"}],"dependency_count":1,"dependent_count":1,"comment_count":0}
|
||||
{"id":"ifcurl-lcr","title":"Phase 4: Bonsai 'Copy view URL' button","description":"Add a button to Bonsai that generates an ifc:// URL from the current state: git remote/ref/file path, active selection as IfcOpenShell selector, current camera as camera+fov or scale params in IFC world coordinates, active clipping planes as clip params, current visibility mode. Default to commit hash ref; secondary option for @heads/ ref.","status":"deferred","priority":3,"issue_type":"feature","owner":"bruno@postle.net","created_at":"2026-04-23T05:48:55Z","created_by":"Bruno Postle","updated_at":"2026-04-23T06:15:10Z","defer_until":"2026-12-01T00:00:00Z","dependencies":[{"issue_id":"ifcurl-lcr","depends_on_id":"ifcurl-0oj","type":"blocks","created_at":"2026-04-23T06:49:01Z","created_by":"Bruno Postle","metadata":"{}"}],"dependency_count":1,"dependent_count":0,"comment_count":0}
|
||||
{"id":"ifcurl-0oj","title":"Phase 4: Bonsai ifc:// OS protocol handler","description":"Register ifc:// as an OS protocol handler in Bonsai on installation (Windows, macOS, Linux). On receiving a URL, resolve the git source, load the model, and apply the view state from the URL parameters (camera, selector, clipping planes, visibility mode). This is the foundation for all other Phase 4 work.","status":"deferred","priority":3,"issue_type":"feature","owner":"bruno@postle.net","created_at":"2026-04-23T05:48:45Z","created_by":"Bruno Postle","updated_at":"2026-04-23T06:15:09Z","defer_until":"2026-12-01T00:00:00Z","dependency_count":0,"dependent_count":3,"comment_count":0}
|
||||
|
|
|
|||
|
|
@ -43,6 +43,14 @@ class TestFetchIfc:
|
|||
hexsha, data = fetch_ifc(url)
|
||||
assert hexsha == local_ifc_repo["hexsha"]
|
||||
|
||||
def test_branch_ref(self, local_ifc_repo):
|
||||
import git as gitpkg
|
||||
branch = gitpkg.Repo(local_ifc_repo["path"]).active_branch.name
|
||||
url = _local_url(local_ifc_repo["path"], ref=f"heads/{branch}")
|
||||
hexsha, data = fetch_ifc(url)
|
||||
assert hexsha == local_ifc_repo["hexsha"]
|
||||
assert data == local_ifc_repo["bytes"]
|
||||
|
||||
def test_bad_ref_raises(self, local_ifc_repo):
|
||||
url = _local_url(local_ifc_repo["path"], ref="heads/nonexistent")
|
||||
with pytest.raises(ValueError, match="not found"):
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ from unittest.mock import MagicMock, patch
|
|||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from ifcurl.service import _t2_cache, _t3_cache, app
|
||||
from ifcurl.service import _t2_cache, _t3_cache, _t2_get, _t2_put, app, configure_allowed_hosts
|
||||
|
||||
client = TestClient(app)
|
||||
|
||||
|
|
@ -25,13 +25,15 @@ SELECTOR_URL = "ifc://example.com/org/repo@heads/main?path=model.ifc&selector=If
|
|||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clear_caches(tmp_path, monkeypatch):
|
||||
"""Reset in-memory caches and redirect filesystem PNG cache between tests."""
|
||||
"""Reset in-memory caches, redirect filesystem PNG cache, and reset allowed-hosts between tests."""
|
||||
monkeypatch.setattr("ifcurl.service.user_cache_dir", lambda *a, **kw: str(tmp_path))
|
||||
_t2_cache.clear()
|
||||
_t3_cache.clear()
|
||||
configure_allowed_hosts(None)
|
||||
yield
|
||||
_t2_cache.clear()
|
||||
_t3_cache.clear()
|
||||
configure_allowed_hosts(None)
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
|
|
@ -287,3 +289,111 @@ class TestAuthentication:
|
|||
client.post("/preview", json={"url": MUTABLE_URL, "token": "request_token"})
|
||||
|
||||
assert received_token == ["request_token"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GET /preview endpoint
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestGetEndpoint:
|
||||
def test_get_returns_png(self, model_with_geometry):
|
||||
ifc_bytes = model_with_geometry.to_string().encode()
|
||||
with patch("ifcurl.service.fetch_ifc", _mock_fetch(ifc_bytes)), \
|
||||
patch("ifcurl.service.render_mod.render", return_value=FAKE_PNG):
|
||||
r = client.get("/preview", params={"url": MUTABLE_URL})
|
||||
assert r.status_code == 200
|
||||
assert r.headers["content-type"] == "image/png"
|
||||
assert r.content == FAKE_PNG
|
||||
|
||||
def test_get_invalid_scheme_returns_400(self):
|
||||
r = client.get("/preview", params={"url": "https://example.com/model.ifc"})
|
||||
assert r.status_code == 400
|
||||
|
||||
def test_get_local_transport_rejected(self):
|
||||
r = client.get("/preview", params={"url": "ifc:///some/path@HEAD?path=m.ifc"})
|
||||
assert r.status_code == 403
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tier 2: LRU eviction
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestTier2Eviction:
|
||||
def test_oldest_entry_evicted_at_max(self, monkeypatch):
|
||||
monkeypatch.setattr("ifcurl.service._T2_MAX", 2)
|
||||
_t2_put("sha_a", "a.ifc", b"data_a")
|
||||
_t2_put("sha_b", "b.ifc", b"data_b")
|
||||
_t2_put("sha_c", "c.ifc", b"data_c") # should evict sha_a
|
||||
assert _t2_get("sha_a", "a.ifc") is None
|
||||
assert _t2_get("sha_b", "b.ifc") == b"data_b"
|
||||
assert _t2_get("sha_c", "c.ifc") == b"data_c"
|
||||
|
||||
def test_get_promotes_entry(self, monkeypatch):
|
||||
monkeypatch.setattr("ifcurl.service._T2_MAX", 2)
|
||||
_t2_put("sha_a", "a.ifc", b"data_a")
|
||||
_t2_put("sha_b", "b.ifc", b"data_b")
|
||||
_t2_get("sha_a", "a.ifc") # promote sha_a to most-recent
|
||||
_t2_put("sha_c", "c.ifc", b"data_c") # should evict sha_b (now oldest)
|
||||
assert _t2_get("sha_a", "a.ifc") == b"data_a"
|
||||
assert _t2_get("sha_b", "b.ifc") is None
|
||||
assert _t2_get("sha_c", "c.ifc") == b"data_c"
|
||||
|
||||
def test_cache_miss_returns_none(self):
|
||||
assert _t2_get("nonexistent", "x.ifc") is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SSRF protection
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestSSRF:
|
||||
def test_local_transport_always_rejected(self):
|
||||
r = client.post("/preview", json={"url": "ifc:///some/path@HEAD?path=m.ifc"})
|
||||
assert r.status_code == 403
|
||||
assert "local" in r.json()["detail"].lower()
|
||||
|
||||
def test_host_not_in_allowlist_rejected(self):
|
||||
configure_allowed_hosts({"allowed.example.com"})
|
||||
r = client.post("/preview", json={"url": MUTABLE_URL}) # host: example.com
|
||||
assert r.status_code == 403
|
||||
assert "allowed-hosts" in r.json()["detail"].lower()
|
||||
|
||||
def test_host_in_allowlist_permitted(self, model_with_geometry):
|
||||
configure_allowed_hosts({"example.com"})
|
||||
ifc_bytes = model_with_geometry.to_string().encode()
|
||||
with patch("ifcurl.service.fetch_ifc", _mock_fetch(ifc_bytes)), \
|
||||
patch("ifcurl.service.render_mod.render", return_value=FAKE_PNG):
|
||||
r = client.post("/preview", json={"url": MUTABLE_URL})
|
||||
assert r.status_code == 200
|
||||
|
||||
def test_no_allowlist_permits_public_host(self, model_with_geometry):
|
||||
ifc_bytes = model_with_geometry.to_string().encode()
|
||||
with patch("ifcurl.service.fetch_ifc", _mock_fetch(ifc_bytes)), \
|
||||
patch("ifcurl.service.render_mod.render", return_value=FAKE_PNG):
|
||||
r = client.post("/preview", json={"url": MUTABLE_URL})
|
||||
assert r.status_code == 200
|
||||
|
||||
def test_loopback_ip_rejected_without_allowlist(self):
|
||||
r = client.post("/preview", json={"url": "ifc://127.0.0.1/org/repo@HEAD?path=m.ifc"})
|
||||
assert r.status_code == 403
|
||||
|
||||
def test_private_ip_rejected_without_allowlist(self):
|
||||
r = client.post("/preview", json={"url": "ifc://192.168.1.1/org/repo@HEAD?path=m.ifc"})
|
||||
assert r.status_code == 403
|
||||
|
||||
def test_link_local_ip_rejected_without_allowlist(self):
|
||||
r = client.post("/preview", json={"url": "ifc://169.254.169.254/org/repo@HEAD?path=m.ifc"})
|
||||
assert r.status_code == 403
|
||||
|
||||
def test_allowlist_with_port(self, model_with_geometry):
|
||||
configure_allowed_hosts({"localhost:3000"})
|
||||
ifc_bytes = model_with_geometry.to_string().encode()
|
||||
with patch("ifcurl.service.fetch_ifc", _mock_fetch(ifc_bytes)), \
|
||||
patch("ifcurl.service.render_mod.render", return_value=FAKE_PNG):
|
||||
r = client.post("/preview", json={
|
||||
"url": "ifc://localhost:3000/org/repo@heads/main?path=model.ifc"
|
||||
})
|
||||
assert r.status_code == 200
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue