Skip to content

fix(background_review): add read_file to review fork tool whitelist - #40060

Open
HeLLGURD wants to merge 1 commit into
NousResearch:mainfrom
HeLLGURD:fix/background-review-whitelist-read-file
Open

fix(background_review): add read_file to review fork tool whitelist#40060
HeLLGURD wants to merge 1 commit into
NousResearch:mainfrom
HeLLGURD:fix/background-review-whitelist-read-file

Conversation

@HeLLGURD

@HeLLGURD HeLLGURD commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Bug

The background review fork ran with a tool whitelist of memory + skills toolsets only. When authoring or patching a skill that references an existing file, the fork had no way to read that file:

  • skill_manage has no
    ead_file action - calling it returned Unknown action 'read_file'
  • skill_view only reads skill-internal files, not arbitrary external paths

ead_file (file toolset) was denied by the whitelist

The model either fabricated content or guessed skill_manage(action='read_file') and hit the deny-wall.

Fix

Add
ead_file to
eview_whitelist after it is built from the toolset definitions.
ead_file is read-only (size-guarded, device-path checked) - no write risk. Also update the review prompt to mention it. Supersedes stale PR #27422 which targeted the since-refactored
un_agent.py path.

Fixes #40003

The background review fork could not read external files needed for skill
authoring. The model either fabricated content or called the non-existent
skill_manage(action='read_file') and hit a hard deny-wall. read_file is
read-only (size-guarded, device-path checked) - adding it to the whitelist
closes the gap without introducing write risk. Supersedes stale PR NousResearch#27422.

Fixes NousResearch#40003.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/file File tools (read, write, patch, search) labels Jun 5, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the current deny-wall: agent/background_review.py:806-815 still derives the review whitelist from only memory and skills, and hermes_cli/plugins.py:2135-2141 enforces it before dispatch.

Problems

  • This is a filesystem-read authority expansion, not only a no-write change. read_file accepts absolute paths through tools/file_tools.py:430-464, whereas the current review contract is explicitly memory/skills-only in tests/run_agent/test_background_review_toolset_restriction.py:88-158. Please establish the intended read scope before widening that boundary.
  • The PR has no regression coverage. Add a whitelist-capture test that proves read_file is allowed while write_file, patch, search_files, terminal, and delegate_task remain blocked.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: background-review fork can't read an external file — model calls skill_manage(action="read_file") and gets "Unknown action"

3 participants