1736942 - macOS build with enabled content sandbox crashes on startup, nsMacUtilsImpl::GetRepoDir, GetStringValueFromBundlePlist

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect, P3)

Description

[Kai Engert [:KaiE:]]

I'm building comm-esr91 on macOS.

On startup, I crash here:
https://searchfox.org/mozilla-esr91/rev/6de35c9d696f55078a70f4e4ec8dec060e5e93b9/dom/ipc/ContentParent.cpp#2444

The failure happens in GetDirFromBundlePlist, GetStringValueFromBundlePlist, here:
https://searchfox.org/mozilla-esr91/rev/6de35c9d696f55078a70f4e4ec8dec060e5e93b9/xpcom/base/nsMacUtilsImpl.cpp#357

I found plugin-container.app/Contents/Info.plist files that contain the <key>MozillaDeveloperRepoPath</key> with a <string>DIR</string> where DIR is an absolute path of the mozilla-esr91 directory.

I see the same values in a local build of comm-central, which doesn't crash.
I haven't checked if this plist file is the one that is being read.

Comment 1

[Kai Engert [:KaiE:]]

To further clarify the circumstances:

It crashes if I attempt to run using "mach run".

The build works fine, if I use "mach package", and then extract and run that.

The problem is with the code that attempts to detect if the executable is a development build - it concludes yes, then it expects to obtain the RepoDir, but it fails to get that, and crashes deliberately.

Comment 2

[Kai Engert [:KaiE:]]

As a workaround until this bug gets fixed, I changed nsContentChild.cpp function IsDevelopmentBuild() to always return false. That allows "mach run" to start up TB without crashing.

Comment 3

[Andrew McCreight [:mccr8]]

This looks like code that was added for sandboxing, so I'll move the bug over there so that people more familiar with this code might see it.

Comment 4

[Haik Aftandilian [:haik]]

@KaiE, do you see those entries (MozillaDeveloperRepoPath and MozillaDeveloperObjPath) in the Info.plist for the parent process too? That would be the Thunderbird.app/Contents/Info.plist file.

The ContentParent.cpp code you referenced should be reading from that file and not the plugin-container files.

The build process should be populating those values with browser/app/Makefile.in and ipc/app/Makefile.in, but it sounds like the values are there as expected.

For development builds (launched with ./mach run), the content processes need to be able to read files from the repo directory which is not allowed by the content process sandbox for packaged builds. Symlinks are used heavily for development builds from locations in $OJB_DIR/dist/Nightly.app/ to locations in the source tree.

For packaged builds (./mach build package), all the files needed should be bundled into the omni.ja file which is in the .app directory and content processes have access to it.

Comment 5

[Kai Engert [:KaiE:]]

(In reply to Haik Aftandilian [:haik] from comment #4)

@KaiE, do you see those entries (MozillaDeveloperRepoPath and MozillaDeveloperObjPath) in the Info.plist for the parent process too? That would be the Thunderbird.app/Contents/Info.plist file.

No. They are missing in that file.

Manually adding the entries to that file allows a successful startup.

Comment 6

[Haik Aftandilian [:haik]]

Then we probably need to add the following to mail/app/macbuild/Contents/Info.plist.in. But that would mean this has been broken for some time which would be surprising.

From the Firefox tree from file browser/app/macbuild/Contents/Info.plist.in:

  <key>MozillaDeveloperRepoPath</key>
  <string>@MOZ_DEVELOPER_REPO_PATH@</string>
  <key>MozillaDeveloperObjPath</key>
  <string>@MOZ_DEVELOPER_OBJ_PATH@</string>

@KaiE, are you able to test that?

And is it possible ./mach run has been broken for a long time or perhaps something changed in Thunderbird that explains why we hit this now?

Comment 7

[Kai Engert [:KaiE:]]

(In reply to Haik Aftandilian [:haik] from comment #6)

And is it possible ./mach run has been broken for a long time or perhaps something changed in Thunderbird that explains why we hit this now?

I think this wasn't seen earlier, because it only happens if a developer attempts to use a Thunderbird branch build on Windows or Mac.

For nightly builds (comm-central) on Mac, the sandbox content level is zero (sandbox disabled), and the code isn't reached.

Comment 8

[Kai Engert [:KaiE:]]

https://searchfox.org/mozilla-esr91/rev/6de35c9d696f55078a70f4e4ec8dec060e5e93b9/security/sandbox/common/SandboxSettings.cpp#91

Comment 9

[Kai Engert [:KaiE:]]

Attachment: Bug 1736942 - MacOS developer builds with enabled Content Sandbox require the repository path in Info.plist. r=rjl

Comment 10

[Kai Engert [:KaiE:]]

(In reply to Haik Aftandilian [:haik] from comment #6)

@KaiE, are you able to test that?

Yes, that works, thanks. I've submitted a patch in your name.

Comment 11

[Kai Engert [:KaiE:]]

Hmm, my local commit attributes Haik, but phab changed the author.

Comment 12

[Kai Engert [:KaiE:]]

Comment on attachment 9247190 [details]
Bug 1736942 - MacOS developer builds with enabled Content Sandbox require the repository path in Info.plist. r=rjl

[Approval Request Comment]
Regression caused by (bug #): the change that enabled the content sandbox
User impact if declined: none
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): only affects developer builds

Comment 13

[Haik Aftandilian [:haik]]

(In reply to Kai Engert (:KaiE:) from comment #7)

(In reply to Haik Aftandilian [:haik] from comment #6)

And is it possible ./mach run has been broken for a long time or perhaps something changed in Thunderbird that explains why we hit this now?

I think this wasn't seen earlier, because it only happens if a developer attempts to use a Thunderbird branch build on Windows or Mac.

For nightly builds (comm-central) on Mac, the sandbox content level is zero (sandbox disabled), and the code isn't reached.

Ah, good to know, thanks. Glad we got this figured out.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bc69936d57b0
MacOS developer builds with enabled Content Sandbox require the repository path in Info.plist. r=rjl

Comment 15

[Wayne Mery (:wsmwk)]

Comment on attachment 9247190 [details]
Bug 1736942 - MacOS developer builds with enabled Content Sandbox require the repository path in Info.plist. r=rjl

[Triage Comment]
Approved for beta

Comment 16

[Rob Lemley [:rjl]]

Thunderbird 94.0b5:
https://hg.mozilla.org/releases/comm-beta/rev/b3d833c8e77c

Comment 17

[Wayne Mery (:wsmwk)]

Comment on attachment 9247190 [details]
Bug 1736942 - MacOS developer builds with enabled Content Sandbox require the repository path in Info.plist. r=rjl

[Triage Comment]
approved for esr91

Comment 18

[Rob Lemley [:rjl]]

Thunderbird 91.3.1:
https://hg.mozilla.org/releases/comm-esr91/rev/87ca0ae2f332