[2.0] fix(security): apply service connection visibility to MCP and ingestion clients by mohityadav766 · Pull Request #36316 · open-metadata/OpenMetadata

mohityadav766 · 2026-10-27T07:19:25Z

Describe your changes:

Backport of #36312 to 2.0, for 2.0.7. #36266, the 2.0 backport of #36219, is already on 2.0.

This also backports #35774 first, because 2.0 lacked it. On 2.0, MCP get_entity returned a service's stored connection without masking, and patch_entity could read or change /connection. With #35774, MCP masks credentials; the follow-up then drops the connection entirely for callers without EditAll on the service.

Commits:

Fixes MCP getEntity returns a service connection unmasked #34566: mask service connections in MCP responses (Fixes #34566: mask service connections in MCP responses #35774), adapted to 2.0: raw EntityInterface, and 2.0's JsonUtils.deepCopy signature.
fix(security): apply service connection visibility to MCP and ingestion clients (fix(security): apply service connection visibility to MCP and ingestion clients #36312). 2.0's data quality SDK has no masked-credential check, so only the missing-connection error is added there.
fix(security): apply service connection visibility to MCP and ingestion clients #36312's REST commit: PATCH, follow and unfollow, restore and the timestamp /history pass the service through decryptOrNullify; a JSON Patch copy/move on a service needs EditAll.
fix(security): apply service connection visibility to MCP and ingestion clients #36312's two typing commits: Optional narrowing in the dbt lineage dialect, and a type-only cast in patch_request.py. jsonpatch 1.38 (released 2026-10-26) makes basedpyright fail the Python static checks on 2.0 as well.

Type of change:

Bug fix

Tests:

The unit tests of both PRs, on 2.0: ServiceConnectionAccessTest, McpResponseUtilsTest, GetEntityToolTest, PatchEntityToolTest, PasswordEntityMaskerTest; test_dbt.py::TestLineageDialectWithoutConnection, test_base_workflow.py, test_workflow_config_builder.py. The integration-test module compiles (McpIntegrationIT from #35774).

🤖 Generated with Claude Code

Summary by Gitar

Access control:
- Hides a user's email from get_entity callers without PII access, as UserResource does
Ingestion:
- Workflows that cannot read a service connection fail with a clear error, except dbt
- dbt lineage takes its SQL dialect from the service's serviceType when no connection is returned
Documentation:
- Added ADRs for MCP credential masking and service connection visibility

_{This will update automatically on new commits.}

* Fixes #34566: mask service connections in MCP responses * mask more entity types * mask alert webhooks * patch entity IT (cherry picked from commit ec5752d)

…on clients MCP get_entity and patch_entity drop a service's connection for callers without EditAll on it, through the ServiceConnectionAccess check that ServiceEntityResource now shares; editors still get it masked. Ingestion copes with a connection that does not come back: dbt takes the lineage SQL dialect from the service's serviceType instead of failing lineage for every model, and other workflows and the data quality SDK fail with an error naming the token as the cause. Amends the service-connection visibility and MCP credential-masking decision records. (cherry picked from commit 845ac76)

…age dialect basedpyright reports the optional serviceConnection config and serviceName as new errors. (cherry picked from commit 230ca60)

github-actions · 2026-10-27T07:19:36Z

❌ 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.

greptile-apps · 2026-10-27T07:24:28Z

+    try (MockedStatic<Entity> entityMock = mockStatic(Entity.class);
+        MockedStatic<McpChangeEventUtil> changeEventMock = mockStatic(McpChangeEventUtil.class);
+        MockedStatic<JsonUtils> jsonMock = mockStatic(JsonUtils.class)) {
+      entityMock
+          .when(() -> Entity.getEntityRepository(Entity.DATABASE_SERVICE))
+          .thenReturn(serviceRepository);
+      jsonMock.when(() -> JsonUtils.convertValue(any(), eq(Map.class))).thenReturn(Map.of());
+
+      new PatchEntityTool().execute(authorizer, securityContext, params);
+
+      verify(serviceRepository).patch(any(), any(String.class), any(), any(), any(), any(), any());


Test checks only mocked calls

execute_servicePatchOutsideTheConnectionReachesTheRepository mocks Entity, McpChangeEventUtil, JsonUtils, and the repository, then only checks that patch was called. It can pass while the saved service or returned response is wrong. The repository guide requires tests to check observable outcomes rather than internal calls. Replace this test with an outcome-based test; the new integration test already checks that the description is saved and the connection stays unchanged. This requirement must be satisfied before merging.

Context Used: CLAUDE.md (source)

greptile-apps · 2026-10-27T07:24:28Z

+  public static Map<String, Object> hideConnectionFromNonEditors(
+      Map<String, Object> doc,
+      EntityInterface entity,
+      Authorizer authorizer,
+      SecurityContext securityContext) {
+    if (entity instanceof ServiceEntityInterface) {
+      String entityType = Entity.getEntityTypeFromObject(entity);


Unchanged variables omit required final

The new Java helpers leave unchanged parameters and local variables without final, including doc, entityType, and copy. The repository guide explicitly requires final on variables and parameters that do not change. This also occurs in maskedCopy and ServiceConnectionAccess.canEditConnection, with the pattern appearing elsewhere in the added Java code. Apply the requirement before merging.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

greptile-apps · 2026-10-27T07:24:28Z

+
+    assertThat(patched.path("result").path("isError").asBoolean(false)).isFalse();
+    assertThat(refused.path("result").path("isError").asBoolean(false)).isTrue();
+    JsonNode stored = get("services/databaseServices/name/" + serviceFqn, JsonNode.class);


Test bypasses the required client

The new assertion reads the saved service through get(...). The repository guide requires backend integration tests to use SdkClients for API calls. Read the service through the SDK so this assertion also exercises the supported client. This requirement must be satisfied before merging.

Context Used: CLAUDE.md (source)

jsonpatch 1.38 types JsonPatch.patch as a Sequence, so basedpyright rejects the append that adds array-field replace operations and fails the Python static checks. The patches built there always hold a list, so the append is unchanged at runtime. (cherry picked from commit 7887432)

…ponse PATCH, follow and unfollow, restore and the timestamp history now pass the service through decryptOrNullify, as GET and list do. A stored version that cannot be decrypted is returned without its connection, and a JSON Patch that copies or moves from another path needs EditAll on the service. (cherry picked from commit eb67b5c)

gitar-bot · 2026-10-28T11:33:11Z

Code Review ✅ Approved

🔴 High risk · Applying EDIT_ALL checks to service-connection responses and MCP patches changes credential access; a mistake could expose stored secrets or disrupt ingestion.

Backports service connection visibility to 2.0: MCP get_entity and patch_entity now mask or withhold service connections, ingestion workflows fail clearly when a connection is unreadable, and the dbt lineage dialect falls back to serviceType. No issues found.

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 · 2026-10-28T11:48:50Z

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

Vishnuujain and others added 3 commits October 27, 2026 12:48

Fixes #34566: mask service connections in MCP responses (#35774)

9db1f0f

* Fixes #34566: mask service connections in MCP responses * mask more entity types * mask alert webhooks * patch entity IT (cherry picked from commit ec5752d)

fix(ingestion): narrow the optional connection fields in the dbt line…

2fcfbff

…age dialect basedpyright reports the optional serviceConnection config and serviceName as new errors. (cherry picked from commit 230ca60)

mohityadav766 requested review from a team as code owners October 27, 2026 07:19

mohityadav766 requested review from ulixius9 and removed request for a team October 27, 2026 07:19

github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels Oct 27, 2026

greptile-apps Bot reviewed Oct 27, 2026

View reviewed changes

mohityadav766 temporarily deployed to test October 27, 2026 07:48 — with GitHub Actions Inactive

mohityadav766 had a problem deploying to test October 27, 2026 07:48 — with GitHub Actions Failure

mohityadav766 temporarily deployed to test October 27, 2026 07:48 — with GitHub Actions Inactive

mohityadav766 had a problem deploying to test October 27, 2026 09:38 — with GitHub Actions Failure

mohityadav766 temporarily deployed to test October 27, 2026 09:38 — with GitHub Actions Inactive

Navigation Menu

Repository navigation

[2.0] fix(security): apply service connection visibility to MCP and ingestion clients - #36316

[2.0] fix(security): apply service connection visibility to MCP and ingestion clients#36316
mohityadav766 wants to merge 5 commits into
2.0from
security/connection-visibility-followups-2.0

mohityadav766 commented Oct 27, 2026 •

edited by gitar-bot Bot

Loading

Uh oh!

github-actions Bot commented Oct 27, 2026

Uh oh!

greptile-apps Bot Oct 27, 2026

Uh oh!

greptile-apps Bot Oct 27, 2026

Uh oh!

greptile-apps Bot Oct 27, 2026

Uh oh!

gitar-bot Bot commented Oct 28, 2026

Uh oh!

sonarqubecloud Bot commented Oct 28, 2026

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mohityadav766 commented Oct 27, 2026 • edited by gitar-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes:

Type of change:

Tests:

Summary by Gitar

Uh oh!

github-actions Bot commented Oct 27, 2026

❌ PR checklist incomplete

Uh oh!

greptile-apps Bot Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot Bot commented Oct 28, 2026

Uh oh!

sonarqubecloud Bot commented Oct 28, 2026

Quality Gate passed for 'open-metadata-ingestion'

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mohityadav766 commented Oct 27, 2026 •

edited by gitar-bot Bot

Loading