Agent Detection Lite
local_agentdetect
- #1Beacon endpoint does not verify the user has access to the submitted contextid
- #2Double HTML-escaping of already-formatted course/module names
- #3Report references signal fields that the Lite client never sends
- #4Unique index on nullable contextid cannot prevent duplicate NULL rows
- #5Flag type 'low_suspicion' is a magic string instead of a class constant
A Moodle local plugin (Lite edition) that detects automated browser agents (e.g. Perplexity Comet, Playwright, Puppeteer) during quiz sessions by collecting browser fingerprint signals, monitoring interaction patterns (mouse, keyboard, scroll), and computing a combined detection score that flags suspicious sessions. Provides a system-level admin report and a course-level report for teachers, plus quiz-page badges for flagged students.
The plugin shows strong overall Moodle compliance — proper use of the $DB API with parameterised queries, correct require_login/require_capability/sesskey checks on report and external endpoints, a complete Privacy API implementation (including userlist and metadata providers), backup/restore plugin classes, a comprehensive PHPUnit test suite, and use of the modern before_footer_html_generation hook system.
No critical or high-severity issues were identified. There is no SQL injection, XSS, code execution, direct filesystem access, or auth bypass. The custom beacon.php endpoint correctly enforces login and sesskey, and the external function report_signals validates context access.
The findings are limited to low/info-severity items: a minor access-control gap in beacon.php where the user's access to the submitted contextid is not validated (limited blast radius — a user can only submit signals about themselves and the impact is data pollution in the manager-only admin report), several double-escaping/dead-code issues, a non-portable nullable-column unique index, and a few code-quality observations. The combination of multiple low findings with one minor security gap places the plugin above the B threshold but below A.
Overview
local_agentdetect (Lite) is a quiz-focused agent-detection plugin that injects a browser-side detector via the before_footer_html_generation hook on mod-quiz-* pages, collects fingerprint and interaction signals, and reports them to the server via an external web service and a sendBeacon endpoint. Server-side it stores compacted signal records, manages aggregate flag records, fires custom events, and exposes admin/course reports and an AJAX-fed badge on quiz overview/review pages.
Security posture
The plugin demonstrates solid Moodle security hygiene:
- All database access goes through
$DBwith parameterised SQL — no string concatenation of user input into queries. report.phprequireslocal/agentdetect:viewsignalsat system context;coursereport.phprequireslocal/agentdetect:viewreportsat course context.- The custom
beacon.phpendpoint callsrequire_login()andconfirm_sesskey(), and validates JSON input. - The external function
report_signalscallsvalidate_context()andconfirm_sesskey();get_user_flagscallsvalidate_context()andrequire_capability(). - Privacy API is fully implemented (
provider,core_userlist_provider, metadata, export, and delete in all three forms). - Backup/restore classes scope data to the course context and map
useridcorrectly. - User-supplied JSON fields are compacted to a known shape (
compact_signal_data) with integer casts before persistence, and displayed values are passed throughs()for HTML-escaping. - A registered capability with
RISK_PERSONALis correctly applied to roles that view detection data.
Findings of note
One minor security gap and several code-quality/best-practice issues:
beacon.phpdoes not validate user access to the submittedcontextid— it only validates that the context exists. Practical impact is limited to data pollution in the manager-only admin signals report because (a) the user can only submit signals about themselves via$USER->id, (b) the course report filters by enrollment, and (c) the unique index on(userid, contextid)bounds flag growth.- Double-escaping in
local_agentdetect_format_context_link()and an attribute-levels()call — display bug, not security. - Dead code path —
report.phpreferences$data->pageUrland$data->pageTitle, but the Lite client never emits these fields aftercompact_signal_data. - Nullable-column unique index —
userid_contextidis declared UNIQUE on(userid, contextid)wherecontextidis NULL-able; standard SQL engines treat NULLs as distinct, so the constraint cannot prevent duplicate NULL-contextid rows under concurrent requests. - Hardcoded
'low_suspicion'string instead of a class constant, breaking the consistency ofFLAG_*constants used elsewhere. target="_blank"link to user-supplied URL withoutrel="noopener noreferrer"(modern browsers default to noopener, but explicit is recommended).fix-indent.jsis a one-off developer utility shipped in the plugin distribution.- Backup includes PII (
ipaddress,useragent) inside signal records — flows through course backups; worth considering for redaction.
Bottom line
This is a well-engineered plugin with comprehensive tests and proper Moodle conventions throughout. The findings are minor and primarily about polish and defensive depth, not exploitable weaknesses.
Findings
The beacon.php endpoint checks login, sesskey, and that the context exists (via context::instance_by_id($contextid, IGNORE_MISSING)), but it does not verify that the user has access to the chosen context. By contrast, the equivalent external function local_agentdetect\external\report_signals::execute() calls self::validate_context($context), which performs require_login($course, false, $cm, false, true) and enforces the global context restriction.
A logged-in user can therefore submit signals tagged with any existing contextid — including courses and modules they are not enrolled in. The signal is stored against $USER->id (server-side, so the user cannot impersonate another user) but with an attacker-chosen contextid.
Real-world impact is limited:
- The user can only submit signals about themselves (
$USER->id). - The course report filters by enrolled users (
coursereport.phpline 110-128), so non-enrolled signals will not appear there. - The unique index
(userid, contextid)bounds the number of flag rows per (user, context) pair to one. - The main observable effect is pollution of the system-level admin signals report (
local/agentdetect:viewsignals, manager only), where signals are shown regardless of context.
This is a missing-access-control issue with low blast radius — worth fixing but not exploitable for damage to other users.
Low risk. Exploitation requires an authenticated session and only allows pollution of the system-level admin signals report (a manager-only view). The attacker cannot:
- Submit signals on behalf of another user (
$USER->idis server-side). - Escalate privileges or modify other users' data.
- Cause data loss or schema corruption.
- Reach the course report for users they are not enrolled with (enrollment filter blocks this).
The blast radius is confined to confusing data in an admin-only UI. Worth fixing for defensive depth and consistency with the external function path, but no real harm potential.
beacon.php is the page-unload reporting endpoint used by navigator.sendBeacon in amd/src/detector.js (handlePageUnload). It is a parallel path to the external function local_agentdetect_report_signals, which is the primary periodic-reporting path. Both ultimately call signal_manager::store_signal() with the user's $USER->id and the submitted $contextid.
The external function path enforces context access via validate_context(). The beacon path does not — likely because the original implementation focused on minimal bootstrap for unload-time performance.
Logged in as a student who is enrolled in Course A but not Course B, post the following JSON to /local/agentdetect/beacon.php with the user's current sesskey:
{
"sesskey": "<student-sesskey>",
"sessionid": "abc123",
"signaltype": "combined",
"contextid": <courseB-contextid>,
"signaldata": "{\"combinedScore\":85,\"verdict\":\"HIGH_CONFIDENCE_AGENT\"}"
}
The row is inserted into local_agentdetect_signals with contextid = courseB-contextid and surfaces in the manager-only /local/agentdetect/report.php view. Course B's teacher will not see it (enrollment filter), but a site manager will.
$contextid = clean_param($data['contextid'] ?? 0, PARAM_INT);
// Validate context exists, fall back to system context (0) if not.
if ($contextid > 0) {
$ctx = \context::instance_by_id($contextid, IGNORE_MISSING);
if (!$ctx) {
$contextid = 0;
}
}
Add an access check matching the external function — for example, by attempting require_login() with the course/cm derived from the context, or by calling external_api::validate_context($ctx) (note that this resets $PAGE, so use it carefully in this AJAX-script bootstrap), or by manually verifying the context against get_context_info_array($ctx->id):
if ($contextid > 0) {
$ctx = \context::instance_by_id($contextid, IGNORE_MISSING);
if (!$ctx) {
$contextid = 0;
} else {
[, $course, $cm] = get_context_info_array($ctx->id);
try {
require_login($course, false, $cm, false, true);
} catch (\moodle_exception $e) {
http_response_code(403);
exit;
}
}
}
local_agentdetect_format_context_link() passes course/module names through format_string() (which already HTML-encodes the result), then wraps the same value with s() before handing it to html_writer::link. For attribute values, html_writer::attribute() then escapes the value a third time when rendering.
The consequence is a display bug: a course shortname containing & (e.g. R&D 101) renders as the literal text R&D 10 in the link label and the link title. There is no security impact — output is over-escaped, not under-escaped — but it produces incorrect visual output for any course or module name containing HTML-sensitive characters.
The same anti-pattern appears in report.php where the page-URL title attribute is pre-escaped with s() before being placed in the attributes array.
Low risk. This is a cosmetic/code-quality bug — over-escaping is safer than under-escaping, but it produces wrong text in reports for any context whose name contains HTML-sensitive characters. No security impact.
local_agentdetect_format_context_link() is the helper that renders the Context column in both the admin and course reports. Because the function is called once per row (with per-request caching keyed by contextid), the bug is visible on every report row where the course or module name contains characters like &, <, or >.
$shortname = format_string($course->shortname);
$modname = format_string($cm->name);
$shortshort = $truncate($shortname, 10);
$shortmod = $truncate($modname, 25);
$url = new moodle_url('/mod/' . $cm->modname . '/view.php', ['id' => $cm->id]);
$title = $shortname . ' / ' . $modname;
return html_writer::link($url, s($shortshort . ' / ' . $shortmod), ['title' => s($title)]);
Drop the s() calls — format_string() already produces HTML-safe output for link text, and html_writer::attribute() already escapes attribute values. Also note the truncation occurs after format_string(), which can cut HTML entities mid-sequence; consider truncating raw values first, then format_string-ing them once:
$shortshort = $truncate($course->shortname, 10);
$shortmod = $truncate($cm->name, 25);
return html_writer::link(
$url,
format_string($shortshort . ' / ' . $shortmod),
['title' => format_string($course->shortname . ' / ' . $cm->name)]
);
if ($context instanceof context_course) {
$course = get_course($context->instanceid);
$shortname = format_string($course->shortname);
$label = $truncate($shortname, 10);
$url = new moodle_url('/course/view.php', ['id' => $course->id]);
return html_writer::link($url, s($label), ['title' => format_string($course->fullname)]);
}
Same as the previous location — drop the s() call around $label because format_string() already escapes for HTML.
$pagelink = html_writer::link(
$pageurl,
s($displaytext),
['title' => s($pageurl), 'target' => '_blank']
);
Remove s() from the title attribute (and arguably from the link text too, since html_writer will escape the attribute via attribute()):
$pagelink = html_writer::link(
$pageurl,
s($displaytext),
['title' => $pageurl, 'target' => '_blank', 'rel' => 'noopener noreferrer']
);
The link text itself uses raw $displaytext from JSON, so s() there is correct. Only the attribute escaping is redundant.
Both report.php (the admin signals JSON download and table) and signal_manager::compact_signal_data() reference a pageUrl/pageTitle field on the decoded signal data:
report.phpline 124-125 (JSON download):'page_url' => $data->pageUrl ?? null, 'page_title' => $data->pageTitle ?? nullreport.phpline 373-374 (signals table):$pageurl = $data->pageUrl ?? null; $pagetitle = $data->pageTitle ?? null;
However, the Lite JS client never emits these fields. amd/src/detector.js::buildCombinedPayload() and buildFingerprintPayload() produce payloads containing fingerprint, interaction, comet, combinedScore, verdict, and detectedAgent — and signal_manager::compact_signal_data() strips anything else. Consequently the page column always renders - and the JSON download always reports null for these fields.
This is dead code on the Lite edition. It's harmless functionally, but it implies a feature is present when it isn't, and the JSON download contract is misleading.
Low risk. No security impact — s() is applied to the (always-null) page URL before it would be rendered, and the URL scheme is regex-validated. This is a maintainability and contract-clarity issue: callers of the JSON download API may write client code that expects page_url to be populated, only to find it perpetually null.
The plugin advertises itself as a Lite edition with deliberately compact data collection. The contract between client and server is enforced by compact_signal_data(), which whitelists the keys it preserves and drops everything else. The PHP-side display code predates or post-dates that contract and refers to fields that never reach storage.
'question_types' => $data->interaction->questionTypes ?? [],
'text_input_focus_count' => $data->interaction->textInputFocusCount ?? 0,
'page_url' => $data->pageUrl ?? null,
'page_title' => $data->pageTitle ?? null,
Either remove the page_url/page_title references in this Lite build (and the table column rendering below), or have the JS client emit these fields in buildCombinedPayload() and preserve them through compact_signal_data(). If preserving them, both fields must be sanitised — pageUrl against clean_param(..., PARAM_URL) and pageTitle truncated to a sensible length.
// Get page URL from signal data.
$data = json_decode($signal->signaldata);
$pageurl = $data->pageUrl ?? null;
$pagetitle = $data->pageTitle ?? null;
Same as above — either drop this block (and the surrounding URL-validation/rendering logic) on the Lite edition, or emit and persist these fields in the JS/PHP pipeline.
The flags table declares a UNIQUE index on (userid, contextid) but contextid is NOTNULL="false":
Both MySQL/MariaDB and PostgreSQL treat NULL values as distinct in unique indexes, so the constraint cannot prevent two rows for the same userid with contextid IS NULL. The application-level guard in signal_manager::update_user_flag() uses get_record_select with contextid IS NULL to dedupe, but the read-then-write check is not atomic — under concurrent requests for the same user with no specific context (e.g. two beacons firing simultaneously, or a beacon + an external call), both can pass the check and both insert, producing duplicate NULL-context flag rows.
A student spamming beacons could deliberately race this and produce a small set of duplicate flag entries that are then displayed as separate rows in the admin report. Not a major issue, but the schema does not match the application's invariant.
Low risk. Mostly a data-integrity nuance, not exploitable for harm. The duplicate rows would only show in admin views and the reports' GROUP BY queries would still aggregate scores correctly. Worth aligning the schema with the application's invariant for clarity.
signal_manager::update_user_flag() is called from every signal store. For signals submitted via beacon.php with contextid = 0, the stored row gets contextid = null (see signal_manager::store_signal line 86: $record->contextid = $contextid ?: null;). Multiple simultaneous beacons from the same browser can therefore race the dedup check.
<FIELD NAME="contextid" TYPE="int" LENGTH="10" NOTNULL="false" COMMENT="Context ID if context-specific flag"/>
...
<INDEX NAME="userid_contextid" UNIQUE="true" FIELDS="userid, contextid"/>
Choose one of:
- Make
contextidNOT NULL with a sentinel value (e.g. the system context ID) so the unique constraint applies uniformly. This is the simplest fix but changes the semantics of "no context". - Drop the UNIQUE designation and rely on the application guard, accepting that duplicates are theoretically possible under contention.
- Wrap the get-or-insert in a transaction with explicit locking (
get_record_selectfollowed byinsert_recordis the read-modify-write pattern), or useINSERT ... ON DUPLICATE KEY UPDATE/an upsert helper. Moodle does not expose a portable upsert, but you can wrap the block in\core\transaction::start_delegated_transaction().
signal_manager defines class constants for three flag types — FLAG_SUSPECTED, FLAG_CONFIRMED, and FLAG_CLEARED — but uses the bare string 'low_suspicion' everywhere the fourth flag type is referenced. This appears in signal_manager.php line 275, and the same string is matched by string literal in coursereport.php, report.php, external/get_user_flags.php, and the language file.
The inconsistency means a typo in any of these literals would silently break flag matching (e.g. 'low_susicion' would fail equality checks but not produce any compile-time error). A FLAG_LOW_SUSPICION constant would centralise the value and make typos discoverable.
Low risk. No security impact. Pure code-quality / maintainability concern.
The flag-type enum is used in several places — SQL flagtype != 'cleared' filters, switch statements that produce verdict badges, and the flagtype:* language string lookup. Centralising the values reduces drift between these touchpoints.
$flag->flagtype = $score >= self::FLAG_THRESHOLD_HIGH ? self::FLAG_SUSPECTED : 'low_suspicion';
Add a constant and use it everywhere:
/** @var string Flag type for low-suspicion matches. */
const FLAG_LOW_SUSPICION = 'low_suspicion';
Then replace the string literal across signal_manager.php, coursereport.php, report.php, and external/get_user_flags.php.
In report.php, the page-URL column renders user-supplied URLs as links opening in a new tab via target="_blank", but does not set rel="noopener noreferrer". Modern browsers default target="_blank" to rel="noopener" since the late-2020 update, so the reverse-tabnabbing window.opener vector is largely mitigated, but explicit rel is still recommended for defensive depth and to avoid leaking referrer headers to the external host.
Note that as observed in finding #3, the pageUrl field is never actually populated by the Lite client, so this code path does not currently execute. But if the field is wired up in a future release, this should be set.
Informational. Mitigated by modern browser defaults; setting rel explicitly is the canonical recommendation but does not address an active exploit.
The link is rendered in the admin signals report (managers only). If pageUrl is ever populated, the rendered URL can be any arbitrary HTTPS URL chosen by the user's client.
$pagelink = html_writer::link(
$pageurl,
s($displaytext),
['title' => s($pageurl), 'target' => '_blank']
);
Add an explicit rel:
$pagelink = html_writer::link(
$pageurl,
s($displaytext),
['title' => $pageurl, 'target' => '_blank', 'rel' => 'noopener noreferrer']
);
fix-indent.js is a one-off Node script that re-indents a block of report.php in place. It is clearly a developer utility — it isn't referenced from any other file, isn't mentioned in README.md, and operates by reading and rewriting report.php directly. It does no harm because it isn't executed at runtime, but it should not be part of a released plugin distribution.
The .gitignore already excludes DETECT.md and other internal artifacts; adding fix-indent.js to that list (or deleting the file) keeps the released package clean.
Informational. No runtime impact. Pure release hygiene.
Plugin distributions generally aim to ship only files needed at runtime plus documented build tooling (build.js, package.json for the AMD pipeline). Stray developer scripts confuse plugin reviewers and add unnecessary surface area.
Delete the file from the repository, or move it to a tools/ directory and add it to .gitignore so it isn't packaged with releases.
The backup plugin (backup_local_agentdetect_plugin::define_course_plugin_structure) selects s.* from local_agentdetect_signals, which includes ipaddress and useragent fields. When a course is backed up and restored elsewhere, these personal identifiers travel with the backup file.
Moodle's backup framework already lets users choose whether to include user data, so site administrators can opt out at backup time. But the plugin could be more conservative by default — dropping the ipaddress and useragent columns from the backup definition, or only including them under a privacy-sensitive flag. The plugin's Privacy API is fully implemented (covering metadata, export, and deletion), so this is more about defensive design than a compliance gap.
Informational. Operators retain control of user-data backup inclusion, and the Privacy API covers per-user deletion and export. This is a design suggestion, not a compliance gap.
The Privacy API implementation in classes/privacy/provider.php already declares these fields as personal data. Including them in backups is consistent with Moodle's general approach (backups can include user data), but a quieter default would reduce the chance of IP/UA history being copied across instances unexpectedly.
$signal = new backup_nested_element('signal', ['id'], [
'userid', 'contextid', 'sessionid', 'signaltype',
'fingerprintscore', 'interactionscore', 'combinedscore',
'verdict', 'signaldata', 'useragent', 'ipaddress', 'timecreated',
]);
Consider whether useragent and ipaddress need to follow a course backup. If not, drop them from the element definition:
$signal = new backup_nested_element('signal', ['id'], [
'userid', 'contextid', 'sessionid', 'signaltype',
'fingerprintscore', 'interactionscore', 'combinedscore',
'verdict', 'signaldata', 'timecreated',
]);
If they must be preserved for forensic purposes, the alternative is to document the privacy implication in the plugin README.
provider::get_contexts_for_userid() unconditionally calls $contextlist->add_system_context() regardless of whether the user has any signals or flags with a NULL contextid. The export and delete methods do filter correctly by userid in the system-context branch, so this is functionally safe — the system context will simply produce empty exports/deletes for users with no system-context data.
It is, however, slightly misleading: privacy tooling will report "data exists at system context for user X" when it may not. A small efficiency and clarity win is to check whether system-context records exist before adding it.
Informational. No security or privacy impact — the export/delete functions correctly filter records by userid, so a spurious system-context entry won't leak data. Only the context list is inflated.
The system-context inclusion exists because signals and flags with NULL contextid (created when beacon.php falls back to context 0, or when an admin sets a system-wide flag) need to be discoverable via the privacy provider. The implementation is overly broad but not incorrect.
public static function get_contexts_for_userid(int $userid): contextlist {
$contextlist = new contextlist();
// Signals with a context.
$sql = "SELECT DISTINCT contextid FROM {local_agentdetect_signals}
WHERE userid = :userid1 AND contextid IS NOT NULL";
$contextlist->add_from_sql($sql, ['userid1' => $userid]);
// Flags with a context.
$sql = "SELECT DISTINCT contextid FROM {local_agentdetect_flags}
WHERE userid = :userid1 AND contextid IS NOT NULL";
$contextlist->add_from_sql($sql, ['userid1' => $userid]);
// Always include system context for records without a specific context.
$contextlist->add_system_context();
return $contextlist;
}
Gate the system-context addition on whether NULL-context records exist for the user:
$sql = "SELECT 1 FROM {local_agentdetect_signals}
WHERE userid = :userid AND contextid IS NULL
UNION
SELECT 1 FROM {local_agentdetect_flags}
WHERE userid = :userid AND contextid IS NULL";
if ($DB->record_exists_sql($sql, ['userid' => $userid])) {
$contextlist->add_system_context();
}
Strong test coverage: The plugin ships PHPUnit tests for both signal_manager and the privacy provider, including event-trigger assertions, flag-escalation behaviour, and context isolation. Tests use the standard Moodle generators and resetAfterTest, which is the correct pattern.
Sensible client-side discipline: signal_manager::compact_signal_data() whitelists the fields it persists and casts numeric scores to int, blocking any attempt to inject unexpected structure through the JSON signaldata field. The same shape is enforced consistently in the JS-side buildCombinedPayload().
No bundled third-party runtime code: The AMD modules are built locally from amd/src/*.js using build.js. package.json lists Babel/Terser as dev dependencies for the build only — the minified amd/build/*.min.js files contain only the plugin's own source. No thirdpartylibs.xml is required.
Capability design is appropriate: All four declared capabilities carry RISK_PERSONAL or RISK_CONFIG, and they correctly map to the contexts where they are checked (course level for viewreports/manageflags, system level for viewsignals/configure). The local/agentdetect:viewsignals is correctly restricted to manager archetype — only a system manager can view detection signals in the admin report.