docs: correct env var redaction setting keys - #29009
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, 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 addresses inaccuracies in the environment variable redaction documentation. The previous documentation referenced non-existent configuration keys that were incompatible with the project's JSON schema. By updating the documentation to reflect the correct nested structure and including the necessary 'enabled' flag, this change ensures that users can correctly configure and activate the security feature. 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 the 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 counterproductive. 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. Footnotes
|
|
📊 PR Size: size/S
|
There was a problem hiding this comment.
Code Review
This pull request updates the configuration documentation in docs/reference/configuration.md to reflect changes in how environment variable redaction is configured. Specifically, the flat security.allowedEnvironmentVariables and security.blockedEnvironmentVariables settings have been replaced with a nested security.environmentVariableRedaction object containing enabled, allowed, and blocked properties. The JSON configuration example has also been updated to match this new structure. There are no review comments, and I have no feedback to provide.
Note: Security Review has been skipped due to the limited scope of the PR.
|
@googlebot I signed it! |
Empty commit; no file changes.
Summary
The environment variable redaction section of
docs/reference/configuration.mddocumented two setting keys that don't exist, and omitted the one that actually turns the feature on. This corrects the section to match the schema and the code.Details
The section told readers to configure redaction with
security.allowedEnvironmentVariablesandsecurity.blockedEnvironmentVariables. Neither key exists anywhere inschemas/settings.schema.json, and sincesecurityis declared"additionalProperties": false, the documented JSON example is schema-invalid — an editor using the published schema flags it as an error.The real settings sit one level deeper.
packages/cli/src/config/config.tsreadssettings.security?.environmentVariableRedaction?.blocked,...?.allowedand...?.enabled, and the schema defines exactly those three undersecurity.environmentVariableRedaction. The same documentation page already lists them correctly in the settings reference further up, so the page was contradicting itself.I also added
enabledto the section. It defaults tofalse(packages/core/src/config/config.ts:params.enableEnvironmentVariableRedaction ?? false), but the section describes the redaction rules at length without mentioning that any of it requires opting in. Since this is a security feature, the failure mode is quiet and bad: you add the documented keys, nothing reads them, the feature is off anyway, and you believe your secrets are being redacted.The example now uses the nested shape with
enabled: true, so copying it produces working config rather than ignored config.Related Issues
Fixes #29007
How to Validate
This is docs-only, so I verified against the schema and the code rather than at runtime:
grep -c "allowedEnvironmentVariables\|blockedEnvironmentVariables" schemas/settings.schema.jsonreturns0.security.environmentVariableRedactionwith exactlyallowed,blockedandenabled(itselfadditionalProperties: false).securityproperties: the old example's two keys are both unknown (so invalid underadditionalProperties: false), the new example's singleenvironmentVariableRedactionkey is known. I checked this by loadingsettings.schema.jsonand diffing each example's keys againstproperties.security.properties.packages/core/src/config/config.tssetsthis.enableEnvironmentVariableRedaction = params.enableEnvironmentVariableRedaction ?? false, and the schema'senableddefault isfalse.security.environmentVariableRedaction.*entries), which the section now agrees with.A reviewer can confirm quickly by putting the old example in a
settings.jsonwith$schemaset and watching the editor flag both keys.Pre-Merge Checklist
I left the platform matrix unchecked deliberately rather than ticking boxes I didn't exercise: this touches a single
.mdfile with no code path, so there's nothing platform-specific to validate. I also didn't runnpm run preflightfor the same reason — happy to if you'd like it on the record.