chore: Implement secure CI/CD pipeline #6516
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @galz10, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the security of the CI/CD pipeline by implementing a robust two-stage workflow. It separates the build and test processes into an 'untrusted' stage that handles initial code validation without secret access, and a 'trusted' stage that executes sensitive integration tests only after the untrusted stage successfully completes and using pre-built artifacts. This architecture prevents potentially malicious code from directly accessing secrets, thereby mitigating the risk of secret exfiltration and ensuring a more secure development lifecycle.
Highlights
- Two-Stage CI/CD Pipeline: The core change introduces a two-stage CI/CD pipeline: an 'untrusted' stage (
ci.yml) for initial checks without secrets, and a 'trusted' stage (trusted-ci.yml) for integration tests requiring secrets. - Untrusted CI Workflow (
ci.yml): The untrustedci.ymlworkflow now handles linting, formatting, type checking, and unit tests, then builds the project and uploads compiled artifacts. It operates without access to any sensitive secrets. - Trusted CI Workflow (
trusted-ci.yml): The new trustedtrusted-ci.ymlworkflow is triggered only upon successful completion ofci.yml. It checks out the base branch, downloads artifacts from the untrusted stage, and then runs integration tests using necessary secrets likeGEMINI_API_KEY. - Integration Test Enforcement: To ensure comprehensive testing,
trusted-ci.ymlincludes a step that explicitly fails the workflow if theGEMINI_API_KEYsecret is not available, guaranteeing that integration tests are never inadvertently skipped. - GitHub Environment for Trusted Workflow: The
testjob within the trusted workflow is associated with a GitHub Environment namedtrusted-ci-env, enabling additional security measures such as requiring manual approval for sensitive operations.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a two-stage CI/CD pipeline to improve security by isolating integration tests that require secrets. The change in package.json adds a new test:unit script intended for the initial, untrusted workflow. My review has identified a critical issue in the implementation of this new script. The current command will incorrectly trigger the setup for integration tests, which will likely cause the untrusted CI workflow to fail. I have provided a detailed comment with a suggested fix to ensure the pipeline functions as intended.
| "build:sandbox": "node scripts/build_sandbox.js --skip-npm-install-build", | ||
| "bundle": "npm run generate && node esbuild.config.js && node scripts/copy_bundle_assets.js", | ||
| "test": "npm run test --workspaces --if-present", | ||
| "test:unit": "vitest run --exclude 'integration-tests/**'", |
There was a problem hiding this comment.
The current test:unit command, vitest run --exclude 'integration-tests/**', has a critical flaw. When vitest is run from the project root, it discovers all vitest.config.ts files, including integration-tests/vitest.config.ts.
While --exclude prevents integration tests from running, vitest will still execute the globalSetup script from the integration test configuration (integration-tests/globalSetup.ts). This setup script likely requires secrets that are unavailable in the untrusted CI workflow, which would cause the entire 'untrusted' workflow to fail and block all pull requests.
To fix this, the test:unit script should explicitly run tests for workspaces and scripts without relying on vitest's broad discovery, which incorrectly includes the integration test setup. The suggested change ensures only unit tests are run.
| "test:unit": "vitest run --exclude 'integration-tests/**'", | |
| "test:unit": "npm test && npm run test:scripts", |
galz10
force-pushed
the
galzahavi/add/secure_cicd
branch
from
034fec4 to
7388f8a
Compare
This commit introduces a more secure and robust CI/CD pipeline by separating
untrusted and trusted workflow execution.
Key changes include:
- Introduced a new `.github/workflows/trusted-ci.yml` workflow for
secret-dependent integration tests, triggered by `workflow_run` after
untrusted checks pass in `.github/workflows/ci.yml`.
- `ci.yml` now focuses on linting, unit tests, and building/uploading
combined artifacts, without access to sensitive secrets.
- `trusted-ci.yml` downloads pre-built artifacts, ensuring no direct
execution of untrusted PR source code in the privileged environment.
- Added a "Fail if no access to secrets" step in `trusted-ci.yml` to
enforce integration test execution for all pull requests.
- Refined job permissions in `trusted-ci.yml` to adhere to the principle
of least privilege.
- Added `environment: trusted-ci-env` to the `test` job in `trusted-ci.yml`
to enable manual approval for sensitive operations (to be configured
via GitHub Environments).
galz10
force-pushed
the
galzahavi/add/secure_cicd
branch
from
7388f8a to
e95af5b
Compare
|
Hey, thanks for implementing this. I was actually thinking of doing this in a different way. I was thinking of using an RPC replay library like pollyjs to record RPCs locally and then replay them in CI/CD. This would have a few benefits:
What do you think? |
TLDR
This PR enhances CI/CD security by splitting the pipeline into two distinct workflows:
ci.yml): Runs on all PRs. It lints, runs unit tests, and builds the code. It has no access to secrets.trusted-ci.yml): Runs only after the untrusted workflow succeeds. It downloads the artifacts built in the first stage and runs integration tests using the necessary secrets.This separation prevents untrusted code from pull requests from having access to sensitive secrets, mitigating the risk of secret exfiltration.
Dive Deeper
The core of this change is the introduction of a two-stage CI process using
workflow_runas a trigger.ci.ymlis now the "untrusted" stage. It handles all checks that don't require secrets, such as linting, formatting, type checking, and unit tests. Its final step is to build the project and upload the compileddistdirectories as a single artifact.trusted-ci.ymlworkflow acts as the "trusted" stage. It is triggered only upon the successful completion of aci.ymlrun associated with a pull request. Instead of checking out the potentially malicious PR code, it checks out the base branch and then downloads the build artifacts from the untrusted stage. These pre-built artifacts are then used to run the integration tests, which require access toGEMINI_API_KEY.This approach ensures that the environment with access to secrets never directly executes code from the pull request, only the artifacts generated in the sandboxed, secret-less environment.
To enforce that integration tests are never skipped, a step was added to
trusted-ci.ymlthat explicitly fails the workflow if theGEMINI_API_KEYsecret is not available. This guarantees that all PRs are fully tested.Finally, the
testjob in the trusted workflow is associated with a GitHub Environment namedtrusted-ci-env. This allows for additional protections, such as requiring manual approval before the job can run, providing an extra layer of security for sensitive operations.Reviewer Test Plan
.github/workflows/ci.ymland.github/workflows/trusted-ci.yml.ci.ymldoes not contain any steps that access secrets.trusted-ci.ymlis triggered byworkflow_runand not directly bypull_request.trusted-ci.ymlchecks out the base branch and downloads artifacts rather than checking out the PR head.Testing Matrix
The changes are to the CI/CD pipeline. The existing test suite should pass as before. The primary validation is observing that the GitHub Actions workflows execute correctly.
Linked issues / bugs