fix: code review fixes

Signed-off-by: Mouad BANI <mouad-mb@outlook.com>

Author: mbani01

services/apps/packages_worker/src/security-contacts/extractors/registry/npm.ts

@@ -10,7 +10,7 @@ const SOURCE = 'npm-registry'
 function npmPackagePath(parsed: ParsedPurl): string {
   const full = parsed.namespace ? `${parsed.namespace}/${parsed.name}` : parsed.name
   // Scoped packages must percent-encode the slash for the registry path.
-  return full.startsWith('@') ? full.replace('/', '%2F') : full
+  return full.startsWith('@') ? full.replaceAll('/', '%2F') : full
 }
 
 export function mapNpm(doc: unknown, sourceUrl: string, fetchedAt: string): RawContact[] {

services/apps/packages_worker/src/security-contacts/extractors/securityInsights.ts

@@ -45,6 +45,22 @@ function classifyValue(raw: string): { channel: ContactChannel; value: string }
   return { channel: 'github-handle', value: v }
 }
 
+// project-si-source may only point at trusted raw-content hosts (SSRF guard).
+const REDIRECT_ALLOWED_HOSTS = new Set([
+  'raw.githubusercontent.com',
+  'gist.githubusercontent.com',
+  'gitlab.com',
+])
+
+function isAllowedRedirect(url: string): boolean {
+  try {
+    const u = new URL(url)
+    return u.protocol === 'https:' && REDIRECT_ALLOWED_HOSTS.has(u.hostname.toLowerCase())
+  } catch {
+    return false
+  }
+}
+
 // MAINTAINERS.md / OWNERS.md links are documents, not contacts.
 function isDocumentUrl(value: string): boolean {
   const v = value.toLowerCase()
@@ -211,7 +227,9 @@ export const extractSecurityInsights: Extractor = async (target, deps) => {
     }
 
     const redirect = (doc as any)?.header?.['project-si-source']
-    if (typeof redirect === 'string' && /^https?:\/\//i.test(redirect)) {
+    // Only follow redirects to trusted raw-content hosts — a repo-controlled URL must not
+    // be able to point the worker at internal/metadata endpoints (SSRF).
+    if (typeof redirect === 'string' && isAllowedRedirect(redirect)) {
       const redirected = await fetchText(redirect, deps.fetchTimeoutMs)
       if (redirected.text) return parseSecurityInsights(redirected.text, redirect, fetchedAt)
     }

services/apps/packages_worker/src/security-contacts/extractors/securityMd.ts

@@ -90,8 +90,8 @@ export function parseSecurityMd(
     }
   }
 
-  // PVR redirect language corroborates A2. Emitted unconditionally here; the pipeline
-  // vetoes it when A2 reports PVR disabled. TODO(CM-1243): apply the A2 veto in Step 6.
+  // PVR redirect language corroborates A2. Emitted unconditionally; processBatch vetoes it
+  // when A2 authoritatively reports PVR disabled.
   if (PVR_RE.test(text)) {
     add('github-pvr', `https://github.com/${owner}/${name}/security/advisories/new`)
   }

services/apps/packages_worker/src/security-contacts/processBatch.ts

@@ -56,7 +56,7 @@ async function fetchBatch(qx: QueryExecutor, config: Config): Promise<SweepRow[]
     FROM repos r
     JOIN package_repos pr ON pr.repo_id = r.id
     JOIN packages p ON p.id = pr.package_id AND p.is_critical
-    WHERE r.host IN ('github', 'github.com')
+    WHERE r.host = 'github'
       AND (
         r.contacts_last_refreshed IS NULL
         OR r.contacts_last_refreshed < NOW() - INTERVAL '$(updateIntervalHours) hours'
@@ -97,6 +97,13 @@ async function processRepo(
   // One failing extractor must not sink the repo.
   const results = await Promise.allSettled(EXTRACTORS.map((extract) => extract(target, deps)))
 
+  // If every extractor failed (e.g. a transient network outage), skip the write entirely so we
+  // don't delete previously good contacts/policies; leaving contacts_last_refreshed retries next sweep.
+  if (results.every((r) => r.status === 'rejected')) {
+    log.warn({ repoId: target.repoId }, 'All extractors failed — skipping write to preserve data')
+    return
+  }
+
   let contacts: RawContact[] = []
   const policies: Partial<RepoPolicies> = {}
   for (const r of results) {

services/apps/packages_worker/src/security-contacts/writeContacts.ts

@@ -4,8 +4,8 @@ import { RepoPolicies, ScoredContact } from './types'
 
 /**
  * Idempotent per-repo recompute: replace the repo's security_contacts rows and refresh
- * the policy columns in one transaction. pvr_enabled uses COALESCE so an "unknown" result
- * (A2 endpoint failure) never clears a previously known value.
+ * the policy columns in one transaction. Policy columns use COALESCE so a run that doesn't
+ * rediscover a field (partial/failed extractor pass) never clears a previously known value.
  */
 export async function writeContacts(
   qx: QueryExecutor,
@@ -36,11 +36,13 @@ export async function writeContacts(
     }
 
     await tx.result(
+      // COALESCE preserves previously stored values when a run doesn't (re)discover a field —
+      // a partial/failed extractor pass must not wipe still-valid policy URLs or the PVR flag.
       `UPDATE repos SET
-         security_policy_url         = $(securityPolicyUrl),
-         vulnerability_reporting_url = $(vulnerabilityReportingUrl),
-         bug_bounty_url              = $(bugBountyUrl),
-         security_txt_url            = $(securityTxtUrl),
+         security_policy_url         = COALESCE($(securityPolicyUrl), security_policy_url),
+         vulnerability_reporting_url = COALESCE($(vulnerabilityReportingUrl), vulnerability_reporting_url),
+         bug_bounty_url              = COALESCE($(bugBountyUrl), bug_bounty_url),
+         security_txt_url            = COALESCE($(securityTxtUrl), security_txt_url),
          pvr_enabled                 = COALESCE($(pvrEnabled), pvr_enabled),
          contacts_last_refreshed     = NOW()
        WHERE id = $(repoId)`,