Skip to content

feat: add refresh security contats api (CM-1348)#4393

Merged
ulemons merged 4 commits into
mainfrom
feat/security-contacts-refresh
Jul 24, 2026
Merged

feat: add refresh security contats api (CM-1348)#4393
ulemons merged 4 commits into
mainfrom
feat/security-contacts-refresh

Conversation

@ulemons

@ulemons ulemons commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds POST /akrites-external/contacts/ingest, an on-demand security-contacts ingest endpoint. CM-1313 shipped the worker side (ingestSecurityContactsForPurlWorkflow), whose docblock already names "the akrites API on a cache miss" as the intended caller, but nothing in the backend actually invoked it. This wires that gap: given a single purl, it triggers the Temporal workflow synchronously, waits for it to finish (~10-20s), and returns the same ContactDetail payload as GET /akrites-external/contacts/detail.

Changes

  • New handler ingestAkritesExternalContactDetail calls packagesTemporal.workflow.execute(...) and re-reads via getContactDetailsByPurls once the workflow resolves, returning NotFoundError if the workflow reports no linked repo or the re-read comes back empty.
  • Uses a deterministic workflowId (security-contacts-ondemand:<sha256(purl)>) with workflowIdConflictPolicy: USE_EXISTING + workflowIdReusePolicy: ALLOW_DUPLICATE, so concurrent refresh requests for the same purl attach to one running workflow instead of each starting their own.
  • This is the first call site in the codebase using workflow.execute() (start + await result) rather than the fire-and-forget workflow.start() used everywhere else — intentional here since the endpoint is explicitly synchronous by spec, but worth flagging in review since it's a new pattern.
  • Added purlBodySchema (single-purl body, validated via validateOrThrow) alongside the existing array-based purlsBodySchema.
  • Dedicated, env-tunable rate limiter (AKRITES_CONTACT_INGEST_RATE_LIMIT_MAX / _WINDOW_MS, default 20/hr) separate from the shared read-endpoint limiter, since this endpoint blocks far longer and triggers real external side effects (GitHub calls, DB writes) on every call. Refactored the existing blast-radius limiter to share the same envTunableRateLimiter helper.
  • Scoped behind READ_MAINTAINER_ROLES, matching /contacts/detail.
  • No batch variant by design — batching purls here would multiply concurrent Temporal workflow starts (amplification risk).
  • OpenAPI updated with the new route, its latency, rate-limit envs, and a note that this should move to an async job pattern in the future.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Performance improvement
  • Chore / dependency update
  • Documentation

JIRA ticket

CM-1348

ulemons added 2 commits July 24, 2026 12:02
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
@ulemons ulemons self-assigned this Jul 24, 2026
@ulemons ulemons added the Feature Created by Linear-GitHub Sync label Jul 24, 2026
Copilot AI review requested due to automatic review settings July 24, 2026 11:51
@cursor

cursor Bot commented Jul 24, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
New authenticated endpoint that blocks on Temporal, writes contact data via GitHub-backed workflows, and exposes maintainer-scoped PII—mitigated by strict rate limits and an ingest-once gate, but long-running sync calls and first use of workflow.execute are operational risks.

Overview
Adds POST /akrites-external/contacts/ingest so Akrites can trigger a single-PURL, synchronous security-contacts ingest and get the same ContactDetail shape as the read endpoints.

The handler short-circuits when contactsLastRefreshed is set (no workflow), returns 404 for unknown purls or packages without a resolved repo, otherwise workflow.execute on ingestSecurityContactsForPurlWorkflow with a SHA-256–derived workflow ID and USE_EXISTING so concurrent requests for one purl share one run, then re-reads via getContactDetailsByPurls. contactsLastRefreshed is exposed on the contact DAL row/query for that gate.

Routing/rate limits: contacts routes use per-route limiters (ingest before scope check); ingest gets a dedicated env-tunable limiter (default 20/hr). Blast-radius workflow limiting is refactored to share envTunableRateLimiter. purlBodySchema, OpenAPI, and unit tests cover the new path.

