feat: add refresh security contats api (CM-1348)#4393
Conversation
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
PR SummaryMedium Risk Overview The handler short-circuits when 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 Reviewed by Cursor Bugbot for commit a8f78cd. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
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
foundonly 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.
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
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 | nullcontactsLastRefreshedfield, 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>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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.
There was a problem hiding this comment.
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_DUPLICATEstarts another run when a second request read the null timestamp before the first run completed but reachesexecuteafterward; moreover, different PURLs linked to the same repo get different workflow IDs, and the daily workflow has another ID. Those executions can overlap even thoughwriteContactsexplicitly assumes one writer perrepoId(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,

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 sameContactDetailpayload asGET /akrites-external/contacts/detail.Changes
ingestAkritesExternalContactDetailcallspackagesTemporal.workflow.execute(...)and re-reads viagetContactDetailsByPurlsonce the workflow resolves, returningNotFoundErrorif the workflow reports no linked repo or the re-read comes back empty.security-contacts-ondemand:<sha256(purl)>) withworkflowIdConflictPolicy: USE_EXISTING+workflowIdReusePolicy: ALLOW_DUPLICATE, so concurrent refresh requests for the same purl attach to one running workflow instead of each starting their own.workflow.execute()(start + await result) rather than the fire-and-forgetworkflow.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.purlBodySchema(single-purl body, validated viavalidateOrThrow) alongside the existing array-basedpurlsBodySchema.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 sameenvTunableRateLimiterhelper.READ_MAINTAINER_ROLES, matching/contacts/detail.Type of change
JIRA ticket
CM-1348