feat(cargo-wdk): add `--signtool-args` passthrough to customize driver signing by svasista-ms · Pull Request #699 · microsoft/windows-drivers-rs

svasista-ms · 2026-07-07T11:13:52Z

Adds --signtool-args passthrough so driver signing can be customized (certificate selection, digest algorithm, timestamping, extra file operands).

When --signtool-args is omitted, cargo-wdk signs with the auto-generated WDR test certificate and default switches.
When --signtool-args is provided, the caller owns the full signtool sign option set except the sign verb and the trailing file operand (spplied by cargo-wdk). Supplying it with --sign-mode=off is rejected.

⚠️ Behavior change

Packaging removes any existing <target>/<profile>/<name>_package folder at the start of each build and recreates it, so stale signing artifacts (e.g. a WDRLocalTestCert.cer from a previous --sign-mode=test build) don't persist across rebuilds.

Screenshots

Resolves #605

…te selection

…separate module

Copilot

Pull request overview

Adds a cargo wdk build --signtool-args passthrough so driver signing can be customized (certificate selection, digest, timestamping, extra operands), and adjusts packaging to stage artifacts in a fresh directory and assemble the final package folder last—preventing stale signing artifacts from persisting across rebuilds.

Changes:

Add --signtool-args to the CLI and plumb it through to signtool sign invocation.
Rework packaging to build in a clean per-build staging directory and rename into the final package folder at the end (fixes stale cert artifact scenarios).
Expand integration/unit tests and documentation to cover the new signing behavior and staging semantics.

Reviewed changes

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

Show a summary per file

File	Description
crates/cargo-wdk/tests/build_command_test.rs	Adds regression + functional integration tests for staging behavior and `--signtool-args`.
crates/cargo-wdk/src/providers/mod.rs	Extends filesystem error enum to cover directory removal failures.
crates/cargo-wdk/src/providers/fs.rs	Adds `remove_dir_all` wrapper to the FS provider for testable directory cleanup.
crates/cargo-wdk/src/cli.rs	Introduces `--signtool-args` and validates signing flag combinations via `TryFrom<&BuildArgs> for SignMode`.
crates/cargo-wdk/src/actions/build/tests.rs	Updates build action unit test expectations for staging-dir + final assembly flow and new `SignMode` shape.
crates/cargo-wdk/src/actions/build/package_task.rs	Implements staging directory flow, package folder assembly, signtool argument tokenization + passthrough, and updates signing behavior.
crates/cargo-wdk/src/actions/build/mod.rs	Adjusts `BuildAction` to clone `SignMode` (now contains owned data).
crates/cargo-wdk/README.md	Documents `--signtool-args`, quoting/tokenization rules, and updated signing/staging semantics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov-commenter · 2026-07-07T11:45:46Z

Codecov Report

❌ Patch coverage is 97.61905% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.87%. Comparing base (21ec8f8) to head (3e87ce0).

Files with missing lines	Patch %	Lines
crates/cargo-wdk/src/actions/build/package_task.rs	96.72%	0 Missing and 6 partials ⚠️
crates/cargo-wdk/src/cli.rs	99.40%	1 Missing ⚠️
crates/cargo-wdk/src/providers/exec.rs	80.00%	0 Missing and 1 partial ⚠️
crates/cargo-wdk/src/providers/fs.rs	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #699      +/-   ##
==========================================
+ Coverage   79.93%   80.87%   +0.93%     
==========================================
  Files          26       26              
  Lines        5633     5923     +290     
  Branches     5633     5923     +290     
==========================================
+ Hits         4503     4790     +287     
  Misses       1002     1002              
- Partials      128      131       +3

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…kaging process

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

krishnakumar4a4 · 2026-07-14T13:03:18Z

+
+/// Builds a `clap::Error` with the given message, rendered with the standard
+/// `cargo wdk build` usage for a consistent CLI experience.
+fn build_error(message: impl std::fmt::Display) -> clap::Error {


nit: we could inline this at call site, no need of a separate function

Yes, it was inline initially. But I added the function because ArgumentConflict error can be constructed from multiple places (2 in try_from for SignMode). Once args for all tools are added we could use anyhow to construct CLI errors.

krishnakumar4a4 · 2026-07-14T13:08:43Z

+
+/// Arguments to `signtool sign` for signing the driver binary and catalog file.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct SigntoolArgs(pub Vec<String>);


Move this declaration to the top to keep all arg type declarations at one place.

krishnakumar4a4 · 2026-07-14T13:15:25Z

+            .expect("args parse");
+        let err = SignMode::try_from(&args).expect_err("should be rejected");
+        assert!(
+            err.to_string().contains("`--sign-mode=off`"),


use the exact message for assertion

`--signtool-args` cannot be used with `--sign-mode=off`.

Can we not match exactly and avoid using contains?

Fixed ✅, using the full message for assertion 👍

krishnakumar4a4 · 2026-07-14T13:19:17Z

+    }
+
+    #[test]
+    fn build_off_mode_maps_to_off() {


Suggested change

fn build_off_mode_maps_to_off() {

fn build_sign_mode_off_maps_correctly() {

krishnakumar4a4 · 2026-07-14T13:23:21Z

+        let err = parse_build_args(&["--signtool-args", "/n \"CN=Contoso"])
+            .expect_err("unterminated quote should be rejected");
+        assert!(
+            err.to_string().contains("unterminated"),


Try and use exact message assertion

Similar suggestion as this: https://github.com/microsoft/windows-drivers-rs/pull/699/changes#r3579259207

krishnakumar4a4 · 2026-07-14T13:25:12Z

+    }
+
+    #[test]
+    fn build_signtool_args_with_verify_signature_maps_both() {


Suggested change

fn build_signtool_args_with_verify_signature_maps_both() {

fn build_verify_signature_works_with_signtool_args() {

krishnakumar4a4 · 2026-07-14T13:29:20Z

@@ -304,25 +402,119 @@ mod tests {

    #[test]
    fn build_rejects_verify_signature_when_sign_mode_is_off() {


As we have multiple tests for build sub command, We should move all the build tests to a submodule here and avoid using build as prefix for each test.

Done ✅ , moved to a separate mod

…tion

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

…ions

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

+Driver Signing:
+      --signtool-args <ARGS>       Additional arguments forwarded verbatim to
+                                   `signtool sign`, as a single quoted string
+                                   (e.g. `--signtool-args "/fd SHA256 /f cert.pfx"`)
+


Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

svasista-ms added 3 commits July 7, 2026 09:53

feat: test signing customization

bdbbe77

Refactor build signing options to use --signtool-args for certifica…

4d0063f

…te selection

test: move functional tests for --signtool-args passthrough into a …

9753ca4

…separate module

Copilot AI review requested due to automatic review settings July 7, 2026 11:13

Copilot started reviewing on behalf of svasista-ms July 7, 2026 11:14 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread crates/cargo-wdk/src/actions/build/package_task.rs Outdated

Comment thread crates/cargo-wdk/src/actions/build/package_task.rs Outdated

Comment thread crates/cargo-wdk/tests/build_command_test.rs

refactor: improve staging and final package directory handling in pac…

6099a73

…kaging process

Copilot AI review requested due to automatic review settings July 8, 2026 10:31

Copilot started reviewing on behalf of svasista-ms July 8, 2026 10:32 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

Comment thread crates/cargo-wdk/src/actions/build/package_task.rs Outdated

Comment thread crates/cargo-wdk/src/actions/build/package_task.rs Outdated

Comment thread crates/cargo-wdk/src/actions/build/package_task.rs Outdated

Comment thread crates/cargo-wdk/README.md

feat: enhance signtool argument handling and improve security warnings

3f7206b

Copilot AI review requested due to automatic review settings July 12, 2026 05:32

Copilot started reviewing on behalf of svasista-ms July 12, 2026 05:32 View session

Copilot AI reviewed Jul 12, 2026

View reviewed changes

Comment thread crates/cargo-wdk/src/providers/exec.rs

Comment thread crates/cargo-wdk/src/actions/build/package_task.rs Outdated

feat: enhance signtool argument handling and improve parsing logic

dcadac9

Copilot AI review requested due to automatic review settings July 12, 2026 12:58

Copilot started reviewing on behalf of svasista-ms July 12, 2026 12:59 View session

Copilot AI reviewed Jul 12, 2026

View reviewed changes

Comment thread crates/cargo-wdk/src/actions/build/package_task.rs

svasista-ms marked this pull request as ready for review July 13, 2026 04:27

refactor: remove staging paths in packaging process

b66c678

Copilot AI review requested due to automatic review settings July 14, 2026 17:15

Copilot started reviewing on behalf of svasista-ms July 14, 2026 17:16 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread crates/cargo-wdk/tests/build_command_test.rs Outdated

Comment thread crates/cargo-wdk/src/cli.rs

krishnakumar4a4 requested changes Jul 15, 2026

View reviewed changes

test: add tests for handling empty signtool args and sign mode valida…

653627a

…tion

Copilot AI review requested due to automatic review settings July 15, 2026 04:48

Copilot started reviewing on behalf of svasista-ms July 15, 2026 04:49 View session