appium / appium-inspector

A GUI inspector for mobile apps and more, powered by a (separately installed) Appium server
https://appium.github.io/appium-inspector/
Other
1.21k stars 291 forks source link

ci: GHA to create packages for electron #1785

Closed KazuCocoa closed 2 weeks ago

KazuCocoa commented 2 weeks ago

e.g. https://github.com/appium/appium-inspector/actions/runs/11700110550

kazu $ codesign -dv --verbose=4 /Applications/Appium\ Inspector.app
Executable=/Applications/Appium Inspector.app/Contents/MacOS/Appium Inspector
Identifier=io.appium.inspector
Format=app bundle with Mach-O thin (arm64)
CodeDirectory v=20500 size=767 flags=0x10000(runtime) hashes=13+7 location=embedded
VersionPlatform=1
VersionMin=720896
VersionSDK=917504
Hash type=sha256 size=32
CandidateCDHash sha256=cab17f33d3335bc99051326a0b1a847325a795e8
CandidateCDHashFull sha256=cab17f33d3335bc99051326a0b1a847325a795e8bee71d444e031323784af64f
Hash choices=sha256
CMSDigest=cab17f33d3335bc99051326a0b1a847325a795e8bee71d444e031323784af64f
CMSDigestType=2
Executable Segment base=0
Executable Segment limit=16384
Executable Segment flags=0x1
Page size=4096
CDHash=cab17f33d3335bc99051326a0b1a847325a795e8
Signature size=9067
Authority=Developer ID Application: OpenJS Foundation, Inc. (UY52UFTVTM)
Authority=Developer ID Certification Authority
Authority=Apple Root CA
Timestamp=Nov 6, 2024 at 12:49:19 AM
Info.plist entries=31
TeamIdentifier=UY52UFTVTM
Runtime Version=14.0.0
Sealed Resources version=2 rules=13 files=62
Internal requirements count=1 size=180
eglitise commented 2 weeks ago

Are the test runs already using the OpenJS certificates? I downloaded the macOS artifact and installed the arm64 dmg file, but it was still returning the same warning about Apple being unable to check it for malicious software. Unsure if this is to be expected.

eglitise commented 2 weeks ago

We will also need to disable the runs in Azure, but I guess that can only be done in the Azure configuration.

KazuCocoa commented 2 weeks ago

Are the test runs already using the OpenJS certificates?

Only https://github.com/appium/appium-inspector/actions/runs/11700110550 , yes. This PR's description is by the package. Other old ones did not have the certificate secrets.

Unsure if this is to be expected.

Well, even dmg, I think downloaded content from the internet will be handled as malicious software. Possibly downloaded via GitHub release page is different, though. Invalid certificate case is addressed as a "damaged" content ( https://github.com/appium/appium-inspector/actions/runs/11699133241 is not openjs's one), so I think the certificate itself is fine.

We will also need to disable the runs in Azure, but I guess that can only be done in the Azure configuration.

Yea, this PR merge will remove the script for Azure, so after this merge, Azure's script will not run. I'll remove Azure's pipeline itself from the web page after confirming this via GHA package release work

jlipps commented 2 weeks ago

@eglitise @KazuCocoa i think we need to do 'notarization' in addition to code signing to do away with that warning. I think I have the details from OpenJS necessary to make this happen, but not sure how to do the technical notarization steps with electron-builder.

eglitise commented 2 weeks ago

I just tried with the Windows app. The executable is certainly signed now, unlike the current release (on the right): image However, if I look at the signing details, Windows still thinks there's something missing: image Not entirely sure if this causes issues during installation, though - I did get the same Windows Defender SmartScreen warning on first open, but it's possible that I downloaded the artifact from 'Create packages' run 2 (which was not signed), and I cannot get the popup again with either the run 2 or run 3 artifacts.

KazuCocoa commented 2 weeks ago

Maybe we could create a version with the current Azure config, which has the same signature once and compare this result with it

eglitise commented 2 weeks ago

with the current Azure config, which has the same signature

Do you mean that the new OpenJS certificates were also added to Azure? If not, then the version built there will be completely unsigned, like in my first screenshot.

KazuCocoa commented 2 weeks ago

Yea. I have updated Azure's CSC_LINK and CSC_KEY_PASSWORD with latest one we recently obtained. So this script and Azure only have differences in the destination of content upload.

Then, this PR itself can be merged and what we need to do here (potentially) is the certificate notarization as a followup PR to env var update later?

eglitise commented 2 weeks ago

Sounds good 👍

jlipps commented 2 weeks ago

FWIW we do not have new windows certs at all. Only macOS certs were provided so far. So it's not surprising that windows doesn't work.

KazuCocoa commented 2 weeks ago

Let me release a new version with this by adding a tag

KazuCocoa commented 2 weeks ago

https://github.com/appium/appium-inspector/releases/tag/untagged-38ffb8027be18a982f9d @jlipps @eglitise, how do we currently update the release note? Manual or script...?

Packages look fine.

eglitise commented 2 weeks ago

I do it manually. Not sure if it was worth running a release just for this though, since there's not really any other changes.

eglitise commented 2 weeks ago

@jlipps could we also get Windows certs? I think both would be equally important for improving the installation UX.

jlipps commented 2 weeks ago

Yes, I will kick off the process for Windows certs with OpenJSF

KazuCocoa commented 2 weeks ago

I have removed the Azure project. So from now on, our package creation env can be managed in this repository config (env var etc) only