fix: address Codacy findings on the core audit PR

- plugin_manager.prepare_plugin_config: when the fallback legacy-boolean
  pass also fails, log a warning instead of a bare except/pass.
- api_client.js: request() refuses any endpoint that is not a plain path
  under /api/v3 ("//host", backslashes, ".." or "." segments, whitespace,
  control characters) with INVALID_ENDPOINT before calling fetch(), and
  plugin ids are URL-encoded wherever they are put into a URL (also in the
  app-shell batch load).
- test_update_all.js: pins both against the shipped client.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-09-17 12:54:16 -04:00
co-authored by Claude Opus 5
parent efc7856294
commit ff7ff17f46
4 changed files with 53 additions and 11 deletions
+4 -2
View File
@@ -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 "
+16
View File
@@ -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;
}
+2 -2
View File
@@ -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;
@@ -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<Object>} 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<Object>} 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<Object>} 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 || {};