fix(windows): preserve YaXUnit /C launch argument - #45
Conversation
WalkthroughОбновлена обработка зарезервированных ключей запуска и формат передачи ChangesОбработка аргументов запуска
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/use_cases/run_tests/helpers.rs`:
- Around line 347-350: Обновите логирование вокруг
effective_enterprise_launch_keys: сначала получите фактически используемый
список additional_launch_keys, затем передайте его длину в сообщение о resolved
ключах вместо значения из config.tools.enterprise.additional_launch_keys.
Сохраните передачу этого же списка в EnterpriseDsl.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a448c000-0555-4375-9b29-0752ebf9aa1c
📒 Files selected for processing (3)
src/platform/process.rssrc/use_cases/check_syntax.rssrc/use_cases/run_tests/helpers.rs
|
@coderabbitai review |
✅ Action performedReview finished.
|
- pass managed /C payload as a separate argv value - share reserved launch key alias matching across CLI paths - update launch/test docs and regression coverage
62ed707 to
45f38df
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/cli/execute.rs (1)
1469-1473: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winВынести общий список зарезервированных ключей
/C//Execute//Outвdomain/runner.rs.
is_reserved_raw_launch_keyиis_wait_reserved_raw_keyдублируют один и тот же массив["c", "execute", "out"]и одну и ту же проверку черезlaunch_key_alias_matches. Стоит добавить единую функцию (например,pub(crate) fn is_typed_option_alias(raw: &str) -> bool) вsrc/domain/runner.rsрядом сlaunch_key_alias_matchesи использовать её в обоих местах — это устранит риск расхождения списков при будущих изменениях.
src/cli/execute.rs#L1469-L1473: заменить телоis_reserved_raw_launch_keyна вызов нового общего хелпера изcrate::domain::runner.src/use_cases/launch_app.rs#L269-L273: заменить телоis_wait_reserved_raw_keyна вызов того же общего хелпера.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/execute.rs` around lines 1469 - 1473, Вынеси общий список и проверку зарезервированных алиасов в новую функцию `is_typed_option_alias` рядом с `launch_key_alias_matches` в `src/domain/runner.rs`; затем в `src/cli/execute.rs#L1469-L1473` замени тело `is_reserved_raw_launch_key` вызовом этого хелпера, а в `src/use_cases/launch_app.rs#L269-L273` аналогично обнови `is_wait_reserved_raw_key`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/enterprise.rs`:
- Around line 159-162: Update the Windows launch flow between build_launch_args
and process.rs so the /C switch and its payload are passed as one raw argument
in the form /C"payload", rather than separate args entries. Use Windows
CommandExt::raw_arg, reject payloads containing embedded quotes or newline
characters, and preserve the existing argument behavior for other launch
options.
---
Nitpick comments:
In `@src/cli/execute.rs`:
- Around line 1469-1473: Вынеси общий список и проверку зарезервированных
алиасов в новую функцию `is_typed_option_alias` рядом с
`launch_key_alias_matches` в `src/domain/runner.rs`; затем в
`src/cli/execute.rs#L1469-L1473` замени тело `is_reserved_raw_launch_key`
вызовом этого хелпера, а в `src/use_cases/launch_app.rs#L269-L273` аналогично
обнови `is_wait_reserved_raw_key`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d3bac15-6d9d-4e35-9a7b-b09dba611ced
📒 Files selected for processing (10)
SKILL/references/project-workflows.mddocs/CAPABILITIES.mddocs/CONFIGURATION.mdsrc/cli/args.rssrc/cli/execute.rssrc/config/model.rssrc/domain/runner.rssrc/platform/enterprise.rssrc/use_cases/launch_app.rstests/cli_launch.rs
| if let Some(c) = &launch.c { | ||
| args.push(quoted_c_arg(c)); | ||
| args.push("/C".to_owned()); | ||
| args.push(c.clone()); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "raw_arg" --type=rust
rg -n "force_quotes|CommandExt" --type=rust
rg -n "1cv8" src/platform/process.rs -A3 -B3 2>/dev/null
fd -e rs . src/platform | xargs -I{} rg -n "raw_arg|CommandExt" {}Repository: alkoleft/v8-runner-rust
Length of output: 230
🏁 Script executed:
#!/bin/bash
set -eu
echo "== relevant files =="
git ls-files 'src/platform/*.rs' | sort
echo "== enterprise outline =="
ast-grep outline src/platform/enterprise.rs || true
echo "== process outline =="
ast-grep outline src/platform/process.rs || true
echo "== lines in enterprise.rs around launch args =="
sed -n '1,230p' src/platform/enterprise.rs | cat -n
echo "== process.rs first 220 =="
sed -n '1,220p' src/platform/process.rs | cat -n
echo "== search process launching =="
rg -n "Command|args\\(|spawn|exec|ProcessRunner|process" src/platform -SRepository: alkoleft/v8-runner-rust
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -eu
echo "== related docs/comments about raw args / Windows cmd.exe =="
rg -n "`#44`|raw|1cv8|1cv8c|C\\\"|RunUnitTests|ProcessRunner|windows|windows-msvc|raw_arg|force_quotes|quoted_c_arg" . -S \
-g '!target/**' -g '!dist/**' -g '!build/**' -g '!node_modules/**' || trueRepository: alkoleft/v8-runner-rust
Length of output: 50380
🌐 Web query:
Rust std::process::Command on Windows command line escaping args() quote arguments documentation
💡 Result:
On Windows, Rust's std::process::Command handles arguments differently than on Unix because the Windows API (CreateProcessW) accepts a single command-line string rather than an array [1][2]. Rust is responsible for serializing the arguments passed via.arg or.args into this single string, which the child process then parses [1][3]. Key points regarding argument escaping and quoting on Windows include: Standard Behavior: For most applications that follow standard C run-time argument parsing rules, Rust's default escaping logic is safe and sufficient [1][3]. Arguments are passed literally, meaning shell-specific features like glob patterns, variable expansion, or pipe operators are not interpreted [4][2]. Batch File Caveat: A critical exception exists for.bat and.cmd files [1][3]. Because these are executed via cmd.exe, they utilize non-standard, complex argument parsing rules [1][4]. Rust attempts to escape these arguments automatically, but due to the inherent complexity of cmd.exe, it is not always possible to safely escape every input [1][3]. In scenarios where Rust cannot safely escape an argument for a batch file, it will return an InvalidInput error at runtime when attempting to spawn the process [1][3]. Raw Arguments: If you are interacting with tools that use non-standard parsing or if you need to manually control the command line, you can use the CommandExt::raw_arg method [5][3]. This method appends text to the command line without Rust's automatic quoting or escaping [5][6]. This should be used with extreme caution as it bypasses safety mechanisms and can expose your application to command injection vulnerabilities [1][5]. Security Warning: When dealing with untrusted input, particularly when invoking cmd.exe or batch files, exercise significant caution [1][2]. Maliciously crafted arguments can potentially lead to arbitrary shell command execution [1][3]. Where possible, validate input to a safe, known-good subset (such as alphanumeric characters) before passing it to Command [1][5]. For complex command lines involving batch files, consider writing the arguments to a temporary batch file and executing that file instead of building a complex, dynamic command string [5][7].
Citations:
- 1: https://doc.rust-lang.org/std/process/index.html
- 2: https://github.com/rust-lang/rust/blob/master/library/std/src/process.rs
- 3: GHSA-q455-m56c-85mh
- 4: https://doc.rust-lang.org/std/process/struct.Command.html
- 5: https://doc.rust-lang.org/std/os/windows/process/trait.CommandExt.html
- 6: https://github.com/rust-lang/rust/blob/main/library/std/src/sys/args/windows.rs
- 7: https://doc.rust-lang.org/stable/std/os/windows/process/trait.CommandExt.html
🌐 Web query:
Rust std::process::Command.raw_arg appends with space documentation
💡 Result:
The raw_arg method in Rust’s std::os::windows::process::CommandExt trait does not automatically append a space [1][2]. It appends the provided literal text to the command line without performing any quoting or escaping [3][1]. Because raw_arg treats the input as raw text to be added directly to the command line string [4], the responsibility for formatting—including adding necessary spaces between arguments—lies entirely with the caller [1][2]. This method is specifically intended for scenarios where you need to manually construct the command line for applications that do not follow standard argument parsing rules (such as cmd.exe) [1][4]. When using raw_arg, you must handle the assembly of the command line string manually, ensuring that spaces are included where appropriate to separate distinct arguments [1][2]. It is highly recommended to use this method with caution, as it bypasses the standard library's automatic escaping and can introduce security vulnerabilities if used with untrusted inputs [4][5][6].
Citations:
- 1: https://doc.rust-lang.org/std/os/windows/process/trait.CommandExt.html
- 2: https://doc.rust-lang.org/stable/std/os/windows/process/trait.CommandExt.html
- 3: https://doc.rust-lang.org/stable/std/process/struct.Command.html
- 4: https://doc.rust-lang.org/stable/src/std/process.rs.html
- 5: https://doc.rust-lang.org/std/process/index.html
- 6: GHSA-q455-m56c-85mh
Не разделять /C и payload на два Vec<String> при запуске на Windows.
build_launch_args кладёт в args "/C" и затем c.clone() как разные элементы, а process.rs передаёт их через стандартный запуск, где нет Windows-специфичного raw_arg. На Windows такой Command::args(...) не склеит их слитно и payload с пробелами/кавычками попадёт под стандартное экранирование/кавычки, поэтому для 1cv8/1cv8c нужно собирать слитную форму /C"payload" через CommandExt::raw_arg в src/platform/process.rs с отказом при встроенных кавычках и переводах строк.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/platform/enterprise.rs` around lines 159 - 162, Update the Windows launch
flow between build_launch_args and process.rs so the /C switch and its payload
are passed as one raw argument in the form /C"payload", rather than separate
args entries. Use Windows CommandExt::raw_arg, reject payloads containing
embedded quotes or newline characters, and preserve the existing argument
behavior for other launch options.
|
@Agrajaga PR был переработан, проверь пожалуйста под windows |
|
@alkoleft Проверил переработанный вариант под Windows на merged master Реальный запуск через Итого: передача Дополнительно: |
2 participants
Summary
/C"RunUnitTests=..."parameter through a guarded Windows raw command-line argument for1cv8and1cv8c;Root cause
std::process::Command::argescapes the quotes embedded in the complete 1C/Cparameter. The resulting Windows command line contains/C\"RunUnitTests=...\", which the 1C parser ignores, so Enterprise starts in normal mode instead of starting YaXUnit.Impact
YaXUnit launches on Windows now receive the literal command-line form expected by 1C:
Password values are also removed from process debug output.
Validation
cargo check --all-targetspasses.cargo fmt --all -- --checkpasses.git diff --checkpasses.A live YaXUnit information-base run was not performed because this checkout does not contain a configured test information base.
Fixes #44
Summary by CodeRabbit
Исправления
/C,/executeи/out, включая варианты с разными префиксами, пробелами и разделителями./Cи значение payload теперь передаются раздельно./C"RunUnitTests", корректно отклоняются до запуска клиента.Документация