Skip to content

Conversation

abertelrud
Copy link
Contributor

@abertelrud abertelrud commented Dec 8, 2021

This is the 5.6 cherry-pick of #3918 and its refinement #3920.

This change makes the sandbox cover the regular "/tmp" and NSTemporaryDirectory() paths. If environment entries are used to set other temporary directories in the subprocess, then the calling code should pass those to the writableDirectories parameter.

rdar://86150592
(cherry picked from commit 5546a97 and 3ba3305)

…eTemporaryDirectory` is set. (swiftlang#3918)

It seems that `TSCBasic.determineTempDirectory()` lets environment variables override the temporary directory, which might be alright for the process itself but not necessarily for spawned subprocesses.  If the subprocess uses NSTemporaryDirectory(), then that still won't be covered by the sandbox.

This change makes the sandbox cover the regular "/tmp" and NSTemporaryDirectory() paths.  If environment entries are used to set other temporary directories in the subprocess, then the calling code should pass those to the `writableDirectories` parameter.

rdar://86150592
(cherry picked from commit 5546a97 and  3ba3305)
@abertelrud abertelrud self-assigned this Dec 8, 2021
@abertelrud abertelrud added 5.6 ready Author believes the PR is ready to be merged & any feedback has been addressed labels Dec 8, 2021
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud
Copy link
Contributor Author

@swift-ci please test

@abertelrud
Copy link
Contributor Author

abertelrud commented Dec 8, 2021

Self-hosted is failing because there is no 5.6 toolchain yet (it's not a required build though).

@abertelrud abertelrud merged commit b39c83e into swiftlang:release/5.6 Dec 9, 2021
@abertelrud abertelrud deleted the eng/adjust-sandbox-to-always-include-nstemporarydirectory-when-tmp-dir-is-writable-5.6 branch December 9, 2021 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Author believes the PR is ready to be merged & any feedback has been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants