From ec16cb26d6ba7b5aa88f2f363d53e6101241c40e Mon Sep 17 00:00:00 2001 From: Twisted Vortek Date: Tue, 21 Jul 2026 19:47:17 +0400 Subject: [PATCH 1/2] feat: implement comprehensive bot framework with core services, database integration, safety utilities, and modular cogs --- .env.example | 7 +- AGENTS.md | 113 +++++++++++++++++++++++- pyproject.toml | 4 +- src/cogs/admin/massunban.py | 6 +- src/cogs/marketplace/marketplace.py | 5 +- src/cogs/marketplace/reviews.py | 7 +- src/cogs/marketplace_enhanced.py | 4 +- src/cogs/portfolio/portfolio_manager.py | 6 +- src/cogs/radio/radio.py | 6 +- src/cogs/resources/feeds.py | 7 +- src/core/app.py | 100 +++++++-------------- src/core/runtime_state.py | 7 ++ src/database/database.py | 4 - src/services/admin_notifier.py | 39 +++++--- src/services/directus_sync.py | 6 ++ src/services/mentorship_service.py | 35 ++++---- src/services/rss_service.py | 30 ++++++- src/utils/embeds.py | 5 +- src/utils/guild_gate.py | 22 ++--- src/utils/logger.py | 15 ++-- src/utils/marketplace/__init__.py | 0 src/utils/safety.py | 2 +- src/utils/security/audit.py | 11 ++- src/utils/security/rate_limiter.py | 35 +++++++- src/utils/security/rbac.py | 20 ++++- src/utils/security/validation.py | 6 +- 26 files changed, 331 insertions(+), 171 deletions(-) create mode 100644 src/utils/marketplace/__init__.py diff --git a/.env.example b/.env.example index 9dd4c0f..cca884b 100644 --- a/.env.example +++ b/.env.example @@ -6,9 +6,9 @@ DISCORD_TOKEN=your_discord_bot_token_here POSTGRES_HOST=postgres POSTGRES_PORT=5432 POSTGRES_DB=veka_bot -POSTGRES_USER=veka_user +POSTGRES_USER=veka_bot_user POSTGRES_PASSWORD=your_secure_password_here -DATABASE_URL=postgresql://veka_user:your_secure_password_here@postgres:5432/veka_bot +DATABASE_URL=postgresql://veka_bot_user:your_secure_password_here@postgres:5432/veka_bot # Access Control (comma-separated Discord user IDs) ADMIN_IDS=123456789,987654321 @@ -25,6 +25,9 @@ MARKETPLACE_CHANNEL_ID= # Operational alerts channel (Discord channel ID) ADMIN_ALERT_CHANNEL_ID= +# Mass unban log channel (Discord channel ID, falls back to LOGS_CHANNEL_ID if unset) +MASSUNBAN_LOG_CHANNEL_ID= + # Radio Configuration RADIO_STREAM_URL=https://www.youtube.com/watch?v=jfKfPfyJRdk RADIO_VOICE_CHANNEL_ID= diff --git a/AGENTS.md b/AGENTS.md index c80ed67..a036aef 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,7 +40,7 @@ Config in `pyproject.toml`. Ruff: `line-length=120`, `quote-style="single"`, sel - Global singleton: `from src.database.database import db`. - **`$1`/`$2` parameter style** (asyncpg native). - Methods: `fetch`, `fetch_one`/`fetchrow`, `fetchval`, `execute`, `execute_many`. All raise `DatabaseUnavailableError` on failure and flip `runtime_state.db_available = False`. -- Migrations: `.sql` files in `migrations/` (currently 001–014), auto-applied on connect by `db.run_migrations()`, tracked in `schema_migrations` table. Add as `migrations/00N_name.sql`. +- Migrations: `.sql` files in `migrations/` (currently 001–016), auto-applied on connect by `db.run_migrations()`, tracked in `schema_migrations` table. Add as `migrations/00N_name.sql`. Note: two files share the `005_` prefix — ordering is filesystem-dependent. - Connection pool strips libpq-only keepalive params (`keepalives`, `tcp_keepalives_*`) to avoid PostgreSQL rejecting them as unknown server_settings. ## Conventions @@ -50,7 +50,7 @@ Config in `pyproject.toml`. Ruff: `line-length=120`, `quote-style="single"`, sel - All user-facing output uses embed helpers from `src/utils/embeds.py`: `veka_embed`, `success_embed`, `error_embed`, `info_embed`, `alert_embed`. Pass `contributor_source=__name__` (resolved against `_STATIC_CONTRIBUTOR_MAP` keyed by full module path). - Logging: `logging.getLogger('VEKA.')` or `get_logger('VEKA.')` from `src/utils/logger.py`. - Rate limiting: `@rate_limit('bucket_name')` from `src.utils.security`. -- RBAC from `src.utils.security`: `@require_mod()`, `@require_admin()`, `@require_verified()`. Also `admin_only()`/`staff_only()` in `safety.py` (parallel auth via `ADMIN_IDS`/`OWNER_IDS` from config). RBAC hierarchy: USER < VERIFIED < MODERATOR < ADMIN < OWNER. +- RBAC from `src.utils.security`: `@require_mod()`, `@require_admin()`, `@require_verified()`. Also `admin_only()`/`staff_only()` in `safety.py` (parallel auth via `ADMIN_IDS`/`OWNER_IDS` from config). RBAC hierarchy: USER < VERIFIED < INTERN < DONATOR < ACTIVE_PRO < STAFF < ADMIN < FOUNDER. `@require_mod()` is aliased to `require_staff()` (see issue #27). - Single `.env` file for config. Either `DATABASE_URL` or individual `POSTGRES_*` vars. Seven comma-separated ID env vars (`ADMIN_IDS`, `OWNER_IDS`, `FOUNDER_IDS`, `STAFF_IDS`, `INTERN_IDS`, `DONATOR_IDS`, `ACTIVE_PRO_IDS`). `load_dotenv()` called in both `config.py` and `app.py`. ## Hardcoded values in `src/config/config.py` @@ -70,3 +70,112 @@ Implemented in `src/cogs/admin/honeypot.py` (loaded as `src.cogs.admin.honeypot` Implemented in `src/cogs/admin/massunban.py` (loaded as `src.cogs.admin.massunban`). Slash-only command group `/massunban` with subcommands: `run`, `status`, `cancel`, `recent`. Also `!massunban` prefix group with same subcommands. Admin-only, requires double confirmation (two button clicks). Resumable job model with per-user tracking in DB (`massunban_jobs`, `massunban_job_items` tables, migration `016_massunban_schema.sql`). Sequential unban processing with adaptive rate limiting (base 1.5s interval, progressive backoff on repeated 429s). On rate limit, persists progress and pauses; resumes automatically. DMs unbanned users with apology template on failure. Logs per-user results to `LOGS_CHANNEL_ID` (or `MASSUNBAN_LOG_CHANNEL_ID` if set). Startup resume for interrupted jobs via `cog_load()`. **Known limitation:** Discord's `Guild.bans()` API does not expose ban timestamps or the moderator who placed the ban. The `start_datetime`, `end_datetime`, and `banned_by` filters only apply to bans logged by this bot via its audit system (`audit_logs` table). Bans placed externally or before the bot was running cannot be filtered by date/moderator — they are included when no filters are specified, or skipped when filters are active. + +## Known Issues (Codebase Audit — 2026-07-21) + +### CRITICAL + +1. **`rss_service.py` queries a column that does not exist in the correct schema** (`src/services/rss_service.py:99`). The service calls `SELECT 1 FROM rss_cache WHERE feed_url = $1 AND entry_id = $2` and later inserts with both columns. However, migration `005_community_additions.sql` creates an incompatible `rss_cache` with only `feed_url` as primary key and no `entry_id` column. Migration `005_guild_and_rss_schema.sql` drops and recreates it with the correct schema including `entry_id` — but **only if `005_community_additions.sql` runs first**. If the migration runner applies them alphabetically, `005_community_additions` and `005_guild_and_rss_schema` share the same `005_` prefix and their ordering is filesystem-dependent. The `DROP TABLE IF EXISTS rss_cache` in the second file is the fix, but it depends on run order. Fix: rename `005_guild_and_rss_schema.sql` to `006a_...` or higher to guarantee ordering, then reconcile the numbering with existing higher migrations. + +2. **`bot.loop.create_task()` is deprecated in Python 3.10+ and removed in future asyncio** (`src/cogs/admin/massunban.py:198,221,943`). `bot.loop` returns the event loop but using `.create_task()` on it directly is deprecated in favour of `asyncio.create_task()`. Should be `asyncio.create_task(...)` throughout. Current code will emit `DeprecationWarning` in Python 3.13 runtime. + +3. **`on_disconnect` closes the database pool on every gateway disconnect** (`src/core/app.py:190-195`). Discord bots experience frequent transient reconnections (voice region switches, gateway resumption). Closing the pool on every disconnect means the DB pool is destroyed and must be fully re-established, causing `DatabaseUnavailableError` during the reconnect window. The `db_health_check` loop handles pool recovery, but there is a gap window between disconnect and recovery. Fix: remove the `db.close()` from `on_disconnect` and rely solely on the health-check loop to detect and recover broken connections. + +### HIGH + +4. **Widespread use of `datetime.utcnow()` (deprecated since Python 3.12)** — found in 11+ files: `src/utils/embeds.py:98`, `src/services/admin_notifier.py:36`, `src/cogs/radio/radio.py:140,234`, `src/cogs/marketplace/marketplace.py:97,159`, `src/cogs/marketplace/reviews.py:340,341`, `src/cogs/portfolio/portfolio_manager.py:51,272`, `src/utils/security/audit.py:70`, `src/cogs/marketplace_enhanced.py:243`. Python 3.12 deprecated `datetime.utcnow()` and it will be removed in a future version. All usages should be replaced with `datetime.now(UTC)` (importing `UTC` from `datetime`). The `admin_notifier.py` case is especially important because it controls deduplication logic in `_should_alert`. + +5. **`MASSUNBAN_LOG_CHANNEL_ID` is not documented in `.env.example`**. `src/config/config.py` reads it, `massunban.py` imports it, and `README.md` lists it — but `.env.example` has no entry for it. Operators deploying fresh will not know to set it, and mass unban logs will silently fall back to `LOGS_CHANNEL_ID` (or nowhere if that is also unset). + +6. **`rss_cache` migration creates naive `TIMESTAMP` columns without timezone** (`migrations/005_guild_and_rss_schema.sql:36`, plus `001`, `003`, `004`, `008`, `010`, `013`). Most older migrations use `TIMESTAMP DEFAULT NOW()` (no timezone) while newer ones (`014`, `015`, `016`) correctly use `TIMESTAMPTZ`. The bot's Python code mixes `datetime.utcnow()` (naive) and `datetime.now(UTC)` (aware). Comparing aware datetimes fetched from TIMESTAMPTZ columns against naive timestamps from TIMESTAMP columns will raise `TypeError` in asyncpg comparisons. Fix: add a migration to `ALTER TABLE ... ALTER COLUMN ... TYPE TIMESTAMPTZ USING ... AT TIME ZONE 'UTC'` for the affected columns. + +### MEDIUM + +7. **Radio `_started_at` uses `datetime.utcnow()` (naive) then computes `_get_uptime()` with `datetime.utcnow()` subtraction** (`radio.py:140,234`). This is safe as long as both calls use naive UTC, but mixing with any aware datetime in the future will break silently. Additionally, `_started_at` is not reset to `None` after disconnection (`_disconnect()` does not clear it), so `/uptime` will show stale uptime after a radio disconnect/reconnect. + +8. **`feeds.py` sends subsequent feed embeds to `interaction.channel` instead of using followup** (`src/cogs/resources/feeds.py:124`). For the first entry, it uses `interaction.followup.send()`, but subsequent entries use `interaction.channel.send()`. If the channel is None (DMs, certain thread types) this will raise `AttributeError`. It also bypasses ephemeral state — the first embed may be ephemeral but subsequent ones are always public. + +9. **`safe_background_task` decorator only fires the alert on exactly 3 consecutive failures** (`src/utils/safety.py:261`). The alert fires when `count == 3`, not `count >= 3`, meaning if failures continue beyond 3 they will be logged but no additional alerts are sent. Once a cooldown expires (30 min), a repeat alert won't fire either because the counter is cumulative (not reset between cooldown windows). Fix: change `count == 3` to `count >= 3`. + +10. **Honeypot in-memory cache (`_honeypot_cache`) is never invalidated on bot reconnect** (`src/cogs/admin/honeypot.py:32-45`). The cache is loaded once via `_load_cache()` with `_cache_loaded` as a guard flag. If the DB is temporarily unavailable during initial load, `_cache_loaded` remains `False` and will retry — good. However, after a successful load, if a honeypot is added/modified by a different cog reload or external DB change, the cache goes stale indefinitely (only `_invalidate_cache()` on explicit cog commands clears it). This is acceptable for normal usage but should be noted. + +### LOW + +11. **`_update_job_progress` in `massunban.py` uses an f-string SQL query with a whitelisted column** (`massunban.py:878-886`). Although there is a `_VALID_COUNTER_COLS` whitelist check immediately before the f-string, the `counter_col` value originates from a dict literal keyed by `result['status']` (not user input), making SQL injection practically impossible. However, it violates the project's own convention of "parameterized queries only". Fix: use separate `execute` calls for each counter column. + +12. **`runtime_state.startup_time` is set at module import time**, not at `on_ready`. If the bot takes a long time to connect, `/uptime` and status embeds will show inflated uptime. Consider setting `startup_time` inside `on_ready` for accuracy. + +13. **`migrate rss_cache` has two `005_` numbered migrations with different schemas**. If a new developer or CI runs migrations on a clean DB, the sort order between `005_community_additions.sql` and `005_guild_and_rss_schema.sql` is not guaranteed and could create an inconsistent schema state. The `DROP TABLE IF EXISTS` guard in the latter file helps, but the situation is fragile. Document this prominently or renumber the files. + +### CRITICAL (Second Pass — 2026-07-21) + +14. **Audit log time-based filtering is completely broken** (`src/utils/security/audit.py:113,128`). Both `get_recent` and `get_user_actions` build SQL like `INTERVAL '$2 hours'` where the `$2` is **inside a SQL string literal** — asyncpg will not substitute it. PostgreSQL receives the literal string `$2 hours` and either errors or returns wrong results. Fix: use `INTERVAL '1 hour' * $N` pattern or build the interval string in Python. + +15. **Rate limiter decorator never actually consumes a token** (`src/utils/security/rate_limiter.py:162-192`). The `rate_limit` decorator calls `is_rate_limited()` (read-only check) instead of `check()` (consume + check). This means the rate limiter as applied to commands never enforces limits — a user can exceed the limit on every request as long as no other code path calls `check()`. Fix: replace `is_rate_limited()` with `check()` in the decorator. + +16. **`on_command_error` calls `ctx.send()` without `safe_send()`** (`src/core/app.py:198-234`). If the channel is a DM or the user has DMs disabled, `ctx.send()` raises `nextcord.Forbidden`, which would crash the error handler itself — a secondary unhandled exception in the error path. Fix: wrap with `safe_send()`. + +17. **`assert DISCORD_TOKEN is not None` is stripped in `-O` mode** (`src/core/app.py:314`). If Python runs with optimizations (`python -O main.py`), the assert is removed and `bot.run(None)` is called, producing a confusing error. Fix: use an explicit `if` check with `raise SystemExit`. + +18. **`get_user`/`create_user` has a race condition** (`src/database/database.py:214-231`). `get_user` does `SELECT` then `INSERT` if not found. Between the two queries, a concurrent request for the same `discord_id` can insert first, causing a `UniqueViolationError` that is misreported as `DatabaseUnavailableError`. The `create_user` function already has `ON CONFLICT DO UPDATE` — fix `get_user` to use it directly instead of the SELECT-then-INSERT pattern. + +19. **Tool config targets wrong Python version** (`pyproject.toml`). `requires-python = ">=3.13"` but `[tool.ruff] target-version = "py312"` and `[tool.mypy] python_version = "3.12"`. Ruff won't flag 3.13-specific issues, and mypy checks against 3.12 semantics. Fix: set both to `py313` / `"3.13"`. + +20. **`asyncio.create_task()` without storing the reference in `rss_service.py`** (`src/services/rss_service.py:65-71,84-92`). Fire-and-forget tasks can be garbage-collected before they complete. The Python docs warn: "Save a reference to the result of this function, to avoid a task disappearing mid-execution." Fix: store task references in a set and remove on completion. + +### HIGH (Second Pass — 2026-07-21) + +21. **Rate limiter `buckets` dict grows unboundedly** (`src/utils/security/rate_limiter.py`). There is no eviction/TTL mechanism. Every unique `user_id:command` combination creates an entry that is never removed. Over weeks, this is a memory leak. Fix: add periodic cleanup or LRU eviction with a TTL. + +22. **`AdminNotifier._get_channel` caches the channel reference forever** (`src/services/admin_notifier.py:16-30`). Once `self._channel` is set (including to `None` on failure), it is never refreshed. If the channel ID changes or the initial fetch fails, the notifier is permanently broken until restart. Fix: add TTL or re-fetch on failure. + +23. **`feeds.py` sends subsequent embeds to `interaction.channel` instead of `followup`** (`src/cogs/resources/feeds.py:124`). The first entry uses `interaction.followup.send()`, but subsequent entries use `interaction.channel.send()`. If channel is None (DMs, threads), this raises `AttributeError`. Also bypasses ephemeral state. Fix: use `interaction.followup.send()` consistently. + +24. **`mentorship_service.get_user_stats` always reports 0 active mentorships** (`src/services/mentorship_service.py:175-186`). `get_completed_mentorships` filters by `status = 'completed'`, so the returned list never contains active entries. The loop counting `status == 'active'` will always yield 0. Fix: query for active mentorships separately or remove the filter. + +25. **`runtime_state._load_git_metadata()` runs `subprocess.check_output` at module import time** (`src/core/runtime_state.py:9-14,20-30`). Every import of `runtime_state` spawns a `git` subprocess. If git is absent, the exception is caught but the spawn adds latency to every cold start. Fix: make lazy/cached with `@functools.lru_cache`. + +26. **`sanitize_text` uses `html.escape()` which is wrong for Discord** (`src/utils/security/validation.py:47`). Discord uses Markdown, not HTML. HTML-escaping produces visible `&`, `<`, `>` in chat. Fix: use Discord-specific escaping (backslash-escape special chars) or simply strip dangerous content. + +27. **RBAC `require_role` and `require_permission` decorators don't work with slash commands** (`src/utils/security/rbac.py:197`). They extract `ctx` from `args[1]` and call `ctx.send()`. For slash commands, `args[1]` is an `Interaction` which has a different API (`interaction.response.send_message`). Fix: detect the context type and use the appropriate API, or split into separate decorators. + +28. **`setup_logging()` ignores the `LOG_LEVEL` env var** (`src/utils/logger.py:35`). The function defaults to `'INFO'` and is called from `app.py` with no arguments. The `LOG_LEVEL` config variable is never read. Fix: read `LOG_LEVEL` from config and pass it. + +### MEDIUM (Second Pass — 2026-07-21) + +29. **Directus `_rehost_image` downloads entire images into memory** (`src/services/directus_sync.py:52`). No size limit on the download. Large images could cause memory pressure. Fix: add a `Content-Length` check or stream to a temp file. + +30. **`guild_gate.py` uses `asyncio.ensure_future()` inside a synchronous check predicate** (`src/utils/guild_gate.py`). The `commands.check` predicate is synchronous but fires coroutines via `ensure_future`. These responses may fail silently. Fix: use `commands.check()` with an async predicate (nextcord supports this). + +31. **No log rotation** (`src/utils/logger.py`). `bot.log` grows unboundedly via a plain `FileHandler`. Fix: use `RotatingFileHandler` or `TimedRotatingFileHandler`. + +32. **`.env.example` has `POSTGRES_USER=veka_user` but `docker-compose.dev.yml` uses `POSTGRES_USER=veka_bot_user`**. Deploying with the example file will create a user mismatch. Fix: align the two files. + +33. **`MASSUNBAN_LOG_CHANNEL_ID` missing from `.env.example`** (already in existing issue #5, also affects `.env.example` consistency). + +34. **`alert_state_cache` mixes heterogeneous data without type safety** (`src/core/runtime_state.py`). At least 5 subsystems write to it with different key patterns (healthy_count, failure counts, dedup timestamps, RSS counters). A wrong key could interfere. Fix: use dedicated typed fields or separate caches per subsystem. + +35. **`.github/workflows/deploy-discord-bot.yml` is documented in AGENTS.md but does not exist in the repository**. Either it was removed, renamed, or the documentation is stale. + +### LOW (Second Pass — 2026-07-21) + +36. **Massive duplicated error-handling logic in `app.py`** (`src/core/app.py:198-286`). `on_command_error` and `on_application_command_error` are near-identical `isinstance` chains. Fix: extract into a shared helper. + +37. **`bot.runtime_state` and `bot.notifier` are monkey-patched with `# type: ignore`** (`src/core/app.py`). Bypasses type checking. Fix: use a typed `Bot` subclass with properly typed attributes. + +38. **`error_embed` uses orange color instead of red** (`src/utils/embeds.py:111-115`). It calls `veka_embed` which defaults to `ORANGE`. Fix: default to `nextcord.Color.red()`. + +39. **`admin_only()`/`staff_only()` in `safety.py` bypass the RBAC system** (`src/utils/safety.py`). Two parallel auth systems (ID-list checks vs. RBAC role hierarchy) can return different results. Fix: consolidate to use RBAC exclusively. + +40. **`runtime_state.last_db_error` is never cleared** (`src/core/runtime_state.py`). Once set, stale error text persists for the process lifetime. Fix: clear on successful recovery. + +41. **`rate_limit` decorator only handles prefix commands** (`src/utils/security/rate_limiter.py:162-192`). Extracts `ctx` from `args[1]` which assumes cog method signature `(self, ctx, ...)`. Does not handle slash commands at all. Fix: add a slash command variant or detect context type. + +42. **`rss_service.py` `datetime.strptime` with fixed format fails on many RSS date formats** (`src/services/rss_service.py:142`). The format `'%a, %d %b %Y %H:%M:%S %z'` only matches RFC 2822. ISO 8601 and other formats silently fail, leaving entries unsorted. Fix: use `email.utils.parsedate_to_datetime` or `dateutil.parser`. + +43. **Missing `src/utils/marketplace/__init__.py`**. The `fraud_detection.py` module may not be importable as a package depending on how Python resolves it. Fix: add the `__init__.py`. + +44. **Duplicated unique constraint on `profiles.user_id`** across migrations `005_community_additions` (`profiles_user_id_unique`) and `006_profiles_and_requests` (`unique_profile_user_id`). Two constraints on the same column. Harmless but wasteful. + +45. **`_is_staff_user` in `safety.py` creates a `SimpleNamespace` to fake a context** (`src/utils/safety.py:103-106`). Fragile coupling — if `rbac.get_user_role` changes its context expectations, this breaks silently. + diff --git a/pyproject.toml b/pyproject.toml index a2471e3..879e120 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,7 @@ dev = [ ] [tool.ruff] -target-version = "py312" +target-version = "py313" line-length = 120 [tool.ruff.lint] @@ -36,7 +36,7 @@ quote-style = "single" docstring-code-format = true [tool.mypy] -python_version = "3.12" +python_version = "3.13" ignore_missing_imports = true check_untyped_defs = true warn_redundant_casts = true diff --git a/src/cogs/admin/massunban.py b/src/cogs/admin/massunban.py index bc863ea..0dcf1a8 100644 --- a/src/cogs/admin/massunban.py +++ b/src/cogs/admin/massunban.py @@ -195,7 +195,7 @@ def __init__(self, bot: commands.Bot) -> None: async def cog_load(self) -> None: """Resume incomplete jobs on startup.""" - self.bot.loop.create_task(self._resume_incomplete_jobs()) + asyncio.create_task(self._resume_incomplete_jobs()) async def _resume_incomplete_jobs(self) -> None: """Wait for bot to be ready, then scan for resumable jobs.""" @@ -218,7 +218,7 @@ async def _resume_incomplete_jobs(self) -> None: job['id'], ) self._active_jobs[job['id']] = False - self.bot.loop.create_task(self._execute_job(job['id'], resumed=True)) + asyncio.create_task(self._execute_job(job['id'], resumed=True)) except Exception: logger.error('Failed to resume job %s', job['id'], exc_info=True) @@ -940,7 +940,7 @@ async def _handle_rate_limit_pause(self, job_id: int, result: dict) -> None: logger.warning('Failed to send rate limit notification for job %s', job_id, exc_info=True) # Schedule resume using asyncio task (not call_later) - self.bot.loop.create_task(self._scheduled_resume(job_id, retry_after_secs)) + asyncio.create_task(self._scheduled_resume(job_id, retry_after_secs)) async def _scheduled_resume(self, job_id: int, delay: float) -> None: """Wait then resume a rate-limited job, checking for cancellation first.""" diff --git a/src/cogs/marketplace/marketplace.py b/src/cogs/marketplace/marketplace.py index f6e0963..f69e95d 100644 --- a/src/cogs/marketplace/marketplace.py +++ b/src/cogs/marketplace/marketplace.py @@ -1,5 +1,6 @@ import datetime import logging +from datetime import UTC import nextcord from nextcord.ext import commands @@ -94,7 +95,7 @@ async def mp_post( category_id = category_record['id'] - listing_id = f'MP{int(datetime.datetime.utcnow().timestamp())}' + listing_id = f'MP{int(datetime.datetime.now(UTC).timestamp())}' # Ensure user exists in db await db.execute('INSERT INTO users (discord_id) VALUES ($1) ON CONFLICT DO NOTHING', str(interaction.user.id)) @@ -156,7 +157,7 @@ async def mp_post( 'category': category, 'status': 'active', 'image_url': image_url or None, - 'listing_created_at': datetime.datetime.utcnow().isoformat(), + 'listing_created_at': datetime.datetime.now(UTC).isoformat(), } ) diff --git a/src/cogs/marketplace/reviews.py b/src/cogs/marketplace/reviews.py index 764d860..44357e1 100644 --- a/src/cogs/marketplace/reviews.py +++ b/src/cogs/marketplace/reviews.py @@ -335,10 +335,11 @@ async def bump_slash(self, interaction: nextcord.Interaction, listing_id: str): await safe_send(interaction, embed=embed, ephemeral=True) return - from datetime import datetime, timedelta + from datetime import UTC, datetime, timedelta - if listing['bumped_at'] and listing['bumped_at'] > datetime.utcnow() - timedelta(hours=24): - time_left = listing['bumped_at'] + timedelta(hours=24) - datetime.utcnow() + now = datetime.now(UTC) + if listing['bumped_at'] and listing['bumped_at'] > now - timedelta(hours=24): + time_left = listing['bumped_at'] + timedelta(hours=24) - now hours = int(time_left.total_seconds() // 3600) embed = await error_embed( 'Too Soon', diff --git a/src/cogs/marketplace_enhanced.py b/src/cogs/marketplace_enhanced.py index 57499f7..c2b2266 100644 --- a/src/cogs/marketplace_enhanced.py +++ b/src/cogs/marketplace_enhanced.py @@ -4,7 +4,7 @@ """ import logging -from datetime import datetime, timedelta +from datetime import UTC, datetime, timedelta from decimal import Decimal import nextcord @@ -240,7 +240,7 @@ async def offer_slash(self, interaction: nextcord.Interaction, listing_id: str, await safe_send(interaction, embed=embed, ephemeral=True) return - expires = datetime.utcnow() + timedelta(days=3) + expires = datetime.now(UTC) + timedelta(days=3) await db.execute( """INSERT INTO marketplace_offers (listing_id, buyer_id, offered_price, message, expires_at) diff --git a/src/cogs/portfolio/portfolio_manager.py b/src/cogs/portfolio/portfolio_manager.py index 82a682d..d8b3184 100644 --- a/src/cogs/portfolio/portfolio_manager.py +++ b/src/cogs/portfolio/portfolio_manager.py @@ -1,5 +1,5 @@ import logging -from datetime import datetime +from datetime import UTC, datetime import nextcord import validators @@ -48,7 +48,7 @@ async def portfolio_add_slash( tag_list = [t.strip() for t in tags.split(',') if t.strip()] if tags else [] - project_id = f'proj-{int(datetime.utcnow().timestamp())}' + project_id = f'proj-{int(datetime.now(UTC).timestamp())}' user = await get_or_create_user(str(interaction.user.id)) await db.execute( @@ -269,7 +269,7 @@ def check(m): tags_raw = (await self.bot.wait_for('message', check=check, timeout=60)).content tag_list = [t.strip() for t in tags_raw.split(',') if t.strip()] - project_id = f'proj-{int(datetime.utcnow().timestamp())}' + project_id = f'proj-{int(datetime.now(UTC).timestamp())}' user = await get_or_create_user(str(ctx.author.id)) await db.execute( diff --git a/src/cogs/radio/radio.py b/src/cogs/radio/radio.py index bd2097f..a4168cf 100644 --- a/src/cogs/radio/radio.py +++ b/src/cogs/radio/radio.py @@ -8,7 +8,7 @@ import asyncio import logging import time -from datetime import datetime +from datetime import UTC, datetime import nextcord from nextcord.ext import commands, tasks @@ -137,7 +137,7 @@ async def _auto_join(self): self._voice_client = await channel.connect(self_deaf=True) # type: ignore[call-arg] self._play_stream() - self._started_at = datetime.utcnow() + self._started_at = datetime.now(UTC) self._auto_started = True self._manual_stop = False logger.info('Radio started in channel %s', channel.name) @@ -231,7 +231,7 @@ def _get_uptime(self) -> str: """Format uptime since radio started.""" if not self._started_at: return 'Not started' - delta = datetime.utcnow() - self._started_at + delta = datetime.now(UTC) - self._started_at hours, remainder = divmod(int(delta.total_seconds()), 3600) minutes, seconds = divmod(remainder, 60) if hours: diff --git a/src/cogs/resources/feeds.py b/src/cogs/resources/feeds.py index 7b5a4c2..51dec78 100644 --- a/src/cogs/resources/feeds.py +++ b/src/cogs/resources/feeds.py @@ -116,12 +116,9 @@ async def feed_latest( entries = entries[:5] - for idx, entry in enumerate(entries): + for _idx, entry in enumerate(entries): embed = await self.create_feed_embed(entry, category) - if idx == 0: - await interaction.followup.send(embed=embed) - else: - await interaction.channel.send(embed=embed) + await interaction.followup.send(embed=embed) async def create_feed_embed(self, entry: dict, category: str) -> nextcord.Embed: embed = await info_embed( diff --git a/src/core/app.py b/src/core/app.py index 2c7c3d2..b7e6b29 100644 --- a/src/core/app.py +++ b/src/core/app.py @@ -1,4 +1,5 @@ import asyncio +import logging from datetime import UTC, datetime import aiohttp @@ -57,6 +58,7 @@ def get_intents() -> nextcord.Intents: def build_bot() -> commands.Bot: bot = commands.Bot(command_prefix=BOT_PREFIX, intents=get_intents(), help_command=None) bot.runtime_state = runtime_state # type: ignore[attr-defined] + bot.notifier = None # type: ignore[attr-defined] return bot @@ -112,6 +114,7 @@ async def db_health_check(): if healthy_count >= CONSECUTIVE_HEALTHY_REQUIRED: runtime_state.db_available = True + runtime_state.last_db_error = None runtime_state.last_recovery_time = datetime.now(UTC) runtime_state.alert_state_cache.pop('healthy_count', None) if hasattr(bot, 'notifier'): @@ -188,94 +191,52 @@ async def on_ready(): @bot.event async def on_disconnect(): - try: - await db.close() - logger.info('Database connection closed on disconnect') - except Exception as exc: - logger.error(f'Error closing database on disconnect: {exc}') - - @bot.event - async def on_command_error(ctx, error): - original_error = error.original if isinstance(error, commands.CommandInvokeError) else error + logger.warning('Gateway disconnected (DB pool left open for health-check recovery)') + def _handle_command_error_common(original_error: Exception) -> str | None: + """Shared error classification for both prefix and slash commands. Returns user message or None.""" if isinstance(original_error, commands.CommandNotFound): - await ctx.send('Command not found. Use /help or !help to see available commands.') - return - + return 'Command not found. Use /help or !help to see available commands.' if isinstance(original_error, commands.MissingPermissions | commands.MissingRole | commands.NotOwner): - await ctx.send('You do not have permission to use this command.') - return - + return 'You do not have permission to use this command.' if isinstance(original_error, commands.CommandOnCooldown): - await ctx.send(f'This command is on cooldown. Try again in {original_error.retry_after:.1f}s.') - return - + return f'This command is on cooldown. Try again in {original_error.retry_after:.1f}s.' if isinstance(original_error, DatabaseUnavailableError): - await ctx.send( + return ( 'This feature is temporarily unavailable due to database connectivity issues. Please try again later.' ) - logger.warning('Database unavailable during command: %s | %s', original_error, format_context(ctx)) - return - if isinstance( original_error, commands.BadArgument | commands.MissingRequiredArgument | commands.UserInputError | ValidationError, ): - await ctx.send('Invalid command input. Please check your arguments and try again.') - logger.warning('Validation error: %s | %s', original_error, format_context(ctx)) - return - + return 'Invalid command input. Please check your arguments and try again.' if isinstance(original_error, asyncio.TimeoutError | aiohttp.ClientError | ExternalRequestError): - await ctx.send('A network or external service error occurred. Please try again later.') - logger.warning('External request error: %s | %s', original_error, format_context(ctx)) - return - - logger.error('Unhandled command error: %s | %s', original_error, format_context(ctx), exc_info=True) - await ctx.send('An internal error occurred while processing your command.') + return 'A network or external service error occurred. Please try again later.' + return None @bot.event - async def on_application_command_error(interaction, error): + async def on_command_error(ctx, error): original_error = error.original if isinstance(error, commands.CommandInvokeError) else error - if isinstance(original_error, commands.CommandOnCooldown): - await safe_send( - interaction, - content=f'This command is on cooldown. Try again in {original_error.retry_after:.1f}s.', - ephemeral=True, - ) - return - if isinstance(original_error, commands.MissingPermissions | commands.MissingRole | commands.NotOwner): - await safe_send(interaction, content='You do not have permission to use this command.', ephemeral=True) + user_msg = _handle_command_error_common(original_error) + if user_msg: + await safe_send(ctx, content=user_msg) + log_level = logging.WARNING if not isinstance(original_error, commands.CommandNotFound) else logging.DEBUG + logger.log(log_level, '%s | %s', type(original_error).__name__, format_context(ctx)) return - if isinstance(original_error, DatabaseUnavailableError): - await safe_send( - interaction, - content='This feature is temporarily unavailable due to database connectivity issues. Please try again later.', - ephemeral=True, - ) - logger.warning( - 'Database unavailable during slash command: %s | %s', original_error, format_context(interaction) - ) - return + logger.error('Unhandled command error: %s | %s', original_error, format_context(ctx), exc_info=True) + await safe_send(ctx, content='An internal error occurred while processing your command.') - if isinstance( - original_error, - commands.BadArgument | commands.MissingRequiredArgument | commands.UserInputError | ValidationError, - ): - await safe_send( - interaction, content='Invalid command input. Please check your arguments and try again.', ephemeral=True - ) - logger.warning('Validation error: %s | %s', original_error, format_context(interaction)) - return + @bot.event + async def on_application_command_error(interaction, error): + original_error = error.original if isinstance(error, commands.CommandInvokeError) else error - if isinstance(original_error, asyncio.TimeoutError | aiohttp.ClientError | ExternalRequestError): - await safe_send( - interaction, - content='A network or external service error occurred. Please try again later.', - ephemeral=True, - ) - logger.warning('External request error: %s | %s', original_error, format_context(interaction)) + user_msg = _handle_command_error_common(original_error) + if user_msg: + await safe_send(interaction, content=user_msg, ephemeral=True) + log_level = logging.WARNING if not isinstance(original_error, commands.CommandNotFound) else logging.DEBUG + logger.log(log_level, '%s | %s', type(original_error).__name__, format_context(interaction)) return logger.error( @@ -311,7 +272,8 @@ def run_bot() -> None: try: load_extensions(bot, EXTENSIONS) - assert DISCORD_TOKEN is not None + if DISCORD_TOKEN is None: + raise SystemExit('DISCORD_TOKEN is not set. Check your .env file.') bot.run(DISCORD_TOKEN) except Exception as exc: logger.error(f'Failed to start bot: {exc}') diff --git a/src/core/runtime_state.py b/src/core/runtime_state.py index 2137216..a9d8168 100644 --- a/src/core/runtime_state.py +++ b/src/core/runtime_state.py @@ -1,9 +1,11 @@ +import functools import os import subprocess from dataclasses import dataclass, field from datetime import UTC, datetime +@functools.lru_cache(maxsize=1) def _load_git_metadata() -> str: try: commit = subprocess.check_output( @@ -17,6 +19,7 @@ def _load_git_metadata() -> str: return os.getenv('COMMIT_SHA', 'unknown') +@functools.lru_cache(maxsize=1) def _load_git_branch() -> str: try: branch = subprocess.check_output( @@ -47,5 +50,9 @@ class RuntimeState: last_recovery_time: datetime | None = None alert_state_cache: dict = field(default_factory=dict) + def clear_db_error(self) -> None: + """Clear stale DB error text after successful recovery.""" + self.last_db_error = None + runtime_state = RuntimeState() diff --git a/src/database/database.py b/src/database/database.py index 792790d..fed1942 100644 --- a/src/database/database.py +++ b/src/database/database.py @@ -212,10 +212,6 @@ async def run_migrations(self) -> None: async def get_user(discord_id: str): - user = await db.fetch_one('SELECT * FROM users WHERE discord_id = $1', discord_id) - if user: - return user - return await create_user(discord_id) diff --git a/src/services/admin_notifier.py b/src/services/admin_notifier.py index b4bbc1e..f218395 100644 --- a/src/services/admin_notifier.py +++ b/src/services/admin_notifier.py @@ -1,4 +1,4 @@ -from datetime import datetime, timedelta +from datetime import UTC, datetime, timedelta import nextcord @@ -9,31 +9,45 @@ logger = get_logger('VEKA.admin_notifier') +_CHANNEL_CACHE_TTL = timedelta(minutes=5) + class AdminNotifier: def __init__(self, bot: nextcord.Client): self.bot = bot - self._channel = None + self._channel: nextcord.abc.Messageable | None = None + self._channel_fetched_at: datetime | None = None async def _get_channel(self): if not ADMIN_ALERT_CHANNEL_ID: return None - if self._channel is None: - self._channel = self.bot.get_channel(ADMIN_ALERT_CHANNEL_ID) # type: ignore[assignment] - if self._channel is None: - try: - self._channel = await self.bot.fetch_channel(ADMIN_ALERT_CHANNEL_ID) # type: ignore[assignment] - except Exception as e: - logger.warning('Failed to fetch admin alert channel: %s', e) - + now = datetime.now(UTC) + if self._channel is not None and self._channel_fetched_at is not None: + if now - self._channel_fetched_at < _CHANNEL_CACHE_TTL: + return self._channel + # TTL expired — reset so we re-fetch below + self._channel = None + + channel = self.bot.get_channel(ADMIN_ALERT_CHANNEL_ID) # type: ignore[arg-type] + if channel is None: + try: + channel = await self.bot.fetch_channel(ADMIN_ALERT_CHANNEL_ID) # type: ignore[arg-type] + except Exception as e: + logger.warning('Failed to fetch admin alert channel: %s', e) + self._channel = None + self._channel_fetched_at = now + return None + + self._channel = channel + self._channel_fetched_at = now return self._channel def _should_alert(self, dedupe_key: str | None, cooldown_minutes: int) -> bool: if not dedupe_key: return True - now = datetime.utcnow() + now = datetime.now(UTC) cache = runtime_state.alert_state_cache last_alert_time = cache.get(dedupe_key) @@ -58,7 +72,8 @@ async def send_alert( return embed = await alert_embed(title=title, description=description, severity=severity, contributor_source=__name__) - assert self._channel is not None # guaranteed by _get_channel check above + if self._channel is None: + return try: await self._channel.send(embed=embed) logger.info('Sent admin alert: [%s] %s', severity, title) diff --git a/src/services/directus_sync.py b/src/services/directus_sync.py index 99b0506..5f49dff 100644 --- a/src/services/directus_sync.py +++ b/src/services/directus_sync.py @@ -54,6 +54,12 @@ async def _rehost_image(session: aiohttp.ClientSession, url: str) -> str | None: Directus files so it stays available after the source link expires. Returns a persistent asset URL, or None if the source can't be fetched (dead link).""" try: + async with session.head(url) as head_resp: + content_length = head_resp.headers.get('Content-Length') + if content_length and int(content_length) > 8 * 1024 * 1024: # 8 MB limit + logger.warning('Image too large (%s bytes), skipping re-host: %s', content_length, url) + return None + async with session.get(url) as resp: if resp.status != 200: return None diff --git a/src/services/mentorship_service.py b/src/services/mentorship_service.py index 3740d73..b77773f 100644 --- a/src/services/mentorship_service.py +++ b/src/services/mentorship_service.py @@ -166,26 +166,29 @@ async def get_mentorship_stats(self) -> dict: async def get_user_stats(self, user_id: str) -> dict: resolved_id = await self._resolve_user_id(user_id) - mentorships_as_mentor = await self.get_completed_mentorships(user_id, as_mentor=True) - mentorships_as_mentee = await self.get_completed_mentorships(user_id, as_mentor=False) + + all_as_mentor = await self.get_user_mentorships(user_id) + all_as_mentee_rows = await db.fetch( + """ + SELECT * FROM mentorships WHERE mentee_id = $1 + """, + resolved_id, + ) + + mentor_active = [m for m in all_as_mentor if m['mentor_id'] == resolved_id and m['status'] == 'active'] + mentor_completed = [m for m in all_as_mentor if m['mentor_id'] == resolved_id and m['status'] == 'completed'] + mentee_active = [m for m in all_as_mentee_rows if m['status'] == 'active'] + mentee_completed = [m for m in all_as_mentee_rows if m['status'] == 'completed'] return { 'as_mentor': { - 'total': len([m for m in mentorships_as_mentor if m['mentor_id'] == resolved_id]), - 'active': len( - [m for m in mentorships_as_mentor if m['mentor_id'] == resolved_id and m['status'] == 'active'] - ), - 'completed': len( - [m for m in mentorships_as_mentor if m['mentor_id'] == resolved_id and m['status'] == 'completed'] - ), + 'total': len(mentor_active) + len(mentor_completed), + 'active': len(mentor_active), + 'completed': len(mentor_completed), }, 'as_mentee': { - 'total': len([m for m in mentorships_as_mentee if m['mentee_id'] == resolved_id]), - 'active': len( - [m for m in mentorships_as_mentee if m['mentee_id'] == resolved_id and m['status'] == 'active'] - ), - 'completed': len( - [m for m in mentorships_as_mentee if m['mentee_id'] == resolved_id and m['status'] == 'completed'] - ), + 'total': len(mentee_active) + len(mentee_completed), + 'active': len(mentee_active), + 'completed': len(mentee_completed), }, } diff --git a/src/services/rss_service.py b/src/services/rss_service.py index 75a5af5..3bea32c 100644 --- a/src/services/rss_service.py +++ b/src/services/rss_service.py @@ -13,6 +13,8 @@ logger = logging.getLogger('VEKA.rss') +_background_tasks: set[asyncio.Task] = set() + class RSSService: def __init__(self, bot=None): @@ -62,13 +64,15 @@ async def fetch_feed(self, url: str) -> dict | None: runtime_state.alert_state_cache[fail_key] = 0 if self.bot and hasattr(self.bot, 'notifier'): self.bot.notifier.clear_cooldown(f'rss_alert_{url}') - asyncio.create_task( + task = asyncio.create_task( self.bot.notifier.send_alert( title='RSS Feed Recovered', description=f'The RSS feed `{url}` is now responding correctly.', severity='INFO', ) ) + _background_tasks.add(task) + task.add_done_callback(_background_tasks.discard) return feed_data @@ -81,7 +85,7 @@ async def fetch_feed(self, url: str) -> dict | None: runtime_state.alert_state_cache[fail_key] = fails if fails >= 3 and self.bot and hasattr(self.bot, 'notifier'): - asyncio.create_task( + task = asyncio.create_task( self.bot.notifier.send_alert( title='RSS Feed Failing', description=f'The RSS feed `{url}` has failed {fails} consecutive times.\n**Error:** {str(exc)[:500]}', @@ -90,6 +94,8 @@ async def fetch_feed(self, url: str) -> dict | None: cooldown_minutes=120, ) ) + _background_tasks.add(task) + task.add_done_callback(_background_tasks.discard) return None async def process_and_dedupe(self, url: str, entries: list[dict]) -> list[dict]: @@ -139,7 +145,7 @@ async def get_latest_new_entries(self, category: str, limit: int = 5) -> list[di try: all_new_entries.sort( - key=lambda x: datetime.strptime(x['published'], '%a, %d %b %Y %H:%M:%S %z'), + key=lambda x: self._parse_entry_date(x['published']), reverse=True, ) except Exception: @@ -149,3 +155,21 @@ async def get_latest_new_entries(self, category: str, limit: int = 5) -> list[di def get_available_categories(self) -> list[str]: return list(RSS_FEEDS.keys()) + + @staticmethod + def _parse_entry_date(date_str: str) -> datetime: + """Parse an RSS entry date string, supporting RFC 2822 and ISO 8601 formats.""" + from email.utils import parsedate_to_datetime + + # Try RFC 2822 first (most RSS feeds) + try: + return parsedate_to_datetime(date_str) + except Exception: + pass + # Try ISO 8601 + try: + return datetime.fromisoformat(date_str.replace('Z', '+00:00')) + except Exception: + pass + # Fallback: return epoch so sorting still works (oldest first) + return datetime.min.replace(tzinfo=None) diff --git a/src/utils/embeds.py b/src/utils/embeds.py index 0448bec..0656d6f 100644 --- a/src/utils/embeds.py +++ b/src/utils/embeds.py @@ -1,7 +1,7 @@ import functools import logging import subprocess -from datetime import datetime +from datetime import UTC, datetime from pathlib import Path from typing import Any @@ -95,7 +95,7 @@ async def veka_embed( if footer_text: embed.set_footer(text=footer_text) if timestamp: - embed.timestamp = datetime.utcnow() + embed.timestamp = datetime.now(UTC) return embed @@ -112,6 +112,7 @@ async def error_embed( description: str | None = None, **kwargs: Any, ) -> nextcord.Embed: + kwargs.setdefault('color', nextcord.Color.red()) return await veka_embed(title=title, description=description, **kwargs) diff --git a/src/utils/guild_gate.py b/src/utils/guild_gate.py index bc7b893..a13878f 100644 --- a/src/utils/guild_gate.py +++ b/src/utils/guild_gate.py @@ -21,7 +21,7 @@ def is_main_guild(guild: nextcord.Guild | None) -> bool: def main_server_only(): """Decorator: command only works in the main guild. Others see 'not available'.""" - def predicate(ctx_or_interaction): + async def predicate(ctx_or_interaction) -> bool: if isinstance(ctx_or_interaction, commands.Context): guild = ctx_or_interaction.guild elif isinstance(ctx_or_interaction, nextcord.Interaction): @@ -36,15 +36,10 @@ def predicate(ctx_or_interaction): msg = 'This command is only available in the main VEKA server.' try: if isinstance(ctx_or_interaction, commands.Context): - import asyncio - - if asyncio.get_running_loop().is_running(): - asyncio.ensure_future(ctx_or_interaction.send(msg)) + await ctx_or_interaction.send(msg) elif isinstance(ctx_or_interaction, nextcord.Interaction): if not ctx_or_interaction.response.is_done(): - import asyncio - - asyncio.ensure_future(ctx_or_interaction.response.send_message(msg, ephemeral=True)) + await ctx_or_interaction.response.send_message(msg, ephemeral=True) except Exception: pass return False @@ -59,7 +54,7 @@ def owner_in_external_only(): Others see 'not allowed'. """ - def predicate(ctx_or_interaction): + async def predicate(ctx_or_interaction) -> bool: guild: nextcord.Guild | None = None user: nextcord.User | nextcord.Member | None = None if isinstance(ctx_or_interaction, commands.Context): @@ -85,15 +80,10 @@ def predicate(ctx_or_interaction): msg = 'This command is not available in this server.' try: if isinstance(ctx_or_interaction, commands.Context): - import asyncio - - if asyncio.get_running_loop().is_running(): - asyncio.ensure_future(ctx_or_interaction.send(msg)) + await ctx_or_interaction.send(msg) elif isinstance(ctx_or_interaction, nextcord.Interaction): if not ctx_or_interaction.response.is_done(): - import asyncio - - asyncio.ensure_future(ctx_or_interaction.response.send_message(msg, ephemeral=True)) + await ctx_or_interaction.response.send_message(msg, ephemeral=True) except Exception: pass return False diff --git a/src/utils/logger.py b/src/utils/logger.py index 46e657d..94dbe01 100644 --- a/src/utils/logger.py +++ b/src/utils/logger.py @@ -2,6 +2,7 @@ import os import sys from collections.abc import MutableMapping +from logging.handlers import RotatingFileHandler from typing import Any @@ -32,11 +33,15 @@ def process(self, msg: str, kwargs: MutableMapping[str, Any]) -> tuple[str, Muta return msg, kwargs -def setup_logging(log_level: str = 'INFO') -> None: +def setup_logging(log_level: str | None = None) -> None: """Sets up global logging configuration. Call this once at startup.""" + if log_level is None: + from src.config.config import LOG_LEVEL + + log_level = LOG_LEVEL or 'INFO' + # Ensure logs directory exists if file logging is still desired - if not os.path.exists('logs'): - os.makedirs('logs', exist_ok=True) + os.makedirs('logs', exist_ok=True) level = getattr(logging, log_level.upper(), logging.INFO) @@ -56,8 +61,8 @@ def setup_logging(log_level: str = 'INFO') -> None: console_handler.setFormatter(formatter) root_logger.addHandler(console_handler) - # File handler (fallback for local dev) - file_handler = logging.FileHandler('logs/bot.log', encoding='utf-8') + # File handler (fallback for local dev) — rotate at 5 MB, keep 3 backups + file_handler = RotatingFileHandler('logs/bot.log', encoding='utf-8', maxBytes=5 * 1024 * 1024, backupCount=3) file_handler.setFormatter(formatter) root_logger.addHandler(file_handler) diff --git a/src/utils/marketplace/__init__.py b/src/utils/marketplace/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/utils/safety.py b/src/utils/safety.py index 3e73946..1097921 100644 --- a/src/utils/safety.py +++ b/src/utils/safety.py @@ -258,7 +258,7 @@ async def run_safe_task(coro, name: str, logger_obj, bot=None): logger_obj.error('Task %s failed (consecutive failures: %d): %s', name, count, exc, exc_info=True) - if count == 3 and bot and hasattr(bot, 'notifier') and bot.notifier: + if count >= 3 and bot and hasattr(bot, 'notifier') and bot.notifier: await bot.notifier.send_alert( title=f'Background Task Failing: {name}', description=f'Task `{name}` has failed {count} consecutive times.\nLast error: `{exc}`', diff --git a/src/utils/security/audit.py b/src/utils/security/audit.py index 7e062a8..59cdc21 100644 --- a/src/utils/security/audit.py +++ b/src/utils/security/audit.py @@ -3,8 +3,9 @@ Tracks all important actions for security and compliance """ +import json import logging -from datetime import datetime +from datetime import UTC, datetime from functools import wraps from typing import Any @@ -53,8 +54,6 @@ async def record( return try: - import json - details_json = json.dumps(details) if details else None await self._db.execute( @@ -67,7 +66,7 @@ async def record( guild_id, channel_id, severity, - datetime.utcnow(), + datetime.now(UTC), ) # Also log to file for immediate visibility @@ -110,7 +109,7 @@ async def get_recent( if hours: param_count += 1 - query += f" AND created_at > NOW() - INTERVAL '${param_count} hours'" + query += f" AND created_at > NOW() - INTERVAL '1 hour' * ${param_count}" params.append(hours) query += ' ORDER BY created_at DESC' @@ -125,7 +124,7 @@ async def get_user_actions(self, user_id: str, hours: int = 24) -> dict[str, int """SELECT action, COUNT(*) as count FROM audit_logs WHERE user_id = $1 - AND created_at > NOW() - INTERVAL '$2 hours' + AND created_at > NOW() - INTERVAL '1 hour' * $2 GROUP BY action""", user_id, hours, diff --git a/src/utils/security/rate_limiter.py b/src/utils/security/rate_limiter.py index a8f4351..34e8604 100644 --- a/src/utils/security/rate_limiter.py +++ b/src/utils/security/rate_limiter.py @@ -7,6 +7,9 @@ import logging import time from functools import wraps +from typing import Any + +from src.utils.safety import safe_send logger = logging.getLogger('VEKA.security.rate_limiter') @@ -25,6 +28,7 @@ def __init__(self): # user_id:command -> (tokens, last_update) self.buckets: dict[str, tuple[float, float]] = {} self.lock = asyncio.Lock() + self._cleanup_task: asyncio.Task | None = None # Default limits per command type self.default_limits = { @@ -130,10 +134,10 @@ def get_remaining(self, user_id: str, command: str) -> int: """Get remaining requests for user""" key = self._get_key(user_id, command) + # Best-effort value without the lock (safe for int reads on CPython) if key not in self.buckets: max_requests, _ = self._get_limit(command) return max_requests - tokens, _ = self.buckets[key] return max(0, int(tokens)) @@ -152,6 +156,28 @@ async def reset(self, user_id: str | None = None, command: str | None = None): # Reset all self.buckets.clear() + async def cleanup_stale_buckets(self, max_age: float = 600.0): + """Remove bucket entries older than max_age seconds (default 10 min).""" + async with self.lock: + now = time.time() + stale_keys = [key for key, (_, last_update) in self.buckets.items() if now - last_update > max_age] + for key in stale_keys: + del self.buckets[key] + if stale_keys: + logger.debug('Cleaned up %d stale rate-limit buckets', len(stale_keys)) + + def start_cleanup_loop(self, interval: float = 300.0): + """Start a periodic cleanup task (call once at bot startup).""" + if self._cleanup_task is not None: + return + + async def _loop(): + while True: + await asyncio.sleep(interval) + await self.cleanup_stale_buckets() + + self._cleanup_task = asyncio.create_task(_loop()) + # Global rate limiter instance rate_limiter = RateLimiter() @@ -172,7 +198,7 @@ async def quiz(self, ctx): def decorator(func): @wraps(func) - async def wrapper(*args, **kwargs): + async def wrapper(*args: Any, **kwargs: Any): # Get context (first arg is usually self, second is ctx) ctx = args[1] if len(args) > 1 else kwargs.get('ctx') @@ -182,9 +208,12 @@ async def wrapper(*args, **kwargs): is_limited, retry_after = await rate_limiter.is_rate_limited(user_id, command_type, member=member) if is_limited: - await ctx.send(f'⏱️ Rate limited! Try again in {retry_after:.0f} seconds.') + await safe_send(ctx, f'⏱️ Rate limited! Try again in {retry_after:.0f} seconds.', ephemeral=True) return + # Consume a token now that we've passed the check + await rate_limiter.check(ctx, member=member) + return await func(*args, **kwargs) return wrapper diff --git a/src/utils/security/rbac.py b/src/utils/security/rbac.py index 407dbbc..eb2c36d 100644 --- a/src/utils/security/rbac.py +++ b/src/utils/security/rbac.py @@ -7,6 +7,8 @@ from enum import Enum from functools import wraps +import nextcord + from src.config.config import ( ACTIVE_PRO_IDS, ADMIN_IDS, @@ -200,7 +202,14 @@ async def wrapper(*args, **kwargs): user_role = self.get_user_role(ctx) if ROLE_HIERARCHY.index(user_role) < ROLE_HIERARCHY.index(min_role): - await ctx.send(f'❌ You need {min_role.value} role or higher to use this command.') + msg = f'❌ You need {min_role.value} role or higher to use this command.' + if isinstance(ctx, nextcord.Interaction): + if ctx.response.is_done(): + await ctx.followup.send(msg, ephemeral=True) + else: + await ctx.response.send_message(msg, ephemeral=True) + else: + await ctx.send(msg) return return await func(*args, **kwargs) @@ -229,7 +238,14 @@ async def wrapper(*args, **kwargs): user_role = self.get_user_role(ctx) if not self.has_permission(user_role, permission): - await ctx.send("❌ You don't have permission to use this command.") + msg = "❌ You don't have permission to use this command." + if isinstance(ctx, nextcord.Interaction): + if ctx.response.is_done(): + await ctx.followup.send(msg, ephemeral=True) + else: + await ctx.response.send_message(msg, ephemeral=True) + else: + await ctx.send(msg) return return await func(*args, **kwargs) diff --git a/src/utils/security/validation.py b/src/utils/security/validation.py index 6a5ac17..5d82be4 100644 --- a/src/utils/security/validation.py +++ b/src/utils/security/validation.py @@ -3,7 +3,6 @@ Prevents injection attacks and malformed data """ -import html import logging import re from typing import Any @@ -43,10 +42,7 @@ def sanitize_text(text: str, max_length: int = 2000, allow_markdown: bool = Fals if not text: return '' - # HTML escape to prevent injection - text = html.escape(text) - - # Remove null bytes + # Strip null bytes text = text.replace('\x00', '') # Limit length From 42fffb64f10075d1b88a09b2a287a65c31c75680 Mon Sep 17 00:00:00 2001 From: MJkhan1400 Date: Fri, 24 Jul 2026 14:13:13 +0400 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20resolve=20mypy=20errors=20=E2=80=94?= =?UTF-8?q?=20missing=20method,=20type=20mismatches,=20and=20int/Snowflake?= =?UTF-8?q?=20arg?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add missing _build_preview_embed method (was called but never defined) - Fix admin_notifier channel assignment type mismatch - Fix stats.py get_user return type (Member vs User) - Wrap fetch_ban user_id in nextcord.Object for Snowflake compliance --- src/cogs/admin/massunban.py | 40 +++++++++++++++++++++++++++++++++- src/cogs/stats.py | 4 ++-- src/services/admin_notifier.py | 2 +- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/cogs/admin/massunban.py b/src/cogs/admin/massunban.py index 0dcf1a8..ac518aa 100644 --- a/src/cogs/admin/massunban.py +++ b/src/cogs/admin/massunban.py @@ -775,7 +775,7 @@ async def _process_unban_item(self, guild: nextcord.Guild, item: dict, job: dict # Check if already unbanned try: - await guild.fetch_ban(user_id) + await guild.fetch_ban(nextcord.Object(user_id)) except nextcord.NotFound: # Already unbanned result['status'] = 'skipped' @@ -1022,6 +1022,44 @@ async def _complete_job(self, job_id: int, guild: nextcord.Guild) -> None: except Exception: logger.error('Failed to complete job %s', job_id, exc_info=True) + async def _build_preview_embed( + self, + job_id: int, + ban_count: int, + start_dt: datetime, + end_dt: datetime, + banned_by: nextcord.Member | None, + reason: str, + no_audit_count: int, + ) -> nextcord.Embed: + embed = await veka_embed( + title='Mass Unban — Confirm', + description=f'**{ban_count} ban(s)** match your filters.', + contributor_source=__name__, + ) + embed.add_field(name='Job ID', value=str(job_id), inline=False) + embed.add_field( + name='Date Range', + value=f'{start_dt.strftime("%Y-%m-%d %H:%M:%S UTC")} → {end_dt.strftime("%Y-%m-%d %H:%M:%S UTC")}', + inline=False, + ) + if banned_by: + embed.add_field(name='Moderator Filter', value=banned_by.mention, inline=False) + if reason: + embed.add_field(name='Reason', value=reason, inline=False) + if no_audit_count > 0: + embed.add_field( + name='Note', + value=f'{no_audit_count} ban(s) have no audit history and will be included.', + inline=False, + ) + embed.add_field( + name='Confirm', + value='This action will unban all matched users. Double confirmation required.', + inline=False, + ) + return embed + async def _cancel_job(self, job_id: int, _runner_id: int) -> None: """Cancel a running job.""" self._active_jobs[job_id] = True diff --git a/src/cogs/stats.py b/src/cogs/stats.py index 60a76bb..0d8f5a1 100644 --- a/src/cogs/stats.py +++ b/src/cogs/stats.py @@ -186,8 +186,8 @@ def _resolve_member_name(self, user_id: str, guild: nextcord.Guild | None = None member = guild.get_member(uid) if member: return member.display_name - member = self.bot.get_user(uid) - return member.display_name if member else f'User {uid}' + user = self.bot.get_user(uid) + return user.display_name if user else f'User {uid}' # ============================================================ # Commands — Most Streamed diff --git a/src/services/admin_notifier.py b/src/services/admin_notifier.py index f218395..c3ed7c3 100644 --- a/src/services/admin_notifier.py +++ b/src/services/admin_notifier.py @@ -39,7 +39,7 @@ async def _get_channel(self): self._channel_fetched_at = now return None - self._channel = channel + self._channel = channel # type: ignore[assignment] self._channel_fetched_at = now return self._channel