Files
picpeak/backend/src/utils/databaseEngine.js
T
Paul NothaftandPaul 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 <[email protected]>
2026-08-13 18:51:20 +02:00

450 lines
20 KiB
JavaScript

'use strict';
/**
* Which database engine is this process actually using, and is that what the
* operator intended? (#1038)
*
* knexfile.js selects its config block by NODE_ENV, and the `development`
* block defaults to sqlite3. The Docker image never set NODE_ENV, so every
* deployment that doesn't go through our compose files — Kubernetes, Helm,
* plain `docker run` — silently landed on SQLite and ignored DB_HOST /
* DB_USER / DB_PASSWORD entirely. wait-for-db.sh is shell and reads DB_HOST
* directly, so the same container happily reported "PostgreSQL is up" while
* the app wrote to a SQLite file.
*
* Now that the image pins NODE_ENV=production, those installs would resolve to
* Postgres on their next pull — and come up against an EMPTY database, which
* reads as total data loss. Blocking the boot would protect the data but take
* the galleries offline for an operator who did nothing wrong, so instead we
* STAY on SQLite (the engine that holds their data), say so loudly, and point
* at the migration script. Nothing moves until the operator decides.
*
* decideBootEngine() is pure so the matrix is testable; the probes around it
* are deliberately thin.
*/
const fs = require('fs');
const { resolveSqliteFilename } = require('./sqlitePath');
// Shared with knexfile so the engine guard can never probe a different target
// than the application opens (#1038).
const { pgConnectionFromEnv } = require('./pgConnection');
// Diagnostics go through an injected sink, never a module-level logger: the
// resolver's STDOUT is a protocol channel (wait-for-db.sh captures it), and the
// app logger writes there whenever LOG_TO_CONSOLE=true.
const warnToStderr = (msg) => process.stderr.write(`${msg}\n`);
/** Absolute path of the SQLite file this install would use — the SAME
* resolution knexfile performs, so the guard can never probe a different file
* than the one knex opens. */
function resolveSqlitePath() {
return resolveSqliteFilename(process.env.DATABASE_PATH || './data/photo_sharing.db');
}
/** Human-readable "engine + target", safe to log — never includes credentials. */
function describeEngine(knexConfig) {
const client = knexConfig?.client || 'unknown';
if (client === 'pg') {
const c = knexConfig.connection || {};
return `postgres (${c.host || 'unknown-host'}:${c.port || 5432}/${c.database || 'unknown-db'})`;
}
const filename = knexConfig?.connection?.filename || resolveSqlitePath();
return `sqlite (${filename})`;
}
/**
* Which engine should this boot actually use?
*
* @param {object} state
* @param {string} state.configuredClient what knexfile resolved to
* @param {string=} state.explicitClient DATABASE_CLIENT, if the operator set it
* @param {boolean} state.pgHasData the Postgres target already holds galleries
* @param {boolean} state.sqliteHasData a SQLite file exists AND holds events
* @returns {{ client: string, overridden: boolean, reason: string|null }}
*/
function decideBootEngine({
configuredClient, explicitClient, pgHasData, sqliteHasData,
migrationInProgress = false, migrationCompleted = false, pgConfigured = false,
}) {
// A migration that never finished outranks everything, including an explicit
// DATABASE_CLIENT=pg: Postgres may hold a half-written copy while SQLite is
// still the database of record. Deleting the marker is the documented
// override. (Explicit sqlite3 already points at the data, so leave it alone.)
if (migrationInProgress && sqliteHasData && explicitClient !== 'sqlite3') {
return { client: 'sqlite3', overridden: true, reason: 'migration-incomplete' };
}
// The data was migrated to Postgres, but nothing in the environment says so:
// DATABASE_CLIENT is unset and NODE_ENV still resolves to the development
// block, i.e. sqlite3. That is the state the affected installs are IN — it is
// why they ended up on SQLite in the first place — so an operator can easily
// migrate before fixing it. The source file has been renamed away by then, so
// honouring the implicit sqlite3 would create a NEW, empty database and serve
// it. The marker is durable proof of where the data actually is.
if (!explicitClient && configuredClient !== 'pg' && migrationCompleted && pgConfigured) {
return { client: 'pg', overridden: true, reason: 'migrated-to-postgres' };
}
// An explicit DATABASE_CLIENT is an instruction, not a guess. Never override
// it — this is also the documented way to force Postgres and start fresh.
if (explicitClient) {
return {
client: explicitClient,
overridden: false,
reason: explicitClient === 'pg' && sqliteHasData && !pgHasData
? 'explicit-pg-leaves-sqlite-behind'
: null,
};
}
// A migration started and never finished. Postgres may hold a partial copy,
// which would otherwise read as "occupied" and win — while SQLite is still
// the database of record.
if (configuredClient === 'pg' && migrationInProgress && sqliteHasData) {
return { client: 'sqlite3', overridden: true, reason: 'migration-incomplete' };
}
// Both sides hold data and nothing records which is authoritative. This is
// the shape of an install that ran on Postgres, silently fell to SQLite when
// NODE_ENV was lost, and kept working there: the Postgres rows are real but
// stale, and the SQLite rows are real and newer. A completed migration would
// have left a marker; without one, guessing either way hides data and splits
// subsequent writes across two databases. Stop and let a human decide.
if (configuredClient === 'pg' && !migrationCompleted && pgHasData && sqliteHasData) {
return { client: null, overridden: false, reason: 'ambiguous-both-populated' };
}
// Configured for Postgres, Postgres holds no galleries, and real data sits in
// a SQLite file: this install has been unknowingly running on SQLite. Keep
// serving from where the data actually is. Deliberately keyed on DATA, not on
// "has tables" — a stray migration run against the empty Postgres would
// otherwise blind this check and strand the operator on an empty database.
if (configuredClient === 'pg' && !pgHasData && sqliteHasData) {
return { client: 'sqlite3', overridden: true, reason: 'stranded-sqlite-data' };
}
return { client: configuredClient, overridden: false, reason: null };
}
/** Marker written by scripts/migrate-sqlite-to-postgres.js once the data is in
* Postgres. Its presence pins the install to Postgres for good: without it, a
* Postgres that is merely EMPTY (every gallery deleted, say) would look
* identical to one that was never migrated, and the boot would fall back to a
* stale SQLite file that has been out of date since the migration. */
function migrationMarkerPath(sqlitePath = resolveSqlitePath()) {
return `${sqlitePath}.migrated-to-postgres`;
}
function hasMigrationMarker(sqlitePath = resolveSqlitePath()) {
return fs.existsSync(migrationMarkerPath(sqlitePath));
}
/** The marker's contents, or null when absent/unreadable. */
function readMigrationMarker(sqlitePath = resolveSqlitePath()) {
try {
return JSON.parse(fs.readFileSync(migrationMarkerPath(sqlitePath), 'utf8'));
} catch (_) {
return null;
}
}
/** `host:port/database`, the identity the migration records and compares. */
function currentPgTargetId() {
const c = pgConnectionFromEnv();
return `${c.host}:${c.port}/${c.database}`;
}
/** Written before the migration touches Postgres, cleared only on success.
* While it exists, Postgres may hold a PARTIAL copy — or just the bootstrap
* admin that schema creation seeds — and SQLite is still the authoritative
* database. Without this pin, a migration that failed after writing anything
* to Postgres would make the next boot switch engines and hide the real data. */
function migrationInProgressPath(sqlitePath = resolveSqlitePath()) {
return `${sqlitePath}.migration-in-progress`;
}
function hasMigrationInProgress(sqlitePath = resolveSqlitePath()) {
return fs.existsSync(migrationInProgressPath(sqlitePath));
}
// Tables that are EMPTY on a freshly migrated schema, so a row in any of them
// means a human has used this install. Deliberately wider than `events`:
// judging occupancy by galleries alone would abandon an install whose galleries
// were all deleted but whose admins, customers and accounting records remain.
// Mirrors USER_DATA_TABLES in scripts/migrate-sqlite-to-postgres.js.
const USER_DATA_TABLES = [
'events', 'photos', 'photo_feedback', 'admin_users', 'customer_accounts',
'quotes', 'invoices', 'projects', 'expenses', 'inbound_documents',
];
// core/001_init.js seeds an admin with must_change_password = true when
// ADMIN_PASSWORD is set; setupService writes false once a human completes
// first-run setup. So the FLAG, not the table, is what distinguishes an
// untouched bootstrap row from a real account. Dropping the whole table (as an
// earlier revision did) made a legitimately set-up Postgres look empty, which
// would hand the install to a stale SQLite file and lose the admin's
// credentials and configuration.
const isUntouchedBootstrapRow = (v) => v === true || v === 1 || v === '1';
// Has anyone actually USED this install's admin accounts? Layered, because no
// single column survives every path:
// - more than one admin → somebody created accounts
// - any admin has logged in → real use, even if the password was later reset
// - must_change_password false → first-run setup was completed
// Only the exact shape core/001_init.js leaves behind — one admin, never logged
// in, still flagged — reads as an untouched bootstrap seed.
function adminsIndicateUse(rows) {
if (rows.length > 1) return true;
return rows.some((r) => r.last_login || !isUntouchedBootstrapRow(r.must_change_password));
}
async function countsAsUse(conn, table, { ignoreBootstrapAdmins }) {
if (table === 'admin_users' && ignoreBootstrapAdmins) {
const cols = ['must_change_password'];
if (await conn.schema.hasColumn('admin_users', 'last_login')) cols.push('last_login');
return adminsIndicateUse(await conn('admin_users').select(cols));
}
const row = await conn(table).count('* as count').first();
return Number(row?.count || 0) > 0;
}
async function anyUserData(conn, { ignoreBootstrapAdmins = false } = {}) {
for (const table of USER_DATA_TABLES) {
if (!(await conn.schema.hasTable(table))) continue;
if (await countsAsUse(conn, table, { ignoreBootstrapAdmins })) return true;
}
return false;
}
/** True when a SQLite file exists and carries user data. */
async function probeSqliteData(sqlitePath = resolveSqlitePath(), onWarn = warnToStderr) {
if (hasMigrationMarker(sqlitePath)) return false;
if (!fs.existsSync(sqlitePath)) return false;
const knex = require('knex');
const probe = knex({
client: 'sqlite3',
connection: { filename: sqlitePath },
useNullAsDefault: true,
});
try {
// Same discrimination as the Postgres side. An accidental SQLite database
// gets a seeded admin from core/001_init.js when ADMIN_PASSWORD is set, and
// counting that as use would make a healthy Postgres install look like a
// both-populated conflict and refuse to boot. A setup-completed or
// logged-in admin still counts.
return await anyUserData(probe, { ignoreBootstrapAdmins: true });
} catch (err) {
// Unreadable or corrupt: fail CLOSED. Reporting "no data" here would switch
// the install to an empty Postgres — the precise failure this module exists
// to prevent. Staying on SQLite surfaces the real error instead.
onWarn(
`[database-engine] SQLite at ${sqlitePath} exists but could not be probed (${err.message}); `
+ 'assuming it holds data and staying on it.'
);
return true;
} finally {
await probe.destroy();
}
}
/** True when the configured Postgres target already holds user data. */
async function probePgData(pgConnection, onWarn = warnToStderr) {
const knex = require('knex');
const probe = knex({ client: 'pg', connection: pgConnection, pool: { min: 0, max: 1 } });
try {
// Two very different failures hide behind one catch, and they need opposite
// answers, so establish reachability first — this branch returns, so
// everything below it is reachable-by-construction.
try {
await probe.raw('SELECT 1');
} catch (err) {
// Cannot reach Postgres at all. The app could not run on it either way,
// so report "occupied" to avoid diverting a healthy pg install to a stale
// SQLite file over a transient network blip — startup then fails with the
// real connection error, exactly as it always has.
onWarn(`[database-engine] Postgres unreachable while probing (${err.message}); leaving the configured engine alone.`);
return true;
}
try {
// Substantive use only: an untouched bootstrap admin does not make a
// Postgres target worth switching to, but a completed setup does.
return await anyUserData(probe, { ignoreBootstrapAdmins: true });
} catch (err) {
// Connected, but the query failed — a half-built or damaged schema. That
// is NOT evidence of data: reporting "occupied" here would boot the empty
// Postgres and hide a populated SQLite file, the exact failure this guard
// exists to prevent. Say "not proven occupied" and let the SQLite side win
// if it actually holds data.
onWarn(`[database-engine] Postgres reachable but could not be inspected (${err.message}); treating it as unproven rather than occupied.`);
return false;
}
} finally {
await probe.destroy();
}
}
const CONFLICT_MESSAGE = (sqlitePath, pgTarget) => `
${'='.repeat(78)}
REFUSING TO START — two databases, both with data, and no record of which is current.
sqlite : ${sqlitePath}
postgres : ${pgTarget}
This is what an install looks like after it ran on PostgreSQL, lost NODE_ENV or
DATABASE_CLIENT, and kept working on SQLite without anyone noticing (see
https://github.com/PicPeak/picpeak/issues/1038). The PostgreSQL rows are real
but probably old; the SQLite rows are real and probably newer.
Starting either one would hide the other's galleries and split every new upload
across two databases, so PicPeak will not choose for you. Compare them, then say
which is authoritative:
DATABASE_CLIENT=sqlite3 keep serving the SQLite file (its data is newer)
DATABASE_CLIENT=pg keep serving PostgreSQL
To combine them, start on SQLite and run: node scripts/migrate-sqlite-to-postgres.js
(it replaces the PostgreSQL contents with the SQLite data and records the switch).
${'='.repeat(78)}
`.trim();
const STRANDED_WARNING = (sqlitePath, pgTarget) => `
${'='.repeat(78)}
STILL RUNNING ON SQLITE — Postgres is configured but empty.
data in use : ${sqlitePath}
configured : ${pgTarget} (no galleries in it)
This install has been running on SQLite. Until now the image left NODE_ENV
unset, so knexfile.js fell back to its development block and ignored DB_HOST /
DB_USER / DB_PASSWORD — see https://github.com/PicPeak/picpeak/issues/1038.
Nothing has changed for you: your galleries are served from the SQLite file
above, exactly as before. Switching engines now would start from an empty
database, so PicPeak will not do that on its own.
To move your data to Postgres when you are ready:
node scripts/migrate-sqlite-to-postgres.js
It copies every row into Postgres and leaves the SQLite file untouched as a
fallback. To go to Postgres WITHOUT the data, set DATABASE_CLIENT=pg.
${'='.repeat(78)}
`.trim();
/**
* Resolve the engine for this boot, log what happened, and return the client
* the process should use. Called before migrations touch anything.
*/
async function resolveBootEngine({ knexConfig, logger }) {
const explicitClient = process.env.DATABASE_CLIENT || null;
const configuredClient = knexConfig?.client;
const sqlitePath = resolveSqlitePath();
// Probe whenever Postgres is the engine in play — including when it was named
// explicitly, otherwise the "leaving SQLite behind" warning is unreachable.
const effectiveClient = explicitClient || configuredClient;
const migrationInProgress = hasMigrationInProgress(sqlitePath);
const marker = readMigrationMarker(sqlitePath);
const migrationCompleted = hasMigrationMarker(sqlitePath);
// The marker vouches for ONE Postgres. If the configuration now points at a
// different one, it says nothing about that target — and trusting it would
// boot an unrelated empty database while the real data sits in the recorded
// one and in the renamed rollback copy.
const markerTargetMismatch = Boolean(
migrationCompleted && marker && marker.target && marker.target !== currentPgTargetId(),
);
const pgConfigured = Boolean(process.env.DB_HOST || process.env.DB_PASSWORD);
// Probe when Postgres is in play, and also whenever a migration is pinned or
// finished — those decisions need to know what each side holds.
const probing = effectiveClient === 'pg' || migrationInProgress || migrationCompleted;
const decision = decideBootEngine({
configuredClient,
explicitClient,
pgHasData: probing
? await probePgData(
knexConfig.client === 'pg' ? knexConfig.connection : pgConnectionFromEnv(),
(m) => logger.warn(m),
)
: true,
sqliteHasData: probing ? await probeSqliteData(sqlitePath, (m) => logger.warn(m)) : false,
migrationInProgress,
migrationCompleted,
pgConfigured,
});
if (markerTargetMismatch) {
logger.error(`
${'='.repeat(78)}
REFUSING TO START — this install was migrated to a different PostgreSQL.
migrated to : ${marker.target}
configured : ${currentPgTargetId()}
${migrationMarkerPath(sqlitePath)} records where the data was moved. The current
settings point somewhere else, so starting would open an unrelated database and
present an empty installation while your galleries stay in the one above.
Either restore the original connection settings, or — if this move is deliberate
and the data is already in the new target — update the "target" field in that
marker file to match.
${'='.repeat(78)}
`.trim());
return { client: null, overridden: false, reason: 'marker-target-mismatch' };
}
if (decision.reason === 'ambiguous-both-populated') {
logger.error(CONFLICT_MESSAGE(sqlitePath, describeEngine({
client: 'pg', connection: pgConnectionFromEnv(),
})));
return decision;
}
if (decision.reason === 'migrated-to-postgres') {
logger.warn(
`This install's data was migrated to PostgreSQL (${migrationMarkerPath(sqlitePath)}), but the `
+ 'environment still resolves to SQLite. Using PostgreSQL — set NODE_ENV=production (or '
+ 'DATABASE_CLIENT=pg) to make that explicit.'
);
} else if (decision.reason === 'migration-incomplete') {
logger.warn(
`A SQLite → PostgreSQL migration did not finish (${migrationInProgressPath(sqlitePath)} is still `
+ 'present), so PostgreSQL may hold a partial copy. Staying on SQLite, which is still the '
+ 'database of record. Re-run scripts/migrate-sqlite-to-postgres.js with the backend stopped; '
+ 'delete that file only if you have decided to abandon the migration.'
);
} else if (decision.overridden && decision.reason === 'stranded-sqlite-data') {
logger.warn(STRANDED_WARNING(sqlitePath, describeEngine(knexConfig)));
} else if (decision.reason === 'explicit-pg-leaves-sqlite-behind') {
logger.warn(
'DATABASE_CLIENT=pg is set explicitly, so PicPeak is starting on an empty Postgres while '
+ `gallery data exists at ${sqlitePath}. Run scripts/migrate-sqlite-to-postgres.js to bring it across.`
);
}
// Describe what was DECIDED, not what knexfile said: after a marker override
// knexConfig still describes SQLite while the process goes to Postgres.
logger.info(`Database engine: ${decision.client === 'pg'
? describeEngine(knexConfig.client === 'pg' ? knexConfig : { client: 'pg', connection: pgConnectionFromEnv() })
: `sqlite (${sqlitePath})`}`);
return decision;
}
module.exports = {
resolveSqlitePath,
pgConnectionFromEnv,
isUntouchedBootstrapRow,
adminsIndicateUse,
migrationMarkerPath,
hasMigrationMarker,
readMigrationMarker,
currentPgTargetId,
migrationInProgressPath,
hasMigrationInProgress,
describeEngine,
decideBootEngine,
probeSqliteData,
probePgData,
resolveBootEngine,
};