Made temp directory creation respect the TMPDIR environment variable …

[shannah]

…rather than forcing it inside /tmp directory. When running inside some sandboxed environments /tmp will not be available.

[DanBloomberg]

That section needs to be surrounded by braces { ... }, as in the section above, because C only permits variables to be declared following an opening brace.

Also, I note that you have pathJoin on line 3311, whereas the master has it on line 3287.

Do you have a sandboxed unix environment with TMPDIR defined?

[shannah]

Thanks. I'll amend it tomorrow.

I'm using the Mac app sandbox which doesn't allow access to /tmp.

In my particular environment I have TMPDIR defined but on environments that don't have it by default it is nice to at least have an option for configuring it from the environment.

A better solution may be to just use mktemp() (or one of its variants) which, at least on Mac will use the system config temp dir, which the app always has access to even inside the sandbox.

[shannah]

I have made the requested change (wrapped code in block for C89 compatibility).

Just for the record, I experimented with an alternate fix in which I use the iOS path for getting the temp directory. E.g.

size_t n = confstr(_CS_DARWIN_USER_TEMP_DIR, result, nbytes);

As this should have worked for OS X also. But this didn't seem fix my issue - the temp directory it retrieved was still not accessible. If I get some free time and more curiosity, I'll examine that path further, but for now having the ability to at least control it via environment variable TMPDIR gets past the road block.

[DanBloomberg]

This issue came up 5 years ago, during a major re-write of the code handling temp directories.
It was dropped after the dust cleared.

Here's the problem. Putting your change int makeTempDirname() will not cause rewriting of temp directory names in general.
And that was for a reason: we had determined that rewriting the names was very confusing to users. It was necessary in Windows, however, where it was retained.

So, for example, lept_mkdir() makes a pathname starting with "/tmp". This is done in genPathname(), which was the major target for fixing the temp directory issues. genPathname() explicitly states that TMPDIR is ignored in unix. It was done for simplicity, ease of maintenance and avoiding issues. For the latter, it worked for 5 years :-)

It's not that this can't be done. For example, modifying

    if (!is_win32 || dirlen < 4 ||
        (dirlen == 4 && strncmp(cdir, "/tmp", 4) != 0) ||  /* not in "/tmp" */
        (dirlen > 4 && strncmp(cdir, "/tmp/", 5) != 0)) {  /* not in "/tmp/" */
        stringCopy(pathout, cdir, dirlen);

to

    if (dirlen < 4 ||
        (dirlen == 4 && strncmp(cdir, "/tmp", 4) != 0) ||  /* not in "/tmp" */
        (dirlen > 4 && strncmp(cdir, "/tmp/", 5) != 0)) {  /* not in "/tmp/" */
        stringCopy(pathout, cdir, dirlen);
    } else if (!is_win32) {
        char *tmpDir = getenv("TMPDIR");
        char *path1;
        if (!tmpDir) tmpDir = "/tmp";
        if (dirlen == 4)
            path1 = stringJoin(tmpDir, NULL);
        else
            path1 = stringJoin(tmpDir, cdir + 4);
        stringCopy(pathout, path1, strlen(path1));
        LEPT_FREE(path1);

gets most of the way toward the unix path rewrite. (Some of the regression tests still fail)

If you can't write to /tmp on your unix system, you'll need to use your workaround with TMPDIR.

Dan

[shannah]

It's puzzling to me that this hasn't been reported before. As far as I can tell this should affect all Mac versions when running in the sandbox. I.e. without this workaround Leptonica can't be used in Mac with sandbox enabled.

[DanBloomberg]

Yes, it's a puzzle. You are the first to bring this up in 5 years.

…

On Wed, Dec 30, 2020 at 12:20 PM Steve Hannah ***@***.***> wrote: It's puzzling to me that this hasn't been reported before. As far as I can tell this should affect all Mac versions when running in the sandbox. I.e. without this workaround Leptonica can't be used in Mac with sandbox enabled. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#561 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD7KMLDCUUGYJRGEZXZF7LLSXODSLANCNFSM4VN2QXBA> .

[stweil]

@shannah, I use Tesseract OCR with Leptonica on MacOS (Intel since several years, now also M1) and don't have problems with /tmp (which links to private/tmp).

Do you have an example how I can reproduce the problem?

[stweil]

Would it be possible to use the OS_IOS code above for MacOS, too?

[stweil]

Commit 05ff50e uses the OS_IOS code for MacOS, too. This makes the pull request here obsolete.

[shannah]

@shannah, I use Tesseract OCR with Leptonica on MacOS (Intel since several years, now also M1) and don't have problems with /tmp (which links to private/tmp).

Are you using it in an app with sandbox enabled?

[stweil]

No, I use Tesseract from the command line. Is it possible to "emulate" a sandbox environment? How?

[stweil]

@shannah, does the terminal app run with sandbox enabled, or can I get a command line somehow with sandbox enabled? I still would like to reproduce the problem which should be fixed by this pull request.

Or is this PR no longer needed?

[shannah]

I haven't had to recompile it in a while, but last i tried, this patch was necessary to run inside sandbox.
When running in terminal it isn't running in sandbox.

It may be possible to emulate the sandbox using the sandbox-exec command (https://paolozaino.wordpress.com/2015/08/04/how-to-run-your-applications-in-a-mac-os-x-sandbox-to-enhance-security/) but i haven't tried this, and would require a sandbox config file to be created first.

[stweil]

sandbox-exec does not work for me:

% sudo sandbox-exec -f sandbox-conf /System/Applications/Utilities/Terminal.app/Contents/MacOS/Terminal
sandbox-exec: execvp() of '/System/Applications/Utilities/Terminal.app/Contents/MacOS/Terminal' failed: Operation not permitted

[stweil]

I could run zsh in a sandbox now with this modified sandbox configuration file (allow default instead of deny default):

;; This is my first sandbox configuration file!
(version 1) 
(allow default)

;; Let's allow file read and write in specific locations and not
;; all over my filesystem!
;; Please note you can add more (regex "^/Users/user_name/xxxxxxxxxxx") lines depending
;; on what your MyApp needs to function properly.
(allow file-write* file-read-data file-read-metadata
  (regex "^/Users/user_name/[Directories it requires to write and read from]")
  (regex "^/Applications/MyApp.app")
  (regex "^(/private)?/tmp/"))

;; You can also add a separate section for reading and writing files outside your
;; user_name account directory.
(allow file-read-data file-read-metadata
  (regex "^/dev/autofs.*")
  (regex "^/System/Library")
  (regex "^/Applications/MyApp.app")
  (regex "^/usr/lib")
  (regex "^/var")
  (regex "^/Users/user_name"))

;; If your MyApp requires to access sysctl (in read)
(allow mach* sysctl-read)

;; If you want to import extra rules from
;; an existing sandbox configuration file:
;; (import "/usr/share/sandbox/bsd.sb")

;; If you want to decide in which filesystem paths
;; MyApp is forbidden to write:
(deny file-write-data
   (regex #"^(/private)?/etc/localtime$"
     #"^/usr/share/nls/"
         #"^/usr/share/zoneinfo/"))

;; If your MyApp wants to run extra processes it's be allowed to run only
;; child processes and nothing else
(allow process-exec
  (regex "^/Applications/MyApp.app"))

;; If your MyApp requires network access you can grant it here:
(allow network*)

Obviously it depends on the sandbox configuration whether /tmp is writable. In my case it is, so writing to /tmp is not a general problem with sandboxed applications.

[shannah]

Yes. Apps in the appstore don't have a choice as to whether /tmp is writable, so to emulate that condition you would need to make it unwritable in the sandbox config.

[stweil]

TMPDIR is set in the MacOS shell by default. It points to a directory with more than 2000 entries in my case while /tmp points to /private/tmp with only 5 entries. So obviously a lot of applications prefer the directory from TMPDIR, and maybe Leptonica should do so, too.

[shannah]

Makes sense.

[stweil]

last i tried, this patch was necessary to run inside sandbox

I think that you would get a different result now.

This pull request is no longer needed because since commit 05ff50e MacOS uses the configured path for temporary files which should give the same result as using TMPDIR.

[stweil]

@shannah, please try the latest code which should fix the issues with macOS applications running in the sandbox. Do you still think that TMPDIR should be respected in the Leptonica code (maybe not only for macOS / iOS, but also for other operating systems like Linux, too)? Or can this pull request be closed?