Vault - Site backup and migration

The plugin's privacy provider implements \core_privacy\local\metadata\null_provider and returns the language string privacy:metadata ("The Vault plugin doesn't store any personal data."). However, the plugin does store local personal data about the admin who initiated each operation: userid, fullname, email (and username for restores) are persisted in the JSON details column of tool_vault_operation, and these values are subsequently displayed on the backup/restore details pages via get_performedby().

Because null_provider is mutually exclusive with storing personal data, the declaration is incorrect and the plugin should:

• Drop null_provider.
• Implement \core_privacy\local\request\plugin\provider, declaring the relevant fields in tool_vault_operation (and tool_vault_log if log lines reference user identifiers).
• Implement core_userlist_provider and the export/delete callbacks.

The data is admin-only and limited in scope, so the practical impact is small, but this is a compliance gap that prevents a true GDPR data-export/delete request from removing the admin's name and email from the Vault history.

operation_model::format_log_line() builds the per-line markup with \html_writer::span($message, $class). Moodle's html_writer::span calls html_writer::tag which does not escape the $contents argument (verified in lib/classes/output/html_writer.php). The resulting string is then injected into Mustache templates with triple-brace placeholders — {{{logs}}}, {{{logsshort}}} — so any HTML that ends up in a log row is rendered verbatim.

The same pattern applies to error_with_backtrace.mustache ({{{ error }}} and {{{.}}} for the backtrace), where the source is a captured \Throwable::getMessage() / getTraceAsString() stored in tool_vault_operation.details['error'].

The practical attack surface is narrow:

• Pages that render these values are gated by moodle/site:config or by knowing the unguessable 32-character accesskey for progress.php.
• Most log messages are static strings or interpolate values that are themselves controlled (file/table names from a controlled archive, display_size() output, integer counters).
• The prepare_api_exception() / prepare_s3_exception() helpers already wrap remote API/S3 responses with s() before they become exception messages.

The residual risk is an admin-to-admin (or accesskey-holder) XSS path through:

• A maliciously crafted backup whose __metadata__.json / DB-dump file/table names contain HTML (these become parts of 'failed to insert chunk of records into table $tablename' warnings).
• A \dml_exception / generic PHP exception whose driver-supplied message contains < characters.

Because the surrounding pages already require the highest privilege available, exploitation gives the attacker nothing they don't already have, but the inconsistency with the Moodle output convention is worth fixing.

apikey_form_legacy declares returnurl as PARAM_URL (which only normalises a URL — it does not require a local one), and main::process() redirects to whatever the form returns once an API key is submitted. Combined with main_forgetapikey (which uses PARAM_LOCALURL correctly), the inconsistency means that an admin who is tricked into clicking a crafted Vault sign-in URL with an external returnurl parameter could be bounced to an attacker-controlled host after submitting their key.

Because the entire flow is inside admin_externalpage_setup() it requires moodle/site:config, so the realistic phishing value is low: a victim who can be coerced into clicking the link is already an admin and could have been redirected by other means. Still, the fix is trivial — use PARAM_LOCALURL like the sibling action.

The plugin reaches into private/protected core internals via \ReflectionObject / \ReflectionProperty in two places:

• dbops::calculate_row_packet_sizes() calls the protected mysqli_native_moodle_database::emulate_bound_params() to estimate per-row packet size before issuing a multi-row insert.
• restoreactions\upgrade::disable_caches() and enable_caches() overwrite the private static cache_factory::$instance so a cache_factory_disabled instance is used during the simulated upgrade.

Both are deliberate workarounds for missing public APIs and both are documented inline. Neither is a security vulnerability — Moodle has no public way to do these things — but they are fragile: any core refactor that renames or removes the internals (a credible scenario across the 4.x → 5.x → 5.1+ jump) will break the plugin silently.

Recommend tracking via Moodle Tracker so a public extension point eventually replaces the reflection use, and add a defensive try { … } catch (\ReflectionException $e) { … } so a missing internal degrades to a slower-but-correct path rather than a fatal during backup/restore.

after_upgrade_task::queue() writes directly to the core task_adhoc table instead of going through \core\task\manager::queue_adhoc_task(). The reason is documented in the docblock: at the moment this is called the standard task manager may not yet be available because the upgrade is still in flight.

The pattern is unavoidable given the constraint, but it does mean future schema changes to task_adhoc (e.g. additional NOT NULL columns) would silently break this path. The risk is functional rather than security.