Skip to content

Conversation

@d4mr
Copy link
Contributor

@d4mr d4mr commented Oct 7, 2024

Fixes a bug related to GCP application credential private key serialisation/deserialisation


PR-Codex overview

This PR focuses on enhancing the handling of private_key in clientOptions.credentials for compatibility with both new and older keys in the getGcpKmsAccount.ts file.

Detailed summary

  • Added a check to see if clientOptions.credentials exists.
  • If private_key is present, it sanitizes the key by replacing newline characters for backwards compatibility.
  • Included a comment regarding a previous bug related to cryptoKeyVersion.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@d4mr d4mr requested review from joaquim-verges and removed request for joaquim-verges October 7, 2024 14:04
Comment on lines 43 to 48
let rawPrivateKey: string | undefined;
if (clientOptions?.credentials) {
if ("private_key" in clientOptions.credentials) {
rawPrivateKey = clientOptions.credentials.private_key;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO a bit clearer to just "sanitize" the private key in clientOptions and continue passing the clientOptions below.

  if (clientOptions?.credentials) {
    if ("private_key" in clientOptions.credentials) {
      clientOptions.credentials.private_key =
        clientOptions.credentials.private_key?.split(String.raw`\n`).join("\n");
    }
  }

@d4mr d4mr changed the title Potential GCP KMS bugfix GCP KMS bugfix Oct 7, 2024
@d4mr d4mr merged commit cd02aa3 into main Oct 7, 2024
5 checks passed
@d4mr d4mr deleted the pb/fix-gcp-kms branch October 7, 2024 19:14
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.

3 participants