Merge branch 'test/suite-audit-2026-07-15'
This commit is contained in:
fabi
113
.github/workflows/checks.yml
vendored
Normal file
113
.github/workflows/checks.yml
vendored
Normal file
@@ -0,0 +1,113 @@
|
||||
# The checks that were only ever running on a laptop.
|
||||
#
|
||||
# Before this file, CI ran Playwright (chromium-desktop) and the dependency audit — and nothing
|
||||
# else. `cargo test` (40 tests), the frontend vitest suite (5 files, including the offline
|
||||
# upload-queue and auth-token logic), svelte-check, and the e2e typecheck were all green on a
|
||||
# developer's machine and gated NOTHING. A check that only ever runs locally is not running.
|
||||
#
|
||||
# In .github/workflows/ because Gitea Actions scans it too (see audit.yml). Rust is installed
|
||||
# explicitly: the common Gitea runner images ship Node and Docker, not cargo.
|
||||
name: Checks
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
push:
|
||||
branches: [main]
|
||||
|
||||
jobs:
|
||||
backend:
|
||||
name: Backend — cargo test + clippy + fmt
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 20
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- name: Install Rust toolchain
|
||||
run: |
|
||||
if ! command -v cargo > /dev/null; then
|
||||
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal
|
||||
echo "$HOME/.cargo/bin" >> "$GITHUB_PATH"
|
||||
fi
|
||||
rustup component add clippy rustfmt
|
||||
|
||||
- uses: actions/cache@v4
|
||||
with:
|
||||
path: |
|
||||
~/.cargo/registry
|
||||
~/.cargo/git
|
||||
backend/target
|
||||
key: cargo-${{ hashFiles('backend/Cargo.lock') }}
|
||||
restore-keys: cargo-
|
||||
|
||||
# SQLx runs its queries against a live database at TEST time (see backend/tests/), so the
|
||||
# DB-backed tests need one. The pure unit tests don't care, but starting it unconditionally
|
||||
# keeps the job simple and honest about what it covers.
|
||||
- name: Start Postgres
|
||||
run: |
|
||||
docker run -d --name ci-pg -p 5432:5432 \
|
||||
-e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=eventsnap_ci \
|
||||
postgres:16-alpine
|
||||
for _ in $(seq 1 30); do
|
||||
docker exec ci-pg pg_isready -U postgres > /dev/null 2>&1 && break
|
||||
sleep 1
|
||||
done
|
||||
|
||||
- name: Test
|
||||
working-directory: ./backend
|
||||
env:
|
||||
DATABASE_URL: postgres://postgres:postgres@localhost:5432/eventsnap_ci
|
||||
# `#[sqlx::test]` creates a fresh database per test and opens its own pool; run at unbounded
|
||||
# parallelism against a default `max_connections=100` Postgres, a full suite can exhaust the
|
||||
# server's connection slots and fail with PoolTimedOut — a pure infra flake, not a real
|
||||
# failure. Cap the concurrency so the gate stays trustworthy on a loaded runner.
|
||||
run: cargo test --all-features -- --test-threads=8
|
||||
|
||||
- name: Clippy
|
||||
working-directory: ./backend
|
||||
run: cargo clippy --all-targets -- -D warnings
|
||||
|
||||
# NOTE: `cargo fmt --check` is deliberately NOT gated. The tree has never been rustfmt'd, so
|
||||
# turning it on means a 112-file mechanical reformat that would bury every real diff under it.
|
||||
# Formatting is not a correctness gate; do that cleanup on its own, then add the check here.
|
||||
|
||||
frontend:
|
||||
name: Frontend — vitest + svelte-check
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 15
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: '22'
|
||||
cache: 'npm'
|
||||
cache-dependency-path: 'frontend/package-lock.json'
|
||||
|
||||
- name: Install deps
|
||||
working-directory: ./frontend
|
||||
run: npm ci || npm install
|
||||
|
||||
- name: Unit tests
|
||||
working-directory: ./frontend
|
||||
run: npm run test:unit
|
||||
|
||||
- name: svelte-check
|
||||
working-directory: ./frontend
|
||||
run: npx svelte-check --threshold error
|
||||
|
||||
e2e-typecheck:
|
||||
name: E2E — typecheck
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 10
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: '22'
|
||||
- name: Install deps
|
||||
working-directory: ./e2e
|
||||
run: npm install
|
||||
# Playwright TRANSPILES specs without typechecking them, so a type error in a spec is
|
||||
# invisible until the assertion it guards silently does the wrong thing at runtime.
|
||||
- name: tsc --noEmit
|
||||
working-directory: ./e2e
|
||||
run: npx tsc --noEmit
|
||||
8
.github/workflows/e2e.yml
vendored
8
.github/workflows/e2e.yml
vendored
@@ -46,6 +46,14 @@ jobs:
|
||||
working-directory: ./e2e
|
||||
run: npm run test:e2e -- --project=chromium-desktop
|
||||
|
||||
# 09-mobile is `testIgnore`d on chromium-desktop (it needs hasTouch + a phone viewport), so
|
||||
# running only that project left 22 tests — focus traps, touch targets, safe-area insets,
|
||||
# viewport reflow, upload-cancel — never executing in CI. On a phone-first event app, where
|
||||
# essentially every real guest is on a phone, that was the wrong half of the suite to skip.
|
||||
- name: Run E2E tests (mobile)
|
||||
working-directory: ./e2e
|
||||
run: npm run test:e2e -- --project=chromium-mobile
|
||||
|
||||
- name: Upload Playwright report
|
||||
if: failure()
|
||||
uses: actions/upload-artifact@v4
|
||||
|
||||
31
README.md
31
README.md
@@ -182,6 +182,28 @@ The `/media` volume holds originals, previews, thumbnails, exports, and DB backu
|
||||
|
||||
---
|
||||
|
||||
## Running the backend test suite
|
||||
|
||||
```bash
|
||||
cd backend
|
||||
|
||||
# The DB-backed integration tests (backend/tests/) need a live Postgres. `#[sqlx::test]` creates a
|
||||
# throwaway database per test and runs backend/migrations/ into it — it does NOT touch this one's data.
|
||||
docker run -d --name eventsnap-test-pg -p 55433:5432 \
|
||||
-e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=eventsnap postgres:16-alpine
|
||||
|
||||
export DATABASE_URL=postgres://postgres:postgres@localhost:55433/eventsnap
|
||||
cargo test # 44 unit + 12 DB-backed
|
||||
cargo clippy --all-targets -- -D warnings
|
||||
```
|
||||
|
||||
**`cargo test` requires `DATABASE_URL`** — without it the integration tests panic rather than skip.
|
||||
That is deliberate. The riskiest code in this repo is SQL (the export epoch state machine, the
|
||||
atomic quota increment, the `FOR SHARE` upload lock), and for a long time *not one line of it* was
|
||||
executed by `cargo test` — every backend test was a pure-function test, so the tests clustered
|
||||
tightly around the code that could not break and stopped exactly where it started to. Tests that
|
||||
silently skip when the database is absent recreate that hole; they were meant to be a gate.
|
||||
|
||||
## Running the E2E test suite
|
||||
|
||||
Playwright-based end-to-end tests live in [`e2e/`](e2e/). They spin up an isolated docker-compose stack (Postgres on `:55432`, Caddy on `:3101`) and exercise the SvelteKit frontend against the real Rust backend with rate limits disabled.
|
||||
@@ -198,7 +220,14 @@ npm run stack:down # tear it down
|
||||
|
||||
See [`e2e/README.md`](e2e/README.md) for the full UA matrix, Samsung Internet escalation tiers, and the Phase 2/3 roadmap.
|
||||
|
||||
CI runs this on every PR — see [`.github/workflows/e2e.yml`](.github/workflows/e2e.yml).
|
||||
CI runs this on every PR — see [`.github/workflows/e2e.yml`](.github/workflows/e2e.yml) (desktop **and**
|
||||
mobile projects), plus [`checks.yml`](.github/workflows/checks.yml) for `cargo test`/clippy, the frontend
|
||||
unit tests, svelte-check and the e2e typecheck, and [`audit.yml`](.github/workflows/audit.yml) for
|
||||
dependency advisories.
|
||||
|
||||
**Playwright runs with `retries: 0`, including in CI.** This repo's real bugs are races, and from the
|
||||
outside a race is indistinguishable from a flake — so a retry silently resolves that ambiguity in
|
||||
favour of "flake" every time. A flake here is a bug report; treat it as one.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -75,10 +75,10 @@ impl IntoResponse for AppError {
|
||||
}
|
||||
|
||||
let mut resp = (status, axum::Json(body)).into_response();
|
||||
if let Some(secs) = retry_after_secs {
|
||||
if let Ok(val) = axum::http::HeaderValue::from_str(&secs.to_string()) {
|
||||
resp.headers_mut().insert(axum::http::header::RETRY_AFTER, val);
|
||||
}
|
||||
if let Some(secs) = retry_after_secs
|
||||
&& let Ok(val) = axum::http::HeaderValue::from_str(&secs.to_string())
|
||||
{
|
||||
resp.headers_mut().insert(axum::http::header::RETRY_AFTER, val);
|
||||
}
|
||||
resp
|
||||
}
|
||||
|
||||
@@ -98,6 +98,10 @@ pub async fn get_config(
|
||||
Ok(Json(rows.into_iter().collect()))
|
||||
}
|
||||
|
||||
/// Documents the wire shape of `PATCH /admin/config` (a flat `{key: value}` object).
|
||||
/// `patch_config` extracts the `HashMap` directly rather than going through this newtype, so it is
|
||||
/// never constructed in Rust — it stays as the serde-derived description of the request body.
|
||||
#[allow(dead_code)]
|
||||
#[derive(Deserialize)]
|
||||
pub struct PatchConfigRequest(pub HashMap<String, String>);
|
||||
|
||||
|
||||
@@ -85,6 +85,19 @@ pub async fn truncate_all(
|
||||
// could serve the previous test's toggles.
|
||||
state.config_cache.invalidate();
|
||||
|
||||
// The other two in-memory singletons that TRUNCATE used to leave standing.
|
||||
//
|
||||
// `disk_cache` holds a free-space reading for up to its TTL. TRUNCATE has just deleted every
|
||||
// uploaded file, which materially changes free space — so without this the next test can
|
||||
// compute a storage quota from the PREVIOUS test's disk. That was harmless only while quotas
|
||||
// were globally disabled in e2e (they no longer are: see specs/02-upload/quota.spec.ts, which
|
||||
// steers the per-user limit off `free_disk_bytes`), i.e. two holes were masking each other.
|
||||
state.disk_cache.invalidate();
|
||||
|
||||
// `sse_tickets` maps a ticket to a session token hash. TRUNCATE deletes the sessions, so every
|
||||
// surviving ticket is a dangling reference to a user that no longer exists.
|
||||
state.sse_tickets.clear();
|
||||
|
||||
Ok(StatusCode::NO_CONTENT)
|
||||
}
|
||||
|
||||
|
||||
@@ -168,14 +168,14 @@ pub async fn upload(
|
||||
// Validate caption length. Counted in chars (code points) to match the
|
||||
// "Zeichen" wording in the error message — `.len()` would be bytes and
|
||||
// reject perfectly valid German/emoji captions early.
|
||||
if let Some(ref cap) = caption {
|
||||
if cap.chars().count() > MAX_CAPTION_LENGTH {
|
||||
let _ = tokio::fs::remove_file(&temp_abs).await;
|
||||
return Err(AppError::BadRequest(format!(
|
||||
"Beschreibung ist zu lang. Maximum: {} Zeichen.",
|
||||
MAX_CAPTION_LENGTH
|
||||
)));
|
||||
}
|
||||
if let Some(ref cap) = caption
|
||||
&& cap.chars().count() > MAX_CAPTION_LENGTH
|
||||
{
|
||||
let _ = tokio::fs::remove_file(&temp_abs).await;
|
||||
return Err(AppError::BadRequest(format!(
|
||||
"Beschreibung ist zu lang. Maximum: {} Zeichen.",
|
||||
MAX_CAPTION_LENGTH
|
||||
)));
|
||||
}
|
||||
|
||||
// Determine the file type from its magic bytes and require it to be on the
|
||||
@@ -555,6 +555,9 @@ pub struct QuotaEstimate {
|
||||
pub limit_bytes: Option<i64>,
|
||||
pub active_uploaders: i64,
|
||||
pub free_disk_bytes: i64,
|
||||
/// The tolerance factor the limit above was computed with. Carried on the snapshot so the
|
||||
/// number is self-describing; no caller reads it back today.
|
||||
#[allow(dead_code)]
|
||||
pub tolerance: f64,
|
||||
}
|
||||
|
||||
|
||||
@@ -3,6 +3,10 @@ use serde::Serialize;
|
||||
use sqlx::PgPool;
|
||||
use uuid::Uuid;
|
||||
|
||||
// Row shape for `comment`: every field is populated by sqlx from `SELECT *` / `RETURNING *`.
|
||||
// `deleted_at` is not read in Rust today (the soft-delete filter lives in SQL), but it is part of
|
||||
// the row and stays here so the struct keeps mirroring the table.
|
||||
#[allow(dead_code)]
|
||||
#[derive(Debug, sqlx::FromRow)]
|
||||
pub struct Comment {
|
||||
pub id: Uuid,
|
||||
@@ -87,16 +91,8 @@ impl Comment {
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn soft_delete(pool: &PgPool, id: Uuid) -> Result<(), sqlx::Error> {
|
||||
sqlx::query("UPDATE comment SET deleted_at = NOW() WHERE id = $1")
|
||||
.bind(id)
|
||||
.execute(pool)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Event-scoped variant of [`Self::soft_delete`]. Returns `false` if the
|
||||
/// comment doesn't exist or belongs to a different event.
|
||||
/// Event-scoped soft delete. Returns `false` if the comment doesn't exist or belongs to a
|
||||
/// different event.
|
||||
/// Executor-generic so the delete and the keepsake regeneration can share one transaction
|
||||
/// (see `Upload::soft_delete_in_event` for why that must be atomic).
|
||||
pub async fn soft_delete_in_event(
|
||||
|
||||
@@ -2,6 +2,11 @@ use chrono::{DateTime, Utc};
|
||||
use sqlx::PgPool;
|
||||
use uuid::Uuid;
|
||||
|
||||
// Row shape for `event`: every field is populated by sqlx from `SELECT *` / `RETURNING *`. Several
|
||||
// (`slug`, `cover_image_path`, `export_epoch`, `created_at`) are not read through this struct today
|
||||
// — callers that need them query the column directly — but they are part of the row and stay here so
|
||||
// the struct keeps mirroring the table.
|
||||
#[allow(dead_code)]
|
||||
#[derive(Debug, sqlx::FromRow)]
|
||||
pub struct Event {
|
||||
pub id: Uuid,
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
use sqlx::PgPool;
|
||||
use uuid::Uuid;
|
||||
|
||||
// Row shape for `hashtag`, populated by sqlx from `RETURNING *` in `upsert`. Callers only use
|
||||
// `id` today; `event_id`/`tag` are the rest of the row and stay part of the struct.
|
||||
#[allow(dead_code)]
|
||||
#[derive(Debug, sqlx::FromRow)]
|
||||
pub struct Hashtag {
|
||||
pub id: Uuid,
|
||||
@@ -61,22 +63,6 @@ impl Hashtag {
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn tags_for_upload(
|
||||
pool: &PgPool,
|
||||
upload_id: Uuid,
|
||||
) -> Result<Vec<String>, sqlx::Error> {
|
||||
let rows: Vec<(String,)> = sqlx::query_as(
|
||||
"SELECT h.tag FROM hashtag h
|
||||
JOIN upload_hashtag uh ON uh.hashtag_id = h.id
|
||||
WHERE uh.upload_id = $1
|
||||
ORDER BY h.tag",
|
||||
)
|
||||
.bind(upload_id)
|
||||
.fetch_all(pool)
|
||||
.await?;
|
||||
Ok(rows.into_iter().map(|r| r.0).collect())
|
||||
}
|
||||
}
|
||||
|
||||
/// Extract `#hashtags` from text (caption or body). Tags are restricted to
|
||||
|
||||
@@ -2,6 +2,9 @@ use chrono::{DateTime, Utc};
|
||||
use sqlx::PgPool;
|
||||
use uuid::Uuid;
|
||||
|
||||
// Row shape for `session`, populated by sqlx from `RETURNING *`. Session validation is done in SQL
|
||||
// (expiry/last-seen predicates), so no field is read in Rust — the struct is the row's shape.
|
||||
#[allow(dead_code)]
|
||||
#[derive(Debug, sqlx::FromRow)]
|
||||
pub struct Session {
|
||||
pub id: Uuid,
|
||||
|
||||
@@ -3,6 +3,10 @@ use serde::Serialize;
|
||||
use sqlx::PgPool;
|
||||
use uuid::Uuid;
|
||||
|
||||
// Row shape for `upload`: every field is populated by sqlx from `RETURNING *` in `create`. Callers
|
||||
// mostly use `id` and hand the rest to the compression/feed queries, so most fields are never read
|
||||
// through this struct — they stay here so it keeps mirroring the table.
|
||||
#[allow(dead_code)]
|
||||
#[derive(Debug, sqlx::FromRow)]
|
||||
pub struct Upload {
|
||||
pub id: Uuid,
|
||||
@@ -75,15 +79,6 @@ impl Upload {
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn find_by_id(pool: &PgPool, id: Uuid) -> Result<Option<Self>, sqlx::Error> {
|
||||
sqlx::query_as::<_, Self>(
|
||||
"SELECT * FROM upload WHERE id = $1 AND deleted_at IS NULL",
|
||||
)
|
||||
.bind(id)
|
||||
.fetch_optional(pool)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Lean lookup for the public media aliases (`get_original`/`get_preview`/
|
||||
/// `get_thumbnail`): returns ONLY the file paths + mime for a visible upload —
|
||||
/// excluding soft-deleted rows, hidden owners (`uploads_hidden`), and banned owners
|
||||
|
||||
@@ -22,6 +22,10 @@ impl UserRole {
|
||||
}
|
||||
}
|
||||
|
||||
// Row shape for `user`: every field is populated by sqlx from `SELECT *` / `RETURNING *`.
|
||||
// `uploads_hidden`, `failed_pin_attempts` and `created_at` are enforced/updated in SQL rather than
|
||||
// read in Rust, but they are part of the row and stay here so the struct keeps mirroring the table.
|
||||
#[allow(dead_code)]
|
||||
#[derive(Debug, sqlx::FromRow)]
|
||||
pub struct User {
|
||||
pub id: Uuid,
|
||||
|
||||
@@ -34,6 +34,17 @@ impl DiskCache {
|
||||
}
|
||||
}
|
||||
|
||||
/// Drop the cached reading so the next `snapshot()` re-measures the filesystem.
|
||||
///
|
||||
/// Used by the e2e TRUNCATE endpoint. Truncating deletes every uploaded file, which materially
|
||||
/// changes free space — but the cached reading survives for up to the TTL, so the next test can
|
||||
/// compute a quota from the PREVIOUS test's disk. That matters now that the quota tests steer
|
||||
/// the per-user limit off `free_disk_bytes`: a stale reading makes the limit wrong and the test
|
||||
/// flaky, for reasons that have nothing to do with the code under test.
|
||||
pub fn invalidate(&self) {
|
||||
*self.inner.write().unwrap() = None;
|
||||
}
|
||||
|
||||
/// Cached `(total, free)` bytes for the filesystem that holds `media_path`.
|
||||
///
|
||||
/// Returns `None` when the mount can't be resolved — callers MUST treat that as
|
||||
@@ -47,10 +58,11 @@ impl DiskCache {
|
||||
/// caller to ask about a different volume (e.g. the exports volume, which is a separate mount)
|
||||
/// would have silently got the wrong filesystem's free space.
|
||||
pub fn snapshot(&self, path: &Path) -> Option<DiskInfo> {
|
||||
if let Some((cached_path, info, at)) = self.inner.read().unwrap().as_ref() {
|
||||
if cached_path == path && at.elapsed() < TTL {
|
||||
return Some(*info);
|
||||
}
|
||||
if let Some((cached_path, info, at)) = self.inner.read().unwrap().as_ref()
|
||||
&& cached_path == path
|
||||
&& at.elapsed() < TTL
|
||||
{
|
||||
return Some(*info);
|
||||
}
|
||||
let info = read_disk_for_path(path)?;
|
||||
*self.inner.write().unwrap() = Some((path.to_path_buf(), info, Instant::now()));
|
||||
|
||||
@@ -389,6 +389,10 @@ async fn invalidate_missing_files(
|
||||
/// start immediately.
|
||||
pub const REGEN_DEBOUNCE: Duration = Duration::from_secs(20);
|
||||
|
||||
// Export worker entry point: every argument is state the spawned worker is BORN with (notably
|
||||
// `epoch`). Bundling them into a struct would be a pure-refactor risk on the epoch logic for no
|
||||
// gain, so the arity stands.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn spawn_export_jobs(
|
||||
event_id: Uuid,
|
||||
event_name: String,
|
||||
|
||||
@@ -1,73 +0,0 @@
|
||||
//! Shared shape for long-running background work.
|
||||
//!
|
||||
//! Today's [`compression`](crate::services::compression) and [`export`](crate::services::export)
|
||||
//! pipelines each implement their own progress + SSE plumbing. They could converge on the
|
||||
//! trait sketched here so future jobs (analytics, archival, ...) plug into one progress
|
||||
//! pipeline.
|
||||
//!
|
||||
//! This module is intentionally a *sketch*: the existing services are not yet wired to
|
||||
//! it. The aim is to (a) document the convention so new jobs follow it, (b) make the
|
||||
//! refactor mechanical when someone is ready to do it. See `docs/IDEAS.md` —
|
||||
//! "Maintainability principles" — for the rationale.
|
||||
//!
|
||||
//! Example of an eventual implementor:
|
||||
//!
|
||||
//! ```ignore
|
||||
//! struct ZipExport { event_id: Uuid, /* … */ }
|
||||
//!
|
||||
//! impl BackgroundJob for ZipExport {
|
||||
//! fn name(&self) -> &'static str { "zip-export" }
|
||||
//! async fn run(self, ctx: JobContext) -> Result<()> {
|
||||
//! for (i, item) in items.iter().enumerate() {
|
||||
//! ctx.report(percent(i, items.len())).await?;
|
||||
//! // … write to zip …
|
||||
//! }
|
||||
//! Ok(())
|
||||
//! }
|
||||
//! }
|
||||
//! ```
|
||||
|
||||
use anyhow::Result;
|
||||
|
||||
/// Handle handed to a running job: reports progress and emits SSE events.
|
||||
///
|
||||
/// Wraps the existing SSE broadcaster and an optional `export_job` row. Implementors
|
||||
/// don't need to know about `state.sse_tx` directly — they call [`JobContext::report`]
|
||||
/// and get the same effect.
|
||||
pub struct JobContext {
|
||||
pub job_id: Option<uuid::Uuid>,
|
||||
pub event_kind: &'static str,
|
||||
pub sse_tx: tokio::sync::broadcast::Sender<crate::state::SseEvent>,
|
||||
pub pool: sqlx::PgPool,
|
||||
}
|
||||
|
||||
impl JobContext {
|
||||
/// Update progress (0..=100) and broadcast an SSE tick. Cheap to call often —
|
||||
/// rate-limit at the call site if a job emits at > 10 Hz.
|
||||
pub async fn report(&self, percent: u8) -> Result<()> {
|
||||
if let Some(job_id) = self.job_id {
|
||||
sqlx::query("UPDATE export_job SET progress_pct = $1 WHERE id = $2")
|
||||
.bind(percent as i16)
|
||||
.bind(job_id)
|
||||
.execute(&self.pool)
|
||||
.await?;
|
||||
}
|
||||
let _ = self.sse_tx.send(crate::state::SseEvent::new(
|
||||
self.event_kind,
|
||||
serde_json::json!({ "progress_pct": percent }).to_string(),
|
||||
));
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
/// One unit of work that publishes progress through a [`JobContext`].
|
||||
///
|
||||
/// `run` consumes `self`; spawn with `tokio::spawn` at the caller. Errors propagate;
|
||||
/// the caller is responsible for mapping them to `export_job.error_message` or
|
||||
/// equivalent. Implementors stay small — the trait deliberately has no `cancel`
|
||||
/// or `pause`; we have not needed those yet.
|
||||
#[allow(async_fn_in_trait)]
|
||||
pub trait BackgroundJob: Send + 'static {
|
||||
fn name(&self) -> &'static str;
|
||||
async fn run(self, ctx: JobContext) -> Result<()>;
|
||||
}
|
||||
@@ -2,7 +2,6 @@ pub mod compression;
|
||||
pub mod config;
|
||||
pub mod disk;
|
||||
pub mod export;
|
||||
pub mod jobs;
|
||||
pub mod maintenance;
|
||||
pub mod rate_limiter;
|
||||
pub mod sse_tickets;
|
||||
|
||||
@@ -123,12 +123,58 @@ mod tests {
|
||||
assert!(rl.check("k", 1, w), "the slot should expire once the window passes");
|
||||
}
|
||||
|
||||
/// `retry_after` is not a "some number in range" — it is the time until the oldest slot
|
||||
/// in the window frees up, and it is surfaced to clients as the backoff they sleep for
|
||||
/// (see `upload-queue.ts`). Asserting only `(1..=60)` spans the entire reachable domain
|
||||
/// of a 60s window, so a hardcoded `Err(1)` would satisfy it while telling every client
|
||||
/// to hammer the server a second later. Pin the actual value.
|
||||
#[test]
|
||||
fn retry_after_is_between_one_and_window() {
|
||||
fn retry_after_is_the_remaining_window() {
|
||||
let rl = RateLimiter::new();
|
||||
assert!(rl.check_with_retry("k", 1, MIN).is_ok());
|
||||
let retry = rl.check_with_retry("k", 1, MIN).unwrap_err();
|
||||
assert!((1..=60).contains(&retry), "retry_after {retry} out of range");
|
||||
|
||||
// The slot was consumed just now, so essentially the whole window remains.
|
||||
// `as_secs()` truncates the sub-second remainder, so a 30s window reports 29.
|
||||
let w30 = Duration::from_secs(30);
|
||||
assert!(rl.check_with_retry("a", 1, w30).is_ok());
|
||||
let a = rl.check_with_retry("a", 1, w30).unwrap_err();
|
||||
assert_eq!(a, 29, "retry_after must be the remaining window, got {a}");
|
||||
|
||||
// A different window must yield a different retry_after: no single constant can
|
||||
// satisfy both this and the assertion above.
|
||||
let w10 = Duration::from_secs(10);
|
||||
assert!(rl.check_with_retry("b", 1, w10).is_ok());
|
||||
let b = rl.check_with_retry("b", 1, w10).unwrap_err();
|
||||
assert_eq!(b, 9, "retry_after must scale with the window, got {b}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retry_after_counts_down_as_the_window_elapses() {
|
||||
let rl = RateLimiter::new();
|
||||
let w = Duration::from_secs(30);
|
||||
assert!(rl.check_with_retry("k", 1, w).is_ok());
|
||||
let first = rl.check_with_retry("k", 1, w).unwrap_err();
|
||||
|
||||
std::thread::sleep(Duration::from_millis(1200));
|
||||
let second = rl.check_with_retry("k", 1, w).unwrap_err();
|
||||
|
||||
// A client that waits 1.2s must be told to wait ~1.2s less — otherwise the advertised
|
||||
// backoff is a constant, not a deadline.
|
||||
let shaved = first - second;
|
||||
assert!(
|
||||
(1..=2).contains(&shaved),
|
||||
"1.2s of waiting must shorten the advertised backoff by ~1s (got {first} then {second})"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retry_after_floors_at_one_second() {
|
||||
let rl = RateLimiter::new();
|
||||
let w = Duration::from_millis(800);
|
||||
assert!(rl.check_with_retry("k", 1, w).is_ok());
|
||||
let retry = rl.check_with_retry("k", 1, w).unwrap_err();
|
||||
// The sub-second remainder truncates to 0; clients must never be told "retry in 0s"
|
||||
// (that's a busy-loop). The `.max(1)` floor is what prevents it.
|
||||
assert_eq!(retry, 1, "a sub-second remainder must floor to 1, got {retry}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -140,6 +186,59 @@ mod tests {
|
||||
assert!(rl.check("k", 1, MIN), "clear() must free the window");
|
||||
}
|
||||
|
||||
/// `prune()` is a memory-leak guard: without it a long-lived process keeps one HashMap
|
||||
/// entry per IP that ever connected. Nothing in the public API observes the map size, so
|
||||
/// the only way to catch a no-op body (`fn prune(&self) {}`) is to look at the map — the
|
||||
/// tests module can see the private field.
|
||||
#[test]
|
||||
fn prune_drops_keys_whose_windows_have_fully_expired() {
|
||||
let rl = RateLimiter::new();
|
||||
|
||||
// A key whose only timestamp is older than the 24h ceiling. We can't sleep for a day,
|
||||
// so backdate the Instant directly.
|
||||
let ancient = Instant::now()
|
||||
.checked_sub(Duration::from_secs(25 * 60 * 60))
|
||||
.expect("backdating an Instant by 25h");
|
||||
rl.windows
|
||||
.lock()
|
||||
.unwrap()
|
||||
.insert("stale".to_string(), vec![ancient]);
|
||||
|
||||
// ...alongside a key that is still inside its window.
|
||||
assert!(rl.check("live", 5, MIN));
|
||||
assert_eq!(rl.windows.lock().unwrap().len(), 2);
|
||||
|
||||
rl.prune();
|
||||
|
||||
let map = rl.windows.lock().unwrap();
|
||||
assert!(
|
||||
!map.contains_key("stale"),
|
||||
"prune() must drop keys whose timestamps have all expired"
|
||||
);
|
||||
assert!(
|
||||
map.contains_key("live"),
|
||||
"prune() must keep keys that still have live timestamps"
|
||||
);
|
||||
assert_eq!(map.len(), 1, "exactly one key should survive the prune");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prune_does_not_reset_a_live_window() {
|
||||
// The counterpart to the test above: pruning must reclaim memory, never quota. If
|
||||
// prune() dropped live keys, every background sweep would hand attackers a fresh
|
||||
// budget.
|
||||
let rl = RateLimiter::new();
|
||||
assert!(rl.check("k", 1, MIN));
|
||||
assert!(!rl.check("k", 1, MIN));
|
||||
|
||||
rl.prune();
|
||||
|
||||
assert!(
|
||||
!rl.check("k", 1, MIN),
|
||||
"prune() must not clear a window that is still active"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn client_ip_takes_rightmost_forwarded_for_entry() {
|
||||
// The right-most entry is the hop our trusted proxy (Caddy) appended.
|
||||
|
||||
@@ -31,6 +31,13 @@ impl SseTicketStore {
|
||||
}
|
||||
}
|
||||
|
||||
/// Drop every outstanding ticket. Used by the e2e TRUNCATE endpoint: tickets are bound to a
|
||||
/// session token hash, and TRUNCATE deletes the sessions out from under them, so anything left
|
||||
/// here is a dangling reference to a user that no longer exists.
|
||||
pub fn clear(&self) {
|
||||
self.inner.lock().unwrap().clear();
|
||||
}
|
||||
|
||||
/// Mint a new ticket bound to the caller's session (identified by token hash).
|
||||
pub fn issue(&self, token_hash: String) -> String {
|
||||
let ticket = random_ticket();
|
||||
|
||||
257
backend/tests/common/mod.rs
Normal file
257
backend/tests/common/mod.rs
Normal file
@@ -0,0 +1,257 @@
|
||||
//! Shared fixtures for the DB-backed integration tests.
|
||||
//!
|
||||
//! Every helper here executes SQL that is **character-for-character identical** to what `src/`
|
||||
//! actually runs (see the `// SRC:` markers). That is the whole point: a paraphrased query is a
|
||||
//! query nobody runs, and a test that passes against a paraphrase proves nothing about production.
|
||||
|
||||
#![allow(dead_code)] // each integration-test crate uses a different subset of these helpers
|
||||
|
||||
use sqlx::PgPool;
|
||||
use uuid::Uuid;
|
||||
|
||||
/// Insert a bare, unreleased event (epoch 0, uploads open).
|
||||
pub async fn seed_event(pool: &PgPool, slug: &str) -> Uuid {
|
||||
sqlx::query_scalar("INSERT INTO event (slug, name) VALUES ($1, $2) RETURNING id")
|
||||
.bind(slug)
|
||||
.bind("Hochzeit")
|
||||
.fetch_one(pool)
|
||||
.await
|
||||
.expect("seed event")
|
||||
}
|
||||
|
||||
/// Insert a guest with a zeroed byte total.
|
||||
pub async fn seed_user(pool: &PgPool, event_id: Uuid, name: &str) -> Uuid {
|
||||
sqlx::query_scalar(
|
||||
"INSERT INTO \"user\" (event_id, display_name, recovery_pin_hash)
|
||||
VALUES ($1, $2, 'x') RETURNING id",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(name)
|
||||
.fetch_one(pool)
|
||||
.await
|
||||
.expect("seed user")
|
||||
}
|
||||
|
||||
/// SRC: `handlers/host.rs::release_gallery` — the claim + epoch bump, verbatim.
|
||||
/// Returns the POST-increment epoch, exactly as the handler consumes it.
|
||||
pub async fn release_gallery(pool: &PgPool, slug: &str) -> Option<i64> {
|
||||
let claimed: Option<(Uuid, String, i64)> = sqlx::query_as(
|
||||
"UPDATE event
|
||||
SET export_released_at = NOW(),
|
||||
uploads_locked_at = COALESCE(uploads_locked_at, NOW()),
|
||||
export_epoch = export_epoch + 1
|
||||
WHERE slug = $1 AND export_released_at IS NULL
|
||||
RETURNING id, name, export_epoch",
|
||||
)
|
||||
.bind(slug)
|
||||
.fetch_optional(pool)
|
||||
.await
|
||||
.expect("release_gallery");
|
||||
|
||||
if let Some((event_id, _, epoch)) = claimed {
|
||||
// The handler arms both jobs in the SAME transaction; for a single-connection fixture the
|
||||
// sequencing is equivalent.
|
||||
let mut conn = pool.acquire().await.expect("acquire");
|
||||
enqueue_types_at_epoch(&mut conn, event_id, epoch, &["zip", "html"]).await;
|
||||
Some(epoch)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// SRC: `handlers/host.rs::open_event` — the one statement that retires an entire generation.
|
||||
/// Returns rows affected.
|
||||
pub async fn open_event(pool: &PgPool, slug: &str) -> u64 {
|
||||
sqlx::query(
|
||||
"UPDATE event
|
||||
SET uploads_locked_at = NULL,
|
||||
export_released_at = NULL,
|
||||
export_epoch = export_epoch + 1
|
||||
WHERE slug = $1 AND (uploads_locked_at IS NOT NULL OR export_released_at IS NOT NULL)",
|
||||
)
|
||||
.bind(slug)
|
||||
.execute(pool)
|
||||
.await
|
||||
.expect("open_event")
|
||||
.rows_affected()
|
||||
}
|
||||
|
||||
/// SRC: `services/export.rs::enqueue_types_at_epoch` — verbatim upsert.
|
||||
pub async fn enqueue_types_at_epoch(
|
||||
conn: &mut sqlx::PgConnection,
|
||||
event_id: Uuid,
|
||||
epoch: i64,
|
||||
types: &[&str],
|
||||
) {
|
||||
for export_type in types {
|
||||
sqlx::query(
|
||||
"INSERT INTO export_job (event_id, type, status, progress_pct, epoch)
|
||||
VALUES ($1, $2::export_type, 'pending', 0, $3)
|
||||
ON CONFLICT (event_id, type) DO UPDATE
|
||||
SET status = 'pending', progress_pct = 0, file_path = NULL,
|
||||
error_message = NULL, completed_at = NULL,
|
||||
epoch = EXCLUDED.epoch
|
||||
WHERE export_job.status <> 'done' OR export_job.epoch <> EXCLUDED.epoch",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(export_type)
|
||||
.bind(epoch)
|
||||
.execute(&mut *conn)
|
||||
.await
|
||||
.expect("enqueue_types_at_epoch");
|
||||
}
|
||||
}
|
||||
|
||||
/// SRC: `services/export.rs::claim_job` — verbatim. `true` = we won the generation.
|
||||
pub async fn claim_job(pool: &PgPool, event_id: Uuid, export_type: &str, epoch: i64) -> bool {
|
||||
sqlx::query(
|
||||
"UPDATE export_job SET status = 'running'
|
||||
WHERE event_id = $1 AND type = $2::export_type
|
||||
AND epoch = $3 AND status = 'pending'",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(export_type)
|
||||
.bind(epoch)
|
||||
.execute(pool)
|
||||
.await
|
||||
.expect("claim_job")
|
||||
.rows_affected()
|
||||
> 0
|
||||
}
|
||||
|
||||
/// SRC: `services/export.rs::finalize_job` — verbatim. This IS the publish step.
|
||||
pub async fn finalize_job(
|
||||
pool: &PgPool,
|
||||
event_id: Uuid,
|
||||
export_type: &str,
|
||||
epoch: i64,
|
||||
file_path: &str,
|
||||
) -> bool {
|
||||
sqlx::query(
|
||||
"UPDATE export_job
|
||||
SET status = 'done', progress_pct = 100, file_path = $3, completed_at = NOW()
|
||||
WHERE event_id = $1 AND type = $2::export_type
|
||||
AND epoch = $4 AND status = 'running'",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(export_type)
|
||||
.bind(file_path)
|
||||
.bind(epoch)
|
||||
.execute(pool)
|
||||
.await
|
||||
.expect("finalize_job")
|
||||
.rows_affected()
|
||||
> 0
|
||||
}
|
||||
|
||||
/// SRC: `services/export.rs::update_progress` — verbatim. Doubles as the worker's liveness check:
|
||||
/// `false` means "your generation was retired, stop working".
|
||||
pub async fn update_progress(
|
||||
pool: &PgPool,
|
||||
event_id: Uuid,
|
||||
export_type: &str,
|
||||
epoch: i64,
|
||||
pct: i16,
|
||||
) -> bool {
|
||||
sqlx::query(
|
||||
"UPDATE export_job SET progress_pct = $3
|
||||
WHERE event_id = $1 AND type = $2::export_type
|
||||
AND epoch = $4 AND status = 'running'",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(export_type)
|
||||
.bind(pct)
|
||||
.bind(epoch)
|
||||
.execute(pool)
|
||||
.await
|
||||
.expect("update_progress")
|
||||
.rows_affected()
|
||||
> 0
|
||||
}
|
||||
|
||||
/// SRC: `services/export.rs::invalidate_and_arm` — the ViewerOnly ZIP carry-forward, verbatim.
|
||||
/// Returns `rows_affected() == 1`, which is what the production code branches on.
|
||||
pub async fn carry_zip_forward(pool: &PgPool, event_id: Uuid, epoch: i64) -> bool {
|
||||
sqlx::query(
|
||||
"UPDATE export_job SET epoch = $2
|
||||
WHERE event_id = $1 AND type = 'zip'::export_type
|
||||
AND status = 'done' AND epoch = $2 - 1",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(epoch)
|
||||
.execute(pool)
|
||||
.await
|
||||
.expect("carry_zip_forward")
|
||||
.rows_affected()
|
||||
== 1
|
||||
}
|
||||
|
||||
/// SRC: `services/export.rs::invalidate_and_arm` — the epoch bump, verbatim.
|
||||
pub async fn bump_epoch(pool: &PgPool, slug: &str) -> Option<(Uuid, String, i64)> {
|
||||
sqlx::query_as(
|
||||
"UPDATE event SET export_epoch = export_epoch + 1
|
||||
WHERE slug = $1 AND export_released_at IS NOT NULL
|
||||
RETURNING id, name, export_epoch",
|
||||
)
|
||||
.bind(slug)
|
||||
.fetch_optional(pool)
|
||||
.await
|
||||
.expect("bump_epoch")
|
||||
}
|
||||
|
||||
/// The event's authoritative epoch.
|
||||
pub async fn event_epoch(pool: &PgPool, event_id: Uuid) -> i64 {
|
||||
sqlx::query_scalar("SELECT export_epoch FROM event WHERE id = $1")
|
||||
.bind(event_id)
|
||||
.fetch_one(pool)
|
||||
.await
|
||||
.expect("event_epoch")
|
||||
}
|
||||
|
||||
/// The raw job row, bypassing `export_current` — what the WORKER sees.
|
||||
pub async fn job_row(
|
||||
pool: &PgPool,
|
||||
event_id: Uuid,
|
||||
export_type: &str,
|
||||
) -> Option<(String, i64, Option<String>)> {
|
||||
sqlx::query_as(
|
||||
"SELECT status::text, epoch, file_path FROM export_job
|
||||
WHERE event_id = $1 AND type = $2::export_type",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(export_type)
|
||||
.fetch_optional(pool)
|
||||
.await
|
||||
.expect("job_row")
|
||||
}
|
||||
|
||||
/// Does `export_current` expose this job at all? (The view itself, without the `status` filter —
|
||||
/// it is what `handlers/admin.rs::export_status` reports to the host UI.)
|
||||
pub async fn in_export_current(pool: &PgPool, event_id: Uuid, export_type: &str) -> bool {
|
||||
sqlx::query_scalar::<_, i64>(
|
||||
"SELECT COUNT(*) FROM export_current
|
||||
WHERE event_id = $1 AND type = $2::export_type",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(export_type)
|
||||
.fetch_one(pool)
|
||||
.await
|
||||
.expect("in_export_current")
|
||||
> 0
|
||||
}
|
||||
|
||||
/// THE download predicate. SRC: `handlers/admin.rs::download_export` reads exactly this shape —
|
||||
/// `SELECT c.file_path FROM export_current c WHERE ... AND c.status = 'done'`. If this returns
|
||||
/// `Some`, a guest can download the keepsake; if `None`, they get a 404.
|
||||
pub async fn downloadable(pool: &PgPool, event_id: Uuid, export_type: &str) -> Option<String> {
|
||||
sqlx::query_scalar(
|
||||
"SELECT c.file_path FROM export_current c
|
||||
WHERE c.event_id = $1 AND c.type = $2::export_type AND c.status = 'done'",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(export_type)
|
||||
.fetch_optional(pool)
|
||||
.await
|
||||
.expect("downloadable")
|
||||
.flatten()
|
||||
}
|
||||
363
backend/tests/export_epoch.rs
Normal file
363
backend/tests/export_epoch.rs
Normal file
@@ -0,0 +1,363 @@
|
||||
//! DB-backed integration tests for the export epoch state machine (migration 014).
|
||||
//!
|
||||
//! These run against a REAL Postgres: `#[sqlx::test]` creates a throwaway database per test and
|
||||
//! runs `backend/migrations/` into it, so the schema, the enums, the `UNIQUE (event_id, type)`
|
||||
//! constraint and the `export_current` view are the production ones — not a mock.
|
||||
//!
|
||||
//! THE INVARIANT, from migration 014:
|
||||
//!
|
||||
//! An export is downloadable IFF
|
||||
//! event.export_released_at IS NOT NULL
|
||||
//! AND export_job.epoch = event.export_epoch
|
||||
//! AND export_job.status = 'done'
|
||||
//!
|
||||
//! Readiness is DERIVED (the `export_current` view), never stored. Every test below pins one leg of
|
||||
//! that invariant with the exact SQL `src/` executes (see `tests/common/mod.rs`).
|
||||
|
||||
mod common;
|
||||
|
||||
use common::*;
|
||||
use sqlx::PgPool;
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// 1. Epoch monotonicity
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// Release, reopen and re-release each bump `export_epoch`, and `RETURNING export_epoch` hands the
|
||||
/// caller the POST-increment value.
|
||||
///
|
||||
/// PREVENTS: a worker born with the PRE-increment epoch. It would be inert from the instant it
|
||||
/// started — every one of its writes is `epoch`-guarded, so `claim_job`/`finalize_job` would match
|
||||
/// nothing, the job row would sit at `pending` 0% forever with no live worker, and the host's
|
||||
/// download button would spin and then 404. The keepsake would never be built at all.
|
||||
#[sqlx::test]
|
||||
async fn release_returns_post_increment_epoch(pool: PgPool) {
|
||||
let event_id = seed_event(&pool, "wedding").await;
|
||||
assert_eq!(event_epoch(&pool, event_id).await, 0, "a fresh event starts at epoch 0");
|
||||
|
||||
let released = release_gallery(&pool, "wedding").await.expect("release claims the event");
|
||||
assert_eq!(released, 1, "RETURNING must give the epoch AFTER the +1, not before");
|
||||
assert_eq!(event_epoch(&pool, event_id).await, released, "worker's epoch == event's epoch");
|
||||
|
||||
// The jobs armed by the release carry exactly that epoch — this is what makes the worker's
|
||||
// guarded writes match.
|
||||
for t in ["zip", "html"] {
|
||||
let (status, epoch, _) = job_row(&pool, event_id, t).await.expect("job armed");
|
||||
assert_eq!(status, "pending");
|
||||
assert_eq!(epoch, released, "{t} job must be armed at the epoch the worker was born with");
|
||||
}
|
||||
}
|
||||
|
||||
/// Epoch is strictly monotonic across the whole release/reopen/re-release cycle, and a second
|
||||
/// release attempt while already released is rejected WITHOUT bumping.
|
||||
///
|
||||
/// PREVENTS: epoch reuse. If a reopen could return the event to an epoch some old `done` row still
|
||||
/// carries, a retired keepsake — one that a guest asked to be taken down from — would silently
|
||||
/// become downloadable again.
|
||||
#[sqlx::test]
|
||||
async fn epoch_is_strictly_monotonic_across_reopen(pool: PgPool) {
|
||||
let event_id = seed_event(&pool, "wedding").await;
|
||||
|
||||
assert_eq!(release_gallery(&pool, "wedding").await, Some(1));
|
||||
|
||||
// A duplicate release is a no-op (`WHERE export_released_at IS NULL`) and must NOT bump.
|
||||
assert_eq!(release_gallery(&pool, "wedding").await, None, "already released");
|
||||
assert_eq!(event_epoch(&pool, event_id).await, 1, "a rejected release must not move the epoch");
|
||||
|
||||
// Reopen retires the generation with ONE write.
|
||||
assert_eq!(open_event(&pool, "wedding").await, 1);
|
||||
assert_eq!(event_epoch(&pool, event_id).await, 2, "reopen bumps");
|
||||
|
||||
// And re-releasing bumps again — never back to 1.
|
||||
assert_eq!(release_gallery(&pool, "wedding").await, Some(3), "re-release bumps again");
|
||||
assert_eq!(event_epoch(&pool, event_id).await, 3);
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// 2. A retired-epoch worker is inert
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// A worker holding a retired epoch cannot write anything anybody can see: once the rows have been
|
||||
/// re-armed at a newer epoch, its `update_progress` and `finalize_job` both match 0 rows, and
|
||||
/// `export_current` never exposes its output.
|
||||
///
|
||||
/// PREVENTS: the classic lost race — a slow worker from BEFORE a takedown finishing afterwards and
|
||||
/// publishing an archive that still contains the photo a guest asked to have removed. "Please take
|
||||
/// my photo out" is the one request that most needs to reach the keepsake, and the keepsake is the
|
||||
/// artifact people keep forever.
|
||||
#[sqlx::test]
|
||||
async fn retired_epoch_worker_writes_are_no_ops(pool: PgPool) {
|
||||
let event_id = seed_event(&pool, "wedding").await;
|
||||
let old_epoch = release_gallery(&pool, "wedding").await.unwrap();
|
||||
|
||||
// Worker A is born at epoch 1 and claims the ZIP.
|
||||
assert!(claim_job(&pool, event_id, "zip", old_epoch).await, "worker A wins its claim");
|
||||
assert!(update_progress(&pool, event_id, "zip", old_epoch, 40).await, "still live at 40%");
|
||||
|
||||
// ── A takedown lands mid-export: `invalidate_and_arm(Affects::Both)` bumps and re-arms. ──
|
||||
let (_, _, new_epoch) = bump_epoch(&pool, "wedding").await.expect("bump on a released event");
|
||||
assert_eq!(new_epoch, old_epoch + 1);
|
||||
let mut conn = pool.acquire().await.unwrap();
|
||||
enqueue_types_at_epoch(&mut conn, event_id, new_epoch, &["zip", "html"]).await;
|
||||
drop(conn);
|
||||
|
||||
// Worker A is now INERT BY CONSTRUCTION. Every write is guarded on its own birth epoch.
|
||||
assert!(
|
||||
!update_progress(&pool, event_id, "zip", old_epoch, 90).await,
|
||||
"the liveness check must report `false` so worker A stops grinding through the gallery"
|
||||
);
|
||||
assert!(
|
||||
!finalize_job(&pool, event_id, "zip", old_epoch, "exports/Gallery.1.zip").await,
|
||||
"worker A's finalize MUST affect 0 rows — this is the write that would have published a \
|
||||
keepsake still containing the taken-down photo"
|
||||
);
|
||||
|
||||
// The re-armed row is untouched by the loser: still pending at the LIVE epoch, waiting for the
|
||||
// fresh worker. (If worker A had won, this row would read `done` at epoch 1.)
|
||||
let (status, epoch, file_path) = job_row(&pool, event_id, "zip").await.unwrap();
|
||||
assert_eq!((status.as_str(), epoch), ("pending", new_epoch));
|
||||
assert_eq!(file_path, None, "the loser's file_path must never be recorded");
|
||||
|
||||
// And nothing is downloadable — not the stale archive, not anything.
|
||||
assert_eq!(downloadable(&pool, event_id, "zip").await, None);
|
||||
}
|
||||
|
||||
/// The documented, deliberate nuance in `claim_job`: after a bare `open_event` (which writes
|
||||
/// NOTHING to `export_job` — that is the point of the design), a worker at the old epoch still WINS
|
||||
/// its claim and can still write `done`. That is wasted work, not incorrectness: retirement is
|
||||
/// enforced at READ time. `export_current` must refuse to expose the row.
|
||||
///
|
||||
/// PREVENTS: someone "optimising" `claim_job` into a cross-table `EXISTS (SELECT ... FROM event)`
|
||||
/// guard — the exact unsound guard migration 014 removed (under READ COMMITTED, a blocked UPDATE
|
||||
/// re-evaluates same-row predicates but answers other-table subqueries from a stale snapshot).
|
||||
/// This test pins the read-time enforcement so the write-time guard is never re-added.
|
||||
#[sqlx::test]
|
||||
async fn reopen_retires_at_read_time_not_write_time(pool: PgPool) {
|
||||
let event_id = seed_event(&pool, "wedding").await;
|
||||
let epoch = release_gallery(&pool, "wedding").await.unwrap();
|
||||
|
||||
assert!(claim_job(&pool, event_id, "zip", epoch).await);
|
||||
|
||||
// Host reopens uploads. No export_job row is touched.
|
||||
assert_eq!(open_event(&pool, "wedding").await, 1);
|
||||
|
||||
// The in-flight worker's row-local writes still match — it was never told to stop.
|
||||
assert!(
|
||||
finalize_job(&pool, event_id, "zip", epoch, "exports/Gallery.1.zip").await,
|
||||
"documented: the claim/finalize is guarded on the JOB row's epoch, not the event's"
|
||||
);
|
||||
let (status, _, _) = job_row(&pool, event_id, "zip").await.unwrap();
|
||||
assert_eq!(status, "done", "the row really does say done");
|
||||
|
||||
// …and yet it is invisible. `export_current` requires the event to be released AND the epochs to
|
||||
// match; the reopen broke both. A worker at a dead epoch writes a row nobody can see.
|
||||
assert!(!in_export_current(&pool, event_id, "zip").await);
|
||||
assert_eq!(
|
||||
downloadable(&pool, event_id, "zip").await,
|
||||
None,
|
||||
"a reopened event must serve NO keepsake, however finished the job row looks"
|
||||
);
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// 3. `export_current` exactness (table-driven)
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// The view is the ONE definition of "downloadable". Released + `done` + matching epoch ⇒ present;
|
||||
/// break ANY single leg ⇒ absent. Nothing else may make it appear or disappear.
|
||||
///
|
||||
/// PREVENTS, leg by leg:
|
||||
/// * `released` — serving a keepsake for an event whose uploads are still open, i.e. an archive
|
||||
/// missing every photo taken after the snapshot.
|
||||
/// * `done` — handing out a half-written ZIP (a corrupt keepsake, downloaded once, kept forever).
|
||||
/// * `epoch` — the retired-generation download: the 404-forever keepsake, or worse, the archive
|
||||
/// still containing content that was taken down.
|
||||
#[sqlx::test]
|
||||
async fn export_current_is_exactly_the_invariant(pool: PgPool) {
|
||||
// (name, released?, status, job epoch offset from the event epoch, expected visible)
|
||||
let cases: &[(&str, bool, &str, i64, bool)] = &[
|
||||
("released + done + current epoch", true, "done", 0, true),
|
||||
("NOT released (done, epoch matches)", false, "done", 0, false),
|
||||
("NOT done: pending", true, "pending", 0, false),
|
||||
("NOT done: running", true, "running", 0, false),
|
||||
("NOT done: failed", true, "failed", 0, false),
|
||||
("stale epoch (done, released)", true, "done", -1, false),
|
||||
("future epoch (done, released)", true, "done", 1, false),
|
||||
("migration-014 retired sentinel epoch -1", true, "done", -2, false),
|
||||
];
|
||||
|
||||
for (i, (name, released, status, offset, expect_visible)) in cases.iter().enumerate() {
|
||||
let slug = format!("case{i}");
|
||||
let event_id = seed_event(&pool, &slug).await;
|
||||
|
||||
// Get the event to a known epoch (1) either by releasing it, or — for the unreleased case —
|
||||
// by releasing and reopening, which leaves it unreleased at a non-zero epoch.
|
||||
let event_epoch_now = if *released {
|
||||
release_gallery(&pool, &slug).await.unwrap()
|
||||
} else {
|
||||
release_gallery(&pool, &slug).await.unwrap();
|
||||
open_event(&pool, &slug).await;
|
||||
event_epoch(&pool, event_id).await
|
||||
};
|
||||
|
||||
// Plant a single ZIP job row in the exact state under test. `-2` encodes "the sentinel the
|
||||
// migration stamps on retired rows", which must never equal a non-negative event epoch.
|
||||
// (Clear the rows the release armed first — `UNIQUE (event_id, type)`.)
|
||||
sqlx::query("DELETE FROM export_job WHERE event_id = $1")
|
||||
.bind(event_id)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("clear armed jobs");
|
||||
|
||||
let job_epoch = if *offset == -2 { -1 } else { event_epoch_now + offset };
|
||||
sqlx::query(
|
||||
"INSERT INTO export_job (event_id, type, status, progress_pct, epoch, file_path)
|
||||
VALUES ($1, 'zip', $2::export_status, 100, $3, 'exports/Gallery.zip')",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(*status)
|
||||
.bind(job_epoch)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("plant job row");
|
||||
|
||||
let visible = downloadable(&pool, event_id, "zip").await.is_some();
|
||||
assert_eq!(
|
||||
visible, *expect_visible,
|
||||
"export_current exactness violated for case: {name} \
|
||||
(released={released}, status={status}, job_epoch={job_epoch}, event_epoch={event_epoch_now})"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// 4. The ViewerOnly ZIP carry-forward
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// Branch A — the ZIP is `done` at the outgoing epoch: the carry-forward re-stamps it to the new
|
||||
/// epoch (rows_affected = 1), so only the HTML viewer is rebuilt and the finished ZIP stays
|
||||
/// downloadable throughout.
|
||||
///
|
||||
/// PREVENTS: rebuilding a multi-GB archive because someone deleted a comment. The ZIP holds media,
|
||||
/// not comments — a needless rebuild would 404 the photo download for minutes to change nothing
|
||||
/// inside it.
|
||||
#[sqlx::test]
|
||||
async fn viewer_only_carries_a_done_zip_forward(pool: PgPool) {
|
||||
let event_id = seed_event(&pool, "wedding").await;
|
||||
let e1 = release_gallery(&pool, "wedding").await.unwrap();
|
||||
|
||||
// Both halves finish at epoch 1 — the keepsake is live.
|
||||
for t in ["zip", "html"] {
|
||||
assert!(claim_job(&pool, event_id, t, e1).await);
|
||||
assert!(finalize_job(&pool, event_id, t, e1, &format!("exports/{t}.{e1}.zip")).await);
|
||||
}
|
||||
let zip_file = downloadable(&pool, event_id, "zip").await.expect("zip is live");
|
||||
|
||||
// ── A comment is moderated: invalidate_and_arm(Affects::ViewerOnly). ──
|
||||
let (_, _, e2) = bump_epoch(&pool, "wedding").await.unwrap();
|
||||
let carried = carry_zip_forward(&pool, event_id, e2).await;
|
||||
assert!(carried, "a `done` ZIP at epoch-1 MUST be carried forward (rows_affected == 1)");
|
||||
|
||||
// Only the viewer is re-armed…
|
||||
let mut conn = pool.acquire().await.unwrap();
|
||||
enqueue_types_at_epoch(&mut conn, event_id, e2, &["html"]).await;
|
||||
drop(conn);
|
||||
|
||||
// …and the ZIP is STILL DOWNLOADABLE, at the new epoch, pointing at the same, unrenamed file.
|
||||
let (status, epoch, _) = job_row(&pool, event_id, "zip").await.unwrap();
|
||||
assert_eq!((status.as_str(), epoch), ("done", e2), "the ZIP row rode the epoch bump");
|
||||
assert_eq!(
|
||||
downloadable(&pool, event_id, "zip").await,
|
||||
Some(zip_file),
|
||||
"the carried archive must never stop being served — same file, new epoch"
|
||||
);
|
||||
|
||||
// The viewer, meanwhile, is correctly retired and pending a rebuild.
|
||||
assert_eq!(job_row(&pool, event_id, "html").await.unwrap().0, "pending");
|
||||
assert_eq!(downloadable(&pool, event_id, "html").await, None);
|
||||
}
|
||||
|
||||
/// Branch B — THE BUG WE JUST FIXED. If the ZIP is still `pending`/`running` when the comment is
|
||||
/// moderated (which is MINUTES for a real multi-GB gallery, and deleting a comment right after
|
||||
/// release is an utterly ordinary thing to do), the carry-forward matches NOTHING
|
||||
/// (rows_affected = 0) — so the caller must NOT assume it carried, and must re-arm the ZIP too.
|
||||
///
|
||||
/// PREVENTS: the stranded ZIP. Blindly re-arming only the viewer would leave the ZIP row at the
|
||||
/// retired epoch; the in-flight worker then finishes and writes `done` at an epoch `export_current`
|
||||
/// no longer matches, nothing ever re-arms it, and `GET /export/zip` 404s FOREVER — a keepsake the
|
||||
/// couple paid for that simply never appears, short of a reboot.
|
||||
#[sqlx::test]
|
||||
async fn viewer_only_carry_forward_matches_nothing_when_zip_unfinished(pool: PgPool) {
|
||||
for zip_state in ["pending", "running"] {
|
||||
let slug = format!("wedding-{zip_state}");
|
||||
let event_id = seed_event(&pool, &slug).await;
|
||||
let e1 = release_gallery(&pool, &slug).await.unwrap();
|
||||
|
||||
// The ZIP worker is still going; only the viewer has finished.
|
||||
if zip_state == "running" {
|
||||
assert!(claim_job(&pool, event_id, "zip", e1).await);
|
||||
}
|
||||
assert!(claim_job(&pool, event_id, "html", e1).await);
|
||||
assert!(finalize_job(&pool, event_id, "html", e1, "exports/Memories.1.zip").await);
|
||||
|
||||
// ── The comment is moderated. ──
|
||||
let (_, _, e2) = bump_epoch(&pool, &slug).await.unwrap();
|
||||
let carried = carry_zip_forward(&pool, event_id, e2).await;
|
||||
|
||||
assert!(
|
||||
!carried,
|
||||
"a {zip_state} ZIP has nothing to carry forward — the UPDATE must affect 0 rows \
|
||||
(its `status = 'done'` predicate is the whole precondition)"
|
||||
);
|
||||
|
||||
// The carry-forward's OWN result decides. It didn't match ⇒ rebuild the ZIP as well.
|
||||
let types: &[&str] = if carried { &["html"] } else { &["zip", "html"] };
|
||||
let mut conn = pool.acquire().await.unwrap();
|
||||
enqueue_types_at_epoch(&mut conn, event_id, e2, types).await;
|
||||
drop(conn);
|
||||
|
||||
// THE ASSERTION THAT WOULD HAVE CAUGHT THE BUG: the ZIP must not be stranded at the dead
|
||||
// epoch. It is re-armed at the live one, so a fresh worker will actually build it.
|
||||
let (status, epoch, _) = job_row(&pool, event_id, "zip").await.unwrap();
|
||||
assert_eq!(
|
||||
(status.as_str(), epoch),
|
||||
("pending", e2),
|
||||
"the unfinished ZIP MUST be re-armed at the new epoch, not left stranded at {e1}"
|
||||
);
|
||||
|
||||
// Even if the old in-flight worker now "finishes", it is inert and cannot resurrect itself.
|
||||
assert!(!finalize_job(&pool, event_id, "zip", e1, "exports/Gallery.1.zip").await);
|
||||
assert_eq!(downloadable(&pool, event_id, "zip").await, None);
|
||||
}
|
||||
}
|
||||
|
||||
/// The re-arm upsert must never clobber the archive it just carried forward.
|
||||
///
|
||||
/// `enqueue_types_at_epoch`'s `WHERE export_job.status <> 'done' OR export_job.epoch <> EXCLUDED.epoch`
|
||||
/// is the "startup recovery must not clobber a good half" rule, expressed as the readiness predicate
|
||||
/// itself. PREVENTS: boot recovery resetting a perfectly good, downloadable ZIP back to `pending`
|
||||
/// and making the keepsake 404 while it needlessly rebuilds.
|
||||
#[sqlx::test]
|
||||
async fn enqueue_preserves_a_done_half_at_the_current_epoch(pool: PgPool) {
|
||||
let event_id = seed_event(&pool, "wedding").await;
|
||||
let e1 = release_gallery(&pool, "wedding").await.unwrap();
|
||||
|
||||
// The ZIP finished; the HTML worker was killed mid-flight (crash) and sits at `running`.
|
||||
assert!(claim_job(&pool, event_id, "zip", e1).await);
|
||||
assert!(finalize_job(&pool, event_id, "zip", e1, "exports/Gallery.1.zip").await);
|
||||
assert!(claim_job(&pool, event_id, "html", e1).await);
|
||||
|
||||
// Boot recovery re-arms both types at the SAME epoch.
|
||||
let mut conn = pool.acquire().await.unwrap();
|
||||
enqueue_types_at_epoch(&mut conn, event_id, e1, &["zip", "html"]).await;
|
||||
drop(conn);
|
||||
|
||||
// The good half survives untouched…
|
||||
let (status, epoch, file_path) = job_row(&pool, event_id, "zip").await.unwrap();
|
||||
assert_eq!((status.as_str(), epoch), ("done", e1), "a done half at the live epoch is preserved");
|
||||
assert_eq!(file_path.as_deref(), Some("exports/Gallery.1.zip"), "file_path not nulled");
|
||||
assert!(downloadable(&pool, event_id, "zip").await.is_some());
|
||||
|
||||
// …and only the missing half is re-armed.
|
||||
assert_eq!(job_row(&pool, event_id, "html").await.unwrap().0, "pending");
|
||||
}
|
||||
268
backend/tests/upload_concurrency.rs
Normal file
268
backend/tests/upload_concurrency.rs
Normal file
@@ -0,0 +1,268 @@
|
||||
//! DB-backed integration tests for the two concurrency guards in the upload commit path
|
||||
//! (`handlers/upload.rs`). Both are SQL — a `FOR SHARE` row lock and an atomic compare-and-increment
|
||||
//! — and both are load-bearing for things a user can actually lose: a wedding photo, or the disk.
|
||||
//!
|
||||
//! `#[sqlx::test]` gives each test a fresh database with the real migrations applied.
|
||||
|
||||
mod common;
|
||||
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::time::Duration;
|
||||
|
||||
use chrono::{DateTime, Utc};
|
||||
use common::*;
|
||||
use sqlx::PgPool;
|
||||
use uuid::Uuid;
|
||||
|
||||
/// SRC: `handlers/upload.rs:313-322` — the guarded quota increment, verbatim.
|
||||
/// Returns `rows_affected()`; the handler aborts the whole upload tx when this is 0.
|
||||
async fn quota_inc(
|
||||
exec: impl sqlx::PgExecutor<'_>,
|
||||
user_id: Uuid,
|
||||
size: i64,
|
||||
limit: i64,
|
||||
) -> u64 {
|
||||
sqlx::query(
|
||||
"UPDATE \"user\" SET total_upload_bytes = total_upload_bytes + $2
|
||||
WHERE id = $1 AND total_upload_bytes + $2 <= $3",
|
||||
)
|
||||
.bind(user_id)
|
||||
.bind(size)
|
||||
.bind(limit)
|
||||
.execute(exec)
|
||||
.await
|
||||
.expect("quota_inc")
|
||||
.rows_affected()
|
||||
}
|
||||
|
||||
async fn total_bytes(pool: &PgPool, user_id: Uuid) -> i64 {
|
||||
sqlx::query_scalar("SELECT total_upload_bytes FROM \"user\" WHERE id = $1")
|
||||
.bind(user_id)
|
||||
.fetch_one(pool)
|
||||
.await
|
||||
.expect("total_bytes")
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// 5. The atomic quota increment
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// Two attempts sized off ONE stale snapshot, each of which "fits" on its own, cannot both commit.
|
||||
/// The predicate re-reads `total_upload_bytes` inside the UPDATE, so the second matches 0 rows.
|
||||
///
|
||||
/// PREVENTS: one guest filling the disk. The handler's pre-flight quota check runs BEFORE the body is
|
||||
/// streamed — minutes earlier, for a 500 MB video. If the commit trusted that snapshot, a guest could
|
||||
/// start N uploads that each individually fit under the limit and land all N, blowing straight through
|
||||
/// the quota and (in a 1 GB container) taking the event down for everyone.
|
||||
#[sqlx::test]
|
||||
async fn quota_two_attempts_from_one_stale_snapshot_cannot_both_commit(pool: PgPool) {
|
||||
let event_id = seed_event(&pool, "wedding").await;
|
||||
let user_id = seed_user(&pool, event_id, "Gierige Gudrun").await;
|
||||
|
||||
const LIMIT: i64 = 100;
|
||||
const SIZE: i64 = 60;
|
||||
|
||||
// THE STALE SNAPSHOT: the pre-flight check both uploads were admitted on.
|
||||
let snapshot = total_bytes(&pool, user_id).await;
|
||||
assert_eq!(snapshot, 0);
|
||||
// Each upload, judged against that snapshot alone, fits: 0 + 60 <= 100. Twice.
|
||||
assert!(snapshot + SIZE <= LIMIT);
|
||||
|
||||
assert_eq!(quota_inc(&pool, user_id, SIZE, LIMIT).await, 1, "the first upload commits");
|
||||
assert_eq!(
|
||||
quota_inc(&pool, user_id, SIZE, LIMIT).await,
|
||||
0,
|
||||
"the second MUST affect 0 rows — it was admitted on a snapshot that is now a lie \
|
||||
(60 + 60 = 120 > 100). rows_affected() == 0 is what makes the handler abort."
|
||||
);
|
||||
|
||||
assert_eq!(total_bytes(&pool, user_id).await, SIZE, "never 120 — the quota held");
|
||||
}
|
||||
|
||||
/// The same, but genuinely CONCURRENT: two transactions that both read `total = 0`, then both try to
|
||||
/// commit 60 bytes against a 100-byte limit. The second UPDATE blocks on the first's row lock and —
|
||||
/// because every predicate is on the ROW BEING UPDATED — Postgres re-evaluates it against the
|
||||
/// post-commit row (EPQ) rather than the statement's original snapshot. It matches nothing.
|
||||
///
|
||||
/// PREVENTS: exactly the same disk-filling overrun, on the path it actually happens — two uploads
|
||||
/// in flight at once, which is the normal case at a party.
|
||||
#[sqlx::test]
|
||||
async fn quota_guard_is_atomic_under_concurrent_transactions(pool: PgPool) {
|
||||
let event_id = seed_event(&pool, "wedding").await;
|
||||
let user_id = seed_user(&pool, event_id, "Gierige Gudrun").await;
|
||||
|
||||
const LIMIT: i64 = 100;
|
||||
const SIZE: i64 = 60;
|
||||
|
||||
let mut tx1 = pool.begin().await.unwrap();
|
||||
let mut tx2 = pool.begin().await.unwrap();
|
||||
|
||||
// Both transactions read the same snapshot and both would pass a naive `total + size <= limit`
|
||||
// check done in Rust.
|
||||
for tx in [&mut tx1, &mut tx2] {
|
||||
let seen: i64 = sqlx::query_scalar("SELECT total_upload_bytes FROM \"user\" WHERE id = $1")
|
||||
.bind(user_id)
|
||||
.fetch_one(&mut **tx)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(seen, 0, "both see an empty quota");
|
||||
}
|
||||
|
||||
// tx1 takes the row lock and commits.
|
||||
assert_eq!(quota_inc(&mut *tx1, user_id, SIZE, LIMIT).await, 1);
|
||||
tx1.commit().await.unwrap();
|
||||
|
||||
// tx2's UPDATE was written against the stale snapshot but is evaluated against the row as it
|
||||
// now stands.
|
||||
assert_eq!(
|
||||
quota_inc(&mut *tx2, user_id, SIZE, LIMIT).await,
|
||||
0,
|
||||
"the loser MUST see 0 rows affected — this is the entire quota guarantee"
|
||||
);
|
||||
tx2.rollback().await.unwrap();
|
||||
|
||||
assert_eq!(total_bytes(&pool, user_id).await, SIZE);
|
||||
|
||||
// And an upload that legitimately fits in what's left still succeeds — the guard rejects
|
||||
// overruns, not everything.
|
||||
assert_eq!(quota_inc(&pool, user_id, 40, LIMIT).await, 1, "0 + 60 + 40 == 100, exactly at the limit");
|
||||
assert_eq!(total_bytes(&pool, user_id).await, LIMIT);
|
||||
assert_eq!(quota_inc(&pool, user_id, 1, LIMIT).await, 0, "and one byte more is refused");
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// 6. The `FOR SHARE` upload lock vs. the release
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// SRC: `handlers/upload.rs:297-303` — the in-transaction re-check under a row lock, verbatim.
|
||||
async fn lock_and_read_event(
|
||||
tx: &mut sqlx::PgConnection,
|
||||
event_id: Uuid,
|
||||
) -> (Option<DateTime<Utc>>, Option<DateTime<Utc>>) {
|
||||
sqlx::query_as("SELECT uploads_locked_at, export_released_at FROM event WHERE id = $1 FOR SHARE")
|
||||
.bind(event_id)
|
||||
.fetch_one(tx)
|
||||
.await
|
||||
.expect("FOR SHARE re-check")
|
||||
}
|
||||
|
||||
/// THE GUARD AGAINST SILENT, PERMANENT PHOTO LOSS.
|
||||
///
|
||||
/// An upload holding `FOR SHARE` on the event row must BLOCK the `UPDATE event SET
|
||||
/// export_released_at = NOW()` in `release_gallery` until it commits. Either the upload commits first
|
||||
/// — and the release (hence the export snapshot) is strictly ordered after it, so the keepsake
|
||||
/// CONTAINS the photo — or the release commits first and the upload observes the lock and rejects
|
||||
/// (reversibly: the client keeps the blob and resumes after a reopen).
|
||||
///
|
||||
/// PREVENTS: the lost wedding photo. Without this serialization: a guest starts a 500 MB video, the
|
||||
/// pre-flight lock check passes, the host releases the gallery, the export workers snapshot the
|
||||
/// uploads table, and THEN the upload commits. The photo appears in the live feed but is missing from
|
||||
/// the downloaded keepsake, forever — nothing ever regenerates it and nobody ever notices.
|
||||
#[sqlx::test]
|
||||
async fn for_share_upload_lock_serializes_against_release(pool: PgPool) {
|
||||
let event_id = seed_event(&pool, "wedding").await;
|
||||
let user_id = seed_user(&pool, event_id, "Fotograf Fritz").await;
|
||||
|
||||
// ── The guest's upload transaction takes the share lock. ──
|
||||
let mut upload_tx = pool.begin().await.unwrap();
|
||||
let (locked, released) = lock_and_read_event(&mut upload_tx, event_id).await;
|
||||
assert!(locked.is_none() && released.is_none(), "uploads are open, so we proceed to commit");
|
||||
|
||||
// ── Concurrently, the host hits "Galerie freigeben". ──
|
||||
let release_done = Arc::new(AtomicBool::new(false));
|
||||
let release_task = {
|
||||
let pool = pool.clone();
|
||||
let release_done = release_done.clone();
|
||||
tokio::spawn(async move {
|
||||
sqlx::query(
|
||||
"UPDATE event
|
||||
SET export_released_at = NOW(),
|
||||
uploads_locked_at = COALESCE(uploads_locked_at, NOW()),
|
||||
export_epoch = export_epoch + 1
|
||||
WHERE id = $1 AND export_released_at IS NULL",
|
||||
)
|
||||
.bind(event_id)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("release");
|
||||
release_done.store(true, Ordering::SeqCst);
|
||||
})
|
||||
};
|
||||
|
||||
// The release MUST be stuck behind our `FOR SHARE` row lock. (`FOR SHARE` conflicts with the
|
||||
// `FOR UPDATE` lock the UPDATE needs, so Postgres makes it wait — this is not a timing race,
|
||||
// it is a lock-conflict guarantee; the sleep only gives it every chance to wrongly proceed.)
|
||||
tokio::time::sleep(Duration::from_millis(750)).await;
|
||||
assert!(
|
||||
!release_done.load(Ordering::SeqCst),
|
||||
"the release MUST block while an upload holds FOR SHARE — if it can slip past, the export \
|
||||
snapshot is taken while a photo is still committing and that photo is lost forever"
|
||||
);
|
||||
|
||||
// The photo commits. It is now unambiguously part of the upload set.
|
||||
let upload_id: Uuid = sqlx::query_scalar(
|
||||
"INSERT INTO upload (event_id, user_id, original_path, mime_type, original_size_bytes)
|
||||
VALUES ($1, $2, 'originals/wedding/x.jpg', 'image/jpeg', 1234) RETURNING id",
|
||||
)
|
||||
.bind(event_id)
|
||||
.bind(user_id)
|
||||
.fetch_one(&mut *upload_tx)
|
||||
.await
|
||||
.unwrap();
|
||||
upload_tx.commit().await.unwrap();
|
||||
|
||||
// Only now can the release proceed.
|
||||
tokio::time::timeout(Duration::from_secs(5), release_task)
|
||||
.await
|
||||
.expect("the release must unblock once the upload commits")
|
||||
.unwrap();
|
||||
|
||||
// THE PAYOFF: the export snapshot — the very query the ZIP worker runs — sees the photo. Order
|
||||
// enforced by the lock: upload commit < release < snapshot.
|
||||
let snapshot: Vec<Uuid> = sqlx::query_scalar(
|
||||
"SELECT u.id 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",
|
||||
)
|
||||
.bind(event_id)
|
||||
.fetch_all(&pool)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(snapshot, vec![upload_id], "the released keepsake CONTAINS the in-flight photo");
|
||||
}
|
||||
|
||||
/// The other side of the same lock: once the release has COMMITTED, the next upload's `FOR SHARE`
|
||||
/// re-read sees `export_released_at` set and the handler rejects it with `UploadsLocked`.
|
||||
///
|
||||
/// PREVENTS: the same lost photo, on the losing side of the race — a photo committing AFTER the
|
||||
/// export snapshot would be in the live feed but missing from the keepsake. Rejecting is the correct
|
||||
/// outcome, and it is reversible: `UploadsLocked` (not Forbidden) tells the client to keep the blob
|
||||
/// and resume when the host reopens.
|
||||
#[sqlx::test]
|
||||
async fn upload_after_release_commits_sees_the_lock_and_is_rejected(pool: PgPool) {
|
||||
let event_id = seed_event(&pool, "wedding").await;
|
||||
|
||||
// Before the release, the re-check passes.
|
||||
let mut tx = pool.begin().await.unwrap();
|
||||
let (locked, released) = lock_and_read_event(&mut tx, event_id).await;
|
||||
assert!(locked.is_none() && released.is_none());
|
||||
tx.rollback().await.unwrap();
|
||||
|
||||
assert_eq!(release_gallery(&pool, "wedding").await, Some(1));
|
||||
|
||||
// After it, the identical re-check sees the release and the handler bails out.
|
||||
let mut tx = pool.begin().await.unwrap();
|
||||
let (locked, released) = lock_and_read_event(&mut tx, event_id).await;
|
||||
assert!(released.is_some(), "the FOR SHARE re-read MUST observe the committed release");
|
||||
assert!(locked.is_some(), "release locks uploads in the same statement (release ⇒ lock)");
|
||||
tx.rollback().await.unwrap();
|
||||
|
||||
// And a reopen makes it uploadable again — the rejection was reversible, not terminal.
|
||||
assert_eq!(open_event(&pool, "wedding").await, 1);
|
||||
let mut tx = pool.begin().await.unwrap();
|
||||
let (locked, released) = lock_and_read_event(&mut tx, event_id).await;
|
||||
assert!(locked.is_none() && released.is_none(), "the guest can resume their upload");
|
||||
tx.rollback().await.unwrap();
|
||||
}
|
||||
@@ -1,18 +1,50 @@
|
||||
import type { Page, Locator } from '@playwright/test';
|
||||
|
||||
/**
|
||||
* Page object for `/export` (see [frontend/src/routes/export/+page.svelte]).
|
||||
*
|
||||
* The two archive buttons are BOTH labelled exactly "Download" — the format is carried by
|
||||
* the surrounding card ("ZIP-Archiv" / "HTML-Viewer"), not by the accessible name. Earlier
|
||||
* versions of this file looked for `/zip.*herunter/i` and `/^herunterladen$/i`, which match
|
||||
* nothing on this page; any test asserting `not.toBeVisible()` on those passed no matter
|
||||
* what the page rendered. Locators here must match the real UI, so scope by card.
|
||||
*
|
||||
* "Herunterladen" IS a real label — but only inside the HTML-guide confirm modal, which is
|
||||
* a different element entirely.
|
||||
*/
|
||||
export class ExportPage {
|
||||
readonly page: Page;
|
||||
/** Empty state shown while the host has not released the export yet. */
|
||||
readonly notAvailableBanner: Locator;
|
||||
/** Any "Download" button on the page (both cards). Useful for absence assertions. */
|
||||
readonly downloadButtons: Locator;
|
||||
readonly zipDownloadButton: Locator;
|
||||
readonly htmlDownloadButton: Locator;
|
||||
/** Confirm button inside the "Hinweis zum HTML-Viewer" modal. */
|
||||
readonly htmlGuideModalContinue: Locator;
|
||||
|
||||
constructor(page: Page) {
|
||||
this.page = page;
|
||||
this.notAvailableBanner = page.locator('[data-testid="export-not-available"]');
|
||||
this.zipDownloadButton = page.getByRole('button', { name: /zip.*herunter|herunter.*zip/i }).first();
|
||||
this.htmlDownloadButton = page.getByRole('button', { name: /html.*herunter|herunter.*html/i }).first();
|
||||
this.htmlGuideModalContinue = page.getByRole('button', { name: /^herunterladen$/i });
|
||||
this.notAvailableBanner = page.getByText('Export noch nicht verfügbar');
|
||||
this.downloadButtons = page.getByRole('button', { name: 'Download', exact: true });
|
||||
this.zipDownloadButton = this.cardButton('ZIP-Archiv');
|
||||
this.htmlDownloadButton = this.cardButton('HTML-Viewer');
|
||||
this.htmlGuideModalContinue = page
|
||||
.getByRole('dialog')
|
||||
.getByRole('button', { name: /^herunterladen$/i });
|
||||
}
|
||||
|
||||
/**
|
||||
* The "Download" button inside the card whose heading is `heading`.
|
||||
*
|
||||
* Scoped to the card element (`div.rounded-xl`) rather than "any div containing the
|
||||
* heading" — the latter also matches the page wrapper, which contains BOTH cards' buttons.
|
||||
*/
|
||||
private cardButton(heading: string): Locator {
|
||||
return this.page
|
||||
.locator('div.rounded-xl')
|
||||
.filter({ has: this.page.getByRole('heading', { name: heading, exact: true }) })
|
||||
.getByRole('button', { name: 'Download', exact: true });
|
||||
}
|
||||
|
||||
async goto() {
|
||||
|
||||
@@ -23,7 +23,25 @@ export default defineConfig({
|
||||
outputDir: './test-results',
|
||||
fullyParallel: false, // Single shared backend → tests TRUNCATE between, so don't run in parallel.
|
||||
forbidOnly: !!process.env.CI,
|
||||
retries: process.env.CI ? 2 : 0,
|
||||
// NO RETRIES — not even in CI. This is deliberate and it is the opposite of the usual advice.
|
||||
//
|
||||
// The standard case for retries is "the environment is flaky, the product isn't." That argument
|
||||
// does not hold here. This backend's real bugs ARE races (the last several fixes were all export
|
||||
// concurrency), and from the outside a race is indistinguishable from a flake — so a retry
|
||||
// resolves that ambiguity, silently, in favour of "flake", every single time.
|
||||
//
|
||||
// The numbers make it concrete. A test that fails 3% of runs reports a bug roughly 1 run in 33.
|
||||
// Under `retries: 2` it fails the build only when it fails three times in a row: ~1 in 37,000.
|
||||
// We had exactly such a test, and it was reporting a REAL user-facing bug (a suggestion button
|
||||
// that committed on mousedown and destroyed itself mid-click). Retries would have buried it
|
||||
// forever, and buried it GREEN, so nobody would even have seen an amber.
|
||||
//
|
||||
// Worse, a retry does not re-run the failing conditions — it runs a CLEANER environment (the
|
||||
// interfering background worker from the previous test has since finished). So the mechanism is
|
||||
// specifically good at hiding exactly the cross-test contamination this suite is prone to.
|
||||
//
|
||||
// A flake here is a bug report. Treat it as one.
|
||||
retries: 0,
|
||||
workers: 1, // One worker. Multi-worker needs per-worker isolated DBs (Phase 2+).
|
||||
reporter: [
|
||||
['list'],
|
||||
@@ -127,7 +145,12 @@ export default defineConfig({
|
||||
{
|
||||
name: 'webkit-iphone',
|
||||
use: { ...devices['iPhone 14 Pro'] },
|
||||
grep: /@smoke/,
|
||||
// The PRIMARY user of this app is a wedding guest opening a QR link in iOS Safari. Gating
|
||||
// that on `@smoke` — which exists on exactly two specs — meant the entire iOS guarantee was
|
||||
// one happy path and one join test. Every other UA here is a secondary browser and a smoke
|
||||
// check is proportionate; WebKit is not. Give it the core journeys the guest actually walks:
|
||||
// join/recover, upload, and browse the feed.
|
||||
testMatch: ['**/__smoke/**', '**/01-auth/**', '**/02-upload/**', '**/03-feed/**'],
|
||||
},
|
||||
{
|
||||
name: 'firefox-android',
|
||||
|
||||
@@ -26,12 +26,39 @@ test.describe('Auth — /recover route', () => {
|
||||
await expect(recover.errorMessage).toContainText(/PIN ist falsch|falsch/i);
|
||||
});
|
||||
|
||||
test('non-existent name returns the "not found" error', async ({ page }) => {
|
||||
// An unknown name must be INDISTINGUISHABLE from a wrong PIN — same status, same message.
|
||||
//
|
||||
// This test used to assert the exact opposite: that an unknown name produced a distinct
|
||||
// "nicht gefunden" error. That IS the account-enumeration oracle the F4 audit fix removed
|
||||
// (recover now answers an unknown name with the same 401 "PIN ist falsch."), so reintroducing
|
||||
// the vulnerability would have made this test pass and `recover-enumeration.spec.ts` fail — two
|
||||
// tests asserting contradictory things about the same behaviour, one of them for the attacker.
|
||||
//
|
||||
// It also passed for a reason unrelated to the name: this test takes no `guest` fixture, so the
|
||||
// per-test TRUNCATE leaves no event row at all, and recover 404s with "Event nicht gefunden." —
|
||||
// which the old regex happily matched.
|
||||
test('an unknown name is indistinguishable from a wrong PIN (no enumeration oracle)', async ({
|
||||
page,
|
||||
guest,
|
||||
}) => {
|
||||
const handle = await guest('Known');
|
||||
await clearAllStorage(page);
|
||||
const recover = new RecoverPage(page);
|
||||
|
||||
await recover.goto();
|
||||
await recover.recover('Doesnt-Exist-' + Date.now(), '1234');
|
||||
await expect(recover.errorMessage).toContainText(/nicht gefunden|kein/i);
|
||||
const unknownNameError = (await recover.errorMessage.textContent())?.trim();
|
||||
|
||||
await recover.goto();
|
||||
await recover.recover('Known', handle.pin === '9999' ? '0000' : '9999');
|
||||
const wrongPinError = (await recover.errorMessage.textContent())?.trim();
|
||||
|
||||
expect(unknownNameError).toBeTruthy();
|
||||
expect(
|
||||
unknownNameError,
|
||||
'an unknown name must not be distinguishable from a wrong PIN — that is an account-enumeration oracle'
|
||||
).toBe(wrongPinError);
|
||||
expect(unknownNameError).not.toMatch(/nicht gefunden|kein benutzer/i);
|
||||
});
|
||||
|
||||
test('lockout expires and counter resets (per recover handler)', async ({ api, guest, db }) => {
|
||||
|
||||
183
e2e/specs/02-upload/quota.spec.ts
Normal file
183
e2e/specs/02-upload/quota.spec.ts
Normal file
@@ -0,0 +1,183 @@
|
||||
/**
|
||||
* Storage-quota ENFORCEMENT — the branch that runs in production and, until now, in zero tests.
|
||||
*
|
||||
* `quota_enabled` and `storage_quota_enabled` both default to TRUE in production
|
||||
* (upload.rs: `config::get_bool(.., "quota_enabled", true)`), but global-setup turns them off and
|
||||
* the per-test TRUNCATE re-asserts them as 'false' before EVERY test. So the entire quota path was
|
||||
* dead under test: the pre-check, the `QuotaExceeded` rejection, and — most importantly — the
|
||||
* atomic increment
|
||||
*
|
||||
* UPDATE "user" SET total_upload_bytes = total_upload_bytes + $2
|
||||
* WHERE id = $1 AND total_upload_bytes + $2 <= $3
|
||||
*
|
||||
* whose enforcement is `rows_affected() == 0`. That UPDATE is the ONLY thing stopping two
|
||||
* concurrent uploads from the same guest (phone + laptop, or a double-tap) from both passing the
|
||||
* stale pre-check and both committing. The five existing unit tests cover `quota_limit_bytes()` —
|
||||
* the arithmetic that computes the number — which gave the appearance of coverage while the
|
||||
* enforcement had none.
|
||||
*
|
||||
* Blast radius if it regresses: one guest silently fills the disk, and then NOBODY at the wedding
|
||||
* can upload. Those photos don't exist anywhere else.
|
||||
*
|
||||
* These tests steer the limit via `quota_tolerance` (limit = floor(free_disk * tolerance / active))
|
||||
* rather than hoping a real disk happens to be nearly full.
|
||||
*/
|
||||
import { test, expect } from '../../fixtures/test';
|
||||
import { join } from 'node:path';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { uploadPausedMidStream } from '../../helpers/upload-client';
|
||||
|
||||
const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
|
||||
const SAMPLE_BYTES = readFileSync(join(process.cwd(), 'fixtures', 'media', 'sample.jpg'));
|
||||
const SIZE = SAMPLE_BYTES.byteLength;
|
||||
|
||||
interface Quota {
|
||||
enabled: boolean;
|
||||
used_bytes: number;
|
||||
limit_bytes: number | null;
|
||||
active_uploaders: number;
|
||||
free_disk_bytes: number | null;
|
||||
}
|
||||
|
||||
const quotaOf = async (jwt: string): Promise<Quota> =>
|
||||
(await fetch(BASE + '/api/v1/me/quota', { headers: { Authorization: `Bearer ${jwt}` } })).json();
|
||||
|
||||
function upload(jwt: string, name: string) {
|
||||
const form = new FormData();
|
||||
form.append('file', new Blob([SAMPLE_BYTES], { type: 'image/jpeg' }), name);
|
||||
form.append('content_type', 'image/jpeg');
|
||||
return fetch(BASE + '/api/v1/upload', {
|
||||
method: 'POST',
|
||||
headers: { Authorization: `Bearer ${jwt}` },
|
||||
body: form,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Pick a `quota_tolerance` that makes the per-user ceiling land on `targetBytes`.
|
||||
* limit = floor(free_disk * tolerance / max(active, 1)) ⇒ tolerance = target * active / free.
|
||||
*/
|
||||
async function setLimitTo(
|
||||
api: any,
|
||||
adminToken: string,
|
||||
jwt: string,
|
||||
targetBytes: number
|
||||
): Promise<number> {
|
||||
const q = await quotaOf(jwt);
|
||||
expect(q.free_disk_bytes, 'the disk must be readable, else quota fails OPEN and proves nothing').toBeTruthy();
|
||||
const active = Math.max(q.active_uploaders, 1);
|
||||
const tolerance = (targetBytes * active) / (q.free_disk_bytes as number);
|
||||
await api.patchConfig(adminToken, { quota_tolerance: tolerance.toExponential(12) });
|
||||
|
||||
const after = await quotaOf(jwt);
|
||||
expect(after.enabled).toBe(true);
|
||||
return after.limit_bytes as number;
|
||||
}
|
||||
|
||||
test.describe('Upload — storage quota enforcement', () => {
|
||||
test.beforeEach(async ({ api, adminToken }) => {
|
||||
await api.patchConfig(adminToken, {
|
||||
quota_enabled: 'true',
|
||||
storage_quota_enabled: 'true',
|
||||
});
|
||||
});
|
||||
|
||||
test('an upload that would exceed the quota is rejected with 413 and stores nothing', async ({
|
||||
api,
|
||||
adminToken,
|
||||
guest,
|
||||
}) => {
|
||||
const g = await guest('QuotaOver');
|
||||
// Ceiling below one file: the very first upload must be refused.
|
||||
const limit = await setLimitTo(api, adminToken, g.jwt, Math.floor(SIZE / 2));
|
||||
expect(limit).toBeLessThan(SIZE);
|
||||
|
||||
const res = await upload(g.jwt, 'too-big.jpg');
|
||||
expect(res.status, 'over-quota upload must be refused').toBe(413);
|
||||
expect((await res.json()).error).toBe('quota_exceeded');
|
||||
|
||||
// Nothing was accounted, and nothing reached the feed.
|
||||
expect((await quotaOf(g.jwt)).used_bytes).toBe(0);
|
||||
const feed = await (await fetch(BASE + '/api/v1/feed', { headers: { Authorization: `Bearer ${g.jwt}` } })).json();
|
||||
expect(feed.uploads).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('an upload within the quota still succeeds (the guard is not simply "always no")', async ({
|
||||
api,
|
||||
adminToken,
|
||||
guest,
|
||||
}) => {
|
||||
const g = await guest('QuotaUnder');
|
||||
await setLimitTo(api, adminToken, g.jwt, SIZE * 4);
|
||||
|
||||
expect((await upload(g.jwt, 'fine.jpg')).status).toBe(201);
|
||||
expect((await quotaOf(g.jwt)).used_bytes).toBe(SIZE);
|
||||
});
|
||||
|
||||
test('two CONCURRENT uploads cannot BOTH slip past the quota (atomic increment)', async ({
|
||||
api,
|
||||
adminToken,
|
||||
guest,
|
||||
}) => {
|
||||
const g = await guest('QuotaRacer');
|
||||
|
||||
// Room for exactly ONE file.
|
||||
const limit = await setLimitTo(api, adminToken, g.jwt, Math.floor(SIZE * 1.5));
|
||||
expect(limit).toBeGreaterThanOrEqual(SIZE);
|
||||
expect(limit).toBeLessThan(SIZE * 2);
|
||||
|
||||
// Both uploads are held OPEN MID-BODY, and this is the entire point of the test.
|
||||
//
|
||||
// The handler loads the user row (and therefore snapshots `total_upload_bytes`) at its very
|
||||
// FIRST line — before it streams a single byte of the body. So holding both requests inside
|
||||
// the body loop guarantees both are carrying the same stale `total = 0`, and both pre-checks
|
||||
// will conclude "this fits". Only the conditional UPDATE can then stop the second.
|
||||
//
|
||||
// Firing them with a plain `Promise.all` does NOT reproduce this: the first request completes
|
||||
// and commits before the second even reads the user row, so the *pre-check* rejects the second
|
||||
// and the test passes with the atomic guard deleted. (It did. That is how this test was caught
|
||||
// being vacuous, and why it is written this way.)
|
||||
const head = SAMPLE_BYTES.subarray(0, 32);
|
||||
const tail = SAMPLE_BYTES.subarray(32);
|
||||
const a = uploadPausedMidStream(g.jwt, head, tail, { filename: 'race-a.jpg' });
|
||||
const b = uploadPausedMidStream(g.jwt, head, tail, { filename: 'race-b.jpg' });
|
||||
|
||||
// Both are now past the pre-flight checks and sitting in the body loop, each holding total = 0.
|
||||
await Promise.all([a.checkPassed, b.checkPassed]);
|
||||
|
||||
a.finish();
|
||||
b.finish();
|
||||
const [ra, rb] = await Promise.all([a.response, b.response]);
|
||||
const statuses = [ra.status, rb.status].sort();
|
||||
|
||||
expect(
|
||||
statuses,
|
||||
`exactly one upload may win the race; got ${JSON.stringify(statuses)} — if BOTH are 201 the atomic guard is gone and a single guest can overrun the disk, after which NOBODY at the party can upload`
|
||||
).toEqual([201, 413]);
|
||||
|
||||
// The invariant that actually matters: accounting never exceeds the ceiling.
|
||||
const q = await quotaOf(g.jwt);
|
||||
expect(q.used_bytes).toBe(SIZE);
|
||||
expect(q.used_bytes).toBeLessThanOrEqual(limit);
|
||||
});
|
||||
|
||||
test('GET /me/quota reports the live usage the UI shows the guest', async ({
|
||||
api,
|
||||
adminToken,
|
||||
guest,
|
||||
}) => {
|
||||
// Zero test hits before this — and it is the source of the "X von Y MB genutzt" widget.
|
||||
const g = await guest('QuotaWidget');
|
||||
await setLimitTo(api, adminToken, g.jwt, SIZE * 10);
|
||||
|
||||
const before = await quotaOf(g.jwt);
|
||||
expect(before.enabled).toBe(true);
|
||||
expect(before.used_bytes).toBe(0);
|
||||
|
||||
expect((await upload(g.jwt, 'one.jpg')).status).toBe(201);
|
||||
|
||||
const after = await quotaOf(g.jwt);
|
||||
expect(after.used_bytes).toBe(SIZE);
|
||||
expect(after.limit_bytes).toBeGreaterThan(after.used_bytes);
|
||||
});
|
||||
});
|
||||
129
e2e/specs/05-admin/authz-sweep.spec.ts
Normal file
129
e2e/specs/05-admin/authz-sweep.spec.ts
Normal file
@@ -0,0 +1,129 @@
|
||||
/**
|
||||
* Table-driven privilege sweep over EVERY host- and admin-gated route.
|
||||
*
|
||||
* Before this file, exactly THREE authorization assertions existed in the whole suite (guest →
|
||||
* /host/event/close, guest → GET /admin/config, host → GET /admin/config). Sixteen of nineteen
|
||||
* privileged routes had no test proving a guest is turned away — including:
|
||||
*
|
||||
* POST /host/users/{id}/pin-reset → reset any guest's PIN, then log in as them via /recover.
|
||||
* That is account takeover, and nothing guarded it.
|
||||
* DELETE /host/upload/{id} → delete any guest's photo (unrecoverable after the party).
|
||||
* POST /host/event/open → retire the released keepsake and reopen uploads to everyone.
|
||||
* PATCH /admin/config → only GET was 403-tested; a guest WRITING config could
|
||||
* disable quotas and rate limits outright.
|
||||
*
|
||||
* The point of a table is that adding a route to the router and forgetting to protect it should
|
||||
* make a test fail. So this asserts the WHOLE privileged surface, not a sample of it.
|
||||
*
|
||||
* Deliberately NOT covered here: the read-only-ban model (a banned user keeps read access and can
|
||||
* still download the keepsake) is BY DESIGN, and is asserted in 07-adversarial/authorization-deep.
|
||||
*/
|
||||
import { test, expect } from '../../fixtures/test';
|
||||
|
||||
const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
|
||||
|
||||
type Method = 'GET' | 'POST' | 'PATCH' | 'DELETE';
|
||||
interface Route {
|
||||
method: Method;
|
||||
path: (victimId: string) => string;
|
||||
name: string;
|
||||
body?: unknown;
|
||||
/** admin-only routes must also reject a HOST, not just a guest. */
|
||||
adminOnly?: boolean;
|
||||
}
|
||||
|
||||
const ROUTES: Route[] = [
|
||||
// ── Host surface ────────────────────────────────────────────────────────
|
||||
{ method: 'GET', path: () => '/api/v1/host/event', name: 'GET /host/event' },
|
||||
{ method: 'POST', path: () => '/api/v1/host/event/close', name: 'POST /host/event/close' },
|
||||
{ method: 'POST', path: () => '/api/v1/host/event/open', name: 'POST /host/event/open' },
|
||||
{ method: 'POST', path: () => '/api/v1/host/gallery/release', name: 'POST /host/gallery/release' },
|
||||
{ method: 'POST', path: () => '/api/v1/host/export/rebuild', name: 'POST /host/export/rebuild' },
|
||||
{ method: 'GET', path: () => '/api/v1/host/users', name: 'GET /host/users' },
|
||||
{ method: 'POST', path: (v) => `/api/v1/host/users/${v}/ban`, name: 'POST /host/users/{id}/ban', body: {} },
|
||||
{ method: 'POST', path: (v) => `/api/v1/host/users/${v}/unban`, name: 'POST /host/users/{id}/unban', body: {} },
|
||||
{ method: 'PATCH', path: (v) => `/api/v1/host/users/${v}/role`, name: 'PATCH /host/users/{id}/role', body: { role: 'host' } },
|
||||
{ method: 'POST', path: (v) => `/api/v1/host/users/${v}/pin-reset`, name: 'POST /host/users/{id}/pin-reset', body: {} },
|
||||
{ method: 'GET', path: () => '/api/v1/host/pin-reset-requests', name: 'GET /host/pin-reset-requests' },
|
||||
{ method: 'DELETE', path: (v) => `/api/v1/host/pin-reset-requests/${v}`, name: 'DELETE /host/pin-reset-requests/{id}' },
|
||||
{ method: 'DELETE', path: (v) => `/api/v1/host/upload/${v}`, name: 'DELETE /host/upload/{id}' },
|
||||
{ method: 'DELETE', path: (v) => `/api/v1/host/comment/${v}`, name: 'DELETE /host/comment/{id}' },
|
||||
|
||||
// ── Admin surface ───────────────────────────────────────────────────────
|
||||
{ method: 'GET', path: () => '/api/v1/admin/stats', name: 'GET /admin/stats', adminOnly: true },
|
||||
{ method: 'GET', path: () => '/api/v1/admin/config', name: 'GET /admin/config', adminOnly: true },
|
||||
{ method: 'PATCH', path: () => '/api/v1/admin/config', name: 'PATCH /admin/config', adminOnly: true, body: { quota_enabled: 'false' } },
|
||||
{ method: 'GET', path: () => '/api/v1/admin/export/jobs', name: 'GET /admin/export/jobs', adminOnly: true },
|
||||
];
|
||||
|
||||
async function call(route: Route, jwt: string, victimId: string): Promise<number> {
|
||||
const res = await fetch(BASE + route.path(victimId), {
|
||||
method: route.method,
|
||||
headers: {
|
||||
Authorization: `Bearer ${jwt}`,
|
||||
...(route.body !== undefined ? { 'Content-Type': 'application/json' } : {}),
|
||||
},
|
||||
...(route.body !== undefined ? { body: JSON.stringify(route.body) } : {}),
|
||||
});
|
||||
return res.status;
|
||||
}
|
||||
|
||||
test.describe('Authorization sweep — every privileged route', () => {
|
||||
for (const route of ROUTES) {
|
||||
test(`a GUEST is refused: ${route.name}`, async ({ guest }) => {
|
||||
const attacker = await guest('AuthzAttacker');
|
||||
const victim = await guest('AuthzVictim');
|
||||
|
||||
const status = await call(route, attacker.jwt, victim.userId);
|
||||
|
||||
// 403 exactly. NOT "some 4xx" — a 404 would mean the request got past the role gate and
|
||||
// merely failed to find the resource, which is precisely the vacuity this suite has been
|
||||
// bitten by: it would still pass with the RequireHost/RequireAdmin extractor deleted.
|
||||
expect(status, `${route.name} must reject a guest with 403, got ${status}`).toBe(403);
|
||||
});
|
||||
}
|
||||
|
||||
for (const route of ROUTES.filter((r) => r.adminOnly)) {
|
||||
test(`a HOST is refused: ${route.name}`, async ({ host, guest }) => {
|
||||
const victim = await guest('AuthzVictim');
|
||||
|
||||
const status = await call(route, host.jwt, victim.userId);
|
||||
|
||||
expect(status, `${route.name} is admin-only and must reject a host with 403, got ${status}`).toBe(403);
|
||||
});
|
||||
}
|
||||
|
||||
// The truncate endpoint wipes every table AND the media directory. It is test-mode only, but it
|
||||
// is reachable over HTTP and gated solely by RequireAdmin — so it deserves the same proof.
|
||||
test('a GUEST is refused: POST /admin/__truncate (wipes the whole event)', async ({ guest }) => {
|
||||
const attacker = await guest('TruncateAttacker');
|
||||
const res = await fetch(`${BASE}/api/v1/admin/__truncate`, {
|
||||
method: 'POST',
|
||||
headers: { Authorization: `Bearer ${attacker.jwt}` },
|
||||
});
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
test('a HOST is refused: POST /admin/__truncate', async ({ host }) => {
|
||||
const res = await fetch(`${BASE}/api/v1/admin/__truncate`, {
|
||||
method: 'POST',
|
||||
headers: { Authorization: `Bearer ${host.jwt}` },
|
||||
});
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
// An unauthenticated caller must not get further than an authenticated-but-unprivileged one.
|
||||
test('an ANONYMOUS caller is refused every privileged route', async () => {
|
||||
const victim = '00000000-0000-0000-0000-000000000000';
|
||||
for (const route of ROUTES) {
|
||||
const res = await fetch(BASE + route.path(victim), {
|
||||
method: route.method,
|
||||
headers: route.body !== undefined ? { 'Content-Type': 'application/json' } : {},
|
||||
...(route.body !== undefined ? { body: JSON.stringify(route.body) } : {}),
|
||||
});
|
||||
expect([401, 403], `${route.name} must reject an anonymous caller, got ${res.status}`).toContain(
|
||||
res.status
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -16,8 +16,40 @@ test.describe('Export — release and download', () => {
|
||||
await signIn(page, g);
|
||||
const exportPage = new ExportPage(page);
|
||||
await exportPage.goto();
|
||||
// The page shouldn't show download buttons before release.
|
||||
await expect(page.getByRole('button', { name: /^herunterladen$/i })).not.toBeVisible();
|
||||
|
||||
// POSITIVE anchor first. An absence-only assertion ("no download button") is green on a
|
||||
// blank page, a 404, or an unhydrated shell — i.e. it would pass even with the buttons
|
||||
// rendered, if the locator were wrong. Pin the empty state we actually expect.
|
||||
await expect(exportPage.notAvailableBanner).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
// And only THEN the absence: no download affordance exists before release. This uses the
|
||||
// real button label ("Download"), so rendering a download button here turns it red.
|
||||
await expect(exportPage.downloadButtons).toHaveCount(0);
|
||||
});
|
||||
|
||||
test('/export shows enabled download buttons once released and the jobs are done', async ({
|
||||
page,
|
||||
guest,
|
||||
signIn,
|
||||
db,
|
||||
}) => {
|
||||
const g = await guest('PostRelease');
|
||||
await db.setExportReleased(SLUG, true);
|
||||
await db.fakeExportJob(SLUG, 'zip', 'done');
|
||||
await db.fakeExportJob(SLUG, 'html', 'done');
|
||||
|
||||
await signIn(page, g);
|
||||
const exportPage = new ExportPage(page);
|
||||
await exportPage.goto();
|
||||
|
||||
// The mirror of the test above: this is what proves the "before release" locators can
|
||||
// actually SEE a download button when one exists. Without this, a typo'd locator makes
|
||||
// the pre-release test unfalsifiable.
|
||||
await expect(exportPage.notAvailableBanner).toHaveCount(0);
|
||||
await expect(exportPage.zipDownloadButton).toBeVisible({ timeout: 10_000 });
|
||||
await expect(exportPage.zipDownloadButton).toBeEnabled();
|
||||
await expect(exportPage.htmlDownloadButton).toBeVisible();
|
||||
await expect(exportPage.htmlDownloadButton).toBeEnabled();
|
||||
});
|
||||
|
||||
test('export status API reflects released flag', async ({ guest, db }) => {
|
||||
|
||||
@@ -110,7 +110,7 @@ test.describe('Adversarial — PIN brute-force', () => {
|
||||
expect(correct.status).toBe(429);
|
||||
});
|
||||
|
||||
test('parallel wrong-PIN attempts may NOT all hit lockout (race-condition finding)', async ({ guest }) => {
|
||||
test('parallel wrong-PIN attempts still lock the account (counter is not lost to the race)', async ({ guest }) => {
|
||||
const g = await guest('BruteParallel');
|
||||
const wrong = g.pin === '0000' ? '1111' : '0000';
|
||||
|
||||
@@ -124,12 +124,22 @@ test.describe('Adversarial — PIN brute-force', () => {
|
||||
)
|
||||
);
|
||||
const statuses = attempts.map((r) => r.status);
|
||||
expect(statuses.filter((s) => s === 200)).toHaveLength(0);
|
||||
// Documented behavior: lockout counter may race so not every status is 429.
|
||||
// Critical invariant: no attempt succeeded.
|
||||
if (!statuses.some((s) => s === 429)) {
|
||||
console.warn('[finding] PIN-attempt counter races under parallel requests — none hit lockout.');
|
||||
}
|
||||
expect(statuses.filter((s) => s === 200), 'a wrong PIN must never authenticate').toHaveLength(0);
|
||||
|
||||
// The in-flight requests all read `pin_locked_until` before any of them wrote it, so
|
||||
// *which* of the 10 come back 429 is genuinely racy and can't be asserted. What is NOT
|
||||
// racy — and is the property this test exists to guard — is the state left behind:
|
||||
// `failed_pin_attempts` is incremented with an atomic `SET x = x + 1 ... RETURNING`, so
|
||||
// 10 wrong PINs must push it past the 3-strike threshold and leave the account locked.
|
||||
//
|
||||
// We prove that with a follow-up request using the CORRECT pin: it must be refused with
|
||||
// 429 (locked), not 200. Delete the lockout counter and this line goes 200 → red.
|
||||
const correct = await fetch(`${BASE}/api/v1/recover`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ display_name: g.displayName, pin: g.pin }),
|
||||
});
|
||||
expect(correct.status, 'after 10 wrong PINs the account must be locked, even for the right PIN').toBe(429);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -80,27 +80,39 @@ test.describe('Adversarial — deep authorization', () => {
|
||||
expect(row?.caption).toBe('original caption');
|
||||
});
|
||||
|
||||
// These two fire at a REAL upload, and demand exactly 403.
|
||||
//
|
||||
// They used to POST to the all-zeros UUID and accept `[403, 404]`. Both handlers check
|
||||
// `user.is_banned` BEFORE they look the upload up (social.rs) — so the 404 came from the
|
||||
// *lookup*, not the guard. Delete the ban check entirely and the request still 404s on the
|
||||
// nonexistent upload, and both tests still passed. They were the only coverage of
|
||||
// ban-blocks-like and ban-blocks-comment, and they guarded nothing.
|
||||
test('banned user cannot toggle a like', async ({ api, host, guest }) => {
|
||||
const target = await guest('BannedLike');
|
||||
const uploadId = await seedUpload(host.jwt, { caption: 'likeable' });
|
||||
await api.banUser(host.jwt, target.userId);
|
||||
|
||||
const res = await fetch(`${BASE}/api/v1/upload/00000000-0000-0000-0000-000000000000/like`, {
|
||||
const res = await fetch(`${BASE}/api/v1/upload/${uploadId}/like`, {
|
||||
method: 'POST',
|
||||
headers: { Authorization: `Bearer ${target.jwt}` },
|
||||
});
|
||||
expect([403, 404]).toContain(res.status);
|
||||
expect(res.status, 'a banned user must be Forbidden, not merely miss the resource').toBe(403);
|
||||
});
|
||||
|
||||
test('banned user cannot post a comment', async ({ api, host, guest }) => {
|
||||
const target = await guest('BannedComment');
|
||||
const uploadId = await seedUpload(host.jwt, { caption: 'commentable' });
|
||||
await api.banUser(host.jwt, target.userId);
|
||||
|
||||
const res = await fetch(`${BASE}/api/v1/upload/00000000-0000-0000-0000-000000000000/comments`, {
|
||||
const res = await fetch(`${BASE}/api/v1/upload/${uploadId}/comments`, {
|
||||
method: 'POST',
|
||||
headers: { Authorization: `Bearer ${target.jwt}`, 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ body: 'should be rejected' }),
|
||||
});
|
||||
expect([403, 404]).toContain(res.status);
|
||||
expect(res.status, 'a banned user must be Forbidden, not merely miss the resource').toBe(403);
|
||||
|
||||
// ...and nothing was written.
|
||||
expect(await listComments(host.jwt, uploadId)).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('banned user can still read the feed (read-only access preserved)', async ({ api, host, guest }) => {
|
||||
@@ -130,15 +142,40 @@ test.describe('Adversarial — deep authorization', () => {
|
||||
expect(stillWorks.status).toBe(200);
|
||||
});
|
||||
|
||||
test('promote endpoint cannot be used to make oneself admin', async ({ host }) => {
|
||||
const res = await fetch(`${BASE}/api/v1/host/users/${'00000000-0000-0000-0000-000000000000'}/role`, {
|
||||
// Privilege escalation, tested against REAL targets.
|
||||
//
|
||||
// The old version PATCHed the all-zeros UUID and accepted `[400, 403, 404]`. The role whitelist
|
||||
// rejects "admin" with a 400 before the target is ever looked up — so adding `"admin"` to the
|
||||
// whitelist would make the request 404 on the nonexistent user instead, which was in the accepted
|
||||
// list. It never promoted anyone, never targeted *oneself*, and could not detect escalation.
|
||||
test('a host cannot promote a real guest to admin', async ({ api, host, guest, adminToken }) => {
|
||||
const victim = await guest('EscalationTarget');
|
||||
|
||||
const res = await fetch(`${BASE}/api/v1/host/users/${victim.userId}/role`, {
|
||||
method: 'PATCH',
|
||||
headers: { Authorization: `Bearer ${host.jwt}`, 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ role: 'admin' }),
|
||||
});
|
||||
// 400 (invalid role for host-callable endpoint) or 403/404.
|
||||
expect([400, 403, 404]).toContain(res.status);
|
||||
// Critically, NOT 200/204.
|
||||
expect([200, 204]).not.toContain(res.status);
|
||||
expect(res.status).not.toBe(204);
|
||||
expect(res.status).not.toBe(200);
|
||||
|
||||
// The assertion that actually matters: nobody got promoted.
|
||||
const users = await api.listUsers(adminToken);
|
||||
const row = users.find((u: any) => u.id === victim.userId);
|
||||
expect(row?.role, 'the guest must NOT have become an admin').not.toBe('admin');
|
||||
});
|
||||
|
||||
test('a host cannot promote THEMSELVES to admin', async ({ api, host, adminToken }) => {
|
||||
const res = await fetch(`${BASE}/api/v1/host/users/${host.userId}/role`, {
|
||||
method: 'PATCH',
|
||||
headers: { Authorization: `Bearer ${host.jwt}`, 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ role: 'admin' }),
|
||||
});
|
||||
expect(res.status).not.toBe(204);
|
||||
expect(res.status).not.toBe(200);
|
||||
|
||||
const users = await api.listUsers(adminToken);
|
||||
const me = users.find((u: any) => u.id === host.userId);
|
||||
expect(me?.role, 'the host must NOT have self-promoted to admin').not.toBe('admin');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
*/
|
||||
import { test, expect } from '../../fixtures/test';
|
||||
import { mintSseTicket } from '../../helpers/sse';
|
||||
import { seedUpload } from '../../helpers/seed';
|
||||
|
||||
const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
|
||||
|
||||
@@ -30,19 +31,50 @@ test.describe('Adversarial — small-scale abuse', () => {
|
||||
expect(statuses.some((s) => s === 201 || s === 409)).toBe(true);
|
||||
});
|
||||
|
||||
test('10 MB comment body is rejected (multipart-less endpoint)', async ({ guest }) => {
|
||||
const g = await guest('BigComment');
|
||||
const huge = 'A'.repeat(10 * 1024 * 1024);
|
||||
const res = await fetch(`${BASE}/api/v1/upload/00000000-0000-0000-0000-000000000000/comments`, {
|
||||
// The comment body cap lives in [backend/src/handlers/social.rs] `add_comment`:
|
||||
// if text_chars == 0 || text_chars > 500 → 400
|
||||
// It must be exercised against a REAL upload: the handler looks the upload up (and
|
||||
// 404s) *before* it reaches the length check, so posting to a non-existent id proves
|
||||
// nothing about the cap.
|
||||
async function postComment(jwt: string, uploadId: string, body: string) {
|
||||
return fetch(`${BASE}/api/v1/upload/${uploadId}/comments`, {
|
||||
method: 'POST',
|
||||
headers: { Authorization: `Bearer ${g.jwt}`, 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ body: huge }),
|
||||
headers: { Authorization: `Bearer ${jwt}`, 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ body }),
|
||||
});
|
||||
// 400 (length cap), 404 (no such upload), 413 (payload too large), 429 (rate-limited),
|
||||
// or 502 (Caddy rejected the body before it reached the backend) — all fine.
|
||||
expect([400, 404, 413, 429, 502]).toContain(res.status);
|
||||
// Not 200 — that would mean we accepted a 10 MB comment.
|
||||
expect(res.status).not.toBe(200);
|
||||
}
|
||||
|
||||
test('comment body over the 500-char cap is rejected with 400', async ({ guest }) => {
|
||||
const g = await guest('LongComment');
|
||||
const uploadId = await seedUpload(g.jwt);
|
||||
|
||||
const res = await postComment(g.jwt, uploadId, 'A'.repeat(501));
|
||||
expect(res.status, '501 chars must be rejected by the length cap').toBe(400);
|
||||
const json: any = await res.json().catch(() => ({}));
|
||||
expect((json.message ?? '').toLowerCase()).toMatch(/500 zeichen/);
|
||||
});
|
||||
|
||||
test('comment body exactly at the 500-char cap is accepted', async ({ guest }) => {
|
||||
const g = await guest('MaxComment');
|
||||
const uploadId = await seedUpload(g.jwt);
|
||||
|
||||
// The boundary must be inclusive — otherwise the "cap" is really 499 and the
|
||||
// rejection test above would also pass on an off-by-one implementation.
|
||||
const res = await postComment(g.jwt, uploadId, 'A'.repeat(500));
|
||||
expect(res.status, '500 chars is the documented maximum and must be accepted').toBe(201);
|
||||
});
|
||||
|
||||
test('10 MB comment body never reaches the handler (body-size limit rejects it)', async ({ guest }) => {
|
||||
const g = await guest('BigComment');
|
||||
const uploadId = await seedUpload(g.jwt);
|
||||
const huge = 'A'.repeat(10 * 1024 * 1024);
|
||||
|
||||
const res = await postComment(g.jwt, uploadId, huge);
|
||||
// This asserts ONLY what it can prove: a 10 MB JSON body is refused somewhere on the
|
||||
// path (Caddy's request-body limit → 502/413, or the backend's own body limit → 413,
|
||||
// or the 500-char cap if it does get through → 400). The upload exists, so a 404 here
|
||||
// would be a bug, and a 201 would mean we stored a 10 MB comment.
|
||||
expect([400, 413, 502]).toContain(res.status);
|
||||
});
|
||||
|
||||
test('SSE: 10 concurrent streams from one user do not crash the server', async ({ guest }) => {
|
||||
|
||||
Binary file not shown.
@@ -26,14 +26,20 @@ test.describe('Adversarial — UI render escape', () => {
|
||||
});
|
||||
|
||||
await page.goto('/account');
|
||||
await page.waitForLoadState('domcontentloaded');
|
||||
|
||||
// Render guard FIRST. `domcontentloaded` fires before Svelte hydrates, so asserting
|
||||
// the *absence* of a <b> at that point passes on a page that never rendered the name
|
||||
// at all — a false green on an XSS test. Prove the payload actually reached the DOM
|
||||
// (as escaped text) before concluding anything about how it was rendered.
|
||||
await expect(page.getByText(payload, { exact: false }).first()).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
const fired = await page.evaluate(() => (window as any).__xssFired === true);
|
||||
expect(fired).toBe(false);
|
||||
|
||||
// <b> tag inside the name should also not render as bold — Svelte escapes the entire string.
|
||||
const boldCount = await page.locator('b:has-text("BOLD")').count();
|
||||
expect(boldCount).toBe(0);
|
||||
// <b> tag inside the name should also not render as bold — Svelte escapes the entire
|
||||
// string. toHaveCount auto-retries, so this can't win by racing hydration.
|
||||
await expect(page.locator('b:has-text("BOLD")')).toHaveCount(0);
|
||||
await expect(page.locator('script:has-text("__xssFired")')).toHaveCount(0);
|
||||
});
|
||||
|
||||
test('rendering of a known SQL-injection-shaped name does not break the page', async ({ page, api }) => {
|
||||
|
||||
@@ -16,15 +16,29 @@ import { seedUpload, seedComment } from '../../helpers/seed';
|
||||
|
||||
const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
|
||||
|
||||
/**
|
||||
* Every payload sets `window.__x = 1` if it executes. The marker is deliberately terse:
|
||||
* the join handler caps display names at 50 chars, and a payload that trips that cap is
|
||||
* rejected at the API — which means it is NEVER stored and NEVER rendered, so the test
|
||||
* that "nothing executed" proves nothing at all. Each payload below is < 50 chars, so it
|
||||
* survives the join and actually reaches the render sink under test.
|
||||
*/
|
||||
const XSS_PAYLOADS = [
|
||||
`<script>window.__xssFired=true</script>`,
|
||||
`<img src=x onerror="window.__xssFired=true">`,
|
||||
`"><svg onload="window.__xssFired=true">`,
|
||||
`<iframe src="javascript:window.parent.__xssFired=true"></iframe>`,
|
||||
`javascript:window.__xssFired=true`,
|
||||
`<a href="javascript:window.__xssFired=true">click</a>`,
|
||||
`<script>window.__x=1</script>`, // 29
|
||||
`<img src=x onerror="window.__x=1">`, // 34
|
||||
`"><svg onload="window.__x=1">`, // 29
|
||||
`<iframe src="javascript:parent.__x=1"></iframe>`, // 47
|
||||
`javascript:window.__x=1`, // 23
|
||||
`<a href="javascript:window.__x=1">c</a>`, // 39
|
||||
];
|
||||
|
||||
// Guard the invariant the payloads depend on: if the display-name cap ever changes, or a
|
||||
// payload is edited past it, we want a loud failure here rather than six silent no-ops.
|
||||
const NAME_MAX = 50;
|
||||
for (const p of XSS_PAYLOADS) {
|
||||
if (p.length > NAME_MAX) throw new Error(`XSS payload exceeds the ${NAME_MAX}-char display-name cap and would never be stored: ${p}`);
|
||||
}
|
||||
|
||||
const SQLI_PAYLOADS = [
|
||||
`'; DROP TABLE "user"; --`,
|
||||
`' OR 1=1 --`,
|
||||
@@ -35,19 +49,10 @@ const SQLI_PAYLOADS = [
|
||||
test.describe('Adversarial — input injection (display name)', () => {
|
||||
for (const payload of XSS_PAYLOADS) {
|
||||
test(`name with XSS payload ${JSON.stringify(payload).slice(0, 40)} never executes`, async ({ api, page }) => {
|
||||
// Payloads > 50 chars are rejected by the join handler — that's a valid defense.
|
||||
// Only if the API accepts the payload do we proceed to assert it never executes
|
||||
// when rendered.
|
||||
let res;
|
||||
try {
|
||||
res = await api.join(payload);
|
||||
} catch (e: any) {
|
||||
if (/→ 400/.test(e.message ?? '')) {
|
||||
// Defended at the API. No need to render.
|
||||
return;
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
// No try/catch escape hatch: every payload is short enough to be accepted, so a
|
||||
// rejection here is a real failure (the payload would never be rendered, and the
|
||||
// "nothing executed" assertions below would be vacuous).
|
||||
const res = await api.join(payload);
|
||||
expect(res.jwt).toBeTruthy();
|
||||
|
||||
// Render the name in the account page by signing in.
|
||||
@@ -74,12 +79,12 @@ test.describe('Adversarial — input injection (display name)', () => {
|
||||
// so a "nothing fired" pass can't be because the name was never rendered.
|
||||
await expect(page.getByText(payload, { exact: false }).first()).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
const fired = await page.evaluate(() => (window as any).__xssFired === true);
|
||||
expect(fired, 'window.__xssFired should never be set').toBe(false);
|
||||
const fired = await page.evaluate(() => (window as any).__x === 1);
|
||||
expect(fired, 'window.__x should never be set').toBe(false);
|
||||
expect(dialogs, 'no dialogs should appear').toHaveLength(0);
|
||||
|
||||
// Inline script tag in the displayed name should be rendered as text, not parsed.
|
||||
const scriptCount = await page.locator('script:has-text("window.__xssFired")').count();
|
||||
const scriptCount = await page.locator('script:has-text("window.__x")').count();
|
||||
expect(scriptCount, 'no executable script tags rendered from name').toBe(0);
|
||||
});
|
||||
}
|
||||
@@ -101,11 +106,11 @@ test.describe('Adversarial — stored XSS (caption)', () => {
|
||||
// Wait for the caption text to land in the DOM (escaped, as literal text).
|
||||
await expect(page.getByText('CAPMARK', { exact: false }).first()).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
expect(await page.evaluate(() => (window as any).__xssFired === true), 'caption XSS must not fire').toBe(false);
|
||||
expect(await page.evaluate(() => (window as any).__x === 1), 'caption XSS must not fire').toBe(false);
|
||||
expect(dialogs, 'no dialogs from a caption').toHaveLength(0);
|
||||
// The payload must be inert text, not a live element / script.
|
||||
expect(await page.locator('img[onerror]').count(), 'no live onerror img from caption').toBe(0);
|
||||
expect(await page.locator('script:has-text("__xssFired")').count(), 'no executable script from caption').toBe(0);
|
||||
expect(await page.locator('script:has-text("window.__x")').count(), 'no executable script from caption').toBe(0);
|
||||
});
|
||||
}
|
||||
});
|
||||
@@ -114,8 +119,8 @@ test.describe('Adversarial — stored XSS (comment)', () => {
|
||||
// The two payloads that actually execute on render (script injection via innerHTML
|
||||
// does not) — enough to prove the comment body is escaped without a slow 6× lightbox loop.
|
||||
const COMMENT_PAYLOADS = [
|
||||
`<img src=x onerror="window.__xssFired=true">`,
|
||||
`"><svg onload="window.__xssFired=true">`,
|
||||
`<img src=x onerror="window.__x=1">`,
|
||||
`"><svg onload="window.__x=1">`,
|
||||
];
|
||||
for (const payload of COMMENT_PAYLOADS) {
|
||||
test(`comment with XSS payload ${JSON.stringify(payload).slice(0, 40)} renders inert`, async ({ guest, page, signIn }) => {
|
||||
@@ -139,7 +144,7 @@ test.describe('Adversarial — stored XSS (comment)', () => {
|
||||
// Wait until the comment (marker) has rendered.
|
||||
await expect(lightbox.getByText('CMTMARK', { exact: false })).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
expect(await page.evaluate(() => (window as any).__xssFired === true), 'comment XSS must not fire').toBe(false);
|
||||
expect(await page.evaluate(() => (window as any).__x === 1), 'comment XSS must not fire').toBe(false);
|
||||
expect(dialogs, 'no dialogs from a comment').toHaveLength(0);
|
||||
expect(await page.locator('img[onerror]').count(), 'no live onerror img from comment').toBe(0);
|
||||
});
|
||||
|
||||
@@ -421,6 +421,103 @@ test.describe('Flow re-review — reopen→re-release export integrity + stale-k
|
||||
expect(listing.some((n) => n.includes(keep)), 'the kept photo survives').toBe(true);
|
||||
});
|
||||
|
||||
test('BANNING a guest after release removes their photos from the keepsake', async ({
|
||||
api,
|
||||
host,
|
||||
guest,
|
||||
}) => {
|
||||
// `ban_user` calls `invalidate_and_arm(Affects::Both)` — but every ban in the suite ran against
|
||||
// an UNRELEASED event, where `invalidate_and_arm` returns early. So this regeneration path was
|
||||
// dead code under test.
|
||||
//
|
||||
// The stakes are the whole point of a ban: the host bans someone for posting something abusive,
|
||||
// and if the keepsake doesn't rebuild, that content stays in the archive every guest downloads,
|
||||
// forever. The export query already filters `is_banned` — so the pipeline AGREES the content
|
||||
// shouldn't be there; only the regeneration was missing.
|
||||
const offender = await guest('Offender');
|
||||
const innocent = await guest('Innocent');
|
||||
const badPhoto = await seedUpload(offender.jwt, { caption: 'abusive' });
|
||||
const goodPhoto = await seedUpload(innocent.jwt, { caption: 'lovely' });
|
||||
|
||||
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
||||
await waitExportDone(host.jwt);
|
||||
expect(await downloadZipEntries(host.jwt)).toHaveLength(2);
|
||||
|
||||
await api.banUser(host.jwt, offender.userId);
|
||||
|
||||
await waitExportDone(host.jwt);
|
||||
const listing = await downloadZipEntries(host.jwt);
|
||||
expect(
|
||||
listing.some((n) => n.includes(badPhoto)),
|
||||
"a banned guest's photo must not survive in the keepsake everyone downloads"
|
||||
).toBe(false);
|
||||
expect(listing.some((n) => n.includes(goodPhoto)), 'everyone else keeps their photos').toBe(true);
|
||||
expect(listing).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('UNBANNING a guest after release restores their photos to the keepsake', async ({
|
||||
api,
|
||||
host,
|
||||
guest,
|
||||
}) => {
|
||||
// The mirror image, and the one that LOSES data. A host bans someone by mistake (or bans, then
|
||||
// reconsiders) — unban must put their photos back into the archive. If the regeneration is
|
||||
// missing, that guest's photos are absent from the keepsake everyone keeps, and after the
|
||||
// wedding they are gone: the export is the only copy anyone takes home.
|
||||
const g = await guest('Forgiven');
|
||||
const photo = await seedUpload(g.jwt, { caption: 'restore me' });
|
||||
await seedUpload(host.jwt, { caption: 'host photo' });
|
||||
|
||||
await api.banUser(host.jwt, g.userId);
|
||||
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
||||
await waitExportDone(host.jwt);
|
||||
// Banned at release time → their photo is correctly absent.
|
||||
expect(await downloadZipEntries(host.jwt)).toHaveLength(1);
|
||||
|
||||
const unban = await post(`/api/v1/host/users/${g.userId}/unban`, host.jwt);
|
||||
expect(unban.status).toBe(204);
|
||||
|
||||
await waitExportDone(host.jwt);
|
||||
const listing = await downloadZipEntries(host.jwt);
|
||||
expect(
|
||||
listing.some((n) => n.includes(photo)),
|
||||
'an unbanned guest must get their photos back in the keepsake — it is the only copy anyone keeps'
|
||||
).toBe(true);
|
||||
expect(listing).toHaveLength(2);
|
||||
});
|
||||
|
||||
test('a HOST comment takedown after release keeps the keepsake downloadable', async ({
|
||||
host,
|
||||
guest,
|
||||
}) => {
|
||||
// `DELETE /host/comment/{id}` had ZERO test hits of any kind, and it is one of the two callers
|
||||
// of the ViewerOnly carry-forward. A comment doesn't live in the ZIP (which holds media), so the
|
||||
// ZIP must be CARRIED FORWARD, not rebuilt — but it must still be downloadable afterwards, and
|
||||
// the viewer must lose the comment.
|
||||
const g = await guest('Rude');
|
||||
const uploadId = await seedUpload(host.jwt, { caption: 'pic' });
|
||||
const cRes = await fetch(BASE + `/api/v1/upload/${uploadId}/comments`, {
|
||||
method: 'POST',
|
||||
headers: { Authorization: `Bearer ${g.jwt}`, 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ body: 'unfreundlich' }),
|
||||
});
|
||||
const commentId = (await cRes.json()).id;
|
||||
|
||||
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
||||
await waitExportDone(host.jwt);
|
||||
|
||||
const del = await fetch(BASE + `/api/v1/host/comment/${commentId}`, {
|
||||
method: 'DELETE',
|
||||
headers: { Authorization: `Bearer ${host.jwt}` },
|
||||
});
|
||||
expect(del.status).toBe(204);
|
||||
|
||||
// The keepsake must still be downloadable, with its media intact.
|
||||
await waitExportDone(host.jwt);
|
||||
expect(await downloadZipEntries(host.jwt)).toHaveLength(1);
|
||||
expect(await zipJobEpoch()).toBe(await eventEpoch());
|
||||
});
|
||||
|
||||
test('a comment deleted while the ZIP is still BUILDING does not strand the ZIP', async ({
|
||||
host,
|
||||
guest,
|
||||
|
||||
@@ -9,12 +9,36 @@ describe('avatarPalette', () => {
|
||||
});
|
||||
|
||||
it('is deterministic for the same name', () => {
|
||||
// Same input, repeated calls AND a separately-constructed equal string — the palette
|
||||
// must be a pure function of the name's characters, not of identity or call order.
|
||||
expect(avatarPalette('Alice')).toBe(avatarPalette('Alice'));
|
||||
expect(avatarPalette('Alice')).toBe(avatarPalette('Ali' + 'ce'));
|
||||
expect(avatarPalette('Zoë Müller')).toBe(avatarPalette('Zoë Müller'));
|
||||
});
|
||||
|
||||
it('returns a real palette entry (not neutral) for a non-empty name', () => {
|
||||
expect(avatarPalette('Bob')).toMatch(/bg-(blue|purple|green|amber|rose|teal)-100/);
|
||||
});
|
||||
|
||||
it('maps different names to different palette entries', () => {
|
||||
// Without this, `name => name ? PALETTE[0] : NEUTRAL` (i.e. the hash loop deleted)
|
||||
// passes every other test in this file — the palette would be a constant and every
|
||||
// avatar in the app would render the same colour.
|
||||
expect(avatarPalette('Alice')).not.toBe(avatarPalette('Bob'));
|
||||
});
|
||||
|
||||
it('spreads names across the whole palette, not just one bucket', () => {
|
||||
const names = [
|
||||
'Alice', 'Bob', 'Carol', 'Dave', 'Erin', 'Frank', 'Grace', 'Heidi',
|
||||
'Ivan', 'Judy', 'Mallory', 'Niaj', 'Olivia', 'Peggy', 'Rupert', 'Sybil',
|
||||
'Trent', 'Victor', 'Walter', 'Xena'
|
||||
];
|
||||
const distinct = new Set(names.map((n) => avatarPalette(n)));
|
||||
// 6 colours in the palette; 20 names must land on more than a couple of them. This
|
||||
// catches a hash that collapses (e.g. always returns index 0, or ignores all but the
|
||||
// first character in a way that clusters).
|
||||
expect(distinct.size).toBeGreaterThanOrEqual(4);
|
||||
});
|
||||
});
|
||||
|
||||
describe('initials', () => {
|
||||
|
||||
@@ -214,6 +214,21 @@
|
||||
onSseEvent('new-upload', (data) => {
|
||||
try {
|
||||
const upload: FeedUpload = JSON.parse(data);
|
||||
// GRID view must NOT prepend live. Its rows are POSITIONAL windows
|
||||
// (`uploads.slice(i * COLS, …)` in VirtualFeed), so inserting at the head shifts
|
||||
// every tile by one slot: each row's keyed `{#each}` then sees a different set of
|
||||
// ids and Svelte DESTROYS AND RECREATES the tile nodes. Two consequences at a
|
||||
// party, where photos arrive continuously — a tap in flight is swallowed when its
|
||||
// node is torn out, and the photo under the user's finger silently becomes a
|
||||
// DIFFERENT photo, so they like or open one they never chose.
|
||||
//
|
||||
// The "neue Beiträge" pill already exists for exactly this: buffer, and let the
|
||||
// user pull the new photos in when they are not mid-tap. List view is keyed by id
|
||||
// at the top level and anchored, so its nodes survive a prepend — it stays live.
|
||||
if (viewMode === 'grid') {
|
||||
feedStale = true;
|
||||
return;
|
||||
}
|
||||
uploads = [upload, ...uploads];
|
||||
} catch { /* ignore */ }
|
||||
}),
|
||||
|
||||
@@ -615,39 +615,56 @@
|
||||
{:else if exportReady}
|
||||
<p class="flex items-center justify-between gap-2">
|
||||
<span class="font-medium text-green-700 dark:text-green-300">Keepsake ist bereit.</span>
|
||||
<span class="flex items-center gap-3">
|
||||
<!-- Rebuilding a READY keepsake is disruptive: guests who tap Download during the
|
||||
rebuild get nothing until it finishes. Worth a confirm, unlike the failed case. -->
|
||||
<button
|
||||
onclick={() => (confirmAction = {
|
||||
title: 'Keepsake neu erstellen?',
|
||||
message:
|
||||
'Das Keepsake wird aus dem aktuellen Stand der Galerie neu erzeugt. ' +
|
||||
'Während der Erstellung können Gäste es nicht herunterladen.',
|
||||
confirmLabel: 'Neu erstellen',
|
||||
tone: 'danger',
|
||||
run: rebuildExport
|
||||
})}
|
||||
disabled={rebuilding}
|
||||
data-testid="export-rebuild"
|
||||
class="font-medium text-gray-500 underline disabled:opacity-50 dark:text-gray-400"
|
||||
>
|
||||
Neu erstellen
|
||||
</button>
|
||||
<a href="/export" class="font-medium text-blue-600 underline dark:text-blue-400">Zum Download</a>
|
||||
</span>
|
||||
<a href="/export" class="font-medium text-blue-600 underline dark:text-blue-400">Zum Download</a>
|
||||
</p>
|
||||
{:else}
|
||||
<p class="text-red-700 dark:text-red-300">Keepsake-Erstellung fehlgeschlagen.</p>
|
||||
<button
|
||||
onclick={rebuildExport}
|
||||
disabled={rebuilding}
|
||||
data-testid="export-rebuild"
|
||||
class="mt-2 rounded-lg bg-red-600 px-3 py-1.5 text-xs font-medium text-white transition hover:bg-red-700 disabled:opacity-50 dark:bg-red-500 dark:hover:bg-red-400"
|
||||
>
|
||||
{rebuilding ? 'Wird gestartet…' : 'Erneut versuchen'}
|
||||
</button>
|
||||
{/if}
|
||||
|
||||
<!-- ONE button, mounted in every state — deliberately OUTSIDE the branches above.
|
||||
Those branches are driven by SSE (`export-progress` / `export-available`), and
|
||||
`rebuildExport` itself makes the backend broadcast `export-progress` at 0%
|
||||
immediately. If the button lived inside a branch, activating it would flip
|
||||
`exportGenerating` and UNMOUNT THE BUTTON BEING PRESSED — and a worker tick
|
||||
landing between mousedown and mouseup would do the same unprompted. Chromium
|
||||
fires no `click` when the element dies mid-sequence, so the host would tap
|
||||
"Erneut versuchen" and nothing at all would happen, on the one screen whose
|
||||
entire purpose is recovering a broken keepsake. (Same class of bug as the feed
|
||||
autocomplete: never let a handler destroy the node that is being clicked.)
|
||||
|
||||
So the button's EXISTENCE is invariant; only its label and `disabled` change. -->
|
||||
<button
|
||||
onclick={() => {
|
||||
// Rebuilding a READY keepsake is disruptive — guests who tap Download during
|
||||
// the rebuild get nothing until it finishes — so it gets a confirm. A FAILED
|
||||
// keepsake has nothing to lose, so it retries immediately.
|
||||
if (exportReady) {
|
||||
confirmAction = {
|
||||
title: 'Keepsake neu erstellen?',
|
||||
message:
|
||||
'Das Keepsake wird aus dem aktuellen Stand der Galerie neu erzeugt. ' +
|
||||
'Während der Erstellung können Gäste es nicht herunterladen.',
|
||||
confirmLabel: 'Neu erstellen',
|
||||
tone: 'danger',
|
||||
run: rebuildExport
|
||||
};
|
||||
} else {
|
||||
void rebuildExport();
|
||||
}
|
||||
}}
|
||||
disabled={rebuilding || exportGenerating}
|
||||
data-testid="export-rebuild"
|
||||
class="mt-2 rounded-lg px-3 py-1.5 text-xs font-medium transition disabled:opacity-50
|
||||
{exportReady
|
||||
? 'text-gray-500 underline dark:text-gray-400'
|
||||
: 'bg-red-600 text-white hover:bg-red-700 dark:bg-red-500 dark:hover:bg-red-400'}"
|
||||
>
|
||||
{rebuilding
|
||||
? 'Wird gestartet…'
|
||||
: exportReady
|
||||
? 'Neu erstellen'
|
||||
: 'Erneut versuchen'}
|
||||
</button>
|
||||
</div>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user