fix(demos): download link and remove button on ban edit page (#1464)

web/api/handlers/_register.php

@@ -69,6 +69,7 @@
 Api::register('bans.add_comment',         'api_bans_add_comment',          0, true);
 Api::register('bans.edit_comment',        'api_bans_edit_comment',         0, true);
 Api::register('bans.remove_comment',      'api_bans_remove_comment',       ADMIN_OWNER);
+Api::register('bans.remove_demo',         'api_bans_remove_demo',          0, true);
 Api::register('bans.group_ban',           'api_bans_group_ban',            ADMIN_OWNER | ADMIN_ADD_BAN);
 Api::register('bans.ban_member_of_group', 'api_bans_ban_member_of_group',  ADMIN_OWNER | ADMIN_ADD_BAN);
 Api::register('bans.ban_friends',         'api_bans_ban_friends',          ADMIN_OWNER | ADMIN_ADD_BAN);

web/api/handlers/bans.php

@@ -349,6 +349,67 @@ function api_bans_edit_comment(array $params): array
     ];
 }
 
+function api_bans_remove_demo(array $params): array
+{
+    global $userbank, $username;
+
+    $bid = (int) ($params['bid'] ?? 0);
+    if ($bid <= 0) {
+        throw new ApiError('validation', 'Missing or invalid ban id.', 'bid');
+    }
+
+    $row = $GLOBALS['PDO']
+        ->query(
+            'SELECT ba.aid, ad.gid
+             FROM `:prefix_bans` AS ba
+             LEFT JOIN `:prefix_admins` AS ad ON ba.aid = ad.aid
+             WHERE ba.bid = :bid'
+        )
+        ->single([':bid' => $bid]);
+
+    if (!$row) {
+        throw new ApiError('not_found', 'Ban not found.');
+    }
+
+    $canEdit = $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::EditAllBans))
+        || ($userbank->HasAccess(WebPermission::EditOwnBans) && (int) $row['aid'] === $userbank->GetAid())
+        || ($userbank->HasAccess(WebPermission::EditGroupBans) && (int) $row['gid'] === (int) $userbank->GetProperty('gid'));
+
+    if (!$canEdit) {
+        throw new ApiError('forbidden', 'You do not have permission to edit this ban.');
+    }
+
+    $demo = $GLOBALS['PDO']
+        ->query(
+            "SELECT filename FROM `:prefix_demos`
+             WHERE demid = :bid AND UPPER(demtype) = 'B'"
+        )
+        ->single([':bid' => $bid]);
+
+    if (!$demo) {
+        throw new ApiError('not_found', 'No demo is attached to this ban.');
+    }
+
+    $onDisk = basename((string) $demo['filename']);
+    $path   = SB_DEMOS . '/' . $onDisk;
+    $listing = is_dir(SB_DEMOS) ? scandir(SB_DEMOS) : false;
+    if ($onDisk !== '' && $listing !== false && in_array($onDisk, $listing, true) && is_file($path)) {
+        if (!unlink($path)) {
+            throw new ApiError('server_error', 'Unable to delete demo file from disk.');
+        }
+    }
+
+    $GLOBALS['PDO']
+        ->query("DELETE FROM `:prefix_demos` WHERE demid = :bid AND UPPER(demtype) = 'B'")
+        ->execute([':bid' => $bid]);
+
+    Log::add(LogType::Message, 'Demo Removed', "$username removed the demo from ban #$bid");
+
+    return [
+        'removed' => true,
+    ];
+}
+
 function api_bans_remove_comment(array $params): array
 {
     global $username;

web/includes/View/AdminBansEditView.php

@@ -16,10 +16,14 @@
  *   - `$ban_name`        Current player name on the ban row.
  *   - `$ban_authid`      Current Steam2 authid on the ban row.
  *   - `$ban_ip`          Current IP on the ban row.
- *   - `$ban_demo`        Pre-built "Uploaded: <b>safename</b>" snippet
- *                        (or empty), htmlspecialchars'd in the page handler
- *                        per #1113. Rendered with `{nofilter}`; the safety
- *                        annotation lives at the call site in the template.
+ *   - `$ban_id`          Ban row id (`:prefix_bans.bid`); drives the demo
+ *                        download URL and the remove-demo JSON action.
+ *   - `$has_demo`        Whether a demo row exists for this ban.
+ *   - `$ban_demo`        Pre-built "Uploaded: <a …><b>safename</b></a>"
+ *                        snippet (or empty), htmlspecialchars'd in the
+ *                        page handler per #1113. Rendered with
+ *                        `{nofilter}`; the safety annotation lives at the
+ *                        call site in the template.
  *   - `$customreason`    `false` when custom reasons are disabled, otherwise
  *                        the list of reason strings from `bans.customreasons`.
  *
@@ -38,6 +42,8 @@ final class AdminBansEditView extends View
      */
     public function __construct(
         public readonly bool $can_edit_ban,
+        public readonly int $ban_id,
+        public readonly bool $has_demo,
         public readonly string $ban_name,
         public readonly string $ban_authid,
         public readonly string $ban_ip,

web/pages/admin.edit.ban.php

@@ -67,7 +67,7 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body,
 // names and bind both. The subquery's `:demo_bid` and the outer
 // `:bid` both pull from `$_GET['id']`, which is already int-cast above.
 $GLOBALS['PDO']->query("
-    				SELECT bid, ba.ip, ba.type, ba.authid, ba.name, created, ends, length, reason, ba.aid, ba.sid AS ba_sid, ad.user, ad.gid, CONCAT(se.ip,':',se.port) AS server_addr, se.sid AS se_sid, mo.icon, (SELECT origname FROM `:prefix_demos` WHERE demtype = 'b' AND demid = :demo_bid) AS dname
+    				SELECT bid, ba.ip, ba.type, ba.authid, ba.name, created, ends, length, reason, ba.aid, ba.sid AS ba_sid, ad.user, ad.gid, CONCAT(se.ip,':',se.port) AS server_addr, se.sid AS se_sid, mo.icon, (SELECT origname FROM `:prefix_demos` WHERE UPPER(demtype) = 'B' AND demid = :demo_bid) AS dname
     				FROM `:prefix_bans` AS ba
     				LEFT JOIN `:prefix_admins` AS ad ON ba.aid = ad.aid
     				LEFT JOIN `:prefix_servers` AS se ON se.sid = ba.sid
@@ -338,22 +338,31 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body,
     : false;
 /** @var false|list<string> $customReason */
 
-\Sbpp\View\Renderer::render($theme, new \Sbpp\View\AdminBansEditView(
-    can_edit_ban: $canEditBan,
-    ban_name:     (string) $res['name'],
-    ban_authid:   trim((string) $res['authid']),
-    ban_ip:       (string) $res['ip'],
+$hasDemo = !empty($res['dname']);
+$banDemoHtml = '';
+if ($hasDemo) {
     // Issue #1113: dname is the admin-supplied original filename of the
     // demo (POST'd by whoever edited the ban + uploaded the demo, stored
     // as `:prefix_demos.origname`). It used to be interpolated raw into
     // HTML rendered with `nofilter`, so a filename like
     // `<img src=x onerror=…>` turned the edit-ban page into stored XSS
     // for any admin viewing that ban. htmlspecialchars + ENT_QUOTES so
-    // the value is safe inside both the surrounding `<b>…</b>` text and
-    // the `nofilter` render in the template.
-    ban_demo: !empty($res['dname'])
-        ? 'Uploaded: <b>' . htmlspecialchars((string) $res['dname'], ENT_QUOTES, 'UTF-8') . '</b>'
-        : '',
+    // the value is safe inside the link text. The href is server-built
+    // from the int-cast bid only (#1464).
+    $safeName = htmlspecialchars((string) $res['dname'], ENT_QUOTES, 'UTF-8');
+    $demoUrl  = 'getdemo.php?type=B&id=' . rawurlencode((string) $_GET['id']);
+    $banDemoHtml = 'Uploaded: <a href="' . $demoUrl . '" data-testid="editban-demo-download"><b>'
+        . $safeName . '</b></a>';
+}
+
+\Sbpp\View\Renderer::render($theme, new \Sbpp\View\AdminBansEditView(
+    can_edit_ban: $canEditBan,
+    ban_id:       (int) $_GET['id'],
+    has_demo:     $hasDemo,
+    ban_name:     (string) $res['name'],
+    ban_authid:   trim((string) $res['authid']),
+    ban_ip:       (string) $res['ip'],
+    ban_demo:     $banDemoHtml,
     customreason: $customReason,
 ));
 
@@ -380,6 +389,7 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body,
 $redirectUrl = 'index.php?p=banlist' . $pagelink;
 $redirectUrlJs = json_encode($redirectUrl, JSON_THROW_ON_ERROR | JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT);
 $postSuccessJs = $postSuccess ? 'true' : 'false';
+$banIdJs = json_encode((int) $_GET['id'], JSON_THROW_ON_ERROR);
 ?>
 <script>
 (function () {
@@ -389,8 +399,16 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body,
     var validationErrors = <?=$validationErrorsJs?>;
     var postSuccess = <?=$postSuccessJs?>;
     var redirectUrl = <?=$redirectUrlJs?>;
+    var banId = <?=$banIdJs?>;
 
     function $id(id) { return document.getElementById(id); }
+    function setBusy(btn, busy) {
+        if (window.SBPP && typeof window.SBPP.setBusy === 'function') {
+            window.SBPP.setBusy(btn, busy);
+            return;
+        }
+        if (btn) btn.disabled = busy;
+    }
     function setMsg(id, text) {
         var el = $id(id);
         if (!el) return;
@@ -486,22 +504,65 @@ function toast(kind, title, body) {
     // here; we still treat `name` as untrusted text and write it via
     // textContent + a leading static label so any HTML metacharacters
     // render literally, not as markup.
-    window.demo = function (id, name) {
+    function setDemoStatus(name) {
         var msg = $id('demo.msg');
         if (msg) {
             msg.textContent = '';
             var label = document.createTextNode('Uploaded: ');
+            var a = document.createElement('a');
+            a.href = 'getdemo.php?type=B&id=' + encodeURIComponent(String(banId));
+            a.setAttribute('data-testid', 'editban-demo-download');
             var b = document.createElement('b');
             b.textContent = String(name);
+            a.appendChild(b);
             msg.appendChild(label);
-            msg.appendChild(b);
+            msg.appendChild(a);
+        }
+        var removeBtn = $id('removedemo');
+        if (removeBtn) {
+            removeBtn.hidden = false;
         }
+    }
+
+    window.demo = function (id, name) {
+        setDemoStatus(name);
         var did = $id('did');
         var dname = $id('dname');
         if (did) did.value = id;
         if (dname) dname.value = name;
     };
 
+    document.addEventListener('DOMContentLoaded', function () {
+        var removeBtn = $id('removedemo');
+        if (!removeBtn) return;
+        removeBtn.addEventListener('click', function () {
+            if (!window.confirm('Remove the demo attached to this ban? This cannot be undone.')) {
+                return;
+            }
+            if (!window.sb || !window.sb.api || typeof window.sb.api.call !== 'function') {
+                toast('red', 'Error', 'Unable to remove demo — panel API unavailable.');
+                return;
+            }
+            setBusy(removeBtn, true);
+            window.sb.api.call(Actions.BansRemoveDemo, { bid: banId }).then(function (r) {
+                setBusy(removeBtn, false);
+                if (!r.ok) {
+                    var err = r.error && r.error.message ? r.error.message : 'Unable to remove demo.';
+                    toast('red', 'Demo NOT Removed', err);
+                    return;
+                }
+                var msg = $id('demo.msg');
+                if (msg) msg.textContent = '';
+                var did = $id('did');
+                var dname = $id('dname');
+                if (did) did.value = '';
+                if (dname) dname.value = '';
+                removeBtn.hidden = true;
+                toast('green', 'Demo removed', 'The demo has been deleted from this ban.');
+            });
+        });
+    });
+
     document.addEventListener('DOMContentLoaded', function () {
         window.selectLengthTypeReason(<?=$banLengthSeconds?>, <?=$banType?>, <?=$banReasonJs?>);
     });

web/scripts/api-contract.js

@@ -181,6 +181,10 @@
  * @typedef {Object} ApiBansRemoveCommentRequest
  * @typedef {Object} ApiBansRemoveCommentResponse
  */
+/**
+ * @typedef {Object} ApiBansRemoveDemoRequest
+ * @typedef {Object} ApiBansRemoveDemoResponse
+ */
 /**
  * Autocomplete backend for the command palette (#1123 C2).  Returns up to
  * `limit` matching ban rows for a free-text query that the palette types into
@@ -662,6 +666,7 @@ var Actions = Object.freeze({
     BansPlayerHistory: 'bans.player_history',
     BansPrepareReban: 'bans.prepare_reban',
     BansRemoveComment: 'bans.remove_comment',
+    BansRemoveDemo: 'bans.remove_demo',
     BansSearch: 'bans.search',
     BansSendMessage: 'bans.send_message',
     BansSetupBan: 'bans.setup_ban',

web/tests/api/BansTest.php

@@ -26,6 +26,38 @@ private function seedBan(string $steam = 'STEAM_0:1:42', string $reason = 'test'
         return (int)$pdo->lastInsertId();
     }
 
+    private function seedDemoForBan(int $bid, string $filename = 'testdemo1464.dem'): void
+    {
+        $path = SB_DEMOS . '/' . $filename;
+        if (!is_dir(SB_DEMOS)) {
+            mkdir(SB_DEMOS, 0775, true);
+        }
+        file_put_contents($path, 'demo-bytes');
+        $pdo = Fixture::rawPdo();
+        $pdo->prepare(sprintf(
+            'INSERT INTO `%s_demos` (demid, demtype, filename, origname) VALUES (?, ?, ?, ?)',
+            DB_PREFIX
+        ))->execute([$bid, 'B', $filename, 'evidence.dem']);
+    }
+
+    private function createAdminWithFlags(int $mask): int
+    {
+        $pdo = Fixture::rawPdo();
+        $stmt = $pdo->prepare(sprintf(
+            'INSERT INTO `%s_admins` (user, authid, password, gid, email, validate, extraflags, immunity)
+             VALUES (?, ?, ?, -1, ?, NULL, ?, 50)',
+            DB_PREFIX,
+        ));
+        $stmt->execute([
+            'flagged-' . $mask,
+            'STEAM_0:0:' . (4_000_000 + $mask),
+            password_hash('x', PASSWORD_BCRYPT),
+            'flagged-' . $mask . '@example.test',
+            $mask,
+        ]);
+        return (int) $pdo->lastInsertId();
+    }
+
     private function seedSubmission(string $name = 'Bob', string $steam = 'STEAM_0:1:99'): int
     {
         $pdo = Fixture::rawPdo();
@@ -952,4 +984,52 @@ public function testPlayerHistoryBidPathReturnsEmptyWhenOnlyFocalExists(): void
         $this->assertSame(0, (int)$env['data']['total']);
         $this->assertSame([], $env['data']['items']);
     }
+
+    public function testRemoveDemoRejectsAnonymous(): void
+    {
+        $bid = $this->seedBan();
+        $env = $this->api('bans.remove_demo', ['bid' => $bid]);
+        $this->assertEnvelopeError($env, 'forbidden');
+    }
+
+    public function testRemoveDemoNotFoundWhenNoDemoAttached(): void
+    {
+        $this->loginAsAdmin();
+        $bid = $this->seedBan();
+        $env = $this->api('bans.remove_demo', ['bid' => $bid]);
+        $this->assertEnvelopeError($env, 'not_found');
+    }
+
+    public function testRemoveDemoForbiddenWhenAdminCannotEditBan(): void
+    {
+        $bid = $this->seedBan();
+        $this->seedDemoForBan($bid, 'forbidden1464.dem');
+        $aid = $this->createAdminWithFlags(ADMIN_ADD_BAN);
+        $this->loginAs($aid);
+
+        $env = $this->api('bans.remove_demo', ['bid' => $bid]);
+        $this->assertEnvelopeError($env, 'forbidden');
+    }
+
+    public function testRemoveDemoSuccess(): void
+    {
+        $this->loginAsAdmin();
+        $bid = $this->seedBan();
+        $filename = 'testdemo1464.dem';
+        $this->seedDemoForBan($bid, $filename);
+        $path = SB_DEMOS . '/' . $filename;
+
+        $env = $this->api('bans.remove_demo', ['bid' => $bid]);
+        $this->assertTrue($env['ok'], json_encode($env));
+        $this->assertTrue($env['data']['removed']);
+
+        $pdo = Fixture::rawPdo();
+        $row = $pdo->prepare(sprintf(
+            'SELECT COUNT(*) FROM `%s_demos` WHERE demid = ? AND UPPER(demtype) = ?',
+            DB_PREFIX
+        ));
+        $row->execute([$bid, 'B']);
+        $this->assertSame(0, (int) $row->fetchColumn());
+        $this->assertFileDoesNotExist($path);
+    }
 }

web/tests/api/PermissionMatrixTest.php

@@ -79,6 +79,7 @@ public static function expectedMatrix(): array
             'bans.add_comment'            => ['perm' => 0, 'requireAdmin' => true,  'public' => false],
             'bans.edit_comment'           => ['perm' => 0, 'requireAdmin' => true,  'public' => false],
             'bans.remove_comment'         => ['perm' => ADMIN_OWNER, 'requireAdmin' => false, 'public' => false],
+            'bans.remove_demo'            => ['perm' => 0, 'requireAdmin' => true,  'public' => false],
             'bans.group_ban'              => ['perm' => ADMIN_OWNER | ADMIN_ADD_BAN, 'requireAdmin' => false, 'public' => false],
             'bans.ban_member_of_group'    => ['perm' => ADMIN_OWNER | ADMIN_ADD_BAN, 'requireAdmin' => false, 'public' => false],
             'bans.ban_friends'            => ['perm' => ADMIN_OWNER | ADMIN_ADD_BAN, 'requireAdmin' => false, 'public' => false],