domverse/photoanalyzer
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects 1 Releases Wiki Activity

US07-02: Harden API Authorization and Path Boundaries (#84)

Browse Source
This commit was merged in pull request #84.
This commit is contained in:
domverse
2026-08-16 23:07:36 +02:00
parent d143fee4d2
commit d2e44b5657
21 changed files with 1134 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
tests/e2e/test_phase_a_pipeline.py
View File

@@ -22,6 +22,8 @@ import numpy as np
import pytest
from PIL import Image
from tests.conftest import session_client
REPO = Path(__file__).resolve().parents[2]
@@ -92,7 +94,7 @@ class ServerController:
pytest.fail(f"server exited: {err.decode(errors='replace')}")
try:
if httpx.get(f"{self.base}/api/v1/health/ready", timeout=1).status_code == 200:
self.client = httpx.Client(base_url=self.base, timeout=10)
self.client = session_client(self.base, timeout=10)
return
except httpx.HTTPError:
time.sleep(0.2)

17
tests/e2e/test_review_ui.py
View File

@@ -17,6 +17,9 @@ import httpx
import numpy as np
import pytest
from PIL import Image
from playwright.sync_api import expect
from tests.conftest import session_client
REPO = Path(__file__).resolve().parents[2]
@@ -111,7 +114,7 @@ def server(tmp_path):
proc.terminate()
pytest.fail("server never became ready")
client = httpx.Client(base_url=base, timeout=5)
client = session_client(base, timeout=5)
def cluster_by_method(method):
clusters = client.get("/api/v1/duplicates/clusters").json()["items"]
@@ -148,9 +151,9 @@ def test_fuzzy_decision_requires_confirmation(page, server):
page.get_by_test_id("confirm").wait_for()
assert "open" in page.get_by_test_id("cluster-state").inner_text()
page.get_by_test_id("confirm-yes").click()
page.wait_for_function(
"document.querySelector('[data-testid=cluster-state]').innerText.includes('dismissed')"
)
# A locator assertion, not wait_for_function: the app's CSP forbids eval, and
# Playwright's polling predicate is evaluated as a string in the page (US07-02).
expect(page.get_by_test_id("cluster-state")).to_contain_text("dismissed")
def test_stale_version_shows_conflict(page, server):
@@ -176,9 +179,9 @@ def test_decision_persists_after_reload(page, server):
page.goto(f"{server.base}/app/#/duplicates/{cluster['id']}")
page.get_by_test_id("not-duplicate").click()
page.get_by_test_id("confirm-yes").click()
page.wait_for_function(
"document.querySelector('[data-testid=cluster-state]').innerText.includes('dismissed')"
)
# A locator assertion, not wait_for_function: the app's CSP forbids eval, and
# Playwright's polling predicate is evaluated as a string in the page (US07-02).
expect(page.get_by_test_id("cluster-state")).to_contain_text("dismissed")
page.reload()
page.get_by_test_id("cluster-state").wait_for()
assert "dismissed" in page.get_by_test_id("cluster-state").inner_text()

280
tests/e2e/test_security.py Normal file
View File

@@ -0,0 +1,280 @@
"""US07-02 black-box security tests: authorization and path boundaries.
Everything here talks to a real ``photo_pipeline serve`` child process over HTTP with
its own ``httpx.Client``, deliberately outside the session helper the rest of the
suite uses — an attacker does not get a bootstrapped client.
The threat is a page in the user's browser, not a remote attacker: the app listens on
127.0.0.1, so any site the user visits can send requests to it and can point an
``<img>`` at its media endpoints. Each journey below is one of those attempts.
"""
from __future__ import annotations
import json
from contextlib import contextmanager
from pathlib import Path
import httpx
import pytest
from tests.e2e._pipeline_harness import Server, image, seed_library
TIMEOUT = 20
SENTINEL_KEY = "immich-sentinel-9f3a2b"
SESSION_COOKIE = "pp_session"
CSRF_HEADER = "X-CSRF-Token"
# A GET, a mutation, and a media endpoint: the three shapes the policy must cover.
PROTECTED = [
("GET", "/api/v1/workflow", None),
("POST", "/api/v1/albums/proposals", {}),
("GET", "/api/v1/assets/{asset}/thumbnail?size=256", None),
]
@pytest.fixture(scope="module")
def stack(tmp_path_factory):
"""One server, one album of two photos, and a credential sentinel in its config."""
tmp_path = tmp_path_factory.mktemp("security")
seeded = seed_library(tmp_path, {"a": 1, "b": 2}, {"a": "sfw", "b": "sfw"})
outside = tmp_path / "outside"
outside.mkdir()
image(outside / "secret.jpg", 99)
server = Server(
seeded,
extra_env={
"PHOTO_PIPELINE_IMMICH_API_KEY": SENTINEL_KEY,
"PHOTO_PIPELINE_IMMICH_SERVER_URL": "http://127.0.0.1:1",
},
).start()
yield server, seeded, outside
server.stop()
@contextmanager
def anonymous(server):
with httpx.Client(base_url=server.base, timeout=TIMEOUT) as client:
yield client
@contextmanager
def authenticated(server):
"""A browser that has loaded the app: session cookie in the jar, token in a header."""
with httpx.Client(base_url=server.base, timeout=TIMEOUT) as client:
client.headers[CSRF_HEADER] = client.get("/api/v1/session").json()["csrf_token"]
yield client
def call(client, method, path, body, asset):
return client.request(method, path.format(asset=asset), json=body)
def test_the_api_refuses_every_caller_without_a_session(stack):
server, seeded, _ = stack
with anonymous(server) as client:
for method, path, body in PROTECTED:
response = call(client, method, path, body, seeded.asset_ids["a"])
assert response.status_code == 401, path
assert response.json()["error"]["code"] == "unauthenticated"
# Liveness and readiness stay open: an orchestrator holds no session.
assert client.get("/api/v1/health/ready").status_code == 200
assert client.get("/api/v1/health/live").status_code == 200
def test_a_guessed_session_cookie_is_refused(stack):
server, _, _ = stack
with anonymous(server) as client:
client.cookies.set(SESSION_COOKIE, "guessed", domain="127.0.0.1")
response = client.get("/api/v1/workflow")
assert response.status_code == 401
assert response.json()["error"]["code"] == "unauthenticated"
def test_the_bootstrap_issues_a_strict_httponly_cookie(stack):
server, _, _ = stack
with anonymous(server) as client:
response = client.get("/api/v1/session")
cookie = response.headers["set-cookie"].lower()
assert "httponly" in cookie and "samesite=strict" in cookie and "path=/" in cookie
assert response.json()["csrf_token"]
# The token is in the body, which no other origin may read: no CORS header
# grants access to it.
assert "access-control-allow-origin" not in response.headers
def test_a_session_without_the_csrf_token_may_read_but_not_mutate(stack):
server, _, _ = stack
with authenticated(server) as client:
del client.headers[CSRF_HEADER]
assert client.get("/api/v1/albums/proposals").status_code == 200
response = client.post("/api/v1/albums/proposals", json={})
assert response.status_code == 403
assert response.json()["error"]["code"] == "csrf_failed"
response = client.post(
"/api/v1/albums/proposals", json={}, headers={CSRF_HEADER: "guessed"}
)
assert response.status_code == 403
# And nothing was created behind the refusal.
with authenticated(server) as client:
assert client.get("/api/v1/albums/proposals").json()["items"] == []
def test_a_foreign_origin_cannot_mutate_even_with_a_session(stack):
server, _, _ = stack
with authenticated(server) as client:
for origin in ("http://evil.example", "http://127.0.0.1:1", "null"):
response = client.post(
"/api/v1/albums/proposals", json={}, headers={"Origin": origin}
)
assert response.status_code == 403, origin
assert response.json()["error"]["code"] == "origin_not_allowed"
# This app's own origin is accepted, so the check is not simply refusing all.
allowed = client.post(
"/api/v1/albums/proposals", json={}, headers={"Origin": server.base}
)
assert allowed.status_code == 200
def test_a_rebinding_host_is_refused(stack):
"""A name that resolves to 127.0.0.1 makes the browser treat the attacker's page
as same-origin. The Host header still carries that name, so it is checked."""
server, _, _ = stack
with authenticated(server) as client:
response = client.get("/api/v1/workflow", headers={"Host": "photos.evil.example"})
assert response.status_code == 403
assert response.json()["error"]["code"] == "host_not_allowed"
def test_media_cannot_be_embedded_by_another_page(stack):
server, seeded, _ = stack
url = f"/api/v1/assets/{seeded.asset_ids['a']}/thumbnail?size=256"
with authenticated(server) as client:
# What an <img> on another site produces: no Origin, but a cross-site marker.
blocked = client.get(url, headers={"Sec-Fetch-Site": "cross-site"})
assert blocked.status_code == 403
assert blocked.json()["error"]["code"] == "cross_site_blocked"
served = client.get(url, headers={"Sec-Fetch-Site": "same-origin"})
assert served.status_code == 200
assert served.headers["content-type"] == "image/webp"
assert served.headers["cross-origin-resource-policy"] == "same-origin"
# Photos must never land in a shared cache.
assert served.headers["cache-control"].startswith("private")
def test_every_response_carries_the_default_headers_and_no_cors(stack):
server, seeded, _ = stack
with authenticated(server) as client:
responses = [
client.get("/api/v1/workflow"),
client.get(f"/api/v1/assets/{seeded.asset_ids['a']}/thumbnail?size=256"),
client.get("/app/"),
client.get("/api/v1/does-not-exist"),
]
for response in responses:
assert response.headers["x-content-type-options"] == "nosniff"
assert response.headers["x-frame-options"] == "DENY"
assert response.headers["referrer-policy"] == "no-referrer"
assert "frame-ancestors 'none'" in response.headers["content-security-policy"]
assert "access-control-allow-origin" not in response.headers
assert "access-control-allow-credentials" not in response.headers
def test_a_traversal_attempt_addresses_nothing(stack):
"""Identifiers are database keys, not paths: traversal has nowhere to land."""
server, _, _ = stack
with authenticated(server) as client:
for path in (
"/api/v1/albums/proposals/..%2F..%2F..%2Fetc%2Fpasswd",
"/api/v1/albums/proposals/../../../etc/passwd",
"/api/v1/assets/..%2F..%2Fetc%2Fpasswd/thumbnail?size=256",
"/api/v1/assets/%2Fetc%2Fpasswd/thumbnail?size=256",
):
response = client.get(path)
assert response.status_code in (404, 422), path
assert "root:" not in response.text, path
def test_an_oversized_request_is_refused_before_it_is_parsed(stack):
server, _, _ = stack
with authenticated(server) as client:
response = client.post(
"/api/v1/albums/proposals",
content=json.dumps({"albums": ["x" * 2_000_000]}),
headers={"Content-Type": "application/json"},
)
assert response.status_code == 413
assert response.json()["error"]["code"] == "payload_too_large"
def test_a_malformed_request_reports_the_field_and_nothing_else(stack):
server, _, _ = stack
with authenticated(server) as client:
for content in ("{", '{"albums": 5}', ""):
response = client.post(
"/api/v1/albums/proposals",
content=content,
headers={"Content-Type": "application/json"},
)
assert response.status_code == 422, content
error = response.json()["error"]
assert error["code"] == "invalid_request"
assert "Traceback" not in response.text and "photo_pipeline/" not in response.text
def test_errors_reveal_neither_the_credential_nor_an_internal(stack):
"""The API key is configured but must appear nowhere; failures additionally say
nothing about where the library lives or how the server is built.
Successful responses are a different matter: path previews are the point of the
rename and archive views, and the operator is the one who owns those paths.
"""
server, seeded, _ = stack
with authenticated(server) as client:
responses = [
client.post("/api/v1/upload-preflight", json={}),
client.get("/api/v1/workflow"),
client.get("/api/v1/upload-batches"),
client.get("/api/v1/assets/unknown-asset/thumbnail?size=256"),
client.get("/api/v1/archive-plans/unknown-plan"),
client.post("/api/v1/archive-locations", json={"name": "x", "root": "/nope"}),
client.get("/api/v1/does-not-exist"),
]
for response in responses:
assert SENTINEL_KEY not in response.text
assert "Traceback" not in response.text
if response.status_code >= 400:
assert str(seeded.lib) not in response.text
assert "photo_pipeline/" not in response.text
assert "sqlite" not in response.text.lower()
def test_a_symlink_swapped_under_an_asset_cannot_be_served(stack):
"""TOCTOU on the media path: the file the database points at is replaced by a
link to something outside the library between the scan and the request."""
server, seeded, outside = stack
secret = outside / "secret.jpg"
original = seeded.lib / "b.jpg"
original.unlink()
original.symlink_to(secret)
with authenticated(server) as client:
response = client.get(f"/api/v1/assets/{seeded.asset_ids['b']}/thumbnail?size=256")
assert response.status_code == 403
assert response.json()["error"]["code"] == "path_not_allowed"
# The refusal names no filesystem location, and no bytes escaped with it.
assert str(outside) not in response.text and str(seeded.lib) not in response.text
assert secret.read_bytes()[:16] not in response.content
def test_the_frontend_shell_stays_reachable_without_a_session(stack):
"""It must load before any JavaScript can ask for a session."""
server, _, _ = stack
with anonymous(server) as client:
response = client.get("/app/", headers={"Sec-Fetch-Site": "none"})
assert response.status_code == 200
assert response.headers["content-type"].startswith("text/html")
assert Path("frontend/index.html").exists()