Remove tokens from Circle CI config (#21058)
Summary: While these were intentionally used in the open, and never were abused, it has become a distraction whenever they are flagged. We'll have to move this functionality to a service outside of Circle CI, as we cannot securely pass secrets to forks and PRs in Circle CI. By necessity, these PR analysis scripts must run alongside PRs. The The controller you requested could not be found. token has already been revoked. The The controller you requested could not be found. token is not under our control, and is still valid as of this writing. The eslint token has public_repo scope, with no access to any private repos. It's no different than having a random account commenting on any public repo. Unfortunately, revoking the The controller you requested could not be found. token affects React's use of this bot account as well. --- Q: What does the React team need this token for? A: It's used to analyze how a PR will impact the build size for React. Q: What does the React Native team need this token for? A: We do lightweight automated PR code reviews with it (eslint, flagging large PRs, etc) Q: What can someone do with the access token? A: The token was for the The controller you requested could not be found. GitHub account. The account has no privileged access to any organization, so in effect it's like having the token to a random GitHub account. The token has public_repo access scope, which allows it to interact with any public repository on GitHub. The attacker can leave comments on any issue, pull request, or commit, on any public open source repository on GitHub. They could spam or leave arbitrary messages. It's no different than using any random newly created GitHub account to do this, but the bot is named "React Linter", so people could have used it to make React look bad. Q: Why didn't we just remove the token from the open source repositories? CircleCI allows you to use environment variables to keep secrets out of the repo. A: We have configured CircleCI to hide environment variables from Circle CI jobs triggered by non-Facebook org forks and pull requests (otherwise, anyone could add a file to their fork that echoes $SUPER_SECRET and then read it from the Circle CI logs). This allows us to do things like publish to npm only on commits that actually land on the main repo, without letting random people do the same on their forks. Q: Why can't we run these scripts on Circle CI jobs that do have access to secret environment variables? A: It's by necessity. These scripts are meant to run on pull requests and forks. They're used to lint pull requests, after all. Q: Why can't we run these scripts on internal Facebook infrastructure? A: Automatic importing of arbitrary code from external sources into internal Facebook systems without a FB engineer's involvement is disallowed. We're happy to let Circle CI run unvetted code in this manner. Q: What do other projects do in similar situations? A: A common solution for open source projects that need to run scripts with access to GitHub without exposing the access token on CI is to use a private cloud server (i.e. a droplet in Digital Ocean, an instance on AWS...). Q: Why don't we use the same infra used by react-native-bot to run react-linter? A: React-Native-Bot runs once an hour or so, querying for recent issues and PRs. It does not use webhooks, and instead performs the same kind of search queries you'd use on GitHub, therefore it's not great for picking up when a PR has been updated. Circle CI is great for running scripts whenever a PR is created or updated, as Circle outages aside, we can be fairly certain a script will run any time a PR is updated. If you want to track build sizes, you really want to make sure any new commit added to a PR will trigger a re-run. Pull Request resolved: https://github.com/facebook/react-native/pull/21058 Differential Revision: D9809842 Pulled By: hramos fbshipit-source-id: 6ca5d2f5b48e077ec822a3aea5237534bd828850
This commit is contained in:
parent
cb87c3f410
commit
023d650a0d
|
@ -538,12 +538,18 @@ jobs:
|
||||||
# Analyze pull request and raise any lint/flow issues.
|
# Analyze pull request and raise any lint/flow issues.
|
||||||
# Issues will be posted to the PR itself via GitHub bots.
|
# Issues will be posted to the PR itself via GitHub bots.
|
||||||
# This workflow should only fail if the bots fail to run.
|
# This workflow should only fail if the bots fail to run.
|
||||||
|
# The public github tokens are publicly visible by design
|
||||||
analyze_pr:
|
analyze_pr:
|
||||||
<<: *defaults
|
<<: *defaults
|
||||||
docker:
|
docker:
|
||||||
- image: circleci/node:10
|
- image: circleci/node:10
|
||||||
environment:
|
environment:
|
||||||
- PATH: "/opt/yarn/yarn-v1.5.1/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
|
- PATH: "/opt/yarn/yarn-v1.5.1/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
|
||||||
|
- PUBLIC_PULLBOT_GITHUB_TOKEN_A: "a6edf8e8d40ce4e8b11a"
|
||||||
|
- PUBLIC_PULLBOT_GITHUB_TOKEN_B: "150e1341f4dd9c944d2a"
|
||||||
|
- PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A: "78a72af35445ca3f8180"
|
||||||
|
- PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B: "b1a98e0bbd56ff1ccba1"
|
||||||
|
|
||||||
steps:
|
steps:
|
||||||
- checkout
|
- checkout
|
||||||
- run: *setup-artifacts
|
- run: *setup-artifacts
|
||||||
|
@ -554,10 +560,9 @@ jobs:
|
||||||
- run:
|
- run:
|
||||||
name: Analyze Code
|
name: Analyze Code
|
||||||
command: |
|
command: |
|
||||||
# GITHUB_TOKEN=eslint-bot public_repo access token
|
|
||||||
if [ -n "$CIRCLE_PR_NUMBER" ]; then
|
if [ -n "$CIRCLE_PR_NUMBER" ]; then
|
||||||
echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"; yarn add @octokit/rest@15.10.0
|
echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"; yarn add @octokit/rest@15.10.0
|
||||||
echo -e "\\x1B[36mAnalyzing code\\x1B[0m"; GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" ./scripts/circleci/analyze_code.sh
|
echo -e "\\x1B[36mAnalyzing code\\x1B[0m"; GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" ./scripts/circleci/analyze_code.sh
|
||||||
else
|
else
|
||||||
echo "Skipping code analysis."
|
echo "Skipping code analysis."
|
||||||
fi
|
fi
|
||||||
|
@ -567,11 +572,10 @@ jobs:
|
||||||
- run:
|
- run:
|
||||||
name: Analyze Pull Request
|
name: Analyze Pull Request
|
||||||
command: |
|
command: |
|
||||||
# DANGER_GITHUB_API_TOKEN=React-Linter public_repo access token
|
|
||||||
if [ -n "$CIRCLE_PR_NUMBER" ]; then
|
if [ -n "$CIRCLE_PR_NUMBER" ]; then
|
||||||
cd bots
|
cd bots
|
||||||
yarn install --non-interactive --cache-folder ~/.cache/yarn
|
yarn install --non-interactive --cache-folder ~/.cache/yarn
|
||||||
DANGER_GITHUB_API_TOKEN="80aa64c50f38a267e9ba""575d41d528f9c234edb8" yarn danger
|
DANGER_GITHUB_API_TOKEN="$PUBLIC_PULLBOT_GITHUB_TOKEN_A""$PUBLIC_PULLBOT_GITHUB_TOKEN_B" yarn danger
|
||||||
else
|
else
|
||||||
echo "Skipping pull request analysis."
|
echo "Skipping pull request analysis."
|
||||||
fi
|
fi
|
||||||
|
|
Loading…
Reference in New Issue