Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.openmetadata.service.events.ChangeEventHandler.copyChangeEvent;
import static org.openmetadata.service.formatter.util.FormatterUtil.createChangeEventForEntity;
import static org.openmetadata.service.resources.tags.TagLabelUtil.addDerivedTags;
import static org.openmetadata.service.util.InputSanitizer.sanitize;

import jakarta.json.JsonPatch;
import jakarta.ws.rs.core.SecurityContext;
Expand Down Expand Up @@ -283,7 +284,7 @@ private void applyColumnUpdates(
.ifPresent(name -> column.setDisplayName(name.trim().isEmpty() ? null : name));

Optional.ofNullable(updateColumn.getDescription())
.ifPresent(desc -> column.setDescription(desc.trim().isEmpty() ? null : desc));
.ifPresent(desc -> column.setDescription(desc.trim().isEmpty() ? null : sanitize(desc)));

Optional.ofNullable(updateColumn.getTags())
.ifPresent(tags -> column.setTags(addDerivedTags(tags)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
import static org.openmetadata.service.util.EntityUtil.nextVersion;
import static org.openmetadata.service.util.EntityUtil.objectMatch;
import static org.openmetadata.service.util.EntityUtil.tagLabelMatch;
import static org.openmetadata.service.util.InputSanitizer.sanitize;
import static org.openmetadata.service.util.LineageUtil.addDataProductsLineage;
import static org.openmetadata.service.util.LineageUtil.addDomainLineage;
import static org.openmetadata.service.util.LineageUtil.removeDataProductsLineage;
Expand Down Expand Up @@ -701,7 +702,7 @@ public final T copy(T entity, CreateEntity request, String updatedBy) {
entity.setId(UUID.randomUUID());
entity.setName(request.getName());
entity.setDisplayName(request.getDisplayName());
entity.setDescription(request.getDescription());
entity.setDescription(sanitize(request.getDescription()));
entity.setOwners(owners);
entity.setDomains(domains);
entity.setTags(request.getTags());
Expand Down Expand Up @@ -1841,6 +1842,10 @@ private PatchResponse<T> patchCommonWithOptimisticLocking(
String impersonatedBy) {
// Start timing JSON patch application
T updated = JsonUtils.applyPatch(original, patch, entityClass);

// Sanitize description to prevent XSS attacks
updated.setDescription(sanitize(updated.getDescription()));

updated.setUpdatedBy(user);
updated.setUpdatedAt(System.currentTimeMillis());

Expand Down Expand Up @@ -6113,7 +6118,7 @@ public static class DescriptionTaskWorkflow extends TaskWorkflow {
@Override
public EntityInterface performTask(String user, ResolveTask resolveTask) {
EntityInterface aboutEntity = threadContext.getAboutEntity();
aboutEntity.setDescription(resolveTask.getNewValue());
aboutEntity.setDescription(sanitize(resolveTask.getNewValue()));
return aboutEntity;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import static org.openmetadata.service.util.EntityUtil.fieldAdded;
import static org.openmetadata.service.util.EntityUtil.fieldDeleted;
import static org.openmetadata.service.util.EntityUtil.fieldUpdated;
import static org.openmetadata.service.util.InputSanitizer.sanitize;

import io.jsonwebtoken.lang.Collections;
import jakarta.json.JsonPatch;
Expand Down Expand Up @@ -580,8 +581,8 @@ protected void resolveTask(ThreadContext threadContext, String user, ResolveTask
Entity.getCollectionDAO().changeEventDAO().insert(JsonUtils.pojoToMaskedJson(changeEvent));
}

// Update the attributes
threadContext.getThread().getTask().withNewValue(resolveTask.getNewValue());
// Update the attributes (sanitize to prevent XSS)
threadContext.getThread().getTask().withNewValue(sanitize(resolveTask.getNewValue()));
closeTask(threadContext.getThread(), user, new CloseTask());
}

Expand Down Expand Up @@ -1013,6 +1014,9 @@ public final PatchResponse<Post> patchPost(
// Apply JSON patch to the original post to get the updated post
Post updated = JsonUtils.applyPatch(post, patch, Post.class);

// Sanitize the message to prevent XSS attacks
updated.setMessage(sanitize(updated.getMessage()));

restorePatchAttributes(post, updated);

// Update the attributes
Expand Down Expand Up @@ -1048,6 +1052,15 @@ public final PatchResponse<Thread> patchThread(

// Apply JSON patch to the original thread to get the updated thread
Thread updated = JsonUtils.applyPatch(original, patch, Thread.class);

// Sanitize content to prevent XSS attacks
updated.setMessage(sanitize(updated.getMessage()));
if (updated.getTask() != null) {
updated.getTask().setOldValue(sanitize(updated.getTask().getOldValue()));
updated.getTask().setSuggestion(sanitize(updated.getTask().getSuggestion()));
updated.getTask().setNewValue(sanitize(updated.getTask().getNewValue()));
}

// update the "updatedBy" and "updatedAt" fields
updated.withUpdatedAt(System.currentTimeMillis()).withUpdatedBy(user);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.openmetadata.service.jdbi3.EntityRepository.validateOwners;
import static org.openmetadata.service.jdbi3.EntityRepository.validateReviewers;
import static org.openmetadata.service.util.EntityUtil.getEntityReferences;
import static org.openmetadata.service.util.InputSanitizer.sanitize;

import java.util.List;
import java.util.UUID;
Expand All @@ -24,7 +25,7 @@ default T copy(T entity, CreateEntity request, String updatedBy) {
entity.setId(UUID.randomUUID());
entity.setName(request.getName());
entity.setDisplayName(request.getDisplayName());
entity.setDescription(request.getDescription());
entity.setDescription(sanitize(request.getDescription()));
entity.setOwners(owners);
entity.setDomains(domains);
entity.setTags(request.getTags());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ public WebAnalyticEvent createToEntity(CreateWebAnalyticEvent create, String use
return copy(new WebAnalyticEvent(), create, user)
.withName(create.getName())
.withDisplayName(create.getDisplayName())
.withDescription(create.getDescription())
.withEventType(create.getEventType());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ public class WorkflowMapper implements EntityMapper<Workflow, CreateWorkflow> {
@Override
public Workflow createToEntity(CreateWorkflow create, String user) {
return copy(new Workflow(), create, user)
.withDescription(create.getDescription())
.withRequest(create.getRequest())
.withWorkflowType(create.getWorkflowType())
.withDisplayName(create.getDisplayName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

package org.openmetadata.service.resources.data;

import static org.openmetadata.service.util.InputSanitizer.sanitize;

import java.util.UUID;
import org.openmetadata.schema.api.data.CreateDataContract;
import org.openmetadata.schema.entity.data.DataContract;
Expand All @@ -32,7 +34,7 @@ public static DataContract createEntity(CreateDataContract create, String user)
.withId(UUID.randomUUID())
.withName(create.getName())
.withDisplayName(create.getDisplayName())
.withDescription(create.getDescription())
.withDescription(sanitize(create.getDescription()))
.withEntity(entity)
.withEntityStatus(create.getEntityStatus())
.withSchema(create.getSchema())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.openmetadata.service.resources.databases;

import static org.openmetadata.service.util.EntityUtil.getEntityReference;
import static org.openmetadata.service.util.InputSanitizer.sanitize;

import java.util.UUID;
import org.openmetadata.schema.api.data.CreateTable;
Expand Down Expand Up @@ -34,7 +35,7 @@ public Table createToEntity(CreateTable create, String user) {
public CustomMetric createCustomMetricToEntity(CreateCustomMetric create, String user) {
return new CustomMetric()
.withId(UUID.randomUUID())
.withDescription(create.getDescription())
.withDescription(sanitize(create.getDescription()))
.withName(create.getName())
.withColumnName(create.getColumnName())
.withOwners(create.getOwners())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ public class DataInsightChartMapper
public DataInsightChart createToEntity(CreateDataInsightChart create, String user) {
return copy(new DataInsightChart(), create, user)
.withName(create.getName())
.withDescription(create.getDescription())
.withDataIndexType(create.getDataIndexType())
.withDimensions(create.getDimensions())
.withMetrics(create.getMetrics())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ public class TestCaseMapper implements EntityMapper<TestCase, CreateTestCase> {
public TestCase createToEntity(CreateTestCase create, String user) {
MessageParser.EntityLink entityLink = MessageParser.EntityLink.parse(create.getEntityLink());
return copy(new TestCase(), create, user)
.withDescription(create.getDescription())
.withName(create.getName())
.withDisplayName(create.getDisplayName())
.withParameterValues(create.getParameterValues())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ public class TestDefinitionMapper implements EntityMapper<TestDefinition, Create
@Override
public TestDefinition createToEntity(CreateTestDefinition create, String user) {
return copy(new TestDefinition(), create, user)
.withDescription(create.getDescription())
.withEntityType(create.getEntityType())
.withTestPlatforms(create.getTestPlatforms())
.withSupportedDataTypes(create.getSupportedDataTypes())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ public class TestSuiteMapper implements EntityMapper<TestSuite, CreateTestSuite>
public TestSuite createToEntity(CreateTestSuite create, String user) {
TestSuite testSuite =
copy(new TestSuite(), create, user)
.withDescription(create.getDescription())
.withDisplayName(create.getDisplayName())
.withDataContract(create.getDataContract())
.withName(create.getName());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.openmetadata.service.resources.feeds;

import static org.openmetadata.service.util.InputSanitizer.sanitize;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand All @@ -18,7 +20,7 @@ public Thread createToEntity(CreateThread create, String user) {
return new Thread()
.withId(randomUUID)
.withThreadTs(System.currentTimeMillis())
.withMessage(create.getMessage())
.withMessage(sanitize(create.getMessage()))
.withCreatedBy(create.getFrom())
.withAbout(create.getAbout())
.withAddressedTo(create.getAddressedTo())
Expand All @@ -40,8 +42,8 @@ private TaskDetails getTaskDetails(CreateTaskDetails create) {
.withAssignees(formatAssignees(create.getAssignees()))
.withType(create.getType())
.withStatus(TaskStatus.Open)
.withOldValue(create.getOldValue())
.withSuggestion(create.getSuggestion());
.withOldValue(sanitize(create.getOldValue()))
.withSuggestion(sanitize(create.getSuggestion()));
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.openmetadata.service.resources.feeds;

import static org.openmetadata.service.util.InputSanitizer.sanitize;

import java.util.Collections;
import java.util.UUID;
import org.openmetadata.schema.api.feed.CreatePost;
Expand All @@ -9,7 +11,7 @@ public class PostMapper {
public Post createToEntity(CreatePost create, String user) {
return new Post()
.withId(UUID.randomUUID())
.withMessage(create.getMessage())
.withMessage(sanitize(create.getMessage()))
.withFrom(create.getFrom())
.withReactions(Collections.emptyList())
.withPostTs(System.currentTimeMillis());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.openmetadata.service.resources.feeds;

import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
import static org.openmetadata.service.util.InputSanitizer.sanitize;

import jakarta.ws.rs.core.Response;
import java.util.UUID;
Expand All @@ -22,10 +23,9 @@ public Suggestion createToEntity(CreateSuggestion create, String user) {
validate(create);
return new Suggestion()
.withId(UUID.randomUUID())
.withDescription(create.getDescription())
.withDescription(sanitize(create.getDescription()))
.withEntityLink(create.getEntityLink())
.withType(create.getType())
.withDescription(create.getDescription())
.withTagLabels(create.getTagLabels())
.withStatus(SuggestionStatus.Open)
.withCreatedBy(UserUtil.getUserOrBot(user))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.openmetadata.service.resources.teams;

import static org.openmetadata.service.util.InputSanitizer.sanitize;

import java.util.UUID;
import org.openmetadata.schema.api.teams.CreateUser;
import org.openmetadata.schema.entity.teams.User;
Expand All @@ -16,7 +18,7 @@ public User createToEntity(CreateUser create, String user) {
.withName(create.getName().toLowerCase())
.withFullyQualifiedName(EntityInterfaceUtil.quoteName(create.getName().toLowerCase()))
.withEmail(create.getEmail().toLowerCase())
.withDescription(create.getDescription())
.withDescription(sanitize(create.getDescription()))
.withDisplayName(create.getDisplayName())
.withIsBot(create.getIsBot())
.withIsAdmin(create.getIsAdmin())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Copyright 2021 Collate
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.openmetadata.service.util;

import java.util.ArrayList;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import lombok.experimental.UtilityClass;
import org.owasp.html.HtmlPolicyBuilder;
import org.owasp.html.PolicyFactory;

/**
* Utility class for sanitizing user input to prevent XSS attacks. This sanitization is applied
* server-side to ensure security regardless of whether requests come from the UI or direct API
* calls.
*/
@UtilityClass
public class InputSanitizer {

private static final Pattern ENTITY_LINK_PATTERN = Pattern.compile("<#E::[^>]+>");
private static final String ENTITY_LINK_PLACEHOLDER = "__OM_ENTITY_LINK_%d__";

private static final PolicyFactory CONTENT_POLICY =
new HtmlPolicyBuilder()
.allowElements("p", "br", "div", "span")
.allowAttributes("style", "class")
.onElements("div", "span", "p", "pre", "code")
Comment on lines +38 to +39
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Security: Allowing style attribute enables CSS-based attacks

Details

The style attribute is allowed on multiple elements (div, span, p, pre, code), which can enable CSS-based attacks. While the OWASP HTML Sanitizer provides some protection, allowing arbitrary style values can lead to:

  1. Data exfiltration via CSS selectors and url() in backgrounds
  2. UI redressing where attackers overlay fake UI elements
  3. User tracking through unique CSS-based fingerprinting
  4. Content hiding/manipulation to confuse users

Recommendation: Either remove the style attribute entirely, or use .allowStyling(CssSchema.DEFAULT) which provides safer CSS handling. If custom styling is required, consider allowlisting specific CSS properties:

.allowAttributes("style")
.matching(StyleAttributePolicy.allowedProperties("color", "font-weight", "text-align"))
.onElements("div", "span", "p", "pre", "code")

Alternatively, rely on class attributes (which are already allowed) with predefined CSS classes for styling.


Was this helpful? React with 👍 / 👎

.allowElements("ul", "ol", "li")
.allowElements("strong", "b", "em", "i", "u", "s", "del", "mark")
.allowElements("pre", "code")
.allowElements("blockquote")
.allowElements("h1", "h2", "h3", "h4", "h5", "h6")
.allowElements("table", "thead", "tbody", "tr", "td", "th")
.allowAttributes("class")
.onElements("table", "thead", "tbody", "tr", "td", "th")
.allowElements("a")
.allowAttributes("href", "title", "target")
.onElements("a")
.allowElements("img")
.allowAttributes("src", "alt", "title", "width", "height")
.onElements("img")
.allowUrlProtocols("https", "http")
.requireRelNofollowOnLinks()
.toFactory();

/**
* Sanitizes HTML content to prevent XSS attacks while preserving OpenMetadata entity links.
* Entity links have the format {@code <#E::entityType::fqn>} and are used for rich content
* referencing entities.
*
* @param content the content to sanitize
* @return sanitized content with entity links preserved
*/
public static String sanitize(String content) {
if (content == null || content.isEmpty()) {
return content;
}

List<String> entityLinks = new ArrayList<>();
Matcher matcher = ENTITY_LINK_PATTERN.matcher(content);
StringBuffer protectedContent = new StringBuffer();
int index = 0;

while (matcher.find()) {
entityLinks.add(matcher.group());
matcher.appendReplacement(protectedContent, String.format(ENTITY_LINK_PLACEHOLDER, index++));
}
matcher.appendTail(protectedContent);

String sanitized = CONTENT_POLICY.sanitize(protectedContent.toString());

for (int i = 0; i < entityLinks.size(); i++) {
sanitized = sanitized.replace(String.format(ENTITY_LINK_PLACEHOLDER, i), entityLinks.get(i));
}

return sanitized;
}
}
Loading
Loading