Skip to content

Conversation

@debasishchakraborty-egovt
Copy link
Collaborator

@debasishchakraborty-egovt debasishchakraborty-egovt commented Nov 27, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced MDMS data query processing with improved handling of special characters in identifiers to ensure reliable and consistent query execution.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Walkthrough

The change adds SQL injection prevention to LIKE clause queries in the MDMS query builder. A new helper method escapeLikeWildcards() escapes wildcard characters and backslashes in input values before they're used in LIKE patterns with an ESCAPE clause.

Changes

Cohort / File(s) Summary
SQL Injection Prevention in LIKE Clauses
core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/MdmsDataQueryBuilder.java
Added escapeLikeWildcards(String input) private helper method to escape percent signs, underscores, and backslashes. Modified LIKE clause to use escaped tenantId with ESCAPE '\' to prevent wildcard injection attacks.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file modification with focused security enhancement
  • New private utility method with straightforward escape logic
  • Pattern applies consistently to one query clause
  • Low complexity; verify escape sequence handles all wildcard characters

Poem

🐰 A rabbit hops through SQL gates,
Where wildcards once escaped their fates,
Now backslashes guard the way,
Injection attacks won't stay—
Safe queries bloom, the burrow celebrates! 🛡️

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main objective of the pull request - fixing SQL vulnerability through escaping LIKE clause wildcards and adding a helper method to prevent wildcard injection.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sqlVulnerablity

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2a4365 and 3197c65.

📒 Files selected for processing (1)
  • core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/MdmsDataQueryBuilder.java (2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: holashchand
Repo: egovernments/Digit-Core PR: 741
File: core-services/egov-otp/src/main/java/org/egov/persistence/repository/TokenRepository.java:30-34
Timestamp: 2025-05-23T08:43:33.818Z
Learning: The MultiStateInstanceUtil.replaceSchemaPlaceholder method currently inserts raw schema names into SQL strings without proper sanitization (missing whitelist validation and Matcher.quoteReplacement usage), posing SQL injection risks. The team is aware of this issue and plans to address it in future designs across all services.
📚 Learning: 2025-05-23T08:43:33.818Z
Learnt from: holashchand
Repo: egovernments/Digit-Core PR: 741
File: core-services/egov-otp/src/main/java/org/egov/persistence/repository/TokenRepository.java:30-34
Timestamp: 2025-05-23T08:43:33.818Z
Learning: The MultiStateInstanceUtil.replaceSchemaPlaceholder method currently inserts raw schema names into SQL strings without proper sanitization (missing whitelist validation and Matcher.quoteReplacement usage), posing SQL injection risks. The team is aware of this issue and plans to address it in future designs across all services.

Applied to files:

  • core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/MdmsDataQueryBuilder.java
📚 Learning: 2025-05-22T11:09:09.323Z
Learnt from: holashchand
Repo: egovernments/Digit-Core PR: 739
File: core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/impl/SchemaDefinitionDbRepositoryImpl.java:70-77
Timestamp: 2025-05-22T11:09:09.323Z
Learning: In the MDMS V2 service ecosystem, the MultiStateInstanceUtil.replaceSchemaPlaceholder method is widely used across multiple services, making it difficult to change its implementation even when security improvements like SQL injection protection are identified. Security improvements to this shared functionality need to be planned as part of future architectural changes.

Applied to files:

  • core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/MdmsDataQueryBuilder.java
📚 Learning: 2025-05-22T11:05:57.893Z
Learnt from: holashchand
Repo: egovernments/Digit-Core PR: 739
File: core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/impl/SchemaDefinitionDbRepositoryImpl.java:70-77
Timestamp: 2025-05-22T11:05:57.893Z
Learning: In the MDMS V2 service, tenant ID validation is performed in the MultiStateInstanceUtil.replaceSchemaPlaceholder method, which checks that the tenant ID contains a dot and has sufficient length for schema extraction in a central instance environment.

Applied to files:

  • core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/MdmsDataQueryBuilder.java
🔇 Additional comments (2)
core-services/mdms-v2/src/main/java/org/egov/infra/mdms/repository/querybuilder/MdmsDataQueryBuilder.java (2)

68-81: Correct implementation with proper escape ordering.

The helper method correctly escapes LIKE wildcards in the right order:

  1. Backslash first (to prevent double-escaping)
  2. Then percent sign
  3. Then underscore

This ensures that an input like "test%" becomes "test\%" rather than incorrectly double-escaping to "test\\\%". The null handling is also appropriate.

The escape character used here (backslash) matches the ESCAPE '\\' clause specified in line 40, ensuring consistency between the escape logic and the SQL syntax.


38-44: Based on my verification, I can now provide the rewritten review comment:


LIKE wildcard injection fix is technically correct for PostgreSQL.

The implementation properly escapes special characters and uses the correct ESCAPE syntax for PostgreSQL. The escapeLikeWildcards() helper correctly escapes backslash first (preventing double-escaping), then % and _ wildcards. The fix safely prevents attacks like tenantId = "pb%" from bypassing access controls.

However, note that:

  1. Test coverage: No unit tests were found in the codebase to verify injection attempts are properly blocked. Consider adding tests for cases like:

    • tenantId = "pb%" (should match only literal "pb%")
    • tenantId = "test_value" (should match literal value, not wildcard)
  2. Similar vulnerabilities elsewhere: The DocumentQueryBuilder class in egov-document-uploader uses ILIKE with wildcards on the name and postedby fields without escaping—the same vulnerability pattern. This may need a similar fix if in scope.

  3. MdmsDataQueryBuilderV2: Uses exact match (=) for tenantId, so no vulnerability there.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants