johnno1962 / InjectionNext

Fourth evolution of Code Injection for Xcode
MIT License
77 stars 2 forks source link

InjectionNext: simplify code signing ID finding. #10

Closed oryonatan closed 4 months ago

oryonatan commented 4 months ago

Replace the textbox with a combobox that is automatically filled with the output of the "security find-identity -v -p codesigning" command in order to simplify injection on devices.

oryonatan commented 4 months ago

@johnno1962 I am using swift Regex which requires macOS13 I can also implement it using NSRegularExpression, if you want to support older versions of macOS.

johnno1962 commented 4 months ago

Hi @oryonatan, thanks for this. Without wanting to be too precious I'm not keen to bump the minimum deployment version just for a regular expression (not least because I'm about to bump the version of app and could do without the merge conflict!). The project already uses my own regex library that doesn't cause this sort of issue for which the code would be:

    var codeSigningID: String {
        guard let sha1: String = sha1Regex[#"([0-9A-F]{40})"#] else { return "" }
        return sha1
    }

The idea is you can subscript into a string with a regex and groups are assigned to the output tuple which is in this case just a string. Any chance you'd be prepared to use/test this code instead and revert the project file change? After your change is the user still able to use the following as a build phase?

defaults write com.johnholdsworth.InjectionNext signingidentity $EXPANDED_CODE_SIGN_IDENTITY
oryonatan commented 4 months ago

HI @johnno1962 I modified the code to user SwiftRegex and verified that it works.

I am not sure about where to check this build phase ... what I checked is that InjectionNext still works with our App on devices after the change.

johnno1962 commented 4 months ago

I was thinking the defaults write could have been added to the reinstated bundle copy script and avoid a lot of extra commands running, parsing results etc each time a person injects which may slow it down.

oryonatan commented 4 months ago

I was thinking the defaults write could have been added to the reinstated bundle copy script and avoid a lot of extra commands running, parsing results etc each time a person injects which may slow it down.

ok but right now there is no bundle copy script, right? so is there anything I should check before merging?

johnno1962 commented 4 months ago

Please leave the merging into main to me on this project, there are a couple of other changes I'd like to discuss.

johnno1962 commented 4 months ago

There is a copy_bundle.sh in the bundles branch I prepared for you. I have an abstraction for running commands in the Popen package. Can you try the following change? Topen is "Task open" and returns a stream on which you can read lines.


    func fetchCodeSigningIdentities() -> [String] {
        var identities: [String] = []

        let security = Topen(exec: "/usr/bin/security",
            arguments: ["find-identity", "-v", "-p", "codesigning"])

        while let line = security.readLine() {
            let components = line.split(separator: ")", maxSplits: 1)
            if components.count >= 2 {
                let identity = components[1]
                identities.append(String(identity))
            }
        }

        return identities
    }
johnno1962 commented 4 months ago

I need to think about this some more. To be honest I miss the option of being able to set the default if we're moving back to a copy bundles approach eventually anyway which would be as near as possible to automatic. I'm trying to think if there is a way to have the best of both worlds and I won't be able to merge this today. If you move to using Topen(), I'll merge tomorrow morning and try and re-instate being able to use the default on main.

oryonatan commented 4 months ago

Hi @johnno1962 , changed the code to use Popen

oryonatan commented 4 months ago

regarding setting the default, do you mean keeping the same ID on every launch? I think that'll be a good idea, and should be quite easy if we save the selection to the NSUserDefaults

johnno1962 commented 4 months ago

func parseSecurityOutput can be removed now right? Yes, it would be good to make the user's selection "sticky". Try to save just the expanded ID in the default rather the longer string and make it settable if possible.

johnno1962 commented 4 months ago

I've made a push paving the way for a codesigningIdentity default if you want to rebase. Also, while you're editing the nib can you remove the obsolete menu item "Compiler Injection" please which is no longer connected.

oryonatan commented 4 months ago

ooo

oryonatan commented 4 months ago

I just pushed a commit with more organized data validation/saving

oryonatan commented 4 months ago

I guess we worked in the same time, you can check my code

johnno1962 commented 4 months ago

Can we route all interaction with persistent user defaults state though the new Defaults struct?

oryonatan commented 4 months ago

rebased the code and changed to use the Defaults class

I was however unable to check my change, it seems like the project wont build due to some SPM issue (or maybe my XCode just went on a strike)

johnno1962 commented 4 months ago

You really don't like SPM do you? Understandable at times but the project is less SPM than it was though you now need to clone it with --recurse-submodues. I've fixed the build problem. There was a group with no files so git didn't create the directory and none of the relative paths would resolve. There was also a typo. Should be OK now. Code looking good now.

johnno1962 commented 4 months ago

Did you remove the "Compiler Injection" item from the menu bar menu while you're editing the nib? (or I can pick that up)

oryonatan commented 4 months ago

Hi, it works after rebase, but I had to remove the Bundle target dependency for it to compile.

oryonatan commented 4 months ago

didn't touch the "Compiler Injection" icon

johnno1962 commented 4 months ago

Never mind, I'll tidy up the nib later. You really shouldn't have needed to "remove the Bundle target dependency for it to compile". That's a double check that the package itself is compiling. I can merge this now and pick up any pieces with a fresh clone if there any. Awaiting your confirmation...

oryonatan commented 4 months ago

from my side - you can merge I am still getting compilation errors on the bundle

error: Build input files cannot be found: '.... Did you forget to declare these files as outputs of any script phases or custom build rules which produce them? (in target 'InjectionBundle' from project 'InjectionNext')

oryonatan commented 4 months ago

update - it was my bad, I forgot to pull submodules, now everything builds fine

johnno1962 commented 4 months ago

Thanks @oryonatan.

johnno1962 commented 4 months ago

Executive decision required.. Do you think it would be better if "Select Xcode" also did a "Launch Xcode"?

johnno1962 commented 4 months ago

Xcode 16 beta4 checks out.