I've tested this build: https://github.com/emlynmac/jamulus/suites/5835725008/artifacts/196029418

And it seems to be good. Would be great if @ann0see and @hoffie @softins @pljones could have a look.

I'd rather this were two pull requests as the two things it does are unrelated.

I've loaded the build onto my Mac running Mojave.

1. It seems to fix Can't Load or Save Mixer Channels Setup on Mac #2560. I can save the mixer setup to a file anywhere within my home directory and load it again; it doesn't need to be in a folder within the sandbox. Thanks!
2. It doesn't change the behaviour of the server recording directory as mentioned in macOS: Allow r/w access outside of default ini file location #2131 (comment). If I click on the Recording Directory button, I get a finder dialog in which I can create or navigate to a recording directory, but it will only allow me to navigate within the sandbox, not to any arbitrary directory within my home directory. I assume the new entitlement that was added does not apply to this operation.
3. Interestingly, when I started Finder from the desktop, it didn't show me the ~/Library directory, so I was unable to navigate to the Recordings directory I created in the sandbox above. After a while I looked in View::Show View Options and found a checkbox for Show Library Folder. Problem solved, although a user would need to know they must then navigate into Library/Containers/io.jamulus.JamulusServer/Data to find their recordings directory.

I'd rather this were two pull requests as the two things it does are unrelated.

Sadly, they’re related as it’s not possible to build with signing without the codeql change

2. It doesn't change the behaviour of the server recording directory as mentioned in macOS: Allow r/w access outside of default ini file location #2131 (comment). If I click on the Recording Directory button, I get a finder dialog in which I can create or navigate to a recording directory, but it will only allow me to navigate within the sandbox, not to any arbitrary directory within my home directory. I assume the new entitlement that was added does not apply to this operation.

This is likely due to how that's done.
The entitlement to read/write is from the user choosing a location / file. This is the way it must be done in a sandboxed app since the all-files was deprecated: https://developer.apple.com/documentation/bundleresources/entitlements/com_apple_security_files_all

Maybe the server UI can change to have a means to choose a location. I've updated this PR in the meantime to only address the client file access issue.

I'd rather this were two pull requests as the two things it does are unrelated.

Sadly, they’re related as it’s not possible to build with signing without the codeql change

I think the suggestion is to do a separate PR with just the codeql change, first get that merged, and then update this PR to contain only the entitlements change as a fix for #2560

I don't have a strong view either way.

The person who originally reported the issue on Facebook has tried the build listed above, and confirmed that it fixes the problem with saving and loading mixer channel files.

I'd rather this were two pull requests as the two things it does are unrelated.

Sadly, they’re related as it’s not possible to build with signing without the codeql change

I think the suggestion is to do a separate PR with just the codeql change, first get that merged, and then update this PR to contain only the entitlements change as a fix for #2560

I don't have a strong view either way.

Yeah, I understand there's a desire here for that to be separated out into another PR, but better to have directly related changes merged together IMHO.

But they just happened to be encountered together. The codeql change would be needed even if we didn't address the reported bug by changing the entitlements.

But they just happened to be encountered together. The codeql change would be needed even if we didn't address the reported bug by changing the entitlements.

Yes, this is true; and we need the change in this PR to make this fix actually work. So rather than the overhead of making a whole other one-line PR, builds etc. I see no reason why not to include the change here where it is needed to make the fix work.

@pljones You can always make a PR for the change to disable codeQL if you want to separate this out, at which time the changes from this PR will become none for that aspect.

@emlynmac I'm not sure why this was closed (there was no comment), but I raised and merged PR #2564 for the CodeQL part, and have updated your branch to contain just the entitlement change. I kept a backup branch called bugfix-2560-codeql-tests in your repo for reference about the attempts to control CodeQL.

You will need to do a git fetch to get my changes.

So I'm re-opening this PR ready to merge just the entitlements. I'll update the title in a moment.

OK, it won't let me re-open because I force-pushed to your branch. I might need to create a fresh PR.