cleanEnv() in cmd/review-bot/main_test.go is used to build a clean subprocess environment for flag-validation tests. It strips GITEA_*, LLM_*, REVIEWER_*, and other prefixes — but does not strip VCS_TYPE or VCS_URL.
PR #131 added VCS type routing that reads VCS_TYPE at startup. If VCS_TYPE=github or VCS_URL is set in the test runner's environment (e.g., in a CI environment where review-bot tests itself), subprocess tests like TestMainSubprocess_MissingFlags will see unexpected VCS routing behavior, potentially passing or failing for the wrong reason.
For example, TestMainSubprocess_MissingFlags passes --gitea-url (the deprecated alias) as a flag; if VCS_TYPE=github leaks through from the environment, the routing path changes.
Source
PR: #131 — feat: implement GitHub API methods and VCS routing (issue #130)
Code quality rule: No environment variable leakage in subprocess tests
File: cmd/review-bot/main_test.go line 975 (cleanEnv function)
What needs to happen
Add VCS_TYPE and VCS_URL to the cleanEnv() filter:
casestrings.HasPrefix(key,"VCS_"),
This prevents environment contamination in all current and future subprocess tests.
## What was missed
`cleanEnv()` in `cmd/review-bot/main_test.go` is used to build a clean subprocess environment for flag-validation tests. It strips `GITEA_*`, `LLM_*`, `REVIEWER_*`, and other prefixes — but does not strip `VCS_TYPE` or `VCS_URL`.
PR #131 added VCS type routing that reads `VCS_TYPE` at startup. If `VCS_TYPE=github` or `VCS_URL` is set in the test runner's environment (e.g., in a CI environment where review-bot tests itself), subprocess tests like `TestMainSubprocess_MissingFlags` will see unexpected VCS routing behavior, potentially passing or failing for the wrong reason.
For example, `TestMainSubprocess_MissingFlags` passes `--gitea-url` (the deprecated alias) as a flag; if `VCS_TYPE=github` leaks through from the environment, the routing path changes.
## Source
- PR: #131 — feat: implement GitHub API methods and VCS routing (issue #130)
- Code quality rule: No environment variable leakage in subprocess tests
- File: `cmd/review-bot/main_test.go` line 975 (`cleanEnv` function)
## What needs to happen
Add `VCS_TYPE` and `VCS_URL` to the `cleanEnv()` filter:
```go
case strings.HasPrefix(key, "VCS_"),
```
This prevents environment contamination in all current and future subprocess tests.
## References
- [PR #131](https://gitea.weiker.me/rodin/review-bot/pulls/131)
- [Issue #130](https://gitea.weiker.me/rodin/review-bot/issues/130)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
What was missed
cleanEnv()incmd/review-bot/main_test.gois used to build a clean subprocess environment for flag-validation tests. It stripsGITEA_*,LLM_*,REVIEWER_*, and other prefixes — but does not stripVCS_TYPEorVCS_URL.PR #131 added VCS type routing that reads
VCS_TYPEat startup. IfVCS_TYPE=githuborVCS_URLis set in the test runner's environment (e.g., in a CI environment where review-bot tests itself), subprocess tests likeTestMainSubprocess_MissingFlagswill see unexpected VCS routing behavior, potentially passing or failing for the wrong reason.For example,
TestMainSubprocess_MissingFlagspasses--gitea-url(the deprecated alias) as a flag; ifVCS_TYPE=githubleaks through from the environment, the routing path changes.Source
cmd/review-bot/main_test.goline 975 (cleanEnvfunction)What needs to happen
Add
VCS_TYPEandVCS_URLto thecleanEnv()filter:This prevents environment contamination in all current and future subprocess tests.
References