From eefb7601698fbe3a2422a846d02e9e9727d96f10 Mon Sep 17 00:00:00 2001 From: timmy Date: Mon, 10 Aug 2026 08:55:13 +0000 Subject: [PATCH] feat: attach screenshots to issue comments (#467) --- README.md | 3 + frontend/dashboard.css | 8 + frontend/dashboard.js | 64 +++++++- frontend/index.html | 10 ++ frontend/issue-attachment.js | 107 +++++++++++++ frontend/service-worker.js | 1 + src/gitea_proxy.py | 27 ++++ src/main.py | 84 ++++++++++ src/request_boundary.py | 8 + tests/test_issue_attachment_ui.py | 134 ++++++++++++++++ tests/test_issue_attachments.py | 256 ++++++++++++++++++++++++++++++ tests/test_request_boundary.py | 7 + tests/test_service_worker.py | 1 + 13 files changed, 702 insertions(+), 8 deletions(-) create mode 100644 frontend/issue-attachment.js create mode 100644 tests/test_issue_attachment_ui.py create mode 100644 tests/test_issue_attachments.py diff --git a/README.md b/README.md index b95ef8c..65a5707 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,9 @@ threads, create and self-assign issues, discover, claim, and release issue assig list repository labels and open milestones, set or clear due dates on assigned issues, create issue comments, close assigned issues, inspect/comment on assigned pull requests, merge assigned pull requests, and submit pull-request reviews. +Assigned-issue comments can include one PNG, JPEG, or WebP screenshot up to 2 MB. +The screenshot uploads before the comment is posted; validation or upload failures keep +both the typed comment and removable preview available for retry. Pull-request replies and mobile My Work issue and PR comments use Gitea's issue-comment API. In issue, pull-request, and unread-update conversations, typing at least two characters after `@` offers repository-scoped teammate suggestions; diff --git a/frontend/dashboard.css b/frontend/dashboard.css index e10dfef..3b2df09 100644 --- a/frontend/dashboard.css +++ b/frontend/dashboard.css @@ -255,6 +255,14 @@ textarea { resize: vertical; min-height: 120px; } .issue-comment { padding:10px 0; border-bottom:1px solid #1b2d45; } .issue-comment-composer { display:grid; gap:8px; margin-top:16px; } .issue-comment-composer button { min-height:44px; width:100%; } +.visually-hidden { position:absolute; width:1px; height:1px; padding:0; margin:-1px; overflow:hidden; clip:rect(0,0,0,0); white-space:nowrap; border:0; } +.issue-attachment-controls { display:flex; max-width:100%; } +.issue-attachment-trigger { min-height:44px; display:inline-flex; align-items:center; justify-content:center; padding:8px 12px; border:1px solid #60a5fa; border-radius:10px; color:#dbeafe; font-weight:700; cursor:pointer; } +.issue-attachment-preview { display:grid; grid-template-columns:64px minmax(0,1fr); gap:8px 12px; align-items:center; max-width:100%; overflow:hidden; padding:10px; border:1px solid #315781; border-radius:10px; background:#101f34; } +.issue-attachment-preview[hidden] { display:none; } +.issue-attachment-preview img { grid-row:span 2; width:64px; height:64px; object-fit:cover; border-radius:8px; } +.issue-attachment-preview .small { min-width:0; overflow-wrap:anywhere; } +.issue-attachment-preview button { min-height:44px; width:auto; justify-self:start; } .mention-options { display:grid; max-width:100%; max-height:220px; overflow:auto; border:1px solid #315781; border-radius:10px; background:#101f34; box-shadow:0 10px 28px rgba(0,0,0,.35); } .mention-options[hidden] { display:none; } .mention-option { min-height:44px; max-width:100%; overflow:hidden; padding:9px 12px; border:0; border-bottom:1px solid #203a5c; border-radius:0; text-align:left; text-overflow:ellipsis; white-space:nowrap; background:#101f34; color:#dbeafe; } diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 2124e2d..07877b6 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -272,6 +272,37 @@ mobile: window.matchMedia('(max-width: 600px)').matches, }); const issueController = createIssueSheet({ fetchJson: fetchReviewJson, storage: localStorage }); + const issueAttachmentController = issueAttachment.mount({ + input: qs('#issue-attachment'), + preview: qs('#issue-attachment-preview'), + image: qs('#issue-attachment-image'), + meta: qs('#issue-attachment-meta'), + remove: qs('#remove-issue-attachment'), + status: qs('#issue-comment-status'), + createObjectURL: file => URL.createObjectURL(file), + revokeObjectURL: url => URL.revokeObjectURL(url), + readDataUrl: file => new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(reader.result); + reader.onerror = () => reject(new Error('The screenshot could not be read. Choose it again.')); + reader.readAsDataURL(file); + }), + upload: payload => { + const repository = payload.repository.split('/').map(encodeURIComponent).join('/'); + return fetchReviewJson( + 'api/v1/repos/' + repository + '/issues/' + encodeURIComponent(payload.number) + '/attachments', + { + method: 'POST', + headers: { Accept: 'application/json', 'Content-Type': 'application/json' }, + body: JSON.stringify({ + filename: payload.filename, + content_type: payload.content_type, + data: payload.data, + }), + }, + ); + }, + }); const planningLoader = createIssueSheet.createPlanningLoader({ loadLabels: item => issueController.loadLabels(item), loadMilestones: item => issueController.loadMilestones(item), @@ -2143,6 +2174,7 @@ return; } mobileComposerViewport.close(qs('#issue-sheet .issue-sheet-panel')); + issueAttachmentController.clear(); qs('#issue-sheet').classList.remove('open'); selectedIssue = null; selectedIssueOffline = false; @@ -3729,7 +3761,7 @@ const textarea = qs('#' + kind + '-comment'); const status = qs('#' + kind + '-comment-status'); const body = textarea.value.trim(); - if (!body) { + if (!body && (kind !== 'issue' || !issueAttachmentController.state())) { status.textContent = 'Write a comment before posting.'; textarea.focus(); return; @@ -3741,17 +3773,21 @@ item.repository + '#' + item.number + ':operation'); postButton.disabled = true; nextButton.disabled = true; - status.textContent = 'Posting comment and opening next…'; + status.textContent = kind === 'issue' && issueAttachmentController.state() ? + 'Uploading screenshot before opening next…' : 'Posting comment and opening next…'; try { - const result = await controller.submit(item, body, operationId); + const preparedBody = kind === 'issue' ? + await issueAttachmentController.prepareComment(item, body) : body; + const result = await controller.submit(item, preparedBody, operationId); const stillOpen = kind === 'issue' ? selectedIssue === item : selectedPull === item; if (!stillOpen) return; + if (kind === 'issue') issueAttachmentController.clear(); if (!result.completed) status.textContent = 'Comment saved, but Today still needs completion.'; else if (result.delivery === 'posted') status.textContent = 'Comment posted.'; else if (result.background) status.textContent = 'Queued for sync when the connection returns.'; else status.textContent = 'Saved for next launch; background delivery unavailable.'; } catch (error) { - status.textContent = error.message + ' Your draft and Today position are safe; retry.'; + status.textContent = error.message + ' Your draft, screenshot, and Today position are safe; retry.'; textarea.focus(); } finally { postButton.disabled = false; @@ -3763,28 +3799,40 @@ qs('#send-issue-comment').addEventListener('click', async () => { if (!selectedIssue) return; const body = qs('#issue-comment').value.trim(); - if (!body) { + if (!body && !issueAttachmentController.state()) { qs('#issue-comment-status').textContent = 'Write a comment before posting.'; qs('#issue-comment').focus(); return; } const button = qs('#send-issue-comment'); button.disabled = true; - qs('#issue-comment-status').textContent = 'Posting comment…'; + qs('#issue-comment-status').textContent = issueAttachmentController.state() ? + 'Uploading screenshot…' : 'Posting comment…'; + let preparedBody; try { - const comment = await issueController.comment(selectedIssue, body); + preparedBody = await issueAttachmentController.prepareComment(selectedIssue, body); + } catch (error) { + qs('#issue-comment-status').textContent = error.message + ' Your comment and screenshot are safe; retry.'; + qs('#issue-comment').focus(); + button.disabled = false; + return; + } + try { + const comment = await issueController.comment(selectedIssue, preparedBody); if (issueConversation) renderIssueConversation(issueConversation.append(comment)); qs('#issue-comment').value = ''; + issueAttachmentController.clear(); qs('#issue-comment-status').textContent = 'Comment posted.'; } catch (error) { if (canQueueMessage(error)) { const operationId = localStorage.getItem('stackchain.issue-comment.v1:' + selectedIssue.repository + '#' + selectedIssue.number + ':operation'); qs('#issue-comment-status').textContent = 'Saving for background delivery…'; const admission = await authoredOutbox.enqueueDurably({ kind:'issue-comment', repository:selectedIssue.repository, - number:selectedIssue.number, body, operationId }); + number:selectedIssue.number, body:preparedBody, operationId }); refreshMyWorkView(); if (admission.background) { qs('#issue-comment').value = ''; + issueAttachmentController.clear(); qs('#issue-comment-status').textContent = 'Queued for sync when the connection returns.'; } else { qs('#issue-comment-status').textContent = 'Saved for next launch; background delivery unavailable.'; diff --git a/frontend/index.html b/frontend/index.html index 4897fae..f097a01 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -297,6 +297,15 @@
+
+ + +
+
@@ -660,6 +669,7 @@ + diff --git a/frontend/issue-attachment.js b/frontend/issue-attachment.js new file mode 100644 index 0000000..0659a69 --- /dev/null +++ b/frontend/issue-attachment.js @@ -0,0 +1,107 @@ +(function(root, factory) { + const api = factory(); + if (typeof module === 'object' && module.exports) module.exports = api; + else root.issueAttachment = api; +})(typeof self !== 'undefined' ? self : this, function() { + 'use strict'; + + const MAX_BYTES = 2 * 1024 * 1024; + const IMAGE_TYPES = new Set(['image/png', 'image/jpeg', 'image/webp']); + + function create(options) { + const readDataUrl = options.readDataUrl; + const upload = options.upload; + let selected = null; + let confirmed = null; + + function select(file) { + if (!file || !IMAGE_TYPES.has(file.type)) { + throw new Error('Choose a PNG, JPEG, or WebP screenshot.'); + } + if (!Number.isFinite(file.size) || file.size <= 0 || file.size > MAX_BYTES) { + throw new Error('Choose a screenshot that is 2 MB or smaller.'); + } + selected = file; + confirmed = null; + return state(); + } + + function clear() { + selected = null; + confirmed = null; + } + + function state() { + return selected ? { + name: selected.name, + size: selected.size, + uploaded: Boolean(confirmed), + } : null; + } + + async function prepareComment(item, body) { + const text = String(body || '').trim(); + if (!selected) return text; + if (!confirmed) { + const dataUrl = await readDataUrl(selected); + const marker = ';base64,'; + const markerAt = String(dataUrl).indexOf(marker); + if (markerAt < 0) throw new Error('The screenshot could not be read. Choose it again.'); + confirmed = await upload({ + repository: item.repository, + number: item.number, + filename: selected.name, + content_type: selected.type, + data: String(dataUrl).slice(markerAt + marker.length), + }); + if (!confirmed || typeof confirmed.markdown !== 'string' || !confirmed.markdown) { + confirmed = null; + throw new Error('The server did not confirm the screenshot upload.'); + } + } + return text ? text + '\n\n' + confirmed.markdown : confirmed.markdown; + } + + return { select, clear, state, prepareComment }; + } + + function mount(options) { + const controller = create(options); + const clearSelection = controller.clear; + let previewUrl = ''; + + function clearPreview() { + if (previewUrl) options.revokeObjectURL(previewUrl); + previewUrl = ''; + options.image.src = ''; + options.preview.hidden = true; + options.input.value = ''; + clearSelection(); + } + + options.input.addEventListener('change', event => { + const file = event.target.files && event.target.files[0]; + try { + controller.select(file); + } catch (error) { + options.status.textContent = error.message; + options.input.value = ''; + return; + } + if (previewUrl) options.revokeObjectURL(previewUrl); + previewUrl = options.createObjectURL(file); + options.image.src = previewUrl; + options.meta.textContent = file.name + ' · ' + Math.ceil(file.size / 1024) + ' KB'; + options.preview.hidden = false; + options.status.textContent = 'Screenshot ready to upload with this comment.'; + }); + options.remove.addEventListener('click', () => { + clearPreview(); + options.status.textContent = 'Screenshot removed. Your comment is unchanged.'; + }); + + return Object.assign(controller, { clear: clearPreview }); + } + + return { create, mount, MAX_BYTES }; +}); diff --git a/frontend/service-worker.js b/frontend/service-worker.js index d0a61d7..f95206d 100644 --- a/frontend/service-worker.js +++ b/frontend/service-worker.js @@ -42,6 +42,7 @@ const SHELL = [ BASE + 'static/later-picker.js', BASE + 'static/pick-work.js', BASE + 'static/conversation.js', + BASE + 'static/issue-attachment.js', BASE + 'static/issue-sheet.js', BASE + 'static/create-issue-sheet.js', BASE + 'static/create-and-start.js', diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index a77cfc6..a5be57a 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -878,6 +878,33 @@ async def comment_on_issue(repository: str, number: int, body: str) -> dict: return _normalize_issue_comment(comment) +async def upload_assigned_issue_attachment( + repository: str, + number: int, + filename: str, + content_type: str, + content: bytes, +) -> dict: + if not await is_assigned_issue(repository, number): + raise IssueNotAvailableError("Assigned issue not found") + response = await _get_client().post( + f"/api/v1/repos/{repository}/issues/{number}/assets", + headers=_auth(), + params={"name": filename}, + files={"attachment": (filename, content, content_type)}, + ) + response.raise_for_status() + attachment = response.json() + if not isinstance(attachment, dict): + raise ValueError("Gitea attachment response was not an object") + name = attachment.get("name") + url = _safe_web_url(attachment.get("browser_download_url")) + size = attachment.get("size") + if not isinstance(name, str) or not name or not url or not isinstance(size, int): + raise ValueError("Gitea did not confirm the attachment") + return {"name": name, "url": url, "size": size} + + async def repo_labels(repository: str) -> list[dict]: response = await _get_client().get( f"/api/v1/repos/{repository}/labels", diff --git a/src/main.py b/src/main.py index e0bad49..a5991fa 100644 --- a/src/main.py +++ b/src/main.py @@ -1,4 +1,6 @@ import asyncio +import base64 +import binascii import hmac import math import os @@ -310,6 +312,46 @@ class IssueComment(BaseModel): return value +class IssueAttachment(BaseModel): + filename: str = Field(min_length=1, max_length=255) + content_type: Literal["image/png", "image/jpeg", "image/webp"] + data: str = Field(min_length=1, max_length=2_800_000) + + @model_validator(mode="after") + def validate_filename(self): + if ( + "/" in self.filename + or "\\" in self.filename + or any(ord(character) < 32 for character in self.filename) + ): + raise ValueError("attachment filename must be a plain file name") + extension = self.filename.rsplit(".", 1)[-1].lower() if "." in self.filename else "" + expected = { + "image/png": {"png"}, + "image/jpeg": {"jpg", "jpeg"}, + "image/webp": {"webp"}, + } + if extension not in expected[self.content_type]: + raise ValueError("attachment filename must match the selected image type") + return self + + def content(self) -> bytes: + try: + content = base64.b64decode(self.data, validate=True) + except (ValueError, binascii.Error) as exc: + raise ValueError("attachment data must be valid base64") from exc + if len(content) > 2 * 1024 * 1024: + raise ValueError("screenshot must be 2 MB or smaller") + signatures = { + "image/png": content.startswith(b"\x89PNG\r\n\x1a\n"), + "image/jpeg": content.startswith(b"\xff\xd8\xff"), + "image/webp": content.startswith(b"RIFF") and content[8:12] == b"WEBP", + } + if not signatures[self.content_type]: + raise ValueError("file contents do not match the selected image type") + return content + + class IssueCreation(BaseModel): title: str = Field(min_length=1, max_length=255) body: str = Field(default="", max_length=10_000) @@ -2728,6 +2770,48 @@ async def comment_on_assigned_issue( return JSONResponse(result, status_code=201) +@app.post("/api/v1/repos/{owner}/{repo}/issues/{number}/attachments", status_code=201) +async def attach_to_assigned_issue( + attachment: IssueAttachment, + owner: str, + repo: str, + number: int = PathParam(gt=0), +): + repository = f"{owner}/{repo}" + try: + content = attachment.content() + except ValueError as exc: + raise HTTPException(status_code=422, detail=str(exc)) from exc + try: + result = await asyncio.wait_for( + gitea_proxy.upload_assigned_issue_attachment( + repository, + number, + attachment.filename, + attachment.content_type, + content, + ), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + except gitea_proxy.IssueNotAvailableError as exc: + raise HTTPException(status_code=404, detail="Assigned issue not found") from exc + except HTTPException: + raise + except Exception: + return JSONResponse( + {"error": "The screenshot could not be uploaded. Your draft is safe; please retry."}, + status_code=503, + headers={"Retry-After": "1"}, + ) + safe_name = ( + result["name"].replace("\\", "\\\\").replace("[", "\\[").replace("]", "\\]") + .replace("\r", " ").replace("\n", " ") + ) + safe_url = result["url"].replace("<", "%3C").replace(">", "%3E") + result["markdown"] = f"![{safe_name}](<{safe_url}>)" + return JSONResponse(result, status_code=201) + + @app.patch("/api/v1/repos/{owner}/{repo}/issues/{number}/close") async def close_assigned_issue( request: Request, diff --git a/src/request_boundary.py b/src/request_boundary.py index 34cd81f..38ffba4 100644 --- a/src/request_boundary.py +++ b/src/request_boundary.py @@ -5,6 +5,7 @@ from starlette.types import ASGIApp, Message, Receive, Scope, Send SESSION_BODY_LIMIT = 16 * 1024 API_MUTATION_BODY_LIMIT = 64 * 1024 +ISSUE_ATTACHMENT_BODY_LIMIT = 3 * 1024 * 1024 MUTATION_METHODS = frozenset({"POST", "PUT", "PATCH"}) @@ -13,6 +14,13 @@ def request_body_limit(method: str, path: str) -> int | None: normalized_method = method.upper() if normalized_method == "POST" and path == "/api/v1/session": return SESSION_BODY_LIMIT + if ( + normalized_method == "POST" + and path.startswith("/api/v1/repos/") + and "/issues/" in path + and path.endswith("/attachments") + ): + return ISSUE_ATTACHMENT_BODY_LIMIT if normalized_method in MUTATION_METHODS and path.startswith("/api/v1/"): return API_MUTATION_BODY_LIMIT return None diff --git a/tests/test_issue_attachment_ui.py b/tests/test_issue_attachment_ui.py new file mode 100644 index 0000000..3582e8c --- /dev/null +++ b/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.*?)\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 diff --git a/tests/test_issue_attachments.py b/tests/test_issue_attachments.py new file mode 100644 index 0000000..b661e2c --- /dev/null +++ b/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]()", + } + 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"").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]()" + ) + + +@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 diff --git a/tests/test_request_boundary.py b/tests/test_request_boundary.py index f4d65e4..f460ed2 100644 --- a/tests/test_request_boundary.py +++ b/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 diff --git a/tests/test_service_worker.py b/tests/test_service_worker.py index cb4c334..754f406 100644 --- a/tests/test_service_worker.py +++ b/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", -- 2.43.0