Repository navigation
fix(tools-mcps): instalação migrada não manda o segredo inline desvinculado (CRM-877) - #84
Merged
daniloleonecarneiro merged 4 commits intoNov 5, 2026
Conversation
…usuário desvinculou (CRM-877) A migração das credenciais copia o segredo inline para o cofre e mantém o valor na linha. Sem referência ao cofre, o processor voltava a usar esse valor. Com a marca INTEGRATION_CREDENTIALS_MIGRATED gravada pelo CRM: - tool e MCP custom não mandam header de autenticação sem credential_refs; - o Knowledge Nexus sem credential_id não monta a tool; - agente externo sem credential_id falha dizendo que a credencial foi desvinculada. Sem a marca, ou com erro na leitura, o inline continua valendo. A tool anexada por id não levava o credential_refs da linha ao builder: o cofre nunca era consultado e a tool vinculada autenticava com o inline.
Reviewer's GuideThe PR makes migrated installations honor credential unlinking by retiring unreferenced inline auth secrets, while retaining inline fallback before migration or when migration-state reads fail; it also fixes credential-reference propagation so linked tools continue using the vault and adds comprehensive regression coverage. Sequence diagram for migrated tool authenticationsequenceDiagram
participant Builder as ToolBuilder
participant Policy as retire_unreferenced_auth_headers
participant Config as installation_integration_migrated
participant Vault as resolve_credential_refs
participant Server as HTTP_or_MCP_Server
Builder->>Policy: retire_unreferenced_auth_headers(headers, credential_refs, migrated)
alt Auth header has no credential reference
Policy->>Config: installation_integration_migrated()
Config-->>Policy: true
Policy-->>Builder: headers without unlinked auth header
else Credential reference exists
Policy-->>Builder: headers retained
Builder->>Vault: resolve_credential_refs(credential_refs)
Vault-->>Builder: vault secret
end
Builder->>Server: send request with resulting headers
Flow diagram for inline secret handling after migrationflowchart TD
A[Inline secret or auth header present] --> B{credential reference exists?}
B -->|Yes| C[resolve_credential_refs]
C --> D[Use vault credential]
B -->|No| E{installation_integration_migrated}
E -->|No or read error| F[Keep inline fallback]
E -->|Yes| G[Retire inline auth secret]
G --> H[Tool-specific result]
H --> I[HTTP or MCP: omit auth header]
H --> J[Knowledge Nexus: do not build tool]
H --> K[External agent: raise ValueError]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/services/adk/mcp_service.py" line_range="1007" />
<code_context>
+ )
+
credential_refs = tool_config.get("credential_refs") or {}
+ headers = retire_unreferenced_auth_headers(
+ headers, credential_refs, migrated=installation_integration_migrated
+ )
</code_context>
<issue_to_address>
**Custom MCP servers keep unlinked secrets**
When a migrated installation supplies a custom server through `custom_mcp_servers`, `build_lazy_tools` passes that server directly to `mcp_context` and `LazyMCPTool` without calling `_resolve_mcp_headers`, so unlinked inline auth headers are still sent.
Route custom server configurations through `_resolve_mcp_headers` before passing them to `mcp_context` or `LazyMCPTool`.
</issue_to_address>
### Comment 2
<location path="src/services/adk/custom_tools.py" line_range="164-166" />
<code_context>
+ )
+
credential_refs = tool_config.get("credential_refs") or {}
+ headers = retire_unreferenced_auth_headers(
+ headers, credential_refs, migrated=installation_integration_migrated
+ )
if not credential_refs:
return headers
</code_context>
<issue_to_address>
**Migration status is queried per tool**
When an agent has multiple custom HTTP or MCP tools with nonempty auth headers lacking references, each tool with an unreferenced auth header calls `installation_integration_migrated`, which opens a new session and executes a database query. Building an agent with many such tools therefore adds one serial database round trip per tool, increasing build latency and database load.
Read and cache the migration status once per agent/tool build, then pass that result to each header-retirement check.
Also at `src/services/adk/tool_builder.py:75-77`, `src/services/adk/mcp_service.py:1007-1009`.
</issue_to_address>
### Comment 3
<location path="tests/unit/services/test_unlinked_inline_auth.py" line_range="58-62" />
<code_context>
+ assert integration_migrated(_db_answering(raises=RuntimeError("boom"))) is False
+
+ def test_reads_the_flag_the_crm_migration_writes(self):
+ db = _db_answering((True,))
+ integration_migrated(db)
+
+ sql, params = db.execute.call_args.args
+ assert "installation_configs" in str(sql)
+ assert params == {"name": "INTEGRATION_CREDENTIALS_MIGRATED"}
+
</code_context>
<issue_to_address>
**Wrong flag query passes test**
When the migration-status query selects a different or incorrect JSON field, `test_reads_the_flag_the_crm_migration_writes` supplies a mocked `(True,)` row regardless of the SQL, then checks only the table name and bind parameter. A query that selects the wrong JSON field still passes, so a real migrated installation can be treated as not migrated and keep sending unlinked secrets.
Assert the expected SQL expression or test the query against a database fixture containing the migration flag.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 3 findings to address first, and if a credential reference is missing or mismatched after migration, this change strips the inline auth header or makes an external-agent credential unusable, so integrations can fail until the configuration is corrected. Reverting restores the prior inline-secret behavior, but any outage during that period cannot be undone.
Blocking findings: src/services/adk/mcp_service.py:1007, src/services/adk/custom_tools.py:166, tests/unit/services/test_unlinked_inline_auth.py:62
…e migration read per build (CRM-877) get_agent expands custom_mcp_server_ids into custom_mcp_servers entries carrying only url and raw headers, and build_lazy_tools connected to them as they were: an unlinked header still went out, and a linked one never reached the vault. The expansion now carries credential_refs and those entries resolve through _resolve_mcp_headers. The migration record is read once per tool or MCP build instead of once per tool.
…ingle read (CRM-877)
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
A
integration_credentials:migratedo CRM copia o segredo inline para o cofre, liga a referência e mantém o valor na linha. Quando o usuário desvinculava a credencial, o processor via a referência vazia e voltava a autenticar com o valor antigo, que o usuário considerava aposentado.Agora, na instalação com a marca
INTEGRATION_CREDENTIALS_MIGRATED(a mesma que o CRM grava e lê para os bots):credential_refsnão é enviado. A decisão é por header:authorization,x-api-key,api-key,apikeyex-auth-token, a mesma lista que a migração importa. Os outros headers seguem.credential_id, anexus_api_keyinline não é usada e a tool não é montada (fica o aviso de campo faltando).credential_id, o build falha dizendo que não há credencial vinculada, em vez de autenticar com o inline.Sem a marca, ou com erro na leitura, nada muda: o inline continua sendo o fallback. A marca é lida uma vez por build de tools e uma vez por build de MCPs, numa sessão própria, e só quando existe segredo inline sem referência.
Também corrigido, porque o fix dependia disso: as expansões que o processor faz a partir dos ids não levavam
credential_refs:custom_tool_idseconvert_to_http_tool;get_agentemcustom_mcp_servers, que obuild_lazy_toolsconectava com os headers crus.Nesses caminhos o cofre nunca era consultado, e a tool ou o MCP vinculado autenticava com o inline.
Por que a marca e não uma contagem: com o delete real das credenciais, contar linhas com
imported_fromou segredos pendentes não separa "migrou e depois desvinculou" de "nunca migrou". A marca só é gravada pela task de migração e nada a desfaz.Test plan
pytest tests/unit --ignore=tests/unit/test_exception_handlers.py(como no CI): 747 passedtests/unit/services/test_unlinked_inline_auth.py(novo): 49 passed"true"contando como migradocredential_refsfora da tool por id, doconvert_to_http_toole da expansão do MCPAuthorizationrecebido:Bearer antigo/Bearer antigoBearer antigo/Bearer antigoBearer antigo/Bearer do-cofreBearer do-cofreget_agent→build_lazy_tools, cada conexão)Bearer antigo,Bearer antigoRunnerUtils.get_and_build_agent) / Nexusinstallation_configs(permission denied), a tool desvinculada mandouBearer antigo.Linhas, marca e papel temporário apagados no fim.
Notas
integration_credentials:record_migration(CRM). Até lá, o inline continua valendo.installation_configsé tabela global, sem RLS; a leitura não depende do tenant da sessão.apiKeyinline e não consulta o cofre.