MDL Shield

Flexible sections format

format_flexsections

Print Report
Plugin Information

Flexible sections course format for Moodle. Sections can be nested inside other sections, each section can be displayed on the same page as its parent or on a separate page as a link, and hiding a section cascades visibility to all subsections and activities.

Version:2026041700
Release:5.0.3
Reviewed for:5.1
Privacy API
Unit Tests
Behat Tests
Reviewed:2026-04-18
67 files·8,160 lines
Grade Justification

The plugin is a well-maintained course format with consistent security hygiene. All state-changing endpoints in page_set_course() (addchildsection, mergeup, deletesection, movesection, switchcollapsed, marker, hide, show) are gated by both confirm_sesskey() and has_capability(). The state actions class (classes/courseformat/stateactions.php) performs capability checks before every mutation (moodle/course:update, moodle/course:movesections, moodle/course:setcurrentsection). All SQL queries use $DB parameterised placeholders or $DB->get_in_or_equal(); there is no string concatenation into raw SQL and no use of PHP superglobals, direct database drivers, raw HTTP, filesystem bypass, or dangerous functions like eval/exec. The recent 5.0.3 CHANGELOG entry explicitly documents fixing two sesskey gaps flagged by an external scanner, and the current code reflects those fixes. Templates pass section and course titles through format_string() before rendering, and mustache triple-brace usage is confined to already-escaped values. The two low findings reported are: a minor Privacy API compliance gap around long-preference continuation keys (coursesectionspreferences_{id}#1, #2…) that fall outside the core_courseformat privacy provider's declared metadata, and a small number of hardcoded English strings in moodle_exception arguments. Neither has security impact.

AI Summary

format_flexsections is a mature and well-structured course format plugin that extends Moodle's standard course format with nested (subsection) capabilities, per-section display-as-link, accordion behaviour, and cascade-hide semantics.

Security posture: Strong. State-changing HTTP parameters in lib.php::page_set_course() are each protected by both confirm_sesskey() and a capability check. AJAX entry points in classes/courseformat/stateactions.php (which subclass \core_courseformat\stateactions) also enforce require_capability() for every mutation. The recent 5.0.3 release explicitly fixed two sesskey omissions found by an external security scanner, and those fixes are verified in the current source.

Data handling: All SQL uses $DB parameterised methods or $DB->get_in_or_equal() — no string-concatenation. Direct manipulation of course_sections and course_format_options is present but is the standard pattern for course format plugins and mirrors how core_courseformat\base itself operates. Section-deletion uses a lock_config to avoid concurrent corruption and properly raises course_section_deleted events. Files are cleaned up via the File API (get_file_storage()->delete_area_files()).

Third-party code: None bundled; no thirdpartylibs.xml needed.

Template safety: All user-controlled strings (title, name, sectionname, coursename) that are interpolated with {{{...}}} are first passed through format_string() in their backing PHP exporters, so triple-brace output does not introduce XSS.

Findings summary:

  • 1 low (compliance) — Privacy null_provider is incomplete for the long-preference continuation keys produced by the preferences trait.
  • 1 low (code quality) — Two moodle_exception calls in stateactions.php pass hardcoded English messages instead of language-string keys.

No critical, high, or medium issues were identified.

Findings

complianceLow
Privacy null_provider does not declare long-preference continuation keys

The plugin declares \core_privacy\local\metadata\null_provider in classes/privacy/provider.php, asserting that it stores no personal data. However, the preferences trait in classes/local/helpers/preferences.php persists user state by writing to set_user_preference() under the key pattern coursesectionspreferences_{courseid} and — for values longer than 1300 characters — additional continuation keys coursesectionspreferences_{courseid}#1, #2, #3, etc.

Moodle core's \core_courseformat\privacy\provider (Moodle 5.x) declares the coursesectionspreferences prefix and exports coursesectionspreferences_{courseid} per course, but it does not enumerate the #N continuation suffix keys written by this plugin's set_long_preference(). As a result, a GDPR data export for a user who has collapsed-state preferences large enough to spill past 1300 characters (e.g. a course with many hundreds of sections — and the plugin's own tests use 500 sections as a realistic case) will export only the first shard and silently omit the rest.

This is purely a compliance completeness issue — the data at stake is UI state (which sections are collapsed) rather than sensitive information, and when a user is deleted Moodle's user-preference cleanup still removes all keys for that user regardless of name. However, the plugin's explicit null_provider claim is not strictly accurate, and the core provider cannot know to export the additional shards.

Risk Assessment

Low risk. No security impact — collapsed/expanded state is not sensitive data, and Moodle's user deletion flow removes all rows in mdl_user_preferences for the target user regardless of preference name (the cleanup queries by userid, not by name). The practical consequence is a small gap in GDPR data-subject export completeness for users with very large courses. The finding is reported because the plugin's own privacy:metadata lang string says "does not store any personal data", which is a stronger claim than the behaviour supports.

Context

The preferences trait is mixed into the main format_flexsections class (see use preferences; near the top of lib.php). It is used by every section collapse/expand interaction. Because Moodle's user-preference column is varchar(1333), a single preference cannot hold the JSON for very large courses, so the trait transparently shards values across suffixed keys. The trait works correctly for storage/retrieval; the gap is only on the privacy-subsystem side.

Identified Code
class provider implements null_provider {
    /**
     * Get the language string identifier with the component's language
     * file to explain why this plugin stores no data.
     *
     * @return  string
     */
    public static function get_reason(): string {
        return 'privacy:metadata';
    }
}
Suggested Fix

Replace null_provider with a metadata\provider + user_preference_provider implementation that declares the continuation-key family and exports them alongside the primary preference. For example:

class provider implements
    \core_privacy\local\metadata\provider,
    \core_privacy\local\request\user_preference_provider {

    public static function get_metadata(collection $collection): collection {
        $collection->add_user_preference(
            'coursesectionspreferences_overflow',
            'privacy:metadata:preference:coursesectionspreferences_overflow'
        );
        return $collection;
    }

    public static function export_user_preferences(int $userid): void {
        // Iterate over the user's preferences, find any matching
        // 'coursesectionspreferences_{id}#N', and export them.
    }
}

Alternatively, rewrite the preferences trait so the split keys live under a namespace that core's provider already covers, or document clearly in privacy:metadata that overflow shards are part of the same logical preference.

Identified Code
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);
    }
}
Suggested Fix

This is the code creating the undeclared continuation preferences. No change needed here other than ensuring its privacy metadata is declared at the provider level (see the fix suggested on classes/privacy/provider.php).

code qualityLow
Hardcoded English strings passed to moodle_exception instead of language keys

Two moodle_exception throws in classes/courseformat/stateactions.php pass an English sentence as the first argument. moodle_exception's first argument is intended to be a language string key (with optional component), not free text.

When a string key is not found in any language file, Moodle falls back to displaying the raw first argument, which is why these appear to work — but:

  • The error message cannot be translated.
  • The error page / debug output shows [[the-english-sentence]] brackets in some contexts because Moodle flags it as an unresolved string key.
  • It is inconsistent with the plugin's otherwise-complete lang/en/format_flexsections.php.

The errorsectiondepthexceeded string in the same file demonstrates the correct pattern.

Risk Assessment

Low risk. No security impact — the messages do not leak sensitive data. Pure code quality / i18n hygiene issue that a senior Moodle reviewer would flag during manual review.

Context

These exceptions are thrown from AJAX state-action handlers invoked via the core_courseformat_course_update web service when a client triggers section_mergeup. In normal UI use the conditions are prevented by the JS side, so the messages are primarily seen by developers debugging broken payloads. Still, they surface in web-service responses and server logs.

Identified Code
if (!$targetsectionid) {
    throw new moodle_exception("Action section_mergeup requires targetsectionid");
}
Suggested Fix

Define a language string (e.g. errormergerequirestarget) in lang/en/format_flexsections.php and reference it:

if (!$targetsectionid) {
    throw new moodle_exception('errormergerequirestarget', 'format_flexsections');
}

Alternatively, since these are developer-facing (they indicate a malformed client request rather than a user-facing condition), switch them to coding_exception, which is documented as acceptable with an English message.

Identified Code
if (!$targetsection->parent) {
    throw new moodle_exception("Action section_mergeup can't merge top level parentless sections");
}
Suggested Fix

Same treatment as the previous location — either a proper language string or a coding_exception for developer-facing errors.

Additional AI Notes

Active security maintenance. The CHANGELOG.md for 5.0.3 (2026-04-17) explicitly credits an external scan (mdlshield.com) for identifying two missing sesskey validations on the addchildsection and movesection URL handlers, plus an $_SERVER['HTTP_REFERER'] reference that was swapped for get_caller_page_url(), and also tightened PARAM_RAWPARAM_INT for move-section parameters. All three fixes are verified in the current source (see lib.php::page_set_course() around lines 825–870 and lib.php::get_caller_page_url()). This is a positive signal that the maintainers are responsive to security findings.

Direct manipulation of course_sections / course_format_options. lib.php::move_section(), lib.php::delete_section_with_children(), lib.php::mergeup_section(), and backup/moodle2/restore_format_flexsections_plugin.class.php::cleanup_after_restore() all issue $DB->execute() / $DB->set_field() / $DB->delete_records() calls against the core course_sections and course_format_options tables. This is not flagged as a finding because (a) all queries are parameterised, (b) direct access to those two tables is the established pattern for course format plugins — core's own \core_courseformat\base does the same — and (c) the plugin's custom nested-section logic is not expressible through the non-flat core helpers. A \core\lock\lock_config lock is used in delete_section_with_children() to serialise destructive operations, and course_section_deleted events are raised with proper snapshot data.

Deprecated but retained API usage is version-guarded. lib.php::duplicate_section() calls duplicate_module() for $CFG->branch < 502 and switches to \core_courseformat\formatactions::cm()->duplicate() on 5.2+, which is the correct migration path. get_max_sections() is overridden by the plugin; core deprecated the method in 5.1 but the plugin's override does not call parent:: and therefore does not emit deprecation notices, which is intentional given the plugin still supports Moodle 5.0.

Test coverage. The plugin ships PHPUnit tests in tests/format_flexsections_test.php, tests/backup_restore_test.php, and tests/local/helpers/preferences_test.php, plus Behat steps in tests/behat/behat_format_flexsections.php. The tests specifically exercise inplace-editable permission boundaries, section deletion cascade, recursive hide, partial-import cleanup (regression for issue #17), and long-preference handling. This is meaningfully above average for a format plugin.

Safe template output. Triple-brace mustache variables ({{{title}}}, {{{coursename}}}, {{{sectionname}}}, {{{name}}}) are used throughout the templates, but in every case the backing PHP (classes/output/courseformat/content.php, the state exporters, and the section_title() renderer) routes the value through format_string() or core's inplace_editable_render_section_name() before it reaches the template. No unescaped user-controlled strings are passed to {{{...}}}.

This review was generated by an AI system and may contain inaccuracies. Findings should be verified by a human reviewer before acting on them.