[WIP] moving copy-igvmfile from test to prod vmgstool

[SethHollandsworth]

we probably can't gate the new copy-igvmfile test subcommand to canary regions so we'll need to make a prod build that has copy-igvmfile as a normal command. This PR moves those functions from test into main

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Moves the copy-igvmfile functionality out of the test subcommand and into the production vmgstool CLI so it can be shipped as a normal command.

Changes:

• Promoted CopyIgvmfile from TestOperation to a top-level CLI command in main.rs
• Moved IGVM/DLL resource parsing logic and ResourceCode enum into production code paths
• Made resource_dll_parser a non-optional dependency to support production builds

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
vm/vmgs/vmgstool/src/test.rs	Removes CopyIgvmfile and associated helpers/tests from the test-only command set
vm/vmgs/vmgstool/src/main.rs	Adds a new CopyIgvmfile CLI command plus production implementations of IGVM read/write helpers and tests
vm/vmgs/vmgstool/Cargo.toml	Promotes resource_dll_parser to a required dependency and updates test_helpers feature

[Copilot]

read_igvmfile takes ownership of a PathBuf, which forces callers to allocate/clone (e.g., to_path_buf()) and is inconsistent with the surrounding APIs that accept impl AsRef<Path>. Consider changing the signature to accept impl AsRef<Path> (or &Path) and only allocate when needed (if ever), while still including the path in error context.

[Copilot]

The error message on Ok(None) is very specific (\"File is not a valid PE DLL\"), but None can also plausibly represent “valid PE, but resource not found” depending on try_find_resource_from_dll semantics. Since this is now a production command, please make the message accurately describe the failure (and ideally include the resource type/id and path) or split the cases if the parser can distinguish them.

Suggested change

	let file = File::open(dll_path).map_err(Error::DataFile)?;
	// Try to find the resource in the DLL
	let descriptor = resource_dll_parser::DllResourceDescriptor::new(b"VMFW", resource_code as u32);
	let (start, len) = resource_dll_parser::try_find_resource_from_dll(&file, &descriptor)
	.map_err(Error::IgvmFile)?
	.ok_or_else(|| Error::IgvmFile(anyhow::anyhow!("File is not a valid PE DLL")))?;
	let file = File::open(&dll_path).map_err(Error::DataFile)?;
	// Try to find the resource in the DLL
	let resource_id = resource_code as u32;
	let descriptor = resource_dll_parser::DllResourceDescriptor::new(b"VMFW", resource_id);
	let (start, len) = resource_dll_parser::try_find_resource_from_dll(&file, &descriptor)
	.map_err(Error::IgvmFile)?
	.ok_or_else(|| {
	Error::IgvmFile(anyhow::anyhow!(
	"Unable to load resource type 'VMFW' id {} from '{}': file is not a valid PE DLL or the requested resource was not found",
	resource_id,
	dll_path.display()
	))
	})?;

[Copilot]

This allocates len bytes directly from DLL metadata with no upper bound. Now that copy-igvmfile is a production command, it’s safer to guard against pathological/hostile inputs (e.g., a crafted DLL advertising a huge resource) by enforcing a reasonable maximum IGVM size (or checking against the VMGS/destination capacity) before allocating/reading.

[Copilot]

Making resource_dll_parser a required dependency means every vmgstool build now pulls it in, even when copy-igvmfile isn’t used. If build size/compile time or platform support is a concern, consider introducing a dedicated feature for the copy-igvmfile command (enabled by default for your prod build) and keeping the dependency optional behind that feature, rather than tying it to test_helpers or always-on.

[Copilot]

Making resource_dll_parser a required dependency means every vmgstool build now pulls it in, even when copy-igvmfile isn’t used. If build size/compile time or platform support is a concern, consider introducing a dedicated feature for the copy-igvmfile command (enabled by default for your prod build) and keeping the dependency optional behind that feature, rather than tying it to test_helpers or always-on.

[Copilot]

The new test helper is substantial and further increases the size/complexity of main.rs. Consider moving create_test_vmfw_dll (and the IGVM read/write test) into a dedicated test module/file (or a small internal tests_util module) so production CLI code remains easier to navigate.