Cloze question editor
tiny_cloze
- #1Behat test framework loaded on production requests to detect a test site
- #2Capability upgrade writes directly to core role_capabilities instead of assign_capability()
- #3CLI tool's config.php path assumes the Moodle 5.1+ public/ layout
- #4JavaScript unit tests use `if` instead of `it`, silently skipping assertions
A TinyMCE editor plugin that adds a toolbar button and "Insert" menu item for authoring Moodle embedded-answer (Cloze) questions. Almost all logic lives in the AMD JavaScript modules under amd/src/; the PHP layer is small and consists of the Tiny plugininfo, a capability-management helper used by an upgrade step and CLI tool, a null privacy provider, and language strings.
The plugin is a client-side authoring aid with a small, generally clean PHP surface. Database access is fully parameterised (the $DB API with placeholder/array parameters — no SQL string building), and there is no filesystem, cURL/HTTP, shell/eval, or PHP superglobal misuse. The tiny/cloze:use capability is correctly enforced through the inherited editor_tiny\plugin::is_enabled() base method, and the null_provider privacy implementation is appropriate because the plugin stores no personal data. The JavaScript manipulates only the question author's own editor content, which later flows through Moodle's question engine, so it presents no cross-user XSS path.
All identified issues are low-severity code-quality items: loading the Behat test-framework chain into production requests instead of using the BEHAT_SITE_RUNNING constant; modifying the core role_capabilities table directly instead of via assign_capability() (skipping access-cache invalidation, most visibly in the manually-run CLI tool); a CLI config.php path that assumes the Moodle 5.1+ public/ layout; and three unit-test blocks that silently never run due to an if/it typo. None are exploitable, and the privileged operations are confined to admin-initiated upgrade or CLI execution. The plugin ships Behat and JavaScript tests and follows Moodle conventions throughout, so these represent minor technical debt rather than risk.
Overview
tiny_cloze is a TinyMCE editor plugin that adds a button and menu item for authoring Moodle embedded-answer (Cloze) questions. Almost all of the logic lives in the AMD JavaScript modules (amd/src/); the PHP layer is small and comprises the editor plugininfo, a capability-management helper used by an upgrade step, a CLI tool, a null privacy provider, and language strings.
Security posture
The plugin is well-behaved from a security standpoint:
- All database access uses the
$DBAPI with placeholder/array parameters — no SQL string concatenation. - No raw filesystem, cURL/HTTP,
eval/exec, or PHP superglobal usage. - The
tiny/cloze:usecapability is correctly enforced by the inheritededitor_tiny\plugin::is_enabled()base method, which callshas_capability('tiny/cloze:use', $context)automatically. - The privacy API is implemented as a
null_provider, which is correct — the plugin stores no personal data. - The JavaScript operates entirely inside the question author's own editor session. The cloze syntax it produces becomes the author's own question text and is later rendered through Moodle's question engine, so there is no XSS vector against other users.
Findings
All findings are low severity and concern code quality / maintainability rather than security:
- Production pages load the Behat test framework to detect a test site, instead of using the
BEHAT_SITE_RUNNINGconstant. - The capability upgrade writes directly to the core
role_capabilitiestable instead of usingassign_capability(), skipping access-cache invalidation (notably in the standalone CLI tool) and the standard event. - The CLI permission tool's
config.phppath assumes the Moodle 5.1+public/layout and would break on the 4.x layout the plugin still advertises. - Three JavaScript unit-test blocks use
ifwhereitwas intended and never execute.
No security vulnerabilities were identified.
Findings
plugininfo.php performs an unconditional require_once of the core Behat utility file at file scope, and get_plugin_configuration_for_context() calls \behat_util::is_test_site() to decide whether the editor is running on a test site.
Because the require_once is at file scope, it executes every time the plugininfo class is autoloaded — which the Tiny editor manager does for every editor instance on every page, since it calls is_enabled() on each registered plugin (see lib/editor/tiny/classes/manager.php). As a result, every production page that renders a TinyMCE editor pulls in the Behat/testing utility chain: behat_util extends \core\test\testing_util, and util.php itself requires behat_command.php, behat_config_manager.php, filelib.php, clilib.php and csslib.php.
The idiomatic way to detect a Behat run — used by core's own tiny_autosave plugin — is the BEHAT_SITE_RUNNING constant, which requires no test-framework includes.
This is not a correctness or security problem: on a production site is_test_site() returns early (the behattestdir.txt marker file is absent) before doing any database work. It is unnecessary overhead and an architectural smell — production behaviour should not depend on loading test-only code.
Low risk. There is no security impact and no functional breakage: on a production (non-Behat) site is_test_site() short-circuits at the missing behattestdir.txt marker before running any database query. The cost is the avoidable loading of the Behat/testing class chain (and its require dependencies) on every page that shows a Tiny editor, plus coupling production code paths to test-only classes. The blast radius is performance/maintainability, not data or access.
get_plugin_configuration_for_context() returns a small set of configuration booleans (testsite, multianswerrgx) that the AMD code reads to decide which question types to offer. The Tiny manager (manager.php::get_plugin_configuration()) iterates every registered plugin, calls is_enabled() (which autoloads each plugin class, triggering this file-scope require_once), and only then calls get_plugin_info() → get_plugin_configuration_for_context(). So both the include and the is_test_site() call are reached for ordinary users on ordinary editor pages.
require_once(__DIR__ . '/../../../../../behat/classes/util.php');
Remove this top-level require_once entirely and detect the Behat run with the standard constant instead — no test-framework file needs to be loaded.
// When on the test site, check that the simulation config for an existing regex question type is set.
if (\behat_util::is_test_site()) {
return [
'testsite' => true,
'multianswerrgx' => (bool)get_config('tiny_cloze', 'simulate_multianswerrgx'),
];
}
// When on the test site, check that the simulation config for an existing regex question type is set.
if (defined('BEHAT_SITE_RUNNING') && BEHAT_SITE_RUNNING) {
return [
'testsite' => true,
'multianswerrgx' => (bool)get_config('tiny_cloze', 'simulate_multianswerrgx'),
];
}
The capability::update_role_permissions() method (invoked from db/upgrade.php during the plugin upgrade and from the cli/upgrade_permissions.php tool) grants tiny/cloze:use to the student, teacher and editingteacher roles by calling $DB->update_record('role_capabilities', ...) and $DB->insert_record('role_capabilities', ...) directly, rather than using the core assign_capability() API.
This bypasses several things that assign_capability() does (verified in lib/accesslib.php):
- Access-cache invalidation —
assign_capability()ends withaccesslib_clear_role_cache($roleid)to flush the MUC role caches. The plugin performs none. The web upgrade path is saved by the fact that Moodle callspurge_all_caches()at the end of an upgrade, but the standalone CLI tool (which the README explicitly tells admins to run) performs no purge, so the granted permissions may not take effect until caches are manually purged. - The
capability_assignedevent is never triggered, so the change is not auditable through the standard event stream.
Two further code-quality problems are present in the same method:
- The system context id is hardcoded as the literal
1(in bothget_capabilities()and the insert). While the system context is conventionally id 1, the intent is clearer and more robust using\context_system::instance()->id. - A bare
echois performed from inside the class method for the insert case (while the update case uses the$result->logarray). During a web upgrade thisechoemits raw, unstyled text (a literal\nrather than the<br/>the wrapper produces), and mixing direct output into a data-layer method is poor separation of concerns.
Low risk. This is not exploitable: the code path runs only during an admin-initiated plugin upgrade or when an admin runs the CLI tool from the shell, and it grants exactly the capability/roles already declared in db/access.php. The practical impact is a correctness/operational one — the standalone CLI tool can leave the access caches stale so the change appears to succeed but does not take effect until a cache purge — plus minor maintainability issues (hardcoded context id, output emitted from a data-layer method, no audit event). On uninstall, core's capabilities_cleanup() removes the plugin's capability rows, so the direct writes do not leak after removal.
The capability tiny/cloze:use is declared in db/access.php with student/teacher/editingteacher archetypes. Because changing archetypes does not retroactively update existing roles, the plugin added an upgrade step (and a CLI fallback) to push the new defaults to existing installs, guarded by is_modified_since_install() so admin customisations are preserved. The grants are confined to the three intended roles and to the system context, so there is no privilege escalation beyond what db/access.php already declares.
$this->capabilities = $DB->get_records(
'role_capabilities',
['capability' => self::CAPABILITY_USE, 'contextid' => 1]
);
Replace the hardcoded 1 with \context_system::instance()->id (or the SYSCONTEXTID constant) so the system-context id is not a magic number.
$rec->permission = CAP_ALLOW;
$rec->timemodified = time();
$rec->modifierid = $USER->id;
$DB->update_record('role_capabilities', $rec);
Use the core API, which updates the row, fires the event and clears the role cache:
assign_capability(self::CAPABILITY_USE, CAP_ALLOW, $rec->roleid, \context_system::instance()->id, true);
foreach ($roles as $role) {
$new = new \stdClass();
$new->contextid = 1;
$new->roleid = $role->id;
$new->capability = self::CAPABILITY_USE;
$new->permission = CAP_ALLOW;
$new->timemodified = time();
$new->modifierid = $USER->id;
$DB->insert_record('role_capabilities', $new);
echo "Inserted CAP_ALLOW for role '{$role->shortname}'.\n";
$result->changed++;
}
Grant via the core API and collect the message into $result->log (let the caller decide how to output it) rather than echo-ing from inside the class:
foreach ($roles as $role) {
assign_capability(self::CAPABILITY_USE, CAP_ALLOW, $role->id, \context_system::instance()->id, true);
$result->log[] = "Inserted CAP_ALLOW for role '{$role->shortname}'.";
$result->changed++;
}
If direct DB writes are kept for any reason, call accesslib_clear_all_caches(false) (or accesslib_clear_role_cache($roleid) per role) after the loop so the CLI path takes effect immediately.
cli/upgrade_permissions.php bootstraps Moodle with require_once(dirname(__DIR__, 7) . '/config.php'). The fixed depth of 7 only resolves correctly on the Moodle 5.1+ public/ layout, where the plugin lives at public/lib/editor/tiny/plugins/cloze/cli/ and seven dirname steps land on the project root that holds config.php.
On the pre-5.1 layout the plugin lives at lib/editor/tiny/plugins/cloze/cli/, so dirname(__DIR__, 7) overshoots the Moodle root by one directory and config.php is not found, aborting the script with a fatal require_once error.
The plugin's version.php advertises support back to Moodle 4.3 ($plugin->supported = [403, 502], $plugin->requires = 2023100900), so on the lower half of that range this CLI fallback tool is broken. For the 5.2 target under review the path is correct.
Low risk. No security impact — the script needs filesystem/shell access, which is already privileged. The issue is portability: on the 4.x layout that the plugin still claims to support, the hardcoded dirname(__DIR__, 7) points above the Moodle root and the tool fails to start. Impact is limited to an admin-run convenience tool, and the primary (automated) upgrade path is unaffected.
This CLI script is a manual fallback for admins whose automated permission upgrade did not run (e.g. because defaults had been modified). It defines CLI_SCRIPT, loads config.php, adminlib.php and clilib.php, then calls capability::update_role_permissions(). It requires shell access and is therefore an admin-only operation.
require_once(dirname(__DIR__, 7) . '/config.php');
Either document that this tool is for the 5.1+ layout only, or resolve config.php in a layout-independent way — for example walk up from __DIR__ until a config.php is found, or compute the root from the known plugin sub-path. The automated upgrade in db/upgrade.php is unaffected because it runs inside the already-bootstrapped upgrade process and does not depend on this path math.
Three blocks in the Mocha test suite begin with if (...) where it(...) was clearly intended. Written as if ('description', function () { ... }), the comma operator discards the description string and evaluates to the (never-invoked) function expression, which is truthy; the if body is empty, so the assertions inside the function never run.
The affected blocks are:
'Check that sequence is correct'forgetQuestionTypes()without regex (line 74)'Check that sequence is correct'forgetQuestionTypes()with regex (line 87)'Test custom grades 80, 60, 40, 33'forisCustomGrade()(line 104)
The practical effect is a silent loss of test coverage: the suite reports these cases as passing context when in fact none of their assertions execute. This includes the checks that would catch a regression in question-type ordering and in custom-grade detection.
Low risk. Test-only code with no runtime or security impact. The concern is reduced confidence in the JavaScript test suite: regressions in question-type ordering or custom-grade handling would not be caught by the affected cases because their assertions never execute.
tests/js/cloze.test.js is the Mocha/jsdom unit-test suite for the pure helper functions in amd/src/cloze.js (run via npm test, which copies the sources to .mjs files). Most describe/it blocks are correct; only these three were mistyped.
if ('Check that sequence is correct', function () {
Change if to it so the block is registered and executed as a test case (apply the same fix at lines 74, 87 and 104):
it('Check that sequence is correct', function () {
if ('Check that sequence is correct', function () {
it('Check that sequence is correct', function () {
if ('Test custom grades 80, 60, 40, 33', function () {
it('Test custom grades 80, 60, 40, 33', function () {
No bundled third-party runtime libraries. The amd/build/*.min.js files are the plugin's own compiled AMD modules, and tests/js/package.json declares only mocha and jsdom as dev dependencies for the local unit-test runner (not shipped or loaded by Moodle). No thirdpartylibs.xml is required, and nothing duplicates a library shipped by core.
Privacy API is correct. classes/privacy/provider.php implements \core_privacy\local\metadata\null_provider and returns the privacy:metadata string, which is the appropriate declaration for a plugin that stores no personal data.
Capability enforcement is correct. The plugin does not override is_enabled(), so it inherits editor_tiny\plugin::is_enabled(), which automatically gates the plugin on has_capability('tiny/cloze:use', $context). The default grant of tiny/cloze:use to students is not a privilege escalation: the cloze button only appears on multianswer question-editing pages, which already require question-authoring capabilities to reach, and any author can type cloze syntax manually regardless.
Minor: the upgrade and CLI diagnostic messages (e.g. in db/upgrade.php and classes/capability.php) are hardcoded English rather than get_string() calls. This is conventional and tolerated for admin-facing upgrade/CLI output, so it is noted only for completeness rather than flagged as a finding.
Good test footprint overall. The plugin ships a broad Behat suite (tests/behat/) covering dialogue UI, question creation, numeric/regex answers and error handling, alongside the JavaScript unit tests — a stronger automated-test posture than many editor plugins, the if/it typos in finding 4 notwithstanding.