diff --git a/apps/board/app.py b/apps/board/app.py index fc67b0b..cf08581 100644 --- a/apps/board/app.py +++ b/apps/board/app.py @@ -78,6 +78,10 @@ def create_app(overrides: dict | None = None) -> Flask: # picture is silently refused has no way to tell what went wrong. MAX_CONTENT_LENGTH=10 * 1024 * 1024, UPLOAD_MAX_BYTES=int(os.environ.get("BOARD_UPLOAD_MAX_BYTES", 8 * 1024 * 1024)), + # Board pictures, deliberately inside the volume that already + # holds board.db: one directory to back up, not two, and no + # second mount to remember when moving the app to a new box. + UPLOAD_DIR=os.environ.get("BOARD_UPLOAD_DIR", "/data/uploads"), ) if overrides: app.config.update(overrides) @@ -87,10 +91,11 @@ def create_app(overrides: dict | None = None) -> Flask: # restart, which is a confusing way to find out the variable is unset. raise RuntimeError("BOARD_SECRET_KEY is required (generate one with `openssl rand -hex 32`).") - from . import auth, board, content, members + from . import auth, board, content, members, uploads app.register_blueprint(auth.bp, url_prefix=URL_PREFIX) app.register_blueprint(members.bp, url_prefix=URL_PREFIX) app.register_blueprint(content.bp, url_prefix=URL_PREFIX) + app.register_blueprint(uploads.bp, url_prefix=URL_PREFIX) app.register_blueprint(board.bp, url_prefix=URL_PREFIX) app.teardown_appcontext(close_db) diff --git a/apps/board/board.py b/apps/board/board.py index d6e3f93..9e58a90 100644 --- a/apps/board/board.py +++ b/apps/board/board.py @@ -18,6 +18,7 @@ from __future__ import annotations from flask import (Blueprint, abort, current_app, flash, g, redirect, render_template, request, url_for) +from . import uploads from .db import get_db from .render import excerpt, to_html from .security import admin_required, login_required @@ -130,7 +131,9 @@ def thread(thread_id: int): return render_template("thread.html", thread=row, author=author["display_name"], comments=comments, body_html=to_html(row["body_md"]), to_html=to_html, may_edit=may_edit, may_delete=may_delete, - is_admin=is_admin()) + is_admin=is_admin(), + thread_images=uploads.for_threads([thread_id]).get(thread_id, []), + comment_images=uploads.for_comments([c["id"] for c in comments])) @bp.route("/nuevo", methods=["GET", "POST"]) @@ -147,11 +150,20 @@ def new_thread(): flash(f"Espera {wait} segundos antes de publicar otra vez.", "error") return redirect(url_for("board.new_thread")) + # Checked before the thread exists, so a refused picture does not leave a + # half-made post behind for its author to find and wonder about. + try: + staged = uploads.stage(request.files.getlist("pictures")) + except uploads.RejectedUpload as exc: + flash(str(exc), "error") + return redirect(url_for("board.new_thread")) + title, body = cleaned cursor = get_db().execute( "INSERT INTO threads (author_id, title, body_md) VALUES (?, ?, ?)", (g.member["id"], title, body), ) + uploads.save(staged, g.member["id"], thread_id=cursor.lastrowid) return redirect(url_for("board.thread", thread_id=cursor.lastrowid)) @@ -203,10 +215,17 @@ def comment(thread_id: int): flash(f"Espera {wait} segundos antes de comentar otra vez.", "error") return redirect(url_for("board.thread", thread_id=thread_id)) - get_db().execute( + try: + staged = uploads.stage(request.files.getlist("pictures")) + except uploads.RejectedUpload as exc: + flash(str(exc), "error") + return redirect(url_for("board.thread", thread_id=thread_id)) + + cursor = get_db().execute( "INSERT INTO comments (thread_id, author_id, body_md) VALUES (?, ?, ?)", (thread_id, g.member["id"], body), ) + uploads.save(staged, g.member["id"], comment_id=cursor.lastrowid) return redirect(url_for("board.thread", thread_id=thread_id) + "#final") diff --git a/apps/board/content.py b/apps/board/content.py index f9ad999..5f9aec0 100644 --- a/apps/board/content.py +++ b/apps/board/content.py @@ -34,6 +34,10 @@ from . import gitea, tokens from .db import get_db from .render import to_html from .security import admin_required +# One list of accepted formats for the whole app, kept in the module that +# knows what each one looks like on the wire, so the editor and the board +# cannot drift apart about what a picture is. +from .uploads import IMAGE_EXTENSIONS bp = Blueprint("content", __name__) @@ -51,7 +55,6 @@ COLLECTIONS = { CATEGORIES = ["Turismo", "Cultura", "Gastronomía", "Comunidad", "Comercio"] UPLOAD_FOLDER = "static/uploads" -IMAGE_EXTENSIONS = {"jpg", "jpeg", "png", "webp", "gif", "avif"} TITLE_MAX = 140 BODY_MAX = 100_000 diff --git a/apps/board/schema.sql b/apps/board/schema.sql index 4be14bd..b46b7a6 100644 --- a/apps/board/schema.sql +++ b/apps/board/schema.sql @@ -55,6 +55,32 @@ CREATE TABLE IF NOT EXISTS comments ( CREATE INDEX IF NOT EXISTS comments_thread ON comments(thread_id, created_at) WHERE deleted_at IS NULL; +-- Pictures attached to a thread or a comment. +-- +-- The file itself lives in /data/uploads; this is the record of what it is and +-- what it belongs to. `stored_name` is generated, never the name the browser +-- sent, and is UNIQUE because it is also the URL. +-- +-- The CHECK is the shape of the thing: an attachment hangs off exactly one of +-- the two, never both and never neither. Without it a row with both columns +-- set would be served under whichever parent was still alive, which is a +-- quiet way for a deleted thread's photo to stay readable. +CREATE TABLE IF NOT EXISTS attachments ( + id INTEGER PRIMARY KEY, + thread_id INTEGER REFERENCES threads(id), + comment_id INTEGER REFERENCES comments(id), + stored_name TEXT NOT NULL UNIQUE, + original_name TEXT NOT NULL, + content_type TEXT NOT NULL, + bytes INTEGER NOT NULL, + uploaded_by INTEGER NOT NULL REFERENCES members(id), + created_at TEXT NOT NULL DEFAULT (datetime('now')), + CHECK ((thread_id IS NULL) <> (comment_id IS NULL)) +); + +CREATE INDEX IF NOT EXISTS attachments_thread ON attachments(thread_id); +CREATE INDEX IF NOT EXISTS attachments_comment ON attachments(comment_id); + -- Gitea access tokens for the editor. -- -- Kept here rather than in the session cookie. Flask signs cookies but does not diff --git a/apps/board/static/board.css b/apps/board/static/board.css index 174fabc..80cbdb4 100644 --- a/apps/board/static/board.css +++ b/apps/board/static/board.css @@ -306,3 +306,21 @@ input[type="file"] { padding: 0.5rem 0; font-size: 0.9rem; } + +/* Attached pictures. A row that wraps, thumbnails rather than full-bleed: + a thread with four photos should still read as a conversation. */ +.shots { + list-style: none; + margin: 0.75rem 0 0; + padding: 0; + display: flex; + flex-wrap: wrap; + gap: 0.5rem; +} +.shots img { + display: block; + max-height: 220px; + max-width: 100%; + border-radius: var(--radius-md); + border: 1px solid var(--border-light); +} diff --git a/apps/board/templates/_attachments.html b/apps/board/templates/_attachments.html new file mode 100644 index 0000000..9394444 --- /dev/null +++ b/apps/board/templates/_attachments.html @@ -0,0 +1,16 @@ +{# Pictures on a thread or a comment. Each one links to itself so a photo can + be opened at full size without needing a viewer — the link is the viewer. #} +{% macro attachments(images) %} +{% if images %} + +{% endif %} +{% endmacro %} diff --git a/apps/board/templates/thread.html b/apps/board/templates/thread.html index df48f51..0b51c00 100644 --- a/apps/board/templates/thread.html +++ b/apps/board/templates/thread.html @@ -1,4 +1,5 @@ {% extends "base.html" %} +{% from "_attachments.html" import attachments %} {% block title %}{{ thread.title }}{% endblock %} {% block main %} @@ -11,6 +12,7 @@

