From 4b7a17bf3333daa919c384e8af8c296475498ccd Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Fri, 19 Jun 2026 19:22:43 +0000 Subject: [PATCH] stage #4487: read-only memory write 403 (#4480) + v0.51.518 CHANGELOG --- CHANGELOG.md | 6 +++ api/routes.py | 20 ++++++++- tests/test_memory_write_symlink_guard.py | 57 ++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2172497..143250ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] +## [v0.51.518] — 2026-06-19 — Release SC (read-only memory writes report 403, not 500) + +### Fixed + +- **Writing to a read-only memory file now returns an actionable 403 instead of an opaque 500 (#4480).** On a fresh two-container install, `SOUL.md` can land mode `0444` (or on a read-only mounted volume), so saving from the Memory tab raised a `PermissionError`/`EROFS` that bubbled up as a generic 500. `/api/memory/write` now catches those two cases and returns a 403 naming the file, its mode, and a `chmod 644` / fix-ownership hint; any other `OSError` is still re-raised unchanged, and the existing symlinked-target guard is preserved. Thanks @franksong2702. + ## [v0.51.517] — 2026-06-19 — Release SB (multi-container gateway URL config) ### Fixed diff --git a/api/routes.py b/api/routes.py index afcfdff6..64c5c6d9 100644 --- a/api/routes.py +++ b/api/routes.py @@ -5,6 +5,7 @@ Extracted from server.py (Sprint 11) so server.py is a thin shell. import html as _html import copy +import errno import io import gzip import json @@ -17724,7 +17725,24 @@ def _handle_memory_write(handler, body): # (#4217/#4234/#4240). if target.is_symlink(): return bad(handler, "Cannot write to a symlinked memory file") - target.write_text(body["content"], encoding="utf-8") + try: + target.write_text(body["content"], encoding="utf-8") + except OSError as exc: + if not isinstance(exc, PermissionError) and getattr(exc, "errno", None) != errno.EROFS: + raise + mode_hint = "" + try: + mode_hint = f" (mode {target.stat().st_mode & 0o777:o})" + except OSError: + pass + return bad( + handler, + ( + f"{target.name} is not writable{mode_hint}: {target}. " + "Run chmod 644 on the file or fix ownership on the shared volume." + ), + 403, + ) return j(handler, {"ok": True, "section": section, "path": str(target)}) diff --git a/tests/test_memory_write_symlink_guard.py b/tests/test_memory_write_symlink_guard.py index c240bf76..e1b5fddc 100644 --- a/tests/test_memory_write_symlink_guard.py +++ b/tests/test_memory_write_symlink_guard.py @@ -12,6 +12,8 @@ setup); only the concrete target file is guarded. """ import os +import errno +from pathlib import Path import pytest @@ -60,6 +62,61 @@ def test_memory_write_rejects_symlinked_memory_file(tmp_path, monkeypatch): assert outside.read_text(encoding="utf-8") == "important" +def test_memory_write_read_only_soul_returns_403(tmp_path, monkeypatch): + """A read-only SOUL.md write must return an actionable 403, not bubble as 500.""" + home = tmp_path / "home" + home.mkdir() + target = home / "SOUL.md" + target.write_text("original", encoding="utf-8") + original_write_text = Path.write_text + + def fake_write_text(self, *args, **kwargs): + if self == target: + raise PermissionError("read-only test file") + return original_write_text(self, *args, **kwargs) + + monkeypatch.setattr(Path, "write_text", fake_write_text) + + cap = _patch_memory_routes(monkeypatch, home) + routes._handle_memory_write( + _FakeHandler(), + {"section": "soul", "content": "# Soul\n"}, + ) + + assert "bad" in cap, f"expected 403, got {cap}" + assert cap["bad"][1] == 403 + assert "SOUL.md" in cap["bad"][0] + assert "writable" in cap["bad"][0].lower() + assert "chmod 644" in cap["bad"][0] + + +def test_memory_write_read_only_filesystem_returns_403(tmp_path, monkeypatch): + """Docker read-only volume writes can raise EROFS instead of PermissionError.""" + home = tmp_path / "home" + home.mkdir() + target = home / "SOUL.md" + target.write_text("original", encoding="utf-8") + original_write_text = Path.write_text + + def fake_write_text(self, *args, **kwargs): + if self == target: + raise OSError(errno.EROFS, "read-only file system") + return original_write_text(self, *args, **kwargs) + + monkeypatch.setattr(Path, "write_text", fake_write_text) + + cap = _patch_memory_routes(monkeypatch, home) + routes._handle_memory_write( + _FakeHandler(), + {"section": "soul", "content": "# Soul\n"}, + ) + + assert "bad" in cap, f"expected 403, got {cap}" + assert cap["bad"][1] == 403 + assert "SOUL.md" in cap["bad"][0] + assert "chmod 644" in cap["bad"][0] + + def test_memory_write_allows_symlinked_memories_directory(tmp_path, monkeypatch): """A symlinked parent memories directory is allowed (deliberate decision) as long as the concrete target file itself is not a symlink."""