Migrate to the new pokebedrock.com #61

smell-of-curry · 2026-03-31T01:56:11Z

smell-of-curry commented

2026-03-31 01:56:11 +00:00

(Migrated from github.com)

Summary by CodeRabbit

Refactor
- Updated moderation integration to use the players-service infliction APIs (GET-based lookups and JSON-based mutations).
- Streamlined moderation client behavior and unified authentication/header handling.
- Improved infliction and player syncing across moderation actions (including kicks).
- Updated default moderation-related service endpoint configuration.
Bug Fixes
- Improved handling of “not found” (treated as no active inflictions) and “rate limited” responses (retryable).

## Summary by CodeRabbit * **Refactor** * Updated moderation integration to use the players-service infliction APIs (GET-based lookups and JSON-based mutations). * Streamlined moderation client behavior and unified authentication/header handling. * Improved infliction and player syncing across moderation actions (including kicks). * Updated default moderation-related service endpoint configuration. * **Bug Fixes** * Improved handling of “not found” (treated as no active inflictions) and “rate limited” responses (retryable).

coderabbitai[bot] commented

2026-03-31 01:56:23 +00:00

(Migrated from github.com)

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5180cd2a-bc74-4631-a3d9-640ab04207f4

📥 Commits

Reviewing files that changed from the base of the PR and between d562cb180d and c64b660539.

📒 Files selected for processing (4)

pokebedrock/command/kick.go
pokebedrock/config.go
pokebedrock/form/moderate.go
pokebedrock/moderation/service.go

🚧 Files skipped from review as they are similar to previous changes (4)

pokebedrock/config.go
pokebedrock/form/moderate.go
pokebedrock/command/kick.go
pokebedrock/moderation/service.go

📝 Walkthrough

Walkthrough

The pull request refactors moderation integration around the players-service API. It introduces updated infliction and player-context models, changes retrieval and mutation endpoints and signatures, updates player upserts, changes default service URLs, and adjusts kick and moderation-form call sites.

Changes

Moderation service integration

Layer / File(s)	Summary
Moderation data contracts and API client `pokebedrock/moderation/model.go`, `pokebedrock/moderation/service.go`	Infliction models now include IDs without JSON tags, `UserContext` identifies players, timestamps are converted between internal and API formats, and moderation operations use players-service REST endpoints with query parameters and centralized authentication.
Player upsert worker `pokebedrock/moderation/service.go`	Queued player details are posted to `/api/players` using the new upsert payload and authentication handling.
Infliction call sites `pokebedrock/command/kick.go`, `pokebedrock/form/moderate.go`	Kick and moderation forms use the new add and remove method signatures, passing user context, inflictions, or infliction IDs.
Default service endpoints `pokebedrock/config.go`	Default roles and moderation URLs are updated for the new service locations.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the overall migration work, which updates config and moderation service calls for the new pokebedrock.com backend.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch migrate-to-new-website

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Error: can't load config: can't unmarshal config by viper (flags, file): 1 error(s) decoding:

'output.formats' expected a map, got 'slice'
The command is terminated due to an error: can't load config: can't unmarshal config by viper (flags, file): 1 error(s) decoding:

'output.formats' expected a map, got 'slice'

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

