Training credits custom field
customfield_mutrain
A custom field type plugin for Moodle that provides a "Training credits" field, storing decimal values representing training credits. Part of the MuTMS plugin suite. The plugin extends Moodle's core custom field framework (`core_customfield`) with a `data_controller` and `field_controller` that handle decimal input with localized number formatting (comma/period decimal separator support), server-side validation, and companion plugin cleanup on deletion.
The plugin is small, focused, and well-written. It extends Moodle's core custom field framework correctly, uses $DB API for all database operations, properly validates form input on both the validation and save paths, and includes PHPUnit tests with good coverage.
The only findings are low-severity code quality issues: using PARAM_RAW instead of PARAM_LOCALISEDFLOAT for numeric input (mitigated by manual validation), SQL string concatenation with database-sourced integer IDs in the upgrade script (no injection risk), and direct deletion of records in a companion plugin's tables. An unused variable in the upgrade script rounds out the findings at info level.
There are no security vulnerabilities, no missing access checks (the plugin has no web-accessible endpoints), and the Privacy API is correctly implemented as null_provider — consistent with all core custom field types.
Plugin Overview
customfield_mutrain is a lightweight custom field type plugin that adds a "Training credits" field type to Moodle's custom fields framework. It is part of the MuTMS (MuTrain Management Suite) plugin ecosystem and works in conjunction with tool_mutrain.
Architecture
The plugin consists of:
data_controller— handles form rendering, validation, saving, and export of decimal training credit valuesfield_controller— manages field configuration and deletion (including cleanup of companion plugin data)privacy/provider— correctly implementsnull_providersince data storage is handled by core'scustomfield_datatabledb/upgrade.php— migrates data fromintvaluetodecvaluecolumn in core's custom field data table
Key Observations
- The plugin has no web-accessible pages, no AJAX endpoints, and no web service definitions — all interaction goes through the core custom field framework
- Input validation is thorough: values are checked to be numeric and non-negative in both
instance_form_validation()andinstance_form_save() - The plugin correctly supports localized decimal separators (comma vs. period) through manual
str_replace(), thoughPARAM_LOCALISEDFLOATwould be the standard approach - PHPUnit tests cover initialization, configuration form, instance form (including required fields and negative numbers), value get/export, and deletion
- The tight coupling with
tool_mutrain(direct table operations) is mitigated byclass_exists()guards
Findings
The form field for training credits uses PARAM_RAW as its parameter type. While the value is manually validated as numeric in both instance_form_validation() and instance_form_save(), Moodle provides PARAM_LOCALISEDFLOAT specifically for this use case — user-entered numbers that may use locale-specific decimal separators.
The core customfield_number plugin uses the dedicated float form element type instead of a text field with PARAM_RAW. The manual str_replace(',', '.', ...) pattern reimplements what PARAM_LOCALISEDFLOAT and unformat_float() handle automatically.
Not a security issue — the value is validated server-side before storage and the saved value always passes through format_float(). This is a code quality concern about using the appropriate Moodle API.
Low risk. No security impact. The value is properly validated before storage and always stored as a formatted decimal number. The PARAM_RAW type means the raw form value passes through without Moodle's automatic cleaning, but the plugin's own validation is equivalent. This is strictly a code quality / best practice concern.
The instance_form_definition() method creates a text input for training credits. The value flows through instance_form_validation() where it is checked with is_numeric() and $value < 0, and through instance_form_save() where the same checks are repeated before the value is formatted with format_float() and saved. The manual comma-to-period replacement on lines 85 and 150 handles locale differences.
$mform->addElement('text', $elementname, $this->get_field()->get_formatted_name(), 'size=5');
$mform->setType($elementname, PARAM_RAW);
Use PARAM_LOCALISEDFLOAT instead of PARAM_RAW, or use the float form element type as core's customfield_number does:
$mform->addElement('text', $elementname, $this->get_field()->get_formatted_name(), 'size=5');
$mform->setType($elementname, PARAM_LOCALISEDFLOAT);
Alternatively, use the float element:
$mform->addElement('float', $elementname, $this->get_field()->get_formatted_name());
The upgrade script concatenates database-sourced field IDs directly into SQL queries using implode(',', $fieldids) instead of using Moodle's $DB->get_in_or_equal() helper for parameterized IN clauses.
While the values originate from $DB->get_fieldset() and are guaranteed to be integer IDs from the database (not user input), the pattern of string concatenation into SQL does not follow the parameterized query best practice.
Low risk. No SQL injection vulnerability exists because the concatenated values are integer IDs retrieved from the database via $DB->get_fieldset(). There is no path for user input to influence these values. The risk is purely about code hygiene and following Moodle's parameterized query conventions.
This code runs during plugin upgrade (one-time migration) to move training credit values from the intvalue column to the decvalue column in core's customfield_data table. The $fieldids array contains integer IDs returned by $DB->get_fieldset() — these are database-sourced values, not user input.
$fieldids = $DB->get_fieldset('customfield_field', 'id', ['type' => 'mutrain']);
if ($fieldids) {
$fieldids = implode(',', $fieldids);
$sql = "UPDATE {customfield_data}
SET decvalue = intvalue
WHERE fieldid IN ($fieldids)
AND intvalue IS NOT NULL AND decvalue IS NULL";
$DB->execute($sql);
$sql = "UPDATE {customfield_data}
SET intvalue = NULL
WHERE fieldid IN ($fieldids)";
$DB->execute($sql);
}
Use $DB->get_in_or_equal() for parameterized queries:
$fieldids = $DB->get_fieldset('customfield_field', 'id', ['type' => 'mutrain']);
if ($fieldids) {
[$insql, $params] = $DB->get_in_or_equal($fieldids);
$sql = "UPDATE {customfield_data}
SET decvalue = intvalue
WHERE fieldid $insql
AND intvalue IS NOT NULL AND decvalue IS NULL";
$DB->execute($sql, $params);
$sql = "UPDATE {customfield_data}
SET intvalue = NULL
WHERE fieldid $insql";
$DB->execute($sql, $params);
}
The field_controller::delete() method directly deletes records from tool_mutrain_completion and tool_mutrain_field tables, which belong to the tool_mutrain companion plugin. This creates a tight coupling between the two plugins.
Ideally, tool_mutrain would provide an API method (or observe a deletion event) for cleaning up its own data when a custom field is deleted. The class_exists() guard ensures this code only runs when the companion plugin is installed, which partially mitigates the coupling concern.
Low risk. The $DB->delete_records() calls use parameterized queries and are safe. The concern is architectural: direct cross-plugin table manipulation can break if tool_mutrain changes its schema. However, since both plugins are maintained together as part of the MuTMS suite, this is a pragmatic pattern with acceptable risk.
The delete() method overrides the parent field_controller::delete() to clean up companion plugin data before calling parent::delete(). Both plugins are part of the MuTMS suite and are designed to work together. The class_exists() check makes this gracefully degrade when tool_mutrain is not installed.
if (class_exists(framework::class)) {
$DB->delete_records('tool_mutrain_completion', ['fieldid' => $fieldid]);
$DB->delete_records('tool_mutrain_field', ['fieldid' => $fieldid]);
}
Consider having tool_mutrain observe the custom field deletion event and clean up its own tables, or expose a cleanup API method:
if (class_exists(framework::class)) {
framework::on_field_deleted($fieldid);
}
The variable $dbman is assigned via $DB->get_manager() but is never used in the upgrade function. This is dead code that should be removed.
No risk. This is dead code with no functional or security impact. It slightly increases memory usage during upgrade but is otherwise harmless.
The xmldb_customfield_mutrain_upgrade() function assigns $dbman = $DB->get_manager() at the top of the function, likely copied from a template. The function only performs DML operations ($DB->execute()), not DDL operations, so the database manager is unnecessary.
$dbman = $DB->get_manager();
Remove the unused variable assignment:
function xmldb_customfield_mutrain_upgrade($oldversion) {
global $DB;
if ($oldversion < 2025120945) {
The plugin's Privacy API implementation is correct. It uses null_provider because the custom field data is stored in core's customfield_data table, which is handled by core_customfield's own privacy provider. This matches the pattern used by all six core custom field type plugins (checkbox, date, number, select, text, textarea).
The plugin includes a comprehensive PHPUnit test suite in tests/phpunit/plugin_test.php covering field/data controller initialization, configuration form submission, instance form submission (including required field validation, valid values, and negative number rejection), value get/export behavior, and field deletion. This is good test coverage for a plugin of this size.
The plugin has no web-accessible endpoints — no PHP pages, no AJAX handlers, no web service definitions. All user interaction occurs through the core custom field framework, which handles access control, sesskey validation, and capability checks. This significantly reduces the plugin's attack surface.
The version.php declares support for Moodle versions [500, 502] (Moodle 5.0 through 5.2), and composer.json specifies "moodle/moodle": "5.1.*||5.2.*". There is a minor discrepancy — version.php includes 5.0 support while composer.json does not — but this has no functional impact since version.php is the authoritative source for Moodle's plugin compatibility checks.