Bug 216834 – [macOS] Issue sandbox extension for the diagnostic daemon

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Per Arne Vollan]

On internal installs, the UI process should issue a mach lookup sandbox extension for accessing the diagnostic daemon, and send it to the WebContent process.

[Per Arne Vollan]

<rdar://problem/58496834>

[Per Arne Vollan]

Created attachment 409383 [details]
Patch

[Per Arne Vollan]

*** Bug 206115 has been marked as a duplicate of this bug. ***

[Per Arne Vollan]

Created attachment 409385 [details]
Patch

[Brent Fulgham]

Comment on attachment 409385 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=409385&action=review

r=me, but I think you could use an existing FileSystem function instead of the raw 'stat' operation.

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:68
> +#import <sys/stat.h>

You don't need to include this if you use , FileSystem::fileIsDirectory

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:211
> +    static bool isInternal = !stat("/AppleInternal", &st);

I think it might be better to do:

static bool isInternal = FileSystem::fileIsDirectory("/AppleInternal", FileSystem::ShouldFollowSymbolicLinks::No);

[Per Arne Vollan]

Created attachment 409488 [details]
Patch

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #5)
> Comment on attachment 409385 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=409385&action=review
> 
> r=me, but I think you could use an existing FileSystem function instead of
> the raw 'stat' operation.
> 
> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:68
> > +#import <sys/stat.h>
> 
> You don't need to include this if you use , FileSystem::fileIsDirectory
> 
> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:211
> > +    static bool isInternal = !stat("/AppleInternal", &st);
> 
> I think it might be better to do:
> 
> static bool isInternal = FileSystem::fileIsDirectory("/AppleInternal",
> FileSystem::ShouldFollowSymbolicLinks::No);

Will fix, thanks for reviewing!

[Per Arne Vollan]

Created attachment 409489 [details]
Patch

[EWS]

Committed r267498: <https://trac.webkit.org/changeset/267498>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409489 [details].

Comment on attachment 409385 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=409385&action=review

>> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:211
>> +    static bool isInternal = !stat("/AppleInternal", &st);
> 
> I think it might be better to do:
> 
> static bool isInternal = FileSystem::fileIsDirectory("/AppleInternal", FileSystem::ShouldFollowSymbolicLinks::No);

Either way it’s not the recommended way to test whether a computer is running the Apple-internal version of macOS. See OSVariantSPI.h in WTF.

[Per Arne Vollan]

(In reply to mitz from comment #10)
> Comment on attachment 409385 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=409385&action=review
> 
> >> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:211
> >> +    static bool isInternal = !stat("/AppleInternal", &st);
> > 
> > I think it might be better to do:
> > 
> > static bool isInternal = FileSystem::fileIsDirectory("/AppleInternal", FileSystem::ShouldFollowSymbolicLinks::No);
> 
> Either way it’s not the recommended way to test whether a computer is
> running the Apple-internal version of macOS. See OSVariantSPI.h in WTF.

Ah, thanks! I will look into that.

[Per Arne Vollan]

Created attachment 409584 [details]
Patch

[Per Arne Vollan]

Created attachment 409585 [details]
Patch