Skip to content

Conversation

@rinej
Copy link
Collaborator

@rinej rinej commented Oct 3, 2025

  • Making certificate-password optional
  • Making keychain-password optional - if empty use autogenerated pass

@rinej rinej requested a review from Copilot October 3, 2025 10:14
Copy link

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 makes two inputs optional for iOS app signing: certificate-password and keychain-password. The changes remove required validation for these parameters and implement fallback logic to handle cases where they are not provided.

  • Remove validation that required certificate-password and keychain-password inputs
  • Add auto-generation of keychain password when not provided using openssl rand -base64 32
  • Add conditional certificate import logic to handle password-protected and unprotected certificates

Reviewed Changes

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

File Description
action.yml Removes validation checks and implements fallback logic for optional password parameters
README.md Updates documentation to clarify that both password parameters are now optional

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

action.yml Outdated
Comment on lines 258 to 259
if ! security import $CERTIFICATE_PATH -A -t cert -f pkcs12 -k $KEYCHAIN_PATH; then
echo "Certificate import failed. If this P12 file requires a password, please provide certificate-password input."
Copy link

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

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

The error message could be more specific about the actual failure reason. Consider capturing the security command's error output to provide more actionable feedback to users.

Suggested change
if ! security import $CERTIFICATE_PATH -A -t cert -f pkcs12 -k $KEYCHAIN_PATH; then
echo "Certificate import failed. If this P12 file requires a password, please provide certificate-password input."
SECURITY_IMPORT_ERROR=$(security import $CERTIFICATE_PATH -A -t cert -f pkcs12 -k $KEYCHAIN_PATH 2>&1)
if [ $? -ne 0 ]; then
echo "Certificate import failed. If this P12 file requires a password, please provide certificate-password input."
echo "Error output from 'security import':"
echo "$SECURITY_IMPORT_ERROR"

Copilot uses AI. Check for mistakes.
@rinej rinej requested a review from thymikee October 3, 2025 10:53
cursor[bot]

This comment was marked as outdated.

action.yml Outdated
Comment on lines 241 to 242
# Mask the keychain password to prevent accidental logging
echo "::add-mask::$KEYCHAIN_PASSWORD"
Copy link
Contributor

Choose a reason for hiding this comment

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

do we even need to log this? I'd remove it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done ✅

@rinej rinej merged commit a7bfe11 into main Oct 7, 2025
1 check passed
@thymikee thymikee deleted the feat/make-certificate-password-optional branch October 7, 2025 14:53
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