Skip to content

fix(banlist): hide comment author/editor under banlist.hideadminname (#1500) #1501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rumblefrog merged 1 commit into from
Jun 9, 2026
Merged

fix(banlist): hide comment author/editor under banlist.hideadminname (#1500) #1501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 10 additions & 2 deletions web/api/handlers/bans.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -944,14 +944,22 @@ function api_bans_detail(array $params): array
)->resultset([$bid]);
foreach ($commentRows as $crow) {
$editTime = $crow['edittime'] !== null ? (int)$crow['edittime'] : null;
// #1500: comment author/editor are admin usernames; suppress them
// for public callers when `banlist.hideadminname` is on, same as
// the focal admin.name and removed_by fields above. `author_hidden`
// lets the drawer render "Hidden" (policy) distinctly from "unknown"
// (deleted admin -> author already null), matching the server-side
// inline disclosure's three-way label.
$authorHidden = $hideAdmin && $crow['author'] !== null;
$comments[] = [
'cid' => (int)$crow['cid'],
'added' => (int)$crow['added'],
'added_human'=> Config::time((int)$crow['added']),
'author' => $crow['author'] !== null ? (string)$crow['author'] : null,
'author' => ($authorHidden || $crow['author'] === null) ? null : (string)$crow['author'],
'author_hidden' => $authorHidden,
'text' => (string)$crow['commenttxt'],
'edited_at' => $editTime,
'edited_by' => $crow['editor'] !== null ? (string)$crow['editor'] : null,
'edited_by' => (!$hideAdmin && $crow['editor'] !== null) ? (string)$crow['editor'] : null,
];
}
}
Expand Down
12 changes: 10 additions & 2 deletions web/api/handlers/comms.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,14 +542,22 @@ function api_comms_detail(array $params): array
)->resultset([$cid]);
foreach ($commentRows as $crow) {
$editTime = $crow['edittime'] !== null ? (int)$crow['edittime'] : null;
// #1500: comment author/editor are admin usernames; suppress them
// for public callers when `banlist.hideadminname` is on, same as
// the focal admin.name and removed_by fields above. `author_hidden`
// lets the drawer render "Hidden" (policy) distinctly from "unknown"
// (deleted admin -> author already null), matching the server-side
// inline disclosure's three-way label.
$authorHidden = $hideAdmin && $crow['author'] !== null;
$comments[] = [
'cid' => (int)$crow['cid'],
'added' => (int)$crow['added'],
'added_human'=> Config::time((int)$crow['added']),
'author' => $crow['author'] !== null ? (string)$crow['author'] : null,
'author' => ($authorHidden || $crow['author'] === null) ? null : (string)$crow['author'],
'author_hidden' => $authorHidden,
'text' => (string)$crow['commenttxt'],
'edited_at' => $editTime,
'edited_by' => $crow['editor'] !== null ? (string)$crow['editor'] : null,
'edited_by' => (!$hideAdmin && $crow['editor'] !== null) ? (string)$crow['editor'] : null,
];
}
}
Expand Down
18 changes: 14 additions & 4 deletions web/pages/page.banlist.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,11 @@ function setPostKey()
//-----------------------------------
if (Config::getBool('config.enablepubliccomments') || $userbank->is_admin()) {
$view_comments = true;
// #1500: comment author/editor are admin usernames. Null them at the
// data layer for public viewers when banlist.hideadminname is on, so a
// third-party theme that renders the name directly can't re-leak it
// (parity with the focal $data['admin'] = false gate above).
$commentsHideAdmin = Config::getBool('banlist.hideadminname') && !$userbank->is_admin();
$GLOBALS['PDO']->query("SELECT cid, aid, commenttxt, added, edittime,
(SELECT user FROM `:prefix_admins` WHERE aid = C.aid) AS comname,
(SELECT user FROM `:prefix_admins` WHERE aid = C.editaid) AS editname
Expand Down Expand Up @@ -1043,7 +1048,7 @@ function setPostKey()
$cdata['delcomlink'] = "";
}

$cdata['comname'] = $crow['comname'];
$cdata['comname'] = $commentsHideAdmin ? '' : $crow['comname'];
$cdata['added'] = Config::time($crow['added']);
$commentText = html_entity_decode($crow['commenttxt'], ENT_QUOTES | ENT_HTML5, 'UTF-8');
$commentText = encodePreservingBr($commentText);
Expand All @@ -1053,7 +1058,7 @@ function setPostKey()

if (!empty($crow['edittime'])) {
$cdata['edittime'] = Config::time($crow['edittime']);
$cdata['editname'] = $crow['editname'];
$cdata['editname'] = $commentsHideAdmin ? '' : $crow['editname'];
} else {
$cdata['edittime'] = "";
$cdata['editname'] = "";
Expand Down Expand Up @@ -1281,10 +1286,15 @@ function setPostKey()
$_GET["comment"],
]);

