ci: hide encryption key from circleci logs #23585
Conversation
alexeagle
added
area: build & ci
| # $KEY is set on CI only for non-PR builds. See /.circleci/README.md | ||
| # Turn off debug mode to avoid echo the key into the log. | ||
| set +x | ||
| openssl aes-256-cbc -d -in .circleci/github_token -k "${KEY}" -out "${HOME}/.git_credentials" |
There was a problem hiding this comment.
Not related to this PR:
Since this line is CircleCI-specific, it might be better to move it somewhere in .circleci/ and let this script assume that .git_credentials is ready.
There was a problem hiding this comment.
I'm really on the fence about that. I think this line either belongs here (close to where it is used) or in the .circleci/config.yml (close to where the $KEY is configured)
In the latter case it's easier to avoid leaking it. WDYT?
alexeagle
force-pushed
the
publish_snapshot
branch
from
0e032a4
to
74b79b4
Compare
alexeagle
added
action: merge
alexeagle
force-pushed
the
publish_snapshot
branch
5 times, most recently
from
079a6e1
to
5579bdd
Compare
.circleci/config.yml
Outdated
| name: Decrypt github credentials | ||
| # See below - ideally this job should not trigger for non-upstream builds | ||
| # But since it does, we have to check if $KEY is set before attempting to | ||
| # decrypt credentials. |
There was a problem hiding this comment.
I doubt circleci will fix this, I haven't seen a lot of movement on issues I've escalated with them, and this would be low on our list :(
.circleci/config.yml
Outdated
| @@ -158,6 +158,10 @@ jobs: | |||
| publish_snapshot: | |||
| <<: *job_defaults | |||
| steps: | |||
| - run: | |||
| name: Skip this job for Pull Requests | |||
| command: '[[ -v CIRCLE_PR_NUMBER ]] && circleci step halt || true' | |||
There was a problem hiding this comment.
Why not akso check that we are on angular/angular?
There was a problem hiding this comment.
We could ... but we have that logic inside publish-build-artifacts as well.
I certainly wouldn't want to duplicate the logic more than necessary.
This guard definitely has to be here since the decryption fails otherwise. I think I'd rather leave most of the logic in the shell script rather than the circle config.
There was a problem hiding this comment.
Fine with me.
(Wouldn't the decryption fail as well if this is not angular/angular?)
There was a problem hiding this comment.
I like your whisper font :)
I guess you are right, this task will fail if you set up circleci for your fork of Angular. And that seems like a reasonable thing to do. Will update...
alexeagle
force-pushed
the
publish_snapshot
branch
from
5579bdd
to
51f3efd
Compare
alexeagle
force-pushed
the
publish_snapshot
branch
from
51f3efd
to
004c10c
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Also add a new github token under a new symmetric key.
After this PR lands I'll update the KEY variable held by circleci so that master publishing will work again.