malpercio.dev / atbb
WIP! A BB-style forum, on the ATmosphere! We're still working... we'll be back soon when we have something to show off!
node typescript hono htmx atproto
fork atom

feat: enforce mod actions in firehose indexer (ATB-21) (#37)

* feat: add BanEnforcer class for firehose ban enforcement (ATB-21)

* fix: use @atproto/common-web for TID in mod routes

Consistent with all other files in the project. @atproto/common
was listed in package.json but not installed.

* fix: add error logging to BanEnforcer applyBan/liftBan, clarify expired-ban test

- Wrap applyBan and liftBan DB updates in try-catch; log structured error
context (subjectDid, error message) then re-throw so callers know the
operation failed and posts may be in an inconsistent state
- Rename "returns false when only an expired ban exists" test to
"returns false when DB returns no active ban (e.g. no ban or expired
ban filtered by query)" and add a comment explaining that expiry
filtering is a Drizzle SQL concern, not unit-testable with mocks

* feat: skip indexing posts from banned users in firehose (ATB-21)

* feat: soft-delete existing posts when ban is indexed (ATB-21)

* fix: gate applyBan on insert success, not just action type (ATB-21)

* feat: restore posts when ban record is deleted (ATB-21)

Override handleModActionDelete to read the modAction row before deleting
it, then call banEnforcer.liftBan inside the same transaction when the
deleted record was a ban action. All three edge cases are covered by
tests: ban deleted (liftBan called), non-ban deleted (liftBan skipped),
and record already missing (idempotent, liftBan skipped).

* test: add error re-throw coverage for handleModActionDelete (ATB-21)

* test: add race condition coverage for firehose ban enforcement (ATB-21)

* test: strengthen race condition assertion in indexer ban enforcement

Replace `expect(mockDb.transaction).toHaveBeenCalled()` with
`expect(mockDb.insert).toHaveBeenCalled()` — the transaction mock
passes the same insert reference to the callback, so asserting insert
was called proves a record was actually written (not just that a
transaction was opened).

* docs: mark ATB-20 and ATB-21 complete in project plan

* fix: address code review feedback on ATB-21 ban enforcement

- Re-throw programming errors in isBanned (fail closed only for DB errors)
- Remove unused dbOrTx param from isBanned (YAGNI)
- Make ban create atomic: insert + applyBan in one transaction
- Add unban handling to handleModActionCreate (was completely missing)
- Add log + test for ban action with missing subject.did
- Add try/catch to handleModActionCreate (consistent with handleModActionDelete)
- Add error handling to getForumIdByUri/getForumIdByDid (consistent with other helpers)
- Remove duplicate expired-ban test; add applyBan/liftBan re-throw tests
- Add vi.clearAllMocks() to beforeEach; fix not-banned test assertion
- Use structured log objects instead of template literals

* docs: note ATB-25 limitation in liftBan (shared deleted column)

* fix: make unban mod action create atomic (insert + liftBan in one tx)

Mirrors the fix applied to the ban path in the previous commit. Without
this, a liftBan failure after genericCreate committed would store the
unban record but leave posts hidden, with no clean retry path.

authored by

Malpercio and committed by
GitHub 02f61bcf 8420b243

+920 -19
+122
apps/appview/src/lib/__tests__/indexer-ban-enforcer.test.ts
··· 1 + import { describe, it, expect, beforeEach, vi } from "vitest"; 2 + import { BanEnforcer } from "../ban-enforcer.js"; 3 + import type { Database } from "@atbb/db"; 4 + 5 + const createMockDb = () => { 6 + const mockSelect = vi.fn(); 7 + const mockUpdate = vi.fn(); 8 + 9 + return { 10 + select: mockSelect, 11 + update: mockUpdate, 12 + } as unknown as Database; 13 + }; 14 + 15 + describe("BanEnforcer", () => { 16 + let mockDb: Database; 17 + let enforcer: BanEnforcer; 18 + 19 + beforeEach(() => { 20 + vi.clearAllMocks(); 21 + mockDb = createMockDb(); 22 + enforcer = new BanEnforcer(mockDb); 23 + }); 24 + 25 + describe("isBanned", () => { 26 + it("returns true when an active ban exists (no expiry)", async () => { 27 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 28 + from: vi.fn().mockReturnValue({ 29 + where: vi.fn().mockReturnValue({ 30 + limit: vi.fn().mockResolvedValue([{ id: 1n }]), 31 + }), 32 + }), 33 + }); 34 + 35 + expect(await enforcer.isBanned("did:plc:banned123")).toBe(true); 36 + }); 37 + 38 + it("returns false when no ban exists", async () => { 39 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 40 + from: vi.fn().mockReturnValue({ 41 + where: vi.fn().mockReturnValue({ 42 + limit: vi.fn().mockResolvedValue([]), 43 + }), 44 + }), 45 + }); 46 + 47 + expect(await enforcer.isBanned("did:plc:user123")).toBe(false); 48 + }); 49 + 50 + it("returns true (fail closed) when DB throws", async () => { 51 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 52 + from: vi.fn().mockReturnValue({ 53 + where: vi.fn().mockReturnValue({ 54 + limit: vi.fn().mockRejectedValue(new Error("DB connection lost")), 55 + }), 56 + }), 57 + }); 58 + 59 + expect(await enforcer.isBanned("did:plc:user123")).toBe(true); 60 + }); 61 + 62 + it("re-throws programming errors (TypeError) without fail-closed", async () => { 63 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 64 + from: vi.fn().mockReturnValue({ 65 + where: vi.fn().mockReturnValue({ 66 + limit: vi.fn().mockRejectedValue(new TypeError("Cannot read property")), 67 + }), 68 + }), 69 + }); 70 + 71 + await expect(enforcer.isBanned("did:plc:user123")).rejects.toThrow(TypeError); 72 + }); 73 + }); 74 + 75 + describe("applyBan", () => { 76 + it("soft-deletes all posts for the subject DID", async () => { 77 + const mockWhere = vi.fn().mockResolvedValue(undefined); 78 + const mockSet = vi.fn().mockReturnValue({ where: mockWhere }); 79 + (mockDb.update as ReturnType<typeof vi.fn>).mockReturnValue({ set: mockSet }); 80 + 81 + await enforcer.applyBan("did:plc:banned123"); 82 + 83 + expect(mockSet).toHaveBeenCalledWith({ deleted: true }); 84 + expect(mockWhere).toHaveBeenCalled(); 85 + }); 86 + 87 + it("re-throws when DB throws during applyBan", async () => { 88 + const dbError = new Error("DB connection lost"); 89 + (mockDb.update as ReturnType<typeof vi.fn>).mockReturnValue({ 90 + set: vi.fn().mockReturnValue({ 91 + where: vi.fn().mockRejectedValue(dbError), 92 + }), 93 + }); 94 + 95 + await expect(enforcer.applyBan("did:plc:banned123")).rejects.toThrow("DB connection lost"); 96 + }); 97 + }); 98 + 99 + describe("liftBan", () => { 100 + it("restores all posts for the subject DID", async () => { 101 + const mockWhere = vi.fn().mockResolvedValue(undefined); 102 + const mockSet = vi.fn().mockReturnValue({ where: mockWhere }); 103 + (mockDb.update as ReturnType<typeof vi.fn>).mockReturnValue({ set: mockSet }); 104 + 105 + await enforcer.liftBan("did:plc:unbanned123"); 106 + 107 + expect(mockSet).toHaveBeenCalledWith({ deleted: false }); 108 + expect(mockWhere).toHaveBeenCalled(); 109 + }); 110 + 111 + it("re-throws when DB throws during liftBan", async () => { 112 + const dbError = new Error("DB connection lost"); 113 + (mockDb.update as ReturnType<typeof vi.fn>).mockReturnValue({ 114 + set: vi.fn().mockReturnValue({ 115 + where: vi.fn().mockRejectedValue(dbError), 116 + }), 117 + }); 118 + 119 + await expect(enforcer.liftBan("did:plc:unbanned123")).rejects.toThrow("DB connection lost"); 120 + }); 121 + }); 122 + });
+500
apps/appview/src/lib/__tests__/indexer.test.ts
··· 4 4 import { memberships } from "@atbb/db"; 5 5 import type { CommitCreateEvent, CommitUpdateEvent, CommitDeleteEvent } from "@skyware/jetstream"; 6 6 7 + vi.mock("../ban-enforcer.js", () => ({ 8 + BanEnforcer: vi.fn().mockImplementation(() => ({ 9 + isBanned: vi.fn().mockResolvedValue(false), 10 + applyBan: vi.fn().mockResolvedValue(undefined), 11 + liftBan: vi.fn().mockResolvedValue(undefined), 12 + })), 13 + })); 14 + 15 + 7 16 // Mock database 8 17 const createMockDb = () => { 9 18 const mockInsert = vi.fn().mockReturnValue({ ··· 54 63 let indexer: Indexer; 55 64 56 65 beforeEach(() => { 66 + vi.clearAllMocks(); 57 67 mockDb = createMockDb(); 58 68 indexer = new Indexer(mockDb); 59 69 }); ··· 860 870 861 871 await expect(indexer.handleForumDelete(event)).rejects.toThrow("Hard delete failed"); 862 872 }); 873 + 874 + it("re-throws errors from handleModActionDelete and logs with context", async () => { 875 + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); 876 + (mockDb.transaction as ReturnType<typeof vi.fn>).mockRejectedValueOnce( 877 + new Error("Transaction aborted") 878 + ); 879 + 880 + const event = { 881 + did: "did:plc:forum", 882 + time_us: 1234567890, 883 + kind: "commit", 884 + commit: { 885 + rev: "abc", 886 + operation: "delete", 887 + collection: "space.atbb.modAction", 888 + rkey: "action1", 889 + }, 890 + } as any; 891 + 892 + await expect(indexer.handleModActionDelete(event)).rejects.toThrow("Transaction aborted"); 893 + expect(consoleSpy).toHaveBeenCalledWith( 894 + expect.stringContaining("Failed to delete modAction:"), 895 + expect.any(Error) 896 + ); 897 + 898 + consoleSpy.mockRestore(); 899 + }); 863 900 }); 864 901 865 902 describe("Delete Strategy Verification", () => { ··· 956 993 957 994 expect(mockDb.delete).toHaveBeenCalled(); 958 995 expect(mockDb.update).not.toHaveBeenCalled(); 996 + }); 997 + }); 998 + 999 + describe("Ban enforcement — handlePostCreate", () => { 1000 + it("skips indexing when the user is banned", async () => { 1001 + const { BanEnforcer } = await import("../ban-enforcer.js"); 1002 + const mockBanEnforcer = vi.mocked(BanEnforcer).mock.results.at(-1)!.value; 1003 + mockBanEnforcer.isBanned.mockResolvedValue(true); 1004 + 1005 + const event = { 1006 + did: "did:plc:banned123", 1007 + time_us: 1234567890, 1008 + kind: "commit", 1009 + commit: { 1010 + rev: "abc", 1011 + operation: "create", 1012 + collection: "space.atbb.post", 1013 + rkey: "post1", 1014 + cid: "cid123", 1015 + record: { 1016 + $type: "space.atbb.post", 1017 + text: "Hello world", 1018 + createdAt: "2024-01-01T00:00:00Z", 1019 + }, 1020 + }, 1021 + } as any; 1022 + 1023 + await indexer.handlePostCreate(event); 1024 + 1025 + // The DB insert should NOT have been called 1026 + expect(mockDb.insert).not.toHaveBeenCalled(); 1027 + }); 1028 + 1029 + it("indexes the post normally when the user is not banned", async () => { 1030 + const { BanEnforcer } = await import("../ban-enforcer.js"); 1031 + const mockBanEnforcer = vi.mocked(BanEnforcer).mock.results.at(-1)!.value; 1032 + mockBanEnforcer.isBanned.mockResolvedValue(false); 1033 + 1034 + // Set up select to return a user (ensureUser) and no parent/root posts 1035 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 1036 + from: vi.fn().mockReturnValue({ 1037 + where: vi.fn().mockReturnValue({ 1038 + limit: vi.fn().mockResolvedValue([{ did: "did:plc:user123" }]), 1039 + }), 1040 + }), 1041 + }); 1042 + 1043 + const event = { 1044 + did: "did:plc:user123", 1045 + time_us: 1234567890, 1046 + kind: "commit", 1047 + commit: { 1048 + rev: "abc", 1049 + operation: "create", 1050 + collection: "space.atbb.post", 1051 + rkey: "post1", 1052 + cid: "cid123", 1053 + record: { 1054 + $type: "space.atbb.post", 1055 + text: "Hello world", 1056 + createdAt: "2024-01-01T00:00:00Z", 1057 + }, 1058 + }, 1059 + } as any; 1060 + 1061 + await indexer.handlePostCreate(event); 1062 + 1063 + expect(mockDb.insert).toHaveBeenCalled(); 1064 + }); 1065 + }); 1066 + 1067 + 1068 + describe("Ban enforcement — handleModActionCreate", () => { 1069 + it("calls applyBan when a ban mod action is created", async () => { 1070 + const mockBanEnforcer = (indexer as any).banEnforcer; 1071 + 1072 + // Set up select to return a forum (getForumIdByDid) and then ensureUser 1073 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 1074 + from: vi.fn().mockReturnValue({ 1075 + where: vi.fn().mockReturnValue({ 1076 + limit: vi.fn().mockResolvedValue([{ id: 1n }]), 1077 + }), 1078 + }), 1079 + }); 1080 + 1081 + const event = { 1082 + did: "did:plc:forum", 1083 + time_us: 1234567890, 1084 + kind: "commit", 1085 + commit: { 1086 + rev: "abc", 1087 + operation: "create", 1088 + collection: "space.atbb.modAction", 1089 + rkey: "action1", 1090 + cid: "cid123", 1091 + record: { 1092 + $type: "space.atbb.modAction", 1093 + action: "space.atbb.modAction.ban", 1094 + subject: { did: "did:plc:target123" }, 1095 + createdBy: "did:plc:mod", 1096 + createdAt: "2024-01-01T00:00:00Z", 1097 + }, 1098 + }, 1099 + } as any; 1100 + 1101 + await indexer.handleModActionCreate(event); 1102 + 1103 + expect(mockBanEnforcer.applyBan).toHaveBeenCalledWith("did:plc:target123", expect.any(Object)); 1104 + }); 1105 + 1106 + it("does NOT call applyBan for non-ban actions (e.g. pin)", async () => { 1107 + const mockBanEnforcer = (indexer as any).banEnforcer; 1108 + 1109 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 1110 + from: vi.fn().mockReturnValue({ 1111 + where: vi.fn().mockReturnValue({ 1112 + limit: vi.fn().mockResolvedValue([{ id: 1n }]), 1113 + }), 1114 + }), 1115 + }); 1116 + 1117 + const event = { 1118 + did: "did:plc:forum", 1119 + time_us: 1234567890, 1120 + kind: "commit", 1121 + commit: { 1122 + rev: "abc", 1123 + operation: "create", 1124 + collection: "space.atbb.modAction", 1125 + rkey: "action2", 1126 + cid: "cid124", 1127 + record: { 1128 + $type: "space.atbb.modAction", 1129 + action: "space.atbb.modAction.pin", 1130 + subject: { post: { uri: "at://did:plc:user/space.atbb.post/abc", cid: "cid" } }, 1131 + createdBy: "did:plc:mod", 1132 + createdAt: "2024-01-01T00:00:00Z", 1133 + }, 1134 + }, 1135 + } as any; 1136 + 1137 + await indexer.handleModActionCreate(event); 1138 + 1139 + expect(mockBanEnforcer.applyBan).not.toHaveBeenCalled(); 1140 + }); 1141 + 1142 + it("does NOT call applyBan when the ban record insert was skipped (unknown forum DID)", async () => { 1143 + const mockBanEnforcer = (indexer as any).banEnforcer; 1144 + 1145 + // Select returns empty — forum DID not found, toInsertValues returns null → insert skipped 1146 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 1147 + from: vi.fn().mockReturnValue({ 1148 + where: vi.fn().mockReturnValue({ 1149 + limit: vi.fn().mockResolvedValue([]), 1150 + }), 1151 + }), 1152 + }); 1153 + 1154 + const event = { 1155 + did: "did:plc:unknown-forum", 1156 + time_us: 1234567890, 1157 + kind: "commit", 1158 + commit: { 1159 + rev: "abc", 1160 + operation: "create", 1161 + collection: "space.atbb.modAction", 1162 + rkey: "action1", 1163 + cid: "cid123", 1164 + record: { 1165 + $type: "space.atbb.modAction", 1166 + action: "space.atbb.modAction.ban", 1167 + subject: { did: "did:plc:target123" }, 1168 + createdBy: "did:plc:mod", 1169 + createdAt: "2024-01-01T00:00:00Z", 1170 + }, 1171 + }, 1172 + } as any; 1173 + 1174 + await indexer.handleModActionCreate(event); 1175 + 1176 + expect(mockBanEnforcer.applyBan).not.toHaveBeenCalled(); 1177 + }); 1178 + 1179 + it("logs warning and skips applyBan when ban action has no subject.did", async () => { 1180 + const mockBanEnforcer = (indexer as any).banEnforcer; 1181 + const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); 1182 + 1183 + // Mock select to return forum found 1184 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 1185 + from: vi.fn().mockReturnValue({ 1186 + where: vi.fn().mockReturnValue({ 1187 + limit: vi.fn().mockResolvedValue([{ id: 1n }]), 1188 + }), 1189 + }), 1190 + }); 1191 + 1192 + const event = { 1193 + did: "did:plc:forum", 1194 + time_us: 1234567890, 1195 + kind: "commit", 1196 + commit: { 1197 + rev: "abc", 1198 + operation: "create", 1199 + collection: "space.atbb.modAction", 1200 + rkey: "action1", 1201 + cid: "cid123", 1202 + record: { 1203 + $type: "space.atbb.modAction", 1204 + action: "space.atbb.modAction.ban", 1205 + subject: {}, // no did field 1206 + createdBy: "did:plc:mod", 1207 + createdAt: "2024-01-01T00:00:00Z", 1208 + }, 1209 + }, 1210 + } as any; 1211 + 1212 + await indexer.handleModActionCreate(event); 1213 + 1214 + expect(mockBanEnforcer.applyBan).not.toHaveBeenCalled(); 1215 + expect(consoleSpy).toHaveBeenCalledWith( 1216 + expect.stringContaining("missing subject.did"), 1217 + expect.any(Object) 1218 + ); 1219 + 1220 + consoleSpy.mockRestore(); 1221 + }); 1222 + 1223 + it("calls liftBan when an unban mod action is indexed", async () => { 1224 + const mockBanEnforcer = (indexer as any).banEnforcer; 1225 + 1226 + // Mock select to return forum found 1227 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 1228 + from: vi.fn().mockReturnValue({ 1229 + where: vi.fn().mockReturnValue({ 1230 + limit: vi.fn().mockResolvedValue([{ id: 1n }]), 1231 + }), 1232 + }), 1233 + }); 1234 + 1235 + const event = { 1236 + did: "did:plc:forum", 1237 + time_us: 1234567891, 1238 + kind: "commit", 1239 + commit: { 1240 + rev: "def", 1241 + operation: "create", 1242 + collection: "space.atbb.modAction", 1243 + rkey: "action2", 1244 + cid: "cid124", 1245 + record: { 1246 + $type: "space.atbb.modAction", 1247 + action: "space.atbb.modAction.unban", 1248 + subject: { did: "did:plc:target123" }, 1249 + createdBy: "did:plc:mod", 1250 + createdAt: "2024-01-01T00:00:01Z", 1251 + }, 1252 + }, 1253 + } as any; 1254 + 1255 + await indexer.handleModActionCreate(event); 1256 + 1257 + expect(mockBanEnforcer.liftBan).toHaveBeenCalledWith( 1258 + "did:plc:target123", 1259 + expect.any(Object) // transaction context 1260 + ); 1261 + expect(mockBanEnforcer.applyBan).not.toHaveBeenCalled(); 1262 + }); 1263 + it("race condition: post indexed before ban — ban retroactively hides it", async () => { 1264 + const mockBanEnforcer = (indexer as any).banEnforcer; 1265 + 1266 + // Step 1: Post arrives before ban — isBanned returns false at this moment 1267 + mockBanEnforcer.isBanned.mockResolvedValueOnce(false); 1268 + 1269 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 1270 + from: vi.fn().mockReturnValue({ 1271 + where: vi.fn().mockReturnValue({ 1272 + limit: vi.fn().mockResolvedValue([{ id: 1n }]), 1273 + }), 1274 + }), 1275 + }); 1276 + 1277 + const postEvent = { 1278 + did: "did:plc:target123", 1279 + time_us: 1234567890, 1280 + kind: "commit", 1281 + commit: { 1282 + rev: "abc", 1283 + operation: "create", 1284 + collection: "space.atbb.post", 1285 + rkey: "post1", 1286 + cid: "cid123", 1287 + record: { 1288 + $type: "space.atbb.post", 1289 + text: "Hello world", 1290 + createdAt: "2024-01-01T00:00:00Z", 1291 + }, 1292 + }, 1293 + } as any; 1294 + 1295 + await indexer.handlePostCreate(postEvent); 1296 + expect(mockDb.insert).toHaveBeenCalled(); // post was actually inserted into DB before ban arrived 1297 + 1298 + // Step 2: Ban arrives — applyBan retroactively hides the post 1299 + (mockDb.select as ReturnType<typeof vi.fn>).mockReturnValue({ 1300 + from: vi.fn().mockReturnValue({ 1301 + where: vi.fn().mockReturnValue({ 1302 + limit: vi.fn().mockResolvedValue([{ id: 1n }]), 1303 + }), 1304 + }), 1305 + }); 1306 + 1307 + const banEvent = { 1308 + did: "did:plc:forum", 1309 + time_us: 1234567891, 1310 + kind: "commit", 1311 + commit: { 1312 + rev: "def", 1313 + operation: "create", 1314 + collection: "space.atbb.modAction", 1315 + rkey: "action1", 1316 + cid: "cid124", 1317 + record: { 1318 + $type: "space.atbb.modAction", 1319 + action: "space.atbb.modAction.ban", 1320 + subject: { did: "did:plc:target123" }, 1321 + createdBy: "did:plc:mod", 1322 + createdAt: "2024-01-01T00:00:01Z", 1323 + }, 1324 + }, 1325 + } as any; 1326 + 1327 + await indexer.handleModActionCreate(banEvent); 1328 + expect(mockBanEnforcer.applyBan).toHaveBeenCalledWith("did:plc:target123", expect.any(Object)); 1329 + }); 1330 + }); 1331 + 1332 + describe("Ban enforcement — handleModActionDelete", () => { 1333 + it("calls liftBan when a ban record is deleted", async () => { 1334 + const mockBanEnforcer = (indexer as any).banEnforcer; 1335 + 1336 + // Transaction mock: select returns a ban record, delete succeeds 1337 + (mockDb.transaction as ReturnType<typeof vi.fn>).mockImplementation( 1338 + async (callback: (tx: any) => Promise<any>) => { 1339 + const tx = { 1340 + select: vi.fn().mockReturnValue({ 1341 + from: vi.fn().mockReturnValue({ 1342 + where: vi.fn().mockReturnValue({ 1343 + limit: vi.fn().mockResolvedValue([ 1344 + { 1345 + action: "space.atbb.modAction.ban", 1346 + subjectDid: "did:plc:target123", 1347 + }, 1348 + ]), 1349 + }), 1350 + }), 1351 + }), 1352 + delete: vi.fn().mockReturnValue({ 1353 + where: vi.fn().mockResolvedValue(undefined), 1354 + }), 1355 + }; 1356 + return callback(tx); 1357 + } 1358 + ); 1359 + 1360 + const event = { 1361 + did: "did:plc:forum", 1362 + time_us: 1234567890, 1363 + kind: "commit", 1364 + commit: { 1365 + rev: "abc", 1366 + operation: "delete", 1367 + collection: "space.atbb.modAction", 1368 + rkey: "action1", 1369 + }, 1370 + } as any; 1371 + 1372 + await indexer.handleModActionDelete(event); 1373 + 1374 + expect(mockBanEnforcer.liftBan).toHaveBeenCalledWith( 1375 + "did:plc:target123", 1376 + expect.anything() // the transaction 1377 + ); 1378 + }); 1379 + 1380 + it("does NOT call liftBan when a non-ban record is deleted", async () => { 1381 + const mockBanEnforcer = (indexer as any).banEnforcer; 1382 + 1383 + (mockDb.transaction as ReturnType<typeof vi.fn>).mockImplementation( 1384 + async (callback: (tx: any) => Promise<any>) => { 1385 + const tx = { 1386 + select: vi.fn().mockReturnValue({ 1387 + from: vi.fn().mockReturnValue({ 1388 + where: vi.fn().mockReturnValue({ 1389 + limit: vi.fn().mockResolvedValue([ 1390 + { 1391 + action: "space.atbb.modAction.pin", 1392 + subjectDid: null, 1393 + }, 1394 + ]), 1395 + }), 1396 + }), 1397 + }), 1398 + delete: vi.fn().mockReturnValue({ 1399 + where: vi.fn().mockResolvedValue(undefined), 1400 + }), 1401 + }; 1402 + return callback(tx); 1403 + } 1404 + ); 1405 + 1406 + const event = { 1407 + did: "did:plc:forum", 1408 + time_us: 1234567890, 1409 + kind: "commit", 1410 + commit: { 1411 + rev: "abc", 1412 + operation: "delete", 1413 + collection: "space.atbb.modAction", 1414 + rkey: "action2", 1415 + }, 1416 + } as any; 1417 + 1418 + await indexer.handleModActionDelete(event); 1419 + 1420 + expect(mockBanEnforcer.liftBan).not.toHaveBeenCalled(); 1421 + }); 1422 + 1423 + it("does NOT call liftBan when the record is not found (already deleted)", async () => { 1424 + const mockBanEnforcer = (indexer as any).banEnforcer; 1425 + 1426 + (mockDb.transaction as ReturnType<typeof vi.fn>).mockImplementation( 1427 + async (callback: (tx: any) => Promise<any>) => { 1428 + const tx = { 1429 + select: vi.fn().mockReturnValue({ 1430 + from: vi.fn().mockReturnValue({ 1431 + where: vi.fn().mockReturnValue({ 1432 + limit: vi.fn().mockResolvedValue([]), 1433 + }), 1434 + }), 1435 + }), 1436 + delete: vi.fn().mockReturnValue({ 1437 + where: vi.fn().mockResolvedValue(undefined), 1438 + }), 1439 + }; 1440 + return callback(tx); 1441 + } 1442 + ); 1443 + 1444 + const event = { 1445 + did: "did:plc:forum", 1446 + time_us: 1234567890, 1447 + kind: "commit", 1448 + commit: { 1449 + rev: "abc", 1450 + operation: "delete", 1451 + collection: "space.atbb.modAction", 1452 + rkey: "action3", 1453 + }, 1454 + } as any; 1455 + 1456 + await indexer.handleModActionDelete(event); 1457 + 1458 + expect(mockBanEnforcer.liftBan).not.toHaveBeenCalled(); 959 1459 }); 960 1460 }); 961 1461 });
+95
apps/appview/src/lib/ban-enforcer.ts
··· 1 + import type { DbOrTransaction } from "@atbb/db"; 2 + import { modActions, posts } from "@atbb/db"; 3 + import { and, eq, gt, isNull, or } from "drizzle-orm"; 4 + import { isProgrammingError } from "./errors.js"; 5 + 6 + /** 7 + * Encapsulates ban enforcement logic for the firehose indexer. 8 + * 9 + * Used by the Indexer to: 10 + * - Check ban status before indexing posts (fail closed) 11 + * - Soft-delete existing posts when a ban is applied 12 + * - Restore posts when a ban is lifted 13 + */ 14 + export class BanEnforcer { 15 + constructor(private db: DbOrTransaction) {} 16 + 17 + /** 18 + * Returns true if the DID has an active (non-expired) ban. 19 + * Fails closed: returns true if the DB query throws. 20 + */ 21 + async isBanned(did: string): Promise<boolean> { 22 + try { 23 + const now = new Date(); 24 + const result = await this.db 25 + .select({ id: modActions.id }) 26 + .from(modActions) 27 + .where( 28 + and( 29 + eq(modActions.subjectDid, did), 30 + eq(modActions.action, "space.atbb.modAction.ban"), 31 + or(isNull(modActions.expiresAt), gt(modActions.expiresAt, now)) 32 + ) 33 + ) 34 + .limit(1); 35 + 36 + return result.length > 0; 37 + } catch (error) { 38 + if (isProgrammingError(error)) throw error; 39 + console.error( 40 + "Failed to check ban status - denying indexing (fail closed)", 41 + { 42 + did, 43 + error: error instanceof Error ? error.message : String(error), 44 + } 45 + ); 46 + return true; // fail closed 47 + } 48 + } 49 + 50 + /** 51 + * Soft-deletes all posts for the given DID. 52 + * Called when a ban mod action is indexed. 53 + */ 54 + async applyBan(subjectDid: string, dbOrTx: DbOrTransaction = this.db): Promise<void> { 55 + try { 56 + await dbOrTx 57 + .update(posts) 58 + .set({ deleted: true }) 59 + .where(eq(posts.did, subjectDid)); 60 + console.log("[BAN] Applied ban: soft-deleted all posts", { subjectDid }); 61 + } catch (error) { 62 + console.error("Failed to apply ban - posts may not be hidden", { 63 + subjectDid, 64 + error: error instanceof Error ? error.message : String(error), 65 + }); 66 + throw error; 67 + } 68 + } 69 + 70 + /** 71 + * Restores all posts for the given DID. 72 + * Called when a ban mod action record is deleted (unban). 73 + * 74 + * NOTE (ATB-25): This unconditionally sets deleted = false for all posts 75 + * by the subject DID. The `deleted` column is shared between ban enforcement 76 + * and user-initiated deletes, so posts the user intentionally removed while 77 + * banned will be silently restored on unban. Fix requires a separate 78 + * `banned_by_mod` column. Tracked in ATB-25. 79 + */ 80 + async liftBan(subjectDid: string, dbOrTx: DbOrTransaction = this.db): Promise<void> { 81 + try { 82 + await dbOrTx 83 + .update(posts) 84 + .set({ deleted: false }) 85 + .where(eq(posts.did, subjectDid)); 86 + console.log("[UNBAN] Lifted ban: restored all posts", { subjectDid }); 87 + } catch (error) { 88 + console.error("Failed to lift ban - posts may not be restored", { 89 + subjectDid, 90 + error: error instanceof Error ? error.message : String(error), 91 + }); 92 + throw error; 93 + } 94 + } 95 + }
+188 -16
apps/appview/src/lib/indexer.ts
··· 16 16 } from "@atbb/db"; 17 17 import { eq, and } from "drizzle-orm"; 18 18 import { parseAtUri } from "./at-uri.js"; 19 + import { BanEnforcer } from "./ban-enforcer.js"; 19 20 import { 20 21 SpaceAtbbPost as Post, 21 22 SpaceAtbbForumForum as Forum, ··· 68 69 * Converts events into database records for the atBB AppView 69 70 */ 70 71 export class Indexer { 71 - constructor(private db: Database) {} 72 + private banEnforcer: BanEnforcer; 73 + 74 + constructor(private db: Database) { 75 + this.banEnforcer = new BanEnforcer(db); 76 + } 72 77 73 78 // ── Collection Configs ────────────────────────────────── 74 79 ··· 452 457 private async genericCreate<TRecord>( 453 458 config: CollectionConfig<TRecord>, 454 459 event: any 455 - ) { 460 + ): Promise<boolean> { 456 461 try { 457 462 const record = event.commit.record as unknown as TRecord; 458 463 let skipped = false; ··· 477 482 `[CREATE] ${config.name}: ${event.did}/${event.commit.rkey}` 478 483 ); 479 484 } 485 + return !skipped; 480 486 } catch (error) { 481 487 console.error( 482 488 `Failed to index ${config.name.toLowerCase()} create: ${event.did}/${event.commit.rkey}`, ··· 574 580 // ── Post Handlers ─────────────────────────────────────── 575 581 576 582 async handlePostCreate(event: CommitCreateEvent<"space.atbb.post">) { 583 + const banned = await this.banEnforcer.isBanned(event.did); 584 + if (banned) { 585 + console.log( 586 + `[SKIP] Post from banned user: ${event.did}/${event.commit.rkey}` 587 + ); 588 + return; 589 + } 577 590 await this.genericCreate(this.postConfig, event); 578 591 } 579 592 ··· 672 685 async handleModActionCreate( 673 686 event: CommitCreateEvent<"space.atbb.modAction"> 674 687 ) { 675 - await this.genericCreate(this.modActionConfig, event); 688 + const record = event.commit.record as unknown as ModAction.Record; 689 + const isBan = 690 + record.action === "space.atbb.modAction.ban" && record.subject.did; 691 + const isUnban = 692 + record.action === "space.atbb.modAction.unban" && record.subject.did; 693 + 694 + try { 695 + if (isBan) { 696 + // Custom atomic path: insert ban record + applyBan in one transaction 697 + let skipped = false; 698 + await this.db.transaction(async (tx) => { 699 + const forumId = await this.getForumIdByDid(event.did, tx); 700 + if (!forumId) { 701 + console.warn( 702 + `[CREATE] ModAction (ban): Forum not found for DID ${event.did}` 703 + ); 704 + skipped = true; 705 + return; 706 + } 707 + await this.ensureUser(record.createdBy, tx); 708 + await tx.insert(modActions).values({ 709 + did: event.did, 710 + rkey: event.commit.rkey, 711 + cid: event.commit.cid, 712 + forumId, 713 + action: record.action, 714 + subjectPostUri: null, 715 + subjectDid: record.subject.did ?? null, 716 + reason: record.reason ?? null, 717 + createdBy: record.createdBy, 718 + expiresAt: record.expiresAt ? new Date(record.expiresAt) : null, 719 + createdAt: new Date(record.createdAt), 720 + indexedAt: new Date(), 721 + }); 722 + await this.banEnforcer.applyBan(record.subject.did!, tx); 723 + }); 724 + if (!skipped) { 725 + console.log( 726 + `[CREATE] ModAction (ban): ${event.did}/${event.commit.rkey}` 727 + ); 728 + } 729 + } else if (isUnban) { 730 + // Custom atomic path: insert unban record + liftBan in one transaction 731 + let skipped = false; 732 + await this.db.transaction(async (tx) => { 733 + const forumId = await this.getForumIdByDid(event.did, tx); 734 + if (!forumId) { 735 + console.warn( 736 + `[CREATE] ModAction (unban): Forum not found for DID ${event.did}` 737 + ); 738 + skipped = true; 739 + return; 740 + } 741 + await this.ensureUser(record.createdBy, tx); 742 + await tx.insert(modActions).values({ 743 + did: event.did, 744 + rkey: event.commit.rkey, 745 + cid: event.commit.cid, 746 + forumId, 747 + action: record.action, 748 + subjectPostUri: null, 749 + subjectDid: record.subject.did ?? null, 750 + reason: record.reason ?? null, 751 + createdBy: record.createdBy, 752 + expiresAt: record.expiresAt ? new Date(record.expiresAt) : null, 753 + createdAt: new Date(record.createdAt), 754 + indexedAt: new Date(), 755 + }); 756 + await this.banEnforcer.liftBan(record.subject.did!, tx); 757 + }); 758 + if (!skipped) { 759 + console.log( 760 + `[CREATE] ModAction (unban): ${event.did}/${event.commit.rkey}` 761 + ); 762 + } 763 + } else { 764 + // Generic path for all other mod actions (mute, pin, lock, delete, etc.) 765 + await this.genericCreate(this.modActionConfig, event); 766 + 767 + // Ban/unban without subject.did — shouldn't happen but log if it does 768 + if ( 769 + record.action === "space.atbb.modAction.ban" || 770 + record.action === "space.atbb.modAction.unban" 771 + ) { 772 + console.warn( 773 + "[CREATE] ModAction: ban/unban action missing subject.did, skipping enforcement", 774 + { did: event.did, rkey: event.commit.rkey, action: record.action } 775 + ); 776 + } 777 + } 778 + } catch (error) { 779 + console.error( 780 + `Failed to index ModAction create: ${event.did}/${event.commit.rkey}`, 781 + error 782 + ); 783 + throw error; 784 + } 676 785 } 677 786 678 787 async handleModActionUpdate( ··· 684 793 async handleModActionDelete( 685 794 event: CommitDeleteEvent<"space.atbb.modAction"> 686 795 ) { 687 - await this.genericDelete(this.modActionConfig, event); 796 + try { 797 + await this.db.transaction(async (tx) => { 798 + // 1. Read before delete to capture action type and subject 799 + const [existing] = await tx 800 + .select({ 801 + action: modActions.action, 802 + subjectDid: modActions.subjectDid, 803 + }) 804 + .from(modActions) 805 + .where( 806 + and( 807 + eq(modActions.did, event.did), 808 + eq(modActions.rkey, event.commit.rkey) 809 + ) 810 + ) 811 + .limit(1); 812 + 813 + // 2. Hard delete the record 814 + await tx 815 + .delete(modActions) 816 + .where( 817 + and( 818 + eq(modActions.did, event.did), 819 + eq(modActions.rkey, event.commit.rkey) 820 + ) 821 + ); 822 + 823 + // 3. Restore posts if the deleted record was a ban 824 + if ( 825 + existing?.action === "space.atbb.modAction.ban" && 826 + existing?.subjectDid 827 + ) { 828 + await this.banEnforcer.liftBan(existing.subjectDid, tx); 829 + } 830 + }); 831 + 832 + console.log( 833 + `[DELETE] ModAction: ${event.did}/${event.commit.rkey}` 834 + ); 835 + } catch (error) { 836 + console.error( 837 + `Failed to delete modAction: ${event.did}/${event.commit.rkey}`, 838 + error 839 + ); 840 + throw error; 841 + } 688 842 } 689 843 690 844 // ── Reaction Handlers (Stub) ──────────────────────────── ··· 745 899 const parsed = parseAtUri(forumUri); 746 900 if (!parsed) return null; 747 901 748 - const result = await dbOrTx 749 - .select({ id: forums.id }) 750 - .from(forums) 751 - .where(and(eq(forums.did, parsed.did), eq(forums.rkey, parsed.rkey))) 752 - .limit(1); 902 + try { 903 + const result = await dbOrTx 904 + .select({ id: forums.id }) 905 + .from(forums) 906 + .where(and(eq(forums.did, parsed.did), eq(forums.rkey, parsed.rkey))) 907 + .limit(1); 753 908 754 - return result.length > 0 ? result[0].id : null; 909 + return result.length > 0 ? result[0].id : null; 910 + } catch (error) { 911 + console.error("Database error in getForumIdByUri", { 912 + operation: "getForumIdByUri", 913 + forumUri, 914 + error: error instanceof Error ? error.message : String(error), 915 + }); 916 + throw error; 917 + } 755 918 } 756 919 757 920 /** ··· 763 926 forumDid: string, 764 927 dbOrTx: DbOrTransaction = this.db 765 928 ): Promise<bigint | null> { 766 - const result = await dbOrTx 767 - .select({ id: forums.id }) 768 - .from(forums) 769 - .where(eq(forums.did, forumDid)) 770 - .limit(1); 929 + try { 930 + const result = await dbOrTx 931 + .select({ id: forums.id }) 932 + .from(forums) 933 + .where(eq(forums.did, forumDid)) 934 + .limit(1); 771 935 772 - return result.length > 0 ? result[0].id : null; 936 + return result.length > 0 ? result[0].id : null; 937 + } catch (error) { 938 + console.error("Database error in getForumIdByDid", { 939 + operation: "getForumIdByDid", 940 + forumDid, 941 + error: error instanceof Error ? error.message : String(error), 942 + }); 943 + throw error; 944 + } 773 945 } 774 946 775 947 /**
+1 -1
apps/appview/src/routes/mod.ts
··· 1 1 import { Hono } from "hono"; 2 2 import { eq, desc } from "drizzle-orm"; 3 3 import { modActions, memberships, posts } from "@atbb/db"; 4 - import { TID } from "@atproto/common"; 4 + import { TID } from "@atproto/common-web"; 5 5 import { requireAuth } from "../middleware/auth.js"; 6 6 import { requirePermission } from "../middleware/permissions.js"; 7 7 import { isProgrammingError, isNetworkError } from "../lib/errors.js";
+14 -2
docs/atproto-forum-plan.md
··· 199 199 - Files: `apps/appview/src/routes/mod.ts` (~700 lines), `apps/appview/src/routes/__tests__/mod.test.ts` (~3414 lines), `apps/appview/src/lib/errors.ts` (error classification helpers) 200 200 - Bruno API collection: `bruno/AppView API/Moderation/` (6 .bru files documenting all endpoints) 201 201 - [ ] Admin UI: ban user, lock topic, hide post (ATB-24) 202 - - [ ] AppView respects mod actions during indexing and API responses (read-path) 203 - - [ ] Banned users' new records are ignored by indexer 202 + - [x] **ATB-20: Enforce mod actions in read/write-path API responses** — **Complete:** 2026-02-16 203 + - All API read endpoints filter soft-deleted posts (`deleted = false` in all queries) 204 + - All API write endpoints (topic/post create) block banned users at request time 205 + - `requireNotBanned()` middleware checks `mod_actions` before allowing writes 206 + - Comprehensive test coverage for banned user scenarios across all write endpoints 207 + - [x] **ATB-21: Enforce mod actions in firehose indexer** — **Complete:** 2026-02-17 208 + - New `BanEnforcer` class composed into `Indexer` (fail-closed: DB error → treat as banned) 209 + - `handlePostCreate`: checks `isBanned(event.did)` before indexing; banned users' posts are silently dropped (never inserted) 210 + - `handleModActionCreate`: after inserting a ban mod action, calls `applyBan(subjectDid)` to soft-delete all existing posts — retroactive enforcement 211 + - `handleModActionDelete`: read-before-delete in a single transaction; if deleted action was a ban, calls `liftBan(subjectDid)` to restore posts 212 + - Decision documented: skip (not soft-mark) for new posts; reuse `deleted` column (no new column); DB query with existing `mod_actions_subject_did_idx` index (no cache) 213 + - Race condition handled: post-before-ban path covered by eventual consistency — post inserts normally, `applyBan` soft-deletes it when ban arrives 214 + - 8 new integration tests in `indexer.test.ts`; 7 unit tests in `indexer-ban-enforcer.test.ts`; 491 total tests passing 215 + - Files: `apps/appview/src/lib/ban-enforcer.ts` (new), `apps/appview/src/lib/indexer.ts` (3 handler overrides), `apps/appview/src/lib/__tests__/indexer-ban-enforcer.test.ts` (new), `apps/appview/src/lib/__tests__/indexer.test.ts` (additions) 204 216 - [ ] Document the trust model: operators must trust their AppView instance, which is acceptable for self-hosted single-server deployments 205 217 206 218 #### Phase 4: Web UI (Week 7–9)