Logstore xAPI

logstore_xapi

A more recent review of this plugin is available. View the latest review →

Plugin Information

logstore_xapi is a logging-store subplugin (admin/tool/log/store/xapi) that captures Moodle events, transforms them into xAPI (Experience API) statements, and transmits them to an external Learning Record Store (LRS). It provides an admin report UI for inspecting failed and historical events and re-queuing them for resend, scheduled tasks that batch and emit statements, and email notifications to selected cohorts when send failures accumulate.

marketplace.moodle.com/plugins/logstore_xapi

davidpesce/moodle-logstore_xapiv5.0.3

Version:2026072000

Release:5.0.3

Reviewed for:5.1

Privacy API

Unit Tests

Behat Tests

Reviewed:2026-07-21

444 files·34,069 lines

Grade Justification

The plugin demonstrates a strong overall security posture. Every browser-facing entry point enforces require_login(), require_sesskey(), and an appropriate require_capability() check; the report filter is a moodleform (automatic sesskey); all SQL is parameterised via $DB placeholders or the repository abstraction; report output escapes user data and even double-escapes the untrusted LRS response body; and outbound TLS verification is explicitly enabled. The plugin also ships a dedicated security check that surfaces disabled LRS certificate verification in the site security report, and carries a broad PHPUnit suite.

The most significant issue is a compliance gap: the Privacy API provider declares that two tables store personal data yet implements every export and deletion method as an empty stub, so subject-access requests return nothing and erasure requests silently leave data in place — most importantly in the persistent logstore_xapi_failed_log table. Its impact is bounded because the primary logstore_xapi_log queue is transient (emptied by cron). The remaining items are low-severity code-quality and best-practice observations: a raw cURL client that bypasses core's proxy and egress controls (though the endpoint is admin-only and TLS is explicitly verified), decoding of the event other field with bare unserialize() instead of core's hardened format-aware decoder, an inline <script> block in an admin setting, and an eager autoloader that includes the whole src/ tree per request. No vulnerability is exploitable by students, teachers, or unauthenticated visitors, and none permits access to or modification of other users' data.

AI Summary

This is a mature, security-conscious logstore plugin, and the review found no vulnerabilities exploitable by low-privilege or unauthenticated users.

Strengths observed:

All AJAX and report entry points enforce require_login() + require_sesskey() + require_capability(); the report filter form is a moodleform (automatic sesskey handling).
Database access is uniformly parameterised — direct $DB calls use placeholders, and the transformer layer reads exclusively through a repository wrapper around get_records()/get_record() with bound array conditions.
Report output is escaped, and the untrusted LRS response body is deliberately double-escaped before display.
Outbound LRS requests explicitly enable TLS peer and host verification, and a dedicated \core\check\check surfaces any disabled verification in the site security report.

Findings:

#	Severity	Area
1	medium	Privacy API export/erasure implemented as no-op stubs
2	low	Raw cURL client bypasses proxy + egress controls
3	low	Bare `unserialize()` of event `other` diverges from core's safe decoder
4	low	Inline `<script>` in an admin setting
5	low	Autoloader eagerly includes the whole `src/` tree per request
6	info	Inconsistent escaping of report table cells (not exploitable)

The headline issue is the non-functional Privacy API: the provider advertises stored personal data but no-ops all export and deletion requests, so GDPR erasure silently fails while data persists in the failed-events table. Everything else is code quality or hardening.

Findings

complianceMedium

Privacy provider declares stored personal data but implements export and deletion as no-ops

The Privacy API provider's get_metadata() declares that both logstore_xapi_log and logstore_xapi_failed_log store the user's id, so the plugin explicitly is not a null provider — it promises to honour data-subject requests.

However, although the class implements the full \core_privacy\local\request\plugin\provider and core_userlist_provider interfaces, every request method is an empty stub:

get_contexts_for_userid() returns an empty contextlist unconditionally.
get_users_in_context(), export_user_data(), delete_data_for_all_users_in_context(), delete_data_for_users(), and delete_data_for_user() all have empty bodies.

Consequences:

A subject access / data export request returns nothing for this plugin, even though the user's event rows exist.
A right-to-erasure request completes successfully but deletes nothing. The user's personal data is retained — most importantly in logstore_xapi_failed_log, which persists failed events (including userid, IP address, and the serialized event other payload) indefinitely until an admin manually clears them.

Secondary inaccuracy: the metadata maps the failed-log field as moodleuserid, but the actual column (per db/install.xml) is userid, so the declared field name does not match the schema.

Risk Assessment

Medium risk. This is a genuine GDPR/Privacy-API compliance failure rather than a memory-safety or access-control bug. The blast radius is every user who exercises a data-subject right on a site running this plugin: exports omit their xAPI queue/failure data, and erasure requests report success while leaving rows behind. Impact is bounded by the transient nature of the main queue, but the persistent failed-events table can retain userid, IP address, and event payloads indefinitely. Because the interface is implemented (not declared absent), the failure is silent — an administrator or DPO processing an erasure has no signal that data was left in place, which is what elevates this above a merely "incomplete" implementation.

Context

This is a logstore, so retaining and forwarding user activity data is its core purpose. The primary logstore_xapi_log table is a short-lived queue that the emit_task empties every minute after sending, but logstore_xapi_failed_log accumulates events that failed to transmit and holds them until an administrator resends or clears them, making it a durable store of personal data. The provider correctly advertises this via get_metadata() but never acts on it.

classes/privacy/provider.php:77

Affected Code

public static function get_contexts_for_userid(int $userid): contextlist {
    return new contextlist();
}

public static function get_users_in_context(userlist $userlist) {
}

public static function export_user_data(approved_contextlist $contextlist) {
}

public static function delete_data_for_all_users_in_context(\context $context) {
}

Suggested Fix

Implement the request methods so they actually locate, export, and delete the user's rows in logstore_xapi_log and logstore_xapi_failed_log (keyed on userid). For example, get_contexts_for_userid() should add the system context (or the contexts referenced by the user's events), export_user_data() should write the matching rows via writer::with_context(...)->export_data(...), and the delete methods should run $DB->delete_records_select(...) for the approved users/contexts. The logstore_standard provider is a suitable reference implementation.

Also correct the declared field name from moodleuserid to userid in get_metadata().

classes/privacy/provider.php:60

Affected Code

$collection->add_database_table(
    'logstore_xapi_failed_log',
    [
        'moodleuserid' => 'privacy:metadata:logstore_xapi_failed_log:userid',
    ],
    'privacy:metadata:logstore_xapi_failed_log'
);

Suggested Fix

Use the real column name userid as the metadata key so the declaration matches the schema.

code qualityLow

LRS statements sent via a raw cURL client, bypassing Moodle's HTTP layer

The loader sends xAPI statements to the LRS using PHP's cURL functions directly (curl_init(), curl_setopt_array(), curl_exec()) rather than a Moodle-sanctioned HTTP client such as the \curl class or \core\http_client.

A raw client does not pass through:

the site's proxy configuration ($CFG->proxyhost etc.), so on proxied networks the plugin cannot reach the LRS; and
core's curl_security_helper, which the sanctioned wrappers use to enforce the site's blocked-hosts and allowed-ports egress lists.

Mitigating factors:

The destination is the admin-only endpoint setting (PARAM_URL, only writable with moodle/site:config), so no user below site administrator can influence the request target — this is not a server-side request forgery vector for lower-privilege roles.
The code explicitly sets CURLOPT_SSL_VERIFYPEER => true and CURLOPT_SSL_VERIFYHOST => 2. This is in fact stricter than Moodle's \curl wrapper, whose curl::resetopt() ships CURLOPT_SSL_VERIFYPEER = 0; the in-code comment documenting this is accurate.

Additional consideration: the endpoint accepts http://. If an administrator configures a plaintext endpoint, the Basic-auth LRS credentials and learner PII in the statements would traverse the network in cleartext.

Risk Assessment

Low risk. Because the target URL is influenced only by a site administrator and TLS verification is explicitly enabled, there is no practical SSRF or man-in-the-middle exploit reachable by any Moodle role. The real defects are operational and stylistic: loss of proxy support, bypass of core's outbound egress controls, and divergence from Moodle's HTTP coding guidelines. The cleartext-credentials concern requires an administrator to deliberately configure a non-TLS endpoint. Set to no specific exploitable role since the residual risk is a network-position / admin-configuration matter rather than a privilege-based attack.

Context

load() is invoked from the scheduled tasks (emit_task, failed_task, historical_task) in cron context. build_curl_options() assembles the cURL option array, correctly forcing TLS verification on by default and only disabling it when the administrator turns off the sslverification setting. The URL is derived solely from the admin-configured endpoint value via correct_endpoint().

src/loader/moodle_curl_lrs.php:105

Identified Code

$request = curl_init();
curl_setopt_array($request, build_curl_options($config, $url, $auth, $postdata));

$responsetext = curl_exec($request);
$responsecode = curl_getinfo($request, CURLINFO_RESPONSE_CODE);

Suggested Fix

Route the request through a sanctioned client so the site proxy and curl_security_helper apply, while keeping the explicit TLS options the plugin already sets. For example, using the \curl class:

$curl = new \curl();
$options = [
    'CURLOPT_SSL_VERIFYPEER' => true,
    'CURLOPT_SSL_VERIFYHOST' => 2,
    'CURLOPT_RETURNTRANSFER' => true,
];
if (!empty($config['lrs_ssl_cabundle'])) {
    $options['CURLOPT_CAINFO'] = $config['lrs_ssl_cabundle'];
}
if (isset($config['lrs_ssl_verification']) && empty($config['lrs_ssl_verification'])) {
    $options['CURLOPT_SSL_VERIFYPEER'] = false;
    $options['CURLOPT_SSL_VERIFYHOST'] = 0;
}
$curl->setHeader([...]);
$responsetext = $curl->post($url, $postdata, $options);

Consider validating (or requiring) an https:// endpoint to avoid cleartext transmission of credentials.

code qualityLow

Event `other` field decoded with bare unserialize() instead of core's safe, format-aware decoder

Across 23 transformer and utility files, the event payload is decoded with unserialize($event->other) passing no options. Moodle core reads this exact field through \tool_log\helper\reader::decode_other(), which:

auto-detects whether the stored value is PHP-serialized or JSON (if ($other === 'N;' || preg_match('~^.:~', $other)) { unserialize(...) } else { json_decode(...) }); and
hardens the deserializer with unserialize($other, ['allowed_classes' => [stdClass::class]]).

The plugin's bare calls create two gaps:

Hardening (defense-in-depth). No allowed_classes restriction is applied. In practice this is not exploitable: the other bytes are always produced by core's buffered writer via serialize() of a scalar/array event payload, so user-supplied values become length-prefixed string values and an attacker cannot inject an O:-object descriptor without direct database write access. It is nonetheless a divergence from the core-sanctioned, restricted decoder.
Robustness / compatibility. The code assumes the PHP-serialized format. The plugin's own queue always uses serialize(), so live events are fine, but the historical-replay feature sources rows from logstore_standard_log, whose other column is JSON on sites using logstore_standard's default jsonformat = 1 setting. There, unserialize() returns false and the transform fails.

Risk Assessment

Low risk. The object-injection angle is theoretical only: the other bytes originate from core's serialize() of trusted event data, and there is no code path by which a user below database-administrator can place a crafted O:-object descriptor into that column. The tangible impact is (a) a hardening gap relative to the core decoder and (b) a real robustness defect — historical replay silently fails to transform events on the (default) JSON logstore format. Adopting decode_other() fixes both at once.

Context

The transformer layer converts each stored Moodle event into xAPI statements. Event-specific details live in the serialized other field, which these transformers decode to extract ids and names (discussion id, badge issue id, SCORM attempt id, certificate code, etc.). The decoded values feed only into JSON structures sent to the LRS — never into SQL or HTML — which is why the deserialization surface, not downstream use, is the concern here.

src/transformer/events/mod_forum/post_created.php:44

Identified Code

$other = unserialize($event->other);

Suggested Fix

Replace with the core decoder, which handles both formats safely:

$other = \tool_log\helper\reader::decode_other($event->other);

If a direct call is preferred, at minimum forbid object instantiation: unserialize($event->other, ['allowed_classes' => false]).

src/transformer/events/core/badge_awarded.php:56

Identified Code

$other = unserialize($event->other);

Suggested Fix

Use \tool_log\helper\reader::decode_other($event->other) (or unserialize(..., ['allowed_classes' => false])).

src/transformer/utils/get_enrolment_context.php:47

Identified Code

$info = unserialize($event->other);

Suggested Fix

Use \tool_log\helper\reader::decode_other($event->other) (or unserialize(..., ['allowed_classes' => false])).

src/transformer/events/mod_scorm/status_submitted.php:45

Identified Code

$unserializedcmi = unserialize($event->other);

Suggested Fix

Use \tool_log\helper\reader::decode_other($event->other) (or unserialize(..., ['allowed_classes' => false])).

src/transformer/events/tool_certificate/certificate_revoked.php:40

Identified Code

$code = unserialize($event->other)['code'];

Suggested Fix

Decode once with \tool_log\helper\reader::decode_other($event->other) and index into the result.

best practiceLow

Inline <script> block emitted by the event-routes admin setting

The custom admin_setting_configroutes::output_html() appends a literal <script> element to the settings markup to wire up the per-group "select all / deselect all" toggles.

The script body is entirely static — no PHP variables are interpolated into it — so it is not an XSS vector. However, inline JavaScript:

violates Moodle's JavaScript coding guidelines (JS should be delivered as AMD modules);
is inconsistent with the plugin's own approach elsewhere (it already ships amd/src/cohort_selector.js and amd/src/replayevents.js); and
will be blocked on sites that enforce a strict Content-Security-Policy disallowing inline scripts, silently breaking the toggle links.

Risk Assessment

Low risk. No injection is possible because the script is a fixed string with no user or configuration data interpolated. The issue is purely a coding-standards and forward-compatibility concern (CSP hardening), scoped to an admin-only settings page.

Context

This setting renders the list of Moodle events that can be routed to the LRS, grouped by component. It is only reachable by a user with moodle/site:config on the plugin settings page. The rest of the method correctly escapes all dynamic values (event class names, labels) with s().

classes/admin_setting_configroutes.php:111

Identified Code

