Conversation
… workflow - Eliminated the Azure login and OIDC token fetching steps for Windows signing. - Updated environment variable handling for Azure signing to use client-secret authentication due to electron-builder limitations.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe desktop release workflow's Azure code signing authentication method was changed from OIDC federated token-based login to client-secret authentication. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on e421342..291a7e2
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (1)
• .github/workflows/desktop-release.yml
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/desktop-release.yml (1)
66-67:⚠️ Potential issue | 🟡 MinorStale comment references OIDC federated credential.
The rationale now reads inaccurately since signing uses client-secret auth. Consider updating to reflect the current gating reason (e.g., single signing credential / production-tag policy) to avoid confusing future maintainers.
📝 Proposed edit
# Sign Windows builds only on production v* tags (not beta-v*, not workflow_dispatch). - # This matches the single OIDC federated credential configured in Entra ID. + # Signing uses the Azure service principal's client secret; restrict to production tags + # to minimize credential exposure and match the Entra ID app's intended usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/desktop-release.yml around lines 66 - 67, Update the stale comment that currently reads "Sign Windows builds only on production v* tags (not beta-v*, not workflow_dispatch). This matches the single OIDC federated credential configured in Entra ID." to reflect the current authentication method and gating rationale: mention that signing uses client-secret auth (or a single signing credential) and that signing is intentionally limited to production v* tags, not beta or manual dispatches; edit the comment near the Windows signing step so it references "single signing credential / client-secret auth" and "production v* tag policy" instead of the OIDC federated credential terminology.
🧹 Nitpick comments (1)
.github/workflows/desktop-release.yml (1)
23-26:id-token: writepermission is no longer required.With the
azure/loginOIDC step removed, nothing in this workflow requests a federated GitHub OIDC token. You can dropid-token: writeto adhere to least-privilege.🔒 Proposed change
permissions: contents: write - id-token: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/desktop-release.yml around lines 23 - 26, The workflow's permissions block unnecessarily grants id-token: write; remove the `id-token: write` entry from the `permissions:` section (the lines defining `contents: write` and `id-token: write`) so only required permissions remain (e.g., keep `contents: write`) and adhere to least-privilege since `azure/login`/OIDC is no longer used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/desktop-release.yml:
- Around line 66-67: Update the stale comment that currently reads "Sign Windows
builds only on production v* tags (not beta-v*, not workflow_dispatch). This
matches the single OIDC federated credential configured in Entra ID." to reflect
the current authentication method and gating rationale: mention that signing
uses client-secret auth (or a single signing credential) and that signing is
intentionally limited to production v* tags, not beta or manual dispatches; edit
the comment near the Windows signing step so it references "single signing
credential / client-secret auth" and "production v* tag policy" instead of the
OIDC federated credential terminology.
---
Nitpick comments:
In @.github/workflows/desktop-release.yml:
- Around line 23-26: The workflow's permissions block unnecessarily grants
id-token: write; remove the `id-token: write` entry from the `permissions:`
section (the lines defining `contents: write` and `id-token: write`) so only
required permissions remain (e.g., keep `contents: write`) and adhere to
least-privilege since `azure/login`/OIDC is no longer used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6124e9a-47a6-44dd-bb0c-dd55c6571e86
📒 Files selected for processing (1)
.github/workflows/desktop-release.yml
Description
Motivation and Context
FIX #
Screenshots
API Changes
Change Type
Testing Performed
Checklist
High-level PR Summary
This PR simplifies the Windows desktop release workflow by removing Azure OIDC token fetching steps and switching from federated token authentication to client-secret based authentication. The change removes the Azure login step and the PowerShell script that fetched GitHub OIDC tokens, replacing the
AZURE_FEDERATED_TOKENenvironment variable withAZURE_CLIENT_SECRETin the build step. This is done because electron-builder 26 does not yet support OIDC federated tokens for Azure signing, requiring a fallback to client-secret authentication.⏱️ Estimated Review Time: 5-15 minutes
💡 Review Order Suggestion
.github/workflows/desktop-release.ymlSummary by CodeRabbit