Skip to content

TRUNK-6550: Logging should not be vulnerable to injection attacks#6128

Open
jwnasambu wants to merge 2 commits into
openmrs:masterfrom
jwnasambu:TRUNK-6549
Open

TRUNK-6550: Logging should not be vulnerable to injection attacks#6128
jwnasambu wants to merge 2 commits into
openmrs:masterfrom
jwnasambu:TRUNK-6549

Conversation

@jwnasambu
Copy link
Copy Markdown
Contributor

@jwnasambu jwnasambu commented May 26, 2026

Description of what I changed

I updated logging statements to prevent log injection vulnerabilities by sanitizing untrusted input before writing it to logs. Specifically, carriage return (\r) and newline (\n) characters are replaced to prevent attackers from forging or manipulating log entries. This ensures log integrity and aligns with secure logging practices recommended by SonarQube and OWASP.

Issue I worked on

https://openmrs.atlassian.net/browse/TRUNK-6550

Checklist: I completed these to help reviewers :)

  • My IDE is configured to follow the code style of this project.

    No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend

  • I have added tests to cover my changes. (If you refactored
    existing code that was well tested you do not have to add tests)

    No? -> write tests and add them to this commit git add . && git commit --amend

  • I ran mvn clean package right before creating this pull request and
    added all formatting changes to my commit.

    No? -> execute above command

  • All new and existing tests passed.

    No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.

  • My pull request is based on the latest changes of the master branch.

    No? Unsure? -> execute command git pull --rebase upstream master

@jwnasambu jwnasambu marked this pull request as draft May 26, 2026 12:34
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 26, 2026

Codecov Report

❌ Patch coverage is 56.75676% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.12%. Comparing base (708c62c) to head (2cad134).

Files with missing lines Patch % Lines
...mrs/web/filter/initialization/TestInstallUtil.java 0.00% 6 Missing ⚠️
...org/openmrs/module/web/ModuleResourcesServlet.java 42.85% 3 Missing and 1 partial ⚠️
...ain/java/org/openmrs/module/web/ModuleServlet.java 81.81% 2 Missing ⚠️
...eb/filter/initialization/InitializationFilter.java 66.66% 2 Missing ⚠️
...va/org/openmrs/web/filter/update/UpdateFilter.java 50.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6128      +/-   ##
============================================
+ Coverage     59.04%   59.12%   +0.07%     
- Complexity     9237     9254      +17     
============================================
  Files           693      693              
  Lines         37257    37271      +14     
  Branches       5485     5487       +2     
============================================
+ Hits          21999    22037      +38     
+ Misses        13287    13252      -35     
- Partials       1971     1982      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jwnasambu jwnasambu force-pushed the TRUNK-6549 branch 2 times, most recently from c1b5d35 to 8b02b2b Compare May 27, 2026 11:20
@jwnasambu jwnasambu marked this pull request as ready for review May 27, 2026 11:46
@sonarqubecloud
Copy link
Copy Markdown

@jwnasambu
Copy link
Copy Markdown
Contributor Author

jwnasambu commented May 27, 2026

@dkayiwa, @chibongho kindly feel free to review my PR at your convenient time please!

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