Skip to content

Fix/entity name validation v2#27886

Open
jaya6400 wants to merge 17 commits into
open-metadata:mainfrom
jaya6400:fix/entity-name-validation-v2
Open

Fix/entity name validation v2#27886
jaya6400 wants to merge 17 commits into
open-metadata:mainfrom
jaya6400:fix/entity-name-validation-v2

Conversation

@jaya6400

@jaya6400 jaya6400 commented May 4, 2026

Copy link
Copy Markdown

Describe your changes:

Fixes #23268

I updated the regex patterns in JSON schema definitions to block reserved
FQN separator characters (::, >, ") and newlines (\n) from being
used in entity and column names.

The backend was accepting entity names with special characters like double
quotes and newlines, which caused issues in FQN construction since these
are reserved separator characters in OpenMetadata's FQN system.

Changes made:

  • openmetadata-spec/src/main/resources/json/schema/type/basic.json — updated pattern for entityName and testCaseEntityName
  • openmetadata-spec/src/main/resources/json/schema/entity/data/table.json — updated pattern for columnName

Pattern changed from ^((?!::).)*$ to ^((?!::)[^><\"|\\x00-\\x1f])*$

Unit Test: #27521 (comment)

Type of change:

  • Bug fix

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes #23268: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Migration scripts are not needed for this change as it only tightens input validation at the schema level. Existing data is not affected — only new entity creation requests will be validated against the updated pattern.


Summary by Gitar

  • Frontend regex synchronization:
    • Updated TEST_CASE_NAME_REGEX in regex.constants.ts to alias ENTITY_NAME_REGEX, ensuring frontend validation for test case names strictly matches backend requirements.
  • CSV import validation:
    • Refactored shouldValidateUserNameWithEmailPrefix to improve resilience, ensuring proper error reporting when name pattern or email format validations fail.
  • Test and infrastructure hardening:
    • Standardized test JVM timezone to UTC and fixed project root discovery in TestSuiteBootstrap.java to resolve cross-platform CI issues.
    • Added comprehensive integration tests for pipeline task name validation in PipelineValidation.spec.ts and PipelineResourceIT.java.
    • Corrected data type mismatches for timeout in PipelineClass.ts and PipelineServiceClass.ts to stabilize integration tests.

This will update automatically on new commits.

------|----------|
| openmetadata-spec/src/main/resources/json/schema/type/basic.json | Updated entityName and testCaseEntityName patterns to block >, <, ", |, and ASCII control characters in addition to the existing :: exclusion |
| openmetadata-spec/src/main/resources/json/schema/entity/data/table.json | Added new stricter pattern to columnName, tableConstraints.columns.items, and partitionColumnDetails.columnName; constraint/partition column fields now have a pattern but no minLength, matching pre-existing inline behaviour |
| openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java | Refactored shouldValidateUserNameWithEmailPrefix into a dedicated method using startsWith instead of a hard-coded pattern string, making it resilient to future pattern changes |
| openmetadata-ui/src/main/resources/ui/src/constants/regex.constants.ts | Updated ENTITY_NAME_REGEX to block | and control chars; TEST_CASE_NAME_REGEX was NOT updated despite the corresponding backend testCaseEntityName receiving the same stricter pattern, leaving a frontend-backend mismatch for test case names |
| openmetadata-integration-tests/src/test/java/org/openmetadata/it/bootstrap/TestSuiteBootstrap.java | Sets JVM timezone to UTC at bootstrap, and uses Paths.get() for safer cross-platform project root resolution |

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Entity or Column Name Input] --> B{Passes new pattern?}
    B -->|blocked chars absent| C[Pass validation]
    B -->|contains :: > < pipe quote or ctrl char| D[Reject with 400]

    C --> E{User entity via CSV import?}
    E -->|Yes| F{shouldValidateUserNameWithEmailPrefix}
    F -->|No name or email violation present| G[Run email-prefix check]
    F -->|Name pattern or email violation exists| H[Skip email-prefix check]
    G --> I{Email prefix valid?}
    I -->|Yes| J[Import success]
    I -->|No| K[Import failure]
    H --> K
    E -->|No| J

    subgraph Schemas updated
        S1[basic.json entityName and testCaseEntityName]
        S2[table.json columnName constraint and partition cols]
        S3[pipeline.json task name]
        S4[searchIndex.json field name]
        S5[schema.json topic field name]
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Entity or Column Name Input] --> B{Passes new pattern?}
    B -->|blocked chars absent| C[Pass validation]
    B -->|contains :: > < pipe quote or ctrl char| D[Reject with 400]

    C --> E{User entity via CSV import?}
    E -->|Yes| F{shouldValidateUserNameWithEmailPrefix}
    F -->|No name or email violation present| G[Run email-prefix check]
    F -->|Name pattern or email violation exists| H[Skip email-prefix check]
    G --> I{Email prefix valid?}
    I -->|Yes| J[Import success]
    I -->|No| K[Import failure]
    H --> K
    E -->|No| J

    subgraph Schemas updated
        S1[basic.json entityName and testCaseEntityName]
        S2[table.json columnName constraint and partition cols]
        S3[pipeline.json task name]
        S4[searchIndex.json field name]
        S5[schema.json topic field name]
    end
Loading

Comments Outside Diff (1)

  1. openmetadata-ui/src/main/resources/ui/src/constants/regex.constants.ts, line 51 (link)

    P1 TEST_CASE_NAME_REGEX not updated to match new backend pattern

    ENTITY_NAME_REGEX was updated to block | and ASCII control characters, and the backend testCaseEntityName schema received the same tightening. However, TEST_CASE_NAME_REGEX still uses the old lookahead-only form /^((?!::)(?!")(?!>).)*$/ — it does not exclude | or control characters (other than \n). A user submitting my|testcase in the data quality test case form will pass frontend validation (line 410 in TestCaseFormV1.tsx) but receive a 400 from the backend, creating a confusing silent failure.

    The constant should be updated to: /^((?!::)[^><"|\u0000-\u001f])*$/

Reviews (1): Last reviewed commit: "fix: address pipeline validation review ..." | Re-trigger Greptile

Greptile Summary

This PR tightens entity-name validation across the backend, frontend, and generated ingestion models. The main changes are:

  • Stricter JSON schema patterns for entity, test case, column, pipeline task, search index, and topic schema field names.
  • Frontend regex updates so test case names use the same validation as entity names.
  • CSV import validation cleanup for user name and email-prefix checks.
  • Integration and UI test coverage for the new validation behavior.
  • Generated ingestion model updates for schemas with unsupported regex patterns.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/src/constants/regex.constants.ts Updates entity-name validation and reuses it for test case names.
openmetadata-ui/src/main/resources/ui/src/constants/regex.constants.test.ts Adds frontend regex coverage for reserved FQN characters and control characters.
openmetadata-spec/src/main/resources/json/schema/type/basic.json Tightens shared entity and test case entity name schema patterns.
openmetadata-spec/src/main/resources/json/schema/entity/data/table.json Adds stricter column-name validation for table column references and partition metadata.
openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java Refactors CSV user validation gating around name and email validation messages.
scripts/datamodel_generation.py Updates generated ingestion model post-processing for tightened schema regex handling.

Comments Outside Diff (1)

  1. openmetadata-ui/src/main/resources/ui/src/constants/regex.constants.ts, line 51 (link)

    P1 TEST_CASE_NAME_REGEX not updated to match new backend pattern

    ENTITY_NAME_REGEX was updated to block | and ASCII control characters, and the backend testCaseEntityName schema received the same tightening. However, TEST_CASE_NAME_REGEX still uses the old lookahead-only form /^((?!::)(?!")(?!>).)*$/ — it does not exclude | or control characters (other than \n). A user submitting my|testcase in the data quality test case form will pass frontend validation (line 410 in TestCaseFormV1.tsx) but receive a 400 from the backend, creating a confusing silent failure.

    The constant should be updated to: /^((?!::)[^><"|\u0000-\u001f])*$/

Reviews (16): Last reviewed commit: "fix: correct quote escaping in validator..." | Re-trigger Greptile

Context used (3)

  • Context used - CLAUDE.md (source)
  • Context used - openmetadata-ui-core-components/CLAUDE.md (source)
  • Context used - AGENTS.md (source)

@jaya6400 jaya6400 requested a review from a team as a code owner May 4, 2026 12:09
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

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!

@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

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!

@jaya6400

jaya6400 commented May 4, 2026

Copy link
Copy Markdown
Author

Hi @aniketkatkar97,

Created this new PR to replace #27521, the previous one had unintended files from a mvn spotless:apply run. Cherry-picked only the relevant commits for a clean diff (15 files).

Updates since last review:

  • Fixed ValidatorUtilTest.java assertion to match updated regex pattern
  • Fixed EntityCsv.java contains() string to match actual validator output
  • Added missing | pipe character test case in regex.constants.test.ts

Could a maintainer add the safe to test label? @harshach @sonika-shah @Rohit0301 @anuj-kumary

Thanks!

@aniketkatkar97 aniketkatkar97 added the safe to test Add this label to run secure Github workflows on PRs label May 5, 2026
@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 63%
64.07% (74407/116127) 47.6% (43831/92076) 48.62% (13364/27484)

@gitar-bot

gitar-bot Bot commented May 26, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 1 resolved / 2 findings

Tightens entity and column name validation regex to reject reserved FQN characters and control codes, resolving the missing pipe character test coverage. Consider whether silently skipping empty extension values in CSV imports might mask user configuration errors.

💡 Edge Case: Silently skipping empty extension values may hide user errors

📄 openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java:613-615

The change at line 613-615 converts an empty extension value from an error (previously reported via deferredFailure) to a silent skip (continue). While this may be intentional to allow partial extension definitions, it means users who accidentally leave a value blank in their CSV will receive no feedback that data was dropped. Consider at minimum logging a warning, or documenting this as intentional lenient behavior.

Fix
if (value.isEmpty()) {
  LOG.debug("Skipping empty extension value for key '{}' in record {}", key, csvRecord.getRecordNumber());
  continue;
}
✅ 1 resolved
Quality: Missing test coverage for pipe | character rejection

📄 openmetadata-ui/src/main/resources/ui/src/constants/regex.constants.test.ts:162-170
The updated regex blocks | (pipe) along with >, <, ", and control characters, but the frontend test suite (regex.constants.test.ts) does not include a test case for |. All other newly-blocked characters have corresponding test assertions. Adding this ensures the full intent of the regex is validated.

Also worth noting: according to the ANTLR grammar in Fqn.g4, the actual FQN-reserved characters are . and " (with \ as escape). Characters >, <, and | are not FQN separators per the grammar. The PR description calls them "reserved FQN separator characters" — consider updating documentation/comments to clarify these are blocked for safety/consistency rather than being FQN separators.

🤖 Prompt for agents
Code Review: Tightens entity and column name validation regex to reject reserved FQN characters and control codes, resolving the missing pipe character test coverage. Consider whether silently skipping empty extension values in CSV imports might mask user configuration errors.

1. 💡 Edge Case: Silently skipping empty extension values may hide user errors
   Files: openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java:613-615

   The change at line 613-615 converts an empty extension value from an error (previously reported via `deferredFailure`) to a silent skip (`continue`). While this may be intentional to allow partial extension definitions, it means users who accidentally leave a value blank in their CSV will receive no feedback that data was dropped. Consider at minimum logging a warning, or documenting this as intentional lenient behavior.

   Fix:
   if (value.isEmpty()) {
     LOG.debug("Skipping empty extension value for key '{}' in record {}", key, csvRecord.getRecordNumber());
     continue;
   }

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@jaya6400

Copy link
Copy Markdown
Author

Hi maintainers, I updated the branch and I see number of CI checks has been reduced now.

One workflow still appears to be awaiting maintainer approval. Could someone please approve/rerun it?

Also, Copilot is already listed as a reviewer, but I don’t have an option to re-request a fresh review from my account. If a fresh Copilot review is required, could someone please re-request it?

@ayush-shah @chirag-madlani @aniketkatkar97

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Comment thread openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java Outdated
@ayush-shah

Copy link
Copy Markdown
Member

Could you help validate two things in the current validation change?

  • In PipelineValidation.spec.ts, could the Dagster service fixture use a valid URI for host and a numeric value for timeout? I am asking because the test should fail only on pipeline task-name validation, but the current service payload has unrelated invalid-looking connector fields.
  • In EntityCsv.java, could we avoid matching the full rendered regex string when deciding whether to run validateUserNameWithEmailPrefix? That message is now coupled to the exact schema regex, so future regex wording changes can silently change CSV validation behavior. Would checking a stable field/path or a narrower stable prefix be safer?

@jaya6400

Copy link
Copy Markdown
Author

Could you help validate two things in the current validation change?

  • In PipelineValidation.spec.ts, could the Dagster service fixture use a valid URI for host and a numeric value for timeout? I am asking because the test should fail only on pipeline task-name validation, but the current service payload has unrelated invalid-looking connector fields.
  • In EntityCsv.java, could we avoid matching the full rendered regex string when deciding whether to run validateUserNameWithEmailPrefix? That message is now coupled to the exact schema regex, so future regex wording changes can silently change CSV validation behavior. Would checking a stable field/path or a narrower stable prefix be safer?

Thanks for pointing these out. I updated both areas in latest commit 783df67:

  1. PipelineValidation.spec.ts now uses a valid Dagster URI for host and a numeric timeout, so the service fixture should not introduce unrelated validation failures. I also updated the shared Playwright pipeline fixtures that had the same Dagster payload.

  2. EntityCsv.java no longer checks the full rendered regex string. It now uses parsed validation entries and skips validateUserNameWithEmailPrefix based on stable name-pattern/email-format validation signals instead of coupling the logic to the exact schema regex text.

I added focused tests for the EntityCsv behavior as well.

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

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

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.

@jaya6400

Copy link
Copy Markdown
Author

Hi team,

I updated PR #27886 against main and cleaned it up to 18 file changes. Greptile gave it a 5/5 confidence score.

I inspected the CI logs for the remaining 28 failing checks, but they do not seem related to my code changes. It looks like an environment mismatch or an unrelated issue. Can a maintainer please confirm this? Do I need to add any other tests for python files, or can these checks be skipped to merge this PR?

Also, please link issue #23268 to the 'Shipping' project board to clear the checklist block.

Thanks! @ayush-shah @aniketkatkar97 @ShaileshParmar11 @PubChimps @pmbrull @chirag-madlani @harshach
IMG_20260620_011250

@aniketkatkar97

Copy link
Copy Markdown
Member

Hey @jaya6400, can you please sync your branch with the latest main of Openmetadata? I see the your forked repo's main is behind by a week.

@jaya6400

Copy link
Copy Markdown
Author

Hey @jaya6400, can you please sync your branch with the latest main of Openmetadata? I see the your forked repo's main is behind by a week.

Thanks @aniketkatkar97 , for pointing it out. I have synced my forked main branch with the Openmetadata repo and safely rebased my feature branch (fix/entity-name-validation-v2) on top of the latest changes.
Capture

jaya6400 added 17 commits July 11, 2026 21:07
…names

Fixes open-metadata#23268

Updated regex patterns in JSON schema definitions to block reserved
FQN separator characters (::, >, ") and newlines in entity names.

Files changed:
- openmetadata-spec/.../type/basic.json: entityName, testCaseEntityName
- openmetadata-spec/.../entity/data/table.json: columnName
Updated pattern to use \x00-\x1f range to block all control characters
including \r, \n, \t, \0 in addition to reserved FQN characters.
This also restores the \r restriction that was present in the original
dot metacharacter but lost in the character class rewrite.
…hema fields and UI

- Added < and | to blocked characters (consistent with entityLink pattern)
- Updated searchIndexFieldName pattern in searchIndex.json
- Added pattern validation to tableConstraint columns and partitionColumnDetails in table.json
- Updated UI ENTITY_NAME_REGEX to match backend validation
- Added unit tests for new blocked characters in regex.constants.test.ts

Addresses review feedback on open-metadata#27521
Addresses inconsistency flagged in code review - columnName definition
was missing < and | from blocked characters unlike other patterns.
…erved characters

Added Java integration tests to verify that entity names containing
reserved FQN characters (::, >, <, ", |) and control characters are
rejected by the backend validation.

Files changed:
- TableResourceIT.java: added column name validation tests
- TopicResourceIT.java: added schema field name validation tests
- PipelineResourceIT.java: added task name validation tests
- SearchIndexResourceIT.java: added search index field name validation tests
- TestSuiteBootstrap.java: updated bootstrap for test suite
- pipeline.json: updated pattern for task names
- schema.json: updated pattern for schema field names
…less formatting

- Added minLength: 1 to pipeline task name to prevent empty strings
- Applied mvn spotless:apply formatting fixes
…ttern

- TEST_CASE_NAME_REGEX still used the old lookahead-only pattern and did not block `|` or ASCII control characters, unlike ENTITY_NAME_REGEX and
the tightened testCaseEntityName pattern in basic.json. This allowed
names like `my|testcase` to pass client-side validation on the data
quality test case form, then fail with a 400 from the backend.
- Reuses ENTITY_NAME_REGEX directly to prevent future drift between the two constants.
@gitar-bot

gitar-bot Bot commented Jul 12, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 1 resolved / 2 findings

Tightens entity and column name validation regex to reject reserved FQN characters and control codes, resolving the missing pipe character test coverage. Consider whether silently skipping empty extension values in CSV imports might mask user configuration errors.

💡 Edge Case: Silently skipping empty extension values may hide user errors

📄 openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java:613-615

The change at line 613-615 converts an empty extension value from an error (previously reported via deferredFailure) to a silent skip (continue). While this may be intentional to allow partial extension definitions, it means users who accidentally leave a value blank in their CSV will receive no feedback that data was dropped. Consider at minimum logging a warning, or documenting this as intentional lenient behavior.

Fix
if (value.isEmpty()) {
  LOG.debug("Skipping empty extension value for key '{}' in record {}", key, csvRecord.getRecordNumber());
  continue;
}
✅ 1 resolved
Quality: Missing test coverage for pipe | character rejection

📄 openmetadata-ui/src/main/resources/ui/src/constants/regex.constants.test.ts:162-170
The updated regex blocks | (pipe) along with >, <, ", and control characters, but the frontend test suite (regex.constants.test.ts) does not include a test case for |. All other newly-blocked characters have corresponding test assertions. Adding this ensures the full intent of the regex is validated.

Also worth noting: according to the ANTLR grammar in Fqn.g4, the actual FQN-reserved characters are . and " (with \ as escape). Characters >, <, and | are not FQN separators per the grammar. The PR description calls them "reserved FQN separator characters" — consider updating documentation/comments to clarify these are blocked for safety/consistency rather than being FQN separators.

🤖 Prompt for agents
Code Review: Tightens entity and column name validation regex to reject reserved FQN characters and control codes, resolving the missing pipe character test coverage. Consider whether silently skipping empty extension values in CSV imports might mask user configuration errors.

1. 💡 Edge Case: Silently skipping empty extension values may hide user errors
   Files: openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java:613-615

   The change at line 613-615 converts an empty extension value from an error (previously reported via `deferredFailure`) to a silent skip (`continue`). While this may be intentional to allow partial extension definitions, it means users who accidentally leave a value blank in their CSV will receive no feedback that data was dropped. Consider at minimum logging a warning, or documenting this as intentional lenient behavior.

   Fix:
   if (value.isEmpty()) {
     LOG.debug("Skipping empty extension value for key '{}' in record {}", key, csvRecord.getRecordNumber());
     continue;
   }

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 👍 / 👎 | Gitar

@sonarqubecloud

Copy link
Copy Markdown

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Backend: Validate if the entity names are valid in terms of Name, Fqn

6 participants