fix: conda version constraints on Windows

[deepakganesh78]

Summary

• quote cmd.exe metacharacters at the command-line layer when Nox invokes .bat or .cmd launchers
• remove the conda-specific workaround that passed literal quotes through to newer conda versions
• exercise constrained conda installs on Windows again and add regression coverage for batch launchers

Fixes #951.

Validation

• python -m pytest tests/test_command.py tests/test_sessions.py -q (203 passed, 6 skipped)
• prek run ruff-check --files nox/popen.py nox/sessions.py noxfile.py tests/test_command.py tests/test_sessions.py
• prek run ruff-format --files nox/popen.py nox/sessions.py noxfile.py tests/test_command.py tests/test_sessions.py
• prek run mypy --files nox/popen.py nox/sessions.py noxfile.py

[henryiii]

Ran a local review, but I'm not on Windows currently, so it's not not able to verify things, so don't trust it too heavily:

🤖 Claude Opus 5 medium effort review from macOS 🤖

Overview

The PR replaces the conda-specific arg-quoting workaround (_dblquote_pkg_install_args) with a general fix at the process-launch layer: when args[0] is a .bat/.cmd file on Windows, popen() now builds the command line itself and wraps any argument containing & < > ^ | in double quotes, so cmd.exe does not treat them as redirection/pipe operators. This is the right layer for the fix — CreateProcess hands .bat/.cmd to cmd.exe, and subprocess.list2cmdline only quotes for spaces/tabs, so requests<99 reached cmd.exe unquoted (I confirmed CPython has no batch-specific quoting, checked on 3.14). It also generalizes beyond conda to any batch launcher, and correctly leaves conda.exe untouched. The noxfile change re-enables the constrained conda install on Windows, which is the real validation.

Issues

1. Arguments containing " now produce a mis-parsed command line (regression)

_windows_batch_command uses list2cmdline, which escapes embedded quotes with backslashes — but cmd.exe does not understand backslash escapes, it just counts quote characters. Verified locally:

'"urllib3<1.25"' -> '"\"urllib3<1.25\""'
'a"<b'           -> '"a\"<b"'

cmd.exe sees the quote opened at index 0 and closed at index 2, so < ends up outside quotes and is still a redirection. The old code handled both cases explicitly: it passed already-double-quoted args through unchanged, and raised ValueError for un-escapable ones. Since #312, session.conda_install('"urllib3<1.25"') was the sanctioned self-quoting idiom, and tests/test_sessions.py still parametrizes already_dbl_quoted — but that test only asserts the arg reaches _run, so the breakage downstream in popen is invisible to it.

Suggested fix in _windows_batch_command: for an arg that contains both a metacharacter and a ", either unwrap a fully-quoted arg before re-quoting, or raise a clear error (restoring the old contract). Silently emitting a command line cmd.exe will mis-parse is the worst of the three options — it can create stray files or truncate the package spec.

2. % and ! are not protected, but the docstring claims metacharacters are

cmd.exe expands %VAR% even inside double quotes (and !VAR! under delayed expansion). % can't be neutralized at the CreateProcess layer at all, so this may be unfixable here — but "protects cmd.exe metacharacters" overstates what the function does. Either narrow the docstring to the set actually handled, or add a note.

Smaller points

• tests/test_sessions.py:745 — the comment # this will be double quoted if unquoted constraint is present is now false; drop it. The already_dbl_quoted parametrization is vestigial at that layer and belongs in test_command.py instead (see issue 1).
• The deleted ValueError tests removed the only coverage of quote-containing args, with nothing replacing it.
• f"{quoted[0]}{quoted[2:]}" is clever but hard to read. A concrete example in the comment would help: ' requests<99' → '" requests<99"' → '"requests<99"'.
• _CMD_META sits above __dir__, away from the other module constants (DEFAULT_*). Minor placement inconsistency.
• sys.platform.startswith("win") — most of the codebase uses "win32"; nox/command.py also hoists this into a module-level _PLATFORM. Worth matching for consistency (behaviour is the same).

Things that check out

• .bat/.cmd detection is case-insensitive and command.run always passes a resolved str path from which(), so args[0].casefold() is safe.
• An executable path containing a metacharacter (C:\a&b\conda.bat) falls into the quoting branch and is handled.
• test_windows_batch_command is a pure-function test and correctly runs on all platforms; the integration test is properly gated to Windows and covers all five metacharacters against both suffixes.
• No imports became unused (sys and Iterable are still needed in sessions.py and noxfile.py).

I could not execute the Windows path here, so the cmd.exe parsing claims above are from the generated command lines plus cmd.exe quoting rules, not an observed run.

[henryiii]

🤖 AI text below 🤖

Followed up on the earlier macOS review from a Windows 11 machine, so everything below is from actual execution (Python 3.13, cmd.exe), not reasoning about quoting rules.

The core fix works

Reproduced the baseline bug first: unpatched, requests<99 passed to a .bat fails with "The system cannot find the file specified" (<99 parsed as redirection), and name&value actually executes value as a command. With this PR, requests<99 arrives in the child intact, and the full test suite passes on Windows including the new test_run_windows_batch_metacharacter_arg parametrizations (5 metacharacters × 2 suffixes). Thanks — this is the right layer for the fix.

Issue 1 from the review (embedded quotes) is confirmed, and slightly worse than predicted

argument	before this PR (raw list2cmdline)	with this PR
"urllib3<1.25" (the self-quoting idiom from #312)	✅ arrives intact	❌ rc=1, "cannot find the file specified"
a"<b	✅ arrives intact	❌ rc=1
a">b	✅ arrives intact	❌ rc=0, silent success, argument truncated, stray file b created containing the redirected output

The mechanism is as the review guessed: list2cmdline wraps the argument and backslash-escapes interior quotes, but cmd.exe has no backslash escapes and just counts " characters, so the wrapping quote closes at the first interior quote and the metacharacter lands unquoted. Ironically, without the outer wrap the odd \" toggles quote mode such that the metacharacter lands inside a quoted span — which is why all three of these worked before the PR. The a">b case is the worst failure mode: no error, corrupted argument, stray file.

Issue 2 (%VAR%) is confirmed

%TESTVAR% expands to its value even inside the added double quotes ("pkg<9%TESTVAR%" → the child sees pkg<9EXPANDED). This can't be prevented at the CreateProcess layer, so it's just a docstring accuracy issue.

Pushed a follow-up commit

Since maintainer edits are enabled, I pushed a commit on top that:

• raises a clear ValueError from _windows_batch_command for arguments mixing a metacharacter and " (restoring the contract of the removed _dblquote_pkg_install_args "Cannot escape" branch, and keeping the a">b case from silently corrupting the command). With this PR the quoting idiom is obsolete anyway — plain urllib3<1.25 now works, and newer conda rejects literal quotes even when they arrive intact (that's session.conda_install fails when specifying min/max version with newer conda #951) — so a loud error pointing at the argument beats silently mis-parsing it;
• keeps the leading-space list2cmdline trick for quote-free arguments (a plain f'"{arg}"' would mishandle trailing backslashes) and adds a concrete example to the comment;
• narrows the docstring to the &<>^| set actually handled, with a note about %;
• adds regression tests: a pure-function test for the three arguments above, and a Windows integration test asserting a">b raises and creates no stray file;
• drops the now-stale "this will be double quoted" comment in test_sessions.py.

Verified on Windows: 207 passed, 6 skipped (no conda on this machine, so the conda integration test skips; the batch-launcher mechanics are what's exercised), ruff check/ruff format clean on the touched files (the two RUF036 hits are pre-existing on main), mypy clean.

[henryiii]

And trying a GPT 5.6 Sol review (not from Windows):

🤖 AI text below 🤖

The new Windows batch quoting still permits arguments that cmd.exe expands or interprets and regresses support for previously accepted pre-quoted requirements.

Full review comments:

• [P1] Reject unescapable percent expansions — /Users/henryfs/git/software/nox/nox/popen.py:49-50
  On Windows, an argument such as %PATH% passed to a .bat or .cmd file is silently expanded by cmd.exe, potentially changing data or injecting command text from the environment. Since the comment acknowledges that quoting cannot preserve %, this path should reject such arguments rather than passing them through unchanged.

• [P2] Preserve pre-quoted metacharacter arguments — /Users/henryfs/git/software/nox/nox/popen.py:57-62
  On Windows, callers that already quote a requirement such as "urllib3<1.25" now hit this ValueError, whereas the removed session helper explicitly accepted this existing input form. Strip or recognize balanced outer quotes before applying the new escaping so previously valid conda_install calls continue to work.

• [P1] Quote parentheses for batch commands — /Users/henryfs/git/software/nox/nox/popen.py:43-43
  When a batch argument contains ( or ) without whitespace, this set treats it as ordinary text and list2cmdline leaves it unquoted, even though parentheses are cmd.exe control characters. Such inputs can cause syntax errors or alter command grouping, so they must receive the same protection as the other metacharacters.

[henryiii]

🤖 AI text below 🤖

Investigated all three findings from the GPT review on a Windows 11 machine (real cmd.exe execution, not reasoning about quoting rules). All three are real; fixed in 921af06.

[P1] Percent expansion — confirmed, worse than stated

%VAR% expands even inside double quotes, and it's a working injection vector: passing %EV% as an argument with EV=& echo INJECTED in the environment actually executed the injected command. Probing cmd's scan rules while I was at it:

input (with DEF=VALUE defined)	child received
%UND%DEF%	%UNDVALUE — after a failed spec, cmd retries at the second %
%%DEF%%	%VALUE% — doubling % does not protect
%DEF:V=W% / %DEF:~0,3%	WALUE / VAL — : modifiers expand at the command line too
%undefined_xy%, 100%, https://x/%20a%20b.whl	intact — undefined references stay literal
%RANDOM%, %__CD__%, %COMSPEC%, %PROMPT%, %PATHEXT% (minimal env)	all expanded — cmd resolves/defines these itself

Fix: reject an argument only when it references a variable that would actually expand — defined in the child environment (case-insensitive, checked against the env that popen() passes to the child), a cmd dynamic/auto variable, or a hidden =C:-style name. Scanning checks every consecutive pair of % characters, which covers the retry-on-undefined behavior above. Blanket % rejection would have broken percent-encoded URLs and trailing-% args that cmd provably passes through literally, so those stay allowed.

Not guarded: delayed expansion (!VAR!). It's off by default for cmd /c, and rejecting ! would break pip-style != constraints, so I left it alone.

[P2] Pre-quoted arguments — confirmed regression

'"urllib3<1.25"' (the #312 workaround) hit the new ValueError. Fix: an argument with balanced outer quotes and no interior quotes now passes through unchanged — its own quotes already protect the metacharacters, and the command line is byte-identical to what the force-quoting branch would build from the unquoted form. The child sees urllib3<1.25 without quotes, which is what newer conda wants.

[P1] Parentheses — confirmed, and the failure is silent

Top-level parens are harmless, but inside an if (...) block — which conda.bat uses — an unquoted ) is live:

argument	plain echo .bat	.bat with if "1"=="1" ( ... %* )
a)b	intact	rc=255, b was unexpected at this time.
(xyz)	intact	rc=0, arrives as (xyz — silent corruption

Quoting fixes both, so ( and ) are now in _CMD_META.

Coverage

Unit tests for rejection/pass-through/literal-percent behavior, plus Windows integration tests including a conda.bat-style if (...) block regression test. 228 passed / 6 skipped across test_command.py + test_sessions.py; ruff and mypy clean.

[henryiii]

(I can't copy paste between machines very easily, so I"m using PR comments to communicate between them, sorry!)

[henryiii]

Okay, I'm happy with that. Didn't grow too much (or at all) after a few back-and-forths between GPT and Claude (used /simplify and manually cleaned up too), and removes the workarounds we had (even in our own noxfile).

Thanks!