MDL Shield

This is an older published review of this plugin. View the latest review →

Sentinel

local_sentinel

Published by Exputo

Plugin Information

local_sentinel ("Sentinel") is a per-instance Moodle fleet-monitoring plugin. It exposes operational data about a single Moodle site — status/version, environment, plugin inventory, health (cron/tasks/disk/backup), authentication and web-service-token stats, core report checks, recent config changes, config drift, and core-file integrity — through authenticated REST web services (pull) and an optional outbound push to a central dashboard. It ships a data-egress filter so admins can withhold slices/fields, a core-file integrity scanner that diffs the tree against a dashboard-provided manifest, and CLI + admin-UI setup helpers that provision a dedicated web-service role, user, and token.

Version:2026061201
Release:2.21.1
Reviewed for:5.2
Privacy API
Unit Tests
Behat Tests
Reviewed:2026-09-14
73 files·12,167 lines
Grade Justification

The plugin is engineered to a high standard and its security-relevant surfaces are, with one exception, handled correctly. Every web-service function enforces a capability check (local/sentinel:view for reads, local/sentinel:manage for writes) via a shared base class; every state-changing admin action is sesskey-guarded; all admin pages go through admin_externalpage_setup(); all dynamic output in the renderer is escaped with s(); all SQL uses $DB placeholders / get_in_or_equal; the manifest ingest validates format, digest (hash_equals), and caps gzip inflation against a bomb; outbound requests enforce HTTPS and use the sanctioned \curl wrapper with the egress security helper enabled; and a Privacy API metadata provider (with unit tests) declares the external transmission of personal data. The single ceiling-setting issue is a medium hardening gap: the push and self-registration flows send a shared secret, enrollment key, and web-service token over HTTPS through \curl without enabling CURLOPT_SSL_VERIFYPEER, relying on core's weak shipped default (peer-chain verification off). Exploiting it requires an active man-in-the-middle on the network path between the Moodle server and an admin-configured dashboard — a position no Moodle role grants — and HTTPS is already enforced so nothing travels in cleartext, which meaningfully constrains the risk. The remaining items are low/informational: best-effort (name-pattern) secret redaction in the config collectors, one direct insert into the core external_services_users table where a core API exists, a missing db/uninstall.php that leaves the manifest artefacts under dataroot orphaned, and an admin help string that overstates at-rest protection. No high or critical issues, no unauthenticated exposure, and no path by which a low-privilege user can reach other users' data.

AI Summary

local_sentinel is a mature, well-tested fleet-monitoring plugin that turns a Moodle site into a data source for a central dashboard, over both authenticated web services (pull) and an optional outbound push.

Security posture is strong across the board:

  • Access control — a shared external\base class enforces local/sentinel:view on every read endpoint and local/sentinel:manage on the two write endpoints, validated against the system context. Admin pages use admin_externalpage_setup() (site-config capability) and every POST action calls require_sesskey().
  • Output & queries — the Overview renderer escapes all dynamic values with s(); every DB query uses parameter placeholders or get_in_or_equal; engine-specific size queries are dispatched per dbtype with graceful fallbacks.
  • Untrusted input — the manifest ingest (set_manifest) validates the version format, base64, a hash_equals SHA-256 digest, and the first line shape, and caps gzip inflation at 32 MB to defeat decompression bombs. Provisioning codes are parsed strictly and HTTPS-only.
  • Privacy — a metadata provider declares the external transmission of the personal data involved, and its fields are asserted against lang strings in tests. No local user tables, so no export/delete providers are needed.

Findings:

  1. Medium — TLS peer verification not enabled on outbound push/registration. Both push_snapshot and register send a shared secret, enrollment key, and web-service token via \curl without setting CURLOPT_SSL_VERIFYPEER, inheriting core's 0 default. HTTPS is enforced (no cleartext), but an active network MITM could present a self-signed certificate with a matching hostname and intercept the credentials. Requires a network position, not a Moodle role.
  2. Low — best-effort secret redaction. config_drift/config_changes redact secrets by setting class and name substrings; a secret held in a plain-text setting under an unrecognised name could be forwarded to the dashboard. Heavily mitigated by the capability gate, the egress filter, and documentation.
  3. Low — direct insert into core external_services_users in the setup helper, where webservice::add_ws_authorised_user() exists.
  4. Low — no db/uninstall.php; the ~MB manifest artefacts under $CFG->dataroot/local_sentinel/ are orphaned on uninstall.
  5. Info — misleading help text: the enrollment key is described as "Stored encrypted by Moodle" but admin_setting_configpasswordunmask stores plaintext (UI-masked only).

Overall this is a carefully written plugin; the outbound-TLS default is the one item worth fixing with a one-line option change.

Findings

securityMedium
Outbound push and self-registration do not enable TLS peer verification (rely on \curl's insecure default)

Both outbound HTTP paths — the push_snapshot scheduled task and register::run() — build their client with new \curl(['ignoresecurity' => false]) and never set CURLOPT_SSL_VERIFYPEER.

Moodle core's \curl::resetopt() initialises every instance with CURLOPT_SSL_VERIFYPEER = 0 and CURLOPT_SSL_VERIFYHOST = 2. That means the certificate chain is not validated against a CA — only that the presented certificate's hostname matches. Hostname matching alone does not stop an active man-in-the-middle: an attacker on the network path can present a self-signed certificate carrying the correct (publicly known) dashboard hostname and both checks pass.

The requests carry material that is sensitive in transit:

  • push_snapshot sends the X-Sentinel-Secret shared secret plus the full snapshot (which can include DB host, hostnames, usernames, admin activity, config values).
  • register sends the X-Sentinel-Enrollment-Key, the generated push_secret, and a web-service token that grants read access to every Sentinel endpoint for the site.

To be clear, the author did not disable security — this is a gap left by relying on core's weak shipped default. The plugin already does the right things elsewhere: it enforces HTTPS (refusing http://) and keeps the core egress security helper active (ignoresecurity => false). The remaining hardening step is to opt into peer verification.

Risk Assessment

Medium risk. The attacker is a network position on the server-to-dashboard path, not any Moodle role — so exploitableBy is null and the pattern alone cannot be critical. Two factors hold it below high: the flow is HTTPS-only (so this is a peer-authentication gap, not cleartext transmission), and the weakness is inherited from a core-shipped default rather than an author decision to disable verification. Two factors keep it above low: the payload is auth-critical (a WS token that unlocks the full monitoring API for the site, plus the push/enrollment secrets), and a successful MITM yields both credential capture and the ability to forge the dashboard's registration response. In practice most fleet operators run the dashboard on a network they trust, which further limits real-world exposure. The fix is a one-line setopt enabling CURLOPT_SSL_VERIFYPEER.

Context

push_snapshot::execute() reads the admin-configured endpoint/secret, refuses non-HTTPS, records an attempt, builds the egress-filtered snapshot, and POSTs it. register::run() similarly refuses non-HTTPS, persists the endpoint/secret, mints a WS token via the setup helper, and POSTs the identity+credentials payload. Both go through the sanctioned \curl wrapper with the egress security helper enabled, so the destination host itself is subject to core's blocked-host/allowed-port checks — but that helper governs where the request may go, not whether the TLS peer is authenticated. The endpoints are configured only by a site administrator (PARAM_URL settings, or an HTTPS-validated provisioning code), so there is no lower-privilege SSRF angle; the exposure is purely in-transit interception.

Proof of Concept

On the network path between the Moodle server and the configured dashboard, an attacker terminates the TLS connection with a self-signed certificate whose CN/SAN is set to the dashboard hostname (public, taken from dashboardbaseurl/pushendpoint). Because CURLOPT_SSL_VERIFYPEER is 0, libcurl does not validate the chain; CURLOPT_SSL_VERIFYHOST=2 is satisfied by the matching name. The next push_snapshot/register run completes the handshake to the attacker, disclosing X-Sentinel-Secret / X-Sentinel-Enrollment-Key / push_secret / ws_token and the snapshot body.

Affected Code
        $curl = new \curl(['ignoresecurity' => false]);
        $curl->setHeader([
            'Content-Type: application/json',
            'X-Sentinel-Secret: ' . $secret,
            'X-Fleetmonitor-Site: ' . $snapshot['site']['siteidentifier'],
        ]);
        $response = $curl->post($endpoint, $body);
Suggested Fix

Enable peer verification explicitly on the instance (host verification is already on by default):

$curl = new \curl(['ignoresecurity' => false]);
$curl->setopt([
    'CURLOPT_SSL_VERIFYPEER' => 1,
    'CURLOPT_SSL_VERIFYHOST' => 2,
]);

This is a one-line hardening change against endpoints that already present valid certificates (the flow is HTTPS-only). Do not switch HTTP clients — \curl is the correct wrapper here.

Affected Code
        $curl = new \curl(['ignoresecurity' => false]);
        $curl->setHeader([
            'Content-Type: application/json',
            'X-Sentinel-Enrollment-Key: ' . $enrollmentkey,
        ]);
        $response = $curl->post($base . '/api/register/', json_encode($payload));
Suggested Fix

Set CURLOPT_SSL_VERIFYPEER => 1 (and CURLOPT_SSL_VERIFYHOST => 2) on the \curl instance before the post(), as shown for push_snapshot. The registration payload carries a push secret and a web-service token, so peer verification matters most here.

securityLow
Secret redaction in config drift / config changes is best-effort (name-pattern based)
Exploitable by:
manager

The config_drift and config_changes collectors export setting values to the dashboard, redacting secrets by two heuristics: the setting's admin class (admin_setting_configpasswordunmask*) and a fixed list of name substrings (pass, secret, token, apikey, salt, credential, webhook, …).

This is genuinely defence-in-depth and the plugin documents it as best-effort. The residual gap: a secret stored in a plain-text setting (e.g. admin_setting_configtext) under a name that matches none of the patterns — for example a third-party plugin keeping an API credential under a name like license, serialkey, or endpoint_auth — would be exported verbatim to the dashboard (config drift ships the current value; config changes ships the mdl_config_log value).

The surrounding controls make this low, not higher: reaching the data requires the local/sentinel:view capability (manager archetype), the whole slice and individual sub-fields are admin-excludable through the egress filter, and the recipient is an admin-configured dashboard. The point of raising it is that the dashboard side should treat received configuration as potentially secret-bearing, and the maintainer may wish to expand the pattern list or invert to an allow-list for high-risk components.

Risk Assessment

Low risk. This is the plugin's intended, documented data-export behaviour with several mitigations layered on: the local/sentinel:view capability gate (manager archetype and, in practice, a restricted service token), the per-slice and per-field egress filter, and password-class/name-pattern redaction. The realistic recipient of any missed secret is the authorized dashboard, not an arbitrary attacker; the mild escalation angle is that a manager holding the capability could read a value beyond their normal UI reach. Impact is bounded to disclosure of a configuration value the site operator chose to monitor. Worth noting for defence-in-depth, not a direct vulnerability.

Context

config_drift::collect() temporarily elevates the current user to a site admin (via \core\session\manager::set_user(), reverted in a finally block) so admin_get_root() fully populates the settings tree, then walks every admin_settingpage comparing each setting to its declared default. That elevation is bounded and takes no user input, but it does mean the collector reads settings a non-admin caller could not see in the UI — which is by design, since exposing drift is the function's purpose. The collected values pass through the redaction heuristic before leaving the site via the WS response or the push payload.

Identified Code
    private const SENSITIVE_NAME_PATTERNS = [
        'pass',
        'secret',
        'token',
        'apikey',
        'api_key',
        'privatekey',
        'private_key',
        'salt',
        'clientkey',
        'client_secret',
        'credential',
        'webhook',
    ];
Suggested Fix

Consider augmenting the heuristic, e.g.:

  • Add commonly-seen secret-bearing tokens (key, license, dsn, sas, signature, bearer) while accepting some over-redaction, since a withheld non-secret is safer than a leaked secret.
  • Document clearly (already partly done) that redaction is best-effort so dashboard operators treat all exported config as sensitive.
  • Optionally, for config_drift, skip any setting whose current value has high entropy / length characteristic of a credential.

No change is required for correctness — this is a data-minimisation improvement.

Identified Code
            $sensitive = config_drift::name_is_sensitive((string) $row->name);
            $entries[] = [
                'id' => (int) $row->id,
                'time' => (int) $row->timemodified,
                'userid' => (int) $row->userid,
                'username' => $row->username,
                'plugin' => $row->plugin,
                'name' => $row->name,
                'oldvalue' => $sensitive ? self::REDACTED : $row->oldvalue,
                'newvalue' => $sensitive ? self::REDACTED : $row->value,
            ];
Suggested Fix

Same heuristic applies. Note that password-class settings are already double-protected here because admin_setting_configpasswordunmask masks their value in mdl_config_log (********); the residual risk is only for secrets stored in plain-text settings under unrecognised names.

code qualityLow
Direct insert into core external_services_users instead of the core API

The setup helper adds the service account to the Sentinel external service with a raw $DB->insert_record('external_services_users', …) into a core-owned table. Core exposes webservice::add_ws_authorised_user() (in webservice/lib.php) for exactly this, which centralises the row shape and any future side effects.

The operation is idempotent (guarded by a prior record_exists), well-formed, and only reachable from admin-run setup (the CLI script or the site-config-gated admin page / registration flow), so the practical risk is negligible. It is flagged as a maintainability/robustness point: writing directly to a table the plugin does not own can silently drift if core changes the schema or adds bookkeeping around service membership.

Risk Assessment

Low risk. No security impact — the code path is admin/CLI-only and idempotent, and the token itself is generated through the sanctioned \core_external\util::generate_token(). This is a code-quality note about coupling to a core table's schema where a maintained API exists.

Context

helper::run() is the idempotent bootstrap invoked by cli/setup.php, the setup.php admin page (site-config capability), and register::ensure_ws_token(). It enables web services + REST, creates/reuses the role and service user via proper APIs (create_role, assign_capability, user_create_user, role_assign, \core_external\util::generate_token), and the only direct core-table write is this service-membership insert.

Identified Code
        if (!$inservice) {
            $DB->insert_record('external_services_users', (object) [
                'externalserviceid' => $service->id,
                'userid' => $user->id,
                'iprestriction' => '',
                'validuntil' => 0,
                'timecreated' => time(),
            ]);
            $result->steps[] = 'Added user to the service.';
        } else {
Suggested Fix

Prefer the core API:

require_once($CFG->dirroot . '/webservice/lib.php');
$webservicemanager = new \webservice();
if (!$DB->record_exists('external_services_users', [
        'externalserviceid' => $service->id, 'userid' => $user->id])) {
    $adduser = (object) [
        'externalserviceid' => $service->id,
        'userid' => $user->id,
        'iprestriction' => '',
        'validuntil' => 0,
    ];
    $webservicemanager->add_ws_authorised_user($adduser);
}

Confirm the exact expected object shape against the core method before switching.

code qualityLow
No db/uninstall.php — integrity manifest artefacts under dataroot are orphaned on uninstall

The plugin has no db/uninstall.php, yet it persists data outside its own database tables that will not be cleaned up when the plugin is removed:

  • $CFG->dataroot/local_sentinel/manifest.txt (~1.3–3 MB), manifest_meta.json, and scan_result.json, written by manifest_store.
  • The provisioned sentinel role and service user created by the setup helper (the external service and its tokens are removed automatically when the plugin's db/services.php definition is uninstalled, but the role and user are not).

Moodle removes config_plugins rows automatically, so the JSON state blobs are handled, but the dataroot files remain as dead weight after uninstall. manifest_store::reset() already exists to delete exactly these files.

Risk Assessment

Low risk. No security impact — this is housekeeping. On uninstall a few megabytes of manifest data are left in dataroot, and the dedicated role/user remain. The plugin already has the helper needed to clean the files; adding the uninstall hook makes removal tidy.

Context

The integrity feature stores its reference manifest and last scan result as files under a plugin-specific dataroot subdirectory (an established pattern for plugin data too large for config_plugins). Compact summary state lives in config_plugins via integrity_state/push_state/registration_state, which Moodle purges on uninstall; the larger files do not have an equivalent automatic cleanup.

Identified Code
    public static function dir(): string {
        global $CFG;
        $dir = $CFG->dataroot . '/local_sentinel';
        make_writable_directory($dir);
        return $dir;
    }
Suggested Fix

Add db/uninstall.php:

<?php
defined('MOODLE_INTERNAL') || die();

function xmldb_local_sentinel_uninstall() {
    \local_sentinel\manifest_store::reset();
    // Optionally remove the leftover local_sentinel/ directory itself,
    // and consider whether the provisioned service role/user should be
    // removed or intentionally retained for audit continuity.
    return true;
}

Using the storage location under a plugin-owned dataroot subdirectory is an accepted pattern; the gap is only the missing cleanup hook.

best practiceInfo
Enrollment-key help text overstates at-rest protection ("Stored encrypted by Moodle")

The enrollmentkey setting help text tells the administrator the value is "Stored encrypted by Moodle." The setting uses admin_setting_configpasswordunmask, which extends admin_setting_configtext: it masks the value in the UI and in mdl_config_log, but writes the raw value to mdl_config_plugins in plaintext. Encryption at rest would require admin_setting_encryptedpassword.

The same masking applies to pushsecret (whose description does not make the encryption claim). This is not a vulnerability — it matches how Moodle stores the vast majority of such settings — but the wording could give an operator false confidence about database-at-rest exposure of the enrollment key.

Risk Assessment

Informational. No functional or security impact — the storage behaviour is identical to standard Moodle password-style settings. The note is purely about accuracy of an administrator-facing security statement.

Context

settings.php registers enrollmentkey and pushsecret as admin_setting_configpasswordunmask fields. These correctly mask the value in the admin form and prevent it from appearing verbatim in the config-change log, which is the relevant protection for the config_changes collector; the only inaccuracy is the documentation's encryption claim.

Identified Code
$string['enrollmentkey_desc'] = 'The shared enrollment key issued by the dashboard operator. '
    . 'Sent as the X-Sentinel-Enrollment-Key header when registering. Stored encrypted by Moodle.';
Suggested Fix

Correct the claim, e.g.:

$string['enrollmentkey_desc'] = 'The shared enrollment key issued by the dashboard operator. '
    . 'Sent as the X-Sentinel-Enrollment-Key header when registering. Masked in the UI; '
    . 'stored in the site configuration.';

Alternatively, if at-rest encryption is genuinely wanted, switch the setting to admin_setting_encryptedpassword.

Additional AI Notes

Strong security fundamentals. Every external function routes through \local_sentinel\external\base::authorise() / authorise_manage(), which validate the system context and require local/sentinel:view or local/sentinel:manage; the write endpoints (set_manifest, request_integrity_scan) are correctly separated onto the higher-risk :manage capability. This is verified by unit tests (test_authorisation_required, test_write_functions_require_manage_capability).

Untrusted manifest handling is defensive. set_manifest validates the version string, base64 decodes, gzip-inflates with a 32 MB cap (gzip-bomb defence), verifies a SHA-256 digest with hash_equals, and checks the first line's shape before storing. The integrity scanner only ever hashes on-disk files whose relative path already appears in the manifest, reports unexpected files by path only (never hashing them), and treats symlinks git-style (hashing the target string, not following them) — so a poisoned manifest cannot drive arbitrary file reads or content exfiltration.

Redirect header forwarding is not a concern here. The push/register \curl calls follow redirects with custom credential headers attached, but the first-hop host is the admin-configured dashboard that is the intended recipient of those very credentials, so a redirect cannot expose them to a party that is not already trusted with them.

Privacy API is implemented correctly. The plugin stores no personal data locally, so it declares only external-location transmissions via add_external_location_link() and omits the export/delete providers — the right choice. A test asserts every declared field resolves to a lang string, guarding against drift as collectors gain fields.

The config_drift collector briefly impersonates a site admin (\core\session\manager::set_user(), restored in a finally) to populate the full settings tree under a web-service/cron context. It takes no user input and is reverted, so it is not exploitable, but it is an unusual pattern worth keeping in mind if the walk ever gains side effects — those would run with elevated privileges.

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

Published reviews of this plugin

2026-09-152.22.4CurrentA2026-09-142.22.4A
2026-09-142.21.1B+