diff --git a/README.md b/README.md index 906351e..3997ecf 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ Or ask your agent to use a skill: Vault organization requires the Obsidian CLI. In Obsidian, enable **Settings > General > Command line interface**, **Use [[Wikilinks]]**, and **Automatically update internal links**. Put naming conventions in your vault's -`CLAUDE.md` and configure [managed zones](docs/configuration.md#vault-configuration). +`CLAUDE.md` and configure [managed zones](docs/configuration.md#shared-preferences-and-host-locations). Maintenance tools ship in the CLI: run `obsidian-knowledge garden --help`. See [vault maintenance](docs/CLI.md#vault-maintenance) for audit and repair commands. diff --git a/docs/MCP.md b/docs/MCP.md index 1488621..feccf81 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -52,6 +52,12 @@ sending a token across a network. Tunnel provisioning is separate from this serv overwriting. Parent directories are created as needed. New Markdown notes apply the declarative [configured creation defaults](configuration.md#note-creation) before writing; replacements never rerun them. No running app is required. + Replacements preserve the existing UID, GID and permission bits on the + temporary inode before publication. If ownership cannot be preserved, the + write fails and leaves the original file intact. New files use the writer's + identity and the destination directory's group inheritance; shared-vault + operators must configure those permissions rather than relying on a package + default group. This does not copy extended ACLs or other extended attributes. - `vault_patch(path, old_str, new_str, expected_sha256=None)` replaces exactly one occurrence of `old_str` in a UTF-8 file, including frontmatter. Empty, missing, and ambiguous matches (including overlaps) are rejected. Untouched diff --git a/lib/vault_index/vault_files.py b/lib/vault_index/vault_files.py index 4da7497..d6349ed 100644 --- a/lib/vault_index/vault_files.py +++ b/lib/vault_index/vault_files.py @@ -73,10 +73,14 @@ def write_vault_file( target.parent.mkdir(parents=True, exist_ok=True) target = resolve_vault_file(vault_root, relative_path) - mode = stat.S_IMODE(target.stat().st_mode) if target.exists() else 0o644 + original_stat = target.stat() if target.exists() else None + mode = stat.S_IMODE(original_stat.st_mode) if original_stat is not None else 0o644 fd, temp_name = tempfile.mkstemp(prefix=f".{target.name}.", suffix=".tmp", dir=target.parent) temp = Path(temp_name) try: + # NOTE: docs/MCP.md, vault_write: retain existing access ownership before publication. + if original_stat is not None: + os.fchown(fd, original_stat.st_uid, original_stat.st_gid) os.fchmod(fd, mode) with os.fdopen(fd, "wb") as stream: fd = -1 diff --git a/tests/conftest.py b/tests/conftest.py index 2e456d8..e5ee3a3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,8 +28,11 @@ @pytest.fixture(autouse=True) def _isolated_preferences(tmp_path, monkeypatch): - """Never read or mutate the operator's shared preferences during tests.""" + """Isolate preferences and clear ambient vault registry overrides.""" monkeypatch.setenv("OBSIDIAN_KNOWLEDGE_CONFIG", str(tmp_path / "preferences.yaml")) + # Tests exercise HOME-based discovery; caller overrides must not supersede it. + monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) + monkeypatch.delenv("OBSIDIAN_KNOWLEDGE_VAULTS_CONFIG", raising=False) @pytest.fixture(autouse=True) diff --git a/tests/test_vault_file_ownership.py b/tests/test_vault_file_ownership.py new file mode 100644 index 0000000..371ca4e --- /dev/null +++ b/tests/test_vault_file_ownership.py @@ -0,0 +1,60 @@ +"""Atomic replacement must preserve the existing note's access ownership.""" + +from __future__ import annotations + +import os +import stat +from pathlib import Path + +import pytest + +from lib.vault_index.vault_files import write_vault_file + + +def test_replacement_applies_original_owner_before_mode_and_publication(tmp_path, monkeypatch): + target = tmp_path / "note.txt" + target.write_bytes(b"original") + target.chmod(0o660) + before = target.stat() + calls = [] + chown = os.fchown + chmod = os.fchmod + + def preserve_owner(fd, uid, gid): + assert target.read_bytes() == b"original" + calls.append(("owner", uid, gid)) + chown(fd, uid, gid) + + def preserve_mode(fd, mode): + calls.append(("mode", mode)) + chmod(fd, mode) + + monkeypatch.setattr(os, "fchown", preserve_owner) + monkeypatch.setattr(os, "fchmod", preserve_mode) + write_vault_file(tmp_path, Path("note.txt"), b"replacement", replace=True) + after = target.stat() + assert calls == [("owner", before.st_uid, before.st_gid), ("mode", 0o660)] + assert (after.st_uid, after.st_gid, stat.S_IMODE(after.st_mode)) == (before.st_uid, before.st_gid, 0o660) + assert target.read_bytes() == b"replacement" + + +def test_ownership_failure_preserves_original_inode_and_removes_temporary(tmp_path, monkeypatch): + target = tmp_path / "note.txt" + target.write_bytes(b"original") + before = target.stat() + + def denied(fd, uid, gid): + raise PermissionError("ownership preservation denied") + + monkeypatch.setattr(os, "fchown", denied) + with pytest.raises(PermissionError, match="ownership preservation denied"): + write_vault_file(tmp_path, Path("note.txt"), b"replacement", replace=True) + after = target.stat() + assert (after.st_ino, after.st_uid, after.st_gid, after.st_mode) == ( + before.st_ino, + before.st_uid, + before.st_gid, + before.st_mode, + ) + assert target.read_bytes() == b"original" + assert list(tmp_path.iterdir()) == [target]