mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-19 13:21:46 +00:00
Bug fixes - May 2nd 2026 (#941)
* fix: collab chat input hidden by mobile bottom nav bar Closes #939 * chore: prepare database for nest + typeorm * fix(ssrf): relax internal network resolution (#947) * docs(ssrf): update Internal-Network-Access wiki to reflect relaxed guard Loopback, link-local, and .local/.internal hostnames are now all overridable with ALLOW_INTERNAL_NETWORK=true (commit9a08368). Merge the two-tier "always blocked / conditionally blocked" structure into a single table, add a warning about cloud metadata exposure. * fix(ssrf): let .local/.internal hostnames pass to IP-level checks The pre-DNS hostname block was redundant: any .local/.internal host that resolves to a private IP is already gated by isPrivateNetwork + ALLOW_INTERNAL_NETWORK, and any that resolves to loopback/link-local is caught by isAlwaysBlocked unconditionally. Dropping the hostname pre-check means Docker/LAN deployments can reach services on .local hostnames (e.g. immich.local) with ALLOW_INTERNAL_NETWORK=true, while loopback and link-local IPs (including 169.254.169.254) remain hard-blocked with no override. Reverts the isAlwaysBlocked guard loosening from9a08368. * fix(auth): trim username and email on all write paths Self-registration stored values verbatim, so trailing whitespace could produce rows that lookup code (which trims input) silently misses. Trim username and email before validation and INSERT in registerUser, adminService.updateUser, and oidcService.findOrCreateUser. updateSettings and adminService.createUser already trimmed correctly. Adds a one-shot backfill migration (trimUserWhitespace) that trims existing dirty rows; collisions are resolved by appending __migrated_<id> to the value with a loud console.warn so operators can review affected accounts. 18 new tests covering registration trim, duplicate detection, admin update trim, trip-member lookup regression, and all migration branches. * feat(notices): add v3014-whitespace-collision admin notice Adds a dismissible banner for admins on v3.0.14+ that fires only when the whitespace-trimming migration detected a username/email collision (stored in app_settings as whitespace_migration_collision=true). Notice conditions: existingUserBeforeVersion(3.0.14) + role=admin + custom predicate reading the app_settings flag. Predicate registered in registry.ts; migration step writes the flag when hadCollision=true. All 15 translation files updated with title/body keys. 7 integration tests added (SN-COLLISION-1 through -7) covering all condition branches: shown when all conditions met, hidden when flag absent/false, hidden for non-admin, hidden for new user, hidden below min app version, hidden after dismissal.
This commit is contained in:
@@ -368,6 +368,53 @@ describe('Admin user management', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Admin user management — whitespace normalization
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('Admin user management — whitespace normalization', () => {
|
||||
it('ADMIN-UPDATE-TRIM-1 — PUT /admin/users/:id trims username before storing', async () => {
|
||||
const { user: admin } = createAdmin(testDb);
|
||||
const { user } = createUser(testDb);
|
||||
|
||||
const res = await request(app)
|
||||
.put(`/api/admin/users/${user.id}`)
|
||||
.set('Cookie', authCookie(admin.id))
|
||||
.send({ username: ' trimmedadmin ' });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const row = testDb.prepare('SELECT username FROM users WHERE id = ?').get(user.id) as { username: string };
|
||||
expect(row.username).toBe('trimmedadmin');
|
||||
});
|
||||
|
||||
it('ADMIN-UPDATE-TRIM-2 — PUT /admin/users/:id trims email before storing', async () => {
|
||||
const { user: admin } = createAdmin(testDb);
|
||||
const { user } = createUser(testDb);
|
||||
|
||||
const res = await request(app)
|
||||
.put(`/api/admin/users/${user.id}`)
|
||||
.set('Cookie', authCookie(admin.id))
|
||||
.send({ email: ' newemail@example.com ' });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const row = testDb.prepare('SELECT email FROM users WHERE id = ?').get(user.id) as { email: string };
|
||||
expect(row.email).toBe('newemail@example.com');
|
||||
});
|
||||
|
||||
it('ADMIN-UPDATE-TRIM-3 — PUT /admin/users/:id with whitespace-padded username that trims to existing returns 409', async () => {
|
||||
const { user: admin } = createAdmin(testDb);
|
||||
const { user: existing } = createUser(testDb, { username: 'carol' });
|
||||
const { user: target } = createUser(testDb);
|
||||
|
||||
const res = await request(app)
|
||||
.put(`/api/admin/users/${target.id}`)
|
||||
.set('Cookie', authCookie(admin.id))
|
||||
.send({ username: ` ${existing.username} ` });
|
||||
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// System stats
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -218,6 +218,54 @@ describe('Registration', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Registration — whitespace normalization
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('Registration — whitespace normalization', () => {
|
||||
it('AUTH-REG-TRIM-1 — username with surrounding whitespace is trimmed before storage', async () => {
|
||||
const res = await request(app).post('/api/auth/register').send({
|
||||
username: ' trimmeduser ',
|
||||
email: 'trimmed@example.com',
|
||||
password: 'Str0ng!Pass',
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
const row = testDb.prepare('SELECT username FROM users WHERE email = ?').get('trimmed@example.com') as { username: string };
|
||||
expect(row.username).toBe('trimmeduser');
|
||||
});
|
||||
|
||||
it('AUTH-REG-TRIM-2 — email with surrounding whitespace is trimmed before storage', async () => {
|
||||
const res = await request(app).post('/api/auth/register').send({
|
||||
username: 'emailtrimuser',
|
||||
email: ' emailtrim@example.com ',
|
||||
password: 'Str0ng!Pass',
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
const row = testDb.prepare('SELECT email FROM users WHERE username = ?').get('emailtrimuser') as { email: string };
|
||||
expect(row.email).toBe('emailtrim@example.com');
|
||||
});
|
||||
|
||||
it('AUTH-REG-TRIM-3 — whitespace-padded username that trims to existing username returns 409', async () => {
|
||||
createUser(testDb, { username: 'alice', email: 'alice@example.com' });
|
||||
const res = await request(app).post('/api/auth/register').send({
|
||||
username: ' alice ',
|
||||
email: 'alice2@example.com',
|
||||
password: 'Str0ng!Pass',
|
||||
});
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
it('AUTH-REG-TRIM-4 — whitespace-padded email that trims to existing email returns 409', async () => {
|
||||
createUser(testDb, { username: 'bob', email: 'bob@example.com' });
|
||||
const res = await request(app).post('/api/auth/register').send({
|
||||
username: 'bob2',
|
||||
email: ' bob@example.com ',
|
||||
password: 'Str0ng!Pass',
|
||||
});
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Session / Me
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -39,7 +39,7 @@ import { createApp } from '../../src/app';
|
||||
import { createTables } from '../../src/db/schema';
|
||||
import { runMigrations } from '../../src/db/migrations';
|
||||
import { resetTestDb } from '../helpers/test-db';
|
||||
import { createUser } from '../helpers/factories';
|
||||
import { createUser, createAdmin } from '../helpers/factories';
|
||||
import { authCookie } from '../helpers/auth';
|
||||
import { SYSTEM_NOTICES } from '../../src/systemNotices/registry';
|
||||
import type { SystemNotice } from '../../src/systemNotices/types';
|
||||
@@ -242,3 +242,129 @@ describe('POST /api/system-notices/:id/dismiss', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// v3014-whitespace-collision notice
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Helper: creates an admin user whose first_seen_version is before 3.0.14
|
||||
* (so existingUserBeforeVersion('3.0.14') passes) and whose login_count is
|
||||
* high enough to suppress the firstLogin and v3-upgrade notice conditions.
|
||||
*/
|
||||
function setupCollisionAdmin() {
|
||||
const { user } = createAdmin(testDb);
|
||||
testDb.prepare('UPDATE users SET login_count = 5, first_seen_version = ? WHERE id = ?').run('3.0.0', user.id);
|
||||
return user;
|
||||
}
|
||||
|
||||
describe('v3014-whitespace-collision notice', () => {
|
||||
const NOTICE_ID = 'v3014-whitespace-collision';
|
||||
const originalAppVersion = process.env.APP_VERSION;
|
||||
|
||||
beforeEach(() => {
|
||||
process.env.APP_VERSION = '3.0.14';
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalAppVersion === undefined) {
|
||||
delete process.env.APP_VERSION;
|
||||
} else {
|
||||
process.env.APP_VERSION = originalAppVersion;
|
||||
}
|
||||
});
|
||||
|
||||
it('SN-COLLISION-1 — shown to admin when collision flag is set and user predates 3.0.14', async () => {
|
||||
const user = setupCollisionAdmin();
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('whitespace_migration_collision', 'true')").run();
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/system-notices/active')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.find((n: { id: string }) => n.id === NOTICE_ID)).toBeDefined();
|
||||
});
|
||||
|
||||
it('SN-COLLISION-2 — hidden when collision flag is absent', async () => {
|
||||
const user = setupCollisionAdmin();
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/system-notices/active')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.find((n: { id: string }) => n.id === NOTICE_ID)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('SN-COLLISION-3 — hidden when collision flag is explicitly false', async () => {
|
||||
const user = setupCollisionAdmin();
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('whitespace_migration_collision', 'false')").run();
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/system-notices/active')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.find((n: { id: string }) => n.id === NOTICE_ID)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('SN-COLLISION-4 — hidden for non-admin user even when collision flag is set', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
testDb.prepare('UPDATE users SET login_count = 5, first_seen_version = ? WHERE id = ?').run('3.0.0', user.id);
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('whitespace_migration_collision', 'true')").run();
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/system-notices/active')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.find((n: { id: string }) => n.id === NOTICE_ID)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('SN-COLLISION-5 — hidden for user whose first_seen_version is >= 3.0.14 (new account)', async () => {
|
||||
const { user } = createAdmin(testDb);
|
||||
testDb.prepare('UPDATE users SET login_count = 5, first_seen_version = ? WHERE id = ?').run('3.0.14', user.id);
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('whitespace_migration_collision', 'true')").run();
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/system-notices/active')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.find((n: { id: string }) => n.id === NOTICE_ID)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('SN-COLLISION-6 — hidden when app version is below 3.0.14', async () => {
|
||||
process.env.APP_VERSION = '3.0.13';
|
||||
const user = setupCollisionAdmin();
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('whitespace_migration_collision', 'true')").run();
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/system-notices/active')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.find((n: { id: string }) => n.id === NOTICE_ID)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('SN-COLLISION-7 — hidden after admin dismisses it', async () => {
|
||||
const user = setupCollisionAdmin();
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('whitespace_migration_collision', 'true')").run();
|
||||
|
||||
const before = await request(app)
|
||||
.get('/api/system-notices/active')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
expect(before.body.find((n: { id: string }) => n.id === NOTICE_ID)).toBeDefined();
|
||||
|
||||
const dismiss = await request(app)
|
||||
.post(`/api/system-notices/${NOTICE_ID}/dismiss`)
|
||||
.set('Cookie', authCookie(user.id));
|
||||
expect(dismiss.status).toBe(204);
|
||||
|
||||
const after = await request(app)
|
||||
.get('/api/system-notices/active')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
expect(after.body.find((n: { id: string }) => n.id === NOTICE_ID)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -677,6 +677,20 @@ describe('Trip members', () => {
|
||||
expect(res.body.error).toMatch(/already/i);
|
||||
});
|
||||
|
||||
it('TRIP-013 — Adding a member by whitespace-padded username resolves correctly → 201', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: invitee } = createUser(testDb, { username: 'paddeduser' });
|
||||
const trip = createTrip(testDb, owner.id, { title: 'Padded Trip' });
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/api/trips/${trip.id}/members`)
|
||||
.set('Cookie', authCookie(owner.id))
|
||||
.send({ identifier: ' paddeduser ' });
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(res.body.member.id).toBe(invitee.id);
|
||||
});
|
||||
|
||||
it('TRIP-014 — DELETE /api/trips/:id/members/:userId removes a member → 200', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
/**
|
||||
* Unit tests for trimUserWhitespace — the backfill migration that normalises
|
||||
* leading/trailing whitespace in stored usernames and emails.
|
||||
* Tests TRIM-MIG-001 through TRIM-MIG-010.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import Database from 'better-sqlite3';
|
||||
import { trimUserWhitespace } from '../../../src/db/migrations';
|
||||
|
||||
function makeDb() {
|
||||
const db = new Database(':memory:');
|
||||
db.exec('PRAGMA foreign_keys = ON');
|
||||
db.exec(`
|
||||
CREATE TABLE users (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
username TEXT UNIQUE NOT NULL,
|
||||
email TEXT UNIQUE NOT NULL,
|
||||
password_hash TEXT NOT NULL DEFAULT 'x',
|
||||
role TEXT NOT NULL DEFAULT 'user'
|
||||
)
|
||||
`);
|
||||
return db;
|
||||
}
|
||||
|
||||
function insert(db: Database.Database, username: string, email: string): number {
|
||||
const r = db.prepare('INSERT INTO users (username, email) VALUES (?, ?)').run(username, email);
|
||||
return Number(r.lastInsertRowid);
|
||||
}
|
||||
|
||||
function row(db: Database.Database, id: number) {
|
||||
return db.prepare('SELECT username, email FROM users WHERE id = ?').get(id) as { username: string; email: string };
|
||||
}
|
||||
|
||||
describe('trimUserWhitespace — clean data (no-op)', () => {
|
||||
it('TRIM-MIG-001 — leaves already-clean rows untouched', () => {
|
||||
const db = makeDb();
|
||||
const id = insert(db, 'alice', 'alice@example.com');
|
||||
trimUserWhitespace(db);
|
||||
expect(row(db, id)).toEqual({ username: 'alice', email: 'alice@example.com' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('trimUserWhitespace — non-colliding dirty rows', () => {
|
||||
it('TRIM-MIG-002 — trims trailing whitespace from username', () => {
|
||||
const db = makeDb();
|
||||
const id = insert(db, 'alice ', 'alice@example.com');
|
||||
trimUserWhitespace(db);
|
||||
expect(row(db, id).username).toBe('alice');
|
||||
});
|
||||
|
||||
it('TRIM-MIG-003 — trims leading whitespace from username', () => {
|
||||
const db = makeDb();
|
||||
const id = insert(db, ' alice', 'alice@example.com');
|
||||
trimUserWhitespace(db);
|
||||
expect(row(db, id).username).toBe('alice');
|
||||
});
|
||||
|
||||
it('TRIM-MIG-004 — trims surrounding whitespace from email', () => {
|
||||
const db = makeDb();
|
||||
const id = insert(db, 'alice', ' alice@example.com ');
|
||||
trimUserWhitespace(db);
|
||||
expect(row(db, id).email).toBe('alice@example.com');
|
||||
});
|
||||
|
||||
it('TRIM-MIG-005 — emits a console.warn for each trimmed row', () => {
|
||||
const db = makeDb();
|
||||
insert(db, 'bob ', 'bob@example.com');
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
trimUserWhitespace(db);
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('[migration] Trimmed username'));
|
||||
warn.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe('trimUserWhitespace — username collision handling', () => {
|
||||
it('TRIM-MIG-006 — renames the dirty row to <trimmed>__migrated_<id> on collision', () => {
|
||||
const db = makeDb();
|
||||
insert(db, 'carol', 'carol@example.com');
|
||||
const dirtyId = insert(db, 'carol ', 'carol2@example.com');
|
||||
trimUserWhitespace(db);
|
||||
expect(row(db, dirtyId).username).toBe(`carol__migrated_${dirtyId}`);
|
||||
});
|
||||
|
||||
it('TRIM-MIG-007 — emits a WHITESPACE COLLISION warning for username collision', () => {
|
||||
const db = makeDb();
|
||||
insert(db, 'dan', 'dan@example.com');
|
||||
insert(db, 'dan ', 'dan2@example.com');
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
trimUserWhitespace(db);
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('WHITESPACE COLLISION username'));
|
||||
warn.mockRestore();
|
||||
});
|
||||
|
||||
it('TRIM-MIG-008 — the renamed value does not conflict with the existing clean row', () => {
|
||||
const db = makeDb();
|
||||
const cleanId = insert(db, 'eve', 'eve@example.com');
|
||||
const dirtyId = insert(db, 'eve ', 'eve2@example.com');
|
||||
trimUserWhitespace(db);
|
||||
expect(row(db, cleanId).username).toBe('eve');
|
||||
expect(row(db, dirtyId).username).toBe(`eve__migrated_${dirtyId}`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('trimUserWhitespace — email collision handling', () => {
|
||||
it('TRIM-MIG-009 — renames dirty email as <local>__migrated_<id>@<domain> on collision', () => {
|
||||
const db = makeDb();
|
||||
insert(db, 'frank', 'frank@example.com');
|
||||
const dirtyId = insert(db, 'frank2', ' frank@example.com ');
|
||||
trimUserWhitespace(db);
|
||||
expect(row(db, dirtyId).email).toBe(`frank__migrated_${dirtyId}@example.com`);
|
||||
});
|
||||
|
||||
it('TRIM-MIG-010 — emits a WHITESPACE COLLISION warning for email collision', () => {
|
||||
const db = makeDb();
|
||||
insert(db, 'grace', 'grace@example.com');
|
||||
insert(db, 'grace2', 'grace@example.com ');
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
trimUserWhitespace(db);
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('WHITESPACE COLLISION email'));
|
||||
warn.mockRestore();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user