Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 5, 2026

This pull request introduces support for a new "hyperlink" custom property type (hyperlink-cp) in the OpenMetadata platform. It includes backend schema and validation logic, integration tests for the new type, and frontend test coverage for advanced search and right entity panel display. Additionally, there are some test code cleanups and improvements for consistency.

Support for Hyperlink Custom Property Type

  • Added the hyperlink-cp custom property type to the JSON schema, allowing entities to have hyperlink fields with URL and optional display text.
  • Implemented backend validation in EntityRepository to ensure only HTTP/HTTPS URLs are accepted for the hyperlink-cp type. [1] [2]
  • Updated TypeRepository to recognize hyperlink-cp as a valid type that requires no additional config validation.

Integration and UI Test Enhancements

  • Added integration tests in TypeResourceIT to verify creation, updating, and multiple additions of hyperlink custom properties, as well as validation of the hyperlink type itself. [1] [2] [3]
  • Extended Playwright UI test data and logic to include the new hyperlink custom property in advanced search filter scenarios. [1] [2]
  • Updated right entity panel Playwright tests for more robust and consistent custom property rendering and search, including support for the hyperlink type. [1] [2] [3] [4] [5] [6] [7] [8]

Test Code Cleanup

  • Removed redundant cleanup code in RightEntityPanelFlow.spec.ts to streamline test execution.

Copilot AI changed the title [WIP] Add hyperlink support as custom property field type Add Hyperlink support as custom property field type for all entities Jan 5, 2026
Copilot AI requested a review from harshach January 5, 2026 20:32
@harshach
Copy link
Collaborator

harshach commented Jan 5, 2026

@copilot @gitar-bot tests should be added to openmetadata-integration-tests not in openmetadata-service module

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026

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!

Copy link
Contributor Author

Copilot AI commented Jan 5, 2026

@copilot @gitar-bot tests should be added to openmetadata-integration-tests not in openmetadata-service module

Moved the hyperlink tests from openmetadata-service to openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TypeResourceIT.java. The tests include:

  • test_addHyperlinkCustomProperty()
  • test_updateHyperlinkCustomPropertyDescription()
  • test_addMultipleHyperlinkCustomProperties()

Commit: ea0a229

@harshach
Copy link
Collaborator

harshach commented Jan 5, 2026

@gitar-bot apply the suggestions

@harshach
Copy link
Collaborator

harshach commented Jan 5, 2026

Gitar fix CI

@gitar-bot
Copy link

gitar-bot bot commented Jan 5, 2026

Gitar fix CI

I've analyzed all 16 CI failures. They're caused by infrastructure issues, not code problems:

Main Issue (15 jobs): All test jobs are blocked by a missing "safe to test" label. This is a security gate that prevents the workflows from executing. Jobs affected:

  • Python tests (4 jobs)
  • Playwright E2E tests (6 jobs)
  • Integration tests (2 jobs)
  • Test report jobs (2 jobs)

Secondary Issue (1 job): The generate-types job successfully generated all types including hyperlink-cp, but the push failed due to a Git race condition when commit b94eda2 was added during execution.

What I Can't Do:
I don't have permissions to add the "safe to test" label. A repository maintainer needs to:

  1. Add the "safe to test" label to this PR
  2. Re-run the failed workflows

Once the label is added, the actual tests will run and we can see if there are any real code issues with the hyperlink implementation.

@harshach harshach added the safe to test Add this label to run secure Github workflows on PRs label Jan 5, 2026
@gitar-bot
Copy link

gitar-bot bot commented Jan 29, 2026

🔍 CI failure analysis for db6e22c: Maven CI shows 4 test failures (3 AWS credentials + 1 Apps test) - unrelated to PR. Playwright shows infrastructure issue with 3 shards (all 1,676 tests passed but jobs failed). Total: infrastructure issues, no code-related failures.

Complete CI Analysis for Commit db6e22c7 (Latest Merge from Main)


Maven PostgreSQL CI - UNRELATED FAILURES

Test Results: 7,854 tests run, 701 skipped, 4 failed

Issue

4 Java test failures in maven-postgresql-ci build.

Root Cause

3 AWS Credentials Tests (AwsCredentialsUtilTest):

  • testBuildCredentialsProviderWithEmptyCredentials
  • testBuildCredentialsProviderWithNoCredentials
  • testBuildCredentialsProviderWithOnlyAccessKey

All fail with:

IllegalArgumentException: AWS credentials not configured. Either provide accessKeyId/secretAccessKey 
or set enabled=true to use IAM authentication via the default credential provider chain.

1 Apps Test (AppsResourceTest):

  • post_trigger_app_200 - Max retries exceeded polling for eventual assert

Details

Relationship to PR: Completely unrelated

  • PR modifies only 48 files: custom property schemas, TypeScript UI, Java entity repositories, Playwright tests
  • None of the failing tests are in files modified by this PR
  • PR adds hyperlink custom property support - no AWS or Apps functionality
  • Failures appear to be environment/infrastructure configuration issues

Playwright E2E Tests - INFRASTRUCTURE ISSUE

Shard 2: 562 passed, job conclusion: failure

Shard 3: 581 passed, job conclusion: failure

Shard 6: 533 passed, job conclusion: failure

Issue

All three Playwright test shards show all tests passing but the CI jobs are marked as failed.

Root Cause

This is an infrastructure issue, not a test failure. Possible causes:

  • Artifact upload failure after tests complete
  • Post-test cleanup step failure
  • CI runner timeout or resource issue

The test execution itself was successful (100% pass rate on all shards).

Details

Relationship to PR: Completely unrelated

  • All 1,676 Playwright tests passed (562 + 581 + 533) - no test failures related to PR changes
  • Job failures occur after test execution completes

Summary

Maven (Unrelated): 4 test failures (AWS credential config + Apps polling) - PR modifies different Java files

Playwright (Infrastructure Issue): 3 shards affected - all 1,676 tests passed (100% success rate), but jobs failed due to post-test infrastructure issue

Total: Infrastructure/configuration issues only - no code-related test failures

Code Review 👍 Approved with suggestions 5 resolved / 7 findings

Well-structured implementation of hyperlink custom property type with proper URL validation, comprehensive test coverage, and significant code cleanup (~2500 lines removed). Two minor suggestions from previous review remain applicable but are non-blocking.

💡 Quality: Duplicated URL validation logic in backend and frontend

📄 openmetadata-ui/src/main/resources/ui/src/components/common/CustomPropertyTable/PropertyValue.tsx 📄 openmetadata-ui/src/main/resources/ui/src/components/common/CustomPropertyTable/PropertyValue.tsx

The URL protocol validation logic (http:/https: check) is duplicated in three places:

  1. Backend: EntityRepository.java - validateHyperlinkUrl()
  2. Frontend edit form: PropertyValue.tsx - validateSafeUrl()
  3. Frontend display: PropertyValue.tsx - isSafeUrl()

While having validation in multiple layers is good for defense-in-depth, the frontend has the same isSafeUrl logic defined twice (once for validation, once for rendering). Consider extracting the frontend validation into a shared utility function to maintain consistency and reduce duplication.

// utils/urlValidation.ts
export const isSafeUrl = (url: string): boolean => {
  try {
    const parsed = new URL(url);
    return ['http:', 'https:'].includes(parsed.protocol);
  } catch {
    return false;
  }
};
💡 Edge Case: Missing validation for URL length limits

📄 openmetadata-spec/src/main/resources/json/schema/type/customProperties/complexTypes.json

The URL validation doesn't enforce any maximum length limit for the URL or displayText fields. While the schema marks URL as required, there's no maximum length constraint. Extremely long URLs could potentially cause display issues or storage concerns.

Consider adding maxLength validation in both the schema (complexTypes.json) and frontend form validation to prevent excessively long URLs (e.g., 2048 characters for URL, 256 for displayText).

Example schema change:

"url": {
  "type": "string",
  "description": "The URL that the hyperlink points to.",
  "format": "uri",
  "maxLength": 2048
}
✅ 5 resolved
Security: Frontend URL validation can be bypassed via API calls

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java
The frontend URL validation in PropertyValue.tsx is a good defense-in-depth measure, but the primary security control relies on the backend validation in EntityRepository.java. The backend validation is present and properly restricts protocols to http/https, which is correct.

However, there's an inconsistency: the backend allows empty URLs (returns early if url is null/empty), but the frontend form marks URL as required. This inconsistency could lead to confusion, though it's not a security issue.

The implementation overall is secure - the backend properly validates:

  1. URL format via java.net.URI
  2. Protocol restriction to http/https only
  3. Proper error messages

The frontend also double-checks with isSafeUrl() before rendering, which is good defense in depth.

Quality: Localization strings not translated for non-English languages

📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/de-de.json 📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/fr-fr.json
New localization keys (display-text, hyperlink, invalid-url, url-must-use-http-or-https) are added to all language files, but they remain in English for all non-English locales.

For example, in de-de.json:

"display-text": "Display Text",
"hyperlink": "Hyperlink",
"invalid-url": "Invalid URL format",
"url-must-use-http-or-https": "URL must use http or https protocol"

These should be translated to maintain proper internationalization. While this doesn't affect functionality, it degrades the user experience for non-English users.

Consider adding a follow-up task to get these strings translated, or use a translation service to provide initial translations.

Security: Frontend URL validation uses different logic than backend

📄 openmetadata-ui/src/main/resources/ui/src/components/common/CustomPropertyTable/PropertyValue.tsx:586 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java:2541
The frontend validateSafeUrl function in PropertyValue.tsx checks for ['http:', 'https:'].includes(parsed.protocol) which includes the trailing colon, while the backend validateHyperlinkUrl method uses scheme.equalsIgnoreCase("http") without the colon.

While both approaches work correctly (JavaScript's URL API returns protocol with colon, Java's URI returns scheme without), this inconsistency could lead to confusion during maintenance. The validation logic is fundamentally correct on both sides, but consider adding a comment in the frontend code explaining the JavaScript URL API behavior for clarity.

The defense-in-depth approach of validating on both frontend and backend is good security practice.

Security: Hyperlink URL lacks protocol validation for XSS protection

📄 openmetadata-ui/src/main/resources/ui/src/components/common/CustomPropertyTable/PropertyValue.tsx:1040-1055 🔗 CWE-79: Cross-site Scripting (XSS)
The hyperlink URL is rendered directly in the href attribute of Typography.Link without validating that it uses a safe protocol. While Ant Design's form validation checks for valid URL format, it may still allow javascript: protocol URLs which could lead to XSS attacks.

The form validation:

{
  type: 'url',
  message: t('message.invalid-url'),
}

This validates URL format but doesn't restrict to safe protocols. When rendered:

<Typography.Link
  href={hyperlinkValue.url}
  ...

Impact: An attacker could set a hyperlink value to javascript:alert(document.cookie) which would execute JavaScript when clicked.

Suggested fix: Add protocol validation to ensure only http: or https: URLs are accepted:

const isSafeUrl = (url: string) => {
  try {
    const parsed = new URL(url);
    return ['http:', 'https:'].includes(parsed.protocol);
  } catch {
    return false;
  }
};

// In the form validation:
{
  validator: (_, value) => 
    isSafeUrl(value) ? Promise.resolve() : Promise.reject('URL must use http or https protocol'),
}

// Or sanitize when rendering:
href={isSafeUrl(hyperlinkValue.url) ? hyperlinkValue.url : '#'}
Edge Case: Hyperlink type shows nothing instead of placeholder when empty

📄 openmetadata-ui/src/main/resources/ui/src/components/common/CustomPropertyTable/PropertyValue.tsx:1074-1079
When a hyperlink custom property has no value, users see nothing instead of the "No Data" placeholder that other property types show.

The condition at line 1074 includes isHyperlinkType:

return !isUndefined(value) || isTableType || isHyperlinkType ? (
  propertyValue
) : (
  <span className="text-grey-muted" data-testid="no-data">
    {t('message.no-data')}
  </span>
);

When value is undefined and isHyperlinkType is true, this condition evaluates to true and calls getPropertyValue(). However, getPropertyValue() returns null for hyperlink type when there's no URL:

if (!hyperlinkValue?.url) {
  return null;
}

This results in nothing being rendered - neither the hyperlink nor the "No Data" placeholder.

Suggested fix: Remove isHyperlinkType from the condition, or handle the empty state properly:

return !isUndefined(value) || isTableType ? (
  propertyValue
) : (
  ...
);

Or in getPropertyValue() for hyperlink, don't return null but let the outer condition handle it.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@aniketkatkar97 aniketkatkar97 merged commit 64c98ef into main Jan 29, 2026
33 of 39 checks passed
@aniketkatkar97 aniketkatkar97 deleted the copilot/add-hyperlink-support branch January 29, 2026 09:34
@github-actions
Copy link
Contributor

Failed to cherry-pick changes to the 1.12.0 branch.
Please cherry-pick the changes manually.
You can find more details here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants