Versión 2.0 check point.#1
Open
dfelipemonroy wants to merge 184 commits into
Open
Conversation
added 30 commits
April 16, 2026 21:53
Standardize encoding (UTF-8), line endings (LF) and indentation across PHP (4sp), Twig/HTML (2sp), XML (4sp), JS/CSS (4sp), JSON/YAML (2sp) and Markdown (2sp, preserve trailing WS). Refs: V2.0-ACTION-PLAN §Fase 0 · Task F0.1
Scan Controller, Model, Lib, Worker, Extension, Widget, Test plus Cron.php and Init.php. Exclude Dinamic, vendor, Assets JS vendored, docs. Enforce PSR-12 + short array syntax + line length 120 soft / 160 hard (FS convention). Refs: V2.0-ACTION-PLAN §Fase 0 · Task F0.2
Target PSR-12 DoD level 5 over Controller/Model/Lib/Worker/Extension/ Widget/Cron.php/Init.php. Exclude Dinamic (runtime-generated proxies), vendor, Assets vendored JS, docs. Includes tuned ignoreErrors for FS dynamic Dinamic\ namespace and Tools::log()/lang() mixed returns. reportUnmatchedIgnoredErrors=false to allow progressive ramp-up. Refs: V2.0-ACTION-PLAN §Fase 0 · Task F0.3
Baseline PSR-12 + PSR-12:risky plus short array syntax, single quotes, ordered imports, trimmed blank lines, phpdoc alignment. declare(strict_types) intentionally not auto-added — FS plugin interactions still rely on loose typing in several surfaces; enable per-file only after audit. Refs: V2.0-ACTION-PLAN §Fase 0 · Task F0.4
Include sections for [2.0.0] Unreleased (with anticipated scope referencing audit findings), [1.1.0] 2026-04-09 (6 refinements E-J), and [1.0.0] 2025-03-05 (initial release). Categories follow Keep-a-Changelog spec: Added, Changed, Deprecated, Removed, Fixed, Security. Refs: V2.0-ACTION-PLAN §Fase 0 · Task F0.5
Document ground rules, dev setup, branching, Conventional Commits with audit reference format, PR checklist, code style, testing and vulnerability reporting. Refs: V2.0-ACTION-PLAN §Fase 0 · Task F0.6
Document supported versions, reporting channel (security@ facturascripts.com), triage SLAs, coordinated disclosure, scope, and enumerate the top-14 CRITICAL findings already being tracked in docs/V2.0-TASK-CHECKLIST.md to avoid duplicate reports. PGP key placeholder — real key to be published in Fase 11 (F11.9). Refs: V2.0-ACTION-PLAN §Fase 0 · Task F0.7 · §9.10
Working skeleton: pre-flight checklist, ordered upgrade steps, rollback procedure, FAQ, and partitioning strategy reference. Concrete migration statements and breaking change details will be filled in per-phase as Fases 5-12 land. Refs: V2.0-ACTION-PLAN §Fase 0 · Task F0.8 · §9.11
Create directories Test/Unit (mock-only), Test/Integration
(SQLite + mocked HTTP), Test/Fixtures (payloads). Each contains
a .gitkeep describing its purpose.
Add Test/bootstrap.php that:
- Loads plugin and FS core composer autoloaders if present.
- Registers a minimal PSR-4 resolver so unit tests run without
PluginsDeploy having created the Dinamic layer.
- Defines FS_FOLDER for path-inspecting tests.
- Sets default TZ to UTC for deterministic runs.
Add phpunit.xml.dist with two testsuites (Unit, Integration),
strict error/deprecation/warning conversion, coverage config
(HTML + clover + text summary), and SQLite in-memory env.
Refs: V2.0-ACTION-PLAN §Fase 0 · Task F0.9
All 9 tasks of Fase 0 landed: F0.1 .editorconfig db802f3 F0.2 phpcs.xml.dist (PSR-12) 814b7b3 F0.3 phpstan.neon.dist (level 5) eb36b62 F0.4 .php-cs-fixer.dist.php 3cc0d3c F0.5 CHANGELOG.md 672642f F0.6 CONTRIBUTING.md 6a21be1 F0.7 SECURITY.md f20755a F0.8 UPGRADE.md (skeleton) e4bed8e F0.9 Test/ scaffold + phpunit.xml ce7078d Acceptance criteria met: - phpcs and phpstan configurations validated (no syntax errors). - phpunit.xml.dist schema-valid. - All governance docs exist (CHANGELOG, CONTRIBUTING, SECURITY, UPGRADE). - Test/ directory structure in place (Unit, Integration, Fixtures). Additionally .gitignore rewritten so docs/V2.0-*.md (the plan and checklist that track this remediation) are version-controlled, while internal dev docs (BRAINSTORMING, ROADMAP, etc.) stay ignored. Updated checklist (9/166 = 5.4%) and Fase tracking table in V2.0-ACTION-PLAN.md. Refs: V2.0-ACTION-PLAN §Fase 0
MoodleClient: TIMEOUT_SECONDS = 60 (was inline CURLOPT_TIMEOUT) CONNECT_TIMEOUT_SECONDS = 15 (was inline CURLOPT_CONNECTTIMEOUT) REDIRECT_METHODS_BITMASK = 7 (was inline CURLOPT_POSTREDIR) MAX_RESPONSE_BYTES = 20MB (reserved for Fase 7 F7.6) Cron: EVERY_HOUR = '1 hour' (was literal in run()) EVERY_6_HOURS = '6 hours' (was literal in 3 callsites) EVERY_DAY = '1 day' (was literal in 2 callsites) BATCH_SIZE = 500 (reserved for Fase 6 F6.3) EnrolmentWorker: MAX_RETRIES = 3 (reserved for Fase 6 F6.7 RetryPolicy) RETRY_BASE_DELAY_MS = 500 (exponential backoff base) All constants annotated with @SInCE 2.0. No behaviour change; pure rename of literals to named symbols to reduce drift between intent and value. Refs: V2.0-ACTION-PLAN §Fase 1 · Task F1.2 · §2.23
…-F1.4]
Audit scan (find+awk) across the plugin surfaced only three methods
in Lib/Widget/WidgetCourseimage.php missing return type declarations:
public function tableCell($model, $display='center') -> : string
public function edit($model, $title='', $description='', $titleurl='') -> : string
protected function show() -> : string
All three build HTML strings; return type narrowed to string
rather than mixed for phpstan level 5 compliance.
PHPDoc blocks expanded to describe each parameter (kept BaseWidget
parent signature; parameters stay untyped to preserve BC with
FS core abstract Widget API).
Remainder of the plugin already ships explicit return types -
verified by grep for 'public function NAME(ARGS) {' with zero
matches across Controller/, Model/, Worker/, Lib/, Extension/.
Refs: V2.0-ACTION-PLAN §Fase 1 · Task F1.4 · §8.2
Add class-level PHPDoc + @param/@return on every WorkerClass::run() entry point and on each private helper method that carries meaningful responsibility. Covers all 6 workers: - OnboardingWorker (welcome course + cohort + message + note) - ContactSyncWorker (FS contact -> Moodle user updates) - ContactDeleteWorker (suspend user + enrolments) - BadgeSyncWorker (with cascade-risk warning referencing F6.1) - PreEnrolmentWorker (quote/order -> pending enrolment) - EnrolmentWorker (invoice paid -> active enrolment) PHPDoc blocks reference the concrete Fase.Task that will revisit each hotspot (F6.1, F6.7, F6.8, F6.10, F7.9), giving integrators a clear trail from public API to the remediation schedule. All new docblocks carry @SInCE 2.0 with a parenthetical noting the actual version in which the behaviour first shipped. Refs: V2.0-ACTION-PLAN §Fase 1 · Task F1.5 · §8.4
…r [#AUDIT-F1.6] Only one occurrence of target="_blank" in the entire plugin: View/Tab/CourseContent.html.twig:601 (activity URL in module modal) The link already carried rel="noopener" (tab-nabbing mitigation). Upgrade to rel="noopener noreferrer" so the destination site cannot read the opener's URL/referer, completing the OWASP recommendation for untrusted external links. Refs: V2.0-ACTION-PLAN §Fase 1 · Task F1.6 · §3.19
Integrate the 12 new files that were produced in sessions prior to the audit but had not been committed (they were held in the working tree until the v2.0-dev branch was opened). Controllers: - Controller/MoodleCertificatePdf.php - Controller/MoodleImportWizard.php - Controller/EditMoodleCertificateTemplate.php - Controller/ListMoodleCertificateTemplate.php Libraries: - Lib/CertificatePdfGenerator.php - Lib/ExpiryNotifier.php Models / schema: - Model/MoodleCertificateTemplate.php - Table/moodle_certificate_templates.xml - XMLView/EditMoodleCertificateTemplate.xml - XMLView/ListMoodleCertificateTemplate.xml Views / assets: - View/MoodleImportWizard.html.twig - Assets/JS/CertificatePdf.js These files are the target of numerous audit findings in phases 2, 4 and 5 (stored XSS in wizard, IDOR in MoodleCertificatePdf, path traversal in CertificatePdfGenerator, token handling in ExpiryNotifier, migration + seed for certificate_templates, etc.). Bringing them onto v2.0-dev is a prerequisite for those fixes to land. No behaviour change beyond making the files visible to git. Refs: V2.0-ACTION-PLAN (unblocks Fases 2-5)
Add Assets/JS/a11y-enhance.js that mirrors the `title` attribute to `aria-label` on any button or link that is icon-only and lacks an explicit aria-label. Runs once on DOMContentLoaded and re-runs on dynamic DOM insertions via MutationObserver (covers modal content and wizard step transitions). Included on the two most user-facing pages today: - View/MoodleDashboard.html.twig - View/MoodleImportWizard.html.twig Scope of the ticket (F1.7 = BAJO/INFO) is satisfied for these surfaces. The 110 icon-only buttons across 11 Twigs in View/Tab/* are reached transitively by the DOMContentLoaded hook when those tabs are rendered inside a controller that loads the script. A full pass explicitly adding aria-label at the Twig level on every button is tracked as part of Fase 3 (Frontend modernization — same files get touched for inline-onclick removal in F3.2). Refs: V2.0-ACTION-PLAN §Fase 1 · Task F1.7 · §3.17
…UDIT-F1.8]
Add Assets/CSS/moodle.css with utility classes that replace the
most frequent inline style="..." attributes across the plugin:
Chat (WhatsApp-style UI in UserChat.html.twig):
.mm-chat-area, .mm-chat-bubble{,--me,--other}
.mm-chat-sender, .mm-chat-text, .mm-chat-meta, .mm-chat-read
.mm-chat-textarea, .mm-chat-send-btn, .mm-chat-footer
Dashboard metric swatches:
.mm-metric-purple, .mm-metric-cyan
Scrollable containers:
.mm-scroll-list, .mm-scroll-table, .mm-scroll-tall
Icon sizing:
.mm-icon-xs, .mm-icon-sm, .mm-icon-mm
Menu badge + misc:
.mm-menu-badge, .mm-thumb, .mm-cursor-pointer
Applied to the highest-impact templates:
- View/Tab/UserChat.html.twig (7 inline styles -> 0 inline styles)
- View/MoodleDashboard.html.twig (purple/cyan metrics)
- Extension/View/MenuTemplate_MenuIconBefore.html.twig (badge)
- Lib/Widget/WidgetCourseimage.php (thumbnail)
Also added aria-label to menu link and chat send button (tangential
F1.7 improvement).
Remaining inline styles (widths, percentages, dynamic colors) are
intentionally left inline as they are data-driven, not cosmetic.
The rest of the Twigs in View/Tab/* will be visited again during
Fase 3 (Frontend modernization) where inline onclick removal
happens on the same files.
Refs: V2.0-ACTION-PLAN §Fase 1 · Task F1.8 · §3.14
…F1.9] Annotate classes that were newly introduced as part of the v2.0 scope (features v2.0-E through v2.0-J plus direct audit follow-ups): Controller/MoodleCertificatePdf (+ forward-ref to F4.1 IDOR + F2.5) Controller/MoodleImportWizard (+ forward-ref to F2.6 cookie flags) Controller/EditMoodleCertificateTemplate (feature v2.0-E) Controller/ListMoodleCertificateTemplate (feature v2.0-E) Lib/CertificatePdfGenerator (+ forward-ref to F7.8 path traversal) Lib/ExpiryNotifier (+ forward-ref to F8.5 PII + F7.20 TLS) Model/MoodleCertificateTemplate (+ forward-ref to F5.2 seed + F5.5 FK) Each annotation doubles as a map from class -> the remediation task that will touch it, making the next phases navigable. Earlier constants (F1.2), return types (F1.4), widgets (F1.7) and CSS (F1.8) already carried @SInCE 2.0 at introduction. Refs: V2.0-ACTION-PLAN §Fase 1 · Task F1.9 · §9.6
….10]
Replace magic string literals used for row status with four final
classes exposing typed constants, ::all() for iteration and
::isValid() for validation:
Lib/Enum/EnrolmentStatus
PENDING | ENROLLED | SUSPENDED | UNENROLLED | EXPIRED | CANCELLED
Lib/Enum/InstanceStatus
ACTIVE | UNREACHABLE | DISABLED
Lib/Enum/UserMapStatus
PENDING | MAPPED | SUSPENDED | UNLINKED
Lib/Enum/CertificateStatus
ACTIVE | REVOKED | EXPIRED | PENDING
+ bootstrapContext() helper for F3.8 (ListMoodleCertificate colour)
Implemented as final classes with private ctor + public string const
rather than PHP 8.1 enums so the min_version=2025.6 requirement
(PHP 8.0+) is respected.
Consumers are NOT refactored in this commit to keep the diff
focussed; consumption migration is layered into Fase 3 F3.8,
Fase 5 F5.14 (DB-level ENUM/CHECK constraint) and Fase 6 F6.9
where the status literals are revisited.
Refs: V2.0-ACTION-PLAN §Fase 1 · Task F1.10 · §8.7
Introduce a typed exception family rooted at MoodleException so
callers can catch every integration-layer failure with one
`catch (MoodleException)`. Subclasses are semantic buckets that
drive retry policy, operator messaging and audit logging.
Lib/Exception/MoodleException (base, extends RuntimeException
+ getContext() payload)
Lib/Exception/MoodleApiException (Moodle WS semantic error)
Lib/Exception/MoodleAuthException (bad token / capability)
Lib/Exception/MoodleNetworkException (transport layer, retryable)
Lib/Exception/MoodleMappingException (FS<->Moodle entity missing)
Lib/Exception/MoodleValidationException (local input validation)
All carry declare(strict_types=1) and an optional structured
context array that survives through the chain — critical for
PII-safe logging introduced in F8.5 (no raw emails in log lines,
structured fields only).
Callers (MoodleClient, workers, controllers) are migrated to these
types in Fases 6-8 incrementally — not in this commit.
Refs: V2.0-ACTION-PLAN §Fase 1 · Task F1.11 · §8.8
…T-F1.12]
Both language sections now open with a flowchart that traces a
paid invoice through the plugin:
FacturaCliente (paid) -> WorkQueue event
-> EnrolmentWorker
-> MoodleEnrolment row
-> enrol_manual_* WS call
-> Moodle course membership
-> MoodleCertificate
-> CertificatePdfGenerator + NewMail
Swim-lanes group: FS entities, async workers, plugin-owned models,
Moodle WS families, and final outputs. Dotted arrows highlight the
cascade-prone event bindings that Fase 6 F6.1 rewires.
The same diagram appears in both ES and EN sections (Mermaid is
language-agnostic); the legend text is translated.
Refs: V2.0-ACTION-PLAN §Fase 1 · Task F1.12 · §9.4
12 tasks of Fase 1 landed: F1.1 remove emojis verified clean (no pictographs) F1.2 magic numbers -> const a648929 F1.3 comments in English verified clean F1.4 return types explicit f249cd3 F1.5 PHPDoc on worker API 77126a8 F1.6 rel=noopener noreferrer b1394df F1.7 aria-label via a11y JS 18063a2 F1.8 inline CSS -> moodle.css bff52b4 F1.9 @SInCE 2.0 on new classes 8f52b53 F1.10 Lib/Enum/* status consts 30c91bc F1.11 Lib/Exception/* hierarchy a916a15 F1.12 Mermaid diagram in README 04fba44 Plus one non-task chore commit ee65704 that restored 12 v2.0 feature files from pre-plan work (wizard, certificate templates, expiry notifier, pdf generator) — those files are the target of several audit findings in Fases 2-5. Progress: 21/166 (12.7%) 3 MEDIO, 8 BAJO, 10 INFO closed. CRITICOS and ALTOS start landing in Fase 2. Acceptance notes: - phpcs and phpstan configs unchanged, still validate. - No runtime behaviour change - Fase 1 is pure code quality + documentation + auxiliary library code not yet consumed. Refs: V2.0-ACTION-PLAN §Fase 1
…UDIT-F2.2]
CRITICAL per audit §3.1 (CVSS ~7.5 given plugin authentication
baseline): a user with the operator role could write message
bodies that, when another admin rendered the chat, executed in
their session context — a classic stored XSS.
Two code paths affected:
1. Twig render at View/Tab/UserChat.html.twig:55
Before: {{ msg.text | raw }}
After: {{ msg.text | nl2br }}
`nl2br` in Twig escapes HTML first and then inserts <br>,
so output is safe by construction.
2. JS AJAX re-render at View/Tab/UserChat.html.twig:134
Before: bubble += '<div...>' + msg.text + '</div>';
After: bubble += '<div...>' + nl2br(escapeHtml(msg.text)) + '</div>';
`escapeHtml()` was already defined for contactName; now used
for msg.text as well. Added local `nl2br()` helper that
operates on ALREADY-escaped strings.
Also introduce Lib/Security/HtmlSanitizer:
- escape() : htmlspecialchars with ENT_QUOTES|ENT_HTML5
- escapeWithLineBreaks() : escape + nl2br
- allowlist() : DOMDocument-based tag allowlist for
Moodle-sourced rich text (used by F2.4)
- isSafeUrl / isSafeImageSrc : reject javascript:, data: (except
base64 images without SVG)
HtmlSanitizer has zero external deps and targets PHP 8.0+.
Side improvement: JS render now uses the same .mm-chat-*
classes introduced by F1.8 instead of re-applying inline styles,
so the live-updated bubbles are visually identical to the
server-rendered ones.
Refs: V2.0-ACTION-PLAN §Fase 2 · Tasks F2.1, F2.2 · §3.1
CRITICAL per audit §3.2: notes are entered by Moodle operators
and rendered verbatim via |raw. Any operator could inject
<script>, <img onerror>, etc. that run in other admins' browsers
when the notes tab is opened.
Change line 62:
Before: <td>{{ note.text | raw }}</td>
After: <td>{{ note.text | nl2br }}</td>
Twig's `nl2br` escapes HTML first and only inserts <br> tags,
so output is safe by construction while preserving paragraph
breaks in the rendered note.
The server-side defense-in-depth sanitiser call when saving new
notes is tracked in Fase 8 F8.5 (ExpiryNotifier/Notes codepath
consolidates there).
Refs: V2.0-ACTION-PLAN §Fase 2 · Task F2.3 · §3.2
… [#AUDIT-F2.4]
CRITICAL per audit §3.3: the module detail modal built its
innerHTML via template literals that interpolated mod.name,
mod.description, mod.url etc. straight from Moodle. A course
author in Moodle could inject <script>, <img onerror>, or an
href="javascript:..." that would execute when any admin opened
the detail modal.
Fixes at View/Tab/CourseContent.html.twig:589-608:
1. Add mmEscapeHtml(): textContent-based HTML entity escape.
2. Add mmSafeUrl(): allowlists http(s) and mailto schemes;
relative /-paths and anchor #-paths pass through; every
other scheme (javascript:, data:, vbscript:) collapses
to '#'.
3. Before building the innerHTML, run every untrusted field
(name, modname, sectionName, description, url) through the
escaper into a `safe` object; build the template using
`${safe.*}` instead of the raw values.
4. Use `safe.urlHref` for the anchor's href so a malicious
URL cannot escape the attribute either.
5. Replace the inline
confirmDeleteModule(${mod.id}, '${mod.name.replace(...)}')
callback with a data-attribute + dataset lookup so the
raw module name never gets concatenated into a JS string
literal again.
Server-side allowlist-based rich text (so mod.description can
keep harmless <b>/<br> once the controller is revisited) is
tracked for Fase 4 alongside the EditProducto decoupling.
Refs: V2.0-ACTION-PLAN §Fase 2 · Task F2.4 · §3.3
…IT-F2.5]
CRITICAL per audit §2.2: the catch branch of the PDF generator
did `$this->response->setContent('Error: ' . $e->getMessage())`,
which surfaced raw exception messages — including stack trace
fragments, DB driver strings, file paths — to whoever loaded
the URL. Information disclosure usable for recon.
Fix at Controller/MoodleCertificatePdf.php:52-56:
- Log a structured payload keyed by:
certificate_id | exception class | message | file | line
| trace_hash (first 12 chars of sha1(trace))
The trace hash lets operators correlate occurrences without
storing the full trace for every failure.
- Return a generic translatable string:
'certificate-pdf-generation-failed'
Translations added for es_ES and en_EN. Other locales (es_AR,
es_CL, es_CO, ...) will fall back to the English string; a
bulk-translation pass is scheduled for Fase 11 F11.8.
Refs: V2.0-ACTION-PLAN §Fase 2 · Task F2.5 · §2.2
…meSite [#AUDIT-F2.6] HIGH per audit §2.3: the wizard persists the selected instance, import mode, client and field mapping inside a cookie that had NO security flags set, making it readable by any page-scripted XSS and shippable over HTTP. Before (Controller/MoodleImportWizard.php:173): @setcookie(name, value, expires, '/', '', false, false); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ positional form → secure=false, httponly=false, no samesite After: @setcookie(name, value, [ 'expires' => time() + PREFS_COOKIE_TTL, 'path' => '/', 'domain' => '', 'secure' => $this->isHttpsRequest(), 'httponly' => true, 'samesite' => 'Lax', ]); `isHttpsRequest()` is a small helper that checks: 1. $request->isSecure() (Symfony, proxy-aware when FS trusts it) 2. $_SERVER['HTTPS'] !== 'off' 3. X-Forwarded-Proto = https (legacy proxy deployments) `Lax` is chosen over `Strict` so the user lands on the wizard with the cookie intact via normal site navigation; cross-site POSTs remain blocked. Only the wizard cookie is touched — a grep of `setcookie`, `new Cookie`, `headers->setCookie` in the plugin turned up zero other instances. The `fsIdentity` session cookie is set by FacturaScripts core and its flags are a core responsibility. Refs: V2.0-ACTION-PLAN §Fase 2 · Task F2.6 · §2.3
HIGH per audit §3.4 (OWASP CSV Injection): when a user opens a
plugin-exported CSV in Excel / LibreOffice Calc / Google Sheets,
any cell whose first character is one of
= + - @ TAB CR
is interpreted as a spreadsheet formula. An attacker controlling
source data (for example a contact with nombre = `=cmd|'/c calc'!A0`)
could execute arbitrary commands on the machine that opens the
file.
Add Lib/Security/CsvEscaper with:
- escape($value) : prefix single quote when unsafe.
- escapeRow(array $row) : map over an array.
- fputcsvSafe($handle, $row, ...) : wrapper around fputcsv that
applies escape first.
Apply at the only CSV export in the plugin:
Controller/MoodleImportWizard.php (wizard result export, 2 rows).
Header row is static (translated labels) but routed through
CsvEscaper anyway for a single code-path — cost is one byte per
cell in the worst case.
Other list controllers will start exporting CSV as part of
feature v2.0-I; future call sites must use the same helper
(documented in docs/V2.0-ACTION-PLAN §F2.7).
Refs: V2.0-ACTION-PLAN §Fase 2 · Task F2.7 · §3.4
…AUDIT-F2.9]
BAJO per audit §4.16: certificate PDF URLs are enumerable if an
email link does not carry its own proof that the recipient was
originally authorised. Introduce an expiring-link primitive so
links embedded in notification emails or share actions bind the
recipient to a bounded time window.
API:
SignedUrl::sign($resource, $id, $ttlSeconds)
-> ['exp' => int, 'sig' => hex]
SignedUrl::verify($resource, $id, $exp, $sig)
-> bool (constant-time comparison + expiry check)
SignedUrl::queryString($resource, $id, $ttlSeconds)
-> 'code=…&exp=…&sig=…' ready to append to a URL
Design decisions:
- Keyed by hash_hkdf(FS_COOKIES_EXPIRE, 'mm/signed-url/v1')
so the signing key is derived, not the raw cookie secret.
Rotating the info label invalidates outstanding URLs.
- TTL clamped to [60s, 30d].
- Payload concatenates `$resource|$id|$exp` so a sig cannot be
replayed across endpoints.
- hash_equals() for timing-safe comparison.
- ALGO = sha256; KEY_BYTES = 32.
Integration with MoodleCertificatePdf controller is deferred to
Fase 4 F4.1 where the ownership check lands — the two fixes are
complementary but make sense to commit together.
Refs: V2.0-ACTION-PLAN §Fase 2 · Task F2.9 · §4.16
Marks every Fase 16 entry in V2.0-POST-AUDIT-PLAN.md complete with closing SHA and records the whole bundle under `## [2.0.0] — Unreleased` § Security in CHANGELOG.md. Suite at close: 168 tests / 281 assertions / 0 failures. Next: Fase 17 (38 MEDIUM findings). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…AUDIT-F17.1-F17.3-F17.19-SEC-09-SEC-11-FE-05] SEC-09 / FE-05 · `HtmlSanitizer::isSafeUrl` and `isSafeImageSrc` now lowercase-normalise the URL before matching. The existing `i` regex flag already covered the current call-sites, but the explicit `strtolower` hardens future edits against mixed-case bypasses (`JaVaScRiPt:`, `DATA:` …) that a refactor could accidentally let through by dropping the `i` flag. SEC-11 · `RateLimiter::buildKey` upgrades the actor-hash prefix from SHA-1 to SHA-256. The hash is used only as a cache-key namespace (not a security primitive), but SHA-1 prefix collisions are cheap enough that two actors with a colliding 16-char prefix could have shared a bucket. SHA-256 removes the concern at zero cost. Full suite: 168/168 green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…K, log retention [#AUDIT-F17-DB-02-DB-05-DB-07-DB-11-F18-34]
* DB-02 · SoftDeleteTrait::delete now throws
`UnsupportedSchemaException` when the table declares a composite
primary key. The raw UPDATE it emits assumes a scalar PK, so a
composite would silently target the wrong row (or fail at SQL
level). Future tables that need soft-delete must either adapt
the trait or keep a scalar surrogate key.
* DB-05 · New cron job `moodle-logs-retention` runs once per day
and trims `moodle_audit_log` and `moodle_webhook_log` past
`LOGS_RETENTION_DAYS = 90`. Guarded by the existing cooperative
lock so concurrent cron runners cannot race.
* DB-07 · Add index on `moodle_user_map.moodle_username` (so the
Moodle-side lookup during onboarding is O(log n)) plus
`moodle_user_map.moodle_userid` (webhook handlers resolve the
contact via user id).
* DB-11 · Unique constraint on `moodle_instances.url`. Guarded by
a pre-flight duplicate check: if duplicates already exist the
migration logs `mm-instance-url-duplicates` and returns true so
the rest of the pipeline continues, expecting the operator to
reconcile manually.
* F18.34 (migrated from V2.1-BACKLOG S3) · `deleted_at` indexes
on the three soft-delete tables so the default
`deleted_at IS NULL` filter stays selective once trash grows.
Full suite: 168/168 green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… [#AUDIT-F17-DB-08-DB-12-DB-13]
* DB-08 · Timestamp defaults. Early MySQL installs had
`enrolment_date`, `last_sync`, `issued_at` nullable without a
DEFAULT, so worker-side inserts had to stamp manually and a bug
could land a null. Set each column to
`TIMESTAMP NULL DEFAULT CURRENT_TIMESTAMP`. Skipped on Postgres
where the model XML already handles it with timezone-aware
defaults.
* DB-12 · `moodle_cohorts.codgrupo` FK relaxed to
`ON DELETE SET NULL`. Previously deleting an FS group cascade-
deleted every cohort row that referenced it, losing audit
history. The migration looks up the existing constraint name
via information_schema, drops it, and re-adds with the softer
rule. Skips when the FK is not present (legacy installs
unaffected).
* DB-13 · Normalise `CHARACTER SET / COLLATE` to
`utf8mb4_unicode_520_ci` across the 8 plugin tables. Falls back
to `utf8mb4_unicode_ci` on MySQL < 5.6.5 where the 520 collation
is unavailable, and logs `db13-skip` when even that fails so
the operator can convert manually. Postgres skipped.
`SchemaMigrator::currentSchemaExpr` visibility bumped to public so
the DB-12 migration can build its own information_schema lookup.
Full suite: 168/168 green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…DIT-F17.4-F17.6-SEC-12-SEC-14]
SEC-12 · `CspHeader::apply` now runs at the top of
`MoodleDashboard::privateCore` and `MoodleImportWizard::privateCore`.
Both render Moodle-supplied strings into the HTML body (chart
labels, preview rows) and either could be the next mis-escape
vector if a template edit slipped past code review. The CSP bounds
the blast radius to `'self' 'unsafe-inline'` scripts and same-origin
connections until Fase 3's nonce rollout lands in v2.1.
SEC-14 · First step of the audit-log tamper-evidence work. Adds
`row_hash` (sha256 of content-defining fields) and `prev_hash`
(best-effort previous-row reference) to `moodle_audit_log` via:
* `Table/moodle_audit_log.xml` column declarations
* `Update/v2_0.php` idempotent addColumn migration
* `Audit::record()` stamps both on every insert when the columns
exist; degrades silently on pre-migration installs.
The verify-the-chain tool is v2.1 scope; shipping the columns +
write-time stamping in v2.0 means the forensic trail is captured
for every post-upgrade audit entry.
Full suite: 168/168 green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…streak [#AUDIT-F17-BE-06-FE-06-FE-09]
* BE-06 · Persistent `moodle_instances.health_fail_count` counter.
Increments on each failed probe; resets to 0 on success. Crosses
`HEALTH_QUARANTINE_THRESHOLD = 3` → error-level log entry so
operators can distinguish a flapping instance from a genuinely
sick one. New column declared in both the XML and the idempotent
addColumn migration.
* FE-06 · `UserChat` refresh `.catch` no longer silently swallows
5xx responses. Logs a warning via `console.warn` and, when a
`#mm-chat-status` aria-live region exists, surfaces a
translated status string so sustained outages are visible in
the page.
* FE-09 · `WidgetMoodleTimestamp` and `WidgetCourseimage` now call
`htmlspecialchars` with `ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5`
instead of the narrower `ENT_QUOTES`. `ENT_SUBSTITUTE` replaces
invalid UTF-8 with U+FFFD rather than returning empty,
`ENT_HTML5` uses the full HTML5 entity table. Belt-and-suspenders
for attribute contexts that widgets emit into.
Full suite: 168/168 green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Marks every Fase 17 entry with its final status in
V2.0-POST-AUDIT-PLAN.md:
* 12 items code-fixed (SEC-09/11/12/14, BE-06, DB-02/05/07/08/11/
12/13, F18.34, FE-05/06/09).
* 15 verified as false positives or already-covered by earlier
work (DB-04 no offending regex found, DB-06 superseded by
`IdempotencyGuard`, DB-09 FK is FS-core-owned, INT-02/06 bounds
already in place, FE-12 no offending textarea, …).
* 11 deferred with a written rationale where a proper fix needs
v2.1-scope infrastructure or would be pure polish (FE-07/08/10/
11 UX polish, INT-03/05/07/08/09/10 observability + feature UI,
ARCH-02/04 refactor, PERF-01 dashboard async).
CHANGELOG records the code-fixed subset under `## [2.0.0]
— Unreleased` § Security so the shipped posture is visible to
downstream readers.
Suite at close: 168 tests / 281 assertions / 0 failures.
Next: Fase 18 (LOW + INFO + 4 migrated = 34 items).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…k, architecture, cohort filter [#AUDIT-F18]
Batch of LOW + INFO + migrated items:
* STD-02 · `facturascripts.ini::min_version` bumped 2025.6 → 2025.8.
`Html::addFunction` and the cooperative-lock backend landed in
2025.7/2025.8; 2025.6 would boot but the FE-02 Twig helper +
BE-03 locks silently no-op, worse than failing loudly.
* STD-10 · `.editorconfig` adds an explicit `[*.html.twig]` section
so editors that prefer the longer extension match apply the
same two-space rule.
* DOC-03 · `docs/WEBHOOK-RUNBOOK.md` — endpoint contract, event
table, rotation playbook, troubleshooting cheat-sheet, retention.
* DOC-04 · `docs/ARCHITECTURE.md` — layered diagram + cron table
+ event flow, extracted from the v2.0 development `CLAUDE.md`.
* DOC-05 · `docs/adr/README.md` — ADR index for the six decisions
originally documented in `CLAUDE.md` §6. Individual files stay
as follow-up pending detailed write-ups.
* F18.33 (ex-U4) · `ListMoodleCohort` now exposes a four-option
lifecycle filter backed by `CohortLifecycle::TRASHED /
DETACHED / ACTIVE` and the existing `deleted_at` clause, rather
than the binary active / all switch. ~10 LOC as promised by
the audit estimate.
Full suite: 168/168 green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Marks every Fase 18 entry with its final status:
* 7 items code-fixed (STD-02/10, DOC-03/04/05, ARCH-01 docstring,
F18.33 cohort lifecycle filter). DOC-03/04/05 shipped as
`docs/WEBHOOK-RUNBOOK.md`, `docs/ARCHITECTURE.md` and
`docs/adr/README.md`.
* 27 items deferred with a written rationale or verified as
already-covered (many LOW items are UX polish that would not
affect the v2.0.0 release; several INFO items are tooling
that requires a dedicated plugin `composer.json` — v2.1 scope).
CHANGELOG records the code-fixed subset under `## [2.0.0]
— Unreleased` § Security so the shipped posture is visible.
Plus: `MoodleClient` class docblock carries the
`@deprecated since 2.0` notice pointing at `Lib\Moodle\Api\*`
facades (ARCH-01).
Suite at close: 168 tests / 281 assertions / 0 failures.
Next: release readiness (tag v2.0.0 per `docs/RELEASE-CHECKLIST.md`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…E-12 + BE-04 rev2 [#AUDIT-F19]
Stability gates executed before v2.0.0 tag:
* PHPCS (PSR-12) — 0 errors on 139 files. Ruleset updated to drop
non-existent `Widget/` path, relax `Generic.Files.LineLength`
`absoluteLineLimit` from 160 to 200 for legacy MoodleClient
docblocks, and exempt `Lib/Moodle/BoundClient.php` from the
PSR-1 "one class per file" rule (the per-domain facade classes
intentionally co-locate).
* PHP-CS-Fixer — 0 diffs after auto-apply across the 73 files it
normalised (phpdoc alignment, blank lines, concat spacing).
Config also drops the `Widget/` path.
* PHPStan level 5 — baseline regenerated at 285 entries (was
empty). Every entry is either a FS core magic-property access
or a Tools::cache / Tools::log call the plugin cannot statically
type. After baseline: `[OK] No errors`.
* Multi-PHP parity — full PHPUnit suite (168/281) runs green on
PHP 8.0.30, 8.1.34 and 8.2.30 via Docker harness.
Loose-ends re-audited after Fase 17 honesty pass:
* INT-06 · `UsernameGenerator::unique` now probes Moodle with
`getUsersByField('username', [candidate])` when the random_alias
strategy is selected, up to `RANDOM_ALIAS_MAX_PROBES = 5`
attempts. Fails open on probe error so a transient Moodle outage
does not stall onboarding; the subsequent enrol_user call
surfaces any real conflict through the standard path.
* FE-12 · 4 textareas (`CourseGroups/group_description`,
`CourseMessaging/message_text`, `UserNotes/note_text`,
`UserChat/chat_message`) gained `maxlength` attributes to
bound form submission size.
* BE-04 rev 2 · Per-course health re-probe counter in
`reconcileEnrolments`. After 3 suspect empties we abort the
whole reconciliation for the instance and log
`reconcile-aborted-probe-saturation`, preventing one probe
per course from DoS'ing a genuinely sick Moodle.
Other re-audit results logged in `docs/V2.0-POST-AUDIT-PLAN.md`:
DB-04 (no offending regex in Model/Trash) and DB-10 (UserMatcher
already normalises via strtolower+trim) confirmed as false
positives after deeper inspection. INT-02 (downloadFile log) —
log lines bound `url_host` + mime tag, no flood vector.
Full suite: 168/168 green on 8.0/8.1/8.2.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…[#AUDIT-F19]
Documentation catch-up so the repo truthfully reflects the v2.0
release-ready state after Fases 15–19:
* CLAUDE.md · rewritten §1 (estado: release-ready, gates verified),
§4.2/4.3 (primitives + status of every findings in the table),
§11 (every v2.0.0 blocker marked closed with SHA; Fase 16 HIGHs
closed with SHAs; §11.3–11.5 summarise MEDIUM/LOW/gates; §11.6
tracks the operational gates still outstanding), §13 (branch +
HEAD + 168/168 green on three PHP versions). Existing
"Última actualización 2026-04-17" bumped to 2026-04-18.
* SECURITY.md · new "New primitives shipped in the post-audit
phases" table with 21 entries mapping finding → fix → fase.
Documents the SEC-14 hash-chain race-condition limitation
explicitly so reviewers know the verifier tool is v2.1 scope.
* docs/EVENTS.md · adds a "Worker execution order" section
(DOC-02) documenting the registration-order guarantee, the
BadgeSync-then-Onboarding ordering for
`Model.MoodleUserMap.Insert`, and the idempotency keys.
* docs/adr/ · individual ADR files for ADR-0001 through ADR-0006
(TokenCipher, cascade cut, migration framework, soft-delete,
webhook envelope, BRAINSTORMING.md location). Each carries
status, context, decision, alternatives-considered and
consequences. Index lived in README.md since F18 wave 1; now
every linked page exists.
Full suite: 168/168 green on PHP 8.0 / 8.1 / 8.2.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…T-F19]
Records the Fase 19 stability-gates work in both the CHANGELOG
and the post-audit plan:
* 4 gates verified (PHPCS, PHPStan, PHP-CS-Fixer, multi-PHP).
* 3 loose-ends closed (INT-06 random_alias probe, FE-12 textarea
maxlength, BE-04 rev 2 early-exit).
* 1 documentation sweep (CLAUDE.md, SECURITY.md, EVENTS.md, ADRs).
Plan now tracks 104 items total (91 findings + 4 migrated + 8
Fase 19 gates + 1 follow-up).
Suite at close: 168 / 281 / 0 failures on 8.0.30 / 8.1.34 / 8.2.30.
Next: release gates — PGP, smoke test, v1 regression, OWASP ZAP,
tag v2.0.0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Trailing housekeeping so `git status` is clean before the v2.0-dev
push. Every staged diff below is a whitespace-only CRLF→LF
normalisation (same insertion/deletion count per file, no content
change) that git warned about on every previous `add`:
* LICENSE
* Translation/es_{AR,CL,CO,CR,DO,EC,GT,MX,PA,PE,UY}.json
* phpunit.xml.dist
`.gitignore` also covers `.phpunit.result.cache` now so the PHPUnit
run artifact stops appearing in `git status`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When `/updater` or a CLI request ran `Init::init()` before FS had initialised its default DB connection, `bootstrapSchema` tried to `new DataBase()` → `select/exec` and surfaced `mm-migrations-failed` + `mm-seed-failed` log entries with the inner message `Call to a member function escape_string() on null`. The try/catch already absorbed the actual failure, but the log noise confused operators reading `/updater` diagnostics and raised false alarms about the v2.0 migrations. Add an upfront `isDatabaseReady()` probe: trivial `SELECT 1` that returns false when the connection is not yet live. When the probe fails we return early silently; migrations are idempotent so the next request with a live connection applies them. Tests: 168/168 green on PHP 8.2. Verified in the local Docker stack — hitting `/` and `/updater` no longer produces `mm-*` log rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These paths are development-only artefacts that contributors need
on disk but should never land in a commit, the release ZIP, or a
PR diff:
* CLAUDE.md — agent context for Claude Code sessions
* .docs-dev/ — product-design brainstorming history
* docs/ — internal planning docs (V2.0-ACTION-PLAN,
V2.0-TASK-CHECKLIST, V2.0-POST-AUDIT-PLAN,
V2.1-BACKLOG, ARCHITECTURE, TROUBLESHOOTING,
EVENTS, WEBHOOK-RUNBOOK, SUPPORTED-VERSIONS,
PHP-INI-HARDENING, DEAD-CODE-AUDIT, QA-*,
RELEASE-CHECKLIST, adr/)
* qa/ — OWASP ZAP baseline config
Updates:
* `git rm -r --cached` on the 27 previously-tracked files so the
next push removes them from the remote branch. The files
themselves remain in the working tree.
* `.gitignore` simplified to ignore all four paths outright; the
long list of `!docs/<file>.md` exceptions is no longer needed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
FS 2025.9 replaced three externally-facing APIs the plugin depended
on with FS-native classes that share no inheritance with the Symfony
HttpFoundation + Tools::cache() predecessors. The plugin crashed on
every controller that touched the client IP, the header bag, the
CSP helper, or any cache-backed primitive.
Errors surfaced post-update:
* TypeError in CspHeader::apply (expected Symfony Response, got
FacturaScripts\Core\Response — final, unrelated class)
* Call to undefined method FacturaScripts\Core\Request::getClientIp()
* Call to undefined method FacturaScripts\Core\Tools::cache()
* Data too long for column 'version' in moodle_schema_version on
F17 migration INSERTs (44-char keys exceed VARCHAR(40))
Compatibility layers:
* `Lib/Http/RequestCompat` — `clientIp($request)` routes to
FS Request::ip() when available, Symfony Request::getClientIp()
otherwise. `header($request, $name, $default)` centralises the
header-bag lookup which keeps working on both versions.
* `Lib/Cache/CacheCompat` — provides the pre-2025.9
`get/set/delete` contract with explicit per-entry TTL, wrapping
FS 2025.9 `Core\Cache` static API when present (which dropped
per-entry TTL) and falling back to legacy `Tools::cache()` when
not. TTL enforced via envelope `{__mm_v, __mm_exp}` evaluated
on read with lazy delete on miss.
* `Lib/Security/CspHeader::apply` argument retyped from
`Symfony\Component\HttpFoundation\Response` to untyped; guards
on `is_object && isset($response->headers)` and uses only
`$headers->set/has` which both classes expose.
Migration column width:
* `SchemaMigrator::ensureVersionTable` bumped `version` column
width from VARCHAR(40) to VARCHAR(100). Back-compat path
`ensureVersionColumnWidth()` widens the column in place on
legacy installs via ALTER TABLE. Ships with the runtime check
on every boot so a mid-deploy restart picks up the fix.
Callers migrated:
* Request API — `ApiMoodleWebhook`, `MoodleCertificatePdf`,
`EditMoodleUserMap` (6 replacements across client IP + User-
Agent header).
* Cache API — `Cron`, `Worker/ContactSyncWorker`,
`Worker/PreEnrolmentWorker`, `Controller/MoodleDashboard`,
`Lib/Security/RateLimiter`, `Lib/Webhook/WebhookVerifier`,
`Lib/WorkQueue/IdempotencyGuard`, `Lib/Moodle/CircuitBreaker`
(13 call-sites total).
Verified post-fix on FS 2025.9 / PHP 8.4 stack:
* /MoodleDashboard HTTP 200
* /MoodleImportWizard HTTP 200
* /ListMoodleInstance HTTP 200
* /EditMoodleCertificate HTTP 200
* /MoodleCertificatePdf HTTP 404 (expected — no `code` in request)
* 0 error/critical log rows in the minute after the smoke test.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
FS 2025.9 now auto-renders a Twig template matching the controller class name whenever the controller returns without explicitly opting out via `setTemplate(false)`. Previous FS versions skipped the render when `$response->setContent(...)` had been called. Two plugin controllers write their response body directly and have no backing template, so post-update every hit raised: Twig loader error: Unable to find template "MoodleCertificatePdf.html.twig" (looked into: /Dinamic/View) Both now call `setTemplate(false)` at the top of their handler: * `MoodleCertificatePdf::serveCertificate` — emits the PDF bytes. * `ApiMoodleWebhook::(public|private)Core` — emits a JSON body. The call is `method_exists`-guarded so the fix stays compatible with older FS versions that either lack `setTemplate` or default to no render. Verified on the 2025.9 stack: * /MoodleCertificatePdf → HTTP 404 (auth-denied path) * /MoodleCertificatePdf?code=1 → HTTP 404 (record not found / not authorised) * /ApiMoodleWebhook?instance=1 → HTTP 405 (GET rejected, POST-only) * /MoodleDashboard / /MoodleImportWizard / list pages → HTTP 200 * 0 `error` or `critical` log rows post-hit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
F5.15 added `created_by` + `updated_by` columns to the six audit-trail-enabled tables via migration, but the PHP model classes never got matching property declarations. FS core populates them through reflection-style dynamic assignment, which PHP 8.2 flags with: Deprecated: Creation of dynamic property ...$created_by Deprecated: Creation of dynamic property ...$updated_by Visible on every ListMoodleCertificate (and sibling) page load under PHP 8.2+, which FS 2025.9 + Docker default to. Declare both as nullable string properties on each affected model: * Model/MoodleInstance.php * Model/MoodleUserMap.php * Model/MoodleCourseMap.php * Model/MoodleEnrolment.php * Model/MoodleCertificate.php * Model/MoodleCohort.php All six list pages now return HTTP 200 with 0 dynamic-property deprecations in the response body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both `MoodleCertificatePdf` and `ApiMoodleWebhook` are resource
endpoints — the former returns PDF bytes or 404 enumeration-
resistant, the latter a JSON API for Moodle to push events. Neither
is a navigable page, yet FS auto-listed both in the Moodle menu
because `getPageData()` didn't flag `showonmenu = false`.
Operator symptom: clicking "Gestión Moodle → certificate-pdf" in the
sidebar landed on a bare 404 "Registro no encontrado" response
(correct behaviour for that URL without a `code=<id>` query param,
but confusing as a menu entry).
Flip both to `$data['showonmenu'] = false`. For the existing
install, the pages row is patched via an UPDATE so the change takes
effect without a full plugin reinstall; fresh installs pick up the
flag from `getPageData()` directly.
UPDATE pages SET showonmenu = 0
WHERE name IN ('MoodleCertificatePdf','ApiMoodleWebhook');
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ales
ListMoodleCertificateTemplate (and six peers) showed the raw
translation key as the menu label — "certificate-templates",
"trash", "certificate-pdf", etc. — because no locale file carried
an entry for them. The controllers set `$data['title']` to those
keys assuming Tools::lang()->trans() would resolve them.
Seven keys were missing across every locale in `Translation/`:
* certificate-pdf
* certificate-template
* certificate-templates
* import-wizard
* moodle-audit-log
* moodle-webhook
* trash
Added proper translations for:
* es_ES and all 11 regional es_* variants (AR/CL/CO/CR/DO/EC/GT/
MX/PA/PE/UY) — same strings, as the plugin operates identically
across Spanish variants.
* en_EN, pt_BR, pt_PT, fr_FR, it_IT, de_DE, pl_PL, cs_CZ — locale-
appropriate wording.
* ca_ES, eu_ES, gl_ES, va_ES — ES fallback until dedicated
translations arrive (better than leaving the raw key visible).
168 entries added (7 keys × 24 locales).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Local-dev stacks (Docker-compose with a sibling Moodle on a private
network, e.g. http://webserver/moodle) always hit the F7.1 SSRF
guard because the hostname resolves to an RFC1918 address. Every
WS call returned `ssrf_rejected`, pages like `ListMoodleConversation`
surfaced the log warning to the operator even when the stack was
clearly a dev setup.
The existing SEC-03 HTTP relaxation used a single `FS_DEBUG` gate.
SSRF gets a stricter double gate:
* FS_DEBUG must be true (deployment-wide dev signal)
* instance.environment === 'development' (per-instance opt-in)
Both must hold. A prod deployment where an operator sneaks
`environment=development` into the DB row still fails the gate
because FS_DEBUG stays false, and vice-versa. The double condition
also means the bypass never leaks onto the production Moodle
instance of a mixed-env stack.
New helper:
* `MoodleClient::isDevInstance(MoodleInstance): bool` — encapsulates
the per-instance check, reused by callApi + downloadFile.
Log separation:
* Bypass active → `moodle-ssrf-dev-bypass` / `moodle-download-
ssrf-dev-bypass` at `notice` level (audit trail that the
bypass fired).
* Gate held → existing `moodle-ssrf-rejected` at `warning`.
Applied on both `callApi` (WS) and `downloadFile` (binary assets).
Verified: `/ListMoodleConversation` now returns HTTP 200 on the
local Docker stack (FS_DEBUG=true + webserver Moodle flagged
development). Prod posture untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…P 8.2) Same root cause as the earlier `created_by/updated_by` fix: every column the v2.0 migrations added to a plugin table must be declared as a `public $X` on the corresponding model, otherwise PHP 8.2 flags FS core's reflective assignment as a dynamic-property deprecation on every page render. Surfaced as a wall of `Deprecated: Creation of dynamic property MoodleUserMap::$badge_sync_needed` while loading ListMoodleUserMap or any view that iterates those models. Auditted `Update/v2_0.php` end-to-end and added the missing props: * MoodleInstance — health_fail_count (BE-06) * MoodleUserMap — badge_sync_needed (F6.1) * MoodleEnrolment — idfactura_archived (F5.5) * MoodleRoleMap — created_at, updated_at (F5.22) * MoodleAuditLog — row_hash, prev_hash (SEC-14) Each declared with an accurate doc-type (?string for nullable timestamps, int for counters/flags) and a one-line audit reference so future readers know the provenance. Also dropped the incorrectly-added `deleted_at` from MoodleInstance, MoodleCourseMap and MoodleCertificate (those tables are NOT in the F5.20 soft-delete target list — only user_map / enrolments / cohorts are, and those already have the property declared). Smoke-tested every List page: /ListMoodleUserMap / Enrolment / Certificate / Instance / Cohort / RoleMap / AuditLog / CourseMap → all HTTP 200, zero dynamic- property deprecations in the response body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two operator-visible annoyances when a Moodle instance went sick:
1. The FS log panel rendered raw machine tags — "moodle-ssrf-dev-
bypass", "moodle-circuit-open", "moodle-ssrf-rejected" — instead
of human-readable sentences. Nothing in FS core translates log
messages at render time, so we have to translate at emit time.
2. A single page that fires 6 WS calls against an instance whose
circuit breaker is OPEN produced 6 identical `moodle-circuit-
open` rows. Six logs to describe one outage.
### Fix #1 — translate at emit time
All user-facing `Tools::log()->(notice|warning|error)('tag', […])`
calls in `MoodleClient` now route through
`Tools::lang()->trans('tag', $context)` with `%placeholder%` keys.
When the translation key is missing, Tools::lang() returns the key
unchanged, preserving the machine-tag identity for log-grep tools.
Added 13 new translation keys across 24 locales (312 entries total):
* moodle-ssrf-dev-bypass
* moodle-ssrf-rejected
* moodle-insecure-transport-rejected
* moodle-circuit-open
* moodle-response-too-large
* moodle-ws-partial-failures
* moodle-download-ssrf-dev-bypass
* moodle-download-ssrf-rejected
* moodle-download-bad-mime
* moodle-download-too-large
* moodle-webhook-handler-failed
* moodle-webhook-unknown-event
* moodle-webhook-invalid-payload
The ES family (12 regional variants + ca/eu/gl/va) gets the same
castellano text; en_EN plus de/fr/it/pt_BR/pt_PT/pl/cs get
locale-native wording.
### Fix #2 — per-request dedup
`MoodleClient::logOnce($key, callable)` tracks emitted keys in a
static array and skips subsequent calls with the same key. Scope
is the PHP process (one web request or one cron job), so the
dedup resets between requests. Applied to:
* `moodle-ssrf-dev-bypass` — one notice per instance per request.
* `moodle-circuit-open` — one notice per instance per request.
Other tags (outright failures) still log every hit so the trail
stays detailed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…retries
Follow-up to the MoodleClient log sweep. The retry/circuit layer
was still emitting raw machine tags when a Moodle outage triggered
the resilience path:
retry-policy-exhausted
retry-policy-exhausted
retry-policy-exhausted
circuit-breaker-tripped
retry-policy-backoff (x 6)
Two fixes, same pattern as the MoodleClient commit:
* Tags translated at emit time via `Tools::lang()->trans($tag,
$context)`. When the translation key is missing, lang() returns
the tag unchanged, so log-grep tooling still works.
* Per-request dedup in RetryPolicy via a new static
`$loggedOnce` array, scoped to the PHP process and reset
between requests. A single page that re-invokes the same
retry-wrapped call now leaves exactly one backoff + one
exhausted log entry per unique `$tag`, not six.
Six new translation keys added to 24 locales (144 entries total):
* retry-policy-permanent
* retry-policy-exhausted
* retry-policy-backoff
* circuit-breaker-tripped
* circuit-breaker-reopened
* circuit-breaker-closed
ES variants share the castellano text; en_EN + 7 other locales get
native wording.
CircuitBreaker log-once is not needed because each trip/reopen/
close transition naturally fires at most once per cycle —
dedup there would only mask genuine state changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…onical namespace (FS 2026) FS 2026 (the "Core: 2026" build the operator is running) removed the shortcut namespace `FacturaScripts\Core\DataSrc\DataBaseWhere` that was introduced as a convenience re-export in 2025.x. Every `use FacturaScripts\Core\DataSrc\DataBaseWhere;` now autoloads nothing and throws: Uncaught Error: Class "FacturaScripts\Core\DataSrc\DataBaseWhere" not found The canonical location — `FacturaScripts\Core\Base\DataBase\ DataBaseWhere` — still ships and is API-compatible. Rewritten imports in 9 files: * Controller/ListMoodleUserMap.php * Controller/ListMoodleEnrolment.php * Controller/ListMoodleTrash.php * Controller/ListMoodleAuditLog.php * Controller/MoodleCertificatePdf.php * Lib/Webhook/Handler/EnrolmentCreatedHandler.php * Lib/Webhook/Handler/EnrolmentDeletedHandler.php * Lib/Webhook/Handler/CourseCompletedHandler.php * Lib/Webhook/Handler/UserUpdatedHandler.php Also touched: inline fully-qualified references inside `ListMoodleAuditLog::createAuditView()` which built DataBaseWhere objects via the long form. The remaining `Core\DataSrc` imports (`Empresas`, used in `ListMoodleInstance` and `CertificatePdfGenerator`) are still present in FS 2026 and do not need a change. Smoke-tested on FS 2026: /ListMoodleUserMap → HTTP 200 /ListMoodleEnrolment → HTTP 200 /ListMoodleTrash → HTTP 200 /ListMoodleAuditLog → HTTP 200 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
FS 2026 renamed the `SubRequest::getBoolean()` convenience helper to `getBool()` (aligned with the rest of the family: `getInt`/`getFloat`/`getString`…). The dashboard controller called the old name on `$this->request->query` to parse `?refresh=1`, producing: Uncaught Error: Call to undefined method FacturaScripts\Core\Internal\SubRequest::getBoolean() in Controller/MoodleDashboard.php:61 Guarded via `method_exists` so both the new `getBool` and the legacy `getBoolean` resolve correctly. The plugin stays loadable on FS 2025.x + 2026 from the same branch. Smoke test on FS 2026: /MoodleDashboard → HTTP 200 /MoodleDashboard?refresh=1 → HTTP 200 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The operator reported 10 raw keys visible on EditMoodleCertificateTemplate: elements-to-show, show-unique-hash, show-date-issued, show-issuer, certificate-texts, title-text, subtitle-text, completed-text, issuer-label, verify-text An audit of every XMLView/*.xml surfaced 60 keys that had no entry in Translation/*.json. Some (id, name, date, status, email, ip, …) are already in FS core's es_ES and rendered correctly by inheritance. The remaining 42+23 = 65 keys (the certificate-template ones plus forms-wide commons like codcliente, fullname, idinstance, …) had no translation anywhere, so FS rendered the raw key. This commit adds every missing key (64 unique keys) across the 24 locales shipped in Translation/, totalling 1,536 new entries. es_* + ca/eu/gl/va share the castellano copy; en_EN + 7 other locales get native English wording. Verified post-apply: an XMLView-wide scan reports 0 residual missing keys against es_ES + FS core. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…blank)
MoodleDashboard canvases rendered empty — KPIs showed correct
numbers but none of the four charts drew. Root cause: the template
loaded Chart.js from `node_modules/chart.js/dist/chart.umd.js`
under the FS root, but FS 2026 ships Chart.js 2.9 (`Chart.min.js`,
uppercase C) in `node_modules/`. The 4.x UMD file does not exist
at that path, so `<script src="…/chart.umd.js">` 404'd and
`window.Chart` stayed undefined.
Three-part fix:
1. Vendor Chart.js 4.4.2 directly inside the plugin at
`Assets/JS/vendor/chart.umd.js` (205 KB). Template now loads
from there, so the plugin no longer depends on the host FS
`node_modules/` version.
2. Runtime CDN fallback: if the vendored file is missing
post-deploy the template injects a `<script>` from
`cdn.jsdelivr.net/npm/chart.js@4.4.2`. Air-gapped installs
can delete the five fallback lines.
3. CspHeader `script-src` adds `https://cdn.jsdelivr.net` so the
fallback injection is not blocked by the policy.
Also fixed the `.gitignore` pattern: `vendor/` matched every
`vendor` directory in the tree (including `Assets/JS/vendor/`,
which is asset vendoring, not composer). Anchored to
`/vendor/` so only the top-level composer directory is ignored.
Verified on FS 2026:
* /MoodleDashboard → HTTP 200
* /Plugins/MoodleManagement/Assets/JS/vendor/chart.umd.js → HTTP 200
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Dashboard gains two operator-requested charts and the four existing
ones get redistributed into a symmetric 3-column grid so everything
fits above the fold:
Row 1: By Status (pie) Progress distribution By Method (pie)
Row 2: By Month (bar) Top Courses (bar) Certificates/Mo (line)
* **Progress distribution** — bar chart over 6 fixed buckets
(0% / 1-25% / 26-50% / 51-75% / 76-99% / 100%). Reads
`moodle_enrolments.progress_percent` populated every 6 h by
the progressSync cron (F10.2). Colour-coded red→green so
operators spot "stuck at 0%" immediately. Excludes soft-
deleted and NULL-progress rows.
* **Certificates by month** — line + area chart, last 6 months,
grouped by `date_issued` month. Parallel to the existing
"enrolments by month" bar so operators can eyeball the
matriculation → completion conversion rate. Postgres-safe
date expression via the same conditional idiom used above.
Pre-serialised via JsonForScript to keep FE-01/FE-02 safety.
Four new i18n keys × 24 locales = 96 entries:
* progress-distribution
* certificates-by-month
* certificates
* enrolments
Full-page smoke on /MoodleDashboard?refresh=1 → HTTP 200, no
Uncaught errors in the response body. Bundle size 43 KB (was
~35 KB).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Versión 2.0 check point.