fix(review-2): medium/low + docs — config validation, hygiene, doc sync
- compression_concurrency wired to boot config (state.rs) and removed as a dead admin control. - patch_config gains per-key integer/range validation so numerically-valid-but- nonsensical values (-1/NaN/3.5) are rejected instead of silently reverting to default at read time. - clearAuth now also clears the display name (shared-device residual). - e2e/Caddyfile.test mirrors prod security headers + the SSE encode-exclusion so the test stack is representative and can catch header regressions. - .gitignore: ignore root-level Playwright artifacts (test-results/, report). - docs: USER_JOURNEYS §10 and SECURITY-BACKLOG updated to match the implemented read-only-ban behavior and the export-path fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
3
.gitignore
vendored
3
.gitignore
vendored
@@ -22,6 +22,9 @@ e2e/playwright-report/
|
||||
e2e/test-results/
|
||||
e2e/.cache/
|
||||
e2e/.env.test
|
||||
# Playwright artifacts when run from the repo root instead of e2e/
|
||||
/test-results/
|
||||
/playwright-report/
|
||||
|
||||
# OS
|
||||
.DS_Store
|
||||
|
||||
@@ -121,15 +121,18 @@ pub async fn patch_config(
|
||||
// Numeric keys validated as f64; boolean keys validated as truthy strings; the
|
||||
// privacy note is free text. Splitting these explicitly is verbose but makes the
|
||||
// failure mode for typos obvious (`Unbekannter Schlüssel: ...`).
|
||||
const NUMERIC_KEYS: &[&str] = &[
|
||||
"max_image_size_mb",
|
||||
"max_video_size_mb",
|
||||
"upload_rate_per_hour",
|
||||
"feed_rate_per_min",
|
||||
"export_rate_per_day",
|
||||
"quota_tolerance",
|
||||
"estimated_guest_count",
|
||||
"compression_concurrency",
|
||||
// (key, integer_only, min, max). Ranges reject values that `parse::<f64>` would
|
||||
// accept but that silently revert to the hardcoded default at read time
|
||||
// (get_usize/get_i64 can't parse negatives/NaN/fractionals). `compression_concurrency`
|
||||
// is intentionally absent — it's read once at boot, so a live edit was a no-op.
|
||||
const NUMERIC_SPECS: &[(&str, bool, f64, f64)] = &[
|
||||
("max_image_size_mb", true, 1.0, 1024.0),
|
||||
("max_video_size_mb", true, 1.0, 10240.0),
|
||||
("upload_rate_per_hour", true, 1.0, 100_000.0),
|
||||
("feed_rate_per_min", true, 1.0, 100_000.0),
|
||||
("export_rate_per_day", true, 1.0, 100_000.0),
|
||||
("quota_tolerance", false, 0.0, 1.0),
|
||||
("estimated_guest_count", true, 1.0, 1_000_000.0),
|
||||
];
|
||||
const BOOL_KEYS: &[&str] = &[
|
||||
"rate_limits_enabled",
|
||||
@@ -150,10 +153,26 @@ pub async fn patch_config(
|
||||
// update behind — validation must fully precede any write.
|
||||
for (key, value) in &body {
|
||||
let key_str = key.as_str();
|
||||
if NUMERIC_KEYS.contains(&key_str) {
|
||||
if value.parse::<f64>().is_err() {
|
||||
if let Some(&(_, integer_only, min, max)) =
|
||||
NUMERIC_SPECS.iter().find(|(k, ..)| *k == key_str)
|
||||
{
|
||||
let n = value.trim().parse::<f64>().ok().filter(|n| n.is_finite());
|
||||
let n = match n {
|
||||
Some(n) => n,
|
||||
None => {
|
||||
return Err(AppError::BadRequest(format!(
|
||||
"Ungültiger Wert für {key}: muss eine Zahl sein."
|
||||
)))
|
||||
}
|
||||
};
|
||||
if integer_only && n.fract() != 0.0 {
|
||||
return Err(AppError::BadRequest(format!(
|
||||
"Ungültiger Wert für {key}: muss eine Zahl sein."
|
||||
"Ungültiger Wert für {key}: muss eine ganze Zahl sein."
|
||||
)));
|
||||
}
|
||||
if n < min || n > max {
|
||||
return Err(AppError::BadRequest(format!(
|
||||
"Wert für {key} liegt außerhalb des zulässigen Bereichs ({min}–{max})."
|
||||
)));
|
||||
}
|
||||
} else if BOOL_KEYS.contains(&key_str) {
|
||||
@@ -282,7 +301,7 @@ pub async fn download_zip(
|
||||
));
|
||||
}
|
||||
|
||||
let path = state.config.media_path.join("exports").join("Gallery.zip");
|
||||
let path = state.config.export_path.join("Gallery.zip");
|
||||
if !path.exists() {
|
||||
return Err(AppError::NotFound("Exportdatei nicht gefunden.".into()));
|
||||
}
|
||||
@@ -308,7 +327,7 @@ pub async fn download_html(
|
||||
));
|
||||
}
|
||||
|
||||
let path = state.config.media_path.join("exports").join("Memories.zip");
|
||||
let path = state.config.export_path.join("Memories.zip");
|
||||
if !path.exists() {
|
||||
return Err(AppError::NotFound("Exportdatei nicht gefunden.".into()));
|
||||
}
|
||||
|
||||
@@ -36,8 +36,12 @@ pub struct AppState {
|
||||
impl AppState {
|
||||
pub fn new(pool: PgPool, config: AppConfig) -> Self {
|
||||
let (sse_tx, _) = broadcast::channel(256);
|
||||
let compression =
|
||||
CompressionWorker::new(pool.clone(), config.media_path.clone(), 2, sse_tx.clone());
|
||||
let compression = CompressionWorker::new(
|
||||
pool.clone(),
|
||||
config.media_path.clone(),
|
||||
config.compression_concurrency,
|
||||
sse_tx.clone(),
|
||||
);
|
||||
Self {
|
||||
pool,
|
||||
config,
|
||||
|
||||
@@ -64,8 +64,11 @@ decompression-bomb cap (`image::Limits` 12000×12000 / 256 MiB) lives in
|
||||
(not just the exact dev sentinel) and enforces `len ≥ 32` unconditionally in prod.
|
||||
- **Unspoofable client IP in the rate limiter** — `client_ip` now takes the right-most
|
||||
`X-Forwarded-For` entry (the hop Caddy appends), so a client-supplied left-most value is ignored.
|
||||
- **Live role/ban re-check** — the auth extractor re-reads the user row and trusts the DB role and
|
||||
`is_banned` flag rather than the JWT claim, revoking demoted/banned sessions immediately.
|
||||
- **Live role/ban re-check** — the auth extractor re-reads the user row and trusts the DB `role`
|
||||
and `is_banned` flag rather than the JWT claim, so a demote/ban takes effect immediately (no
|
||||
30-day token window). Bans are enforced on write handlers + the host/admin extractors, preserving
|
||||
the documented read-only access for banned guests (USER_JOURNEYS §10); demoted hosts lose host
|
||||
powers at once.
|
||||
- **Container healthchecks** — `app` and `frontend` now have healthchecks and Caddy waits on
|
||||
`service_healthy`.
|
||||
- **Event-scoped `ban_user`** — the ban UPDATE is now scoped by `event_id` like its siblings.
|
||||
|
||||
@@ -147,12 +147,18 @@ the Host can clean up later).
|
||||
|
||||
## 10. Banned-guest experience
|
||||
|
||||
1. The banned user's next authenticated request returns HTTP 403 with a clear message
|
||||
("Du bist gesperrt.").
|
||||
2. They can still browse the read-only feed (and download the export once it's released).
|
||||
3. They cannot upload, like, or comment.
|
||||
4. If `hide_uploads` was set on the ban, their existing uploads are filtered out of the
|
||||
feed for everyone (the `v_feed` view already enforces this).
|
||||
1. The ban takes effect immediately on the banned user's existing session — the auth
|
||||
layer re-reads the live `is_banned` flag on every request rather than trusting the
|
||||
JWT, so there's no 30-day token-lifetime window.
|
||||
2. Their next authenticated *write* request returns HTTP 403 with a clear message
|
||||
("Du bist gesperrt."). Ban enforcement lives on the write handlers and the
|
||||
host/admin extractors, not the base auth extractor.
|
||||
3. They can still browse the read-only feed (and download the export once it's released).
|
||||
4. They cannot upload, like, or comment; a banned host also loses all host/admin
|
||||
actions (including unbanning themselves).
|
||||
5. If `hide_uploads` was set on the ban, their existing uploads are filtered out of the
|
||||
feed for everyone (`v_feed` enforces this), and a live `user-hidden` SSE event evicts
|
||||
their cards from every open feed + the diashow without a reload.
|
||||
|
||||
## 11. Admin — instance configuration
|
||||
|
||||
|
||||
@@ -4,7 +4,17 @@
|
||||
# of HTTPS/Let's Encrypt.
|
||||
|
||||
:3101 {
|
||||
encode zstd gzip
|
||||
# Mirror prod: exclude the SSE stream from compression so buffering doesn't
|
||||
# delay real-time events (and so the test stack exercises the real behavior).
|
||||
@compressible not path /api/v1/stream
|
||||
encode @compressible zstd gzip
|
||||
|
||||
# Mirror prod's security headers (minus HSTS, which is HTTPS-only).
|
||||
header {
|
||||
X-Content-Type-Options "nosniff"
|
||||
X-Frame-Options "DENY"
|
||||
Referrer-Policy "strict-origin-when-cross-origin"
|
||||
}
|
||||
|
||||
reverse_proxy /api/* app:3000
|
||||
reverse_proxy /media/* app:3000
|
||||
|
||||
@@ -3,11 +3,26 @@
|
||||
* flow. Centralising the API contract (routes, field names, expected statuses)
|
||||
* means an API change is a one-file edit, not a 7-file hunt.
|
||||
*/
|
||||
import { uploadRaw, JPEG_MAGIC } from './upload-client';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { uploadRaw } from './upload-client';
|
||||
import { db } from '../fixtures/db';
|
||||
|
||||
const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
|
||||
|
||||
// A real, decodable JPEG. Magic-bytes-only fakes now get auto-cleaned by the
|
||||
// backend when compression fails (a failed transcode refunds quota + deletes the
|
||||
// row), which would delete the seeded upload out from under the test. Read lazily
|
||||
// at call time (cwd is the e2e dir at runtime, like the gallery-path spec) so test
|
||||
// collection can't trip on a module-load read.
|
||||
let sampleJpg: Buffer | null = null;
|
||||
function sampleImage(): Buffer {
|
||||
if (!sampleJpg) {
|
||||
sampleJpg = readFileSync(join(process.cwd(), 'fixtures', 'media', 'sample.jpg'));
|
||||
}
|
||||
return sampleJpg;
|
||||
}
|
||||
|
||||
export type SeedUploadOptions = {
|
||||
caption?: string;
|
||||
/** Mark compression done so the card is fully rendered in the feed. Default true. */
|
||||
@@ -16,9 +31,7 @@ export type SeedUploadOptions = {
|
||||
|
||||
/** Seed a real, accepted upload owned by `jwt` and return its id. */
|
||||
export async function seedUpload(jwt: string, opts: SeedUploadOptions = {}): Promise<string> {
|
||||
const body = new Uint8Array(1024);
|
||||
body.set(JPEG_MAGIC, 0);
|
||||
const res = await uploadRaw(jwt, body, {
|
||||
const res = await uploadRaw(jwt, sampleImage(), {
|
||||
filename: 'a.jpg',
|
||||
contentType: 'image/jpeg',
|
||||
caption: opts.caption,
|
||||
|
||||
@@ -83,6 +83,9 @@ export function clearAuth(): void {
|
||||
if (!browser) return;
|
||||
localStorage.removeItem(TOKEN_KEY);
|
||||
localStorage.removeItem(USER_ID_KEY);
|
||||
// Clear the display name too — on a shared device the next user shouldn't see
|
||||
// the previous guest's name pre-filled on /join.
|
||||
localStorage.removeItem(DISPLAY_NAME_KEY);
|
||||
// PIN is intentionally kept so the user can recover
|
||||
isAuthenticated.set(false);
|
||||
// Hooks fire in registration order. Keep them dependency-free of each other —
|
||||
|
||||
Reference in New Issue
Block a user