Skip to content

Commit 4b098a3

Browse files
Move debug log defaults out of /tmp and reject symlinks (#23)
The default debug_log_path and resolver_debug_log_path were in /tmp, which is world-writable on shared hosts. An attacker who pre-creates a symlink at /tmp/historian-debug.log pointing at e.g. an authorized_keys file could trick Historian (with debug mode on) into overwriting an arbitrary file, because _prepare_private_file opened with O_CREAT and no O_NOFOLLOW/O_EXCL. Two defenses, per the issue: - Move defaults under the XDG data dir (~/.local/share/historian/), which is not world-writable, matching how database_path already works. - Add O_NOFOLLOW to _prepare_private_file so a symlinked target is rejected at open time rather than followed, even for custom paths. Updates config.py, config.example.json, and docs/debugging.md. Adds tests for the new defaults and symlink rejection.
1 parent 9fd1cdb commit 4b098a3

5 files changed

Lines changed: 46 additions & 10 deletions

File tree

docs/debugging.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ Enable unified debugging in `config.json`:
55
```json
66
{
77
"debug_enabled": true,
8-
"debug_log_path": "/tmp/historian-debug.log",
9-
"resolver_debug_log_path": "/tmp/historian-resolver.log",
8+
"debug_log_path": "~/.local/share/historian/debug.log",
9+
"resolver_debug_log_path": "~/.local/share/historian/resolver.log",
1010
"log_level": "INFO"
1111
}
1212
```
@@ -26,8 +26,8 @@ Useful checks:
2626

2727
```console
2828
uv run historian doctor --live
29-
tail -f /tmp/historian-debug.log
30-
less /tmp/historian-resolver.log
29+
tail -f ~/.local/share/historian/debug.log
30+
less ~/.local/share/historian/resolver.log
3131
```
3232

3333
`doctor` reports whether debug mode is enabled and whether both configured paths are writable.

historian/config.example.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
"resolver_api_key": "",
1010
"resolver_include_reasoning": false,
1111
"debug_enabled": false,
12-
"debug_log_path": "/tmp/historian-debug.log",
13-
"resolver_debug_log_path": "/tmp/historian-resolver.log",
12+
"debug_log_path": "~/.local/share/historian/debug.log",
13+
"resolver_debug_log_path": "~/.local/share/historian/resolver.log",
1414
"cli_token_path": "~/.config/historian/cli-token",
1515
"request_timeout_seconds": 60.0,
1616
"resolver_max_retries": 3,

historian/config.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ class Settings:
9898
resolver_api_key: str = ""
9999
resolver_include_reasoning: bool = False
100100
debug_enabled: bool = False
101-
debug_log_path: str = "/tmp/historian-debug.log"
102-
resolver_debug_log_path: str = "/tmp/historian-resolver.log"
101+
debug_log_path: str = str(_xdg_data_home() / "historian" / "debug.log")
102+
resolver_debug_log_path: str = str(_xdg_data_home() / "historian" / "resolver.log")
103103
cli_token_path: str = str(_xdg_config_home() / "historian" / "cli-token")
104104
request_timeout_seconds: float = 60.0
105105
resolver_max_retries: int = 3

historian/debug.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
def _prepare_private_file(path: Path, *, clear: bool) -> None:
2121
path.parent.mkdir(parents=True, exist_ok=True)
22-
flags = os.O_WRONLY | os.O_CREAT | (os.O_TRUNC if clear else os.O_APPEND)
22+
flags = os.O_WRONLY | os.O_CREAT | os.O_NOFOLLOW | (os.O_TRUNC if clear else os.O_APPEND)
2323
descriptor = os.open(path, flags, 0o600)
2424
os.close(descriptor)
2525
os.chmod(path, 0o600)

tests/test_debug.py

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
import logging
55
import os
66

7+
import pytest
8+
79
from historian.app import build_app
810
from historian.cli import main
911
from historian.config import Settings
10-
from historian.debug import QueryTranscript, configure_logging
12+
from historian.debug import QueryTranscript, _prepare_private_file, configure_logging
1113

1214
from conftest import event
1315

@@ -157,3 +159,37 @@ def test_operational_log_uses_metadata_not_event_payload(tmp_path, vesper_manife
157159
assert "event-1" in content
158160
assert "music.playback.started" in content
159161
assert "do-not-store" not in content
162+
163+
164+
def test_default_debug_paths_are_not_in_tmp(monkeypatch) -> None:
165+
"""Default debug log paths must live under the XDG data dir, not /tmp."""
166+
monkeypatch.setenv("XDG_DATA_HOME", "/tmp/xdg-data-fixture")
167+
settings = Settings()
168+
assert not settings.debug_log_path.startswith("/tmp/historian")
169+
assert not settings.resolver_debug_log_path.startswith("/tmp/historian")
170+
assert "/historian/debug.log" in settings.debug_log_path
171+
assert "/historian/resolver.log" in settings.resolver_debug_log_path
172+
173+
174+
def test_prepare_private_file_refuses_symlink(tmp_path) -> None:
175+
"""_prepare_private_file must not follow a pre-existing symlink (O_NOFOLLOW).
176+
177+
A symlink at the target path is an attack vector for overwriting an
178+
arbitrary file; opening it must fail rather than write through it.
179+
"""
180+
real_file = tmp_path / "real-target.txt"
181+
real_file.write_text("original\n", encoding="utf-8")
182+
link = tmp_path / "debug.log"
183+
os.symlink(real_file, link)
184+
with pytest.raises(OSError):
185+
_prepare_private_file(link, clear=True)
186+
# The target the symlink pointed at must be untouched.
187+
assert real_file.read_text(encoding="utf-8") == "original\n"
188+
189+
190+
def test_prepare_private_file_creates_new_file(tmp_path) -> None:
191+
"""A normal (non-symlink) path is created with owner-only permissions."""
192+
target = tmp_path / "debug.log"
193+
_prepare_private_file(target, clear=True)
194+
assert target.is_file()
195+
assert os.stat(target).st_mode & 0o777 == 0o600

0 commit comments

Comments
 (0)