// #1500: same gate as the per-ban comment thread above — null admin
// usernames for public viewers so this comment-edit surface (reachable
// by anyone via ?comment=N) doesn't leak them regardless of theme.
$commentsHideAdmin = Config::getBool('banlist.hideadminname') && !$userbank->is_admin();

$ocomments = [];
foreach ($cotherdata as $cdrow) {
$coment = [];
$coment['comname'] = $cdrow['comname'];
$coment['comname'] = $commentsHideAdmin ? '' : $cdrow['comname'];
$coment['added'] = Config::time($cdrow['added']);
$commentTextRow = html_entity_decode($cdrow['commenttxt'], ENT_QUOTES | ENT_HTML5, 'UTF-8');
$commentTextRow = encodePreservingBr($commentTextRow);
Expand All @@ -1294,7 +1304,7 @@ function setPostKey()

if ($cdrow['editname'] != "") {
$coment['edittime'] = Config::time($cdrow['edittime']);
$coment['editname'] = $cdrow['editname'];
$coment['editname'] = $commentsHideAdmin ? '' : $cdrow['editname'];
} else {
$coment['editname'] = "";
$coment['edittime'] = "";
Expand Down
18 changes: 14 additions & 4 deletions web/pages/page.commslist.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,11 @@ function setPostKey()
//-----------------------------------
if (Config::getBool('config.enablepubliccomments') || $userbank->is_admin()) {
$view_comments = true;
// #1500: comment author/editor are admin usernames. Null them at the
// data layer for public viewers when banlist.hideadminname is on, so a
// third-party theme that renders the name directly can't re-leak it
// (parity with the focal admin-name gate above).
$commentsHideAdmin = Config::getBool('banlist.hideadminname') && !$userbank->is_admin();
$GLOBALS['PDO']->query("SELECT cid, aid, commenttxt, added, edittime,
(SELECT user FROM `:prefix_admins` WHERE aid = C.aid) AS comname,
(SELECT user FROM `:prefix_admins` WHERE aid = C.editaid) AS editname
Expand Down Expand Up @@ -849,7 +854,7 @@ function setPostKey()
$cdata['delcomlink'] = "";
}

$cdata['comname'] = $crow['comname'];
$cdata['comname'] = $commentsHideAdmin ? '' : $crow['comname'];
$cdata['added'] = Config::time($crow['added']);
$commentText = html_entity_decode($crow['commenttxt'], ENT_QUOTES | ENT_HTML5, 'UTF-8');
$commentText = encodePreservingBr($commentText);
Expand All @@ -859,7 +864,7 @@ function setPostKey()

if (!empty($crow['edittime'])) {
$cdata['edittime'] = Config::time($crow['edittime']);
$cdata['editname'] = $crow['editname'];
$cdata['editname'] = $commentsHideAdmin ? '' : $crow['editname'];
} else {
$cdata['edittime'] = "";
$cdata['editname'] = "";
Expand Down Expand Up @@ -1067,17 +1072,22 @@ function setPostKey()
$_GET["comment"],
]);

// #1500: same gate as the per-comm comment thread above — null admin
// usernames for public viewers so this comment-edit surface (reachable
// by anyone via ?comment=N) doesn't leak them regardless of theme.
$commentsHideAdmin = Config::getBool('banlist.hideadminname') && !$userbank->is_admin();

foreach ($cotherdata as $cdrow) {
$coment = [];
$coment['comname'] = $cdrow['comname'];
$coment['comname'] = $commentsHideAdmin ? '' : $cdrow['comname'];
$coment['added'] = Config::time($cdrow['added']);
$commentText = html_entity_decode($cdrow['commenttxt'], ENT_QUOTES | ENT_HTML5, 'UTF-8');
$commentText = encodePreservingBr($commentText);
$commentText = preg_replace('@(https?://([-\w\.]+)+(:\d+)?(/([\w/_\.]*(\?\S+)?)?)?)@', '<a href="$1" target="_blank">$1</a>', $commentText);
$coment['commenttxt'] = $commentText;
if ($cdrow['editname'] != "") {
$coment['edittime'] = Config::time($cdrow['edittime']);
$coment['editname'] = $cdrow['editname'];
$coment['editname'] = $commentsHideAdmin ? '' : $cdrow['editname'];
} else {
$coment['editname'] = "";
$coment['edittime'] = "";
Expand Down
51 changes: 51 additions & 0 deletions web/tests/api/BansTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,57 @@ public function testDetailAdminViewExposesEverything(): void
]);
}

public function testDetailHidesCommentAuthorForPublicWhenHideAdminName(): void
{
// #1500: with public comments ENABLED, the comment author/editor
// (admin usernames) must still be suppressed for public callers
// under banlist.hideadminname — same as the focal admin.name and
// removed_by fields. Pre-fix the handler leaked them: the
// testDetailPublicViewHidesAdminFields case above turns public
// comments OFF, so it never exercised this combination.
Fixture::rawPdo()->prepare(sprintf(
"REPLACE INTO `%s_settings` (`setting`, `value`) VALUES
('banlist.hideadminname', '1'),
('config.enablepubliccomments', '1')",
DB_PREFIX
))->execute();
\Config::init($GLOBALS['PDO']);

$bid = $this->seedBan('STEAM_0:1:1500', 'comment-author-leak');
// Author AND editor set to the seeded admin so both subqueries
// resolve to a real username (the value that must NOT leak).
Fixture::rawPdo()->prepare(sprintf(
"INSERT INTO `%s_comments` (`bid`, `type`, `aid`, `editaid`, `commenttxt`, `added`, `edittime`)
VALUES (?, 'B', ?, ?, ?, UNIX_TIMESTAMP(), UNIX_TIMESTAMP())",
DB_PREFIX
))->execute([$bid, Fixture::adminAid(), Fixture::adminAid(), 'leaky comment']);

// Public caller (not logged in).
$env = $this->api('bans.detail', ['bid' => $bid]);
$this->assertTrue($env['ok'], json_encode($env));
$this->assertTrue($env['data']['comments_visible'], 'public comments are enabled');
$this->assertCount(1, $env['data']['comments']);
$this->assertSame('leaky comment', $env['data']['comments'][0]['text']);
$this->assertNull($env['data']['comments'][0]['author'],
'comment author must be hidden for public + hideadminname (#1500)');
$this->assertNull($env['data']['comments'][0]['edited_by'],
'comment editor must be hidden for public + hideadminname (#1500)');
$this->assertTrue($env['data']['comments'][0]['author_hidden'],
'author_hidden sentinel must be true when a real name was suppressed (#1500 m1: lets the drawer render "Hidden" vs "unknown")');
$this->assertNull($env['data']['admin']['name'], 'focal admin name is hidden too');

// Admins still see the author + editor; author_hidden is false.
$this->loginAsAdmin();
$adminEnv = $this->api('bans.detail', ['bid' => $bid]);
$this->assertTrue($adminEnv['ok'], json_encode($adminEnv));
$this->assertNotNull($adminEnv['data']['comments'][0]['author'],
'admins still see the comment author');
$this->assertNotNull($adminEnv['data']['comments'][0]['edited_by'],
'admins still see the comment editor');
$this->assertFalse($adminEnv['data']['comments'][0]['author_hidden'],
'author_hidden must be false for admin viewers (#1500 m1)');
}

public function testDetailReportsUnbannedForPre2AdminLiftWithRemoveTypeNull(): void
{
// #1352: pre-2.0 admin-lifted bans whose `RemoveType IS NULL`
Expand Down
51 changes: 51 additions & 0 deletions web/tests/api/CommsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,57 @@ public function testDetailAdminViewExposesEverything(): void
$this->assertSame('Gag', $env['data']['block']['type_label']);
}

public function testDetailHidesCommentAuthorForPublicWhenHideAdminName(): void
{
// #1500: with public comments ENABLED, the comment author/editor
// (admin usernames) must still be suppressed for public callers
// under banlist.hideadminname — same as the focal admin.name and
// removed_by fields. Pre-fix the handler leaked them: the
// testDetailPublicViewHidesAdminFields case above turns public
// comments OFF, so it never exercised this combination.
Fixture::rawPdo()->prepare(sprintf(
"REPLACE INTO `%s_settings` (`setting`, `value`) VALUES
('banlist.hideadminname', '1'),
('config.enablepubliccomments', '1')",
DB_PREFIX
))->execute();
\Config::init($GLOBALS['PDO']);

$cid = $this->seedComm('STEAM_0:0:1500', 'comment-author-leak');
// Author AND editor set to the seeded admin so both subqueries
// resolve to a real username (the value that must NOT leak).
Fixture::rawPdo()->prepare(sprintf(
"INSERT INTO `%s_comments` (`bid`, `type`, `aid`, `editaid`, `commenttxt`, `added`, `edittime`)
VALUES (?, 'C', ?, ?, ?, UNIX_TIMESTAMP(), UNIX_TIMESTAMP())",
DB_PREFIX
))->execute([$cid, Fixture::adminAid(), Fixture::adminAid(), 'leaky comment']);

// Public caller (not logged in).
$env = $this->api('comms.detail', ['cid' => $cid]);
$this->assertTrue($env['ok'], json_encode($env));
$this->assertTrue($env['data']['comments_visible'], 'public comments are enabled');
$this->assertCount(1, $env['data']['comments']);
$this->assertSame('leaky comment', $env['data']['comments'][0]['text']);
$this->assertNull($env['data']['comments'][0]['author'],
'comment author must be hidden for public + hideadminname (#1500)');
$this->assertNull($env['data']['comments'][0]['edited_by'],
'comment editor must be hidden for public + hideadminname (#1500)');
$this->assertTrue($env['data']['comments'][0]['author_hidden'],
'author_hidden sentinel must be true when a real name was suppressed (#1500 m1: lets the drawer render "Hidden" vs "unknown")');
$this->assertNull($env['data']['admin']['name'], 'focal admin name is hidden too');

// Admins still see the author + editor; author_hidden is false.
$this->loginAsAdmin();
$adminEnv = $this->api('comms.detail', ['cid' => $cid]);
$this->assertTrue($adminEnv['ok'], json_encode($adminEnv));
$this->assertNotNull($adminEnv['data']['comments'][0]['author'],
'admins still see the comment author');
$this->assertNotNull($adminEnv['data']['comments'][0]['edited_by'],
'admins still see the comment editor');
$this->assertFalse($adminEnv['data']['comments'][0]['author_hidden'],
'author_hidden must be false for admin viewers (#1500 m1)');
}

public function testDetailReportsUnmutedForLiftedBlock(): void
{
// After comms.unblock the block's state must be 'unmuted' (the
Expand Down
1 change: 1 addition & 0 deletions web/tests/api/__snapshots__/bans/detail_admin_view.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"added": "<*>",
"added_human": "<*>",
"author": "admin",
"author_hidden": false,
"text": "note for the drawer",
"edited_at": null,
"edited_by": null
Expand Down
Loading
Loading