6 ms·

> For one thing, developers can run whatever code in CI before it's been reviewed. Yeah I don't think this gets talked about enough. If you're talking about

by nickjj 5y ago

> For one thing, developers can run whatever code in CI before it's been reviewed. Yeah I don't think this gets talked about enough. If you're talking about private repos in an organization then CI often runs on any pull request. That means a developer is able to make CI run in an unreviewed PR. Of course for it to make its way into a protected branch (main, etc.) it'll likely need a code review but nothing is stopping that developer who opened the unreviewed PR to modify the CI yaml file in a commit to make that PR's pipeline do something different. Requiring a team lead or someone to allow every individual PR's pipeline to run (what GitHub does by default in public repos) would add too much friction and not all major git hosts support the idea of locking down the pipelines file by decoupling it from the code repo. Edit: Depending on which CI provider you use, this situation is mostly preventable -- "mostly" in the sense that you can control how much damage can be done. Check out this comment later in this thread: https://news.ycombinator.com/item?id=29967077 https://news.ycombinator.com/item?id=29967077

heavenlyblue 5y ago
The solution to that is simple: do not do any tests with production secrets :)
reflectiv 5y ago
Each environment should have its own keys for the services it is talking to so ideally this would restrict the scope of damage.
- nickjj 5y ago
  > Each environment should have its own keys for the services it is talking to so ideally this would restrict the scope of damage. If a developer changes the CI pipeline file to make their PR's code run in `deployment: "production"` instead of `deployment: "test"` doesn't that bypass this? Edit: I'll leave my original question here because I think it's an important one but I answered this myself. It depends on which CI provider you're using but some of them do let you restrict specific deployments from being run only on specific branches or by specific folks (such as repo admins). In the above case if the production deployment was only allowed to run on the main branch and the only way code makes its way into the main branch is after at least 1 person reviewed + merged it (or whatever policy your company wants) then a rogue developer can't edit the pipeline in an unreviewed PR to make something run in production. Also with deployment specific environment variables then a rogue developer is also not able to edit a pipeline file to try and run commands that may affect production such as doing a `terraform apply` or pushing an unreviewed Docker image to your production registry.
  - Nextgrid 5y ago
    CI should only have environment variables needed for testing. For building/deploying to production, it just has to push the code/package/container image, not run it, meaning it has no need for production-level credentials. CI should never ever have access to anything related to production; not just for security but also to prevent potentially bad code being run in tests from trashing production data.
  - acdha 5y ago
    > If a developer changes the CI pipeline file to make their PR's code run in `deployment: "production"` instead of `deployment: "test"` doesn't that bypass this? As a concrete example, GitLab has the concept of protected branches and code owners, both of which allow you to restrict access to the corresponding environments’ credentials to a smaller group of people who have permission to touch the sensitive branches. That allows you to say things like “anyone can run in development but only our release engineers can merge to staging/production” or “changes to the CI configuration must be approved by the DevOps team”, respectively. That does, of course, not prevent someone from running a Bitcoin miner in whatever environment you use to run untrusted merge requests but that’s better than access to your production data.
- shitlord 5y ago
  If you're using AWS you should just create separate accounts for each environment. This doesn't solve the issue of a compromised release orchestrator deploying malicious code to production. But it is helpful for other reasons: it reduces single points of failure, IAM policies becomes harder to over-scope, etc.
formerly_proven 5y ago
Yeah but I mean... that's why CI and CD are separate things. CI should not need any privileges. CI builds should be hermetic (no network access, no persistence, ideally fully reproducible from what's going in). CI should not talk to servers on your network, let alone have credentials, especially for production systems.
- NAHWheatCracker 5y ago
  CI for me has to access repositories. Eg. downloading libraries from PyPI, Maven, NPM, etc... For private repositories, that means access to credentials. Probably read-only credentials, but it requires network access. Would you be suggesting that everyone should commit all dependencies?
  - formerly_proven 5y ago
    That's a fair point. I think for many projects which only consume dependencies the answer can be "yes, just commit all your dependencies" in many instances. Pulling from internal repos shouldn't be critical though as long as the read-only tokens you're using can only access what the developers can read anyway; pulling "unintended" code in a CI should never be able to escalate because we're running anything a dev pushed.
    - NAHWheatCracker 5y ago
      I've heard of doing this, although not worked on such a project. Committing node_modules seems wild to me and I foresee issues with committing all the Python wheels necessary for different platforms on some projects. I'm a proponent of lock-file approaches, which gain 99% of the benefits with far less pain. It requires network access, though.
      - emteycz 5y ago
        Yarn v2 is much better for committing dependencies than committing node_modules. However, it's problematic. Only use it if you're certain it will solve a specific problem you have. Consider using Docker to build images that include a snapshot of node_modules.
cshokie 5y ago
It is also possible to segment the build pipelines into separate CI and PR builds, where only CI builds have access to any secrets. The PR pipeline just builds and tests, and all of the other tasks only happen in CI once a change has been merged into the main branch. That mitigates the “random person creating a malicious PR” problem because it has to be accepted and merged before it can do anything bad.

by nickjj 5y ago

heavenlyblue 5y ago

The solution to that is simple: do not do any tests with production secrets :)

reflectiv 5y ago

Each environment should have its own keys for the services it is talking to so ideally this would restrict the scope of damage.

nickjj 5y ago
> Each environment should have its own keys for the services it is talking to so ideally this would restrict the scope of damage. If a developer changes the CI pipeline file to make their PR's code run in `deployment: "production"` instead of `deployment: "test"` doesn't that bypass this? Edit: I'll leave my original question here because I think it's an important one but I answered this myself. It depends on which CI provider you're using but some of them do let you restrict specific deployments from being run only on specific branches or by specific folks (such as repo admins). In the above case if the production deployment was only allowed to run on the main branch and the only way code makes its way into the main branch is after at least 1 person reviewed + merged it (or whatever policy your company wants) then a rogue developer can't edit the pipeline in an unreviewed PR to make something run in production. Also with deployment specific environment variables then a rogue developer is also not able to edit a pipeline file to try and run commands that may affect production such as doing a `terraform apply` or pushing an unreviewed Docker image to your production registry.
- Nextgrid 5y ago
  CI should only have environment variables needed for testing. For building/deploying to production, it just has to push the code/package/container image, not run it, meaning it has no need for production-level credentials. CI should never ever have access to anything related to production; not just for security but also to prevent potentially bad code being run in tests from trashing production data.
- acdha 5y ago
  > If a developer changes the CI pipeline file to make their PR's code run in `deployment: "production"` instead of `deployment: "test"` doesn't that bypass this? As a concrete example, GitLab has the concept of protected branches and code owners, both of which allow you to restrict access to the corresponding environments’ credentials to a smaller group of people who have permission to touch the sensitive branches. That allows you to say things like “anyone can run in development but only our release engineers can merge to staging/production” or “changes to the CI configuration must be approved by the DevOps team”, respectively. That does, of course, not prevent someone from running a Bitcoin miner in whatever environment you use to run untrusted merge requests but that’s better than access to your production data.
shitlord 5y ago
If you're using AWS you should just create separate accounts for each environment. This doesn't solve the issue of a compromised release orchestrator deploying malicious code to production. But it is helpful for other reasons: it reduces single points of failure, IAM policies becomes harder to over-scope, etc.

formerly_proven 5y ago

Yeah but I mean... that's why CI and CD are separate things. CI should not need any privileges. CI builds should be hermetic (no network access, no persistence, ideally fully reproducible from what's going in). CI should not talk to servers on your network, let alone have credentials, especially for production systems.

NAHWheatCracker 5y ago
CI for me has to access repositories. Eg. downloading libraries from PyPI, Maven, NPM, etc... For private repositories, that means access to credentials. Probably read-only credentials, but it requires network access. Would you be suggesting that everyone should commit all dependencies?
- formerly_proven 5y ago
  That's a fair point. I think for many projects which only consume dependencies the answer can be "yes, just commit all your dependencies" in many instances. Pulling from internal repos shouldn't be critical though as long as the read-only tokens you're using can only access what the developers can read anyway; pulling "unintended" code in a CI should never be able to escalate because we're running anything a dev pushed.
  - NAHWheatCracker 5y ago
    I've heard of doing this, although not worked on such a project. Committing node_modules seems wild to me and I foresee issues with committing all the Python wheels necessary for different platforms on some projects. I'm a proponent of lock-file approaches, which gain 99% of the benefits with far less pain. It requires network access, though.
    - emteycz 5y ago
      Yarn v2 is much better for committing dependencies than committing node_modules. However, it's problematic. Only use it if you're certain it will solve a specific problem you have. Consider using Docker to build images that include a snapshot of node_modules.

cshokie 5y ago

It is also possible to segment the build pipelines into separate CI and PR builds, where only CI builds have access to any secrets. The PR pipeline just builds and tests, and all of the other tasks only happen in CI once a change has been merged into the main branch. That mitigates the “random person creating a malicious PR” problem because it has to be accepted and merged before it can do anything bad.