[![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/smell-of-curry/pokebedrock-hub/pull/61?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)  No actionable comments were generated in the recent review. 🎉 <details> <summary>ℹ️ Recent review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `5180cd2a-bc74-4631-a3d9-640ab04207f4` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between d562cb180d477ecdbea2e51ee2931cbaf63e9232 and c64b660539ebd7f11934cb6b8f0f5bd4ded76e45. </details> <details> <summary>📒 Files selected for processing (4)</summary> * `pokebedrock/command/kick.go` * `pokebedrock/config.go` * `pokebedrock/form/moderate.go` * `pokebedrock/moderation/service.go` </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (4)</summary> * pokebedrock/config.go * pokebedrock/form/moderate.go * pokebedrock/command/kick.go * pokebedrock/moderation/service.go </details> </details> ---  <details> <summary>📝 Walkthrough</summary> ## Walkthrough The pull request refactors moderation integration around the players-service API. It introduces updated infliction and player-context models, changes retrieval and mutation endpoints and signatures, updates player upserts, changes default service URLs, and adjusts kick and moderation-form call sites. ### Changes **Moderation service integration** |Layer / File(s)|Summary| |---|---| |**Moderation data contracts and API client** `pokebedrock/moderation/model.go`, `pokebedrock/moderation/service.go`|Infliction models now include IDs without JSON tags, `UserContext` identifies players, timestamps are converted between internal and API formats, and moderation operations use players-service REST endpoints with query parameters and centralized authentication.| |**Player upsert worker** `pokebedrock/moderation/service.go`|Queued player details are posted to `/api/players` using the new upsert payload and authentication handling.| |**Infliction call sites** `pokebedrock/command/kick.go`, `pokebedrock/form/moderate.go`|Kick and moderation forms use the new add and remove method signatures, passing user context, inflictions, or infliction IDs.| |**Default service endpoints** `pokebedrock/config.go`|Default roles and moderation URLs are updated for the new service locations.| **Estimated code review effort:** 3 (Moderate) | ~20 minutes </details>   <details> <summary>🚥 Pre-merge checks | ✅ 5</summary> <details> <summary>✅ Passed checks (5 passed)</summary> | Check name | Status | Explanation | | :------------------------: | :------- | :----------------------------------------------------------------------------------------------------------------------------------- | | Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. | | Title check | ✅ Passed | The title matches the overall migration work, which updates config and moderation service calls for the new pokebedrock.com backend. | | Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. | | Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. | | Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. | </details> </details>   <details> <summary>✨ Finishing Touches</summary> <details> <summary>📝 Generate docstrings</summary> - [ ]  Create stacked PR - [ ]  Commit on current branch </details> <details> <summary>🧪 Generate unit tests (beta)</summary> - [ ]  Create PR with unit tests - [ ]  Commit unit tests in branch `migrate-to-new-website` </details> </details>   > [!WARNING] > There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. > > <details> > <summary>🔧 golangci-lint (2.12.2)</summary> > > Error: can't load config: can't unmarshal config by viper (flags, file): 1 error(s) decoding: > > * 'output.formats' expected a map, got 'slice' > The command is terminated due to an error: can't load config: can't unmarshal config by viper (flags, file): 1 error(s) decoding: > > * 'output.formats' expected a map, got 'slice' > > > > > </details>   --- Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=smell-of-curry/pokebedrock-hub&utm_content=61)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. <details> <summary>❤️ Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> Comment `@coderabbitai help` to get the list of available commands.

coderabbitai[bot] (Migrated from github.com) reviewed

2026-03-31 02:06:38 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pokebedrock/moderation/service.go (1)
239-253: ⚠️ Potential issue | 🟡 Minor

Fail fast on blank infliction IDs.

If inflictionID is empty, this builds DELETE /api/inflictions/ and leaves the backend to interpret it. Returning a local validation error here makes stale or malformed infliction payloads much easier to diagnose.
Suggested guard
 func (s *Service) RemoveInfliction(inflictionID string) error {
+	if inflictionID == "" {
+		return errors.New("infliction ID is required")
+	}
+
 	var lastErr error
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pokebedrock/moderation/service.go` around lines 239 - 253, The
RemoveInfliction method currently proceeds when inflictionID is empty, causing a
DELETE to /api/inflictions/ and leaving the backend to handle it; add an early
validation guard at the top of Service.RemoveInfliction that checks if
inflictionID == "" and returns a clear local error (e.g.,
fmt.Errorf("inflictionID is required") or a typed validation error) so the
request is never sent and callers get an immediate, diagnosable failure instead
of a backend  response.

🧹 Nitpick comments (1)

pokebedrock/command/kick.go (1)

49-52: Use the victim's XUID while you already have the live player object.

This request only sends Name, even though victim.XUID() is available here. Including the XUID makes the new UserContext lookup deterministic across name changes and casing differences.

Suggested tweak

 		err := moderation.GlobalService().AddInfliction(
-			moderation.UserContext{Name: victim.Name()},
+			moderation.UserContext{
+				XUID: victim.XUID(),
+				Name: victim.Name(),
+			},
 			infliction,
 		)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pokebedrock/command/kick.go` around lines 49 - 52, The UserContext passed to
moderation.GlobalService().AddInfliction only sets Name via victim.Name() which
can change; update the call to populate moderation.UserContext with the victim's
stable XUID as well (use victim.XUID()) alongside Name so lookups are
deterministic across name/case changes—i.e., modify the UserContext construction
in the AddInfliction call to include the XUID field when creating the infliction
for the victim.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pokebedrock/config.go`:
- Around line 99-100: ReadConfig() must normalize/migrate existing
Service.ModerationURL values so older installs that include "/api" don't keep
producing malformed endpoints; after loading the config (or when DefaultConfig()
is not written) check c.Service.ModerationURL (and similarly c.Service.RolesURL
if needed) for embedded "/api" paths (e.g. strings.Contains(..., "/api") or
strings.HasSuffix(..., "/api")) and strip the "/api" segment(s) so the field
holds only the base URL the moderation client expects, then persist the
corrected config back to disk (use the existing Save/Write config routine) so
upgrades are fixed in-place.

In `@pokebedrock/moderation/service.go`:
- Around line 226-228: The current debug log in the moderation write path uses
fmt.Sprintf("Successfully added infliction for %+v", userCtx) which dumps the
entire UserContext (XUID/DiscordID/IP) and creates a PII retention risk; change
the log to record a bounded identifier instead (e.g., userCtx.XUID or a
hashed/masked variant) and update the s.log.Debug call in the function handling
the HTTP response (the block that checks resp.StatusCode and returns nil) to log
only that safe identifier or its masked/hash form rather than %+v of
UserContext.
- Around line 334-338: The host extraction for the IPs field in the
apiUpsertPlayer construction incorrectly splits p.Addr().String() on ":" which
breaks IPv6 addresses; update the code that builds the IPs slice (where
apiUpsertPlayer is created and p.Addr() is used) to call
net.SplitHostPort(p.Addr().String()) and use the returned host value (or fall
back to the original Addr string if SplitHostPort returns an error) so IPv4 and
IPv6 addresses are handled correctly before assigning to the IPs field.

---

Outside diff comments:
In `@pokebedrock/moderation/service.go`:
- Around line 239-253: The RemoveInfliction method currently proceeds when
inflictionID is empty, causing a DELETE to /api/inflictions/ and leaving the
backend to handle it; add an early validation guard at the top of
Service.RemoveInfliction that checks if inflictionID == "" and returns a clear
local error (e.g., fmt.Errorf("inflictionID is required") or a typed validation
error) so the request is never sent and callers get an immediate, diagnosable
failure instead of a backend  response.

---

Nitpick comments:
In `@pokebedrock/command/kick.go`:
- Around line 49-52: The UserContext passed to
moderation.GlobalService().AddInfliction only sets Name via victim.Name() which
can change; update the call to populate moderation.UserContext with the victim's
stable XUID as well (use victim.XUID()) alongside Name so lookups are
deterministic across name/case changes—i.e., modify the UserContext construction
in the AddInfliction call to include the XUID field when creating the infliction
for the victim.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 755725b0-af0c-42ea-bbef-d40554a1f08d

📥 Commits

Reviewing files that changed from the base of the PR and between e71c63774c and d562cb180d.

📒 Files selected for processing (5)

pokebedrock/command/kick.go
pokebedrock/config.go
pokebedrock/form/moderate.go
pokebedrock/moderation/model.go
pokebedrock/moderation/service.go

**Actionable comments posted: 3** > [!CAUTION] > Some comments are outside the diff and can’t be posted inline due to platform limitations. > > > > <details> > <summary>⚠️ Outside diff range comments (1)</summary><blockquote> > > <details> > <summary>pokebedrock/moderation/service.go (1)</summary><blockquote> > > `239-253`: _⚠️ Potential issue_ | _🟡 Minor_ > > **Fail fast on blank infliction IDs.** > > If `inflictionID` is empty, this builds `DELETE /api/inflictions/` and leaves the backend to interpret it. Returning a local validation error here makes stale or malformed infliction payloads much easier to diagnose. > > <details> > <summary>Suggested guard</summary> > > ```diff > func (s *Service) RemoveInfliction(inflictionID string) error { > + if inflictionID == "" { > + return errors.New("infliction ID is required") > + } > + > var lastErr error > ``` > </details> > > <details> > <summary>🤖 Prompt for AI Agents</summary> > > ``` > Verify each finding against the current code and only fix it if needed. > > In `@pokebedrock/moderation/service.go` around lines 239 - 253, The > RemoveInfliction method currently proceeds when inflictionID is empty, causing a > DELETE to /api/inflictions/ and leaving the backend to handle it; add an early > validation guard at the top of Service.RemoveInfliction that checks if > inflictionID == "" and returns a clear local error (e.g., > fmt.Errorf("inflictionID is required") or a typed validation error) so the > request is never sent and callers get an immediate, diagnosable failure instead > of a backend response. > ``` > > </details> > > </blockquote></details> > > </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>pokebedrock/command/kick.go (1)</summary><blockquote> `49-52`: **Use the victim's XUID while you already have the live player object.** This request only sends `Name`, even though `victim.XUID()` is available here. Including the XUID makes the new `UserContext` lookup deterministic across name changes and casing differences. <details> <summary>Suggested tweak</summary> ```diff err := moderation.GlobalService().AddInfliction( - moderation.UserContext{Name: victim.Name()}, + moderation.UserContext{ + XUID: victim.XUID(), + Name: victim.Name(), + }, infliction, ) ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@pokebedrock/command/kick.go` around lines 49 - 52, The UserContext passed to moderation.GlobalService().AddInfliction only sets Name via victim.Name() which can change; update the call to populate moderation.UserContext with the victim's stable XUID as well (use victim.XUID()) alongside Name so lookups are deterministic across name/case changes—i.e., modify the UserContext construction in the AddInfliction call to include the XUID field when creating the infliction for the victim. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary> ``` Verify each finding against the current code and only fix it if needed. Inline comments: In `@pokebedrock/config.go`: - Around line 99-100: ReadConfig() must normalize/migrate existing Service.ModerationURL values so older installs that include "/api" don't keep producing malformed endpoints; after loading the config (or when DefaultConfig() is not written) check c.Service.ModerationURL (and similarly c.Service.RolesURL if needed) for embedded "/api" paths (e.g. strings.Contains(..., "/api") or strings.HasSuffix(..., "/api")) and strip the "/api" segment(s) so the field holds only the base URL the moderation client expects, then persist the corrected config back to disk (use the existing Save/Write config routine) so upgrades are fixed in-place. In `@pokebedrock/moderation/service.go`: - Around line 226-228: The current debug log in the moderation write path uses fmt.Sprintf("Successfully added infliction for %+v", userCtx) which dumps the entire UserContext (XUID/DiscordID/IP) and creates a PII retention risk; change the log to record a bounded identifier instead (e.g., userCtx.XUID or a hashed/masked variant) and update the s.log.Debug call in the function handling the HTTP response (the block that checks resp.StatusCode and returns nil) to log only that safe identifier or its masked/hash form rather than %+v of UserContext. - Around line 334-338: The host extraction for the IPs field in the apiUpsertPlayer construction incorrectly splits p.Addr().String() on ":" which breaks IPv6 addresses; update the code that builds the IPs slice (where apiUpsertPlayer is created and p.Addr() is used) to call net.SplitHostPort(p.Addr().String()) and use the returned host value (or fall back to the original Addr string if SplitHostPort returns an error) so IPv4 and IPv6 addresses are handled correctly before assigning to the IPs field. --- Outside diff comments: In `@pokebedrock/moderation/service.go`: - Around line 239-253: The RemoveInfliction method currently proceeds when inflictionID is empty, causing a DELETE to /api/inflictions/ and leaving the backend to handle it; add an early validation guard at the top of Service.RemoveInfliction that checks if inflictionID == "" and returns a clear local error (e.g., fmt.Errorf("inflictionID is required") or a typed validation error) so the request is never sent and callers get an immediate, diagnosable failure instead of a backend response. --- Nitpick comments: In `@pokebedrock/command/kick.go`: - Around line 49-52: The UserContext passed to moderation.GlobalService().AddInfliction only sets Name via victim.Name() which can change; update the call to populate moderation.UserContext with the victim's stable XUID as well (use victim.XUID()) alongside Name so lookups are deterministic across name/case changes—i.e., modify the UserContext construction in the AddInfliction call to include the XUID field when creating the infliction for the victim. ``` </details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ]  Push a commit to this branch (recommended) - [ ]  Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `755725b0-af0c-42ea-bbef-d40554a1f08d` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e71c63774cfc85a6f852ace3346a7c300d2554a4 and d562cb180d477ecdbea2e51ee2931cbaf63e9232. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `pokebedrock/command/kick.go` * `pokebedrock/config.go` * `pokebedrock/form/moderate.go` * `pokebedrock/moderation/model.go` * `pokebedrock/moderation/service.go` </details> </details>

pokebedrock/config.go Outdated

⚠️ Potential issue | 🔴 Critical

Changing the defaults is not enough for existing installs.

ReadConfig() only writes DefaultConfig() when config.toml is missing. Upgraded servers keep their old Service.ModerationURL, but the new moderation client now appends /api/... itself, so those nodes will keep calling malformed endpoints until the stored value is migrated or normalized.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pokebedrock/config.go` around lines 99 - 100, ReadConfig() must
normalize/migrate existing Service.ModerationURL values so older installs that
include "/api" don't keep producing malformed endpoints; after loading the
config (or when DefaultConfig() is not written) check c.Service.ModerationURL
(and similarly c.Service.RolesURL if needed) for embedded "/api" paths (e.g.
strings.Contains(..., "/api") or strings.HasSuffix(..., "/api")) and strip the
"/api" segment(s) so the field holds only the base URL the moderation client
expects, then persist the corrected config back to disk (use the existing
Save/Write config routine) so upgrades are fixed in-place.

_⚠️ Potential issue_ | _🔴 Critical_ **Changing the defaults is not enough for existing installs.** `ReadConfig()` only writes `DefaultConfig()` when `config.toml` is missing. Upgraded servers keep their old `Service.ModerationURL`, but the new moderation client now appends `/api/...` itself, so those nodes will keep calling malformed endpoints until the stored value is migrated or normalized. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@pokebedrock/config.go` around lines 99 - 100, ReadConfig() must normalize/migrate existing Service.ModerationURL values so older installs that include "/api" don't keep producing malformed endpoints; after loading the config (or when DefaultConfig() is not written) check c.Service.ModerationURL (and similarly c.Service.RolesURL if needed) for embedded "/api" paths (e.g. strings.Contains(..., "/api") or strings.HasSuffix(..., "/api")) and strip the "/api" segment(s) so the field holds only the base URL the moderation client expects, then persist the corrected config back to disk (use the existing Save/Write config routine) so upgrades are fixed in-place. ``` </details>

pokebedrock/moderation/service.go Outdated

					
				@ -180,0 +195,4 @@

						UserContext: userCtx,

						Infliction:  internalToAPICreate(infliction),

					}

					rawRequest, err := json.Marshal(apiReq)

⚠️ Potential issue | 🟠 Major

Avoid logging the full UserContext.

UserContext now carries XUID, DiscordID, and IP fields. %+v will dump whichever of those are present into debug logs, which creates a new PII retention path for moderation writes. Log a bounded identifier instead.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pokebedrock/moderation/service.go` around lines 226 - 228, The current debug
log in the moderation write path uses fmt.Sprintf("Successfully added infliction
for %+v", userCtx) which dumps the entire UserContext (XUID/DiscordID/IP) and
creates a PII retention risk; change the log to record a bounded identifier
instead (e.g., userCtx.XUID or a hashed/masked variant) and update the
s.log.Debug call in the function handling the HTTP response (the block that
checks resp.StatusCode and returns nil) to log only that safe identifier or its
masked/hash form rather than %+v of UserContext.

✅ Addressed in commits cdafaa6 to c64b660

_⚠️ Potential issue_ | _🟠 Major_ **Avoid logging the full `UserContext`.** `UserContext` now carries XUID, DiscordID, and IP fields. `%+v` will dump whichever of those are present into debug logs, which creates a new PII retention path for moderation writes. Log a bounded identifier instead. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@pokebedrock/moderation/service.go` around lines 226 - 228, The current debug log in the moderation write path uses fmt.Sprintf("Successfully added infliction for %+v", userCtx) which dumps the entire UserContext (XUID/DiscordID/IP) and creates a PII retention risk; change the log to record a bounded identifier instead (e.g., userCtx.XUID or a hashed/masked variant) and update the s.log.Debug call in the function handling the HTTP response (the block that checks resp.StatusCode and returns nil) to log only that safe identifier or its masked/hash form rather than %+v of UserContext. ``` </details>   ✅ Addressed in commits cdafaa6 to c64b660

pokebedrock/moderation/service.go Outdated

					
				@ -262,2 +298,4 @@

				// closeBody drains and closes an HTTP response body so the connection can be

				// reused by the keep-alive pool.

				func closeBody(resp *http.Response) {

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n 'strings\.Split\(p\.Addr\(\)\.String\(\), ":"\)\[0\]' -- pokebedrock/moderation/service.go

python - <<'PY'
samples = ["127.0.0.1:19132", "[2001:db8::1]:19132"]
for s in samples:
    print(f"{s} -> {s.split(':')[0]}")
PY

Repository: smell-of-curry/pokebedrock-hub

Length of output: 199

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check imports in the service.go file to see if net package is already imported
echo "=== Checking imports in service.go ===" 
head -50 pokebedrock/moderation/service.go | grep -E "^(import|"fmt"|"net")" || echo "Checking full imports..."
rg -n "^import" pokebedrock/moderation/service.go -A 20

# Check if there are similar problematic patterns using strings.Split on addresses elsewhere
echo -e "\n=== Checking for similar patterns ==="
rg -n 'strings\.Split.*Addr.*\[0\]' pokebedrock/

# Verify net.SplitHostPort behavior with test cases
echo -e "\n=== Testing net.SplitHostPort behavior ==="
python3 <<'PYTHON'
import re

test_cases = [
    "127.0.0.1:19132",
    "[2001:db8::1]:19132",
    "[::1]:8080",
    "localhost:5000"
]

# Simulate what net.SplitHostPort would do
# It correctly handles IPv6 by recognizing [host]:port format
for addr in test_cases:
    # net.SplitHostPort logic: if addr starts with [, find matching ], then :
    if addr.startswith('['):
        idx = addr.find(']')
        if idx != -1 and idx + 1 < len(addr) and addr[idx + 1] == ':':
            host = addr[:idx+1]  # includes brackets
            port = addr[idx+2:]
            print(f"net.SplitHostPort('{addr}') -> host: '{host}', port: '{port}'")
        else:
            print(f"net.SplitHostPort('{addr}') -> ERROR: malformed")
    else:
        parts = addr.rsplit(':', 1)
        if len(parts) == 2:
            print(f"net.SplitHostPort('{addr}') -> host: '{parts[0]}', port: '{parts[1]}'")

# Compare with the broken strings.Split approach
print("\n=== Broken strings.Split approach ===")
for addr in test_cases:
    broken = addr.split(':')[0]
    print(f"strings.Split('{addr}', ':')[0] -> '{broken}'")
PYTHON

Repository: smell-of-curry/pokebedrock-hub

Length of output: 1192

This host extraction breaks IPv6 clients.

Splitting p.Addr().String() on : truncates host:port strings like [2001:db8::1]:19132 to [2001, writing invalid IPs into the player upsert flow and breaking cross-service player matching for IPv6 users. The net package is already imported; use net.SplitHostPort() instead.

Suggested fix

-					body := apiUpsertPlayer{
-						XUID: p.XUID(),
-						Name: p.Name(),
-						IPs:  []string{strings.Split(p.Addr().String(), ":")[0]},
-					}
+					host, _, err := net.SplitHostPort(p.Addr().String())
+					if err != nil {
+						s.log.Error(fmt.Sprintf("failed to parse player address %q: %v", p.Addr().String(), err))
+						return
+					}
+
+					body := apiUpsertPlayer{
+						XUID: p.XUID(),
+						Name: p.Name(),
+						IPs:  []string{host},
+					}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pokebedrock/moderation/service.go` around lines 334 - 338, The host
extraction for the IPs field in the apiUpsertPlayer construction incorrectly
splits p.Addr().String() on ":" which breaks IPv6 addresses; update the code
that builds the IPs slice (where apiUpsertPlayer is created and p.Addr() is
used) to call net.SplitHostPort(p.Addr().String()) and use the returned host
value (or fall back to the original Addr string if SplitHostPort returns an
error) so IPv4 and IPv6 addresses are handled correctly before assigning to the
IPs field.

✅ Addressed in commits cdafaa6 to c64b660

_⚠️ Potential issue_ | _🟠 Major_ <details> <summary>🧩 Analysis chain</summary> 🏁 Script executed: ```shell #!/bin/bash set -euo pipefail rg -n 'strings\.Split\(p\.Addr\(\)\.String\(\), ":"\)\[0\]' -- pokebedrock/moderation/service.go python - <<'PY' samples = ["127.0.0.1:19132", "[2001:db8::1]:19132"] for s in samples: print(f"{s} -> {s.split(':')[0]}") PY ``` Repository: smell-of-curry/pokebedrock-hub Length of output: 199 --- 🏁 Script executed: ```shell #!/bin/bash set -euo pipefail # Check imports in the service.go file to see if net package is already imported echo "=== Checking imports in service.go ===" head -50 pokebedrock/moderation/service.go | grep -E "^(import|"fmt"|"net")" || echo "Checking full imports..." rg -n "^import" pokebedrock/moderation/service.go -A 20 # Check if there are similar problematic patterns using strings.Split on addresses elsewhere echo -e "\n=== Checking for similar patterns ===" rg -n 'strings\.Split.*Addr.*\[0\]' pokebedrock/ # Verify net.SplitHostPort behavior with test cases echo -e "\n=== Testing net.SplitHostPort behavior ===" python3 <<'PYTHON' import re test_cases = [ "127.0.0.1:19132", "[2001:db8::1]:19132", "[::1]:8080", "localhost:5000" ] # Simulate what net.SplitHostPort would do # It correctly handles IPv6 by recognizing [host]:port format for addr in test_cases: # net.SplitHostPort logic: if addr starts with [, find matching ], then : if addr.startswith('['): idx = addr.find(']') if idx != -1 and idx + 1 < len(addr) and addr[idx + 1] == ':': host = addr[:idx+1] # includes brackets port = addr[idx+2:] print(f"net.SplitHostPort('{addr}') -> host: '{host}', port: '{port}'") else: print(f"net.SplitHostPort('{addr}') -> ERROR: malformed") else: parts = addr.rsplit(':', 1) if len(parts) == 2: print(f"net.SplitHostPort('{addr}') -> host: '{parts[0]}', port: '{parts[1]}'") # Compare with the broken strings.Split approach print("\n=== Broken strings.Split approach ===") for addr in test_cases: broken = addr.split(':')[0] print(f"strings.Split('{addr}', ':')[0] -> '{broken}'") PYTHON ``` Repository: smell-of-curry/pokebedrock-hub Length of output: 1192 --- </details> **This host extraction breaks IPv6 clients.** Splitting `p.Addr().String()` on `:` truncates host:port strings like `[2001:db8::1]:19132` to `[2001`, writing invalid IPs into the player upsert flow and breaking cross-service player matching for IPv6 users. The `net` package is already imported; use `net.SplitHostPort()` instead. <details> <summary>Suggested fix</summary> ```diff - body := apiUpsertPlayer{ - XUID: p.XUID(), - Name: p.Name(), - IPs: []string{strings.Split(p.Addr().String(), ":")[0]}, - } + host, _, err := net.SplitHostPort(p.Addr().String()) + if err != nil { + s.log.Error(fmt.Sprintf("failed to parse player address %q: %v", p.Addr().String(), err)) + return + } + + body := apiUpsertPlayer{ + XUID: p.XUID(), + Name: p.Name(), + IPs: []string{host}, + } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@pokebedrock/moderation/service.go` around lines 334 - 338, The host extraction for the IPs field in the apiUpsertPlayer construction incorrectly splits p.Addr().String() on ":" which breaks IPv6 addresses; update the code that builds the IPs slice (where apiUpsertPlayer is created and p.Addr() is used) to call net.SplitHostPort(p.Addr().String()) and use the returned host value (or fall back to the original Addr string if SplitHostPort returns an error) so IPv4 and IPv6 addresses are handled correctly before assigning to the IPs field. ``` </details>   ✅ Addressed in commits cdafaa6 to c64b660