Problems in CI when uploading data to coveralls

[abravalheri]

Recently we started noticing errors in the CI tasks during the upload to coveralls.
This can be seen in #448 and pyscaffold/configupdater#24.

The weird part is that both repositories have COVERALLS_REPO_TOKEN configured in .cirrus.yml.

[abravalheri]

We can see that a new version of the Python package for coveralls was published a few days ago:
https://pypi.org/project/coveralls/#history

---

Things that might help

Coveralls mention a list of extra env variables that we can try to set in the cirrus-ci:
https://docs.coveralls.io/supported-ci-services

CI_NAME
CI_BUILD_NUMBER
CI_BUILD_URL
CI_BRANCH
CI_PULL_REQUEST (optional)

Coveralls Python CLI mentions:

coveralls debug

[abravalheri]

Thanks @FlorianWilhelm for pointing out what seems to be the problem in #448. Regular contributors cannot access Cirrus CI secrets.

This behaviour seems to make sense, since PRs could expose secrets, but it is not very handy is it? :P

I suppose the alternative would be exposing a plain text, but coveralls docs seem to recommend keeping the token secret:

The option repo_token (found on your repository’s page on Coveralls) is used to specify which project on Coveralls your project maps to. This is only needed for private repos and should be kept secret – anyone could use it to submit coverage data on your repo’s behalf.

Not very sure if we should just ignore these errors when they happen. But it would be nice to know how much a PR changes in terms of coverage.

The text implies the token is only needed for private repos, however, if I remember it correctly, we need it to make Cirrus CI work.

[abravalheri]

I opened issues in coveralls-python and in the support repository of coveralls itself + a discussion in cirrus-ci docs asking for some guidance about this problem.

[abravalheri]

According to the discussion with the Cirrus-CI maintainer, using shared secrets in PRs is a bit tricky.

Currently, we use the Only users with write access policy for decrypting secure env vars. We could change this setting to Everyone but then, there is a risk of non-well intentioned PRs change the CI configuration files to expose the secrets in plain text.

We could in addition to that change the config resolution strategy to Latest from the default branch, this way Cirrus will not run scripts defined directly on .cirrus.yml that could have been tampered with (only previously merged/hopefully reviewed .cirrus.yml would be allowed), however that does not solve the problem of contributors submitting malicious changes to tox.ini and therefore does not change the attach surface area. Moreover, it makes it hard for us to develop/test changes in the CI.

For the time being I am still waiting for the feedback from the coveralls/coveralls-python team.

[abravalheri]

I am also investigating some alternatives to coveralls:

https://community.codecov.io/t/add-support-of-uploading-from-cirrus-ci-without-token/1028/32

[abravalheri]

So:

There are 2 discussions (a bit huge) that I had with coveralls/cirrus maintainers to try to understand what are the best approaches:

• COVERALLS_REPO_TOKEN as repository secret forbids coveralls to run for regular contributors (non-maintainers) lemurheavy/coveralls-public#1557
• Using secrets in PRs for regular contributors (non maintainers) cirruslabs/cirrus-ci-docs#866

I think the easiest way forward would be allowing encrypted vars to be decoded by everyone in the Cirrus interface, or create a plain env var in the Cirrus UI (as per the second link).

We still would not have the token in the clear, but a person submitting a PR to PyScaffold would be able to retrieve/print it.

If we want to make things a bit more secure we could also program Cirrus to always use the .cirrus.yml file from the PR's target branches instead of the PR code. That would prevent contributors from changing the file to expose the secrets, but make it more difficult for ourselves to implement changes in Cirrus. The contributors could still change the tox.ini file thought and have access to the secrets anyway.

@FlorianWilhelm, what are your thoughts about the matter?

[abravalheri]

Update: I double checked my understanding with the Cirrus maintainers. We can simply have a "environment variable overwritten" from the GUI (without using encryption or having to allow encrypted variables from being decoded by everyone). The security concerns are basically the same

[FlorianWilhelm]

So the worst case would be that someone gets our coverall credentials? In this case, I would rather go for a simple solution and do as you have suggested accepting the risk of a PR maintainer stealing the coveralls credentials.

[abravalheri]

Thank you very much @FlorianWilhelm .

Let's try it out, according to the coveralls maintainer the attach surface should be small enough, the worst can happen is someone submitting wrong coverage stats for PyScaffold.

[abravalheri]

The last commits should have solved this problem... But we are only going to know once we have a new contribution...
Let's close the issue for the time being, and then we re-open it later if we find the same problem again.