Polaris-react: Percy does not run on forked PRs

Created on 8 Nov 2018  路  6Comments  路  Source: Shopify/polaris-react

Percy and Codecov do not run on forked PRs because CircleCI does not pass secrets to builds from forked PRs. See https://circleci.com/docs/2.0/oss/#pass-secrets-to-builds-from-forked-pull-requests

As @BPScott and I discussed in Slack, this is okay for now but we should at least suppress the 401 error for the CircleCI Percy workflow until we can fix this. See #558 and this Percy workflow run as an example of the 401 failure.

Most helpful comment

This was one of our major concerns too.

We're still erring towards keeping this disabled for now but I don't think we've made a final decision

All 6 comments

Looks like I was wrong about Codecov

Repo tokens are not required for public repos tested on Travis-Org, CircleCI or AppVeyor.

So I just need to add a check for Percy.

馃憢 @BPScott #581 mentions including a partial fix for this issue. Any updates on what is yet to be done?

@tmlayton and I (well mostly Tim) are emailing with Percy about this.

There is no issue with CodeCov as repo tokens are not needed on public repos. We have removed the environment variable for our CodeCov token, and coverage is reported for both PRs from our repo and forks.

581 means CI will no-longer error when a running Percy tests from forked repos - it now just doesn't run the Percy tests.

Ideally though we'd still want Percy tests to run for those PRs from forked repos so this ticket isn't resolved just yet.


The concern here is that the only way to get this working is to expose our build environment variables to builds ran on forked repos. This is somewhat scary as it means leaking credentials - somebody could fork our repo, modify the build script to run env and see any secrets we have configured. Admittedly that list of variables at the moment only contains the PERCY_TOKEN but I don't want a situation where we forget we did this in the future and put in something actually secret in there.

only contains the PERCY_TOKEN

I wouldn't want to explain the per-percy-run cost skyrocketing because someone else hijacked our token.

This was one of our major concerns too.

We're still erring towards keeping this disabled for now but I don't think we've made a final decision

I enabled the sharing of this key a month ago or so when I marked all checks as required

Was this page helpful?
0 / 5 - 0 ratings