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

Travis documentation suggests "Display [API token] in build log": seems insecure #675

Open
mcomella opened this issue Sep 29, 2018 · 2 comments

Comments

@mcomella
Copy link

I was going through the getting started guide and saw this section for Travis:

image

In particular:

If you have an open source project, you should ensure "Display value in build log" enabled, so that PRs from forks work.

If I understand correctly, this will insert your DANGER_GITHUB_API_TOKEN's contents into the build log. For open source projects, build logs are often public, e.g. https://travis-ci.org/mozilla-mobile/focus-android/builds/434818832 This means anyone can find and then use your API token: I would recommend removing these lines.

It looks like this would need to be removed from the CLI init and the Travis documentation: https://github.com/danger/danger-js/search?q=display+value+in+build+log&unscoped_q=display+value+in+build+log

mcomella added a commit to mcomella/danger-js that referenced this issue Sep 29, 2018
… log.

It's insecure to post API tokens to public build logs and most open
source repositories have public build logs. However, I do not
understand the effects not enabling this option would have on users of
danger.
mcomella added a commit to mcomella/danger-js that referenced this issue Sep 29, 2018
@kraftbj
Copy link

kraftbj commented Oct 29, 2018

@mcomella I agree with you that it feels very wrong to expose the token like this. The side-effect is PRs from forks won't be able to have Danger comment on the PR. I'm not sure a better way to handle it off the top of my head, though.

@orta
Copy link
Member

orta commented Oct 29, 2018

Yeah, I'm afraid those are the reasonable constraints from the CI host's perspective.

An alternative is that you can use the GitHub App for danger-js - which uses the GitHub App creds embedded inside danger, but it's pretty undocumented because I'm still not entirely sold on checks support, and have never used this myself on a project, but it should work

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

No branches or pull requests

3 participants