Skip to content

Feature: Add workflow to update windows cert thumbprint#140

Open
infeo wants to merge 3 commits intodevelopfrom
feature/update-desktop-flow
Open

Feature: Add workflow to update windows cert thumbprint#140
infeo wants to merge 3 commits intodevelopfrom
feature/update-desktop-flow

Conversation

@infeo
Copy link
Copy Markdown
Member

@infeo infeo commented Mar 30, 2026

also switches from cert serial numbers to thumbprints (due serial ids are not "unique" (see https://security.stackexchange.com/a/35694))

The workflow:

  • downloads the MSI and extracts the cert thumbprint
  • creates a PR
  • sends a notification

also switches from cert serial numbers to thumbprints
@infeo infeo self-assigned this Mar 30, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 87cb3155-f097-43bd-b40e-f4898a5aff59

📥 Commits

Reviewing files that changed from the base of the PR and between cff6da4 and 8f325e9.

📒 Files selected for processing (1)
  • docs/security/verify-installers.md
✅ Files skipped from review due to trivial changes (1)
  • docs/security/verify-installers.md

Walkthrough

Adds a GitHub Actions workflow .github/workflows/update-windows-info.yml triggered by repository_dispatch events of type desktop-release. The workflow validates the dispatched version, creates a branch, downloads the MSI from the release assets, extracts the Authenticode signer certificate thumbprint via PowerShell, updates the marked Windows thumbprint row and version in docs/security/verify-installers.md, commits and pushes changes, opens a PR with gh when changes exist, and posts a Slack notification. The documentation was updated to use Get-AuthenticodeSignature and a certificate thumbprints table.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly summarizes the main change: adding a workflow to update Windows certificate thumbprints, which is the primary objective of this PR.
Description check ✅ Passed The description clearly explains the rationale for switching to thumbprints and outlines the workflow's three main functions, all directly related to the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/update-desktop-flow

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
.github/workflows/update-windows-info.yml (1)

84-84: Set PR base branch explicitly.

gh pr create currently relies on repo defaults. Given this automation targets develop, set --base develop to prevent accidental PRs against a different default branch.

Suggested patch
-          PR_URL=$(gh pr create --title "Desktop release ${DESKTOP_VERSION}" --body-file pr_body.md)
+          PR_URL=$(gh pr create --base develop --title "Desktop release ${DESKTOP_VERSION}" --body-file pr_body.md)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/update-windows-info.yml at line 84, The GH CLI invocation
that creates the release PR uses repo defaults and must explicitly set the
target branch: update the gh pr create command that assigns PR_URL (the line
using PR_URL=$(gh pr create --title "Desktop release ${DESKTOP_VERSION}"
--body-file pr_body.md)) to include --base develop so the PR is always opened
against the develop branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/update-windows-info.yml:
- Around line 33-34: The workflow currently sets MSI_URL from ASSETS_JSON and
blindly passes it to curl (MSI_URL variable and the curl invocation), which
yields an opaque error if no .msi asset exists; add an explicit guard after
computing MSI_URL that checks if MSI_URL is "null" or empty and if so prints a
clear error message (including ASSETS_JSON or a short context) and exits
non‑zero before calling curl so the job fails fast with a readable message.
- Around line 57-59: The current replacement logic using
$MarkedRow/$PreviousRow/$NewRow always prepends a new row for
$env:DESKTOP_VERSION, causing duplicate rows on retries; change the block so it
searches $Content for an existing row matching the same version (e.g. a line
starting with '|  ' + $env:DESKTOP_VERSION + '  |') and if found replace that
existing version row with the new thumbprint row (using the same
AUTOMATION_MARKER placement), otherwise insert the new row as before; update the
$Content.Replace usage to either replace the matched version-line or perform the
existing prepend when no match is found so the operation is idempotent across
retries.

In `@docs/security/verify-installers.md`:
- Around line 65-67: The table row containing the "Version(s)" / "Certificate
Thumbprint" entry for "1.19.2" currently embeds the automation marker inside the
second cell; remove the embedded marker from the cell so the row remains
strictly two columns (the row with "1.19.2" and the thumbprint
`20F30D7C5B1AB3ACAFA4AB27874ACBC4B47B0697`), and place the automation marker
instead as a standalone HTML comment on its own line either immediately above or
below the table (or in a dedicated comment line), ensuring no extra table cells
are created.
- Around line 35-38: Add explicit fenced-code languages: label the command
blocks containing "Get-AuthenticodeSignature -FilePath" as "powershell" and
label the sample output block that starts with "SignerCertificate               
Status    StatusMessage" as "text" (the same change should be applied to the
second command/output pair around lines 47-51). Update the opening triple
backticks for the command blocks to ```powershell and the output blocks to
```text so lint rule MD040 passes and rendered readability improves.

---

Nitpick comments:
In @.github/workflows/update-windows-info.yml:
- Line 84: The GH CLI invocation that creates the release PR uses repo defaults
and must explicitly set the target branch: update the gh pr create command that
assigns PR_URL (the line using PR_URL=$(gh pr create --title "Desktop release
${DESKTOP_VERSION}" --body-file pr_body.md)) to include --base develop so the PR
is always opened against the develop branch.
🪄 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: 7931e35c-a321-4ce8-8a51-d9f3bb8179f0

📥 Commits

Reviewing files that changed from the base of the PR and between bba87b1 and 11a0924.

📒 Files selected for processing (2)
  • .github/workflows/update-windows-info.yml
  • docs/security/verify-installers.md

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
docs/security/verify-installers.md (1)

47-51: ⚠️ Potential issue | 🟡 Minor

Add a language tag to the sample output code fence.

This block is still unlabeled; use text to satisfy MD040 and keep rendering consistent.

Suggested patch
-```
+```text
 SignerCertificate                Status    StatusMessage           Path
 -----------------                ------    -------------           ----
 BB0E...                          Valid     Signature verified.     Cryptomator-1.19.1-x64.msi
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @docs/security/verify-installers.md around lines 47 - 51, Add the language
tag "text" to the unlabeled sample output code fence in the docs (the block
showing "SignerCertificate Status StatusMessage Path" with "BB0E..." and
"Cryptomator-1.19.1-x64.msi") so the fence becomes ```text — update the sample
output code fence surrounding that table to include the text language tag to
satisfy MD040 and ensure consistent rendering.


</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @docs/security/verify-installers.md:

  • Around line 64-66: The Markdown has the table starting immediately after prose
    which triggers MD058; insert a single blank line between the preceding paragraph
    and the table that begins with the header "| Version(s) | Certificate
    Thumbprint |" (i.e., add an empty line before that table
    header) so the table is separated from the paragraph.

Duplicate comments:
In @docs/security/verify-installers.md:

  • Around line 47-51: Add the language tag "text" to the unlabeled sample output
    code fence in the docs (the block showing "SignerCertificate Status
    StatusMessage Path" with "BB0E..." and "Cryptomator-1.19.1-x64.msi") so the
    fence becomes ```text — update the sample output code fence surrounding that
    table to include the text language tag to satisfy MD040 and ensure consistent
    rendering.

</details>

<details>
<summary>🪄 Autofix (Beta)</summary>

Fix all unresolved CodeRabbit comments on this PR:

- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: Organization UI

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `2409db79-8f52-4380-9673-6370447c18ea`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 11a0924eb35f0e9b7392d028d2133dfca9a77615 and cff6da471df9a663db0461c8e82ab82a89d8db9b.

</details>

<details>
<summary>📒 Files selected for processing (2)</summary>

* `.github/workflows/update-windows-info.yml`
* `docs/security/verify-installers.md`

</details>

<details>
<summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary>

* .github/workflows/update-windows-info.yml

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant