Skip to content

Restrict Trusted Signing to merged PR release paths in pyRevitCI#3374

Merged
jmcouffin merged 6 commits into
developfrom
copilot/update-github-actions-workflow
May 11, 2026
Merged

Restrict Trusted Signing to merged PR release paths in pyRevitCI#3374
jmcouffin merged 6 commits into
developfrom
copilot/update-github-actions-workflow

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 11, 2026

  • Re-check new review comments and current workflow state
  • Inspect latest CI workflow runs/logs for this branch via GitHub MCP
  • Confirm whether signing fork-guard feedback is already satisfied on both signing steps
  • Address unsigned _signed artifact handling for non-signing runs
  • Validate workflow YAML and targeted behavior after edits
  • Run parallel validation (Code Review + CodeQL)
  • Commit/push and reply to remaining new comments
Original prompt

Update the GitHub Actions workflow at .github/workflows/main.yml in pyrevitlabs/pyRevit so Trusted Signing only runs when pull requests are merged, instead of on ordinary push runs.

Context:

  • The current workflow pyRevitCI triggers on pull_request (opened, reopened, closed) and push to develop and master.
  • Over the past month, too many signing operations have been generated because the signing steps still run on push events.
  • The relevant file is .github/workflows/main.yml on branch develop.
  • The workflow already defines:
    • WipRun for merged PRs into develop
    • ReleaseRun for merged PRs into master, and also workflow_dispatch on master
  • The current signing-related logic is too broad:
    • Require Trusted Signing secrets allows github.event_name == 'push'
    • Sign files with Trusted Signing (DLLs and EXEs) runs for any trusted non-PR event
    • Sign files with Trusted Signing (installers) runs for any trusted non-PR event

Requested behavior:

  • Signing should happen only when PRs are merged.
  • Preserve existing release behavior if it is already intentionally modeled through ReleaseRun.
  • In practice, update the signing-related if: conditions to use the existing WipRun / ReleaseRun booleans rather than raw push checks.
  • Remove signing on ordinary push events.
  • Keep the workflow otherwise unchanged unless a small comment update is needed for accuracy.

Acceptance criteria:

  1. Require Trusted Signing secrets no longer runs on ordinary push events.
  2. Sign files with Trusted Signing (DLLs and EXEs) no longer runs on ordinary push events.
  3. Sign files with Trusted Signing (installers) no longer runs on ordinary push events.
  4. Signing still runs for merged PRs into develop and merged PRs into master.
  5. If ReleaseRun already includes manual release dispatch, do not break that behavior.
  6. Update any misleading nearby comment that says push runs will sign, if applicable.

Please create a pull request with the workflow change.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: The past month changes generated way to many signing. I'd like the signing tonhappen only when merging the prs

Assistant: Yes — the current workflow still signs on every push, which is why you’re getting too many signing operations.

The fix is to gate both signing steps so they run only for:

  • merged PRs to develop (WipRun == true)
  • merged PRs to master (ReleaseRun == true)
  • optionally workflow_dispatch on master if you still want manual release signing

Right now, these conditions are too broad:

  • Require Trusted Signing secrets includes github.event_name == 'push'
  • both Sign files with Trusted Signing ... steps run for any trusted non-PR event, including push

Suggested change in .github/workflows/main.yml:

      # Only sign on merged PR runs (WIP/Release) or manual release dispatches.
      - name: Require Trusted Signing secrets
        if: >
          github.repository == env.MainRepo &&
          (env.WipRun == 'true' || env.ReleaseRun == 'true')
        shell: pwsh
        run: |
          if (-not "${{ secrets.AZURE_ENDPOINT }}") { throw "Missing secret: AZURE_ENDPOINT" }
          if (-not "${{ secrets.AZURE_TENANT_ID }}") { throw "Missing secret: AZURE_TENANT_ID" }
          if (-not "${{ secrets.AZURE_CLIENT_ID }}") { throw "Missing secret: AZURE_CLIENT_ID" }
          if (-not "${{ secrets.AZURE_CLIENT_SECRET }}") { throw "Missing secret: AZURE_CLIENT_SECRET" }
          if (-not "${{ secrets.AZURE_CODE_SIGNING_NAME }}") { throw "Missing secret: AZURE_CODE_SIGNING_NAME" }
          if (-not "${{ secrets.AZURE_CERT_PROFILE_NAME }}") { throw "Missing secret: AZURE_CERT_PROFILE_NAME" }
      
      - name: Sign files with Trusted Signing (DLLs and EXEs)
        if: >
          github.repository == env.MainRepo &&
          (env.WipRun == 'true' || env.ReleaseRun == 'true')
        uses: azure/trusted-signing-action@v1.2.0
        with:
          azure-tenant-id: ${{ secrets.AZURE_TENANT_ID }}
          azure-client-id: ${{ secrets.AZURE_CLIENT_ID }}
          azure-client-secret: ${{ secrets.AZURE_CLIENT_SECRET }}
          endpoint: ${{ secrets.AZURE_ENDPOINT }}
          signing-account-name: ${{ secrets.AZURE_CODE_SIGNING_NAME }}
          certificate-profile-name: ${{ secrets.AZURE_CERT_PROFILE_NAME }}

          files-folder: bin/
          files-folder-filter: pyrevit*.exe,pyrevit*.dll,pyRevit*.dll
          files-folder-recurse: true
          file-digest: SHA256
          timestamp-rfc3161: http://timestamp.acs.microsoft.com
          timestamp-digest: SHA256

      - name: Sign files with Trusted Signing (installers)
        if: >
          github.repository == env.MainRepo &&
          (env.WipRun == 'true' || env.ReleaseRu...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

Copy link
Copy Markdown
Contributor

@devloai devloai Bot left a comment

Choose a reason for hiding this comment

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

PR Summary:

This PR was created by the Copilot agent to fix overly broad Trusted Signing conditions in .github/workflows/main.yml that were causing signing operations to trigger on every ordinary push event. The intent is to restrict signing to only run on merged PRs (WipRun / ReleaseRun), not on plain pushes.

Review Summary:

The PR contains zero file changes. The Copilot agent created an "Initial plan" commit (0 additions, 0 deletions, 0 changed files) and never actually modified .github/workflows/main.yml. The three signing-related steps remain unchanged on the PR branch:

  • Require Trusted Signing secrets (line 156): still includes github.event_name == 'push' in its condition — signing still fires on every push.
  • Sign files with Trusted Signing (DLLs and EXEs) (lines 166–170): condition github.event_name != 'pull_request' is still satisfied by push events.
  • Sign files with Trusted Signing (installers) (lines 191–195): same overly broad condition.
  • The misleading comment on line 148 ("push run will sign") also still exists.

All three of the PR's acceptance criteria (1–3) are unmet. This PR cannot be merged as-is — the Copilot agent needs to actually commit the fix it described.

The required change for all three if: conditions is:

if: >
  github.repository == env.MainRepo &&
  (env.WipRun == 'true' || env.ReleaseRun == 'true')

Suggestions

  • Apply the actual workflow fix by updating all three Trusted Signing if: conditions to use env.WipRun == 'true' || env.ReleaseRun == 'true' instead of the current push-inclusive conditions. Apply
  • Remove the push: trigger block entirely from the workflow if push events should never produce signing artifacts, to avoid confusion and unnecessary CI runs. Apply

Copilot AI changed the title [WIP] Update Trusted Signing workflow to trigger only on merged PRs Restrict Trusted Signing to merged PR release paths in pyRevitCI May 11, 2026
Copilot AI requested a review from jmcouffin May 11, 2026 16:48
@jmcouffin jmcouffin marked this pull request as ready for review May 11, 2026 17:02
@jmcouffin jmcouffin requested a review from Copilot May 11, 2026 17:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the pyRevitCI GitHub Actions workflow to reduce Trusted Signing usage by gating signing-related steps to the workflow’s merged-PR “release paths” (WipRun / ReleaseRun), and updates the nearby comment to reflect the new behavior.

Changes:

  • Restricts Require Trusted Signing secrets to github.repository == env.MainRepo && (env.WipRun == 'true' || env.ReleaseRun == 'true').
  • Restricts both Trusted Signing action steps (binaries + installers) to the same WipRun/ReleaseRun gating.
  • Updates the comment that previously implied ordinary push runs would perform signing.

Comment thread .github/workflows/main.yml
Comment thread .github/workflows/main.yml
Comment thread .github/workflows/main.yml
Comment thread .github/workflows/main.yml
@jmcouffin jmcouffin requested a review from Copilot May 11, 2026 17:43
@jmcouffin
Copy link
Copy Markdown
Contributor

@devloai please review again

Copy link
Copy Markdown
Contributor

@devloai devloai Bot left a comment

Choose a reason for hiding this comment

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

Re-review Summary

Good progress! The PR is now substantive — Copilot made real changes since my last review. Here's what the current state looks like:

✅ All acceptance criteria met:

  1. Require Trusted Signing secretspush and workflow_dispatch removed from condition; gated to WipRun || ReleaseRun with fork guard preserved. ✓
  2. Sign files with Trusted Signing (DLLs and EXEs)WipRun || ReleaseRun gate added alongside the existing fork guard. ✓
  3. Sign files with Trusted Signing (installers) — Same. ✓
  4. Upload Installers — Correctly gated: unsigned _signed artifacts won't be uploaded on non-signing runs. ✓
  5. Publish Release — Fork guard added defensively (good hygiene). ✓

Minor nit:

One low-severity comment on the updated comment text (see inline). ReleaseRun already covers workflow_dispatch via line 40, so "manual release dispatches" is implicit — but the phrasing is mildly confusing. Easy one-line fix suggested.

Bottom line:

This PR is ready to merge modulo that cosmetic comment. The logic is sound and all three signing steps are now properly scoped to merged-PR-only contexts. 🎉

Suggestions

  • Simplify comment on line 148 to remove 'manual release dispatches' redundancy Apply

Comment thread .github/workflows/main.yml Outdated
jmcouffin and others added 2 commits May 11, 2026 19:47
Co-authored-by: devloai[bot] <168258904+devloai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment on lines 149 to +154
- name: Require Trusted Signing secrets
if: >
github.repository == env.MainRepo &&
(github.event_name != 'pull_request' ||
github.event.pull_request.head.repo.full_name == github.repository) &&
(env.WipRun == 'true' || env.ReleaseRun == 'true' ||
github.event_name == 'workflow_dispatch' ||
github.event_name == 'push')
(env.WipRun == 'true' || env.ReleaseRun == 'true')
Comment on lines 243 to +249
- name: Publish Release (Release)
id: publish_release
if: (env.ReleaseRun == 'true' && github.repository == env.MainRepo)
if: >
env.ReleaseRun == 'true' &&
github.repository == env.MainRepo &&
(github.event_name != 'pull_request' ||
github.event.pull_request.head.repo.full_name == github.repository)
@jmcouffin jmcouffin merged commit ed31887 into develop May 11, 2026
@jmcouffin jmcouffin deleted the copilot/update-github-actions-workflow branch May 11, 2026 17:49
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.

3 participants