feat(webdav): add tag-based file exclusion (#710)
Hide sensitive files/folders from the WebDAV MCP tool surface by tagging them with a configured Nextcloud system tag. Defence-in-depth control for users who connect LLMs to accounts holding contracts, medical records, credentials, etc. A new EXCLUDED_TAGS env var (comma-separated tag names, empty by default) gates an exclusion layer that runs at the start of every WebDAV tool call: tag names are resolved to tag IDs, those IDs are expanded to the set of tagged paths, then listings/searches are filtered and read/write/delete/move/copy operations on excluded paths raise ToolError. Tagged folders exclude their descendants via prefix match. Empty EXCLUDED_TAGS disables the feature entirely. The threat model is preventing accidental data exfiltration via the LLM tool surface — not hiding files from a determined operator. The docs explicitly recommend creating exclusion tags with user_assignable=false so the credentials the MCP server uses cannot remove the tag. Implementation: - config.py: add `excluded_tags` to _DEFAULTS, Settings, and the _field_map alongside other comma-separated env vars. - client/webdav.py: get_files_by_tag now requests <d:resourcetype/> and surfaces is_directory so tagged directories can recursively exclude descendants. - server/tag_exclusion.py (new): get_excluded_tag_names, get_excluded_file_paths, is_path_excluded. - server/webdav.py: exclusion guards in all 11 WebDAV tools; read/write/create/delete/move/copy raise ToolError, list/search tools silently filter excluded entries. Existing f-string log calls converted to lazy %-style. - tests: 17 new unit tests covering path-matching edge cases (shared-prefix non-match, descendants of excluded dirs), tag-name parsing, and get_excluded_file_paths with mocked WebDAV; 1 new client test asserting <d:resourcetype/> -> is_directory parsing. - docs/configuration.md: new "Tag-Based File Exclusion" section with per-tool effect table, security guidance, and per-call cost note. - README.md: feature mention under Key Features. Closes #710. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
b23f7d9534
commit
22ed9e99a0
@@ -354,3 +354,63 @@ async def test_remove_tag_from_file_not_assigned(mocker):
|
||||
|
||||
# Verify result (should succeed even with 404)
|
||||
assert result is True
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_get_files_by_tag_detects_directories(mocker):
|
||||
"""get_files_by_tag must flag tagged folders via <d:resourcetype/>.
|
||||
|
||||
Tagged folders need ``is_directory=True`` so the tag-exclusion layer
|
||||
(issue #710) can hide their descendants.
|
||||
"""
|
||||
mock_http_client = AsyncMock()
|
||||
client = WebDAVClient(mock_http_client, "testuser")
|
||||
|
||||
# Two-entry response: one regular file, one collection (folder).
|
||||
xml_content = b"""<?xml version="1.0"?>
|
||||
<d:multistatus xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
|
||||
<d:response>
|
||||
<d:href>/remote.php/dav/files/testuser/Secret.txt</d:href>
|
||||
<d:propstat>
|
||||
<d:prop>
|
||||
<oc:fileid>101</oc:fileid>
|
||||
<d:displayname>Secret.txt</d:displayname>
|
||||
<d:getcontentlength>42</d:getcontentlength>
|
||||
<d:getcontenttype>text/plain</d:getcontenttype>
|
||||
<d:getlastmodified>Wed, 01 Jan 2025 00:00:00 GMT</d:getlastmodified>
|
||||
<d:getetag>"abc"</d:getetag>
|
||||
<d:resourcetype/>
|
||||
</d:prop>
|
||||
</d:propstat>
|
||||
</d:response>
|
||||
<d:response>
|
||||
<d:href>/remote.php/dav/files/testuser/Private/</d:href>
|
||||
<d:propstat>
|
||||
<d:prop>
|
||||
<oc:fileid>102</oc:fileid>
|
||||
<d:displayname>Private</d:displayname>
|
||||
<d:getlastmodified>Wed, 01 Jan 2025 00:00:00 GMT</d:getlastmodified>
|
||||
<d:getetag>"def"</d:getetag>
|
||||
<d:resourcetype><d:collection/></d:resourcetype>
|
||||
</d:prop>
|
||||
</d:propstat>
|
||||
</d:response>
|
||||
</d:multistatus>"""
|
||||
|
||||
mock_response = AsyncMock()
|
||||
mock_response.content = xml_content
|
||||
mock_response.raise_for_status = mocker.Mock()
|
||||
mock_http_client.request = AsyncMock(return_value=mock_response)
|
||||
|
||||
files = await client.get_files_by_tag(42)
|
||||
|
||||
assert len(files) == 2
|
||||
by_path = {f["path"]: f for f in files}
|
||||
|
||||
assert by_path["/Secret.txt"]["is_directory"] is False
|
||||
assert by_path["/Private/"]["is_directory"] is True
|
||||
|
||||
# Sanity-check the REPORT body asks for resourcetype.
|
||||
call_args = mock_http_client.request.call_args
|
||||
assert "<d:resourcetype/>" in call_args.kwargs["content"]
|
||||
assert "<oc:systemtag>42</oc:systemtag>" in call_args.kwargs["content"]
|
||||
|
||||
@@ -0,0 +1,152 @@
|
||||
"""Unit tests for tag-based file exclusion (issue #710)."""
|
||||
|
||||
import os
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from nextcloud_mcp_server.config import _reload_config
|
||||
from nextcloud_mcp_server.server.tag_exclusion import (
|
||||
_normalise_path,
|
||||
get_excluded_file_paths,
|
||||
get_excluded_tag_names,
|
||||
is_path_excluded,
|
||||
)
|
||||
|
||||
|
||||
class TestNormalisePath:
|
||||
@pytest.mark.unit
|
||||
def test_strips_leading_and_trailing_slash(self):
|
||||
assert _normalise_path("/foo/bar/") == "foo/bar"
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_unchanged_when_already_clean(self):
|
||||
assert _normalise_path("foo/bar") == "foo/bar"
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_empty_string(self):
|
||||
assert _normalise_path("") == ""
|
||||
|
||||
|
||||
class TestIsPathExcluded:
|
||||
@pytest.mark.unit
|
||||
def test_empty_set_excludes_nothing(self):
|
||||
assert is_path_excluded("/anything", set()) is False
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_direct_match(self):
|
||||
assert is_path_excluded("/Secret.txt", {"Secret.txt"}) is True
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_path_argument_is_normalised(self):
|
||||
# The path argument is normalised before comparison; the excluded
|
||||
# set is expected to already contain normalised entries (it always
|
||||
# is, in practice, because get_excluded_file_paths builds it).
|
||||
assert is_path_excluded("/Secret.txt/", {"Secret.txt"}) is True
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_descendant_of_excluded_directory(self):
|
||||
assert is_path_excluded("/Private/notes.md", {"Private"}) is True
|
||||
assert is_path_excluded("/Private/sub/file.txt", {"Private"}) is True
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_unrelated_path_not_excluded(self):
|
||||
assert is_path_excluded("/Public/notes.md", {"Private"}) is False
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_shared_prefix_is_not_a_match(self):
|
||||
# 'foobar' must NOT be excluded just because 'foo' is.
|
||||
# This is the bug a naive `startswith(exc)` would have.
|
||||
assert is_path_excluded("/foobar/x", {"foo"}) is False
|
||||
assert is_path_excluded("/foobar", {"foo"}) is False
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_excluded_path_itself(self):
|
||||
# The excluded entry itself is excluded (not just its descendants).
|
||||
assert is_path_excluded("/Private", {"Private"}) is True
|
||||
|
||||
|
||||
class TestGetExcludedTagNames:
|
||||
@pytest.mark.unit
|
||||
@patch.dict(os.environ, {"EXCLUDED_TAGS": ""}, clear=False)
|
||||
def test_empty_returns_empty_list(self):
|
||||
_reload_config()
|
||||
assert get_excluded_tag_names() == []
|
||||
|
||||
@pytest.mark.unit
|
||||
@patch.dict(os.environ, {"EXCLUDED_TAGS": "secret"}, clear=False)
|
||||
def test_single_tag(self):
|
||||
_reload_config()
|
||||
assert get_excluded_tag_names() == ["secret"]
|
||||
|
||||
@pytest.mark.unit
|
||||
@patch.dict(os.environ, {"EXCLUDED_TAGS": " a , b , c "}, clear=False)
|
||||
def test_strips_whitespace_around_each_tag(self):
|
||||
_reload_config()
|
||||
assert get_excluded_tag_names() == ["a", "b", "c"]
|
||||
|
||||
@pytest.mark.unit
|
||||
@patch.dict(os.environ, {"EXCLUDED_TAGS": "a,,b,"}, clear=False)
|
||||
def test_skips_empty_entries(self):
|
||||
_reload_config()
|
||||
assert get_excluded_tag_names() == ["a", "b"]
|
||||
|
||||
|
||||
class TestGetExcludedFilePaths:
|
||||
@pytest.mark.unit
|
||||
async def test_returns_empty_set_when_feature_disabled(self, mocker):
|
||||
mocker.patch(
|
||||
"nextcloud_mcp_server.server.tag_exclusion.get_excluded_tag_names",
|
||||
return_value=[],
|
||||
)
|
||||
webdav = AsyncMock()
|
||||
result = await get_excluded_file_paths(webdav)
|
||||
assert result == set()
|
||||
webdav.get_tag_by_name.assert_not_called()
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_skips_unknown_tag(self, mocker):
|
||||
mocker.patch(
|
||||
"nextcloud_mcp_server.server.tag_exclusion.get_excluded_tag_names",
|
||||
return_value=["does-not-exist"],
|
||||
)
|
||||
webdav = AsyncMock()
|
||||
webdav.get_tag_by_name = AsyncMock(return_value=None)
|
||||
|
||||
result = await get_excluded_file_paths(webdav)
|
||||
|
||||
assert result == set()
|
||||
webdav.get_tag_by_name.assert_awaited_once_with("does-not-exist")
|
||||
webdav.get_files_by_tag.assert_not_called()
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_collects_paths_from_multiple_tags(self, mocker):
|
||||
mocker.patch(
|
||||
"nextcloud_mcp_server.server.tag_exclusion.get_excluded_tag_names",
|
||||
return_value=["secret", "no-ai"],
|
||||
)
|
||||
webdav = AsyncMock()
|
||||
webdav.get_tag_by_name = AsyncMock(
|
||||
side_effect=[
|
||||
{"id": 1, "name": "secret"},
|
||||
{"id": 2, "name": "no-ai"},
|
||||
]
|
||||
)
|
||||
webdav.get_files_by_tag = AsyncMock(
|
||||
side_effect=[
|
||||
[
|
||||
{"path": "/Secret.txt", "is_directory": False},
|
||||
{"path": "/Private/", "is_directory": True},
|
||||
],
|
||||
[
|
||||
# Same dir under a second tag — set dedupes it.
|
||||
{"path": "/Private", "is_directory": True},
|
||||
{"path": "/Other/notes.md", "is_directory": False},
|
||||
],
|
||||
]
|
||||
)
|
||||
|
||||
result = await get_excluded_file_paths(webdav)
|
||||
|
||||
assert result == {"Secret.txt", "Private", "Other/notes.md"}
|
||||
assert webdav.get_files_by_tag.await_count == 2
|
||||
Reference in New Issue
Block a user