Skip to content
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.

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

Rotate breached credentials in CircleCI #12902

Conversation

matiasgarciaisaia
Copy link
Member

The rest of the secrets are defined in environment variables, so we only need to update this Access Key ID here.

See https://circleci.com/blog/january-4-2023-security-alert/

The rest of the secrets are defined in environment variables, so we only
need to update this Access Key ID here.

See https://circleci.com/blog/january-4-2023-security-alert/
@straight-shoota
Copy link
Member

Perhaps we should move this to an environment variable as well?
Not sure what's the point in having the key fixed in code. It's just less flexible that way.

@matiasgarciaisaia
Copy link
Member Author

matiasgarciaisaia commented Jan 5, 2023

I'm leaning towards the other end of the spectrum - have IDs clearly visible in plain text in the config, and only secrets in environment variables. That usually helps us know which credentials we need to rotate (I should have guessed which of our AWS IAM users was the owner of the access key ID - by matching just the last four characters of it), and, as a byproduct, there's a bit more of traceability on when did we change what (and who, too, given I'm making a commit here).

But it's also true I don't have an strong opinion on the subject, either - and I'm open to be educated on best practices :)

@straight-shoota
Copy link
Member

This is only about nightly builds, which only happen in a fixed schedule on master branch. And it's not a huge problem if one fails because of a broken access key. So I suppose we can manage with having the key in code.

@straight-shoota straight-shoota merged commit 549f251 into crystal-lang:master Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants