Skip to content

Fail closed on malformed protected list responses - #6553

Merged
ChrisJBurns merged 2 commits into
mainfrom
fix-resource-template-response-filtering
Sep 8, 2026
Merged

ChrisJBurns merged 2 commits into
mainfrom
fix-resource-template-response-filtering

Conversation

@ChrisJBurns

@ChrisJBurns ChrisJBurns commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Protected tool, prompt, and resource lists were decoded permissively, so malformed, duplicate, or case-folded authorization fields could be interpreted differently by the filter and the client.
  • Validate list containers and per-item identifiers before policy evaluation, return a generic internal error for invalid responses, and leave the tool annotation cache unchanged when validation fails.

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 or ambiguously encoded protected list results now return a generic internal JSON-RPC error instead of passing through unfiltered.

Implementation plan

Approved implementation plan
  1. Strictly validate the list field and authorization identifier for existing protected list responses.
  2. Reject malformed or ambiguous results before authorization or cache mutation.
  3. Add table-driven coverage across tools, prompts, and resources.
  4. Run repository verification and independent code review.

Special notes for reviewers

  • The changed pkg/authz package passes within the race-enabled task test run. The repository-wide command currently fails only in unrelated existing pkg/plugins/pluginsvc tests.
  • 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.

@ChrisJBurns
ChrisJBurns force-pushed the fix-resource-template-response-filtering branch from bb243d7 to c79e33d Compare September 8, 2026 20:19
@ChrisJBurns ChrisJBurns changed the title Filter unauthorized resource template descriptors Fail closed on malformed protected list responses Sep 8, 2026
@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label Sep 8, 2026

@jhrozek jhrozek 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.

Reviewed the fail-closed validation added for tools/prompts/resources list responses. It correctly extends the same protection already applied to resource templates (GHSA-5vxv-9f7g-x8j2) — rejects malformed/ambiguous JSON (case-folded aliases, duplicate keys, wrong types) instead of silently passing responses through unfiltered. Field names match the MCP spec, error handling stays generic to the client, and tests cover the malformed-input cases well.

One related gap noted for a follow-up: filterFindToolResponse still does best-effort passthrough on its embedded find_tool payload, but that's pre-existing and out of scope here.

@ChrisJBurns

Copy link
Copy Markdown
Collaborator Author

/retest

@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Sep 8, 2026
@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.18919% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.71%. Comparing base (a1f16ce) to head (c79e33d).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/authz/response_filter.go 89.18% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6553   +/-   ##
=======================================
  Coverage   78.70%   78.71%           
=======================================
  Files         777      777           
  Lines       76949    76983   +34     
=======================================
+ Hits        60564    60596   +32     
- Misses      16380    16382    +2     
  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 ab27a23 into main Sep 8, 2026
56 of 58 checks passed
@ChrisJBurns
ChrisJBurns deleted the fix-resource-template-response-filtering branch September 8, 2026 21:23
@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/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants