Skip to content

Connector seed migrations' downgrade() lacks a provenance guard on the public_mcp_apps row #1559

Description

@yiboyasss

Context

Found and fixed for Salesforce's own seed migration while addressing PR #1459 review feedback (commit range up to 2eac27d). The same template is used by other connectors' seed migrations and has the identical gap.

The bug

Each connector's seed migration (src/xagent/migrations/versions/*_seed_*_mcp_app.py) follows this pattern in downgrade():

if "public_mcp_apps" in existing_tables:
    bind.execute(
        sa.delete(PUBLIC_MCP_APPS_TABLE).where(PUBLIC_MCP_APPS_TABLE.c.app_id == APP_ID)
    )

This deletes the row unconditionally by app_id, with no check that the row actually matches the shape this migration seeded -- unlike the matching oauth_providers deletion further down, which does guard on the seeded row's stable fields (name/auth_url/token_url) specifically so an admin-created provider row survives a downgrade.

Concretely: upgrade() skips inserting when app_id already exists (so a pre-existing operator row is left alone), but downgrade() doesn't check the same thing before deleting -- it just deletes whatever currently has that app_id, whether or not this migration is the one that put it there. Two consequences:

  1. A pre-existing operator row that happened to already occupy that app_id before the migration ever ran gets deleted on downgrade even though upgrade() never touched it.
  2. That deletion can then make the "any apps still using this provider?" count (used to decide whether to also delete the oauth_providers row) read as zero, letting the provider row get deleted too.

What was fixed

src/xagent/migrations/versions/20260818_seed_salesforce_mcp_app.py's downgrade() now guards the public_mcp_apps deletion the same way the oauth_providers deletion already was (match on name/transport/provider_name, all stable and non-env-dependent).

Where this pattern likely still exists

At minimum confirmed in src/xagent/migrations/versions/20260818_seed_jira_mcp_app.py's downgrade(), same shape. Likely present in every other connector's seed migration that follows this template (zoom, intercom, slack, chrome, hubspot/docs/slides, etc.) -- worth an audit pass across all of them rather than patching one at a time, since this is a template-level gap, not connector-specific logic.

Not fixed for jira or the others as part of #1459 since those migrations belong to different, already-merged connectors this PR doesn't otherwise touch.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions