fix(security): block MINIMAX_API_HOST workspace env injection and remove env-driven URL routing [AI-assisted]#67300
Conversation
Greptile SummaryThis PR closes a workspace Confidence Score: 5/5Safe to merge — the security fix is correct, well-tested, and no regressions are introduced on the critical paths. All three fix sites (blocklist, VLM, TTS) are correct and independently tested. The only remaining finding is a P2 unexplained type loosening in readMinimaxOverrides that does not affect runtime behavior or security. No P0/P1 issues remain. extensions/minimax/speech-provider.ts — unexplained SpeechProviderOverrides → Record<string, unknown> type change in readMinimaxOverrides (P2 only). Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/minimax/speech-provider.ts
Line: 75-76
Comment:
**Unexplained type loosening**
The parameter type of `readMinimaxOverrides` was widened from `SpeechProviderOverrides | undefined` to `Record<string, unknown> | undefined`. This change isn't mentioned in the PR description and isn't directly needed by the security fix. Inside the function body, `overrides.model` etc. now resolve to `unknown` rather than `string | undefined`, which silently weakens the compile-time contract. If the intent was to decouple this helper from the SDK type, a comment explaining the reason would help, and the explicit SDK type is generally preferable here for type-safety.
```suggestion
function readMinimaxOverrides(
overrides: SpeechProviderOverrides | undefined,
): MinimaxTtsProviderOverrides {
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: finalize issue changes" | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b4444f6de
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@copilot resolve the merge conflicts in this pull request |
…ove env-driven URL routing [AI-assisted] (openclaw#67300) * fix: address issue * fix: address review feedback * fix: finalize issue changes * fix: address PR review feedback * address review feedback * docs: add changelog entry for PR merge
Summary
.envfiles could setMINIMAX_API_HOST, which was not covered by the_BASE_URLsuffix blocklist insrc/infra/dotenv.ts. This allowed an attacker-controlled workspace to silently redirect credentialed MiniMax requests (including VLM and TTS paths that honor trusted runtime env) to an arbitrary origin, exfiltrating the operator's MiniMax API key via theAuthorization: Bearerheader..envfile in the current working directory — no shared gateway, no adversarial co-tenant, no trusted local state. Any repository could carry this file and trigger credential exfiltration onopenclawstartup.MINIMAX_API_HOSTis now explicitly blocked from workspace.envloading. The fix is at the trust boundary: attacker-controlled workspace dotenv values no longer flow intoprocess.envfor this key.provider-catalog.tsandminimax-web-search-provider.tsstill honorMINIMAX_API_HOSTwhen it comes from operator-controlled runtime env rather than workspace.env.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
MINIMAX_API_HOSTdid not match the_BASE_URLsuffix blocklist rule, soloadWorkspaceDotEnvFile()accepted it and wrote it intoprocess.env. MiniMax request paths and routing helpers already trustedprocess.env.MINIMAX_API_HOST, which is appropriate for operator-controlled runtime env but unsafe when populated from an attacker-controlled workspace.env.MINIMAX_API_HOSTinBLOCKED_WORKSPACE_DOTENV_KEYS; no test asserting the env var is rejected from workspace.env..envtrust-boundary issues where_HOST-suffixed vars were left uncovered.Regression Test Plan (if applicable)
src/agents/minimax-vlm.normalizes-api-key.test.ts,extensions/minimax/speech-provider.test.ts,src/infra/dotenv.test.tsMINIMAX_API_HOSTenv fallback still routes MiniMax VLM requests when present in runtime env.MINIMAX_API_HOSTenv fallback still resolves the MiniMax TTS base URL.MINIMAX_API_HOSTis blocked from workspace.envand appears in the workspace blocklist completeness test.User-visible / Behavior Changes
MINIMAX_API_HOSTis no longer accepted from workspace.env. Trusted operator-controlled runtime env behavior is unchanged: operators can still useMINIMAX_API_HOSTfrom their normal runtime environment for MiniMax VLM, MiniMax TTS, and existing CN/global routing behavior.Diagram (if applicable)
Security Impact (required)
MINIMAX_API_HOSTcan no longer steer credentialed MiniMax requests.envcan no longer overrideMINIMAX_API_HOST. Trusted runtime env behavior remains unchanged, so legitimate operator CN/private endpoint routing continues to work.Repro + Verification
Environment
.envwithMINIMAX_API_HOST=http://127.0.0.1:<port>Steps
pnpm test src/agents/minimax-vlm.normalizes-api-key.test.ts— includes the updated trusted-env fallback case for MiniMax VLM.pnpm test extensions/minimax/speech-provider.test.ts— the MiniMax TTS env-fallback case remains covered and is renamed to reflect trusted runtime-env behavior.pnpm test src/infra/dotenv.test.ts— blocklist completeness coverage includesMINIMAX_API_HOSTand asserts it is not loaded from workspace.env.Expected
MINIMAX_API_HOSTfrom workspace.envis not loaded intoprocess.env.MINIMAX_API_HOSTfor MiniMax routing still works.Actual
MINIMAX_API_HOST, while trusted runtime-env fallback remains covered for MiniMax VLM/TTS.Evidence
MINIMAX_API_HOST; MiniMax VLM/TTS regression coverage is aligned with preserving trusted runtime-env fallback rather than removing it)Human Verification (required)
MINIMAX_API_HOST; MiniMax VLM trusted env fallback test coverage; MiniMax TTS trusted env fallback test coverage.MINIMAX_API_HOSTremains honored; workspace.envno longer injects the key.Review Conversations
Compatibility / Migration
.env.MINIMAX_API_HOSTis no longer accepted from workspace.env.Risks and Mitigations
MINIMAX_API_HOST.