Reviewed by Cursor Bugbot for commit a8f78cd. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a synchronous Akrites endpoint that triggers security-contact ingestion through the packages Temporal worker and returns refreshed contact details.

Changes:

  • Adds single-PURL validation and ingestion handler.
  • Adds deterministic workflow execution and dedicated rate limiting.
  • Documents and tests the new API.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
services/libs/types/src/enums/temporal.ts Adds the workflow ID prefix.
backend/src/api/public/v1/packages/purl.ts Adds single-PURL body validation.
backend/src/api/public/v1/packages/ingestAkritesExternalContactDetail.ts Implements synchronous workflow execution and response lookup.
backend/src/api/public/v1/packages/ingestAkritesExternalContactDetail.test.ts Tests handler behavior and workflow options.
backend/src/api/public/v1/akrites-external/openapi.yaml Documents the endpoint contract.
backend/src/api/public/v1/akrites-external/index.ts Registers the route and rate limiter.
Comments suppressed due to low confidence (2)

backend/src/api/public/v1/packages/ingestAkritesExternalContactDetail.ts:55

  • found only means that a linked repository exists; it does not mean ingestion succeeded. The worker catches processing/write failures and still returns { found: true } (ingestSingle.ts:95-109), while this re-read returns a package row even with no contacts (api.ts:873-915). Consequently the endpoint can return 200 with stale or empty data after a failed ingest. Return an explicit outcome from the workflow and reject/mark partial failures before reporting success.
  if (!result.found) {
    throw new NotFoundError('Purl has no linked repository to ingest security contacts from')
  }

backend/src/api/public/v1/akrites-external/openapi.yaml:1274

  • This repeats the 10–20-second limit even though retries can take about 95 seconds after pickup and scheduling time is not bounded by a schedule-to-close or workflow execution timeout.
        Not yet implemented: this is a synchronous, blocking call. Future work should
        make this async — an accept-and-poll job, similar to POST /blast-radius/jobs —
        so callers aren't held open for 10-20s per request.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread backend/src/api/public/v1/packages/ingestAkritesExternalContactDetail.ts Outdated
Comment thread backend/src/api/public/v1/akrites-external/index.ts
Comment thread backend/src/api/public/v1/akrites-external/openapi.yaml Outdated
Comment thread backend/src/api/public/v1/packages/ingestAkritesExternalContactDetail.ts Outdated
Comment thread backend/src/api/public/v1/akrites-external/openapi.yaml Outdated
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings July 24, 2026 14:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

backend/src/api/public/v1/packages/ingestAkritesExternalContactDetail.test.ts:78

  • This second fixture override also supplies a string for the Date | null contactsLastRefreshed field, so the test remains invalid under TypeScript checking.
      .mockResolvedValueOnce([baseRow({ contactsLastRefreshed: '2024-01-01T00:00:00.000Z' })])

Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings July 24, 2026 14:14

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a8f78cd. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

backend/src/api/public/v1/packages/ingestAkritesExternalContactDetail.ts:64

  • These settings do not guarantee one writer per repo. ALLOW_DUPLICATE starts another run when a second request read the null timestamp before the first run completed but reaches execute afterward; moreover, different PURLs linked to the same repo get different workflow IDs, and the daily workflow has another ID. Those executions can overlap even though writeContacts explicitly assumes one writer per repoId (services/apps/packages_worker/src/security-contacts/writeContacts.ts:56). Serialize/lock by repo and handle the completed-run race before permitting another write.
    workflowId: ingestWorkflowId(purl),
    workflowIdConflictPolicy: WorkflowIdConflictPolicy.USE_EXISTING,
    workflowIdReusePolicy: WorkflowIdReusePolicy.ALLOW_DUPLICATE,

@ulemons
ulemons merged commit 8a7f246 into main Jul 24, 2026
15 checks passed
@ulemons
ulemons deleted the feat/security-contacts-refresh branch July 24, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature Created by Linear-GitHub Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants