feat: introduce sign mode for driver packaging, allowing skipping of signing steps by svasista-ms · Pull Request #649 · microsoft/windows-drivers-rs

svasista-ms · 2026-04-27T10:47:36Z

This PR adds a --sign-mode option to the cargo wdk build command that lets you turn off test signing which is useful for production/HLK scenarios.

Resolves #588

Functionality

--sign-mode takes two values:

test: generates a self-signed cert and signs artifacts with it. Same behavior as today
off: skips all signing including cert generation

test is the default so if --sign-mode is omitted the command behaves exactly as today and thus remains backwards compatible.

An error is returned if you try to use --verify-signature with --sign-mode=off because verification does not make sense when nothing is signed.

Future Direction

This PR is part of a broader goal to replicate Visual Studio's SignMode setting in cargo-wdk. In the future we plan to:

Introduce a third --sign-mode value called prod to enable a production signing mode that will not generate a test cert and will instead require a user-specified cert
Add CLI options for specifying certs, timestamp servers and signing algorithms

Changes

New sign_mode field in BuildArgs [1]
CLI-layer logic to reject use of --verify-signature with --sign-mode=off [2]
New SignMode enum with Test and Off variants [3].
Plumbed SignMode from BuildAction through PackageTask
In PackageTask::run(), gated generate_certificate, the certificate copy and signtool invocations on SignMode::Test[4]
Unit tests for the new paths [5]
Integration tests [6]

Screenshots

cargo wdk build --help showing --sign-mode:

Building with --sign-mode=off:

Building with --sign-mode=test:

Building without the --sign-mode option -- falls back to the default (test) behavior:

Error when using --verify-signature with --sign-mode=off:

…signing steps

Copilot

Pull request overview

Adds a new build-time signing mode to cargo-wdk so production packaging can skip test-signing (and associated certificate generation/verification), aligning with the request in #588 to produce unsigned driver binaries for external signing.

Changes:

Introduces SignMode (test default, off) and wires it through CLI → BuildAction → PackageTask.
Updates packaging flow to conditionally skip certificate generation, signtool signing, and signature verification when --sign-mode=off.
Adds unit + integration tests covering --sign-mode=off, CLI validation, and help-surface expectations.

Reviewed changes

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

Show a summary per file

File	Description
crates/cargo-wdk/tests/build_command_test.rs	Adds integration tests for `--sign-mode=off`, including “not Authenticode signed” validation.
crates/cargo-wdk/src/cli.rs	Adds `--sign-mode` flag, passes it into build params, and rejects `--verify-signature` with `--sign-mode=off`.
crates/cargo-wdk/src/actions/mod.rs	Defines the `SignMode` enum with parsing/display behavior.
crates/cargo-wdk/src/actions/build/tests.rs	Extends build action tests/mocks to cover sign-mode off behavior and propagation.
crates/cargo-wdk/src/actions/build/package_task.rs	Implements sign-mode branching in packaging, skipping signing steps when off.
crates/cargo-wdk/src/actions/build/mod.rs	Threads `sign_mode` from build action into package task params.

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

codecov-commenter · 2026-04-27T11:15:49Z

Codecov Report

❌ Patch coverage is 82.14286% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.74%. Comparing base (ebc705a) to head (1c1374a).

Files with missing lines	Patch %	Lines
crates/cargo-wdk/src/actions/mod.rs	0.00%	6 Missing ⚠️
crates/cargo-wdk/src/actions/build/package_task.rs	83.33%	1 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #649      +/-   ##
==========================================
+ Coverage   78.69%   78.74%   +0.05%     
==========================================
  Files          25       25              
  Lines        5234     5275      +41     
  Branches     5234     5275      +41     
==========================================
+ Hits         4119     4154      +35     
- Misses        995     1001       +6     
  Partials      120      120

☔ View full report in Codecov by Sentry.
📢 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.

Co-authored-by: Copilot <copilot@github.com>

…ification enabled Co-authored-by: Copilot <copilot@github.com>

Copilot

Pull request overview

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

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

Copilot · 2026-04-27T13:21:21Z

+                )?;
+            }
+            SignMode::Off => {
+                info!("Sign mode is 'Off'; skipping certificate generation and signing");


When --sign-mode=off, the package directory is not cleaned (it’s only created if missing). If a previous build ran in Test mode, WDRLocalTestCert.cer may already exist in the package folder and will be left behind because the copy step is skipped, which can defeat the goal of producing a package without the test cert artifact. Consider explicitly removing any existing cert file(s) (both in the package folder and staged under target) when SignMode::Off is selected, or recreate/clean the package directory before copying artifacts (may require adding a remove_file/remove_dir_all method to the Fs provider so this is testable).

Suggested change

info!("Sign mode is 'Off'; skipping certificate generation and signing");

info!("Sign mode is 'Off'; skipping certificate generation and signing");

for cert_path in [&self.dest_cert_file_path, &self.src_cert_file_path] {

match std::fs::remove_file(cert_path) {

Ok(()) => {

info!(

"Removed stale certificate artifact: {}",

cert_path.to_string_lossy()

);

}

Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}

Err(err) => return Err(err.into()),

}

}

Checked how MSBuild/WDK handles this in WindowsDriver.Common.targets (10.0.26100.0): all signing targets are gated on SignMode == TestSign|ProductionSign, same as this PR. There's no mode-switch cleanup. I think we should let cleanup be handled by cargo wdk clean similar to MSBuild's Clean. If this UX becomes an issue, I can revisit it and implement something to ensure the final package is not stale.

Why are we adding the cert to the package? Is that something Visual Studio does too?

Good question! cargo-wdk actually follows the cargo-make based approach in rust-driver-makefile.toml:452-468, which in turn was derived from WDK's MSBuild targets. Shipping the cert for "Test-Signing" workflows makes sense because the cert needs to be installed on the target (test) machine. I need to re-verify the behavior for C++ drivers in Visual Studio.

Okay.

Yeah, let's verify the C++ behaviour.

Copilot

Pull request overview

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

gurry · 2026-05-07T04:33:09Z

+/// * `Test` - Use test-signing, which currently auto-generates a self-signed
+///   certificate (`WDRLocalTestCert` in the `WDRTestCertStore` store) and signs
+///   the driver binary and catalog file with it.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]


Suggested change

#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]

#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]

#[value(rename_all = "lower")]

In help etc. we want to say "test" and "off" instead of "Test" and "Off" as typing the former is easier for users.

gurry · 2026-05-07T04:34:15Z

+                )?;
+            }
+            SignMode::Off => {
+                info!("Sign mode is 'Off'; skipping certificate generation and signing");


Suggested change

info!("Sign mode is 'Off'; skipping certificate generation and signing");

info!("Sign mode is 'off'; skipping certificate generation and signing");

We want to use and promote lower case values of sign mode everywhere since they are easier to type.

gurry · 2026-05-07T04:35:05Z

-            self.run_signtool_verify(&self.dest_driver_binary_path)?;
-            self.run_signtool_verify(&self.dest_cat_file_path)?;
+            if matches!(self.sign_mode, SignMode::Off) {
+                warn!("Skipping signature verification because sign mode is 'Off'");


Suggested change

warn!("Skipping signature verification because sign mode is 'Off'");

warn!("Skipping signature verification because sign mode is 'off'");

gurry · 2026-05-07T04:39:08Z

It's odd that we're printing the error twice:

Why is that? Is this true for other errors as well? What can we do to fix it?

gurry · 2026-05-07T04:52:16Z

+// Given: A driver project
+// When: --sign-mode=off is provided
+// Then: It builds successfully and skips all certificate generation and
+// signing steps (no makecert / certmgr / signtool sign calls).


Remove these comments for all the new tests for consistency because we don't use them on existing tests either. The function names themselves are super verbose so no need for comments.

gurry · 2026-05-07T05:00:43Z

+////////////////////////////////////////////////////////////////////////////////
+/// Sign-mode integration tests
+////////////////////////////////////////////////////////////////////////////////
+


Remove this header comment as we don't use such comments anywhere else in this file

…ests

feat: introduce sign mode for driver packaging, allowing skipping of …

ba4efe9

…signing steps

Copilot AI review requested due to automatic review settings April 27, 2026 10:47

Copilot started reviewing on behalf of svasista-ms April 27, 2026 10:48 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

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

svasista-ms and others added 2 commits April 27, 2026 17:27

refactor: remove Authenticode signature checks from sign mode off tests

c3a080e

Co-authored-by: Copilot <copilot@github.com>

test: add verification skip test for sign mode off with signature ver…

490f323

…ification enabled Co-authored-by: Copilot <copilot@github.com>

Copilot AI review requested due to automatic review settings April 27, 2026 13:14

Copilot started reviewing on behalf of svasista-ms April 27, 2026 13:15 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Merge branch 'main' into sign-mode-off

a13c7e3

svasista-ms marked this pull request as ready for review May 6, 2026 11:43

Copilot AI review requested due to automatic review settings May 6, 2026 11:43

Copilot started reviewing on behalf of svasista-ms May 6, 2026 11:44 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

gurry reviewed May 7, 2026

View reviewed changes

refactor: sign mode variant names to lowercase and clean up related t…

1c1374a

…ests

-                info!("Sign mode is 'Off'; skipping certificate generation and signing");
+                info!("Sign mode is 'Off'; skipping certificate generation and signing");
+                for cert_path in [&self.dest_cert_file_path, &self.src_cert_file_path] {
+                    match std::fs::remove_file(cert_path) {
+                        Ok(()) => {
+                            info!(
+                                "Removed stale certificate artifact: {}",
+                                cert_path.to_string_lossy()
+                            );
+                        }
+                        Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
+                        Err(err) => return Err(err.into()),
+                    }
+                }

	#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
	#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
	#[value(rename_all = "lower")]

	info!("Sign mode is 'Off'; skipping certificate generation and signing");
	info!("Sign mode is 'off'; skipping certificate generation and signing");

	warn!("Skipping signature verification because sign mode is 'Off'");
	warn!("Skipping signature verification because sign mode is 'off'");

Conversation

svasista-ms commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Functionality

Future Direction

Changes

Screenshots

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codecov-commenter commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gurry May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gurry commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

svasista-ms commented Apr 27, 2026 •

edited

Loading

codecov-commenter commented Apr 27, 2026 •

edited

Loading

gurry May 7, 2026 •

edited

Loading

gurry commented May 7, 2026 •

edited

Loading