Skip to content

Restore link handling, add diagnostics, and harden downloads/shortenings#24

Merged
Rajrooter merged 2 commits into
mainfrom
codex/review-codebase-for-improvements-and-fixes-qlx3sf
Feb 3, 2026
Merged

Restore link handling, add diagnostics, and harden downloads/shortenings#24
Rajrooter merged 2 commits into
mainfrom
codex/review-codebase-for-improvements-and-fixes-qlx3sf

Conversation

@Rajrooter

@Rajrooter Rajrooter commented Feb 3, 2026

Copy link
Copy Markdown
Owner

User description

Motivation

  • Link drop handling had been non-functional and produced no diagnostics, so restore automatic detection and provide simple checks to confirm the bot is responsive.
  • Downloads and URL shortening were brittle and could lead to excessive downloads or invalid schemes, so add guards to make these operations safer.

Description

  • Added a !ping command to quickly verify the bot is receiving and responding to messages.
  • Restored an on_message listener that processes commands, extracts URLs via URL_REGEX, filters with is_valid_url/is_media_url, and forwards accepted links to a new _handle_link workflow wrapped in a try/except that logs full tracebacks.
  • Implemented _handle_link to build a pending entry, call link_preview and make_verdict_embed, persist with storage.add_pending_link, present a LinkActionView, update the pending entry with the bot message id, and increment guild pending counters.
  • Hardened download and shortening helpers by rejecting non-HTTP(S) schemes, checking Content-Length against MAX_DOWNLOAD_BYTES, ensuring allowed content types, and switching the TinyURL call to https; also emit a startup warning if message_content intent is disabled and improve command error logging.

Testing

  • No automated tests were executed for this change.

Codex Task


PR Type

Enhancement, Bug fix


Description

  • Restored link detection via on_message listener with URL extraction and filtering

  • Added !ping command for bot responsiveness diagnostics

  • Hardened downloads by validating schemes, checking Content-Length, and verifying content types

  • Implemented _handle_link workflow with verdict generation, embed creation, and storage persistence

  • Added !pendinglinks command to display user's pending links

  • Switched TinyURL API to HTTPS and added message content intent warning at startup


Diagram Walkthrough

flowchart LR
  msg["Discord Message"] -- "on_message listener" --> extract["Extract URLs via regex"]
  extract -- "Filter valid/non-media" --> validate["Validate scheme & content"]
  validate -- "Pass checks" --> handle["_handle_link workflow"]
  handle -- "Get verdict & preview" --> embed["Create verdict embed"]
  embed -- "Store & display" --> view["LinkActionView with actions"]
  view -- "Update storage" --> persist["Persist bot message ID"]
  ping["ping command"] -- "Quick check" --> respond["Bot responds with Pong"]
Loading

File Walkthrough

Relevant files
Enhancement, bug fix, error handling
main.py
Link handling restoration with safety hardening                   

main.py

  • Added scheme validation in download_bytes to reject non-HTTP(S) URLs
  • Added Content-Length header check against MAX_DOWNLOAD_BYTES limit
  • Switched TinyURL API endpoint from HTTP to HTTPS in shorten_link
  • Implemented !ping command for bot diagnostics
  • Restored on_message listener that extracts URLs, filters with
    is_valid_url/is_media_url, and processes valid links
  • Implemented _handle_link method to generate verdicts, create embeds,
    store pending entries, and display action views
  • Added !pendinglinks command to retrieve and display user's pending
    links
  • Added startup warning when message content intent is disabled
+77/-1   

@vercel

vercel Bot commented Feb 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
discord-link-bot Ready Ready Preview, Comment Feb 3, 2026 2:28pm
discord-link-bot-wfz6 Ready Ready Preview, Comment Feb 3, 2026 2:28pm

@qodo-code-review

qodo-code-review Bot commented Feb 3, 2026

Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
SSRF via URL fetch

Description: User-controlled URLs are accepted from chat (on_message extracts links and forwards to
_handle_link at lines 915-936), and the codebase contains network-capable helpers that
operate on arbitrary URLs (download_bytes performs session.get(url) after only scheme
validation at lines 196-215; shorten_link forwards the full URL to TinyURL at lines
399-408), which can enable SSRF/internal network access (e.g., http://127.0.0.1:...,
http://169.254.169.254/..., private RFC1918 hosts, DNS rebinding, or redirects) unless
is_valid_url/other callers explicitly block private/loopback/link-local targets and
restrict redirects. main.py [196-972]

Referred Code
async def download_bytes(url: str) -> Optional[bytes]:
    try:
        parsed = urlparse(url)
        if parsed.scheme not in {"http", "https"}:
            return None
        async with aiohttp.ClientSession() as session:
            async with session.get(url, timeout=aiohttp.ClientTimeout(total=30)) as resp:
                if resp.status == 200:
                    content_length = resp.headers.get("Content-Length")
                    if content_length:
                        try:
                            if int(content_length) > MAX_DOWNLOAD_BYTES:
                                return None
                        except ValueError:
                            pass
                    content_type = resp.headers.get('Content-Type', '')
                    if (not content_type) or any(ct in content_type for ct in ALLOWED_CONTENT_TYPES):
                        data = await resp.read()
                        if len(data) <= MAX_DOWNLOAD_BYTES:
                            return data
    except Exception as e:


 ... (clipped 756 lines)
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unhandled link failures: The new _handle_link workflow performs multiple network/storage operations without local
error handling, so a single failure can abort processing and lose context for which
link/message failed.

Referred Code
async def _handle_link(self, message: discord.Message, link: str):
    verdict, reason = get_link_verdict()
    preview = await link_preview(link)
    embed = make_verdict_embed(link, verdict, reason, preview)
    entry = {
        "url": link,
        "timestamp": datetime.datetime.utcnow().isoformat(),
        "author": str(message.author),
        "user_id": message.author.id,
        "guild_id": message.guild.id if message.guild else None,
        "archived": False,
        "expires_at": None,
    }
    pending_id = await asyncio.to_thread(storage.add_pending_link, entry)
    view = LinkActionView(link, message.author.id, message, pending_id, self, ai_verdict=verdict)
    prompt_msg = await safe_send(message.channel, embed=embed, view=view)
    if prompt_msg:
        view.message = prompt_msg
        self.pending_links[prompt_msg.id] = pending_id
        await asyncio.to_thread(storage.update_pending_with_bot_msg_id, pending_id, prompt_msg.id)
        if message.guild:


 ... (clipped 2 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Unstructured error logs: The new error logging uses free-form f-strings (and stack traces) rather than structured
logging fields, reducing auditability and increasing the risk of accidental sensitive-data
inclusion.

Referred Code
except Exception as e:
    logger.error(f"on_message failed: {e}", exc_info=True)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing action logs: The PR adds state-changing persistence of user-submitted links to storage without any
explicit audit log capturing who performed the action and the outcome.

Referred Code
async def _handle_link(self, message: discord.Message, link: str):
    verdict, reason = get_link_verdict()
    preview = await link_preview(link)
    embed = make_verdict_embed(link, verdict, reason, preview)
    entry = {
        "url": link,
        "timestamp": datetime.datetime.utcnow().isoformat(),
        "author": str(message.author),
        "user_id": message.author.id,
        "guild_id": message.guild.id if message.guild else None,
        "archived": False,
        "expires_at": None,
    }
    pending_id = await asyncio.to_thread(storage.add_pending_link, entry)
    view = LinkActionView(link, message.author.id, message, pending_id, self, ai_verdict=verdict)
    prompt_msg = await safe_send(message.channel, embed=embed, view=view)
    if prompt_msg:
        view.message = prompt_msg
        self.pending_links[prompt_msg.id] = pending_id
        await asyncio.to_thread(storage.update_pending_with_bot_msg_id, pending_id, prompt_msg.id)
        if message.guild:


 ... (clipped 1 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
URL validation gaps: The PR validates schemes in download_bytes but does not apply similar scheme validation in
shorten_link or before persisting/displaying extracted URLs, which may allow
unexpected/non-HTTP(S) inputs depending on is_valid_url behavior.

Referred Code
async def download_bytes(url: str) -> Optional[bytes]:
    try:
        parsed = urlparse(url)
        if parsed.scheme not in {"http", "https"}:
            return None
        async with aiohttp.ClientSession() as session:
            async with session.get(url, timeout=aiohttp.ClientTimeout(total=30)) as resp:
                if resp.status == 200:
                    content_length = resp.headers.get("Content-Length")
                    if content_length:
                        try:
                            if int(content_length) > MAX_DOWNLOAD_BYTES:
                                return None
                        except ValueError:
                            pass
                    content_type = resp.headers.get('Content-Type', '')
                    if (not content_type) or any(ct in content_type for ct in ALLOWED_CONTENT_TYPES):
                        data = await resp.read()
                        if len(data) <= MAX_DOWNLOAD_BYTES:
                            return data
    except Exception as e:


 ... (clipped 757 lines)

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review

qodo-code-review Bot commented Feb 3, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Views are not persistent across restarts

The LinkActionView is not persistent and will break on bot restarts. To fix
this, make it a persistent view by setting timeout=None and registering it in
the setup_hook.

Examples:

main.py [739-750]
class LinkActionView(discord.ui.View):
    def __init__(self, link: str, author_id: int, original_message, pending_db_id: str, cog, ai_verdict: str = ""):
        super().__init__(timeout=300)
        self.link = link
        self.author_id = author_id
        self.original_message = original_message
        self.pending_db_id = pending_db_id
        self.cog = cog
        self.message = None
        self.ai_verdict = ai_verdict

 ... (clipped 2 lines)

Solution Walkthrough:

Before:

# main.py

class LinkActionView(discord.ui.View):
    def __init__(self, link: str, author_id: int, ...):
        # Implicitly calls super().__init__() with a default timeout.
        # The view is not persistent across bot restarts.
        ...

class MyBot(commands.Bot):
    async def setup_hook(self):
        # ...
        # The view is not registered here.
        ...

async def _handle_link(self, message: discord.Message, link: str):
    # ...
    view = LinkActionView(...)
    await safe_send(message.channel, embed=embed, view=view)

After:

# main.py

class LinkActionView(discord.ui.View):
    def __init__(self, ...):
        # Explicitly set timeout to None to make the view persistent.
        super().__init__(timeout=None)
        # Buttons must have a unique custom_id to be re-identified.
        # e.g., self.save_now_button.custom_id = f"save_now:{pending_db_id}"
        ...

class MyBot(commands.Bot):
    async def setup_hook(self):
        # ...
        # Register the view so it persists after restarts.
        # Note: The __init__ may need to be adapted for this.
        self.add_view(LinkActionView(...))
        ...

async def _handle_link(self, message: discord.Message, link: str):
    # ...
    view = LinkActionView(...)
    await safe_send(message.channel, embed=embed, view=view)
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical design flaw where a bot restart would break the core link-handling workflow, making the suggestion highly impactful and relevant.

High
Security
Harden file download size check

In download_bytes, return None within the except ValueError block to abort
downloads when the Content-Length header is invalid, preventing potential
excessive memory usage.

main.py [205-210]

 if content_length:
     try:
         if int(content_length) > MAX_DOWNLOAD_BYTES:
             return None
     except ValueError:
-        pass
+        # If Content-Length is not a valid integer, abort the download.
+        logger.debug(f"Invalid Content-Length header received: {content_length}")
+        return None
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a flaw in the new download size check that could lead to excessive memory usage if the Content-Length header is invalid, which constitutes a resource exhaustion vulnerability.

Medium
Possible issue
Avoid spam by handling multiple links

To avoid channel spam, handle multiple links found in a message with a single,
consolidated UI instead of sending a separate prompt for each link.

main.py [932-935]

 if not filtered:
     return
-for link in filtered:
-    await self._handle_link(message, link)
 
+if len(filtered) > 1:
+    # TODO: Implement multi-link handling, possibly using MultiLinkSelectView
+    # For example:
+    # view = MultiLinkSelectView(filtered, message.author.id, message, self)
+    # await safe_send(message.channel, content=multi_link_message(len(filtered)), view=view)
+    # For now, just process the first link to avoid spam.
+    await self._handle_link(message, filtered[0])
+else:
+    await self._handle_link(message, filtered[0])
+
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: This is a valid and important suggestion that identifies a significant user experience issue where multiple links in a message would spam the channel, correctly noting that UI elements for multi-link handling exist but are unused.

Medium
Paginate command output to prevent errors

Paginate the output of the pendinglinks command to prevent errors from exceeding
Discord's message character limit when a user has many pending links.

main.py [962-972]

 @commands.command(name="pendinglinks")
 async def pending_links_command(self, ctx: commands.Context):
     links = await asyncio.to_thread(storage.get_pending_links_for_user, ctx.author.id)
     if not links:
         await safe_send(ctx, content="✅ No pending links.")
         return
+    
     lines = []
-    for idx, entry in enumerate(links, start=1):
+    limit = 20  # Display up to 20 links to avoid hitting character limits
+    for idx, entry in enumerate(links[:limit], start=1):
         url = entry.get("url", "unknown")
-        lines.append(f"{idx}. {url}")
-    await safe_send(ctx, content="🕒 **Your pending links:**\n" + "\n".join(lines))
+        # Truncate long URLs for better readability
+        url_display = url[:100] + '...' if len(url) > 100 else url
+        lines.append(f"{idx}. <{url_display}>")
 
+    content = "🕒 **Your pending links:**\n" + "\n".join(lines)
+    if len(links) > limit:
+        content += f"\n\n*...and {len(links) - limit} more. Showing first {limit}.*"
+
+    await safe_send(ctx, content=content)
+
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential issue where the pendinglinks command could fail by exceeding Discord's message character limit, and proposes a reasonable solution to make the command more robust.

Low
  • Update

@Rajrooter Rajrooter merged commit ecd71c1 into main Feb 3, 2026
2 checks passed
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

Discord-Link-Bot/main.py

Lines 949 to 953 in 954a1df

"archived": False,
"expires_at": None,
}
pending_id = await asyncio.to_thread(storage.add_pending_link, entry)
view = LinkActionView(link, message.author.id, message, pending_id, self, ai_verdict=verdict)

P2 Badge Avoid orphaned pending entries when prompt fails

If safe_send fails later in _handle_link (e.g., the bot can read but lacks Send Messages in the channel), the pending link is already persisted and never cleaned up. That leaves orphaned pending entries that users can’t action via the UI and that will accumulate in storage, since the only cleanup paths are tied to button interactions. Consider only writing the pending entry after the prompt message is successfully sent, or delete it when safe_send returns None.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant