[kext] Fix dev build documentation #1799
Conversation
📝 WalkthroughWalkthroughThis pull request updates the documentation and build scripts for the Portmaster Windows kernel extension. The changes focus on refining the build and testing instructions in the README.md file and introducing a new PowerShell script ( Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
windows_kext/README.md (1)
Line range hint
24-33: Enhance security warning for test signing.The current warning about test signing could be more explicit about security implications.
-Not recommended for a work machine. Usually done on virtual machine dedicated for testing. +WARNING: Test signing reduces system security by allowing unsigned drivers to load. Only enable this on isolated virtual machines dedicated to driver development and testing. Never enable test signing on production machines or machines connected to sensitive networks.🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Make sure that the noun ‘build’ is correct. Did you mean the past participle “built”?
Context: ...g-wfp-callout-drivers) -> The driver is build on top of WFP. ### Building (For testi...(BE_VB_OR_NN)
🪛 markdownlint-cli2 (0.17.2)
4-4: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3(MD001, heading-increment)
🧹 Nitpick comments (2)
windows_kext/link-dev.ps1 (1)
5-17: Validate required files before linking.The script assumes all required files exist but doesn't validate them.
Consider validating input files:
$requiredFiles = @( "driver.lib", "$WDKPath\lib\$SDKVersion\km\x64\wdmsec.lib", # ... add other libs ) foreach ($file in $requiredFiles) { if (-not (Test-Path $file)) { Write-Error "Required file not found: $file" Exit 1 } }windows_kext/README.md (1)
55-55: Clarify Linux build limitations.The note about Linux builds needs more context about what works and what doesn't.
cargo build --release -> Build also works on linux +> Note: While the Rust compilation works on Linux, the subsequent linking step requires Windows with WDK installed.Also applies to: 56-56
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
windows_kext/README.md(2 hunks)windows_kext/link-dev.ps1(1 hunks)
🧰 Additional context used
🪛 LanguageTool
windows_kext/README.md
[grammar] ~10-~10: Make sure that the noun ‘build’ is correct. Did you mean the past participle “built”?
Context: ...g-wfp-callout-drivers) -> The driver is build on top of WFP. ### Building (For testi...
(BE_VB_OR_NN)
[grammar] ~60-~60: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...works on linux Link and sign: On a windows machine copy driver.lib from the proj...
(A_WINDOWS)
🪛 markdownlint-cli2 (0.17.2)
windows_kext/README.md
4-4: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
66-66: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (1)
windows_kext/link-dev.ps1 (1)
14-15: Verify KMDF library version compatibility.The script uses KMDF version 1.15, which might not be the latest version. Please verify if this version is intentional and compatible with the target Windows versions.
✅ Verification successful
KMDF version 1.15 usage is verified and appropriate
The codebase consistently uses KMDF 1.15 across build scripts and specifically targets Windows 10, which is fully compatible with this KMDF version. No version inconsistencies were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for KMDF version references in the codebase rg -i "kmdf.*1\.15|wdf.*1\.15" --type cpp --type rustLength of output: 55
Script:
#!/bin/bash # Search for any KMDF version patterns and Windows Kit paths rg -i "kmdf.*\d+\.\d+|wdf.*\d+\.\d+" --type-add 'config:*.{inf,props,vcxproj}' --type config --type cpp --type rust # Look for Windows Kit path patterns rg -i "windows.*kit.*\\\\.*\\\\wdf" --type-add 'scripts:*.{ps1,bat,cmd}' --type scripts # Search for Windows target version specifications rg -i "target.*windows|windows.*target|windows.*version" --type-add 'config:*.{inf,props,vcxproj}' --type configLength of output: 1708
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
windows_kext/release/README.md (3)
5-5: Fix hyphenation in "up to date".-Make sure the deriver version in `kextinterface/version.txt` is up to date +Make sure the deriver version in `kextinterface/version.txt` is up-to-date🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: It appears that hyphens are missing in the adjective “up-to-date”.
Context: ...rsion inkextinterface/version.txtis up to date - Execute:cargo run_This will ...(UP_TO_DATE_HYPHEN)
14-14: Capitalize "Windows" as it's a proper noun.-copy the zip and extract it on a windows machine. +copy the zip and extract it on a Windows machine.🧰 Tools
🪛 LanguageTool
[grammar] ~14-~14: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...ed. - copy the zip and extract it on a windows machine. - update.\build_cab.ps1: s...(A_WINDOWS)
26-26: Fix typo in "Sing the the".-Sing the the output cab file: `portmaster-kext-release-bundle-v...\PortmasterKext_v....cab` +Sign the output cab file: `portmaster-kext-release-bundle-v...\PortmasterKext_v....cab`🧰 Tools
🪛 LanguageTool
[duplication] ~26-~26: Possible typo: you repeated a word.
Context: ...le-v... .\build_cab.ps1 ``` - Sing the the output cab file: `portmaster-kext-relea...(ENGLISH_WORD_REPEAT_RULE)
windows_kext/README.md (1)
70-72: Add language identifier to code block.Add
ps1as the language identifier to the code block for better syntax highlighting.-``` +```ps1 SignTool sign /v /s TestCertStoreName /n TestCertName driver.sys<details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 70-70: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 32d6e1cb045860f3194c097547eaf6482b83c0d9 and 857df4086f0ce448ed9e581b96b6e8abc768ebb6. </details> <details> <summary>📒 Files selected for processing (4)</summary> * `windows_kext/README.md` (2 hunks) * `windows_kext/release/README.md` (1 hunks) * `windows_kext/release/src/main.rs` (1 hunks) * `windows_kext/release/templates/build_cab.ps1` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>windows_kext/README.md</summary> [uncategorized] ~10-~10: This verb may not be in the correct form. Consider using a different form for this context. Context: ...g-wfp-callout-drivers) -> The driver is build on top of WFP. ### Building (For relea... (AI_EN_LECTOR_REPLACEMENT_VERB_FORM) --- [grammar] ~64-~64: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized. Context: ...works on linux __Link and sign:__ On a windows machine copy `driver.lib` from the proj... (A_WINDOWS) --- [uncategorized] ~64-~64: Possible missing comma found. Context: ... linux __Link and sign:__ On a windows machine copy `driver.lib` from the project targ... (AI_HYDRA_LEO_MISSING_COMMA) </details> <details> <summary>windows_kext/release/README.md</summary> [uncategorized] ~5-~5: It appears that hyphens are missing in the adjective “up-to-date”. Context: ...rsion in `kextinterface/version.txt` is up to date - Execute: `cargo run` _This will ... (UP_TO_DATE_HYPHEN) --- [grammar] ~14-~14: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized. Context: ...ed. - copy the zip and extract it on a windows machine. - update `.\build_cab.ps1`: s... (A_WINDOWS) --- [duplication] ~26-~26: Possible typo: you repeated a word. Context: ...le-v... .\build_cab.ps1 ``` - Sing the the output cab file: `portmaster-kext-relea... (ENGLISH_WORD_REPEAT_RULE) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>windows_kext/README.md</summary> 4-4: Heading levels should only increment by one level at a time Expected: h2; Actual: h3 (MD001, heading-increment) --- 70-70: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (4)</summary><blockquote> <details> <summary>windows_kext/release/src/main.rs (1)</summary> `14-19`: **LGTM! Improved filename clarity.** The new filename format includes version information, making it easier to identify specific releases. </details> <details> <summary>windows_kext/README.md (3)</summary> `10-10`: **Fix grammatical error in WFP description.** ```diff -The driver is build on top of WFP. +The driver is built on top of WFP.🧰 Tools
🪛 LanguageTool
[uncategorized] ~10-~10: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...g-wfp-callout-drivers) -> The driver is build on top of WFP. ### Building (For relea...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
64-65: Fix capitalization and improve clarity.-On a windows machine copy `driver.lib` from the project target directory (`driver/target/x86_64-pc-windows-msvc/release/driver.lib`) in the same folder as `link-dev.ps1`. +On a Windows machine, copy `driver.lib` from the project target directory (`driver/target/x86_64-pc-windows-msvc/release/driver.lib`) to the same folder as `link-dev.ps1`.🧰 Tools
🪛 LanguageTool
[grammar] ~64-~64: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...works on linux Link and sign: On a windows machine copydriver.libfrom the proj...(A_WINDOWS)
[uncategorized] ~64-~64: Possible missing comma found.
Context: ... linux Link and sign: On a windows machine copydriver.libfrom the project targ...(AI_HYDRA_LEO_MISSING_COMMA)
71-71: Clarify certificate store and name placeholders.The SignTool command uses placeholder values that need to be replaced.
- SignTool sign /v /s TestCertStoreName /n TestCertName driver.sys + SignTool sign /v /s PrivateCertStore /n CN=DriverCertificate driver.sysNote: The store name and certificate name should match the values used in the certificate creation steps above.
Summary by CodeRabbit
Documentation
New Features
Chores