Compare commits

..

8 Commits

Author SHA1 Message Date
fabi
bac30404e3 fix(export): stop a caption bricking the viewer, and ship readable archives
Two defects in the keepsake, both silent server-side and both only visible by
extracting the real artifact and trying to use it.

1. A CAPTION COULD BRICK THE VIEWER.

The viewer's data is inlined as `<script>window.__EXPORT_DATA__={…}</script>` -- it has
to be, since guests open index.html over file:// where fetching a sibling data.json is
blocked. The escape was `</` -> `<\/`. Against XSS that holds; I fired
`</script><img src=x onerror=…>` through a real Chromium parser and it round-trips
inert.

It does not stop the caption steering the HTML TOKENIZER. `<!--<script` with no later
`-->` drives the parser into script-data-double-escaped state, where the template's own
`</script>` only steps back to script-data-escaped instead of closing the element.
Everything after it -- including the viewer bundle -- is swallowed as script data.
Nothing executes and nothing leaks: `__EXPORT_DATA__` is simply never assigned and the
keepsake opens blank. A denial of the deliverable, not an XSS.

Reproduced in Chromium before changing anything, and the near-miss is worth recording:
`<!--<script>alert(1)</script>-->` comes back CLEAN, because the trailing `-->` returns
the parser to script-data state. A probe using the terminated form quietly repairs the
thing it is testing for.

Fix: escape every `<` as `<`, not just `</`. `<` never appears in JSON structural
syntax -- only inside string values -- so a global replace is sound, and one rule covers
`</script`, `<!--` and `<script` together. That is the point: the old escape was named
for the single case it handled. Only the INLINED copy is escaped; data.json is written
separately, in no HTML context, and stays literal.

2. EVERY ENTRY IN BOTH ARCHIVES WAS STORED MODE 0000.

`ZipEntryBuilder::new` leaves the external file attribute at zero and async_zip's host
compatibility defaults to Unix, so `unzip -Z` showed `?---------` on every line of both
Gallery.zip and Memories.zip. Windows Explorer ignores Unix modes, which is why this
survived; on Linux and macOS `unzip` faithfully applies what the archive asks for and
the guest gets a folder of photos none of which they can open.

Unconditional -- every keepsake ever produced, no hostile input required -- and
invisible server-side: the export succeeds, the ZIP is well-formed, the job writes
`done`, /export/status is green.

Found by accident. The browser test for defect 1 failed with ERR_ACCESS_DENIED on
file://, which looked exactly like a Playwright sandbox quirk; I twice "worked around"
it (fresh context, then a separately launched browser) before checking the extracted
files and finding mode 000. The workaround was suppressing a real bug. Both workarounds
are gone -- the ordinary `page` fixture loads the archive fine now.

Fix: all six ZipEntryBuilder sites route through one `keepsake_entry` helper stamping
`S_IFREG | 0644`, so the mode cannot be forgotten at a call site.

Tests: 3 unit (no `<` survives; the payload still decodes to the original value, because
this is a transport encoding and not a sanitiser; a clean payload is untouched) and 2
e2e that release for real, download the real archives, and check them from outside the
app -- one opening index.html over file:// in Chromium and asserting the viewer booted,
the captions came back verbatim and nothing executed; one asserting every stored mode
and every extracted file is readable. Both assertions verified to FAIL against the
pre-fix artifacts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-29 21:37:27 +02:00
fabi
5d0c7cd949 Merge branch 'refactor/share-export-visibility-filter' 2026-07-29 20:50:04 +02:00
fabi
a20b96d893 refactor(export): share the visibility filter between the row query and the estimate
`query_uploads` selects the rows the archives are built from; `estimate_export_bytes`
sizes them for the disk preflight. They stated the same WHERE clause separately, and
the direction of drift matters: an estimate that MISSES rows the archive writes
under-reserves, which is precisely the ENOSPC the preflight exists to prevent.

The integration test claimed to guard this and cannot. Both sides of
`the_estimate_sums_exactly_the_rows_the_archive_will_contain` are `SRC:`-marked
hand-copies in tests/common/mod.rs -- neither is production code -- so drift means
production moved while both copies sat still, and the test goes on passing. The
convention is sound for pinning behaviour; it is structurally incapable of detecting
divergence from the thing it copies.

So fix it where it can be fixed. One `export_visibility_where!()` fragment,
`concat!`-ed into both queries at compile time (still `&'static str`, no allocation),
with the `u`/`usr` alias contract stated. Divergence is now impossible by
construction rather than watched for.

The tests keep their value and lose the overclaim: the docstrings now say they pin
WHICH uploads may be counted -- each excluded row in the fixture is excluded by a
different predicate, so weakening any one of them still fails here -- and say plainly
that they do not detect drift, with a pointer to what does.

No behaviour change. The filters were verified identical before the hoist
(`u.event_id = $1 AND u.deleted_at IS NULL AND usr.uploads_hidden = FALSE AND
usr.is_banned = FALSE`); 99 backend tests still pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-29 20:50:04 +02:00
fabi
24ac862f81 Merge branch 'fix/video-poster-race' 2026-07-29 20:10:54 +02:00
fabi
a3d8ae72e3 fix(e2e): stop the video poster assertion racing the ffmpeg thumbnail
Pre-existing, and it fired for real during the full-suite run on a cold stack.

The lightbox binds `poster={upload.thumbnail_url ?? undefined}`, so the attribute is
absent until compression produces the thumbnail. This test asserted on it immediately
after seeding, never waiting for the worker -- unlike the Range test further down the
same file, which does poll. Against a warm stack the worker usually wins; against a
freshly rebuilt one (`stack:down -v`, cold ffmpeg) it doesn't.

That is the worst possible time for a false failure: the first run after a rebuild is
exactly when you are trying to establish whether a change broke something. Poll for
`compression_status = 'done'` before the poster assertion. The `src` assertion needs
no wait and keeps none.

Verified with --repeat-each=3.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-29 20:10:54 +02:00
fabi
e1653cc54e Merge branch 'chore/db-memory-and-social-rate-limits' 2026-07-29 19:57:34 +02:00
fabi
35390800c7 chore: raise the db memory limit and rate-limit social writes
Two smaller operational items.

POSTGRES 512M -> 1G. DATABASE_MAX_CONNECTIONS is 30 for a ~100-guest event (feed
polling + SSE + uploads at once), and 30 backends plus Postgres 16's default
shared_buffers leaves very little headroom at 512M. An OOM here doesn't degrade one
feature -- every request path touches the database, so it takes the event down.
Memory is the cheaper knob than shrinking the pool back and reintroducing the
queueing it was raised to fix. .env.example now names the pairing explicitly, the way
it already does for COMPRESSION_WORKER_CONCURRENCY.

SOCIAL WRITES WERE UNTHROTTLED. toggle_like, add_comment and delete_comment were the
only mutating endpoints in the app with no limit at all -- upload, join, recover,
export and admin login all carry one. Asymmetric coverage rather than a deliberate
decision.

Low severity, and honestly so: a like fans an SSE broadcast to every client, but the
export regeneration a comment deletion triggers is contained (REGEN_DEBOUNCE 20s,
workers born with their epoch, superseded ones inert). So the ceiling is 120/min --
far above anything a real guest produces. This bounds a script, not an enthusiastic
double-tapper.

ONE bucket across all three actions: separate buckets would let a caller triple the
aggregate write rate by alternating between them. Keyed per USER, matching the feed
and upload limits -- at a venue every guest is behind one NAT, and an IP key is what
made the /join and /feed limits turn guests away in the first place.

Migration 020 seeds both keys, and both are wired into the admin allowlist, the
config UI and the e2e reseed -- the step two earlier per-area toggles missed, which
left switches that existed in code and could never be flipped.

Tests: 4 e2e, including that the shared bucket really is shared (the part most likely
to be lost in a refactor) and that one guest hitting the ceiling doesn't block
another behind the same IP.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-29 19:57:34 +02:00
fabi
14ebe1e543 Merge branch 'docs/backup-restore-and-quota-tolerance' 2026-07-29 19:51:57 +02:00
15 changed files with 626 additions and 27 deletions

View File

@@ -18,6 +18,10 @@ POSTGRES_PASSWORD=CHANGE_ME_use_a_strong_password
POSTGRES_DB=eventsnap
# Connection pool size. Default 10. For a busy event (~100 guests polling the feed
# + SSE + uploads at once) raise to ~30 so requests don't queue on a pool permit.
# PAIRED WITH THE DB CONTAINER'S MEMORY LIMIT: 30 backends plus Postgres 16's default
# shared_buffers is already snug in the 1G that docker-compose.yml allots the `db`
# service. If you raise this, raise `db.deploy.resources.limits.memory` with it — an
# OOM in Postgres doesn't degrade one feature, it takes the whole event down.
DATABASE_MAX_CONNECTIONS=30
# ── Authentication ────────────────────────────────────────────────────────────

View File

@@ -0,0 +1 @@
DELETE FROM config WHERE key IN ('social_rate_per_min', 'social_rate_enabled');

View File

@@ -0,0 +1,16 @@
-- Per-user rate limit for social writes (likes, comments, comment deletions).
--
-- These were the only writes in the app with no limit at all. Every other mutating
-- path -- upload, join, recover, export, admin login -- carries one; social.rs
-- carried none, so the coverage was asymmetric rather than deliberately open.
--
-- Severity is genuinely low for an invited-guest event, and the amplification worry
-- turned out to be contained: a like fans an SSE broadcast to ~100 clients, but the
-- export regeneration it could otherwise trigger is debounced (REGEN_DEBOUNCE 20s)
-- and superseded workers are inert. So this closes the gap for symmetry, not urgency,
-- and the ceiling is set high enough that no real guest will ever meet it -- a
-- double-tapping enthusiast at a wedding is not the thing being defended against.
INSERT INTO config (key, value) VALUES
('social_rate_per_min', '120'),
('social_rate_enabled', 'true')
ON CONFLICT (key) DO NOTHING;

View File

@@ -127,6 +127,9 @@ pub async fn patch_config(
// Same shape for /recover: the per-(ip, name) bucket is the anti-guessing control,
// this only bounds a name-cycling flood in front of a cost-12 bcrypt (migration 019).
("recover_ip_rate_per_min", true, 1.0, 100_000.0),
// Aggregate ceiling on likes + comments + comment deletions, per user per minute.
// These were the only mutating endpoints with no limit at all (migration 020).
("social_rate_per_min", true, 1.0, 100_000.0),
("quota_tolerance", false, 0.0, 1.0),
("estimated_guest_count", true, 1.0, 1_000_000.0),
];
@@ -141,6 +144,7 @@ pub async fn patch_config(
// missing from this allowlist — so the switch existed in code and could never be flipped.
"admin_login_rate_enabled",
"recover_rate_enabled",
"social_rate_enabled",
"quota_enabled",
"storage_quota_enabled",
"upload_count_quota_enabled",

View File

@@ -10,8 +10,40 @@ use crate::error::AppError;
use crate::models::comment::{Comment, CommentDto};
use crate::models::hashtag::{self, Hashtag};
use crate::models::upload::Upload;
use crate::services::config;
use crate::state::AppState;
/// Throttle a social write. Keyed PER USER, like the feed and upload limits and for the same
/// reason: at a venue every guest sits behind one NAT, so an IP key hands the whole party a
/// single bucket and the most active guest starves everyone else.
///
/// These were the only mutating endpoints in the app with no limit at all — the coverage was
/// asymmetric, not deliberately open. The ceiling is set well above anything a real guest
/// produces; this bounds a script, not an enthusiastic double-tapper.
async fn check_social_rate(state: &AppState, user_id: Uuid) -> Result<(), AppError> {
let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await;
let social_rate_on = config::get_bool(&state.config_cache, "social_rate_enabled", true).await;
if !(rate_limits_on && social_rate_on) {
return Ok(());
}
let rate_limit = config::get_usize(&state.config_cache, "social_rate_per_min", 120).await;
// ONE bucket across likes, comments and comment deletions. Separate buckets would let a
// caller triple the aggregate write rate just by alternating between them.
state
.rate_limiter
.check_with_retry(
format!("social:{user_id}"),
rate_limit,
std::time::Duration::from_secs(60),
)
.map_err(|retry_after_secs| {
AppError::TooManyRequests(
"Zu viele Aktionen. Bitte warte kurz und versuche es erneut.".into(),
Some(retry_after_secs),
)
})
}
#[derive(Serialize)]
pub struct LikeResponse {
/// The caller's like state *after* this toggle. The client sets `liked_by_me` from
@@ -35,6 +67,7 @@ pub async fn toggle_like(
if user.is_banned {
return Err(AppError::Forbidden("Du bist gesperrt.".into()));
}
check_social_rate(&state, auth.user_id).await?;
// Event-scope: the upload must belong to the caller's event (404 otherwise),
// matching the host handlers' find_by_id_and_event pattern.
@@ -141,6 +174,7 @@ pub async fn add_comment(
if user.is_banned {
return Err(AppError::Forbidden("Du bist gesperrt.".into()));
}
check_social_rate(&state, auth.user_id).await?;
// Event-scope: only comment on an upload that belongs to the caller's event.
Upload::find_by_id_and_event(&state.pool, upload_id, auth.event_id)
@@ -216,6 +250,7 @@ pub async fn delete_comment(
if auth.is_banned {
return Err(AppError::Forbidden("Du bist gesperrt.".into()));
}
check_social_rate(&state, auth.user_id).await?;
let comment = Comment::find_by_id(&state.pool, comment_id)
.await?
.ok_or_else(|| AppError::NotFound("Kommentar nicht gefunden.".into()))?;

View File

@@ -63,6 +63,7 @@ pub async fn truncate_all(
('export_rate_per_day', '3'),
('join_ip_rate_per_min', '60'),
('recover_ip_rate_per_min', '30'),
('social_rate_per_min', '120'),
('quota_tolerance', '0.75'),
('estimated_guest_count', '100'),
('compression_concurrency', '2'),
@@ -71,6 +72,7 @@ pub async fn truncate_all(
('feed_rate_enabled', 'false'),
('export_rate_enabled', 'false'),
('join_rate_enabled', 'false'),
('social_rate_enabled', 'false'),
('admin_login_rate_enabled', 'false'),
('quota_enabled', 'false'),
('storage_quota_enabled', 'false'),

View File

@@ -20,6 +20,29 @@ use crate::state::SseEvent;
static VIEWER_DIR: Dir<'_> = include_dir!("$CARGO_MANIFEST_DIR/static/export-viewer");
// ── Shared visibility filter ─────────────────────────────────────────────────
/// The predicate that decides what lands in a keepsake, as ONE definition.
///
/// Two queries have to agree on it: [`query_uploads`], which selects the rows the archives are
/// built from, and [`estimate_export_bytes`], which sizes them for the disk preflight. They used
/// to state it separately, and the direction of drift matters — an estimate that misses rows the
/// archive writes UNDER-reserves, which is the exact ENOSPC the preflight exists to prevent.
///
/// A `SRC:`-marked copy in the integration tests cannot catch that: drift means production moved
/// and the copy didn't, so both sides of such a test sit still and it keeps passing. Sharing the
/// fragment removes the failure by construction instead, and leaves the test doing what it is
/// actually good at — pinning the behaviour.
///
/// CONTRACT: callers must alias `upload` as `u` and join `"user"` as `usr`, and bind the event id
/// as `$1`.
macro_rules! export_visibility_where {
() => {
"WHERE u.event_id = $1 AND u.deleted_at IS NULL
AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE"
};
}
// ── DB query rows ────────────────────────────────────────────────────────────
#[derive(sqlx::FromRow)]
@@ -557,7 +580,7 @@ async fn run_zip_export_inner(
};
let entry_name = format!("{folder}/{date}_{name_safe}_{}.{ext}", row.id);
let builder = ZipEntryBuilder::new(entry_name.into(), Compression::Stored);
let builder = keepsake_entry(entry_name, Compression::Stored);
// Open BEFORE writing the entry header. A missing source is skipped (the media file
// was deleted, or its processing failed) — but it must be skipped without aborting the
@@ -950,7 +973,7 @@ async fn run_html_export_inner(
// Write data.json
{
let builder = ZipEntryBuilder::new("data.json".into(), Compression::Deflate);
let builder = keepsake_entry("data.json".into(), Compression::Deflate);
let mut entry = zip.write_entry_stream(builder).await?;
let mut cursor = AllowStdIo::new(std::io::Cursor::new(data_json.as_bytes()));
fcopy(&mut cursor, &mut entry).await?;
@@ -959,7 +982,7 @@ async fn run_html_export_inner(
// Write README.txt
{
let builder = ZipEntryBuilder::new("README.txt".into(), Compression::Deflate);
let builder = keepsake_entry("README.txt".into(), Compression::Deflate);
let mut entry = zip.write_entry_stream(builder).await?;
let mut cursor = AllowStdIo::new(std::io::Cursor::new(README_TEXT.as_bytes()));
fcopy(&mut cursor, &mut entry).await?;
@@ -992,7 +1015,7 @@ async fn run_html_export_inner(
};
let entry_name = format!("media/{name}");
let builder = ZipEntryBuilder::new(entry_name.into(), Compression::Stored);
let builder = keepsake_entry(entry_name, Compression::Stored);
let mut zip_entry = zip.write_entry_stream(builder).await?;
let mut f = src_file.compat();
fcopy(&mut f, &mut zip_entry).await?;
@@ -1051,7 +1074,7 @@ async fn run_html_export_inner(
// ── DB helpers ───────────────────────────────────────────────────────────────
async fn query_uploads(pool: &PgPool, event_id: Uuid) -> Result<Vec<ExportUploadRow>> {
Ok(sqlx::query_as::<_, ExportUploadRow>(
Ok(sqlx::query_as::<_, ExportUploadRow>(concat!(
"SELECT u.id, u.original_path, u.mime_type, u.caption,
usr.display_name AS uploader_name,
COUNT(DISTINCT l.user_id) AS like_count,
@@ -1059,11 +1082,12 @@ async fn query_uploads(pool: &PgPool, event_id: Uuid) -> Result<Vec<ExportUpload
FROM upload u
JOIN \"user\" usr ON usr.id = u.user_id
LEFT JOIN \"like\" l ON l.upload_id = u.id
WHERE u.event_id = $1 AND u.deleted_at IS NULL
AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE
",
export_visibility_where!(),
"
GROUP BY u.id, usr.display_name
ORDER BY u.created_at ASC",
)
))
.bind(event_id)
.fetch_all(pool)
.await?)
@@ -1267,15 +1291,16 @@ fn is_superseded_archive(
/// downscaled to 2000px first, which only makes this estimate more conservative — the direction we
/// want, since being wrong low means ENOSPC halfway through.
///
/// Matches [`query_uploads`]' visibility filter exactly, so hidden/banned uploads aren't counted.
/// Shares [`query_uploads`]' visibility filter via [`export_visibility_where`], so hidden/banned
/// uploads can't be counted here but skipped there (or the reverse, which under-reserves).
pub async fn estimate_export_bytes(pool: &PgPool, event_id: Uuid) -> Result<u64> {
let (bytes,): (i64,) = sqlx::query_as(
let (bytes,): (i64,) = sqlx::query_as(concat!(
"SELECT COALESCE(SUM(u.original_size_bytes), 0)::bigint
FROM upload u
JOIN \"user\" usr ON usr.id = u.user_id
WHERE u.event_id = $1 AND u.deleted_at IS NULL
AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE",
)
",
export_visibility_where!(),
))
.bind(event_id)
.fetch_one(pool)
.await
@@ -1527,6 +1552,36 @@ async fn maybe_broadcast_complete(
/// double-clicking `index.html` (file://), where browsers block a cross-origin
/// `fetch()` of a sibling `data.json` — so the data is inlined into the page.
/// (`data.json` is still written separately for the http-served case.)
/// Permissions stamped on every entry in both archives: `rw-r--r--`.
///
/// `ZipEntryBuilder::new` leaves the external file attribute at zero, and the host compatibility
/// defaults to Unix — so every entry was written with a stored mode of **0000**. Windows Explorer
/// ignores Unix modes and was fine, which is exactly why this survived: on Linux and macOS
/// `unzip` faithfully applies what the archive asks for, and the guest gets a directory of files
/// none of which they can open. `?---------` on every line of `unzip -Z`.
///
/// That is the keepsake — the artifact the whole event exists to produce — arriving unreadable,
/// after distribution, with no server-side symptom at all.
/// `S_IFREG | 0644`. The type bits are included because the mode is written whole into the high
/// half of the external file attribute: without them extractors see a file of type "unknown"
/// (`unzip -Z` renders `?rw-r--r--`), which works but is not what the archive means to say.
const KEEPSAKE_ENTRY_MODE: u16 = 0o100_644;
/// Build a ZIP entry for the keepsake. ALL entries in both archives go through here so the mode
/// can't be forgotten at one of the six call sites.
fn keepsake_entry(name: String, compression: Compression) -> ZipEntryBuilder {
ZipEntryBuilder::new(name.into(), compression).unix_permissions(KEEPSAKE_ENTRY_MODE)
}
/// Escape a JSON payload for inlining inside a `<script>` element.
///
/// See the call site in [`write_viewer_with_data`] for why this is every `<` and not just `</`.
/// Kept separate so the property that matters — no `<` survives, and the value still decodes to
/// the original — can be asserted without building a ZIP.
fn escape_json_for_script(data_json: &str) -> String {
data_json.replace('<', "\\u003c")
}
async fn write_viewer_with_data(
dir: &include_dir::Dir<'_>,
zip: &mut ZipFileWriter<tokio::fs::File>,
@@ -1538,8 +1593,31 @@ async fn write_viewer_with_data(
if path == "index.html" {
let html = std::str::from_utf8(file.contents())
.context("export-viewer index.html is not valid UTF-8")?;
// Escape `</` so a caption containing `</script>` can't break out of the tag.
let safe = data_json.replace("</", "<\\/");
// Escape EVERY `<`, not just `</`.
//
// `</` -> `<\/` stops the obvious break-out (`</script><img onerror=…>`) and is inert
// against XSS. It does not stop the caption steering the HTML TOKENIZER. A caption
// containing `<!--<script` with no later `-->` puts the parser into
// script-data-double-escaped state; from there the template's own `</script>` only
// steps back to script-data-escaped instead of closing the element, and the rest of the
// document — including the viewer bundle — is swallowed as script data. Nothing
// executes and nothing leaks; `window.__EXPORT_DATA__` is simply never assigned and the
// keepsake opens blank.
//
// That failure is silent and POST-DISTRIBUTION: the export succeeds, the ZIP is
// well-formed, the job writes `done`, /export/status is green, and the host hands out a
// file that only fails when a guest double-clicks index.html — in every copy, with no
// way to fix it after the fact. Reachable from any guest-authored caption or comment,
// since both are embedded in the viewer.
//
// `<` never appears in JSON structural syntax — only inside string values — so a global
// replace is sound, and `<` is valid in both JSON and a JS string literal. One
// rule covers `</script`, `<!--` and `<script` together, which is the point: the
// previous escape was named for the single case it did handle.
//
// NOTE this is deliberately only for the INLINED copy. `data.json` is written
// separately, in no HTML context, and must stay literal.
let safe = escape_json_for_script(data_json);
// Match the live app's colour theme: inject the same `:root:root{…}` override
// the app builds at runtime so a rose/sage/custom event exports a rose/sage
// keepsake (not the embedded default gold). The CSS is generated purely from
@@ -1553,13 +1631,13 @@ async fn write_viewer_with_data(
Some(idx) => format!("{}{}{}", &html[..idx], head_inject, &html[idx..]),
None => format!("{head_inject}{html}"),
};
let builder = ZipEntryBuilder::new(path.into(), Compression::Deflate);
let builder = keepsake_entry(path, Compression::Deflate);
let mut entry = zip.write_entry_stream(builder).await?;
let mut cursor = AllowStdIo::new(std::io::Cursor::new(injected.as_bytes()));
fcopy(&mut cursor, &mut entry).await?;
entry.close().await?;
} else {
let builder = ZipEntryBuilder::new(path.into(), Compression::Deflate);
let builder = keepsake_entry(path, Compression::Deflate);
let mut entry = zip.write_entry_stream(builder).await?;
let mut cursor = AllowStdIo::new(std::io::Cursor::new(file.contents()));
fcopy(&mut cursor, &mut entry).await?;
@@ -1790,6 +1868,62 @@ mod tests {
}
}
/// Every `<` is escaped, whatever it is part of.
///
/// PREVENTS the regression to `</` -> `<\\/`, which is named for the one case it handles.
/// `<!--<script` with no later `-->` drives the HTML tokenizer into
/// script-data-double-escaped state, where the template's own `</script>` no longer closes
/// the element — the viewer bundle is swallowed as script data, `__EXPORT_DATA__` is never
/// assigned, and the keepsake opens blank in every copy the host has already handed out.
#[test]
fn no_left_angle_bracket_survives_inlining() {
for payload in [
r#"{"caption":"<!--<script"}"#,
r#"{"caption":"</script><img src=x onerror=alert(1)>"}"#,
r#"{"caption":"<!--"}"#,
r#"{"caption":"<script>"}"#,
r#"{"caption":"a < b"}"#,
] {
let escaped = escape_json_for_script(payload);
assert!(
!escaped.contains('<'),
"a surviving `<` can still steer the tokenizer: {escaped}"
);
}
}
/// The escape must not change what the viewer READS — it is a transport encoding, not a
/// sanitiser. A caption is guest-authored text that has to render back exactly.
#[test]
fn the_payload_still_decodes_to_the_original_value() {
// `<` appears only inside JSON string values, never in structural syntax, so a global
// replace is sound — this is the assertion that says so.
for caption in [
"<!--<script",
"</script><img src=x onerror=alert(1)>",
"a < b und c > d",
"ganz normale Bildunterschrift",
"Herz <3",
] {
let json = serde_json::json!({ "posts": [{ "caption": caption }] }).to_string();
let escaped = escape_json_for_script(&json);
let back: serde_json::Value =
serde_json::from_str(&escaped).expect("the escaped form must still be valid JSON");
assert_eq!(
back["posts"][0]["caption"].as_str(),
Some(caption),
"the caption must survive the round trip unchanged"
);
}
}
/// Nothing else in the document is touched.
#[test]
fn a_payload_with_no_angle_brackets_is_unchanged() {
let json = r#"{"posts":[{"caption":"schönes Foto"}]}"#;
assert_eq!(escape_json_for_script(json), json);
}
#[test]
fn a_lone_armed_job_reserves_for_one_archive() {
// A ViewerOnly regeneration re-arms only the HTML half — reserving for two would refuse

View File

@@ -293,6 +293,10 @@ pub async fn set_user_moderation(pool: &PgPool, user_id: Uuid, banned: bool, hid
/// SRC: `services/export.rs::query_uploads` — the visibility filter, verbatim, projected down to
/// `(id, original_size_bytes)`. This is the row set that ACTUALLY lands in the archives.
///
/// Production builds this WHERE from `export_visibility_where!()`, shared with
/// `estimate_export_bytes`. A copy here can pin the behaviour but CANNOT detect production moving
/// away from it — that is what sharing the fragment is for, not this.
pub async fn export_visible_uploads(pool: &PgPool, event_id: Uuid) -> Vec<(Uuid, i64)> {
sqlx::query_as(
"SELECT u.id, u.original_size_bytes
@@ -309,7 +313,9 @@ pub async fn export_visible_uploads(pool: &PgPool, event_id: Uuid) -> Vec<(Uuid,
.expect("export_visible_uploads")
}
/// SRC: `services/export.rs::estimate_export_bytes` — verbatim.
/// SRC: `services/export.rs::estimate_export_bytes` — verbatim. Same caveat as above: production
/// shares its WHERE with `query_uploads` via `export_visibility_where!()`, so these two copies
/// agreeing proves the behaviour, not the absence of drift.
pub async fn estimate_export_bytes(pool: &PgPool, event_id: Uuid) -> i64 {
let (bytes,): (i64,) = sqlx::query_as(
"SELECT COALESCE(SUM(u.original_size_bytes), 0)::bigint

View File

@@ -16,10 +16,18 @@
//!
//! What these tests pin is the ESTIMATE — the part that decides. The arithmetic on top of it lives
//! in `services/export.rs`'s unit tests; the filesystem selection lives in `is_superseded_archive`.
//! The risk here is drift: if `query_uploads` ever gains or loses a visibility predicate and
//! `estimate_export_bytes` doesn't, the preflight silently sizes the wrong gallery. So rather than
//! restating the filter, these assert the estimate against the row set the archive actually
//! contains.
//!
//! ON DRIFT, precisely, because it is easy to overclaim here. The hazard is that `query_uploads`
//! (which selects the rows the archives are built from) and `estimate_export_bytes` (which sizes
//! them) could disagree — and an estimate missing rows the archive writes UNDER-reserves, the one
//! direction that reintroduces the ENOSPC. **These tests cannot catch that**, and neither can any
//! test in this harness: both sides here are `SRC:`-marked hand-copies in `tests/common/mod.rs`,
//! so if production moved and the copies didn't, they would sit still and keep passing.
//!
//! That is fixed where it can be — the two queries now share one `export_visibility_where!()`
//! fragment in `services/export.rs`, so they cannot diverge by construction. What is left for
//! these tests is what the convention is genuinely good at: pinning the BEHAVIOUR, so a change
//! that deliberately alters the filter has to come here and say so.
mod common;
@@ -29,9 +37,10 @@ use sqlx::PgPool;
/// The estimate must equal the sum over EXACTLY the rows `query_uploads` returns — computed from
/// that row set, not from a restatement of its WHERE clause.
///
/// PREVENTS: the two queries drifting apart. An estimate that counts rows the archive skips is
/// merely pessimistic; one that MISSES rows the archive writes under-reserves, which is the whole
/// failure being guarded against.
/// PINS: which uploads the preflight is allowed to count. Each excluded row below is excluded by a
/// DIFFERENT predicate, so a change that drops or weakens any one of them fails here and has to be
/// argued for. (It does not detect production drifting away from these copies — see the file
/// header; `export_visibility_where!()` is what makes that impossible.)
#[sqlx::test]
async fn the_estimate_sums_exactly_the_rows_the_archive_will_contain(pool: PgPool) {
let event_id = seed_event(&pool, "wedding").await;

View File

@@ -17,7 +17,15 @@ services:
deploy:
resources:
limits:
memory: 512M
# 1G, not 512M. DATABASE_MAX_CONNECTIONS defaults to 30 for a ~100-guest event
# (feed polling + SSE + uploads at once), and 30 backends plus Postgres 16's
# default shared_buffers leaves very little headroom at 512M. An OOM here does
# not degrade one feature — it takes the event down, because every request
# path touches the database. Memory is the cheaper knob than shrinking the
# pool back and reintroducing the queueing it was raised to fix.
#
# Raising DATABASE_MAX_CONNECTIONS further means raising this too.
memory: 1G
app:
build:

View File

@@ -0,0 +1,136 @@
/**
* Regression guard — likes, comments and comment deletions are rate limited.
*
* These were the only mutating endpoints in the app with no limit at all. Every other write path
* -- upload, join, recover, export, admin login -- carried one; `social.rs` carried none, so the
* coverage was asymmetric rather than deliberately open.
*
* Severity is genuinely low for an invited-guest event, and the amplification worry is contained:
* a like does fan an SSE broadcast to every connected client, but the export regeneration a
* comment deletion triggers is debounced (REGEN_DEBOUNCE 20s) and superseded workers are inert. So
* this closes the gap for symmetry, and the ceiling is set well above anything a real guest
* produces -- it bounds a script, not an enthusiastic double-tapper.
*
* The bucket is shared across all three actions on purpose: separate buckets would let a caller
* triple the aggregate write rate just by alternating between them. That is what the second test
* pins, and it is the part most likely to be lost in a refactor.
*
* Keyed per USER, not per IP — at a venue every guest is behind one NAT, so an IP key would hand
* the whole party one bucket. Third test.
*/
import { test, expect } from '../../fixtures/test';
import { seedUpload } from '../../helpers/seed';
import { BASE } from '../../helpers/env';
const like = (jwt: string, uploadId: string) =>
fetch(`${BASE}/api/v1/upload/${uploadId}/like`, {
method: 'POST',
headers: { Authorization: `Bearer ${jwt}` },
});
const comment = (jwt: string, uploadId: string, body: string) =>
fetch(`${BASE}/api/v1/upload/${uploadId}/comments`, {
method: 'POST',
headers: { Authorization: `Bearer ${jwt}`, 'Content-Type': 'application/json' },
body: JSON.stringify({ body }),
});
test.describe('Social — rate limit', () => {
test('a burst of likes past the ceiling returns 429 with Retry-After', async ({
api,
adminToken,
guest,
}) => {
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
social_rate_enabled: 'true',
social_rate_per_min: '3',
});
const g = await guest('Tapper');
const uploadId = await seedUpload(g.jwt);
// Sequential, not parallel: a toggle flips state, so ordering matters for the assertion.
const statuses: number[] = [];
for (let i = 0; i < 5; i++) statuses.push((await like(g.jwt, uploadId)).status);
expect(statuses.slice(0, 3), 'the first three are within the ceiling').toEqual([200, 200, 200]);
expect(statuses.slice(3), 'everything past it is refused').toEqual([429, 429]);
const limited = await like(g.jwt, uploadId);
expect(limited.status).toBe(429);
expect(
limited.headers.get('retry-after'),
'a 429 without Retry-After tells the client nothing about when to come back'
).toBeTruthy();
});
test('likes and comments share one bucket', async ({ api, adminToken, guest }) => {
// THE assertion. Per-action buckets would let a caller triple the aggregate write rate by
// alternating, which defeats the point of having a ceiling at all.
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
social_rate_enabled: 'true',
social_rate_per_min: '2',
});
const g = await guest('Mixer');
const uploadId = await seedUpload(g.jwt);
expect((await like(g.jwt, uploadId)).status).toBe(200);
expect((await comment(g.jwt, uploadId, 'schön!')).status).toBe(201);
// Two writes spent, whichever endpoints they went to.
expect(
(await comment(g.jwt, uploadId, 'noch eins')).status,
'a comment must consume the same budget a like does'
).toBe(429);
expect((await like(g.jwt, uploadId)).status).toBe(429);
});
test('one guest hitting the ceiling does not block another', async ({
api,
adminToken,
guest,
}) => {
// Keyed per user, not per IP. Every request in this suite comes from one address, which is
// exactly the venue-NAT shape that made the /join and /feed limits turn guests away.
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
social_rate_enabled: 'true',
social_rate_per_min: '2',
});
const noisy = await guest('Noisy');
const quiet = await guest('Quiet');
const uploadId = await seedUpload(noisy.jwt);
for (let i = 0; i < 3; i++) await like(noisy.jwt, uploadId);
expect((await like(noisy.jwt, uploadId)).status).toBe(429);
expect(
(await like(quiet.jwt, uploadId)).status,
'a second guest behind the same IP must have their own budget'
).toBe(200);
});
test('flipping social_rate_enabled off bypasses the limit', async ({
api,
adminToken,
guest,
}) => {
// The toggle has to actually be honoured, or the admin switch is decorative — the failure
// mode two other per-area toggles already shipped with.
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
social_rate_enabled: 'false',
social_rate_per_min: '2',
});
const g = await guest('Unlimited');
const uploadId = await seedUpload(g.jwt);
const statuses: number[] = [];
for (let i = 0; i < 6; i++) statuses.push((await like(g.jwt, uploadId)).status);
expect(statuses.every((s) => s === 200)).toBe(true);
});
});

View File

@@ -40,10 +40,19 @@ test.describe('Video — the lightbox plays it', () => {
page,
guest,
signIn,
db,
}) => {
const g = await guest('VideoWatcher');
const id = await seedVideo(g.jwt);
// The poster assertion below needs the ffmpeg thumbnail to EXIST — the lightbox binds
// `poster={upload.thumbnail_url ?? undefined}`, so the attribute is simply absent until
// compression finishes. Without this wait the test races the worker and fails against a
// cold stack (first run after `stack:down -v`, cold ffmpeg), which is exactly when a suite
// is least likely to be believed. The `src` assertion is unconditional; only the poster
// needs the wait.
await expect.poll(() => db.compressionStatus(id), { timeout: 60_000 }).toBe('done');
await signIn(page, g);
await page.goto('/feed');

View File

@@ -0,0 +1,103 @@
/**
* Regression guard — the keepsake extracts to files a guest can actually open.
*
* `ZipEntryBuilder::new` leaves the external file attribute at zero, and async_zip's host
* compatibility defaults to Unix — so every entry in BOTH archives was written with a stored mode
* of 0000. `unzip -Z` showed `?---------` on every line.
*
* Windows Explorer ignores Unix modes, which is exactly why this survived. On Linux and macOS,
* `unzip` faithfully applies what the archive asks for, and the guest gets a folder of photos none
* of which they can open — plus an index.html the browser refuses with ERR_ACCESS_DENIED.
*
* Unconditional: it affected every keepsake ever produced, no hostile input required. And it is
* invisible server-side — the export succeeds, the ZIP is well-formed, the job writes `done`,
* /export/status is green. The only way to see it is to extract the real artifact and try to read
* it, which is what this does.
*
* Found while chasing an unrelated ERR_ACCESS_DENIED that looked like a Playwright quirk.
*/
import { test, expect } from '../../fixtures/test';
import { execFileSync } from 'node:child_process';
import { mkdtempSync, writeFileSync, rmSync, readFileSync, statSync, readdirSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { seedUpload } from '../../helpers/seed';
import { BASE } from '../../helpers/env';
/** Walk every file under `dir`, ignoring the archives we dropped there ourselves. */
function walk(dir: string, skip: string[] = []): string[] {
return readdirSync(dir, { withFileTypes: true }).flatMap((e) => {
const p = join(dir, e.name);
if (e.isDirectory()) return walk(p, skip);
return skip.includes(e.name) ? [] : [p];
});
}
test.describe('Export — the archives extract to readable files', () => {
test('every entry in both keepsake archives is owner-readable', async ({ host, guest, db }) => {
test.setTimeout(120_000);
const bearer = { Authorization: `Bearer ${host.jwt}` };
const g = await guest('Archivist');
const id = await seedUpload(g.jwt, { caption: 'ein Foto' });
await expect.poll(() => db.compressionStatus(id), { timeout: 30_000 }).toBe('done');
expect(
(await fetch(`${BASE}/api/v1/host/gallery/release`, { method: 'POST', headers: bearer }))
.status
).toBe(204);
await expect
.poll(
async () => {
const res = await fetch(`${BASE}/api/v1/export/status`, { headers: bearer });
const s = await res.json();
return s.zip?.status === 'done' && s.html?.status === 'done';
},
{ timeout: 90_000, intervals: [500] }
)
.toBe(true);
for (const kind of ['zip', 'html'] as const) {
const ticketRes = await fetch(`${BASE}/api/v1/export/ticket`, {
method: 'POST',
headers: bearer,
});
const { ticket } = (await ticketRes.json()) as { ticket: string };
const dl = await fetch(`${BASE}/api/v1/export/${kind}?ticket=${encodeURIComponent(ticket)}`);
expect(dl.status, `downloading the ${kind} archive`).toBe(200);
const dir = mkdtempSync(join(tmpdir(), `eventsnap-perms-${kind}-`));
try {
const zipPath = join(dir, 'archive.zip');
writeFileSync(zipPath, Buffer.from(await dl.arrayBuffer()));
// The mode as STORED in the archive — this is what a guest's unzip will apply. Reading it
// from the central directory catches the defect even on a filesystem that would mask it.
const listing = execFileSync('unzip', ['-Z', zipPath], { encoding: 'utf8' });
const modes = listing
.split('\n')
.filter((l) => /^[?d-][rwx-]{9}\s/.test(l))
.map((l) => l.slice(0, 10));
expect(modes.length, `${kind}: no entries listed`).toBeGreaterThan(0);
for (const m of modes) {
expect(m, `${kind}: an entry is stored mode ${m} — the guest cannot open it`).toMatch(
/^.r[w-]-/
);
}
// And extraction really does produce readable files.
execFileSync('unzip', ['-qo', zipPath, '-d', dir]);
const files = walk(dir, ['archive.zip']);
expect(files.length, `${kind}: nothing extracted`).toBeGreaterThan(0);
for (const f of files) {
expect(statSync(f).mode & 0o400, `${f} is not owner-readable`).toBeTruthy();
// The assertion that matters to a guest: the bytes actually come out.
expect(() => readFileSync(f)).not.toThrow();
}
} finally {
rmSync(dir, { recursive: true, force: true });
}
}
});
});

View File

@@ -0,0 +1,125 @@
/**
* Regression guard — a guest-authored caption cannot brick the offline keepsake.
*
* The viewer's data is inlined as `<script>window.__EXPORT_DATA__={…};</script>` (it must be:
* guests open index.html over file://, where a cross-origin fetch of a sibling data.json is
* blocked). Captions and comments are guest text and land in that payload.
*
* The escape used to be `</` → `<\/`. Against XSS that holds — `</script><img src=x onerror=…>`
* round-trips inert. It does NOT stop the caption steering the HTML TOKENIZER: `<!--<script` with
* no later `-->` drives the parser into script-data-double-escaped state, where the template's own
* `</script>` steps back to script-data-escaped instead of closing the element. Everything after —
* including the viewer bundle — is swallowed as script data. Nothing executes and nothing leaks;
* `__EXPORT_DATA__` is never assigned and the keepsake renders blank.
*
* What makes it worth a browser-level test rather than a unit test alone: the failure is SILENT and
* POST-DISTRIBUTION. The export succeeds, the ZIP is well-formed, the job writes `done`,
* /export/status is green, and the host hands out a file that only fails when a guest
* double-clicks it — in every copy, unfixably. It is not visible by reading the escape. It is only
* visible by running a real parser over the real artifact, which is what this does: release, pull
* the actual Memories.zip, extract index.html, open it over file:// in Chromium, and assert the
* viewer actually booted.
*
* The near-miss worth recording: `<!--<script>alert(1)</script>-->` comes back CLEAN, because the
* trailing `-->` returns the parser to script-data state. A probe using the terminated form
* quietly repairs the very thing it is testing for. Only the unterminated variant exposes it.
*/
import { test, expect } from '../../fixtures/test';
import { execFileSync } from 'node:child_process';
import { mkdtempSync, writeFileSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { seedUpload } from '../../helpers/seed';
import { BASE } from '../../helpers/env';
/** Unterminated on purpose — see the header. The terminated form self-repairs. */
const TOKENIZER_PAYLOAD = '<!--<script';
/** The classic break-out. Already handled, kept so the fix can never regress on it. */
const BREAKOUT_PAYLOAD = '</script><img src=x onerror=window.__XSS__=1>';
test.describe('Export — a caption cannot brick the keepsake viewer', () => {
test('the exported viewer boots with a tokenizer-hostile caption in it', async ({
page,
host,
guest,
db,
}) => {
test.setTimeout(120_000);
const bearer = { Authorization: `Bearer ${host.jwt}` };
const g = await guest('Trickster');
const a = await seedUpload(g.jwt, { caption: TOKENIZER_PAYLOAD });
const b = await seedUpload(g.jwt, { caption: BREAKOUT_PAYLOAD });
for (const id of [a, b]) {
await expect.poll(() => db.compressionStatus(id), { timeout: 30_000 }).toBe('done');
}
expect(
(await fetch(`${BASE}/api/v1/host/gallery/release`, { method: 'POST', headers: bearer }))
.status
).toBe(204);
await expect
.poll(
async () => {
const res = await fetch(`${BASE}/api/v1/export/status`, { headers: bearer });
return (await res.json()).html?.status;
},
{ timeout: 90_000, intervals: [500] }
)
.toBe('done');
const ticketRes = await fetch(`${BASE}/api/v1/export/ticket`, {
method: 'POST',
headers: bearer,
});
const { ticket } = (await ticketRes.json()) as { ticket: string };
const dl = await fetch(`${BASE}/api/v1/export/html?ticket=${encodeURIComponent(ticket)}`);
expect(dl.status).toBe(200);
const dir = mkdtempSync(join(tmpdir(), 'eventsnap-viewer-'));
try {
const zipPath = join(dir, 'Memories.zip');
writeFileSync(zipPath, Buffer.from(await dl.arrayBuffer()));
// Extract the WHOLE archive: index.html pulls in the viewer's own JS/CSS, and the point of
// this test is that those later resources are still reachable by the parser.
execFileSync('unzip', ['-qo', zipPath, '-d', dir]);
// file://, not http://. That is how a guest actually opens the keepsake, and it is the
// whole reason the data is inlined rather than fetched from a sibling data.json.
let xss = false;
page.on('dialog', (d) => {
xss = true;
void d.dismiss();
});
await page.goto('file://' + join(dir, 'index.html'));
// 1. The payload was assigned at all. This is the assertion that fails on the old escape —
// the second script block is never reached, so the global stays undefined.
const captions = await page.evaluate(() => {
const d = (window as unknown as { __EXPORT_DATA__?: { posts?: { caption?: string }[] } })
.__EXPORT_DATA__;
return d?.posts?.map((p) => p.caption ?? '') ?? null;
});
expect(captions, '__EXPORT_DATA__ was never assigned — the viewer is bricked').not.toBeNull();
// 2. The captions survived verbatim. The escape is a transport encoding, not a sanitiser:
// a guest's text has to come back exactly, or we have silently rewritten their words.
expect(captions).toContain(TOKENIZER_PAYLOAD);
expect(captions).toContain(BREAKOUT_PAYLOAD);
// 3. And nothing executed.
expect(
await page.evaluate(() => (window as unknown as { __XSS__?: number }).__XSS__ === 1),
'the caption must be inert, not merely non-fatal'
).toBe(false);
expect(xss).toBe(false);
// 4. The viewer actually rendered — the whole document parsed, not just the head. If the
// tokenizer had swallowed the bundle, the body would be empty of viewer output.
await expect(page.locator('body')).not.toBeEmpty();
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
});

View File

@@ -84,9 +84,16 @@
{ key: 'feed_rate_enabled', label: 'Feed-Limit aktiv', kind: 'bool' },
{ key: 'export_rate_enabled', label: 'Export-Limit aktiv', kind: 'bool' },
{ key: 'join_rate_enabled', label: 'Join-Limit aktiv', kind: 'bool' },
{ key: 'social_rate_enabled', label: 'Interaktions-Limit aktiv', kind: 'bool' },
{ key: 'upload_rate_per_hour', label: 'Upload-Limit pro Stunde', kind: 'number' },
{ key: 'feed_rate_per_min', label: 'Feed-Anfragen pro Minute', kind: 'number' },
{ key: 'export_rate_per_day', label: 'Export-Downloads pro Tag', kind: 'number' }
{ key: 'export_rate_per_day', label: 'Export-Downloads pro Tag', kind: 'number' },
{
key: 'social_rate_per_min',
label: 'Interaktionen pro Minute',
kind: 'number',
hint: 'Likes, Kommentare und Kommentar-Löschungen zusammen, pro Gast. Bewusst hoch angesetzt — soll ein Skript bremsen, keinen begeisterten Gast.'
}
]
},
{