// Inline JS for select all / deselect all toggles.
$html .= '<script>
document.addEventListener("click", function(e) {
    var el = e.target.closest(".logstore-xapi-groupaction");
    if (!el) return;
    e.preventDefault();

Suggested Fix

Move the toggle behaviour into a small AMD module (mirroring cohort_selector.js) and initialise it from output_html() via $PAGE->requires->js_call_amd('logstore_xapi/routes', 'init'), removing the inline <script>.

best practiceLow

Autoloader eagerly includes the entire src/ tree on every load

autoload_src() walks the whole src/ directory with a RecursiveDirectoryIterator and require_onces every .php file it finds — all loaders, the ~200 event transformers, and the ~80 utility functions.

Because autoload.php is pulled in at the top of classes/log/store.php (the log writer) and settings.php, this runs on essentially every request that logs an event, loading hundreds of files whether or not the current event has a transformer.

This is a performance and footprint concern rather than a security issue. PHP's opcache avoids recompilation, but the recursive directory walk and the require_once bookkeeping for hundreds of files still execute per request. It also loads test-only doubles (e.g. TestRepository) into production runtime.

Risk Assessment

Low risk. No security impact. The cost is added latency and memory on high-traffic sites and the inclusion of test scaffolding in production. Worth addressing for efficiency, but the behaviour is correct.

Context

The src/ tree uses a functional (non-class) code style, so the functions must be included before use. The current approach trades per-request overhead for simplicity. On a busy site every logged event triggers the store writer, which triggers this include sweep.

src/autoload.php:36

Identified Code

function autoload_src() {
    $directory = new \RecursiveDirectoryIterator(__DIR__);
    $iterator = new \RecursiveIteratorIterator($directory);
    foreach ($iterator as $info) {
        $pathname = $info->getPathname();
        if (substr($pathname, -4) === '.php' && $pathname != __FILE__) {
            require_once($pathname);
        }
    }
}

Suggested Fix

Load only what the active code path needs. Since the transformer/loader dispatch already builds fully-qualified function names (\src\transformer\events\..., \src\loader\{loader}\load), register a lightweight autoloader (or explicit require_once at the dispatch site) that includes the single file backing the function actually being invoked, rather than eagerly including the whole tree.

best practiceInfo

Inconsistent output escaping of report table cells

In report.php, the response cell is carefully escaped (via json_encode(...) with JSON_HEX_* flags wrapped in s()), and the username cell is passed through s(). The eventname and errortype cells, however, are pushed into the html_table verbatim. html_writer::table() does not auto-escape cell content, so it relies on the caller having escaped it.

This is not exploitable: eventname holds a PHP event class name (constrained to identifier characters such as \, letters, digits, and underscores — it cannot contain markup) and errortype holds an integer error code. Neither value is user-influenced. It is raised only as a defense-in-depth consistency observation.

Risk Assessment

Info. There is no realistic injection path because the unescaped values are drawn from a fixed vocabulary of event class names and integer error codes, none of which can carry HTML. Flagged purely so that output escaping in this security-sensitive admin report is applied uniformly.

Context

The report page (id = 0 error log, id = 1 historical log) is reachable only with logstore/xapi:viewerrorlog or logstore/xapi:managehistoric (manager-level) capabilities. The cell values originate from the plugin's own log tables and, ultimately, from Moodle's event system, which populates eventname with registered event class names only.

report.php:280

Identified Code

$row[] = $result->eventname;

Suggested Fix

For consistency with the other cells, wrap dynamic values in s():

$row[] = s($result->eventname);

Apply the same to the errortype cell.

Additional AI Notes

Overall security posture is strong. The browser-facing scripts (ajax/moveback_to_log.php, ajax/search_cohorts.php, report.php) consistently combine require_login(), require_sesskey(), and require_capability(); search_cohorts.php additionally restricts to moodle/site:config. The report filter is a moodleform, so sesskey validation on the resend action is handled by the framework. No missing-authorisation or CSRF gaps were found.

SQL is uniformly parameterised. Direct $DB usage relies on placeholders and helpers (get_in_or_equal, sql_like with sql_like_escape), and the entire transformer layer reads through a repository wrapper around get_records()/get_record() with bound array conditions. The one interpolated column name in logstore_xapi_get_distinct_options_from_failed_table() is only ever called with the hard-coded literals 'errortype' and 'response', so it is not injectable.

Good handling of untrusted external data. The report explicitly treats the LRS response body as untrusted and double-escapes it (json_encode with JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_QUOT, then s()) before rendering it inside a <pre> — a deliberate and correct defensive measure.

Nice defensive touch: the plugin registers a dedicated \core\check\check (ssl_verification) that reports disabled LRS TLS verification in the site security overview, and flips to a warning only once the store is enabled.

The account_homepage setting is declared PARAM_TEXT although it holds a URL/IRI used as the xAPI actor account homePage; PARAM_URL would be more appropriate. This is admin-set and flows only into JSON, so it is not a security issue.

No bundled third-party runtime code. thirdpartylibs.xml is present (empty), and the only Composer dependency (yetanalytics/statementfactory) is declared under require-dev and is referenced nowhere in the runtime code (only in composer.lock), so no unshipped library is depended on at runtime and nothing duplicates a core-bundled library.

Test coverage is good: the plugin ships an extensive PHPUnit suite with per-module transformer fixtures plus targeted tests for build_curl_options, the SSL-verification check, the cohort selector, and the report/moveback flows.

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