feat(mcp): read and change display preferences

An assistant rendering a temperature, a distance or a clock time was
guessing: the session instructions named only the trip currency, and nothing
could read what the user had picked.

The write works off an allow-list rather than off the settings row, because
that row also holds the webhook URL, the ntfy token and the Mapbox, Carto
and LLM keys, and getUserSettings hands some of those back in cleartext.
Everything outside the list is refused, including a key nobody has heard of
yet. The guard test asserts against the service's own encrypted and masked
sets rather than a copy of them, so a sixth key added there fails it.
This commit is contained in:
Maurice
2026-08-28 13:45:30 +02:00
committed by Maurice
parent 6270495327
commit ed67a5e9f8
4 changed files with 739 additions and 4 deletions
+170
View File
@@ -0,0 +1,170 @@
import {
McpController, Tool, type McpContext,
TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE,
demoDenied, errorResult, ok,
} from '../../nest-mcp';
import { z } from 'zod';
import { MASKED_SETTING_VALUE, SUPPORTED_LANGUAGE_CODES, settingsBulkRequestSchema } from '@trek/shared';
import { AuthService } from '../auth/auth.service';
import { SettingsService } from './settings.service';
/**
* The only settings keys this surface may read or write.
*
* An allow-list rather than a deny-list, because `settings` is one flat
* key/value table that also holds credentials: ENCRYPTED_SETTING_KEYS in
* settings.service.ts covers webhook_url, ntfy_token, mapbox_access_token,
* carto_api_key and llm_api_key, and getUserSettings masks only three of those
* and hands the rest back decrypted. Exposing that accessor as a tool would
* turn a `settings:read` token into a credential reader. A deny-list would put
* the burden on whoever adds the next secret key to remember this file; an
* allow-list leaves anything new invisible until somebody names it here.
*
* The membership is what DisplaySettingsTab.tsx offers, plus dark_mode (its
* own control, in AppearanceSettingsTab and the navbar theme toggle). Keys from
* DEFAULTABLE_USER_SETTING_KEYS that are credentials or renderer plumbing
* (map_tile_url, map_provider, mapbox_*, maplibre_style, llm_*, carto_api_key)
* are left off on purpose: they configure the install, not how a person reads
* a temperature, and every one of them is on MANAGED_LOCKED_SETTING_KEYS or
* next to something that is.
*
* Each value schema is at most as permissive as what the REST route stores
* today, which validates nothing here. Narrowing is deliberate: a model gets a
* refusal it can act on instead of parking 'kelvin' in a row the client then
* silently ignores.
*/
const DISPLAY_PREFERENCES = {
start_page: z.enum(['dashboard', 'active_trip']),
// The planner falls back to its plan view for a tab that does not exist, and
// trip-page plugins are addressable as `plugin:<id>`, so the id set is not
// closed. Core ids are named in the tool description instead.
start_trip_tab: z.string().min(1).max(64),
default_currency: z.union([z.literal(''), z.string().regex(/^[A-Z]{3}$/, 'Expected a three-letter uppercase ISO 4217 code, e.g. EUR, or "" to clear it')]),
language: z.string().refine(
(value) => SUPPORTED_LANGUAGE_CODES.includes(value),
{ message: `Expected one of: ${SUPPORTED_LANGUAGE_CODES.join(', ')}` },
),
temperature_unit: z.enum(['celsius', 'fahrenheit']),
distance_unit: z.enum(['metric', 'imperial']),
time_format: z.enum(['12h', '24h']),
// Booleans are the pre-'auto' form and still sit in older rows, so they stay
// writable: this is exactly the VALID_VALUES.dark_mode set the admin-defaults
// path accepts, and refusing them here would make a value TREK itself wrote
// unwritable through this tool.
dark_mode: z.union([z.enum(['light', 'dark', 'auto']), z.boolean()]),
map_booking_labels: z.boolean(),
map_always_show_routes: z.boolean(),
map_poi_pill_enabled: z.boolean(),
blur_booking_codes: z.boolean(),
optimize_from_accommodation: z.boolean(),
} satisfies Record<string, z.ZodType>;
type DisplayPreferenceKey = keyof typeof DISPLAY_PREFERENCES;
export const DISPLAY_PREFERENCE_KEYS = Object.keys(DISPLAY_PREFERENCES) as DisplayPreferenceKey[];
// Set rather than `key in DISPLAY_PREFERENCES`: `in` walks the prototype chain,
// so 'toString' and 'constructor' would both pass for an object literal.
const ALLOWED = new Set<string>(DISPLAY_PREFERENCE_KEYS);
const KEY_LIST = DISPLAY_PREFERENCE_KEYS.join(', ');
/**
* Settings MCP surface: the display half of /api/settings, and nothing else.
*
* New surface rather than a port, so there is no legacy registrar to match.
* It exists because an assistant had no way to learn whether the person it is
* answering reads celsius or fahrenheit, kilometres or miles, 24h or 12h: the
* session instructions name the trip currency and nothing more, so every
* rendered temperature and distance was a guess.
*
* Both tools are user-scoped, like the routes they mirror (JwtAuthGuard, no
* trip in the path), so the trip-access/permission pair does not apply. The
* write carries the demo gate the other write tools carry.
*
* The controller's own two refusals stay out of reach by construction rather
* than by being repeated: assertMayWriteLlmEndpoint guards llm_base_url and
* llm_provider, and isManagedLockedKey guards the operator-owned keys, and not
* one name from either list is on the allow-list above. tools-settings.test.ts
* asserts that intersection is empty instead of trusting the reading.
*/
@McpController()
export class SettingsMcp {
constructor(
private readonly settings: SettingsService,
private readonly auth: AuthService,
) {}
/**
* getUserSettings resolves admin defaults and the managed-install token
* injection before this filter runs, so the caller sees the same effective
* value the web UI does, minus everything not named on the allow-list.
*/
private readDisplayPreferences(userId: number): Record<string, unknown> {
const all = this.settings.getUserSettings(userId);
const picked: Record<string, unknown> = {};
for (const key of DISPLAY_PREFERENCE_KEYS) {
if (Object.hasOwn(all, key)) picked[key] = all[key];
}
return picked;
}
@Tool({
name: 'get_display_settings',
description: `Read the current user's display preferences: ${KEY_LIST}. Call this before rendering a temperature, a distance, a time of day or an amount, so the answer matches what the person sees inside TREK instead of being guessed, and call it before update_display_settings whenever the request is relative ("switch me back", "use the other unit"). Only keys that are actually set are returned; TREK's own fallbacks for the rest are celsius, metric, 24h, language en, start_page dashboard, start_trip_tab plan, and no personal default currency, which means amounts fall back to the currency of the trip they belong to. Stored credentials (map tokens, LLM API keys, webhook URLs, SMTP) are deliberately not part of this surface and never appear in the result.`,
inputSchema: {},
annotations: TOOL_ANNOTATIONS_READONLY,
access: { group: 'settings', mode: 'read' },
})
async getDisplaySettings(_input: Record<string, never>, ctx: McpContext) {
return ok({ settings: this.readDisplayPreferences(ctx.userId) });
}
@Tool({
name: 'update_display_settings',
description: `Change one or more of the current user's display preferences. Pass a settings object holding only the keys to change; anything left out keeps its current value. Writable keys: ${KEY_LIST}. start_page is dashboard or active_trip, start_trip_tab is one of plan, transports, buchungen, listen, finanzplan, dateien, collab (the ids are the planner's internal German ones) or plugin:<id> for a plugin tab, default_currency is a three-letter ISO code or "" to fall back to each trip's own currency, dark_mode is light, dark or auto. Every other key is refused, including API keys, map tokens, LLM endpoint settings and SMTP: those belong to the account owner or to whoever operates the instance and are set in TREK itself, not here. Prefer get_display_settings when you only need to read a value.`,
inputSchema: {
settings: settingsBulkRequestSchema.shape.settings
.describe(`Object of preference key to new value, e.g. {"temperature_unit": "fahrenheit", "distance_unit": "imperial"}. At least one key, and every key must be one of: ${KEY_LIST}.`),
},
annotations: TOOL_ANNOTATIONS_WRITE,
access: { group: 'settings', mode: 'write' },
})
async updateDisplaySettings({ settings }: { settings: Record<string, unknown> }, ctx: McpContext) {
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
const entries = Object.entries(settings);
if (entries.length === 0) {
return errorResult(`No preference given. Pass at least one of: ${KEY_LIST}.`);
}
const rejected = entries.map(([key]) => key).filter((key) => !ALLOWED.has(key));
if (rejected.length > 0) {
return errorResult(`Not a display preference: ${rejected.join(', ')}. This tool writes only ${KEY_LIST}. Credentials and instance configuration (API keys, map tokens, LLM endpoints, SMTP) are not reachable from MCP at all.`);
}
// Everything is validated before anything is written, so a bad key in a
// multi-key call cannot leave half the preferences changed. The service
// wraps its own writes in a transaction, but it never sees this call.
const validated: Record<string, unknown> = {};
for (const [key, value] of entries) {
// The client echoes redacted secrets back as this sentinel and
// bulkUpsertSettings skips them, which for a display preference would be
// a silent no-op rather than the refusal a caller can learn from.
if (value === MASKED_SETTING_VALUE) {
return errorResult(`Invalid value for ${key}: ${MASKED_SETTING_VALUE} is the placeholder TREK shows in place of a redacted secret, not a value.`);
}
const parsed = DISPLAY_PREFERENCES[key as DisplayPreferenceKey].safeParse(value);
if (!parsed.success) {
const reason = parsed.error.issues[0]?.message ?? 'not accepted';
return errorResult(`Invalid value for ${key}: ${JSON.stringify(value)}. ${reason}`);
}
validated[key] = parsed.data;
}
const updated = this.settings.bulkUpsertSettings(ctx.userId, validated);
// Read back rather than echoing the input: an admin default or the managed
// token injection can still shape what the user ends up seeing.
return ok({ success: true, updated, settings: this.readDisplayPreferences(ctx.userId) });
}
}
+4 -2
View File
@@ -3,14 +3,16 @@ import { AdminDefaultUserSettingsController, SettingsController } from './settin
import { AuthModule } from '../auth/auth.module';
import { AuditModule } from '../audit/audit.module';
import { SettingsService } from './settings.service';
import { SettingsMcp } from './settings.mcp';
import { AppConfigModule } from '../app-config/app-config.module';
/** Exports SettingsService for in-container consumers (admin, share, llm-parse). */
@Module({
// AuthModule for the admin gate on the defaults routes.
// AuthModule for the admin gate on the defaults routes, and for the
// AuthService that SettingsMcp's demo gate injects.
imports: [AppConfigModule, AuthModule, AuditModule],
controllers: [SettingsController, AdminDefaultUserSettingsController],
providers: [SettingsService],
providers: [SettingsService, SettingsMcp],
exports: [SettingsService],
})
export class SettingsModule {}
+7 -2
View File
@@ -4,7 +4,12 @@ import { decrypt_api_key, maybe_encrypt_api_key } from '../common/crypto/apiKeyC
import { MASKED_SETTING_VALUE, normalizeAppearance } from '@trek/shared';
import { readEnv } from '../../app-config';
const ENCRYPTED_SETTING_KEYS = new Set([
/**
* Exported so a caller that hands settings to somebody else can assert its own
* allow-list against the live values rather than a copy: a sixth key added here
* has to fail that assertion, which a hand-typed list would not.
*/
export const ENCRYPTED_SETTING_KEYS = new Set([
'webhook_url',
'ntfy_token',
'mapbox_access_token',
@@ -13,7 +18,7 @@ const ENCRYPTED_SETTING_KEYS = new Set([
]);
// Encrypted keys that are masked (••••••••) when returned to the client.
// Keys not in this set but in ENCRYPTED_SETTING_KEYS are decrypted and returned.
const MASKED_SETTING_KEYS = new Set(['webhook_url', 'ntfy_token', 'llm_api_key']);
export const MASKED_SETTING_KEYS = new Set(['webhook_url', 'ntfy_token', 'llm_api_key']);
export const DEFAULTABLE_USER_SETTING_KEYS = [
'temperature_unit',
@@ -0,0 +1,558 @@
/**
* Unit tests for the settings MCP surface (SettingsMcp):
* get_display_settings and update_display_settings.
*
* The security half carries the weight here. `settings` is one flat key/value
* table that holds display preferences next to encrypted credentials, and
* SettingsService.getUserSettings hands several of those back decrypted, so
* these tests pin that the allow-list is the only thing either tool can see or
* touch: a stored mapbox_access_token must not appear in a read, and neither
* that key nor llm_api_key nor an unknown name may be written.
*/
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
const { testDb, dbMock } = vi.hoisted(() => {
const Database = require('better-sqlite3');
const db = new Database(':memory:');
db.exec('PRAGMA journal_mode = WAL');
db.exec('PRAGMA foreign_keys = ON');
db.exec('PRAGMA busy_timeout = 5000');
const mock = {
db,
closeDb: () => {},
reinitialize: () => {},
getPlaceWithTags: () => null,
canAccessTrip: (tripId: any, userId: number) =>
db.prepare(`SELECT t.id, t.user_id FROM trips t LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)`).get(userId, tripId, userId),
isOwner: (tripId: any, userId: number) =>
!!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId),
};
return { testDb: db, dbMock: mock };
});
vi.mock('../../../src/db/database', () => dbMock);
vi.mock('../../../src/config', () => ({
JWT_SECRET: 'test-jwt-secret-for-trek-testing-only',
ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2',
updateJwtSecret: () => {},
}));
vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() }));
import { createTables } from '../../../src/db/schema';
import { runMigrations } from '../../../src/db/migrations';
import { resetTestDb } from '../../helpers/test-db';
import { createUser } from '../../helpers/factories';
import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness';
import { encrypt_api_key } from '../../../src/nest/common/crypto/apiKeyCrypto';
import { MASKED_SETTING_VALUE } from '@trek/shared';
import { DISPLAY_PREFERENCE_KEYS } from '../../../src/nest/settings/settings.mcp';
import { MANAGED_LOCKED_SETTING_KEYS } from '../../../src/nest/common/managed';
import { isAdminOnlyLlmSetting, ENCRYPTED_SETTING_KEYS, MASKED_SETTING_KEYS } from '../../../src/nest/settings/settings.service';
beforeAll(() => {
createTables(testDb);
runMigrations(testDb);
});
beforeEach(() => {
resetTestDb(testDb);
delete process.env.DEMO_MODE;
});
afterAll(() => {
testDb.close();
});
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
function setSetting(userId: number, key: string, value: string): void {
testDb.prepare(
'INSERT INTO settings (user_id, key, value) VALUES (?, ?, ?) ' +
'ON CONFLICT(user_id, key) DO UPDATE SET value = excluded.value',
).run(userId, key, value);
}
function readSetting(userId: number, key: string): string | undefined {
const row = testDb.prepare('SELECT value FROM settings WHERE user_id = ? AND key = ?').get(userId, key) as
| { value: string }
| undefined;
return row?.value;
}
function countSettings(userId: number): number {
return (testDb.prepare('SELECT COUNT(*) as c FROM settings WHERE user_id = ?').get(userId) as { c: number }).c;
}
async function withHarness(userId: number, fn: (h: McpHarness) => Promise<void>) {
const h = await createMcpHarness({ userId, withResources: false });
try { await fn(h); } finally { await h.cleanup(); }
}
async function withScopedHarness(userId: number, scopes: string[] | null, fn: (h: McpHarness) => Promise<void>) {
const h = await createMcpHarness({ userId, withResources: false, scopes });
try { await fn(h); } finally { await h.cleanup(); }
}
async function update(h: McpHarness, settings: Record<string, unknown>) {
return h.client.callTool({ name: 'update_display_settings', arguments: { settings } });
}
/** The message of an isError result, for asserting the wording a model reads. */
function errorText(result: any): string {
return String(result.content?.[0]?.text ?? '');
}
// ---------------------------------------------------------------------------
// get_display_settings
// ---------------------------------------------------------------------------
describe('Tool: get_display_settings', () => {
it('returns an empty object for a user who has set nothing', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await h.client.callTool({ name: 'get_display_settings', arguments: {} });
const data = parseToolResult(result) as any;
expect(data.settings).toEqual({});
});
});
it('returns the display preferences the user has stored', async () => {
const { user } = createUser(testDb);
setSetting(user.id, 'temperature_unit', '"fahrenheit"');
setSetting(user.id, 'distance_unit', '"imperial"');
setSetting(user.id, 'time_format', '"12h"');
setSetting(user.id, 'language', '"de"');
setSetting(user.id, 'default_currency', '"USD"');
setSetting(user.id, 'start_page', '"active_trip"');
setSetting(user.id, 'blur_booking_codes', 'true');
await withHarness(user.id, async (h) => {
const result = await h.client.callTool({ name: 'get_display_settings', arguments: {} });
const data = parseToolResult(result) as any;
expect(data.settings).toEqual({
start_page: 'active_trip',
default_currency: 'USD',
language: 'de',
temperature_unit: 'fahrenheit',
distance_unit: 'imperial',
time_format: '12h',
blur_booking_codes: true,
});
});
});
it('falls back to the admin-set instance default for a key the user has not set', async () => {
const { user } = createUser(testDb);
testDb.prepare('INSERT INTO app_settings (key, value) VALUES (?, ?)')
.run('default_user_setting_temperature_unit', '"fahrenheit"');
await withHarness(user.id, async (h) => {
const result = await h.client.callTool({ name: 'get_display_settings', arguments: {} });
const data = parseToolResult(result) as any;
expect(data.settings.temperature_unit).toBe('fahrenheit');
});
});
it('does not return stored credentials, not even to a settings:read token', async () => {
const { user } = createUser(testDb);
// mapbox_access_token and carto_api_key are encrypted-but-NOT-masked, so
// getUserSettings hands them back in cleartext. That is the leak this
// surface must not have: store them the way the real write path does and
// assert they are nowhere in the result.
setSetting(user.id, 'mapbox_access_token', String(encrypt_api_key('pk.super-secret-token')));
setSetting(user.id, 'carto_api_key', String(encrypt_api_key('carto-secret')));
setSetting(user.id, 'llm_api_key', String(encrypt_api_key('sk-secret')));
setSetting(user.id, 'ntfy_token', String(encrypt_api_key('ntfy-secret')));
setSetting(user.id, 'webhook_url', String(encrypt_api_key('https://hook.example/secret')));
setSetting(user.id, 'temperature_unit', '"celsius"');
await withScopedHarness(user.id, ['settings:read'], async (h) => {
const result = await h.client.callTool({ name: 'get_display_settings', arguments: {} });
const data = parseToolResult(result) as any;
expect(data.settings).toEqual({ temperature_unit: 'celsius' });
const serialized = JSON.stringify(data);
expect(serialized).not.toContain('pk.super-secret-token');
expect(serialized).not.toContain('carto-secret');
expect(serialized).not.toContain('sk-secret');
expect(serialized).not.toContain('ntfy-secret');
expect(serialized).not.toContain('hook.example');
for (const key of ['mapbox_access_token', 'carto_api_key', 'llm_api_key', 'ntfy_token', 'webhook_url']) {
expect(Object.keys(data.settings)).not.toContain(key);
}
});
});
it('leaves out settings that are neither display preferences nor credentials', async () => {
const { user } = createUser(testDb);
setSetting(user.id, 'map_tile_url', '"https://tiles.example/{z}/{x}/{y}.png"');
setSetting(user.id, 'dashboard_fx_from', '"EUR"');
setSetting(user.id, 'time_format', '"24h"');
await withHarness(user.id, async (h) => {
const result = await h.client.callTool({ name: 'get_display_settings', arguments: {} });
const data = parseToolResult(result) as any;
expect(data.settings).toEqual({ time_format: '24h' });
});
});
it('does not leak another user\'s preferences', async () => {
const { user: mine } = createUser(testDb);
const { user: theirs } = createUser(testDb);
setSetting(theirs.id, 'temperature_unit', '"fahrenheit"');
await withHarness(mine.id, async (h) => {
const result = await h.client.callTool({ name: 'get_display_settings', arguments: {} });
const data = parseToolResult(result) as any;
expect(data.settings).toEqual({});
});
});
});
// ---------------------------------------------------------------------------
// update_display_settings: happy paths
// ---------------------------------------------------------------------------
describe('Tool: update_display_settings', () => {
it('writes several preferences at once and reports what changed', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, { temperature_unit: 'fahrenheit', distance_unit: 'imperial' });
const data = parseToolResult(result) as any;
expect(data.success).toBe(true);
expect(data.updated).toBe(2);
expect(data.settings).toEqual({ temperature_unit: 'fahrenheit', distance_unit: 'imperial' });
expect(readSetting(user.id, 'temperature_unit')).toBe('fahrenheit');
expect(readSetting(user.id, 'distance_unit')).toBe('imperial');
});
});
it('leaves untouched keys alone', async () => {
const { user } = createUser(testDb);
setSetting(user.id, 'time_format', '"12h"');
await withHarness(user.id, async (h) => {
await update(h, { temperature_unit: 'celsius' });
expect(readSetting(user.id, 'time_format')).toBe('"12h"');
});
});
it('accepts every non-boolean display preference in one call', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, {
start_page: 'active_trip',
start_trip_tab: 'finanzplan',
default_currency: 'JPY',
language: 'ja',
temperature_unit: 'celsius',
distance_unit: 'metric',
time_format: '24h',
dark_mode: 'auto',
});
const data = parseToolResult(result) as any;
expect(data.updated).toBe(8);
expect(data.settings.start_trip_tab).toBe('finanzplan');
expect(data.settings.dark_mode).toBe('auto');
expect(data.settings.language).toBe('ja');
});
});
it('accepts every boolean display preference', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, {
map_booking_labels: true,
map_always_show_routes: false,
map_poi_pill_enabled: true,
blur_booking_codes: false,
optimize_from_accommodation: true,
});
const data = parseToolResult(result) as any;
expect(data.updated).toBe(5);
expect(data.settings).toEqual({
map_booking_labels: true,
map_always_show_routes: false,
map_poi_pill_enabled: true,
blur_booking_codes: false,
optimize_from_accommodation: true,
});
});
});
it('accepts a boolean dark_mode, the pre-auto form still sitting in older rows', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, { dark_mode: false });
const data = parseToolResult(result) as any;
expect(data.settings.dark_mode).toBe(false);
expect(readSetting(user.id, 'dark_mode')).toBe('false');
});
});
it('accepts an empty default_currency, which falls back to each trip\'s own', async () => {
const { user } = createUser(testDb);
setSetting(user.id, 'default_currency', '"USD"');
await withHarness(user.id, async (h) => {
const result = await update(h, { default_currency: '' });
const data = parseToolResult(result) as any;
expect(data.success).toBe(true);
expect(readSetting(user.id, 'default_currency')).toBe('');
});
});
it('reads back the admin default rather than echoing the input', async () => {
const { user } = createUser(testDb);
testDb.prepare('INSERT INTO app_settings (key, value) VALUES (?, ?)')
.run('default_user_setting_distance_unit', '"imperial"');
await withHarness(user.id, async (h) => {
const result = await update(h, { temperature_unit: 'celsius' });
const data = parseToolResult(result) as any;
expect(data.settings.distance_unit).toBe('imperial');
});
});
});
// ---------------------------------------------------------------------------
// update_display_settings: refusals
// ---------------------------------------------------------------------------
describe('Tool: update_display_settings, refusals', () => {
it('refuses a credential key and stores nothing', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, { mapbox_access_token: 'pk.attacker-token' });
expect(result.isError).toBe(true);
expect(errorText(result)).toContain('mapbox_access_token');
expect(countSettings(user.id)).toBe(0);
});
});
it('refuses llm_api_key and stores nothing, even for a settings:write token', async () => {
const { user } = createUser(testDb);
await withScopedHarness(user.id, ['settings:write'], async (h) => {
const result = await update(h, { llm_api_key: 'sk-attacker' });
expect(result.isError).toBe(true);
expect(countSettings(user.id)).toBe(0);
});
});
it('refuses the two admin-only LLM endpoint keys the REST route answers 403 for', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
for (const settings of [{ llm_base_url: 'http://127.0.0.1:11434' }, { llm_provider: 'local' }]) {
const result = await update(h, settings);
expect(result.isError).toBe(true);
}
expect(countSettings(user.id)).toBe(0);
});
});
it('refuses a key nobody has heard of rather than storing it', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, { made_up_preference: 'whatever' });
expect(result.isError).toBe(true);
expect(errorText(result)).toContain('made_up_preference');
expect(countSettings(user.id)).toBe(0);
expect(readSetting(user.id, 'made_up_preference')).toBeUndefined();
});
});
it('refuses an inherited Object property name, which a plain `in` check would wave through', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, { toString: 'gotcha' });
expect(result.isError).toBe(true);
expect(countSettings(user.id)).toBe(0);
});
});
it('writes nothing at all when one key of a multi-key call is refused', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, { temperature_unit: 'fahrenheit', llm_api_key: 'sk-attacker' });
expect(result.isError).toBe(true);
expect(countSettings(user.id)).toBe(0);
});
});
it('writes nothing when one value of a multi-key call is invalid', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, { temperature_unit: 'fahrenheit', time_format: '36h' });
expect(result.isError).toBe(true);
expect(countSettings(user.id)).toBe(0);
});
});
it('refuses a value outside the allowed set and names the key', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, { temperature_unit: 'kelvin' });
expect(result.isError).toBe(true);
expect(errorText(result)).toContain('temperature_unit');
expect(readSetting(user.id, 'temperature_unit')).toBeUndefined();
});
});
it('refuses an unsupported language code', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const bad = await update(h, { language: 'xx' });
expect(bad.isError).toBe(true);
const good = await update(h, { language: 'fr' });
expect(good.isError).toBeFalsy();
expect(readSetting(user.id, 'language')).toBe('fr');
});
});
it('refuses a currency that is not a three-letter uppercase code', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
for (const value of ['eur', 'EUROS', 'E']) {
const result = await update(h, { default_currency: value });
expect(result.isError).toBe(true);
}
expect(countSettings(user.id)).toBe(0);
});
});
it('refuses a non-boolean for a boolean preference', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, { blur_booking_codes: 'yes' });
expect(result.isError).toBe(true);
expect(countSettings(user.id)).toBe(0);
});
});
it('refuses a start_trip_tab that is empty or absurdly long', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
expect((await update(h, { start_trip_tab: '' })).isError).toBe(true);
expect((await update(h, { start_trip_tab: 'x'.repeat(65) })).isError).toBe(true);
expect(countSettings(user.id)).toBe(0);
});
});
it('refuses the redaction placeholder instead of silently skipping it', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, { start_trip_tab: MASKED_SETTING_VALUE });
expect(result.isError).toBe(true);
expect(errorText(result)).toContain('start_trip_tab');
expect(countSettings(user.id)).toBe(0);
});
});
it('refuses an empty settings object and says what it takes', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const result = await update(h, {});
expect(result.isError).toBe(true);
expect(errorText(result)).toContain('temperature_unit');
expect(countSettings(user.id)).toBe(0);
});
});
it('blocks the demo user', async () => {
process.env.DEMO_MODE = 'true';
const { user } = createUser(testDb, { email: 'demo@trek.app' });
await withHarness(user.id, async (h) => {
const result = await update(h, { temperature_unit: 'fahrenheit' });
expect(result.isError).toBe(true);
expect(countSettings(user.id)).toBe(0);
});
});
it('lets the demo user read', async () => {
process.env.DEMO_MODE = 'true';
const { user } = createUser(testDb, { email: 'demo@trek.app' });
setSetting(user.id, 'temperature_unit', '"celsius"');
await withHarness(user.id, async (h) => {
const result = await h.client.callTool({ name: 'get_display_settings', arguments: {} });
const data = parseToolResult(result) as any;
expect(data.settings.temperature_unit).toBe('celsius');
});
});
});
// ---------------------------------------------------------------------------
// The allow-list itself
// ---------------------------------------------------------------------------
describe('Display-preference allow-list', () => {
it('holds no key the operator owns on a managed install', () => {
// The REST route needs isManagedLockedKey because it takes any key at all.
// Here the two lists must simply not overlap, which is what makes the
// managed lock unreachable rather than merely duplicated.
const overlap = DISPLAY_PREFERENCE_KEYS.filter((key) =>
(MANAGED_LOCKED_SETTING_KEYS as readonly string[]).includes(key));
expect(overlap).toEqual([]);
});
it('holds no key assertMayWriteLlmEndpoint would have to refuse', () => {
// isAdminOnlyLlmSetting is value-dependent, so probe it with the values
// that trip it rather than matching on the key name.
const offenders = DISPLAY_PREFERENCE_KEYS.filter((key) =>
isAdminOnlyLlmSetting(key, 'http://127.0.0.1:11434') || isAdminOnlyLlmSetting(key, 'local'));
expect(offenders).toEqual([]);
});
// Against the live sets, not a copy of them: a sixth encrypted key added to
// the service has to fail here, which is the whole point of the allow-list.
it('holds no key that is encrypted at rest', () => {
const overlap = [...ENCRYPTED_SETTING_KEYS].filter(key => (DISPLAY_PREFERENCE_KEYS as readonly string[]).includes(key));
expect(overlap).toEqual([]);
});
it('holds no key that is masked on the way out', () => {
const overlap = [...MASKED_SETTING_KEYS].filter(key => (DISPLAY_PREFERENCE_KEYS as readonly string[]).includes(key));
expect(overlap).toEqual([]);
});
});
// ---------------------------------------------------------------------------
// Scope gating
// ---------------------------------------------------------------------------
describe('Settings tools, scope gating', () => {
async function listToolNames(userId: number, scopes: string[] | null): Promise<string[]> {
const h = await createMcpHarness({ userId, withResources: false, scopes });
try {
return (await h.client.listTools()).tools.map((t) => t.name);
} finally {
await h.cleanup();
}
}
it('registers both tools with null scopes (full access)', async () => {
const { user } = createUser(testDb);
const names = await listToolNames(user.id, null);
expect(names).toContain('get_display_settings');
expect(names).toContain('update_display_settings');
});
it('registers only the read tool with settings:read', async () => {
const { user } = createUser(testDb);
const names = await listToolNames(user.id, ['settings:read']);
expect(names).toContain('get_display_settings');
expect(names).not.toContain('update_display_settings');
});
it('registers both with settings:write, since write implies read', async () => {
const { user } = createUser(testDb);
const names = await listToolNames(user.id, ['settings:write']);
expect(names).toContain('get_display_settings');
expect(names).toContain('update_display_settings');
});
it('registers neither for a token without a settings scope', async () => {
const { user } = createUser(testDb);
const names = await listToolNames(user.id, ['trips:read']);
expect(names).not.toContain('get_display_settings');
expect(names).not.toContain('update_display_settings');
});
});