Skip to content

Some macOS improvements in bootstrap launcher and test app for codesigning#20863

Merged
markcmiller86 merged 3 commits intodevelopfrom
task-mcm86-13mar26-minor-macos-imprvs
Mar 16, 2026
Merged

Some macOS improvements in bootstrap launcher and test app for codesigning#20863
markcmiller86 merged 3 commits intodevelopfrom
task-mcm86-13mar26-minor-macos-imprvs

Conversation

@markcmiller86
Copy link
Copy Markdown
Member

@markcmiller86 markcmiller86 commented Mar 14, 2026

Description

  1. Fixes security weaknesses in the macOS, Mach-O binary launcher (see below).
  2. Improves the test app used to exercise codesigning and notarization tests apart from masonry build script

I asked ChatGPT to evaluate the macOS launcher (thats the thing that gets executed in response to double clicking the VisIt icon on the mac). It is what turns around and starts the normal VisIt startup scripts. It is also the critical macOS "thing" that macOS security system uses to manage permissions given by the user, etc. ChatGPT found a number of issues with my original, bare-bones coding of this bootstrap and then provided an improved version which I ran and tested a few different ways.

  • argv[0] is not trustworthy. It is not guaranteed to be the true on-disk path of your executable. A caller can set it to essentially anything when invoking your program. So deriving your bundle-relative launcher path from argv[0] is not a secure way to identify “where the app really lives.”
  • Concatenating raw arguments into a shell command string is classic command injection.
  • Using a shell also means behavior depends on environment and shell parsing rules. That is broader attack surface than necessary.
  • In normal Finder double-click launch, argv[0] will probably often look sane enough that this works most of the time. But “works most of the time” is not the same as “safe,” especially once users can also launch from Terminal, scripts, wrappers, symlinks, or other tooling.
  • If your bundled Resources/bin/visit script can itself be modified post-install in some deployments, then this stub becomes a trusted gateway into untrusted content. Whether code signing and bundle sealing fully protect you depends on exactly how the app is packaged, signed, installed, and updated.

The test app for testing codesigning and notarization suffered some similar issues. In particular, it was NOT a Mach-O binary and was instead a python script. That doesn't really suffice for testing all the macOS codesigning stuff. So, I've fixed that too by making it a bona fide, clang-compiled, Mach-O binary which also puts up a dialog box when run via double-click.

Type of change

  • Bug fix~~
  • [ ] New feature
  • [ ] Documentation update
  • [ ] Other

How Has This Been Tested?

Reminders:

  • Please follow the style guidelines of this project.
  • Please perform a self-review of your code before submitting a PR and asking others to review it.
  • Please assign reviewers (see VisIt's PR procedures for more information).

Checklist:

  • I have commented my code where applicable.~~
  • [ ] I have updated the release notes.
  • I have made corresponding changes to the documentation.~~
  • I have added debugging support to my changes.~~
  • [ ] I have added tests that prove my fix is effective or that my feature works.
  • [ ] I have confirmed new and existing unit tests pass locally with my changes.
  • [ ] I have added new baselines for any new tests to the repo.
  • [ ] I have NOT made any changes to protocol or public interfaces in an RC branch.

@markcmiller86 markcmiller86 requested review from biagas and cyrush March 14, 2026 06:03
@markcmiller86 markcmiller86 changed the title Various macOS improvements Some macOS improvements in bootstrap launcher and test app for codesigning Mar 14, 2026
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the compiled, Mach-O binary. I don't think we need to commit this. It is compiled by test_notarize.py from testdmgnot.c.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This has changed because the app being used to test notarization has changed. However, if the codesigning process includes time-stamping information of any kind (which it almost certainly does), the sha256 hash on this file is almost always going to change when someone runs test_notarize.py. I think that is fine though.

@cyrush cyrush added this to the 3.5.0 milestone Mar 16, 2026
@markcmiller86 markcmiller86 merged commit 7a48f4f into develop Mar 16, 2026
0 of 3 checks passed
@markcmiller86 markcmiller86 deleted the task-mcm86-13mar26-minor-macos-imprvs branch March 16, 2026 21:11
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