feat: attach screenshots to issue comments (#467)

tests/test_issue_attachment_ui.py

@@ -0,0 +1,134 @@
+import json
+import re
+import subprocess
+from pathlib import Path
+
+
+ATTACHMENT = Path(__file__).parents[1] / "frontend" / "issue-attachment.js"
+INDEX = Path(__file__).parents[1] / "frontend" / "index.html"
+CSS = Path(__file__).parents[1] / "frontend" / "dashboard.css"
+DASHBOARD = Path(__file__).parents[1] / "frontend" / "dashboard.js"
+SERVICE_WORKER = Path(__file__).parents[1] / "frontend" / "service-worker.js"
+README = Path(__file__).parents[1] / "README.md"
+
+
+def run_node(script: str) -> str:
+    return subprocess.run(
+        ["node", "-e", script], check=True, capture_output=True, text=True
+    ).stdout
+
+
+def test_mobile_attachment_prepares_comment_and_reuses_confirmed_upload():
+    script = f"""
+const attachment = require({json.dumps(str(ATTACHMENT))});
+const calls = [];
+const file = {{name:'checkout.png', type:'image/png', size:8}};
+const controller = attachment.create({{
+  readDataUrl: async selected => {{ calls.push('read:' + selected.name); return 'data:image/png;base64,iVBORw0KGgo='; }},
+  upload: async payload => {{ calls.push(payload); return {{markdown:'![checkout.png](https://forge.example/a.png)'}}; }},
+}});
+controller.select(file);
+(async()=>{{
+  const first = await controller.prepareComment({{repository:'stackchain/api', number:17}}, 'Layout breaks');
+  const second = await controller.prepareComment({{repository:'stackchain/api', number:17}}, 'Layout breaks');
+  process.stdout.write(JSON.stringify({{first,second,calls,state:controller.state()}}));
+}})().catch(error=>{{ console.error(error); process.exit(1); }});
+"""
+    output = json.loads(run_node(script))
+    assert output["first"] == (
+        "Layout breaks\n\n![checkout.png](https://forge.example/a.png)"
+    )
+    assert output["second"] == output["first"]
+    assert output["calls"] == [
+        "read:checkout.png",
+        {
+            "repository": "stackchain/api",
+            "number": 17,
+            "filename": "checkout.png",
+            "content_type": "image/png",
+            "data": "iVBORw0KGgo=",
+        },
+    ]
+    assert output["state"] == {
+        "name": "checkout.png", "size": 8, "uploaded": True
+    }
+
+
+def test_issue_composer_renders_thumb_reachable_screenshot_preview():
+    html = INDEX.read_text()
+    css = CSS.read_text()
+
+    assert 'id="issue-attachment"' in html
+    assert 'type="file"' in html
+    assert 'accept="image/png,image/jpeg,image/webp"' in html
+    assert 'for="issue-attachment"' in html
+    assert 'id="issue-attachment-preview"' in html
+    assert 'id="remove-issue-attachment"' in html
+    assert '.issue-attachment-trigger' in css
+    assert '.issue-attachment-preview' in css
+    assert 'min-height:44px' in css
+
+
+def test_attachment_view_keeps_invalid_draft_and_removes_preview():
+    script = f"""
+const attachment = require({json.dumps(str(ATTACHMENT))});
+class Element {{
+  constructor() {{ this.listeners={{}}; this.hidden=true; this.value=''; this.files=[]; this.textContent=''; this.src=''; }}
+  addEventListener(type, fn) {{ this.listeners[type]=fn; }}
+  dispatch(type) {{ return this.listeners[type]({{target:this}}); }}
+}}
+const input=new Element(), preview=new Element(), image=new Element(), meta=new Element(), remove=new Element(), status=new Element();
+const revoked=[];
+const controller=attachment.mount({{
+  input,preview,image,meta,remove,status,
+  createObjectURL:()=> 'blob:preview', revokeObjectURL:url=>revoked.push(url),
+  readDataUrl:async()=>'', upload:async()=>{{}},
+}});
+input.files=[{{name:'payload.svg',type:'image/svg+xml',size:20}}]; input.dispatch('change');
+const invalid={{message:status.textContent,hidden:preview.hidden}};
+input.files=[{{name:'screen.png',type:'image/png',size:2048}}]; input.dispatch('change');
+const selected={{src:image.src,meta:meta.textContent,hidden:preview.hidden,state:controller.state()}};
+remove.dispatch('click');
+process.stdout.write(JSON.stringify({{invalid,selected,removed:{{hidden:preview.hidden,src:image.src,revoked,state:controller.state()}}}}));
+"""
+    output = json.loads(run_node(script))
+    assert output["invalid"] == {
+        "message": "Choose a PNG, JPEG, or WebP screenshot.", "hidden": True
+    }
+    assert output["selected"]["src"] == "blob:preview"
+    assert output["selected"]["hidden"] is False
+    assert "screen.png" in output["selected"]["meta"]
+    assert output["removed"] == {
+        "hidden": True, "src": "", "revoked": ["blob:preview"], "state": None
+    }
+
+
+def test_issue_comment_actions_upload_before_posting_and_clear_after_acceptance():
+    source = DASHBOARD.read_text()
+
+    assert "issueAttachment.mount({" in source
+    assert "'/attachments'" in source
+    assert "issueAttachmentController.prepareComment(item, body)" in source
+    assert "issueAttachmentController.prepareComment(selectedIssue, body)" in source
+    assert source.count("issueAttachmentController.clear();") >= 2
+
+
+def test_closing_issue_sheet_cannot_carry_a_screenshot_to_another_issue():
+    source = DASHBOARD.read_text()
+    close_body = re.search(
+        r"function closeIssueSheet\(navigate = true\) \{(?P<body>.*?)\n  \}",
+        source,
+        re.DOTALL,
+    ).group("body")
+    assert "issueAttachmentController.clear();" in close_body
+
+
+def test_attachment_runtime_is_available_in_the_offline_app_shell():
+    assert "static/issue-attachment.js" in SERVICE_WORKER.read_text()
+
+
+def test_readme_documents_mobile_screenshot_limits_and_delivery_order():
+    readme = README.read_text()
+    assert "PNG, JPEG, or WebP" in readme
+    assert "2 MB" in readme
+    assert "uploads before the comment is posted" in readme

tests/test_issue_attachments.py

@@ -0,0 +1,256 @@
+import base64
+
+import httpx
+import pytest
+
+from src import gitea_proxy, main
+
+
+PNG_BYTES = b"\x89PNG\r\n\x1a\n" + b"mobile screenshot"
+
+
+@pytest.mark.anyio
+async def test_attachment_endpoint_uploads_valid_screenshot_to_assigned_issue(monkeypatch):
+    calls = []
+
+    async def upload(repository, number, filename, content_type, content):
+        calls.append((repository, number, filename, content_type, content))
+        return {
+            "name": "checkout.png",
+            "url": "https://forge.example/attachments/checkout.png",
+            "size": len(content),
+        }
+
+    monkeypatch.setattr(
+        main.gitea_proxy, "upload_assigned_issue_attachment", upload, raising=False
+    )
+    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",
+            json={
+                "filename": "checkout.png",
+                "content_type": "image/png",
+                "data": base64.b64encode(PNG_BYTES).decode("ascii"),
+            },
+        )
+
+    assert response.status_code == 201
+    assert response.headers["cache-control"] == "no-store"
+    assert response.json() == {
+        "name": "checkout.png",
+        "url": "https://forge.example/attachments/checkout.png",
+        "size": len(PNG_BYTES),
+        "markdown": "![checkout.png](<https://forge.example/attachments/checkout.png>)",
+    }
+    assert calls == [
+        ("stackchain/api", 17, "checkout.png", "image/png", PNG_BYTES)
+    ]
+
+
+@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))
+
+    async def upload(_repository, _number, filename, _content_type, content):
+        return {
+            "name": filename,
+            "url": "https://forge.example/attachments/screen.png",
+            "size": len(content),
+        }
+
+    monkeypatch.setattr(
+        main.gitea_proxy, "upload_assigned_issue_attachment", upload, raising=False
+    )
+    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",
+            json={
+                "filename": "screen.png",
+                "content_type": "image/png",
+                "data": base64.b64encode(screenshot).decode("ascii"),
+            },
+        )
+
+    assert response.status_code == 201
+    assert response.json()["size"] == len(screenshot)
+
+
+@pytest.mark.anyio
+async def test_attachment_endpoint_rejects_spoofed_image_before_upstream(monkeypatch):
+    called = False
+
+    async def upload(*_args):
+        nonlocal called
+        called = True
+
+    monkeypatch.setattr(
+        main.gitea_proxy, "upload_assigned_issue_attachment", upload, raising=False
+    )
+    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",
+            json={
+                "filename": "not-really.png",
+                "content_type": "image/png",
+                "data": base64.b64encode(b"<script>alert(1)</script>").decode("ascii"),
+            },
+        )
+
+    assert response.status_code == 422
+    assert response.headers["cache-control"] == "no-store"
+    assert called is False
+
+
+@pytest.mark.anyio
+async def test_attachment_endpoint_rejects_image_over_two_megabytes_before_upstream(monkeypatch):
+    called = False
+
+    async def upload(*_args):
+        nonlocal called
+        called = True
+        return {"name": "large.png", "url": "https://forge.example/a", "size": 1}
+
+    monkeypatch.setattr(main.gitea_proxy, "upload_assigned_issue_attachment", upload)
+    oversized = b"\x89PNG\r\n\x1a\n" + b"x" * (2 * 1024 * 1024 - 7)
+    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",
+            json={
+                "filename": "large.png",
+                "content_type": "image/png",
+                "data": base64.b64encode(oversized).decode("ascii"),
+            },
+        )
+
+    assert len(oversized) == 2 * 1024 * 1024 + 1
+    assert response.status_code == 422
+    assert called is False
+
+
+@pytest.mark.anyio
+async def test_gitea_attachment_revalidates_assignment_and_sends_multipart():
+    requests = []
+
+    async def handler(request):
+        requests.append(request)
+        if request.url.path == "/api/v1/user":
+            return httpx.Response(200, json={"login": "timmy"})
+        if request.method == "GET":
+            return httpx.Response(200, json={
+                "state": "open", "pull_request": None,
+                "assignees": [{"login": "timmy"}],
+            })
+        return httpx.Response(201, json={
+            "id": 9,
+            "name": "checkout.png",
+            "size": len(PNG_BYTES),
+            "browser_download_url": "https://forge.example/attachments/checkout.png",
+        })
+
+    gitea_proxy.start_client(transport=httpx.MockTransport(handler))
+    try:
+        result = await gitea_proxy.upload_assigned_issue_attachment(
+            "stackchain/api", 17, "checkout.png", "image/png", PNG_BYTES
+        )
+    finally:
+        await gitea_proxy.stop_client()
+
+    assert [(request.method, request.url.path) for request in requests] == [
+        ("GET", "/api/v1/user"),
+        ("GET", "/api/v1/repos/stackchain/api/issues/17"),
+        ("POST", "/api/v1/repos/stackchain/api/issues/17/assets"),
+    ]
+    upload = requests[-1]
+    assert upload.url.params["name"] == "checkout.png"
+    assert "multipart/form-data" in upload.headers["content-type"]
+    assert b'name="attachment"; filename="checkout.png"' in upload.content
+    assert PNG_BYTES in upload.content
+    assert result == {
+        "name": "checkout.png",
+        "url": "https://forge.example/attachments/checkout.png",
+        "size": len(PNG_BYTES),
+    }
+
+
+@pytest.mark.anyio
+async def test_attachment_markdown_escapes_untrusted_confirmed_filename(monkeypatch):
+    async def upload(*_args):
+        return {
+            "name": "screen](not-an-image).png",
+            "url": "https://forge.example/attachments/a.png",
+            "size": len(PNG_BYTES),
+        }
+
+    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",
+            json={
+                "filename": "screen.png",
+                "content_type": "image/png",
+                "data": base64.b64encode(PNG_BYTES).decode("ascii"),
+            },
+        )
+
+    assert response.status_code == 201
+    assert response.json()["markdown"] == (
+        "![screen\\](not-an-image).png](<https://forge.example/attachments/a.png>)"
+    )
+
+
+@pytest.mark.anyio
+async def test_attachment_endpoint_hides_an_issue_that_is_no_longer_assigned(monkeypatch):
+    async def upload(*_args):
+        raise gitea_proxy.IssueNotAvailableError("not assigned")
+
+    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",
+            json={
+                "filename": "screen.png",
+                "content_type": "image/png",
+                "data": base64.b64encode(PNG_BYTES).decode("ascii"),
+            },
+        )
+
+    assert response.status_code == 404
+    assert response.headers["cache-control"] == "no-store"
+
+
+@pytest.mark.anyio
+async def test_attachment_endpoint_rejects_unsafe_or_mismatched_filename(monkeypatch):
+    called = False
+
+    async def upload(*_args):
+        nonlocal called
+        called = True
+
+    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:
+        traversal = await client.post(
+            "/api/v1/repos/stackchain/api/issues/17/attachments",
+            json={
+                "filename": "../payload.png",
+                "content_type": "image/png",
+                "data": base64.b64encode(PNG_BYTES).decode("ascii"),
+            },
+        )
+        mismatch = await client.post(
+            "/api/v1/repos/stackchain/api/issues/17/attachments",
+            json={
+                "filename": "payload.html",
+                "content_type": "image/png",
+                "data": base64.b64encode(PNG_BYTES).decode("ascii"),
+            },
+        )
+
+    assert [traversal.status_code, mismatch.status_code] == [422, 422]
+    assert called is False

tests/test_request_boundary.py

@@ -112,6 +112,13 @@ def test_request_limits_are_route_specific_and_cover_api_mutations():
         == 64 * 1024
     )
     assert main.request_body_limit("GET", "/api/v1/context") is None
+    assert (
+        main.request_body_limit(
+            "POST", "/api/v1/repos/stackchain/project/issues/17/attachments"
+        )
+        == 3 * 1024 * 1024
+    )
+    assert main.request_body_limit("POST", "/unrelated/attachments") is None
     assert main.request_body_limit("POST", "/unrelated") is None
 
 

tests/test_service_worker.py

@@ -437,6 +437,7 @@ def test_install_precaches_complete_subpath_scoped_app_shell():
         "/dashboard/static/later-picker.js",
         "/dashboard/static/pick-work.js",
         "/dashboard/static/conversation.js",
+        "/dashboard/static/issue-attachment.js",
         "/dashboard/static/issue-sheet.js",
         "/dashboard/static/create-issue-sheet.js",
         "/dashboard/static/create-and-start.js",