From 7a94e5675159fda6903e4ddc65d7961aae883590 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Wed, 16 Sep 2026 12:35:41 +0200 Subject: [PATCH] docs: record the verification traps this stack has Four facts that are not derivable from the code and each cost a wrong conclusion while the 5.2.3 upgrade was being done. Three Moodle checks cannot return the answer you are looking for unless they are set up correctly, and all three reported the opposite of the truth before that was noticed: configrw calls is_writable(), which is true for root and docker exec defaults to root; publicpaths fetches $CFG->wwwroot over HTTP, so a wwwroot the container cannot resolve makes every path look unreachable rather than refused; and get_update_info() only compares versions for core, returning the cached API response unfiltered for a plugin, so it kept reporting an update that had just been installed. The fourth is the discriminator that found two regressions in the nginx deny rules before they shipped: a status code does not say who answered, the X-Powered-By header does. Also writes down what the deny rules must keep - the (?!.*\.php/) guard that keeps them off Moodle's slash arguments, and why they refuse the tests/ tree rather than anything named behat - and that routerconfigured and the r.php fallback are one setting in two places. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_01FRHeDxbgUnv868eNhVCCsr Agent-Host: 32116e Signed-off-by: Sebastian Mendel --- AGENTS.md | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..e38ad63 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,67 @@ +# Working on this repository + +Notes that are not derivable from the code and have each cost a wrong +conclusion at least once. + +## Verifying against a running stack + +Most checks here are run through Moodle itself, and three of them cannot +return the answer you are looking for unless you set them up correctly. Each +one reported the opposite of the truth before it was noticed. + +**Run Moodle's checks as `www-data`, not as root.** `docker exec` defaults to +root, and the `configrw` check calls `is_writable()` on `config.php`, which is +true for root whatever the file mode says. The check can then never pass: + +```bash +docker compose exec -T -u www-data moodle php /var/www/html/admin/cli/checks.php +``` + +**Give the container a `wwwroot` it can resolve.** The `publicpaths` check +fetches `$CFG->wwwroot . '/' . $path` over HTTP. With `MOODLE_URL=http://localhost:8099`, +`localhost` inside the container is the container itself, every request fails, +and a refused path is indistinguishable from a reachable one. In a local test +stack use a name the compose network resolves, such as `http://nginx`. + +**`get_update_info()` does not answer "is an update still pending" for a +plugin.** The version comparison in `lib/classes/update/checker.php` sits +behind `if ($component === 'core')`; for plugins the cached API response is +returned unfiltered. Use the plugin manager instead: + +```php +\core_plugin_manager::instance()->get_plugin_info($component)->available_updates() +``` + +**A status code does not say who answered.** When checking whether nginx +refuses a path or hands it to PHP, compare the presence of the `X-Powered-By` +header, not the status: a `pluginfile.php` URL for a file that does not exist +is a 404 either way. This is how two regressions in the deny rules were found +before they shipped. + +## nginx + +The deny rules in `docker/nginx/nginx.conf` mirror the patterns core's +`report_security` public-paths check probes for — the list lives in +`lib/classes/check/environment/publicpaths.php`, and its own comment suggests +generating web server config from it. When core adds a pattern, add it here. + +Two constraints those rules must keep: + +- Every rule carries `(?!.*\.php/)`. Moodle serves user content through slash + arguments on a script, so `/pluginfile.php/.../readme.pdf` and a course + folder named `behat` are ordinary downloads. +- Refuse the `tests/` tree rather than anything named `behat` or `fixtures`. + `admin/tool/behat` is an admin tool whose stylesheet and JavaScript are + served. + +`$CFG->routerconfigured` and the `try_files … /r.php` fallback are one setting +in two places and only work together — with the flag off the router prefixes +its own base path with `/r.php` and an unprefixed request no longer matches, +so the rewrite alone answers 404. + +## Upgrades + +The Moodle sources ship inside the image, so an upgrade is a new image. The +entrypoint copies them into the code volume and runs +`admin/cli/upgrade.php --non-interactive` itself; a failure aborts the +entrypoint rather than serving a half-upgraded site.