Skip to content

fix: parse JSON fields in certificate policy/profile delete responses#7047

Merged
bernie-g merged 2 commits into
mainfrom
fix/cert-policy-delete-response-serialization
Jun 26, 2026
Merged

fix: parse JSON fields in certificate policy/profile delete responses#7047
bernie-g merged 2 commits into
mainfrom
fix/cert-policy-delete-response-serialization

Conversation

@bernie-g

Copy link
Copy Markdown
Contributor

Summary

  • The deleteById methods in both certificate-policy-dal and certificate-profile-dal returned raw database rows without parsing JSON fields (subject, sans, keyUsages, externalConfigs, etc.)
  • Fastify's response schema validation rejected the unparsed strings (expecting objects/arrays), returning a 500 ResponseValidationError to the caller
  • The delete itself succeeded, so the resource was removed, but the API returned 500 "Something went wrong" -- reported by a customer using Terraform, where terraform apply after removing a cert policy from .tf files would error, requiring a second apply to reconcile state

Test plan

  • Reproduced locally: created a certificate policy with JSON fields, deleted it, confirmed 500
  • Applied fix, retested: delete now returns 200 with properly serialized response

The deleteById methods in both certificate-policy-dal and
certificate-profile-dal returned raw database rows without parsing
JSON fields (subject, sans, keyUsages, externalConfigs, etc.).
This caused Fastify's response schema validation to fail with a 500
ResponseValidationError because the Zod schemas expect objects/arrays
but received JSON strings.

The delete itself succeeded, so the resource was removed, but the
API returned a 500 "Something went wrong" to the caller.
@infisical-review-police

Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-infisical-7047-fix-parse-json-fields-in-certificate-policy-profile-del

Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel.

@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a 500 ResponseValidationError on certificate policy and profile delete endpoints caused by raw database rows being returned without parsing JSON-encoded fields (subject, sans, keyUsages, externalConfigs, etc.) before Fastify schema validation ran.

  • certificate-policy-dal.ts: deleteById now calls the existing parseJsonFields helper that create, updateById, and findById already use, with a null guard added for a missing row.
  • certificate-profile-dal.ts: deleteById now applies the same inline JSON parsing pattern (JSON.parse(externalConfigs)) present in create and updateById, and the return type is widened to TCertificateProfile | undefined to reflect the missing-row case.

Confidence Score: 5/5

Targeted fix with no side effects — both DAL methods now match the pattern of their sibling create/updateById/findById methods in the same file.

Both changes are purely additive JSON-parsing steps on the return value of a single DB operation. The logic mirrors identical, already-working code in the same file. No new queries, no schema changes, no control-flow alterations outside the two affected functions.

No files require special attention.

Important Files Changed

Filename Overview
backend/src/services/certificate-policy/certificate-policy-dal.ts Applies the existing parseJsonFields helper to the deleteById return value, matching the pattern already used by create, updateById, and findById; adds a null guard consistent with updateById.
backend/src/services/certificate-profile/certificate-profile-dal.ts Adds manual JSON.parse for externalConfigs and a null guard in deleteById, mirroring the exact pattern already used in create and updateById; changes the return type from TCertificateProfile to `TCertificateProfile

Reviews (1): Last reviewed commit: "fix: parse JSON fields in certificate po..." | Re-trigger Greptile

@bernie-g bernie-g requested review from carlosmonastyrski and sheensantoscapadngan and removed request for sheensantoscapadngan June 26, 2026 15:03
@bernie-g bernie-g merged commit 60bc490 into main Jun 26, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants