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

Do not use secrets in test run #2201

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Do not use secrets in test run #2201

wants to merge 1 commit into from

Conversation

benjamineskola
Copy link
Contributor

This allows them to be used by PRs from forks.

This allows them to be used by PRs from forks.
@@ -2,7 +2,8 @@ DOMAIN="example.com"
STAGING_API_URL="bops-staging.services"
STAGING_API_BEARER="fjisdfjsdiofjdsoi"
OS_VECTOR_TILES_API_KEY="testtest"
NOTIFY_LETTER_API_KEY='testtest'
NOTIFY_API_KEY="test_key_for_ci-d64cc892-8848-4663-9544-487551484e9b-d7bc72d1-8e1f-4d5a-a703-de5beecaf4a8"
NOTIFY_LETTER_API_KEY="test_key_for_ci-d64cc892-8848-4663-9544-487551484e9b-d7bc72d1-8e1f-4d5a-a703-de5beecaf4a8"
Copy link
Member

Choose a reason for hiding this comment

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

Are these real test keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. (Have assumed that because it's specifically for testing and therefore doesn't have permission to do anything, it's less of a risk. Can revert and revoke if that's not the case.)

Problem is that we can't keep it in GH Secrets (if we want to allow PRs from forks) and we can't use a fake key because some of the code is actually instantiating Notify, which throws an error if just given a non-uuid like testtest.

We'd have to track down all of those places and mock Notify (which arguably we should have been doing all along.)

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