This is an older published review of this plugin. View the latest review →
local_sentinel ("Sentinel") is a per-instance operational-monitoring plugin. It collects a Moodle site's health, environment, plugin inventory, authentication signals, recent config changes/drift, core-file integrity, and core check-API results into one versioned JSON snapshot, and exposes it to a central fleet dashboard by read-only web-service pull endpoints and/or an optional outbound scheduled push. It also provisions its own scoped web-service role, service user and token (via CLI and an admin UI), offers a local admin Overview report, and can compare the on-disk code tree against a dashboard-provided pristine manifest. An egress filter lets admins withhold whole slices or individual fields.
The plugin is exceptionally well built from a security standpoint, and no exploitable vulnerabilities were found. Every web-service function calls validate_context() and require_capability() (view functions gate on local/sentinel:view, write functions on local/sentinel:manage), and the service is restrictedusers=1. All three admin pages use admin_externalpage_setup() (verified in core to enforce require_login() + check_access() for moodle/site:config) and every state-changing POST/GET handler calls require_sesskey(). Output is escaped throughout the renderer with s(), check summaries are converted to plain text with html_to_text() before display, and notification messages are sanitised by core's clean_text(). All database access uses parameterised $DB methods; the only engine-specific SQL (DB size / largest tables) is correctly dispatched on $CFG->dbtype with graceful fallbacks. The two outbound HTTP paths (push + self-registration) use Moodle's \curl wrapper with ignoresecurity=false (so core's curl_security_helper blocklist applies), enforce HTTPS, and explicitly override the wrapper's unsafe CURLOPT_SSL_VERIFYPEER=0 default to true with CURLOPT_SSL_VERIFYHOST=2 — verified against curl::resetopt() in core. The integrity scanner reads only the code tree (permitted) and stores artefacts under a fixed $CFG->dataroot/local_sentinel/ path; the set_manifest endpoint validates base64/gzip with an inflate cap, a SHA-256 digest check, and a format check, and manifest paths are used only as lookup keys, never for file access. The Privacy API implementation is thorough (external-location declarations for both transmission paths) and test coverage is strong, including regressions for secret redaction, egress exclusion and capability enforcement. Only minor issues remain: hardcoded English UI labels in the Overview renderer and a direct write to the core user_info_data table in the setup helper (both low, code-quality). Informational observations cover the best-effort secret-redaction model, a transient admin session-elevation used to enumerate the settings tree, and an unused form class.
Overview
local_sentinel is a fleet-monitoring data source: it gathers operational facts about a Moodle site and ships them to a central dashboard by web-service pull and/or an optional scheduled push. The code is mature (release 2.22.4), well-documented, and backed by an extensive PHPUnit suite.
Security posture
The security-relevant paths are handled correctly and defensively:
- Authorisation — Every external function calls
validate_context()+require_capability()via the sharedbase::authorise()/authorise_manage()helpers. Read functions requirelocal/sentinel:view; the two write functions (set_manifest,request_integrity_scan) requirelocal/sentinel:manage. The service isrestrictedusers=1. - Admin pages —
overview.php,alerts.php,connect.phpall useadmin_externalpage_setup()(confirmed in core to enforcerequire_login()and amoodle/site:configaccess check), and every state-changing action callsrequire_sesskey(). - Output encoding — The renderer wraps values in
s(); check summaries pass throughhtml_to_text(); notification bodies are sanitised by coreclean_text(). No unescaped user input reaches the page. - SQL — All queries are parameterised. Engine-specific size queries dispatch on
$CFG->dbtypewith fallbacks and safely escape the table prefix forLIKE. - Outbound HTTP — Push and registration use
\curl(['ignoresecurity' => false])(core egress blocklist active), enforce HTTPS, and override the wrapper's unsafeCURLOPT_SSL_VERIFYPEER=0default totrue. Endpoints are admin-configured only. - Filesystem / integrity — Reads the code tree (allowed); stores artefacts under a fixed
$CFG->dataroot/local_sentinel/path;set_manifestvalidates gzip size, SHA-256 digest and format; manifest paths are used only as array keys. - Privacy — A complete metadata provider declares both external transmissions. Secrets are redacted best-effort in
config_drift/config_changes.
Findings
No exploitable vulnerabilities were identified. The reported items are low code-quality points (hardcoded UI strings; a direct user_info_data write) and informational observations (best-effort secret redaction; a transient admin session-elevation; an unused form class).
Findings
The Overview renderer emits a number of user-facing labels as hardcoded English literals instead of resolving them through get_string() from the language pack. This runs against Moodle's localisation guidelines (the String API), which expect strings shown to users to live in lang/ files so they can be translated and overridden.
Affected labels include the auth-tab table rows (Total tokens, Without IP restriction, Never used, Active in last 7 days, Stale (>90 days), Expiring within 30 days, Total failed (across all accounts), Accounts with failures, Currently locked accounts), the overdue-tasks detail table (Task, Last run, Scheduled for, Late by, Show overdue tasks, never, late), and the active-users line (DAU %d · WAU %d · MAU %d). A small copy-to-clipboard confirmation in connect.php also hardcodes Copied! in inline JavaScript.
This is inconsistent with the rest of the plugin, which ships a thorough lang/en/local_sentinel.php. It is a maintainability / internationalisation gap, not a security issue — all of these values are static text, and the surrounding dynamic values are still escaped with s().
Low risk. This is a code-quality / internationalisation issue with no security impact. The affected pages are reachable only by site administrators (moodle/site:config), and the hardcoded text is not user-controlled. The practical consequence is that these labels cannot be translated or overridden through Moodle's language customisation, and they are inconsistent with the plugin's otherwise complete language file.
These strings render on the Overview admin report (overview.php tabs, rendered by classes/output/renderer.php) and on the Connect page's token copy button. All are static text; the dynamic values interpolated alongside them are integers or already-escaped strings, so there is no injection concern — only the loss of translatability.
$rows = $this->kv_row('Total tokens', (int) ($tokens['total_count'] ?? 0));
$without = (int) ($tokens['without_ip_restriction'] ?? 0);
$rows .= $this->kv_row(
'Without IP restriction',
Move each label into lang/en/local_sentinel.php and resolve with get_string(), e.g. $this->kv_row(get_string('overview_tokens_total', 'local_sentinel'), ...).
html_writer::tag('th', 'Task', ['class' => 'small'])
. html_writer::tag('th', 'Last run', ['class' => 'small'])
. html_writer::tag('th', 'Scheduled for', ['class' => 'small'])
. html_writer::tag('th', 'Late by', ['class' => 'small'])
Replace the literal column headers (and the Show overdue tasks summary, never, and late fragments in the same method) with get_string() calls backed by new language strings.
sprintf(
'DAU %d · WAU %d · MAU %d',
Use a language string with placeholders, e.g. get_string('overview_active_users_counts', 'local_sentinel', (object) ['dau' => ..., 'wau' => ..., 'mau' => ...]).
const orig = btn.textContent;
btn.textContent = 'Copied!';
Pass the label into the inline JS from PHP via a get_string('setup_copied', 'local_sentinel') value, or use the AMD core/str API for the clipboard confirmation.
The config_drift and config_changes collectors expose Moodle setting names and values to holders of local/sentinel:view. To avoid leaking credentials, they suppress settings judged sensitive by (a) admin_setting subclass (admin_setting_configpasswordunmask) and (b) a substring match of the setting name against SENSITIVE_NAME_PATTERNS (pass, secret, token, apikey, dsn, etc.).
As the code comments and README candidly note, this is best-effort: a secret stored in a plain-text setting whose name matches none of the patterns would still be shipped. config_drift in particular reports the current value of every non-secret setting that differs from its default, so the effective data exposed to a view-capability holder is close to "all non-secret site configuration."
This is called out as an informational observation, not a defect. The real access control is the capability gate plus the egress filter, and the plugin is transparent about the limitation. It is surfaced so maintainers weigh it when granting local/sentinel:view and when adding new detection patterns.
Informational. The exposure is bounded by the local/sentinel:view capability (granted to the manager archetype and to the dedicated, restrictedusers=1 service role) and by the admin-controlled egress filter, and the redaction covers the common secret-bearing setting names (including tricky cases like smtppass and LDAP bind_pw, which are unit-tested). The residual risk is a site-specific secret placed in an unrecognised plain-text setting reaching a trusted dashboard. Given the threat model (a trusted operator's dashboard) this is an accepted, documented trade-off rather than a vulnerability.
config_drift::collect() walks the full admin settings tree and records each setting whose current value differs from its declared default, excluding those matched as sensitive. config_changes::collect() tails mdl_config_log and redacts values whose setting name matches the same detector. Both slices are reachable by any holder of local/sentinel:view through get_snapshot / get_config_drift / get_config_changes, and both can be withheld entirely via the egress filter.
private const SENSITIVE_NAME_PATTERNS = [
'pass',
'pw',
'secret',
'token',
'apikey',
No change required for correctness. To reduce residual exposure you could additionally treat any setting whose stored value is very long/high-entropy or whose plugin declares it via a password-class element as sensitive, and continue expanding the pattern list as new credential-bearing settings appear. Keep the capability grant narrow.
$sensitive = config_drift::name_is_sensitive((string) $row->name);
No change required. This correctly reuses the shared name-based detector so both slices redact consistently.
To populate the full settings tree, config_drift::collect() calls elevate_to_admin(), which — when the current user is not already a site admin — swaps the global session user to the first site administrator via \core\session\manager::set_user(reset($admins)), runs admin_get_root(true, true), walks the tree, then restores the original user in a finally block.
The elevation is required because admin_get_root() only fully populates category children under an account that passes each node's access check; a low-privilege web-service token user would otherwise see an empty tree. It is an implementation mechanism, not a privilege grant: the data the walk produces (non-secret setting drift) is exactly what the local/sentinel:view feature is designed to expose, secrets are still filtered, and the swap is reversed before the collector returns.
This is flagged as an architectural observation because transiently reassigning the global $USER inside a stateless web-service request is an unusual pattern worth a maintainer's awareness, even though it was verified to be safe here.
Informational. No privilege escalation or extra data exposure results: the elevated read yields the same values the feature already returns (minus secrets), and the original user is restored deterministically. A non-catchable fatal between set_user() and the finally would end the request as admin, but that only aborts the request and cannot be leveraged by the caller. Surfaced so the unusual session-user swap is understood and preserved carefully in future changes.
config_drift::collect() runs on every get_snapshot / get_config_drift pull and on every scheduled push. In the web-service path the request is authenticated as the low-privilege service token user; the collector elevates to admin only to enumerate settings, then restores. Because it is verified in core that set_user() mutates only in-request globals ($GLOBALS['USER'], $_SESSION['USER'], $ADMIN) and does not persist to the session store, and Moodle web-service requests are stateless, the swap does not leak beyond the request. Other collectors run before/after and are unaffected because the elevation is scoped inside config_drift and unwound in finally.
protected static function elevate_to_admin(): ?\stdClass {
global $USER;
if (!empty($USER->id) && is_siteadmin($USER)) {
return null;
}
$admins = get_admins();
if (empty($admins)) {
return null;
}
$previous = $USER;
\core\session\manager::set_user(reset($admins));
return $previous;
}
The current approach is safe (restore runs in finally; the elevation is skipped when already admin; output is filtered). If a more surgical alternative is desired, consider reading defaults/current values without a full admin_get_root() session swap, or restrict the elevated section as tightly as possible. At minimum, keep the try/finally restore and the is_siteadmin short-circuit intact.
When provisioning the web-service service account, helper::satisfy_required_profile_fields() fills any empty required custom profile field with a placeholder by calling $DB->insert_record('user_info_data', ...) / $DB->update_record('user_info_data', ...) directly, rather than going through the profile-field API (e.g. profile_save_data() or the field object's own save routine).
user_info_data is a core table the plugin does not own, and writing to it directly bypasses the per-field-type save logic that the API applies (value normalisation, format handling). The code mitigates this with a post-write user_not_fully_set_up() safety check that surfaces a warning if a field type rejects the placeholder, so functional risk is contained — but the direct write remains a minor deviation from the intended API.
Low risk. No security impact and no user-facing exposure: the operation is admin-only, writes a fixed placeholder to the service account's own profile rows, and is followed by a validation check. It is reported purely as a code-quality point — a direct write to a core table where a supported API exists.
This runs only during setup of the dedicated sentinel web-service account, triggered from cli/setup.php or the admin Connect page (both require moodle/site:config). Its purpose is to prevent the service account from being rejected at web-service auth time with usernotfullysetup on sites that define required custom profile fields. The placeholder value is immaterial because the account never uses the site UI.
} else {
$DB->insert_record('user_info_data', (object) [
'userid' => $userid,
'fieldid' => $fieldid,
'data' => self::PROFILE_PLACEHOLDER,
'dataformat' => FORMAT_MOODLE,
]);
}
Prefer the profile-field API. For example build a data object with the profile_field_<shortname> property and call profile_save_data($data), or use the field instance's save method, so field-type-specific handling runs. If the surgical direct write is retained deliberately, keep the existing user_not_fully_set_up() safety check.
The class local_sentinel\form\setup_form (a moodleform) is defined but never instantiated anywhere in the plugin. The token-minting workflow it was written for now lives in connect.php, which renders its own HTML forms with manual sesskey handling; setup.php is a redirect to connect.php. The form class is leftover code from the earlier hidden setup page.
This is harmless (it has no side effects and is never reached) but it can mislead maintainers into thinking the plugin routes setup through a moodleform. Removing it, or wiring the Connect/setup flow to use it, would keep the codebase self-consistent.
Informational. No functional or security impact — unreferenced code. Noted for cleanliness only.
A repository-wide search for setup_form matches only its own definition. connect.php implements the mint-token, provisioning-code, push-config and transport handlers as bespoke <form> markup with explicit require_sesskey() calls, so the moodleform subclass is orphaned.
class setup_form extends \moodleform {
Delete classes/form/setup_form.php if the raw forms on connect.php are the intended UI, or refactor connect.php's mint-token handler to instantiate setup_form (which would also inherit moodleform's automatic sesskey/CSRF handling).
No third-party code is bundled. The plugin ships no vendor directories, minified libraries, JavaScript files or Mustache templates (the only client-side script is a small inline clipboard helper in connect.php). A thirdpartylibs.xml is therefore not required, and there is no duplication of libraries already shipped by Moodle core.
Outbound TLS is configured correctly — do not mistake this for a hardening gap. Core's curl::resetopt() ships CURLOPT_SSL_VERIFYPEER = 0. Both task\push_snapshot::execute() and register::run() explicitly override this with CURLOPT_SSL_VERIFYPEER => true and CURLOPT_SSL_VERIFYHOST => 2, enforce an https:// scheme before sending, set a request timeout, and construct the client with ignoresecurity => false so core's curl_security_helper blocklist still applies. Endpoints are admin-configured only. This is exemplary handling of the credential-bearing egress path.
The SSL-certificate collector's verify_peer => false is intentional and appropriate. environment::collect_ssl() opens a raw stream_socket_client('ssl://<host>:<port>') purely to read the certificate presented by the site's own $CFG->wwwroot host and report its expiry. Peer verification is deliberately disabled because the goal is to inspect whatever certificate is presented (including expiring/self-signed ones), the destination is the site's own admin-fixed hostname (not user-influenced, so not SSRF), and no credentials or payload are transmitted. There is no Moodle API for certificate inspection, so a raw socket is the only option here.
local/sentinel:view is a broad read capability. Through the snapshot it exposes site-configuration drift/changes, admin usernames and last-login times, failed-login target accounts, and web-service token metadata. It is gated by the capability (manager archetype + the dedicated restrictedusers=1 service role) and the admin-controlled egress filter, but it should be granted sparingly and treated as roughly equivalent to "read non-secret site configuration and admin/security telemetry."
Test coverage and Privacy API are strong. The suite exercises capability enforcement, egress inclusion/exclusion (including a regression that previously leaked the config log), manifest validation (bad base64, digest mismatch, non-manifest text, bad version, gzip inflate cap), secret-name redaction, and setup/teardown ownership markers. The privacy metadata provider declares both external transmissions and its tests assert every declared field resolves to a language string.