Skip to content

fix(sa-key-rotator): prevent private key log leakage and add active key guard - #190

Merged
meet2mky merged 1 commit into
mainfrom
add-key-rotater
Aug 27, 2026
Merged

fix(sa-key-rotator): prevent private key log leakage and add active key guard#190
meet2mky merged 1 commit into
mainfrom
add-key-rotater

Conversation

@meet2mky

Copy link
Copy Markdown
Collaborator

Description

Follow-up fixes to #189 address code review feedback on rotate_sa_keys.sh:

  1. Prevent Private Key Leakage & JSON Corruption in generate_sa_key_in_memory:
    • Redirects gcloud stderr to an isolated temporary file (err_out=$(mktemp)).
    • Prevents mixing warnings with JSON output and prevents printing raw key material to stderr/Cloud Logging on JSON parse failure.
  2. Safety Guard in prune_older_iam_keys:
    • Adds an explicit check verifying active_key_id is non-empty before initiating IAM key deletion, preventing accidental deletion of all keys if active_key_id is unresolved.

Verification

  • Verified dry run across all 4 targets in gcs-fuse-test and gcs-fuse-test-ml.
  • Built container and redeployed Cloud Run Job via ./deploy.sh.

…ey guard

- Separate stdout and stderr during key generation using a temp file to prevent leaking private key material to logs on parse error or corrupting JSON payload
- Add explicit validation for non-empty active_key_id before IAM key pruning to prevent accidental deletion of all keys

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request improves error handling and safety in the service account key rotation script. Specifically, it refactors key generation to capture stderr in a temporary file for better error reporting on failure, and adds a safety check in the key pruning function to prevent accidental deletion of all keys if the active key ID is empty. There are no review comments to address.

@meet2mky
meet2mky merged commit f508181 into main Aug 27, 2026
8 checks passed
@meet2mky
meet2mky deleted the add-key-rotater branch August 27, 2026 09:33
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