Skip to content

Add inputs for code signing#49

Merged
chris-araman merged 11 commits intomasterfrom
certificate
Aug 2, 2021
Merged

Add inputs for code signing#49
chris-araman merged 11 commits intomasterfrom
certificate

Conversation

@chris-araman
Copy link
Copy Markdown
Collaborator

@chris-araman chris-araman commented Jul 30, 2021

Addresses #45.

Caveat: There's nothing here about provisioning profiles. I intend to work on that next.

Added inputs:

  • code-sign-certificate
  • code-sign-certificate-passphrase
  • code-sign-identity

There's a healthy dose of paranoia here. Secrets are registered upon input/creation to help prevent accidental disclosure. The keychain database and certificate unfortunately must be written to disk given the /usr/bin/security CLI. However, they are written to UUID-named files under RUNNER_TEMP and are removed immediately after use, even after a failure. The keychain is secured with a UUID password.

The temporary keychain we create for the imported certificate is removed in a post action, enabling the user to invoke other actions that make use of the certificate. Perhaps this is too generous, and we should clean it up in a finally block after invoking xcodebuild?

I've validated the new functionality in my private repository. I'm not sure how best to test this via checks.yml. Maybe using a dummy self-signed certificate?

@repo-ranger repo-ranger Bot added the sponsor label Jul 30, 2021
@chris-araman chris-araman requested a review from mxcl July 30, 2021 03:38
@chris-araman chris-araman marked this pull request as draft July 30, 2021 06:52
@chris-araman chris-araman force-pushed the certificate branch 2 times, most recently from 83401b7 to 62f76f9 Compare July 30, 2021 08:59
@chris-araman
Copy link
Copy Markdown
Collaborator Author

I'd like to pass commands to /usr/bin/security via stdin in order to avoid exposure to command line logging, but I haven't had much success with that yet. I'll keep working on it. Perhaps for a future PR.

@chris-araman chris-araman marked this pull request as ready for review July 30, 2021 09:01
Comment thread README.md
@mxcl
Copy link
Copy Markdown
Owner

mxcl commented Jul 30, 2021

This is very thorough! Thank you.

The temporary keychain we create for the imported certificate is removed in a post action, enabling the user to invoke other actions that make use of the certificate. Perhaps this is too generous, and we should clean it up in a finally block after invoking xcodebuild?

Our action leaves the system in a state where you can use the selected Xcode after the action itself has run, so this is consistent with that.

The difference of course is that this is exposing secrets to other tooling and, as we know, these scripts get attacked (eg. codecov’s was recently for a long time).

The general attitude in this sector is that utility trumps security (which is probably not good, but is what it is). So provided people will find it useful, the way you have written it I think is right choice. For now.

Great work.

Comment thread action.yml
Comment thread src/index.ts
Comment thread src/lib.ts
@chris-araman
Copy link
Copy Markdown
Collaborator Author

chris-araman commented Jul 30, 2021

I'm at a loss for why the checks are failing with Swift 5.5, currently Xcode 13 Beta 3 on the macos-11 runners. The logged output has to do with an iOS Simulator, but I would expect this check to be building for macOS. 🤷🏻‍♂️

Any ideas?

@chris-araman chris-araman marked this pull request as draft July 30, 2021 18:09
@chris-araman
Copy link
Copy Markdown
Collaborator Author

I'd like to pass commands to /usr/bin/security via stdin in order to avoid exposure to command line logging

I think I'm close to completing this. Converting to a draft in the meantime.

@chris-araman chris-araman marked this pull request as ready for review July 31, 2021 20:57
@chris-araman
Copy link
Copy Markdown
Collaborator Author

🔒 We now pass commands to /usr/bin/security via stdin to avoid command line logging. I apologize that it's a bit of a rewrite after you've already reviewed. I think this is a more robust solution.

@chris-araman chris-araman force-pushed the certificate branch 2 times, most recently from 93d73cc to 676604b Compare July 31, 2021 21:11
@chris-araman
Copy link
Copy Markdown
Collaborator Author

I'm at a loss for why the checks are failing with Swift 5.5, currently Xcode 13 Beta 3 on the macos-11 runners.

Maybe we need to specify the macOS destination now?
https://github.com/mxcl/xcodebuild/runs/3210635358#step:3:90

xcodebuild: error: Building a Swift package requires that a destination is provided using the "-destination" option. The "-showdestinations" option can be used to list the available destinations.

@mxcl
Copy link
Copy Markdown
Owner

mxcl commented Aug 1, 2021

I’ll force a cron run and see if that fails too. If so, yeah, seems like. I'll fix on main branch.

https://github.com/mxcl/xcodebuild/actions/runs/1087802085

@mxcl
Copy link
Copy Markdown
Owner

mxcl commented Aug 2, 2021

Fixed in #51. Or at least the test I added failed then passed. Rebase or merge!

@chris-araman
Copy link
Copy Markdown
Collaborator Author

Rebased from master. Checks passed. I don't want to merge to master until you've signed off on the change requested, though. 😊

@chris-araman chris-araman merged commit 49dda70 into master Aug 2, 2021
@chris-araman chris-araman deleted the certificate branch August 2, 2021 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants