Conversation
|
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “Anti-Phish” capability for the moderation module by adding a phishing detection service (heuristics + optional external lookups/ML stub), wiring it into message handling, and exposing new moderation config/strings for customization and logging.
Changes:
- Added
phishingService+ supportingphishingHeuristics/phishingConfigmodules for URL phishing scoring. - Integrated phishing checks into moderation lifecycle (
messageCreate,botReady) and added new config + string entries. - Added Jest unit tests plus several ad-hoc local test/simulation scripts.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
modules/moderation/phishingService.js |
Core phishing scoring logic, async DNS/redirect checks, optional external API lookups, optional ML model hook |
modules/moderation/phishingHeuristics.js |
Shared helper functions (IP detection, keyword checks, domain similarity, IDN checks, URL extraction) |
modules/moderation/phishingConfig.js |
Centralized default weights/thresholds and domain/keyword lists |
modules/moderation/events/messageCreate.js |
Hooks phishing detection into scam-link moderation flow (and optional logging) |
modules/moderation/events/botReady.js |
Loads custom phishing patterns from moderation config at startup |
modules/moderation/configs/config.json |
Adds new moderation config schema fields for phishing (log channel + custom patterns) |
modules/moderation/configs/strings.json |
Adds a configurable phishing log message template |
modules/moderation/__tests__/phishingService.test.js |
Adds unit/integration-style tests for heuristics + checkPhishing |
package.json |
Adds dependencies/scripts needed for phishing service and tests |
README.md |
Documents the new phishing detection and test command |
test-phishing.js |
Manual test script for checkPhishing |
quick-test.js |
Quick manual URL checks against checkPhishing |
simulate-phish.js |
Standalone simulation script approximating detection logic |
anti-phish.code-workspace |
VS Code workspace file added to repo |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // asynchronous checks – DNS + remote lookups | ||
| const asyncTasks = []; | ||
| asyncTasks.push( | ||
| dns.resolve(hostname) | ||
| .then(arr => { | ||
| if (!arr || arr.length === 0) { | ||
| riskScore += cfg.weights.dnsFailure; | ||
| reasons.push('No DNS resolution'); | ||
| } | ||
| }) | ||
| .catch(() => { | ||
| riskScore += cfg.weights.dnsFailure; | ||
| reasons.push('DNS resolution failed'); | ||
| features.push(1); | ||
| }) | ||
| ); | ||
|
|
||
| // redirect head check | ||
| asyncTasks.push( | ||
| axios.head(url, { maxRedirects: 5, timeout: 3000 }) | ||
| .then(resp => { | ||
| if (resp.request.res.responseUrl && resp.request.res.responseUrl !== url) { | ||
| riskScore += cfg.weights.redirect; | ||
| reasons.push('URL redirects'); |
There was a problem hiding this comment.
checkPhishing performs outbound network requests (DNS resolve and HTTP HEAD) on user-supplied URLs. In a bot context this is an SSRF risk (probing internal services / metadata endpoints). Consider making network lookups opt-in and/or blocking private, loopback, and link-local IP ranges after resolution before issuing requests.
| <<<<<<< HEAD | ||
| // built-in phishing service (uses ML/heuristics plus configurable patterns) | ||
| const { checkPhishing } = require('../phishingService'); | ||
| ======= | ||
| >>>>>>> bdf48c957889f18888d1525806101cb792e35246 |
There was a problem hiding this comment.
Unresolved Git merge conflict markers are committed in this JS file; this will cause a syntax error at runtime. Resolve the conflict and remove the conflict marker lines.
| <<<<<<< HEAD | |
| // built-in phishing service (uses ML/heuristics plus configurable patterns) | |
| const { checkPhishing } = require('../phishingService'); | |
| ======= | |
| >>>>>>> bdf48c957889f18888d1525806101cb792e35246 | |
| // built-in phishing service (uses ML/heuristics plus configurable patterns) | |
| const { checkPhishing } = require('../phishingService'); |
| <<<<<<< HEAD | ||
| "name": "phishing-custom-patterns", | ||
| "humanName": { | ||
| "de": "Eigene Phishing-Muster", | ||
| "en": "Custom phishing patterns" | ||
| }, | ||
| "default": { | ||
| "en": [], | ||
| "de": [] | ||
| }, | ||
| "description": { | ||
| "en": "Add your own regexes/keywords/strings to flag as phishing (one entry per line, RegExp syntax supported)", | ||
| "de": "Füge eigene Regexe/Stichwörter/String hinzu, die als Phishing markiert werden sollen (jeweils eine pro Zeile, RegExp-Syntax möglich)" | ||
| }, | ||
| "type": "array", | ||
| "content": "string" | ||
| }, | ||
| { | ||
| ======= | ||
| >>>>>>> bdf48c957889f18888d1525806101cb792e35246 | ||
| "name": "action_on_posting_blacklisted_word", |
There was a problem hiding this comment.
Additional merge conflict markers are present later in the same JSON file, leaving it invalid. Resolve this conflict block as well so the moderation config schema remains valid JSON.
| <<<<<<< HEAD | ||
| "stop-discord-phishing": "0.3.3", | ||
| "express": "^4.18.2", | ||
| "axios": "^1.4.0", | ||
| "url-parse": "^1.5.10", |
There was a problem hiding this comment.
Unresolved merge conflict markers in the dependencies section will break package.json parsing. Resolve the conflict and ensure only one coherent dependencies/devDependencies block remains.
| "express": "^4.18.2", | ||
| "axios": "^1.4.0", | ||
| "url-parse": "^1.5.10", | ||
| "tldjs": "^2.3.2", | ||
| "string-similarity": "^4.0.4" |
There was a problem hiding this comment.
New dependencies are added with caret (^) ranges, but the rest of this repo pins exact versions. After resolving the merge conflict, consider pinning these versions too for reproducible installs, and drop unused dependencies like url-parse (no references in the codebase).
| @@ -0,0 +1,14 @@ | |||
| const { checkPhishing, setCustomPatterns } = require('./modules/moderation/phishingService'); | |||
There was a problem hiding this comment.
setCustomPatterns is imported but never used in this script. Remove the unused import to keep the quick test minimal and avoid linter/IDE warnings.
| const { checkPhishing, setCustomPatterns } = require('./modules/moderation/phishingService'); | |
| const { checkPhishing } = require('./modules/moderation/phishingService'); |
| { | ||
| "folders": [ | ||
| { | ||
| "path": "." | ||
| } | ||
| ], | ||
| "settings": {} | ||
| } No newline at end of file |
There was a problem hiding this comment.
Editor workspace files are environment-specific and typically shouldn't be committed to the repository. Consider removing this file and adding *.code-workspace to .gitignore instead.
| { | |
| "folders": [ | |
| { | |
| "path": "." | |
| } | |
| ], | |
| "settings": {} | |
| } |
| <<<<<<< HEAD | ||
| "generate-template": "node generate-template.js", | ||
| "test": "jest" | ||
| ======= | ||
| "generate-template": "node generate-template.js" | ||
| >>>>>>> bdf48c957889f18888d1525806101cb792e35246 |
There was a problem hiding this comment.
Unresolved Git merge conflict markers are present in package.json, which makes the file invalid JSON and will break npm installs. Resolve the conflict and keep the intended script entries.
| <<<<<<< HEAD | ||
| "name": "phishing-log-channel-id", | ||
| "humanName": { | ||
| "de": "Phishing-Protokoll-Kanal", | ||
| "en": "Phishing Log Channel" | ||
| }, | ||
| "default": { | ||
| "en": "", | ||
| "de": "" | ||
| }, | ||
| "description": { | ||
| "en": "Optional channel where auto phishing detections are logged.", | ||
| "de": "Optionaler Kanal, in dem automatische Phishing-Erkennungen protokolliert werden." | ||
| }, | ||
| "type": "channelID", | ||
| "allowNull": true | ||
| }, | ||
| { | ||
| ======= | ||
| >>>>>>> bdf48c957889f18888d1525806101cb792e35246 | ||
| "name": "scam_link_level", |
There was a problem hiding this comment.
Merge conflict markers are present in this JSON file, making it invalid JSON and likely breaking config generation/loading. Resolve the conflict and remove the marker lines.
| const { URL } = require('url'); | ||
|
|
There was a problem hiding this comment.
This import is unused in this module. Removing it avoids confusion and keeps the heuristics module focused on what it actually uses.
| const { URL } = require('url'); |
SCDerox
left a comment
SCDerox
left a comment
There was a problem hiding this comment.
Hi, thanks so much for your contribution.
Please address the following points:
- Please disclose whether Generative Large Language Models ("Generative AI", e.g. Coplilot) were used and to what extent to write this code. In addition, please confirm that you have reviewed and tested any generated code thoroughly. For details, please review our AI Policy.
- Your submission includes large parts of code that is syntactically wrong. Because of this, we have doubts about what extent of testing has been done. Please outline what testing you have completed, including any end-to-end-tests of the bot working.
- Please address Copilot review comments.
- Your code adds new files that are module specific to the root directory. This is against established practice in this project, please refactor.
No code review has yet been done by our team due to the above-mentioned comments.
Hiya.
This is a brand new config that could be fully integrated without much hassle to include an Anti-Phishing system. It can be modified to the user's choice, whether they want it to be overpowered in a sense, or pretty simple. It, in my testing at least, can just detect links that seem harmful. Further testing may need to be done to ensure its bulletproof, but it does work.
Can also detect link shorteners as well.