[prakhar-singh1928]

Pull Request

🔗 Related Issue

Closes: https://github.ibm.com/contextforge-org/internal_issues/issues/1134

---

📝 Summary

POST /v1/import with conflict_strategy=update overwrote tools, resources, prompts and gateways that belong to other teams.

Root causes:

• register_tools_bulk, register_resources_bulk and register_prompts_bulk looked up public rows by name with no tenant predicate. They changed the matched row without an ownership check.
• ImportService called update_gateway() without user_email, which skipped its ownership guard.
• The admin.import routes had no global_only=True, so check_any_team let a team-scoped role satisfy them.
• The Admin UI import route passed a username, or "unknown", as imported_by.

Changes:

• Add user_email to the three bulk registration methods. Before each update, run check_resource_ownership(). Report Permission denied for rows the caller does not own.
• Reject bulk updates that carry no caller identity.
• Pass the importer identity from ImportService to the bulk methods and to update_gateway().
• Set global_only=True on the four admin.import routes.
• Pass the authenticated email as imported_by in the Admin UI import.
• Add a Security entry to CHANGELOG.md.

Behavior change: a team-scoped role that holds admin.import now receives HTTP 403. Operators must move that grant to a global role.

---

📏 Reviewability

• This PR has one clear purpose
• The linked issue is not labeled triage
• Unrelated bugs or improvements are tracked in separate issues/PRs
• Tests are included with the code they validate
• If AI-assisted, I understand and can explain the generated changes

---

🏷️ Type of Change

• Bug fix
• Feature / Enhancement
• Documentation
• Refactor
• Chore (deps, CI, tooling)
• Other (describe below)

---

🧪 Verification

Targeted run of the affected suites:

.venv/bin/python -m pytest \
  tests/unit/mcpgateway/middleware/test_rbac.py \
  tests/unit/mcpgateway/services/test_import_service.py \
  tests/unit/mcpgateway/test_admin_import_export.py \
  tests/unit/mcpgateway/test_admin.py \
  tests/unit/mcpgateway/test_main_extended.py \
  tests/unit/mcpgateway/services/test_tool_service.py \
  tests/unit/mcpgateway/services/test_tool_service_coverage.py \
  tests/unit/mcpgateway/services/test_resource_service.py \
  tests/unit/mcpgateway/services/test_prompt_service.py \
  tests/unit/mcpgateway/db/test_resource_namespacing.py
  

Result: 3632 passed, 10 skipped, 1 failed.
The failure is test_get_prompt_post_hook_policy_does_not_duplicate_prompt_messages. It fails the same way on unmodified main when run with these files, and passes when run alone.

Deny-path regression tests added:

• Non-owner and missing-caller bulk update for tools, resources and prompts.
• Owner bulk update for tools.
• ImportService passes user_email to the bulk methods and to update_gateway().
• All four admin.import routes call the permission check with team_id=None and check_any_team=False.

Check	Command	Status
Lint suite	make lint	Not run. ruff check reports the same 8 findings on unmodified main.
Unit tests	make test	Targeted suites above pass, apart from the one failure described above.
Coverage ≥ 80%	make coverage	Not run

Live-gateway black-box test: pending. It reproduces the two-team scenario from the issue against a running gateway.

---

✅ Checklist

• Code formatted (make black isort pre-commit)
• Tests added/updated for changes
• Documentation updated (if applicable)
• No secrets or credentials committed

---

📓 Notes (optional)

• Not in scope: routers/toolops_router.py gates enrich_a_tool on admin.system_config. toolops_altk_service.py then calls update_tool() without user_email. That path rewrites only name and description. Track it in a separate issue.
• Five existing tests called bulk update with no caller identity. They now pass an authorized caller. This is the intended behavior change.
• ruff findings and black formatting differences exist in the touched files on unmodified main. This PR does not change them.

[Lang-Akshay]

LGTM ✅

Area	Rating	Key Issue
Blocking Changes	0	None
Security	🟢 Clear	SECURITY_VERDICT=CLEAR; cross-tenant update vulnerability resolved
Issue Coverage	🟢 Complete	Linked issue requirements fully satisfied
Code Quality	🟢 High	Clean implementation, adheres to coding standards
Breaking Change	Yes	Documented requirement to grant admin.import globally
Scope	🟢 In scope	All modifications strictly target import ownership enforcement
Test Context	Excluded	Test contents were not loaded

[msureshkumar88]

The ownership guards and authenticated email propagation address the reported issue. No schema migration is required. Two blockers remain.

blocking | mcpgateway/services/tool_service.py:2654 | Separate conflict queries permit an unchecked cross-owner overwrite during concurrent publication (CWE-367/CWE-863) | Load conflicts once, authorize those rows, and mutate the same checked row map.

`_find_unowned_tool_names()` checks one snapshot. `_process_tool_chunk()` queries again at line 2798. A tool published between these queries is absent from `unowned_names`. The update at line 2952 therefore proceeds without checking its ownership. I reproduced this with two real SQLite sessions and the actual bulk method. The publisher commits immediately before the second query. The result reports `updated=1`, `failed=0`, and zero ownership checks. The persisted URL changes while `owner_email` remains the victim's email. Audit and cache side effects are mocked; the ownership spy denies if called. Consolidate loading and authorization within one asynchronous chunk operation. Reject new collisions instead of reloading unchecked rows. This also removes the redundant conflict query.

blocking | tests/unit/mcpgateway/services/test_tool_service.py:9593 | Security tests mock ownership decisions and omit the required live-gateway regression | Add database-backed authorization tests and reproduce the two-team scenario against a running gateway.

The current tool tests return the same rows for both queries and miss the race above. The route test at `tests/unit/mcpgateway/middleware/test_rbac.py:2436` forces permission denial and verifies decorator arguments. The gateway test verifies identity forwarding rather than persisted denial. Add real role grants and authenticated requests. Verify that denied imports preserve victim fields and ownership across tools, resources, prompts, and gateways. Include unauthenticated requests, insufficient permissions, token narrowing, authorized owner/admin updates, and concurrent tool publication. The PR marks its required live-gateway test as pending.

Validation: two focused selections pass, with 79 and 207 tests respectively; the selections overlap. The deterministic race reproducer confirms the persisted unauthorized overwrite. Full validation and a live HTTP reproduction remain unchecked.

[msureshkumar88]

What this change does: Configuration import now checks ownership before updating existing tools, resources, prompts, and gateways. REST import routes require global permission grants, and Admin UI import passes the authenticated email.

The previously reproduced tool publication race is closed. Authorization and mutation use the same loaded rows. Late collisions cannot introduce an unchecked update.

Database-backed tests verify denial, unchanged ownership, and owner/admin success for tools, resources, and prompts. Four black-box tests pass against running gateway subprocesses. They cover direct-write denial, team-scoped import denial, cross-team import denial, and owner/admin import success.

Validation at 1784350:

• Focused selection: 276 passed.
• Broader affected suites: 3,636 passed, 10 skipped, one existing plugin test failed.
• Unchanged base d2d1424: 3,620 passed, 10 skipped, the same plugin test failed.
• New live-gateway tests: four passed.
• Changed application files and the new live test pass Ruff. Existing test-file lint findings also occur on the base.
• GitHub checks: 35 successful, five skipped, no failing or pending checks.

No remaining blocking finding in the reviewed diff. Code review approved.

Not checked: the complete local pre-merge validation gate or production PostgreSQL concurrency. This approval does not waive the AGENTS.md merge gate.

[brian-hussey]

LGTM

[brian-hussey]

LGTM