From 7251504dc45b2bced017c093247af2e93978fa6e Mon Sep 17 00:00:00 2001 From: Marty Oehme Date: Sat, 29 Nov 2025 18:28:52 +0100 Subject: [PATCH 1/5] ref: Extract note parent dir creation function --- topen.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/topen.py b/topen.py index e04e39e..fae010e 100755 --- a/topen.py +++ b/topen.py @@ -63,9 +63,7 @@ def main(cfg: "TConf | None" = None, io: "_IO | None" = None) -> int: fpath = get_notes_file(uuid, notes_dir=cfg.notes_dir, notes_ext=cfg.notes_ext) - if not fpath.parent.exists(): - fpath.parent.mkdir(parents=True, exist_ok=True) - + _ensure_parent_dir(fpath) io.out(f"Editing note: {fpath}") open_editor(fpath, editor=cfg.notes_editor) @@ -139,6 +137,11 @@ def _real_path(p: Path | str) -> Path: return Path(os.path.expandvars(p)).expanduser() +def _ensure_parent_dir(file: Path) -> None: + if not file.parent.exists(): + file.parent.mkdir(parents=True, exist_ok=True) + + def _determine_default_task_rc() -> Path: if _real_path("~/.taskrc").exists(): return _real_path("~/.taskrc") From 84a16ee307377b1bdac0e20c97aae9a541f24ee5 Mon Sep 17 00:00:00 2001 From: Marty Oehme Date: Sat, 29 Nov 2025 18:31:09 +0100 Subject: [PATCH 2/5] ref: Rename path expansion function From `_real_path` to `_expand_path` to better express its use. --- topen.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/topen.py b/topen.py index fae010e..05c3f16 100755 --- a/topen.py +++ b/topen.py @@ -133,7 +133,7 @@ class Opt: is_flag: bool = False -def _real_path(p: Path | str) -> Path: +def _expand_path(p: Path | str) -> Path: return Path(os.path.expandvars(p)).expanduser() @@ -143,11 +143,11 @@ def _ensure_parent_dir(file: Path) -> None: def _determine_default_task_rc() -> Path: - if _real_path("~/.taskrc").exists(): - return _real_path("~/.taskrc") - if _real_path("$XDG_CONFIG_HOME/task/taskrc").exists(): - return _real_path("$XDG_CONFIG_HOME/task/taskrc") - return _real_path("~/.config/task/taskrc") + if _expand_path("~/.taskrc").exists(): + return _expand_path("~/.taskrc") + if _expand_path("$XDG_CONFIG_HOME/task/taskrc").exists(): + return _expand_path("$XDG_CONFIG_HOME/task/taskrc") + return _expand_path("~/.config/task/taskrc") def _strtobool(val: str) -> bool: @@ -261,11 +261,11 @@ class TConf: """If set topen will give no feedback on note editing.""" def __post_init__(self): - self.task_rc = _real_path(self.task_rc) - self.task_data = _real_path(self.task_data) + self.task_rc = _expand_path(self.task_rc) + self.task_data = _expand_path(self.task_data) if self.notes_dir == NON_EXISTENT_PATH: self.notes_dir = self._default_notes_dir() - self.notes_dir = _real_path(self.notes_dir) + self.notes_dir = _expand_path(self.notes_dir) if not self.notes_editor: self.notes_editor = ( os.getenv("EDITOR") From e84adc439261e65d07e1e20aa3eb4ab66adf4d65 Mon Sep 17 00:00:00 2001 From: Marty Oehme Date: Sat, 29 Nov 2025 18:28:52 +0100 Subject: [PATCH 3/5] fix: Correct whitespace separation on editor shell call Switch to using 'sequence'-delineated arguments given to the subprocess run call to correctly handle whitespace. Also check the output, so we exit if we have an error. Test accordingly. --- test/test_cli.py | 11 +++++++++++ topen.py | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 test/test_cli.py diff --git a/test/test_cli.py b/test/test_cli.py new file mode 100644 index 0000000..fe70abb --- /dev/null +++ b/test/test_cli.py @@ -0,0 +1,11 @@ +from pathlib import Path +from unittest.mock import Mock, patch + +from topen import add_annotation, open_editor + + +def test_open_editor_escapes_shell(): + """Ensure filenames with spaces/metas do not allow shell injection.""" + with patch("subprocess.run") as run_mock: + open_editor(Path("my note$1.txt"), "vim") + run_mock.assert_called_once_with(["vim", "my note$1.txt"], check=True) diff --git a/topen.py b/topen.py index 05c3f16..03f559a 100755 --- a/topen.py +++ b/topen.py @@ -97,7 +97,7 @@ def get_notes_file(uuid: str, notes_dir: Path, notes_ext: str) -> Path: def open_editor(file: Path, editor: str) -> None: """Opens a file with the chosen editor.""" - _ = subprocess.run(f"{editor} {file}", shell=True) + _ = subprocess.run([editor, str(file)], check=True) def is_annotation_missing(task: Task, annotation_content: str) -> bool: From 3ef552bbe53c3c4453f7d02d06643a585a948c51 Mon Sep 17 00:00:00 2001 From: Marty Oehme Date: Sat, 29 Nov 2025 18:42:33 +0100 Subject: [PATCH 4/5] ref: Print optional error message on editor process error Editor function takes an optional io object which is used to print an error output if the subprocess errors. --- test/test_cli.py | 8 ++++++++ topen.py | 10 +++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/test/test_cli.py b/test/test_cli.py index fe70abb..1d9601b 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -9,3 +9,11 @@ def test_open_editor_escapes_shell(): with patch("subprocess.run") as run_mock: open_editor(Path("my note$1.txt"), "vim") run_mock.assert_called_once_with(["vim", "my note$1.txt"], check=True) + + +# +# def test_add_annotation_saves_task(): +# task = Mock() +# add_annotation(task, "hello") +# task.add_annotation.assert_called_once_with("hello") +# task.save.assert_called_once() diff --git a/topen.py b/topen.py index 03f559a..b5374e1 100755 --- a/topen.py +++ b/topen.py @@ -65,7 +65,7 @@ def main(cfg: "TConf | None" = None, io: "_IO | None" = None) -> int: _ensure_parent_dir(fpath) io.out(f"Editing note: {fpath}") - open_editor(fpath, editor=cfg.notes_editor) + open_editor(fpath, editor=cfg.notes_editor, io=io) if fpath.exists(): if is_annotation_missing(task, annotation_content=cfg.notes_annot): @@ -95,9 +95,13 @@ def get_notes_file(uuid: str, notes_dir: Path, notes_ext: str) -> Path: return Path(notes_dir).joinpath(f"{uuid}.{notes_ext}") -def open_editor(file: Path, editor: str) -> None: +def open_editor(file: Path, editor: str, io: "_IO | None" = None) -> None: """Opens a file with the chosen editor.""" - _ = subprocess.run([editor, str(file)], check=True) + try: + _ = subprocess.run([editor, str(file)], check=True) + except subprocess.CalledProcessError: + if io: + io.err("Editor exited with an error, aborting.\n") def is_annotation_missing(task: Task, annotation_content: str) -> bool: From e960f56b9347d4257942af0a745aff52150c1e10 Mon Sep 17 00:00:00 2001 From: Marty Oehme Date: Sat, 29 Nov 2025 18:45:28 +0100 Subject: [PATCH 5/5] test: Add simple happypath annotation adding --- test/test_cli.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/test_cli.py b/test/test_cli.py index 1d9601b..0bca9e7 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -11,9 +11,7 @@ def test_open_editor_escapes_shell(): run_mock.assert_called_once_with(["vim", "my note$1.txt"], check=True) -# -# def test_add_annotation_saves_task(): -# task = Mock() -# add_annotation(task, "hello") -# task.add_annotation.assert_called_once_with("hello") -# task.save.assert_called_once() +def test_add_annotation_calls_tasklib(): + task = Mock() + add_annotation(task, "hello") + task.add_annotation.assert_called_once_with("hello")