-
Notifications
You must be signed in to change notification settings - Fork 12
chore: stash move non sensitive data out of sops files #604
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @olgakonsta, 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!
Summary of Changes
Hello team, gemini-code-assist here to provide a summary of this pull request. The primary goal of this PR, as indicated by the title, is a chore task to move non-sensitive configuration data out of the sops-encrypted environment files (.enc.env) and into the standard, unencrypted application configuration files (.yaml). This helps ensure that the .enc.env files only contain truly sensitive secrets, improving clarity and security posture.
Highlights
- Configuration Restructuring: This PR moves several configuration variables, specifically contract addresses and the environment type, from encrypted
.enc.envfiles to their corresponding unencrypted.yamlapplication configuration files. - Reduced Secrets Surface Area: By moving non-sensitive data out of the sops-encrypted files, the
.enc.envfiles now contain fewer variables, focusing solely on sensitive information. - Hardcoded Values: The contract addresses and environment type are now hardcoded directly within the
.yamlfiles for each environment (frontend, holesky, prod, staging).
Changelog
Click here to see the changelog
- stash/app-frontend.yaml
- Hardcoded
GASPV2_TOKEN_ADDRESS,CONTRACT_ADDRESS_ETH,CONTRACT_ADDRESS_ARB,CONTRACT_ADDRESS_BASE,CONTRACT_ADDRESS_SONICwith specific values (lines 20-24). - Hardcoded
ENVIRONMENTto 'frontend' (line 29).
- Hardcoded
- stash/app-holesky.yaml
- Hardcoded
GASPV2_TOKEN_ADDRESS,CONTRACT_ADDRESS_ETH,CONTRACT_ADDRESS_ARB,CONTRACT_ADDRESS_BASE,CONTRACT_ADDRESS_SONICwith specific values (lines 20-24). - Hardcoded
ENVIRONMENTto 'testnet' (line 28).
- Hardcoded
- stash/app-prod.yaml
- Hardcoded
GASPV2_TOKEN_ADDRESS,CONTRACT_ADDRESS_ETH,CONTRACT_ADDRESS_ARB,CONTRACT_ADDRESS_BASE,CONTRACT_ADDRESS_SONICwith specific values (lines 20-24). - Hardcoded
ENVIRONMENTto 'prod' (line 29).
- Hardcoded
- stash/app-staging.yaml
- Hardcoded
GASPV2_TOKEN_ADDRESS,CONTRACT_ADDRESS_ETH,CONTRACT_ADDRESS_ARB,CONTRACT_ADDRESS_BASE,CONTRACT_ADDRESS_SONICwith specific values (lines 20-24). - Hardcoded
ENVIRONMENTto 'frontend' (line 29).
- Hardcoded
- stash/frontend.enc.env
- Removed
GASPV2_TOKEN_ADDRESS,CONTRACT_ADDRESS_ETH,CONTRACT_ADDRESS_ARB,CONTRACT_ADDRESS_BASE,CONTRACT_ADDRESS_SONIC, andENVIRONMENTvariables (lines 7-11, 16). - Updated sops metadata (
sops_lastmodified,sops_mac) (lines 20-21).
- Removed
- stash/holesky.enc.env
- Removed
GASPV2_TOKEN_ADDRESS,CONTRACT_ADDRESS_ETH,CONTRACT_ADDRESS_ARB,CONTRACT_ADDRESS_BASE,CONTRACT_ADDRESS_SONIC, andENVIRONMENTvariables (lines 7-10, 16). - Updated sops metadata (
sops_lastmodified,sops_mac) (lines 20-21).
- Removed
- stash/prod.enc.env
- Removed
GASPV2_TOKEN_ADDRESS,CONTRACT_ADDRESS_ETH,CONTRACT_ADDRESS_ARB,CONTRACT_ADDRESS_BASE,CONTRACT_ADDRESS_SONIC, andENVIRONMENTvariables (lines 7-11, 16). - Updated sops metadata (
sops_lastmodified,sops_mac) (lines 20-21).
- Removed
- stash/staging.enc.env
- Removed
GASPV2_TOKEN_ADDRESS,CONTRACT_ADDRESS_ETH,CONTRACT_ADDRESS_ARB,CONTRACT_ADDRESS_BASE,CONTRACT_ADDRESS_SONIC, andENVIRONMENTvariables (lines 7-10, 16). - Updated sops metadata (
sops_lastmodified,sops_mac) (lines 20-21).
- Removed
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 is currently in preview and 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 to provide feedback.
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.
Secrets hidden deep,
In files that sops does keep.
Addresses now roam free,
In YAML, for all to see.
A chore complete, tidy and neat.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request aims to move non-sensitive configuration data, specifically contract addresses, token addresses, and environment identifiers, out of SOPS-encrypted files and into plain YAML configuration files. This is a good practice as it improves the visibility and manageability of non-sensitive configurations and limits SOPS encryption to truly secret data.
The changes are generally well-implemented. However, there are a few specific configuration values, particularly in the app-staging.yaml file, that require clarification to ensure they are intentional and correct. Additionally, it's worth confirming how the application handles empty string values for contract addresses in certain environments.
Summary of Findings
- Potential Misconfiguration in Staging Environment Name: The
ENVIRONMENTinapp-staging.yamlis set to'frontend', which could lead to staging behaving like a local development environment or connecting to incorrect services based onConfigToChain.tsmappings. This needs clarification. - Potential Contract Address Misconfiguration in Staging: In
app-staging.yaml,CONTRACT_ADDRESS_BASEshares the same value asCONTRACT_ADDRESS_ARB. This could be an error and needs to be verified to prevent incorrect contract interactions. - Handling of Empty Contract Addresses: Several configurations use empty strings for contract addresses (e.g.,
CONTRACT_ADDRESS_BASEandCONTRACT_ADDRESS_SONICinapp-frontend.yaml). Confirmation is needed that the application handles these gracefully.
Merge Readiness
The goal of moving non-sensitive data out of SOPS files is commendable and improves maintainability. However, before this pull request can be merged, it's important to address the points raised regarding specific configuration values in app-staging.yaml (the ENVIRONMENT name and the potentially duplicated CONTRACT_ADDRESS_BASE) as these are flagged with high severity and could lead to significant issues if incorrect. Clarification on how empty contract addresses are handled would also be beneficial.
I am unable to approve pull requests directly; please ensure these concerns are addressed and further review is conducted if necessary before merging.
Run report for d63b8a36
|
| Action | Time | Status | Info | |
|---|---|---|---|---|
| 🟩 | SyncWorkspace |
286.7ms | Passed | |
| 🟩 | SetupToolchain(node:22.14.0) |
5.2s | Passed | |
| ⬛️ | SetupToolchain(system) |
5.2s | Skipped | |
| 🟩 | SyncProject(system, avs-aggregator) |
1ms | Passed | |
| 🟩 | SyncProject(node, stash) |
1.4ms | Passed | |
| 🟩 | SyncProject(system, sequencer) |
1.7ms | Passed | |
| 🟩 | SyncProject(system, contracts) |
1.8ms | Passed | |
| 🟦 | RunTask(stash:buildkit-build-and-push-image-digests) |
630.8ms | Cached | |
| 🟩 | SyncProject(system, updater) |
0.9ms | Passed | |
| 🟩 | SyncProject(system, gasp-node) |
5.8ms | Passed | |
| 🟦 | RunTask(contracts:buildkit-build-and-push-image-digests) |
722.9ms | Cached | |
| 🟦 | RunTask(sequencer:buildkit-build-and-push-image-digests) |
724.3ms | Cached | |
| 🟦 | RunTask(avs-aggregator:buildkit-build-and-push-image-digests) |
274.9ms | Cached | |
| 🟩 | SyncProject(system, gasp-avs) |
1.3ms | Passed | |
| 🟦 | RunTask(updater:buildkit-build-and-push-image-digests) |
239.2ms | Cached | |
| 🟦 | RunTask(gasp-node:buildkit-build-and-push-image-digests-standard-runtime) |
354.4ms | Cached | |
| 🟦 | RunTask(gasp-node:buildkit-build-and-push-image-digests-fast-runtime) |
360.8ms | Cached | |
| 🟩 | SetupToolchain(node:18.20.7) |
8s | Passed | |
| 🟩 | RunTask(contracts:build-image-ci) |
5.6s | Passed | |
| 🟩 | RunTask(stash:build-image-ci) |
6.4s | Passed | |
| And 13 more... |
Expanded report
| Action | Time | Status | Info | |
|---|---|---|---|---|
| 🟩 | RunTask(sequencer:build-image-ci) |
7.1s | Passed | |
| 🟩 | SyncProject(node, ferry-deposit) |
0.5ms | Passed | |
| 🟩 | SyncProject(node, ferry-withdrawal) |
0.5ms | Passed | |
| 🟦 | RunTask(ferry-deposit:buildkit-build-and-push-image-digests) |
291.5ms | Cached | |
| 🟦 | RunTask(ferry-withdrawal:buildkit-build-and-push-image-digests) |
200.5ms | Cached | |
| 🟩 | RunTask(avs-aggregator:build-image-ci) |
6.8s | Passed | |
| 🟩 | RunTask(updater:build-image-ci) |
6.3s | Passed | |
| 🟩 | RunTask(ferry-deposit:build-image-ci) |
7s | Passed | |
| 🟩 | RunTask(ferry-withdrawal:build-image-ci) |
7.2s | Passed | |
| 🟩 | RunTask(gasp-node:build-image-ci) |
11.7s | Passed | |
| 🟩 | RunTask(gasp-avs:buildkit-build-and-push-image-digests-fast-runtime) |
4m 25s | Passed | SLOW |
| 🟩 | RunTask(gasp-avs:buildkit-build-and-push-image-digests-standard-runtime) |
4m 25s | Passed | SLOW |
| 🟩 | RunTask(gasp-avs:build-image-ci) |
21.2s | Passed |
Environment
OS: Linux
Matrix:
prefix = build-docker-images
name = Build docker images
runner = ubuntu-24.04
buildkit-enable = true
buildkit-replicas = 5
command = moon :build-image-ci
Variables:
MOON_COLOR = 2
MOON_DEBUG_PROTO_INSTALL = false
MOON_CACHE = read-write
MOON_VERSION = 1.34.0
Touched files
stash/app-frontend.yaml
stash/app-holesky.yaml
stash/app-prod.yaml
stash/app-staging.yaml
stash/frontend.enc.env
stash/holesky.enc.env
stash/prod.enc.env
stash/staging.enc.env
|
🎉 This PR is included in version stash-v1.1.0-rc.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
No description provided.