fix(security): prevent admin bypass from accessing private resources by bogdanmariusc10 · Pull Request #4341 · IBM/mcp-context-forge

bogdanmariusc10 · 2026-04-20T11:54:02Z

🔗 Related Issue

Closes #4323
Jira Issue: https://jsw.ibm.com/browse/ICACF-21

📝 Summary

This PR fixes a security vulnerability where admin users could view private resources (tools, prompts, servers) belonging to other users through API endpoints. The vulnerability violated the documented principle that private resources should only be accessible to their owners.

Key Changes:

Modified service layer to deny admin bypass for private resources
Admin bypass now only grants access to public and team resources (NOT private)
Fixed API endpoints to properly pass user context for authorization
Added comprehensive test coverage for the security fix

Security Impact:

Private resources are now properly protected at both service and API layers
Admin role no longer implicitly grants access to other users' private resources
Unauthorized access attempts return generic "not found" errors to prevent information disclosure

🏷️ Type of Change

🧪 Verification

Check	Command	Status
Lint suite	`make lint`	✅ Pass
Unit tests	`make test`	✅ Pass
Coverage ≥ 80%	`make coverage`	✅ Pass

Test Coverage:

Unit tests verify admin bypass is denied for private resources
Unit tests verify admin bypass is granted for team resources
Unit tests verify owners can access their own private resources
Integration tests updated to reflect new security behavior
All tests pass with the security fix in place

✅ Checklist

Code formatted (make black isort pre-commit)
Tests added/updated for changes
Documentation updated (inline comments documenting security requirements)
No secrets or credentials committed

📓 Notes

Files Modified:

Service Layer (Core Security Logic):
- mcpgateway/services/base_service.py - Filter private resources in list operations
- mcpgateway/services/prompt_service.py - Deny admin bypass for private prompts
- mcpgateway/services/tool_service.py - Deny admin bypass for private tools + access check in get_tool()
- mcpgateway/services/resource_service.py - Deny admin bypass for private resources
API Layer (Request Handling):
- mcpgateway/main.py - Fixed endpoints to properly pass user_email when token_teams not set
- Used sentinel value pattern to distinguish "token_teams not set" vs "token_teams=None"
Test Coverage:
- tests/unit/mcpgateway/services/test_authorization_access.py - Security-focused tests
- tests/unit/mcpgateway/services/test_prompt_service.py - Updated for new behavior
- tests/unit/mcpgateway/services/test_resource_service.py - Updated for new behavior
- tests/unit/mcpgateway/services/test_tool_service.py - Updated for new behavior
- tests/unit/mcpgateway/test_main.py - Fixed endpoint tests

Security Guarantees:

List operations filter out private resources during admin bypass
Individual resource retrieval performs access control check before returning data
Generic "not found" errors prevent information disclosure
Consistent enforcement across all resource types (tools, prompts, resources, servers)
Resource owners can still access their own private resources when properly authenticated

Acceptance Criteria Met:
✅ Access control applied at API layer for every resource request
✅ Private resources only returned to the user who created them
✅ Admin role does not implicitly grant access to private resources
✅ Unauthorized requests return 403/404 with no resource detail
✅ Tests cover tools, prompts, and servers endpoints
✅ Fix verified across all relevant endpoints

… access paths Extends the original PR's service-level denial so admin bypass (JWT `is_admin=true` + `teams=null`, or non-JWT dev-mode admin) can never see or mutate another user's private resources across any surface. Service-layer access checks: - a2a_service._check_agent_access: admin bypass grants public + team access only, never private. get_agent_by_name / get_agent_card gain user_email / token_teams and enforce the check before returning. - server_service.get_server: new _check_server_access helper; generic ServerNotFoundError + server_access_denied structured log on denial. - gateway_service.get_gateway: new _check_gateway_access helper; same pattern. - prompt_service.get_prompt_details: gates private reads (admin UI path). - resource_service.get_resource_by_id: gates private reads (/resources/{id}/info and admin UI paths). - resource_service.list_resource_templates: admin bypass filters visibility != 'private' from the template enumeration. - tool_service.get_tool: adds token_teams parameter so Layer 1 visibility is evaluated against JWT-scoped teams rather than being widened to the caller's full DB team roles (fixes a scope-widening regression). - completion_service._apply_visibility_scope: admin bypass excludes private so completion suggestions cannot reveal private prompt / resource names. - tag_service._apply_visibility_scope: admin bypass excludes private so the tag enumeration cannot reveal other users' private entity metadata. Endpoint / routing rewire: - main.py: the three inline sentinel_unset blocks replaced with _get_scoped_resource_access_context(), the canonical helper already maintained in auth.py / main.py. Same helper is now threaded into /servers/{id}, /gateways/{id}, /resources/{id}/info, /tools/{id}, /resources/templates/list (REST + JSON-RPC + MCP dispatcher), and the internal A2A agent card path. - admin.py: the five admin detail endpoints (/admin/{servers,gateways, resources,prompts,tools}/{id}) now accept the FastAPI Request and resolve visibility scope via a lazy import of the same helper to avoid the main <-> admin circular import. - base_service._apply_access_control: admin-bypass branch still filters private rows out; docstring updated to state the invariant. Observability: - Each new deny path emits a structured event (`tool_access_denied`, `server_access_denied`, `gateway_access_denied`, `prompt_access_denied`, `resource_access_denied`) so forensics does not rely on HTTP logs. Tests: - New TestDirectGetAccessDenial in tests/unit/mcpgateway/services/ test_authorization_access.py: regressions for every new deny/allow path (server, gateway, a2a by name and card, prompt_details, resource_by_id, tool scope, template enumeration, completion + tag SQL predicates). - Route-level regressions in tests/unit/mcpgateway/test_main.py assert admin-bypass wiring for the REST and JSON-RPC template handlers. - Completion + tag visibility assertions compile the scoped statement with `literal_binds=True` and verify the rendered SQL contains the expected `visibility != 'private'` predicate. - Existing a2a / completion / tag tests that encoded the pre-#4341 admin-sees-everything behavior are updated to the new secure semantics. - Three test_main_extended callers that invoke delete_server / delete_gateway directly now supply a mock Request, matching the new route signatures. Docs / behavior notes: - CHANGELOG.md gains an [Unreleased] breaking-change entry with a migration note for integrators that relied on admin-bypass private reads. - docs/docs/manage/rbac.md access matrix updated: admin bypass no longer reads private, with an admonition pointing at this PR. - docs/docs/architecture/multitenancy.md reconciled with the same owner-only rule for private visibility. Follow-up (tracked separately, not in scope here): - A2A consistency cleanup for a2a_server_service._check_server_access and A2AAgentService.list_tasks -> #4437 Signed-off-by: Jonathan Springer <jps@s390x.com>

jonpspri · 2026-04-24T13:26:55Z

Rebased onto current main and extended the fix in response to an in-depth security pass. Original three commits (the service-layer denial, the _UNSET → sentinel_unset rename, and the test coverage) are preserved; the follow-up work is in a single squashed commit on top. Force-pushed with --force-with-lease.

What the follow-up commit adds

Building on the original service-layer fix for tools / prompts / resources, the same "admin bypass may see public + team, never another user's private" rule now applies consistently across the remaining surfaces:

Service layer — A2A agents / names / cards, get_server, get_gateway, get_prompt_details, get_resource_by_id, list_resource_templates, completion_service._apply_visibility_scope, tag_service._apply_visibility_scope. Each denial returns a generic *NotFoundError (no disclosure) and emits a structured *_access_denied event.
Tool-detail scope fix — tool_service.get_tool now accepts JWT-scoped token_teams instead of widening to the caller's full DB team roles (closes a scope-widening regression).
Endpoint wiring — the three inline sentinel_unset = object() blocks in main.py were replaced with the existing _get_scoped_resource_access_context() helper, which already handles both the JWT admin-bypass path and the non-JWT (basic-auth / dev-mode) admin path consistently. The five admin_get_* endpoints are wired through the same helper.
Tests — TestDirectGetAccessDenial covers every new deny / allow branch; route-level regression tests assert the REST and JSON-RPC resources/templates/list handlers pass (user_email=None, token_teams=None) under admin bypass; completion/tag predicate tests compile real SQL with literal_binds=True and assert the rendered visibility != 'private'. Existing tests that encoded the pre-fix "admin sees everything" semantics are updated to match the new rule.
Docs — rbac.md access matrix and multitenancy.md reconciled with the owner-only rule. CHANGELOG.md picks up an [Unreleased] breaking-change entry with migration guidance for integrators that relied on admin-bypass private reads.

Deferred (tracked separately)

Two adjacent A2A helpers (a2a_server_service._check_server_access, A2AAgentService.list_tasks) still encode the pre-fix semantics. They are not currently reachable via the routers, so they did not block this PR, but they are a footgun for future callers. Filed as #4437 for a follow-up PR.

Suggested verification

tests/unit/mcpgateway/services/test_authorization_access.py::TestDirectGetAccessDenial is a good single landing spot for the new invariants.
mcpgateway/services/a2a_service.py::_check_agent_access is the canonical pattern other services now mirror — a quick scan across server / gateway / tool / prompt / resource helpers should show structural consistency.
The three resources/templates/list handlers and all five admin_get_* endpoints now share _get_scoped_resource_access_context(), which is the helper AGENTS.md already flags as canonical.

Happy to split, trim, or extend anything; the squashed follow-up is one logical unit but I can break it up if that helps review.

… access paths Extends the original PR's service-level denial so admin bypass (JWT `is_admin=true` + `teams=null`, or non-JWT dev-mode admin) can never see or mutate another user's private resources across any surface. Service-layer access checks: - a2a_service._check_agent_access: admin bypass grants public + team access only, never private. get_agent_by_name / get_agent_card gain user_email / token_teams and enforce the check before returning. - server_service.get_server: new _check_server_access helper; generic ServerNotFoundError + server_access_denied structured log on denial. - gateway_service.get_gateway: new _check_gateway_access helper; same pattern. - prompt_service.get_prompt_details: gates private reads (admin UI path). - resource_service.get_resource_by_id: gates private reads (/resources/{id}/info and admin UI paths). - resource_service.list_resource_templates: admin bypass filters visibility != 'private' from the template enumeration. - tool_service.get_tool: adds token_teams parameter so Layer 1 visibility is evaluated against JWT-scoped teams rather than being widened to the caller's full DB team roles (fixes a scope-widening regression). - completion_service._apply_visibility_scope: admin bypass excludes private so completion suggestions cannot reveal private prompt / resource names. - tag_service._apply_visibility_scope: admin bypass excludes private so the tag enumeration cannot reveal other users' private entity metadata. Endpoint / routing rewire: - main.py: the three inline sentinel_unset blocks replaced with _get_scoped_resource_access_context(), the canonical helper already maintained in auth.py / main.py. Same helper is now threaded into /servers/{id}, /gateways/{id}, /resources/{id}/info, /tools/{id}, /resources/templates/list (REST + JSON-RPC + MCP dispatcher), and the internal A2A agent card path. - admin.py: the five admin detail endpoints (/admin/{servers,gateways, resources,prompts,tools}/{id}) now accept the FastAPI Request and resolve visibility scope via a lazy import of the same helper to avoid the main <-> admin circular import. - base_service._apply_access_control: admin-bypass branch still filters private rows out; docstring updated to state the invariant. Observability: - Each new deny path emits a structured event (`tool_access_denied`, `server_access_denied`, `gateway_access_denied`, `prompt_access_denied`, `resource_access_denied`) so forensics does not rely on HTTP logs. Tests: - New TestDirectGetAccessDenial in tests/unit/mcpgateway/services/ test_authorization_access.py: regressions for every new deny/allow path (server, gateway, a2a by name and card, prompt_details, resource_by_id, tool scope, template enumeration, completion + tag SQL predicates). - Route-level regressions in tests/unit/mcpgateway/test_main.py assert admin-bypass wiring for the REST and JSON-RPC template handlers. - Completion + tag visibility assertions compile the scoped statement with `literal_binds=True` and verify the rendered SQL contains the expected `visibility != 'private'` predicate. - Existing a2a / completion / tag tests that encoded the pre-#4341 admin-sees-everything behavior are updated to the new secure semantics. - Three test_main_extended callers that invoke delete_server / delete_gateway directly now supply a mock Request, matching the new route signatures. Docs / behavior notes: - CHANGELOG.md gains an [Unreleased] breaking-change entry with a migration note for integrators that relied on admin-bypass private reads. - docs/docs/manage/rbac.md access matrix updated: admin bypass no longer reads private, with an admonition pointing at this PR. - docs/docs/architecture/multitenancy.md reconciled with the same owner-only rule for private visibility. Follow-up (tracked separately, not in scope here): - A2A consistency cleanup for a2a_server_service._check_server_access and A2AAgentService.list_tasks -> #4437 Signed-off-by: Jonathan Springer <jps@s390x.com>

After rebasing PR #4341 onto main, the post-#4107 admin-bypass detection (is_admin_bypass_granted) interacted with this PR's invariant in a way the original commits did not anticipate. Resolved per Oracle red-team review using the same two-branch hybrid pattern that a2a_service._visible_agent_ids already established: (None, None) anonymous bypass sees public + team only; (email, None) DB-resolved admin additionally sees the caller's own private rows; another user's private is still denied. Applied in BaseService._apply_access_control, BaseService._apply_visibility_scope (the shared helper used by completion and tag services on main), and the per-resource _check_*_access methods in tool/prompt/resource/a2a services. A2A signature reconciliation: get_agent_by_name now correctly awaits _check_agent_access(db, agent, ...) (the post-#4107 async signature), and get_agent_card delegates its visibility gate up to main.handle_a2a_agent_card so the sync method does not need to call the async helper. Test updates: per-resource test_check_*_access_database_admin_bypass and test_check_agent_access_db_admin_bypass_only_with_unrestricted_token now assert the hybrid (own private allowed, another user's private denied); test_get_entities_by_tag_admin_bypass_sees_all_tagged renamed to _excludes_private and flipped to assert private rows are filtered (the #4106 invariant that the admin-bypass branch is reached at all is still tested implicitly via the compiled predicate); completion/tag _apply_visibility_scope tests now pass db= (required after main moved the helper into BaseService); test_agent_card_admin_bypass_denies_private now exercises _check_agent_access directly; test_get_request_identity_* / test_get_scoped_resource_access_context_* patches now target mcpgateway.auth_context (a latent bug from the auth_context refactor that the rebase surfaced). Verified: 1982 tests pass across test_main, test_main_extended, test_admin, and the 8 service test modules. Signed-off-by: Jonathan Springer <jps@s390x.com>

Followup to PR #4341 review cycle 1. Six independent leak paths or stale test patches were missed when commit 94efc94 ('extend admin-bypass private-resource denial across all access paths') was originally written; the cumulative diff after rebase still permitted admin-bypass reads of other users' private resources via several auxiliary surfaces. Each fix below was identified by parallel Oracle audits and is verified with a focused regression test. B1: a2a_server_service._check_server_access (mcpgateway/services/a2a_server_service.py:36-64). The (None, None) anonymous-bypass branch returned True for any visibility, including private. This was reachable from the internal Rust fallback paths in main.py:8842-8845 and 8915-8916, so a trusted internal admin context with email=null, teams=null could fetch another user's private virtual-server card. Fix: change the bypass branch to 'return server.visibility != "private"', mirroring the hybrid policy in a2a_service._check_agent_access. The (email, None) DB-admin shape falls through to the natural flow which already grants public + team + own-private and denies others' private. Test: test_admin_bypass_denies_private + test_db_admin_with_email_sees_own_private. B2: a2a_service._visible_agent_ids (mcpgateway/services/a2a_service.py:381-389). The bypass check used is_admin_bypass_granted(...), which matches both (None, None) AND the (email, None) DB-admin shape. The docstring stated only (None, None) should be unscoped, but the implementation didn't agree, so DB admins received an unscoped None and could enumerate other users' private agents via list_tasks and list_push_configs_for_dispatch. Fix: replace the helper call with an explicit '(user_email is None and token_teams is None)' literal. The (email, None) shape now falls through to the SQL filter at lines 391-405, which already enforces public + team + own-private. Test: test_db_admin_with_email_runs_filtered_query in TestVisibleAgentIds (uses install_admin_user fixture). B3: token_storage_service._refresh_access_token (mcpgateway/services/token_storage_service.py:205). Refresh fetched a Gateway by ID with no visibility check, then read and decrypted gateway.oauth_config['client_secret']. If a token's gateway_id pointed to a private gateway whose owner had since changed (or had always been someone else), the OAuth client_secret was decrypted and forwarded to a non-owner. Fix: refuse refresh when 'visibility == private and owner_email != token_record.app_user_email'. The check is intentionally narrow (private gateways only) so it does not require team-membership queries — broader RBAC happens at the call sites that issue refreshes. The token owner identity is app_user_email (the ContextForge user), not user_id (the OAuth provider's user id). Tests: test_refresh_denied_for_private_gateway_with_other_owner + test_refresh_allowed_for_private_gateway_owned_by_token_owner. B4: utils/gateway_access.check_gateway_access (mcpgateway/utils/gateway_access.py:80-87). Used 'if is_admin_bypass_granted(...): return True' which granted both anonymous AND DB-admin bypass on private gateways — pre-PR-#4341 semantics. Fix: replace with the same two-branch hybrid pattern used elsewhere — anonymous bypass returns 'visibility != private'; DB-admin bypass adds an own-private carve-out. Tests: flipped test_admin_bypass_denies_private (was test_admin_bypass_with_none_token_teams), added test_admin_bypass_sees_team, rewrote test_database_admin_bypass to cover the own/others split, rewrote test_platform_admin_bypass for the same split. B5: resource_service.list_resource_templates (mcpgateway/services/resource_service.py:4054-4072). The bespoke admin-bypass branch only handled (None, None) and let the (email, None) DB-admin shape fall through to the elif which required token_teams to not be None — net result was no WHERE clause applied and all private templates returned. Fix: add a second elif covering the (email, None) DB-admin shape, applying 'visibility != private OR (visibility == private AND owner_email == user_email)' — same predicate used in BaseService._apply_access_control. Test: test_list_resource_templates_db_admin_includes_own_private_only. B6: stale test patches against removed 'mcpgateway.main._get_*' / '_has_valid_*' symbols (tests/unit/mcpgateway/test_internal_a2a_endpoints.py). After commit 939a1eadd moved the helpers to mcpgateway.auth_context, six patch sites still referenced the old underscored names: lines 172, 177 (was _get_rpc_filter_context) and 1122, 1133, 1170, 1176 (was _has_valid_internal_mcp_runtime_auth_header). All six patches raised AttributeError at test collection time. Fix: rewrite to the public name as imported into main.py's namespace ('mcpgateway.main.get_rpc_filter_context' / 'mcpgateway.main.has_valid_internal_mcp_runtime_auth_header'). Verified: 8417 tests pass across services/, utils/test_gateway_access.py, test_internal_a2a_endpoints.py, test_main.py, test_main_extended.py, test_admin.py. New regression tests added for B1, B2, B3, B5; tests flipped from old to new policy for B1 (admin_bypass_sees_private → denies_private), B4 (admin_bypass_with_none_token_teams → denies_private + sees_team, database_admin_bypass split into own/others, platform_admin_bypass split into own/others). Signed-off-by: Jonathan Springer <jps@s390x.com>

…dings Followup to #1cc1fe65a (the review-cycle-1 blocker batch). The cycle-1 review identified six blockers (all fixed in #1cc1fe65a) plus two real-but-non-blocking bugs and several test-coverage gaps. This commit closes both remaining real bugs and the highest-impact coverage gaps. Real bugs: - POST /prompts/{id} returned 422 instead of 404 on PromptNotFoundError. PromptNotFoundError is a subclass of PromptError, so 'isinstance(ex, (ValueError, PromptError))' matched first and emitted 422 — different from the GET endpoint at line 6208 which correctly maps to 404. The status divergence creates an existence oracle: an attacker can probe arbitrary prompt names via POST and infer existence from the status code. Fixed at mcpgateway/main.py:6155-6160 by adding an explicit PromptNotFoundError check ahead of the broader PromptError branch, mirroring the GET endpoint. - tests/unit/mcpgateway/test_internal_a2a_endpoints.py:851-868 ('test_card_server_fallback_returns_200') patched _check_agent_access to False but expected 200 from the server-fallback path — without setting any visibility on the server, the test passed vacuously regardless of policy. Renamed to '_public_returns_200' and added a sibling 'test_card_server_fallback_admin_bypass_denies_private' that sets visibility=private + owner_email=other and asserts 404, exercising the actual fallback path through A2AServerService._check_server_access (the helper B1 fixed). Coverage gaps closed: - tests/unit/mcpgateway/services/test_base_service.py: 'test_db_admin_bypass_includes_own_private_only' compiles the actual SQLAlchemy WHERE for the (email, None) DB-admin shape and asserts visibility/private/owner_email/admin@example.com all appear — same pattern as the existing 'test_completion_apply_visibility_scope_admin_bypass_excludes_private'. Also added 'test_non_admin_with_email_but_null_token_teams_does_not_bypass' covering the negative case (non-admin (email, None) must fall through to TeamManagementService lookup, not the bypass branch). - tests/unit/mcpgateway/test_main.py: 'test_post_prompt_with_args_not_found_returns_404' regression test for the POST 422->404 fix above. - tests/unit/mcpgateway/test_main.py: 'test_get_tool_admin_bypass_private_returns_404' covers the documented 404-not-403 endpoint behavior. The pattern is intentionally focused (one representative endpoint) — extending to /servers, /gateways, /resources/{uri}/info follows the same shape and can be added by reviewers if desired. - tests/unit/mcpgateway/services/test_authorization_access.py: 'test_tool_access_denied_emits_structured_log_event' patches mcpgateway.services.tool_service.structured_logger and asserts the deny-event shape (event_type='tool_access_denied', resource_type, resource_id, custom_fields with visibility + admin_bypass) so the CHANGELOG forensics claim is regression-tested. - tests/unit/mcpgateway/services/test_authorization_access.py: 'test_invoke_tool_private_denial_runs_before_pre_invoke_hook' patches the plugin manager's tool_pre_invoke hook and asserts it is NOT called when the tool is private and the caller is admin-bypass. This protects the critical hook-order property: visibility deny gates plugin side effects (logging, metrics, billing) so plugins do not leak existence/usage information for resources the caller cannot see. Verified: 8424 tests pass across services/, utils/test_gateway_access.py, test_internal_a2a_endpoints.py, test_main.py, test_main_extended.py, test_admin.py (up from 8417 in #1cc1fe65a; +7 new regression tests, no existing tests broken). Signed-off-by: Jonathan Springer <jps@s390x.com>

… accuracy, test hardening) Addresses findings from PR #4341 cycle-2 multi-agent review. Two real bugs and several test/doc hardening items, all isolated to test files plus one CHANGELOG paragraph and the auth_context module docstring. Vacuous test fix: test_invoke_tool_private_denial_runs_before_pre_invoke_hook patched the wrong mock target. It set 'tool_service._plugin_manager = mock_plugin_manager' and asserted '.tool_pre_invoke.assert_not_called()', but production tool_service.invoke_tool resolves the manager via 'await self._get_plugin_manager(...)' and dispatches via 'plugin_manager.invoke_hook(ToolHookType.TOOL_PRE_INVOKE, ...)'. The mocked attribute and method were both unobserved, so the test passed for any hook ordering — including a regression that called the real hook chain on a denied tool. Rewrote to patch '_get_plugin_manager' to return a manager whose 'invoke_hook' method is the AsyncMock under observation. Same exact bug class (vacuous test) that B6 fixed earlier in this PR. Stale docstring at test_authorization_access.py:1169: said 'get_agent_card itself is the unscoped fetcher — visibility is the caller's responsibility'. After cycle-2's S6-a refactor, get_agent_card gates internally and returns None on deny. Rewrote the docstring to reflect the new in-service enforcement point. CHANGELOG OAuth refresh overclaim: the cycle-2 paragraph on the (email, None) DB-admin own-private carve-out listed 'OAuth token refresh' and 'in-process service callers' as paths that fire it. Verified via code review that token_storage_service._refresh_access_token uses a direct owner check (not the hybrid branch). Narrowed the claim to the single verified path (the trusted internal A2A endpoint via main._get_internal_a2a_scope_context), with explicit qualifiers on what would and would not exercise the carve-out elsewhere. auth_context docstring polish: the cycle-2 S7-a rewrite said 'get_current_user is the single exception' to the 'pure primitives' rule in auth.py, but auth.py also has _inject_userinfo_instate and _propagate_tenant_id which take Request and stash payload state. The same docstring claimed the module 'depends only on mcpgateway.auth' but it also imports 'mcpgateway.config'. Both claims softened to be accurate. Test hardening (per cycle-2 efficacy review): - test_db_admin_with_email_runs_filtered_query (a2a_service): now compiles the second '.filter()' call and asserts the visibility predicate scopes private rows to 'owner_email = caller'. Without this, a regression that re-introduced the unscoped-None path could pass the prior 'result is not None' assertion. - test_list_resource_templates_db_admin_includes_own_private_only (authorization_access): switched from 'admin@example.com' (which trips settings.platform_admin_email and never exercises the DB-resolved code path) to 'dba@test.com' with an explicit 'patch is_user_admin to True'. Also asserts the WHERE clause has exactly one OR — multiple ORs would indicate a too-broad predicate. - test_db_admin_bypass_includes_own_private_only (base_service): same hardening — exact predicate substrings + exactly one OR. - test_get_{tool,server,gateway}_admin_bypass_private_returns_404 (test_main): added 'mock_get.assert_called_once()' so the route test proves the route actually called the patched service rather than just asserting the status code. Verified: 8426 tests pass across services/, utils/test_gateway_access.py, test_internal_a2a_endpoints.py, test_main.py, test_main_extended.py, test_admin.py. Signed-off-by: Jonathan Springer <jps@s390x.com>

Three classes of test breakage surfaced after running broader suites against the cycle-2 tip. None reflect a code regression — all are tests that referenced internal symbols renamed by the auth_context refactor, an admin endpoint signature that gained a Request parameter, or a token shape that PR #4341 deliberately stopped permitting. Class 1 (3 tests) — stale patches against renamed _get_* helpers, same class as cycle-1 B6: tests/unit/mcpgateway/test_main_helpers.py and test_main_helpers_extra.py. The auth_context refactor moved get_token_teams_from_request and get_rpc_filter_context out of main.py and dropped the leading underscore. Renamed all 10 references; main.py re-exports both names so the patch path still works. Class 2 (2 tests) — admin_get_server signature gained a Request parameter: tests/unit/mcpgateway/test_admin_module.py. PR #4341 added 'request: Request' to admin.admin_get_server so the route can resolve visibility scope via get_scoped_resource_access_context(request, user). Updated both call sites to pass _make_request() and broadened the _fake_get_server signatures with **_kwargs since admin_get_server now forwards user_email and token_teams kwargs. Class 3 (7 e2e tests) — visibility=private tripped the new public-only-token deny rule: tests/e2e/test_main_apis.py. Each test creates a resource with visibility=private then immediately reads/uses it via the same JWT. Because TEST_AUTH_HEADER uses generate_test_jwt() with default teams=[] and is_admin=False (public-only token shape), PR #4341 correctly denies private-row access even for own-private. The tests were pre-#4341 and used 'private' as a default that no longer permits the round trip. Switched all 7 to visibility=public so CRUD verification still works. Other tests in the same suites that exercise non-GET operations on private resources (test_create_server, test_update_server, test_set_server_state) are unchanged because their code paths don't pass through the visibility deny gate. Added inline comments documenting the policy boundary. Verified: 218 passed, 1 skipped across the 4 touched suites. All 12 original failures now green. Signed-off-by: Jonathan Springer <jps@s390x.com>

…lpers diff-cover surfaced five PR #4341 lines that no test exercised. All five are policy-critical paths in the new hybrid visibility helpers; without coverage, a regression that only broke one branch could ship undetected. Added 17 new tests across 4 files. Coverage gaps closed: - server_service.py:1022-1044 (was 35.7% diff coverage). New TestServerAccessCheckMatrix in test_authorization_access.py with 7 tests mirroring the TestCheckToolAccess pattern in test_tool_service.py. Each test names the production line(s) it covers (no-user-email deny, public-only token deny, own-private allow, JWT team match, DB team lookup, fall-through deny). - gateway_service.py:2734,2739-2761 (was 32.1%). New TestGatewayAccessCheckMatrix with the same 7-test shape; gateway and server helpers are sibling implementations of the canonical hybrid policy. - base_service.py:216 (was 90%). New test_apply_visibility_scope_db_admin_includes_own_private_only — the existing test_apply_visibility_scope_admin_bypass_excludes_private only covered (None, None); the (email, None) DB-admin branch was unexecuted. Uses the same exact-OR-count predicate guard as the other DB-admin tests so a too-broad predicate fails. - a2a_service.py:1155 (was 91.7%). New test_get_agent_card_returns_none_when_visibility_denies covers the deny path of the cycle-2 S6-a in-service gate. - auth_context.py:215 (was 99.1%). The non-object payload guard in decode_internal_mcp_auth_context had a test, but the test was named 'testdecode_*' (missing underscore separator) so pytest's default 'test_*' collection pattern rejected it — the assertion never ran. This is the same bug class as cycle-1 B6. Renamed to test_decode_internal_mcp_auth_context_rejects_non_object_payload with a docstring naming the bug class. Other testCASE-without-underscore names exist elsewhere (test_resource_service.py:2007/2024/2033, test_a2a_service.py:942, test_toolops_altk_service.py:59) but predate PR #4341 and are out of scope. Worth filing a separate cleanup issue. Verified: 8443 tests pass across services/, utils/test_gateway_access.py, test_internal_a2a_endpoints.py, test_main.py, test_main_extended.py, test_admin.py (+17 from this commit; +1 of those is the previously-broken testdecode_* now actually running). Signed-off-by: Jonathan Springer <jps@s390x.com>

…HTTP collapse test_admin_sees_all_servers asserted that an admin-bypass JWT (is_admin=true, teams=null) could see ALL servers including private — pre-PR-#4341 'admin sees everything' semantics. After cycle-2 S3-b documented in CHANGELOG, get_scoped_resource_access_context deliberately collapses HTTP admin requests to (None, None) so HTTP cannot be a stealthy escalation surface. The service layer then applies the anonymous-bypass rule and denies private rows even when the requesting admin is the owner — the (email, None) DB-admin own-private carve-out only fires from internal callers (Rust runtime hop) where the email is preserved through the call chain. Renamed to test_admin_sees_public_and_team_via_http and updated assertions: explicit in-set checks for public and team, plus an explicit not-in for private. The error message on the not-in assertion names PR #4341 and the collapse mechanism so a future regression that re-introduced the leak via HTTP would surface a self-explanatory failure. Other tests in TestServerVisibilityViaAPI (test_team_member_sees_public_and_team, test_outsider_sees_only_public, test_team_admin_sees_public_and_team) already followed the new policy by asserting only what each role can see — this test was the lone outlier. Signed-off-by: Jonathan Springer <jps@s390x.com>

- Modified service layer to deny admin bypass for private resources - Admin bypass now only grants access to public and team resources - Updated _apply_visibility_filter in base_service.py to exclude private resources - Updated _check_tool_access, _check_prompt_access, _check_resource_access methods - Added access control check in get_tool() service method - Fixed API endpoints to properly pass user_email when token_teams not set - Used sentinel value pattern to distinguish token_teams states - Updated all related unit tests to reflect new security behavior - Private resources now only accessible to their owners Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>

- Add test_get_prompt_admin_bypass_with_teams_none for GET /prompts/{id} admin bypass (lines 6451-6452) - Add test_get_prompt_with_args_admin_bypass for POST /prompts/get admin bypass (lines 6373-6374) - Add test_read_resource_admin_bypass for GET /resources/{id} admin bypass (lines 5839-5840) - Add test_get_tool_access_denied_raises_not_found for tool access denial (line 3061) - Fix syntax error in test_base_service.py (line 1) - Update .secrets.baseline These tests cover the admin bypass pattern where is_admin=True and token_teams=None grants unrestricted access to public and team resources (but NOT private resources). Also covers the access control path where get_tool raises ToolNotFoundError when access is denied to maintain security by not revealing tool existence. Improves diff coverage from 86% to 95%+ by covering previously untested authorization paths. Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>

… access paths Extends the original PR's service-level denial so admin bypass (JWT `is_admin=true` + `teams=null`, or non-JWT dev-mode admin) can never see or mutate another user's private resources across any surface. Service-layer access checks: - a2a_service._check_agent_access: admin bypass grants public + team access only, never private. get_agent_by_name / get_agent_card gain user_email / token_teams and enforce the check before returning. - server_service.get_server: new _check_server_access helper; generic ServerNotFoundError + server_access_denied structured log on denial. - gateway_service.get_gateway: new _check_gateway_access helper; same pattern. - prompt_service.get_prompt_details: gates private reads (admin UI path). - resource_service.get_resource_by_id: gates private reads (/resources/{id}/info and admin UI paths). - resource_service.list_resource_templates: admin bypass filters visibility != 'private' from the template enumeration. - tool_service.get_tool: adds token_teams parameter so Layer 1 visibility is evaluated against JWT-scoped teams rather than being widened to the caller's full DB team roles (fixes a scope-widening regression). - completion_service._apply_visibility_scope: admin bypass excludes private so completion suggestions cannot reveal private prompt / resource names. - tag_service._apply_visibility_scope: admin bypass excludes private so the tag enumeration cannot reveal other users' private entity metadata. Endpoint / routing rewire: - main.py: the three inline sentinel_unset blocks replaced with _get_scoped_resource_access_context(), the canonical helper already maintained in auth.py / main.py. Same helper is now threaded into /servers/{id}, /gateways/{id}, /resources/{id}/info, /tools/{id}, /resources/templates/list (REST + JSON-RPC + MCP dispatcher), and the internal A2A agent card path. - admin.py: the five admin detail endpoints (/admin/{servers,gateways, resources,prompts,tools}/{id}) now accept the FastAPI Request and resolve visibility scope via a lazy import of the same helper to avoid the main <-> admin circular import. - base_service._apply_access_control: admin-bypass branch still filters private rows out; docstring updated to state the invariant. Observability: - Each new deny path emits a structured event (`tool_access_denied`, `server_access_denied`, `gateway_access_denied`, `prompt_access_denied`, `resource_access_denied`) so forensics does not rely on HTTP logs. Tests: - New TestDirectGetAccessDenial in tests/unit/mcpgateway/services/ test_authorization_access.py: regressions for every new deny/allow path (server, gateway, a2a by name and card, prompt_details, resource_by_id, tool scope, template enumeration, completion + tag SQL predicates). - Route-level regressions in tests/unit/mcpgateway/test_main.py assert admin-bypass wiring for the REST and JSON-RPC template handlers. - Completion + tag visibility assertions compile the scoped statement with `literal_binds=True` and verify the rendered SQL contains the expected `visibility != 'private'` predicate. - Existing a2a / completion / tag tests that encoded the pre-#4341 admin-sees-everything behavior are updated to the new secure semantics. - Three test_main_extended callers that invoke delete_server / delete_gateway directly now supply a mock Request, matching the new route signatures. Docs / behavior notes: - CHANGELOG.md gains an [Unreleased] breaking-change entry with a migration note for integrators that relied on admin-bypass private reads. - docs/docs/manage/rbac.md access matrix updated: admin bypass no longer reads private, with an admonition pointing at this PR. - docs/docs/architecture/multitenancy.md reconciled with the same owner-only rule for private visibility. Follow-up (tracked separately, not in scope here): - A2A consistency cleanup for a2a_server_service._check_server_access and A2AAgentService.list_tasks -> #4437 Signed-off-by: Jonathan Springer <jps@s390x.com>

…dule Hoist Layer-1 visibility helpers (get_rpc_filter_context, get_scoped_resource_access_context, get_token_teams_from_request, get_request_identity, get_user_email, and the internal MCP runtime trust-header helpers) out of main.py into a new mcpgateway/auth_context.py module. admin.py previously reached back via a lazy 'from mcpgateway.main import _get_scoped_resource_access_context' guarded by '# pylint: disable=import-outside-toplevel', creating a static cyclic import (admin -> main -> admin) flagged by pylint R0401. The new module depends only on mcpgateway.auth, breaking the cycle architecturally instead of masking it. auth.py and auth_context.py now carry purpose docstrings documenting the split: auth.py owns the token/session/team model layer (no Request input, reusable from any context), while auth_context.py owns per-request resolution (takes Request, returns the (email, token_teams, is_admin) tuple and the (email, token_teams) scoped-access tuple). This mirrors the two-layer security model described in AGENTS.md. The helpers are now exported under their public (no-underscore) names; tests are updated to patch and reference them by their new names. The Black hook re-collapsed a few multi-line function calls in test_main.py and detect-secrets refreshed .secrets.baseline line numbers; both are byproducts of the rename and contain no behavior change. Signed-off-by: Jonathan Springer <jps@s390x.com>

After rebasing PR #4341 onto main, the post-#4107 admin-bypass detection (is_admin_bypass_granted) interacted with this PR's invariant in a way the original commits did not anticipate. Resolved per Oracle red-team review using the same two-branch hybrid pattern that a2a_service._visible_agent_ids already established: (None, None) anonymous bypass sees public + team only; (email, None) DB-resolved admin additionally sees the caller's own private rows; another user's private is still denied. Applied in BaseService._apply_access_control, BaseService._apply_visibility_scope (the shared helper used by completion and tag services on main), and the per-resource _check_*_access methods in tool/prompt/resource/a2a services. A2A signature reconciliation: get_agent_by_name now correctly awaits _check_agent_access(db, agent, ...) (the post-#4107 async signature), and get_agent_card delegates its visibility gate up to main.handle_a2a_agent_card so the sync method does not need to call the async helper. Test updates: per-resource test_check_*_access_database_admin_bypass and test_check_agent_access_db_admin_bypass_only_with_unrestricted_token now assert the hybrid (own private allowed, another user's private denied); test_get_entities_by_tag_admin_bypass_sees_all_tagged renamed to _excludes_private and flipped to assert private rows are filtered (the #4106 invariant that the admin-bypass branch is reached at all is still tested implicitly via the compiled predicate); completion/tag _apply_visibility_scope tests now pass db= (required after main moved the helper into BaseService); test_agent_card_admin_bypass_denies_private now exercises _check_agent_access directly; test_get_request_identity_* / test_get_scoped_resource_access_context_* patches now target mcpgateway.auth_context (a latent bug from the auth_context refactor that the rebase surfaced). Verified: 1982 tests pass across test_main, test_main_extended, test_admin, and the 8 service test modules. Signed-off-by: Jonathan Springer <jps@s390x.com>

Followup to PR #4341 review cycle 1. Six independent leak paths or stale test patches were missed when commit 94efc94 ('extend admin-bypass private-resource denial across all access paths') was originally written; the cumulative diff after rebase still permitted admin-bypass reads of other users' private resources via several auxiliary surfaces. Each fix below was identified by parallel Oracle audits and is verified with a focused regression test. B1: a2a_server_service._check_server_access (mcpgateway/services/a2a_server_service.py:36-64). The (None, None) anonymous-bypass branch returned True for any visibility, including private. This was reachable from the internal Rust fallback paths in main.py:8842-8845 and 8915-8916, so a trusted internal admin context with email=null, teams=null could fetch another user's private virtual-server card. Fix: change the bypass branch to 'return server.visibility != "private"', mirroring the hybrid policy in a2a_service._check_agent_access. The (email, None) DB-admin shape falls through to the natural flow which already grants public + team + own-private and denies others' private. Test: test_admin_bypass_denies_private + test_db_admin_with_email_sees_own_private. B2: a2a_service._visible_agent_ids (mcpgateway/services/a2a_service.py:381-389). The bypass check used is_admin_bypass_granted(...), which matches both (None, None) AND the (email, None) DB-admin shape. The docstring stated only (None, None) should be unscoped, but the implementation didn't agree, so DB admins received an unscoped None and could enumerate other users' private agents via list_tasks and list_push_configs_for_dispatch. Fix: replace the helper call with an explicit '(user_email is None and token_teams is None)' literal. The (email, None) shape now falls through to the SQL filter at lines 391-405, which already enforces public + team + own-private. Test: test_db_admin_with_email_runs_filtered_query in TestVisibleAgentIds (uses install_admin_user fixture). B3: token_storage_service._refresh_access_token (mcpgateway/services/token_storage_service.py:205). Refresh fetched a Gateway by ID with no visibility check, then read and decrypted gateway.oauth_config['client_secret']. If a token's gateway_id pointed to a private gateway whose owner had since changed (or had always been someone else), the OAuth client_secret was decrypted and forwarded to a non-owner. Fix: refuse refresh when 'visibility == private and owner_email != token_record.app_user_email'. The check is intentionally narrow (private gateways only) so it does not require team-membership queries — broader RBAC happens at the call sites that issue refreshes. The token owner identity is app_user_email (the ContextForge user), not user_id (the OAuth provider's user id). Tests: test_refresh_denied_for_private_gateway_with_other_owner + test_refresh_allowed_for_private_gateway_owned_by_token_owner. B4: utils/gateway_access.check_gateway_access (mcpgateway/utils/gateway_access.py:80-87). Used 'if is_admin_bypass_granted(...): return True' which granted both anonymous AND DB-admin bypass on private gateways — pre-PR-#4341 semantics. Fix: replace with the same two-branch hybrid pattern used elsewhere — anonymous bypass returns 'visibility != private'; DB-admin bypass adds an own-private carve-out. Tests: flipped test_admin_bypass_denies_private (was test_admin_bypass_with_none_token_teams), added test_admin_bypass_sees_team, rewrote test_database_admin_bypass to cover the own/others split, rewrote test_platform_admin_bypass for the same split. B5: resource_service.list_resource_templates (mcpgateway/services/resource_service.py:4054-4072). The bespoke admin-bypass branch only handled (None, None) and let the (email, None) DB-admin shape fall through to the elif which required token_teams to not be None — net result was no WHERE clause applied and all private templates returned. Fix: add a second elif covering the (email, None) DB-admin shape, applying 'visibility != private OR (visibility == private AND owner_email == user_email)' — same predicate used in BaseService._apply_access_control. Test: test_list_resource_templates_db_admin_includes_own_private_only. B6: stale test patches against removed 'mcpgateway.main._get_*' / '_has_valid_*' symbols (tests/unit/mcpgateway/test_internal_a2a_endpoints.py). After commit 939a1eadd moved the helpers to mcpgateway.auth_context, six patch sites still referenced the old underscored names: lines 172, 177 (was _get_rpc_filter_context) and 1122, 1133, 1170, 1176 (was _has_valid_internal_mcp_runtime_auth_header). All six patches raised AttributeError at test collection time. Fix: rewrite to the public name as imported into main.py's namespace ('mcpgateway.main.get_rpc_filter_context' / 'mcpgateway.main.has_valid_internal_mcp_runtime_auth_header'). Verified: 8417 tests pass across services/, utils/test_gateway_access.py, test_internal_a2a_endpoints.py, test_main.py, test_main_extended.py, test_admin.py. New regression tests added for B1, B2, B3, B5; tests flipped from old to new policy for B1 (admin_bypass_sees_private → denies_private), B4 (admin_bypass_with_none_token_teams → denies_private + sees_team, database_admin_bypass split into own/others, platform_admin_bypass split into own/others). Signed-off-by: Jonathan Springer <jps@s390x.com>

…dings Followup to #1cc1fe65a (the review-cycle-1 blocker batch). The cycle-1 review identified six blockers (all fixed in #1cc1fe65a) plus two real-but-non-blocking bugs and several test-coverage gaps. This commit closes both remaining real bugs and the highest-impact coverage gaps. Real bugs: - POST /prompts/{id} returned 422 instead of 404 on PromptNotFoundError. PromptNotFoundError is a subclass of PromptError, so 'isinstance(ex, (ValueError, PromptError))' matched first and emitted 422 — different from the GET endpoint at line 6208 which correctly maps to 404. The status divergence creates an existence oracle: an attacker can probe arbitrary prompt names via POST and infer existence from the status code. Fixed at mcpgateway/main.py:6155-6160 by adding an explicit PromptNotFoundError check ahead of the broader PromptError branch, mirroring the GET endpoint. - tests/unit/mcpgateway/test_internal_a2a_endpoints.py:851-868 ('test_card_server_fallback_returns_200') patched _check_agent_access to False but expected 200 from the server-fallback path — without setting any visibility on the server, the test passed vacuously regardless of policy. Renamed to '_public_returns_200' and added a sibling 'test_card_server_fallback_admin_bypass_denies_private' that sets visibility=private + owner_email=other and asserts 404, exercising the actual fallback path through A2AServerService._check_server_access (the helper B1 fixed). Coverage gaps closed: - tests/unit/mcpgateway/services/test_base_service.py: 'test_db_admin_bypass_includes_own_private_only' compiles the actual SQLAlchemy WHERE for the (email, None) DB-admin shape and asserts visibility/private/owner_email/admin@example.com all appear — same pattern as the existing 'test_completion_apply_visibility_scope_admin_bypass_excludes_private'. Also added 'test_non_admin_with_email_but_null_token_teams_does_not_bypass' covering the negative case (non-admin (email, None) must fall through to TeamManagementService lookup, not the bypass branch). - tests/unit/mcpgateway/test_main.py: 'test_post_prompt_with_args_not_found_returns_404' regression test for the POST 422->404 fix above. - tests/unit/mcpgateway/test_main.py: 'test_get_tool_admin_bypass_private_returns_404' covers the documented 404-not-403 endpoint behavior. The pattern is intentionally focused (one representative endpoint) — extending to /servers, /gateways, /resources/{uri}/info follows the same shape and can be added by reviewers if desired. - tests/unit/mcpgateway/services/test_authorization_access.py: 'test_tool_access_denied_emits_structured_log_event' patches mcpgateway.services.tool_service.structured_logger and asserts the deny-event shape (event_type='tool_access_denied', resource_type, resource_id, custom_fields with visibility + admin_bypass) so the CHANGELOG forensics claim is regression-tested. - tests/unit/mcpgateway/services/test_authorization_access.py: 'test_invoke_tool_private_denial_runs_before_pre_invoke_hook' patches the plugin manager's tool_pre_invoke hook and asserts it is NOT called when the tool is private and the caller is admin-bypass. This protects the critical hook-order property: visibility deny gates plugin side effects (logging, metrics, billing) so plugins do not leak existence/usage information for resources the caller cannot see. Verified: 8424 tests pass across services/, utils/test_gateway_access.py, test_internal_a2a_endpoints.py, test_main.py, test_main_extended.py, test_admin.py (up from 8417 in #1cc1fe65a; +7 new regression tests, no existing tests broken). Signed-off-by: Jonathan Springer <jps@s390x.com>

… in-service gate Two safety improvements identified by review-cycle-1 oracle audit (suggestions S5-a, S6-a) plus the privatization of an internal-only helper. No behavior change for the canonical hybrid policy; all changes prevent classes of future regressions. S5-a (inline is_user_admin): the second hybrid branch in tool/prompt/resource/a2a/base services and utils/gateway_access.check_gateway_access used 'is_admin_bypass_granted(db, user_email, token_teams)'. The preceding clauses ('token_teams is None and user_email and ...') already exclude the (None, None) anonymous-bypass shape, so the helper call is effectively 'is_user_admin(db, user_email)'. The broader name invites the exact misuse that produced the original B2 bug in _visible_agent_ids (where is_admin_bypass_granted matched both shapes and bypassed the per-agent SQL filter for DB admins). Inlined is_user_admin at all 8 call sites across 6 files. The only remaining caller of is_admin_bypass_granted is mcpgateway/main.py:5817 (SSE generator), where the broader semantics are intentional. S6-a (get_agent_card async + in-service gate): get_agent_card was the lone outlier among the visibility-gated service methods after the rebase fixup commit 17a79ff29 — it was a sync unscoped fetcher with the gate moved up to the call site in main.handle_internal_a2a_agent_card. Every other service (tool_service.get_tool, prompt_service.get_prompt_details, resource_service.get_resource_by_id, server_service.get_server, gateway_service.get_gateway, a2a_service.get_agent_by_name) gates inside the service. A future caller of get_agent_card who forgot the upstream gate would have re-introduced a private-agent leak. Made get_agent_card async, added user_email and token_teams parameters, and pushed the await self._check_agent_access(...) call back inside the method. Updated main.handle_internal_a2a_agent_card to call the new signature and removed its now-redundant explicit gate. Test patches updated to use new_callable=AsyncMock; two tests in test_a2a_service.py now await the call. Privatize has_verified_jwt_payload: the helper at mcpgateway/auth_context.py:369 was declared in the public surface of the module docstring but is only consumed inside auth_context.py itself (lines 400 and 438). Renamed to _has_verified_jwt_payload and moved into the private-surface section of the module docstring. No external callers. Verified: 8426 tests pass across services/, utils/test_gateway_access.py, test_internal_a2a_endpoints.py, test_main.py, test_main_extended.py, test_admin.py. No new test failures. Signed-off-by: Jonathan Springer <jps@s390x.com>

…4 regressions S3-b: the cycle-1 review noted that the CHANGELOG promised a DB-admin own-private carve-out ("Resource owners can still access their own private resources, including DB-resolved admins viewing their own private rows") but that this carve-out is mostly unreachable from public HTTP routes because mcpgateway.auth_context.get_scoped_resource_access_context collapses HTTP admin requests to (None, None). Tightened the CHANGELOG paragraph to scope the carve-out claim to internal/non-HTTP call paths (Rust runtime hop, OAuth refresh, in-process service callers) and added an explicit note that HTTP admin requests are intentionally collapsed by design. Updated the 'What's unchanged' bullet to match. Endpoint coverage: added test_get_server_admin_bypass_private_returns_404 and test_get_gateway_admin_bypass_private_returns_404 to extend the pattern already in place for /tools/{id}. Each test mocks the service to raise the appropriate *NotFoundError and asserts that the route maps to 404 (not 403), preserving the deliberate existence-non-disclosure behavior documented in the CHANGELOG. Verified: 8426 tests pass across the touched suites. Signed-off-by: Jonathan Springer <jps@s390x.com>

… accuracy, test hardening) Addresses findings from PR #4341 cycle-2 multi-agent review. Two real bugs and several test/doc hardening items, all isolated to test files plus one CHANGELOG paragraph and the auth_context module docstring. Vacuous test fix: test_invoke_tool_private_denial_runs_before_pre_invoke_hook patched the wrong mock target. It set 'tool_service._plugin_manager = mock_plugin_manager' and asserted '.tool_pre_invoke.assert_not_called()', but production tool_service.invoke_tool resolves the manager via 'await self._get_plugin_manager(...)' and dispatches via 'plugin_manager.invoke_hook(ToolHookType.TOOL_PRE_INVOKE, ...)'. The mocked attribute and method were both unobserved, so the test passed for any hook ordering — including a regression that called the real hook chain on a denied tool. Rewrote to patch '_get_plugin_manager' to return a manager whose 'invoke_hook' method is the AsyncMock under observation. Same exact bug class (vacuous test) that B6 fixed earlier in this PR. Stale docstring at test_authorization_access.py:1169: said 'get_agent_card itself is the unscoped fetcher — visibility is the caller's responsibility'. After cycle-2's S6-a refactor, get_agent_card gates internally and returns None on deny. Rewrote the docstring to reflect the new in-service enforcement point. CHANGELOG OAuth refresh overclaim: the cycle-2 paragraph on the (email, None) DB-admin own-private carve-out listed 'OAuth token refresh' and 'in-process service callers' as paths that fire it. Verified via code review that token_storage_service._refresh_access_token uses a direct owner check (not the hybrid branch). Narrowed the claim to the single verified path (the trusted internal A2A endpoint via main._get_internal_a2a_scope_context), with explicit qualifiers on what would and would not exercise the carve-out elsewhere. auth_context docstring polish: the cycle-2 S7-a rewrite said 'get_current_user is the single exception' to the 'pure primitives' rule in auth.py, but auth.py also has _inject_userinfo_instate and _propagate_tenant_id which take Request and stash payload state. The same docstring claimed the module 'depends only on mcpgateway.auth' but it also imports 'mcpgateway.config'. Both claims softened to be accurate. Test hardening (per cycle-2 efficacy review): - test_db_admin_with_email_runs_filtered_query (a2a_service): now compiles the second '.filter()' call and asserts the visibility predicate scopes private rows to 'owner_email = caller'. Without this, a regression that re-introduced the unscoped-None path could pass the prior 'result is not None' assertion. - test_list_resource_templates_db_admin_includes_own_private_only (authorization_access): switched from 'admin@example.com' (which trips settings.platform_admin_email and never exercises the DB-resolved code path) to 'dba@test.com' with an explicit 'patch is_user_admin to True'. Also asserts the WHERE clause has exactly one OR — multiple ORs would indicate a too-broad predicate. - test_db_admin_bypass_includes_own_private_only (base_service): same hardening — exact predicate substrings + exactly one OR. - test_get_{tool,server,gateway}_admin_bypass_private_returns_404 (test_main): added 'mock_get.assert_called_once()' so the route test proves the route actually called the patched service rather than just asserting the status code. Verified: 8426 tests pass across services/, utils/test_gateway_access.py, test_internal_a2a_endpoints.py, test_main.py, test_main_extended.py, test_admin.py. Signed-off-by: Jonathan Springer <jps@s390x.com>

Three classes of test breakage surfaced after running broader suites against the cycle-2 tip. None reflect a code regression — all are tests that referenced internal symbols renamed by the auth_context refactor, an admin endpoint signature that gained a Request parameter, or a token shape that PR #4341 deliberately stopped permitting. Class 1 (3 tests) — stale patches against renamed _get_* helpers, same class as cycle-1 B6: tests/unit/mcpgateway/test_main_helpers.py and test_main_helpers_extra.py. The auth_context refactor moved get_token_teams_from_request and get_rpc_filter_context out of main.py and dropped the leading underscore. Renamed all 10 references; main.py re-exports both names so the patch path still works. Class 2 (2 tests) — admin_get_server signature gained a Request parameter: tests/unit/mcpgateway/test_admin_module.py. PR #4341 added 'request: Request' to admin.admin_get_server so the route can resolve visibility scope via get_scoped_resource_access_context(request, user). Updated both call sites to pass _make_request() and broadened the _fake_get_server signatures with **_kwargs since admin_get_server now forwards user_email and token_teams kwargs. Class 3 (7 e2e tests) — visibility=private tripped the new public-only-token deny rule: tests/e2e/test_main_apis.py. Each test creates a resource with visibility=private then immediately reads/uses it via the same JWT. Because TEST_AUTH_HEADER uses generate_test_jwt() with default teams=[] and is_admin=False (public-only token shape), PR #4341 correctly denies private-row access even for own-private. The tests were pre-#4341 and used 'private' as a default that no longer permits the round trip. Switched all 7 to visibility=public so CRUD verification still works. Other tests in the same suites that exercise non-GET operations on private resources (test_create_server, test_update_server, test_set_server_state) are unchanged because their code paths don't pass through the visibility deny gate. Added inline comments documenting the policy boundary. Verified: 218 passed, 1 skipped across the 4 touched suites. All 12 original failures now green. Signed-off-by: Jonathan Springer <jps@s390x.com>

…lpers diff-cover surfaced five PR #4341 lines that no test exercised. All five are policy-critical paths in the new hybrid visibility helpers; without coverage, a regression that only broke one branch could ship undetected. Added 17 new tests across 4 files. Coverage gaps closed: - server_service.py:1022-1044 (was 35.7% diff coverage). New TestServerAccessCheckMatrix in test_authorization_access.py with 7 tests mirroring the TestCheckToolAccess pattern in test_tool_service.py. Each test names the production line(s) it covers (no-user-email deny, public-only token deny, own-private allow, JWT team match, DB team lookup, fall-through deny). - gateway_service.py:2734,2739-2761 (was 32.1%). New TestGatewayAccessCheckMatrix with the same 7-test shape; gateway and server helpers are sibling implementations of the canonical hybrid policy. - base_service.py:216 (was 90%). New test_apply_visibility_scope_db_admin_includes_own_private_only — the existing test_apply_visibility_scope_admin_bypass_excludes_private only covered (None, None); the (email, None) DB-admin branch was unexecuted. Uses the same exact-OR-count predicate guard as the other DB-admin tests so a too-broad predicate fails. - a2a_service.py:1155 (was 91.7%). New test_get_agent_card_returns_none_when_visibility_denies covers the deny path of the cycle-2 S6-a in-service gate. - auth_context.py:215 (was 99.1%). The non-object payload guard in decode_internal_mcp_auth_context had a test, but the test was named 'testdecode_*' (missing underscore separator) so pytest's default 'test_*' collection pattern rejected it — the assertion never ran. This is the same bug class as cycle-1 B6. Renamed to test_decode_internal_mcp_auth_context_rejects_non_object_payload with a docstring naming the bug class. Other testCASE-without-underscore names exist elsewhere (test_resource_service.py:2007/2024/2033, test_a2a_service.py:942, test_toolops_altk_service.py:59) but predate PR #4341 and are out of scope. Worth filing a separate cleanup issue. Verified: 8443 tests pass across services/, utils/test_gateway_access.py, test_internal_a2a_endpoints.py, test_main.py, test_main_extended.py, test_admin.py (+17 from this commit; +1 of those is the previously-broken testdecode_* now actually running). Signed-off-by: Jonathan Springer <jps@s390x.com>

…HTTP collapse test_admin_sees_all_servers asserted that an admin-bypass JWT (is_admin=true, teams=null) could see ALL servers including private — pre-PR-#4341 'admin sees everything' semantics. After cycle-2 S3-b documented in CHANGELOG, get_scoped_resource_access_context deliberately collapses HTTP admin requests to (None, None) so HTTP cannot be a stealthy escalation surface. The service layer then applies the anonymous-bypass rule and denies private rows even when the requesting admin is the owner — the (email, None) DB-admin own-private carve-out only fires from internal callers (Rust runtime hop) where the email is preserved through the call chain. Renamed to test_admin_sees_public_and_team_via_http and updated assertions: explicit in-set checks for public and team, plus an explicit not-in for private. The error message on the not-in assertion names PR #4341 and the collapse mechanism so a future regression that re-introduced the leak via HTTP would surface a self-explanatory failure. Other tests in TestServerVisibilityViaAPI (test_team_member_sees_public_and_team, test_outsider_sees_only_public, test_team_admin_sees_public_and_team) already followed the new policy by asserting only what each role can see — this test was the lone outlier. Signed-off-by: Jonathan Springer <jps@s390x.com>

…-deny rule Closes #4437 Changes: - Modified A2AAgentService._visible_agent_ids to filter out private agents during admin bypass (user_email=None, token_teams=None) - Previously returned None (unrestricted access), now returns filtered list of public + team agent IDs only - Prevents list_tasks from exposing tasks on private agents via admin bypass - Verified a2a_server_service._check_server_access already correct (no changes) Testing: - Added 14 new tests in test_a2a_authorization_access.py covering both _check_server_access and _visible_agent_ids with all visibility scenarios - Updated test_a2a_service.py::test_admin_bypass_returns_none to expect filtered list instead of None - All tests pass Security Impact: - Aligns with PR #4341 invariant: admin bypass must not grant visibility to another user's private resources - No live endpoint currently hits these paths with admin-bypass, so this is a defensive alignment preventing future bypass vulnerabilities Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>

bogdanmariusc10 requested review from brian-hussey, crivetimihai, kevalmahajan and madhav165 as code owners April 20, 2026 11:54

bogdanmariusc10 added security Improves security rbac Role-based Access Control labels Apr 20, 2026

bogdanmariusc10 linked an issue Apr 20, 2026 that may be closed by this pull request

[ICACF-21] [Security][Pen Testing] Restrict outbound requests from the gateway test endpoint to approved hosts #4323

Closed

bogdanmariusc10 added MUST P1: Non-negotiable, critical requirements without which the product is non-functional or unsafe api REST API Related item release-fix Critical bugfix required for the release pentesting labels Apr 20, 2026

jonpspri mentioned this pull request Apr 24, 2026

[CHORE]: Align a2a_server_service and A2AAgentService.list_tasks with post-#4341 admin-bypass private-deny rule #4437

Open

jonpspri force-pushed the 4323-icacf-21-securitypen-testing-restrict-outbound-requests-from-the-gateway-test-endpoint-to-approved-hosts branch from 35f96a4 to 94efc94 Compare April 24, 2026 13:26

jonpspri mentioned this pull request Apr 26, 2026

[CHORE]: Unify user-email extraction across auth helpers (email vs sub precedence drift) #4442

Open

3 tasks

jonpspri force-pushed the 4323-icacf-21-securitypen-testing-restrict-outbound-requests-from-the-gateway-test-endpoint-to-approved-hosts branch from 94efc94 to 6694771 Compare April 26, 2026 13:10

Bogdan-Marius-Catanus and others added 5 commits April 26, 2026 14:12

jonpspri added 8 commits April 26, 2026 14:13

jonpspri force-pushed the 4323-icacf-21-securitypen-testing-restrict-outbound-requests-from-the-gateway-test-endpoint-to-approved-hosts branch from 6694771 to 628451c Compare April 26, 2026 13:18

jonpspri merged commit 2f7ab34 into main Apr 26, 2026
30 checks passed

jonpspri deleted the 4323-icacf-21-securitypen-testing-restrict-outbound-requests-from-the-gateway-test-endpoint-to-approved-hosts branch April 26, 2026 13:28

bogdanmariusc10 mentioned this pull request Apr 29, 2026

fix(security): align A2A service with post-#4341 admin-bypass private-deny rule #4513

Open

10 tasks

cafalchio mentioned this pull request Apr 29, 2026

[TESTING]: Manual test and test coverage on rbac #4515

Open

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(security): prevent admin bypass from accessing private resources#4341

fix(security): prevent admin bypass from accessing private resources#4341
jonpspri merged 13 commits intomainfrom
4323-icacf-21-securitypen-testing-restrict-outbound-requests-from-the-gateway-test-endpoint-to-approved-hosts

bogdanmariusc10 commented Apr 20, 2026

Uh oh!

jonpspri commented Apr 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bogdanmariusc10 commented Apr 20, 2026

🔗 Related Issue

📝 Summary

🏷️ Type of Change

🧪 Verification

✅ Checklist

📓 Notes

Uh oh!

jonpspri commented Apr 24, 2026

What the follow-up commit adds

Deferred (tracked separately)

Suggested verification

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants