Use updated method for removing system-trusted certificate #190
Conversation
|
Seems this doesn't work? (Also surely "remove-trusted-cert" is the inverse of "add-trusted-cert"?) |
djc
changed the title
integration-tests/macos.sh
Outdated
| printf "\n*** Remove test CA ***\n" | ||
| CERT_HASH=$(openssl x509 -in $ANY_CA_PEM -noout -fingerprint -sha1 | cut -d= -f2 | tr -d :) | ||
| security delete-certificate -Z "$CERT_HASH" /Library/Keychains/System.keychain || true | ||
| list | grep "$ANY_CA_SUBJECT" || true |
There was a problem hiding this comment.
Instead of doing the list | grep here, should it be calling the assert_missing() helper on L19..25 instead? It seems to handle the set +e complications so the || true added here wouldn't be needed.
There was a problem hiding this comment.
Seems like a reasonable suggestion!
There was a problem hiding this comment.
Hmmm, the failure seems to indicate the removal didn't work:
+ grep 'OU=GlobalSign Root CA - R3, O=GlobalSign, CN=GlobalSign'
cert[93] = OU=GlobalSign Root CA - R3, O=GlobalSign, CN=GlobalSign
Looking closer the output earlier shows an error from security delete-certificate too:
+ security delete-certificate -Z D69B561148F01C77C54578C10926DF5B856976AD /Library/Keychains/System.keychain
Unable to delete certificate matching "D69B561148F01C77C54578C10926DF5B856976AD"+ true
| list | grep "$ANY_CA_SUBJECT" | ||
| printf "\n*** Remove test CA ***\n" | ||
| CERT_HASH=$(openssl x509 -in $ANY_CA_PEM -noout -fingerprint -sha1 | cut -d= -f2 | tr -d :) | ||
| security delete-certificate -Z "$CERT_HASH" /Library/Keychains/System.keychain || true |
There was a problem hiding this comment.
Maybe this tidying up is not actually worth it. How about deleting reset and all calls to it? I assume GHA gives a fresh, fixed environment for every job run anyway...
There was a problem hiding this comment.
I assume the goal here was to be able to run it on your local machine, in which case leaving it in the same state it came in would be useful...
As suggested in actions/runner-images#12116 (comment).
Fixes #179.