Read online users through the facade, not a lazy require

admin.service.ts reached getOnlineUserIds with require('../../websocket')
inside a try/catch — the last lazy require of the ws singleton in the tree,
and exactly what RealtimeService exists to prevent. The facade gains the
third pass-through next to broadcast and broadcastToUser, delegating at call
time like its siblings so per-file vi.mock stubs keep flowing through.

The catch stays at the admin call site rather than moving into the facade: an
admin list that shows everyone offline is a better answer than a 500, but
that judgement belongs to this caller, not to every future one.

Typechecking the tests turned up two more things behind the arity errors.

admin.service.test.ts built AuthService with an AtlasService in the
membership slot and no mailer at all, so its last four collaborators were
shifted by one. Nothing failed, because no case there reaches a path that
uses them.

Its getOidcSettings/updateOidcSettings wrappers referenced methods AdminService
no longer has. Nothing called them: OIDC settings moved to OidcService and the
two cases moved with it (ADMIN-SVC-047/048 now live in oidc.service.test.ts).
Only the wrappers stayed behind.
This commit is contained in:
Maurice
2026-08-10 17:42:13 +02:00
committed by Maurice
parent 4780a7be90
commit f691da8f7e
5 changed files with 42 additions and 19 deletions
+1
View File
@@ -49,6 +49,7 @@ const admin = new AdminService(
permissions,
new NotificationsService(dbs, realtime, mailer, new WebhookService(dbs), new NtfyService(dbs), notifPrefs),
userCleanup,
new RealtimeService(),
);
export function checkAndNotifyVersion(): Promise<void> {
+6 -3
View File
@@ -23,6 +23,7 @@ import { validatePassword } from '../common/passwordPolicy';
import { UserCleanupService } from '../auth/user-cleanup.service';
import { DatabaseService } from '../database/database.service';
import { AddonsService } from '../addons/addons.service';
import { RealtimeService } from '../realtime/realtime.service';
import { PasskeyService } from '../auth/passkey.service';
import { AuthService } from '../auth/auth.service';
import { PermissionsService } from '../permissions/permissions.service';
@@ -73,6 +74,7 @@ export class AdminService {
private readonly permissions: PermissionsService,
private readonly notifications: NotificationsService,
private readonly userCleanup: UserCleanupService,
private readonly realtime: RealtimeService,
) {}
// ── User CRUD ──────────────────────────────────────────────────────────────
@@ -89,9 +91,10 @@ export class AdminService {
);
let onlineUserIds = new Set<number>();
try {
// Lazy require: the ws singleton must not become a load-time dependency.
const { getOnlineUserIds } = require('../../websocket');
onlineUserIds = getOnlineUserIds();
// The catch stays here rather than in the facade: an admin list that shows
// everyone as offline is a better answer than a 500, and that judgement
// belongs to this caller, not to every future one.
onlineUserIds = this.realtime.getOnlineUserIds();
} catch {
/* */
}
+15 -1
View File
@@ -5,7 +5,7 @@ import type {
TrekWsTripEventName,
TrekWsUserEventName,
} from '@trek/shared';
import { broadcast, broadcastToUser } from '../../websocket';
import { broadcast, broadcastToUser, getOnlineUserIds } from '../../websocket';
/**
* Injectable facade over the websocket module singleton (roadmap Phase 0 item 3).
@@ -89,4 +89,18 @@ export class RealtimeService {
): void {
broadcastToUser(...args);
}
/**
* Which users currently hold an open socket.
*
* AdminService read this through a lazy `require('../../websocket')` inside a
* try/catch, which is what the facade exists to avoid. Deliberately NOT
* guarded here: a guard on the facade would swallow the failure for every
* future caller, and the one caller that wants a degraded answer rather than
* an error keeps its own catch. Delegates at call time, like its two
* siblings, so per-file vi.mock stubs keep flowing through.
*/
getOnlineUserIds(): Set<number> {
return getOnlineUserIds();
}
}
+7 -3
View File
@@ -63,6 +63,7 @@ import { SettingsService } from '../../../src/nest/settings/settings.service';
import { AtlasService } from '../../../src/nest/atlas/atlas.service';
import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service';
import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service';
import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service';
import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service';
import { AuthService } from '../../../src/nest/auth/auth.service';
import { PasskeyService } from '../../../src/nest/auth/passkey.service';
@@ -79,7 +80,11 @@ const realtime = new RealtimeService();
const permissions = new PermissionsService(dbs);
const webauthn = new WebauthnConfigService(dbs);
const userCleanup = new UserCleanupService(dbs);
const auth = new AuthService(dbs, permissions, new AtlasService(dbs), new TripMembershipService(dbs), webauthn, userCleanup);
// Positional and previously wrong: an AtlasService sat in the membership slot
// and the mailer was missing entirely, so `auth` was built with its last four
// collaborators shifted by one. Nothing failed, because none of the cases below
// reach a path that uses them.
const auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs));
const svc = new AdminService(
dbs,
new AddonsService(dbs),
@@ -88,6 +93,7 @@ const svc = new AdminService(
permissions,
makeNotificationsService(dbs, realtime),
userCleanup,
realtime,
);
// Legacy free-function names bound to the service, so the moved cases below read
@@ -100,8 +106,6 @@ const getStats = () => svc.getStats();
const getPermissions = () => svc.getPermissions();
const savePermissions = (p: Record<string, string>) => svc.savePermissions(p);
const getAuditLog = (q: { limit?: string; offset?: string }) => svc.getAuditLog(q);
const getOidcSettings = () => svc.getOidcSettings();
const updateOidcSettings = (d: Parameters<AdminService['updateOidcSettings']>[0]) => svc.updateOidcSettings(d);
const saveDemoBaseline = () => svc.saveDemoBaseline();
const getGithubReleases = (perPage?: string, page?: string) => svc.getGithubReleases(perPage, page);
const checkVersion = () => svc.checkVersion();
@@ -20,6 +20,7 @@ vi.mock('../../../src/db/database', () => ({ db: testDb }));
import { db as dbConn } from '../../../src/db/database';
import { DatabaseService } from '../../../src/nest/database/database.service';
import { AddonsService } from '../../../src/nest/addons/addons.service';
import { PluginsService } from '../../../src/nest/plugins/plugins.service';
import { PluginsController } from '../../../src/nest/plugins/plugins.controller';
import { PluginsFeedController } from '../../../src/nest/plugins/plugins-feed.controller';
@@ -40,7 +41,7 @@ describe('PluginsService.list', () => {
.run('flight', 'Flight', 'desc', 'widget', 'inactive', '1.0.0');
process.env.TREK_PLUGINS_ENABLED = 'true';
const out = new PluginsService(new DatabaseService(dbConn)).list();
const out = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list();
expect(out.enabled).toBe(true);
expect(out.plugins).toHaveLength(1);
expect(out.plugins[0]).toMatchObject({ id: 'flight', name: 'Flight', status: 'inactive' });
@@ -51,14 +52,14 @@ describe('PluginsService.list', () => {
.prepare('INSERT INTO plugins (id, name, description, type, status, version) VALUES (?,?,?,?,?,?)')
.run('flight', 'Flight', 'desc', 'widget', 'inactive', '1.0.0');
const out = new PluginsService(new DatabaseService(dbConn)).list();
const out = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list();
expect(out.enabled).toBe(true);
expect(out.plugins).toHaveLength(1);
});
it('reports disabled when the kill switch is off (TREK_PLUGINS_ENABLED=false)', () => {
process.env.TREK_PLUGINS_ENABLED = 'false';
const out = new PluginsService(new DatabaseService(dbConn)).list();
const out = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list();
expect(out.enabled).toBe(false);
expect(out.plugins).toEqual([]);
});
@@ -78,7 +79,7 @@ describe('PluginsService.list', () => {
const key = 'RWTvBn0aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789abcd';
insert('signed-one', 'acme/signed-one', key);
const p = new PluginsService(new DatabaseService(dbConn)).list().plugins[0];
const p = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins[0];
expect(p.signed).toBe(true);
// Short head…tail, for eyeballing against what the author reads out over the
// phone. NOT a confidentiality measure — the key is public, and the re-trust
@@ -88,7 +89,7 @@ describe('PluginsService.list', () => {
it('reports unsigned for a registry plugin with no pinned key', () => {
insert('plain', 'acme/plain', null);
const p = new PluginsService(new DatabaseService(dbConn)).list().plugins[0];
const p = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins[0];
expect(p.signed).toBe(false);
expect(p.keyFingerprint).toBeNull();
});
@@ -96,7 +97,7 @@ describe('PluginsService.list', () => {
it('reports unsigned for a sideloaded and a dev-linked plugin (they carry no key)', () => {
insert('uploaded', 'local:upload', null);
insert('linked', 'local:link', null);
const plugins = new PluginsService(new DatabaseService(dbConn)).list().plugins;
const plugins = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins;
expect(plugins.map((p) => [p.id, p.signed, p.source_repo])).toEqual([
['linked', false, 'local:link'],
['uploaded', false, 'local:upload'],
@@ -110,14 +111,14 @@ describe('PluginsService.list', () => {
.prepare('UPDATE plugins SET update_block_code = ?, update_block_detail = ?, update_block_version = ? WHERE id = ?')
.run('SIGNATURE_KEY_CHANGED', 'the key changed', '2.0.0', 'blocked');
const byId = Object.fromEntries(new PluginsService(new DatabaseService(dbConn)).list().plugins.map((p) => [p.id, p]));
const byId = Object.fromEntries(new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins.map((p) => [p.id, p]));
expect(byId.blocked.updateBlock).toEqual({ code: 'SIGNATURE_KEY_CHANGED', detail: 'the key changed', version: '2.0.0' });
expect(byId.fine.updateBlock).toBeNull();
});
it('never leaks the raw pinned key into the list response (only the fingerprint)', () => {
insert('signed-one', 'acme/signed-one', 'RWTvBn0aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789abcd');
const p = new PluginsService(new DatabaseService(dbConn)).list().plugins[0] as Record<string, unknown>;
const p = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins[0] as Record<string, unknown>;
expect(p.author_pubkey).toBeUndefined();
});
});
@@ -238,7 +239,7 @@ describe('PluginsService instance config', () => {
testDb.prepare("INSERT INTO plugins (id, name, status, config) VALUES ('x','X','inactive','{}')").run();
testDb.prepare("INSERT INTO plugin_settings_fields (plugin_id, field_key, scope, secret) VALUES ('x','api_key','instance',1)").run();
const svc = new PluginsService(new DatabaseService(dbConn));
const svc = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn)));
const masked = svc.updateInstanceConfig('x', { api_key: 'super-secret', server: 'https://h' });
// client gets the masked view
expect(masked.api_key).toBe('••••••••');
@@ -259,8 +260,8 @@ describe('PluginsService instance config', () => {
});
it('throws for an unknown plugin', () => {
expect(() => new PluginsService(new DatabaseService(dbConn)).updateInstanceConfig('nope', {})).toThrow(/not found/);
expect(() => new PluginsService(new DatabaseService(dbConn)).getInstanceConfig('nope')).toThrow(/not found/);
expect(() => new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).updateInstanceConfig('nope', {})).toThrow(/not found/);
expect(() => new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).getInstanceConfig('nope')).toThrow(/not found/);
});
});
@@ -268,7 +269,7 @@ describe('PluginsService error log', () => {
beforeEach(() => testDb.exec('DELETE FROM plugin_error_log'));
it('lists and clears a plugin error log', () => {
testDb.prepare("INSERT INTO plugin_error_log (plugin_id, level, message) VALUES ('p','error','boom')").run();
const svc = new PluginsService(new DatabaseService(dbConn));
const svc = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn)));
expect(svc.errors('p')).toEqual([{ ts: '2026-01-01', level: 'error', message: 'boom' }]);
svc.clearErrors('p');
expect(svc.errors('p')).toEqual([]);