Repository navigation
fix(integrations): PUT de configuração grava só os campos editáveis (CRM-873) - #85
Merged
Merged
Conversation
…(CRM-873) The PUT merged the whole request body over the stored config and only kept the sensitive fields the body left out, so a body carrying access_token, refresh_token, client_secret, pkce_verifiers, oauth_metadata, mcp_url or headers replaced them. The ten MCP integrations now take only their closed list of editable fields (tools) from the body; the rest is written by the OAuth callback.
Reviewer's GuideO PR corrige a gravação de configuração das integrações ao trocar o merge amplo do corpo por uma lista fechada de campos editáveis. Para os dez provedores MCP, apenas tools pode ser alterado pelo PUT; credenciais e metadados permanecem provenientes do callback OAuth, enquanto os endpoints Google continuam protegidos pelos modelos Pydantic. Novos testes cobrem tanto a rejeição silenciosa de campos sensíveis quanto a manutenção do comportamento editável. Sequence diagram for the protected integration configuration PUTsequenceDiagram
participant Client
participant ProviderRoute
participant merge_editable_config
participant ConfigStore
Client->>ProviderRoute: PUT configuration
ProviderRoute->>ConfigStore: Load stored_config
ConfigStore-->>ProviderRoute: stored_config
ProviderRoute->>merge_editable_config: merge_editable_config(provider, stored_config, incoming)
merge_editable_config-->>ProviderRoute: stored_config plus incoming tools only
ProviderRoute->>ProviderRoute: Set connected from access_token
ProviderRoute->>ConfigStore: Save updated_config
ConfigStore-->>Client: Updated configuration
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/api/mcp_integration_base.py" line_range="51" />
<code_context>
+ incoming: Optional[Dict[str, Any]],
+) -> Dict[str, Any]:
+ """Stored config with only the provider's editable fields taken from the request body."""
+ incoming = incoming if isinstance(incoming, dict) else {}
+ editable = {k: incoming[k] for k in EDITABLE_CONFIG_FIELDS[provider] if k in incoming}
+ return {**stored_config, **editable}
</code_context>
<issue_to_address>
**Malformed config reports successful save**
When the request's nested `config` value is not a JSON object, `merge_editable_config` replaces a non-object `incoming` value with `{}`, so the route persists the unchanged configuration and returns success. Callers sending a malformed `config` value see a successful save even though their changes were discarded.
Reject a non-object nested config with a client error instead of silently treating it as an empty update.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: src/api/mcp_integration_base.py:51
…object (CRM-873) The body is now a model with a config object, so a list, string or null config is a 422 instead of a silent no-op. The editable-fields comment no longer lists headers, which the stored config never holds.
…h the core API (CRM-873) PKCE verifiers, registered client credentials, tokens from the callback and refreshed tokens of the MCP integrations, and the Google credential rows, were saved through POST /agents/:id/integrations of the core whenever no session was passed. The core now ignores OAuth fields sent to that route, so these writes go to the database, opening a session when the caller has none, and a failed write raises instead of falling back. The read before each read-modify-write raises on a database error rather than reading as an empty config.
…he core API (CRM-873)
sourcery-ai
Bot
dismissed
their stale review
November 6, 2026 14:07
Sourcery withdrew this approval because the latest commits introduced blocking findings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
O PUT de configuração das integrações MCP (asana, atlassian, canva, github, hubspot, linear, monday, notion, paypal e supabase) passa a gravar só os campos que o usuário edita. Ele parte da config gravada e copia do corpo apenas os campos de uma lista fechada por provedor (
EDITABLE_CONFIG_FIELDSemmcp_integration_base.py), que hoje étoolsnos dez. O resto da config continua sendo escrito pelo fluxo de OAuth. Oconnectedsegue calculado a partir do token gravado.IntegrationConfigUpdate). Umconfigque não é objeto devolve 422 em vez de responder 200 sem salvar nada.Estado de OAuth direto no banco. O verifier de PKCE, as credenciais do client registrado, os tokens do callback e do refresh das integrações MCP, e as linhas
google_*_credentials, eram salvos peloPOST /agents/:id/integrationsdo core sempre que não havia sessão. O core passa a ignorar campos de OAuth nessa rota (evo-ai-core-service-community#56), então essas escritas vão para o banco (write_integration_configemagent_service.py), abrindo sessão quando o chamador não tem uma. Escrita que falha levanta erro em vez de cair no core, e a leitura antes de cada read-modify-write (read_integration_config) levanta em erro de banco em vez de ler como config vazia.Ordem de merge: 1. a migration que dá default a
evo_core_agent_integrations.tenant_idnas gems de licensing · 2. esta · 3. evo-ai-core-service-community#56.Test plan
pytest tests/unit --ignore=tests/unit/test_exception_handlers.py(como no CI): 846 passedtests/unit/services/test_oauth_state_writes.py(novo): 45 passed. Nos dez serviços MCP, PKCE, client credentials e tokens vão para o banco sem nenhuma chamada HTTP; nos dois Google, as credenciais também; escrita que falha levanta; leitura com erro de banco levanta.tests/unit/test_integration_config_put.py(novo): 54 passed. Por provedor, campos fora da lista não alteram a config gravada,toolssalva como antes econfigque não é objeto devolve 422. Nos dois Google, os campos fora do modelo não chegam à config gravada.extra: allowDict[str, Any]mcp_oauth_service.pye os serviços Google da develop contra os testes novos: 42 falhas000014do core), só auth/permissão e service stubados. Na develop, o PUT altera os campos fora da lista; nesta branch, não altera, etoolssalva nos dez.