Skip to content

fix: 修复 astrbot_file_read_tool 在读取目录时返回误导性 "Permission denied" 错误的 bug。#9088

Merged
Soulter merged 1 commit into
AstrBotDevs:masterfrom
Fronut:fix/astrbot-file-read-tool-directory-permission-denied
Jun 30, 2026
Merged

fix: 修复 astrbot_file_read_tool 在读取目录时返回误导性 "Permission denied" 错误的 bug。#9088
Soulter merged 1 commit into
AstrBotDevs:masterfrom
Fronut:fix/astrbot-file-read-tool-directory-permission-denied

Conversation

@Fronut

@Fronut Fronut commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Problem / 问题

Fixes #9087
当 LLM 调用 astrbot_file_read_tool 并传入目录路径时,工具返回 Error: [Errno 13] Permission denied,暗示是权限不足。实际原因是传入了目录而非文件——底层 _probe_local_file() 对目录执行 open("rb"),Windows 返回 Errno 13,被上层 except PermissionError 捕获后错误展示。

Root Cause / 根因

调用链:

FileReadTool.call()                          # fs.py
  → read_file_tool_result()                  # file_read_utils.py
    → _probe_local_file(path)                # file_read_utils.py
      → Path(path).open("rb")                # 目录 → OSError(Errno 13)
        → except PermissionError             # 被当作权限错误
          → return f"Error: {exc}"           # 误导 LLM

FileReadTool.call() 中没有在文件 I/O 之前检查路径是否为目录。

Modifications / 改动点

改动两个文件:

1. astrbot/core/tools/computer_tools/fs.py

FileReadTool.call() 中,路径规范化完成后、read_file_tool_result 调用之前,增加 os.path.isdir() 检查:

             if not normalized_path:
                 raise ValueError("`path` must be a non-empty string.")
+            if local_env and os.path.isdir(normalized_path):
+                return (
+                    f"Error: `{normalized_path}` is a directory, not a file. "
+                    "Use a file path instead, or use `astrbot_execute_shell` to list directory contents."
+                )
             offset, limit = self._validate_read_window(offset, limit)
  • 检查时机:在 _normalize_rw_path 权限校验完成后、get_booter 之前,不浪费后续 I/O 资源
  • 对 local 和 sandbox 两种模式均有效(sandbox 模式下 normalized_path 为原始 path,os.path.isdir() 在 host 上大概率返回 False,不会误拦截)
  • os 已在文件顶部导入,无需新增 import

2. tests/test_computer_fs_tools.py

新增测试用例 test_file_read_tool_rejects_directory_with_clear_message

@pytest.mark.asyncio
async def test_file_read_tool_rejects_directory_with_clear_message(
    monkeypatch: pytest.MonkeyPatch,
    tmp_path,
):
    """FileReadTool should return a helpful message when given a directory path."""
    workspace = _setup_local_fs_tools(monkeypatch, tmp_path)
    subdir = workspace / "my-directory"
    subdir.mkdir()

    result = await fs_tools.FileReadTool().call(
        _make_context(),
        path="my-directory",
    )

    assert "is a directory, not a file" in result
    assert "my-directory" in result
  • This is NOT a breaking change. / 这不是一个破坏性变更。

Verification Steps / 验证步骤

  1. 运行新增测试:
    pytest tests/test_computer_fs_tools.py::test_file_read_tool_rejects_directory_with_clear_message -xvs
    
  2. 运行全部 file_read 相关测试确认无回归:
    pytest tests/test_computer_fs_tools.py -k "file_read" -xvs
    
  3. 实际调用验证(修改前后对比):
    • 修改前astrbot_file_read_tool(path="D:\some\directory")Error: [Errno 13] Permission denied
    • 修改后:同一调用 → Error: \D:\some\directory` is a directory, not a file. Use a file path instead, or use `astrbot_execute_shell` to list directory contents.`

Screenshots or Test Results / 测试结果

============================= test session starts =============================
tests/test_computer_fs_tools.py::test_file_read_tool_rejects_directory_with_clear_message PASSED
tests/test_computer_fs_tools.py::test_file_read_tool_rejects_large_full_text_read_before_local_stream_read PASSED
tests/test_computer_fs_tools.py::test_file_read_tool_returns_image_call_tool_result_for_images PASSED
tests/test_computer_fs_tools.py::test_file_read_tool_treats_svg_as_text PASSED
tests/test_computer_fs_tools.py::test_file_read_tool_reads_pdf_via_parser PASSED
tests/test_computer_fs_tools.py::test_file_read_tool_reads_docx_via_parser_and_magic PASSED
tests/test_computer_fs_tools.py::test_file_read_tool_reads_epub_via_parser_and_magic PASSED
tests/test_computer_fs_tools.py::test_file_read_tool_stores_long_converted_document_in_workspace PASSED
====================== 8 passed in 2.52s ======================

全部 8 个相关测试通过。

注意:test_file_read_tool_allows_partial_read_for_large_text_file 在 Windows 下因 \r\n vs \n 换行符差异而失败,这是非本次改动引入的已有问题。


Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

Summary by Sourcery

Handle directory paths passed to the file read tool more gracefully and add coverage for this behavior.

Bug Fixes:

  • Prevent astrbot_file_read_tool from misreporting 'Permission denied' when given a directory path by returning a clear directory-specific error message instead.

Tests:

  • Add an async test ensuring FileReadTool returns a helpful message when invoked with a directory path.

@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. area:core The bug / feature is about astrbot's core, backend labels Jun 30, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a check to the FileReadTool to prevent reading directories, returning a helpful error message instead, along with a corresponding unit test. The reviewer pointed out a critical issue where checking os.path.isdir on the host machine is incorrect when running in sandbox mode (local_env is False), and suggested restricting this check to when local_env is True.

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.

Comment thread astrbot/core/tools/computer_tools/fs.py Outdated

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • The directory guard is added only in FileReadTool.call; consider moving this check into _probe_local_file (or a shared helper) so any future direct callers of the probe logic also get a clear directory error rather than a misleading permission error.
  • Since normalized_path may be a Path instance depending on _normalize_rw_path, using Path(normalized_path).is_dir() instead of os.path.isdir(normalized_path) would keep the style consistent with the rest of the module and avoid any implicit coercion issues.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The directory guard is added only in `FileReadTool.call`; consider moving this check into `_probe_local_file` (or a shared helper) so any future direct callers of the probe logic also get a clear directory error rather than a misleading permission error.
- Since `normalized_path` may be a `Path` instance depending on `_normalize_rw_path`, using `Path(normalized_path).is_dir()` instead of `os.path.isdir(normalized_path)` would keep the style consistent with the rest of the module and avoid any implicit coercion issues.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Fronut Fronut force-pushed the fix/astrbot-file-read-tool-directory-permission-denied branch from 20eae9a to 19e5ad1 Compare June 30, 2026 08:02
@Fronut

Fronut commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a check to the file reading tool to reject directory paths when a file path is expected, returning a helpful error message instead. A corresponding unit test was also added to verify this behavior. The review feedback suggests improving the readability of the error message for both users and LLMs by wrapping the directory path and the tool name in single quotes.

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.

Comment thread astrbot/core/tools/computer_tools/fs.py
…stead of misleading Permission denied

When LLM passes a directory path to astrbot_file_read_tool, the tool previously
returned Error: [Errno 13] Permission denied, misleading the LLM into thinking
it was a permissions issue. The real cause: _probe_local_file() calls open('rb')
on the path, which fails on directories with Errno 13 on Windows. This is caught
by except PermissionError and displayed as-is.

Fix: Add os.path.isdir() check in FileReadTool.call() before any file I/O, at
the earliest safe point after path normalization and permission validation.
Returns a clear message: '<path> is a directory, not a file. Use a file path
instead, or use astrbot_execute_shell to list directory contents.'

Changes:
- astrbot/core/tools/computer_tools/fs.py: add isdir guard
- tests/test_computer_fs_tools.py: add test_file_read_tool_rejects_directory_with_clear_message
@Fronut Fronut force-pushed the fix/astrbot-file-read-tool-directory-permission-denied branch from 19e5ad1 to 7379e12 Compare June 30, 2026 08:08
@Fronut

Fronut commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a check to the FileReadTool to prevent reading directories as files, returning a clear error message instead, and includes a corresponding unit test. The reviewer suggests moving this directory check logic deeper into the file probing layer to ensure it also works in sandboxed environments (where local_env is False) rather than just the local environment.

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.

Comment thread astrbot/core/tools/computer_tools/fs.py
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jun 30, 2026
@Soulter Soulter merged commit 41f8960 into AstrBotDevs:master Jun 30, 2026
21 checks passed
@Fronut Fronut deleted the fix/astrbot-file-read-tool-directory-permission-denied branch June 30, 2026 12:50
BegoniaHe pushed a commit to BegoniaHe/AstrBot that referenced this pull request Jul 1, 2026
…stead of misleading Permission denied (AstrBotDevs#9088)

When LLM passes a directory path to astrbot_file_read_tool, the tool previously
returned Error: [Errno 13] Permission denied, misleading the LLM into thinking
it was a permissions issue. The real cause: _probe_local_file() calls open('rb')
on the path, which fails on directories with Errno 13 on Windows. This is caught
by except PermissionError and displayed as-is.

Fix: Add os.path.isdir() check in FileReadTool.call() before any file I/O, at
the earliest safe point after path normalization and permission validation.
Returns a clear message: '<path> is a directory, not a file. Use a file path
instead, or use astrbot_execute_shell to list directory contents.'

Changes:
- astrbot/core/tools/computer_tools/fs.py: add isdir guard
- tests/test_computer_fs_tools.py: add test_file_read_tool_rejects_directory_with_clear_message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] astrbot_file_read_tool 读取目录时返回误导性 "Permission denied"

2 participants