Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ All notable changes to Palinode. Format follows [Keep a Changelog](https://keepa

### Added

- `doctor`: new `memory_dir_writable` check. `memory_dir_exists` is critical but only
asserts the path is a directory, so a memory directory on a read-only mount, owned by
another user, or with its mode tightened by hand reported green while every save failed.
The new check tests `os.W_OK | os.X_OK` and carries remediation naming those three
causes. ([#206](https://github.com/phasespace-labs/palinode/issues/206))

### Changed

### Fixed
Expand Down
16 changes: 16 additions & 0 deletions docs/DOCTOR.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ Pure-disk checks. Cheap, no network.
| Check | Severity ceiling | Catches |
|---|---|---|
| `memory_dir_exists` | critical | `PALINODE_DIR` points at a missing or non-directory path |
| `memory_dir_writable` | critical | `memory_dir` exists but the running user cannot create files in it |
| `db_path_resolvable` | error | `db_path` parent missing, or the file is not openable as SQLite |
| `db_path_under_memory_dir` | warn | `db_path` resolves outside `memory_dir` (the rename-drift signature) |
| `phantom_db_files` | critical | One or more `.palinode.db` files exist outside the configured path |
Expand All @@ -58,6 +59,21 @@ Pure-disk checks. Cheap, no network.

Verifies `Path(config.memory_dir)` exists and is a directory. Without it nothing else works, so this is the single critical-severity gate. Failure prints the resolved path and the `mkdir -p` command. **Fixable via `--fix`** (creates the directory).

#### `memory_dir_writable`

Verifies the running user can create files in `config.memory_dir`. A directory that exists
but is not writable passes `memory_dir_exists` and then fails every save, which is what a
read-only mount, an ownership mismatch, or a hand-tightened mode all look like.

Checks `os.W_OK | os.X_OK`, because creating a file inside a directory needs search
permission as well as write. Uses `os.access` rather than writing a probe file, matching
`audit_log_writable`; that is advisory and can disagree with a real write, but doctor should
not leave files in your memory directory to answer a question.

When `memory_dir` is absent or is not a directory the check passes with a message saying it
did not apply. `memory_dir_exists` already reports that case and one cause should not
produce two failures. Not fixable via `--fix`.

#### `db_path_resolvable`

Verifies the configured `db_path` is openable by SQLite in read-only mode and that its parent directory exists. Uses `PRAGMA schema_version` to validate the SQLite header, which catches a non-SQLite file masquerading as the DB. Full `PRAGMA integrity_check` is left to deeper future checks because it can be slow on large stores.
Expand Down
63 changes: 61 additions & 2 deletions palinode/diagnostics/checks/memory_dir.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
"""
Check: memory_dir_exists
Checks: memory_dir_exists, memory_dir_writable

Verifies that the configured memory directory is present on disk.
Verifies that the configured memory directory is present on disk and that
the running user can create files inside it.
Severity: critical — without it nothing works.
"""
from __future__ import annotations

import os
from pathlib import Path

from palinode.diagnostics.registry import register
Expand Down Expand Up @@ -36,3 +38,60 @@ def memory_dir_exists(ctx: DoctorContext) -> CheckResult:
f" mkdir -p {memory_dir}"
),
)


@register(tags=("fast",))
def memory_dir_writable(ctx: DoctorContext) -> CheckResult:
"""Verify that the running user can create files in config.memory_dir.

A memory directory that exists but is not writable passes
``memory_dir_exists`` and then fails every save. Writing a file needs
search permission on the directory as well as write permission, so both
bits are required here.

Uses ``os.access`` rather than a write probe, matching
``audit_log_writable``. It is advisory and can disagree with a real
write, but doctor should not leave files behind in the user's memory
directory to answer a question.
"""
memory_dir = Path(ctx.config.memory_dir).expanduser().resolve()

if not (memory_dir.exists() and memory_dir.is_dir()):
# memory_dir_exists already reports this. One cause, one failure.
return CheckResult(
name="memory_dir_writable",
severity="critical",
passed=True,
message=(
f"Memory directory is absent, so writability does not apply: "
f"{memory_dir}"
),
remediation=None,
)

if os.access(str(memory_dir), os.W_OK | os.X_OK):
return CheckResult(
name="memory_dir_writable",
severity="critical",
passed=True,
message=f"Memory directory is writable: {memory_dir}",
remediation=None,
)

return CheckResult(
name="memory_dir_writable",
severity="critical",
passed=False,
message=(
f"Memory directory exists but is not writable: {memory_dir} "
"(Every save will fail.)"
),
remediation=(
f"Check ownership, whether the path is a read-only mount, and the\n"
f"directory mode. To fix a mode that was tightened by hand:\n"
f" chmod u+wx {memory_dir}\n"
f"To fix an ownership mismatch:\n"
f" chown $(id -u):$(id -g) {memory_dir}\n"
f"Or set PALINODE_DIR to a writable path."
),
)
114 changes: 114 additions & 0 deletions tests/test_doctor_memory_dir_writable.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
"""
Tests for the memory_dir_writable doctor check.

Covers:
- writable directory passes
- existing directory with write permission removed fails, with remediation
- directory with write but no search permission fails
- absent directory returns the informational pass, not a second failure
- a file where the directory should be takes the same informational pass

Real directories under tmp_path, real mode bits. No mocking of os.access.
"""
from __future__ import annotations

import os
import stat
import sys
from pathlib import Path

import pytest

from palinode.core.config import Config
from palinode.diagnostics.runner import run_one
from palinode.diagnostics.types import DoctorContext


def _ctx(memory_dir: Path) -> DoctorContext:
"""Build a DoctorContext pointed at *memory_dir*."""
cfg = Config(
memory_dir=str(memory_dir),
db_path=str(memory_dir / ".palinode.db"),
)
cfg.doctor.search_roots = [str(memory_dir)]
return DoctorContext(config=cfg)


def test_writable_directory_passes(tmp_path: Path) -> None:
memory_dir = tmp_path / "palinode"
memory_dir.mkdir()

result = run_one(_ctx(memory_dir), "memory_dir_writable")

assert result.passed is True
assert result.severity == "critical"
assert result.remediation is None


@pytest.mark.skipif(
sys.platform == "win32",
reason="POSIX mode bits do not govern directory writability on Windows",
)
@pytest.mark.skipif(
hasattr(os, "geteuid") and os.geteuid() == 0,
reason="os.access reports success for root regardless of mode",
)
def test_unwritable_directory_fails_with_remediation(tmp_path: Path) -> None:
memory_dir = tmp_path / "palinode"
memory_dir.mkdir()
original = stat.S_IMODE(memory_dir.stat().st_mode)
memory_dir.chmod(stat.S_IRUSR | stat.S_IXUSR)
try:
result = run_one(_ctx(memory_dir), "memory_dir_writable")
finally:
# Without this, tmp_path cleanup fails on some platforms.
memory_dir.chmod(original)

assert result.passed is False
assert result.severity == "critical"
assert result.remediation is not None
assert str(memory_dir) in result.message


@pytest.mark.skipif(
sys.platform == "win32",
reason="POSIX mode bits do not govern directory writability on Windows",
)
@pytest.mark.skipif(
hasattr(os, "geteuid") and os.geteuid() == 0,
reason="os.access reports success for root regardless of mode",
)
def test_directory_without_search_permission_fails(tmp_path: Path) -> None:
"""Write permission alone is not enough to create a file inside."""
memory_dir = tmp_path / "palinode"
memory_dir.mkdir()
original = stat.S_IMODE(memory_dir.stat().st_mode)
memory_dir.chmod(stat.S_IRUSR | stat.S_IWUSR)
try:
result = run_one(_ctx(memory_dir), "memory_dir_writable")
finally:
memory_dir.chmod(original)

assert result.passed is False


def test_absent_directory_reports_an_informational_pass(tmp_path: Path) -> None:
"""memory_dir_exists owns the absent case, so this one stays quiet."""
memory_dir = tmp_path / "does-not-exist"

result = run_one(_ctx(memory_dir), "memory_dir_writable")

assert result.passed is True
assert result.remediation is None
assert "does not apply" in result.message


def test_file_where_the_directory_should_be_is_not_a_second_failure(
tmp_path: Path,
) -> None:
memory_dir = tmp_path / "palinode"
memory_dir.write_text("not a directory")

result = run_one(_ctx(memory_dir), "memory_dir_writable")

assert result.passed is True
Loading