Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

crates/vm/src/stdlib/_winapi.rs (1)

233-254: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use WTF-8 conversion for CreateProcess arguments to support surrogate paths.

The current wcstring closure uses WideCString::from_str(s.expect_str()), which narrows to UTF-8-only strings. This is inconsistent with the rest of the file, which uses as_wtf8().to_wide_cstring() to preserve WTF-8 representations (including surrogates) for Windows paths. The expect_str() call will panic on valid Windows inputs containing unpaired surrogates that RustPython can represent.

Use as_wtf8().to_wide_cstring() directly, and remove the .transpose()? error handling since this method returns WideCString directly rather than a Result.

Proposed fix

-        let wcstring = |s: PyStrRef| {
-            widestring::WideCString::from_str(s.expect_str()).map_err(|err| err.to_pyexception(vm))
-        };
+        let wcstring = |s: PyStrRef| s.as_wtf8().to_wide_cstring();
@@
-        let app_name = args.name.map(wcstring).transpose()?;
-        let current_dir = args.current_dir.map(wcstring).transpose()?;
+        let app_name = args.name.map(wcstring);
+        let current_dir = args.current_dir.map(wcstring);
         let mut command_line = args
             .command_line
-            .map(|s| wcstring(s).map(|w| w.into_vec_with_nul()))
-            .transpose()?;
+            .map(|s| wcstring(s).into_vec_with_nul());

🤖 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 `@crates/vm/src/stdlib/_winapi.rs` around lines 233 - 254, The wcstring closure
currently uses WideCString::from_str(s.expect_str()), which rejects WTF-8
surrogate data; replace it by calling s.as_wtf8().to_wide_cstring() so
CreateProcess args preserve surrogate paths, and update the call sites (where
app_name = args.name.map(wcstring).transpose()? , current_dir =
args.current_dir.map(wcstring).transpose()? and command_line mapping) to account
for to_wide_cstring() returning WideCString directly (remove the unnecessary
.transpose()? / Result-handling and adjust the command_line mapping to convert
the WideCString into a NUL-terminated Vec using into_vec_with_nul()) so no
panics occur for unpaired surrogates.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

crates/vm/src/stdlib/_winapi.rs (1)

233-254: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align CreateProcess string conversion with the rest of the Windows API surface.

The wcstring closure at line 234 uses WideCString::from_str(s.expect_str()), but the rest of this refactor standardized Win32 inputs to as_wtf8().to_wide_cstring() to handle Python str values with lone surrogates. This makes CreateProcess the inconsistent outlier for name, command_line, and current_dir, and expect_str() can panic on inputs the other Windows APIs now accept safely.

Since null byte validation is already performed in lines 237–249, the closure can be simplified to directly call to_wide_cstring() without the error-mapping wrapper:

Suggested fix

-        let wcstring = |s: PyStrRef| {
-            widestring::WideCString::from_str(s.expect_str()).map_err(|err| err.to_pyexception(vm))
-        };
+        let wcstring = |s: PyStrRef| s.as_wtf8().to_wide_cstring();

Then apply the pattern to all three usages:

-        let app_name = args.name.map(wcstring).transpose()?;
-        let current_dir = args.current_dir.map(wcstring).transpose()?;
+        let app_name = args.name.map(wcstring);
+        let current_dir = args.current_dir.map(wcstring);
         let mut command_line = args
             .command_line
-            .map(|s| wcstring(s).map(|w| w.into_vec_with_nul()))
-            .transpose()?;
+            .map(|s| wcstring(s).into_vec_with_nul());

🤖 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 `@crates/vm/src/stdlib/_winapi.rs` around lines 233 - 254, The CreateProcess
conversion uses WideCString::from_str with s.expect_str(), which is inconsistent
with other Win32 APIs and can panic on lone surrogates; change the wcstring
closure to call s.as_wtf8().to_wide_cstring() (dropping expect_str and the
manual map_err wrapper) and then apply that wcstring to args.name,
args.command_line, and args.current_dir usages (the existing null-byte checks
remain). Ensure you keep the same transpose/? flow for app_name, current_dir,
and command_line so error handling is preserved.

crates/vm/src/stdlib/winsound.rs (1)

159-169: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject negative beep durations before casting to u32.

Lines 165-168 cast args.duration directly to u32. A call like winsound.Beep(440, -1) turns into 4_294_967_295 ms instead of failing fast.

Suggested fix

     if !(37..=32767).contains(&args.frequency) {
         return Err(vm.new_value_error("frequency must be in 37 thru 32767"));
     }
+    if args.duration < 0 {
+        return Err(vm.new_value_error("duration must be non-negative"));
+    }

     if rustpython_host_env::winsound::beep(args.frequency as u32, args.duration as u32) {
         Ok(())
     } else {
         Err(vm.new_runtime_error("Failed to beep"))

🤖 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 `@crates/vm/src/stdlib/winsound.rs` around lines 159 - 169, The Beep wrapper
currently casts args.duration to u32 without rejecting negatives, allowing
values like -1 to wrap to large u32; update the Beep function to validate
args.duration is non-negative (and optionally within a sensible max) before
casting, returning vm.new_value_error on negative durations; locate the check
near the existing frequency validation in Beep (using BeepArgs and
VirtualMachine) and perform the duration check prior to calling
rustpython_host_env::winsound::beep(args.frequency as u32, args.duration as
u32).

crates/vm/src/stdlib/nt.rs (1)

1068-1075: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Keep filename on non-Io readlink failures.

Line 1075 converts the remaining ReadlinkError variants straight into a Python exception, but those variants no longer have access to path. That drops OSError.filename/message context for path-specific failures that are not plain io::Errors. Please keep explicit path-aware mapping here instead of the catch-all conversion.

🤖 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 `@crates/vm/src/stdlib/nt.rs` around lines 1068 - 1075, The non-Io
ReadlinkError variants in fn readlink currently call err.to_pyexception(vm)
which loses the original path context; update the match arm for Err(err) (from
host_nt::readlink / host_nt::ReadlinkError) to map each non-Io variant into an
OSError that retains the filename by using OSErrorBuilder::with_filename(&err,
path.clone(), vm) (or the appropriate constructor that accepts the error and
path) instead of calling to_pyexception directly, so that OSError.filename and
the message include the original path.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

crates/host_env/src/winsound.rs (1)

19-20: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Reject SND_MEMORY | SND_ASYNC combination in the safe wrapper.

When SND_MEMORY is combined with SND_ASYNC on Win32 PlaySoundW, the caller must keep the sound buffer valid until playback completes. The current play_sound() accepts arbitrary flags, allowing another caller to combine these flags and return while WinMM still holds a pointer into borrowed Rust memory. Add validation to reject this unsafe combination.

♻️ Proposed fix

+const SND_ASYNC: u32 = 0x0001;
+
 /// Returns `Ok(())` when `PlaySoundW` returns non-zero, an error otherwise.
 pub fn play_sound(source: PlaySoundSource<'_>, flags: u32) -> Result<(), PlaySoundError> {
+    if matches!(source, PlaySoundSource::Memory(_)) && flags & SND_ASYNC != 0 {
+        return Err(PlaySoundError);
+    }
+
     let ptr: *const u16 = match source {
         PlaySoundSource::Stop => core::ptr::null(),
         PlaySoundSource::Memory(buf) => buf.as_ptr().cast(),
         PlaySoundSource::Name(s) => s.as_ptr(),
     };

🤖 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 `@crates/host_env/src/winsound.rs` around lines 19 - 20, The safe wrapper must
prevent the unsafe combination SND_MEMORY | SND_ASYNC because PlaySoundW
requires the buffer to outlive async playback; update the play_sound() path that
handles the Memory(&'a [u8]) variant to detect when flags include both
SND_MEMORY and SND_ASYNC and return an Err (or other safe error) instead of
calling into WinMM, rejecting the call; reference the Memory enum variant and
the play_sound() function to locate the check and ensure the validation runs
before any FFI call so no borrowed buffer is handed to PlaySoundW when async is
requested.