diff --git a/requirements.txt b/requirements.txt index 977bd13..3343e85 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,7 @@ fastapi==0.133.1 httpx==0.28.1 pydantic==2.13.4 +Pillow==12.3.0 python-multipart==0.0.22 pytest==9.1.1 pywebpush==2.1.2 diff --git a/src/image_sanitizer.py b/src/image_sanitizer.py new file mode 100644 index 0000000..6fbc571 --- /dev/null +++ b/src/image_sanitizer.py @@ -0,0 +1,38 @@ +"""Authoritative image decoding and metadata removal for evidence uploads.""" + +from __future__ import annotations + +import io + +from PIL import Image, ImageOps + + +MAX_IMAGE_PIXELS = 12 * 1024 * 1024 + + +def sanitize_image(content_type: str, content: bytes) -> bytes: + """Apply orientation and return metadata-free bytes in the claimed format.""" + format_names = { + "image/png": "PNG", + "image/jpeg": "JPEG", + "image/webp": "WEBP", + } + try: + with Image.open(io.BytesIO(content)) as source: + claimed_format = format_names[content_type] + if source.format != claimed_format: + raise ValueError("image content does not match the selected image type") + if getattr(source, "n_frames", 1) != 1: + raise ValueError("animated image evidence is not supported") + if source.width * source.height > MAX_IMAGE_PIXELS: + raise ValueError("attachment must not exceed 12 megapixels") + image = ImageOps.exif_transpose(source) + image.load() + output = io.BytesIO() + save_options = {"quality": 90} if claimed_format in {"JPEG", "WEBP"} else {} + image.save(output, format=claimed_format, **save_options) + return output.getvalue() + except ValueError: + raise + except (KeyError, OSError) as exc: + raise ValueError("attachment must be a valid static image") from exc diff --git a/src/main.py b/src/main.py index 7ef83db..2c4cc30 100644 --- a/src/main.py +++ b/src/main.py @@ -41,6 +41,7 @@ from src.gitea_proxy import ( repos, ) from src.idempotency import IdempotencyLedger, IdempotencyLedgerBusy +from src.image_sanitizer import sanitize_image from src.login_attempt_store import LoginAttemptStore, LoginAttemptStoreError, client_source from src.live_snapshot_store import ( LiveSnapshotMetadata, @@ -796,6 +797,13 @@ def _validate_binary_attachment(filename: str, content_type: str, content: bytes return _validate_attachment_content(content_type, content) +async def _sanitize_attachment(content_type: str, content: bytes) -> bytes: + sanitized = await asyncio.to_thread(sanitize_image, content_type, content) + if len(sanitized) > 2 * 1024 * 1024: + raise ValueError("sanitized screenshot must be 2 MB or smaller") + return sanitized + + class IssueCreation(BaseModel): title: str = Field(min_length=1, max_length=255) body: str = Field(default="", max_length=10_000) @@ -3157,6 +3165,7 @@ async def attach_to_global_search_preview( filename = str(uploaded.filename or "") content_type = str(uploaded.content_type or "") content = _validate_binary_attachment(filename, content_type, await uploaded.read()) + content = await _sanitize_attachment(content_type, content) except (ValueError, ValidationError) as exc: raise HTTPException(status_code=422, detail=str(exc)) from exc @@ -4337,6 +4346,7 @@ async def attach_to_notification( filename = attachment.filename content_type = attachment.content_type content = attachment.content() + content = await _sanitize_attachment(content_type, content) except (ValueError, ValidationError) as exc: raise HTTPException(status_code=422, detail=str(exc)) from exc @@ -5320,6 +5330,7 @@ async def attach_to_assigned_issue( filename = attachment.filename content_type = attachment.content_type content = attachment.content() + content = await _sanitize_attachment(content_type, content) except (ValueError, ValidationError) as exc: raise HTTPException(status_code=422, detail=str(exc)) from exc async def upload_attachment(): @@ -5388,6 +5399,7 @@ async def attach_to_assigned_pull( filename = attachment.filename content_type = attachment.content_type content = attachment.content() + content = await _sanitize_attachment(content_type, content) except (ValueError, ValidationError) as exc: raise HTTPException(status_code=422, detail=str(exc)) from exc diff --git a/tests/test_global_search.py b/tests/test_global_search.py index cc5b570..4af5e90 100644 --- a/tests/test_global_search.py +++ b/tests/test_global_search.py @@ -1,11 +1,19 @@ import asyncio +import io import httpx import pytest +from PIL import Image from src import gitea_proxy, main +def png_bytes(color="red"): + output = io.BytesIO() + Image.new("RGB", (2, 2), color).save(output, format="PNG") + return output.getvalue() + + @pytest.mark.anyio async def test_global_search_endpoint_scopes_every_page_to_an_exact_repository(monkeypatch): requested = [] @@ -343,7 +351,7 @@ async def test_global_search_preview_attachment_is_idempotent_for_exact_visible_ monkeypatch.setattr(main.gitea_proxy, "upload_preview_attachment", upload) transport = httpx.ASGITransport(app=main.app) headers = {"Idempotency-Key": "search-photo-pull-42"} - image = b"\x89PNG\r\n\x1a\nsafe-pixels" + image = png_bytes() files = {"file": ("photo.png", image, "image/png")} async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: first = await client.post( @@ -381,7 +389,7 @@ async def test_global_search_preview_attachment_rejects_kind_mismatch_before_upl async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: response = await client.post( "/api/v1/repos/stackchain/api/issues/42/preview/attachments?kind=pull", - files={"file": ("photo.png", b"\x89PNG\r\n\x1a\nsafe-pixels", "image/png")}, + files={"file": ("photo.png", png_bytes(), "image/png")}, headers={"Idempotency-Key": "search-photo-mismatch-42"}, ) diff --git a/tests/test_image_sanitizer.py b/tests/test_image_sanitizer.py new file mode 100644 index 0000000..f8b893d --- /dev/null +++ b/tests/test_image_sanitizer.py @@ -0,0 +1,63 @@ +import io +from pathlib import Path + +import pytest +from PIL import Image + +from src.image_sanitizer import MAX_IMAGE_PIXELS, sanitize_image + + +def test_runtime_dependencies_install_the_image_decoder(): + requirements = (Path(__file__).parents[1] / "requirements.txt").read_text().splitlines() + assert any(line.startswith("Pillow==") for line in requirements) + + +def encoded_image(format_name, size=(3, 2), *, exif=None): + image = Image.new("RGB", size) + image.putdata( + [(255, 0, 0), (0, 255, 0), (0, 0, 255), (255, 255, 0), (0, 255, 255), (255, 0, 255)] + ) + output = io.BytesIO() + image.save(output, format=format_name, exif=exif or b"") + return output.getvalue() + + +def test_sanitize_image_applies_orientation_and_removes_jpeg_metadata(): + exif = Image.Exif() + exif[274] = 6 + exif[ GPS_TAG := 34853 ] = {1: "N", 2: (1, 1, 1)} + source = encoded_image("JPEG", exif=exif) + + sanitized = sanitize_image("image/jpeg", source) + + with Image.open(io.BytesIO(sanitized)) as result: + assert result.size == (2, 3) + assert not result.getexif() + assert GPS_TAG not in result.info + assert result.format == "JPEG" + + +def test_sanitize_image_rejects_bytes_that_do_not_match_claimed_type(): + source = encoded_image("PNG") + + with pytest.raises(ValueError, match="does not match"): + sanitize_image("image/jpeg", source) + + +def test_sanitize_image_rejects_over_twelve_megapixels_before_decoding(): + image = Image.new("RGB", (4096, 3073)) + output = io.BytesIO() + image.save(output, format="PNG") + + with pytest.raises(ValueError, match="12 megapixels"): + sanitize_image("image/png", output.getvalue()) + + +def test_sanitize_image_rejects_animated_webp(): + first = Image.new("RGB", (2, 2), "red") + second = Image.new("RGB", (2, 2), "blue") + output = io.BytesIO() + first.save(output, format="WEBP", save_all=True, append_images=[second], duration=100) + + with pytest.raises(ValueError, match="animated"): + sanitize_image("image/webp", output.getvalue()) diff --git a/tests/test_issue_attachments.py b/tests/test_issue_attachments.py index 56301d1..7fa2242 100644 --- a/tests/test_issue_attachments.py +++ b/tests/test_issue_attachments.py @@ -1,13 +1,49 @@ import asyncio import base64 +import io +import time import httpx import pytest +from PIL import Image, PngImagePlugin from src import gitea_proxy, main -PNG_BYTES = b"\x89PNG\r\n\x1a\n" + b"mobile screenshot" +def make_png(color="red", size=(2, 2)): + output = io.BytesIO() + Image.new("RGB", size, color).save(output, format="PNG") + return output.getvalue() + + +PNG_BYTES = make_png() + + +def png_with_private_metadata(): + output = io.BytesIO() + metadata = PngImagePlugin.PngInfo() + metadata.add_text("Location", "private coordinates") + Image.new("RGB", (2, 2), "red").save(output, format="PNG", pnginfo=metadata) + return output.getvalue() + + +@pytest.mark.anyio +async def test_attachment_sanitization_keeps_event_loop_responsive(monkeypatch): + def slow_sanitizer(_content_type, content): + time.sleep(0.1) + return content + + monkeypatch.setattr(main, "sanitize_image", slow_sanitizer) + started = time.monotonic() + ticks = [] + + async def tick(): + await asyncio.sleep(0.01) + ticks.append(time.monotonic() - started) + + await asyncio.gather(main._sanitize_attachment("image/png", b"image"), tick()) + + assert ticks[0] < 0.05 @pytest.fixture(autouse=True) @@ -58,6 +94,30 @@ async def test_attachment_endpoint_uploads_valid_screenshot_to_assigned_issue(mo ] +@pytest.mark.anyio +async def test_attachment_endpoint_strips_metadata_before_upstream(monkeypatch): + uploaded = [] + source = png_with_private_metadata() + + async def upload(_repository, _number, filename, content_type, content): + uploaded.append(content) + return {"name": filename, "url": "https://forge.example/a.png", "size": len(content)} + + monkeypatch.setattr(main.gitea_proxy, "upload_assigned_issue_attachment", upload) + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.post( + "/api/v1/repos/stackchain/api/issues/17/attachments", + files={"file": ("evidence.png", source, "image/png")}, + ) + + assert response.status_code == 201 + assert uploaded and uploaded[0] != source + with Image.open(io.BytesIO(uploaded[0])) as sanitized: + assert sanitized.info.get("Location") is None + assert sanitized.getpixel((0, 0)) == (255, 0, 0) + + @pytest.mark.anyio async def test_attachment_endpoint_accepts_binary_multipart_without_base64_expansion(monkeypatch): calls = [] @@ -186,7 +246,7 @@ async def test_attachment_endpoint_rejects_changed_upload_for_used_key(monkeypat json={ "filename": "checkout.png", "content_type": "image/png", - "data": base64.b64encode(PNG_BYTES + b" changed").decode("ascii"), + "data": base64.b64encode(make_png("blue")).decode("ascii"), }, headers={"Idempotency-Key": "attachment-conflict-471"}, ) @@ -198,7 +258,7 @@ async def test_attachment_endpoint_rejects_changed_upload_for_used_key(monkeypat @pytest.mark.anyio async def test_attachment_endpoint_admits_a_normal_phone_screenshot(monkeypatch): - screenshot = b"\x89PNG\r\n\x1a\n" + (b"x" * (100 * 1024)) + screenshot = make_png(size=(390, 844)) async def upload(_repository, _number, filename, _content_type, content): return { diff --git a/tests/test_update_reply_attachments.py b/tests/test_update_reply_attachments.py index 1542bac..9b72f66 100644 --- a/tests/test_update_reply_attachments.py +++ b/tests/test_update_reply_attachments.py @@ -1,9 +1,11 @@ +import io import json import subprocess from pathlib import Path import httpx import pytest +from PIL import Image from src import gitea_proxy, main from tests.dashboard_bundle import dashboard @@ -12,7 +14,9 @@ from tests.dashboard_bundle import dashboard ROOT = Path(__file__).parents[1] OUTBOX = ROOT / "frontend" / "authored-outbox.js" SYNC = ROOT / "frontend" / "background-issue-sync.js" -PNG = b"\x89PNG\r\n\x1a\nmobile-update" +_png = io.BytesIO() +Image.new("RGB", (2, 2), "red").save(_png, format="PNG") +PNG = _png.getvalue() def run_node(script: str):