Bug fixes - April 27th 2026 (#907)

* fix: clean up dangling FK references before deleting a user

Resolves FOREIGN KEY constraint failed (500) on DELETE /api/admin/users/:id
and DELETE /api/auth/me when the target user had rows in trip_members.invited_by,
share_tokens.created_by, budget_items.paid_by_user_id, journeys.user_id,
journey_entries.author_id, journey_contributors.user_id, or
journey_share_tokens.created_by — none of which had ON DELETE clauses.

Introduces deleteUserCompletely() in userCleanupService.ts which wraps all
cleanup and the final DELETE FROM users in a single transaction. Both
adminService.deleteUser and authService.deleteAccount now call it instead of
the bare DELETE. Tests ADMIN-005b and AUTH-040 cover all reference types
including notification sender/recipient and notice dismissals.

* test: extend FK deletion tests to cover journeys, files, and photos

ADMIN-005b and AUTH-040 now also seed and assert:
- owned journey with entries (cascade-deleted via journeys.user_id cleanup)
- trip_files.uploaded_by (SET NULL — file survives, attribution cleared)
- trek_photos.owner_id (SET NULL — photo record survives, owner cleared)
- trip_photos.user_id (CASCADE — photo association removed)

* test: extend user deletion tests to cover all FK relationships

ADMIN-005b and AUTH-040 now seed and assert every user FK relationship:

CASCADE (row deleted): trips, trip_members, tags, mcp_tokens, oauth_tokens,
oauth_consents, vacay_plans, vacay_plan_members, bucket_list,
visited_countries, visited_regions, packing_templates, invite_tokens,
collab_notes, settings, password_reset_tokens, notification_channel_preferences

SET NULL (row survives, column nulled): categories, todo_items.assigned_user_id,
packing_bags, audit_log

Caught and fixed: notification_preferences was dropped in migration 72;
correct table is notification_channel_preferences.

* fix: preserve URL hash and OIDC redirect target through login flow

- Include location.hash in redirect param at all three producer sites
  (ProtectedRoute, axios 401 interceptor, OAuthAuthorizePage) so
  hash fragments survive the login bounce
- Stash redirectTarget in sessionStorage before any OIDC provider
  redirect and restore it after the code exchange, since the IdP
  strips the original ?redirect= param during the roundtrip
- Clear sessionStorage on OIDC error to avoid stale state
- Add tests covering sessionStorage stash on mount, navigate to saved
  redirect after OIDC exchange, fallback to /dashboard, and cleanup
  on error

* fix: use day position instead of ID for accommodation date range clamping

Math.min/Math.max over raw day IDs breaks the start/end picker when a
trip's day IDs are non-monotonic relative to day_number (normal after
repeated generateDays extend/shrink cycles). Replaced with findIndex
lookups so clamping is always based on positional order.

Closes #889

* fix: normalize env var comparisons to be case-insensitive

All NODE_ENV, DEMO_MODE, OIDC_ONLY, FORCE_HTTPS, COOKIE_SECURE, and
ALLOW_INTERNAL_NETWORK checks now use .toLowerCase() so values like
'Production' or 'True' behave identically to their lowercase forms.
Also adds APP_VERSION to the startup banner.

* fix: delete surplus days when shortening a trip

When shrinking a trip's date range, surplus days are now deleted along
with their assignments, notes, and accommodations (cascade). Places
remain in the trip pool; reservations keep their day reference nulled
by the existing ON DELETE SET NULL constraint (issue #909).

Updates TRIP-SVC-011 to reflect the new behaviour; adds TRIP-SVC-016
as a regression test for the empty-day case.

* fix: auto-backup retention deletes itself and manual backups on Docker

Two bugs in cleanupOldBackups:
1. Filter was .endsWith('.zip') — swept manual backup-*.zip files too.
   Now restricted to auto-backup-* prefix.
2. Age was derived from stat.birthtimeMs, which is 0 on overlayfs
   (Docker default), making every backup appear epoch-old and get
   deleted immediately. Age is now parsed from the filename timestamp
   and falls back to mtimeMs (reliable on overlayfs).

Also converts inline require('./services/auditLog') calls to a static
import throughout scheduler.ts, and adds 8 unit tests covering the
fixed retention logic including the overlayfs regression case.

* test: update TRIP-024 to match delete behavior on trip shrink

* feat: add bypass-branch-check label to skip branch enforcement
This commit is contained in:
Julien G.
2026-04-28 05:17:20 +02:00
committed by GitHub
parent ca832e8d88
commit 1f5deeba6c
32 changed files with 937 additions and 106 deletions
+3 -2
View File
@@ -8,6 +8,7 @@ import { updateJwtSecret } from '../config';
import { maybe_encrypt_api_key, decrypt_api_key } from './apiKeyCrypto';
import { getAllPermissions, savePermissions as savePerms, PERMISSION_ACTIONS } from './permissions';
import { revokeUserSessions, revokeUserSessionsForClient } from '../mcp';
import { deleteUserCompletely } from './userCleanupService';
import { validatePassword } from './passwordPolicy';
import { getPhotoProviderConfig } from './memories/helpersService';
import { send as sendNotification } from './notificationService';
@@ -170,7 +171,7 @@ export function deleteUser(id: string, currentUserId: number) {
const userToDel = db.prepare('SELECT id, email FROM users WHERE id = ?').get(id) as { id: number; email: string } | undefined;
if (!userToDel) return { error: 'User not found', status: 404 };
db.prepare('DELETE FROM users WHERE id = ?').run(id);
deleteUserCompletely(userToDel.id);
return { email: userToDel.email };
}
@@ -287,7 +288,7 @@ export function updateOidcSettings(data: {
// ── Demo Baseline ──────────────────────────────────────────────────────────
export function saveDemoBaseline(): { error?: string; status?: number; message?: string } {
if (process.env.DEMO_MODE !== 'true') {
if (process.env.DEMO_MODE?.toLowerCase() !== 'true') {
return { error: 'Not found', status: 404 };
}
try {
+5 -4
View File
@@ -15,6 +15,7 @@ import { decrypt_api_key, maybe_encrypt_api_key, encrypt_api_key } from './apiKe
import { createEphemeralToken } from './ephemeralTokens';
import { revokeUserSessions } from '../mcp';
import { startTripReminders } from '../scheduler';
import { deleteUserCompletely } from './userCleanupService';
import { verifyJwtAndLoadUser } from '../middleware/auth';
import { User } from '../types';
import { DEMO_EMAIL_PRIMARY, isDemoEmail } from './demo';
@@ -130,7 +131,7 @@ export function resolveAuthToggles(): {
oidc_login: get('oidc_login') !== 'false',
oidc_registration: get('oidc_registration') !== 'false',
};
if (process.env.OIDC_ONLY === 'true') {
if (process.env.OIDC_ONLY?.toLowerCase() === 'true') {
result.password_login = false;
result.password_registration = false;
}
@@ -138,7 +139,7 @@ export function resolveAuthToggles(): {
}
// Legacy fallback
const oidcOnlyEnabled = process.env.OIDC_ONLY === 'true' || get('oidc_only') === 'true';
const oidcOnlyEnabled = process.env.OIDC_ONLY?.toLowerCase() === 'true' || get('oidc_only') === 'true';
const oidcConfigured = !!(
(process.env.OIDC_ISSUER || get('oidc_issuer')) &&
(process.env.OIDC_CLIENT_ID || get('oidc_client_id'))
@@ -252,7 +253,7 @@ export function getPendingMfaSecret(userId: number): string | null {
export function getAppConfig(authenticatedUser: { id: number } | null) {
const userCount = (db.prepare('SELECT COUNT(*) as count FROM users').get() as { count: number }).count;
const isDemo = process.env.DEMO_MODE === 'true';
const isDemo = process.env.DEMO_MODE?.toLowerCase() === 'true';
const toggles = resolveAuthToggles();
const version: string = process.env.APP_VERSION ?? require('../../package.json').version;
const hasGoogleKey = !!db.prepare("SELECT maps_api_key FROM users WHERE role = 'admin' AND maps_api_key IS NOT NULL AND maps_api_key != '' LIMIT 1").get();
@@ -527,7 +528,7 @@ export function deleteAccount(userId: number, userEmail: string, userRole: strin
return { error: 'Cannot delete the last admin account', status: 400 };
}
}
db.prepare('DELETE FROM users WHERE id = ?').run(userId);
deleteUserCompletely(userId);
return { success: true };
}
+2 -2
View File
@@ -18,10 +18,10 @@ const COOKIE_NAME = 'trek_session';
* remains the explicit escape hatch for plain-HTTP LAN testing.
*/
export function cookieOptions(clear = false, req?: Request) {
if (process.env.COOKIE_SECURE === 'false') {
if (process.env.COOKIE_SECURE?.toLowerCase() === 'false') {
return buildOptions(clear, false);
}
const envSecure = process.env.NODE_ENV === 'production' || process.env.FORCE_HTTPS === 'true';
const envSecure = process.env.NODE_ENV?.toLowerCase() === 'production' || process.env.FORCE_HTTPS?.toLowerCase() === 'true';
const requestSecure = req?.secure === true;
return buildOptions(clear, envSecure || requestSecure);
}
+1 -1
View File
@@ -170,7 +170,7 @@ export async function send(payload: NotificationPayload): Promise<void> {
const configEntry = EVENT_NOTIFICATION_CONFIG[event];
if (!configEntry) {
logDebug(`notificationService.send: unknown event type "${event}", using fallback`);
if (process.env.NODE_ENV === 'development' && actorId != null) {
if (process.env.NODE_ENV?.toLowerCase() === 'development' && actorId != null) {
const devSender = (db.prepare('SELECT username, avatar FROM users WHERE id = ?').get(actorId) as { username: string; avatar: string | null } | undefined) ?? null;
createNotificationForRecipient({
type: 'simple',
+4 -3
View File
@@ -117,10 +117,11 @@ export function generateDays(tripId: number | bigint | string, startDate: string
}
}
// Overflow dated days (trip shrunk): convert to dateless instead of deleting
const nullify = db.prepare('UPDATE days SET date = NULL, day_number = ? WHERE id = ?');
// Overflow dated days (trip shrunk): delete them (issue #909).
// Cascade removes their assignments, notes, and accommodations.
const del = db.prepare('DELETE FROM days WHERE id = ?');
for (let i = targetDates.length; i < dated.length; i++) {
nullify.run(targetDates.length + (i - targetDates.length) + 1, dated[i].id);
del.run(dated[i].id);
}
// Any remaining unused dateless days: keep as dateless, just renumber.
+21
View File
@@ -0,0 +1,21 @@
import { db } from '../db/database';
function cleanupUserReferences(userId: number): void {
db.prepare('UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?').run(userId);
db.prepare('UPDATE budget_items SET paid_by_user_id = NULL WHERE paid_by_user_id = ?').run(userId);
db.prepare('DELETE FROM share_tokens WHERE created_by = ?').run(userId);
db.prepare('DELETE FROM journey_share_tokens WHERE created_by = ?').run(userId);
// Owned journeys cascade-delete their entries/contributors/share_tokens/photos via journey_id FKs
db.prepare('DELETE FROM journeys WHERE user_id = ?').run(userId);
// Entries authored on other users' journeys (not covered by the cascade above)
db.prepare('DELETE FROM journey_entries WHERE author_id = ?').run(userId);
db.prepare('DELETE FROM journey_contributors WHERE user_id = ?').run(userId);
}
export function deleteUserCompletely(userId: number): void {
const tx = db.transaction((id: number) => {
cleanupUserReferences(id);
db.prepare('DELETE FROM users WHERE id = ?').run(id);
});
tx(userId);
}