{{ thread.title }}

{{ body_html }}
+ {{ attachments(thread_images) }}
{% if may_edit(thread) %} @@ -49,6 +51,7 @@ {% if c.edited_at %}(editado){% endif %}
{{ to_html(c.body_md) }}
+ {{ attachments(comment_images.get(c.id, [])) }}
{% if may_edit(c) %} Editar @@ -68,11 +71,16 @@ {% if thread.locked and not is_admin %}

Este tema está cerrado a nuevas respuestas.

{% else %} -
+ + + + +
{% endif %} diff --git a/apps/board/templates/thread_form.html b/apps/board/templates/thread_form.html index 3f3b18f..0f4ee8e 100644 --- a/apps/board/templates/thread_form.html +++ b/apps/board/templates/thread_form.html @@ -2,7 +2,7 @@ {% block title %}{{ 'Editar tema' if thread else 'Escribir' }}{% endblock %} {% block main %} -

{{ 'Editar tema' if thread else 'Nuevo tema' }}

@@ -15,6 +15,14 @@ + {# Only when writing, not when editing: the edit route does not read files, + and an input that silently does nothing is worse than no input. Photos + already attached survive an edit — they are separate rows. #} + {% if not thread %} + + + {% endif %} +
Cancelar diff --git a/apps/board/tests/conftest.py b/apps/board/tests/conftest.py index 049f6eb..0d0a0b2 100644 --- a/apps/board/tests/conftest.py +++ b/apps/board/tests/conftest.py @@ -34,6 +34,7 @@ def app(tmp_path): "OAUTH_CLIENT_ID": "cid", "OAUTH_CLIENT_SECRET": "secret", "ADMIN_TOKEN": "admintoken", + "UPLOAD_DIR": str(tmp_path / "uploads"), "TESTING": True, }) yield application diff --git a/apps/board/tests/test_uploads.py b/apps/board/tests/test_uploads.py new file mode 100644 index 0000000..56e41b1 --- /dev/null +++ b/apps/board/tests/test_uploads.py @@ -0,0 +1,216 @@ +"""Pictures on the board. + +Two things are being defended here, and they are not the same thing. + +One is the member: a photo they attach must survive, be visible to other +members, and stop being visible when the post comes down. + +The other is the server: an upload is the one place where a member hands over +bytes that this app later serves back. Most of these tests are about the ways +that can be abused, and they are written against the bytes rather than the +filename, because the filename is the attacker's to choose. +""" + +from __future__ import annotations + +import io +from pathlib import Path + +import pytest + +from apps.board import uploads + +PNG = b"\x89PNG\r\n\x1a\n" + b"\x00" * 64 +JPEG = b"\xff\xd8\xff\xe0" + b"\x00" * 64 +WEBP = b"RIFF\x00\x00\x00\x00WEBP" + b"\x00" * 64 +HTML = b"" + + +def picture(data=PNG, name="foto.png"): + return (io.BytesIO(data), name) + + +def post_thread(post, follow_redirects=False, **files): + """follow_redirects is named explicitly so it cannot fall into **files and + be posted as a form field — which it silently was, leaving two tests + asserting against a bare 302 body.""" + payload = {"title": "Con foto", "body": "Mirad esto"} + payload.update(files) + return post("/comunidad/nuevo", payload, content_type="multipart/form-data", + follow_redirects=follow_redirects) + + +# --- what counts as an image --------------------------------------------- + +@pytest.mark.parametrize("data,expected", [ + (PNG, "png"), (JPEG, "jpg"), (WEBP, "webp"), + (b"GIF89a" + b"\x00" * 32, "gif"), + (b"\x00\x00\x00\x20ftypavif" + b"\x00" * 32, "avif"), + (HTML, None), + (b"", None), + (b"\x89PNG", None), # truncated magic +]) +def test_the_bytes_decide(data, expected): + assert uploads._detect(data) is expected + + +def test_a_script_named_like_a_picture_is_refused(app, client, db, post, make_member, sign_in): + """The filename is the uploader's to choose, so it decides nothing. This is + the case that makes sniffing worth the code: stored and served back as + image/png, an HTML file is a script running on our own origin.""" + sign_in(make_member("maria")) + response = post_thread(post, pictures=picture(HTML, "gato.png"), follow_redirects=True) + + assert "no parece una imagen" in response.get_data(as_text=True) + assert db.execute("SELECT 1 FROM attachments").fetchone() is None + + +def test_a_misnamed_but_real_picture_is_kept(app, client, db, post, make_member, sign_in): + """The mirror of the test above, and the reason the extension is ignored + rather than compared: a JPEG called .png is somebody's phone being untidy, + not an attack, and it is stored as what it actually is.""" + sign_in(make_member("maria")) + post_thread(post, pictures=picture(JPEG, "foto.png")) + + row = db.execute("SELECT * FROM attachments").fetchone() + assert row["content_type"] == "image/jpeg" + assert row["stored_name"].endswith(".jpg") + + +def test_an_oversized_picture_is_refused(app, client, db, post, make_member, sign_in): + app.config["UPLOAD_MAX_BYTES"] = 100 + sign_in(make_member("maria")) + response = post_thread(post, pictures=picture(PNG + b"\x00" * 500), follow_redirects=True) + + assert "máximo" in response.get_data(as_text=True) + assert db.execute("SELECT 1 FROM attachments").fetchone() is None + + +def test_too_many_at_once_is_refused(app, client, db, post, make_member, sign_in): + sign_in(make_member("maria")) + response = post("/comunidad/nuevo", { + "title": "Muchas", "body": "Texto", + "pictures": [picture(PNG, f"f{n}.png") for n in range(uploads.MAX_FILES + 1)], + }, content_type="multipart/form-data", follow_redirects=True) + + assert "Como máximo" in response.get_data(as_text=True) + assert db.execute("SELECT 1 FROM attachments").fetchone() is None + + +# --- a refusal must not leave wreckage ----------------------------------- + +def test_a_refused_picture_leaves_no_thread_behind(app, client, db, post, make_member, sign_in): + """The reason staging is separate from saving. Insert the thread first and + a rejected photo leaves its author looking at a post they did not finish + writing, with no way to tell what happened.""" + sign_in(make_member("maria")) + post_thread(post, pictures=picture(HTML, "malo.png")) + + assert db.execute("SELECT 1 FROM threads").fetchone() is None + + +# --- the name on disk ----------------------------------------------------- + +def test_the_uploaded_name_is_never_used_as_a_path(app, client, db, post, make_member, sign_in): + sign_in(make_member("maria")) + post_thread(post, pictures=picture(PNG, "../../../etc/passwd.png")) + + stored = db.execute("SELECT * FROM attachments").fetchone() + assert "/" not in stored["stored_name"] + assert uploads.STORED_NAME.match(stored["stored_name"]) + # The original is kept, but only ever as text to show a person. + assert stored["original_name"] == "../../../etc/passwd.png" + + +def test_a_name_we_did_not_generate_is_not_served(app, client, make_member, sign_in): + sign_in(make_member("maria")) + for name in ("../board.db", "..%2Fboard.db", "board.db", "foto.png"): + assert client.get(f"/comunidad/media/{name}").status_code == 404 + + +# --- who can see them ----------------------------------------------------- + +def test_a_picture_is_served_to_a_member(app, client, db, post, make_member, sign_in): + sign_in(make_member("maria")) + post_thread(post, pictures=picture(PNG)) + name = db.execute("SELECT stored_name FROM attachments").fetchone()["stored_name"] + + response = client.get(f"/comunidad/media/{name}") + assert response.status_code == 200 + assert response.mimetype == "image/png" + assert response.data == PNG + + +def test_a_picture_is_not_served_to_a_stranger(app, client, db, post, make_member, sign_in): + sign_in(make_member("maria")) + post_thread(post, pictures=picture(PNG)) + name = db.execute("SELECT stored_name FROM attachments").fetchone()["stored_name"] + + with client.session_transaction() as session: + session.clear() + response = client.get(f"/comunidad/media/{name}") + assert response.status_code == 302 + assert "/comunidad/login" in response.headers["Location"] + + +def test_deleting_the_thread_takes_its_pictures_out_of_reach( + app, client, db, post, make_member, sign_in): + """Threads are soft-deleted, so without this check the row stays, the file + stays, and the photo of a post somebody asked to have removed is still + readable by anyone who noted the URL.""" + sign_in(make_member("maria")) + post_thread(post, pictures=picture(PNG)) + name = db.execute("SELECT stored_name FROM attachments").fetchone()["stored_name"] + thread_id = db.execute("SELECT id FROM threads").fetchone()["id"] + assert client.get(f"/comunidad/media/{name}").status_code == 200 + + post(f"/comunidad/tema/{thread_id}/eliminar") + assert client.get(f"/comunidad/media/{name}").status_code == 404 + + +def test_the_same_applies_to_a_deleted_comment(app, client, db, post, make_member, sign_in): + member = make_member("maria") + sign_in(member) + post_thread(post, pictures=picture(PNG)) + thread_id = db.execute("SELECT id FROM threads").fetchone()["id"] + post(f"/comunidad/tema/{thread_id}/comentar", + {"body": "Yo también", "pictures": picture(JPEG, "mia.jpg")}, + content_type="multipart/form-data") + + name = db.execute( + "SELECT stored_name FROM attachments WHERE comment_id IS NOT NULL").fetchone()["stored_name"] + comment_id = db.execute("SELECT id FROM comments").fetchone()["id"] + assert client.get(f"/comunidad/media/{name}").status_code == 200 + + post(f"/comunidad/comentario/{comment_id}/eliminar") + assert client.get(f"/comunidad/media/{name}").status_code == 404 + + +# --- and they show up ----------------------------------------------------- + +def test_the_picture_appears_on_the_thread(app, client, db, post, make_member, sign_in): + sign_in(make_member("maria")) + post_thread(post, pictures=picture(PNG)) + thread_id = db.execute("SELECT id FROM threads").fetchone()["id"] + name = db.execute("SELECT stored_name FROM attachments").fetchone()["stored_name"] + + body = client.get(f"/comunidad/tema/{thread_id}").get_data(as_text=True) + assert f"/comunidad/media/{name}" in body + + +def test_the_file_reaches_the_disk(app, client, db, post, make_member, sign_in): + sign_in(make_member("maria")) + post_thread(post, pictures=picture(PNG)) + name = db.execute("SELECT stored_name FROM attachments").fetchone()["stored_name"] + + assert (Path(app.config["UPLOAD_DIR"]) / name).read_bytes() == PNG + + +def test_nothing_is_written_when_no_picture_is_chosen(app, client, db, post, make_member, sign_in): + """An empty file input still arrives in the request; it must not become a + zero-byte attachment.""" + sign_in(make_member("maria")) + post_thread(post, pictures=(io.BytesIO(b""), "")) + + assert db.execute("SELECT 1 FROM attachments").fetchone() is None + assert db.execute("SELECT 1 FROM threads").fetchone() is not None diff --git a/apps/board/uploads.py b/apps/board/uploads.py new file mode 100644 index 0000000..10a1a31 --- /dev/null +++ b/apps/board/uploads.py @@ -0,0 +1,214 @@ +"""Pictures on the board. + +Three decisions worth stating, because each one is a place this could have +gone wrong quietly. + +**Not in the site repository.** `content.py` uploads pictures by committing +them, which is right for a post that is about to be published. A photo attached +to a thread in here is the opposite: it is private, and committing it would put +it through the build pipeline and out onto vienalatina.com. These go to a +directory on disk that only this app serves, behind the same login as the +thread they belong to. + +**The file says what it is; the filename is an opinion.** `content.py` trusts +the extension, which is tolerable there because Hugo serves the result as a +static file. Here *we* serve it back, so the type is read from the first bytes +and the name the browser sent is never used for anything but display. A file +called `gato.png` that is really HTML is refused, rather than stored and later +handed back with a content type that invites the browser to run it. + +**The directory lives inside the volume that gets backed up.** `/data/uploads` +sits next to `board.db` in the same mount, so there is one thing to back up, +not two — and `scripts/backup-board.sh` covers both. +""" + +from __future__ import annotations + +import re +import secrets +from pathlib import Path + +from flask import Blueprint, abort, current_app, send_from_directory + +from .db import get_db +from .security import login_required + +bp = Blueprint("uploads", __name__) + +# How many pictures one post or comment may carry. Not a security limit — +# MAX_CONTENT_LENGTH is — just a bound on what one form submission can become. +MAX_FILES = 4 + +# extension -> content type. The extension is ours, derived from the bytes, +# never taken from the upload. +CONTENT_TYPES = { + "jpg": "image/jpeg", + "png": "image/png", + "gif": "image/gif", + "webp": "image/webp", + "avif": "image/avif", +} + +# What content.py offers writers, kept here so there is one list rather than +# two that drift apart. `jpeg` appears only as an accepted spelling; anything +# stored is named `.jpg`. +IMAGE_EXTENSIONS = set(CONTENT_TYPES) | {"jpeg"} + +# Stored names are generated by this module, so the pattern is a check on our +# own output — which is exactly why it is worth having. It is the last thing +# between a crafted request and send_from_directory. +STORED_NAME = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*-[0-9a-f]{12}\.[a-z]{3,4}$") + + +class RejectedUpload(ValueError): + """The message is written for the member who chose the file.""" + + +def _detect(data: bytes) -> str | None: + """The extension these bytes actually deserve, or None. + + Magic numbers rather than a library: five formats, each identified by a + fixed prefix, is less code than a dependency and has no version to track. + """ + if data.startswith(b"\xff\xd8\xff"): + return "jpg" + if data.startswith(b"\x89PNG\r\n\x1a\n"): + return "png" + if data.startswith((b"GIF87a", b"GIF89a")): + return "gif" + # Both of these are container formats: the marker sits at a fixed offset + # rather than at the very start, so a prefix check would miss them. + if data[:4] == b"RIFF" and data[8:12] == b"WEBP": + return "webp" + if data[4:8] == b"ftyp" and data[8:12] in (b"avif", b"avis"): + return "avif" + return None + + +def slugify(text: str) -> str: + """Only ever applied to a display name to build a readable stored name. + + Falls back to "imagen" rather than the empty string: a name that is all + punctuation, or written in a script this strips entirely, must still + produce something that matches STORED_NAME. + """ + slug = re.sub(r"[^a-z0-9]+", "-", text.lower()).strip("-") + return slug[:48] or "imagen" + + +def directory() -> Path: + path = Path(current_app.config["UPLOAD_DIR"]) + path.mkdir(parents=True, exist_ok=True) + return path + + +def stage(files) -> list[dict]: + """Read and check everything before anything is written or inserted. + + Separate from `save` on purpose. A picture that is refused must not leave a + half-made thread behind, so nothing touches the database until every file + in the submission has passed. + """ + staged = [] + real = [f for f in files if f and f.filename] + if len(real) > MAX_FILES: + raise RejectedUpload(f"Como máximo {MAX_FILES} imágenes por mensaje.") + + for upload in real: + data = upload.read() + if not data: + continue + maximum = current_app.config["UPLOAD_MAX_BYTES"] + if len(data) > maximum: + raise RejectedUpload( + f"«{upload.filename}» pesa {len(data) // 1024}KB y el máximo " + f"es {maximum // 1024}KB." + ) + extension = _detect(data) + if extension is None: + raise RejectedUpload( + f"«{upload.filename}» no parece una imagen. Se aceptan " + f"{', '.join(sorted(CONTENT_TYPES))}." + ) + stem = slugify(upload.filename.rsplit(".", 1)[0]) + staged.append({ + "data": data, + "original_name": upload.filename[:200], + "content_type": CONTENT_TYPES[extension], + # A random suffix rather than a counter: two people uploading + # "foto.jpg" in the same second must not race for one path. + "stored_name": f"{stem}-{secrets.token_hex(6)}.{extension}", + }) + return staged + + +def save(staged: list[dict], member_id: int, + thread_id: int | None = None, comment_id: int | None = None) -> None: + """Write the files, then record them. In that order. + + A row pointing at a file that does not exist renders as a broken image on + every future visit. A file with no row is invisible and gets swept up by + the next audit — so if one of the two has to happen first, it is the file. + """ + folder = directory() + for item in staged: + (folder / item["stored_name"]).write_bytes(item["data"]) + get_db().execute( + """INSERT INTO attachments + (thread_id, comment_id, stored_name, original_name, + content_type, bytes, uploaded_by) + VALUES (?, ?, ?, ?, ?, ?, ?)""", + (thread_id, comment_id, item["stored_name"], item["original_name"], + item["content_type"], len(item["data"]), member_id), + ) + + +def for_threads(thread_ids: list[int]) -> dict[int, list]: + return _grouped("thread_id", thread_ids) + + +def for_comments(comment_ids: list[int]) -> dict[int, list]: + return _grouped("comment_id", comment_ids) + + +def _grouped(column: str, ids: list[int]) -> dict[int, list]: + """One query for a whole page rather than one per comment.""" + if not ids: + return {} + marks = ",".join("?" * len(ids)) + rows = get_db().execute( + f"""SELECT * FROM attachments WHERE {column} IN ({marks}) + ORDER BY id""", + ids, + ).fetchall() + grouped: dict[int, list] = {} + for row in rows: + grouped.setdefault(row[column], []).append(row) + return grouped + + +@bp.route("/media/") +@login_required +def serve(name: str): + """Behind the login, like the thread the picture belongs to. + + The deleted check is the part that is easy to leave out: threads and + comments are *soft*-deleted, so without it, taking a post down would leave + its photo readable forever by anyone who noted the URL. + """ + if not STORED_NAME.match(name): + abort(404) + row = get_db().execute( + """SELECT a.content_type + FROM attachments a + LEFT JOIN threads t ON t.id = a.thread_id + LEFT JOIN comments c ON c.id = a.comment_id + WHERE a.stored_name = ? + AND COALESCE(t.deleted_at, c.deleted_at) IS NULL""", + (name,), + ).fetchone() + if row is None: + abort(404) + # mimetype from our own column, never guessed from the name on disk, and + # paired with the X-Content-Type-Options: nosniff set in app.py. + return send_from_directory(directory(), name, mimetype=row["content_type"]) diff --git a/docs/server-setup.md b/docs/server-setup.md index 8bb0f01..bc3da10 100644 --- a/docs/server-setup.md +++ b/docs/server-setup.md @@ -471,6 +471,17 @@ database is live and in WAL mode — a plain `cp` can capture it missing its mos recent commits. Test a restore before you rely on it: stop the container, gunzip a backup over `/srv/board/data/board.db`, start it again. +**It also archives `/srv/board/data/uploads`**, the pictures members attach to +threads, as a second file `uploads-.tar.gz`. This is not an extra: a +database backup that completes looks exactly like a backup that worked, so +before uploads were covered the nightly job would have gone on reporting +success while silently leaving every photograph out. Restoring them is a plain +`tar -xzf`, into `/srv/board/data/`. + +If there are no uploads yet the log says so by name, rather than saying +nothing — "nobody has posted a photo" and "the path moved a month ago and this +has been archiving air" are otherwise the same empty line. + ### 11.6 Who can do what | | Owner | Admin | User | @@ -574,6 +585,34 @@ Not urgent — leaving it costs a folder and one `wget` in the pipeline: 2. Delete the `wget … decap-cms.js` line from `.woodpecker.yml` 3. Delete the `decap-cms` OAuth application in Gitea +### 11.9 Pictures on the board + +Members can attach images when they start a thread or reply. Nothing to +install: the files go to `/data/uploads` inside the container, which is +`/srv/board/data/uploads` on the host — the same volume that already holds +`board.db`, so there is one directory to back up rather than two. + +**They are deliberately not in the site repository.** `/comunidad/contenido/` +uploads pictures by committing them, which is right for a post about to be +published. A photo in a private thread is the opposite: committing it would +send it through the build pipeline and out onto vienalatina.com. These are +served by the app, behind the same login as the thread. + +Three things the code does that are worth knowing if you ever change it: + +- **The type is read from the first bytes, not the filename.** A file called + `gato.png` containing HTML is refused. Served back as `image/png` from our + own domain, it would otherwise be a script running on vienalatina.com. +- **The stored name is generated.** The name the browser sent is kept only as + text to show a person, never as a path. +- **Deleting a post hides its pictures.** Threads and comments are soft-deleted, + so the serving route checks the parent is still live. Without that, taking a + post down would leave its photo readable by anyone who noted the URL. + +Limits: 4 images per message, and `BOARD_UPLOAD_MAX_BYTES` (8MB by default) +each. Adding a picture to a post *after* publishing it means posting a reply — +editing changes the words, and leaves the pictures alone. + ## 12. Make Gitea look like the site Members sign in to `/comunidad/` through Gitea, so Gitea's sign-in form and its diff --git a/scripts/backup-board.sh b/scripts/backup-board.sh index cc50b3d..ea742fe 100755 --- a/scripts/backup-board.sh +++ b/scripts/backup-board.sh @@ -15,6 +15,7 @@ set -euo pipefail DB="${BOARD_DB:-/srv/board/data/board.db}" +UPLOADS="${BOARD_UPLOADS:-/srv/board/data/uploads}" DEST="${1:-/srv/board/backups}" KEEP_DAYS="${KEEP_DAYS:-30}" STAMP="$(date -u +%Y%m%dT%H%M%SZ)" @@ -39,6 +40,27 @@ if ! gzip -t "$DEST/board-$STAMP.db.gz"; then exit 1 fi -find "$DEST" -name 'board-*.db.gz' -mtime "+$KEEP_DAYS" -delete +# The pictures members attach to threads. Without this the backup covers the +# text of the members area and none of its photographs — and it would do that +# silently, since a database backup that completes looks like a backup that +# worked. The files are immutable once written (a new upload never overwrites +# an old name), so a plain tar of the directory is a consistent snapshot; no +# equivalent of SQLite's .backup is needed. +if [ -d "$UPLOADS" ] && [ -n "$(ls -A "$UPLOADS" 2>/dev/null)" ]; then + tar -czf "$DEST/uploads-$STAMP.tar.gz" -C "$(dirname "$UPLOADS")" "$(basename "$UPLOADS")" + if ! tar -tzf "$DEST/uploads-$STAMP.tar.gz" >/dev/null; then + echo "Uploads archive failed its own integrity check." >&2 + exit 1 + fi + UPLOADS_NOTE="+ $(du -h "$DEST/uploads-$STAMP.tar.gz" | cut -f1) de imágenes" +else + # Said out loud rather than skipped in silence: "no uploads yet" and "the + # path moved and this has been backing up nothing for a month" look + # identical in a log that says nothing. + UPLOADS_NOTE="(sin imágenes en $UPLOADS)" +fi -echo "$(date -u +%FT%TZ) wrote $DEST/board-$STAMP.db.gz ($(du -h "$DEST/board-$STAMP.db.gz" | cut -f1))" +find "$DEST" -name 'board-*.db.gz' -mtime "+$KEEP_DAYS" -delete +find "$DEST" -name 'uploads-*.tar.gz' -mtime "+$KEEP_DAYS" -delete + +echo "$(date -u +%FT%TZ) wrote $DEST/board-$STAMP.db.gz ($(du -h "$DEST/board-$STAMP.db.gz" | cut -f1)) $UPLOADS_NOTE"