Greptile Summary

This PR adds four Windows shell path-resolution variables (PROGRAMFILES, PROGRAMW6432, SYSTEMROOT, WINDIR) to BLOCKED_WORKSPACE_DOTENV_KEYS in dotenv.ts, preventing a malicious workspace .env from redirecting pwsh.exe/powershell.exe resolution to an attacker-controlled binary. The implementation is correct — entries are alphabetically ordered and shouldBlockWorkspaceDotEnvKey already normalizes keys to uppercase before the Set lookup, so all case variants are handled.

Confidence Score: 4/5

Safe to merge — the blocking logic is correct and the security fix is sound; only minor test completeness suggestions remain.

The production change in dotenv.ts is straightforward and correct. The only findings are P2: the test doesn't write uppercase variants of three of the four variable families to the .env file (so those assertions pass trivially), and COMSPEC is a related Windows shell-redirect variable not yet covered. Neither represents a defect in the actual fix.

No files require special attention; both findings are minor test/coverage suggestions.

This is a comment left during a code review.
Path: src/infra/dotenv.test.ts
Line: 302-309

Comment:
**Uppercase variants not written to .env in test**

The .env content only includes mixed-case entries for `ProgramW6432`, `SystemRoot`, and `windir` — their all-uppercase counterparts (`PROGRAMW6432`, `SYSTEMROOT`, `WINDIR`) are never written to the file. After `clearEnv` deletes all 8 keys, the assertions for those three uppercase variants pass trivially (they were deleted and never loaded), so the test doesn't actually exercise blocking of those specific inputs. Adding the uppercase variants to the .env content would make the coverage complete.

```suggestion
            "ProgramFiles=.\\evil-pfiles",
            "PROGRAMFILES=.\\evil-pfiles-upper",
            "ProgramW6432=.\\evil-pw6432",
            "PROGRAMW6432=.\\evil-pw6432-upper",
            "SystemRoot=.\\fake-root",
            "SYSTEMROOT=.\\fake-root-upper",
            "windir=.\\fake-windir",
            "WINDIR=.\\fake-windir-upper",
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/infra/dotenv.ts
Line: 73-79

Comment:
**`COMSPEC` not blocked**

`COMSPEC` is the Windows variable that tells the OS where to find `cmd.exe`. If a workspace `.env` sets `COMSPEC=.\evil.exe`, any code path that spawns a shell via `cmd /c …` (directly or through Node's `spawn`/`exec` with `shell: true`) would resolve to the attacker-supplied binary. Since the PR's stated goal is to block Windows shell trust-root variables, `COMSPEC` fits the same threat model as `WINDIR`/`SYSTEMROOT` and is worth adding to the blocklist for completeness.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: address review feedback" | Re-trigger Greptile}

Codex review: needs maintainer review before merge.

What this changes:

The branch adds COMSPEC to the workspace dotenv blocklist, adds mixed-case and uppercase Windows shell trust-root regression coverage, and records the security hardening in the changelog.

Maintainer follow-up before merge:

This is security-sensitive Windows command-resolution hardening with no discrete automated repair finding; the remaining action is maintainer/security review, validation, and reconciliation with related Windows hardening PRs.

Security review:

Security review cleared: The diff narrows untrusted workspace dotenv influence over Windows command resolution and adds tests/changelog only; it introduces no new dependency, workflow, script, permission, publishing, generated-code, or secret-handling risk.

Addressed in 1ef1c423a859443fbba015f63a0e78027e7e8503.

Quoted comment from @clawsweeper:

Codex review: needs maintainer review before merge.

What this changes:

The PR adds COMSPEC, PROGRAMFILES, PROGRAMW6432, SYSTEMROOT, and WINDIR to the workspace dotenv blocklist and adds a targeted dotenv unit test for mixed-case and uppercase variants.

Maintainer follow-up before merge:

This is a plausible security hardening PR with no blocking finding, but it changes Windows command-resolution trust roots and overlaps a protected maintainer-labeled PR, so the next action is maintainer/security reconciliation and validation rather than an automated repair branch.

Review details

Best possible solution:

Land one canonical, security-reviewed Windows dotenv hardening that blocks these workspace-controlled shell trust roots, including COMSPEC, and reconcile it with #74456 so the repository does not carry parallel partial fixes.

Acceptance criteria:

• pnpm test src/infra/dotenv.test.ts src/agents/shell-utils.test.ts src/process/exec.windows.test.ts
• pnpm check:changed in Testbox before merge if maintainer approval proceeds

What I checked:

• Current main dotenv blocklist missing requested keys: At current main, BLOCKED_WORKSPACE_DOTENV_KEYS contains existing credential, proxy, OpenClaw, and helper-path controls but does not include COMSPEC, PROGRAMFILES, PROGRAMW6432, SYSTEMROOT, or WINDIR. (src/infra/dotenv.ts:13, e8b82d1cf92f)
• Workspace dotenv keys are normalized before blocking: shouldBlockWorkspaceDotEnvKey uppercases the normalized key before checking dangerous host env rules, the explicit set, prefixes, and suffixes, so a blocklist entry such as COMSPEC covers ComSpec and COMSPEC. (src/infra/dotenv.ts:108, e8b82d1cf92f)
• Current main has Windows shell resolution inputs affected by the PR: resolvePowerShellPath reads ProgramFiles, PROGRAMFILES, ProgramW6432, SystemRoot, and WINDIR, while the Windows .cmd wrapper path uses process.env.ComSpec; that makes the requested dotenv blocklist hardening relevant to command resolution. (src/agents/shell-utils.ts:6, e8b82d1cf92f)
• PR patch scope and security pass: The provided PR diff changes only src/infra/dotenv.ts and src/infra/dotenv.test.ts, adds no workflows, dependency sources, lockfiles, package scripts, publishing metadata, generated/vendor files, or new permissions, and the code-execution surface change matches the stated security purpose. (src/infra/dotenv.ts:19, 1ef1c423a859)
• Related protected overlap: The provided related context and local ClawSweeper report show fix(infra): block Windows system path env vars from workspace .env injection #74456 is an open protected maintainer PR for overlapping Windows workspace .env hardening, though it is not identical because this PR also includes COMSPEC.
• Feature-history routing: Available local blame for the central dotenv and Windows command-resolution files points to commit 65b09274... by Peter Steinberger, with the shallow checkout exposing that commit as the baseline for these files. (src/infra/dotenv.ts:13, 65b0927490b3)

Likely related people:

• steipete: Current available blame for the central dotenv and Windows command-resolution files points to Peter Steinberger's baseline commit, and the related fix(infra): block Windows system path env vars from workspace .env injection #74456 review routes adjacent Windows/infra hardening to steipete. (role: recent maintainer; confidence: medium; commits: 65b0927490b3; files: src/infra/dotenv.ts, src/infra/dotenv.test.ts, src/agents/shell-utils.ts)

Remaining risk / open question:

• No tests were run in this read-only sweep; src/infra/dotenv.test.ts and the changed gate still need maintainer validation before merge.
• The PR overlaps open protected fix(infra): block Windows system path env vars from workspace .env injection #74456 but is not an exact duplicate because it adds COMSPEC; a maintainer should decide whether to merge this, combine the coverage, or close one PR after a canonical fix exists.

Codex review notes: model gpt-5.5, reasoning high; reviewed against e8b82d1cf92f.