From 1b0be9dbd8308b607c1a5d1d85e5fc4e1147193e Mon Sep 17 00:00:00 2001 From: David Wertheimer Date: Thu, 30 Jul 2026 09:53:17 -0700 Subject: [PATCH] Document eval usage in Dashboard test harness, add runtime dev-mode guard; fix rollup build-mode warning suppression clickHandlers.js: doEvaluateString backs the Dashboard's internal React test suite, which needs to call arbitrary NotePlan API methods from the webview side to set up test fixtures - hence the eval. The webview only ever loads this plugin's own bundled local JS (no remote content, no note content eval'd/rendered unescaped), so reaching this handler requires either being the trusted test code or an attacker who has already achieved script execution via some unrelated XSS bug - in which case this would let them escalate from webview JS to the privileged backend JSContext. The UI entry point (Dashboard.jsx's showDebugPanel) was already gated behind _logLevel === 'DEV' + FFlag_DebugPanel, but that only hid the button - the bridge handler itself had no runtime check. Added a matching guard directly in doEvaluateString so the protection isn't just a warning comment. scripts/rollup.js: the build-mode path (-b/-nc, used by `npc plugin:dev -nc` and CI) manually reconstructed `inputOptions` from getConfig() picking only external/input/plugins/context/cache - silently dropping the onwarn handler that suppresses EVAL and MODULE_LEVEL_DIRECTIVE warnings. Watch mode spreads the full config so it was never affected. This is what caused the "Use of eval... strongly discouraged" warning to appear during `-nc` builds even though onwarn was coded to suppress it years ago. Added the missing `onwarn: options.onwarn` to the build-mode inputOptions so both paths behave consistently. Verified: full test suite passes (198 suites, 4553 tests); flow error count on clickHandlers.js unchanged (16, confirmed via stash comparison); jgclark.Dashboard builds clean with no eval warning. Co-Authored-By: Claude Sonnet 5 --- jgclark.Dashboard/src/clickHandlers.js | 33 ++++++++++++++++++++++++-- scripts/rollup.js | 1 + 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/jgclark.Dashboard/src/clickHandlers.js b/jgclark.Dashboard/src/clickHandlers.js index a5f6b1bfd..3c0276fa1 100644 --- a/jgclark.Dashboard/src/clickHandlers.js +++ b/jgclark.Dashboard/src/clickHandlers.js @@ -19,6 +19,7 @@ import { cloneDashboardSettingsBeforeSave, getDashboardSettings, getDashboardSettingsDefaults, + getLogSettings, handlerResult, makeDashboardParas, setPluginData, @@ -63,8 +64,30 @@ const windowCustomId = `${pluginID}.main` ****************************************************************************************************************************/ /** - * Evaluate JS string and return result + * Evaluate an arbitrary JS string in the plugin backend and return the result. * WARNING: DO NOT USE THIS FOR ANYTHING OTHER THAN TESTING. + * + * Why eval is used here at all: this is the backing handler for the Dashboard's + * internal React test suite (src/react/components/testing/*.tests.js), which needs + * to call arbitrary NotePlan API methods (e.g. Editor.openNoteByFilename(...)) from + * the webview side to set up test fixtures. Writing a bespoke bridge handler for every + * possible test setup operation isn't practical, so the test harness sends the JS as a + * string and this function evals it in the backend JSContext, which has full NotePlan + * API access (filesystem, DataStore, Editor, etc.). + * + * Reachability / risk: the Dashboard webview only ever loads this plugin's own bundled + * local JS - no remote content, and no note content is eval'd or rendered unescaped + * into the webview - so this can only be reached today by the plugin's own trusted test + * code, or by an attacker who has *already* achieved script execution inside the webview + * via some unrelated XSS bug. In that (currently hypothetical) scenario, this handler + * would let them escalate from webview-only JS execution to the privileged backend + * JSContext. The UI entry point for the test suite (Dashboard.jsx's `showDebugPanel`) is + * already gated behind `_logLevel === 'DEV'` and the `FFlag_DebugPanel` feature flag, but + * that only hides the *button* - the bridge handler itself had no runtime check, so + * anything able to construct a bridge message could reach eval() regardless of dev mode. + * The check below closes that gap by enforcing the same DEV + FFlag_DebugPanel gate here, + * so it can't be bypassed by anything other than the debug test harness itself. + * * @param {MessageDataObject} data * @returns */ @@ -74,8 +97,14 @@ export async function doEvaluateString(data: MessageDataObject): Promise { plugins: [...options.plugins, ...defaultPlugins], context: options.context, cache: cachedBundle, + onwarn: options.onwarn, } const outputOptions = options.output