Teacher Checklist
block_teacher_checklist
A Moodle course-level block that helps editing teachers track the quality of their course setup. It combines an automatic scanner — detecting hidden courses, missing summaries/end dates, empty gradebooks, ungraded assignment submissions and quiz attempts, quizzes/forums with structural problems, disabled completion tracking, and empty visible sections — with a manual to-do list. Items can be marked Done, Ignored, or restored to Pending individually or in bulk through an AJAX web service. A public helper API (external_tasks::replace()) lets companion plugins seed manual items into a course, and the plugin ships with a full Privacy API implementation, backup/restore support, and a PHPUnit suite.
This plugin demonstrates consistently strong security engineering with no exploitable vulnerabilities.
Access control is correct throughout. The dashboard (view.php) calls require_login() plus require_capability('moodle/course:update'); the block's get_content() returns empty content unless the same capability is held; and the AJAX web service (external::toggle_item_status) calls validate_context() and require_capability() before any write. Every privileged operation is scoped to a specific courseid, and record lookups are constrained by both id and courseid, so there is no cross-course IDOR.
Injection and output handling are sound. All database access uses parameterized $DB APIs with placeholders — no string concatenation of user input into SQL. All user-controllable text (manual item titles) is rendered through format_string(), whose default cleaning path runs clean_text() / HTMLPurifier, making the {{{title}}} template usage the correct, XSS-safe Moodle idiom. The one custom form validates the session key with confirm_sesskey(), and the web service relies on Moodle's built-in token/sesskey handling.
Data lifecycle is well covered: a complete Privacy API (metadata, context/user discovery, export, and all deletion entry points), backup limited to manual items with duplicate-safe restore, and portable cross-engine SQL.
The only issues identified are a minor internationalisation gap — hardcoded English status labels (Done/Ignored/Pending) in the GDPR export — and an architectural observation about a public seeding API that intentionally delegates authorization to its callers and is not wired to any web entry point. Neither has security impact.
Overview
block_teacher_checklist (v1.3.0) is a course-setup quality block for editing teachers. It is small, focused, and notably well-engineered: clean separation between the scanner, renderer, and web-service layers; thorough PHPUnit coverage; and bilingual language packs (English and Brazilian Portuguese).
Security posture
The plugin gets the security fundamentals right:
- Authorization —
view.phpenforcesrequire_login()+require_capability('moodle/course:update');external::toggle_item_status()enforcesvalidate_context()+require_capability(); andblock_teacher_checklist::get_content()checks the same capability before rendering anything. All operations are course-scoped, with record lookups bound tocourseid, eliminating cross-course manipulation. - SQL — every query uses
$DBplaceholders; no concatenation of variables into SQL. The subquery-based scanner queries are standard, portable SQL. - XSS — manual titles are passed through
format_string()before output. Its default path runsclean_text()/HTMLPurifier, so the{{{title}}}(unescaped) usage in the Mustache partial is the correct idiom rather than a vulnerability. URLs and icons aremoodle_urlobjects rendered through escaped placeholders. - CSRF — the manual-item form carries a
sesskeyand is gated bydata_submitted() && confirm_sesskey(); the state-changing AJAX path goes through the External API.
Compliance & quality
- Full Privacy (GDPR) provider, including the user-list deletion entry points.
- Backup/restore correctly limits to manual items and avoids duplicates on restore using
sql_compare_text(). - Tests cover the scanner heuristics, the capability gate, invalid-status rejection, and insert-vs-update behaviour.
- No bundled third-party code; no direct DB/filesystem/HTTP access; no DDL outside
db/upgrade.php.
Findings
Only two low-impact items were found:
- Low — hardcoded English status names (
Done/Ignored/Pending) in the privacy export instead ofget_string(). - Info — the public
external_tasks::replace()API performs destructive writes (delete + insert) with no internal capability check, delegating authorization to callers. It is not exposed through any web entry point.
No critical, high, or medium issues were identified.
Findings
The privacy provider translates the numeric status column into human-readable labels using a hardcoded English switch statement rather than the language API.
When a user requests a GDPR data export, the exported status field is always one of the literal English strings Done, Ignored, or Pending, regardless of the user's or site's language. The plugin otherwise ships a complete Brazilian Portuguese language pack, so this is an inconsistency: every other user-facing label is localised, but the exported status is not.
This is a code-quality / internationalisation issue, not a security problem. It does not affect the correctness of the export, only its readability for non-English locales.
Low risk. This is purely an internationalisation/code-quality gap with no security dimension. The blast radius is limited to the wording of the status field inside a subject-access (GDPR) export — non-English users see untranslated status names. There is no data exposure, no incorrect data, and no exploit path.
get_status_name() is called from export_user_data() (line 137) when building the exported record set for a course context. The surrounding export logic is otherwise correct: it filters by courseid and userid, applies format_string() to the title, and uses transform::datetime() for timestamps. Only the status label bypasses the language layer.
private static function get_status_name($status) {
switch ($status) {
case 1:
return 'Done';
case 2:
return 'Ignored';
default:
return 'Pending';
}
}
Add dedicated language strings and resolve them with get_string() so the export respects the active language:
private static function get_status_name($status) {
switch ((int) $status) {
case 1:
return get_string('status_done', 'block_teacher_checklist');
case 2:
return get_string('status_ignored', 'block_teacher_checklist');
default:
return get_string('status_pending', 'block_teacher_checklist');
}
}
Then declare status_done, status_ignored, and status_pending in lang/en/block_teacher_checklist.php (and the pt_br pack).
external_tasks::replace() is a public static helper intended to be called by other plugins to provision manual checklist items into a course. It first deletes all manual items previously provisioned under the given subtype (source) and then inserts the supplied titles.
The method performs no capability check, no require_login(), no context validation, and no session-key check — it trusts whatever code calls it. This is a deliberate and documented design (the calling component is expected to enforce access control), and importantly the method is not wired to any web-service definition (db/services.php) or page entry point, so it cannot be reached directly by an end-user request.
This is recorded as an architectural observation rather than a vulnerability: a public API that issues destructive writes (delete_records + insert_records) against course data is worth highlighting so that any integrating plugin enforces authorization at its own call site.
Informational. Not exploitable as shipped: a grep of the plugin confirms no in-tree caller and no web-service/entry-point exposure, so an end user cannot reach this code path. The note exists because the method is public and destructive — if a future integrator wires it to a request handler without its own capability check, it would become a privilege/data-integrity concern. As delivered, the responsibility-delegation pattern is standard for classes/local/ helpers.
The method lives under classes/local/, the conventional namespace for plugin-internal helper APIs. Per the changelog, it is consumed by a companion plugin (local_virtuallab) to push a shared task list into lab courses. All writes are parameterized and scoped by courseid/type/subtype, and the stored titles are later rendered through format_string(), so there is no injection or stored-XSS concern in the data it writes.
public static function replace(int $courseid, string $source, array $titles): void {
global $DB, $USER;
$conditions = [
'courseid' => $courseid,
'type' => 'manual',
'subtype' => $source,
];
$DB->delete_records('block_teacher_checklist', $conditions);
No change is required for correctness, since the API is not exposed to untrusted input. To harden it defensively for integrators, consider either:
- Documenting the authorization contract explicitly in the method's PHPDoc ("callers MUST verify
moodle/course:updateon the course context before calling"), or - Adding a guard inside the method when callers cannot be trusted:
require_capability('moodle/course:update', \context_course::instance($courseid));
Note that adding the guard would couple the API to a capability model, which may not suit all server-to-server callers — hence documenting the contract is often the better fit for a classes/local/ helper.
Manual titles are double-formatted, which is harmless but redundant. In view.php (line 85) each manual record's title is wrapped in format_string() before being placed into the issue array, and renderer::prepare_issue_for_template() (line 102) then applies format_string() again to the same value. Because format_string() does not re-encode already-encoded entities, the result is correct, but the second call is unnecessary work. Passing the raw title from view.php and letting the renderer be the single sanitisation point (as the block's get_content() already does) would make the data flow clearer.
The scanner runs on every course-page render for editing teachers with no caching. block_teacher_checklist::get_content() instantiates the scanner and executes roughly eight checks — several of which issue their own $DB queries (assignments, forums, quizzes, gradebook, news-forum cmids) — on every page load where the block is shown. The queries are course-scoped and indexed, and get_fast_modinfo() is core-cached, so this is acceptable for typical courses; on very large courses it may add measurable per-request overhead. A short-lived per-request or MUC cache of the computed issue list would reduce repeated work if profiling ever shows it to matter.
Overall the plugin is a good reference example. It avoids every common block-plugin pitfall: no superglobals, no direct database/filesystem/HTTP access, no shell or eval usage, no DDL outside db/upgrade.php, no bundled third-party libraries, parameterized SQL throughout, and consistent output escaping. A db/uninstall.php is correctly absent because the plugin only writes to its own block_teacher_checklist table (dropped automatically on uninstall) and creates no external artifacts. The PHPUnit suite covering the scanner heuristics and the web-service capability gate is a notable strength.