diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7ee3f59a2..4721e514d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -193,6 +193,9 @@ jobs: # this install" — without it the user-visible promise lies). # A release that silently drops any of the three makes every # downstream redistribution non-compliant. + # - demos/.htaccess — blocks direct web access to uploaded + # demos on Apache tarball installs. It's a dotfile inside a + # gitignored directory, so it's the easiest file to lose. - name: Verify required files are bundled in the zip run: | pkg="sourcebans-pp-${GITHUB_REF_NAME}.webpanel-only" @@ -202,7 +205,8 @@ jobs: "${pkg}/includes/vendor/smarty/smarty/src/Smarty.php" \ "${pkg}/LICENSE.txt" \ "${pkg}/LICENSE-plugins.txt" \ - "${pkg}/THIRD-PARTY-NOTICES.txt"; do + "${pkg}/THIRD-PARTY-NOTICES.txt" \ + "${pkg}/demos/.htaccess"; do if ! unzip -l "${pkg}.zip" "${f}" >/dev/null 2>&1; then echo "::error::Missing required artifact file in ${pkg}.zip: ${f}" missing=1 diff --git a/.gitignore b/.gitignore index 98bf06061..f894ede87 100644 --- a/.gitignore +++ b/.gitignore @@ -56,12 +56,14 @@ Temporary Items # Uploaded demo evidence (production) + synthetic demo files written by # `./sbpp.sh db-seed` (dev). `.gitkeep` is the structural marker so the # directory exists for fresh tarball installs and tests that probe -# `is_dir(SB_DEMOS)`. Both upload paths land MD5-hash basenames here; +# `is_dir(SB_DEMOS)`; `.htaccess` blocks direct web access on tarball +# installs. Both upload paths land 32-char hex basenames here; # checking them in would (a) blow up the working tree on every db-seed # and (b) leak real ban evidence into the source tree on a self-hoster # install. /web/demos/* !/web/demos/.gitkeep +!/web/demos/.htaccess # Local stash file used by the install-screenshot capture flow # (`docs/README.md` → "Capturing screenshots"). Gitignored so an diff --git a/AGENTS.md b/AGENTS.md index 2b9a236df..68687e9c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -674,11 +674,11 @@ and screenshot work both depend on it. the CONSOLE + `admin/admin` rows, then inserts the new synthetic dataset (rows AND on-disk demo payloads). The wipe scope is the set of filenames currently in `:prefix_demos`; anything else in - `SB_DEMOS` (the tracked `.gitkeep`, manual panel uploads, a dev's - SDK demoviewer working copy) survives untouched. `./sbpp.sh - db-reset` carries a sibling cleanup that strips MD5-named - basenames from `web/demos/` (those rows vanished with the DB - volume so they'd otherwise be permanent orphans). + `SB_DEMOS` (the tracked `.gitkeep` + `.htaccess`, manual panel + uploads, a dev's SDK demoviewer working copy) survives untouched. + `./sbpp.sh db-reset` carries a sibling cleanup that strips + 32-char-hex basenames from `web/demos/` (those rows vanished with + the DB volume so they'd otherwise be permanent orphans). - Deterministic given a fixed `--seed` — `mt_srand($seed)` pins PHP's RNG so two devs hit the same names/reasons/timestamps AND so `array_rand` picks the same parent IDs for demo attachment across @@ -4966,6 +4966,23 @@ the spec, target a 1920px viewport, not 1440px. motivated #1271 (sidebar's sticky CB short of the document) doesn't apply — `position: fixed` removes the drawer from flow, so it cannot grow `.app`'s height. +- Naming an uploaded demo from anything other than a CSPRNG, or + letting the web server serve `web/demos/` directly. Uploaded demos + are untrusted bytes stored without an extension and must only ever + reach a browser through `getdemo.php`. Three layers enforce this; + keep all three: + `Sbpp\Upload\UploadHandler::randomDiskName()` (CSPRNG, 32-char + lowercase hex, shared by `page.submit.php` and the admin upload + popup); a `demos/` deny in `docker/apache/sbpp-prod.conf`, + `docker/apache/sbpp-dev.conf`, and the shipped `web/demos/.htaccess` + (tarball installs; the prod image ignores it via `AllowOverride None` + and its dotfile rule); and `X-Content-Type-Options: nosniff` from + `web/init.php` plus `Header always set` in both Apache confs. + `getdemo.php` (octet-stream + attachment + nosniff + containment) is + the only download path. Never add a feature that links to + `demos/` directly. Regression guards: + `web/tests/integration/DemoUploadHardeningTest.php` + + `web/tests/e2e/specs/flows/demo-direct-access.spec.ts`. ## Where to find what @@ -4985,7 +5002,8 @@ the spec, target a 1920px viewport, not 1440px. | Render a page | `web/pages/.php` + `web/includes/View/*View.php` | | Add a new edit page in the admin.edit.* cluster (e.g. `admin.edit..php`) | `web/pages/admin.edit..php` (the page handler — thin "validate input, build View, render" shape) + `web/includes/View/AdminEditView.php` (typed View DTO) + `web/themes/default/page_admin_edit_.tpl` (template). Shared helpers live in `web/pages/_admin_edit_helpers.php` (`sbpp_admin_edit_die_with_toast()` for permission / not-found guards, `sbpp_admin_edit_emit_tail_script()` for form-success / validation-error feedback that fires `window.SBPP.showToast()` and writes errors into `.msg` divs, `sbpp_admin_edit_collect_rehash_sids()` for the post-save Rehash Admins step). Anti-patterns to avoid: inline `echo '
...'` blocks, `echo '
…'` banners, MooTools `$('id').value` reads, legacy JS handler names (`ButtonOver`, `ProcessEditAdminPermissions`, etc.) — all swept as part of the v2.0 rewrite. CSRF gate every POST via `\CSRF::rejectIfInvalid();` after the `isset($_POST[''])` arm. | | Wire a `window.opener.(...)` slot on the parent template of a popup file-upload page (e.g. `window.icon`, `window.demo`, `window.mapimg`) | The parent template defines `window. = function (filename) { … }` inside an inline `` and the call throws `TypeError: window.opener.icon is not a function` — popup never closes, the parent form's hidden input never updates, the uploaded asset is orphaned on disk. The `UploadHandler` emits the call unconditionally; the parent's job is to be ready for it. | -| Add a popup file-upload page (demo / icon / mapimage / new asset type) | `Sbpp\Upload\UploadHandler::handle()` (`web/includes/Upload/UploadHandler.php`). The page handler at `web/pages/admin.upload.php` is a thin wrapper passing the per-page knobs (`permission` mask, `field` `$_FILES` key, `allowed` extensions, `destDir`, `callback` JS function name on `window.opener`, `auditOk` / `auditFmt` / `errorMsg` / `title` / `formName` / `formats` strings, optional `renameToHash` for demo-style randomised filenames). The handler runs CSRF + permission check, sanitises `$_FILES[…]['name']` via `sanitiseName()` (basename + strip backslashes + trim leading dots — defends LFI on the icon / mapimage paths where the filename hits disk), `move_uploaded_file()`s to the destination, calls `Log::add(LogType::Message, …)`, and on success emits the `` blob the parent page picks up. The three reference call sites (`admin.uploaddemo.php`, `admin.uploadicon.php`, `admin.uploadmapimg.php`) are 30-line wrappers; new asset types should match that line budget. Anti-pattern: hand-rolling the move / log / popup-emission sequence per page (the pre-v2.0 shape). | +| Add a popup file-upload page (demo / icon / mapimage / new asset type) | `Sbpp\Upload\UploadHandler::handle()` (`web/includes/Upload/UploadHandler.php`). The page handler at `web/pages/admin.upload.php` is a thin wrapper passing the per-page knobs (`permission` mask, `field` `$_FILES` key, `allowed` extensions, `destDir`, `callback` JS function name on `window.opener`, `auditOk` / `auditFmt` / `errorMsg` / `title` / `formName` / `formats` strings, optional `renameToHash` for demo-style randomised filenames via `UploadHandler::randomDiskName()`, which `page.submit.php` also uses; never derive a stored name from time or user input). The handler runs CSRF + permission check, sanitises `$_FILES[…]['name']` via `sanitiseName()` (basename + strip backslashes + trim leading dots — defends LFI on the icon / mapimage paths where the filename hits disk), `move_uploaded_file()`s to the destination, calls `Log::add(LogType::Message, …)`, and on success emits the `` blob the parent page picks up. The three reference call sites (`admin.uploaddemo.php`, `admin.uploadicon.php`, `admin.uploadmapimg.php`) are 30-line wrappers; new asset types should match that line budget. Anti-pattern: hand-rolling the move / log / popup-emission sequence per page (the pre-v2.0 shape). | +| Serve or protect uploaded demo files (`web/demos/`) | `web/getdemo.php` is the only download path (octet-stream + attachment + nosniff + `SB_DEMOS` containment). Direct access is denied by `docker/apache/sbpp-prod.conf`, `docker/apache/sbpp-dev.conf`, and `web/demos/.htaccess` (tarball installs; whitelisted in `.gitignore`, asserted in the release zip by `release.yml`). Self-hoster nginx / IIS rules live in `docs/src/content/docs/getting-started/quickstart.mdx` ("Block direct access to demos"). Guards: `web/tests/integration/DemoUploadHardeningTest.php` + `web/tests/e2e/specs/flows/demo-direct-access.spec.ts`. | | Edit a template | `web/themes/default/*.tpl` | | Reuse the moderation-queue card layout (admin submissions / protests, mobile-stacked summary rows) | `web/themes/default/css/theme.css` (`.queue-row`, `.queue-row__body`, `.queue-row__date` — #1207 PUB-2). Apply by adding `class="queue-row …"` to the outer `
` and dropping the inline `flex` / `flex-shrink:0` styles from the summary children. | | Add visible row actions to a table-rendered admin list (Edit / Unmute / Remove buttons + responsive mobile-card mirror) | `web/themes/default/page_admin_admins_list.tpl` is the density reference: icon-only `` / `