9003b34c8a0396cd28906f089aef33f38a23ffb7
71 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Paul Nothaft
|
9003b34c8a |
fix(docker): default NODE_ENV=production so non-compose deploys don't fall back to SQLite (#1038) (#1040)
* fix(docker): default NODE_ENV=production so non-compose deploys don't fall back to SQLite (#1038) knexfile.js selects its config block by NODE_ENV and the `development` block defaults to sqlite3. The image never set NODE_ENV, so every deployment that doesn't go through our compose files — Kubernetes, Helm, plain `docker run` — silently ran on SQLite and ignored DB_HOST/DB_USER/DB_PASSWORD. It stayed invisible because wait-for-db.sh is shell: it reads DB_HOST directly, connects to Postgres, creates the database and logs "PostgreSQL is up" in the same container where the Node process then writes to a SQLite file. Migrations go through src/database/db.js → the same knexfile, so they also ran against SQLite, leaving the provisioned Postgres database empty. Setting the default alone would be unsafe: an affected install would flip to Postgres on its next image pull and come up against an EMPTY database, which reads as total data loss. So this adds a guard that runs before migrations touch anything: - logs the resolved engine + target at boot - refuses to start when pointed at a virgin Postgres while a populated SQLite file exists, naming the file and the .picpeak export path for moving the data, with PICPEAK_ALLOW_EMPTY_PG=true as the escape hatch - warns but boots when Postgres settings are present yet SQLite is in use Compose files already set NODE_ENV explicitly, so compose users are unaffected. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): stay on SQLite instead of blocking, and add a migration path (#1038) Reworks the guard after walking through what an existing install actually experiences on its next image pull. Blocking was the wrong trade. An operator who had unknowingly been running on SQLite (because the image left NODE_ENV unset) would have pulled the fix and got a CrashLoopBackOff: data safe, galleries offline, for something they did not do. Now the boot RESOLVES the engine before migrations run and stays on whichever one holds the data: - Postgres configured but holding no galleries, while a populated SQLite file exists → keep serving from SQLite, print what happened and how to migrate. - once Postgres holds the data, the next restart switches over on its own. - an explicit DATABASE_CLIENT is always honoured. Keyed on Postgres holding DATA, not on it having tables: a stray `run-migrations` against the empty database creates every table, which would otherwise blind the check. Adds scripts/migrate-sqlite-to-postgres.js, which reuses the .picpeak export/import services rather than hand-rolling a cross-engine copy. Two additions were needed for the SQLite → Postgres direction, both opt-in and CLI-only so the upload/restore UI is untouched: - `allowEngineSwitch` relaxes the importer's same-engine guard - cross-engine row coercion: SQLite has no real date or boolean types, so its rows carry epoch numbers where Postgres wants a timestamp and 0/1 where it wants a boolean, both of which Postgres rejects outright. Driven by the TARGET schema, never guessed from the value. DELTA FROM THE BETA PR: this branch's import service has no resyncSequences() — that landed on main only. Without it a cross-engine load leaves Postgres identity sequences at 1 and the next insert collides on the primary key, so the function is backported here and called ONLY on the cross-engine path. Same-engine restores through the UI keep their current behaviour exactly. Verified end to end on this branch against a real PostgreSQL 15: a seeded SQLite install migrated across with booleans, timestamps and foreign keys intact, and the next INSERT got id 2 rather than colliding. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close four review findings on the SQLite fallback + migration (#1038) External review (codex) found four issues, all confirmed against the code and fixed here. Two of them could have cost data. 1. The engine resolver was reachable only through wait-for-db.sh. A Kubernetes manifest that sets `command`/`args`, or a plain `docker run … node server.js`, bypasses the entrypoint — exactly the deployment styles this fix targets. With NODE_ENV now baked into the image, such an install would have resolved to Postgres and come up against an empty database while its SQLite data sat there unseen. server.js now resolves the engine itself, before anything requires knexfile, via the same script the entrypoint uses. Verified by running `node server.js` directly against an install with stranded SQLite data: it logs the banner and serves SQLite. 2. Cross-engine loads double-encoded JSON. SQLite has no json type, so its json columns are TEXT holding JSON; the export dumps that as a string and serialiseJsonColumns stringified it again, storing `true` as the scalar string "true". app_settings.setting_value is json on every install, so this reshaped every migrated setting. The text is decoded before serialisation now — verified against a real Postgres: json_typeof(setting_value) is `boolean`, matching a native install exactly. 3. The migration could silently miss concurrent writes. If the backend keeps serving, rows written after the export never reach Postgres and vanish from view once the engine switches. The script now fingerprints the SQLite tables whose loss would be noticed, checks for drift BEFORE loading Postgres (so a detected race leaves the target untouched) and again after, and refuses with the exact rows that moved. It also says plainly to stop the backend first. 4. The child phases shared stdout with winston. Outside production, and whenever LOG_TO_CONSOLE=true, createPicpeak's own log line was concatenated with the archive path and the migration failed on a bogus filename. Payloads travel through a result file now; verified with LOG_TO_CONSOLE=true. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 2 — six more data-safety findings (#1038) 1. The engine choice is now PINNED once the data is in Postgres. Previously the boot decided from "does Postgres hold galleries", so an operator who later deleted every gallery would be sent back to the stale pre-migration SQLite file while their settings, admins and CRM data stayed in Postgres. The migration writes a marker next to the database file (and retires the file itself by renaming it); the marker wins over any probe. 2. The migration refused to overwrite Postgres only when it held GALLERIES. A target with admins, customers, invoices or projects but no galleries was wiped without --force. Both the source and target checks now look for user data across the tables that are empty on a fresh install. 3. Same bug in the other direction: an install with no galleries but real admins/settings/customers was refused a migration it was entitled to. 4. Drift detection covered four tables and only count/max(id), so an in-place UPDATE (event edit, password change) or a write to any other table passed unnoticed. It now fingerprints every table the export carries, including max(updated_at). It still is not a substitute for stopping the backend, and the script says so rather than implying a guarantee. 5. probeSqliteData() treated an unreadable or corrupt file as "no data", which would have switched the install to an empty Postgres — the very failure this module exists to prevent. It fails closed now and stays on SQLite so the real error surfaces. 6. The "you are leaving SQLite data behind" warning was unreachable: setting DATABASE_CLIENT skipped the probes, so the branch that produces it never had the inputs. Postgres and SQLite are both probed whenever Postgres is the engine in play. Also: the final verification compares row counts for EVERY table rather than just galleries, and flags only a shortfall — the import legitimately adds an app_settings row (setSessionsValidAfter) that made the strict equality fail on a first real run. Verified against a real PostgreSQL 15 end to end, including: the marker keeps an install on Postgres after every gallery is deleted; removing the marker and restoring the file rolls back to SQLite as documented. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 3 — occupancy, bootstrap admin, secrets in /tmp (#1038) 1. Both engine probes judged occupancy by GALLERIES alone. An install whose galleries were all deleted, but which still has admins, customers or accounting records, was treated as empty: on the SQLite side that meant booting the empty Postgres and appearing to lose everything; on the Postgres side it meant diverting a live install to a stale SQLite file. Both now look across the tables that are empty on a fresh install, matching the migration script. 2. The migration ran migrate-schema BEFORE checking the target, and migration 001 seeds a bootstrap admin when ADMIN_PASSWORD is set (common on legacy installs). The occupancy check then saw that admin and refused, pushing the operator towards --force against a genuinely empty database. The target is read first now. 3. probeSqliteData()'s warning went through the app logger, which writes to STDOUT when LOG_TO_CONSOLE=true — and the resolver's stdout is the protocol channel wait-for-db.sh captures, so DATABASE_CLIENT could have been set to a JSON log line. Diagnostics take an injected sink (stderr in the resolver), and the shell now validates the value it captured instead of trusting it. 4. The .picpeak archive holds password hashes, SMTP credentials and API keys in plaintext, and was only removed on the fully-successful path — any drift or import failure left it in /tmp. Every exit path removes it now. 5. A database-only migration still hauled every business-doc and upload through /tmp and back into the same volume. createPicpeak takes includeFiles:false for this path; rows move, files stay where they already are. Verified against a real PostgreSQL 15: a gallery-less install with only an admin account now stays on SQLite and migrates successfully with ADMIN_PASSWORD set; the resolver emits exactly one token on stdout with LOG_TO_CONSOLE=true and a corrupt database; a drift failure leaves Postgres untouched and no archive behind. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): pin the boot to SQLite while a migration is unfinished (#1038) Review round 4. A migration that dies after touching Postgres leaves rows behind — schema creation alone seeds a bootstrap admin when ADMIN_PASSWORD is set, and a drift or row-count failure can leave a partial load. Since the occupancy probes were widened in round 3, those rows read as "Postgres is occupied", so the next restart would switch engines and hide the SQLite data that is still the database of record. The script now writes a pin file next to the database BEFORE its first Postgres write and clears it only on success (after the success marker exists, so no restart in between can pick the wrong engine). While the pin is present the resolver stays on SQLite and explains why. Verified against a real PostgreSQL 15 by reproducing the exact scenario: a migration failed mid-run with ADMIN_PASSWORD set, leaving one bootstrap admin in Postgres. With the pin the next boot resolves to sqlite3; with the pin removed it resolves to pg — the failure this closes. The subsequent successful re-run clears the pin and the boot moves to Postgres. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 5 — occupancy, path drift, retry, host default (#1038) 1. A seeded bootstrap admin counted as "Postgres is occupied". core/001_init.js inserts one whenever ADMIN_PASSWORD is set, so a Postgres that was initialised once and never used would have beaten a SQLite file full of real galleries — the exact failure the guard exists to prevent, reintroduced by widening the probe in round 3. The two sides are deliberately asymmetric now: the SQLite probe counts any user data (err towards keeping data visible), the Postgres probe ignores rows that schema creation seeds (err towards requiring proof of real use). 2. The guard resolved DATABASE_PATH with its own logic while knexfile trimmed whitespace and collapsed the legacy duplicated-backend form. A path either engine normalised differently meant probing a file nobody uses, concluding there was no SQLite data, and booting an empty Postgres. The resolution now lives in one module both require. 3. Re-running after a partial migration — the documented recovery — was refused unless the operator passed the destructive-sounding --force, because the half-written rows read as target data. An unfinished run of this same script is now recognised as a safe retry. 4. wait-for-db.sh verified readiness against its own default host (`postgres`) while knexfile's production block defaults to `db`. With NODE_ENV now baked in, a bare `docker run` without DB_HOST would have passed the readiness check against one host and then dialled another. The entrypoint exports the exact connection it verified. Compose sets DB_HOST explicitly and is unaffected. Verified: a Postgres holding only a seeded admin now loses to real SQLite data; a DATABASE_PATH with surrounding whitespace resolves to the identical file in both knexfile and the guard. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 6 — explicit-client bypasses, retry scope, cleanup (#1038) 1. An explicit DATABASE_CLIENT bypassed the unfinished-migration pin, because decideBootEngine honoured it first. docker-compose sets DATABASE_CLIENT=pg, so a failed migration would have restarted on a half-written Postgres on exactly the deployments that pin it. Worse in the other direction: with DATABASE_CLIENT=sqlite3, a SUCCESSFUL migration renames the source file, so the next start created a NEW, empty SQLite database and served that. The pin now outranks explicit pg (clearing the marker is the override), explicit sqlite3 is left alone since it already points at the data, and the migration refuses up front when the deployment pins anything other than pg. 2. The retry allowance was bound to the SQLite file, not to the target. An operator who repointed DB_HOST/DB_NAME between attempts could have replaced an unrelated populated database without --force. The pin records the target and the allowance only applies when it matches. 3. The printed rollback did not roll back: with data on both sides and no marker, the resolver still selects Postgres. It now spells out all three steps, including DATABASE_CLIENT=sqlite3. 4. A failure inside createPicpeak left a partial archive — plaintext hashes and credentials — in the caller-supplied temp dir, which that service deliberately does not clean. The export phase removes it on error. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 7 — pin bypass on direct start, real admins (#1038) 1. server.js only ran the engine resolver when DATABASE_CLIENT was unset, so a deployment that both bypasses the entrypoint (Kubernetes `command:`) AND pins DATABASE_CLIENT=pg never consulted the migration pin — the round-6 fix was unreachable on exactly that path, and a failed migration would have served a half-populated Postgres. The resolver now also runs whenever a pin file exists. 2. Round 5 excluded admin_users from Postgres occupancy to stop a seeded bootstrap admin counting as real data. That over-corrected: an install that has completed first-run setup but has no galleries yet has exactly one user-created row — an admin — so Postgres looked empty and, with a stale SQLite file present, the boot would switch away and the admin's credentials and configuration would disappear. core/001_init.js seeds must_change_password=true; setupService writes false once a human completes setup. The FLAG, not the table, distinguishes them, and a legacy NULL counts as a real admin. Verified against a real PostgreSQL 15: a Postgres holding only the seeded row loses to real SQLite data, the same Postgres wins once setup is completed, and a server started directly with DATABASE_CLIENT=pg and a pin present comes up on SQLite with the warning. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 8 — reset admins, CLI config, JSON nulls (#1038) 1. must_change_password is mutable: resetAdminPassword() re-raises it on REAL accounts (userManagementService.js:474). Round 7's discriminator therefore read a gallery-less Postgres whose only admin had been reset as an untouched bootstrap seed — and with a stale SQLite file present, the boot would have switched away and hidden those live credentials. The rule is layered now: more than one admin, any admin that has logged in, or must_change_password false all count as use. Only core/001_init.js's exact leftovers — one admin, never logged in, still flagged — read as a seed. 2. The CLI read process.env directly but never loaded the configuration the child phases get through knexfile, so running it directly (or via `docker exec`, which does not inherit wait-for-db.sh's exports) failed the pre-flight checks even with valid settings in backend/.env or /run/secrets/db_password. Both sources are loaded up front now. 3. The migration's target check counted a seeded bootstrap admin as user data while probePgData classified the identical row as empty, so migrating into a previously-initialised-but-unused Postgres demanded --force. Same rule on both sides. 4. Cross-engine JSON handling is simpler and no longer lossy. SQLite keeps json columns as TEXT holding valid JSON and Postgres accepts JSON text directly, so the correct action is to pass them through untouched. Round 1 parsed then re-serialised them to undo a double-stringify; that round-tripped the JSON literal `null` into SQL NULL, changing data and breaking NOT NULL json columns. Not serialising at all fixes both. Verified against a real PostgreSQL 15: a migrated install now carries json_typeof = null for a JSON null, object for a nested object, and boolean for a boolean — matching a native install exactly. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 9 — probe error classes, marker ordering (#1038) 1. probePgData() answered every failure with "Postgres has data". That is right for an unreachable server — the app cannot run on it either way, and diverting a healthy pg install to a stale SQLite file over a transient blip would be worse — but wrong for a server that answers and then fails the query, which is what a half-built or damaged schema looks like. That is not evidence of data, and reporting it as such booted the empty Postgres and hid a populated SQLite file: the exact failure this guard exists to prevent. Reachability is now established with SELECT 1 first, so the two cases get opposite answers: unreachable → leave the configured engine alone; reachable-but-uninspectable → unproven, and the SQLite side wins if it actually holds data. 2. The success marker was written after the SQLite file was renamed away. A failure in between — a full disk — left the source retired with no marker: the next attempt reported "No SQLite database", the in-progress pin stayed, and the operator never saw the rollback path. The marker is written first and updated with the retired filename once the rename succeeds, so a failure at any point leaves everything recoverable. Verified against a real PostgreSQL 15: a reachable database whose admin_users table lacks the probed column now resolves to sqlite3 rather than hiding the data, while an unreachable host still resolves to pg. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): don't fail the migration on empty SQLite-only tables (#1038) Review round 10. The final verification flagged every source table missing from Postgres, regardless of whether it held rows — and SQLite-only tables do exist: initializeDatabase() creates an `events_new` scratch table and, when its legacy column copy throws, the catch swallows the error and leaves the empty table behind (db.js:236). The importer correctly skips tables Postgres does not have, so verification then reported a mismatch AFTER the data had already landed, exited 1, and left the install pinned to SQLite with no way to finish. An absent target table only matters if the source actually had rows. Empty ones are now listed and skipped. Reproduced both ways against a real PostgreSQL 15 with an events_new table present: without the fix the run ends in "ROW COUNTS DO NOT MATCH" and leaves the in-progress pin; with it, the table is reported as skipped, the migration completes and the pin is released. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): a completed migration overrides an implicit SQLite config (#1038) Review round 11. The migration allowed the one configuration it should have worried about most: DATABASE_CLIENT unset AND NODE_ENV not "production", which resolves to the development block — i.e. sqlite3. That is precisely the state the affected installs are in, since it is why they ended up on SQLite at all, so an operator can easily run the migration before fixing it. The script then renames the source database away, and the next start resolved to the implicit sqlite3, created a NEW empty database and served it — after reporting success. The success marker now overrides an IMPLICITLY resolved sqlite3 when Postgres settings are present, because the marker is durable proof of where the data actually went. An explicit DATABASE_CLIENT=sqlite3 still wins: that is the documented rollback. The script says something rather than refusing — refusing would block exactly the population this exists for. Reproduced with NODE_ENV and DATABASE_CLIENT both empty, against a real PostgreSQL 15: the migration completes, the source is renamed away, and the next boot resolves to pg with the data intact. Before this it resolved to sqlite3 and would have served an empty database. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * refactor(db): drop the dead reachability flag in probePgData (#1038) github-code-quality flagged `if (reachable)` as always true, and it is right: the unreachable branch returns, so everything below it runs only when the probe connected. The variable and the conditional were leftovers from a first draft that used a single catch for both failure classes. No behaviour change — the two error paths still return opposite answers. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): refuse to choose when both databases hold data (#1038) Review round 12. 1. An install that ran on PostgreSQL, lost NODE_ENV/DATABASE_CLIENT, and kept working on SQLite has REAL data on both sides: old rows in Postgres, newer ones in SQLite. The stranded-data rule only protected SQLite when Postgres was empty, so pulling this fix would have booted Postgres and hidden every gallery created since the switch — the exact failure this PR exists to prevent, in a variant I had not considered. A completed migration leaves a marker saying which side is current. Without one, two populated databases are a conflict: the boot stops and prints both targets, the two DATABASE_CLIENT values that resolve it, and the migration command that merges them. This is the only deliberate refusal in the change — guessing here would hide data AND split subsequent writes across two databases. 2. probePgData was handed knexConfig.connection even when knexfile had resolved to SQLite (a completed migration whose environment still says sqlite3), so node-postgres dialled its own localhost defaults instead of DB_HOST/DB_NAME — false "unreachable" diagnostics and a needless delay on every boot. The probe target is now built from the environment when the config is not pg. The conflict is honoured by all three entry points: the resolver exits 3 with an empty stdout, wait-for-db.sh stops the container, and server.js refuses to start. Two existing tests asserted that Postgres wins when both sides hold data. They encoded the pre-conflict assumption and described a state that cannot occur after a real migration (which always leaves a marker); both now pass the marker. Found while testing: the resolver's logger shim had no .error, so the conflict path threw, was swallowed by the fallback, and silently chose Postgres — the precise outcome this refuses to make. The shim is complete now. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): symmetric bootstrap rule, one resolved Postgres target (#1038) Review round 13. Both findings are consequences of earlier rounds. 1. The conflict rule added in round 12 counted an untouched SQLite bootstrap admin as data. core/001_init.js seeds one whenever ADMIN_PASSWORD is set — including into the accidental SQLite database — so a healthy Postgres install that had ever started once without NODE_ENV would have had a seeded-only SQLite file beside it, been declared a both-populated conflict, and REFUSED TO BOOT. The bootstrap discrimination is applied on both sides now; a setup-completed or logged-in admin still counts as real use on either. 2. The CLI's child phases inherited whichever knexfile block NODE_ENV selected. The development block defaults Postgres to localhost/postgres/photo_sharing, production to db/picpeak/picpeak — and this script is explicitly meant to run with NODE_ENV unset. With DB_USER/DB_NAME left to defaults it would therefore have migrated into `photo_sharing`, after which following the script's own advice to set NODE_ENV=production pointed the app at an empty `picpeak`. The target is resolved once, with production defaults, and passed explicitly to every phase — so the block knexfile happens to pick can no longer decide which database the data lands in. The pin and success marker record that same resolved identity. Verified against a real PostgreSQL 15: a live Postgres beside a seeded-only SQLite file now boots pg rather than refusing, flipping that admin to setup-completed restores the conflict, and a migration records localhost:7102/picpeak_r13b as its target rather than a defaulted guess. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): one Postgres identity everywhere; protect the credentials file (#1038) Review round 14. Three of the six findings were the same defect as round 13's, surfacing through paths that fix did not cover: the connection used to PROBE or MIGRATE could differ from the one the application then OPENS, because knexfile's development block points Postgres at localhost/postgres/photo_sharing while production uses db/picpeak/picpeak. 1. server.js exported only DATABASE_CLIENT=pg after the resolver decided, so knexfile filled in host/user/database from whichever block NODE_ENV selected. With SQLite already retired by a migration, that meant opening an empty database. The whole connection is pinned now. 2. Two defaults existed for DB_HOST: wait-for-db.sh resolves and exports `postgres`, knexfile's production block says `db`. Since the entrypoint exports its value, `postgres` is what a running container actually uses — so a `docker exec` migration, which inherits neither, has to agree with that, not with the default that is only reached when the entrypoint did not run. 3. The migration's Postgres phases inherited an unset NODE_ENV and therefore the development block, which ignores DB_SSL entirely — a managed Postgres requiring TLS could never be migrated into. The phases run with production semantics now. 4. core/001_init.js writes data/ADMIN_CREDENTIALS.txt, and that data directory belongs to the SOURCE install. Bootstrapping the Postgres schema replaced the operator's real credentials file with ones for a temporary admin the import immediately discards. The file is preserved across the phase, including when it fails. 5. The boot line described knexConfig, so an install redirected to Postgres by a migration marker still logged "Database engine: sqlite (...)", contradicting the warning printed one line earlier. 6. On a both-populated conflict resolveBootEngine returns client:null, and both migration runners told the operator their data was in "null" and to set DATABASE_CLIENT=null. They now present the two real choices. Verified against a real PostgreSQL 15: a migrated install started directly with NODE_ENV unset now logs `postgres (localhost:7102/picpeak_r14)` and opens it, where before it would have gone to the development block's photo_sharing. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * refactor(db): resolve the PostgreSQL target in exactly one place (#1038) Rounds 13 and 14 both traced back to the same thing, each time through a caller the previous fix had not covered: three different defaults existed for the same connection. knexfile development : localhost / postgres / photo_sharing knexfile production : db / picpeak / picpeak wait-for-db.sh : postgres / picpeak / picpeak (and it EXPORTS them) So a process that probed or migrated against one could hand over to a process that opened another. Patching each caller was not converging — the guard, then the CLI's child phases, then server.js — so this deletes the divergence instead. `src/utils/pgConnection.js` now owns the resolution and knexfile's development and production blocks both derive from it, as does the engine guard. Same shape as the earlier sqlitePath.js extraction, for the same reason. The database NAME is what made this dangerous: a wrong host or user fails loudly at connect time, while a wrong name connects fine and presents an empty installation. BEHAVIOUR CHANGE: with DATABASE_CLIENT=pg and no DB_* variables, a non-production environment now resolves to postgres/picpeak/picpeak instead of localhost/postgres/photo_sharing. Deployments are unaffected — compose sets these explicitly and wait-for-db.sh exports them — but a local machine running Postgres bare now needs DB_HOST=localhost DB_USER=postgres DB_NAME=photo_sharing (or DATABASE_CLIENT=sqlite3, which is what backend/.env already uses). The failure mode of getting this wrong is a refused connection, not a silently empty database. Side effect worth having: DB_SSL is now honoured whatever NODE_ENV says, so the managed-Postgres case is fixed at the root rather than by forcing production semantics onto the migration's child phases. The test block keeps its own photo_sharing_test default — isolation is the point there. Verified: every block plus the guard resolve identically from the same environment; explicit DB_* still wins; production's pool tuning is preserved; and a full SQLite → PostgreSQL migration with NODE_ENV unset lands in the right database with JSON shapes intact. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): two more components that guessed the database instead of asking (#1038) Both found while sweeping for copies of the connection defaults. Checked in detail first — one of my suspicions about them was wrong. scripts/set-admin-password.js hand-rolled its own knex config while all four sibling scripts (reset-admin-password, create-admin, show-admin-credentials, reset-admin-mfa) use the application's connection. Two consequences: - it read DB_CLIENT, a variable nothing else in this codebase sets, so it defaulted to Postgres and could not work on a SQLite install at all; - it defaulted to database `picpeak_dev`, a name no other component uses. It now uses `require('../src/database/db')` like its siblings, so it follows whatever engine the install actually runs on. Timestamps are written as ISO strings because it reaches SQLite now, where raw Date objects are the documented landmine. NOT changed: the script's "all existing sessions have been invalidated" notice is accurate — auth.js compares token iat against password_changed_at — and it deliberately leaves must_change_password alone, which is right for an operator choosing a password rather than being issued one. routes/adminSystem.js re-derived three things the live connection already knows, and each could disagree with it: - the engine, from DATABASE_CLIENT || 'sqlite3' — so a Postgres install without an explicit DATABASE_CLIENT took the SQLite branch; - the Postgres database, from DB_NAME || 'picpeak'; - the SQLite file, from a hardcoded ../../data/photo_sharing.db that ignored DATABASE_PATH entirely. All three now come from db.client.config, with pg_database_size(current_database()). Verified: set-admin-password works on SQLite (new hash verifies, old rejected) and still on PostgreSQL; and on a SQLite install with a custom DATABASE_PATH the size logic reports the real database (1,748,992 bytes) where the old code reported a different file entirely (1,851,392) — or 0 where that path does not exist. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): bind the migration marker to its target; fix a phantom table (#1038) Review round 15. 1. The marker records `host:port/database`, but only its EXISTENCE was checked. Repoint DB_NAME or DB_HOST at a different, empty PostgreSQL after migrating and the marker would vouch for that one too — booting it, presenting an empty installation, and suppressing the SQLite fallback while the real data sits in the recorded target and the renamed rollback copy. The marker is compared against the current connection now, and a mismatch stops the boot with both targets named and the two ways out. 2. `incoming_invoices` is not a table — supplier documents live in `inbound_documents` (core migration 124). Both occupancy lists skip tables that do not exist, so those records were silently not protecting anything: an install whose only remaining data was inbound documents could be switched away from, or overwritten without --force. Verified every other name in the lists against the live schema at the same time. Verified: a marker naming picpeak_original with picpeak_mk configured refuses with exit 3 and prints both; making them agree boots pg. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local> |
||
|
Paul Nothaft
|
ccab9024d4 |
fix(security): bound inbound-mail resources, redact secrets from logs (stable) (#965)
* fix(security): bound inbound-mail resources, redact secrets from logs (GHSA-2qf9, pgmp, r794) GHSA-2qf9 — emailIntakeService downloaded, parsed and persisted every message with no size, attachment-count or attachment-byte limit, reachable unauthenticated by anyone who can email the operator's mailbox: - fetch the envelope with `size` (same cheap pass) and refuse an oversized message BEFORE downloading its source; - cap attachment count and cumulative attachment bytes; - limits env-overridable, defaults generous for real supplier invoices. The teeth were in the dedup key. received_emails.message_id is varchar(512) UNIQUE, and the failure path wrote `err-<uid>-<Date.now()>`, which can never match the envelope-derived messageId the dedup pass compares against — so an oversized (or overlong-Message-ID) mail was re-downloaded every poll forever, and an OOM-kill/restart just resumed the loop. Size-skips are now recorded under the REAL message id, and overlong ids collapse to a stable sha256 key that always fits the column. GHSA-pgmp / r794 — new sanitizeForLog() util (key-name deny-set, recursive, cycle-safe) applied to the three request-body log sites in adminEvents/crud.js, plus sanitizeValidationErrors() because express-validator's errors.array() embeds the SUBMITTED value per field — a rejected plaintext password was still logged. Scope is wider than filed: the update path also logged client_password_hash and a LIVE client_share_token bearer credential. Also: the one-time setup token was logged at warn AND printed to stdout on every first boot, putting a live first-admin credential in combined.log, security.log and `docker logs`. It is now written to the 0600 token file and only surfaced when that write fails — the last-resort path it existed for. (stable) * fix(security): codex round 3 — repair the first-run token recovery flow (GHSA-r794) Two regressions from keeping the setup token out of the logs. 1. server.js decided whether to print the token by calling existsSync() on the candidate path. That answers a different question than "did the write succeed": a stale, read-only or directory-shaped SETUP_TOKEN reports as present, so the banner suppressed the live token and pointed the operator at content that is not it — leaving the current token only in combined.log under default production logging. setupService now records the path the write actually produced and exposes it via writtenSetupTokenFile(). 2. The setup screen, its EN/DE strings, README, SIMPLE_SETUP and .env.example all still told first-time users to run `docker compose logs backend | grep -i "setup token"`. On the normal path that command now returns a path banner and no credential, so the documented browser-first onboarding could not be completed. They now point at `docker compose exec backend cat /app/data/SETUP_TOKEN`, with the log fallback described as what it is — the failure path. Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me (cherry picked from commit 9a54b6f0231c3285df4c4865eb846e63e1ed0dda) --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local> |
||
|
Paul Nothaft
|
9ee3ff45d0 |
fix(security): remove unguarded legacy /api/events router (GHSA-4j34-x562-5vfq)
The legacy gallery router mounted at /api/events exposed create/list/update/
delete/extend guarded by adminAuth ALONE — no requirePermission, no
requireEventOwnership. adminAuth only checks the token is a valid type:'admin'
session, which every back-office role holds, down to read-only `viewer`. So any
non-super-admin account could:
- GET /api/events → every gallery's bcrypt password_hash, share_token, and
client name/email (the list handler selects * and mapEventForApi keeps
those columns),
- PUT /api/events/:id → reset any gallery's password (full takeover),
- DELETE /api/events/:id → delete any gallery,
all bypassing the per-photographer ownership isolation the canonical
/api/admin/events router enforces. Affects any instance with more than the
single super_admin.
Fix: remove the legacy router entirely (mount + require + src/routes/events.js).
It was a superseded duplicate of /api/admin/events and unused by the frontend
EXCEPT for one live route — POST /:id/extend (the "Extend expiration" UI action,
which hit /api/events/:id/extend via the api client's /api base). That route is
migrated to the canonical mount as POST /api/admin/events/:id/extend with the
same guards as every other gallery mutation (adminAuth + requirePermission
('events.edit') + requireEventOwnership), and the frontend is repointed to it.
Behaviour of the extend itself is unchanged (expires_at + reactivate).
Verified end-to-end on a booted instance: /api/events (all methods) now 404;
/api/admin/events/:id/extend returns 401 unauth, 200 for the owner, 403 for a
non-owning editor; the full login→create→extend flow works. Adds a regression
test pinning the router removal and the extend ownership check.
|
||
|
Luca
|
b0912c7427 |
feat(setup): validate setup token at step 1 before advancing
Previously "Continue" on the token step only checked the field was non-empty; a wrong token wasn't caught until the final submit, after the user had filled in email + password. Add a non-burning verify: - backend: POST /setup/verify-token constant-time compares the token without consuming it (createInitialAdmin still claims it atomically on submit), gated on no-admin-exists and rate-limited like /setup/admin. - frontend: step-1 "Continue" calls verifyToken and only advances on a valid token; a wrong token shows the invalidToken error on the field, 429 -> too-many-attempts, 409 -> redirect to login. Adds integration tests for accept-without-burn / reject / closed-once-set. |
||
|
Luca
|
415bffa04c |
feat: zero-config first run — in-browser admin bootstrap + auto-generated secrets
Fresh installs need nothing in .env. See PR description for the full feature. |
||
|
Paul Nothaft
|
56c2386c90 |
feat(gallery): branded URL shortener — /s/<slug> with OG injection (#699)
Issue 3 from #699 (@alexvaltchev's report): expose a custom-named short URL per event that bots scrape for OG previews and browsers redirect to the underlying gallery. WhatsApp / iMessage / Facebook cache the OG metadata by the URL they crawl, so the SHORT URL becomes the cache key — admins can rotate or split-test underlying gallery URLs without re-pushing a fresh link to clients. Additive feature; no existing route, table, or column is modified. ## Backend - `gallery_short_urls` table (migration 150): id, short_slug UNIQUE, event_id FK CASCADE, target_path TEXT, created_by/at, hit_count, last_hit_at, deleted_at/by. hasTable-guarded so the migration is idempotent on re-run. - `src/services/galleryShortUrlService.js` — validator + CRUD + resolver. Slug rules: `/^[a-z0-9](?:[a-z0-9-]{0,62}[a-z0-9])?$/`, reserved blocklist (admin, api, auth, gallery, og, s, login, ...). target_path snapshots at create-time from the event + global short-URL toggle, so a later flip of the toggle does NOT silently change where existing short URLs resolve. - `src/routes/adminShortUrls.js` — `GET/POST /api/admin/events/:eventId/short-urls`, `DELETE /api/admin/short-urls/:id`. Structured errors: 400 INVALID_SLUG, 409 SLUG_TAKEN (with `suggested`), 404 EVENT_NOT_FOUND. Gated by events.view / events.edit + requireEventOwnership. - `server.js` /s/:shortSlug public route. Bot UA → server-render the same OG metadata the existing /og/gallery/<slug> handler produces, then override og:url to point at /s/<shortSlug> itself (cache-key invariant — social platforms key by the URL they scrape). Browser UA → 302 to target_path. Soft-deleted slug → 410 Gone (intentional-delete signal, distinct from 404 unknown slug). Hit accounting is fire-and-forget. ## Frontend - `services/shortUrls.service.ts` — list/create/remove. - `components/admin/ShortUrlsCard.tsx` — per-event card on the EventDetailsPage. Form for custom or auto-generated slug, list with copy-to-clipboard + soft-delete. SLUG_TAKEN error surfaces the service's `suggested` slug with a "use suggested" button. - i18n: events.shortUrls.* added to EN + DE. ## Tests 78 new tests, all passing: - `__tests__/utils/galleryShortUrlValidation.test.js` (48) — pure- function tests for validateSlug: accepts/rejects, reserved-slug blocklist, path-traversal + URL-injection vectors. - `__tests__/integration/galleryShortUrls.test.js` (19) — service layer against a real SQLite DB. Covers custom + auto-generated slugs, collision + SLUG_TAKEN + suggested, target_path snapshotting (backward-compat invariant), soft-delete + slug rotation, hit counting. - `__tests__/integration/galleryShortUrlRoute.test.js` (11) — HTTP-level: 302 redirect for browser UA, 200 + OG HTML for bot UA, og:url canonical points at /s/<slug>, 410 for soft-deleted + orphaned events, 404 unknown + malformed. Regression sweep: 47 existing migration-chain integration tests still pass; migration 150 is additive only. ## Backward compatibility - Existing `/gallery/<slug>`, `/gallery/<32-hex-share-token>`, `/gallery/<slug>/show/<token>`, `/og/gallery/<slug>`, `/og/gallery/<slug>/cover` routes are untouched. - The `/s/` namespace is new; no existing route lives there. - Migration 150 only ADDs the new table — no ALTERs on existing schema, no destructive changes. - target_path is snapshotted at create-time so flipping the global "Use short gallery URLs" setting after a short URL exists does NOT change where that short URL resolves. |
||
|
Paul Nothaft
|
1b8747dc82 |
fix(og): rich social previews for share-token + slideshow URLs (#699)
Two SSR-OG injection bugs reported by @alexvaltchev. Both made his link
previews fall back to the brand logo + site-wide tagline instead of the
event-specific name/photo, even though the bot UA was hitting our
already-existing OG handler. He compensated with a Cloudflare Worker as
SSR middleware — which then created bug 3 below (og:image at the
auth-gated /api/.../hero/ path, not the public /og/.../cover one), so
Instagram never rendered the image either.
## Bug A — slideshow URLs miss the OG handler entirely
`/gallery/<slug>/show/<token>` has 3 segments after `/gallery/`. The OG
route was wired only at `/gallery/:slug/:token?` (1-2 segments), so
slideshow links fell through to the SPA-catchall `/gallery/*` and never
invoked the OG handler at all. Added a second route handler for the
3-segment slideshow shape, sharing the same intercept middleware so a
recognised social crawler still gets the rich preview.
## Bug B — share-token-only URLs resolve to nothing
`/gallery/<32-char-share-token>` (the form produced when migration 525's
short-URLs option strips the event slug) routes to the OG handler with
`slug=<token>`. resolveSlug then queries `events.slug = <token>`, which
never matches because the token is in a separate `share_token` column.
Result: falls through to the "no event found" branch and serves the
generic site-wide OG.
Fix: when the slug shape matches a 32-char hex AND the slug lookup
missed AND no redirect rule applies, try `events.share_token = slug` as
a final fallback. Real slugs are kebab/dot/underscore mixes, never pure
32-hex, so the extra DB roundtrip is gated to only fire for the
token-shaped URL.
## Tests
3 new tests in galleryOgService.shareImage.test.js using non-entropy
32-hex fixtures (deliberately zero-padded to avoid tripping
GitGuardian's Generic High Entropy Secret detector while still
matching the route's /^[a-f0-9]{32}$/i shape check):
- share-token slug resolves via the share_token column (alex's case)
- malformed/expired 32-hex token returns the site-wide fallback (no leak)
- non-hex slugs skip the share_token query entirely (hot-path cost guarded)
All 14 tests in the file pass.
## Out of scope here (separate follow-up)
- Issue 2 (Instagram og:image) — alex-side CF Worker bug pointing
og:image at /api/gallery/<slug>/hero/<id>, which requires gallery
auth. PicPeak already has the right unauthenticated path
(/og/gallery/<slug>/cover) gated by events.og_image_share_enabled
per-event opt-in (#474). Documented in the issue reply.
- Issue 3 (URL shortener with custom names) — real feature request,
meaningfully different from the existing #525 short-URLs option that
just strips the slug. Designing separately.
|
||
|
Luca
|
9b557efbf3 |
feat(workflows): seed invoice-dunning ladder as an editable built-in flow
Boot self-heal seeds the corrected gate-in-loop dunning graph (wait→due, grace wait, invoice_paid check, confirm-no-payment gate, bounded reminder loop with re-check, final notice) keyed on builtin_key='invoice_dunning', sized from the reminder_first/second_days settings. Seeded DISABLED and is_builtin: live reminder behaviour is UNCHANGED (the hardcoded scheduler ladder still runs) — enabling it pre-cutover would double-send, so the engine cutover is a deliberate follow-up. Idempotent (preserves admin edits). Built-ins refuse delete (enforced in the CRUD route). Test covers seed shape + idempotency. |
||
|
Luca
|
1a0d6de04d |
feat(workflows): admin CRUD + run-history + approvals-inbox API
GET/POST/PUT/PATCH/DELETE /api/admin/workflows with graph read/write (PUT writes a fresh node/edge set under version+1 and bumps workflows.version so in-flight runs keep their pinned version). Run-history (/:id/runs, /runs/:runId/steps) and the pending-approval inbox (GET /approvals, POST /approvals/:id/:action → actById) round it out. Gated by the workflows flag + RBAC (view for reads, manage for writes); built-in flows refuse delete; graph validated (exactly one trigger, unique keys, edges reference known nodes). Route tests cover CRUD, validation, version bump, toggle, inbox, and the 403 permission gate. |
||
|
Luca
|
b48d8c2eb8 |
feat(workflows): approval gates — email confirm/deny + token resume
gate_setup action creates a workflow_approvals row (single-use token stored as SHA-256 hash) and emails the admin confirm/deny links immediately (internal mail, no business-hours floor). actByToken / actById finalize the approval and resume the run down the matching confirm/deny edge; both are idempotent (a second click → 'already recorded') and respect expiry. Public GET /api/public/workflow-approvals/:token/:action returns a small HTML confirmation page (clickable from email, single-use so prefetch can't double-act). listPending backs the webview inbox (wired in the CRUD phase). Test covers gate→approval→email→token-confirm→resume + idempotency. |
||
|
Paul Nothaft
|
78c8e9d9f9 |
feat(whatsapp): WhatsApp Business API notification channel (#640 part D)
Ports filpgame/picpeak's WhatsApp integration with substantial adaptation
to fit our codebase patterns. Deliver the gallery-ready notification via
Meta Graph API in addition to (or instead of) email — useful where the
customer base expects WhatsApp by default. Strictly opt-in behind the new
`whatsapp` feature flag.
### Backend
- **Migration 136** (`whatsapp_configs` + `whatsapp_queue`). Loose-FK on
`event_id` matching our `inbound_documents` / `expenses` pattern (NOT
filpgame's hard FK — deleting an event shouldn't RESTRICT on stale queue
rows). Composite index on `(status, retry_count, created_at)` covers the
poll path.
- **`whatsappService.js`**: thin Meta Graph client. Meta API version bumped
v19 → v20 (filpgame's v19 deprecates Q3 2026); configurable via
`WHATSAPP_META_API_VERSION` env var. Timeout dropped 10s → 8s for
processor budget. Errors surface the Meta `error.code` so the processor
can tell retryable from permanent.
- **`whatsappProcessor.js`**: queue processor polling every 30s (configurable
via `WHATSAPP_QUEUE_POLL_MS`), 10 messages per cycle, 3 retries before
marking `failed`. Default language sourced from
`app_settings.general_default_language` (matches our email-language
resolution pattern); replaces filpgame's hardcoded `pt_BR` fallback.
Falls back to `en_US` if nothing is configured. No-ops gracefully when
the `whatsapp` flag is off, the config row is missing, or the access
token isn't set.
- **`adminWhatsapp.js`**: three routes (GET/PUT config, POST test). Gated
by `requireFeatureFlag('whatsapp')` so operators who haven't enabled it
can't see the surface. Access token masked as `'********'` on GET;
masked values silently preserve the stored token on PUT. Enabling with
no Phone Number ID, template name, or token (and none stored) fails at
the validator.
- **Two hook points** in `adminEvents.js`:
- **Create-and-publish-in-one-step**: queues immediately after the
`gallery_created` email when `!isDraft && customerPhone &&
waConfig.enabled`. Password from `req.body` is still in scope.
- **Publish-from-draft** (`POST /:id/publish`): queues with the password
the admin re-typed via PR #627's `PublishGalleryDialog`. When no
password was typed (legacy API consumers without dialog), passes empty
string so the password line renders blank rather than leaking the
`(set at creation)` sentinel.
- **`server.js`**: starts `whatsappQueueProcessor` at boot. Non-fatal if it
fails to start (logged as warning).
- **`feature_flags`**: new `whatsapp` flag in `KNOWN_FLAGS` and
`DEFAULT_FLAGS` (default false).
### Frontend
- **`featureFlags.service.ts`**: `'whatsapp'` added to `FeatureKey` union.
- **`FeaturesTab.tsx`**: WhatsApp card in the Communication section
(between Incoming mail and Messaging). Smartphone icon, "new" status,
sidebar-hidden (no sidebar entry — config lives under Settings).
- **`whatsapp.service.ts`** (new): typed client for the three admin routes.
- **`WhatsAppTab.tsx`** (new): Settings tab. Form for Phone Number ID,
WABA ID, access token (masked toggle), template name, and enabled flag.
Separate card below for a static test send. Token masking matches the
server's `'********'` sentinel — admin can edit other fields without
re-entering the token.
- **`SettingsPage.tsx`**: WhatsApp tab nav item gated on `flags.whatsapp`
(so it shows only when the feature is enabled); render block wires
`<WhatsAppTab />`.
### i18n
22 new EN + 22 new DE entries covering the Settings tab form, the
Features-tab card, plus `admin.activities.whatsapp_config_updated` +
`admin.notificationMessages.whatsappConfigUpdated` for the bell /
dashboard surfaces from PR #637.
### Deliberately NOT included
- filpgame's **password-encryption-at-rest** layer
(`password_encrypted`/`password_iv`/`password_key_version` columns).
Our publish-from-draft password recovery uses the admin re-type flow
from #627 (PublishGalleryDialog) — no plaintext at rest.
### Setup notes for operators
1. Create a Meta Business Account + WhatsApp Business App.
2. Register a phone number and obtain `phone_number_id` + `waba_id`.
3. Create a system-user access token (long-lived recommended).
4. Submit a message template for approval. The default `gallery_ready`
expects 5 body parameters: customer name, event name, gallery link,
password line, expiry date.
5. Enable the `whatsapp` feature flag.
6. Enter credentials under Settings → WhatsApp, send a test, then enable
delivery.
### Test plan
- [x] Backend `node -c` on all new/changed files clean
- [x] `tsc --noEmit` on frontend clean
- [x] Backend dev container restart picks up new files, /health OK
- [ ] Manual: enable `whatsapp` flag → Settings → WhatsApp tab appears
- [ ] Manual: save config with masked-only token (existing token preserved)
- [ ] Manual: enable=true without phone_number_id rejected at PUT
- [ ] Manual: enable=true without stored or new token rejected at PUT
- [ ] Manual: create-and-publish event with customer_phone → queue row
inserts with message_type='gallery_created'
- [ ] Manual: publish-from-draft via PublishGalleryDialog with password →
queue row uses the admin-typed password in the {{4}} line
- [ ] Manual: test send to a real phone with valid Meta config + approved
template → Meta returns messages[0].id, toast shows the id
- [ ] Manual: bell renders "WhatsApp configuration updated" in DE when
the config_updated activity fires (via PR #637 smart default)
|
||
|
Luca
|
fbbbb8ab73 |
feat(accounting): VAT registration/reclaim settings + un-gated VAT-codes read
Slice 1 of the VAT consolidation backend: - PUT /admin/settings/accounting accepts accounting_vat_registered (bool) + accounting_vat_reclaim_countries (ISO-2 list); GET /:type already returns them parsed, so no GET change needed. - New read-only GET /api/admin/vat-codes (adminAuth, NOT accounting-gated) so the invoice/quote editors can populate their VAT dropdown even when the accounting layer is off. Management CRUD stays under /admin/ledger. |
||
|
Luca
|
402dbde0a1 |
Merge origin/beta into feat/accounting-inbound-invoices
Resolves the 7 feature-flag / i18n conflicts (accounting flags vs upstream's Project Overview 'projects' flag, both registered in the same files) as additive unions — accounting + incomingInvoices + expenses AND projects all coexist. Migrations slot cleanly: projects 117-121, accounting 122-129, no collisions. Frontend build + backend node --check pass. |
||
|
Luca
|
03cc250b47 |
feat(accounting): Layer A backend — chart of accounts, VAT codes, Treuhänder export
Prepares picpeak to feed a Treuhänder's double-entry software once a user crosses the CHF ~500k threshold (LI PGR Art. 1045), without becoming an ERP. - migration 129: ledger_accounts (seeded Swiss/LI KMU-Kontenrahmen) + vat_codes (CH/LI MWST 8.1/2.6/3.8/0 + reverse charge), expense_categories gains ledger_account_id, app_settings default-account + VAT-map seeds - ledgerService: full CRUD for accounts + VAT codes + mappings; buildPostings() turns revenue invoices + incoming invoices + expenses into accrual Buchungssätze (Dr/Cr + VAT code); generic/banana/bexio CSV export - routes /api/admin/ledger/* (accounting master gated; export also requires taxReport); 12 unit tests (posting engine + formatters) Accrual basis only — payment/bank postings are Layer B. Output is a guideline (Treuhänder caveat on the UI). |
||
|
Luca
|
5645c304ab |
feat(email): incoming mail (IMAP) intake - backend + standalone flag
Adds a second mail config (incoming/IMAP) alongside the outgoing SMTP one, a 1-minute poller, and a received-emails log. Standalone `incomingMail` feature flag (default off). - deps: imapflow + mailparser (receive-side; picpeak only had nodemailer). - migration 128: email_configs gains imap_* columns (same shape as smtp_*); seed incomingMail flag; new received_emails audit table. - emailIntakeService: polls the mailbox every 60s when the flag is on AND a mailbox is configured (no-op otherwise); parses each unseen message (mailparser flattens forwarded/nested attachments), drops PDF/JPEG/PNG into the incoming-invoices inbox (inbound_documents, source='email'), logs each message in received_emails (dedupe by message-id; duplicate attachments caught by the existing SHA-256 guard), marks it \Seen. - adminEmail: GET/POST /incoming-config (mirrors SMTP config, masks imap_pass, SSRF host guard) + GET /received (paginated log). - server.js starts the poller at boot. Verified: node -c, require-graph, migration-128 harness (imap columns, flag, received_emails). Frontend (IMAP block under SMTP + Received tab + flag card) follows. |
||
|
Luca
|
c305492845 |
feat(accounting): inbound supplier-invoice capture + expense re-bill (backend)
New top-level Accounting area (gated by an `accounting` feature flag, default OFF, + accounting.view/manage permissions), separate from CRM. Lets an admin capture a received supplier invoice (upload OR phone/tablet camera), give it a disposition, and re-bill the cost to a client onto the relevant event's invoice with a contract-driven markup. Mirrors the billable-hours model. Backend foundation only — frontend pages (inbox / expenses UI + camera widget) and the heavy extractors (Tesseract OCR / Swiss-QR decode / isolated rasterise worker) are follow-ups; extractionService is scaffolded so the upload path is already wired. Migrations 122-125 (numbered above the in-flight feat/crm 117-121): - 122 seed `accounting` flag (default OFF, idempotent) - 123 seed accounting.view/manage permissions + grant super_admin/admin - 124 inbound_documents + expenses + expense_categories (+ seed categories) - 125 contracts Spesen-Zuschlag clause (expense_markup_type/_percent/_flat_minor) API: /api/admin/expenses — inbound capture/list/confirm/categorize, expense CRUD, /:id/rebill (event-scoped; markup = expense override -> contract clause -> 0%; mints an editable scheduled invoice), /:id/supplier-payment, categories. adminFeatureFlags KNOWN_FLAGS/DEFAULT_FLAGS gain `accounting`. Conventions: idempotent hasTable/hasColumn-guarded migrations; money in integer *_minor; QR amount stored separately + untrusted; requirePermission guards; camelCase API <-> snake_case columns; multer + 15MB cap for PDF/JPEG/PNG. VAT/tax handling is v1 capture-only — verify with a Treuhaender before relying. Verified: node -c all files, require-graph smoke test, and a SQLite migration harness (schema + seeds + idempotency + defaults assert green). |
||
|
Luca
|
eb263137b9 |
feat(crm): Project Overview phase 2 — project service + routes
Backend API for the cockpit (admin-only, Model A): - projectService: list/get/create/update, assignEvent (re-point events.project_id), getProjectOverview (rollup — invoices/emails/gallery by event, quotes/contracts by customer since they carry no event_id, hours by project_id, + a milestone timeline), getEmailPreview (actual sent HTML). - adminProjects routes (/api/admin/projects): read=events.view, write=events.manage; the overview gates each money-doc type on the admin's own bills/quotes/contracts .view permission. Registered in server.js. All aggregation queries verified against the real schema on a temp DB. |
||
|
Luca
|
1214b6b762 |
fix(security): re-apply SVG CSP on the direct favicon route (PR #603 blocker)
The /favicon.ico + /apple-touch-icon routes stream the file directly, bypassing the secureStatic middleware that locks down served SVGs. An admin-uploaded SVG favicon with <script> would then run at the top-level origin (stored XSS). Re-apply the same CSP (default-src 'none') + nosniff for .svg here, mirroring secureStatic.js. Reported in the #603 review. |
||
|
Luca
|
7ccfdc1aea |
fix(branding): stream favicon bytes directly (Safari ignores the 302)
The /favicon.ico route 302-redirected to the uploaded file. Firefox/Chrome follow that, but Safari does NOT reliably follow a redirect for favicon requests — it falls back to the HTML <link>, i.e. the bundled picpeak default. Stream the file bytes directly for local /uploads favicons (with a path-containment guard); only external URLs and the missing-favicon fallback still redirect. sendFile sets the content-type from the extension. |
||
|
Luca
|
db3e3270f3 |
fix(branding): serve favicon via backend route so Safari picks it up
Safari requests /favicon.ico and /apple-touch-icon*.png at the site root and is unreliable about honouring JS-injected <link rel=icon>, so an admin-set favicon never showed there (index.html only ships /favicon-32x32.png; a bare /favicon.ico 404'd). - Backend: GET /favicon.ico + /apple-touch-icon(.png|-precomposed.png) resolve the configured branding_favicon_url (redirect to its /uploads path or the absolute URL), falling back to the bundled /favicon-32x32.png. - nginx: exact-match (=) locations proxy those paths to the backend, winning over the static-asset regex that previously served them from the build dir. - DynamicFavicon also emits an apple-touch-icon link (belt-and-braces). Requires a frontend image REBUILD (nginx.conf change) in addition to backend. |
||
|
Luca
|
83fdb47fbf |
feat(installer): install picpeak directly from a backup via trigger file
Closes the six-step DR dance ("onboard throwaway admin → restore via
wizard → log out → log back in with originals") by letting admins
recover an install with zero clicks past `docker compose up`.
Convention: drop a file named `RESTORE_ON_INSTALL` (no extension OR
.txt) into the existing `/backup` bind mount. On next container
start, the new boot hook detects it, runs the restore, and starts
the server with the restored state. Admin opens the browser, login
works first try.
Payload variants:
- empty file → auto-picks newest backup-manifest-*.json from
/backup/manifests/. Useful for "restore the latest".
- path inside the file → uses that specific manifest. Useful for
"I want this older backup, not the most recent".
Safety gates (three layers):
1. Trigger file must exist — no auto-magic, admin signals intent
2. DB must be empty (no events, ≤1 admin) — refuses to clobber
production data
3. Restore failure leaves the trigger file in place for retry on
next container start. Success deletes it so subsequent boots
don't redo the work.
Override hook: INSTALL_FROM_BACKUP_FORCE=true skips guard #2 for the
"I know what I'm doing" edge case (dev env rebuilds, etc).
No docker-compose changes required — uses the bind mount picpeak
already has, env vars are optional. The minimal admin workflow now
matches the bare-minimum mental model: "copy my backup files,
restart the container, log in with original credentials."
Tests: 7 scenarios covering trigger detection, payload variants,
safety gates, success/failure trigger-file lifecycle.
|
||
|
Luca
|
dbcecfe2aa |
feat(restore): self-heal restore_allow_force default ON at boot
Fresh installs of picpeak had `restore_allow_force` defaulting to false (or missing entirely). Combined with the "1 active admin user" pre-restore warning that the fresh-install admin auto-creates, this meant the very first restore on every new install hit: Force restore is not allowed by system settings Admins then had to hand-craft SQL to flip the setting before they could recover their data — at the worst possible moment, when they were already mid-disaster. This isn't security: the admin who can SQL the setting on can also flip it via the UI. It's just a sharp edge that bites every new install once. Cure: boot-time self-heal that seeds restore_allow_force=true only when the row doesn't exist. Existing installs that explicitly set the row (true OR false) are NOT touched — admin policy wins. Pattern mirrors _backupPathsBoot.js and _emailTemplateBoot.js. Default-ON rationale matches Stage A's principle: the cost of forgetting (= can't recover from a disaster) outweighs the friction saved (= adversarial admins can't run forced restores). Audit logging keeps the accountability story intact. |
||
|
Luca
|
302fc6b937 |
feat(backup): config-driven walker via backup_paths table
Stage B of the three-stage backup-hardening plan (Stage A:
inline-DB-dump + fail-loud guard already landed). The file-backup
walker used to hard-code its subdirectory list inside
`getFilesToBackupInternal`, which is the same footgun that hid the
`business-docs` gap for ~6 months — a new feature drops artefacts
under STORAGE_PATH and the maintainer has to remember to edit the
walker.
Now driven by a `backup_paths` table:
- Migration 108 creates the table and seeds the 7 canonical
defaults (events/active, events/archived, thumbnails, previews,
heroes, uploads, business-docs). Seed data lives on the
migration as `DEFAULT_PATHS` so the boot self-heal can re-use it.
- `_backupPathsBoot.js` mirrors `_emailTemplateBoot.js`: on every
boot it diffs the canonical list against the current rows and
`INSERT ... ON CONFLICT DO NOTHING`s the missing ones. Keeps
admin edits intact, picks up new defaults shipped after the
install (Knex won't re-run migration 108). Wired into server.js
just before `startBackupService()`.
- Walker now calls `resolveBackupPaths(config)` which:
* reads `backup_paths WHERE include_in_default=true ORDER BY
display_order`
* falls back to a hard-coded `LEGACY_BACKUP_PATHS` if the
table is missing OR empty (defense in depth — never silently
scans nothing)
* gates each row by its `feature_flag` column (matches how
`backup_include_archived` already worked; data-driven now)
- Backward compatible: `getFilesToBackup(true|false)` still works
for legacy callers and the existing businessDocs test. New
callers should pass the full config object so feature gates
other than `backup_include_archived` evaluate correctly.
Tests:
- new: `backupService.configurableWalker.test.js` — 7 cases
covering canonical seed, toggling include_in_default, runtime
INSERT picked up without restart, feature_flag gating both on
and off, empty-table → LEGACY fallback, boolean backward compat
- all 15 backup-walker integration tests pass
(configurableWalker 7 + inlineDbDump 5 + businessDocs 3)
- frontend build clean
- 4 pre-existing integration failures (webhookDelivery, storage
backend, adminPhotos.reference, imageProcessor.storage) confirmed
unrelated via `git stash` baseline run
Stage C (CRM feature coverage audit + diagnostic UI) follows
in a separate commit.
|
||
|
Luca
|
4812fcdec3 |
feat(backup): admin endpoint to verify CRM document-artefact integrity
Diagnostic for the bug fixed in
|
||
|
Luca
|
83933baeec |
fix(crm): self-heal missing CRM email templates at boot + recover queue
The CRM template seeders (crmEmailTemplates / contractEmailTemplates / eventReminderTemplates) were idempotent and ready, but only contractEmailTemplates was actually called (lazily, by contractService sends). crmEmailTemplates had no caller anywhere — every install that didn't pre-exist its templates failed every quote_sent / invoice_sent / storno_issued / invoice_reminder_* send with "Email template '<key>' not found". The queue processor retries 3 times then leaves the row in status='pending', retry_count=3, silently dead with no admin surface (see project_crm_backlog for the eventual System Health page). Fix: wire all three seeders into server.js startServer() right before startEmailQueueProcessor. The new _emailTemplateBoot.js orchestrates all three and then, for any template_key it just inserted, resets retry_count on stuck email_queue rows of that email_type so the queue processor's next tick picks them back up. Recovery is targeted: unrelated retry-exhausted rows (e.g. SMTP-timeout failures) are not touched. Integration test boots a fresh CRM DB, pre-seeds a stuck quote_sent row plus an unrelated stuck row, runs the boot helper, and asserts: templates landed, stuck quote_sent row was reset, unrelated row was left alone. Already-deployed installs heal automatically on the next backend restart after this lands. |
||
|
Luca
|
d543949188 |
feat(crm): backend code — services + routes + utilities + tests
Brings in the full backend CRM stack on top of the consolidated
migration (
|
||
 paul
|
8b72721812 | fix(public-site): honor dark theme surface colors | ||
|
Paul Nothaft
|
38343e62de |
fix(downloads): apply original-filename toggle to individual downloads too (#507)
Follow-up to #498. The toggle reached zip downloads but single-photo downloads still landed on disk with the renamed `event_individual_NNN.jpg` even when the admin had flipped the setting on. Two reasons, fixed in lockstep: - Frontend overrode the server's Content-Disposition with a hardcoded `<a download="X">` attribute (`gallery.service.ts`, `photos.service.ts`) where X was the sanitized `photo.filename` known to the client. So the backend's correctly-formed `Content-Disposition` never reached the disk write. Added `parseContentDispositionFilename` (RFC 5987 + plain `filename=` fallback) and let the server name win when present. - `secureImages.js` (enhanced/maximum protection's secure-download route) was missed in #498 and still emitted a hardcoded `filename="${photo.filename}"` regardless of the toggle. Wired it through `getUseOriginalFilenames` + `buildContentDisposition` so it matches the regular gallery download path. Also exposed `Content-Disposition` via CORS so split (cross-origin) frontend deployments can still read it from JavaScript. Same-origin Docker deploys already had access; this is a defensive addition for the split case. |
||
|
Paul Nothaft
|
0bc7e2af17 |
feat(og): per-event opt-in to use hero photo as social-share preview (#474)
Background: galleryOgService already serves OG/Twitter Card meta tags to social-crawler User-Agents (WhatsApp, Facebook, Slack, Telegram, Discord, ~21 in total) for /gallery/:slug URLs. Today the og:image is always the brand logo with the inline rationale "no protected photo content". #474 asked for a hero/cover photo preview. The trade-off is that any URL embedded in og:image is fetched unauthenticated by every link-preview crawler — so an opted-in image is effectively public to anyone the gallery URL is shared to. Ship as a per-event boolean, default FALSE, so existing galleries never start surfacing photos without explicit admin intent. Schema (migration 102): - events.og_image_share_enabled BOOLEAN NOT NULL DEFAULT FALSE. Backend: - galleryOgService.buildOgMetadata: when opt-in is on AND a hero_photo_id is set AND the photo has a generated thumbnail, emit og:image as /og/gallery/:slug/cover. Falls back to the brand logo on any miss (deleted hero, missing thumbnail, no opt-in) so a half-configured gallery still gets a polished preview rather than a broken-image src. - galleryOgService.handleGalleryOgCover: new public endpoint that streams the hero thumbnail. Validates slug shape, checks the opt-in flag + hero presence + thumbnail existence; returns 404 on any failure. ETag = thumbnail mtime + photo id so a regenerated thumb busts crawler caches. Cache-Control: public, max-age=300 (short — admins shouldn't wait an hour for a cover swap to land in chat previews). - server.js: mount the new GET /og/gallery/:slug/cover route. The existing nginx ^~ /og/gallery/ proxy block already covers it. - adminEvents.js: validator + persistence on POST + PUT. formatBoolean coercion so SQLite (0/1) and Postgres (boolean) both behave correctly. Frontend: - Event type + UpdateEventData carry og_image_share_enabled. - EventDetailsPage adds a checkbox under the HeroPhotoSelector, disabled when no hero photo is picked. Help text deliberately spells out the public-by-design consequence — admins shouldn't flip this on for a sensitive gallery without realising what they're sharing with link-preview crawlers. Tests: 8 new in galleryOgService.shareImage.test.js — pin the cover-vs-logo decision contract (3 cases) plus the defensive fallbacks (deleted hero, missing thumbnail) and the 404 contract on the cover endpoint (4 cases). The 404 tests assert that ensureThumbnail() is NOT called when opt-in is off, so a future refactor can't accidentally widen the unauthenticated cover endpoint to expose a hero the admin hasn't shared. i18n: en + de hand-translated; nl + pt + ru + fr machine-translated and flagged for native review per project convention. |
||
|
Paul Nothaft
|
3122dd08a8 |
fix(customer-routes): Cache-Control: no-store on customer endpoints (#470)
The trigger: PR #458 mounted requireCustomerPortalEnabled which 410'd every /api/customer/* + /api/admin/customers/* request when the master toggle was off. Some browsers cached that 410 (no Cache-Control header was set, so heuristic freshness applied — the wrong default for an authenticated/sensitive surface). PR #470 reverted the middleware, but a customer whose tab cached the 410 still saw 410s until they hard-refreshed. Add noStoreCache middleware and mount it in front of both route groups. Every response (200, 4xx, 5xx) now carries `Cache-Control: no-store, no-cache, must-revalidate, private` plus the HTTP/1.0 Pragma + Expires fallbacks. Any future transient error from these endpoints can no longer get pinned in browser or proxy caches and outlive its cause. Cost is one setHeader per request; applied per route group rather than globally so static assets + galleries keep their own caching strategy unchanged. Includes a dedicated unit test pinning the header set so a future cleanup pass can't quietly drop it and re-introduce the bug. |
||
|
Luca
|
3f4419356a | revert(customer-portal): make the global flag UI-only, drop the kill-switch middleware | ||
|
Luca
|
c0c6b4c0e8 | chore(customer-portal): align flag-gate comments with new dual-enforcement | ||
|
Luca
|
9091ed4012 | feat(clients): scaffold top-level Clients section with sub-nav around Accounts | ||
|
Luca
|
f048011324 |
fix(server): mount /api/admin/feature-flags route
The route was registered in upstream/beta's server.js but dropped during the rebase squash — the Features tab GET/PUT both 404'd, so the customerPortal flag (and every other flag) couldn't be toggled. Restored the mount in its upstream/beta position. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Luca
|
4fa7225732 |
fix(server): drop missing requireCustomerPortal middleware import
server.js was still requiring ./src/middleware/requireCustomerPortal — a file deleted during the AdvancedFeaturesTab cleanup — which crashed the backend on boot in production (MODULE_NOT_FOUND). The customerPortal feature flag is now enforced on the frontend via <RequireFeature flag="customerPortal" /> route guards (App.tsx) and AdminSidebar visibility. Defence in depth is provided by customerAccountsService.isCustomerPortalEnabled() in adminEvents. Routes themselves are still protected by adminAuth / customerAuth. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Luca
|
087ef45942 |
feat(customers): customer portal (#354) on top of feature-flags reorg
Implements the recurring-customer login surface from the-luap/picpeak#354 plugged into the maintainer's new feature-flag infrastructure (PR #443) instead of a parallel toggle. * New `customerPortal` feature flag (foundation flag for the not-yet-built calendar/quotes/bills/messaging customer surfaces). Defaults FALSE on fresh installs, TRUE on existing installs (events > 0) via migration 095 so live customer accounts don't disappear mid-deployment. * Foundation schema: customer_accounts, customer_invitations, event_customer_assignments, customer_password_resets, plus RBAC permissions customers.view / .create / .delete granted to super_admin + admin system roles. * Backend: /api/admin/customers (invite, list, search, assign, deactivate, reset password) + /api/customer/auth/* + /api/customer/* (login, dashboard, accept-invite, reset). Customer JWT bypass minted via /api/customer/events/:slug/access-token so existing gallery middleware stays untouched. * Frontend: /customer/* route tree gated by RequireFeature flag customerPortal, with login / dashboard / accept-invite / reset pages and a customer-side sidebar layout. /admin/customers and /admin/customers/:id gated identically. * Settings → Features grows a "Customers" section with a Customer portal card. The maintainer's Features tab stays the single source of truth — no parallel Advanced features tab. * CustomerAccountPicker on event create/edit forms hides itself when the flag is off; backend ignores customer_account_ids in that case instead of erroring the whole event save. Translations: en + de hand-translated. nl/pt/ru fall through to en — flagged here as needing native review. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Paul Nothaft
|
15e333681f |
feat(settings): Features tab + sidebar reorg with feature-flag gating
Reorganises the admin sidebar around what users actually do, and adds a
single Features page that gates which feature surfaces appear in the
nav. Shrinks the main sidebar from 11 items to 4-6 (depending on
feature flags) and groups configuration screens into a single Settings
home with six logical sections.
Why
---
The current sidebar mixes three concerns: workspaces (Dashboard, Events,
Archives), feature surfaces (Analytics, Users), and configuration
screens that get touched maybe once a month (Email Settings, Branding,
Event Types, Backup, CMS Pages). That's 11 items, half of them config.
Backend
-------
- New `feature_flags` table (key, value, updated_at, updated_by).
Migration 088 detects existing-vs-fresh installs from the events
table:
* Existing install (events>0) → all 9 flags TRUE so nothing
vanishes from an admin's UI on upgrade.
* Fresh install (events=0) → spec defaults: galleries,
reminderEmails, analytics, userManagement TRUE; calendar,
calendarBooking, quotes, bills, messaging FALSE.
- New `/api/admin/feature-flags` (GET/PUT) under `settings.view` and
`settings.edit`. Server enforces the same dependency rules the
frontend does (galleries always TRUE, quotes=false → bills=false,
calendar=false → calendarBooking=false). PUT writes one
`feature_flags_updated` activity log row with the diff.
Frontend
--------
- `FeatureFlagsContext` provides `useFeatureFlags()` (with staged/save/
reset/isDirty) and `useFeatureEnabled(key)`. Mounted inside
AdminLayout so flag fetches carry the auth cookie. Source of truth
is the server response; staged is a local copy that the Features tab
edits and the Save button PUTs.
- `RequireFeature` route guard for /admin/analytics and /admin/users —
redirects to /admin/dashboard when the corresponding flag is OFF.
- AdminSidebar dropped from 11 to 6 items. Removed: Email Settings,
Branding, Event Types, Backup, CMS Pages (now Settings tabs).
Feature-gated: Analytics, Users.
- Old top-level routes (/admin/email, /admin/branding, /admin/event-
types, /admin/backup, /admin/cms) kept as <Navigate> redirects to
/admin/settings?tab=<key> so existing bookmarks don't 404.
- SettingsPage rewritten with a 6-group inner-nav (General /
Content & Appearance / Communication / Privacy & Security /
Integrations / System) and 19 tabs. New Features tab is the
default landing tab. URL ?tab=<key> roundtrips with state — deep
links and the back button work.
- FeaturesTab renders 9 cards across 5 sections. Toggles enabled for
Analytics + User Management (the two flags that gate sidebar items
in this PR). All other toggles disabled with a "Not yet available"
lockedReason — the cards still render so admins see the roadmap, but
the flag has no UI effect until the surface ships in its own PR. The
galleries card is locked TRUE per spec (foundation, can't be off).
- Live SidebarPreview reflects unsaved staged changes — admins see
what their sidebar will look like before they save.
- New i18n keys across all 5 locales (en, de, nl, pt, ru) for the
Features tab copy, the new Settings group labels, and the lifted
tab titles.
Verified end-to-end
-------------------
- Migration on this dev DB (existing install, 977 events): all 9 flags
set to TRUE.
- Migration on simulated fresh install (events table emptied): spec
defaults applied (5 OFF, 4 ON).
- Backend round-trip: GET → PUT → audit-log entry written, dependency
rule enforced (bills forced false when quotes=false even when bills=
true requested).
- UI Playwright spec: sidebar dropped 5 items, old top-level routes
redirect, Features tab is default, Galleries+Calendar+Quotes+Bills+
Messaging+ReminderEmails toggles disabled, Analytics+Users toggles
enabled, toggling Analytics off + saving updates the sidebar +
redirects /admin/analytics to /admin/dashboard.
- Smoke 13/13 still green; no regressions on existing flows.
|
||
|
Luca
|
5703fcb806 | fix(fonts): drop immutable Cache-Control to allow font replacement rollout | ||
|
Luca
|
bac51fe69a | feat(branding): self-hosted webfonts with filesystem scanner | ||
|
Paul Nothaft
|
851744c3c4 |
feat(upload): async photo processing — backend (PR-B part 1)
Move thumbnail / EXIF / dimensions / watermark / webhook work off the
upload request thread and into a background worker pool. Upload
requests now return 202 in seconds even on NFS-backed storage; the
worker(s) drain the pending queue independently and update each
photo's processing_status to 'complete' or 'failed' on its own.
Schema (migration 085_async_photo_processing.js):
- photos.processing_status enum default 'complete' (existing
rows are already done)
- photos.processing_error populated on 'failed'
- photos.processing_started_at timestamp for janitor recovery
- photos.upload_id groups all photos from one upload
request so the frontend can poll
status by group
- indexes on processing_status and upload_id for queue lookups
services/photoProcessor.js
- queueFilesForProcessing(files, options) — shared helper used by
the admin and gallery upload routes. Moves files to final storage
+ inserts pending rows; returns { uploadId, photos, errors }.
- processPhoto(photoId) — worker-mode: reads original from storage
via withLocalCopy (transparent local/S3), generates thumbnail and
EXIF/dimensions or video metadata, queues watermark, fires
photo.uploaded webhook, marks 'complete'. Throws => caller marks
'failed' with the error message.
- processUploadedPhotos kept untouched — chunkedUploadService still
uses the synchronous path.
services/backgroundProcessor.js (new)
- N independent worker loops per backend instance (default 2,
UPLOAD_PROCESSOR_CONCURRENCY env override).
- Multi-pod safe: postgres SELECT FOR UPDATE SKIP LOCKED, sqlite
UPDATE-with-status-guard. Pods race on rows, exactly one wins.
- Janitor every minute resets photos stuck in 'processing' for >10
minutes (worker died, pod restarted) back to 'pending'.
- UPLOAD_PROCESSOR_DISABLED=true opt-out for CI/test.
- Started from server.js after the other long-running workers.
routes/adminPhotos.js — POST /:eventId/upload
- Replaced batch-of-25 sync processing loop with per-file
move-to-storage + insert-pending. Response is now 202 with
upload_id, count, photo_ids in addition to the legacy
successCount / replacedCount fields the existing frontend reads.
- Per-request temp directory cleanup is now a single idempotent
handler on res.finish/res.close (was three inline blocks for
error paths only, leaking dirs on success — original bug from
contributor analysis).
- GET /uploads/:upload_id/status — JSON snapshot of pending /
processing / complete / failed counts plus per-photo state.
- GET /uploads/:upload_id/stream — SSE upgrade. Polls internally
every 1.5s, emits on snapshot change, ends when all photos
reach a terminal state.
- POST /photos/:photoId/retry — flips a 'failed' photo back to
'pending' so the worker picks it up again.
- GET /:eventId/thumbnail/:photoId now returns 503 with Retry-After
while the photo is still pending/processing, and 422 on 'failed'.
The admin grid renders placeholders accordingly.
routes/gallery.js — POST /:eventId/upload (guest)
- Refactored to use queueFilesForProcessing instead of the synchronous
processUploadedPhotos. Same 202 + upload_id shape.
- GET /:slug/photos now filters processing_status to 'complete' (or
NULL for pre-migration rows) so guests never see in-flight photos.
Side-effect timing change:
- photo.uploaded webhook now fires from the worker after the photo
is actually processed (thumbnail + dimensions populated) instead
of from inside the upload request. Same payload fields. Worth a
one-line note in the changelog.
|
||
|
Paul Nothaft
|
5275621fcd |
fix(share): OG/Twitter-card metadata for gallery share URLs (#333)
WhatsApp / Slack / Facebook / Twitter previews showed nothing useful for shared gallery links — the SPA's stub index.html has no OG tags and the meta-injection in DynamicFavicon happens at runtime, which crawlers never see (they don't execute JS). Add a backend OG handler at /og/gallery/:slug that returns minimal HTML with proper og:* and twitter:* meta sourced from the event row + branding settings (event name, formatted date, welcome_message excerpt as description, configured logo as the preview image, FRONTEND_URL-based canonical). Honours slug redirects so renamed galleries still get rich previews. Wire crawler detection in both nginx configs (production and dev) — UA match against the standard list (facebookexternalhit, WhatsApp, Slackbot, Twitterbot, Discordbot, LinkedInBot, etc.) triggers an internal rewrite to /og/gallery/:slug, while humans fall through to the SPA via try_files. The OG endpoint is also wired into the native-install SPA fallback in server.js for setups that bypass nginx. The OG image is intentionally the brand logo, not a gallery photo — crawlers fetch it without auth, and password-protected gallery photos must not leak via share previews. |
||
|
Paul Nothaft
|
1b717ce5ed |
feat: native S3 storage backend (#328) + presigned download follow-up
Lets PicPeak write photos, thumbnails, hero images, watermarks, and archive zips to any S3-compatible bucket (AWS S3, MinIO, Cloudflare R2, Backblaze B2, Wasabi, DigitalOcean Spaces) instead of the local filesystem. Selected via STORAGE_BACKEND=local|s3. Architecture - backend/src/services/storage/StorageBackend.js — abstract interface (put/get/exists/stat/delete/list/copy/rename/signedUrl/putFromFile/ getToFile) — typedef-only, documents the contract. - LocalFsStorage.js — wraps fs with atomic-write-via-tmp-rename, path traversal protection, list-as-walker. - S3StorageBackend.js — thin wrapper around the existing S3StorageAdapter (used by backupService) mapping it onto the canonical interface; supports optional STORAGE_S3_PREFIX namespace. - index.js — factory selected by STORAGE_BACKEND with startup ping (HEADs sentinel key on S3, fs.stat on local) so misconfig fails fast before the first request. Consumer refactors (~12 services + routes), each parametrized over the abstraction: - imageProcessor / videoProcessor — pipe Sharp/ffmpeg output through storage.put; expose withLocalCopy() helper for S3-mode regeneration paths that need a local file for sharp/ffmpeg. - archiveService / downloadZipService — finalize zip in tmp dir, then storage.putFromFile. Atomic-rename pattern preserved on local; S3 emulates via copy + delete (worker prunes orphaned .tmp.* on startup). - photoProcessor / photoReplacementService / adminPhotos upload+delete / routes/v1/events.js POST /events/:id/photos / routes/events.js — every upload path now goes storage.putFromFile(temp) → unlink temp. - gallery.js bulk-download (cached + on-the-fly + selected) — managed photos via storage.get, external-mode unchanged. - protectedImages / secureImages / photoResolver — read via storage.get; resolvePhotoStorageKey returns the canonical key. - watermarkService / watermarkGeneratorService — persistent watermarks via storage.put. - fileWatcher — bails out with a clear log warning when STORAGE_BACKEND=s3 (chokidar can't watch S3); auto-import lands via the S3 prefix walker introduced in the follow-up commit. - expirationChecker — small touch (event.expired webhook fire from #327 shipping in the next commit). Migration tooling - backend/scripts/migrate-storage.js — one-shot --dry-run capable script that walks photos.path, thumbnail_path, hero_path, watermark_path and events.archive_path/download_zip_path; streams local → S3; sha256 size-match skip for idempotent re-run; failures CSV. Presigned-URL "Download All" (#328 follow-up shipped in this commit) - routes/gallery.js — when STORAGE_BACKEND=s3 + event.allow_presigned_download + downloads enabled + watermark NOT enabled, /download-all returns a 302 redirect to a 5-minute presigned S3 URL. Per-event opt-in surface ships in the next commit's UI. Tests - backend/__tests__/integration/storageBackend.test.js — parametrized contract suite running against BOTH LocalFs AND MinIO (18 tests, both backends — 36 cases total). - backend/__tests__/integration/imageProcessor.storage.test.js — same parametrized pattern for the image processor (10 tests × 2 backends). - backend/__tests__/integration/backup-s3.test.js — bootstrap fix: drop the redundant initDb() (001_init handles it) and remove schema-drift in configureS3Backup (app_settings has no created_at anymore and the unique constraint is on setting_key alone, not composite). 0/12 → 7/12 (5 remaining are unrelated assertion drift). - backend/src/services/photoResolver.js — mixed-source events (reference mode with managed-uploaded photos) now fall back to managed when external_relpath is missing instead of throwing. - tests/e2e/s3-storage-roundtrip.spec.ts — Playwright spec that auto-skips against local backend; full upload → serve → delete round-trip when run against an S3-mode backend. Server wiring (server.js) - initStorage() called after database init, before rate limiters. - This commit's diff also includes the webhook delivery worker startup and the S3 auto-importer startup. Those features ship in the next two commits — co-located here for one bisectable diff per file. Docs + ops - README §"Storage Backends" — capability matrix, switching playbook, IAM policy snippet, MinIO/R2/B2 examples. - README §"Webhooks" — also added here (full diff bundled). - .env.example — STORAGE_BACKEND + STORAGE_S3_* + STORAGE_AUTO_IMPORT documented; WEBHOOK_* added in the same diff. - .gitignore — re-anchor the existing `storage/` rule to `/storage/` so backend/src/services/storage/ (the new abstraction code) is trackable. The runtime ./storage/ data dir stays ignored. Out of scope for v1 (per the issue): presigned URLs for individual photo display (always streamed for protection middleware), CDN integration, hybrid hot/cold tiers, S3 → local migration, multi-bucket per-event. |
||
|
Paul Nothaft
|
808b15bafb |
feat: public v1 API + token management + OpenAPI docs (#322)
Adds a long-lived bearer-token mechanism + scoped REST surface designed
for n8n-style automation: create a gallery, upload photos, fetch the
share URL — all via documented HTTPS endpoints instead of poking at the
admin UI's internal routes.
API
- Migration 081 adds `api_tokens` (hashed_token, scopes, owner FK,
last_used/expires/revoked timestamps).
- New apiTokenAuth middleware: parses `Authorization: Bearer pp_live_…`,
resolves to the owner admin user, attaches `req.admin` so existing
permission decorators (events.create etc.) still work. Token-level
scope check (read/write/admin) layers on top as defence in depth —
a leaked read-only token cannot mutate even if its owner is super_admin.
- adminApiTokens route exposes list/create/revoke for admins (cookie-
authed). Plaintext token is returned exactly once on creation.
- v1 surface mounted at /api/v1: POST/GET /events, GET /events/:id,
POST /events/:id/photos (multipart, single file), GET
/events/:id/share-link. Each endpoint annotated with @openapi JSDoc.
Documentation
- swagger-jsdoc + swagger-ui-express produce a live spec at
/api/openapi.json and a Swagger UI at /api/docs (admin-gated).
- backend/scripts/generate-openapi.js writes docs/openapi.{json,yaml}
to the repo so the spec is versioned.
- scripts/sync-api-docs.sh runs in pre-push: regenerates the spec and
copies it into the picpeak-docs Nextra site at app/api/. Writes only,
never commits or pushes the docs repo (PUSH_SKIP_DOCS=1 to bypass).
Frontend
- New Settings → API Tokens tab: generate, list, revoke. Plaintext
tokens are shown once with a copy-to-clipboard control.
|
||
|
Paul Nothaft
|
8d0fb8e157 |
chore: expose pid + uptime on /health for crash-detection monitors
Adds `pid` and `uptime` fields to the /health response so external monitors (and the local E2E watchdog) can detect a silent process restart between two checks — e.g. an unhandled rejection that crashes Node and Docker quietly relaunches the container. Also adds .gitignore patterns for a local-only E2E suite that lives in tests/e2e/local/ on individual machines and is never pushed. |
||
|
Paul Nothaft
|
ad4e5a7506 |
feat: guest selections with per-person identity (#292)
Introduces a new "Per-guest selections" identity mode for event feedback, letting each visitor register under their own name so their likes/favorites/comments/ratings are tracked independently. Includes admin insights (list, per-guest detail, aggregate view, export) and advanced identity features (forget-me, email recovery, invite tokens, merge). New event-level setting - event_feedback_settings.identity_mode = 'simple' | 'guest' (default 'simple' → zero behavior change for existing events). - Admin UI radio under Feedback Settings to toggle per event. Root cause of the previous "all guests share state" bug - generateGuestIdentifier() was sha256(ip + userAgent), so every visitor on the same WiFi + similar device collided into one identity. - Now: when a verified guest JWT is present (x-guest-token header), req.guest.identifier takes precedence — per-person rate limits and per-person deduplication. Phase 1 — identity layer - Migration 078: new gallery_guests, guest_invites, guest_verification_ codes tables; identity_mode column + check constraint; nullable guest_id FK on photo_feedback. - New guest JWT type scoped to (eventId, guestId). - New middleware guestAuth.resolveGuest (non-blocking) + requireGuest. - POST /gallery/:slug/guest, GET /guest/me, DELETE /guest/me. - Gallery feedback route enforces guest identity in guest mode and reads name/email from the verified token (never from the body). - Frontend GuestIdentityContext + GuestNamePromptModal; axios interceptor injects x-guest-token on gallery API calls. - Feedback-only blocking: gallery opens freely, prompt only on first interactive feedback action. - Admin "Guests" tab (conditional on identity_mode='guest') with the AdminGuestsList component. Phase 2 — admin insights - GET /admin/events/:eventId/guests list + aggregated counts. - GET /admin/events/:eventId/guests/:guestId detail with per-type groupings; AdminGuestDetail modal with thumbnail grid + tabs. - GET /admin/events/:eventId/guests/aggregate sorted by distinct guest pick count; GuestSelectionsAggregate component. - Per-guest export (txt/csv/json) and bulk export-all ZIP. Phase 3 — polish - 3.1 Self-service forget-me link in gallery footer. - 3.2 Email-based identity recovery: POST /guest/recover sends a 6-digit code via the existing emailProcessor, POST /guest/verify exchanges it for a token (rate-limited, enumeration-safe). - 3.3 Admin invite tokens: pre-mint identities, share URLs with ?invite=, single-use redemption stripping the param from history. - 3.4 Admin merge endpoint reassigns feedback + soft-deletes sources. Shared helper - useGalleryFeedbackAction hook wraps the identity-check logic for inline like buttons across Masonry/Grid/Justified/Mosaic/Carousel/ Timeline/Premium layouts. Backwards compatibility - Existing events default to 'simple' after migration; behavior unchanged. - Legacy photo_feedback rows keep guest_id NULL; admin shows them in the generic feedback moderation view as before. - feedback_count denormalized stat now uses COALESCE(guest_id, guest_identifier) so per-guest counts are accurate without touching legacy rows. Verified end-to-end against local Docker - Migration clean on existing data. - Simple mode unchanged (no prompt, legacy flow). - Guest mode: Alice registers on click, tokens persist in sessionStorage, feedback rows carry guest_id. - Carol via invite link auto-redeems, sees Alice's "1 likes" badge. - Admin Guests tab shows both with correct counts; detail modal displays thumbnail grid with badges; aggregate view sorts by picker count (photo 227 = 2, others = 1); CSV/JSON export matches DB. - Merge Carol into Alice: feedback reassigned, Carol soft-deleted, Alice count = 4. |
||
|
Paul Nothaft
|
23cd9cb680 |
fix: address Shannon security assessment findings (37 vulnerabilities) (#254)
Remediate 4 Critical, 18 High, 10 Medium, and 5 Low vulnerabilities identified in the Shannon security assessment (2026-03-20). Critical fixes: - Command injection via rsync SSH key path (INJ-VULN-01) - Self-escalation to super_admin role (AUTHZ-VULN-11) - Invite super_admin backdoor (AUTHZ-VULN-12) - Handlebars SSTI in email templates (INJ-VULN-05) Authentication hardening: - Rate limit on share-link login (AUTH-VULN-01) - X-Forwarded-For spoofing bypass (AUTH-VULN-02) - reCAPTCHA fails closed when misconfigured (AUTH-VULN-03) - Token revocation on admin/gallery logout (AUTH-VULN-04/05) - Cookie Secure flag defaults true in production (AUTH-VULN-06) - Remove JWT from admin login response body (AUTH-VULN-07) - Timing-safe gallery slug validation (AUTH-VULN-09) - Account lockout fails closed on DB error (AUTH-VULN-12) - Session endpoint checks token revocation Path traversal & file access: - checksums endpoint path containment (INJ-VULN-03) - manifest validate path containment (INJ-VULN-04) XSS prevention: - Block SVG data URIs in CSS sanitizer (XSS-VULN-01) - Email preview iframe sandbox (XSS-VULN-02) - SSR branding HTML escaping (XSS-VULN-03) - User-Agent sanitization in feedback (XSS-VULN-04) Authorization (IDOR): - Event ownership middleware for all admin routes - Cross-admin user profile read restriction (AUTHZ-VULN-10) SSRF & infrastructure: - Private IP validation for SMTP, S3, rsync hosts - Replace inline JWT with standard adminAuth middleware - CSRF Content-Type enforcement on mutating API endpoints - CSP headers in nginx location blocks Token revocation fix: - Remove overly broad orWhere clause that invalidated all future tokens - Allow empty-body POST requests (logout) in CSRF middleware Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local> |
||
|
Paul Nothaft
|
fc75bcdfc3 |
fix: video upload media type, select all, and dimension repair (#203, #220, #180)
- Fix admin video upload missing media_type/mime_type and video processing (#203) - Fix Gallery-Premium Select All using atomic callbacks instead of stale closure loop (#220) - Add photo dimension repair endpoint and admin UI (#180) - Add E2E tests for all three fixes |
||
|
Paul Nothaft
|
2b25d81144 |
security: comprehensive hardening across frontend, backend, and infrastructure
- Disable production source maps and hide nginx version - Reduce JSON body limit from 10gb to 50mb (uploads use multer, not JSON) - Strip database info and error details from health endpoint - Mask reCAPTCHA secret key in admin settings API responses - Whitelist sort/order query parameters in events and photos endpoints - Stop reflecting arbitrary origins in static file CORS headers - Align nginx security headers with backend Helmet CSP, remove deprecated X-XSS-Protection - Strip EXIF metadata from generated thumbnails and hero images - Bind postgres/redis dev ports to localhost in docker-compose configs - Add safeExec utility (spawn with shell:false) to prevent command injection - Convert all exec/execAsync calls in backup, restore, and database backup services to use safe spawn-based helpers |
||
|
Paul Nothaft
|
9c2a0d272a |
feat: add admin dark mode and SEO/robots.txt settings
Admin Dark Mode: - Add AdminDarkModeContext with light/dark/system preference - Update all admin components with Tailwind dark: classes - Add dark mode toggle in admin header - Persist preference in localStorage SEO Settings: - Add robots.txt configuration in Settings > SEO tab - Block AI crawlers (GPTBot, ChatGPT-User, etc.) with toggle - Custom robots.txt rules management - Add RobotsMetaTags component for gallery pages - Backend service for dynamic robots.txt generation - Database migration for SEO settings storage UI/UX Improvements: - Consistent dark mode styling across all admin pages - Update gallery components with themed CSS classes - Fix input, card, and button styling for dark mode |
||
|
Paul Nothaft
|
f8881d5bd6 |
feat: add customizable event types with admin management
Implements GitHub issue #139 - allows users to create and manage custom event types beyond the default presets (wedding, birthday, corporate, other). Backend: - Add event_types table migration with default system types - Create eventTypeService for CRUD operations with legacy fallback - Add adminEventTypes routes with full REST API - Update event validation to use dynamic event types - Update slug generation to use custom slug_prefix Frontend: - Add EventTypesPage with full CRUD admin interface - Add eventTypes.service.ts API client - Update CreateEventPage to fetch types dynamically - Add Event Types navigation in admin sidebar - Add i18n translations (EN/DE) Backward compatible: existing galleries continue to work, legacy types accepted even if database is empty via fallback mechanisms. |