From a798c461dbd465fdff147611ac59d95fa68224b7 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Fri, 16 Oct 2026 12:12:50 +0200 Subject: [PATCH] fix(multi-user): skip server-wide settings writes for a non-admin browser With PUT /api/settings admin-only in multi-user mode, a non-admin's page must not run into the 403. `_settingsReadOnly()` (settings-ui.js) reads the role from window.__codemanUser; single-user mode and admins are unchanged. For a non-admin, App Settings Save/Apply still saves the per-device keys to localStorage exactly as before, but sends neither the settings PUT nor the model-config PUT, and keeps every synced key at the server's value (the next page load would put it back anyway). Only when the user really changed a synced setting do they get one toast that server-wide settings can only be changed by an admin; a form baseline taken at open keeps an untouched default that no admin ever stored from reading as a change. The background writes (run mode, respawn presets and their load-time migration, last used case, case order, the tunnel toggles) are not sent, and a 403 FORBIDDEN on a tunnel enable is no longer mistaken for the no-password prompt. zh-CN for both toasts. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/web/public/i18n.js | 5 ++ src/web/public/respawn-ui.js | 5 +- src/web/public/session-ui.js | 18 +++- src/web/public/settings-ui.js | 159 +++++++++++++++++++++++++++++++--- 4 files changed, 173 insertions(+), 14 deletions(-) diff --git a/src/web/public/i18n.js b/src/web/public/i18n.js index bff9685b..42657a23 100644 --- a/src/web/public/i18n.js +++ b/src/web/public/i18n.js @@ -847,6 +847,11 @@ 'Settings applied': '设置已应用', 'Save and keep Settings open': '保存并保持设置打开', 'Settings saved locally': '设置已保存到本机', + // Multi-user mode, a non-admin (settings-ui.js / session-ui.js): server-wide settings + // writes are admin-only there. + 'Saved on this device. Server-wide settings can only be changed by an admin.': + '已保存到本设备。服务器全局设置只能由管理员更改。', + 'Server-wide settings can only be changed by an admin': '服务器全局设置只能由管理员更改', 'Tunnel active': '隧道已启用', 'Tunnel starting — QR code will appear when ready...': '隧道正在启动，准备好后将显示二维码…', 'Push notifications enabled': '推送通知已启用', diff --git a/src/web/public/respawn-ui.js b/src/web/public/respawn-ui.js index 726d526a..23924dd7 100644 --- a/src/web/public/respawn-ui.js +++ b/src/web/public/respawn-ui.js @@ -597,7 +597,10 @@ Object.assign(CodemanApp.prototype, { // Update local cache + legacy localStorage this._serverRespawnPresets = custom; localStorage.setItem('codeman-respawn-presets', JSON.stringify(custom)); - // Persist to server (cross-device sync) + // Persist to server (cross-device sync). Not for a non-admin in multi-user mode: the + // presets are a key of the server-wide settings.json there, and its PUT is admin-only + // (403), so they stay on this device (_settingsReadOnly, settings-ui.js). + if (this._settingsReadOnly?.()) return; this._apiPut('/api/settings', { respawnPresets: custom }).catch(() => {}); }, diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 8e83f96c..e74a6169 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -627,6 +627,9 @@ Object.assign(CodemanApp.prototype, { }, async saveLastUsedCase(caseName) { + // Multi-user non-admin: lastUsedCase is a key of the server-wide settings.json, whose + // PUT is admin-only (403), so it is not sent (_settingsReadOnly, settings-ui.js). + if (this._settingsReadOnly?.()) return; try { await fetch('/api/settings', { method: 'PUT', @@ -715,8 +718,9 @@ Object.assign(CodemanApp.prototype, { this._runMode = mode; try { localStorage.setItem('codeman_runMode', mode); } catch {} this._applyRunMode(); - // Sync to server for cross-device persistence - this._apiPut('/api/settings', { runMode: mode }).catch(() => {}); + // Sync to server for cross-device persistence. Not for a non-admin in multi-user mode, + // where settings.json is server-wide and its PUT admin-only: the mode stays on this device. + if (!this._settingsReadOnly?.()) this._apiPut('/api/settings', { runMode: mode }).catch(() => {}); // Close menu document.getElementById('runModeMenu')?.classList.remove('active'); }, @@ -4881,6 +4885,16 @@ Object.assign(CodemanApp.prototype, { }, async saveCaseOrder(order) { + // Multi-user non-admin: the case order is ONE server-wide list (settings.json caseOrder) + // and PUT /api/cases/order is admin-only (403), so the move is not sent: say why, and put + // the list back the way the server orders it. + if (this._settingsReadOnly?.()) { + this.showToast('Server-wide settings can only be changed by an admin', 'warning'); + const select = document.getElementById('quickStartCase'); + await this.loadQuickStartCases(select?.value); + this.renderCaseManageList(); + return; + } try { await fetch('/api/cases/order', { method: 'PUT', diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 3bbfc671..00b6ed2c 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -711,6 +711,10 @@ Object.assign(CodemanApp.prototype, { // Activate focus trap this.activeFocusTrap = new FocusTrap(modal); this.activeFocusTrap.activate(); + + // Multi-user non-admin only: remember what the form shows now, so Save can tell a real + // change to a server-wide setting from an untouched one (see the method). + this._captureSettingsFormBaseline(); }, /** @@ -1976,6 +1980,11 @@ Object.assign(CodemanApp.prototype, { async toggleTunnelFromWelcome() { const btn = document.getElementById('welcomeTunnelBtn'); if (!btn) return; + // The tunnel is a server-wide setting (PUT /api/settings), admin-only in multi-user mode. + if (this._settingsReadOnly()) { + this.showToast('Server-wide settings can only be changed by an admin', 'warning'); + return; + } const isActive = btn.classList.contains('active'); btn.disabled = true; try { @@ -2302,12 +2311,22 @@ Object.assign(CodemanApp.prototype, { async _handleTunnelEnableRefusal(res) { if (!res || res.ok) return false; let message = 'Tunnel refused: set CODEMAN_PASSWORD before exposing Codeman publicly.'; + let errorCode = ''; try { const body = await res.json(); if (body && body.error) message = body.error; + if (body && typeof body.errorCode === 'string') errorCode = body.errorCode; } catch { /* non-JSON body — use the default message */ } + // A non-admin in multi-user mode: PUT /api/settings is admin-only there (403 FORBIDDEN). + // That is not the no-password refusal below, so no security prompt and no retry. The + // callers skip the PUT for a known non-admin; this covers a role that had not resolved yet. + if (res.status === 403 && errorCode === 'FORBIDDEN') { + this._dismissTunnelConnecting?.(); + this.showToast('Server-wide settings can only be changed by an admin', 'warning'); + return true; + } // 403 = the no-password safety refusal (COD-55). Warn loudly and let the // operator acknowledge the risk; on confirm, retry with explicit acknowledgment. if (res.status === 403) { @@ -2356,6 +2375,12 @@ Object.assign(CodemanApp.prototype, { }, async _tunnelPanelToggle(enable) { + // The tunnel is a server-wide setting (PUT /api/settings), admin-only in multi-user mode. + if (this._settingsReadOnly()) { + this.showToast('Server-wide settings can only be changed by an admin', 'warning'); + this.closeTunnelPanel(); + return; + } try { const res = await fetch('/api/settings', { method: 'PUT', @@ -2692,6 +2717,13 @@ Object.assign(CodemanApp.prototype, { }, }; + // One-shot form read for _captureSettingsFormBaseline(): hand back what the form + // shows, before anything below is checked, stored, applied or sent. + if (this._readSettingsFormOnce === true) { + this._readSettingsFormOnce = false; + return settings; + } + // SettingsUpdateSchema is .strict() and checks codexModel with this same // pattern, so one bad character 400s the WHOLE settings PUT while the toast // still says "Settings saved". Refuse it here, before anything is persisted. @@ -2714,6 +2746,14 @@ Object.assign(CodemanApp.prototype, { // general-settings DOM), so the fresh rebuild would drop them on every save. if (_prev.shortcutOverrides !== undefined) settings.shortcutOverrides = _prev.shortcutOverrides; + // Multi-user mode, signed in as a non-admin: settings.json is one file for the whole + // server and PUT /api/settings answers 403 (system-routes.ts), so nothing is sent + // below. The per-device keys still save here exactly as before; every SYNCED key + // keeps the server's value, which the next page load would put back anyway, so this + // device never shows a server-wide change that did not happen. + const settingsReadOnly = this._settingsReadOnly(); + const syncedChangeRefused = settingsReadOnly && this._keepServerSyncedSettings(settings, _prev); + // Save to localStorage this.saveAppSettingsToStorage(settings); this._updateLocalEchoState(); @@ -2934,7 +2974,9 @@ Object.assign(CodemanApp.prototype, { let webhookError = ''; let serverSaved = false; try { - const res = await this._apiPut('/api/settings', { + // A non-admin's save is not sent at all (see settingsReadOnly above): the server + // would refuse it with a 403, the plan-usage collection flip included. + const res = settingsReadOnly ? null : await this._apiPut('/api/settings', { ...serverSettings, ...(_chipFlip !== undefined ? { showPlanUsageLimits: _chipFlip } : {}), notificationPreferences: notifPrefsToSave, @@ -2958,8 +3000,9 @@ Object.assign(CodemanApp.prototype, { // evidence the server kept the flags the Apply refresh below reads. serverSaved = !!res?.ok; - // Save model configuration separately - await this.saveModelConfigFromSettings(); + // Save model configuration separately. For a non-admin it is not sent either, and the + // answer says whether this save would have changed it. + const modelConfigRefused = (await this.saveModelConfigFromSettings()) === true; // The webhook is server state in its own 0600 file (its URL is a secret, kept out of // settings.json), so like the model config above it is saved beside the settings PUT, not in @@ -2968,12 +3011,17 @@ Object.assign(CodemanApp.prototype, { webhookError = this._webhookPending() ? await this.saveWebhook() : ''; if (webhookError) { this.showToast(`Settings saved, but not the webhook: ${webhookError}`, 'warning'); + } else if (syncedChangeRefused || modelConfigRefused) { + // ONE toast for the whole save, and only when the user really changed a server-wide + // setting: a non-admin saving a font size hears "Settings saved" as everyone does. + this.showToast('Saved on this device. Server-wide settings can only be changed by an admin.', 'warning'); } else { this.showToast(keepOpen ? 'Settings applied' : 'Settings saved', 'success'); } - // Show tunnel-specific feedback if toggled on - if (settings.tunnelEnabled) { + // Show tunnel-specific feedback if toggled on (never for a non-admin, whose save + // starts nothing: the box only shows what the server already has) + if (settings.tunnelEnabled && !settingsReadOnly) { this.showToast('Tunnel starting — QR code will appear when ready...', 'info'); } } catch (err) { @@ -2999,7 +3047,9 @@ Object.assign(CodemanApp.prototype, { // gestureControlEnabled from settings.json), so a change only takes effect on // reload. Reload when it actually changed — the server PUT above already // persisted the new value. - if (settings.gestureControlEnabled !== _prevGestureEnabled) { + // `?? false`: a non-admin's save puts a synced key the server never stored back to + // absent (_keepServerSyncedSettings), and absent means off, as in _prevGestureEnabled. + if ((settings.gestureControlEnabled ?? false) !== _prevGestureEnabled) { this.showToast( settings.gestureControlEnabled ? 'Enabling gesture control — reloading…' : 'Disabling gesture control — reloading…', 'info' @@ -3008,6 +3058,72 @@ Object.assign(CodemanApp.prototype, { } }, + /** + * True only for a signed-in NON-ADMIN in multi-user mode, where every server-wide settings + * write is refused with 403 FORBIDDEN (PUT /api/settings, PUT /api/execution/model-config and + * PUT /api/cases/order: settings.json is one file for the whole server). Single-user mode and + * admins answer false. Read at call time, because `window.__codemanUser` resolves after page + * load (admin-ui.js fetches /api/me). Other modules call it as `this._settingsReadOnly?.()`, + * so a harness that loads one of them without this file reads it as writable, which is what + * an admin gets. + */ + _settingsReadOnly() { + // typeof guard: the load path calls this, and some vm harnesses define no `window`. + const me = (typeof window !== 'undefined' && window.__codemanUser) || {}; + return !!(me.multiUser && me.role !== 'admin'); + }, + + /** + * What the App Settings form showed when it opened, read the way saveAppSettings() reads it + * (its one-shot `_readSettingsFormOnce` form read), so a non-admin's save can tell whether the + * user changed a SYNCED setting. The stored blob cannot answer that: a key no admin has ever + * saved is absent from it while the form shows the default, which would read as a change on + * every save. Taken only for a non-admin; nobody else's save asks for it. + */ + _captureSettingsFormBaseline() { + this._settingsFormBaseline = null; + if (!this._settingsReadOnly()) return; + this._readSettingsFormOnce = true; + let pending; + try { + pending = this.saveAppSettings(); + } finally { + // Consumed by the read itself; cleared here too in case it threw before reaching it. + this._readSettingsFormOnce = false; + } + Promise.resolve(pending) + .then((form) => { + this._settingsFormBaseline = form && typeof form === 'object' ? form : null; + }) + .catch(() => {}); + }, + + /** + * A non-admin's save in multi-user mode (saveAppSettings): put every SYNCED key of `settings` + * back to the value this device last had from the server (`prev`; deleted when it had none), + * and answer whether the form changed one since it opened. Synced means outside the per-device + * `displayKeys` merge policy of loadAppSettingsFromServer(), whose own list this reads, never a + * copy: those are the keys the server's settings.json overwrites on every page load. A + * per-device key is left exactly as the user set it. + * @returns {boolean} true when the user changed at least one synced setting + */ + _keepServerSyncedSettings(settings, prev) { + const displayKeys = this._displaySettingKeys; + if (!(displayKeys instanceof Set)) return false; + // Without a baseline (the form was read before the role was known) the stored blob stands + // in, and a key it lacks is not counted: no toast is better than one on every save. + const baseline = this._settingsFormBaseline || prev || {}; + let changed = false; + for (const key of Object.keys(settings)) { + // shortcutOverrides is carried over from `prev` already and never comes from the form. + if (displayKeys.has(key) || key === 'shortcutOverrides') continue; + if (key in baseline && JSON.stringify(settings[key]) !== JSON.stringify(baseline[key])) changed = true; + if (prev && key in prev) settings[key] = prev[key]; + else delete settings[key]; + } + return changed; + }, + /** * After Apply: bring the groups whose contents depend on a SAVED value up to date without * reopening the modal. openAppSettings does the same on open; this is the part of it that @@ -3058,10 +3174,12 @@ Object.assign(CodemanApp.prototype, { } catch (err) { console.warn('Failed to load model config:', err); } + // What the form shows now, so a non-admin's save can tell whether it was changed. + this._modelConfigBaseline = JSON.stringify(this._modelConfigFromSettingsForm()); }, - // Save model configuration from settings modal to server - async saveModelConfigFromSettings() { + /** The model configuration as the settings modal's form shows it (the PUT body). */ + _modelConfigFromSettingsForm() { const defaultModelEl = document.getElementById('appSettingsDefaultModel'); const showRecsEl = document.getElementById('appSettingsShowModelRecommendations'); const exploreEl = document.getElementById('appSettingsModelExplore'); @@ -3075,11 +3193,26 @@ Object.assign(CodemanApp.prototype, { if (testEl?.value) agentTypeOverrides.test = testEl.value; if (reviewEl?.value) agentTypeOverrides.review = reviewEl.value; - const config = { + return { defaultModel: defaultModelEl?.value || '', showRecommendations: showRecsEl?.checked ?? true, agentTypeOverrides, }; + }, + + /** + * Save model configuration from settings modal to server. + * @returns {Promise} true only when a non-admin in multi-user mode changed it: + * the config is a key of the server-wide settings.json and its PUT is admin-only (403), so it is + * not sent, and saveAppSettings() folds this into its one admin-only toast + */ + async saveModelConfigFromSettings() { + const config = this._modelConfigFromSettingsForm(); + + if (this._settingsReadOnly()) { + const baseline = this._modelConfigBaseline; + return typeof baseline === 'string' && JSON.stringify(config) !== baseline; + } try { await fetch('/api/execution/model-config', { @@ -4393,6 +4526,8 @@ Object.assign(CodemanApp.prototype, { // and should not be synced from the server to avoid overriding mobile defaults. // NOTE: Feature toggles (subagentTrackingEnabled, imageWatcherEnabled, ralphTrackerEnabled) // are NOT display keys — they control server-side behavior and must sync from server. + // Published below for _keepServerSyncedSettings() (a non-admin's save in multi-user + // mode), which needs this same split and must never keep a copy of it. const displayKeys = new Set([ 'showFontControls', 'showSystemStats', 'headerStatsStyle', 'showTokenCount', 'showCost', 'showLifecycleLog', 'showResponseViewer', 'showRedrawButton', @@ -4411,6 +4546,7 @@ Object.assign(CodemanApp.prototype, { 'showTileGridButton', 'tileDetachEnabled', ]); + this._displaySettingKeys = displayKeys; // The plan-usage chip is a PER-DEVICE display setting (desktop default ON, // handheld default OFF): desktop can show it while mobile stays hidden. Drop // the server's stored value here so it is NEVER seeded into a device that @@ -4461,8 +4597,9 @@ Object.assign(CodemanApp.prototype, { this._serverRespawnPresets = respawnPresets; // Also update localStorage for offline access localStorage.setItem('codeman-respawn-presets', JSON.stringify(respawnPresets)); - } else { - // Migration: push existing localStorage presets to server + } else if (!this._settingsReadOnly()) { + // Migration: push existing localStorage presets to server. Not for a non-admin in + // multi-user mode: the presets are a server-wide setting there (admin-only PUT). const localPresets = localStorage.getItem('codeman-respawn-presets'); if (localPresets) { const parsed = JSON.parse(localPresets);