From 83aa96ae829f9dc97382f2a3cb963c04bc8fb036 Mon Sep 17 00:00:00 2001 From: FreeShard Bot Date: Tue, 13 Oct 2026 02:25:32 +0200 Subject: [PATCH] fix(#192): HIGH: a read-write sharee can DELETE the owner's entire cale The Radicale rights plugin now returns deny-delete 'rwd'/'rd' for sharees at every depth (including the calendar itself), so a read-write sharee can no longer delete the owner's calendar. The owner gets 'rw' instead of 'RWDrwd', since D/d are deny bits that blocked owners from deleting their own calendars. Tests that pinned the old strings were inverted to assert the owner can delete and sharees cannot. Refs #192 Co-Authored-By: Claude Opus 5.5 --- radicale/radicale_custom_rights.py | 11 ++--- ...e192_sharee_deny_delete_permission.test.js | 40 +++++++++---------- ..._unauthenticated_calendar_deletion.test.js | 21 +++++----- ...ale_custom_rights_case_sensitivity.test.js | 6 +-- .../python/test_radicale_custom_rights.py | 31 ++++++++++---- 5 files changed, 63 insertions(+), 46 deletions(-) diff --git a/radicale/radicale_custom_rights.py b/radicale/radicale_custom_rights.py index 024918d..e8dccd7 100644 --- a/radicale/radicale_custom_rights.py +++ b/radicale/radicale_custom_rights.py @@ -18,11 +18,12 @@ def authorization(self, user, path): segments = sane_path.split("/") owner_id = segments[0] - # 1. Owner always gets full permissions + # 1. Owner always gets full permissions. "D"/"d" are deny-delete bits + # in Radicale, so the owner must never receive them. if user.lower() == owner_id.lower(): if len(segments) == 1: return "RW" - return "RWDrwd" + return "rw" # 2. Accessing someone else's space if len(segments) < 2: @@ -61,9 +62,9 @@ def authorization(self, user, path): has_write = bool(rw_flag) or role in ('WRITE', 'ADMIN') if role in ('READ', 'FREE_BUSY'): has_write = False - if len(segments) > 2: - return "rwd" if has_write else "rd" - return "rw" if has_write else "r" + # "d" denies deleting the collection itself (Radicale checks it + # against the collection path), so only the owner can delete it. + return "rwd" if has_write else "rd" return "" except Exception: diff --git a/server/test/issue192_sharee_deny_delete_permission.test.js b/server/test/issue192_sharee_deny_delete_permission.test.js index 463bbfd..e0ac324 100644 --- a/server/test/issue192_sharee_deny_delete_permission.test.js +++ b/server/test/issue192_sharee_deny_delete_permission.test.js @@ -2,23 +2,13 @@ const test = require('node:test'); const assert = require('node:assert/strict'); -const fs = require('node:fs'); const path = require('node:path'); const { spawnSync } = require('node:child_process'); const pyRightsPath = path.join(__dirname, '../../radicale/radicale_custom_rights.py'); test('Issue #192 — Read-Write Sharee Deny-Delete Permission Enforcement', async (t) => { - await t.test('1. Static source analysis: radicale_custom_rights.py emits "rw" for collection and "rwd" for item sharee access', () => { - const code = fs.readFileSync(pyRightsPath, 'utf8'); - assert.match( - code, - /return\s+["']rwd["']\s+if\s+has_write\s+else\s+["']rd["']/, - 'radicale_custom_rights.py must emit item level "rwd" for write sharees and "rd" for read sharees' - ); - }); - - await t.test('2. Python unit execution: Rights.authorization returns correct collection vs item permissions', () => { + await t.test('Rights.authorization: sharees carry deny-delete "d" on the collection, the owner never gets "D"/"d"', () => { const pythonScript = ` import sys import os @@ -77,22 +67,32 @@ try: rights = radicale_custom_rights.Rights() - # Owner access check (collection level) + # Radicale 3.4 collection DELETE (app/delete.py, permit_delete_collection=True) + # needs "w" on the collection path and is refused if "d" is on that path. + def collection_delete_allowed(perm): + return 'w' in perm and 'd' not in perm + + # Owner: no deny bits, may delete her own calendar owner_perm = rights.authorization('owner1', 'owner1/cal1') - assert owner_perm == 'RWDrwd', f"Owner expected 'RWDrwd', got {owner_perm!r}" + assert owner_perm == 'rw', f"Owner expected 'rw', got {owner_perm!r}" + assert collection_delete_allowed(owner_perm), f"Owner must be able to delete own calendar, got {owner_perm!r}" + owner_item_perm = rights.authorization('owner1', 'owner1/cal1/item.ics') + assert 'd' not in owner_item_perm and 'D' not in owner_item_perm, f"Owner item expected no deny bits, got {owner_item_perm!r}" - # Read-write sharee access check (collection level must NOT include 'd' or 'D') + # Read-write sharee: may write, must NOT delete the owner's collection rw_sharee_col_perm = rights.authorization('sharee_write', 'owner1/cal1') - assert 'd' not in rw_sharee_col_perm and 'D' not in rw_sharee_col_perm, f"Read-write sharee collection expected 'rw', got {rw_sharee_col_perm!r}" - assert rw_sharee_col_perm == 'rw', f"Read-write sharee collection expected 'rw', got {rw_sharee_col_perm!r}" - - # Read-write sharee access check (item level must include 'd') + assert rw_sharee_col_perm == 'rwd', f"Read-write sharee collection expected 'rwd', got {rw_sharee_col_perm!r}" + assert not collection_delete_allowed(rw_sharee_col_perm), f"Read-write sharee could delete owner's calendar: {rw_sharee_col_perm!r}" rw_sharee_item_perm = rights.authorization('sharee_write', 'owner1/cal1/item.ics') assert rw_sharee_item_perm == 'rwd', f"Read-write sharee item expected 'rwd', got {rw_sharee_item_perm!r}" - # Read-only sharee access check + # Read-only sharee ro_sharee_col_perm = rights.authorization('sharee_read', 'owner1/cal1') - assert ro_sharee_col_perm == 'r', f"Read-only sharee collection expected 'r', got {ro_sharee_col_perm!r}" + assert ro_sharee_col_perm == 'rd', f"Read-only sharee collection expected 'rd', got {ro_sharee_col_perm!r}" + assert not collection_delete_allowed(ro_sharee_col_perm) + + # Stranger + assert rights.authorization('stranger', 'owner1/cal1') == '' print("ALL_PYTHON_RIGHTS_TESTS_PASSED") finally: diff --git a/server/test/issue201_prevent_unauthenticated_calendar_deletion.test.js b/server/test/issue201_prevent_unauthenticated_calendar_deletion.test.js index 1a21df9..769cc5e 100644 --- a/server/test/issue201_prevent_unauthenticated_calendar_deletion.test.js +++ b/server/test/issue201_prevent_unauthenticated_calendar_deletion.test.js @@ -61,7 +61,7 @@ test('Issue #201 — Prevent Unauthenticated & Sharee Permanent Calendar Deletio } }); - await t.test('1. Python unit execution: Rights.authorization returns "RWDrwd" for owner and "rw"/"r" for sharees on collection level', () => { + await t.test('1. Python unit execution: Rights.authorization returns "rw" for owner and deny-delete "rwd"/"rd" for sharees on collection level', () => { const pythonScript = ` import sys import os @@ -117,24 +117,23 @@ try: rights = radicale_custom_rights.Rights() - # Owner access check at collection level (must have 'D' and 'd' to delete collection/items) + # In Radicale "D"/"d" are deny-delete bits (checked against the collection + # path on a collection DELETE). The owner must not carry them; sharees must. owner_perm = rights.authorization('owner1', 'owner1/cal1') - assert 'D' in owner_perm and 'd' in owner_perm, f"Owner expected collection delete permission 'D' and 'd', got {owner_perm!r}" - assert owner_perm == 'RWDrwd', f"Owner expected 'RWDrwd', got {owner_perm!r}" + assert 'D' not in owner_perm and 'd' not in owner_perm, f"Owner must not carry deny-delete bits, got {owner_perm!r}" + assert owner_perm == 'rw', f"Owner expected 'rw', got {owner_perm!r}" - # Write sharee access check at collection level (must NOT contain 'd' or 'D' at collection level) + # Write sharee at collection level must carry 'd' so it cannot delete the collection rw_sharee_col_perm = rights.authorization('sharee_write', 'owner1/cal1') - assert 'd' not in rw_sharee_col_perm and 'D' not in rw_sharee_col_perm, f"Write sharee collection level must NOT contain 'd' or 'D', got {rw_sharee_col_perm!r}" - assert rw_sharee_col_perm == 'rw', f"Write sharee collection level expected 'rw', got {rw_sharee_col_perm!r}" + assert rw_sharee_col_perm == 'rwd', f"Write sharee collection level expected 'rwd', got {rw_sharee_col_perm!r}" - # Write sharee access check at item level (must contain 'd' to delete events inside collection) + # Write sharee at item level keeps write access to events rw_sharee_item_perm = rights.authorization('sharee_write', 'owner1/cal1/event1.ics') - assert 'd' in rw_sharee_item_perm, f"Write sharee item level expected 'd', got {rw_sharee_item_perm!r}" assert rw_sharee_item_perm == 'rwd', f"Write sharee item level expected 'rwd', got {rw_sharee_item_perm!r}" - # Read sharee access check at collection level + # Read sharee at collection level ro_sharee_col_perm = rights.authorization('sharee_read', 'owner1/cal1') - assert ro_sharee_col_perm == 'r', f"Read sharee collection level expected 'r', got {ro_sharee_col_perm!r}" + assert ro_sharee_col_perm == 'rd', f"Read sharee collection level expected 'rd', got {ro_sharee_col_perm!r}" print("ALL_ISSUE201_PYTHON_RIGHTS_TESTS_PASSED") finally: diff --git a/server/test/issue277_radicale_custom_rights_case_sensitivity.test.js b/server/test/issue277_radicale_custom_rights_case_sensitivity.test.js index c05421a..576a10d 100644 --- a/server/test/issue277_radicale_custom_rights_case_sensitivity.test.js +++ b/server/test/issue277_radicale_custom_rights_case_sensitivity.test.js @@ -77,13 +77,13 @@ try: rights = radicale_custom_rights.Rights({}) # 1. Sharee uppercase - assert rights.authorization("USER1", "user2/shared-read") == "r", "USER1 should get read permission" + assert rights.authorization("USER1", "user2/shared-read") == "rd", "USER1 should get read permission" # 2. Owner uppercase in path - assert rights.authorization("user1", "USER2/shared-read") == "r", "user1 should get read permission on USER2 path" + assert rights.authorization("user1", "USER2/shared-read") == "rd", "user1 should get read permission on USER2 path" # 3. Mixed case both - assert rights.authorization("User1", "User2/shared-write") == "rw", "User1 should get read-write permission on User2 path" + assert rights.authorization("User1", "User2/shared-write") == "rwd", "User1 should get read-write permission on User2 path" print("OK") finally: diff --git a/server/test/python/test_radicale_custom_rights.py b/server/test/python/test_radicale_custom_rights.py index aa549d1..07f9daa 100644 --- a/server/test/python/test_radicale_custom_rights.py +++ b/server/test/python/test_radicale_custom_rights.py @@ -31,6 +31,13 @@ def strip_path(path): import radicale_custom_rights + +def collection_delete_allowed(permissions): + """Radicale 3.4 DELETE on a calendar collection (radicale/app/delete.py, + permit_delete_collection=True): requires "w" on the collection path and + is refused when the deny-delete bit "d" is present on that same path.""" + return "w" in permissions and "d" not in permissions + class TestRadicaleCustomRights(unittest.TestCase): def setUp(self): self.temp_dir = tempfile.mkdtemp() @@ -107,8 +114,13 @@ def test_owner_principal_root(self): self.assertEqual(perm_case, "RW") def test_owner_calendar_subpath(self): + # Radicale's "D"/"d" are deny-delete bits: the owner must keep the + # ability to delete her own calendar collection and its items. perm = self.rights.authorization("user1", "user1/my-cal") - self.assertEqual(perm, "RWDrwd") + self.assertEqual(perm, "rw") + self.assertTrue(collection_delete_allowed(perm)) + perm_item = self.rights.authorization("user1", "user1/my-cal/item.ics") + self.assertEqual(perm_item, "rw") def test_other_user_principal_root_access_denied(self): perm = self.rights.authorization("user1", "user2") @@ -116,19 +128,24 @@ def test_other_user_principal_root_access_denied(self): def test_sharee_read_only_access(self): perm = self.rights.authorization("user1", "user2/shared-read") - self.assertEqual(perm, "r") + self.assertEqual(perm, "rd") + self.assertFalse(collection_delete_allowed(perm)) perm_item = self.rights.authorization("user1", "user2/shared-read/item.ics") self.assertEqual(perm_item, "rd") def test_sharee_write_access(self): + # Issue #192: a read-write sharee must not be able to delete the + # owner's calendar collection, so the collection path carries "d". perm = self.rights.authorization("user1", "user2/shared-write") - self.assertEqual(perm, "rw") + self.assertEqual(perm, "rwd") + self.assertFalse(collection_delete_allowed(perm)) perm_item = self.rights.authorization("user1", "user2/shared-write/item.ics") self.assertEqual(perm_item, "rwd") def test_sharee_admin_access(self): perm = self.rights.authorization("user1", "user2/shared-admin") - self.assertEqual(perm, "rw") + self.assertEqual(perm, "rwd") + self.assertFalse(collection_delete_allowed(perm)) perm_item = self.rights.authorization("user1", "user2/shared-admin/item.ics") self.assertEqual(perm_item, "rwd") @@ -148,15 +165,15 @@ def test_missing_db_returns_empty(self): def test_sharee_case_insensitive_access(self): # Case-insensitive sharee username matching perm_upper_sharee = self.rights.authorization("USER1", "user2/shared-read") - self.assertEqual(perm_upper_sharee, "r") + self.assertEqual(perm_upper_sharee, "rd") perm_mixed_sharee = self.rights.authorization("User1", "user2/shared-write/item.ics") self.assertEqual(perm_mixed_sharee, "rwd") # Case-insensitive calendar owner matching in path perm_upper_owner = self.rights.authorization("user1", "USER2/shared-read") - self.assertEqual(perm_upper_owner, "r") + self.assertEqual(perm_upper_owner, "rd") perm_mixed_both = self.rights.authorization("USER1", "User2/shared-write") - self.assertEqual(perm_mixed_both, "rw") + self.assertEqual(perm_mixed_both, "rwd") if __name__ == "__main__": unittest.main()