)}
+ {/* Instance configuration, not a taste, which is why it sits here rather
+ than in a user's own Map settings. The origin has to appear in the CSP
+ `connect-src` the server emits at boot, and that list is built from
+ this default — a value on a personal settings row is read by the route
+ calculator and then refused by the browser, so the whole app stops
+ routing with no error it could report. The write route refuses one
+ from a non-admin for the same reason. */}
+ {!managed && (
+
)}
- {/* Routing is the same engine whichever renderer draws the map, so this sits outside
- the provider blocks. Tokens rather than the slate-* around it: theme:lint holds
- new markup to them, and a field added today should not need converting later. */}
- {!managed && (
-
- )}
-
{/* Same deal as the Mapbox token: a managed install brings its own key. */}
{provider === 'leaflet' && !managed && (
diff --git a/server/src/nest/settings/settings.controller.ts b/server/src/nest/settings/settings.controller.ts
index a86b64296..a00389f7a 100644
--- a/server/src/nest/settings/settings.controller.ts
+++ b/server/src/nest/settings/settings.controller.ts
@@ -2,7 +2,7 @@ import { Body, Controller, Get, HttpCode, HttpException, Post, Put, Req, UseGuar
import type { Request } from 'express';
import { MASKED_SETTING_VALUE } from '@trek/shared';
import type { User } from '../../types';
-import { SettingsService, isAdminOnlyLlmSetting } from './settings.service';
+import { SettingsService, isAdminOnlyEndpointSetting } from './settings.service';
import { SettingUpsertDto, SettingsBulkDto } from './settings.dto';
import { AdminDefaultUserSettingsDto } from '../admin/admin.dto';
import { JwtAuthGuard } from '../auth/jwt-auth.guard';
@@ -46,9 +46,9 @@ export class SettingsController {
* behind AdminGuard. A personal row would be a second, invisible place for
* the same value.
*/
- private assertMayWriteLlmEndpoint(settings: Record) {
+ private assertMayWriteInstanceEndpoint(settings: Record) {
for (const [key, value] of Object.entries(settings)) {
- if (isAdminOnlyLlmSetting(key, value)) {
+ if (isAdminOnlyEndpointSetting(key, value)) {
throw new HttpException({ error: 'Admin access required' }, 403);
}
}
@@ -61,8 +61,8 @@ export class SettingsController {
@Put()
upsert(@CurrentUser() user: User, @Body() body: SettingUpsertDto) {
- this.assertMayWriteLlmEndpoint({ [body.key]: body.value });
- // assertMayWriteLlmEndpoint only covers llm_base_url and provider 'local'.
+ this.assertMayWriteInstanceEndpoint({ [body.key]: body.value });
+ // assertMayWriteInstanceEndpoint only covers llm_base_url and provider 'local'.
// llm_api_key and llm_model are writable by every user, and on a managed
// install both cost the operator money, so the key list decides here.
if (isManagedLockedKey(body.key) && this.env.isManaged()) {
@@ -79,7 +79,7 @@ export class SettingsController {
@Post('bulk')
@HttpCode(200) // Express answers bulk with res.json (200), not the POST-default 201.
bulk(@CurrentUser() user: User, @Body() body: SettingsBulkDto) {
- this.assertMayWriteLlmEndpoint(body.settings);
+ this.assertMayWriteInstanceEndpoint(body.settings);
const { allowed, blocked } = splitManagedKeys(body.settings, this.env.isManaged());
try {
const updated = this.settings.bulkUpsertSettings(user.id, allowed);
diff --git a/server/src/nest/settings/settings.mcp.ts b/server/src/nest/settings/settings.mcp.ts
index 60324990d..fd3aa1656 100644
--- a/server/src/nest/settings/settings.mcp.ts
+++ b/server/src/nest/settings/settings.mcp.ts
@@ -83,8 +83,9 @@ const KEY_LIST = DISPLAY_PREFERENCE_KEYS.join(', ');
* 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
+ * than by being repeated: assertMayWriteInstanceEndpoint guards llm_base_url,
+ * llm_provider and routing_base_url, 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.
*/
diff --git a/server/src/nest/settings/settings.service.ts b/server/src/nest/settings/settings.service.ts
index fc9cf331d..53dd6b584 100644
--- a/server/src/nest/settings/settings.service.ts
+++ b/server/src/nest/settings/settings.service.ts
@@ -79,18 +79,30 @@ const VALID_VALUES: Partial> = {
const BOOLEAN_KEYS = new Set(['blur_booking_codes', 'mapbox_3d_enabled', 'mapbox_quality_mode', 'llm_multimodal']);
/**
- * #1772: per-user LLM settings a non-admin must not write. Both of them pick
- * the address this server sends its own LLM requests to (provider 'local' means
- * nothing but "an endpoint I name"), and the SSRF guard in front of those
- * requests allows loopback/LAN on purpose so a self-hosted Ollama works. Only
- * whoever runs the instance can judge what is reachable from it.
+ * Per-user settings that name an address, which a non-admin must not write.
*
- * Clearing stays open to everyone: both Settings sections always send
- * `llm_base_url: ''` while the field is hidden, so a row written before this
- * rule cleans itself up on the next save instead of blocking it.
+ * #1772 for the LLM pair: both pick where this server sends its own LLM
+ * requests (provider 'local' means nothing but "an endpoint I name"), and the
+ * SSRF guard in front of those requests allows loopback and LAN on purpose so a
+ * self-hosted Ollama works. Only whoever runs the instance can judge what is
+ * reachable from it.
+ *
+ * `routing_base_url` (#1797) is the same class for a different reason: it names
+ * an origin the BROWSER connects to, so it has to appear in the CSP
+ * `connect-src` the server emits at boot — and that list is built from the admin
+ * default, which is the only place a value can take effect. A row on a personal
+ * settings record is read by the route calculator and blocked by the browser, so
+ * the whole app stops routing with no catchable error. Refusing it here means
+ * the caller finds out instead of silently breaking their own map.
+ *
+ * Clearing stays open to everyone: the Settings sections send an empty string
+ * while the field is hidden, so a row written before this rule cleans itself up
+ * on the next save instead of blocking it.
*/
-export function isAdminOnlyLlmSetting(key: string, value: unknown): boolean {
- if (key === 'llm_base_url') return typeof value === 'string' && value.trim() !== '';
+export function isAdminOnlyEndpointSetting(key: string, value: unknown): boolean {
+ if (key === 'llm_base_url' || key === 'routing_base_url') {
+ return typeof value === 'string' && value.trim() !== '';
+ }
if (key === 'llm_provider') return value === 'local';
return false;
}
diff --git a/server/tests/e2e/settings.e2e.test.ts b/server/tests/e2e/settings.e2e.test.ts
index 2f0230dff..85aedd3d5 100644
--- a/server/tests/e2e/settings.e2e.test.ts
+++ b/server/tests/e2e/settings.e2e.test.ts
@@ -150,6 +150,31 @@ describe('Settings e2e (real auth guard + temp SQLite)', () => {
.toEqual({ value: 'anthropic' });
});
+ it('PUT 403 for a non-admin naming a routing engine (#1797)', async () => {
+ // Same class for a different reason: this origin has to appear in the CSP
+ // connect-src the server emits at boot, and that list is built from the
+ // admin default. A value on a personal row is read by the route
+ // calculator and then refused by the browser, so the app stops routing
+ // with no error it could report. Refusing tells the caller instead.
+ const res = await request(server).put('/api/settings').set('Cookie', sessionCookie(1))
+ .send({ key: 'routing_base_url', value: 'https://osrm.example.org' });
+ expect(res.status).toBe(403);
+ expect(res.body).toEqual({ error: 'Admin access required' });
+ expect(countRows()).toBe(0);
+ });
+
+ it('PUT lets a non-admin clear a routing engine somebody set before the rule', async () => {
+ expect((await request(server).put('/api/settings').set('Cookie', sessionCookie(1))
+ .send({ key: 'routing_base_url', value: '' })).status).toBe(200);
+ });
+
+ it('POST bulk refuses a routing engine from a non-admin', async () => {
+ const res = await request(server).post('/api/settings/bulk').set('Cookie', sessionCookie(1))
+ .send({ settings: { routing_base_url: 'https://osrm.example.org' } });
+ expect(res.status).toBe(403);
+ expect(countRows()).toBe(0);
+ });
+
it('PUT is unaffected for a non-LLM key with the same value', async () => {
const res = await request(server).put('/api/settings').set('Cookie', sessionCookie(1))
.send({ key: 'start_page', value: 'local' });
diff --git a/server/tests/unit/mcp/tools-settings.test.ts b/server/tests/unit/mcp/tools-settings.test.ts
index 212d74236..850a844d6 100644
--- a/server/tests/unit/mcp/tools-settings.test.ts
+++ b/server/tests/unit/mcp/tools-settings.test.ts
@@ -48,7 +48,7 @@ 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';
+import { isAdminOnlyEndpointSetting, ENCRYPTED_SETTING_KEYS, MASKED_SETTING_KEYS } from '../../../src/nest/settings/settings.service';
beforeAll(() => {
createTables(testDb);
@@ -493,11 +493,11 @@ describe('Display-preference allow-list', () => {
expect(overlap).toEqual([]);
});
- it('holds no key assertMayWriteLlmEndpoint would have to refuse', () => {
- // isAdminOnlyLlmSetting is value-dependent, so probe it with the values
+ it('holds no key assertMayWriteInstanceEndpoint would have to refuse', () => {
+ // isAdminOnlyEndpointSetting 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'));
+ isAdminOnlyEndpointSetting(key, 'http://127.0.0.1:11434') || isAdminOnlyEndpointSetting(key, 'local'));
expect(offenders).toEqual([]);
});