v2.2.4 by byteful · Pull Request #24 · modl-gg/backend

byteful · 2026-05-16T05:08:23Z

Confidence Score: 2/5

This should be fixed before merging.

Minecraft realtime subscriptions can expose tenant-wide events to any API-key plugin connection.
The new presence endpoint can mark all players offline when a roster is omitted.
AI suggestion actions now report failed operations with a successful HTTP status.

Focus on the realtime topic authorization, the new presence endpoint, and the AI suggestion action responses.

Security Review

Authorization scope issue in RealtimeTopicAuthorizer: Minecraft API-key clients can subscribe to tenant-wide realtime player notification and staff 2FA topics without a player or staff scope.

Important Files Changed

Filename	Overview
src/main/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizer.java	Expands panel and Minecraft websocket topic authorization, including tenant-wide Minecraft topics.
src/main/java/gg/modl/backend/player/controller/MinecraftSyncV3Controller.java	Adds split sync endpoints for presence, logs, and server status.
src/main/java/gg/modl/backend/settings/controller/PanelSettingsController.java	Converts settings endpoints to proto responses and publishes settings invalidations.

Prompt To Fix All With AI

Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
src/main/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizer.java:72-76
**Minecraft topics over-authorized**

This now lets any websocket authenticated with the tenant Minecraft API key subscribe to tenant-wide player notifications, staff 2FA pushes, punishments, migration tasks, and stat wipes. The Minecraft principal only carries the server and optional instance id, so there is no player or staff scope to filter these topics. A plugin instance that should only receive its relevant sync data can receive every pushed player notification or staff 2FA verification event for the tenant.

### Issue 2 of 4
src/main/java/gg/modl/backend/player/controller/MinecraftSyncV3Controller.java:91-98
**Missing roster clears presence**

The new `/presence` endpoint treats an absent `onlinePlayers` field as an empty list and immediately calls `applyPresence`. That path marks every stale player not in the supplied set as offline, so a malformed or partial presence heartbeat with no roster succeeds while setting the server count to 0 and reconciling all players for that server as offline. This endpoint should reject requests without an explicit roster, or use a request shape that can distinguish “no roster sent” from “zero players online.”

### Issue 3 of 4
src/main/java/gg/modl/backend/settings/controller/PanelSettingsController.java:360-361
**Failures return success status**

This endpoint now returns an `AISuggestionActionResponse` directly for both successful and failed `applyAISuggestion` results. The previous path returned `400` when `result.success()` was false, but now a failed punishment application returns HTTP 200 with `success=false`. Existing clients that rely on the non-2xx status can show the action as completed even though the service rejected it.

```suggestion
        AITicketAnalysisService.AISuggestionResult result = aiTicketAnalysisService.applyAISuggestion(server, ticketId, staffName);
        if (!result.success()) {
            throw new ValidationException(result.error());
        }
        return toAISuggestionResponse(result);
```

### Issue 4 of 4
src/main/java/gg/modl/backend/settings/controller/PanelSettingsController.java:371-372
**Dismiss failures return success status**

The dismiss path has the same status-code regression as apply. When `dismissAISuggestion` fails, the controller still returns HTTP 200 with `success=false`, while the old implementation returned a bad request. A panel caller that only checks the response status can remove or mark the suggestion as dismissed even though the backend did not dismiss it.

```suggestion
        AITicketAnalysisService.AISuggestionResult result = aiTicketAnalysisService.dismissAISuggestion(server, ticketId);
        if (!result.success()) {
            throw new ValidationException(result.error());
        }
        return toAISuggestionResponse(result);
```

_{Reviews (2): Last reviewed commit: "websocket realtime + proto-json controll..." | Re-trigger Greptile}

Greptile also left 4 inline comments on this PR.

Context used:

Rule used - This is a Java Spring Boot REST API backed by Mong... (source)

# Conflicts: # src/main/java/gg/modl/backend/infrastructure/filter/V1SecurityConfig.java # src/main/java/gg/modl/backend/infrastructure/ratelimit/RateLimitConfig.java # src/main/java/gg/modl/backend/infrastructure/rest/RESTMappingV1.java # src/test/java/gg/modl/backend/ratelimit/RateLimitConfigTest.java

security fixes

Realtime

Replay lite

byteful · 2026-05-22T04:36:30Z

@greptileai review pls

greptile-apps · 2026-05-22T04:39:17Z

+        ticket.setAiAnalysis(result);
+        ticket.setUpdatedAt(new Date());
+        ticketRepository.saveEntity(server, ticket);


Automated actions skipped

This path now only saves the AI analysis result. When enableAutomatedActions is true and the model reports a violation, executeAutomatedAction is never called, so configured automated punishments or ticket actions silently stop running even though the setting still exists.

Suggested change

ticket.setAiAnalysis(result);

ticket.setUpdatedAt(new Date());

ticketRepository.saveEntity(server, ticket);

if (settings.isEnableAutomatedActions() && result.hasViolation()) {

executeAutomatedAction(server, ticket, result, settings);

}

ticket.setAiAnalysis(result);

ticket.setUpdatedAt(new Date());

ticketRepository.saveEntity(server, ticket);

Prompt To Fix With AI

This is a comment left during a code review. Path: src/main/java/gg/modl/backend/ai/service/AITicketAnalysisService.java Line: 99-101 Comment: **Automated actions skipped** This path now only saves the AI analysis result. When `enableAutomatedActions` is true and the model reports a violation, `executeAutomatedAction` is never called, so configured automated punishments or ticket actions silently stop running even though the setting still exists. ```suggestion if (settings.isEnableAutomatedActions() && result.hasViolation()) { executeAutomatedAction(server, ticket, result, settings); } ticket.setAiAnalysis(result); ticket.setUpdatedAt(new Date()); ticketRepository.saveEntity(server, ticket); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-05-22T04:39:18Z

+        InitReplayUploadResponse response = replayService.initUpload(
+            server,
+            request.getMcVersion(),
+            request.getFileSize()
+        );


Target metadata dropped

The v3 upload endpoint calls the overload that sets targetUuid and targetName to null. A v3 client can upload a replay for a player, but the saved ReplayDocument has no target metadata, so player replay lookup by targetUuid cannot find it. The legacy controller passes those fields through.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/main/java/gg/modl/backend/replay/controller/MinecraftReplayV3Controller.java Line: 38-42 Comment: **Target metadata dropped** The v3 upload endpoint calls the overload that sets `targetUuid` and `targetName` to null. A v3 client can upload a replay for a player, but the saved `ReplayDocument` has no target metadata, so player replay lookup by `targetUuid` cannot find it. The legacy controller passes those fields through. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-05-22T04:39:19Z

+            Criteria.where(TicketVerificationFields.EXPIRES_AT).gte(now),
+            Criteria.where("failedAttempts").lt(MAX_FAILED_ATTEMPTS)


Old codes stop working

Adding failedAttempts < MAX_FAILED_ATTEMPTS makes code verification fail for active verification documents that were created before this field existed. Mongo does not match missing fields with $lt, so a user who received a valid code before deploy can enter the correct code after deploy and still get rejected.

Suggested change

Criteria.where(TicketVerificationFields.EXPIRES_AT).gte(now),

Criteria.where("failedAttempts").lt(MAX_FAILED_ATTEMPTS)

Criteria.where(TicketVerificationFields.EXPIRES_AT).gte(now),

new Criteria().orOperator(

Criteria.where("failedAttempts").exists(false),

Criteria.where("failedAttempts").is(null),

Criteria.where("failedAttempts").lt(MAX_FAILED_ATTEMPTS)

)

Prompt To Fix With AI

This is a comment left during a code review. Path: src/main/java/gg/modl/backend/database/mongo/repository/TicketVerificationMongoRepository.java Line: 41-42 Comment: **Old codes stop working** Adding `failedAttempts < MAX_FAILED_ATTEMPTS` makes code verification fail for active verification documents that were created before this field existed. Mongo does not match missing fields with `$lt`, so a user who received a valid code before deploy can enter the correct code after deploy and still get rejected. ```suggestion Criteria.where(TicketVerificationFields.EXPIRES_AT).gte(now), new Criteria().orOperator( Criteria.where("failedAttempts").exists(false), Criteria.where("failedAttempts").is(null), Criteria.where("failedAttempts").lt(MAX_FAILED_ATTEMPTS) ) ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-05-22T04:39:20Z

+        String serverDomain = resolvePanelServerDomain(hello);
+        if (serverDomain == null) {
+            throw new RealtimeAuthenticationException(ErrorCode.ERROR_CODE_FORBIDDEN, "Missing or invalid tenant scope");
+        }


Host tenant ignored

Panel WebSocket auth now requires the tenant domain to be repeated in ClientHello.serverName. A browser connecting from the tenant host with a valid session cookie but no serverName is rejected with “Missing or invalid tenant scope”. The helper that resolves the tenant from Host / X-Forwarded-Host is present in this class but is not used, so this does not match the normal HTTP tenant resolution path.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/main/java/gg/modl/backend/realtime/auth/RealtimeAuthenticator.java Line: 45-48 Comment: **Host tenant ignored** Panel WebSocket auth now requires the tenant domain to be repeated in `ClientHello.serverName`. A browser connecting from the tenant host with a valid session cookie but no `serverName` is rejected with “Missing or invalid tenant scope”. The helper that resolves the tenant from `Host` / `X-Forwarded-Host` is present in this class but is not used, so this does not match the normal HTTP tenant resolution path. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-06-12T03:50:19Z

+                 TOPIC_MINECRAFT_PUNISHMENT_TYPES,
+                 TOPIC_MINECRAFT_STAFF_NOTIFICATIONS,
+                 TOPIC_MINECRAFT_PRESENCE,
+                 TOPIC_MINECRAFT_PUNISHMENTS,
+                 TOPIC_MINECRAFT_PLAYER_NOTIFICATIONS,


Minecraft topics over-authorized

This now lets any websocket authenticated with the tenant Minecraft API key subscribe to tenant-wide player notifications, staff 2FA pushes, punishments, migration tasks, and stat wipes. The Minecraft principal only carries the server and optional instance id, so there is no player or staff scope to filter these topics. A plugin instance that should only receive its relevant sync data can receive every pushed player notification or staff 2FA verification event for the tenant.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/main/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizer.java Line: 72-76 Comment: **Minecraft topics over-authorized** This now lets any websocket authenticated with the tenant Minecraft API key subscribe to tenant-wide player notifications, staff 2FA pushes, punishments, migration tasks, and stat wipes. The Minecraft principal only carries the server and optional instance id, so there is no player or staff scope to filter these topics. A plugin instance that should only receive its relevant sync data can receive every pushed player notification or staff 2FA verification event for the tenant. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-06-12T03:50:20Z

+    ) {
+        Server server = RequestUtil.getRequestServer(httpRequest);
+        minecraftSyncService.applyPresence(
+            server,
+            MinecraftSyncProtoMapper.toOnlinePlayers(request),
+            request.hasServerName() ? request.getServerName() : null,
+            Instant.now()
+        );


Missing roster clears presence

The new /presence endpoint treats an absent onlinePlayers field as an empty list and immediately calls applyPresence. That path marks every stale player not in the supplied set as offline, so a malformed or partial presence heartbeat with no roster succeeds while setting the server count to 0 and reconciling all players for that server as offline. This endpoint should reject requests without an explicit roster, or use a request shape that can distinguish “no roster sent” from “zero players online.”

Prompt To Fix With AI

This is a comment left during a code review. Path: src/main/java/gg/modl/backend/player/controller/MinecraftSyncV3Controller.java Line: 91-98 Comment: **Missing roster clears presence** The new `/presence` endpoint treats an absent `onlinePlayers` field as an empty list and immediately calls `applyPresence`. That path marks every stale player not in the supplied set as offline, so a malformed or partial presence heartbeat with no roster succeeds while setting the server count to 0 and reconciling all players for that server as offline. This endpoint should reject requests without an explicit roster, or use a request shape that can distinguish “no roster sent” from “zero players online.” How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-06-12T03:50:21Z

        AITicketAnalysisService.AISuggestionResult result = aiTicketAnalysisService.applyAISuggestion(server, ticketId, staffName);
-        if (!result.success()) {
-            return ResponseEntity.badRequest().body(Map.of("error", result.error()));
-        }
-        return ResponseEntity.ok(Map.of("success", true));
+        return toAISuggestionResponse(result);


Failures return success status

This endpoint now returns an AISuggestionActionResponse directly for both successful and failed applyAISuggestion results. The previous path returned 400 when result.success() was false, but now a failed punishment application returns HTTP 200 with success=false. Existing clients that rely on the non-2xx status can show the action as completed even though the service rejected it.

Suggested change

AITicketAnalysisService.AISuggestionResult result = aiTicketAnalysisService.applyAISuggestion(server, ticketId, staffName);

if (!result.success()) {

return ResponseEntity.badRequest().body(Map.of("error", result.error()));

}

return ResponseEntity.ok(Map.of("success", true));

return toAISuggestionResponse(result);

AITicketAnalysisService.AISuggestionResult result = aiTicketAnalysisService.applyAISuggestion(server, ticketId, staffName);

if (!result.success()) {

throw new ValidationException(result.error());

}

return toAISuggestionResponse(result);

Prompt To Fix With AI

This is a comment left during a code review. Path: src/main/java/gg/modl/backend/settings/controller/PanelSettingsController.java Line: 360-361 Comment: **Failures return success status** This endpoint now returns an `AISuggestionActionResponse` directly for both successful and failed `applyAISuggestion` results. The previous path returned `400` when `result.success()` was false, but now a failed punishment application returns HTTP 200 with `success=false`. Existing clients that rely on the non-2xx status can show the action as completed even though the service rejected it. ```suggestion AITicketAnalysisService.AISuggestionResult result = aiTicketAnalysisService.applyAISuggestion(server, ticketId, staffName); if (!result.success()) { throw new ValidationException(result.error()); } return toAISuggestionResponse(result); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-06-12T03:50:22Z

        AITicketAnalysisService.AISuggestionResult result = aiTicketAnalysisService.dismissAISuggestion(server, ticketId);
-        if (!result.success()) {
-            return ResponseEntity.badRequest().body(Map.of("error", result.error()));
-        }
-        return ResponseEntity.ok(Map.of("success", true));
+        return toAISuggestionResponse(result);


Dismiss failures return success status

The dismiss path has the same status-code regression as apply. When dismissAISuggestion fails, the controller still returns HTTP 200 with success=false, while the old implementation returned a bad request. A panel caller that only checks the response status can remove or mark the suggestion as dismissed even though the backend did not dismiss it.

Suggested change

AITicketAnalysisService.AISuggestionResult result = aiTicketAnalysisService.dismissAISuggestion(server, ticketId);

if (!result.success()) {

return ResponseEntity.badRequest().body(Map.of("error", result.error()));

}

return ResponseEntity.ok(Map.of("success", true));

return toAISuggestionResponse(result);

AITicketAnalysisService.AISuggestionResult result = aiTicketAnalysisService.dismissAISuggestion(server, ticketId);

if (!result.success()) {

throw new ValidationException(result.error());

}

return toAISuggestionResponse(result);

Prompt To Fix With AI

This is a comment left during a code review. Path: src/main/java/gg/modl/backend/settings/controller/PanelSettingsController.java Line: 371-372 Comment: **Dismiss failures return success status** The dismiss path has the same status-code regression as apply. When `dismissAISuggestion` fails, the controller still returns HTTP 200 with `success=false`, while the old implementation returned a bad request. A panel caller that only checks the response status can remove or mark the suggestion as dismissed even though the backend did not dismiss it. ```suggestion AITicketAnalysisService.AISuggestionResult result = aiTicketAnalysisService.dismissAISuggestion(server, ticketId); if (!result.success()) { throw new ValidationException(result.error()); } return toAISuggestionResponse(result); ``` How can I resolve this? If you propose a fix, please make it concise.

byteful and others added 19 commits April 21, 2026 15:27

warning fix

dc84a41

username fix

acfa3f5

fix player ids

c218037

fix migration script

d80cc2b

realtime services - needs review

3bb8473

fix

2b23596

replay lite stuff

33fe270

security fixes

66e3372

realtime

d8e92b9

security fixes

ba39349

Merge pull request #21 from modl-gg/security-fixes

5867bfd

security fixes

realtime

1daeadd

Merge pull request #22 from modl-gg/realtime

c1aec23

Realtime

Merge dev into replay-lite

f2ffa29

fix replay lite review issues

b7517cf

fix replay lite quota confirms

6d634c4

Merge pull request #23 from modl-gg/replay-lite

033097e

Replay lite

security patches

9540967

byteful changed the title ~~v2.3.0~~ v2.2.4 May 16, 2026

alert system #113 (#25)

9a1ede1

byteful temporarily deployed to staging May 18, 2026 07:48 — with GitHub Actions Inactive

byteful temporarily deployed to staging May 18, 2026 09:47 — with GitHub Actions Inactive

byteful force-pushed the dev branch from 22ae260 to 9a1ede1 Compare May 18, 2026 09:48

byteful temporarily deployed to staging May 18, 2026 09:50 — with GitHub Actions Inactive

fix #92 and #114 (#26)

24d7c40

byteful temporarily deployed to staging May 21, 2026 12:04 — with GitHub Actions Inactive

update gemini model

74b1fe4

byteful temporarily deployed to staging May 22, 2026 02:36 — with GitHub Actions Inactive

Merge branch 'main' into dev

6f9ea3d

byteful deployed to staging May 22, 2026 04:19 — with GitHub Actions Active

byteful marked this pull request as ready for review May 22, 2026 04:28

greptile-apps Bot reviewed May 22, 2026

View reviewed changes

link staff and invitations to roles by id

ddc5570

byteful had a problem deploying to staging June 11, 2026 19:57 — with GitHub Actions Error

byteful added 2 commits June 11, 2026 15:11

merge main into dev

c3a4113

websocket realtime + proto-json controllers

da2650b

byteful had a problem deploying to staging June 12, 2026 03:39 — with GitHub Actions Failure

greptile-apps Bot reviewed Jun 12, 2026

View reviewed changes

byteful had a problem deploying to staging June 12, 2026 05:09 — with GitHub Actions Error

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v2.2.4#24

v2.2.4#24
byteful wants to merge 26 commits into
mainfrom
dev

byteful commented May 16, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

byteful commented May 22, 2026

Uh oh!

greptile-apps Bot May 22, 2026

Uh oh!

greptile-apps Bot May 22, 2026

Uh oh!

greptile-apps Bot May 22, 2026

Uh oh!

greptile-apps Bot May 22, 2026

Uh oh!

greptile-apps Bot Jun 12, 2026

Uh oh!

greptile-apps Bot Jun 12, 2026

Uh oh!

greptile-apps Bot Jun 12, 2026

Uh oh!

greptile-apps Bot Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		Criteria.where(TicketVerificationFields.EXPIRES_AT).gte(now),
		Criteria.where("failedAttempts").lt(MAX_FAILED_ATTEMPTS)

Conversation

byteful commented May 16, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 2/5

Security Review

Important Files Changed

Uh oh!

byteful commented May 22, 2026

Uh oh!

greptile-apps Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

byteful commented May 16, 2026 •

edited by greptile-apps Bot

Loading