REQUEST: Repository maintenance on opentelemetry-collector opentelemetry-collector-contrib

[codeboten]

Affected Repository

https://github.com/open-telemetry/opentelemetry-collector
https://github.com/open-telemetry/opentelemetry-collector-contrib

Requested changes

Please give write access to opentelemetrybot to those two repositories.

Purpose

PRs opened by forking renovate need to be updated after they've been opened to make them useful. Running go mod tidy is required for the PR to pass all the CI checks. For the bot account to be able to update the PR, write permissions to the repositories are needed.

Expected Duration

permanently

Repository Maintainers

• @open-telemetry/collector-contrib-maintainer
• @open-telemetry/collector-maintainers

[trask]

PRs opened by forking renovate need to be updated after they've been opened to make them useful. Running go mod tidy is required for the PR to pass all the CI checks. For the bot account to be able to update the PR, write permissions to the repositories are needed.

I agree this is an important automation use case (the Java Instrumentation repo could benefit from this as well).

Currently, https://github.com/open-telemetry/community/blob/main/assets.md#opentelemetry-bot says:

Important: You do not need to (and should not) give this account any permissions to any OpenTelemetry repository.

I think the main security issue is that currently the OPENTELEMETRYBOT_GITHUB_TOKEN is a "classic" personal access token, which means it's permissions really can't be scoped in a very meaningful way, and so if you give @opentelemetrybot write permission to a repository, anyone with that token could write to that repository.

The problem with using the new (beta) "fine-grained" personal access tokens was that GraphQL (and so most of gh CLI) didn't support them, but it looks like support for GraphQL was added about a month ago 🎉

I'll set up a "fine-grained" personal access token for us to try out. I think we would still need to get everyone off of the "classic" personal access token and delete that (or at least rotate the token and lock it down to specific repos that still need it).

Ideally I think we would have

• shared across the OTel org: one "fine-grained" personal access token that doesn't give write access to anything, and that could be used for most automation
• per repository that needs it: one "fine-grained" personal access token that includes write access for (only) that repo

[codeboten]

Ideally I think we would have

* shared across the OTel org: one "fine-grained" personal access token that doesn't give write access to anything, and that could be used for most automation

* per repository that needs it: one "fine-grained" personal access token that includes write access for (only) that repo

This makes sense to me, happy to use this issue as the test case for fine grained tokens :)

[jmacd]

[pellared]

Can you please make sure that the PAT does not allow editing the GitHub Releases?

As far as I know, the Write permission (even fine-grained for Contents scope) also gives the power to modify (create/delete) the releases. Then the PAT (and bot) could be used to e.g. modify artifacts added to a release. Also if tag protection is not set - it would also allow creation/deletion of Git tags. If so then, adding such PAT increases the attack surface that could be used for supply chain attacks.

Reference: https://docs.github.com/en/rest/overview/permissions-required-for-fine-grained-personal-access-tokens?apiVersion=2022-11-28#contents

[pellared]

One more reference: Supply Chain Attacks via GitHub.com Releases

[codeboten]

Would one way we could potentially work around this issue not to produce releases with any artifacts from repositories that have these tokens enabled?

Specifically for the collector, most of the artifacts are already published from the releases repo

[pellared]

@codeboten This is a good idea 👍

Then we would just need (in Collector and Collector Contrib repos) something that would detect if something publishes an unwanted release.

For instance, we can create a GitHub workflow which triggers when a release is published and creates an issue. I think it could be done like this (not tested):

name: Unwanted release detector
permissions:
  contents: read
  issues: write 
on:
  release:
    types: [published]
jobs:
  report-unwanted-release:
    runs-on: ubuntu-latest
    steps:
      - name: Create issue
        run: gh issue create --title "Unwanted release $GITHUB_REF" --body "${{ github.event.release.html_url }} SHOULD NOT be published." --repo $GITHUB_REPOSITORY
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Instead of creating issues we could try privately report a security vulnerability.

Still the PAT could be used to create tags, which is like a "release of Go module". However, I am sure that the Collector maintainers would detect it almost immediately and they can retract the module(s) and create a repository security advisory.

[pellared]

Still the PAT could be used to create tags

It would be enough to add a * tag protection rule.

[codeboten]

@pellared to be clear i'm suggesting those repos would still produce a github release, just that it wouldn't include any artifacts (beyond source zip/tar files). All artifacts would be produced in the opentelemetry-collector-releases repo, which is where most artifacts are today.

[pellared]

it wouldn't include any artifacts (beyond source zip/tar files)

Then the PAT could be used to edit the releases and add malicious artifacts 😢

[Aneurysm9]

Still the PAT could be used to create tags

It would be enough to add a * tag protection rule.

I think this is critical. We should probably be doing this regardless whether we give a bot account write access given the importance of git tags being immutable to the Go package management system. We would need a custom role to allow approvers to continue to manage collector releases.

[trask]

just providing an update on the plans here

I've set up a fine-grained personal access token and will be using that in the upcoming Java Instrumentation release (this Wednesday).

Assuming everything goes smoothly, I'll ping the repos which are already using the org secret to let them know we'll be switching the org secret over to a fine-grained PAT. I'm thinking to give ~1 week from when this notice goes out until we perform the switch.

After we switch the org secret over to the fine-grained PAT, I'll delete the old PAT, and at that point we can start issuing fine-grained PATs with write access to a single repo for those who want to grant write permission to @opentelemetrybot.

[pellared]

at that point we can start issuing fine-grained PATs with write access to a single repo for those who want to grant write permission to @opentelemetrybot.

The fine-grained PAT may leak e.g. via something like the "Codecov breach" and then it could be used to modify existing releases to contain malicious code (e.g. a ransomware). Unfortunately supply chain attacks are getting "popular".

PS. Even if we accept such risk, we should update https://github.com/open-telemetry/community/blob/main/assets.md#opentelemetry-bot 😉

[trask]

The fine-grained PAT may leak e.g. via something like the "Codecov breach" and then it could be used to modify existing releases to contain malicious code (e.g. a ransomware). Unfortunately supply chain attacks are getting "popular".

@pellared I liked your proposal above about creating a GitHub workflow to detect unwanted releases / release artifacts

PS. Even if we accept such risk, we should update https://github.com/open-telemetry/community/blob/main/assets.md#opentelemetry-bot 😉

👍

[tigrannajaryan]

@codeboten this fell through the cracks. Is it is still actual?