-
Notifications
You must be signed in to change notification settings - Fork 10.6k
refactor(core): Improve environment variable handling in shell execution #14742
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
Summary of ChangesHello @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 refactors the core shell execution service to provide more controlled handling of environment variables when spawning child processes. The primary goal is to prevent the automatic inheritance of all parent process environment variables in specific scenarios, particularly within CI/CD environments, while ensuring that essential variables are still passed. This change improves security and predictability by limiting the exposure of potentially sensitive or irrelevant environment data to spawned shells. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
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 improves security by conditionally passing environment variables to spawned shell processes, preventing the parent process's environment from being inherited in CI environments. The implementation is sound, but the new security logic is duplicated. My feedback focuses on refactoring this duplication to improve maintainability and reduce future risks.
|
Size Change: +866 B (0%) Total Size: 21.5 MB ℹ️ View Unchanged
|
|
/patch preview |
|
✅ Patch workflow(s) dispatched successfully! 📋 Details:
🔗 Track Progress: |
…ion (#14742) Co-authored-by: Jack Wotherspoon <[email protected]> Co-authored-by: christine betts <[email protected]>
|
🚀 Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
|
🚀 Patch Release Started! 📋 Release Details:
⏳ Status: The patch release is now running. You'll receive another update when it completes. 🔗 Track Progress: |
|
✅ Patch Release Complete! 📦 Release Details:
🎉 Status: Your patch has been successfully released and published to npm! 📝 What's Available:
🔗 Links: |
…ion (google-gemini#14742) Co-authored-by: Jack Wotherspoon <[email protected]> Co-authored-by: christine betts <[email protected]>
…e handling in shell execution (google-gemini#14742)
Summary
This PR addresses to improve environment variable handling in the shell execution service.
Details
The shell execution service has been updated to improve environment variables in shell. This involved modifying how
process.envis passed to spawned shell processes and ensuring proper cleanup of test environments.Related Issues
None.
How to Validate
The changes were validated by running existing unit tests for
shellExecutionService.test.ts. Also setting GITHUB_SHA or SURFACE env var.Pre-Merge Checklist