forked from tiny-pilot/tinypilot
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Resolve tinypilot home dir independent from $HOME (tiny-pilot#1572)
Resolves tiny-pilot#1571. There are several places in the app where we rely on the `~` path shortcut, or the `$HOME` environment variable respectively. (For example when [processing the `settings.yml` file](https://github.com/tiny-pilot/tinypilot/blob/master/app/update/settings.py#L21).) However, there are scenarios where the app (or parts of it) are run under a different user, e.g. `root`, or with `sudo`, in which case `~` would erroneously resolve to `/root` instead of `/home/tinypilot`. This PR makes sure that the app always uses `/home/tinypilot` for accessing TinyPilot-specific objects, independent of what the `$HOME` environment variable contains. TinyPilot-specific objects are: - `settings.yml` - `logs/**` (i.e., update result logs) - `.flask-secret-key` - `tinypilot.db` - `app_settings.cfg` (no change needed, as [this is already referenced in an absolute manner](https://github.com/tiny-pilot/tinypilot/blob/504e7f113d21f90824acf229240167cafed39b17/debian-pkg/debian/tinypilot.service#L12)) I suggest that we don’t just hard-code the `/home/tinypilot` path as is, because that would make local development more inconvenient – you always would need to initialize and switch to a `tinypilot` user before being able to start developing. Therefore, I introduced a new environment variable (`TINYPILOT_HOME_DIR`), that lets us overrride the `/home/tinypilot` default. Unfortunately, we cannot put this variable into [`dev_app_settings.cfg`](https://github.com/tiny-pilot/tinypilot/blob/504e7f113d21f90824acf229240167cafed39b17/dev_app_settings.cfg), since the app config can only be read during a Flask request execution context, but not at initialization time (at least not in a “clean” way). To me, the `env.abs_path_in_home_dir` helper function (as proposed here) wouldn’t be too bad either, though. <a data-ca-tag href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1572"><img src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review on CodeApprove" /></a>
- Loading branch information
1 parent
a2a7bc4
commit e305440
Showing
7 changed files
with
86 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import os | ||
import pathlib | ||
|
||
_TINYPILOT_HOME_PATH = pathlib.Path( | ||
os.environ.get('TINYPILOT_HOME_DIR', '/home/tinypilot')) | ||
|
||
|
||
def abs_path_in_home_dir(relative_path): | ||
"""Resolves the full, absolute path for an object in the tinypilot home dir. | ||
Always use this helper function instead of relying on the ~ alias or the | ||
$HOME environment variable, in order to stay agnostic of the app’s execution | ||
context. | ||
In production, $HOME is always supposed to point to /home/tinypilot, but it | ||
might differ in a local development environment, or when invoking the app | ||
via sudo (e.g., when running a privileged script). In order to avoid | ||
surprising behavior in such scenarios, we have hardcoded the path to be on | ||
the safe side. | ||
Args: | ||
relative_path: The path of a file or folder relative to the tinypilot | ||
home dir, without leading slash (as string). | ||
Raises: | ||
ValueError if input path has leading slash (i.e., is absolute), or if | ||
resolved path would be outside tinypilot home dir. | ||
Returns: | ||
The eventual, absolute path (as string). | ||
""" | ||
if relative_path.startswith('/'): | ||
raise ValueError('Input path must not start with slash.') | ||
target = _TINYPILOT_HOME_PATH.joinpath(relative_path).resolve() | ||
if not target.is_relative_to(_TINYPILOT_HOME_PATH): | ||
raise ValueError('Resolved path must be inside tinypilot home dir.') | ||
return str(target) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import platform | ||
import unittest | ||
|
||
import env | ||
|
||
|
||
# On macOS (“Darwin”), the /home path is a symlink that points to | ||
# /System/Volumes/Data. Since `env.abs_path_in_home_dir` resolves symlinks under | ||
# the hood, the tests would yield false negative results on a macOS environment. | ||
# Therefore, we skip this test case when running on macOS, in order to not break | ||
# the dev setup. | ||
@unittest.skipIf(platform.system() == 'Darwin', 'Not executable on macOS.') | ||
class EnvTest(unittest.TestCase): | ||
|
||
def test_accepts_file_in_home_dir(self): | ||
file = env.abs_path_in_home_dir('file') | ||
self.assertEqual('/home/tinypilot/file', file) | ||
|
||
def test_accepts_folder_in_home_dir(self): | ||
folder = env.abs_path_in_home_dir('folder/') | ||
self.assertEqual('/home/tinypilot/folder', folder) | ||
|
||
def test_accepts_nested_path_within_home_dir(self): | ||
nested_path = env.abs_path_in_home_dir('nested/path') | ||
self.assertEqual('/home/tinypilot/nested/path', nested_path) | ||
|
||
def test_accepts_path_traversal_within_home_dir(self): | ||
nested_path = env.abs_path_in_home_dir('folder/../file') | ||
self.assertEqual('/home/tinypilot/file', nested_path) | ||
|
||
def test_rejects_input_with_leading_slash(self): | ||
with self.assertRaises(ValueError): | ||
env.abs_path_in_home_dir('/foo') | ||
|
||
def test_rejects_path_traversal_outside_home_dir(self): | ||
with self.assertRaises(ValueError): | ||
env.abs_path_in_home_dir('../foo') | ||
|
||
with self.assertRaises(ValueError): | ||
env.abs_path_in_home_dir('foo/../../bar') |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters