Skip to content

Commit 2af1e13

Browse files
authored
Merge pull request #562 from dataelement/sara-restrict-workspace-file-delete
fix: restrict workspace file deletion to managers
2 parents df62dc5 + 2e77e0d commit 2af1e13

3 files changed

Lines changed: 102 additions & 3 deletions

File tree

backend/app/api/files.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,21 @@ def _visible_path(agent_id: uuid.UUID, rel_path: str, tenant_id: uuid.UUID | Non
154154
return resolved.path, resolved.relative_root, resolved.is_enterprise
155155

156156

157+
async def _require_agent_file_delete_access(
158+
db: AsyncSession,
159+
current_user: User,
160+
agent_id: uuid.UUID,
161+
) -> None:
162+
"""Allow destructive workspace file operations only for managers/admins."""
163+
_agent, access_level = await check_agent_access(db, current_user, agent_id)
164+
if access_level == "manage" or current_user.role in ("platform_admin", "org_admin", "super_admin"):
165+
return
166+
raise HTTPException(
167+
status_code=status.HTTP_403_FORBIDDEN,
168+
detail="Only agent managers or admins can delete files",
169+
)
170+
171+
157172
@router.get("/", response_model=list[FileInfo])
158173
async def list_files(
159174
agent_id: uuid.UUID,
@@ -656,7 +671,7 @@ async def delete_file(
656671
db: AsyncSession = Depends(get_db),
657672
):
658673
"""Delete a file."""
659-
await check_agent_access(db, current_user, agent_id)
674+
await _require_agent_file_delete_access(db, current_user, agent_id)
660675
if is_focus_file_path(path):
661676
raise HTTPException(
662677
status_code=status.HTTP_410_GONE,

backend/tests/test_files_api.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import uuid
2+
3+
import pytest
4+
from fastapi import HTTPException
5+
6+
from app.api import files as files_api
7+
from app.models.agent import Agent
8+
from app.models.user import User
9+
10+
11+
def make_user(**overrides):
12+
values = {
13+
"id": uuid.uuid4(),
14+
"display_name": "Alice",
15+
"role": "member",
16+
"tenant_id": uuid.uuid4(),
17+
"is_active": True,
18+
}
19+
values.update(overrides)
20+
return User(**values)
21+
22+
23+
def make_agent(creator_id: uuid.UUID, **overrides):
24+
values = {
25+
"id": uuid.uuid4(),
26+
"name": "Ops Bot",
27+
"role_description": "assistant",
28+
"creator_id": creator_id,
29+
"status": "idle",
30+
"agent_type": "native",
31+
}
32+
values.update(overrides)
33+
return Agent(**values)
34+
35+
36+
@pytest.mark.asyncio
37+
async def test_use_access_cannot_delete_agent_workspace_file(monkeypatch, tmp_path):
38+
user = make_user()
39+
agent = make_agent(uuid.uuid4(), tenant_id=user.tenant_id)
40+
workspace_file = tmp_path / str(agent.id) / "workspace" / "important.md"
41+
workspace_file.parent.mkdir(parents=True)
42+
workspace_file.write_text("do not delete", encoding="utf-8")
43+
44+
async def fake_check_agent_access(_db, _current_user, _agent_id):
45+
return agent, "use"
46+
47+
monkeypatch.setattr(files_api.settings, "AGENT_DATA_DIR", str(tmp_path))
48+
monkeypatch.setattr(files_api, "check_agent_access", fake_check_agent_access)
49+
50+
with pytest.raises(HTTPException) as exc:
51+
await files_api.delete_file(
52+
agent_id=agent.id,
53+
path="workspace/important.md",
54+
current_user=user,
55+
db=object(),
56+
)
57+
58+
assert exc.value.status_code == 403
59+
assert workspace_file.exists()
60+
61+
62+
@pytest.mark.asyncio
63+
async def test_manage_access_can_delete_agent_workspace_file(monkeypatch, tmp_path):
64+
user = make_user()
65+
agent = make_agent(user.id, tenant_id=user.tenant_id)
66+
workspace_file = tmp_path / str(agent.id) / "workspace" / "obsolete.md"
67+
workspace_file.parent.mkdir(parents=True)
68+
workspace_file.write_text("delete me", encoding="utf-8")
69+
70+
async def fake_check_agent_access(_db, _current_user, _agent_id):
71+
return agent, "manage"
72+
73+
monkeypatch.setattr(files_api.settings, "AGENT_DATA_DIR", str(tmp_path))
74+
monkeypatch.setattr(files_api, "check_agent_access", fake_check_agent_access)
75+
76+
result = await files_api.delete_file(
77+
agent_id=agent.id,
78+
path="workspace/obsolete.md",
79+
current_user=user,
80+
db=object(),
81+
)
82+
83+
assert result == {"status": "ok", "path": "workspace/obsolete.md"}
84+
assert not workspace_file.exists()

frontend/src/pages/AgentDetail.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6579,7 +6579,7 @@ function AgentDetailInner() {
65796579
• <code>skills/my-skill/SKILL.md</code> — {t('agent.skills.folderFormat', 'Each skill is a folder with a SKILL.md file and optional auxiliary files (scripts/, examples/)')}
65806580
</div>
65816581
</div>
6582-
<FileBrowser api={adapter} rootPath="skills" features={{ newFile: true, edit: true, delete: true, newFolder: true, upload: true, directoryNavigation: true }} title={t('agent.skills.skillFiles')} />
6582+
<FileBrowser api={adapter} rootPath="skills" features={{ newFile: true, edit: true, delete: canManage, newFolder: true, upload: true, directoryNavigation: true }} title={t('agent.skills.skillFiles')} />
65836583

65846584
{/* Browse ClawHub Modal */}
65856585
{showAgentClawhub && (
@@ -6794,7 +6794,7 @@ function AgentDetailInner() {
67946794
upload: (file, path, onProgress) => fileApi.upload(id!, file, path + '/', onProgress),
67956795
downloadUrl: (p) => fileApi.downloadUrl(id!, p),
67966796
};
6797-
return <FileBrowser api={adapter} rootPath="workspace" features={{ upload: true, newFile: true, newFolder: true, edit: true, delete: true, directoryNavigation: true }} />;
6797+
return <FileBrowser api={adapter} rootPath="workspace" features={{ upload: true, newFile: true, newFolder: true, edit: true, delete: canManage, directoryNavigation: true }} />;
67986798
})()
67996799
}
68006800

0 commit comments

Comments
 (0)