Repository navigation
🤖 fix: tighten plugin env key checks and the loader variable list - #6804
Conversation
…iden the loader list - normalizeStdioEntry refuses an env key with '=', as env_vars names do. - On Windows, a forwarded env_vars name that matches an env key without case is dropped, so the configured value wins. - The loader warning also names BASH_ENV, ENV, LD_AUDIT, NODE_PATH and PYTHONPATH. The docs say so.
|
@codex review |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 664fac0d6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An empty env key would be an unnamed =value entry. Only interactive shells read ENV, so it is dropped from the loader warning.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Closes #6713
This PR does the three follow-ups from the #6690 readiness review (plugin env checks).
=inenvkeys:normalizeStdioEntry(mcpConfig.ts) now refuses anenvkey that is empty or contains=, asenv_varsnames already do. A key such asPLUGIN_DATA=/xwould setPLUGIN_DATAin the joined env block. On Windows libuv sorts that block, so Xum writing the reserved values last does not protect them. Theenvchecks moved, unchanged otherwise, intostdioEnvError, so thatnormalizeStdioEntrydoes not grow (CRAP gate).withForwardedHostEnv(mcpServerManager.ts) drops a forwardedenv_varsname whoseenvNameKeymatches anenvkey. On Windows,TOKENandtokenare one variable, so before this change both keys reached the env block. Now theenvvalue wins, as the docs say. Elsewhere names stay case-sensitive.LOADER_ENV_NAME(openaiMcpAuth.ts) addsBASH_ENV(stdio servers start throughbash -c, which runs the file it names), plusLD_AUDIT,NODE_PATHandPYTHONPATH.ENVis not added: only interactive shells read it, andbash -cis not interactive. The "Plugin server environment" section ofdocs/config/mcp-servers.mdxnow says that the review warns about these names and that Windows compares them without case.Validation
=key loaded and the new loader names had no warning (mcpConfig.test.ts), and on win32 both case variants reached the env (mcpServerManager.test.ts).mcpConfig,openaiMcpAuthandmcpServerManagersuites: 443 pass.make typecheck,make static-check,MUX_ESLINT_CONCURRENCY=1 make lint-eslintandmake crap-checkpass.Risks
Low. A plugin whose
envhas a key with=no longer loads that server. Such a key could not set a usable variable anyway.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high