diff --git a/incorporealcms/pages.py b/incorporealcms/pages.py index 93276e9..288f91e 100644 --- a/incorporealcms/pages.py +++ b/incorporealcms/pages.py @@ -19,14 +19,18 @@ bp = Blueprint('pages', __name__, url_prefix='/') @bp.route('/') def display_page(path): """Get the file contents of the requested path and render the file.""" - if is_file_path_actually_dir_path(path): + try: + resolved_path = request_path_to_instance_resource(path) + logger.debug("received request for path '%s', resolved to '%s'", path, resolved_path) + except PermissionError: + abort(400) + except IsADirectoryError: return redirect(f'{path}/', code=301) + except FileNotFoundError: + abort(404) - resolved_path = resolve_page_file(path) - logger.debug("received request for path '%s', resolved to '%s'", path, resolved_path) try: with app.open_instance_resource(resolved_path, 'r') as entry_file: - logger.debug("file '%s' found", resolved_path) parent_navs = generate_parent_navs(path) mtime = datetime.datetime.fromtimestamp(os.path.getmtime(entry_file.name), get_localzone()) entry = entry_file.read() @@ -67,6 +71,45 @@ def render(template_name_or_list, **context): return resp +def request_path_to_instance_resource(path): + """Turn a request URL path to the full page path. + + flask.Flask.open_instance_resource will open a file like /etc/hosts if you tell it to, + which sucks, so we do a lot of work here to make sure we have a valid request to + something inside the pages dir. + """ + # check if the path is allowed + base_dir = os.path.realpath(f'{app.instance_path}/pages/') + resolved_path = os.path.realpath(os.path.join(base_dir, path)) + logger.debug("base_dir: %s, constructed resolved_path: %s", base_dir, resolved_path) + + # bail if the requested real path isn't inside the base directory + if base_dir != os.path.commonpath((base_dir, resolved_path)): + logger.warning("client tried to request a path '%s' outside of the base_dir!", path) + raise PermissionError + + # if this is a file-like requset but actually a directory, redirect the user + if os.path.isdir(resolved_path) and not path.endswith('/'): + logger.info("client requested a path '%s' that is actually a directory", path) + raise IsADirectoryError + + # derive the proper markdown file depending on if this is a dir or file + if os.path.isdir(resolved_path): + absolute_resource = os.path.join(resolved_path, 'index.md') + else: + absolute_resource = f'{resolved_path}.md' + + logger.info("final path = '%s' for request '%s'", absolute_resource, path) + + # does the final file actually exist? + if not os.path.exists(absolute_resource): + logger.warning("requested final path '%s' does not exist!", absolute_resource) + raise FileNotFoundError + + # we checked that the file exists via absolute path, but now we need to give the path relative to instance dir + return absolute_resource.replace(f'{app.instance_path}{os.path.sep}', '') + + def resolve_page_file(path): """Manipulate the request path to find appropriate page file. @@ -81,22 +124,6 @@ def resolve_page_file(path): return path -def is_file_path_actually_dir_path(path): - """Check if requested path which looks like a file is actually a directory. - - If, for example, /foo used to be a file (foo.md) which later became a directory, - foo/, this returns True. Useful for when I make my structure more complicated - than it originally was, or if users are just weird. - """ - if not path.endswith('/'): - logger.debug("requested path '%s' looks like a file", path) - if os.path.isdir(f'{app.instance_path}/pages/{path}'): - logger.debug("...and it's actually a dir") - return True - - return False - - def generate_parent_navs(path): """Create a series of paths/links to navigate up from the given path.""" # derive additional path/location stuff based on path diff --git a/tests/instance/unreachable.md b/tests/instance/unreachable.md new file mode 100644 index 0000000..ae8a9b6 --- /dev/null +++ b/tests/instance/unreachable.md @@ -0,0 +1 @@ +this file exists but the app should not serve it. diff --git a/tests/test_pages.py b/tests/test_pages.py index 1add428..0dcd7bc 100644 --- a/tests/test_pages.py +++ b/tests/test_pages.py @@ -1,7 +1,8 @@ """Unit test helper methods.""" +import pytest from werkzeug.http import dump_cookie -from incorporealcms.pages import generate_parent_navs, is_file_path_actually_dir_path, render, resolve_page_file +from incorporealcms.pages import generate_parent_navs, render, request_path_to_instance_resource, resolve_page_file def test_resolve_page_file_dir_to_index(): @@ -54,30 +55,6 @@ def test_generate_page_navs_subdir_with_title_parsing_real_page(app): ] -def test_is_file_path_actually_dir_path_valid_file_is_yes(app): - """Test that a file request for what's actually a directory is detected as such.""" - with app.app_context(): - assert is_file_path_actually_dir_path('/subdir') - - -def test_is_file_path_actually_dir_path_valid_dir_is_no(app): - """Test that a directory request is still a directory request.""" - with app.app_context(): - assert not is_file_path_actually_dir_path('/subdir/') - - -def test_is_file_path_actually_dir_path_nonsense_file_is_no(app): - """Test that requests to nonsense file-looking paths aren't treated as dirs.""" - with app.app_context(): - assert not is_file_path_actually_dir_path('/antphnathpnthapnthsnthax') - - -def test_is_file_path_actually_dir_path_nonsense_dir_is_no(app): - """Test that a directory request is a directory request even if the dir doesn't exist.""" - with app.app_context(): - assert not is_file_path_actually_dir_path('/antphnathpnthapnthsnthax/') - - def test_render_with_user_dark_theme(app): """Test that a request with the dark theme selected renders the dark theme.""" cookie = dump_cookie("user-style", 'dark') @@ -98,3 +75,73 @@ def test_render_with_no_user_theme(app): with app.test_request_context(): assert b'light.css' in render('base.html').data assert b'dark.css' not in render('base.html').data + + +def test_request_path_to_instance_resource(app): + """Test a normal URL request is transformed into the file path.""" + with app.test_request_context(): + assert request_path_to_instance_resource('index') == 'pages/index.md' + + +def test_request_path_to_instance_resource_direct_file(app): + """Test a normal URL request is transformed into the file path.""" + with app.test_request_context(): + assert request_path_to_instance_resource('no-title') == 'pages/no-title.md' + + +def test_request_path_to_instance_resource_in_subdir(app): + """Test a normal URL request is transformed into the file path.""" + with app.test_request_context(): + assert request_path_to_instance_resource('subdir/page') == 'pages/subdir/page.md' + + +def test_request_path_to_instance_resource_subdir_index(app): + """Test a normal URL request is transformed into the file path.""" + with app.test_request_context(): + assert request_path_to_instance_resource('subdir/') == 'pages/subdir/index.md' + + +def test_request_path_to_instance_resource_relatives_walked(app): + """Test a normal URL request is transformed into the file path.""" + with app.test_request_context(): + assert request_path_to_instance_resource('subdir/more-subdir/../../more-metadata') == 'pages/more-metadata.md' + + +def test_request_path_to_instance_resource_relatives_walked_indexes_work_too(app): + """Test a normal URL request is transformed into the file path.""" + with app.test_request_context(): + assert request_path_to_instance_resource('subdir/more-subdir/../../') == 'pages/index.md' + + +def test_request_path_to_instance_resource_relatives_walked_into_subdirs_also_fine(app): + """Test a normal URL request is transformed into the file path.""" + with app.test_request_context(): + assert request_path_to_instance_resource('subdir/more-subdir/../../subdir/page') == 'pages/subdir/page.md' + + +def test_request_path_to_instance_resource_permission_error_on_ref_above_pages(app): + """Test that attempts to get above the base dir ("/../../foo") fail.""" + with app.test_request_context(): + with pytest.raises(PermissionError): + assert request_path_to_instance_resource('../unreachable') + + +def test_request_path_to_instance_resource_isadirectory_on_file_like_req_for_dir(app): + """Test that a request for e.g. '/foo' when foo is a dir indicate to redirect.""" + with app.test_request_context(): + with pytest.raises(IsADirectoryError): + assert request_path_to_instance_resource('subdir') + + +def test_request_path_to_instance_resource_nonexistant_file_errors(app): + """Test that a request for something not on disk errors.""" + with app.test_request_context(): + with pytest.raises(FileNotFoundError): + assert request_path_to_instance_resource('nthanpthpnh') + + +def test_request_path_to_instance_resource_absolute_file_errors(app): + """Test that a request for something not on disk errors.""" + with app.test_request_context(): + with pytest.raises(PermissionError): + assert request_path_to_instance_resource('/etc/hosts')