-
Notifications
You must be signed in to change notification settings - Fork 58
Add GetDriversFromDirectoryAsync for local driver discovery #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…om local directory Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com>
Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com>
…e parsing Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com>
|
@copilot Translate all the annotations into English. |
Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com>
All Chinese annotations have been translated to English. The changes include:
All bilingual (Chinese / English) comments have been converted to English-only format. Implemented in commit 7ddd2f1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds a new API for discovering driver metadata from a local directory across platforms, wiring it through the public entry point and platform implementations, plus accompanying unit tests and English-only documentation updates.
Changes:
- Added
GetDriversFromDirectoryAsynctoIGeneralDrivelutionand exposed it viaGeneralDrivelution. - Implemented directory-based driver discovery for Windows (
*.inf) and Linux (*.ko+ optional*.deb/*.rpm), with a macOS stub. - Added unit tests for directory discovery scenarios.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/c#/GeneralUpdate.Drivelution/Windows/Implementation/WindowsGeneralDrivelution.cs |
Implements Windows directory scanning/parsing for driver metadata and signature/publisher extraction. |
src/c#/GeneralUpdate.Drivelution/Linux/Implementation/LinuxGeneralDrivelution.cs |
Implements Linux directory scanning/parsing for kernel modules and packages, using external commands for metadata. |
src/c#/GeneralUpdate.Drivelution/MacOS/Implementation/MacOsGeneralDrivelution.cs |
Adds a not-yet-supported stub implementation of the new method. |
src/c#/GeneralUpdate.Drivelution/GeneralDrivelution.cs |
Exposes the new directory discovery method on the unified static entry point; doc comment translation cleanup. |
src/c#/GeneralUpdate.Drivelution/Abstractions/IGeneralDrivelution.cs |
Adds the new API to the core interface; doc comment translation cleanup. |
src/c#/DrivelutionTest/GeneralDrivelutionTests.cs |
Adds tests for directory discovery behavior (non-existent, empty, basic discovery, pattern, cancellation). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// <summary> | ||
| /// Tests that GetDriversFromDirectoryAsync with search pattern filters correctly. | ||
| /// </summary> | ||
| [Fact] | ||
| public async Task GetDriversFromDirectoryAsync_WithSearchPattern_FiltersCorrectly() | ||
| { | ||
| // Arrange | ||
| var testDir = Path.Combine(Path.GetTempPath(), $"drivers_{Guid.NewGuid()}"); | ||
| Directory.CreateDirectory(testDir); | ||
|
|
||
| try | ||
| { | ||
| // Create different types of files | ||
| var infFile = Path.Combine(testDir, "driver1.inf"); | ||
| var txtFile = Path.Combine(testDir, "readme.txt"); | ||
| File.WriteAllText(infFile, "INF content"); | ||
| File.WriteAllText(txtFile, "Text content"); | ||
|
|
||
| // Act | ||
| var result = await GeneralDrivelution.GetDriversFromDirectoryAsync(testDir, "*.inf"); | ||
|
|
||
| // Assert | ||
| Assert.NotNull(result); | ||
|
|
||
| // Should only find .inf files | ||
| // The count depends on whether the platform supports parsing .inf files | ||
| } |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test doesn't assert the filtering behavior it describes: it only checks result is non-null and then exits. As written it will pass even if GetDriversFromDirectoryAsync ignores searchPattern or always returns an empty list. Add assertions that every returned DriverInfo.FilePath matches *.inf and (when running on Windows or when parsing is expected) that exactly one driver is returned for the single .inf file created.
| /// <summary> | ||
| /// Tests that GetDriversFromDirectoryAsync handles cancellation. | ||
| /// </summary> | ||
| [Fact] | ||
| public async Task GetDriversFromDirectoryAsync_WithCancellation_HandlesCancellation() | ||
| { | ||
| // Arrange | ||
| var testDir = Path.Combine(Path.GetTempPath(), $"drivers_{Guid.NewGuid()}"); | ||
| Directory.CreateDirectory(testDir); | ||
|
|
||
| try | ||
| { | ||
| var cts = new CancellationTokenSource(); | ||
| cts.Cancel(); | ||
|
|
||
| // Act | ||
| var result = await GeneralDrivelution.GetDriversFromDirectoryAsync(testDir, null, cts.Token); | ||
|
|
||
| // Assert - Should complete without throwing or return empty list | ||
| Assert.NotNull(result); | ||
| } |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cancellation test only asserts the result is non-null, so it doesn't verify that cancellation is actually honored (e.g., returning an empty/partial list promptly and without doing work). Strengthen it by creating enough files to force iteration and asserting either that the returned list is empty when already-canceled, or that execution stops early (e.g., by checking it returns quickly / fewer results than total).
| // Try to extract publisher from certificate | ||
| using var cert2 = new System.Security.Cryptography.X509Certificates.X509Certificate2(filePath); | ||
| var subject = cert2.Subject; | ||
|
|
||
| // Extract CN (Common Name) from subject | ||
| var cnIndex = subject.IndexOf("CN="); | ||
| if (cnIndex >= 0) | ||
| { | ||
| var cnStart = cnIndex + 3; | ||
| var cnEnd = subject.IndexOf(',', cnStart); | ||
|
|
||
| string publisher; | ||
| if (cnEnd > cnStart) | ||
| { | ||
| publisher = subject.Substring(cnStart, cnEnd - cnStart); | ||
| } | ||
| else | ||
| { | ||
| // No comma after CN, take the rest of the string | ||
| publisher = subject.Substring(cnStart); | ||
| } | ||
|
|
||
| if (!string.IsNullOrEmpty(publisher)) | ||
| { | ||
| driverInfo.TrustedPublishers.Add(publisher); | ||
| } |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The signer certificate extraction is incorrect here: new X509Certificate2(filePath) loads a certificate file (e.g., .cer/.pfx), not the Authenticode signer certificate embedded in a signed driver/INF/PE file. This will usually throw or produce unrelated data, so TrustedPublishers will be wrong/missing. Prefer using X509Certificate.CreateFromSignedFile(filePath) (as already done in WindowsSignatureHelper) and then wrap it in X509Certificate2, and extract the publisher via GetNameInfo/distinguished name parsing rather than manual CN= substring parsing.
| // Try to extract publisher from certificate | |
| using var cert2 = new System.Security.Cryptography.X509Certificates.X509Certificate2(filePath); | |
| var subject = cert2.Subject; | |
| // Extract CN (Common Name) from subject | |
| var cnIndex = subject.IndexOf("CN="); | |
| if (cnIndex >= 0) | |
| { | |
| var cnStart = cnIndex + 3; | |
| var cnEnd = subject.IndexOf(',', cnStart); | |
| string publisher; | |
| if (cnEnd > cnStart) | |
| { | |
| publisher = subject.Substring(cnStart, cnEnd - cnStart); | |
| } | |
| else | |
| { | |
| // No comma after CN, take the rest of the string | |
| publisher = subject.Substring(cnStart); | |
| } | |
| if (!string.IsNullOrEmpty(publisher)) | |
| { | |
| driverInfo.TrustedPublishers.Add(publisher); | |
| } | |
| // Try to extract publisher from Authenticode signer certificate | |
| using var signerCert = System.Security.Cryptography.X509Certificates.X509Certificate.CreateFromSignedFile(filePath); | |
| using var cert2 = new System.Security.Cryptography.X509Certificates.X509Certificate2(signerCert); | |
| // Use certificate API to get a user-friendly name (publisher/common name) | |
| var publisher = cert2.GetNameInfo(System.Security.Cryptography.X509Certificates.X509NameType.SimpleName, false); | |
| if (!string.IsNullOrEmpty(publisher)) | |
| { | |
| driverInfo.TrustedPublishers.Add(publisher); |
| { | ||
| try | ||
| { | ||
| var fileInfo = new FileInfo(filePath); |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fileInfo is created but never used, which adds noise and can trigger analyzer warnings. Remove it or use it for metadata (e.g., file size / last write time) if intended.
| var fileInfo = new FileInfo(filePath); |
| // Try to get package info using dpkg-deb command | ||
| // Use proper argument passing to avoid injection issues | ||
| var escapedPath = debPath.Replace("'", "'\\''"); | ||
| var output = await ExecuteCommandAsync("dpkg-deb", $"-I '{escapedPath}'", cancellationToken); | ||
| var lines = output.Split('\n'); |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ExecuteCommandAsync uses ProcessStartInfo with UseShellExecute = false, so shell-style quoting/escaping (wrapping the path in single quotes and replacing ' with '\'') is not interpreted and can cause dpkg-deb to receive literal quotes/backslashes and fail to find the file. Pass arguments without shell quoting, ideally via ProcessStartInfo.ArgumentList (e.g., -I, --, and debPath as separate arguments). Adding -- also prevents option-injection if a filename begins with -.
| // Try to get module info using modinfo command | ||
| var output = await ExecuteCommandAsync("modinfo", koPath, cancellationToken); | ||
| var lines = output.Split('\n'); |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ExecuteCommandAsync("modinfo", koPath, ...) passes the module path as the raw Arguments string. If koPath contains spaces/quotes, modinfo will see it as multiple arguments and fail. Also, if a filename begins with -, it can be interpreted as an option. Prefer ProcessStartInfo.ArgumentList to pass -- and the path as a single argument (or otherwise quote/escape the path correctly for ProcessStartInfo.Arguments).
| // Default to kernel modules for Linux | ||
| var pattern = searchPattern ?? "*.ko"; | ||
| var driverFiles = Directory.GetFiles(directoryPath, pattern, SearchOption.AllDirectories); | ||
|
|
||
| // Also look for .deb and .rpm packages if no specific pattern was provided | ||
| if (searchPattern == null) | ||
| { | ||
| var debFiles = Directory.GetFiles(directoryPath, "*.deb", SearchOption.AllDirectories); | ||
| var rpmFiles = Directory.GetFiles(directoryPath, "*.rpm", SearchOption.AllDirectories); | ||
| driverFiles = driverFiles.Concat(debFiles).Concat(rpmFiles).ToArray(); | ||
| } | ||
|
|
||
| _logger.Information("Found {Count} driver files matching pattern: {Pattern}", driverFiles.Length, pattern); |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log message reports files "matching pattern: {Pattern}" using pattern, but when searchPattern == null the code also adds *.deb and *.rpm files into driverFiles. This makes the log misleading and complicates troubleshooting. Consider logging the full set of included patterns (e.g., *.ko, *.deb, *.rpm) or logging counts per extension.
| { | ||
| try | ||
| { | ||
| var fileInfo = new FileInfo(filePath); |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fileInfo is created but never used, which adds noise and can trigger analyzer warnings. Remove it or use it for intended metadata extraction.
| var fileInfo = new FileInfo(filePath); |
* add GeneralUpdate.Drivelution * Add comprehensive documentation for GeneralUpdate.Drivelution (#139) * Initial plan * Add comprehensive README documentation for GeneralUpdate.Drivelution Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> * Add comprehensive test suite for GeneralUpdate.Drivelution (#140) * Initial plan * Add comprehensive test cases for GeneralUpdate.Drivelution Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> * Address code review feedback: improve test quality and cleanup Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> * Rename DriverUpdate to Drivelution across codebase (#141) * Initial plan * Replace all instances of DriverUpdate with Drivelution Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> * Update error code prefixes from DU_ to DR_ Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> * Add GetDriversFromDirectoryAsync for local driver discovery (#142) * Initial plan * Add GetDriversFromDirectoryAsync method to read driver information from local directory Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> * Add tests for GetDriversFromDirectoryAsync method Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> * Fix security issues: prevent command injection and improve certificate parsing Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> * Translate all Chinese annotations to English Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com>
GetDriversFromDirectoryAsyncmethod toIGeneralDrivelutioninterfaceGeneralDrivelution.cs✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.