Skip to content

fix: security results update (IN-1093) #4029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joanagmaia merged 2 commits into from
Apr 17, 2026
+30 −24
Merged

fix: security results update (IN-1093) #4029

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ba1883
fix: security results update
joanagmaia Apr 16, 2026
11dde9f
Merge branch 'main' into fix/security-results-update-in-1093
joanagmaia Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 30 additions & 24 deletions services/libs/data-access-layer/src/security_insights/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,12 @@ export async function addEvaluationSuite(
($(id), $(name), $(repo), $(insightsProjectId), $(insightsProjectSlug), $(catalogId), $(result), $(corruptedState), now(), now())
on conflict ("repo", "catalogId")
do update
set "updatedAt" = EXCLUDED."updatedAt",
"name" = EXCLUDED."name",
"result" = EXCLUDED."result",
"corruptedState" = EXCLUDED."corruptedState"
set "updatedAt" = EXCLUDED."updatedAt",
"name" = EXCLUDED."name",
"result" = EXCLUDED."result",
"corruptedState" = EXCLUDED."corruptedState",
"insightsProjectId" = EXCLUDED."insightsProjectId",
"insightsProjectSlug" = EXCLUDED."insightsProjectSlug"
Comment on lines +81 to +86
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

insightsProjectSlug has a FK to insightsProjects.slug with ON UPDATE CASCADE (see backend/src/database/migrations/V1753457861__add_unique_contraint_on_insights_slug.sql). Updating the slug from EXCLUDED during the conflict update can cause the upsert to fail if the caller provides a stale slug (e.g., slug changed between read and write); previously the row would keep the cascaded/current slug. Consider not updating insightsProjectSlug on conflict, or deriving it from insightsProjectId/insightsProjects inside the update so it always remains valid/current.

Suggested change
set "updatedAt" = EXCLUDED."updatedAt",
"name" = EXCLUDED."name",
"result" = EXCLUDED."result",
"corruptedState" = EXCLUDED."corruptedState",
"insightsProjectId" = EXCLUDED."insightsProjectId",
"insightsProjectSlug" = EXCLUDED."insightsProjectSlug"
set "updatedAt" = EXCLUDED."updatedAt",
"name" = EXCLUDED."name",
"result" = EXCLUDED."result",
"corruptedState" = EXCLUDED."corruptedState",
"insightsProjectId" = EXCLUDED."insightsProjectId"

Copilot uses AI. Check for mistakes.
`,
{
id: generateUUIDv4(),
Expand Down Expand Up @@ -154,13 +156,15 @@ export async function addSuiteControlEvaluation(
)
on conflict ("securityInsightsEvaluationSuiteId", "repo", "controlId")
do update
set
"updatedAt" = EXCLUDED."updatedAt",
"name" = EXCLUDED."name",
"result" = EXCLUDED."result",
"message" = EXCLUDED."message",
"corruptedState" = EXCLUDED."corruptedState",
"remediationGuide" = EXCLUDED."remediationGuide"
set
"updatedAt" = EXCLUDED."updatedAt",
"name" = EXCLUDED."name",
"result" = EXCLUDED."result",
"message" = EXCLUDED."message",
"corruptedState" = EXCLUDED."corruptedState",
"remediationGuide" = EXCLUDED."remediationGuide",
"insightsProjectId" = EXCLUDED."insightsProjectId",
"insightsProjectSlug" = EXCLUDED."insightsProjectSlug"
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

insightsProjectSlug is enforced via FK to insightsProjects.slug (ON UPDATE CASCADE). Setting it to EXCLUDED.insightsProjectSlug on conflict makes this upsert sensitive to stale slug inputs (race with slug rename) and can cause FK violations at update time. Safer options are to avoid updating the slug on conflict, or compute the slug from insightsProjectId/insightsProjects in the update.

Suggested change
"insightsProjectSlug" = EXCLUDED."insightsProjectSlug"
"insightsProjectSlug" = (
select "slug"
from "insightsProjects"
where "id" = EXCLUDED."insightsProjectId"
)

Copilot uses AI. Check for mistakes.

`,
{
Expand Down Expand Up @@ -233,19 +237,21 @@ export async function addControlEvaluationAssessment(
)
on conflict ("securityInsightsEvaluationId", "repo", "requirementId")
do update
set "updatedAt" = EXCLUDED."updatedAt",
"applicability" = EXCLUDED."applicability",
"description" = EXCLUDED."description",
"result" = EXCLUDED."result",
"message" = EXCLUDED."message",
"steps" = EXCLUDED."steps",
"stepsExecuted" = EXCLUDED."stepsExecuted",
"runDuration" = EXCLUDED."runDuration",
"recommendation" = EXCLUDED."recommendation",
"start" = EXCLUDED."start",
"end" = EXCLUDED."end",
"value" = EXCLUDED."value",
"changes" = EXCLUDED."changes"
set "updatedAt" = EXCLUDED."updatedAt",
"applicability" = EXCLUDED."applicability",
"description" = EXCLUDED."description",
"result" = EXCLUDED."result",
"message" = EXCLUDED."message",
"steps" = EXCLUDED."steps",
"stepsExecuted" = EXCLUDED."stepsExecuted",
"runDuration" = EXCLUDED."runDuration",
"recommendation" = EXCLUDED."recommendation",
"start" = EXCLUDED."start",
"end" = EXCLUDED."end",
"value" = EXCLUDED."value",
"changes" = EXCLUDED."changes",
"insightsProjectId" = EXCLUDED."insightsProjectId",
"insightsProjectSlug" = EXCLUDED."insightsProjectSlug"
Comment on lines +253 to +254
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

Like the other security insights tables, insightsProjectSlug has an FK to insightsProjects.slug with ON UPDATE CASCADE. Updating it from EXCLUDED during the upsert can fail if the caller’s slug is stale (after a rename) and may undo the cascaded value. Consider leaving insightsProjectSlug untouched on conflict, or deriving it from insightsProjectId/insightsProjects within the update clause to guarantee consistency.

Suggested change
"insightsProjectId" = EXCLUDED."insightsProjectId",
"insightsProjectSlug" = EXCLUDED."insightsProjectSlug"
"insightsProjectId" = EXCLUDED."insightsProjectId"

Copilot uses AI. Check for mistakes.

`,
{
Expand Down
Loading