Skip to content

ci(security): Run the workflow on a protected environment#153

Merged
chouetz merged 4 commits into
masterfrom
nschweitzer/protected_env
Dec 23, 2025
Merged

ci(security): Run the workflow on a protected environment#153
chouetz merged 4 commits into
masterfrom
nschweitzer/protected_env

Conversation

@chouetz
Copy link
Copy Markdown
Member

@chouetz chouetz commented Dec 11, 2025

For security reasons, the secrets for Apple publication were moved on a protected environment.
We restrict the execution of the job to this environment to prevent secret exfiltration

@chouetz chouetz requested a review from a team as a code owner December 11, 2025 16:15
@chouetz chouetz force-pushed the nschweitzer/protected_env branch from 4c234a5 to 81f5d5e Compare December 16, 2025 07:57
@chouetz chouetz force-pushed the nschweitzer/protected_env branch from 81f5d5e to 1238567 Compare December 16, 2025 15:56
Ishirui
Ishirui previously approved these changes Dec 22, 2025
Copy link
Copy Markdown

@Ishirui Ishirui left a comment

Choose a reason for hiding this comment

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

Just nits. Thanks !

Comment thread .github/workflows/build.yml Outdated
Comment on lines +270 to +272
macos-packaging-push:
name: Build macOS installer and sign/notarize artifacts (push)
if: github.event_name == 'push'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: I would like to make it clearer that this only runs on pushes to main (or to v* tags), instead of any pushes to the repo.
In the same vein, maybe naming it macos-packaging-withsign or macos-packaging-protected or something would make more sense - we don't really care why this workflow is triggered so much as what it does different compared to the "normal" macos-packaging job

On first look I was about to ask if this would also trigger on pushes to PR branches before checking the triggers at the top of the workflow file ^^'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You are right, the name was badly chosen, will fix this

Comment thread .github/actions/build-macos-package/action.yml Outdated
Comment on lines +20 to +28
apple-certificate:
description: Apple Developer ID Application Certificate
required: false
apple-private-key:
description: Apple Developer ID Application Private Key
required: false
apple-api-key:
description: Apple App Store Connect API Key
required: false
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we need to pass these as inputs, can we not access the secrets directly from the action here ?
Or do you envision that we'll use this action on other repos, in which case more encapsulation is a good idea ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Reusability is a good question, as we are doing more or less the same in at least 3 different repositories. I could tell this is not really urgent and could be done later but it is a risk of failure (to do the change)...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure we can do differently, as we need to have the environment:name:main used to access the secret. So we really need to have 2 different jobs on the top level passing or not the credentials

Comment thread .github/actions/build-macos-package/action.yml
Comment thread .github/actions/build-macos-package/action.yml
@chouetz chouetz merged commit e16ee0b into master Dec 23, 2025
21 checks passed
@chouetz chouetz deleted the nschweitzer/protected_env branch December 23, 2025 13:10
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.

2 participants