Per-instance Moodle monitoring plugin that exposes operational health, environment, plugin inventory, authentication, config changes/drift, and core-file integrity data for a site. Data is surfaced through capability-gated web-service pull endpoints, an optional scheduled outbound push to a central dashboard, and a local admin Overview report. Also provides one-paste provisioning-code onboarding and a CLI/GUI web-service bootstrap.
The plugin demonstrates a consistently strong security posture with no exploitable vulnerabilities identified across the full source tree.
Access control is enforced correctly everywhere: every web-service function calls require_capability (via authorise() / authorise_manage()) before doing any work, the write functions require a distinct local/sentinel:manage capability on a restrictedusers service, and all three admin pages use admin_externalpage_setup (which enforces require_login + moodle/site:config) with require_sesskey() guarding every state-changing POST.
Data-handling is sound: all SQL uses parameterized $DB methods or get_in_or_equal, all rendered output is escaped with s(), engine-specific queries are dispatched on dbtype, and secret-bearing config values are redacted (best-effort by class + name pattern) before egress. The set_manifest endpoint validates the version format, verifies a SHA-256 digest with hash_equals, and caps gzip inflation to defend against decompression bombs.
Outbound HTTP is handled well: registration and push use the core \curl wrapper, hard-require HTTPS, and explicitly set CURLOPT_SSL_VERIFYPEER => true (overriding the wrapper's unsafe default), with the target URL controlled only by a site administrator and the core curl security helper left active.
The plugin also ships a correct Privacy API implementation (declaring external transmission of personal data), a db/uninstall.php that cleans up dataroot artefacts and provisioned accounts, and a comprehensive PHPUnit suite that pins the security-relevant behaviours.
The only issues found are minor code-quality items — hardcoded English UI labels in the renderer and one direct write to the core user_info_data table instead of the profile API — plus informational observations (a tightly-scoped, non-exploitable admin elevation inside the config-drift collector, and an unused form class). None represent a security risk or reach beyond trusted, capability-gated contexts.
local_sentinel is a site-monitoring plugin: it collects a rich operational snapshot (status, environment, plugins, health, auth, reports, config changes/drift, core-file integrity) and makes it available to a central dashboard via capability-gated web-service pull and an optional scheduled outbound push, plus a local admin Overview report.
The review found no security vulnerabilities. The codebase is defensively written throughout:
- Every external function enforces
require_capabilitybefore collecting data; write functions require a separatelocal/sentinel:managecapability on arestrictedusersservice. - Every admin page uses
admin_externalpage_setup(login +moodle/site:config) and guards each POST withrequire_sesskey(). - All SQL is parameterized; all output is escaped with
s(); engine-specific SQL is dispatched ondbtype. - Outbound requests enforce HTTPS and explicitly enable TLS peer/host verification (overriding the
\curlwrapper's off-by-defaultCURLOPT_SSL_VERIFYPEER); the URL is admin-controlled and the core curl security helper stays active. - Secrets are redacted best-effort before egress; the manifest upload validates a digest with
hash_equalsand caps gzip inflation against bombs. - A proper Privacy API provider,
db/uninstall.phpcleanup, and an extensive PHPUnit suite are all present.
The findings are limited to low / informational code-quality items:
- Hardcoded English UI strings in the Overview renderer (should use
get_string()). - An informational note on the config-drift collector temporarily elevating
$USERto a site admin (tightly scoped, restored infinally, not exploitable). - A direct write to the core
user_info_datatable in the setup helper instead of the profile API. - An unused
setup_formclass (dead code).
Findings
The Overview tab renderer emits numerous user-facing labels as literal English strings rather than resolving them through get_string() against the plugin's language file. Examples include the environment/database/OPcache/SSL row labels (Version, SAPI, Host, Name, Prefix, Total size, Status, Cached scripts, Memory used, Hit rate, Issuer, Subject CN, etc.), the token summary labels (Total tokens, Without IP restriction, Never used, Active in last 7 days, Stale (>90 days), Expiring within 30 days), the active-users line (DAU %d · WAU %d · MAU %d), and the entire overdue-tasks detail table (Task, Last run, Scheduled for, Late by, Show overdue tasks, never).
Moodle's localisation guidelines require all user-visible text to be defined in lang/en/local_sentinel.php and retrieved via get_string(), so that the interface can be translated (AMOS) and remains consistent. The plugin already maintains a thorough language file for the rest of the UI, so these strings are an inconsistency rather than a technical constraint.
This is a maintainability / internationalisation issue only — there is no security impact, since every value is either a static label or a numeric/s()-escaped value.
Low risk. This is a localisation / code-quality defect with no security dimension: the hardcoded text is static and the dynamic values interpolated alongside it are numeric or escaped with s(). The impact is that these labels cannot be translated and are invisible to AMOS, which is inconsistent with the plugin's otherwise complete language file. Blast radius is limited to the admin-only Overview page.
The renderer builds the Overview admin page (a report under Site administration → Reports). The page itself is admin-only. The strings in question are static labels in key/value tables and the overdue-task detail table. Unlike the rest of the plugin's UI — which consistently uses get_string() with a well-maintained language file — these tabs interleave hardcoded English.
$rows = $this->kv_row('Version', s((string) ($php['version'] ?? '')))
. $this->kv_row('SAPI', s((string) ($php['sapi'] ?? '')))
. $this->kv_row('memory_limit', s((string) ($php['memory_limit'] ?? '')))
. $this->kv_row('max_execution_time', s((string) ($php['max_execution_time'] ?? '')))
Move these labels to lang/en/local_sentinel.php and resolve them with get_string(), e.g. $this->kv_row(get_string('overview_php_version', 'local_sentinel'), ...). Purely technical ini names (memory_limit, post_max_size) may be left as-is, but human labels such as Version, SAPI, and Timezone should be translatable.
$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',
Replace the literal labels with get_string() calls backed by new keys in the language file.
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'])
Define these column headers (and the Show overdue tasks summary and never fallback nearby) as language strings and retrieve them via get_string().
To make admin_get_root(true, true) populate the full settings tree (which returns empty category children without an admin session), the config_drift collector swaps the current session user for a site administrator via \core\session\manager::set_user(reset($admins)), walks every setting reading get_setting() / get_defaultsetting(), then restores the previous user.
This collector is reachable from the local_sentinel_get_config_drift and local_sentinel_get_snapshot web-service functions, which require only the local/sentinel:view capability — a lower privilege than the site administrator the code briefly assumes. It also runs from the push scheduled task and the admin Overview page.
After close analysis this is not exploitable: the elevated section performs only read operations on the settings tree, no request parameter flows into it, the original user is restored in a finally block, and the data returned is exactly the function's intended output (drifted settings, with secrets excluded). It is raised here as an informational observation because concentrating a full-admin set_user() inside a handler reachable from a lower-privilege web service is a pattern worth keeping under deliberate control — any future change that performs a write, or that lets caller input influence the elevated block, could turn it into a real privilege boundary crossing.
Informational. Not exploitable as written: the elevated window is read-only, no caller-supplied input reaches it, and the previous user is always restored in a finally. The web-service caller gains nothing beyond the function's designed output, and the endpoint is already gated by local/sentinel:view on a restrictedusers service that only an administrator can grant a token for. The value of the note is defensive: a full-admin set_user() inside a handler reachable from a lower-privilege web service is a sensitive pattern that should be maintained carefully so it does not regress into a genuine escalation.
config_drift::collect() calls elevate_to_admin(), runs admin_get_root(true, true) inside an output buffer, walks the settings tree via walk()/check_setting() (which skip password-class and secret-named settings), then calls restore_user() in a finally. The function returns a list of settings whose current value differs from the declared default. The elevation exists solely because admin_get_root() yields an unpopulated tree for non-admin sessions.
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;
}
Prefer reading defaults/current values without a full session swap where feasible. If elevation is genuinely required for admin_get_root() to populate, keep the current tight scoping and add guard rails:
- Keep the restore in a
finally(already done) and confirm no write or capability-dependent action can occur between elevate and restore. - Consider
is_siteadmin()-gating only the tree population, and reading each setting's value/default through the setting object without needing an elevated$USERfor the read step. - Add a code comment explicitly stating that no request-controlled data may be introduced into the elevated block, to protect the invariant during future edits.
When provisioning the dedicated web-service account, satisfy_required_profile_fields() fills any required custom profile field with a placeholder by inserting/updating rows directly in the core user_info_data table via $DB->insert_record() / $DB->update_record(), rather than going through the profile field API (e.g. profile_save_data() or each field's own edit_save_data()).
Direct manipulation of a core table the plugin does not own bypasses any field-type-specific save logic (data transformation, normalisation, format handling). The plugin's own comments acknowledge the resulting fragility — it adds a safety-net user_not_fully_set_up() re-check and a warning for field types whose is_empty() validates the stored value, precisely the case the field API would handle.
This runs only during setup (CLI or the admin Connect page, both requiring moodle/site:config), targets only the plugin's own service account, and writes a benign n/a placeholder — so the practical risk is negligible. It is flagged as a code-quality item because a supported core API exists for this operation.
Low risk. Not a security issue: the operation is admin-triggered, scoped to the plugin's own service account, and writes a constant placeholder. The concern is correctness/maintainability — bypassing the profile API can miss field-type save behaviour, which the author has partially compensated for with a post-write validation check. Using the supported API would be more robust and removes the direct dependency on the core table's shape.
helper::run() creates the sentinel web-service user, then calls satisfy_required_profile_fields() so a site with required custom profile fields does not reject the account with usernotfullysetup at web-service auth time. The write targets only the freshly created service account with a fixed placeholder string. Both entry points (CLI cli/setup.php and the Connect admin page) require moodle/site:config.
$existing = $DB->get_record('user_info_data', ['userid' => $userid, 'fieldid' => $fieldid]);
if ($existing) {
$existing->data = self::PROFILE_PLACEHOLDER;
$existing->dataformat = FORMAT_MOODLE;
$DB->update_record('user_info_data', $existing);
} else {
$DB->insert_record('user_info_data', (object) [
'userid' => $userid,
'fieldid' => $fieldid,
'data' => self::PROFILE_PLACEHOLDER,
'dataformat' => FORMAT_MOODLE,
]);
}
Use the profile field API so field-type save hooks run, e.g. build a user object with the required profile_field_<shortname> properties and call profile_save_data($user), or invoke the specific field object's edit_save_data(). This also removes the need for the manual dataformat handling and reduces the edge case the code already warns about.
The \local_sentinel\form\setup_form moodleform class is defined but never instantiated or referenced anywhere in the plugin. The legacy setup.php page it appears to have been written for is now a thin redirect to connect.php, and the Connect page implements the mint-token flow with a hand-built form + require_sesskey() handler rather than this class.
Dead code is a maintenance liability: it can drift out of sync with the live flow, and a future contributor may wire it up assuming it is current. There is no security impact — the class is never executed.
Informational. No runtime or security impact, since the class is never loaded during any request. The note is about maintainability: unused code should be removed or connected to the live flow so the codebase reflects what actually runs.
A repository-wide search for setup_form finds no references outside the class definition file itself. The mint-token action is instead handled inline in connect.php (the do_setup POST handler) which calls \local_sentinel\setup\helper::run() directly.
class setup_form extends \moodleform {
Either remove classes/form/setup_form.php if the hand-built form on connect.php is the intended UI, or refactor the Connect page's mint-token section to use this moodleform (which would also centralise its sesskey/validation handling). Whichever direction is chosen, avoid keeping an unreferenced form class in the tree.
Overall the plugin is a strong example of secure Moodle development. Web-service authorisation, sesskey protection on admin POST handlers, parameterized SQL, s()/format_string() output escaping, a correct Privacy API provider, a db/uninstall.php cleanup hook, and an extensive PHPUnit suite (including explicit tests that unauthorised users are refused and that write functions require local/sentinel:manage) are all present and correct.
Outbound HTTP is configured correctly. Both register::run() and the push_snapshot task use the sanctioned core \curl wrapper, hard-require an https:// scheme, and explicitly set CURLOPT_SSL_VERIFYPEER => true / CURLOPT_SSL_VERIFYHOST => 2 — deliberately overriding the wrapper's off-by-default peer verification (confirmed in core curl::resetopt(), which sets CURLOPT_SSL_VERIFYPEER = 0). The destination URL is controlled only by a site administrator, ignoresecurity is left false so the core curl security helper (blocked hosts / allowed ports) still applies, and timeouts are bounded. No SSRF or cleartext-credential exposure was found.
The SSL certificate probe in the environment collector is safe by design. collect_ssl() opens a raw stream_socket_client('ssl://…') with verify_peer => false, but it connects only to the host/port parsed from the site's own $CFG->wwwroot, transmits no data, and reads only the presented certificate's metadata (issuer, CN, validity) for expiry reporting. Disabling verification is required to inspect self-signed/expired certificates, and no user-controlled input reaches the target, so this is neither an SSRF nor a MITM-exposure concern.
Secret redaction in the config_changes and config_drift slices is best-effort by design. Detection is by admin_setting subclass plus a name-pattern list, so a secret stored in a plain-text setting under an unrecognised name could still be transmitted. This residual exposure is meaningfully mitigated: the endpoints require the local/sentinel:view capability on a restrictedusers service (token issuance is an admin action), the recipient is an admin-configured dashboard over HTTPS, and administrators can withhold the whole slice via the egress filter. The pattern list is broad (it even matches pass, pw, bind passwords, cloud access keys, DSNs, and connection strings) and is regression-tested.
Capability archetypes are reasonable and gated in depth. local/sentinel:view (RISK_PERSONAL) and local/sentinel:manage (RISK_CONFIG|RISK_DATALOSS) default to the manager archetype, but reaching the web-service functions additionally requires a token on the restrictedusers=1 Sentinel service, which only an administrator can provision — so the archetype defaults do not by themselves grant access. The RISK_DATALOSS flag on :manage is arguably broader than the actual operations (store a manifest, queue a scan), which only over-warns and is harmless.
No bundled third-party code and no thirdpartylibs.xml is needed. The tree contains no vendor directories, minified libraries, or attributed third-party files; the only JavaScript is a small inline copy-to-clipboard snippet loaded via the sanctioned $PAGE->requires->js_amd_inline(). Core-shipped libraries are not duplicated.
Manifest handling is hardened. set_manifest validates the version string format, verifies a SHA-256 digest with hash_equals(), caps gzip inflation at 32 MB against decompression bombs, and checks the first line's <sha1>\t<path> shape. Manifest paths are only ever used as lookup keys and for the 'missing' list — never to open files — so a crafted path cannot cause path traversal or arbitrary file reads during the scan. Unexpected on-disk files are reported by path only and never hashed, preventing off-site fingerprinting of stray secret-bearing files.