[mohityadav766]

Describe your changes:

Follow-up to #36219, which returns a service connection only to bots and, masked, to callers who can edit the service.

• MCP: get_entity and patch_entity now apply the same rule. A caller without EditAll on the service gets it without connection; editors still get it masked. The check moved to ServiceConnectionAccess, which ServiceEntityResource now shares.
• REST: PATCH, follow and unfollow, restore and the timestamp /history now pass the service through decryptOrNullify, as GET and list do. A stored version that can't be decrypted comes back without its connection, and a JSON Patch copy/move on a service needs EditAll.
• dbt: when no connection comes back, the SQL dialect for query lineage comes from the service's serviceType. Before, lineage failed for every model.
• Workflows and the data quality SDK: a missing connection now fails with an error that names the token as the cause, instead of "Unknown error" or "call add_table() first". dbt runs without it.
• CI unblock: patch_request.py gets a type-only cast. jsonpatch 1.38 (released 2026-10-26) types JsonPatch.patch as a Sequence, so basedpyright fails the Python static checks, and with them py-tests-status, on every PR. The patch there always holds a list, so runtime behavior is unchanged.
• Decision records amended: 2026-10-26-service-connection-visibility-follows-edit-permission and 2026-10-20-mcp-responses-mask-stored-credentials (v2 each).

Type of change:

• Bug fix

Tests:

• MetadataServiceResourceIT: patch, follow, /history, copy and restore responses per role (they fail before this change); ServiceEntityResourceTest.
• ServiceConnectionAccessTest; McpResponseUtilsTest, GetEntityToolTest, PatchEntityToolTest: a viewer gets no connection, an editor gets it masked.
• test_dbt.py::TestLineageDialectWithoutConnection, test_base_workflow.py (missing connection, dbt), test_workflow_config_builder.py (no connection).

🤖 Generated with Claude Code

[github-actions]

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

• No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

[gitar-bot]

💡 Quality: Version decrypt failure is swallowed silently via catch (Exception)

decryptOrNullifyVersion catches every Exception and quietly removes the connection. That covers the case the comment describes (a ciphertext that can't be decrypted), but it also hides secrets-manager outages, misconfiguration and authorization lookup errors. In those cases a bot reading /history silently gets versions with no connection, and nothing is logged. The repo rules forbid catch (Exception e) and require at least a log line. Catch the specific exception that decryption or masking throws (for example InvalidServiceConnectionException, or the secrets-manager exception type) and log the version id and number at WARN before removing the connection.

Catch the specific failure and log it before removing the connection:

private void decryptOrNullifyVersion(SecurityContext securityContext, T version) {
  try {
    decryptOrNullify(securityContext, version);
  } catch (InvalidServiceConnectionException e) {
    LOG.warn(
        "Version {} of service {} could not be decrypted; returning it without connection",
        version.getVersion(),
        version.getId(),
        e);
    nullifyConnection(version);
  }
}

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review 👍 Approved with suggestions 0 closed / 1 findings

🔴 High risk · The new EditAll gate controls service-connection visibility across REST and MCP; a misapplied check could expose stored credentials.

Applies the service connection visibility rule to MCP get_entity and patch_entity, REST PATCH, follow, restore and history responses, and dbt lineage dialect fallback. Approved with suggestions: decryptOrNullifyVersion in ServiceEntityResource.java catches every Exception and silently strips the connection, which can hide secrets-manager outages and authorization errors; catch the specific exception type and log the version id at WARN.

💡 Quality: Version decrypt failure is swallowed silently via catch (Exception)

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java:181-187

decryptOrNullifyVersion catches every Exception and quietly removes the connection. That covers the case the comment describes (a ciphertext that can't be decrypted), but it also hides secrets-manager outages, misconfiguration and authorization lookup errors. In those cases a bot reading /history silently gets versions with no connection, and nothing is logged. The repo rules forbid catch (Exception e) and require at least a log line. Catch the specific exception that decryption or masking throws (for example InvalidServiceConnectionException, or the secrets-manager exception type) and log the version id and number at WARN before removing the connection.

Catch the specific failure and log it before removing the connection

private void decryptOrNullifyVersion(SecurityContext securityContext, T version) {
  try {
    decryptOrNullify(securityContext, version);
  } catch (InvalidServiceConnectionException e) {
    LOG.warn(
        "Version {} of service {} could not be decrypted; returning it without connection",
        version.getVersion(),
        version.getId(),
        e);
    nullifyConnection(version);
  }
}

🤖 Prompt for agents

Code Review: Applies the service connection visibility rule to MCP get_entity and patch_entity, REST PATCH, follow, restore and history responses, and dbt lineage dialect fallback. Approved with suggestions: `decryptOrNullifyVersion` in `ServiceEntityResource.java` catches every `Exception` and silently strips the connection, which can hide secrets-manager outages and authorization errors; catch the specific exception type and log the version id at WARN.

1. 💡 Quality: Version decrypt failure is swallowed silently via catch (Exception)
   Files: openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java:181-187

   `decryptOrNullifyVersion` catches every `Exception` and quietly removes the connection. That covers the case the comment describes (a ciphertext that can't be decrypted), but it also hides secrets-manager outages, misconfiguration and authorization lookup errors. In those cases a bot reading `/history` silently gets versions with no connection, and nothing is logged. The repo rules forbid `catch (Exception e)` and require at least a log line. Catch the specific exception that decryption or masking throws (for example `InvalidServiceConnectionException`, or the secrets-manager exception type) and log the version id and number at WARN before removing the connection.

   Fix (Catch the specific failure and log it before removing the connection):
   private void decryptOrNullifyVersion(SecurityContext securityContext, T version) {
     try {
       decryptOrNullify(securityContext, version);
     } catch (InvalidServiceConnectionException e) {
       LOG.warn(
           "Version {} of service {} could not be decrypted; returning it without connection",
           version.getVersion(),
           version.getId(),
           e);
       nullifyConnection(version);
     }
   }

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
84.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud