fix: don't fail the whole service list when one connection can't be decrypted

[oluies]

Fixes #31887

Problem

ServiceEntityResource.decryptOrNullify(SecurityContext, ResultList<T>) decrypted each service inside a forEach with no error handling:

listOrEmpty(services.getData()).forEach(service -> decryptOrNullify(securityContext, service));

decryptOrNullify(securityContext, service) → retrieveServiceConnectionConfig → SecretsManager.decryptServiceConnectionConfig, which throws when a config cannot be decrypted. The exception propagates out of the loop and fails the whole list request.

The practical effect is that one service with an undecryptable connection — after a Fernet key change, for instance — empties the entire service list in the UI. Every other service disappears with it, and since rendering the list is what fails, there is no way to reach the broken service's edit form to repair its credentials. Recovery means editing dbservice_entity.json in the metadata database or restoring the previous key.

Because the code is in the shared base class, this applies to every service type.

Fix

The class already declares an abstract hook:

protected abstract T nullifyConnection(T service);

implemented by all eleven service resources (ApiService, DashboardService, DatabaseService, DriveService, LLMService, McpService, MessagingService, MetadataService, MlModelService, …), typically as return service.withConnection(null);.

Nothing in the main source tree ever called it — the only call site in the repository is a unit test (McpServiceResourceTest:107). This PR wires it up, which is what the method's own name ("decryptOrNullify") implies was intended: catch per service, log a warning, and return that service without a connection.

The single-service GET /{id} path is deliberately left throwing, so the error stays visible where it is actionable and where the user is looking at exactly the entity that is broken.

Notes for reviewers

• @Slf4j added to the class; it had no logger.
• The forEach relies on nullifyConnection mutating the element in place. That holds: openmetadata-spec/pom.xml sets <generateBuilders>true</generateBuilders> without useInnerClassBuilders, so jsonschema2pojo emits fluent setters that assign to this and return this. Happy to switch to an index-based List.set if you would rather not depend on that.
• Catching Exception rather than the specific type is deliberate: the throw sites wrap several causes (InvalidServiceConnectionException, SecretsManagerException, and whatever an external secrets manager raises), and the intent is that no single service can take down the list.

Testing

I was not able to build locally — this was developed against a source checkout without the Maven toolchain, so the change is uncompiled and CI will be its first real check. What I did verify:

• Formatting matches the project's Spotless setup: google-java-format (GOOGLE style) makes no changes to the file, and every line is within the 100-column limit, so the result is stable under reflowLongStrings=false.
• The file parses cleanly (google-java-format's parser accepts it).
• nullifyConnection really is uncalled, and withConnection really does mutate, both checked against the tree as described above.

Please push back if you would like a regression test added — a ResultList containing one service whose config fails to decrypt, asserting the other entries still come back and the failed one has a null connection, would fit in the existing service resource tests. I did not add one blind, without being able to run the suite.

Greptile Summary

This PR makes service-list connection decryption failures recover per entity, returning the affected service without its connection instead of failing the entire request.

• Adds per-service exception handling and connection nullification.
• Adds warning-level diagnostics for failed connection decryption.
• The diagnostic currently exposes potentially secret-bearing exception text.

Confidence Score: 3/5

This PR should not merge until the warning stops recording secret-bearing decryption exception messages.

Failed field-by-field decryption can leave plaintext values in the temporary connection object, whose generated string representation is embedded in the newly logged exception message.

Files Needing Attention: openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java

Security Review

The new warning can log generated connection-object contents from decryption exceptions. Because password fields may already have been decrypted before a later field fails, this can disclose plaintext credentials to application logs.

Important Files Changed

Filename	Overview
openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java	Correctly isolates per-service decryption failures, but logs an exception message that can contain plaintext connection secrets.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Resource as ServiceEntityResource
  participant Secrets as SecretsManager
  participant Logs
  Client->>Resource: List services
  loop Each service
    Resource->>Secrets: Decrypt connection fields
    Secrets-->>Secrets: Decrypt fields in place
    Secrets--xResource: Later field throws
    Resource->>Logs: Warn with exception message containing connection.toString()
    Resource-->>Resource: Nullify returned connection
  end
  Resource-->>Client: Service list

Loading

_{Reviews (1): Last reviewed commit: "fix: don't fail the whole service list w..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

• Knowledge Base — Auth and Security: Authentication, Authorization, Secrets, and SCIM

[github-actions]

❌ PR checklist incomplete

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

• Linked issue One undecryptable service connection makes the entire service list endpoint fail (decryptOrNullify never nullifies) #31887 is not added to the Shipping project.

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.

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[gitar-bot]

💡 Quality: nullifyConnection relies on discarded return value's side effect

In the catch block, nullifyConnection(service) is called but its return value is discarded; the connection is only cleared because the generated withConnection(null) happens to mutate in place (currently true given generateBuilders=true without useInnerClassBuilders). If the codegen config ever switches to immutable/inner-class builders, withConnection would return a new instance and the original (undecryptable/encrypted) connection config would remain in the list and be serialized to the client, defeating the null-out. Assign the result back to the list to remove the hidden dependency, e.g. iterate by index and call data.set(i, nullifyConnection(service)).

Assign the nullified service back into the list instead of relying on in-place mutation.:

List<T> data = listOrEmpty(services.getData());
for (int i = 0; i < data.size(); i++) {
  T service = data.get(i);
  try {
    decryptOrNullify(securityContext, service);
  } catch (Exception e) {
    LOG.warn(
        "Failed to decrypt connection of service '{}'; returning it without one: {}",
        service.getFullyQualifiedName(),
        e.getMessage());
    data.set(i, nullifyConnection(service));
  }
}
return services;

• 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 resolved / 1 findings

Prevents decryption failures on individual services from breaking the entire service list response. Consider assigning the return value of nullifyConnection(service) rather than relying on in-place mutation side effects.

💡 Quality: nullifyConnection relies on discarded return value's side effect

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

In the catch block, nullifyConnection(service) is called but its return value is discarded; the connection is only cleared because the generated withConnection(null) happens to mutate in place (currently true given generateBuilders=true without useInnerClassBuilders). If the codegen config ever switches to immutable/inner-class builders, withConnection would return a new instance and the original (undecryptable/encrypted) connection config would remain in the list and be serialized to the client, defeating the null-out. Assign the result back to the list to remove the hidden dependency, e.g. iterate by index and call data.set(i, nullifyConnection(service)).

Assign the nullified service back into the list instead of relying on in-place mutation.

List<T> data = listOrEmpty(services.getData());
for (int i = 0; i < data.size(); i++) {
  T service = data.get(i);
  try {
    decryptOrNullify(securityContext, service);
  } catch (Exception e) {
    LOG.warn(
        "Failed to decrypt connection of service '{}'; returning it without one: {}",
        service.getFullyQualifiedName(),
        e.getMessage());
    data.set(i, nullifyConnection(service));
  }
}
return services;

🤖 Prompt for agents

Code Review: Prevents decryption failures on individual services from breaking the entire service list response. Consider assigning the return value of nullifyConnection(service) rather than relying on in-place mutation side effects.

1. 💡 Quality: nullifyConnection relies on discarded return value's side effect
   Files: openmetadata-service/src/main/java/org/openmetadata/service/resources/services/ServiceEntityResource.java:100

   In the catch block, `nullifyConnection(service)` is called but its return value is discarded; the connection is only cleared because the generated `withConnection(null)` happens to mutate in place (currently true given `generateBuilders=true` without `useInnerClassBuilders`). If the codegen config ever switches to immutable/inner-class builders, `withConnection` would return a new instance and the original (undecryptable/encrypted) connection config would remain in the list and be serialized to the client, defeating the null-out. Assign the result back to the list to remove the hidden dependency, e.g. iterate by index and call `data.set(i, nullifyConnection(service))`.

   Fix (Assign the nullified service back into the list instead of relying on in-place mutation.):
   List<T> data = listOrEmpty(services.getData());
   for (int i = 0; i < data.size(); i++) {
     T service = data.get(i);
     try {
       decryptOrNullify(securityContext, service);
     } catch (Exception e) {
       LOG.warn(
           "Failed to decrypt connection of service '{}'; returning it without one: {}",
           service.getFullyQualifiedName(),
           e.getMessage());
       data.set(i, nullifyConnection(service));
     }
   }
   return services;

Options

Display: compact → Showing less information.

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}

[greptile-apps]

Decryption warning exposes credentials

When field-by-field decryption fails after processing secret fields, e.getMessage() contains the generated connection object's string representation, causing password or private-key material to be written to application logs. Log the service identity without the exception message.

How this was verified: The decryption failure message embeds toDecryptObject.toString(), whose generated representation includes password fields, and this warning logs that message unchanged.

Suggested change

	LOG.warn(
	"Failed to decrypt connection of service '{}'; returning it without one: {}",
	service.getFullyQualifiedName(),
	e.getMessage());
	LOG.warn(
	"Failed to decrypt connection of service '{}'; returning it without one",
	service.getFullyQualifiedName());

Knowledge Base Used: Auth and Security: Authentication, Authorization, Secrets, and SCIM