Skip to content

Integrated Discord Identity#464

Merged
cwilvx merged 5 commits intotestnetfrom
integrate-discord-identity
Sep 15, 2025
Merged

Integrated Discord Identity#464
cwilvx merged 5 commits intotestnetfrom
integrate-discord-identity

Conversation

@HakobP-Solicy
Copy link
Contributor

@HakobP-Solicy HakobP-Solicy commented Sep 5, 2025

PR Type

Enhancement


Description

  • Add Discord identity verification support

  • Implement Discord proof parser and message fetching

  • Extend Web2 proof system with Discord integration

  • Add Discord bot token configuration


Diagram Walkthrough

flowchart LR
  A["Discord Message URL"] --> B["DiscordProofParser"]
  B --> C["Discord API"]
  C --> D["Message Verification"]
  D --> E["Identity Proof"]
  F["Web2ProofParser"] --> B
  G["Node Call Handler"] --> H["getDiscordMessage"]
  H --> C
Loading

File Walkthrough

Relevant files
Enhancement
index.ts
Integrate Discord parser into verification system               

src/libs/abstraction/index.ts

  • Import DiscordProofParser class
  • Add Discord case to proof verification switch
  • Update parser type union to include Discord
+8/-1     
discord.ts
Implement Discord proof parser class                                         

src/libs/abstraction/web2/discord.ts

  • Create DiscordProofParser class extending Web2ProofParser
  • Implement Discord message URL parsing logic
  • Add Discord API integration for message fetching
  • Implement proof data extraction from Discord messages
+94/-0   
parsers.ts
Add Discord URL format support                                                     

src/libs/abstraction/web2/parsers.ts

  • Add Discord URL formats to supported formats
  • Include discord.com and ptb.discord.com domains
+4/-0     
discord.ts
Create Discord API integration utility                                     

src/libs/identity/tools/discord.ts

  • Create Discord utility class with singleton pattern
  • Implement Discord message URL parsing and validation
  • Add Discord API client with rate limiting
  • Define Discord message type interfaces
+151/-0 
manageNodeCall.ts
Add Discord message fetching endpoint                                       

src/libs/network/manageNodeCall.ts

  • Add getDiscordMessage node call handler
  • Import Discord utility class
  • Implement Discord message fetching endpoint
  • Remove unused GCR imports
+87/-7   
Configuration changes
.env.example
Add Discord configuration variables                                           

.env.example

  • Add DISCORD_BOT_TOKEN environment variable
  • Add DISCORD_API_URL configuration option
+4/-1     

Summary by CodeRabbit

  • New Features

    • Retrieve Discord messages by URL and verify Discord-based proofs.
    • Link Discord as a social identity; earn/deduct points when Discord is linked/unlinked; incentive hooks added.
    • Node API supports fetching Discord messages to power client flows.
  • Chores

    • .env.example now includes DISCORD_BOT_TOKEN and DISCORD_API_URL; trailing newline fixed.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 5, 2025

Walkthrough

Adds Discord support: new env vars, a Discord API client and types, message URL parsing/fetching, a Discord Web2 proof parser wired into verifyWeb2Proof, a manageNodeCall action to fetch Discord messages, and incentive/points plumbing treating Discord like other Web2 identities.

Changes

Cohort / File(s) Summary of changes
Environment configuration
.env.example
Added DISCORD_BOT_TOKEN and DISCORD_API_URL; ensured trailing newline.
Web2 proof parsing (Discord)
src/libs/abstraction/web2/parsers.ts, src/libs/abstraction/web2/discord.ts, src/libs/abstraction/index.ts
Added discord URL prefixes to Web2 formats; implemented DiscordProofParser (format verification, message URL parsing, fetch via Discord client, payload parsing); wired parser selection for context === "discord"; public verify API unchanged.
Discord API client utilities
src/libs/identity/tools/discord.ts
New Discord singleton using Axios; reads DISCORD_BOT_TOKEN/DISCORD_API_URL; implements extractMessageDetails, isSnowflake, getMessageById, getMessageByUrl, 429 handling; exports Discord and DiscordMessage types.
Node call handling
src/libs/network/manageNodeCall.ts
Added getDiscordMessage case: validates discordUrl, uses Discord to fetch message, returns structured payload (id, timestamp, authorUsername, authorId, channelId, guildId); replaced unused GCR/crypto imports for this path and adjusted error handling.
Incentives / Point system
src/features/incentive/PointSystem.ts, src/libs/blockchain/gcr/gcr_routines/IncentiveManager.ts, src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts
Added Discord as a linked social identity: new constant LINK_DISCORD; getUserIdentitiesFromGCR exposes Discord identities; added awardDiscordPoints/deductDiscordPoints in PointSystem; wired discordLinked/discordUnlinked in IncentiveManager; GCR routines updated to treat "discord" like Twitter/GitHub for first-connection, link, and unlink flows.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant App as verifyWeb2Proof
  participant Parser as DiscordProofParser
  participant API as Discord API v10

  Client->>App: verifyWeb2Proof(payload with context="discord")
  App->>Parser: select parser and readData(proofUrl)
  Parser->>Parser: verifyProofFormat(proofUrl, "discord")
  Parser->>API: GET /channels/{channelId}/messages/{messageId}\nAuthorization: Bot <token>
  API-->>Parser: 200 OK (message content) / error
  Parser->>Parser: parsePayload(message.content)
  Parser-->>App: { message, signature, type }
  App-->>Client: verification result
Loading
sequenceDiagram
  autonumber
  participant Caller
  participant Node as manageNodeCall
  participant D as Discord (singleton)
  participant API as Discord API v10

  Caller->>Node: action="getDiscordMessage", data.discordUrl
  Node->>D: getInstance()
  Node->>D: getMessageByUrl(discordUrl)
  D->>D: extractMessageDetails(url)
  D->>API: GET /channels/{channelId}/messages/{messageId}
  API-->>D: 200 OK (DiscordMessage) / 4xx/5xx
  D-->>Node: DiscordMessage or throw
  alt success
    Node-->>Caller: { success: true, message: { id, timestamp, authorUsername, authorId, channelId, guildId } }
  else error
    Node-->>Caller: { success: false, error }
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Backup & Restore #404 — touches the same manageNodeCall area and verifyWeb2Proof handling; relevant for overlapping node-call cases and parser selection.
  • Unified Crypto refactor #281 — modifies Web2 proof parsing/abstraction and formats; likely overlaps with the new DiscordProofParser and formats additions.

Suggested labels

ReviewedByDojoReviews

"I twitch my whiskers, hop online,
Tokens set and channels shine.
Snowflakes parsed, messages found,
Proofs hop home on solid ground.
Carrots await each linked friend—hop, hooray! 🥕🐇"

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Integrated Discord Identity" succinctly and accurately summarizes the primary change: adding Discord identity and proof support across the codebase (proof parser, Discord client, node call, and env vars). It is concise, specific to the main feature, and avoids noisy details like file lists or emojis. The title is appropriate for quick scanning of project history and clearly reflects the PR objectives.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch integrate-discord-identity

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

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
The .env.example includes a full Discord bot token value. This is a potential credential leak and may encourage insecure practices. Replace with a placeholder like DISCORD_BOT_TOKEN=YOUR_DISCORD_BOT_TOKEN and rotate any exposed token immediately.

Token handling: Multiple places construct Authorization headers manually; ensure no logs include tokens. The Discord utility centralizes axios with headers and basic 429 handling—prefer reusing it to reduce risk of accidental leakage and to standardize rate-limit handling.

URL parsing robustness: Consider validating and normalizing Discord hosts consistently (e.g., discord.com/discordapp.com) in all code paths; currently the parser in DiscordProofParser splits strings by index which is brittle and could be abused if a malformed URL bypasses expected checks.

⚡ Recommended focus areas for review

Duplicated Logic

Message fetching and URL parsing logic duplicate what exists in the Discord utility class; use the shared Discord client (with rate limit handling) to avoid inconsistencies and drift.

async getMessageFromUrl(messageUrl: string) {
    const apiUrl = "https://discord.com/api/v10"
    const parts = messageUrl.split("/").filter(Boolean)
    const channelId = parts[5]
    const messageId = parts[6]

    const res = await axios.get(
        `${apiUrl}/channels/${channelId}/messages/${messageId}`,
        {
            headers: { Authorization: `Bot ${this.botToken}` },
        },
    )

    return res.data
}

async readData(proofUrl: string): Promise<{
    message: string
    signature: string
    type: SigningAlgorithm
}> {
    this.verifyProofFormat(proofUrl, "discord")

    const { channelId, messageId } = this.parseDiscordMessageUrl(proofUrl)

    const res = await axios.get(
        `https://discord.com/api/v10/channels/${channelId}/messages/${messageId}`,
        {
            headers: {
                Authorization: `Bot ${this.botToken}`,
            },
        },
    )

    if (res.status !== 200) {
        throw new Error(`Failed to fetch Discord message: ${res.status}`)
    }

    const content = (res.data?.content as string) || ""

    const payload = this.parsePayload(content)
    if (!payload) {
        throw new Error("Invalid proof format")
    }

    return payload
}
Env Handling

The parser reads DISCORD_BOT_TOKEN directly and proceeds with empty string when missing; prefer validating and throwing a clear error or using the shared Discord singleton which already enforces presence.

constructor() {
    super()
    this.discord = Discord.getInstance()
    this.botToken = process.env.DISCORD_BOT_TOKEN ?? ""
}
Secret Exposure

The example file contains a real-looking Discord bot token; replace with a placeholder to avoid leaking credentials and to prevent users from accidentally committing secrets.

GITHUB_TOKEN=

DISCORD_BOT_TOKEN=MTQxMjEyMjcyMzc0Mzk1NzA2NA.GKYDD0.GHyxhRm2xcGK2-f6yOH4p5sfbC8cJtJWTz-yVk
DISCORD_API_URL=https://discord.com/api/v10

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Sep 5, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Remove leaked Discord token

The .env.example includes what appears to be a real DISCORD_BOT_TOKEN, which is
a severe secret leakage risk. Immediately revoke/rotate the token, replace it
with a placeholder in the example file, and audit repo history/CI artifacts for
exposure; rely on environment-specific secret management rather than committing
live credentials.

Examples:

.env.example [7]
DISCORD_BOT_TOKEN=MTQxMjEyMjcyMzc0Mzk1NzA2NA.GKYDD0.GHyxhRm2xcGK2-f6yOH4p5sfbC8cJtJWTz-yVk

Solution Walkthrough:

Before:

# .env.example

TWITTER_USERNAME=
TWITTER_PASSWORD=
TWITTER_EMAIL=

GITHUB_TOKEN=

DISCORD_BOT_TOKEN=MTQxMjEyMjcyMzc0Mzk1NzA2NA.GKYDD0.GHyxhRm2xcGK2-f6yOH4p5sfbC8cJtJWTz-yVk
DISCORD_API_URL=https://discord.com/api/v10

After:

# .env.example

TWITTER_USERNAME=
TWITTER_PASSWORD=
TWITTER_EMAIL=

GITHUB_TOKEN=

DISCORD_BOT_TOKEN=
DISCORD_API_URL=https://discord.com/api/v10
Suggestion importance[1-10]: 10

__

Why: This suggestion correctly identifies a critical security vulnerability where a live DISCORD_BOT_TOKEN was committed to the repository, which requires immediate remediation.

High
Security
Remove exposed Discord bot token
Suggestion Impact:The commit removed the hardcoded Discord bot token and set the value to an empty placeholder, aligning with the suggestion to avoid exposing a real token. It also cleared the Discord API URL.

code diff:

-DISCORD_BOT_TOKEN=MTQxMjEyMjcyMzc0Mzk1NzA2NA.GKYDD0.GHyxhRm2xcGK2-f6yOH4p5sfbC8cJtJWTz-yVk
-DISCORD_API_URL=https://discord.com/api/v10
+DISCORD_BOT_TOKEN=
+DISCORD_API_URL=

The Discord bot token appears to be a real token exposed in the example file.
This is a critical security vulnerability as tokens should never be committed to
version control, even in example files.

.env.example [7]

-DISCORD_BOT_TOKEN=MTQxMjEyMjcyMzc0Mzk1NzA2NA.GKYDD0.GHyxhRm2xcGK2-f6yOH4p5sfbC8cJtJWTz-yVk
+DISCORD_BOT_TOKEN=your_discord_bot_token_here

[Suggestion processed]

Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies a hardcoded secret (DISCORD_BOT_TOKEN) in a committed file, which is a critical security vulnerability.

High
General
Use existing parsing method consistently
Suggestion Impact:The commit removed hardcoded index-based parsing and direct axios calls in getMessageFromUrl, delegating to the shared Discord client (this.discord.getMessageByUrl). It also centralized parsing via this.discord.extractMessageDetails and validated URLs with parseDiscordMessageUrl, aligning with the suggestion’s intent to use existing parsing logic consistently.

code diff:

     async getMessageFromUrl(messageUrl: string) {
-        const apiUrl = "https://discord.com/api/v10"
-        const parts = messageUrl.split("/").filter(Boolean)
-        const channelId = parts[5]
-        const messageId = parts[6]
-
-        const res = await axios.get(
-            `${apiUrl}/channels/${channelId}/messages/${messageId}`,
-            {
-                headers: { Authorization: `Bot ${this.botToken}` },
-            },
-        )
-
-        return res.data
+        return await this.discord.getMessageByUrl(messageUrl)
     }

The getMessageFromUrl method uses hardcoded array indices which could fail if
the URL format changes. It also duplicates logic already implemented in
parseDiscordMessageUrl and readData methods.

src/libs/abstraction/web2/discord.ts [39-53]

 async getMessageFromUrl(messageUrl: string) {
-    const apiUrl = "https://discord.com/api/v10"
-    const parts = messageUrl.split("/").filter(Boolean)
-    const channelId = parts[5]
-    const messageId = parts[6]
-
+    const { channelId, messageId } = this.parseDiscordMessageUrl(messageUrl)
+    
     const res = await axios.get(
-        `${apiUrl}/channels/${channelId}/messages/${messageId}`,
+        `${this.discord.api_url}/channels/${channelId}/messages/${messageId}`,
         {
             headers: { Authorization: `Bot ${this.botToken}` },
         },
     )
 
     return res.data
 }

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that getMessageFromUrl uses fragile, hardcoded index-based parsing, and it improves robustness and maintainability by reusing the parseDiscordMessageUrl method.

Low
  • Update

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/libs/abstraction/index.ts (1)

62-63: Handle parser initialization errors (e.g., missing env) gracefully.

getInstance() for GitHub/Discord can throw; currently uncaught and will bubble. Catch and return a user-facing error.

-    const instance = await parser.getInstance()
+    let instance: any
+    try {
+        instance = await parser.getInstance()
+    } catch (error: any) {
+        return {
+            success: false,
+            message: `Failed to initialize ${payload.context} parser: ${error?.message ?? String(error)}`,
+        }
+    }
src/features/incentive/PointSystem.ts (2)

321-329: Bug: awarding wallet points even when already linked or chain-conflicting.

You add points unconditionally before deciding the response code, enabling duplicate awards.

Apply:

-            // Award points by updating the GCR
-            await this.addPointsToGCR(
-                userId,
-                pointValues.LINK_WEB3_WALLET,
-                "web3Wallets",
-                chain,
-                referralCode,
-            )
+            // Award only when not duplicate and no chain conflict
+            if (!walletIsAlreadyLinked && !hasExistingWalletOnChain) {
+                await this.addPointsToGCR(
+                    userId,
+                    pointValues.LINK_WEB3_WALLET,
+                    "web3Wallets",
+                    chain,
+                    referralCode,
+                )
+            }

Also applies to: 333-351


231-246: Bug: Twitter follow bonus can be re-added on every addPointsToGCR call.

This double-counts FOLLOW_DEMOS across unrelated award paths.

Apply:

-            if (isFollowingDemos) {
-                account.points.breakdown.demosFollow = pointValues.FOLLOW_DEMOS
-                account.points.totalPoints += pointValues.FOLLOW_DEMOS
-            }
+            if (isFollowingDemos) {
+                const already = account.points.breakdown?.demosFollow || 0
+                if (already === 0) {
+                    account.points.breakdown.demosFollow = pointValues.FOLLOW_DEMOS
+                    account.points.totalPoints += pointValues.FOLLOW_DEMOS
+                }
+            }

Also applies to: 248-249

🧹 Nitpick comments (16)
.env.example (2)

7-8: Satisfy dotenv-linter ordering rule.

Reorder to place DISCORD_API_URL before DISCORD_BOT_TOKEN.

-DISCORD_BOT_TOKEN=
-DISCORD_API_URL=
+DISCORD_API_URL=
+DISCORD_BOT_TOKEN=

7-8: Document defaults and secret handling.

Add brief comments to signal the default API URL and that the bot token must never be committed.

I can add a short README section or inline comments if you want.

src/libs/abstraction/web2/parsers.ts (1)

11-14: Make context type-safe to avoid indexing with arbitrary strings.

Constrain context to a union of supported keys to prevent undefined lookups and improve DX.

Add near the top of the file:

export type Web2Context = "github" | "twitter" | "discord"

Then change:

formats: Record<Web2Context, string[]> = { ... }

verifyProofFormat(proofUrl: string, context: Web2Context) { ... }
src/libs/identity/tools/discord.ts (7)

91-97: Avoid logging full message URLs.

Reduce leakage in logs by not echoing user-supplied URLs verbatim.

-            console.error(`Failed to extract details from URL: ${messageUrl}`)
+            console.warn("Failed to extract details from Discord URL")

127-137: Simplify axios response handling.

Axios throws on non-2xx; the status check is unreachable on error paths.

-        if (res.status === 200) return res.data
-        throw new Error("Failed to get Discord message")
+        return res.data

71-88: Confirm whether DMs should be supported (/channels/@me/...).

Current validation requires a snowflake guildId and will reject valid DM links that use @me. If DMs are in scope, allow guildId === "@me" and keep snowflake checks for channelId and messageId. If not, explicitly document guild-only support.

Potential tweak:

const [_, guildId, channelId, messageId] = parts
const isDm = guildId === "@me"
if (
  (!isDm && !this.isSnowflake(guildId)) ||
  !this.isSnowflake(channelId) ||
  !this.isSnowflake(messageId)
) {
  throw new Error("One or more IDs are not valid Discord snowflakes")
}

105-125: Optional: smarter rate-limit/backoff handling.

Consider honoring x-ratelimit-reset-after (seconds, float) and adding jitter/retries for 5xx to reduce thundering herd.

Sketch:

const ra = parseFloat(e.response.headers["retry-after"] ?? e.response.headers["x-ratelimit-reset-after"] ?? "1")
const ms = Math.max(0, Math.ceil(ra * 1000) + Math.floor(Math.random()*250))
await new Promise(r => setTimeout(r, ms))
return await this.axios.get<T>(url)

61-69: Host check comment vs behavior.

Comment says “Normalize … to discord.com” but code only validates; it doesn’t normalize. Either update the comment or actually normalize.


35-36: Minor: restrict visibility if public access isn’t required.

If nothing outside reads api_url/bot_token, make them private to avoid accidental use.


52-98: Add tests for URL parsing edge cases.

Cover: ptb/canary, discordapp.com, invalid paths, non-snowflake IDs, and optional DM handling.

I can scaffold Jest tests for extractMessageDetails if helpful.

src/libs/network/manageNodeCall.ts (1)

268-273: Validate input type and scheme for discordUrl.

Guard against non-string and non-https inputs before parsing.

-            if (!data.discordUrl) {
+            if (typeof data.discordUrl !== "string" || !data.discordUrl) {
                 response.result = 400
                 response.response = "No Discord URL specified"
                 break
             }
+            try {
+                const u = new URL(data.discordUrl)
+                if (u.protocol !== "https:") throw new Error("Invalid scheme")
+            } catch {
+                response.result = 400
+                response.response = "Invalid Discord URL"
+                break
+            }
src/libs/abstraction/index.ts (1)

19-23: Unify Discord proof fetching via the Discord utility.

DiscordProofParser currently reimplements URL parsing and calls axios directly. Prefer the central Discord helper (rate limiting, host validation).

I can prep a PR updating DiscordProofParser.readData to:

  • Use Discord.getInstance().extractMessageDetails() and .getMessageById()
  • Drop duplicate URL parsing and bot token handling in the parser.

Also applies to: 31-33

src/features/incentive/PointSystem.ts (2)

37-47: Verify Discord identity shape and null-safety.

Assumes getWeb2Identities(..., "discord") returns objects with .username. If it can be absent, add a fallback to avoid runtime errors.

Apply defensiveness:

-        if (Array.isArray(discordIdentities) && discordIdentities.length > 0) {
-            linkedSocials.discord = discordIdentities[0].username
-        }
+        if (Array.isArray(discordIdentities) && discordIdentities.length > 0) {
+            const d = discordIdentities[0] || {}
+            linkedSocials.discord = d.username ?? d.userId ?? String(d)
+        }

Also applies to: 71-79


776-828: Optional: also verify Discord linkage on deduct.

Current check uses points state only; consider also confirming a linked Discord identity for consistency with award flow.

src/libs/abstraction/web2/discord.ts (2)

87-93: Singleton accessor shouldn’t be async.

There’s no await path; async here forces unnecessary awaits at call sites.

Apply:

-    static async getInstance() {
+    static getInstance() {
         if (!this.instance) {
             this.instance = new DiscordProofParser()
         }
 
         return this.instance
     }

1-1: Remove axios import if no longer used after refactor.

Apply:

-import axios from "axios"
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7b3c3ee and ee9f49e.

📒 Files selected for processing (9)
  • .env.example (1 hunks)
  • src/features/incentive/PointSystem.ts (7 hunks)
  • src/libs/abstraction/index.ts (3 hunks)
  • src/libs/abstraction/web2/discord.ts (1 hunks)
  • src/libs/abstraction/web2/parsers.ts (1 hunks)
  • src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (5 hunks)
  • src/libs/blockchain/gcr/gcr_routines/IncentiveManager.ts (2 hunks)
  • src/libs/identity/tools/discord.ts (1 hunks)
  • src/libs/network/manageNodeCall.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/libs/abstraction/index.ts (3)
src/libs/abstraction/web2/twitter.ts (1)
  • TwitterProofParser (5-49)
src/libs/abstraction/web2/github.ts (1)
  • GithubProofParser (6-98)
src/libs/abstraction/web2/discord.ts (1)
  • DiscordProofParser (6-94)
src/libs/network/manageNodeCall.ts (1)
src/libs/identity/tools/discord.ts (2)
  • Discord (31-151)
  • DiscordMessage (4-29)
src/features/incentive/PointSystem.ts (1)
src/libs/blockchain/gcr/gcr_routines/ensureGCRForUser.ts (1)
  • ensureGCRForUser (8-33)
src/libs/abstraction/web2/discord.ts (1)
src/libs/identity/tools/discord.ts (1)
  • Discord (31-151)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (1)
src/libs/blockchain/gcr/gcr_routines/IncentiveManager.ts (1)
  • IncentiveManager (9-113)
🪛 dotenv-linter (3.3.0)
.env.example

[warning] 8-8: [UnorderedKey] The DISCORD_API_URL key should go before the DISCORD_BOT_TOKEN key

(UnorderedKey)

🔇 Additional comments (7)
src/libs/identity/tools/discord.ts (2)

139-143: Nice: clean URL → IDs → fetch flow.

The delegation via extractMessageDetails keeps getMessageByUrl lean.


43-50: Good: singleton axios client with auth header.

Centralizing the client reduces duplication and eases future telemetry.

src/libs/blockchain/gcr/gcr_routines/IncentiveManager.ts (1)

97-112: New Discord hooks look consistent with PointSystem.

Signatures align with PointSystem.awardDiscordPoints/deductDiscordPoints.

src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (1)

671-689: Query for Discord first-connection mirrors Twitter/GitHub correctly.

JSONB path and currentAccount exclusion look good.

src/features/incentive/PointSystem.ts (3)

17-18: LINK_DISCORD constant addition looks good.

No issues with the new point value.


448-460: GitHub ownership check before awarding — good hardening.

Validates linkage prior to points; message is clear.


631-634: All deductGithubPoints invocations updated to include githubUserId. The signature change is reflected in every call site (e.g. IncentiveManager.ts:87).

Comment on lines +274 to +286
const discord = Discord.getInstance()
let message: DiscordMessage | null = null

try {
message = await discord.getMessageByUrl(data.discordUrl)
} catch (error) {
response.result = 400
response.response = {
success: false,
error: "Failed to get Discord message",
}
break
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid crashing when DISCORD_BOT_TOKEN is missing (wrap getInstance).

Discord.getInstance() can throw; this will bubble and crash the handler. Return a 500 with a clear error instead.

-            const discord = Discord.getInstance()
-            let message: DiscordMessage | null = null
+            let discord: Discord
+            try {
+                discord = Discord.getInstance()
+            } catch (e) {
+                response.result = 500
+                response.response = { success: false, error: "Discord not configured" }
+                break
+            }
+            let message: DiscordMessage | null = null
📝 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
const discord = Discord.getInstance()
let message: DiscordMessage | null = null
try {
message = await discord.getMessageByUrl(data.discordUrl)
} catch (error) {
response.result = 400
response.response = {
success: false,
error: "Failed to get Discord message",
}
break
}
let discord: Discord
try {
discord = Discord.getInstance()
} catch (e) {
response.result = 500
response.response = { success: false, error: "Discord not configured" }
break
}
let message: DiscordMessage | null = null
try {
message = await discord.getMessageByUrl(data.discordUrl)
} catch (error) {
response.result = 400
response.response = {
success: false,
error: "Failed to get Discord message",
}
break
}
🤖 Prompt for AI Agents
In src/libs/network/manageNodeCall.ts around lines 274 to 286, calling
Discord.getInstance() is unsafe because it can throw and crash the handler; wrap
the getInstance() call in a try/catch separate from getMessageByUrl so if
getInstance() throws you set response.result = 500 and response.response = {
success: false, error: "Discord bot token missing or Discord client
initialization failed" } (or similar clear message) and break, otherwise proceed
to call getMessageByUrl inside its own try/catch that returns a 400 on failure
as currently implemented.

Comment on lines +288 to +349
response.result = message ? 200 : 400
if (message) {
let guildIdFromUrl: string | undefined
let channelIdFromUrl: string | undefined
let messageIdFromUrl: string | undefined

try {
const details = discord.extractMessageDetails(
data.discordUrl,
)
guildIdFromUrl = details.guildId
channelIdFromUrl = details.channelId
messageIdFromUrl = details.messageId
} catch {
// non-fatal, e.g. if URL format was unexpected
}

const payload = {
id: message.id,
content: message.content,
timestamp: message.timestamp,
edited_timestamp: message.edited_timestamp ?? null,

authorUsername: message.author?.username ?? null,
authorGlobalName: message.author?.global_name ?? null,
authorId: message.author?.id ?? null,
authorIsBot: !!message.author?.bot,

channelId: message.channel_id ?? channelIdFromUrl ?? null,
guildId:
(message as any).guild_id ?? guildIdFromUrl ?? null,

attachments: (message.attachments || []).map(a => ({
id: a.id,
filename: a.filename,
size: a.size,
url: a.url,
proxy_url: a.proxy_url,
content_type: a.content_type ?? null,
})),
embedsCount: Array.isArray(message.embeds)
? message.embeds.length
: 0,
mentions: (message.mentions || []).map(m => ({
id: m.id,
username: m.username,
})),
replyToId: message.referenced_message?.id ?? null,
originalMessageIdFromUrl: messageIdFromUrl ?? null,
}

response.response = {
message: payload,
success: true,
}
} else {
response.response = {
success: false,
error: "Failed to get Discord message",
}
}
break
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential data exfiltration: returning private Discord content verbatim.

Unlike tweets, Discord messages may be from private servers. Anyone who can hit this endpoint can fetch content the bot can see. Consider access control and data minimization (only return fields required for proof parsing).

Suggested mitigations:

  • Require authenticated caller (signed account) and rate-limit/allowlist guilds.
  • Return only id, content, timestamp, authorId, channelId (omit attachments/mentions) unless an internal flag whitelists extended fields.
🤖 Prompt for AI Agents
In src/libs/network/manageNodeCall.ts around lines 288-349, the current response
returns full Discord message contents (attachments, embeds, mentions, etc.)
which risks data exfiltration; change this to require an
authenticated/authorized caller before returning private content and minimize
default fields returned: only include id, content, timestamp, authorId,
channelId by default; remove attachments, embedsCount, mentions, proxy URLs, and
other PII unless the caller is explicitly whitelisted or presents an internal
feature-flag/permission (e.g., admin token or signed account) that allows
extended fields; additionally ensure callers are rate-limited and/or
guild-allowlisted when requesting extended data and log/validate
discordUrl-derived guild/channel IDs against allowlist before returning
non-minimal fields.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (3)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (2)

259-262: Discord link incentive call now uses correct args — resolved.

Passing editOperation.account as userId and editOperation.referralCode as the optional referral aligns with IncentiveManager.discordLinked(userId, referralCode?). Good fix.


316-331: Don’t gate Discord unlink on presence of userId.

The current condition can skip deductions and leave points undiscounted. Call discordUnlinked for any Discord unlink.

-            } else if (
-                context === "discord" &&
-                removedIdentity &&
-                removedIdentity.userId
-            ) {
-                await IncentiveManager.discordUnlinked(editOperation.account)
+            } else if (context === "discord") {
+                await IncentiveManager.discordUnlinked(editOperation.account)
             }
src/libs/identity/tools/discord.ts (1)

35-54: Good: readonly secrets + host allowlist mitigate SSRF.

Readonly fields and strict host validation are the right guardrails here.

🧹 Nitpick comments (5)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (2)

297-302: Avoid unnecessary lookup for Discord unlink.

discordUnlinked doesn’t need removedIdentity or a userId. Restrict the pre-removal lookup to GitHub only.

-        if (context === "github" || context === "discord") {
+        if (context === "github") {
             removedIdentity =
                 accountGCR.identities.web2[context].find(
                     (id: Web2GCRData["data"]) => id.username === username,
                 ) || null
         }

312-314: Comment nit: broaden wording.

Update to reflect all Web2 unlinking, not just Twitter.

-            /**
-             * Deduct incentive points for Twitter unlinking
-             */
+            /**
+             * Deduct incentive points for Web2 unlinking
+             */
src/libs/identity/tools/discord.ts (2)

124-137: Handle Discord’s retry_after and X-RateLimit-Reset-After.

Discord commonly returns retry_after (body) or x-ratelimit-reset-after (header). Prefer these over—or in addition to—retry-after.

-            if (e?.response?.status === 429) {
-                const retryAfter = Number(
-                    e.response.headers["retry-after"] ?? 1,
-                )
-                await new Promise(r =>
-                    setTimeout(r, Math.ceil(retryAfter * 1000)),
-                )
-                return await this.axios.get<T>(url)
+            if (e?.response?.status === 429) {
+                const headers = e.response.headers ?? {}
+                const data = e.response.data ?? {}
+                const retryAfterSec = parseFloat(
+                    headers["x-ratelimit-reset-after"] ??
+                        data.retry_after ??
+                        headers["retry-after"] ??
+                        "1",
+                )
+                await new Promise(r =>
+                    setTimeout(r, Math.ceil(retryAfterSec * 1000)),
+                )
+                return await this.axios.get<T>(url)
             }

148-150: Include status code in error for faster triage.

Small improvement to error context.

-        if (res.status === 200) return res.data
-        throw new Error("Failed to get Discord message")
+        if (res.status === 200) return res.data
+        throw new Error(`Failed to get Discord message (status ${res.status})`)
src/libs/abstraction/web2/parsers.ts (1)

11-16: Consider pattern-based validation to avoid enumerating subdomains.

Longer-term, parse the URL and validate host/path (e.g., any subdomain of discord.com with /channels/) instead of maintaining a growing prefix list.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ee9f49e and ab5dfa2.

📒 Files selected for processing (7)
  • .env.example (1 hunks)
  • src/features/incentive/PointSystem.ts (7 hunks)
  • src/libs/abstraction/web2/discord.ts (1 hunks)
  • src/libs/abstraction/web2/parsers.ts (1 hunks)
  • src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (5 hunks)
  • src/libs/identity/tools/discord.ts (1 hunks)
  • src/libs/network/manageNodeCall.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/libs/network/manageNodeCall.ts
  • src/libs/abstraction/web2/discord.ts
  • src/features/incentive/PointSystem.ts
  • .env.example
🧰 Additional context used
🧬 Code graph analysis (1)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (1)
src/libs/blockchain/gcr/gcr_routines/IncentiveManager.ts (1)
  • IncentiveManager (9-113)
🔇 Additional comments (2)
src/libs/identity/tools/discord.ts (1)

86-92: Confirm DM link support is intentionally excluded.

This requires guildId to be a snowflake; DM links use @me and will be rejected. Is excluding DMs desired?

src/libs/abstraction/web2/parsers.ts (1)

11-16: LGTM: expanded Discord prefixes cover common link sources.

Adding ptb, canary, and discordapp.com aligns with real-world URLs and the extractor.

Comment on lines +675 to +689
} else if (type === "discord") {
/**
* Check if this Discord userId exists anywhere
*/
const result = await gcrMainRepository
.createQueryBuilder("gcr")
.where(
"EXISTS (SELECT 1 FROM jsonb_array_elements(gcr.identities->'web2'->'discord') as discord_id WHERE discord_id->>'userId' = :userId)",
{
userId: data.userId,
},
)
.andWhere("gcr.pubkey != :currentAccount", { currentAccount })
.getOne()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Harden Discord first-connection query with COALESCE to avoid JSONB errors.

If web2.discord is absent or null on some accounts, jsonb_array_elements can error. Wrap with COALESCE(..., '[]'::jsonb). Consider applying similarly to Twitter/GitHub for consistency.

-                .where(
-                    "EXISTS (SELECT 1 FROM jsonb_array_elements(gcr.identities->'web2'->'discord') as discord_id WHERE discord_id->>'userId' = :userId)",
-                    {
-                        userId: data.userId,
-                    },
-                )
+                .where(
+                    "EXISTS (SELECT 1 FROM jsonb_array_elements(COALESCE(gcr.identities->'web2'->'discord', '[]'::jsonb)) AS discord_id WHERE discord_id->>'userId' = :userId)",
+                    { userId: data.userId },
+                )
📝 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
} else if (type === "discord") {
/**
* Check if this Discord userId exists anywhere
*/
const result = await gcrMainRepository
.createQueryBuilder("gcr")
.where(
"EXISTS (SELECT 1 FROM jsonb_array_elements(gcr.identities->'web2'->'discord') as discord_id WHERE discord_id->>'userId' = :userId)",
{
userId: data.userId,
},
)
.andWhere("gcr.pubkey != :currentAccount", { currentAccount })
.getOne()
} else if (type === "discord") {
/**
* Check if this Discord userId exists anywhere
*/
const result = await gcrMainRepository
.createQueryBuilder("gcr")
.where(
"EXISTS (SELECT 1 FROM jsonb_array_elements(COALESCE(gcr.identities->'web2'->'discord', '[]'::jsonb)) AS discord_id WHERE discord_id->>'userId' = :userId)",
{ userId: data.userId },
)
.andWhere("gcr.pubkey != :currentAccount", { currentAccount })
.getOne()
🤖 Prompt for AI Agents
In src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts around lines 675
to 689, the EXISTS query using jsonb_array_elements on
gcr.identities->'web2'->'discord' can fail when that path is null or absent;
wrap the expression in COALESCE(gcr.identities->'web2'->'discord', '[]'::jsonb)
inside jsonb_array_elements to default to an empty array and prevent errors, and
apply the same COALESCE pattern to the equivalent Twitter/GitHub queries for
consistency.

Comment on lines +56 to +63
this.axios = axios.create({
baseURL: this.api_url,
headers: {
Authorization: `Bot ${this.bot_token}`,
"Content-Type": "application/json",
},
})
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add network timeouts to prevent hanging calls.

External calls should have timeouts to avoid request pileups and stuck flows.

         this.axios = axios.create({
             baseURL: this.api_url,
             headers: {
                 Authorization: `Bot ${this.bot_token}`,
                 "Content-Type": "application/json",
             },
+            timeout: 10000, // 10s
         })
📝 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
this.axios = axios.create({
baseURL: this.api_url,
headers: {
Authorization: `Bot ${this.bot_token}`,
"Content-Type": "application/json",
},
})
}
this.axios = axios.create({
baseURL: this.api_url,
headers: {
Authorization: `Bot ${this.bot_token}`,
"Content-Type": "application/json",
},
timeout: 10000, // 10s
})
}
🤖 Prompt for AI Agents
In src/libs/identity/tools/discord.ts around lines 56 to 63, the axios instance
is created without any network timeout which can cause hanging requests; update
the axios.create call to include a sensible timeout (e.g. timeout: 5000) in the
config so requests fail fast, and ensure callers handle Axios timeout errors
(AxiosError.code === 'ECONNABORTED') appropriately; optionally also set a longer
retry/backoff policy elsewhere if needed.

@tcsenpai
Copy link
Contributor

@cwilvx

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (1)

671-683: Discord first-connection query hardened with COALESCE — mirror this for Twitter/GitHub

Nice use of COALESCE to avoid jsonb_array_elements on null. Apply the same pattern to the Twitter/GitHub queries for consistency and to avoid potential JSONB errors. This echoes a prior suggestion.

-                .where(
-                    "EXISTS (SELECT 1 FROM jsonb_array_elements(gcr.identities->'web2'->'twitter') as twitter_id WHERE twitter_id->>'userId' = :userId)",
-                    {
-                        userId: data.userId,
-                    },
-                )
+                .where(
+                    "EXISTS (SELECT 1 FROM jsonb_array_elements(COALESCE(gcr.identities->'web2'->'twitter', '[]'::jsonb)) AS twitter_id WHERE twitter_id->>'userId' = :userId)",
+                    { userId: data.userId },
+                )

-                .where(
-                    "EXISTS (SELECT 1 FROM jsonb_array_elements(gcr.identities->'web2'->'github') as github_id WHERE github_id->>'userId' = :userId)",
-                    {
-                        userId: data.userId,
-                    },
-                )
+                .where(
+                    "EXISTS (SELECT 1 FROM jsonb_array_elements(COALESCE(gcr.identities->'web2'->'github', '[]'::jsonb)) AS github_id WHERE github_id->>'userId' = :userId)",
+                    { userId: data.userId },
+                )
src/libs/identity/tools/discord.ts (1)

35-64: Nice: readonly secrets, allowlist, and timeout are in place.

Readonly envs + host allowlist + 10s timeout address earlier concerns.

🧹 Nitpick comments (6)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (2)

251-263: Discord first-connection + incentives: params look correct; minor consistency nit

The isFirst check and IncentiveManager call are correct. For consistency with walletLinked (which uses accountGCR.pubkey), consider passing accountGCR.pubkey instead of editOperation.account.

-                    await IncentiveManager.discordLinked(
-                        editOperation.account,
-                        editOperation.referralCode,
-                    )
+                    await IncentiveManager.discordLinked(
+                        accountGCR.pubkey,
+                        editOperation.referralCode,
+                    )

623-629: Type the isFirstConnection params more strictly

Broadened union to include "discord" is fine. To prevent runtime surprises, consider a discriminated union/overloads so userId is required for twitter/github/discord and omitted for web3.

Example (conceptual):

private static async isFirstConnection(
  type: "twitter" | "github" | "discord",
  data: { userId: string },
  ...
): Promise<boolean>;
private static async isFirstConnection(
  type: "web3",
  data: { chain: string; subchain: string; address: string },
  ...
): Promise<boolean>;
src/libs/identity/tools/discord.ts (4)

120-140: Harden rate-limit handling (retry_after body, jitter, cap attempts).

Discord often returns retry_after in body; add jitter and limit retries to avoid thundering herds.

Apply this diff:

-    private async get<T>(url: string, delay = 0): Promise<AxiosResponse<T>> {
-        if (delay > 0) {
-            await new Promise(r => setTimeout(r, delay))
-        }
-
-        try {
-            return await this.axios.get<T>(url)
-        } catch (e: any) {
-            if (e?.response?.status === 429) {
-                const retryAfter = Number(
-                    e.response.headers["retry-after"] ?? 1,
-                )
-                await new Promise(r =>
-                    setTimeout(r, Math.ceil(retryAfter * 1000)),
-                )
-                return await this.axios.get<T>(url)
-            }
-            throw e
-        }
-    }
+    private async get<T>(url: string): Promise<AxiosResponse<T>> {
+        let attempts = 0
+        for (;;) {
+            try {
+                return await this.axios.get<T>(url)
+            } catch (e: any) {
+                const status = e?.response?.status
+                if (status === 429 && attempts < 2) {
+                    const headers = e.response?.headers ?? {}
+                    const headerRetry = Number(headers["retry-after"])
+                    const bodyRetry =
+                        typeof e.response?.data?.retry_after === "number"
+                            ? e.response.data.retry_after
+                            : undefined
+                    const retryAfterSec = Number.isFinite(headerRetry)
+                        ? headerRetry
+                        : bodyRetry ?? 1
+                    const jitterMs = Math.floor(Math.random() * 250)
+                    await new Promise(r =>
+                        setTimeout(
+                            r,
+                            Math.ceil(retryAfterSec * 1000) + jitterMs,
+                        ),
+                    )
+                    attempts++
+                    continue
+                }
+                throw e
+            }
+        }
+    }

57-64: Add a descriptive User-Agent header.

Discord recommends meaningful User-Agent strings; helps with diagnostics.

Apply this diff:

         this.axios = axios.create({
             baseURL: this.api_url,
             headers: {
                 Authorization: `Bot ${this.bot_token}`,
                 "Content-Type": "application/json",
+                "User-Agent": "IdentityProofBot/1.0",
             },
             timeout: 10000, // 10s
         })

150-152: Remove unreachable status check.

Axios throws on non-2xx by default; the branch never runs.

Apply this diff:

-        if (res.status === 200) return res.data
-        throw new Error("Failed to get Discord message")
+        return res.data

4-29: Prefer official Discord API types.

Using discord-api-types/v10 improves compatibility and reduces drift.

Apply this diff:

+import type { APIMessage } from "discord-api-types/v10"
@@
-export type DiscordMessage = {
-    id: string
-    channel_id: string
-    guild_id?: string
-    author: {
-        id: string
-        username: string
-        global_name?: string
-        bot?: boolean
-    }
-    content: string
-    timestamp: string
-    edited_timestamp?: string | null
-    mention_everyone: boolean
-    attachments: Array<{
-        id: string
-        filename: string
-        size: number
-        url: string
-        proxy_url: string
-        content_type?: string
-    }>
-    embeds: any[]
-    mentions: Array<{ id: string; username: string }>
-    referenced_message?: DiscordMessage | null
-}
+export type DiscordMessage = APIMessage
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ab5dfa2 and cf48dad.

📒 Files selected for processing (2)
  • src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (5 hunks)
  • src/libs/identity/tools/discord.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (1)
src/libs/blockchain/gcr/gcr_routines/IncentiveManager.ts (1)
  • IncentiveManager (9-113)
🔇 Additional comments (6)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (4)

298-301: Scoping removedIdentity to GitHub only is appropriate

Capturing removedIdentity only for GitHub (where userId is needed downstream) is correct and avoids unnecessary reads for other contexts.


316-320: GitHub unlink gating is sound

The guard ensures a userId exists before calling githubUnlinked. Given the prior exists check, removedIdentity should be present in normal flows.


325-326: Discord unlink no longer gated by userId — good fix

This addresses the earlier issue where the deduction could be skipped.


667-671: GitHub first-connection branch retains semantics

Return logic unchanged and correct.

src/libs/identity/tools/discord.ts (2)

88-104: Clarify DM URL support (“@me”).

Current validation rejects DMs (guildId must be a snowflake). If that’s intentional, adjust the error to say “DM URLs (@me) are not supported.” If DMs should be supported, add an explicit branch.

I can update the parser either way once you confirm the requirement.

Also applies to: 95-102


2-2: Avoid Node-only 'url' import — use the global URL when bundling

Replace import { URL } from "url" in src/libs/identity/tools/discord.ts and rely on the global URL to avoid pulling Node built-ins into browser bundles; keep the import only if this module is strictly server-side. Repo search for client/UI imports was inconclusive — verify whether this module is imported by any browser bundles before changing.

Comment on lines +44 to +56
// Validate host to avoid accidental redirection to internal networks
const parsed = new URL(this.api_url)
const host = parsed.hostname.toLowerCase()
const isTrusted =
host === "localhost" ||
host.endsWith(".discord.com") ||
host === "discord.com" ||
host === "discordapp.com"

if (!isTrusted) {
throw new Error(`Untrusted DISCORD_API_URL host: ${host}`)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enforce HTTPS for non-local DISCORD_API_URL.

Prevent accidental plaintext transport; allow http only for local dev.

Apply this diff:

         const parsed = new URL(this.api_url)
-        const host = parsed.hostname.toLowerCase()
+        const host = parsed.hostname.toLowerCase()
         const isTrusted =
             host === "localhost" ||
             host.endsWith(".discord.com") ||
             host === "discord.com" ||
             host === "discordapp.com"
 
         if (!isTrusted) {
             throw new Error(`Untrusted DISCORD_API_URL host: ${host}`)
         }
+
+        // Require TLS unless explicitly local
+        const protocol = parsed.protocol
+        const isLocalHost =
+            host === "localhost" || host === "127.0.0.1" || host === "::1"
+        if (!isLocalHost && protocol !== "https:") {
+            throw new Error("DISCORD_API_URL must use https for non-local hosts")
+        }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/libs/identity/tools/discord.ts around lines 44 to 56, the host validation
allows non-HTTPS DISCORD_API_URL which can permit plaintext transport; update
validation to require parsed.protocol === "https:" for non-local hosts (allow
"http:" only when hostname is localhost or 127.0.0.1 or ::1). Concretely: after
parsing the URL and deriving host, determine if the host is local (localhost,
127.0.0.1, ::1) and if not local, assert parsed.protocol is "https:" and throw
an Error naming the protocol/host when the check fails; keep the existing
trusted-host checks intact.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/libs/identity/tools/discord.ts (1)

44-52: Enforce HTTPS and align trusted hosts with parser (also allow discordapp.com and local dev).

The constructor currently trusts only discord.com, while the URL parser allows discordapp.com. Also require TLS for non-local hosts to avoid downgrade risks.

Apply:

-        const host = parsed.hostname.toLowerCase()
-        const isTrusted =
-            host.endsWith(".discord.com") || host === "discord.com"
-
-        if (!isTrusted) {
-            throw new Error(`Untrusted DISCORD_API_URL host: ${host}`)
-        }
+        const host = parsed.hostname.toLowerCase()
+        const isLocalHost =
+            host === "localhost" || host === "127.0.0.1" || host === "::1"
+        const isTrusted =
+            isLocalHost ||
+            host === "discord.com" ||
+            host.endsWith(".discord.com") ||
+            host === "discordapp.com" ||
+            host.endsWith(".discordapp.com")
+        if (!isTrusted) {
+            throw new Error(`Untrusted DISCORD_API_URL host: ${host}`)
+        }
+        if (!isLocalHost && parsed.protocol !== "https:") {
+            throw new Error("DISCORD_API_URL must use https for non-local hosts")
+        }
🧹 Nitpick comments (8)
src/libs/identity/tools/discord.ts (8)

54-61: Add a descriptive User-Agent header.

Discord recommends a clear UA for bots; also helps observability.

         this.axios = axios.create({
             baseURL: this.api_url,
             headers: {
                 Authorization: `Bot ${this.bot_token}`,
                 "Content-Type": "application/json",
+                "User-Agent": "KynesysNode/DiscordClient",
             },
             timeout: 10000, // 10s
         })

86-93: Clarify URL format error and explicitly reject DM links.

Current error is vague; DM URLs use /channels/@me/... and will fail later—fail fast with a clearer message.

-            if (parts.length !== 4 || parts[0] !== "channels") {
-                throw new Error("Invalid Discord message URL format")
-            }
+            if (parts.length !== 4 || parts[0] !== "channels") {
+                throw new Error(
+                    "Invalid Discord message URL format. Expected /channels/{guildId}/{channelId}/{messageId}."
+                )
+            }
+            if (parts[1] === "@me") {
+                throw new Error("DM message URLs (/channels/@me/...) are not supported.")
+            }

92-102: Report which ID(s) are invalid to aid debugging.

-            const [_, guildId, channelId, messageId] = parts
-
-            if (
-                !this.isSnowflake(guildId) ||
-                !this.isSnowflake(channelId) ||
-                !this.isSnowflake(messageId)
-            ) {
-                throw new Error(
-                    "One or more IDs are not valid Discord snowflakes",
-                )
-            }
+            const [_, guildId, channelId, messageId] = parts
+            const invalid: string[] = []
+            if (!this.isSnowflake(guildId)) invalid.push("guildId")
+            if (!this.isSnowflake(channelId)) invalid.push("channelId")
+            if (!this.isSnowflake(messageId)) invalid.push("messageId")
+            if (invalid.length) {
+                throw new Error(`Invalid Discord snowflake(s): ${invalid.join(", ")}`)
+            }

105-112: Include the underlying error in the warning.

Helps triage malformed links quickly.

-            console.warn("Failed to extract details from Discord URL")
+            console.warn("Failed to extract details from Discord URL", err)

115-118: Snowflake regex nit.

Discord snowflakes are 17–19 digits today; tightening avoids accidental long IDs. Optional.

-        return /^\d{17,20}$/.test(id)
+        return /^\d{17,19}$/.test(id)

120-140: Harden rate-limit/timeout handling with capped retries and better headers.

Respect x-ratelimit-reset-after, retry timeouts once, cap retries.

-    private async get<T>(url: string, delay = 0): Promise<AxiosResponse<T>> {
-        if (delay > 0) {
-            await new Promise(r => setTimeout(r, delay))
-        }
-
-        try {
-            return await this.axios.get<T>(url)
-        } catch (e: any) {
-            if (e?.response?.status === 429) {
-                const retryAfter = Number(
-                    e.response.headers["retry-after"] ?? 1,
-                )
-                await new Promise(r =>
-                    setTimeout(r, Math.ceil(retryAfter * 1000)),
-                )
-                return await this.axios.get<T>(url)
-            }
-            throw e
-        }
-    }
+    private async get<T>(url: string, delay = 0): Promise<AxiosResponse<T>> {
+        const maxRetries = 2
+        for (let attempt = 0; attempt <= maxRetries; attempt++) {
+            if (delay > 0 && attempt === 0) {
+                await new Promise(r => setTimeout(r, delay))
+            }
+            try {
+                return await this.axios.get<T>(url)
+            } catch (e: any) {
+                const status = e?.response?.status
+                if (status === 429 && attempt < maxRetries) {
+                    const h = e.response.headers || {}
+                    const resetAfter =
+                        Number(h["x-ratelimit-reset-after"] ?? h["retry-after"] ?? 1)
+                    const ms = Math.ceil((isNaN(resetAfter) ? 1 : resetAfter) * 1000)
+                    await new Promise(r => setTimeout(r, ms))
+                    continue
+                }
+                if (e?.code === "ECONNABORTED" && attempt < maxRetries) {
+                    await new Promise(r => setTimeout(r, 250))
+                    continue
+                }
+                throw e
+            }
+        }
+        // Unreachable, but satisfies type system
+        throw new Error("GET failed after retries")
+    }

142-152: Redundant status check; let Axios throw on non-2xx.

Simplify and surface original Axios error details to callers.

-        if (res.status === 200) return res.data
-        throw new Error("Failed to get Discord message")
+        return res.data

26-27: Prefer unknown[] over any[] for embeds.

Avoids accidental unsafe usage; callers can refine as needed.

-    embeds: any[]
+    embeds: unknown[]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cf48dad and 9036b99.

📒 Files selected for processing (1)
  • src/libs/identity/tools/discord.ts (1 hunks)
🔇 Additional comments (1)
src/libs/identity/tools/discord.ts (1)

31-38: LGTM: Singleton with readonly config is clean.

Good encapsulation; readonly fields prevent mutation and the lazy singleton is straightforward.

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.

3 participants