Flexible sections format
format_flexsections
Flexible sections course format. Organises course content into any number of sections that can be nested to arbitrary depth. Each section can be displayed on the same page as its parent or on a separate page, and the plugin integrates with the modern reactive course editor (course index, drag/drop-free move dialogs, bulk edit tools, hooks and web-service based state actions).
This is a mature, professionally written course format plugin with no security vulnerabilities identified across its PHP, JavaScript and Mustache code.
Security posture is strong:
- Every state-changing action in
lib.php::page_set_course()(add/merge/delete/move sections, markers, visibility, collapse toggles) is gated behind bothconfirm_sesskey()and an appropriatehas_capability()/has_all_capabilities()check, protecting the GET-based link fallbacks against CSRF. - The reactive-editor AJAX operations in
classes/courseformat/stateactions.phpeach callrequire_capability()(andvalidate_sections()); sesskey is enforced by the core web-service layer they run under. - The inplace section-name callback and the
update_inplace_editableweb service delegate to core'sinplace_editable_update_section_name(), which validates the context, requiresmoodle/course:updateandclean_param()s the input — confirmed by reading core and by the plugin's own tests. - All database access uses the
$DBAPI with placeholders (including the backup cleanup and the long-preference LIKE query); no raw SQL concatenation, no direct filesystem or schema access outsidedb/upgrade.php. - Template output follows core's patterns; unescaped
{{{title}}}/{{{sectionname}}}values are produced byget_section_name()/format_string()and so are already sanitised.
Only minor issues remain: a hardcoded English string used for an accessibility heading instead of get_string(), and an incomplete Privacy API declaration (overflow user-preference chunks are stored but not exported in a subject-access request). A best-practice note covers the use of PHP reflection to mutate a core object's protected property. None of these have real-world harm potential, and the plugin ships with broad, security-aware automated test coverage.
format_flexsections is a course format plugin by a well-known core contributor that lets courses organise content into arbitrarily nested sections. The review covered all PHP classes, the legacy procedural entry points (lib.php, format.php), the AMD JavaScript modules, all Mustache templates, the backup/restore handler, settings, hooks and tests.
No critical, high or medium issues were found. The plugin is a good example of correct Moodle security practice:
- Authorisation is consistently enforced. The GET-driven actions in
page_set_course()all require a valid sesskey and the relevant capability. The reactivestateactionsmethods callrequire_capability()and validate that section IDs belong to the course before acting. Deleting/merging sections additionally checks the capability on every activity in all affected subsections (can_delete_section(),can_mergeup_section()). - Data access is safe. Every query uses parameterised
$DBcalls, including the two-step renumbering in the restore cleanup and thesql_like-based preference cleanup. Section-summary files are handled through the File API. - Output is correctly escaped, matching core's own templates; the unescaped title/name values originate from
format_string().
Findings (all low / informational):
page_title()returns the hardcoded English literal'Topic outline'on Moodle 4.4+, which is surfaced as a screen-reader heading — an internationalisation gap.- The Privacy API provider is a
null_provider, but the plugin stores overflow user-preference chunks (coursesectionspreferences_<id>#N) that no provider exports in a data-subject request. - A best-practice note on the use of reflection to remove an action link from a core activity-chooser-button object.
The code is well tested (capability, permission-boundary, recursion-safety and backup/restore regression tests) and bundles no third-party libraries.
Findings
In lib.php, format_flexsections::page_title() returns the untranslated English literal 'Topic outline' on Moodle 4.4 and later, instead of a language string from get_string().
This value is not dead code: core's core_courseformat\output\local\content::export_for_template() sets 'title' => $format->page_title(), and the plugin's own templates/local/content.mustache renders it as <h2 class="accesshide">{{{title}}}</h2> — a visually hidden heading read out by screen readers.
As a result, assistive-technology users on a non-English site hear the English phrase "Topic outline" regardless of their configured language. Core formats return a translatable string here (for example format_topics returns get_string('sectionoutline')), or defer to the parent which returns $PAGE->title.
The author's own // TODO it is possible it is not used anymore. Review. comment reflects uncertainty about this branch.
Low risk. This is an internationalisation / coding-standard defect, not a security issue. The blast radius is limited to the text of a screen-reader-only heading on the course main page for sites not using English. There is no data exposure or control-flow impact. It is worth fixing for accessibility and translation completeness.
page_title() overrides core_courseformat\base::page_title(). On Moodle 4.4+ the override short-circuits to a literal string; on older branches it uses get_string('topicoutline'). The returned value flows into the course content exporter and is printed in content.mustache as an accesshide <h2>.
public function page_title(): string {
global $CFG;
if ((int)$CFG->branch >= 404) {
// TODO it is possible it is not used anymore. Review.
return 'Topic outline';
} else {
return get_string('topicoutline');
}
}
Return a translatable string in both branches, or drop the override entirely so the parent implementation ($PAGE->title) is used. For example:
public function page_title(): string {
return get_string('topicoutline');
}
If a flexsections-specific wording is desired, add a key to lang/en/format_flexsections.php and reference it via get_string('...', 'format_flexsections').
The plugin's privacy provider implements null_provider and the privacy:metadata string states "The Flexible sections format plugin does not store any personal data." However, the preferences trait (classes/local/helpers/preferences.php) does store per-user personal data via set_user_preference().
To work around the 1333-character limit on a single preference, set_long_preference() splits the JSON of section collapse/expand state across multiple preferences named coursesectionspreferences_<courseid>, coursesectionspreferences_<courseid>#1, coursesectionspreferences_<courseid>#2, and so on.
Core's core_courseformat\privacy\provider::export_user_preferences() exports only the exact base name (get_user_preferences('coursesectionspreferences_<courseid>', ...)); it has no knowledge of the #N suffix convention, which is invented by this plugin. Consequently, when a user has enough collapsed sections for the value to exceed one chunk, the overflow preferences (#1, #2, …) are not included in a subject-access (data export) request by any provider.
Notably, the plugin is aware of these extra preferences for deletion — format_flexsections::delete_format_data() explicitly removes the #-suffixed rows with a sql_like query on course deletion — but the corresponding export side is not implemented.
Low risk. This is a GDPR/Privacy API completeness gap, not a security vulnerability. The data involved is low-sensitivity interface state (which sections a user has collapsed), and only the overflow chunks of large states are affected. Deletion of the data is already handled correctly. The practical impact is that a data-export request could be slightly incomplete for power users; the null_provider declaration is also technically inaccurate for this plugin.
The section collapse/expand state is per-user UI state persisted to the user_preferences table. For most users the value fits in a single preference (the base name), which core's courseformat subsystem does export. The gap only manifests for users whose state JSON exceeds ~1300 characters (roughly 40+ sections with stored state), producing #1+ chunks that no provider exports.
class provider implements null_provider {
public static function get_reason(): string {
return 'privacy:metadata';
}
}
Implement \core_privacy\local\metadata\provider and \core_privacy\local\request\user_preference_provider instead of null_provider, declare the section-preferences data in get_metadata(), and in export_user_preferences() reassemble and export the full value (including the #N chunks) using get_long_preference(). Deletion is already handled in delete_format_data().
public static function set_long_preference(string $name, ?string $value): void {
$allpreferences = array_filter(get_user_preferences(), function ($prefname) use ($name) {
return $prefname === $name || (strpos($prefname, "{$name}#") === 0);
}, ARRAY_FILTER_USE_KEY);
$len = ceil(core_text::strlen((string)$value) / 1300);
for ($cnt = 0; $cnt < $len; $cnt++) {
$pref = self::get_preference_name($name, $cnt);
set_user_preference($pref, core_text::substr($value, $cnt * 1300, 1300));
unset($allpreferences[$pref]);
}
foreach (array_keys($allpreferences) as $pref) {
unset_user_preference($pref);
}
}
No change required here; this is the source of the extra preferences that the privacy provider must account for.
The before_activitychooserbutton_exported hook callback removes the core-provided "subsection" entry from the activity chooser button by using ReflectionObject to make the protected actionlinks property of \core_course\output\activitychooserbutton accessible and then overwriting it.
Core's activitychooserbutton class currently exposes only add_action_link(); it provides no public method to read or remove existing action links, so reflection is the only available mechanism to achieve this. The code degrades gracefully (?? null guards), but reaching into a core class's internal state is fragile: a future core change to the property name, visibility or structure would silently break this behaviour.
Informational. There is no security impact — the manipulation only affects which button appears in the activity chooser, and the surrounding code handles a missing property safely. The concern is maintainability/robustness against core changes, which a senior reviewer would typically flag.
The hook fires when the activity chooser button is exported for a flexsections course. The plugin removes the generic "subsection" action because it manages subsections through its own controls. This runs only in course editing contexts for users who can add activities.
// Remove action link added by submodule. Use Reflections to set protected property $activitychooserbutton->actionlinks.
$refobject = new \ReflectionObject($activitychooserbutton);
$refproperty = $refobject->getProperty('actionlinks');
$refproperty->setAccessible(true);
$actionlinks = $refproperty->getValue($activitychooserbutton);
$actionlinks = array_filter($actionlinks, fn($a) => ($a->attributes['data-modname'] ?? null) !== 'subsection');
$refproperty->setValue($activitychooserbutton, array_values($actionlinks));
Where possible, prefer a public core API for manipulating the button's action links. If none exists, consider proposing a removal/accessor method upstream (for example a remove_action_link() or a getter) so this workaround can be replaced, and add a guard/test to detect if the reflected property disappears in a future Moodle release.
No third-party libraries are bundled. The files under amd/build/ are the minified/transpiled output of the plugin's own sources in amd/src/ (standard Moodle grunt build artifacts), not vendored code, so the absence of thirdpartylibs.xml is correct and not a finding.
State-changing operations are consistently protected. The legacy GET-link handlers in lib.php::page_set_course() (addchildsection, mergeup, deletesection, movesection/moveparent/movebefore, switchcollapsed, marker, hide, show) each require confirm_sesskey() together with a capability check, and the reactive classes/courseformat/stateactions.php methods each call require_capability() and validate_sections(). This is a good example of correct CSRF/authorisation handling.
Recursion safety is handled deliberately. Methods that walk the section parent hierarchy (get_section_depth(), section_has_parent(), find_collapsed_parent(), can_delete_section(), delete_section_with_children()) guard against broken/cyclic parent references, and a dedicated unit test (test_broken_parent) exercises this. delete_section_with_children() also takes a named lock before mutating the course structure.
Automated test coverage is strong and security-aware, including tests that assert web-service permission failures, that sections with protected activities cannot be deleted/merged, that move/add require moodle/course:movesections, and backup/restore regression tests for partial imports.
Database portability looks good. The backup cleanup and renumbering logic in restore_format_flexsections_plugin uses parameterised, engine-neutral SQL and the same two-step negative-numbering technique as the core-aligned move_section(); no MySQL-only functions were observed.