* fix(#3988): default show_cli_sessions to true for new installs, grandfather existing OFF Combines the best of #3997 (rodboev) and #4222 (Sanjays2402): - config.py default flipped True so NEW installs surface CLI/TUI/Telegram/ Discord/Hermes One sessions in the sidebar without hunting for the toggle. - load_settings() backfill grandfathers ESTABLISHED installs OFF: when the saved settings predate the flip (onboarding_completed True + key absent), pin False so an existing user's sidebar doesn't silently change. Fresh installs get True. - Default-true hydration at every read site (config.py / boot.js / panels.js checkbox) via the !==false idiom — no config-ON-but-UI-OFF mismatch (#4006 class). - 10 tests: config default, bool-key round-trip, grandfather matrix (fresh/mid-onboarding/established-absent/explicit-true/explicit-false/ save-preserves-grandfather), and both frontend hydration sites. Co-authored-by: Rod Boev <rod.boev@gmail.com> Co-authored-by: Sanjays2402 <Sanjays2402@users.noreply.github.com> * fix(#4217): guard /api/file/delete and /api/file/rename against symlink targets * chore(release): stamp v0.51.427 (Release ON) for #3988 + #4217 * chore: drop unused imports (ruff F401) in absorbed test files * fix(#4217,#3988): address review — dangling-symlink ordering + boot.js catch fallback Greptile review on #4230 + the related-PR reviews surfaced two fixes on the shipped code: 1. (#4217) _handle_file_delete/_handle_file_rename checked target.exists() BEFORE is_symlink(), so a DANGLING workspace symlink resolved to a missing target -> misclassified as 404 'File not found' and left permanently undeletable/unrenameable. Moved the no-follow is_symlink() guard before the follow-based exists() check in both handlers. +2 dangling-symlink tests. 2. (#3988) boot.js settings-load-FAILURE catch block hardcoded _showCliSessions=false, inconsistent with the new True default (the #4006 catch-block-fallback class). Now defaults true. +1 test pinning the fallback. Both confirmed by the new tests (20/20 in the two files). * fix(#3988,#4217): address Opus review — broaden grandfather signal + move-handler parity Opus #4230 ship-review surfaced 2 LOW findings, both now fixed in-line: 1. (#3988) The grandfather keyed only on onboarding_completed, so a CLI-configured user who tweaked a WebUI setting but never ran the wizard would get flipped ON (and onboarding.py later auto-persisting onboarding_completed=True would make it permanent). Now: established = show_cli_sessions absent AND (onboarding_completed OR any persisted setting other than a falsy onboarding flag). A genuinely new / still-mid-onboarding file still falls through to True. +1 test. 2. (#4217) _handle_file_move still checked exists() before is_symlink() — the same dangling-symlink-404 bug just fixed in delete/rename. Reordered for parity so a dangling symlinked source is rejected 400, not 404. 22 tests across the two files pass; ruff clean. --------- Co-authored-by: nesquena-hermes <agent@nesquena-hermes> Co-authored-by: Rod Boev <rod.boev@gmail.com> Co-authored-by: Sanjays2402 <Sanjays2402@users.noreply.github.com>
253 lines
8.1 KiB
Python
253 lines
8.1 KiB
Python
"""Regression tests for #4217 — symlink guards on /api/file/delete and /api/file/rename."""
|
|
import os
|
|
|
|
import pytest
|
|
|
|
import api.routes as routes
|
|
|
|
|
|
@pytest.fixture()
|
|
def workspace(tmp_path):
|
|
"""Yield a real temporary workspace directory and clean up after."""
|
|
ws = tmp_path / "ws"
|
|
ws.mkdir()
|
|
yield ws
|
|
|
|
|
|
class _FakeHandler:
|
|
pass
|
|
|
|
|
|
def _patch_routes(ws):
|
|
"""Monkey-patch routes helpers so handlers run without an HTTP server."""
|
|
|
|
class _S:
|
|
workspace = str(ws)
|
|
|
|
cap = {}
|
|
orig_j = routes.j
|
|
orig_bad = routes.bad
|
|
orig_get = routes.get_session_for_file_ops
|
|
routes.j = lambda h, o: (cap.__setitem__("ok", o), True)[1]
|
|
routes.bad = lambda h, m, c=400: (cap.__setitem__("bad", (m, c)), True)[1]
|
|
routes.get_session_for_file_ops = lambda sid: _S()
|
|
return cap, (orig_j, orig_bad, orig_get)
|
|
|
|
|
|
def _restore_routes(originals):
|
|
routes.j, routes.bad, routes.get_session_for_file_ops = originals
|
|
|
|
|
|
# ── DELETE symlink guards ────────────────────────────────────────────────
|
|
|
|
def test_delete_workspace_symlink_to_dir_rejected(workspace):
|
|
real_dir = workspace / "real_dir"
|
|
real_dir.mkdir()
|
|
link = workspace / "link_to_dir"
|
|
try:
|
|
os.symlink(str(real_dir), str(link))
|
|
except (OSError, NotImplementedError):
|
|
pytest.skip("platform does not support symlinks")
|
|
|
|
cap, orig = _patch_routes(workspace)
|
|
try:
|
|
routes._handle_file_delete(
|
|
_FakeHandler(),
|
|
{"session_id": "x", "path": "link_to_dir", "recursive": True},
|
|
)
|
|
finally:
|
|
_restore_routes(orig)
|
|
|
|
assert "bad" in cap, f"expected 400, got {cap}"
|
|
assert cap["bad"][1] == 400
|
|
assert "Cannot delete a symlinked entry" in cap["bad"][0]
|
|
assert real_dir.exists(), "real target directory was deleted through the symlink"
|
|
|
|
|
|
def test_delete_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_delete(
|
|
_FakeHandler(),
|
|
{"session_id": "x", "path": "link_to_file.txt"},
|
|
)
|
|
finally:
|
|
_restore_routes(orig)
|
|
|
|
assert "bad" in cap, f"expected 400, got {cap}"
|
|
assert cap["bad"][1] == 400
|
|
assert "Cannot delete a symlinked entry" in cap["bad"][0]
|
|
assert real_file.exists(), "real target file was deleted through the symlink"
|
|
|
|
|
|
# ── RENAME symlink guards ───────────────────────────────────────────────
|
|
|
|
def test_rename_workspace_symlink_to_dir_rejected(workspace):
|
|
real_dir = workspace / "real_dir"
|
|
real_dir.mkdir()
|
|
link = workspace / "link_to_dir"
|
|
try:
|
|
os.symlink(str(real_dir), str(link))
|
|
except (OSError, NotImplementedError):
|
|
pytest.skip("platform does not support symlinks")
|
|
|
|
cap, orig = _patch_routes(workspace)
|
|
try:
|
|
routes._handle_file_rename(
|
|
_FakeHandler(),
|
|
{"session_id": "x", "path": "link_to_dir", "new_name": "renamed"},
|
|
)
|
|
finally:
|
|
_restore_routes(orig)
|
|
|
|
assert "bad" in cap, f"expected 400, got {cap}"
|
|
assert cap["bad"][1] == 400
|
|
assert "Cannot rename a symlinked entry" in cap["bad"][0]
|
|
|
|
|
|
def test_rename_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_rename(
|
|
_FakeHandler(),
|
|
{"session_id": "x", "path": "link_to_file.txt", "new_name": "renamed.txt"},
|
|
)
|
|
finally:
|
|
_restore_routes(orig)
|
|
|
|
assert "bad" in cap, f"expected 400, got {cap}"
|
|
assert cap["bad"][1] == 400
|
|
assert "Cannot rename a symlinked entry" in cap["bad"][0]
|
|
|
|
|
|
# ── Non-symlink operations still work ───────────────────────────────────
|
|
|
|
def test_delete_real_dir_still_works(workspace):
|
|
real_dir = workspace / "deleteme"
|
|
real_dir.mkdir()
|
|
(real_dir / "child.txt").write_text("x")
|
|
|
|
cap, orig = _patch_routes(workspace)
|
|
try:
|
|
routes._handle_file_delete(
|
|
_FakeHandler(),
|
|
{"session_id": "x", "path": "deleteme", "recursive": True},
|
|
)
|
|
finally:
|
|
_restore_routes(orig)
|
|
|
|
assert "ok" in cap, f"expected success, got {cap}"
|
|
assert cap["ok"]["ok"] is True
|
|
assert not real_dir.exists()
|
|
|
|
|
|
def test_rename_real_file_still_works(workspace):
|
|
real_file = workspace / "old_name.txt"
|
|
real_file.write_text("content")
|
|
|
|
cap, orig = _patch_routes(workspace)
|
|
try:
|
|
routes._handle_file_rename(
|
|
_FakeHandler(),
|
|
{"session_id": "x", "path": "old_name.txt", "new_name": "new_name.txt"},
|
|
)
|
|
finally:
|
|
_restore_routes(orig)
|
|
|
|
assert "ok" in cap, f"expected success, got {cap}"
|
|
assert cap["ok"]["ok"] is True
|
|
assert not real_file.exists()
|
|
assert (workspace / "new_name.txt").exists()
|
|
|
|
|
|
# ── Existing move guard pinned ──────────────────────────────────────────
|
|
|
|
def test_move_workspace_symlink_still_rejected(workspace):
|
|
real_file = workspace / "real.txt"
|
|
real_file.write_text("data")
|
|
dest = workspace / "dest"
|
|
dest.mkdir()
|
|
link = workspace / "link.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_move(
|
|
_FakeHandler(),
|
|
{"session_id": "x", "path": "link.txt", "dest_dir": "dest"},
|
|
)
|
|
finally:
|
|
_restore_routes(orig)
|
|
|
|
assert "bad" in cap, f"expected 400, got {cap}"
|
|
assert cap["bad"][1] == 400
|
|
assert "Cannot move a symlinked entry" in cap["bad"][0]
|
|
|
|
|
|
# ── Dangling symlinks: guard must fire BEFORE the exists() 404 (#4230 review) ──
|
|
|
|
def test_delete_dangling_symlink_rejected_not_404(workspace):
|
|
"""A dangling workspace symlink (target removed) must be rejected by the
|
|
symlink guard (400), not misclassified as 404 'File not found' and left
|
|
permanently undeletable. The is_symlink() check must precede exists()."""
|
|
link = workspace / "dangling_del"
|
|
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_delete(
|
|
_FakeHandler(),
|
|
{"session_id": "x", "path": "dangling_del", "recursive": True},
|
|
)
|
|
finally:
|
|
_restore_routes(orig)
|
|
|
|
assert "bad" in cap, f"expected 400, got {cap}"
|
|
assert cap["bad"][1] == 400
|
|
assert "Cannot delete a symlinked entry" in cap["bad"][0]
|
|
|
|
|
|
def test_rename_dangling_symlink_rejected_not_404(workspace):
|
|
"""A dangling workspace symlink must be rejected by the rename guard (400),
|
|
not 404'd before the guard fires."""
|
|
link = workspace / "dangling_ren"
|
|
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_rename(
|
|
_FakeHandler(),
|
|
{"session_id": "x", "path": "dangling_ren", "new_name": "renamed"},
|
|
)
|
|
finally:
|
|
_restore_routes(orig)
|
|
|
|
assert "bad" in cap, f"expected 400, got {cap}"
|
|
assert cap["bad"][1] == 400
|
|
assert "Cannot rename a symlinked entry" in cap["bad"][0]
|