From: Adam Dullage <redacted>
Date: Tue, 10 May 2022 12:34:20 +0000 (+0100)
Subject: Filename Validation Improvements
X-Git-Url: http://git.99rst.org/?a=commitdiff_plain;h=0675c2b2ae2c62c08e293577d066b8d9136d5b53;p=flatnotes.git

Filename Validation Improvements
---

diff --git a/flatnotes/error_responses.py b/flatnotes/error_responses.py
index 1363dcc..bcf452d 100644
--- a/flatnotes/error_responses.py
+++ b/flatnotes/error_responses.py
@@ -5,11 +5,11 @@ file_exists_response = JSONResponse(
     status_code=409,
 )
 
-filename_contains_path_response = JSONResponse(
+invalid_filename_response = JSONResponse(
     content={
-        "message": "The specified filename contains path information which is forbidden."
+        "message": "The specified filename contains invalid characters."
     },
-    status_code=403,
+    status_code=400,
 )
 
 file_not_found_response = JSONResponse(
diff --git a/flatnotes/flatnotes.py b/flatnotes/flatnotes.py
index 6cff52d..9e8ca21 100644
--- a/flatnotes/flatnotes.py
+++ b/flatnotes/flatnotes.py
@@ -12,8 +12,8 @@ from whoosh.qparser import MultifieldParser
 from whoosh.searching import Hit
 
 
-class FilenameContainsPathError(Exception):
-    def __init__(self, message="Specified filename contains path information"):
+class InvalidFilenameError(Exception):
+    def __init__(self, message="The specified filename is invalid"):
         self.message = message
         super().__init__(self.message)
 
@@ -29,8 +29,8 @@ class Note:
     def __init__(
         self, flatnotes: "Flatnotes", filename: str, new: bool = False
     ) -> None:
-        if not self._is_path_safe(filename):
-            raise FilenameContainsPathError
+        if not self._is_valid_filename(filename):
+            raise InvalidFilenameError
         self._flatnotes = flatnotes
         self._filename = filename
         if new and os.path.exists(self.filepath):
@@ -57,8 +57,8 @@ class Note:
 
     @filename.setter
     def filename(self, new_filename):
-        if not self._is_path_safe(new_filename):
-            raise FilenameContainsPathError
+        if not self._is_valid_filename(new_filename):
+            raise InvalidFilenameError
         new_filepath = os.path.join(self._flatnotes.dir, new_filename)
         os.rename(self.filepath, new_filepath)
         self._filename = new_filename
@@ -79,10 +79,13 @@ class Note:
         os.remove(self.filepath)
 
     # Functions
-    def _is_path_safe(self, filename: str) -> bool:
-        """Return False if the declared filename contains path
-        information e.g. '../note.md' or 'folder/note.md'."""
-        return os.path.split(filename)[0] == ""
+    def _is_valid_filename(self, filename: str) -> bool:
+        r"""Return False if the declared filename contains any of the following
+        characters: <>:"/\|?*"""
+        invalid_chars = r'<>:"/\|?*'
+        return not any(
+            invalid_char in filename for invalid_char in invalid_chars
+        )
 
 
 class NoteHit(Note):
diff --git a/flatnotes/main.py b/flatnotes/main.py
index 7d7ea21..409db0b 100644
--- a/flatnotes/main.py
+++ b/flatnotes/main.py
@@ -11,14 +11,14 @@ from auth import (
 from error_responses import (
     file_exists_response,
     file_not_found_response,
-    filename_contains_path_response,
+    invalid_filename_response,
 )
 from fastapi import Depends, FastAPI, HTTPException
 from fastapi.responses import HTMLResponse
 from fastapi.staticfiles import StaticFiles
 from models import LoginModel, NoteHitModel, NoteModel, NotePatchModel
 
-from flatnotes import FilenameContainsPathError, Flatnotes, Note
+from flatnotes import InvalidFilenameError, Flatnotes, Note
 
 logging.basicConfig(
     format="%(asctime)s [%(levelname)s]: %(message)s",
@@ -72,8 +72,8 @@ async def post_note(data: NoteModel, _: str = Depends(validate_token)):
         note = Note(flatnotes, data.filename, new=True)
         note.content = data.content
         return NoteModel.dump(note, include_content=True)
-    except FilenameContainsPathError:
-        return filename_contains_path_response
+    except InvalidFilenameError:
+        return invalid_filename_response
     except FileExistsError:
         return file_exists_response
 
@@ -88,8 +88,8 @@ async def get_note(
     try:
         note = Note(flatnotes, filename)
         return NoteModel.dump(note, include_content=include_content)
-    except FilenameContainsPathError:
-        return filename_contains_path_response
+    except InvalidFilenameError:
+        return invalid_filename_response
     except FileNotFoundError:
         return file_not_found_response
 
@@ -105,8 +105,8 @@ async def patch_note(
         if new_data.new_content is not None:
             note.content = new_data.new_content
         return NoteModel.dump(note, include_content=True)
-    except FilenameContainsPathError:
-        return filename_contains_path_response
+    except InvalidFilenameError:
+        return invalid_filename_response
     except FileNotFoundError:
         return file_not_found_response
 
@@ -116,8 +116,8 @@ async def delete_note(filename: str, _: str = Depends(validate_token)):
     try:
         note = Note(flatnotes, filename)
         note.delete()
-    except FilenameContainsPathError:
-        return filename_contains_path_response
+    except InvalidFilenameError:
+        return invalid_filename_response
     except FileNotFoundError:
         return file_not_found_response