Let members post pictures, and back them up

Phase B. Images on threads and comments, stored in /data/uploads —
inside the volume that already holds board.db, so there is one directory
to back up rather than two and no second mount to remember.

Not in the site repository, which is the distinction that matters: the
content editor uploads by committing, and a private photo committed
there would go through the build pipeline and out onto vienalatina.com.

The type is decided by the first bytes, not the filename. content.py
trusts the extension, which is tolerable where Hugo serves the result;
here we serve it back, so an HTML file called gato.png would be a script
running on our own origin. Five magic-number checks, no new dependency.
The uploaded name is kept only as text to show a person; the name on
disk is generated. Staging is separate from saving so a refused picture
cannot leave a half-made thread behind.

Threads and comments are soft-deleted, so the serving route checks the
parent is still live. Without it, taking a post down leaves its photo
readable by anyone who noted the URL.

And the hole this phase existed to close: backup-board.sh archived
board.db and nothing else, so the first upload would have made the
nightly backup silently incomplete while still reporting success. It now
archives the uploads directory too, verifies the archive, and names the
directory when it is empty rather than passing over it in silence.
Tested by restoring: destroyed the directory, restored from the archive,
compared bytes.

IMAGE_EXTENSIONS moves to uploads.py and content.py imports it, so the
editor and the board cannot drift apart about what counts as a picture.

46 new tests, 176 in total.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NizVpJ2dwzCbjCrTLCjeHn
This commit is contained in:
Claude 2026-09-28 09:02:18 +00:00
parent a79e04396b
commit 9355121b7c
No known key found for this signature in database
13 changed files with 603 additions and 8 deletions

View File

@ -78,6 +78,10 @@ def create_app(overrides: dict | None = None) -> Flask:
# picture is silently refused has no way to tell what went wrong. # picture is silently refused has no way to tell what went wrong.
MAX_CONTENT_LENGTH=10 * 1024 * 1024, MAX_CONTENT_LENGTH=10 * 1024 * 1024,
UPLOAD_MAX_BYTES=int(os.environ.get("BOARD_UPLOAD_MAX_BYTES", 8 * 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: if overrides:
app.config.update(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. # 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`).") 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(auth.bp, url_prefix=URL_PREFIX)
app.register_blueprint(members.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(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.register_blueprint(board.bp, url_prefix=URL_PREFIX)
app.teardown_appcontext(close_db) app.teardown_appcontext(close_db)

View File

@ -18,6 +18,7 @@ from __future__ import annotations
from flask import (Blueprint, abort, current_app, flash, g, redirect, from flask import (Blueprint, abort, current_app, flash, g, redirect,
render_template, request, url_for) render_template, request, url_for)
from . import uploads
from .db import get_db from .db import get_db
from .render import excerpt, to_html from .render import excerpt, to_html
from .security import admin_required, login_required 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"], return render_template("thread.html", thread=row, author=author["display_name"],
comments=comments, body_html=to_html(row["body_md"]), comments=comments, body_html=to_html(row["body_md"]),
to_html=to_html, may_edit=may_edit, may_delete=may_delete, 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"]) @bp.route("/nuevo", methods=["GET", "POST"])
@ -147,11 +150,20 @@ def new_thread():
flash(f"Espera {wait} segundos antes de publicar otra vez.", "error") flash(f"Espera {wait} segundos antes de publicar otra vez.", "error")
return redirect(url_for("board.new_thread")) 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 title, body = cleaned
cursor = get_db().execute( cursor = get_db().execute(
"INSERT INTO threads (author_id, title, body_md) VALUES (?, ?, ?)", "INSERT INTO threads (author_id, title, body_md) VALUES (?, ?, ?)",
(g.member["id"], title, body), (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)) 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") flash(f"Espera {wait} segundos antes de comentar otra vez.", "error")
return redirect(url_for("board.thread", thread_id=thread_id)) 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 (?, ?, ?)", "INSERT INTO comments (thread_id, author_id, body_md) VALUES (?, ?, ?)",
(thread_id, g.member["id"], body), (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") return redirect(url_for("board.thread", thread_id=thread_id) + "#final")

View File

@ -34,6 +34,10 @@ from . import gitea, tokens
from .db import get_db from .db import get_db
from .render import to_html from .render import to_html
from .security import admin_required 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__) bp = Blueprint("content", __name__)
@ -51,7 +55,6 @@ COLLECTIONS = {
CATEGORIES = ["Turismo", "Cultura", "Gastronomía", "Comunidad", "Comercio"] CATEGORIES = ["Turismo", "Cultura", "Gastronomía", "Comunidad", "Comercio"]
UPLOAD_FOLDER = "static/uploads" UPLOAD_FOLDER = "static/uploads"
IMAGE_EXTENSIONS = {"jpg", "jpeg", "png", "webp", "gif", "avif"}
TITLE_MAX = 140 TITLE_MAX = 140
BODY_MAX = 100_000 BODY_MAX = 100_000

View File

@ -55,6 +55,32 @@ CREATE TABLE IF NOT EXISTS comments (
CREATE INDEX IF NOT EXISTS comments_thread CREATE INDEX IF NOT EXISTS comments_thread
ON comments(thread_id, created_at) WHERE deleted_at IS NULL; 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. -- Gitea access tokens for the editor.
-- --
-- Kept here rather than in the session cookie. Flask signs cookies but does not -- Kept here rather than in the session cookie. Flask signs cookies but does not

View File

@ -306,3 +306,21 @@ input[type="file"] {
padding: 0.5rem 0; padding: 0.5rem 0;
font-size: 0.9rem; 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);
}

View File

@ -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 %}
<ul class="shots">
{% for image in images %}
<li>
<a href="{{ url_for('uploads.serve', name=image.stored_name) }}">
<img src="{{ url_for('uploads.serve', name=image.stored_name) }}"
alt="{{ image.original_name }}" loading="lazy">
</a>
</li>
{% endfor %}
</ul>
{% endif %}
{% endmacro %}

View File

@ -1,4 +1,5 @@
{% extends "base.html" %} {% extends "base.html" %}
{% from "_attachments.html" import attachments %}
{% block title %}{{ thread.title }}{% endblock %} {% block title %}{{ thread.title }}{% endblock %}
{% block main %} {% block main %}
@ -11,6 +12,7 @@
</div> </div>
<h1 class="card__title">{{ thread.title }}</h1> <h1 class="card__title">{{ thread.title }}</h1>
<div class="prose">{{ body_html }}</div> <div class="prose">{{ body_html }}</div>
{{ attachments(thread_images) }}
<div class="actions"> <div class="actions">
{% if may_edit(thread) %} {% if may_edit(thread) %}
@ -49,6 +51,7 @@
{% if c.edited_at %}<span class="muted small">(editado)</span>{% endif %} {% if c.edited_at %}<span class="muted small">(editado)</span>{% endif %}
</div> </div>
<div class="prose">{{ to_html(c.body_md) }}</div> <div class="prose">{{ to_html(c.body_md) }}</div>
{{ attachments(comment_images.get(c.id, [])) }}
<div class="actions"> <div class="actions">
{% if may_edit(c) %} {% if may_edit(c) %}
<a class="linkish" href="{{ url_for('board.edit_comment', comment_id=c.id) }}">Editar</a> <a class="linkish" href="{{ url_for('board.edit_comment', comment_id=c.id) }}">Editar</a>
@ -68,11 +71,16 @@
{% if thread.locked and not is_admin %} {% if thread.locked and not is_admin %}
<p class="muted">Este tema está cerrado a nuevas respuestas.</p> <p class="muted">Este tema está cerrado a nuevas respuestas.</p>
{% else %} {% else %}
<form class="card" method="post" action="{{ url_for('board.comment', thread_id=thread.id) }}"> <form class="card" method="post" enctype="multipart/form-data"
action="{{ url_for('board.comment', thread_id=thread.id) }}">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}"> <input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<label for="body">Responder</label> <label for="body">Responder</label>
<textarea id="body" name="body" rows="5" required <textarea id="body" name="body" rows="5" required
placeholder="Se puede usar Markdown: **negrita**, listas, enlaces."></textarea> placeholder="Se puede usar Markdown: **negrita**, listas, enlaces."></textarea>
<label for="pictures">Imágenes <span class="muted small">(opcional)</span></label>
<input id="pictures" name="pictures" type="file" multiple accept="image/*">
<button class="btn" type="submit">Publicar respuesta</button> <button class="btn" type="submit">Publicar respuesta</button>
</form> </form>
{% endif %} {% endif %}

View File

@ -2,7 +2,7 @@
{% block title %}{{ 'Editar tema' if thread else 'Escribir' }}{% endblock %} {% block title %}{{ 'Editar tema' if thread else 'Escribir' }}{% endblock %}
{% block main %} {% block main %}
<form class="card" method="post" <form class="card" method="post" enctype="multipart/form-data"
action="{{ url_for('board.edit_thread', thread_id=thread.id) if thread else url_for('board.new_thread') }}"> action="{{ url_for('board.edit_thread', thread_id=thread.id) if thread else url_for('board.new_thread') }}">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}"> <input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<h1>{{ 'Editar tema' if thread else 'Nuevo tema' }}</h1> <h1>{{ 'Editar tema' if thread else 'Nuevo tema' }}</h1>
@ -15,6 +15,14 @@
<textarea id="body" name="body" rows="14" required <textarea id="body" name="body" rows="14" required
placeholder="Se puede usar Markdown: **negrita**, listas, enlaces.">{{ thread.body_md if thread else '' }}</textarea> placeholder="Se puede usar Markdown: **negrita**, listas, enlaces.">{{ thread.body_md if thread else '' }}</textarea>
{# 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 %}
<label for="pictures">Imágenes <span class="muted small">(opcional)</span></label>
<input id="pictures" name="pictures" type="file" multiple accept="image/*">
{% endif %}
<div class="actions"> <div class="actions">
<button class="btn" type="submit">{{ 'Guardar' if thread else 'Publicar' }}</button> <button class="btn" type="submit">{{ 'Guardar' if thread else 'Publicar' }}</button>
<a class="linkish" href="{{ url_for('board.thread', thread_id=thread.id) if thread else url_for('board.threads') }}">Cancelar</a> <a class="linkish" href="{{ url_for('board.thread', thread_id=thread.id) if thread else url_for('board.threads') }}">Cancelar</a>

View File

@ -34,6 +34,7 @@ def app(tmp_path):
"OAUTH_CLIENT_ID": "cid", "OAUTH_CLIENT_ID": "cid",
"OAUTH_CLIENT_SECRET": "secret", "OAUTH_CLIENT_SECRET": "secret",
"ADMIN_TOKEN": "admintoken", "ADMIN_TOKEN": "admintoken",
"UPLOAD_DIR": str(tmp_path / "uploads"),
"TESTING": True, "TESTING": True,
}) })
yield application yield application

View File

@ -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"<!DOCTYPE html><script>alert(1)</script>"
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

214
apps/board/uploads.py Normal file
View File

@ -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/<name>")
@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"])

View File

@ -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 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. 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-<stamp>.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 ### 11.6 Who can do what
| | Owner | Admin | User | | | 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` 2. Delete the `wget … decap-cms.js` line from `.woodpecker.yml`
3. Delete the `decap-cms` OAuth application in Gitea 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 ## 12. Make Gitea look like the site
Members sign in to `/comunidad/` through Gitea, so Gitea's sign-in form and its Members sign in to `/comunidad/` through Gitea, so Gitea's sign-in form and its

View File

@ -15,6 +15,7 @@
set -euo pipefail set -euo pipefail
DB="${BOARD_DB:-/srv/board/data/board.db}" DB="${BOARD_DB:-/srv/board/data/board.db}"
UPLOADS="${BOARD_UPLOADS:-/srv/board/data/uploads}"
DEST="${1:-/srv/board/backups}" DEST="${1:-/srv/board/backups}"
KEEP_DAYS="${KEEP_DAYS:-30}" KEEP_DAYS="${KEEP_DAYS:-30}"
STAMP="$(date -u +%Y%m%dT%H%M%SZ)" STAMP="$(date -u +%Y%m%dT%H%M%SZ)"
@ -39,6 +40,27 @@ if ! gzip -t "$DEST/board-$STAMP.db.gz"; then
exit 1 exit 1
fi 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"