electron / packager

Customize and package your Electron app with OS-specific bundles (.app, .exe, etc.) via JS or CLI
https://npm.im/@electron/packager
BSD 2-Clause "Simplified" License
178 stars 17 forks source link

Entitlements for saving to user-selected dir #271

Closed positlabs closed 8 years ago

positlabs commented 8 years ago

I am trying to save a user-selected output directory path so I can save to it in the future, without having the user re-select the dir every time the app is restarted. I think I have more than enough entitlements in the plists, so I'm not sure why it's borking.

Which version of electron-packager are you using?

5.2.1 (but actually latest/master)

What parameters are you passing to the packager() function?

{
        dir: './',
        name: 'GifGoat',
        'app-bundle-id': 'GIFGOAT-HELPER',
        icon: './assets/icons/gifgoat-icon.icns',
        // platform: 'darwin',
        platform: 'mas',
        arch: 'x64',

        version: require('./node_modules/electron-prebuilt/package.json').version, // v0.36.8
        'app-version': require('./package.json').version,
        'build-version': require('./package.json').version,

        asar: true,
        'asar-unpack-dir': 'bin/',

        'app-bundle-id': '...',
        'app-category-type': 'public.app-category.utilities',

        sign: '3rd Party Mac Developer Application: ...',
        'sign-entitlements': './build/mac-extras/parent.plist',
        'entitlements-inherit': './build/mac-extras/child.plist',
        prune: true,
        ignore: [
            '/bin/ffmpeg/linux($|/)',
            '/bin/ffmpeg/win32($|/)',
            '/dist($|/)',
            '/build($|/)',
            '/test($|/)',
        ],
        out: './dist/',
        overwrite: true
}

What is the host platform are you running electron-packager on?

OSX 10.11.3

What target platform(s)/architecture(s) are you building for?

darwin & mas, x64

Is there a stack trace in the error message you're seeing?

screen shot 2016-02-21 at 3 03 46 pm

Please provide either a failing testcase or detailed steps to reproduce your problem.

parent.plist

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
  <dict>
    <key>com.apple.security.app-sandbox</key>
    <true/>
    <key>com.apple.security.application-groups</key>
    <true/>
    <key>com.apple.security.files.user-selected.read-write</key>
    <true/>
    <key>com.apple.security.files.bookmarks.document-scope</key>
    <true/>
    <key>com.apple.security.files.bookmarks.app-scope</key>
    <true/>
  </dict>
</plist>

child.plist

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
  <dict>
    <key>com.apple.security.app-sandbox</key>
    <true/>
    <key>com.apple.security.inherit</key>
    <true/>
  </dict>
</plist>
malept commented 8 years ago

Which version of Electron are you building packages with?

positlabs commented 8 years ago

v0.36.8. I'll add as a comment in the original post, too

malept commented 8 years ago

I'm going to add it to the issue template, too.

Regarding your problem, I think @sethlu mentioned in #223 that he recommended Electron 0.35.x. If you're not dependent on a feature in 0.36.x, could you try that?

sethlu commented 8 years ago

@positlabs Actually, this looks to me like a folder permission error. If you'd like to add one other item in your parent.list the temporary exception of file read and write, not sure if this could be resolved. Also, the Electron v0.36.8 darwin build should be fine with packing/signing, entitlements being quite unecessary as it being meant for outside the Mac App Store. And yea, like @malept mentioned, I would recommend Electron v0.35.6 as the latest version for packing/signing for distribution inside the Mac App Store. Apple Docs: https://developer.apple.com/library/ios/documentation/Miscellaneous/Reference/EntitlementKeyReference/Chapters/AppSandboxTemporaryExceptionEntitlements.html P.S.: May I ask which ffmpeg are you using? I'm not quite sure about the effect due to an addition of libffmpeg.dylib to Electron v0.36.8 because @jasonhinkle has made some trials with building ffmpeg, I guess different from libffmpeg, by himself. Thanks!

positlabs commented 8 years ago

Tried adding this key to entitlements, but I'm still seeing the error. <key>com.apple.security.temporary-exception.files.absolute-path.read-write</key>

Also attempted a build using Electron v0.35.6, to no effect.

I'm using ffmpeg v2.8.6 via prebuilt binaries. It works fine just running electron directly, but fails in the signed app.

I suppose i could lose this feature of having a cached output directory (instead, write to one of the specific document folders that is allowed), but it seems silly that it's not allowed.

sethlu commented 8 years ago

Is the entitlements done like the following?

<key>com.apple.security.temporary-exception.files.absolute-path.read-write</key>
<array>
    <string>/</string>
</array>
positlabs commented 8 years ago

That did it! Thanks @sethlu!

I think i remember reading something about apple not liking when apps use that entitlement, though. Seems like it's just a convenient way to circumvent their file i/o security model. I will try submitting to mac app store and see what they say!

positlabs commented 8 years ago

@sethlu what is the reason behind using v0.35.6 for mas builds?

malept commented 8 years ago

Looks like he's documented this in electron-osx-sign: https://github.com/sethlu/electron-osx-sign/wiki/2.-Electron-Compatibility

positlabs commented 8 years ago

Hmm. I'm not seeing that issue when i build with v0.36.8, but I will follow your recommendation, @sethlu.

sethlu commented 8 years ago

@positlabs Great it works! The reason I'm saying v0.35.6 may be the best compatible mas build for distribution is that when sandboxed, there is an issue in rendering the graphics with Electron (at least on my machine :confused:), of which reason I'm not totally sure about. However, I guess when they upgrade their Chromium (PR: https://github.com/atom/libchromiumcontent/pull/175), the glitch could ease a bit. As the darwin builds don't really need to get sandboxed, they should be fine. P.S.: I'll keep the my list updated for future updates of Electron.

sethlu commented 8 years ago

On the ffmpeg, @jasonhinkle has tried to upload one of his apps to the App Store with his self-built binary. However, it seems that ffmpeg calls some of Apple's private APIs. Not sure if libffmpeg.dylib's doing the same.

positlabs commented 8 years ago

Is that an issue in regards to sandboxing? Or acceptance to the app store?

I'm a web dev, by trade. Not too familiar with native dev stuff. Jumping hurdles as they come :)

jasonhinkle commented 8 years ago

I've compiled a custom version of ffmpeg but I haven't tried submitting it yet, once I do that (this week with luck) if it gets accepted I'll post instructions. I don't really know what side-effects my changes will have as well, it may crash depending on what ffmpeg features you use!

positlabs commented 8 years ago

@jasonhinkle let me know how it goes! I'm going to try submitting by the end of the week.

positlabs commented 8 years ago

First attempt: REJECTED!

screen shot 2016-02-28 at 10 01 58 am

I tried using app-scope bookmarks since the docs describe the exact issue I'm trying to solve.

Security-Scoped Bookmarks and Persistent Resource Access

Your app’s access to file-system locations outside of its container—as granted to your app by way of user intent, such as through Powerbox—does not automatically persist across app launches or system restarts. When your app reopens, you have to start over. (The one exception to this is for files open at the time that your app terminates, which remain in your sandbox thanks to the OS X Resume feature).

Starting in OS X v10.7.3, you can retain access to file-system resources by employing a security mechanism, known as security-scoped bookmarks, that preserves user intent. Here are a few examples of app features that can benefit from this:

  • A user-selected download, processing, or output folder
  • An image browser library file, which points to user-specified images at arbitrary locations
  • A complex document format that supports embedded media stored in other locations

https://developer.apple.com/library/mac/documentation/Security/Conceptual/AppSandboxDesignGuide/AppSandboxInDepth/AppSandboxInDepth.html#//apple_ref/doc/uid/TP40011183-CH3-SW16

@sethlu It seems like this is probably an issue with Electron mas builds. What do you think? Should I file this issue over at https://github.com/atom/electron ?

sethlu commented 8 years ago

I see the security concerns with com.apple.security.temporary-exception.files.absolute-path.read-write; however, if to try any of the other ways with which file open/save could be realized, there should be some kind of methods for user input, in order to be granted with file access.

The app-scope bookmarks actually looks like a nice solution, but (from App Sandbox In Depth):

An app-scoped bookmark provides your sandboxed app with persistent access to a user-specified file or folder. For example, if your app employs a download or processing folder that is outside of the app container, obtain initial access by presenting an NSOpenPanel dialog to obtain the user’s intent to use a specific folder. Then, create an app-scoped bookmark for that folder and store it as part of the app’s configuration (perhaps in a property list file or using the NSUserDefaults class). With the app-scoped bookmark, your app can obtain future access to the folder.

I'm not sure about a way to prompt for any file open/save from Electron right now... Perhaps there have already been an implementation to open/save any file from the web app embedded? The file object seems helpful but looks like some work. Also, if it gives the same result as from NSOpenPanel which gives access to the file/folder. I may look further into that.

I guess forwarding this to https://github.com/atom/electron may give us some better answers. Let me take a look at the Atom source code and see how they get around with this. (Probably it'll malfunction after sandboxed.)

From StackOverflow: Mac OS In App Sandbox Entitlements Directory Read Issue

sethlu commented 8 years ago

All that I can say from Atom (https://github.com/atom/atom/blob/7aab88c4f62eb671bdfb9c036d1f0733f913109d/src/browser/atom-application.coffee) is that there's a dialog object from Electron which may prompt the user for a path.

sethlu commented 8 years ago

Yea, I guess the dialog should do the trick... From Electron (https://github.com/atom/electron/blob/8aced2c31eb796df291ef336a099e82b870c1396/atom/browser/ui/file_dialog_mac.mm), they seem to use NSOpenPanel for file access.

So if there's a way from Node to get something like (in Coffee) {dialog} = require 'electron', then we may have a chance to use the native way to have access to files, and with app-scoped bookmarks.

jasonhinkle commented 8 years ago

I have not done that yet myself but when I was reading about it my impression is that the sandbox exception entitlement requires that the user's file selection has to be done through a native file open/save dialog. Perhaps this page would be helpful? http://www.mylifeforthecode.com/getting-started-with-standard-dialogs-in-electron/

positlabs commented 8 years ago

I am using the dialog module to let the user select an output directory. That works as expected, and the files are saved out with no problem. The problem is that this permission isn't persisting across sessions. I think I need a way to store this path as a bookmark.

This article describes the process of storing bookmarked paths. It seems like some logic will need to be added to Electron before this works automatically.

positlabs commented 8 years ago

@jasonhinkle any luck on submitting that custom ffmpeg build? I've tried submitting without customizing and apple flagged it immediately.

the use of non-public APIs can lead to a poor user experience should these APIs change in the future, and is therefore not permitted. The app includes ': SecIdentityCreate' from the framework '/System/Library/Frameworks/Security.framework/Versions/A/Security'.

sethlu commented 8 years ago

I believe the same was mentioned in https://github.com/electron-userland/electron-osx-sign/issues/5.

positlabs commented 8 years ago

I'm going to close this since it will be resolved by https://github.com/atom/electron/issues/4637

jasonhinkle commented 8 years ago

@positlabs Hey, sorry I didn't get around to submitting due to other stuff coming up. I have a custom build that you can grab which I believe has the private API calls removed - however it also is missing some bits as well hopefully would not affect your app. ffmpeg here as of today https://github.com/jasonhinkle/Tube-DL/tree/master/assets/bin/osx is my custom build.

If you would like to try using that and re-submit your app, i'd be curious to see if it is accepted.

positlabs commented 8 years ago

Awesome! I'll give it a shot and let you know if it works.

positlabs commented 8 years ago

@jasonhinkle when I try using that build of ffmpeg in my app (even non-packaged, unsandboxed), it dies.

Error: ffmpeg was killed with signal SIGTRAP

jasonhinkle commented 8 years ago

Hmm what feature are you using of ffmpeg? I basically commented out the offending code and throw an error if that function is called. I only use ffmpeg for file conversions so my app never hits that part of the code.

It seems to be code that is validating an SSL cert, which indicates probably an http call. To actually re-implement it without the private API call looks like a tricky job.