Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughモデレーター機能と AutoMod を追加。バックエンドに2つの新モデルとCRUDエンドポイントを追加し、ボットにモデレーションコマンド／メッセージ監視を実装、ダッシュボードに管理APIルートと設定UIを追加しました。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as ユーザー (Discord)
participant Bot as ボット (ModeratorCog)
participant API as Bot API Client
participant Backend as バックエンド API
participant DB as データベース
User->>Bot: メッセージ送信
activate Bot
Bot->>API: is_module_enabled(guild_id, "moderator")
activate API
API->>Backend: GET /modules/status
activate Backend
Backend->>DB: モジュール有効フラグ確認
DB-->>Backend: 有効/無効
Backend-->>API: true/false
deactivate Backend
API-->>Bot: true/false
deactivate API
alt モジュール有効
Bot->>API: get_all_automod_settings(guild_id)
activate API
API->>Backend: GET /guilds/automod/{guild_id}/all
activate Backend
Backend->>DB: EnabledAutoModeratorSetting を取得
DB-->>Backend: 設定リスト
Backend-->>API: 設定リスト
deactivate Backend
API-->>Bot: 設定リスト
deactivate API
Bot->>Bot: メッセージ内容スキャン (badword/invite/spoiler/badlink)
alt ルール違反かつ未ホワイトリスト
Bot->>Backend: POST /guilds/moderator/{guild_id} (ログ送信 / 設定参照)
activate Backend
Backend->>DB: ログ参照/保存
DB-->>Backend: OK
Backend-->>Bot: OK
deactivate Backend
Bot->>User: メッセージ削除 / 警告 / タイムアウト等
end
end
deactivate Bot
sequenceDiagram
participant Admin as 管理者
participant Dashboard as ダッシュボード UI
participant APIRoute as Next.js API Route
participant Backend as バックエンド API
participant DB as データベース
Admin->>Dashboard: AutoMod 設定編集・保存
Dashboard->>APIRoute: POST /api/guilds/{guildId}/modules/moderator
activate APIRoute
APIRoute->>APIRoute: validateAdmin()
APIRoute->>Backend: POST /guilds/automod/{guildId}/{type}
activate Backend
Backend->>DB: OnConflict Upsert EnabledAutoModeratorSetting
DB-->>Backend: OK
Backend-->>APIRoute: 設定オブジェクト
deactivate Backend
APIRoute-->>Dashboard: 成功レスポンス (data)
deactivate APIRoute
Dashboard->>Admin: 成功通知・ローカル状態更新
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (6)
src/dashboard/src/lib/api/requests.ts (2)
241-247:fetchModeratorSettingsにguildIdのバリデーションがありません。他の関数（例：
fetchGuildSettings、isModuleEnabled）ではisValidDiscordIdでバリデーションを行っていますが、この関数にはありません。一貫性のために追加することを検討してください。♻️ 提案する修正
export async function fetchModeratorSettings(guildId: string) { + if (!isValidDiscordId(guildId)) { + throw new Error("Invalid Guild ID"); + } const response = await fetch(`${RESOURCE_API_BASE_URL}/guilds/moderator/${guildId}`); if (!response.ok) { throw new Error(`Failed to fetch moderator settings: ${response.statusText}`); - }; + } return response.json(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/lib/api/requests.ts` around lines 241 - 247, fetchModeratorSettings lacks Discord ID validation; add the same check used in fetchGuildSettings/isModuleEnabled by calling isValidDiscordId(guildId) at the top of fetchModeratorSettings and throw a clear Error (or return a rejected Promise) if invalid before making the network request so invalid guild IDs are rejected consistently with other API helpers.
249-255:fetchAutomodSettingsにも同様のバリデーションを追加してください。また、
typeパラメータの検証（"all"または有効なAutoModタイプ）も考慮してください。♻️ 提案する修正
export async function fetchAutomodSettings(guildId: string, type: string) { + if (!isValidDiscordId(guildId)) { + throw new Error("Invalid Guild ID"); + } const response = await fetch(`${RESOURCE_API_BASE_URL}/guilds/automod/${guildId}/${type}`); if (!response.ok) { throw new Error(`Failed to fetch automod settings: ${response.statusText}`); - }; + } return response.json(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/lib/api/requests.ts` around lines 249 - 255, fetchAutomodSettings currently calls the API without validating the response or the incoming type parameter; add the same response.ok check as other request helpers (throwing an Error with response.statusText) and validate the type argument before calling fetch: ensure type === "all" or matches one of the allowed AutoMod types (use the same enum/list used elsewhere in your codebase), and throw a clear synchronous Error for invalid type; reference the fetchAutomodSettings function, the type and guildId parameters, and the RESOURCE_API_BASE_URL constant when making these changes.src/api/src/internal/router/moderator.go (1)
72-83:typeパラメータの検証が不足しています。
getAutoModSettingでは、typeパラメータがモデルで定義された許可値（invite、badword、badlink、spoiler）に対して検証されていません。無効なタイプでも404が返されますが、明示的な検証により、より適切なエラーメッセージを返すことができます。♻️ 提案する修正
func getAutoModSetting(c *gin.Context) { id := c.Param("id") modType := c.Param("type") + + validTypes := map[string]bool{ + model.TypeInvite: true, + model.TypeBadword: true, + model.TypeBadlink: true, + model.TypeSpoiler: true, + } + if !validTypes[modType] { + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid automod type"}) + return + } + db := c.MustGet("db").(*gorm.DB)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/moderator.go` around lines 72 - 83, The getAutoModSetting handler does not validate the "type" path parameter; update getAutoModSetting to explicitly check c.Param("type") against the allowed values defined for model.EnabledAutoModeratorSetting (invite, badword, badlink, spoiler) and return a 400 Bad Request with a clear error message when the type is invalid before querying the DB; if valid, proceed with the existing db lookup and response.src/bot/lib/api.py (2)
338-341:get_all_automod_settingsも同様に404を考慮してください。設定がない場合、空の辞書を返す方が呼び出し元で扱いやすいです。
♻️ 提案する修正
async def get_all_automod_settings(self, guild_id: str) -> Dict[str, Any]: async with self.session.get(f"{self.base_url}/guilds/automod/{guild_id}/all") as resp: - resp.raise_for_status() - return await resp.json() + if resp.status == 404: + return {} + if not resp.ok: + raise Exception(f"Failed to fetch automod settings: {resp.status}") + return await resp.json()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/lib/api.py` around lines 338 - 341, get_all_automod_settings should treat HTTP 404 as “no settings” and return an empty dict instead of raising; modify the async function (get_all_automod_settings) to check the response status or catch the client error from self.session.get / resp.raise_for_status and return {} when resp.status == 404 (or the caught ClientResponseError has status 404), otherwise re-raise or propagate the error and return await resp.json() for successful responses.
320-323:get_moderator_settingsで404のハンドリングを検討してください。
raise_for_status()は404でも例外をスローします。設定がまだ存在しない場合、デフォルト値を返す方が適切かもしれません（他のメソッド如くget_level_settingのパターン参照）。♻️ 提案する修正
async def get_moderator_settings(self, guild_id: str) -> Dict[str, Any]: async with self.session.get(f"{self.base_url}/guilds/moderator/{guild_id}") as resp: - resp.raise_for_status() - return await resp.json() + if resp.status == 404: + return {"guild_id": guild_id, "log_channel_id": None} + if not resp.ok: + raise Exception(f"Failed to fetch moderator settings: {resp.status}") + return await resp.json()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/lib/api.py` around lines 320 - 323, The get_moderator_settings method currently calls resp.raise_for_status() which will raise on 404; change it to mirror get_level_setting's pattern by detecting a 404 and returning a default settings dict (e.g., {} or the same default shape used elsewhere) instead of raising: perform the GET, check resp.status (or catch aiohttp.ClientResponseError and inspect .status) and when status == 404 return the default value, otherwise call resp.raise_for_status() / await resp.json() and return the parsed settings; update the function name get_moderator_settings to implement this behavior.src/dashboard/src/app/dashboard/[guildId]/moderator/page.tsx (1)
40-54: エラーハンドリングで空オブジェクトを使用していますが、型安全性が低い可能性があります。
settingとautomodが空オブジェクト{}で初期化され、エラー時も空オブジェクトにフォールバックしています。ModeratorClientがこれらを適切に処理できることを確認してください。オプショナルとしてnullを使用する方が型的に明確かもしれません。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/page.tsx around lines 40 - 54, The current try/catch in ModeratorContent initializes automod and setting to {} which is type-ambiguous; change their initial values to null (e.g., let automod: AutomodSettings | null = null; let setting: ModeratorSettings | null = null) and assign the fetched values on success, setting to null on error in the catch blocks; update any downstream consumer (e.g., ModeratorClient or render logic inside ModeratorContent) to handle null checks (or provide default values via explicit helpers) so types reflect absence of data instead of an empty object; ensure you update the signatures/types of fetchAutomodSettings/fetchModeratorSettings return types or the local variable types to match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/src/internal/router/moderator.go`:
- Around line 135-145: deleteAutoModSetting currently returns the raw DB error
to the client (c.JSON(..., gin.H{"error": err.Error()})) which leaks internal
details; change it to send a generic 500 response (e.g.
c.JSON(http.StatusInternalServerError, gin.H{"error":"internal server error"})
or c.Status(http.StatusInternalServerError)) and log the actual error
server-side—use the request context logger or call c.Error(err) (or the existing
logger retrieved from context) to record err from the db.Delete on
model.EnabledAutoModeratorSetting so internal details are not exposed to
clients.
- Around line 117-130: The OnConflict upsert block calling
db.Clauses(clause.OnConflict{...}).Create(&setting) omits the
allow_only_verified column from DoUpdates, so existing AutoMod records aren't
getting that field updated; add "allow_only_verified" to the
clause.AssignmentColumns list (alongside "actions", "whitelist_channel_ids",
etc.) so the field is included in the UPSERT update set.
- Around line 62-70: In deleteModeratorSetting replace the direct c.JSON with
err.Error() by calling the common DB error handler (e.g., handleDBError) so
internal DB error details aren't sent to clients; locate deleteModeratorSetting,
catch the error from db.Where(...).Delete(...).Error and call handleDBError(c,
err, "deleting moderator setting") or the existing handleDBError signature used
elsewhere to return a sanitized message and appropriate status code instead of
leaking err.Error().
In `@src/bot/cogs/moderator.py`:
- Line 80: The moderator log calls pass the target user and moderator in the
wrong order: send_moderator_log(guild, moderator, user, ...) expects the
moderator first but the commands currently call send_moderator_log(guild, user,
interaction.user, ...). Fix each call site (kick_command, unban_command,
timeout_command, remove_timeout_command and any similar calls) to swap the
second and third arguments so they call send_moderator_log(guild,
interaction.user, user, ...) and preserve the remaining parameters (action
string and reason/duration) unchanged.
- Around line 284-305: execute_automod currently only handles
delete/warn/timeout so saved "kick"/"ban" actions never run; add handlers for
"kick" and "ban" in execute_automod to call the appropriate member moderation
methods (use message.author.kick(reason=reason) for kick and
message.guild.ban(message.author, reason=reason) or
message.author.ban(reason=reason) for ban), wrap calls in try/except to avoid
crashing when permissions are missing, and ensure you still call
send_mod_log(message.guild, message.author, ...) with the full actions list and
reason/detail so kicks/bans are recorded; reference execute_automod,
message.author, message.guild, and send_mod_log when making the changes.
- Around line 73-78: Replace the bare except in the kick handling so you only
catch and log specific Discord exceptions (e.g., discord.Forbidden,
discord.HTTPException, discord.NotFound) and optionally other expected errors,
then send the appropriate user-facing message; specifically update the
try/except around user.kick(...) and the follow-up send calls (and the other
similar blocks at the listed ranges) to catch those exception types, log the
exception details (including exception instance and context such as
interaction.user and target user) before sending the failure message, and avoid
using a bare except or swallowing BaseException so debugging and permission vs
API error distinctions remain possible.
- Around line 224-232: The code currently calls
self.bot.api.is_module_enabled(guild_id) and then
self.bot.api.get_all_automod_settings(guild_id) for every message, causing two
HTTP GETs on the hot path; change this to a guild-scoped cached lookup so a
single cache-access (not two network calls) is done per message: implement an
in-memory TTL cache keyed by guild_id (or a single combined cache entry that
contains both the module enabled flag and automod settings) inside the moderator
cog (or a shared cache helper), use the cache for reads in the handler that
currently calls is_module_enabled and get_all_automod_settings, and
invalidate/update the cache when settings change (on settings update hooks or
API push events) so the code no longer issues two GET requests per message.
- Around line 266-273: The current allowed-domain check uses substring matching
(any(domain in url ...)) which allows easy bypasses; instead parse each URL (use
urllib.parse.urlparse) to get the hostname (strip any port), normalize to
lowercase, then check either hostname == domain or hostname.endswith("." +
domain) for each entry in allowed_domains (also normalize domains). Update the
loop in the moderator cog where urls is iterated (references: allowed_domains,
urls, execute_automod) to perform this hostname-based equality/subdomain-suffix
check and only call execute_automod when the hostname does not match any allowed
domain.
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts:
- Around line 62-64: Do not infer HTTP status from error.message; instead update
validateAdmin()/checkAdminPermission() usage so validateAdmin throws an Error
object that includes an explicit numeric status property (e.g., error.status =
401/403/502) and change the GET/POST/DELETE handlers in route.ts to detect and
return that status directly (e.g., use (error as any).status when present)
rather than comparing error.message; ensure all three handler catch blocks
mirror this behavior so authorization vs upstream failure are preserved.
- Around line 80-89: The code is concatenating unvalidated client input (target)
into a backend path; restrict allowed values and reject anything else before
building endpoint. In the route handler that reads request.json() and
destructures target/payload, validate target against an explicit allowlist
(e.g., allowedTargets = new Set([...]) defined near that handler), return a
400/400-like response if target is not in the set, and only then construct
endpoint using BACKEND_URL and guildId; additionally ensure you use a safe path
building approach (e.g., treat the "basic" case specially and encode path
segments) when forming endpoint; apply the same validation logic to the other
similar branch handling target (the other block referenced around the second
occurrence).
- Around line 46-61: The handler currently folds failed backend fetches into
empty objects and always returns success: true; instead, check settingRes.ok and
automodRes.ok and if either is false return an error response (e.g.,
NextResponse.json({ success: false, error: '...' }, { status: <appropriate code>
})) rather than defaulting settings/automod to {}. Update the logic around
settingRes, automodRes, settings, automod and the final NextResponse.json in
route.ts so failures from `${BACKEND_URL}/guilds/moderator/${guildId}` or
`${BACKEND_URL}/guilds/automod/${guildId}/all` propagate as errors (or include
error details) to prevent accidental overwrites.
In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/page.tsx:
- Line 2: The import list in page.tsx includes an unused symbol
fetchEmbedSettings; remove fetchEmbedSettings from the import statement that
currently imports fetchAutomodSettings, fetchEmbedSettings,
fetchModeratorSettings, isModuleEnabled so only the used symbols
(fetchAutomodSettings, fetchModeratorSettings, isModuleEnabled) remain, ensuring
no other references to fetchEmbedSettings exist in the file.
- Around line 8-13: Update the Props interface so params reflects a Promise:
change Props to use params: Promise<{ guildId: string }>, then keep the existing
await params in ModeratorPage (no other runtime changes needed); specifically
edit the interface declaration named Props and ensure the ModeratorPage
signature still accepts { params }: Props to satisfy the async-await usage.
---
Nitpick comments:
In `@src/api/src/internal/router/moderator.go`:
- Around line 72-83: The getAutoModSetting handler does not validate the "type"
path parameter; update getAutoModSetting to explicitly check c.Param("type")
against the allowed values defined for model.EnabledAutoModeratorSetting
(invite, badword, badlink, spoiler) and return a 400 Bad Request with a clear
error message when the type is invalid before querying the DB; if valid, proceed
with the existing db lookup and response.
In `@src/bot/lib/api.py`:
- Around line 338-341: get_all_automod_settings should treat HTTP 404 as “no
settings” and return an empty dict instead of raising; modify the async function
(get_all_automod_settings) to check the response status or catch the client
error from self.session.get / resp.raise_for_status and return {} when
resp.status == 404 (or the caught ClientResponseError has status 404), otherwise
re-raise or propagate the error and return await resp.json() for successful
responses.
- Around line 320-323: The get_moderator_settings method currently calls
resp.raise_for_status() which will raise on 404; change it to mirror
get_level_setting's pattern by detecting a 404 and returning a default settings
dict (e.g., {} or the same default shape used elsewhere) instead of raising:
perform the GET, check resp.status (or catch aiohttp.ClientResponseError and
inspect .status) and when status == 404 return the default value, otherwise call
resp.raise_for_status() / await resp.json() and return the parsed settings;
update the function name get_moderator_settings to implement this behavior.
In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/page.tsx:
- Around line 40-54: The current try/catch in ModeratorContent initializes
automod and setting to {} which is type-ambiguous; change their initial values
to null (e.g., let automod: AutomodSettings | null = null; let setting:
ModeratorSettings | null = null) and assign the fetched values on success,
setting to null on error in the catch blocks; update any downstream consumer
(e.g., ModeratorClient or render logic inside ModeratorContent) to handle null
checks (or provide default values via explicit helpers) so types reflect absence
of data instead of an empty object; ensure you update the signatures/types of
fetchAutomodSettings/fetchModeratorSettings return types or the local variable
types to match.
In `@src/dashboard/src/lib/api/requests.ts`:
- Around line 241-247: fetchModeratorSettings lacks Discord ID validation; add
the same check used in fetchGuildSettings/isModuleEnabled by calling
isValidDiscordId(guildId) at the top of fetchModeratorSettings and throw a clear
Error (or return a rejected Promise) if invalid before making the network
request so invalid guild IDs are rejected consistently with other API helpers.
- Around line 249-255: fetchAutomodSettings currently calls the API without
validating the response or the incoming type parameter; add the same response.ok
check as other request helpers (throwing an Error with response.statusText) and
validate the type argument before calling fetch: ensure type === "all" or
matches one of the allowed AutoMod types (use the same enum/list used elsewhere
in your codebase), and throw a clear synchronous Error for invalid type;
reference the fetchAutomodSettings function, the type and guildId parameters,
and the RESOURCE_API_BASE_URL constant when making these changes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5d828d78-416e-4618-9468-6e21f17e655c
📒 Files selected for processing (11)
src/api/src/cmd/main.gosrc/api/src/internal/model/ModeratorSetting.gosrc/api/src/internal/router/moderator.gosrc/bot/cogs/moderator.pysrc/bot/lib/api.pysrc/dashboard/src/app/api/guilds/[guildId]/modules/moderator/route.tssrc/dashboard/src/app/dashboard/[guildId]/moderator/ModeratorClient.tsxsrc/dashboard/src/app/dashboard/[guildId]/moderator/page.tsxsrc/dashboard/src/lib/api/requests.tssrc/dashboard/src/lib/commands.tssrc/dashboard/src/lib/modules.ts
Comment on lines
+62
to
+70
| func deleteModeratorSetting(c *gin.Context) { | ||
| id := c.Param("id") | ||
| db := c.MustGet("db").(*gorm.DB) | ||
| if err := db.Where("guild_id = ?", id).Delete(&model.ModeratorSetting{}).Error; err != nil { | ||
| c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||
| return | ||
| } | ||
| c.Status(http.StatusNoContent) | ||
| } |
There was a problem hiding this comment.
削除エラー時に内部エラー詳細が漏洩する可能性があります。
err.Error()をクライアントに直接返すと、データベース構造や内部実装の詳細が漏洩する可能性があります。handleDBErrorと同様に汎用的なエラーメッセージを使用することを検討してください。
🛡️ 提案する修正
func deleteModeratorSetting(c *gin.Context) {
id := c.Param("id")
db := c.MustGet("db").(*gorm.DB)
if err := db.Where("guild_id = ?", id).Delete(&model.ModeratorSetting{}).Error; err != nil {
- c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to delete moderator setting"})
return
}
c.Status(http.StatusNoContent)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func deleteModeratorSetting(c *gin.Context) { | |
| id := c.Param("id") | |
| db := c.MustGet("db").(*gorm.DB) | |
| if err := db.Where("guild_id = ?", id).Delete(&model.ModeratorSetting{}).Error; err != nil { | |
| c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | |
| return | |
| } | |
| c.Status(http.StatusNoContent) | |
| } | |
| func deleteModeratorSetting(c *gin.Context) { | |
| id := c.Param("id") | |
| db := c.MustGet("db").(*gorm.DB) | |
| if err := db.Where("guild_id = ?", id).Delete(&model.ModeratorSetting{}).Error; err != nil { | |
| c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to delete moderator setting"}) | |
| return | |
| } | |
| c.Status(http.StatusNoContent) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/src/internal/router/moderator.go` around lines 62 - 70, In
deleteModeratorSetting replace the direct c.JSON with err.Error() by calling the
common DB error handler (e.g., handleDBError) so internal DB error details
aren't sent to clients; locate deleteModeratorSetting, catch the error from
db.Where(...).Delete(...).Error and call handleDBError(c, err, "deleting
moderator setting") or the existing handleDBError signature used elsewhere to
return a sanitized message and appropriate status code instead of leaking
err.Error().
Comment on lines
+135
to
+145
| func deleteAutoModSetting(c *gin.Context) { | ||
| id := c.Param("id") | ||
| modType := c.Param("type") | ||
| db := c.MustGet("db").(*gorm.DB) | ||
|
|
||
| if err := db.Where("guild_id = ? AND type = ?", id, modType).Delete(&model.EnabledAutoModeratorSetting{}).Error; err != nil { | ||
| c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||
| return | ||
| } | ||
| c.Status(http.StatusNoContent) | ||
| } |
There was a problem hiding this comment.
deleteAutoModSettingも同様に内部エラーを漏洩しています。
🛡️ 提案する修正
func deleteAutoModSetting(c *gin.Context) {
id := c.Param("id")
modType := c.Param("type")
db := c.MustGet("db").(*gorm.DB)
if err := db.Where("guild_id = ? AND type = ?", id, modType).Delete(&model.EnabledAutoModeratorSetting{}).Error; err != nil {
- c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to delete automod setting"})
return
}
c.Status(http.StatusNoContent)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func deleteAutoModSetting(c *gin.Context) { | |
| id := c.Param("id") | |
| modType := c.Param("type") | |
| db := c.MustGet("db").(*gorm.DB) | |
| if err := db.Where("guild_id = ? AND type = ?", id, modType).Delete(&model.EnabledAutoModeratorSetting{}).Error; err != nil { | |
| c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | |
| return | |
| } | |
| c.Status(http.StatusNoContent) | |
| } | |
| func deleteAutoModSetting(c *gin.Context) { | |
| id := c.Param("id") | |
| modType := c.Param("type") | |
| db := c.MustGet("db").(*gorm.DB) | |
| if err := db.Where("guild_id = ? AND type = ?", id, modType).Delete(&model.EnabledAutoModeratorSetting{}).Error; err != nil { | |
| c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to delete automod setting"}) | |
| return | |
| } | |
| c.Status(http.StatusNoContent) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/src/internal/router/moderator.go` around lines 135 - 145,
deleteAutoModSetting currently returns the raw DB error to the client
(c.JSON(..., gin.H{"error": err.Error()})) which leaks internal details; change
it to send a generic 500 response (e.g. c.JSON(http.StatusInternalServerError,
gin.H{"error":"internal server error"}) or
c.Status(http.StatusInternalServerError)) and log the actual error
server-side—use the request context logger or call c.Error(err) (or the existing
logger retrieved from context) to record err from the db.Delete on
model.EnabledAutoModeratorSetting so internal details are not exposed to
clients.
Comment on lines
+73
to
+78
| try: | ||
| await user.kick(reason=self.reason_parse(reason, interaction.user, "キック")) | ||
| await interaction.followup.send(content=f"🦶 {user.mention} をサーバーからキックしました。", allowed_mentions=discord.AllowedMentions.none()) | ||
| except: | ||
| await interaction.followup.send(content="キックに失敗しました。", allowed_mentions=discord.AllowedMentions.none()) | ||
| return |
There was a problem hiding this comment.
except: で握りつぶさず、例外を限定してログしてください。
ここは BaseException まで捕まえており、いくつかは pass で完全に消えます。権限不足・HTTP 失敗・API 障害の切り分けができなくなるので、discord.Forbidden / discord.HTTPException などを明示してログを残すべきです。
Also applies to: 96-101, 119-124, 147-152, 170-175, 231-234, 291-303
🧰 Tools
🪛 Ruff (0.15.7)
[error] 76-76: Do not use bare except
(E722)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/cogs/moderator.py` around lines 73 - 78, Replace the bare except in
the kick handling so you only catch and log specific Discord exceptions (e.g.,
discord.Forbidden, discord.HTTPException, discord.NotFound) and optionally other
expected errors, then send the appropriate user-facing message; specifically
update the try/except around user.kick(...) and the follow-up send calls (and
the other similar blocks at the listed ranges) to catch those exception types,
log the exception details (including exception instance and context such as
interaction.user and target user) before sending the failure message, and avoid
using a bare except or swallowing BaseException so debugging and permission vs
API error distinctions remain possible.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
src/dashboard/src/app/api/guilds/[guildId]/modules/moderator/route.ts (2)
132-139:⚠️ Potential issue | 🔴 CriticalDELETE 側は
typeをまだ allowlist していません。POST では
isAutomodType()を使っていますが、DELETE は query string のtypeをそのまま${BACKEND_URL}/guilds/automod/${guildId}/${type}に連結しています。../../moderator/otherGuildのような値で想定外の内部パスへ逸脱できるので、ここもisAutomodType()で弾いてください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts around lines 132 - 139, The DELETE handler currently uses the raw query param when calling fetch to `${BACKEND_URL}/guilds/automod/${guildId}/${type}`; validate and allowlist the incoming type using the existing isAutomodType() helper (same used by POST) before building the upstream URL, and if it fails return a 400 JSON error (e.g., "Type is required or invalid"); only use the validated/allowed type value when constructing the BACKEND_URL request to prevent path traversal or unexpected internal paths.
12-39:⚠️ Potential issue | 🟠 Major認可エラーが全部 500 に潰れています。
validateAdmin()とcheckAdminPermission()が plainErrorを投げているので、Line 67 はerror.status || 500、Line 120/149 は常に 500 になります。401/403/上流障害の区別が消えるので、statusを持つエラーを投げて 3 handler でその値をそのまま返してください。Also applies to: 66-71, 119-120, 148-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts around lines 12 - 39, validateAdmin (and similarly checkAdminPermission) currently throws plain Errors which lose HTTP status info; change these to throw Error objects that include a numeric status property (e.g., throw Object.assign(new Error("Unauthorized"), { status: 401 }) or use a small helper like createHttpError(message, status)) so downstream handlers can read error.status instead of defaulting to 500; update all places in validateAdmin and checkAdminPermission that use throw new Error(...) to include the appropriate status codes (401 for Unauthorized, 403 for Forbidden, and propagate upstream error statuses when applicable).src/bot/cogs/moderator.py (4)
299-320:⚠️ Potential issue | 🟠 Major保存できる
kick/banアクションが実行されません。バックエンドの
EnabledAutoModeratorSetting.Actionsはkick/banを受け付けますが、ここで処理しているのはdelete/warn/timeoutだけです。今のままだと設定保存は成功しても制裁が走りません。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 299 - 320, The execute_automod function currently only handles "delete"/"warn"/"timeout" but ignores saved "kick" and "ban" actions; update execute_automod to detect "kick" and "ban" in actions and perform the appropriate moderation calls (use message.author.kick(reason=...) and message.guild.ban(message.author, reason=...) or equivalent), wrapping each call in try/except to avoid crashing if permissions are missing, and include the same reason/detail passed into send_mod_log so the logged entry reflects the kick/ban; ensure you reference message.author, message.guild, actions, and send_mod_log when adding these branches.
262-267:⚠️ Potential issue | 🟠 Major
allow_only_verifiedの設定が実行時に無視されています。招待リンク判定は現在
discord.gg/discord.com/inviteを見ているだけで、このフラグを一度も参照していません。ダッシュボードで保存しても挙動が変わらないので、ここで検証済みサーバー許可の分岐を入れる必要があります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 262 - 267, The invite-link automod branch ignores the allow_only_verified flag: when handling automod_map["invite"] you must check the config["allow_only_verified"] (or equivalent field) before treating the message as a violation; update the block that inspects automod_map and message.content to first read the invite config, and if allow_only_verified is true skip enforcement for messages from members whose guild membership is verified (use the member verification check you already have or add a check in _is_whitelisted or a new helper), otherwise proceed to call _is_whitelisted and then execute_automod("招待リンク検知", "許可されていない招待リンク"); ensure the code references automod_map["invite"], config["allow_only_verified"], _is_whitelisted, and execute_automod so the behavior respects the dashboard setting.
281-287:⚠️ Potential issue | 🟠 Major許可ドメイン判定が部分一致のままです。
any(domain in url ...)だとgoogle.com.evil.tldや query string にgoogle.comを含むだけの URL まで許可されます。hostnameを取り出して完全一致またはサブドメイン境界付き suffix で比較しないと簡単に迂回されます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 281 - 287, The current allowed domain check uses substring matching (in the urls loop where allowed_domains, urls, and is_allowed are used), which permits bypasses like google.com.evil.tld; instead parse each URL to extract the hostname (e.g., via urllib.parse.urlparse on each url), normalize it (lowercase, strip trailing dots and optional port), and then set is_allowed only if hostname == domain or hostname.endswith("." + domain) for each domain in allowed_domains; update the validation in the for url in urls loop to use this hostname-based equality/subdomain-suffix check rather than any(domain in url).
239-248:⚠️ Potential issue | 🟠 Majorメッセージごとに 2 回の API 呼び出しはホットパスに重すぎます。
on_messageのたびにis_module_enabled()とget_all_automod_settings()を直列で叩いているので、バックエンド遅延がそのままメッセージ処理遅延になります。ギルド単位の TTL キャッシュか、必要情報をまとめて返す API にしないと AutoMod 自体が負荷源になります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 239 - 248, on_message currently makes two serial API calls (is_module_enabled and get_all_automod_settings) per message which is too heavy; change the logic to either (A) fetch both pieces via a single API call (add or use an endpoint that returns module enabled + automod settings together, e.g., replace separate calls with get_moderator_state/guild_automod_payload) or (B) add a guild-scoped TTL cache in the moderator cog or bot.api keyed by guild_id (store {enabled, automod_map} with a short TTL and invalidate on settings update events) and use the cached value in on_message instead of calling is_module_enabled/get_all_automod_settings on every message; reference is_module_enabled, get_all_automod_settings, and on_message when updating code.src/dashboard/src/app/dashboard/[guildId]/moderator/page.tsx (1)
44-53:⚠️ Potential issue | 🟠 Major取得失敗を空設定に畳まないでください。
fetchAutomodSettings()/fetchModeratorSettings()の 500・接続失敗まで{}に落とすと、画面上は「未設定」に見えたまま再保存できてしまいます。404 だけ空扱いにして、それ以外はエラー表示か保存不可にしないと既存設定を消せます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/page.tsx around lines 44 - 53, The current try/catch around fetchAutomodSettings and fetchModeratorSettings swallows all errors and maps them to empty objects (automod/setting), which hides 500/connection failures and can cause existing settings to be unintentionally overwritten; change the error handling so that only a 404 response is mapped to an empty object and any other errors are rethrown or propagated to the caller/UI (e.g., throw the caught error or set an error state) so the page can show an error and prevent saving; update the try/catch for both fetchAutomodSettings(guildId, "all") -> automod and fetchModeratorSettings(guildId) -> setting to inspect the error (status/code) and treat non-404s as fatal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/src/internal/router/moderator.go`:
- Around line 12-27: The moderator and automod routes in
RegisterModeratorSetting are currently mounted without auth/authorization;
update the route group creation so both basic (the group created as basic) and
automod (the group created as automod) require the authentication and
guild-admin authorization middleware before registering handlers like
getModeratorSetting, saveModeratorSetting, deleteModeratorSetting,
getAllAutoModSettings, getAutoModSetting, saveAutoModSetting and
deleteAutoModSetting—i.e., change the router.Group(...) calls inside
RegisterModeratorSetting to include the shared middleware (authentication +
guild-admin check) so only authenticated guild administrators can access these
endpoints.
In `@src/bot/cogs/moderator.py`:
- Around line 37-54: parse_duration currently accepts unsupported units by
partial matching (e.g., "10d2m5s" becomes "2m5s"); fix parse_duration to either
validate the entire string or add day support: change logic in parse_duration to
first verify a full-string match (e.g., use re.fullmatch with a pattern like
r'^(\d+\s*[dhms])+$') and return None if it fails, and add 'd': 86400 to
unit_seconds so "d" is supported; alternatively, implement the full-match check
and keep only h/m/s if you prefer rejecting unknown units—ensure you update the
regex and unit_seconds in parse_duration and return None for invalid inputs.
- Around line 15-33: The slash commands (kick, ban, unban, timeout,
remove_timeout) never register their options because Command.parameters is left
empty and add_slashcommand is not sending a parameter schema to Discord; update
each Command instance (kick, ban, unban, timeout, remove_timeout) to populate
Command.parameters with the required option definitions using discord.py's
ApplicationCommandOptionType (e.g., member/user as USER, reason as STRING,
duration as INTEGER or STRING) marking required ones accordingly, and modify
add_slashcommand (or the NewSharkBot registration flow) so it includes
Command.parameters when creating/updating the ApplicationCommand payload to the
Discord API so interaction.data.options is populated and
kwargs.get("member"/"user") is non-null.
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts:
- Around line 92-104: The code currently validates payload.type unconditionally
causing requests to save "basic" settings (target === "basic") to return 400;
change the logic so that payload.type is only required/validated when handling
AutoMod targets (i.e., inside the branch where typeof target === "string" &&
isAutomodType(target") or before calling isAutomodType for automod endpoints).
Specifically, keep the existing endpoint assignment logic for target and the
basic branch, but move or gate the type check (the const type = payload.type;
and the subsequent isAutomodType validation) so it only runs when processing
automod targets (use isAutomodType to decide), and return the 400 only for
invalid automod types while allowing basic saves with no type field to proceed.
---
Duplicate comments:
In `@src/bot/cogs/moderator.py`:
- Around line 299-320: The execute_automod function currently only handles
"delete"/"warn"/"timeout" but ignores saved "kick" and "ban" actions; update
execute_automod to detect "kick" and "ban" in actions and perform the
appropriate moderation calls (use message.author.kick(reason=...) and
message.guild.ban(message.author, reason=...) or equivalent), wrapping each call
in try/except to avoid crashing if permissions are missing, and include the same
reason/detail passed into send_mod_log so the logged entry reflects the
kick/ban; ensure you reference message.author, message.guild, actions, and
send_mod_log when adding these branches.
- Around line 262-267: The invite-link automod branch ignores the
allow_only_verified flag: when handling automod_map["invite"] you must check the
config["allow_only_verified"] (or equivalent field) before treating the message
as a violation; update the block that inspects automod_map and message.content
to first read the invite config, and if allow_only_verified is true skip
enforcement for messages from members whose guild membership is verified (use
the member verification check you already have or add a check in _is_whitelisted
or a new helper), otherwise proceed to call _is_whitelisted and then
execute_automod("招待リンク検知", "許可されていない招待リンク"); ensure the code references
automod_map["invite"], config["allow_only_verified"], _is_whitelisted, and
execute_automod so the behavior respects the dashboard setting.
- Around line 281-287: The current allowed domain check uses substring matching
(in the urls loop where allowed_domains, urls, and is_allowed are used), which
permits bypasses like google.com.evil.tld; instead parse each URL to extract the
hostname (e.g., via urllib.parse.urlparse on each url), normalize it (lowercase,
strip trailing dots and optional port), and then set is_allowed only if hostname
== domain or hostname.endswith("." + domain) for each domain in allowed_domains;
update the validation in the for url in urls loop to use this hostname-based
equality/subdomain-suffix check rather than any(domain in url).
- Around line 239-248: on_message currently makes two serial API calls
(is_module_enabled and get_all_automod_settings) per message which is too heavy;
change the logic to either (A) fetch both pieces via a single API call (add or
use an endpoint that returns module enabled + automod settings together, e.g.,
replace separate calls with get_moderator_state/guild_automod_payload) or (B)
add a guild-scoped TTL cache in the moderator cog or bot.api keyed by guild_id
(store {enabled, automod_map} with a short TTL and invalidate on settings update
events) and use the cached value in on_message instead of calling
is_module_enabled/get_all_automod_settings on every message; reference
is_module_enabled, get_all_automod_settings, and on_message when updating code.
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts:
- Around line 132-139: The DELETE handler currently uses the raw query param
when calling fetch to `${BACKEND_URL}/guilds/automod/${guildId}/${type}`;
validate and allowlist the incoming type using the existing isAutomodType()
helper (same used by POST) before building the upstream URL, and if it fails
return a 400 JSON error (e.g., "Type is required or invalid"); only use the
validated/allowed type value when constructing the BACKEND_URL request to
prevent path traversal or unexpected internal paths.
- Around line 12-39: validateAdmin (and similarly checkAdminPermission)
currently throws plain Errors which lose HTTP status info; change these to throw
Error objects that include a numeric status property (e.g., throw
Object.assign(new Error("Unauthorized"), { status: 401 }) or use a small helper
like createHttpError(message, status)) so downstream handlers can read
error.status instead of defaulting to 500; update all places in validateAdmin
and checkAdminPermission that use throw new Error(...) to include the
appropriate status codes (401 for Unauthorized, 403 for Forbidden, and propagate
upstream error statuses when applicable).
In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/page.tsx:
- Around line 44-53: The current try/catch around fetchAutomodSettings and
fetchModeratorSettings swallows all errors and maps them to empty objects
(automod/setting), which hides 500/connection failures and can cause existing
settings to be unintentionally overwritten; change the error handling so that
only a 404 response is mapped to an empty object and any other errors are
rethrown or propagated to the caller/UI (e.g., throw the caught error or set an
error state) so the page can show an error and prevent saving; update the
try/catch for both fetchAutomodSettings(guildId, "all") -> automod and
fetchModeratorSettings(guildId) -> setting to inspect the error (status/code)
and treat non-404s as fatal.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 68b91f3e-c397-4419-9293-051b9032a911
📒 Files selected for processing (4)
src/api/src/internal/router/moderator.gosrc/bot/cogs/moderator.pysrc/dashboard/src/app/api/guilds/[guildId]/modules/moderator/route.tssrc/dashboard/src/app/dashboard/[guildId]/moderator/page.tsx
Comment on lines
+12
to
+27
| func RegisterModeratorSetting(router *gin.RouterGroup) { | ||
| basic := router.Group("/guilds/moderator") | ||
| { | ||
| basic.GET("/:id", getModeratorSetting) | ||
| basic.POST("/:id", saveModeratorSetting) | ||
| basic.DELETE("/:id", deleteModeratorSetting) | ||
| } | ||
|
|
||
| automod := router.Group("/guilds/automod") | ||
| { | ||
| automod.GET("/:id/all", getAllAutoModSettings) | ||
| automod.GET("/:id/:type", getAutoModSetting) | ||
| automod.POST("/:id/:type", saveAutoModSetting) | ||
| automod.DELETE("/:id/:type", deleteAutoModSetting) | ||
| } | ||
| } |
There was a problem hiding this comment.
モデレーター設定 API が未認証で公開されています。
src/api/src/cmd/main.go:71-93 ではこのルーターが認証・認可ミドルウェアなしの r.Group("/") に直接登録されています。今の GET/POST/DELETE /guilds/moderator/* と /guilds/automod/* は、API に到達できるクライアントなら誰でも読み書きできる状態なので、少なくともギルド管理者確認を共通ミドルウェアで必須にしてください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/src/internal/router/moderator.go` around lines 12 - 27, The moderator
and automod routes in RegisterModeratorSetting are currently mounted without
auth/authorization; update the route group creation so both basic (the group
created as basic) and automod (the group created as automod) require the
authentication and guild-admin authorization middleware before registering
handlers like getModeratorSetting, saveModeratorSetting, deleteModeratorSetting,
getAllAutoModSettings, getAutoModSetting, saveAutoModSetting and
deleteAutoModSetting—i.e., change the router.Group(...) calls inside
RegisterModeratorSetting to include the shared middleware (authentication +
guild-admin check) so only authenticated guild administrators can access these
endpoints.
Comment on lines
+37
to
+54
| def parse_duration(self, duration_str: str): | ||
| regex = r"(\d+)\s*([hms])" | ||
| matches = re.findall(regex, duration_str.lower()) | ||
|
|
||
| if not matches: | ||
| return None | ||
|
|
||
| total_seconds = 0 | ||
| unit_seconds = { | ||
| 'h': 3600, | ||
| 'm': 60, | ||
| 's': 1 | ||
| } | ||
|
|
||
| for value, unit in matches: | ||
| total_seconds += int(value) * unit_seconds[unit] | ||
|
|
||
| return total_seconds |
There was a problem hiding this comment.
parse_duration() が不正な単位を部分一致で受け入れています。
Line 38 は h/m/s しか認識しないのに、Line 154 の例は 10d2m5s です。しかも今の実装は unsupported な 10d を無視して 125 秒として通すので、想定より極端に短い timeout を掛けます。全体一致で検証するか、d を正式サポートしてください。
Also applies to: 152-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bot/cogs/moderator.py` around lines 37 - 54, parse_duration currently
accepts unsupported units by partial matching (e.g., "10d2m5s" becomes "2m5s");
fix parse_duration to either validate the entire string or add day support:
change logic in parse_duration to first verify a full-string match (e.g., use
re.fullmatch with a pattern like r'^(\d+\s*[dhms])+$') and return None if it
fails, and add 'd': 86400 to unit_seconds so "d" is supported; alternatively,
implement the full-match check and keep only h/m/s if you prefer rejecting
unknown units—ensure you update the regex and unit_seconds in parse_duration and
return None for invalid inputs.
feat: /user-infoを追加
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
src/bot/cogs/moderator.py (6)
90-92:⚠️ Potential issue | 🟠 Majorベア
exceptを使用せず、具体的な例外をキャッチしてログを残してください。複数箇所でベア
exceptが使用されており、一部はpassで握りつぶしています。権限不足（discord.Forbidden）、HTTP エラー（discord.HTTPException）、その他の例外の切り分けができず、デバッグが困難になります。🛠️ 修正例（kick_command の場合）
- except: - await interaction.followup.send(content="キックに失敗しました。", allowed_mentions=discord.AllowedMentions.none()) - return + except discord.HTTPException as e: + print(f"Kick failed for {user.id}: {e}") + await interaction.followup.send(content="キックに失敗しました。", allowed_mentions=discord.AllowedMentions.none()) + returnAlso applies to: 116-118, 142-144, 173-175, 199-201, 215-216, 303-304, 363-364, 372-373
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 90 - 92, Multiple bare except blocks in src/bot/cogs/moderator.py (e.g., inside the kick command and other handlers) swallow errors and obscure causes; replace each bare "except:" with specific exception handlers: catch discord.Forbidden to send a clear permission error, catch discord.HTTPException to send an actionable failure message, and finally catch Exception as e to log the unexpected error via the module logger (or self.bot.logger) and send a generic failure message; update the handlers referenced (kick_command and the other try/except blocks listed in the review) to log exception details (exception string/traceback) instead of silencing them.
21-43:⚠️ Potential issue | 🔴 Criticalスラッシュコマンドのパラメータが Discord に登録されていません。
Command.parametersが空のままのため、Discord はコマンドオプションを認識せず、kwargs.get("member")等は常にNoneになります。各コマンドにparametersを定義し、Discord API にオプションスキーマを送信する必要があります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 21 - 43, The Command instances (kick, ban, unban, timeout, remove_timeout, user_info) are created without setting their parameters, so Discord never receives option schemas and kwargs.get("member")/other options are always None; update each Command object's parameters attribute to include the expected option definitions (e.g., member, reason, duration) matching the bot's handler expectations and the Discord option types, then re-register with self.bot.add_slashcommand so the Discord API receives the options and the corresponding handler methods (kick_command, ban_command, unban_command, timeout_command, remove_timeout_command, user_info_command) receive populated kwargs.
317-322:⚠️ Potential issue | 🟠 Major
allow_only_verifiedフラグが実装されていません。ダッシュボードで「Discord公式サーバーのみ許可」を設定できますが、招待リンク検知のロジックではこのフラグを一切参照していません。設定しても挙動が変わりません。
🛠️ 修正案
if "invite" in automod_map: config = automod_map["invite"] if "discord.gg/" in message.content or "discord.com/invite/" in message.content: if not self._is_whitelisted(message, config): + # allow_only_verified が有効な場合、公式サーバーの招待リンクを許可 + if config.get("allow_only_verified"): + # TODO: Discord API で招待先サーバーが verified かチェック + pass await self.execute_automod(message, config, "招待リンク検知", "許可されていない招待リンク") return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 317 - 322, The invite-detection branch ignores the allow_only_verified flag; update the block that handles automod_map["invite"] to respect config.get("allow_only_verified"). If allow_only_verified is true, extract the invite code/URL from message.content, resolve the invite via the bot (e.g., using the bot/client fetch_invite method) and inspect the returned invite.guild to confirm it is a verified/official server (check invite.guild.verified or invite.guild.features for a VERIFIED/COMMUNITY-like flag); only skip _is_whitelisted/execute_automod if the guild is verified — otherwise treat it as a disallowed invite. Add safe error handling around fetching (network/permissions) so failures fall back to treating the invite as disallowed and still call execute_automod(message, config, "招待リンク検知", "許可されていない招待リンク") when appropriate.
340-341:⚠️ Potential issue | 🟠 Major許可ドメイン判定が部分一致のため、簡単に迂回されます。
any(domain in url for domain in allowed_domains)ではhttps://google.com.evil.tldやhttps://evil.tld/?next=google.comも許可されてしまいます。URL をパースして hostname で正確に比較してください。🛡️ 修正案
+from urllib.parse import urlparse for url in urls: - is_allowed = any(domain in url for domain in allowed_domains) + hostname = (urlparse(url).hostname or "").lower() + is_allowed = any( + hostname == domain.lower() + or hostname.endswith(f".{domain.lower()}") + for domain in allowed_domains + ) if not is_allowed:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 340 - 341, The current check using any(domain in url for domain in allowed_domains) should be replaced with a hostname-based comparison: for each url in urls, parse the URL (using urllib.parse.urlparse) to get parsed.hostname (adding a default scheme like "http://" if missing), then set is_allowed = any(parsed_hostname == domain for domain in allowed_domains) while guarding against parsed.hostname being None; update the loop in moderator.py where urls, allowed_domains and is_allowed are used to perform this exact hostname equality check.
294-304:⚠️ Potential issue | 🟠 Majorメッセージごとに 2 回 API コールするのはホットパスに重すぎます。
is_module_enabled()とget_all_automod_settings()で毎メッセージ 2 リクエスト発生します。ギルド単位の TTL キャッシュを導入することを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 294 - 304, The code is making two API calls per message via is_module_enabled(guild_id) and get_all_automod_settings(guild_id); add a guild-scoped TTL cache to avoid calling these on every message by caching the results keyed by guild_id with an expiry (e.g., in-memory dict or existing cache helper). Modify the lookup in moderator.py so the call site for is_module_enabled and get_all_automod_settings first checks the cache (and refreshes on expiry), and use the cached value for the subsequent enabled check (is_enabled.get('enabled')) and for automod_map retrieval; ensure cache keys include the module name for module flags and guild_id for automod settings and that cache refresh on miss/exception falls back to performing the API call.
354-375:⚠️ Potential issue | 🟠 MajorAutoMod で選択可能な
kick/banアクションが実行されません。ダッシュボードでは
kickとbanを保存できますが、execute_automod()はdelete/warn/timeoutのみ処理しています。設定を保存しても制裁が実行されません。🛠️ 修正案
if "timeout" in actions: try: await message.author.timeout(discord.utils.utcnow() + datetime.timedelta(minutes=10), reason=reason) except: pass + if "kick" in actions: + try: + await message.author.kick(reason=reason) + except discord.Forbidden: + pass + + if "ban" in actions: + try: + await message.guild.ban(message.author, reason=reason) + except discord.Forbidden: + pass + await self.send_mod_log(message.guild, message.author, ", ".join(actions), f"{reason} ({detail})", message.content)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 354 - 375, The execute_automod function only handles "delete", "warn", and "timeout" but ignores "kick" and "ban" saved from the dashboard; add handling for these actions in execute_automod by checking for "kick" and "ban" in the actions list and calling the Member methods to perform them (use message.author.kick(reason=reason) for kick and message.author.ban(reason=reason) for ban), wrapping each in try/except to avoid crashing on missing permissions or errors, and include them in the actions string passed to send_mod_log so the log reflects the executed sanctions.
🧹 Nitpick comments (2)
src/dashboard/src/app/dashboard/[guildId]/moderator/ModeratorClient.tsx (2)
166-171: 基本設定更新後のローカルステート反映が実装されていません。コメントで「基本設定更新」と記載されていますが、実際の処理がありません。
logChannelIdは既にuseStateで管理されているため問題は少ないですが、コメントを削除するか実装を追加することを検討してください。♻️ 修正案
if (target === "basic") { - // 基本設定更新 + // logChannelId は既にローカルステートで管理されているため追加処理不要 } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/ModeratorClient.tsx around lines 166 - 171, The code path in ModeratorClient.tsx that handles a successful update when target === "basic" leaves only a comment and doesn't update local state; either remove the placeholder comment or update the appropriate state values (e.g., call setAutomodSettings to merge result.data into the existing automod settings and/or update the logChannelId state from the returned payload) so the UI reflects the saved basic settings; locate the conditional around setAutomodSettings and implement the state merge for the "basic" case (or delete the comment if no local change is needed).
257-260: インポート文はファイルの先頭に配置してください。
X,Plus,RoleSelector,CommandsControlのインポートがコンポーネント定義の間に配置されています。これは ESLint ルールや一般的な慣習に反します。♻️ 修正案
ファイル先頭（Line 1-9）に移動:
"use client"; -import { Terminal, Save, Shield, MessageSquare, Link, EyeOff, Settings } from "lucide-react"; +import { Terminal, Save, Shield, MessageSquare, Link, EyeOff, Settings, X, Plus } from "lucide-react"; import { useState } from "react"; import CollapsibleSection from "@/components/CollapsibleSection"; import Modal from "@/components/Modal"; import ChannelSelecter from "@/components/channel-selecter"; +import RoleSelector from "@/components/role-selector"; +import CommandsControl from "@/components/commands"; import Permissions from "@/constants/Discord/Permissions";そして Lines 257-260 を削除。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/ModeratorClient.tsx around lines 257 - 260, Move the imports for X, Plus, RoleSelector, and CommandsControl to the top of the file so all import statements are declared before the component definitions: remove the inline import block that currently appears inside ModeratorClient.tsx between the component definitions (the block importing X, Plus from "lucide-react" and RoleSelector from "@/components/role-selector" and CommandsControl from "@/components/commands") and re-add those four imports at the very beginning of the file with the other imports to satisfy ESLint and conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bot/cogs/moderator.py`:
- Line 168: The timeout command's call to reason_parse uses the wrong action
label "Ban解除"; update the action argument in the timeout flow so it reflects
timeout removal (e.g., "Timeout解除" or "タイムアウト解除") instead of "Ban解除" — locate
the await user.timeout(...) call inside timeout_command and change the third
parameter passed to reason_parse to the correct timeout-specific action string.
- Around line 229-240: member.client_status is being treated like a single
Status enum and its mobile/desktop checks are overwriting each other; change to
use member.status (the Status enum) when looking up STATUS_EMOJIS (e.g., replace
status = member.client_status with status = member.status and use
STATUS_EMOJIS.get(status,...)), and for device detection read the client_status
flags without overwriting (e.g., inspect member.client_status.mobile_status and
member.client_status.desktop_status, append each matching label to a list and
join them into a single string, falling back to "Web" or "不明" if none), then
pass that device string to the existing embed.add_field calls for "ステータス" and
"機種".
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts:
- Around line 110-112: The error handler assumes res.json() always succeeds;
wrap the JSON parse in a try/catch when handling !res.ok so non-JSON responses
don't throw: attempt to await res.json() inside try and use its .error or
message, and in the catch fallback to await res.text() (or a default message)
before returning via NextResponse.json so the code paths around res, res.json(),
and NextResponse.json handle plain text or empty responses safely.
- Around line 130-139: The DELETE handler in route.ts currently trusts the query
param `type` and concatenates it into the backend URL; validate it the same way
POST does by using the existing isAutomodType() function (or equivalent
whitelist) before building the URL, return a 400 NextResponse when validation
fails, and after validation use encodeURIComponent(type) when interpolating into
`${BACKEND_URL}/guilds/automod/${guildId}/${type}` to prevent any path traversal
or injection; reference the DELETE handler in route.ts, the
searchParams.get("type") usage, isAutomodType(), and the fetch call to ensure
the fix is applied in the right spot.
- Around line 66-71: The catch currently assumes error.status exists, which it
doesn't, so update validateAdmin and checkAdminPermission to throw a custom
error type (e.g., HttpError with a status property) or ensure they set
error.status / error.statusCode when throwing; then change the route.ts catch to
derive status from error.status || error.statusCode and fall back to 500, and
include the error message. Specifically: add/throw an HttpError (with status)
from validateAdmin/checkAdminPermission, or map known error names to 401/403 in
the catch block, and use that status when returning NextResponse.json.
---
Duplicate comments:
In `@src/bot/cogs/moderator.py`:
- Around line 90-92: Multiple bare except blocks in src/bot/cogs/moderator.py
(e.g., inside the kick command and other handlers) swallow errors and obscure
causes; replace each bare "except:" with specific exception handlers: catch
discord.Forbidden to send a clear permission error, catch discord.HTTPException
to send an actionable failure message, and finally catch Exception as e to log
the unexpected error via the module logger (or self.bot.logger) and send a
generic failure message; update the handlers referenced (kick_command and the
other try/except blocks listed in the review) to log exception details
(exception string/traceback) instead of silencing them.
- Around line 21-43: The Command instances (kick, ban, unban, timeout,
remove_timeout, user_info) are created without setting their parameters, so
Discord never receives option schemas and kwargs.get("member")/other options are
always None; update each Command object's parameters attribute to include the
expected option definitions (e.g., member, reason, duration) matching the bot's
handler expectations and the Discord option types, then re-register with
self.bot.add_slashcommand so the Discord API receives the options and the
corresponding handler methods (kick_command, ban_command, unban_command,
timeout_command, remove_timeout_command, user_info_command) receive populated
kwargs.
- Around line 317-322: The invite-detection branch ignores the
allow_only_verified flag; update the block that handles automod_map["invite"] to
respect config.get("allow_only_verified"). If allow_only_verified is true,
extract the invite code/URL from message.content, resolve the invite via the bot
(e.g., using the bot/client fetch_invite method) and inspect the returned
invite.guild to confirm it is a verified/official server (check
invite.guild.verified or invite.guild.features for a VERIFIED/COMMUNITY-like
flag); only skip _is_whitelisted/execute_automod if the guild is verified —
otherwise treat it as a disallowed invite. Add safe error handling around
fetching (network/permissions) so failures fall back to treating the invite as
disallowed and still call execute_automod(message, config, "招待リンク検知",
"許可されていない招待リンク") when appropriate.
- Around line 340-341: The current check using any(domain in url for domain in
allowed_domains) should be replaced with a hostname-based comparison: for each
url in urls, parse the URL (using urllib.parse.urlparse) to get parsed.hostname
(adding a default scheme like "http://" if missing), then set is_allowed =
any(parsed_hostname == domain for domain in allowed_domains) while guarding
against parsed.hostname being None; update the loop in moderator.py where urls,
allowed_domains and is_allowed are used to perform this exact hostname equality
check.
- Around line 294-304: The code is making two API calls per message via
is_module_enabled(guild_id) and get_all_automod_settings(guild_id); add a
guild-scoped TTL cache to avoid calling these on every message by caching the
results keyed by guild_id with an expiry (e.g., in-memory dict or existing cache
helper). Modify the lookup in moderator.py so the call site for
is_module_enabled and get_all_automod_settings first checks the cache (and
refreshes on expiry), and use the cached value for the subsequent enabled check
(is_enabled.get('enabled')) and for automod_map retrieval; ensure cache keys
include the module name for module flags and guild_id for automod settings and
that cache refresh on miss/exception falls back to performing the API call.
- Around line 354-375: The execute_automod function only handles "delete",
"warn", and "timeout" but ignores "kick" and "ban" saved from the dashboard; add
handling for these actions in execute_automod by checking for "kick" and "ban"
in the actions list and calling the Member methods to perform them (use
message.author.kick(reason=reason) for kick and
message.author.ban(reason=reason) for ban), wrapping each in try/except to avoid
crashing on missing permissions or errors, and include them in the actions
string passed to send_mod_log so the log reflects the executed sanctions.
---
Nitpick comments:
In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/ModeratorClient.tsx:
- Around line 166-171: The code path in ModeratorClient.tsx that handles a
successful update when target === "basic" leaves only a comment and doesn't
update local state; either remove the placeholder comment or update the
appropriate state values (e.g., call setAutomodSettings to merge result.data
into the existing automod settings and/or update the logChannelId state from the
returned payload) so the UI reflects the saved basic settings; locate the
conditional around setAutomodSettings and implement the state merge for the
"basic" case (or delete the comment if no local change is needed).
- Around line 257-260: Move the imports for X, Plus, RoleSelector, and
CommandsControl to the top of the file so all import statements are declared
before the component definitions: remove the inline import block that currently
appears inside ModeratorClient.tsx between the component definitions (the block
importing X, Plus from "lucide-react" and RoleSelector from
"@/components/role-selector" and CommandsControl from "@/components/commands")
and re-add those four imports at the very beginning of the file with the other
imports to satisfy ESLint and conventions.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4e9e678b-29a7-495b-b42f-4dd14bb01fe3
📒 Files selected for processing (4)
src/bot/cogs/moderator.pysrc/dashboard/src/app/api/guilds/[guildId]/modules/moderator/route.tssrc/dashboard/src/app/dashboard/[guildId]/moderator/ModeratorClient.tsxsrc/dashboard/src/lib/commands.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/dashboard/src/lib/commands.ts
Comment on lines
+66
to
+71
| } catch (error: any) { | ||
| const status = error.status || 500; | ||
| return NextResponse.json( | ||
| { error: error.message || "Internal Server Error" }, | ||
| { status } | ||
| ); |
There was a problem hiding this comment.
error.status は Error オブジェクトに存在しないため、常に 500 が返されます。
validateAdmin が投げる Error オブジェクトには .status プロパティがありません。checkAdminPermission も通常の Error を投げるため、認可エラー（401/403）とサーバーエラー（500）の区別ができません。
🛠️ 修正案
} catch (error: any) {
- const status = error.status || 500;
+ let status = 500;
+ if (error.message === "Unauthorized") status = 401;
+ else if (error.message === "Forbidden") status = 403;
return NextResponse.json(
{ error: error.message || "Internal Server Error" },
{ status }
);
}または、カスタムエラークラスを使用してステータスを明示的に持たせることを推奨します。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts around
lines 66 - 71, The catch currently assumes error.status exists, which it
doesn't, so update validateAdmin and checkAdminPermission to throw a custom
error type (e.g., HttpError with a status property) or ensure they set
error.status / error.statusCode when throwing; then change the route.ts catch to
derive status from error.status || error.statusCode and fall back to 500, and
include the error message. Specifically: add/throw an HttpError (with status)
from validateAdmin/checkAdminPermission, or map known error names to 401/403 in
the catch block, and use that status when returning NextResponse.json.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
src/bot/cogs/moderator.py (7)
341-345:⚠️ Potential issue | 🟠 Major許可ドメイン判定が部分一致のため迂回可能です。
any(domain in url for domain in allowed_domains)はhttps://google.com.evil.tldやhttps://evil.tld/?next=google.comも許可してしまいます。URL をパースしてホスト名で完全一致またはサブドメイン境界付きサフィックスを比較してください。🔒 修正案
+from urllib.parse import urlparse + # on_message内のbadlinkチェック部分 - is_allowed = any(domain in url for domain in allowed_domains) + hostname = (urlparse(url).hostname or "").lower() + is_allowed = any( + hostname == domain.lower() + or hostname.endswith(f".{domain.lower()}") + for domain in allowed_domains + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 341 - 345, The current allow-check uses substring matching (any(domain in url for domain in allowed_domains)) which can be bypassed; instead parse each URL (e.g., via urllib.parse.urlparse) to extract the hostname and then set is_allowed only if the hostname equals an allowed domain or is a subdomain of an allowed domain (hostname == domain or hostname.endswith("." + domain)); update the loop where urls are checked (the block that sets is_allowed and calls execute_automod) to perform this hostname-based exact/subdomain-safe comparison against allowed_domains.
295-308:⚠️ Potential issue | 🟠 Majorメッセージごとに 2 回の API 呼び出しはパフォーマンス上の問題です。
is_module_enabled()とget_all_automod_settings()が毎メッセージで HTTP GET を実行します。高トラフィックサーバーでは API 負荷とレイテンシが問題になります。ギルド単位の TTL キャッシュを検討してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 295 - 308, The code calls is_module_enabled(guild_id) and get_all_automod_settings(guild_id) on every message which causes two HTTP GETs per message; replace these per-message calls with a guild-scoped TTL cache (e.g., in-mem dict or an async LRU/TTL cache) keyed by guild_id, caching the result of is_module_enabled and the automod settings (automod_map) for a short duration; modify the message handler in moderator.py to first check the cache for "moderator:enabled:{guild_id}" and "moderator:automod:{guild_id}" and only call self.bot.api.is_module_enabled or self.bot.api.get_all_automod_settings on cache miss, and ensure cache entries expire after a configured TTL and handle API exceptions as currently done.
355-376:⚠️ Potential issue | 🟠 MajorAutoMod で選択可能な
kick/banアクションが実行されません。ダッシュボードでは
kickとbanを選択できますが、execute_automod()はdelete/warn/timeoutのみを処理しています。設定保存は成功しても制裁が実行されません。また、
except: passは例外を完全に握りつぶしており、デバッグが不可能です。🛠️ 修正案: kick/ban アクションの追加とエラーログ
if "timeout" in actions: try: await message.author.timeout(discord.utils.utcnow() + datetime.timedelta(minutes=10), reason=reason) - except: - pass + except discord.Forbidden: + print(f"AutoMod timeout failed: no permission for {message.author.id}") + except discord.HTTPException as e: + print(f"AutoMod timeout failed: {e}") + + if "kick" in actions: + try: + await message.author.kick(reason=reason) + except discord.Forbidden: + print(f"AutoMod kick failed: no permission for {message.author.id}") + except discord.HTTPException as e: + print(f"AutoMod kick failed: {e}") + + if "ban" in actions: + try: + await message.guild.ban(message.author, reason=reason) + except discord.Forbidden: + print(f"AutoMod ban failed: no permission for {message.author.id}") + except discord.HTTPException as e: + print(f"AutoMod ban failed: {e}") await self.send_mod_log(message.guild, message.author, ", ".join(actions), f"{reason} ({detail})", message.content)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 355 - 376, execute_automod is only handling delete/warn/timeout so configured "kick" and "ban" never run and bare "except: pass" swallows errors; update execute_automod to handle "kick" and "ban" (check for "kick" in actions -> await message.author.kick(reason=reason); check for "ban" in actions -> await message.author.ban(reason=reason)) and replace the silent except blocks around message.delete() and timeout/kick/ban with exception handling that logs the exception (use the cog's logger, e.g., self.logger.exception or self.bot.logger.error with the exception) so failures (permission/forbidden/etc.) are recorded; keep the final send_mod_log call using the same actions list and reason/detail.
21-43:⚠️ Potential issue | 🔴 Criticalスラッシュコマンドのパラメータが Discord に登録されていません。
Commandオブジェクトにパラメータ定義がないため、Discord はコマンドオプションを認識しません。Context snippet 4 によると、コマンドディスパッチャはinteraction.data.get("options", [])から値を取得しますが、パラメータが登録されていない場合、Discord はオプションを送信しません。結果として
kwargs.get("member")やkwargs.get("user")は常にNoneを返し、すべてのコマンドが「ユーザーIDが指定されていません。」エラーで終了します。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 21 - 43, Command オブジェクトにオプション定義が無いため Discord にコマンドパラメータが登録されておらず、kick_command / ban_command / unban_command / timeout_command / remove_timeout_command / user_info_command 側で期待する kwargs（例: "member" や "user"）が常に None になっています。各 Command インスタンス (kick, ban, unban, timeout, remove_timeout, user_info) を作る際にコマンドオプション（名前、型、説明、必須フラグ）を正しく追加して Discord に登録されるようにし、オプション名がハンドラ内で参照しているキーと一致するようにしてください（例: "member" か "user" を使っているハンドラに合わせて定義すること）。
84-92:⚠️ Potential issue | 🟠 Majorベアの
except:を使用せず、例外を明示してログしてください。
except:はSystemExitやKeyboardInterruptまで捕捉します。Discord API エラーと権限エラーの切り分けができなくなり、デバッグが困難になります。この問題は
ban_command、unban_command、timeout_command、remove_timeout_command、user_info_commandにも同様に存在します（Lines 116, 142, 173, 199, 215）。🛠️ 修正案
try: await user.kick(reason=self.reason_parse(reason, interaction.user, "キック")) await interaction.followup.send(content=f"🦶 {user.mention} をサーバーからキックしました。", allowed_mentions=discord.AllowedMentions.none()) except discord.Forbidden: await interaction.followup.send(content="権限がありません。", allowed_mentions=discord.AllowedMentions.none()) return - except: - await interaction.followup.send(content="キックに失敗しました。", allowed_mentions=discord.AllowedMentions.none()) + except discord.HTTPException as e: + print(f"Kick failed for {user_id}: {e}") + await interaction.followup.send(content="キックに失敗しました。", allowed_mentions=discord.AllowedMentions.none()) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 84 - 92, 該当のベア except をやめて、Discord API と一般例外を明示的に捕捉して例外情報をログするように修正してください（例：user.kick 呼び出し周りでは discord.Forbidden は既に捕捉しているため、discord.HTTPException や discord.NotFound、その他の Exception を except Exception as e としてキャッチし、logger.exception/ logger.error で e を記録した上で interaction.followup.send のエラーメッセージを返す）。同じパターンで ban_command、unban_command、timeout_command、remove_timeout_command、user_info_command のベア except を置き換え、各ハンドラで適切な discord.* 例外と汎用 Exception を明示的に扱ってログ出力するように統一してください。
318-323:⚠️ Potential issue | 🟠 Major
allow_only_verifiedフラグが実行時に無視されています。ダッシュボードでは招待リンク対策に「Discord公式サーバーのみ許可」を設定できますが、このコードはそのフラグを一切参照していません。設定しても挙動が変わりません。
47-65:⚠️ Potential issue | 🟡 Minor不正な入力が部分一致で受け入れられる可能性があります。
re.findallは部分一致するため、"10x2m5s"のような入力は"10x"を無視して125秒として処理されます。ユーザーが意図しない短いタイムアウトが設定される可能性があります。🛡️ 修正案: 全体一致検証を追加
def parse_duration(self, duration_str: str): - regex = r"(\d+)\s*([dhms])" + # 全体が有効な形式かどうかを先に検証 + if not re.fullmatch(r'(\d+\s*[dhms])+', duration_str.lower()): + return None + + regex = r"(\d+)\s*([dhms])" matches = re.findall(regex, duration_str.lower())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 47 - 65, The parse_duration function currently uses re.findall which allows partial matches (e.g., "10x2m5s" becomes 125s); change the validation to require the entire input match the expected pattern before summing units: build an anchored regex like r'^\s*(?:\d+\s*[dhms]\s*)+$' (apply to duration_str.lower().strip()) or use re.fullmatch, and return None if it doesn't match; then proceed to extract the number/unit pairs (using re.findall or re.finditer) and use the existing unit_seconds logic in parse_duration to compute total_seconds.src/dashboard/src/app/api/guilds/[guildId]/modules/moderator/route.ts (1)
154-156:⚠️ Potential issue | 🟠 MajorDELETE ハンドラでも同様に認証エラーが 500 として返されます。
POST ハンドラと同じ問題があります。GET ハンドラと一貫したエラーハンドリングを適用してください。
🐛 修正案
} catch (error: any) { - return NextResponse.json({ error: error.message }, { status: 500 }); + let status = 500; + if (error.message === "Unauthorized") status = 401; + else if (error.message === "Forbidden") status = 403; + return NextResponse.json( + { error: error.message || "Internal Server Error" }, + { status } + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts around lines 154 - 156, The DELETE handler's catch currently returns a generic 500 via "return NextResponse.json({ error: error.message }, { status: 500 });" which causes authentication errors to be surfaced as 500; change the error handling in the DELETE route (the catch block in src/.../modules/moderator/route.ts) to mirror the GET handler's behavior by checking the thrown error type/message for authentication/authorization failures and returning the same status (e.g., 401 or 403) and payload as the GET handler instead of always 500; keep using NextResponse.json but select the status dynamically based on the error (fall back to 500 only for unexpected errors).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts:
- Around line 125-127: The POST handler's catch block returns all errors as 500,
so authentication errors thrown by validateAdmin ("Unauthorized" or "Forbidden")
aren't mapped to proper status codes; update the POST catch to mirror the GET
handler's error mapping: inspect error.message from validateAdmin and return 401
for "Unauthorized", 403 for "Forbidden", otherwise 500, ensuring the
NextResponse.json call uses the correct status; locate the POST handler and its
catch block in route.ts and apply the same mapping logic used by the GET handler
for consistent behavior.
---
Duplicate comments:
In `@src/bot/cogs/moderator.py`:
- Around line 341-345: The current allow-check uses substring matching
(any(domain in url for domain in allowed_domains)) which can be bypassed;
instead parse each URL (e.g., via urllib.parse.urlparse) to extract the hostname
and then set is_allowed only if the hostname equals an allowed domain or is a
subdomain of an allowed domain (hostname == domain or hostname.endswith("." +
domain)); update the loop where urls are checked (the block that sets is_allowed
and calls execute_automod) to perform this hostname-based exact/subdomain-safe
comparison against allowed_domains.
- Around line 295-308: The code calls is_module_enabled(guild_id) and
get_all_automod_settings(guild_id) on every message which causes two HTTP GETs
per message; replace these per-message calls with a guild-scoped TTL cache
(e.g., in-mem dict or an async LRU/TTL cache) keyed by guild_id, caching the
result of is_module_enabled and the automod settings (automod_map) for a short
duration; modify the message handler in moderator.py to first check the cache
for "moderator:enabled:{guild_id}" and "moderator:automod:{guild_id}" and only
call self.bot.api.is_module_enabled or self.bot.api.get_all_automod_settings on
cache miss, and ensure cache entries expire after a configured TTL and handle
API exceptions as currently done.
- Around line 355-376: execute_automod is only handling delete/warn/timeout so
configured "kick" and "ban" never run and bare "except: pass" swallows errors;
update execute_automod to handle "kick" and "ban" (check for "kick" in actions
-> await message.author.kick(reason=reason); check for "ban" in actions -> await
message.author.ban(reason=reason)) and replace the silent except blocks around
message.delete() and timeout/kick/ban with exception handling that logs the
exception (use the cog's logger, e.g., self.logger.exception or
self.bot.logger.error with the exception) so failures
(permission/forbidden/etc.) are recorded; keep the final send_mod_log call using
the same actions list and reason/detail.
- Around line 21-43: Command オブジェクトにオプション定義が無いため Discord
にコマンドパラメータが登録されておらず、kick_command / ban_command / unban_command / timeout_command
/ remove_timeout_command / user_info_command 側で期待する kwargs（例: "member" や
"user"）が常に None になっています。各 Command インスタンス (kick, ban, unban, timeout,
remove_timeout, user_info) を作る際にコマンドオプション（名前、型、説明、必須フラグ）を正しく追加して Discord
に登録されるようにし、オプション名がハンドラ内で参照しているキーと一致するようにしてください（例: "member" か "user"
を使っているハンドラに合わせて定義すること）。
- Around line 84-92: 該当のベア except をやめて、Discord API
と一般例外を明示的に捕捉して例外情報をログするように修正してください（例：user.kick 呼び出し周りでは discord.Forbidden
は既に捕捉しているため、discord.HTTPException や discord.NotFound、その他の Exception を except
Exception as e としてキャッチし、logger.exception/ logger.error で e を記録した上で
interaction.followup.send のエラーメッセージを返す）。同じパターンで
ban_command、unban_command、timeout_command、remove_timeout_command、user_info_command
のベア except を置き換え、各ハンドラで適切な discord.* 例外と汎用 Exception を明示的に扱ってログ出力するように統一してください。
- Around line 47-65: The parse_duration function currently uses re.findall which
allows partial matches (e.g., "10x2m5s" becomes 125s); change the validation to
require the entire input match the expected pattern before summing units: build
an anchored regex like r'^\s*(?:\d+\s*[dhms]\s*)+$' (apply to
duration_str.lower().strip()) or use re.fullmatch, and return None if it doesn't
match; then proceed to extract the number/unit pairs (using re.findall or
re.finditer) and use the existing unit_seconds logic in parse_duration to
compute total_seconds.
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts:
- Around line 154-156: The DELETE handler's catch currently returns a generic
500 via "return NextResponse.json({ error: error.message }, { status: 500 });"
which causes authentication errors to be surfaced as 500; change the error
handling in the DELETE route (the catch block in
src/.../modules/moderator/route.ts) to mirror the GET handler's behavior by
checking the thrown error type/message for authentication/authorization failures
and returning the same status (e.g., 401 or 403) and payload as the GET handler
instead of always 500; keep using NextResponse.json but select the status
dynamically based on the error (fall back to 500 only for unexpected errors).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c738485e-34ad-41da-a188-594943ad4322
📒 Files selected for processing (2)
src/bot/cogs/moderator.pysrc/dashboard/src/app/api/guilds/[guildId]/modules/moderator/route.ts
Comment on lines
+125
to
+127
| } catch (error: any) { | ||
| return NextResponse.json({ error: error.message }, { status: 500 }); | ||
| } |
There was a problem hiding this comment.
POST ハンドラで認証エラーが常に 500 として返されます。
validateAdmin は "Unauthorized" または "Forbidden" メッセージで Error を投げますが、POST の catch ブロックは GET ハンドラのようにステータスコードをマッピングしていません。認証エラーが 500 として返され、フロントエンドで適切なリダイレクトができません。
🐛 修正案: GET ハンドラと同様のエラーハンドリングを適用
} catch (error: any) {
- return NextResponse.json({ error: error.message }, { status: 500 });
+ let status = 500;
+ if (error.message === "Unauthorized") status = 401;
+ else if (error.message === "Forbidden") status = 403;
+ return NextResponse.json(
+ { error: error.message || "Internal Server Error" },
+ { status }
+ );
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/src/app/api/guilds/`[guildId]/modules/moderator/route.ts around
lines 125 - 127, The POST handler's catch block returns all errors as 500, so
authentication errors thrown by validateAdmin ("Unauthorized" or "Forbidden")
aren't mapped to proper status codes; update the POST catch to mirror the GET
handler's error mapping: inspect error.message from validateAdmin and return 401
for "Unauthorized", 403 for "Forbidden", otherwise 500, ensuring the
NextResponse.json call uses the correct status; locate the POST handler and its
catch block in route.ts and apply the same mapping logic used by the GET handler
for consistent behavior.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (6)
src/bot/cogs/moderator.py (6)
320-328:⚠️ Potential issue | 🟠 Majorメッセージごとに 2 回 API を叩く実装はホットパスに重すぎます。
is_module_enabled()とget_all_automod_settings()が毎メッセージで直列に走るので、API 遅延がそのまま Bot の処理遅延になります。guild 単位で有効フラグと設定をまとめてキャッシュするか、取得 API を 1 回にまとめてください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 320 - 328, The code calls self.bot.api.is_module_enabled(guild_id, "moderator") and then self.bot.api.get_all_automod_settings(guild_id) for every message, which is too expensive; change the logic so you only hit the API once per guild (or cache results) by either adding a combined endpoint call (e.g., get_moderator_module_and_automod(guild_id)) or introducing a short-lived per-guild cache in the bot (keyed by guild_id) that stores both the module enabled flag and automod settings and is checked/invalidated before making any API calls; update the code paths around is_module_enabled and get_all_automod_settings (and their callers) to use the single combined result or the cache to avoid serial per-message API requests.
380-401:⚠️ Potential issue | 🟠 Major未対応 action を silent ignore したまま、ログだけ成功扱いになります。
ここで実装されているのは
delete/warn/timeoutだけで、その他の action は何も実行されないままsend_mod_log()に載ります。kick/banのような設定値を扱う想定なら分岐を追加し、未対応 action は保存時か実行時に明示的に弾いた方が安全です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 380 - 401, The execute_automod function currently only implements "delete", "warn", and "timeout" but silently treats any other entries in actions as successful and logs them via send_mod_log; update execute_automod to either (A) add explicit handling for expected additional actions such as "kick" and "ban" (using message.guild.kick/member.kick or guild.ban as appropriate) or (B) validate the actions list at the start and raise/log/ignore unsupported actions explicitly so they are not reported as applied—ensure you reference execute_automod, the actions variable, and send_mod_log when adding branches or validation and make failure paths explicit (log an error or return) instead of silently continuing.
21-47:⚠️ Potential issue | 🔴 Criticalコマンドのオプション定義が無いため、主要コマンドが実行不能です。
ここで
Command.parametersを一度も埋めていないので、現状のsrc/bot/lib/command.pyの実装ではkwargsが空のままです。結果として/kick/ban/timeout/clearなどはkwargs.get(...)がNoneになり、早期エラーになるかint(None)で落ちます。登録時に各コマンドの option schema を渡してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 21 - 47, The commands are missing their option schemas, so Command.parameters is never populated and handlers like kick_command, ban_command, timeout_command, clear_command receive empty kwargs causing None/TypeErrors; update each Command instance (kick, ban, unban, timeout, remove_timeout, clear, user_info) to set Command.parameters to the proper option definitions (e.g., user/member argument for target, optional reason string for kick/ban/unban, duration integer or ISO string for timeout/remove_timeout, amount integer for clear, and any required flags) matching how the handlers extract kwargs so kwargs.get("user"/"member"/"duration"/"amount"/"reason") returns valid values; ensure the parameter names exactly match what kick_command, ban_command, timeout_command, remove_timeout_command, clear_command, and user_info_command expect.
88-95:⚠️ Potential issue | 🟠 Major
except:/except: passが障害原因を隠しています。この書き方だと権限不足・対象不在・Discord API 失敗・バックエンド障害が全部同じ扱いになり、AutoMod の
delete/timeout失敗は完全に無音です。discord.Forbidden/discord.NotFound/discord.HTTPExceptionなどに分けて、少なくともログには例外と対象情報を残してください。Also applies to: 114-121, 140-147, 171-178, 197-204, 236-241, 327-330, 387-390, 396-399
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 88 - 95, The bare except around the user.kick call hides root causes; update the try/except in the kick flow (the block using user.kick and reason_parse and interaction.followup.send) to handle discord.Forbidden, discord.NotFound, discord.HTTPException (and a final Exception) separately, sending appropriate user-facing messages while logging the full exception and target info (user.id, user.name, reason, interaction.user) for each failure; ensure logs use the existing logger instance (e.g., self.logger or interaction.client.logger) so the error and context are retained for debugging.
362-368:⚠️ Potential issue | 🟠 Major許可ドメイン判定が部分一致なので簡単に迂回されます。
any(domain in url for domain in allowed_domains)だとhttps://google.com.evil.tldやhttps://evil.tld/?next=google.comまで許可されます。URL をパースしてhostnameを取り、完全一致かサブドメイン境界付き suffix だけを許可してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 362 - 368, The current check uses any(domain in url for domain in allowed_domains) which allows bypasses; instead parse each URL (e.g., using urllib.parse.urlparse) to extract the hostname and then allow only when hostname == domain or hostname.endswith("." + domain) for each domain in allowed_domains; update the loop in the moderator cog where urls are computed (the block using variables allowed_domains, urls and is_allowed) to perform this hostname-based exact-or-subdomain-suffix check and reject other matches.
51-69:⚠️ Potential issue | 🟠 Major
parse_duration()が全文一致しておらず、余分な文字列を無視して通します。
re.findall()だけだとabc10mや10mfooも10mとして受理され、想定より短い timeout が掛かります。re.fullmatch()で文字列全体を検証してから集計した方が安全です。🛠️ 修正例
def parse_duration(self, duration_str: str): - regex = r"(\d+)\s*([dhms])" - matches = re.findall(regex, duration_str.lower()) + normalized = duration_str.lower().strip() + if not re.fullmatch(r"(?:\d+\s*[dhms])+", normalized): + return None + matches = re.findall(r"(\d+)\s*([dhms])", normalized)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/moderator.py` around lines 51 - 69, parse_duration currently uses re.findall which allows extra chars like "abc10m" or "10mfoo" to be accepted; update parse_duration to first validate the entire string with re.fullmatch using a pattern that matches one or more duration tokens (e.g., an anchored pattern for sequences like "1h30m"), and only after a successful full match extract the (value, unit) pairs (via findall or finditer) and sum them; reference the parse_duration function and the regex variable to locate and replace the validation logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bot/cogs/moderator.py`:
- Around line 74-99: The moderator command handlers (e.g., kick_command) lack
permission checks so any member can run them; add an early guard in each handler
(kick_command, ban_command, timeout_command, clear_command and other moderator
handlers) that inspects interaction.user.guild_permissions for the specific
permission required (kick_members for kick_command, ban_members for ban_command,
moderate_members or timeout_members for timeout_command, manage_messages for
clear_command) and if the permission is missing respond with a localized
"権限がありません" followup and return without performing the action; place this check
immediately after defer() and before fetching the target member so all moderator
actions are blocked for unauthorized users.
- Around line 213-225: The reply sends a "max 100" message but then continues to
call purge with amount_int; update the handler (the code around amount =
kwargs.get("amount"), amount_int, and the call to interaction.channel.purge) to
enforce 1 <= amount_int <= 100 before calling purge: either clamp amount_int to
100 or send the follow-up and return immediately after interaction.followup.send
when amount_int > 100, and also validate lower bound (reject or clamp amounts <
1) so purge() is only invoked with a safe, validated integer.
In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/ModeratorClient.tsx:
- Around line 326-329: The delete "X" icon currently uses an onClick directly on
the SVG which prevents keyboard focus and accessibility; update the render for
the whitelist items (where whitelistChannels, setWhitelistChannels and
toggleItem are used) to wrap the <X> icon inside a <button type="button">, move
the onClick handler from the <X> component to that button, and add an
appropriate aria-label (e.g., "Remove channel" or include the id) and
focus/hover styles so it is keyboard-focusable and screen-reader friendly; apply
the same change for the corresponding roles list (the other block using
whitelistRoles / setWhitelistRoles / toggleItem).
- Around line 305-313: The toggle button currently only conveys selection
visually; update the button in ModeratorClient (the element using
toggleItem/actions/setActions/opt/isSelected) to expose its state to assistive
tech by adding an aria-pressed attribute bound to the boolean isSelected value
(e.g., set aria-pressed to the same isSelected used to determine classes),
ensuring it reflects true/false when toggled.
- Around line 119-122: The /clear command registration currently sets
default_member_permissions to Permissions.ManageChannels which is incorrect for
deleting messages; update the command object for name "clear" to use
Permissions.ManageMessages.toString() instead of
Permissions.ManageChannels.toString() so moderators with message-management
rights see and can run the command (modify the default_member_permissions field
where the "clear" command is defined).
- Around line 57-65: unban コマンドの "user" オプションは type 6 (USER) のままでは既に BAN
された・ギルド外のユーザーを選べないため、オプションの type を 3 (STRING) に変更してユーザーID を文字列で受け取るようにしてください。対象は
ModeratorClient 内の unban コマンド定義（name: "unban"、options の "user"
エントリ）で、description を「ユーザーID を入力してください」等に更新し、コマンドハンドラ側で受け取った文字列を正しい Snowflake/ID
として扱い（パース/バリデーションと fetchBan/unban 呼び出しへ渡す）するように修正してください。
- Around line 271-274: This client component statically imports the Node builtin
deflateRawSync from "node:zlib" which causes a client bundle error and it's
unused; remove the import entry for deflateRawSync (the "node:zlib" import) from
the top-level imports in ModeratorClient.tsx (next to X, Plus, RoleSelector,
CommandsControl) and ensure no other Node builtins are statically imported in
this file; if you need similar functionality on the client, move it to a server
module or use a browser-safe library instead.
---
Duplicate comments:
In `@src/bot/cogs/moderator.py`:
- Around line 320-328: The code calls self.bot.api.is_module_enabled(guild_id,
"moderator") and then self.bot.api.get_all_automod_settings(guild_id) for every
message, which is too expensive; change the logic so you only hit the API once
per guild (or cache results) by either adding a combined endpoint call (e.g.,
get_moderator_module_and_automod(guild_id)) or introducing a short-lived
per-guild cache in the bot (keyed by guild_id) that stores both the module
enabled flag and automod settings and is checked/invalidated before making any
API calls; update the code paths around is_module_enabled and
get_all_automod_settings (and their callers) to use the single combined result
or the cache to avoid serial per-message API requests.
- Around line 380-401: The execute_automod function currently only implements
"delete", "warn", and "timeout" but silently treats any other entries in actions
as successful and logs them via send_mod_log; update execute_automod to either
(A) add explicit handling for expected additional actions such as "kick" and
"ban" (using message.guild.kick/member.kick or guild.ban as appropriate) or (B)
validate the actions list at the start and raise/log/ignore unsupported actions
explicitly so they are not reported as applied—ensure you reference
execute_automod, the actions variable, and send_mod_log when adding branches or
validation and make failure paths explicit (log an error or return) instead of
silently continuing.
- Around line 21-47: The commands are missing their option schemas, so
Command.parameters is never populated and handlers like kick_command,
ban_command, timeout_command, clear_command receive empty kwargs causing
None/TypeErrors; update each Command instance (kick, ban, unban, timeout,
remove_timeout, clear, user_info) to set Command.parameters to the proper option
definitions (e.g., user/member argument for target, optional reason string for
kick/ban/unban, duration integer or ISO string for timeout/remove_timeout,
amount integer for clear, and any required flags) matching how the handlers
extract kwargs so kwargs.get("user"/"member"/"duration"/"amount"/"reason")
returns valid values; ensure the parameter names exactly match what
kick_command, ban_command, timeout_command, remove_timeout_command,
clear_command, and user_info_command expect.
- Around line 88-95: The bare except around the user.kick call hides root
causes; update the try/except in the kick flow (the block using user.kick and
reason_parse and interaction.followup.send) to handle discord.Forbidden,
discord.NotFound, discord.HTTPException (and a final Exception) separately,
sending appropriate user-facing messages while logging the full exception and
target info (user.id, user.name, reason, interaction.user) for each failure;
ensure logs use the existing logger instance (e.g., self.logger or
interaction.client.logger) so the error and context are retained for debugging.
- Around line 362-368: The current check uses any(domain in url for domain in
allowed_domains) which allows bypasses; instead parse each URL (e.g., using
urllib.parse.urlparse) to extract the hostname and then allow only when hostname
== domain or hostname.endswith("." + domain) for each domain in allowed_domains;
update the loop in the moderator cog where urls are computed (the block using
variables allowed_domains, urls and is_allowed) to perform this hostname-based
exact-or-subdomain-suffix check and reject other matches.
- Around line 51-69: parse_duration currently uses re.findall which allows extra
chars like "abc10m" or "10mfoo" to be accepted; update parse_duration to first
validate the entire string with re.fullmatch using a pattern that matches one or
more duration tokens (e.g., an anchored pattern for sequences like "1h30m"), and
only after a successful full match extract the (value, unit) pairs (via findall
or finditer) and sum them; reference the parse_duration function and the regex
variable to locate and replace the validation logic.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33495f98-b107-4fa4-ba81-68c85ed02a05
📒 Files selected for processing (3)
src/bot/cogs/moderator.pysrc/dashboard/src/app/dashboard/[guildId]/moderator/ModeratorClient.tsxsrc/dashboard/src/lib/commands.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/dashboard/src/lib/commands.ts
Comment on lines
+271
to
+274
| import { X, Plus } from "lucide-react"; | ||
| import RoleSelector from "@/components/role-selector"; | ||
| import CommandsControl from "@/components/commands"; | ||
| import { deflateRawSync } from "node:zlib"; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Next.js App Router で "use client" のコンポーネントから node:zlib のような Node.js builtin module を import できますか？
💡 Result:
いいえ、Next.js App Routerの"use client"コンポーネントからnode:zlibのようなNode.jsビルトインModuleをimportすることはできません。これはWebpackによるビルドエラー（UnhandledSchemeError: "node:" URIが処理されない）を引き起こします。"use client"はクライアントサイド（ブラウザ）向けのモジュールグラフを定義し、Node.js専用モジュールはブラウザで利用できないためブロックされます。Server Components（"use client"なし）ではimport可能です。
Citations:
- 1: App directory cannot have node internals import in "use client" code vercel/next.js#60491
- 2: https://nextjs.org/docs/app/building-your-application/rendering/client-components
- 3: https://nextjs.org/docs/13/app/building-your-application/rendering/client-components
🏁 Script executed:
# First, find and read the ModeratorClient.tsx file
fd -t f "ModeratorClient.tsx" --exec wc -l {}Repository: SharkBot-Dev/NewSharkBot
Length of output: 145
🏁 Script executed:
# Read the file around the import lines (270-280) and beginning of file to check for "use client"
fd -t f "ModeratorClient.tsx" --exec sh -c 'head -20 "$1" && echo "---" && sed -n "265,280p" "$1"' _ {}Repository: SharkBot-Dev/NewSharkBot
Length of output: 1314
🏁 Script executed:
# Search for usage of deflateRawSync in the file
fd -t f "ModeratorClient.tsx" --exec rg "deflateRawSync" {} -A 2 -B 2Repository: SharkBot-Dev/NewSharkBot
Length of output: 290
🏁 Script executed:
# Search for any usage of deflateRawSync in the entire file
fd -t f "ModeratorClient.tsx" --exec rg "deflateRawSync" {} -cRepository: SharkBot-Dev/NewSharkBot
Length of output: 69
🏁 Script executed:
# More comprehensive check - see all lines that contain deflateRawSync
fd -t f "ModeratorClient.tsx" --exec grep -n "deflateRawSync" {}Repository: SharkBot-Dev/NewSharkBot
Length of output: 115
node:zlib は "use client" コンポーネントで import できません。
このファイルは "use client" なので、Node builtin モジュールを静的 import するとクライアントバンドルで webpack エラー（UnhandledSchemeError）が発生します。加えて、deflateRawSync は使用されていないため、import 行を削除してください。
修正例
-import { deflateRawSync } from "node:zlib";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { X, Plus } from "lucide-react"; | |
| import RoleSelector from "@/components/role-selector"; | |
| import CommandsControl from "@/components/commands"; | |
| import { deflateRawSync } from "node:zlib"; | |
| import { X, Plus } from "lucide-react"; | |
| import RoleSelector from "@/components/role-selector"; | |
| import CommandsControl from "@/components/commands"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/ModeratorClient.tsx
around lines 271 - 274, This client component statically imports the Node
builtin deflateRawSync from "node:zlib" which causes a client bundle error and
it's unused; remove the import entry for deflateRawSync (the "node:zlib" import)
from the top-level imports in ModeratorClient.tsx (next to X, Plus,
RoleSelector, CommandsControl) and ensure no other Node builtins are statically
imported in this file; if you need similar functionality on the client, move it
to a server module or use a browser-safe library instead.
Comment on lines
+326
to
+329
| {whitelistChannels.map(id => ( | ||
| <span key={id} className="flex items-center gap-1 bg-secondary px-2 py-1 rounded text-xs border border-border"> | ||
| ID: {id} | ||
| <X className="w-3 h-3 cursor-pointer hover:text-red-500" onClick={() => toggleItem(whitelistChannels, setWhitelistChannels, id)} /> |
There was a problem hiding this comment.
削除用 X アイコンは button にしてください。
svg に直接 onClick を載せると focus できず、キーボードから操作できません。ロール側も同様です。
🔧 修正例
- <X className="w-3 h-3 cursor-pointer hover:text-red-500" onClick={() => toggleItem(whitelistChannels, setWhitelistChannels, id)} />
+ <button
+ type="button"
+ aria-label={`除外チャンネル ${id} を削除`}
+ onClick={() => toggleItem(whitelistChannels, setWhitelistChannels, id)}
+ className="hover:text-red-500"
+ >
+ <X className="w-3 h-3" />
+ </button>Also applies to: 340-343
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/src/app/dashboard/`[guildId]/moderator/ModeratorClient.tsx
around lines 326 - 329, The delete "X" icon currently uses an onClick directly
on the SVG which prevents keyboard focus and accessibility; update the render
for the whitelist items (where whitelistChannels, setWhitelistChannels and
toggleItem are used) to wrap the <X> icon inside a <button type="button">, move
the onClick handler from the <X> component to that button, and add an
appropriate aria-label (e.g., "Remove channel" or include the id) and
focus/hover styles so it is keyboard-focusable and screen-reader friendly; apply
the same change for the corresponding roles list (the other block using
whitelistRoles / setWhitelistRoles / toggleItem).
This was referenced Mar 28, 2026
Merged
Merged
Merged
Merged
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit