Fix Windows Tauri signing order#584
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe PR replaces Azure action-based Windows signing with a PowerShell ArtifactSigning flow, updates Windows workflow variables and install steps, adds a Tauri signing command, and shifts bundle/finalize checks to focus on the NSIS setup executable. ChangesWindows artifact signing pipeline
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying maple with
|
| Latest commit: |
382bbef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0ba44a5e.maple-ca8.pages.dev |
| Branch Preview URL: | https://fix-windows-tauri-signing-or.maple-ca8.pages.dev |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/ci/desktop-windows-release.sh`:
- Around line 89-103: Keep the post-bundling verification for the patched app
executable in addition to the NSIS setup. In run_finalize_phase and the related
bundling flow, locate the app_exe artifact that Tauri mutates and add an
Authenticode signature check for it alongside
verify_windows_authenticode_signatures used for setup_exe, so both the installer
and installed executable are validated after signCommand runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 27154b3d-7f49-4c30-9a26-8f5b8f76b6aa
📒 Files selected for processing (5)
.github/workflows/desktop-build.yml.github/workflows/release.ymlscripts/ci/_common.shscripts/ci/desktop-windows-release.shscripts/ci/windows-artifact-sign.ps1
| verify_windows_authenticode_signatures "${setup_exe}" | ||
| print_file_hashes "${setup_exe}" | ||
| verify_frontend_dist_unchanged | ||
| } | ||
|
|
||
| run_finalize_phase() { | ||
| local app_exe setup_exe repro_dir | ||
| local setup_exe repro_dir | ||
| local windows_runtime_dlls=() | ||
|
|
||
| print_source_provenance | ||
| configure_reproducible_build_metadata | ||
| configure_tauri_updater_signing_key | ||
|
|
||
| app_exe="$(windows_release_app_exe)" | ||
| setup_exe="$(windows_release_setup_exe_required)" | ||
| verify_windows_authenticode_signatures "${app_exe}" "${setup_exe}" | ||
| verify_windows_authenticode_signatures "${setup_exe}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep verifying the patched app executable after bundling.
Line 89 and Line 103 now verify only the NSIS setup. Since the app exe is the artifact Tauri mutates and signCommand is expected to sign, verify ${app_exe} after bundling as well; otherwise a signed installer can ship an unsigned or wrong-subject installed executable.
Proposed fix
setup_exe="$(windows_release_setup_exe_required)"
- verify_windows_authenticode_signatures "${setup_exe}"
+ verify_windows_authenticode_signatures "${app_exe}" "${setup_exe}"
print_file_hashes "${setup_exe}"
verify_frontend_dist_unchanged
}
run_finalize_phase() {
- local setup_exe repro_dir
+ local app_exe setup_exe repro_dir
local windows_runtime_dlls=()
print_source_provenance
configure_reproducible_build_metadata
configure_tauri_updater_signing_key
+ app_exe="$(windows_release_app_exe)"
setup_exe="$(windows_release_setup_exe_required)"
- verify_windows_authenticode_signatures "${setup_exe}"
+ verify_windows_authenticode_signatures "${app_exe}" "${setup_exe}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| verify_windows_authenticode_signatures "${setup_exe}" | |
| print_file_hashes "${setup_exe}" | |
| verify_frontend_dist_unchanged | |
| } | |
| run_finalize_phase() { | |
| local app_exe setup_exe repro_dir | |
| local setup_exe repro_dir | |
| local windows_runtime_dlls=() | |
| print_source_provenance | |
| configure_reproducible_build_metadata | |
| configure_tauri_updater_signing_key | |
| app_exe="$(windows_release_app_exe)" | |
| setup_exe="$(windows_release_setup_exe_required)" | |
| verify_windows_authenticode_signatures "${app_exe}" "${setup_exe}" | |
| verify_windows_authenticode_signatures "${setup_exe}" | |
| verify_windows_authenticode_signatures "${app_exe}" "${setup_exe}" | |
| print_file_hashes "${setup_exe}" | |
| verify_frontend_dist_unchanged | |
| } | |
| run_finalize_phase() { | |
| local app_exe setup_exe repro_dir | |
| local windows_runtime_dlls=() | |
| print_source_provenance | |
| configure_reproducible_build_metadata | |
| configure_tauri_updater_signing_key | |
| app_exe="$(windows_release_app_exe)" | |
| setup_exe="$(windows_release_setup_exe_required)" | |
| verify_windows_authenticode_signatures "${app_exe}" "${setup_exe}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/ci/desktop-windows-release.sh` around lines 89 - 103, Keep the
post-bundling verification for the patched app executable in addition to the
NSIS setup. In run_finalize_phase and the related bundling flow, locate the
app_exe artifact that Tauri mutates and add an Authenticode signature check for
it alongside verify_windows_authenticode_signatures used for setup_exe, so both
the installer and installed executable are validated after signCommand runs.
2a4c6a7 to
dd0a7f9
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| run_finalize_phase() { | ||
| local app_exe setup_exe repro_dir | ||
| local setup_exe repro_dir | ||
| local windows_runtime_dlls=() | ||
|
|
||
| print_source_provenance | ||
| configure_reproducible_build_metadata | ||
| configure_tauri_updater_signing_key | ||
|
|
||
| app_exe="$(windows_release_app_exe)" | ||
| setup_exe="$(windows_release_setup_exe_required)" | ||
| verify_windows_authenticode_signatures "${app_exe}" "${setup_exe}" | ||
| verify_windows_authenticode_signatures "${setup_exe}" |
There was a problem hiding this comment.
🚩 Authenticode verification scope narrowed from exe+installer to installer-only
The finalize phase previously verified Authenticode signatures on both maple.exe and the NSIS installer (scripts/ci/desktop-windows-release.sh:107). Now it only verifies the installer. This is intentional because Tauri restores target/release/maple.exe to its original unsigned/unpatched bytes after bundling. The signed exe only exists inside the NSIS installer payload. This is a correct behavioral change, but reviewers should confirm that Tauri's signCommand in Tauri 2.x does indeed restore the original exe and sign the NSIS installer itself (not just the embedded exe). If Tauri doesn't sign the NSIS installer via signCommand, the verify_windows_authenticode_signatures call in the bundle phase at scripts/ci/desktop-windows-release.sh:93 would fail, which provides a safety net.
Was this helpful? React with 👍 or 👎 to provide feedback.
| $params = @{ | ||
| Endpoint = $env:MAPLE_WINDOWS_ARTIFACT_SIGNING_ENDPOINT | ||
| CodeSigningAccountName = $env:MAPLE_WINDOWS_ARTIFACT_SIGNING_ACCOUNT_NAME | ||
| CertificateProfileName = $env:MAPLE_WINDOWS_ARTIFACT_SIGNING_CERTIFICATE_PROFILE_NAME | ||
| Files = $File | ||
| FileDigest = "SHA256" | ||
| TimestampRfc3161 = "http://timestamp.acs.microsoft.com" | ||
| TimestampDigest = "SHA256" | ||
| ExcludeEnvironmentCredential = $true | ||
| ExcludeWorkloadIdentityCredential = $true | ||
| ExcludeManagedIdentityCredential = $true | ||
| ExcludeSharedTokenCacheCredential = $true | ||
| ExcludeVisualStudioCredential = $true | ||
| ExcludeVisualStudioCodeCredential = $true | ||
| ExcludeAzureCliCredential = $false | ||
| ExcludeAzurePowerShellCredential = $true | ||
| ExcludeAzureDeveloperCliCredential = $true | ||
| ExcludeInteractiveBrowserCredential = $true | ||
| } |
There was a problem hiding this comment.
🚩 Module parameter names mapped from GitHub Action input names to PowerShell cmdlet parameters
The old GitHub Action used kebab-case input names (e.g., signing-account-name) while the new script uses PascalCase PowerShell cmdlet parameters (e.g., CodeSigningAccountName at scripts/ci/windows-artifact-sign.ps1:45). Note the semantic difference: the action input was signing-account-name but the cmdlet parameter is CodeSigningAccountName (not SigningAccountName). This mapping was presumably derived from the ArtifactSigning module's actual cmdlet signature. If the parameter name is incorrect, Invoke-ArtifactSigning would fail at runtime since $ErrorActionPreference = 'Stop' is set, so any mismatch would be caught immediately in CI. Still worth confirming the parameter names match version 0.1.8 of the module.
Was this helpful? React with 👍 or 👎 to provide feedback.
dd0a7f9 to
382bbef
Compare
1 participant
Summary
Why
Tauri mutates target/release/maple.exe during bundling to add bundle-type metadata, then signs at that point if a Windows signCommand is configured. Signing the executable before tauri bundle is unsafe because Tauri still needs to patch it, and can also leave the file locked on Windows.
Validation
Note: PR CI cannot exercise the live Azure signing secrets. The protected master Windows signing workflow is the final validation point for the real signed build.
Summary by CodeRabbit