fix: disable workspace skills in group sessions#9082
Conversation
dosubot
Bot
added
size:XS
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Instead of directly checking
not event.get_group_id(), consider centralizing group-session detection behind a helper (e.g.,event.is_group_session()or similar) so this privilege boundary logic stays aligned with other parts of the codebase that distinguish group vs. non-group contexts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of directly checking `not event.get_group_id()`, consider centralizing group-session detection behind a helper (e.g., `event.is_group_session()` or similar) so this privilege boundary logic stays aligned with other parts of the codebase that distinguish group vs. non-group contexts.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request disables workspace-scoped skills in group sessions when running on a local runtime to prevent privilege-boundary vulnerabilities, and adds a corresponding unit test. The review feedback suggests extending this security measure to also disable loading EXTRA_PROMPT.md in group sessions to prevent potential prompt injection attacks. Additionally, it is recommended to enhance the new unit test to verify that global-scoped skills are still correctly loaded and not overridden by workspace-scoped skills.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| workspace_skills = ( | ||
| skill_manager.list_workspace_skills( | ||
| workspace_skills = [] | ||
| if runtime == "local" and not event.get_group_id(): |
There was a problem hiding this comment.
While disabling workspace-scoped skills in group sessions is an excellent security improvement to prevent privilege-boundary vulnerabilities, a similar vulnerability exists with EXTRA_PROMPT.md.
In _apply_workspace_extra_prompt (called during _decorate_llm_request), the file EXTRA_PROMPT.md is loaded from the workspace path and appended to the system prompt. Since group sessions share the same workspace directory (derived from event.unified_msg_origin), any non-admin user in the group could plant or modify EXTRA_PROMPT.md to perform a prompt injection attack against other users (including admins) in the same group.
To fully mitigate this privilege-boundary vulnerability, please consider also disabling EXTRA_PROMPT.md loading for group sessions. For example, you can update _apply_workspace_extra_prompt as follows:
def _apply_workspace_extra_prompt(
event: AstrMessageEvent,
req: ProviderRequest,
) -> None:
if event.get_group_id():
return
extra_prompt_path = _get_workspace_path_for_umo(event.unified_msg_origin) / (
"EXTRA_PROMPT.md"
)
# ... rest of the function| workspace_skill_dir = workspace_root / "skills" / "workspace-skill" | ||
| workspace_skill_dir.mkdir(parents=True) | ||
| workspace_skill_dir.joinpath("SKILL.md").write_text( | ||
| "---\ndescription: Workspace scoped skill.\n---\n", | ||
| encoding="utf-8", | ||
| ) | ||
|
|
||
| monkeypatch.setattr( | ||
| module, | ||
| "get_astrbot_workspaces_path", | ||
| lambda: str(workspaces_dir), | ||
| ) | ||
| monkeypatch.setattr( | ||
| "astrbot.core.skills.skill_manager.get_astrbot_data_path", | ||
| lambda: str(data_dir), | ||
| ) | ||
| monkeypatch.setattr( | ||
| "astrbot.core.skills.skill_manager.get_astrbot_skills_path", | ||
| lambda: str(global_skills_dir), | ||
| ) | ||
| monkeypatch.setattr( | ||
| "astrbot.core.skills.skill_manager.get_astrbot_plugin_path", | ||
| lambda: str(plugins_dir), | ||
| ) | ||
|
|
||
| req = ProviderRequest() | ||
| req.conversation = MagicMock(persona_id=None) | ||
|
|
||
| await module._ensure_persona_and_skills( | ||
| req, {"computer_use_runtime": "local"}, mock_context, mock_event | ||
| ) | ||
|
|
||
| assert "Workspace scoped skill." not in req.system_prompt | ||
| assert "## Skills" not in req.system_prompt |
There was a problem hiding this comment.
The current test asserts that "Workspace scoped skill." and "## Skills" are not in the system prompt when workspace skills are skipped. However, this doesn't verify that global/plugin skills are still correctly loaded and not accidentally broken or overridden in group sessions.
To make this regression test more robust, we should also set up a global skill (similar to test_ensure_skills_includes_workspace_skills) and assert that:
- The global skill is loaded and present in the system prompt.
- The workspace skill (even if it has the same name as the global skill) is not loaded and does not override the global skill.
global_skill_dir = global_skills_dir / "workspace-skill"
global_skill_dir.mkdir(parents=True)
global_skill_dir.joinpath("SKILL.md").write_text(
"---\ndescription: Global scoped skill.\n---\n",
encoding="utf-8",
)
workspace_skill_dir = workspace_root / "skills" / "workspace-skill"
workspace_skill_dir.mkdir(parents=True)
workspace_skill_dir.joinpath("SKILL.md").write_text(
"---\ndescription: Workspace scoped skill.\n---\n",
encoding="utf-8",
)
monkeypatch.setattr(
module,
"get_astrbot_workspaces_path",
lambda: str(workspaces_dir),
)
monkeypatch.setattr(
"astrbot.core.skills.skill_manager.get_astrbot_data_path",
lambda: str(data_dir),
)
monkeypatch.setattr(
"astrbot.core.skills.skill_manager.get_astrbot_skills_path",
lambda: str(global_skills_dir),
)
monkeypatch.setattr(
"astrbot.core.skills.skill_manager.get_astrbot_plugin_path",
lambda: str(plugins_dir),
)
req = ProviderRequest()
req.conversation = MagicMock(persona_id=None)
await module._ensure_persona_and_skills(
req, {"computer_use_runtime": "local"}, mock_context, mock_event
)
assert "Global scoped skill." in req.system_prompt
assert "Workspace scoped skill." not in req.system_prompt
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
astrbot-docs | 17505df | Commit Preview URL Branch Preview URL |
Jun 30 2026, 06:20 AM |
dosubot
Bot
added
size:S
|
Would a more reasonable approach be to prohibit non-admiins from creating skills in the workspace? |
2 participants
Motivation
skills/<name>/SKILL.mdin a shared group workspace and have those workspace skills injected into later admin requests whencomputer_use_runtimeislocal.Description
astrbot/core/astr_main_agent.pyto only run whenruntime == "local"and the event is not a group session by checkingnot event.get_group_id().tests/unit/test_astr_main_agent.pythat creates a workspace skill in a group-session workspace and asserts it is not injected into the system prompt.astrbot/core/astr_main_agent.py,tests/unit/test_astr_main_agent.py.Testing
ruff formatandruff checkagainst the modified files and the checks passed.pytest -k 'workspace_skills'was attempted but blocked by dependency fetch/network tunnel errors in the environment.uv run ruff formatanduv run pytestwere also blocked by the same dependency fetch/network tunnel issue, so full CI-style execution could not complete in this environment.Codex Task
Summary by Sourcery
Guard workspace skill discovery against group sessions to avoid injecting group-scoped skills into admin/local requests.
Bug Fixes:
Tests: