fix: show default theme author/version on Settings > Themes (#1466)

AGENTS.md

@@ -4546,6 +4546,7 @@ contributions without contacting every contributor individually.
 | Add a cross-repo JSON contract (vendored schema lock + reader + extractor parity test) | `web/includes/Telemetry/Schema1.php` is the reference shape (`payloadFieldNames(): list<string>` over a Draft-7 JSON Schema lock file). Pair with one PHPUnit extractor parity test (collect() vs. lock file in both directions). The schema lock file is the single source of truth — don't mirror the field list into a markdown doc paired with a separate parity test, that pattern was tried for telemetry and removed because the duplication paid for the drift risk it created. Sync via a manual `make sync-<subsystem>-schema` target — no scheduled auto-PR. See "Cross-repo JSON contracts" under Conventions. |
 | Display a user's own permission flags grouped by category | `Sbpp\View\PermissionCatalog::groupedDisplayFromMask($mask)` (`web/includes/View/PermissionCatalog.php`). Adding a new flag to `web/configs/permissions/web.json` requires a paired entry in `WEB_CATEGORIES`; `PermissionCatalogTest` enforces it. |
 | Live-preview Markdown in a settings textarea | `system.preview_intro_text` JSON action + `web/themes/default/page_admin_settings_settings.tpl` (`.dash-intro-editor` / `.dash-intro-preview`) |
+| Regex-read `theme.conf.php` metadata for the admin Settings → Themes picker (without executing the manifest) | `Sbpp\Theme\ThemeConf` (`web/includes/Theme/ThemeConf.php`) — `parseDefine()` + `sanitizeLink()` / `sanitizeScreenshotFilename()`; wired from `web/pages/admin.settings.php` discovery loop (#1466). JSON theme preview (`api_system_sel_theme`) still `include`s the manifest. Regression: `web/tests/integration/ThemeConfParseTest.php`. |
 | Build an empty-state surface (first-run vs filtered, primary/secondary CTAs) | `.empty-state` rules in `web/themes/default/css/theme.css` + reference shapes in `page_servers.tpl`, `page_dashboard.tpl`, `page_bans.tpl`, `page_comms.tpl`, `page_admin_audit.tpl`, `page_admin_bans_protests.tpl`, `page_admin_bans_submissions.tpl` |
 | Subdivide an admin route into `?section=<slug>` URLs (servers, mods, groups, comms, settings, **admins**, **bans**) | `web/pages/admin.settings.php` is the long-standing reference; #1239 brought servers / mods / groups / comms onto the same shape; #1259 unified the chrome on the Settings-style vertical sidebar; #1275 brought admins (`admins` / `add-admin` / `overrides`) and bans (`add-ban` / `protests` / `submissions` / `import` / `group-ban`) onto the same shape, deleting the page-level ToC (`page_toc.tpl`) along the way so `?section=` is now the **only** sub-route nav contract. The shared partial is `web/themes/default/core/admin_sidebar.tpl` (parameterized on `tabs` / `active_tab` / `sidebar_id` / `sidebar_label`); `web/includes/View/AdminTabs.php` (`Sbpp\View\AdminTabs`) opens `<div class="admin-sidebar-shell">`, emits the `<aside>` + link list, opens `<div class="admin-sidebar-content">`, and the page handler closes both wrappers (`echo '</div></div>'`) AFTER `Renderer::render(...)`. Each `$sections` entry carries `slug` + `name` + `permission` + `url` + `icon` (Lucide name); the link emits `<a href="?p=admin&c=<page>&section=<slug>" data-testid="admin-tab-<slug>" aria-current="page">` — never `<button onclick="openTab(...)">` (the JS handler was deleted at #1123 D1). See "Sub-paged admin routes" in Conventions. |
 | Render sub-views inside a Pattern A section (e.g. protests / submissions current-vs-archive) | `?view=<slug>` query param + a server-rendered `.chip-row` of real anchors (each carries `data-active="true|false"` + `aria-selected`). Reference: the protests / submissions chip rows in `web/pages/admin.bans.php` (`?section=protests&view=archive` / `?section=submissions&view=archive`). Pre-#1275 the chips called `Swap2ndPane()` — a `web/scripts/sourcebans.js` helper deleted at #1123 D1, leaving them dead — and the page rendered both views simultaneously. The new shape only renders the active view's data path; back/forward and link sharing both work. |

ARCHITECTURE.md

@@ -97,6 +97,7 @@ web/
 │   ├── View/                 Sbpp\View\* — typed Smarty view-model DTOs
 │   ├── View/Install/         Sbpp\View\Install\* — install-wizard step DTOs (#1332)
 │   ├── Markup/               Sbpp\Markup\IntroRenderer — admin Markdown -> safe HTML
+│   ├── Theme/                Sbpp\Theme\ThemeConf — regex-read theme.conf.php metadata for the admin Themes picker (#1466)
 │   ├── Servers/              Sbpp\Servers\{SourceQueryCache, RconStatusCache} — per-(ip, port) cache around the xPaw A2S probe (#1311) + per-sid cache around the RCON `status` command (#PLAYER_CTX_MENU)
 │   ├── Upload/               Sbpp\Upload\UploadHandler — shared file-upload handler (perm + CSRF + extension allowlist + filename sanitiser + popup chrome) for the demo / icon / mapimage popup pages
 │   ├── Mail/                 Sbpp\Mail\{Mail,Mailer,EmailType} — Symfony Mailer wrapper + enum

docs/src/content/docs/customization/translating.md

@@ -21,18 +21,26 @@ translations, and select your new theme in the panel's settings.
    the copy to something distinctive (e.g. `german`, `default-fr`).
 
 2. **Edit `theme.conf.php`** in your new theme. Update the metadata
-   so the panel's theme picker displays your theme correctly:
+   so the panel's theme picker displays your theme correctly. The
+   shipped default theme uses **single-quoted** string literals; double
+   quotes work too. Use plain `define('key', 'value');` lines — the
+   picker regex-reads the file without executing it, so concatenation
+   and other PHP expressions are not reflected on the Settings → Themes
+   cards (the JSON theme-preview API does execute the manifest).
 
    ```php
    <?php
-   define('theme_name',       "SourceBans++ Deutsch");
-   define('theme_author',     "Your name");
-   define('theme_version',    "1.0.0");
-   define('theme_link',       "https://your-site.example.com");
-   define('theme_screenshot', "screenshot.jpg");
-   ?>
+   define('theme_name', 'SourceBans++ Deutsch');
+   define('theme_author', 'Your name');
+   define('theme_version', '1.0.0');
+   define('theme_link', 'https://your-site.example.com');
+   define('theme_screenshot', 'screenshot.jpg');
    ```
 
+   Leave `theme_link` empty if you have no homepage; use only the
+   screenshot **filename** (not a path) — it must live in your theme
+   directory next to the templates.
+
 3. **Translate each `.tpl` file.** Open the `.tpl` files in your
    theme directory and replace the English copy with your
    translation. Don't touch the parts wrapped in `{...}`. Those

web/api/handlers/system.php

@@ -250,6 +250,11 @@ function api_system_check_version(array $params): array
     ];
 }
 
+/**
+ * Preview a theme manifest. Includes theme.conf.php so PHP expressions
+ * in the file are honoured; the admin Settings → Themes grid uses
+ * {@see \Sbpp\Theme\ThemeConf::parseDefine()} instead (no second define()).
+ */
 function api_system_sel_theme(array $params): array
 {
     $theme = rawurldecode((string)($params['theme'] ?? ''));

web/includes/Theme/ThemeConf.php

@@ -0,0 +1,74 @@
+<?php
+// SourceBans++ (c) 2014-2026 SourceBans++ Dev Team
+// Licensed under the Elastic License 2.0.
+// See LICENSE.txt for the full license text and THIRD-PARTY-NOTICES.txt for attributions.
+
+declare(strict_types=1);
+
+namespace Sbpp\Theme;
+
+/**
+ * Read theme.conf.php metadata without executing the file (PHP cannot
+ * define() the same constant twice in one request).
+ *
+ * The admin Settings → Themes picker uses this regex reader. JSON API
+ * handlers ({@see api_system_sel_theme}) include the manifest and read
+ * live constants — richer PHP expressions work there but not in the picker.
+ */
+final class ThemeConf
+{
+    /**
+     * Pluck a `define('<key>', "<value>")` or `define('<key>', '<value>')`
+     * literal out of a theme.conf.php source string.
+     */
+    public static function parseDefine(string $src, string $key, string $default): string
+    {
+        $pattern = '/define\(\s*\'' . preg_quote($key, '/') . '\'\s*,\s*(?:"([^"]*)"|\'((?:[^\'\\\\]|\\\\.)*)\')\s*\)\s*;/';
+        if (preg_match($pattern, $src, $m) !== 1) {
+            return $default;
+        }
+
+        if (preg_match('/,\s*"/', $m[0]) === 1) {
+            $value = $m[1];
+        } else {
+            $value = self::unescapeSingleQuoted($m[2] ?? '');
+        }
+
+        return strip_tags($value);
+    }
+
+    /**
+     * Allow only http(s) homepage URLs for theme cards; empty is valid.
+     */
+    public static function sanitizeLink(string $link): string
+    {
+        $link = trim(strip_tags($link));
+        if ($link === '') {
+            return '';
+        }
+        if (preg_match('#^https?://#i', $link) !== 1) {
+            return '';
+        }
+
+        return $link;
+    }
+
+    /**
+     * Screenshot filenames must stay inside the theme directory (basename only).
+     */
+    public static function sanitizeScreenshotFilename(string $name, string $default): string
+    {
+        $name = trim(strip_tags($name));
+        $base = basename(str_replace('\\', '/', $name));
+        if ($base === '' || $base === '.' || $base === '..' || str_contains($base, '/')) {
+            return $default;
+        }
+
+        return $base;
+    }
+
+    private static function unescapeSingleQuoted(string $value): string
+    {
+        return str_replace(['\\\\', "\\'"], ['\\', "'"], $value);
+    }
+}

web/pages/admin.settings.php

@@ -12,6 +12,7 @@
 use Sbpp\View\AdminFeaturesView;
 use Sbpp\View\AdminLogsView;
 use Sbpp\View\AdminSettingsView;
+use Sbpp\Theme\ThemeConf;
 use Sbpp\View\AdminThemesView;
 use Sbpp\View\Perms;
 use Sbpp\View\Renderer;
@@ -258,8 +259,8 @@
  * constant twice in one process, so the regex is the only way to
  * enumerate without resetting state). B18 keeps the regex-based
  * discovery and just enriches it with author / version / link /
- * screenshot — every theme.conf.php is expected to declare those four
- * constants.
+ * screenshot — every theme.conf.php is expected to declare those five
+ * define() keys (single- or double-quoted string literals; see #1466).
  */
 $validThemes = [];
 $themesDir   = opendir(SB_THEMES);
@@ -275,32 +276,21 @@
         $confSrc = (string) @file_get_contents($confPath);
         $validThemes[] = [
             'dir'        => $filename,
-            'name'       => themeConfMatch($confSrc, 'theme_name', $filename),
-            'author'     => themeConfMatch($confSrc, 'theme_author', 'Unknown'),
-            'version'    => themeConfMatch($confSrc, 'theme_version', '?'),
-            'link'       => themeConfMatch($confSrc, 'theme_link', ''),
-            'screenshot' => 'themes/' . $filename . '/' . themeConfMatch($confSrc, 'theme_screenshot', 'screenshot.jpg'),
+            'name'       => ThemeConf::parseDefine($confSrc, 'theme_name', $filename),
+            'author'     => ThemeConf::parseDefine($confSrc, 'theme_author', 'Unknown'),
+            'version'    => ThemeConf::parseDefine($confSrc, 'theme_version', '?'),
+            'link'       => ThemeConf::sanitizeLink(ThemeConf::parseDefine($confSrc, 'theme_link', '')),
+            'screenshot' => 'themes/' . $filename . '/' . ThemeConf::sanitizeScreenshotFilename(
+                ThemeConf::parseDefine($confSrc, 'theme_screenshot', 'screenshot.jpg'),
+                'screenshot.jpg',
+            ),
             'active'     => $filename === SB_THEME,
         ];
     }
     closedir($themesDir);
 }
 usort($validThemes, fn(array $a, array $b): int => strcasecmp($a['name'], $b['name']));
 
-/**
- * Pluck a `define('<key>', "<value>")` literal out of a theme.conf.php
- * source string. Mirrors the legacy regex shape (double-quoted only)
- * so existing theme manifests keep parsing identically.
- */
-function themeConfMatch(string $src, string $key, string $default): string
-{
-    $pattern = '/define\(\s*\'' . preg_quote($key, '/') . '\'\s*,\s*"([^"]*)"\s*\)\s*;/';
-    if (preg_match($pattern, $src, $m) === 1) {
-        return strip_tags($m[1]);
-    }
-    return $default;
-}
-
 /**
  * Whether STEAMAPIKEY is set to a non-empty value at runtime. Wrapped
  * in a function so PHPStan can't narrow the constant value against its

web/scripts/api-contract.js

@@ -573,6 +573,10 @@
  * @typedef {Object} ApiSystemRehashAdminsResponse
  */
 /**
+ * Preview a theme manifest. Includes theme.conf.php so PHP expressions in the
+ * file are honoured; the admin Settings → Themes grid uses {@see
+ * \Sbpp\Theme\ThemeConf::parseDefine()} instead (no second define()).
+ *
  * @typedef {Object} ApiSystemSelThemeRequest
  * @typedef {Object} ApiSystemSelThemeResponse
  */

web/tests/integration/ThemeConfParseTest.php

@@ -0,0 +1,118 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sbpp\Tests\Integration;
+
+use PHPUnit\Framework\TestCase;
+use Sbpp\Theme\ThemeConf;
+
+/**
+ * Issue #1466: the admin Themes picker regex-reads theme.conf.php
+ * without executing it. The shipped default manifest uses
+ * single-quoted define() values; the pre-fix parser only matched
+ * double-quoted literals, so cards showed "by Unknown · v?".
+ */
+final class ThemeConfParseTest extends TestCase
+{
+    public function testDefaultThemeConfParsesSingleQuotedDefines(): void
+    {
+        $src = (string) file_get_contents(ROOT . 'themes/default/theme.conf.php');
+
+        $this->assertSame('SourceBans++ Default', ThemeConf::parseDefine($src, 'theme_name', ''));
+        $this->assertSame('SourceBans++ Dev Team', ThemeConf::parseDefine($src, 'theme_author', 'Unknown'));
+        $this->assertSame('2.0.0', ThemeConf::parseDefine($src, 'theme_version', '?'));
+        $this->assertSame('https://github.com/sbpp/sourcebans-pp', ThemeConf::parseDefine($src, 'theme_link', ''));
+        $this->assertSame('screenshot.jpg', ThemeConf::parseDefine($src, 'theme_screenshot', ''));
+    }
+
+    public function testParseDefineStillAcceptsDoubleQuotedManifests(): void
+    {
+        $src = <<<'PHP'
+<?php
+define('theme_name', "Fork Theme");
+define('theme_author', "Example Author");
+define('theme_version', "1.2.3");
+define('theme_link', "https://example.com/theme");
+define('theme_screenshot', "preview.png");
+PHP;
+
+        $this->assertSame('Fork Theme', ThemeConf::parseDefine($src, 'theme_name', ''));
+        $this->assertSame('Example Author', ThemeConf::parseDefine($src, 'theme_author', 'Unknown'));
+        $this->assertSame('1.2.3', ThemeConf::parseDefine($src, 'theme_version', '?'));
+        $this->assertSame('https://example.com/theme', ThemeConf::parseDefine($src, 'theme_link', 'missing'));
+        $this->assertSame('preview.png', ThemeConf::parseDefine($src, 'theme_screenshot', ''));
+    }
+
+    public function testEmptyDoubleQuotedValueDoesNotFatal(): void
+    {
+        $src = "<?php\ndefine('theme_link', \"\");\n";
+
+        $this->assertSame('', ThemeConf::parseDefine($src, 'theme_link', 'fallback'));
+    }
+
+    public function testEmptySingleQuotedValue(): void
+    {
+        $src = "<?php\ndefine('theme_link', '');\n";
+
+        $this->assertSame('', ThemeConf::parseDefine($src, 'theme_link', 'fallback'));
+    }
+
+    public function testEscapedApostropheInSingleQuotedValue(): void
+    {
+        $src = "<?php\ndefine('theme_author', 'Bob\\'s Fork');\n";
+
+        $this->assertSame("Bob's Fork", ThemeConf::parseDefine($src, 'theme_author', 'Unknown'));
+    }
+
+    public function testDoubleQuotedKeyIsNotMatched(): void
+    {
+        $src = 'define("theme_name", "Wrong key shape");';
+
+        $this->assertSame('fallback', ThemeConf::parseDefine($src, 'theme_name', 'fallback'));
+    }
+
+    public function testSanitizeLinkAllowsEmptyAndHttpUrls(): void
+    {
+        $this->assertSame('', ThemeConf::sanitizeLink(''));
+        $this->assertSame('https://example.com', ThemeConf::sanitizeLink('https://example.com'));
+        $this->assertSame('http://example.com/path', ThemeConf::sanitizeLink('http://example.com/path'));
+    }
+
+    public function testSanitizeLinkRejectsNonHttpSchemes(): void
+    {
+        $this->assertSame('', ThemeConf::sanitizeLink('javascript:alert(1)'));
+        $this->assertSame('', ThemeConf::sanitizeLink('file:///etc/passwd'));
+    }
+
+    public function testSanitizeScreenshotFilenameStripsPaths(): void
+    {
+        $this->assertSame('shot.jpg', ThemeConf::sanitizeScreenshotFilename('shot.jpg', 'screenshot.jpg'));
+        $this->assertSame('shot.jpg', ThemeConf::sanitizeScreenshotFilename('../../../shot.jpg', 'screenshot.jpg'));
+        $this->assertSame('screenshot.jpg', ThemeConf::sanitizeScreenshotFilename('', 'screenshot.jpg'));
+        $this->assertSame('screenshot.jpg', ThemeConf::sanitizeScreenshotFilename('..', 'screenshot.jpg'));
+    }
+
+    public function testDefaultThemeDiscoveryRowMatchesManifest(): void
+    {
+        $filename = 'default';
+        $confSrc  = (string) file_get_contents(ROOT . 'themes/default/theme.conf.php');
+
+        $row = [
+            'author'  => ThemeConf::parseDefine($confSrc, 'theme_author', 'Unknown'),
+            'version' => ThemeConf::parseDefine($confSrc, 'theme_version', '?'),
+            'link'    => ThemeConf::sanitizeLink(ThemeConf::parseDefine($confSrc, 'theme_link', '')),
+        ];
+
+        $this->assertSame('SourceBans++ Dev Team', $row['author']);
+        $this->assertSame('2.0.0', $row['version']);
+        $this->assertSame('https://github.com/sbpp/sourcebans-pp', $row['link']);
+        $this->assertStringContainsString(
+            'themes/' . $filename . '/',
+            'themes/' . $filename . '/' . ThemeConf::sanitizeScreenshotFilename(
+                ThemeConf::parseDefine($confSrc, 'theme_screenshot', 'screenshot.jpg'),
+                'screenshot.jpg',
+            ),
+        );
+    }
+}