Skip to content

fix(external-kms): use sanitized AWS provider schema in read responses#7008

Open
Vligai wants to merge 1 commit into
mainfrom
fix/external-kms-aws-response-schema
Open

fix(external-kms): use sanitized AWS provider schema in read responses#7008
Vligai wants to merge 1 commit into
mainfrom
fix/external-kms-aws-response-schema

Conversation

@Vligai

@Vligai Vligai commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Context

The external-KMS get-by-id and get-by-name read endpoints (GET /external-kms/:id,
GET /external-kms/name/:name) serialize the AWS provider config through the full
ExternalKmsAwsSchema. The provider-specific endpoints (for example GET /external-kms/aws/:id)
already serialize through SanitizedExternalKmsAwsSchema.

This brings the legacy read endpoints in line with that existing schema, so the AWS read response
shape is consistent across the external-KMS API (access key and assume-role identifiers only),
matching how the GCP branch is already handled in the same response. It is not a new control; it
applies the one already used by the provider-specific endpoints to the legacy read endpoints, which
were never migrated to it.

  • Before: legacy AWS reads returned the full ExternalKmsAwsSchema.
  • After: legacy AWS reads return SanitizedExternalKmsAwsSchema, the same shape the provider-specific
    endpoints already return.

Changed files:

  • backend/src/ee/routes/v1/external-kms-router.ts: use SanitizedExternalKmsAwsSchema in the
    providerInput union of the get-by-id / get-by-name response schema (covers both endpoints);
    update the import and the adjacent comment.
  • backend/src/ee/services/external-kms/providers/model.test.ts: unit test for the sanitized schema.

Screenshots

Steps to verify the change

  • Unit test (included in this PR): npm run test:unit -- src/ee/services/external-kms/providers/model.test.ts.
    It asserts the sanitized schema returns the access key and assume-role identifiers only, with a
    negative control over the prior schema.
  • Manual: GET an AWS external-KMS key by id and by name and confirm the response matches the shape
    already returned by the provider-specific endpoint (GET /external-kms/aws/:id).

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Updated CLAUDE.md files (if needed)
  • Read the contributing guide

The get-by-id and get-by-name external-KMS read endpoints serialized the AWS
provider config through the full ExternalKmsAwsSchema, while the provider-specific
endpoints already use SanitizedExternalKmsAwsSchema. Align the legacy read
endpoints with that existing schema so the AWS read response shape is consistent
across the external-KMS API (access key and assume-role identifiers only),
matching how the GCP branch is already handled.

Add a unit test for SanitizedExternalKmsAwsSchema covering the access-key and
assume-role cases.
@infisical-review-police

Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-infisical-7008-fix-external-kms-use-sanitized-aws-provider-schema-in-r

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

@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a credential exposure gap in the legacy AWS external-KMS read endpoints (GET /external-kms/:id and GET /external-kms/name/:name) by replacing the full ExternalKmsAwsSchema with the already-existing SanitizedExternalKmsAwsSchema in the response schema — the same schema the provider-specific endpoint already uses.

  • The SanitizedExternalKmsAwsSchema strips secretKey from access-key credentials and retains only accessKey, while preserving the full assume-role identifiers (assumeRoleArn, externalId) which carry no secret material.
  • A new unit test covers the strip behavior, includes a negative control against the unsanitized schema, and verifies the assume-role branch is unchanged.

Confidence Score: 5/5

Safe to merge — the change is a one-line schema swap in a response definition with no logic changes and backed by a focused unit test.

Both changed files are narrow in scope: the router change touches only the response schema for two read endpoints, and the test file directly validates the sanitization behavior. The SanitizedExternalKmsAwsSchema was already in use on the provider-specific endpoint, so this is a proven, existing construct being applied consistently. No new logic, no new network calls, no migration required.

No files require special attention.

Important Files Changed

Filename Overview
backend/src/ee/routes/v1/external-kms-router.ts Swaps ExternalKmsAwsSchema for SanitizedExternalKmsAwsSchema in the sanitizedExternalSchemaForGetById response schema, aligning the legacy GET-by-id and GET-by-name endpoints with the provider-specific endpoint; the change is minimal and correct.
backend/src/ee/services/external-kms/providers/model.test.ts New unit tests covering the sanitized AWS schema: verifies secretKey is stripped for access-key credentials, includes a negative control against the unsanitized schema, and confirms assume-role identifiers are preserved.

Reviews (1): Last reviewed commit: "fix(external-kms): use sanitized AWS pro..." | Re-trigger Greptile

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.

1 participant