diff --git a/CHANGELOG.md b/CHANGELOG.md index 95f4b202..84bcad82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] +## [v0.51.433] — 2026-06-15 — Release OT (reject symlinked entries in /api/file/save, #4234) + +### Fixed + +- **Saving to a workspace symlink no longer writes through to the symlink's target.** `/api/file/save` resolved the final symlink before writing, so saving to a workspace symlink `link.txt → real.txt` overwrote `real.txt` instead of the link. It now rejects a symlinked path with a 400 (checked before the existence probe, so dangling symlinks are caught too) — completing the symlink hardening from #4217 (delete/rename/move) for the write path. `safe_resolve` already blocked out-of-workspace targets, so this closes the in-workspace overwrite-through-symlink case. (#4234) + ## [v0.51.432] — 2026-06-15 — Release OS (TUI sessions discoverable in the sidebar) ### Fixed diff --git a/api/routes.py b/api/routes.py index 23888603..10de8e9c 100644 --- a/api/routes.py +++ b/api/routes.py @@ -14824,6 +14824,8 @@ def _handle_file_save(handler, body): try: ws_root = Path(s.workspace) target = safe_resolve(ws_root, body["path"]) + if (ws_root / body["path"]).is_symlink(): + return bad(handler, "Cannot save to a symlinked entry") if not target.exists(): return bad(handler, "File not found", 404) if target.is_dir(): diff --git a/tests/test_workspace_symlink_delete_rename.py b/tests/test_workspace_symlink_delete_rename.py index ba1d294f..208ee742 100644 --- a/tests/test_workspace_symlink_delete_rename.py +++ b/tests/test_workspace_symlink_delete_rename.py @@ -138,6 +138,51 @@ def test_rename_workspace_symlink_to_file_rejected(workspace): # ── Non-symlink operations still work ─────────────────────────────────── +def test_save_workspace_symlink_to_file_rejected(workspace): + real_file = workspace / "real_file.txt" + real_file.write_text("important") + link = workspace / "link_to_file.txt" + try: + os.symlink(str(real_file), str(link)) + except (OSError, NotImplementedError): + pytest.skip("platform does not support symlinks") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_save( + _FakeHandler(), + {"session_id": "x", "path": "link_to_file.txt", "content": "changed"}, + ) + finally: + _restore_routes(orig) + + assert "bad" in cap, f"expected 400, got {cap}" + assert cap["bad"][1] == 400 + assert "Cannot save to a symlinked entry" in cap["bad"][0] + assert real_file.read_text() == "important" + + +def test_save_dangling_symlink_rejected_not_404(workspace): + link = workspace / "dangling_save" + try: + os.symlink(str(workspace / "gone_target"), str(link)) + except (OSError, NotImplementedError): + pytest.skip("platform does not support symlinks") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_save( + _FakeHandler(), + {"session_id": "x", "path": "dangling_save", "content": "changed"}, + ) + finally: + _restore_routes(orig) + + assert "bad" in cap, f"expected 400, got {cap}" + assert cap["bad"][1] == 400 + assert "Cannot save to a symlinked entry" in cap["bad"][0] + + def test_delete_real_dir_still_works(workspace): real_dir = workspace / "deleteme" real_dir.mkdir() @@ -178,6 +223,25 @@ def test_rename_real_file_still_works(workspace): # ── Existing move guard pinned ────────────────────────────────────────── +def test_save_real_file_still_works(workspace): + real_file = workspace / "save_me.txt" + real_file.write_text("old") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_save( + _FakeHandler(), + {"session_id": "x", "path": "save_me.txt", "content": "new"}, + ) + finally: + _restore_routes(orig) + + assert "ok" in cap, f"expected success, got {cap}" + assert cap["ok"]["ok"] is True + assert cap["ok"]["size"] == len("new") + assert real_file.read_text() == "new" + + def test_move_workspace_symlink_still_rejected(workspace): real_file = workspace / "real.txt" real_file.write_text("data")