diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index 7ce72b7e..2af661a5 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -510,8 +510,10 @@ class PluginManager: if schema: try: prepared = normalize_legacy_booleans(prepared, schema, upgraded) - except Exception: - pass + except Exception as legacy_error: + self.logger.warning( + "Could not read legacy boolean settings for %s: %s", + plugin_id, legacy_error) if upgraded: self.logger.info( "Plugin %s: reading legacy boolean setting %s as " diff --git a/test/js/unit/test_update_all.js b/test/js/unit/test_update_all.js index f9c2cbbd..8fb9bf64 100644 --- a/test/js/unit/test_update_all.js +++ b/test/js/unit/test_update_all.js @@ -183,6 +183,22 @@ const noSleep = { sleep: async () => {} }; r = await run(() => httpAnswer(200, async () => { throw new SyntaxError('Unexpected end of JSON input'); })); ok('an unreadable 200 is not retried either', r.requests === 1 && r.result.error.error_code === 'API_ERROR', r); + + // Every endpoint is one of the client's own API paths; one that could + // leave baseURL never reaches fetch(), and plugin ids are encoded. + const urls = []; + global.fetch = async (url) => { urls.push(url); return httpAnswer(200, async () => ({ status: 'success' })); }; + const refusal = async (endpoint) => { + try { await PluginAPI.request(endpoint, 'POST'); return null; } catch (e) { return e.error_code; } + }; + const bad = ['//evil.example/x', '/plugins/../../x', '/a\b', '/a b', 'plugins', null]; + const codes = []; + for (const endpoint of bad) codes.push(await refusal(endpoint)); + ok('an endpoint that could leave the API path is refused before fetch()', + codes.every(c => c === 'INVALID_ENDPOINT') && urls.length === 0, { codes, urls }); + await PluginAPI.resetPluginConfig('a/../b&x=1'); + ok('a plugin id is encoded into the URL, not spliced into it', + urls[0] === '/api/v3/plugins/config/reset?plugin_id=a%2F..%2Fb%26x%3D1', urls); delete global.fetch; } diff --git a/web_interface/static/v3/js/app-shell.js b/web_interface/static/v3/js/app-shell.js index 4fbc22f7..ab865447 100644 --- a/web_interface/static/v3/js/app-shell.js +++ b/web_interface/static/v3/js/app-shell.js @@ -838,8 +838,8 @@ // PluginAPI.batch returns already-parsed JSON objects try { const results = await window.PluginAPI.batch([ - {endpoint: `/plugins/config?plugin_id=${pluginId}`, method: 'GET'}, - {endpoint: `/plugins/schema?plugin_id=${pluginId}`, method: 'GET'}, + {endpoint: `/plugins/config?plugin_id=${encodeURIComponent(pluginId)}`, method: 'GET'}, + {endpoint: `/plugins/schema?plugin_id=${encodeURIComponent(pluginId)}`, method: 'GET'}, {endpoint: '/plugins/installed', method: 'GET'} ]); [configData, schemaData, pluginsData] = results; diff --git a/web_interface/static/v3/js/plugins/api_client.js b/web_interface/static/v3/js/plugins/api_client.js index 0e54f03b..11908e4e 100644 --- a/web_interface/static/v3/js/plugins/api_client.js +++ b/web_interface/static/v3/js/plugins/api_client.js @@ -123,6 +123,28 @@ const PluginAPI = { */ baseURL: '/api/v3', + /** + * The endpoint, if it is a path under baseURL; throws INVALID_ENDPOINT + * otherwise. Every endpoint is one of this client's own API paths, so + * anything that could leave that path -- "//host", a backslash, a ".." + * segment, whitespace or control characters -- is a bug, not a request. + * + * @param {string} endpoint - API endpoint, starting with "/" + * @returns {string} The same endpoint + */ + checkEndpoint(endpoint) { + const path = typeof endpoint === 'string' ? endpoint.split(/[?#]/)[0] : ''; + if (!path.startsWith('/') || path.startsWith('//') || + /[\\\s\x00-\x1f\x7f]/.test(endpoint) || + path.split('/').some(segment => segment === '..' || segment === '.')) { + throw { + error_code: 'INVALID_ENDPOINT', + message: `Not an API endpoint: ${String(endpoint)}` + }; + } + return endpoint; + }, + /** * Make an API request with throttling and caching. * @@ -141,7 +163,7 @@ const PluginAPI = { const requestKey = `${method}:${endpoint}:${data ? JSON.stringify(data) : ''}`; const makeRequest = async () => { - const url = `${this.baseURL}${endpoint}`; + const url = `${this.baseURL}${this.checkEndpoint(endpoint)}`; const options = { method, headers: { @@ -161,7 +183,9 @@ const PluginAPI = { // and is not retried. let response; try { - response = await fetch(url, options); + // url is baseURL plus an endpoint checkEndpoint() accepted: a + // path on this origin's API, never a caller-chosen host. + response = await fetch(url, options); // nosemgrep } catch (error) { throw { error_code: 'NETWORK_ERROR', @@ -266,7 +290,7 @@ const PluginAPI = { * @returns {Promise} Plugin configuration */ async getPluginConfig(pluginId) { - const response = await this.request(`/plugins/config?plugin_id=${pluginId}`); + const response = await this.request(`/plugins/config?plugin_id=${encodeURIComponent(pluginId)}`); return response.data || {}; }, @@ -291,7 +315,7 @@ const PluginAPI = { * @returns {Promise} Response data */ async resetPluginConfig(pluginId) { - return await this.request(`/plugins/config/reset?plugin_id=${pluginId}`, 'POST'); + return await this.request(`/plugins/config/reset?plugin_id=${encodeURIComponent(pluginId)}`, 'POST'); }, /** @@ -301,7 +325,7 @@ const PluginAPI = { * @returns {Promise} Plugin schema */ async getPluginSchema(pluginId) { - const response = await this.request(`/plugins/schema?plugin_id=${pluginId}`); + const response = await this.request(`/plugins/schema?plugin_id=${encodeURIComponent(pluginId)}`); return response.data?.schema || null; }, @@ -364,7 +388,7 @@ const PluginAPI = { */ async getPluginHealth(pluginId = null) { const endpoint = pluginId - ? `/plugins/health/${pluginId}` + ? `/plugins/health/${encodeURIComponent(pluginId)}` : '/plugins/health'; const response = await this.request(endpoint); return response.data || {}; @@ -378,7 +402,7 @@ const PluginAPI = { */ async getPluginMetrics(pluginId = null) { const endpoint = pluginId - ? `/plugins/metrics/${pluginId}` + ? `/plugins/metrics/${encodeURIComponent(pluginId)}` : '/plugins/metrics'; const response = await this.request(endpoint); return response.data || {};