Skip to content

Harden find tool response filtering - #6555

Merged
ChrisJBurns merged 1 commit into
mainfrom
fix-find-tool-response-filtering
Sep 9, 2026
Merged

ChrisJBurns merged 1 commit into
mainfrom
fix-find-tool-response-filtering

Conversation

@ChrisJBurns

Copy link
Copy Markdown
Collaborator

Summary

  • Successful find_tool calls carry tool descriptors in serialized text and may repeat them in structuredContent; the response filter previously parsed only the first recognizable text payload, so malformed output could pass through and the structured representation remained unfiltered.
  • Strictly validate the complete internal response shape before authorization or cache mutation, require text and structured representations to agree, and fail malformed or ambiguous results with the existing generic JSON-RPC error.
  • Apply one set of positional authorization decisions to both raw representations so unauthorized descriptors are removed while metadata, extensions, and allowed descriptor fields remain intact.
  • Follow up on review feedback from Fail closed on malformed protected list responses #6553.

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

Does this introduce a user-facing change?

Malformed, ambiguous, or conflicting successful find_tool results now return a generic internal JSON-RPC error. Well-formed results expose authorized descriptors consistently through both text and structured content.

Implementation plan

Approved implementation plan
  1. Validate the raw CallToolResult envelope and every output representation before policy evaluation.
  2. Require the trusted producer shape and reject malformed, ambiguous, or divergent output.
  3. Authorize the validated tool list once and filter both representations by exact list position.
  4. Preserve unrelated raw result, content, output, and allowed tool fields.
  5. Add table-driven regressions and complete independent code, protocol, and security reviews.

Special notes for reviewers

  • ToolHive's producers emit exactly one text content item and optionally an equivalent structuredContent object; successful responses outside that contract now fail closed.
  • The changed pkg/authz tests pass within the race-enabled task test run. The repository-wide command remains non-zero only because of unrelated existing pkg/plugins/pluginsvc failures.
  • task lint reports no findings in changed files; it remains non-zero because of existing gci findings in pkg/authserver/server/provider.go and pkg/authserver/server_impl.go.

@github-actions github-actions Bot added the size/L Large PR: 600-999 lines changed label Sep 8, 2026
@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.38318% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.71%. Comparing base (ab27a23) to head (aa1a69d).

Files with missing lines Patch % Lines
pkg/authz/response_filter.go 88.44% 23 Missing ⚠️
pkg/authz/tool_filter.go 73.33% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6555      +/-   ##
==========================================
+ Coverage   78.69%   78.71%   +0.02%     
==========================================
  Files         777      777              
  Lines       77092    77280     +188     
==========================================
+ Hits        60664    60830     +166     
- Misses      16423    16445      +22     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ChrisJBurns
ChrisJBurns merged commit 9bcaec1 into main Sep 9, 2026
46 checks passed
@ChrisJBurns
ChrisJBurns deleted the fix-find-tool-response-filtering branch September 9, 2026 14:24
@github-actions github-actions Bot mentioned this pull request Sep 10, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR: 600-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants