feat(security-contacts): emails reachability & noise filtering [CM-1324]#4353
Conversation
PR SummaryMedium Risk Overview Refactors the packages worker so extraction returns Also derives GitHub handles from noreply emails for CDP resolution, tightens CDP email provenance, and adds Reviewed by Cursor Bugbot for commit b063400. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds email-reachability classification and filters unreachable security contacts from package details.
Changes:
- Classifies invalid, automated, disposable, and placeholder emails.
- Persists reachability metadata during reconciliation.
- Filters package security contacts by reachability.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
services/libs/data-access-layer/src/osspckgs/api.ts |
Filters unreachable contacts. |
services/libs/common/src/email.ts |
Adds reachability heuristics. |
services/libs/common/src/constants/index.ts |
Exports disposable domains. |
services/apps/packages_worker/src/security-contacts/writeContacts.ts |
Persists classification results. |
services/apps/packages_worker/src/security-contacts/types.ts |
Extends contact types. |
services/apps/packages_worker/src/security-contacts/reconcile.ts |
Classifies reconciled contacts. |
backend/src/osspckgs/migrations/V1784073600__security_contacts_reachability.sql |
Adds reachability columns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
1ec9d67 to
22339ca
Compare
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | |||
| @@ -1,2 +1,3 @@ | |||
| export * from './email-providers' | |||
| export * from './bots' | |||
| export * from './disposable-email-domains' | |||
| SELECT channel, value, role, confidence, score | ||
| FROM security_contacts | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL AND reachable |
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | |||
| SELECT channel, value, role, confidence, score | ||
| FROM security_contacts | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL AND reachable |
| if (BOT_EXACT_LOCAL_PARTS.has(lp)) return true | ||
| // 'bot' as a delimited token: bot-*, *-bot, *.bot, bot_* ... | ||
| return /(^|[.\-_+])bot([.\-_+]|$)/.test(lp) | ||
| } |
There was a problem hiding this comment.
Role-prefixed bot local parts
Medium Severity
The bot heuristic treats any local part ending in -bot as unreachable, but role exemptions only apply to exact local parts like security. Addresses such as security-bot@company.com match the -bot token rule and are marked unreachable even though they are plausible human security contacts.
Reviewed by Cursor Bugbot for commit 9db3785. Configure here.
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | |||
| 'example.com', | ||
| 'example.org', | ||
| 'example.net', | ||
| 'example.edu', |
| 'actions', | ||
| ]) | ||
|
|
||
| const emailDomain = (value: string): string => value.slice(value.indexOf('@') + 1) |
| SELECT channel, value, role, confidence, score | ||
| FROM security_contacts | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL AND reachable |
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
| } finally { | ||
| clearInterval(heartbeatTimer) | ||
| } |
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | |||
…s-tag noreply detection Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
| const PLACEHOLDER_EMAIL_DOMAINS = new Set([ | ||
| 'example.com', | ||
| 'example.org', | ||
| 'example.net', | ||
| 'example.edu', | ||
| ]) |
| } finally { | ||
| clearInterval(heartbeatTimer) | ||
| } |
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | |||
| const scored: ScoredContact[] = merged.map((c) => { | ||
| const contact = { ...c, provenance: dedupeProvenance(c.provenance) } | ||
| return { ...contact, ...scoreContact(contact, now) } | ||
| return { ...contact, ...scoreContact(contact, now), ...classifyReachability(contact) } |
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | ||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachability_reason TEXT; |
| // 2-minute heartbeat timeout just like extraction can, and letting the timer stop early would | ||
| // let Temporal retry this activity while the original attempt is still writing. | ||
| const writeStartedAt = Date.now() | ||
| await writeContactsBatch(qx, outcomes) |
There was a problem hiding this comment.
Cancel drops extracted batch work
Medium Severity
Batch processing now collects every processRepo outcome and only calls writeContactsBatch after all concurrent extractions finish. If the activity is cancelled (or mapWithConcurrency aborts) during extraction, none of the completed repos are persisted and markReposAttempted never runs for extractor failures, unlike the previous per-repo writes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d1e868b. Configure here.
| const lp = localPart.toLowerCase() | ||
| if (ROLE_LOCAL_PARTS.has(lp)) return false |
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | |||
| for (let i = 0; i < ok.length; i += WRITE_CHUNK_SIZE) { | ||
| const chunk = ok.slice(i, i + WRITE_CHUNK_SIZE) | ||
| try { | ||
| await writeContactsChunk(qx, chunk) |
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
| export async function processRepo( | ||
| target: RepoTarget, | ||
| baseDeps: Omit<ExtractorDeps, 'repoTree'>, | ||
| qx: QueryExecutor, |
There was a problem hiding this comment.
Sweep ignores reachable-only contacts
Medium Severity
The batch sweep treats any non-deleted security_contacts row as “has contacts” for weekly refresh, but this change persists email rows with reachable = false and package reads only return reachable contacts. Repos whose only stored rows are unreachable emails can look enriched to the sweep while the API shows no contacts, so they skip the daily retry path for up to a week.
Reviewed by Cursor Bugbot for commit 8de3130. Configure here.
| const handle = parseGitHubNoreplyEmail(c.value) | ||
| if (!handle) continue |
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | |||
| const scored: ScoredContact[] = merged.map((c) => { | ||
| const contact = { ...c, provenance: dedupeProvenance(c.provenance) } | ||
| return { ...contact, ...scoreContact(contact, now) } | ||
| return { ...contact, ...scoreContact(contact, now), ...classifyReachability(contact) } |
| CREATE INDEX IF NOT EXISTS repos_contacts_last_refreshed_idx | ||
| ON repos (contacts_last_refreshed); |
| SELECT channel, value, role, confidence, score | ||
| FROM security_contacts | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL AND reachable |
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | |||
| const PLACEHOLDER_EMAIL_DOMAINS = new Set([ | ||
| 'example.com', | ||
| 'example.org', | ||
| 'example.net', | ||
| 'example.edu', | ||
| ]) |
| 'actions', | ||
| ]) | ||
|
|
||
| const emailDomain = (value: string): string => value.slice(value.indexOf('@') + 1) |
| const scored: ScoredContact[] = merged.map((c) => { | ||
| const contact = { ...c, provenance: dedupeProvenance(c.provenance) } | ||
| return { ...contact, ...scoreContact(contact, now) } | ||
| return { ...contact, ...scoreContact(contact, now), ...classifyReachability(contact) } |
| if (c.channel !== 'email') continue | ||
| const handle = parseGitHubNoreplyEmail(c.value) | ||
| if (!handle) continue |
| SELECT channel, value, role, confidence, score | ||
| FROM security_contacts | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL AND reachable |
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | ||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachability_reason TEXT; |
| for (let i = 0; i < ok.length; i += WRITE_CHUNK_SIZE) { | ||
| const chunk = ok.slice(i, i + WRITE_CHUNK_SIZE) | ||
| try { | ||
| await writeContactsChunk(qx, chunk) |
| if (c.channel !== 'email') return { reachable: true, reachabilityReason: null } | ||
| const r = classifyEmailReachability(c.value) | ||
| return { reachable: r.reachable, reachabilityReason: r.reason } |
| @@ -1,2 +1,3 @@ | |||
| export * from './email-providers' | |||
| export * from './bots' | |||
| export * from './disposable-email-domains' | |||
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
| handle: contact.value, | ||
| tier: contact.tier, | ||
| provenance: [{ source, sourceTier: contact.tier, fetchedAt }], | ||
| provenance: [...contact.provenance, { source, sourceTier: contact.tier, fetchedAt }], |
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | |||
| CREATE INDEX IF NOT EXISTS repos_contacts_last_refreshed_idx | ||
| ON repos (contacts_last_refreshed); |
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | ||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachability_reason TEXT; |
| @@ -1,2 +1,3 @@ | |||
| export * from './email-providers' | |||
| export * from './bots' | |||
| export * from './disposable-email-domains' | |||
| SELECT channel, value, role, confidence, score | ||
| FROM security_contacts | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL AND reachable |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
Reviewed by Cursor Bugbot for commit b063400. Configure here.
| ], | ||
| }) | ||
| } | ||
| return derived |
There was a problem hiding this comment.
GitLab noreply handles not derived
Medium Severity
deriveGithubHandlesFromNoreplyEmails only parses GitHub noreply addresses. GitLab @users.noreply.gitlab.com emails are classified unreachable elsewhere but never get a derived handle for CDP resolution, so GitLab-sourced noreply contacts miss the recovery path GitHub noreply contacts receive.
Reviewed by Cursor Bugbot for commit b063400. Configure here.
| contacts = contacts.filter((c) => c.channel !== 'github-pvr') | ||
| } | ||
|
|
||
| contacts.push(...deriveGithubHandlesFromNoreplyEmails(contacts)) |
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | |||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachable BOOLEAN NOT NULL DEFAULT TRUE; | ||
| ALTER TABLE security_contacts ADD COLUMN IF NOT EXISTS reachability_reason TEXT; |
| import validator from 'validator' | ||
|
|
||
| import { noreplyEmailProviders } from './constants' | ||
| import { disposableEmailDomains, emailSafeKnownBots, noreplyEmailProviders } from './constants' |
| // Extraction is collected here and persisted afterward in one batched call (below) rather | ||
| // than per-repo, since per-repo writes meant up to CONCURRENCY concurrent transactions | ||
| // against a packages-db pool sized for far fewer connections — the actual sweep bottleneck. | ||
| outcomes = await mapWithConcurrency(targets, CONCURRENCY, async (target) => { |
| SELECT channel, value, role, confidence, score | ||
| FROM security_contacts | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL | ||
| WHERE repo_id = pr.repo_id AND deleted_at IS NULL AND reachable |
This pull request introduces a comprehensive system for classifying and tracking the reachability of security contact email addresses. It adds new database fields, implements email reachability heuristics, and ensures that only reachable contacts are considered in package detail queries. The most important changes are grouped below:
Database schema updates:
reachable(boolean) andreachability_reason(text) columns to thesecurity_contactstable to store email reachability status and the reason for unreachability.Email reachability classification:
classifyEmailReachabilityfunction inemail.ts, which uses heuristics to determine if an email is reachable, and if not, provides a specific reason (e.g., invalid, noreply, bot, disposable, etc.).EmailReachabilityReasontype and supporting constants for classifying email addresses. [1] [2] [3]Contact processing and storage:
ScoredContacttype and database writes. [1] [2] [3] [4] [5] [6]Query logic update: