fix(tool): reject out-of-range Read offset and limit by BlueX888 · Pull Request #3303 · agentscope-ai/agentscope · GitHub
Skip to content

fix(tool): reject out-of-range Read offset and limit - #3303

Closed
BlueX888 wants to merge 1 commit into
agentscope-ai:mainfrom
BlueX888:fix/prep-read-offset-nonpositive-negative-slice
Closed

BlueX888 wants to merge 1 commit into
agentscope-ai:mainfrom
BlueX888:fix/prep-read-offset-nonpositive-negative-slice

Conversation

@BlueX888

Copy link
Copy Markdown
Contributor

AgentScope Version

2.0.10

Description

Fixes #3291.

What Problem This Solves

This PR fixes a pagination validation gap in the builtin Read tool.

The Read schema already declares offset as a 1-based line number with ge=1, and limit with ge=1, le=2000 (src/agentscope/tool/_builtin/_read.py:60-72), but — unlike Grep and Glob — nothing validated them at runtime, so tool calls whose arguments bypass schema validation reached the slicing step unvalidated.

_read_text_file slices with lines[start_idx:end_idx] where start_idx = offset - 1 (src/agentscope/tool/_builtin/_read.py:533-535). A non-positive offset makes start_idx negative, so Python addresses the list from its end and the tool returns the tail of the file labeled with line numbers 0 or negative, with a success state. limit <= 0 returns empty text, also with a success state. Reading a 10-line file on main:

read = Read()
await read.call(file_path=p, offset=0)   # -> "     0\tline 10", state=running
await read.call(file_path=p, offset=-3)  # -> lines 7..10 labeled -3..0, state=running
await read.call(file_path=p, limit=0)    # -> "", state=running

Because the tool reports success, the model receives content from the wrong part of the file with impossible line numbers and no signal to self-correct.

Change

Add runtime validation in _read_text_file, before any slicing, mirroring the Grep/Glob pagination guards:

  • offset < 1 -> Error: offset must be at least 1, got: {offset}.
  • limit < 1 or limit > 2000 -> Error: limit must be between 1 and 2000, got: {limit}.

Both return a ToolResultState.ERROR chunk so the model can retry with valid arguments. Valid reads are unchanged, and since offset/limit only apply to plain text files, the guards live in _read_text_file so the PDF and image paths are untouched.

Testing

  • New regression tests on unpatched main (before the fix):
FAILED tests/builtin_read_test.py::ReadToolTest::test_invalid_limit_returns_error
FAILED tests/builtin_read_test.py::ReadToolTest::test_non_positive_offset_returns_error
AssertionError: <ToolResultState.RUNNING: 'running'> != 'error' : 0
  • After the fix:
    • pytest tests/builtin_read_test.py -> 20 passed in 0.86s
    • pytest tests/builtin_read_test.py tests/builtin_write_test.py tests/builtin_edit_test.py tests/builtin_grep_test.py tests/builtin_glob_test.py tests/builtin_bash_test.py tests/builtin_file_cache_test.py -> 122 passed, 61 subtests passed in 2.13s
    • pytest tests -> 2746 passed, 147 skipped, 600 subtests passed in 167.54s
    • pre-commit run --files src/agentscope/tool/_builtin/_read.py tests/builtin_read_test.py -> all hooks passed (mypy, black, flake8, pylint, pyroma)

Checklist

Please check the following items before code is ready to be reviewed.

  • An issue has been created for this PR
  • I have read the CONTRIBUTING.md
  • Docstrings are in Google style
  • Related documentation has been updated (e.g. links, examples, etc.) in documentation repository
  • Code is ready for review

The Read schema declares offset >= 1 and 1 <= limit <= 2000, but
nothing enforced them at runtime: a non-positive offset negative-sliced
the file and silently returned its tail (e.g. offset=0 returned the
last line labeled "0"), and limit <= 0 returned empty text, both with
a success result. Return an error chunk instead so the model can
self-correct, matching the Grep/Glob pagination validation.
@github-actions github-actions Bot added the pr/awaiting-review Waiting for maintainers to review this PR label Oct 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@BlueX888

Copy link
Copy Markdown
Contributor Author

@BlueX888 BlueX888 closed this Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/awaiting-review Waiting for maintainers to review this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Read tool accepts offset <= 0, negative-slicing the file and returning its tail labeled with line numbers 0 or negative

1 participant