security: sanitize image evidence server-side (Closes #983)
All checks were successful
CI / lint (pull_request) Successful in 2m11s
CI / build-release (pull_request) Successful in 5s
CI / browser-journey (pull_request) Successful in 1m11s
CI / release-candidate (pull_request) Has been skipped

This commit is contained in:
timmy 2026-08-16 19:33:46 +00:00
parent 21eb223882
commit 966028e0a6
7 changed files with 192 additions and 6 deletions

View File

@ -1,6 +1,7 @@
fastapi==0.133.1 fastapi==0.133.1
httpx==0.28.1 httpx==0.28.1
pydantic==2.13.4 pydantic==2.13.4
Pillow==12.3.0
python-multipart==0.0.22 python-multipart==0.0.22
pytest==9.1.1 pytest==9.1.1
pywebpush==2.1.2 pywebpush==2.1.2

38
src/image_sanitizer.py Normal file
View File

@ -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

View File

@ -41,6 +41,7 @@ from src.gitea_proxy import (
repos, repos,
) )
from src.idempotency import IdempotencyLedger, IdempotencyLedgerBusy 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.login_attempt_store import LoginAttemptStore, LoginAttemptStoreError, client_source
from src.live_snapshot_store import ( from src.live_snapshot_store import (
LiveSnapshotMetadata, LiveSnapshotMetadata,
@ -796,6 +797,13 @@ def _validate_binary_attachment(filename: str, content_type: str, content: bytes
return _validate_attachment_content(content_type, content) 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): class IssueCreation(BaseModel):
title: str = Field(min_length=1, max_length=255) title: str = Field(min_length=1, max_length=255)
body: str = Field(default="", max_length=10_000) body: str = Field(default="", max_length=10_000)
@ -3157,6 +3165,7 @@ async def attach_to_global_search_preview(
filename = str(uploaded.filename or "") filename = str(uploaded.filename or "")
content_type = str(uploaded.content_type or "") content_type = str(uploaded.content_type or "")
content = _validate_binary_attachment(filename, content_type, await uploaded.read()) content = _validate_binary_attachment(filename, content_type, await uploaded.read())
content = await _sanitize_attachment(content_type, content)
except (ValueError, ValidationError) as exc: except (ValueError, ValidationError) as exc:
raise HTTPException(status_code=422, detail=str(exc)) from exc raise HTTPException(status_code=422, detail=str(exc)) from exc
@ -4337,6 +4346,7 @@ async def attach_to_notification(
filename = attachment.filename filename = attachment.filename
content_type = attachment.content_type content_type = attachment.content_type
content = attachment.content() content = attachment.content()
content = await _sanitize_attachment(content_type, content)
except (ValueError, ValidationError) as exc: except (ValueError, ValidationError) as exc:
raise HTTPException(status_code=422, detail=str(exc)) from exc raise HTTPException(status_code=422, detail=str(exc)) from exc
@ -5320,6 +5330,7 @@ async def attach_to_assigned_issue(
filename = attachment.filename filename = attachment.filename
content_type = attachment.content_type content_type = attachment.content_type
content = attachment.content() content = attachment.content()
content = await _sanitize_attachment(content_type, content)
except (ValueError, ValidationError) as exc: except (ValueError, ValidationError) as exc:
raise HTTPException(status_code=422, detail=str(exc)) from exc raise HTTPException(status_code=422, detail=str(exc)) from exc
async def upload_attachment(): async def upload_attachment():
@ -5388,6 +5399,7 @@ async def attach_to_assigned_pull(
filename = attachment.filename filename = attachment.filename
content_type = attachment.content_type content_type = attachment.content_type
content = attachment.content() content = attachment.content()
content = await _sanitize_attachment(content_type, content)
except (ValueError, ValidationError) as exc: except (ValueError, ValidationError) as exc:
raise HTTPException(status_code=422, detail=str(exc)) from exc raise HTTPException(status_code=422, detail=str(exc)) from exc

View File

@ -1,11 +1,19 @@
import asyncio import asyncio
import io
import httpx import httpx
import pytest import pytest
from PIL import Image
from src import gitea_proxy, main 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 @pytest.mark.anyio
async def test_global_search_endpoint_scopes_every_page_to_an_exact_repository(monkeypatch): async def test_global_search_endpoint_scopes_every_page_to_an_exact_repository(monkeypatch):
requested = [] 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) monkeypatch.setattr(main.gitea_proxy, "upload_preview_attachment", upload)
transport = httpx.ASGITransport(app=main.app) transport = httpx.ASGITransport(app=main.app)
headers = {"Idempotency-Key": "search-photo-pull-42"} 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")} files = {"file": ("photo.png", image, "image/png")}
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
first = await client.post( 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: async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
response = await client.post( response = await client.post(
"/api/v1/repos/stackchain/api/issues/42/preview/attachments?kind=pull", "/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"}, headers={"Idempotency-Key": "search-photo-mismatch-42"},
) )

View File

@ -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())

View File

@ -1,13 +1,49 @@
import asyncio import asyncio
import base64 import base64
import io
import time
import httpx import httpx
import pytest import pytest
from PIL import Image, PngImagePlugin
from src import gitea_proxy, main 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) @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 @pytest.mark.anyio
async def test_attachment_endpoint_accepts_binary_multipart_without_base64_expansion(monkeypatch): async def test_attachment_endpoint_accepts_binary_multipart_without_base64_expansion(monkeypatch):
calls = [] calls = []
@ -186,7 +246,7 @@ async def test_attachment_endpoint_rejects_changed_upload_for_used_key(monkeypat
json={ json={
"filename": "checkout.png", "filename": "checkout.png",
"content_type": "image/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"}, 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 @pytest.mark.anyio
async def test_attachment_endpoint_admits_a_normal_phone_screenshot(monkeypatch): 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): async def upload(_repository, _number, filename, _content_type, content):
return { return {

View File

@ -1,9 +1,11 @@
import io
import json import json
import subprocess import subprocess
from pathlib import Path from pathlib import Path
import httpx import httpx
import pytest import pytest
from PIL import Image
from src import gitea_proxy, main from src import gitea_proxy, main
from tests.dashboard_bundle import dashboard from tests.dashboard_bundle import dashboard
@ -12,7 +14,9 @@ from tests.dashboard_bundle import dashboard
ROOT = Path(__file__).parents[1] ROOT = Path(__file__).parents[1]
OUTBOX = ROOT / "frontend" / "authored-outbox.js" OUTBOX = ROOT / "frontend" / "authored-outbox.js"
SYNC = ROOT / "frontend" / "background-issue-sync.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): def run_node(script: str):