[macOS] codesign native assets during embed

[knopp]

Fixes #148051

Currently only the "embed" phase, which is run during the Runner target build have access to code-signing identity. The flutter assemble target, which does the main build (and also builds native assets) does not have access to the code-signing identity.

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• I followed the breaking change policy and added Data Driven Fixes where supported.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[knopp]

@dcharkes, any idea if this is testable (and how)? Also this PR leaves all the dart codesigning paths unchanged. Those are not really used (the code-signing identity define is never set).

[dcharkes]

@dcharkes, any idea if this is testable (and how)? Also this PR leaves all the dart codesigning paths unchanged. Those are not really used (the code-signing identity define is never set).

We can only test this with an integration test. A test that actually runs flutter build and/or flutter run. And then we need to look in the app bundle to see if the framework/dylib is signed.

flutter/packages/flutter_tools/test/integration.shard/isolated/native_assets_test.dart

Line 285 in cb656c2

expectDylibIsBundledWithFrameworks(exampleDirectory, buildMode, add2appBuildSubcommand.replaceAll('-framework', ''));

We should probably use some kind of commandline tool to inspect what signature the dylib is signed with and check that it's the right one (or the same one as the flutter framework).

[knopp]

The problem here is that the dylibs are already codesigned through codesignDylib with adhoc certificate. There are two issues with this

a) the codesigning is performed twice (second time actual identity replacing the first signature),
b) unless the test is run with an real code-signing identity (i don't know if and how we can do this), it will simply resign the binaries with another adhoc signature so we can't really verify this.

Possible option would be to disable codesigning of macOS native assets in code and only do it in the script, in which case we should be able to verify the signature in tests.

[knopp]

The annoying part about this is that this diverges the macOS / iOS implementation even further. On iOS codesigning through code should work, since it's part of the Runner target and thus has access to identity.

@jmagman, do you know why on macOS Flutter assemble is handled as a separate Xcode target? It would be much easier if everything was handled in the Runner target, but pragmatically, even if doable, that'd be a pretty big change and just the migration itself would likely require some significant xcode project surgery.

[knopp]

@dcharkes, I disabled adhoc code-signing of macOS dylib in dart code (_copyNativeAssetsMacOS) unless actual identity is provided and added test that will fail when macos_assemble.sh doesn't codesign native assets.

Note that as far as I can tell there is currently no way to provide code-signing identity in a way that would reach _copyNativeAssetsMacOS. There however might be in future, for example if the BuildFrameworkCommand is amended to parse dCodesignIdentity, which right now it doesn't seem to.

[jmagman]

@jmagman, do you know why on macOS Flutter assemble is handled as a separate Xcode target? It would be much easier if everything was handled in the Runner target, but pragmatically, even if doable, that'd be a pretty big change and just the migration itself would likely require some significant xcode project surgery.

I totally agree, the divergence has been completely annoying and added so much work over the years.
I would love if someone undertook the work to converge the iOS and macOS projects (xcode_backend.dart vs sh, extensive target/build phase Xcode project file surgery, or even combining the iOS/macOS project directories into a single darwin Xcode project). The macOS tooling implementation has lagged so far behind iOS in the last 5 years, for example #94747, all the macOS spew that gets logged to stderr, etc

[dcharkes]

@dcharkes, I disabled adhoc code-signing of macOS dylib in dart code (_copyNativeAssetsMacOS) unless actual identity is provided and added test that will fail when macos_assemble.sh doesn't codesign native assets.

👍

The test is failing on the CI atm.

[knopp]

@dcharkes, the tests are passing now.

[dcharkes]

LGTM!

Thanks @knopp! 🙏