From 8334a9a27fffd026d020827321157564fecebffd Mon Sep 17 00:00:00 2001 From: ispyisail Date: Wed, 23 Sep 2026 04:06:33 +1200 Subject: [PATCH] Scripting is off until asked for, and says how to turn it on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A script reaches the whole project and, through the export calls, the filesystem. That is a capability most people installing an electrical CAD program never asked for, and leaving it on by default hands it to them anyway. So QET_HAS_SCRIPTING builds now ship with it switched off. QetSettings::scriptingEnabled() is the single answer, read by all three places that need it, with QET_ENABLE_SCRIPTING=1 overriding the stored value. The override is not decoration: a CI job or a batch run has no dialog to tick, and a machine whose HOME is created fresh for each run has nowhere to keep the setting either. It beats a stored "false" on purpose, so a box unticked once cannot lock a build server out of --run for good. Only the exact value "1" counts. --run refuses with exit 3 and a message naming both ways in. Projet > Exécuter un script... asks once, and turns the setting on if the answer is yes. Asking beats grey: a disabled menu entry says something exists and nothing about how to have it, and this is the pattern people already know from macro security in office software. Configurer QElectroTech > Général > Projets has the checkbox, for turning it back off. While the environment forces scripting on, the box is disabled and says why, and applyConf() then leaves the stored value alone rather than quietly overwriting it. runOnProject() checks as well, after both callers have. It is the one function that actually evaluates JavaScript, so it is the one place a future caller cannot forget to ask; the callers check first only to give a better answer than it can. Verified on the built binary, all four states, with an isolated HOME: stored env result absent - refused, exit 3 true - script runs, exit 0 false - refused, exit 3 false 1 script runs, exit 0 tst_scriptingsetting covers the same matrix hermetically, in its own QSettings scope, and was mutation-checked: flipping the default to true turns defaultsToOff() red. Co-Authored-By: Claude Opus 5 (1M context) --- sources/qetdiagrameditor.cpp | 24 +++ sources/scripting/qetscripting.cpp | 40 +++++ .../configpage/generalconfigurationpage.cpp | 27 ++++ .../ui/configpage/generalconfigurationpage.ui | 10 ++ sources/utils/qetsettings.cpp | 47 ++++++ sources/utils/qetsettings.h | 4 + tests/qttest/CMakeLists.txt | 11 ++ tests/qttest/tst_scriptingsetting.cpp | 146 ++++++++++++++++++ 8 files changed, 309 insertions(+) create mode 100644 tests/qttest/tst_scriptingsetting.cpp diff --git a/sources/qetdiagrameditor.cpp b/sources/qetdiagrameditor.cpp index cd29dd096..bbeb00527 100644 --- a/sources/qetdiagrameditor.cpp +++ b/sources/qetdiagrameditor.cpp @@ -57,6 +57,7 @@ #include "ui/backupdialog.h" #include "ui/dialogwaiting.h" #include "undocommand/addelementtextcommand.h" +#include "utils/qetsettings.h" #include "utils/qetutils.h" #include "undocommand/rotateselectioncommand.h" #include "undocommand/rotatetextscommand.h" @@ -3122,6 +3123,29 @@ void QETDiagramEditor::slot_runScript() { QETProject *project = currentProject(); if (!project) return; + // Scripting is off until somebody says otherwise, so the first use has + // to ask. Asking here rather than greying the action out keeps the + // feature discoverable: a disabled menu entry tells a user that + // something exists and nothing about how to have it. + if (!QetSettings::scriptingEnabled()) { + const QMessageBox::StandardButton answer = QET::QetMessageBox::question( + this, + tr("Exécuter un script"), + tr("Les scripts sont désactivés.\n\n" + "Un script s'exécute avec vos droits : il peut lire et " + "modifier le projet ouvert et écrire des fichiers. " + "N'exécutez que des scripts dont vous connaissez " + "l'origine.\n\n" + "Activer les scripts ? Ce réglage est modifiable dans " + "Configurer QElectroTech > Général > Projets."), + QMessageBox::Yes | QMessageBox::Cancel, + QMessageBox::Cancel); + if (answer != QMessageBox::Yes) { + return; + } + QetSettings::setScriptingEnabled(true); + } + const QString script_path = QFileDialog::getOpenFileName( this, tr("Exécuter un script"), diff --git a/sources/scripting/qetscripting.cpp b/sources/scripting/qetscripting.cpp index 9e013e5f2..789b2dee9 100644 --- a/sources/scripting/qetscripting.cpp +++ b/sources/scripting/qetscripting.cpp @@ -20,6 +20,7 @@ #include "qetscriptapi.h" #include "../qetmessagebox.h" #include "../qetproject.h" +#include "../utils/qetsettings.h" #include #include @@ -48,8 +49,33 @@ bool isRunRequest(const QStringList &args) #ifdef QET_HAS_SCRIPTING +namespace { + /** + @brief refusalMessage + What to tell somebody whose script was not run, and how to change + that. Written once because the command line and the graphical + editor both need to say it, and an explanation that names only one + of the two ways out sends half the people down the wrong path. + */ + QString refusalMessage() + { + return QObject::tr( + "Les scripts sont désactivés.\n\n" + "Un script a accès à l'ensemble du projet et peut écrire des " + "fichiers, aussi cette fonction est-elle désactivée par défaut.\n\n" + "Pour l'activer : Configurer QElectroTech > Général > Projets, " + "ou définir la variable d'environnement QET_ENABLE_SCRIPTING=1 " + "pour une exécution sans interface (CI, traitement par lot)."); + } +} + int run(const QStringList &args) { + if (!QetSettings::scriptingEnabled()) { + err << refusalMessage() << "\n"; + return 3; + } + const int idx = args.indexOf(QStringLiteral("--run")); const QString script_path = args.value(idx + 1); const QString project_path = args.value(idx + 2); @@ -88,6 +114,20 @@ namespace { bool runOnProject(const QString &scriptPath, QETProject *project, DiagramView *view) { + // Checked here as well as at each caller, deliberately: this is the + // one function that actually evaluates JavaScript, so it is the one + // place a future caller cannot forget to ask. The callers check first + // only to give a better answer than this one can -- a usable exit code + // on the command line, an offer to switch the setting on in the editor. + if (!QetSettings::scriptingEnabled()) { + err << refusalMessage() << "\n"; + if (view) { + QET::QetMessageBox::warning(nullptr, QObject::tr("Script"), + refusalMessage()); + } + return false; + } + QFile file(scriptPath); if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) { err << "Cannot open script: " << scriptPath << "\n"; diff --git a/sources/ui/configpage/generalconfigurationpage.cpp b/sources/ui/configpage/generalconfigurationpage.cpp index 685d91a3d..f5689046c 100644 --- a/sources/ui/configpage/generalconfigurationpage.cpp +++ b/sources/ui/configpage/generalconfigurationpage.cpp @@ -89,6 +89,25 @@ GeneralConfigurationPage::GeneralConfigurationPage(QWidget *parent) : ui->m_zoom_out_beyond_folio->setChecked(settings.value("diagrameditor/zoom-out-beyond-of-folio", false).toBool()); ui->m_use_gesture_trackpad->setChecked(settings.value("diagramview/gestures", false).toBool()); ui->m_save_label_paste->setChecked(settings.value("diagramcommands/erase-label-on-copy", true).toBool()); + ui->m_enable_scripting->setChecked(QetSettings::scriptingEnabled()); +#ifdef QET_HAS_SCRIPTING + if (QetSettings::scriptingForcedByEnvironment()) { + //QET_ENABLE_SCRIPTING wins over the stored value, so let the box + //say so rather than offer a tick that changes nothing. + ui->m_enable_scripting->setEnabled(false); + ui->m_enable_scripting->setToolTip( + tr("Activé par la variable d'environnement " + "QET_ENABLE_SCRIPTING ; ce réglage est sans effet " + "tant qu'elle est définie.")); + } +#else + //Built without Qt Qml: there is no scripting to allow. Disabled as + //well as hidden, so applyConf() leaves the stored value alone -- + //a hidden box still reports its state, and writing it here would + //quietly clear a preference set on a build that does have Qml. + ui->m_enable_scripting->setVisible(false); + ui->m_enable_scripting->setEnabled(false); +#endif ui->m_use_folio_label->setChecked(settings.value("genericpanel/folio", true).toBool()); ui->m_border_0->setChecked(settings.value("border-columns_0", false).toBool()); ui->m_autosave_sb->setValue(settings.value("diagrameditor/autosave-interval", 0).toInt()); @@ -244,6 +263,14 @@ void GeneralConfigurationPage::applyConf() //DIAGRAM COMMAND settings.setValue("diagramcommands/erase-label-on-copy", ui->m_save_label_paste->isChecked()); + //SCRIPTING + //Left alone while the environment forces it on: the box is disabled + //in that case and writing its state would silently clear the user's + //real preference the first time this dialog is accepted. + if (ui->m_enable_scripting->isEnabled()) { + QetSettings::setScriptingEnabled(ui->m_enable_scripting->isChecked()); + } + //GENERIC PANEL settings.setValue("genericpanel/folio",ui->m_use_folio_label->isChecked()); diff --git a/sources/ui/configpage/generalconfigurationpage.ui b/sources/ui/configpage/generalconfigurationpage.ui index 7813f8a0e..030b66877 100644 --- a/sources/ui/configpage/generalconfigurationpage.ui +++ b/sources/ui/configpage/generalconfigurationpage.ui @@ -218,6 +218,16 @@ + + + Autoriser l'exécution de scripts JavaScript (Projet > Exécuter un script, et --run) + + + Un script s'exécute avec vos droits : il peut lire et modifier le projet ouvert et écrire des fichiers. Désactivé par défaut ; n'exécutez que des scripts dont vous connaissez l'origine. + + + + Qt::Vertical diff --git a/sources/utils/qetsettings.cpp b/sources/utils/qetsettings.cpp index 3b2e7a5ac..0c97f9816 100644 --- a/sources/utils/qetsettings.cpp +++ b/sources/utils/qetsettings.cpp @@ -19,6 +19,7 @@ #include "qetsettings.h" #include #include +#include namespace QetSettings { @@ -105,4 +106,50 @@ namespace QetSettings return default_policy; } } + + /** + * @brief scriptingForcedByEnvironment + * @return true if QET_ENABLE_SCRIPTING is set to 1 in the environment. + * + * The way to turn scripting on where there is nobody to tick a box: + * a headless run, a CI job, a build server. Those have no settings + * file worth writing to -- and on a machine whose HOME is created + * fresh for the run, writing one would not survive anyway. + */ + bool scriptingForcedByEnvironment() + { + return qgetenv("QET_ENABLE_SCRIPTING") == QByteArray("1"); + } + + /** + * @brief scriptingEnabled + * @return whether QElectroTech may run a JavaScript script. + * + * Off unless the user turned it on. A script reaches the whole project + * and the filesystem through the export calls, so it is capability the + * great majority of users never asked for; leaving it on by default + * would hand it to them anyway. @sa setScriptingEnabled + * + * The environment override wins over the stored value, and is checked + * first so that a machine with no settings at all still answers. + */ + bool scriptingEnabled() + { + if (scriptingForcedByEnvironment()) { + return true; + } + QSettings settings; + return settings.value("scripting/enabled", false).toBool(); + } + + /** + * @brief setScriptingEnabled + * Store whether scripting is allowed. @sa scriptingEnabled + * @param enabled + */ + void setScriptingEnabled(bool enabled) + { + QSettings settings; + settings.setValue("scripting/enabled", enabled); + } } diff --git a/sources/utils/qetsettings.h b/sources/utils/qetsettings.h index 857c37796..0108b0474 100644 --- a/sources/utils/qetsettings.h +++ b/sources/utils/qetsettings.h @@ -32,6 +32,10 @@ namespace QetSettings void setHdpiScaleFactorRoundingPolicy(Qt::HighDpiScaleFactorRoundingPolicy policy); Qt::HighDpiScaleFactorRoundingPolicy hdpiScaleFactorRoundingPolicy( Qt::HighDpiScaleFactorRoundingPolicy default_policy = Qt::HighDpiScaleFactorRoundingPolicy::PassThrough); + + bool scriptingEnabled(); + void setScriptingEnabled(bool enabled); + bool scriptingForcedByEnvironment(); } #endif // QETSETTINGS_H diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 0b140cedf..c823b1827 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -143,6 +143,17 @@ add_test(NAME tst_qetstrings COMMAND tst_qetstrings) target_include_directories(tst_qetstrings PRIVATE ${QET_DIR}/sources) target_link_libraries(tst_qetstrings PRIVATE Qt::Test Qt::Widgets Qt::Xml pugixml::pugixml) +# QetSettings::scriptingEnabled() -- whether QElectroTech may run a script. +# Compiles qetsettings.cpp alone: the setting is deliberately a plain +# QSettings read, so the test needs nothing else of QElectroTech. +add_executable( + tst_scriptingsetting + tst_scriptingsetting.cpp + ${QET_DIR}/sources/utils/qetsettings.cpp) +add_test(NAME tst_scriptingsetting COMMAND tst_scriptingsetting) +target_include_directories(tst_scriptingsetting PRIVATE ${QET_DIR}/sources) +target_link_libraries(tst_scriptingsetting PRIVATE Qt::Test Qt::Gui) + # CrashHandler::formatInt() -- the async-signal-safe decimal formatter the # signal handler uses for the "Signal: N" line of a crash dump. Compiles # crashhandler.cpp and logring.cpp alongside; the handler deliberately diff --git a/tests/qttest/tst_scriptingsetting.cpp b/tests/qttest/tst_scriptingsetting.cpp new file mode 100644 index 000000000..09be495c4 --- /dev/null +++ b/tests/qttest/tst_scriptingsetting.cpp @@ -0,0 +1,146 @@ +/* + Copyright 2006-2026 The QElectroTech Team + This file is part of QElectroTech. + + QElectroTech is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 2 of the License, or + (at your option) any later version. + + QElectroTech is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with QElectroTech. If not, see . +*/ + +/* + QetSettings::scriptingEnabled() -- the switch that decides whether + QElectroTech will run JavaScript at all. + + Its default is the whole point: a setting nobody has touched must read + as off, because that is the state every existing installation is in + after an upgrade. The other case worth pinning is the environment + override, which exists so a headless run has a way in without a dialog + -- and which must beat a stored "false", or a CI machine that once had + the box unticked can never script again. + + This test owns its own QSettings scope (organization + application + name), so it cannot read or write the real configuration of whoever + runs it. +*/ + +#include "utils/qetsettings.h" + +#include +#include + +class TstScriptingSetting : public QObject +{ + Q_OBJECT + + private slots: + void initTestCase(); + void init(); + + void defaultsToOff(); + void storedValueIsHonoured(); + void environmentOverridesAStoredFalse(); + void environmentIsReportedSeparately(); + void writingDoesNotDependOnReading(); + + private: + void clearStoredValue(); +}; + +void TstScriptingSetting::initTestCase() +{ + // A scope of this test's own: whatever this writes must not land in + // the configuration of the account running the suite. + QCoreApplication::setOrganizationName( + QStringLiteral("QElectroTech-tst_scriptingsetting")); + QCoreApplication::setApplicationName( + QStringLiteral("tst_scriptingsetting")); + QSettings settings; + settings.clear(); +} + +void TstScriptingSetting::clearStoredValue() +{ + QSettings settings; + settings.remove(QStringLiteral("scripting/enabled")); + settings.sync(); +} + +void TstScriptingSetting::init() +{ + clearStoredValue(); + qunsetenv("QET_ENABLE_SCRIPTING"); +} + +void TstScriptingSetting::defaultsToOff() +{ + QVERIFY2(!QetSettings::scriptingEnabled(), + "an untouched installation must not run scripts"); +} + +void TstScriptingSetting::storedValueIsHonoured() +{ + QetSettings::setScriptingEnabled(true); + QVERIFY(QetSettings::scriptingEnabled()); + + QetSettings::setScriptingEnabled(false); + QVERIFY(!QetSettings::scriptingEnabled()); +} + +void TstScriptingSetting::environmentOverridesAStoredFalse() +{ + // The headless case: no dialog to tick, and a stored false that must + // not be able to lock a CI job out of --run forever. + QetSettings::setScriptingEnabled(false); + qputenv("QET_ENABLE_SCRIPTING", "1"); + QVERIFY(QetSettings::scriptingEnabled()); + + // Only "1" counts. An empty or accidental value is not consent. + for (const QByteArray &value : {QByteArray(""), QByteArray("0"), + QByteArray("true"), QByteArray("yes")}) { + qputenv("QET_ENABLE_SCRIPTING", value); + QVERIFY2(!QetSettings::scriptingEnabled(), + qPrintable(QStringLiteral("accepted QET_ENABLE_SCRIPTING=%1") + .arg(QString::fromUtf8(value)))); + } +} + +void TstScriptingSetting::environmentIsReportedSeparately() +{ + // The configuration dialog asks this to explain why its checkbox is + // disabled, so it must answer about the environment alone and not be + // confused by the stored value. + QetSettings::setScriptingEnabled(true); + QVERIFY(!QetSettings::scriptingForcedByEnvironment()); + + qputenv("QET_ENABLE_SCRIPTING", "1"); + QVERIFY(QetSettings::scriptingForcedByEnvironment()); +} + +void TstScriptingSetting::writingDoesNotDependOnReading() +{ + // While the environment forces scripting on, scriptingEnabled() says + // true whatever is stored -- so read the stored value directly to be + // sure a write still lands. The configuration dialog relies on this: + // it skips the write in that state on purpose, and would be silently + // wrong if setScriptingEnabled() were a no-op instead. + qputenv("QET_ENABLE_SCRIPTING", "1"); + QetSettings::setScriptingEnabled(false); + QSettings settings; + QCOMPARE(settings.value(QStringLiteral("scripting/enabled")).toBool(), false); + + QetSettings::setScriptingEnabled(true); + QSettings other; + QCOMPARE(other.value(QStringLiteral("scripting/enabled")).toBool(), true); +} + +QTEST_MAIN(TstScriptingSetting) +#include "tst_scriptingsetting.moc"