Skip to content

Fixing ci signing for windows#1330

Merged
alirana01 merged 1 commit into
masterfrom
IEP-1647-upd
Oct 16, 2025
Merged

Fixing ci signing for windows#1330
alirana01 merged 1 commit into
masterfrom
IEP-1647-upd

Conversation

@alirana01

@alirana01 alirana01 commented Oct 16, 2025

Copy link
Copy Markdown
Collaborator

Fixing ci signing for windows

Summary by CodeRabbit

  • Chores
    • Improved Windows build and release process with enhanced validation checks and error handling
    • Added verification steps for certificate and keystore conversions
    • Increased robustness of build tool discovery and signing processes

@coderabbitai

coderabbitai Bot commented Oct 16, 2025

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

Walkthrough

Modified CI release workflow to enhance Windows signing process with robust path validation, keystore password parameters, and PFX certificate inspection. Added early existence checks for espressif-ide.exe and signtool.exe, implemented dynamic tool discovery with filtering and sorting, and introduced post-PFX creation verification using certutil.

Changes

Cohort / File(s) Summary
Keystore conversion password handling
.github/workflows/ci_release.yml
Added -srckeypass and -destkeypass parameters to keytool import-keystore command for improved password plumbing.
PFX verification and inspection
.github/workflows/ci_release.yml
Introduced new pwsh-based step to inspect and dump PFX certificate data using certutil after PFX creation.
Windows signing flow refactoring
.github/workflows/ci_release.yml
Added early validation for espressif-ide.exe presence, replaced direct path lookup with robust signtool discovery using Get-ChildItem filtering and sorting, implemented validation checks for signtool.exe and cert.pfx, updated signing command with full quoted paths, and recomputed tool paths in signature verification step.

Sequence Diagram(s)

sequenceDiagram
    participant Workflow
    participant FileSystem
    participant Signtool
    participant Certutil
    
    Workflow->>FileSystem: Validate espressif-ide.exe exists
    alt NOT found
        Workflow->>Workflow: throw error
    end
    
    Workflow->>FileSystem: Search for signtool.exe
    FileSystem-->>Workflow: Get-ChildItem with filter & sort
    
    alt signtool NOT found
        Workflow->>Workflow: throw error
    end
    
    alt cert.pfx NOT found
        Workflow->>Workflow: throw error
    end
    
    rect rgb(200, 230, 255)
        note right of Workflow: New verification step
        Workflow->>Certutil: Inspect PFX with certutil
        Certutil-->>Workflow: Certificate data dump
    end
    
    Workflow->>Signtool: Sign executable with full path
    Signtool-->>Workflow: Signed executable
    
    Workflow->>Signtool: Verify signature with discovered path
    Signtool-->>Workflow: Verification result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Workflow file contains multiple interdependent changes across validation logic, path discovery, and signing operations. Requires careful verification of error handling paths, tool discovery correctness, and proper integration with the critical signing workflow.

Possibly related PRs

Suggested reviewers

  • sigmaaa
  • kolipakakondal

Poem

🐰 With signtool discovered and paths made so bright,
Early checks prevent silent failures at night,
PFX certs now inspected before they take flight,
Windows signing workflows now robustly tight!

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch IEP-1647-upd

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0cade9d and da9cb5e.

📒 Files selected for processing (1)
  • .github/workflows/ci_release.yml (1 hunks)

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.

@alirana01 alirana01 merged commit 8bd0164 into master Oct 16, 2025
1 of 4 checks passed
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