rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 6 Wiki Activity

PR #131: cleanEnv() does not strip VCS_TYPE or VCS_URL env vars #135

New Issue
Closed
opened 2026-05-14 22:16:23 +00:00 by rodin · 0 comments
rodin commented 2026-05-14 22:16:23 +00:00
Owner
Copy Link
Copy Source

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:

case strings.HasPrefix(key, "VCS_"),

This prevents environment contamination in all current and future subprocess tests.

References

## 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)
rodin added the ai-reviewbug labels 2026-05-14 22:16:23 +00:00
rodin closed this issue 2026-05-14 22:53:44 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ai-review
bug
needs-review
ready
Ready for human review
 self-reviewed
Self-review passed since last push
 wip
Work in progress - do not start new work on this PR
No Label ai-review bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
security-review-bot rodin (Rodin)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#135
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?