Restore encounter panel privilege enforcement#2612
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR removes redundant privilege-flag resets in two encounter panel actions, adds a regression test for denied privilege paths, and refactors ChangesPrivilege Flag Reset Fix and Validation
Encounter Display Action Refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java`:
- Around line 56-57: readEncounterPageUtilSource currently constructs a file
path with Path.of(...) which bypasses repository path validation; change it to
use the repository's PathValidationUtils to build/validate the path (e.g., call
the PathValidationUtils method that validates/resolves a path segment before
reading) or, alternatively, restrict readEncounterPageUtilSource to only accept
an enum/constant for allowed filenames and map those to validated paths via
PathValidationUtils; update references to readEncounterPageUtilSource to pass
only validated/known names and ensure any thrown IOException handling remains
intact.
- Around line 36-54: The current tests in EncounterPanelPrivilegeRegressionTest
(methods shouldHonorConfiguredRolePrivilege_forEpisodePanel and
shouldHonorConfiguredRolePrivilege_forPregnancyPanel) only assert source text
via readEncounterPageUtilSource and should be changed to exercise the runtime
behavior: invoke the corresponding action handlers (EctDisplayEpisode2Action and
EctDisplayPregnancy2Action) or their public entry points under a test context
with a user that lacks the _newCasemgmt.episode/_newCasemgmt.pregnancy
privilege, and assert the denied-privilege outcome (e.g., exception, redirect,
error response or security check result) instead of matching source strings;
remove or replace the contains/containsPattern assertions and use the real
action invocation plus assertions on the denial branch to avoid tying tests to
implementation details like helper extraction or local variable names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5c3a8b4a-1651-442c-b366-7d876bf41f7f
📒 Files selected for processing (3)
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayEpisode2Action.javasrc/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayPregnancy2Action.javasrc/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
💤 Files with no reviewable changes (2)
- src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayEpisode2Action.java
- src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayPregnancy2Action.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: SpotBugs + Find Security Bugs
🧰 Additional context used
📓 Path-based instructions (8)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Useio.github.carlos_emr.carlos.*namespace for ALL new code; specific packages: DAOs inio.github.carlos_emr.carlos.commn.dao.*, Models inio.github.carlos_emr.carlos.commn.model.*, exception: ProviderDao inio.github.carlos_emr.carlos.dao.ProviderDao
UsePathValidationUtilsfor ALL file path operations that include user input
NEVER log or expose PHI (Protected Health Information) in logs or error messages
UseSpringUtils.getBean(ClassName.class)for Spring bean injection in Struts2 actions and non-Spring-managed classes
JavaDoc is required on all public classes and methods; do NOT include@authortags, use@sincetags with git log timestamps, use CARLOS copyright header for new filesUse package namespace io.github.carlos_emr.carlos.* for all new code instead of org.oscarehr.*
**/*.java: Java package names remain asio.github.carlos_emr.carlos.*for backward compatibility - DO NOT change these internal paths
Use parameterized queries ONLY - never string concatenation for SQL queries
PHI (Patient Health Information) must NEVER be logged or exposed - useLogSafeutility or exclude PHI fields from log statements
UsePathValidationUtilsfor ALL file path operations to prevent path traversal attacks
Class naming: Suffix == role + lifecycle. Use sanctioned suffixes only:*Action,*ViewModelAssembler,*ViewModel,*Loader,*Resolver,*Composer,*Validator,*Persister,*Calculator,*Parser,*ImportService,*Service,*Dao,*Dto,*Command. Never use*Manager,*Helper,*Utils, or compound suffixes
Spring integration: Prefer constructor injection for new actions and services. ReserveSpringUtils.getBean()only for legacy compatibility shims that cannot be constructor-wired yet
JavaDoc required for: public entrypoint classes, non-obvious public/protected methods, and healthcare-domain-specific method contracts. NOT required for: carrier DTOs, trivial records, getters/setters, private helpers
Do NOT add@au...
Files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
**/*.{jsp,java}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use OWASP Encoder for ALL user input output: in JSP use
${e:forHtml(value)}with taglib<%@ taglib uri="owasp.encoder.jakarta" prefix="e" %>, in Java useEncode.forHtml(value), with context-specific variants (forHtmlAttribute,forJavaScript,forCssString,forUri,forUriComponent)Use OWASP Encoder for ALL user inputs - prefer
<carlos:encode>tag or CARLOS EL functions (${carlos:forXxx()}) over legacy<e:forXxx>orEncode.*static calls
Files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
**/*.{java,sql}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use parameterized queries ONLY in SQL/HQL operations; never use string concatenation for SQL queries
Files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
**/*Test.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*Test.java: Unit tests must useCarlosUnitTestBasebase class for mocked tests (no database); integration tests must useCarlosTestBasefor H2 database tests
Follow BDD naming convention for test methods:should<Action>_<preposition><Condition>()with ONE underscore, camelCase, andshouldprefixUse BDD (Behavior-Driven Development) test naming convention:
should<Action>_<prepositionOrContext><Condition>()with exactly ONE underscore, camelCase throughout, starting lowercase after underscore (e.g.,shouldReturnTickler_whenValidIdProvided())
Files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
src/test/**/*.java
📄 CodeRabbit inference engine (.github/instructions/test-rules.instructions.md)
src/test/**/*.java: Use CarlosTestBase for integration tests with Spring context and H2 database
Use CarlosUnitTestBase for unit tests with mocked SpringUtils and no database
Use BDD naming convention: should_() with ONE underscore, camelCase, and 'should' prefix for test methods
Use hibernateTemplate.flush() NOT entityManager.flush() for HibernateDaoSupport DAOs
HBM property names are case-sensitive - verify property names match the .hbm.xml file
DAO save*() methods may override fields like update_date - account for this in test assertions
Do not assert instance identity across Spring contexts using isSameAs()
HQL LIKE queries require explicit '%' wildcards in test data
Files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
src/**/*.java
📄 CodeRabbit inference engine (GEMINI.md)
src/**/*.java: Keep Java packages asio.github.carlos_emr.carlos.*for backward compatibility - do NOT change these internal namespace paths
Preserve existing copyright headers in modified files and preserve GPL version exactly as-is; never remove existing copyright notices
Files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
**/*.{java,jsp,js,xml,sql,py,sh,gradle,xml}
📄 CodeRabbit inference engine (CLAUDE.md)
New copyright headers use CARLOS project header (see
docs/copyright-header-carlos.md). Modified files preserve all existing headers. Never remove existing copyright notices (violates GPL). Never change GPL version.
Files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
src/test/java/**/*Test.java
📄 CodeRabbit inference engine (CLAUDE.md)
src/test/java/**/*Test.java: Test class structure: Choose correct base class -CarlosTestBasefor integration tests with Spring context and database,CarlosUnitTestBasefor unit tests with mocked SpringUtils, domain-specific bases likeDemographicUnitTestBasefor unit tests with test data builders
Use@Tag("integration"),@Tag("dao"), and CRUD tags (@Tag("create"),@Tag("read"),@Tag("update"),@Tag("delete"),@Tag("query"),@Tag("search")) for filtering test execution
Useio.github.carlos_emr.carlos.test.logging.LogCapturefrom test utilities for Log4j2 assertions. Do not define localAbstractAppenderorCapturingAppender. Close static mocks in@AfterEach
For Log4j2 and static mocking coordination in tests: SpringUtils.getBean() returns instances from possibly different Spring contexts. Assert type withisInstanceOf()not identity withisSameAs()
Files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
🧠 Learnings (9)
📚 Learning: 2026-03-21T14:58:44.822Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 704
File: src/test/java/io/github/carlos_emr/carlos/commn/dao/Hl7TextInfoDaoIntegrationTest.java:109-110
Timestamp: 2026-03-21T14:58:44.822Z
Learning: For the test-only HibernateTemplate implementation in src/test/java/io/github/carlos_emr/carlos/test/base/HibernateTemplate.java, positional parameter binding is 1-based (it calls query.setParameter(i + 1, ...)). Therefore, in any test code that calls hibernateTemplate.find(queryString, ...), all HQL positional parameters in queryString must use ?1, ?2, etc. Never use ?0 (or other 0-based indices), otherwise Hibernate will raise a runtime parameter binding error.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
📚 Learning: 2026-05-24T01:02:42.707Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/webserv/rest/ReportingServiceUnitTest.java:163-270
Timestamp: 2026-05-24T01:02:42.707Z
Learning: In this repository’s Java test code (under src/test/java), CRUD/operation tags on test methods such as Tag("create"), Tag("read"), or Tag("query") are optional “filtering vocabulary” and are NOT required. Since CI does not enforce these CRUD/operation tags, do not flag missing CRUD/operation Tag values on test methods. Only enforce the mandatory test-type tags defined in CLAUDE.md (e.g., Tag("unit"), Tag("fast")).
Applied to files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
📚 Learning: 2026-05-24T01:02:47.540Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/UtilDateUtilitiesUnitTest.java:42-44
Timestamp: 2026-05-24T01:02:47.540Z
Learning: In this repository, the `CarlosUnitTestBase` test base class is intended only for tests that need to mock `SpringUtils` and `LogAction` statics. For pure static-method unit tests (e.g., tests like `UtilDateUtilitiesUnitTest`, `SafeEncodeUnitTest`, `QueryAppenderUnitTest`, `TextualizerUnitTest`, `RequestNegotiationUnitTest`), do not require/flag extending `CarlosUnitTestBase`—absence of `CarlosUnitTestBase` is expected when static mocking of `SpringUtils`/`LogAction` isn’t needed.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
📚 Learning: 2026-05-24T01:02:46.757Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/DateUtilsConvertDate8CharUnitTest.java:39-42
Timestamp: 2026-05-24T01:02:46.757Z
Learning: When reviewing this repo’s Java code that uses CRUD/operation method `Tag(...)` annotations, treat most operation tags (e.g., `Tag("read")`) as optional filtering vocabulary. Only `Tag("integration")` and `Tag("dao")` are required for type/layer classification. Because CI (e.g., `bdd-test-naming.yml`/Surefire) enforces only method naming conventions and does not verify operation tags, do not raise review issues solely for missing optional CRUD operation `Tag` annotations.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
📚 Learning: 2026-05-24T01:02:47.540Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/UtilDateUtilitiesUnitTest.java:42-44
Timestamp: 2026-05-24T01:02:47.540Z
Learning: In this repository’s JUnit 5 tests, treat CRUD/operation tags (Tag("read"), Tag("create"), Tag("update"), Tag("delete")) as optional filtering vocabulary. Only Tag("integration") and Tag("dao") are required for type/layer classification. If a test is missing one or more CRUD/operation tags, do not flag it as a required change; only enforce what CI actually checks (method/test naming conventions) and rely on Tag("integration")/Tag("dao") for the relevant classification.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
📚 Learning: 2026-05-24T01:02:48.955Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/utility/CachedDateFormatsUnitTest.java:48-50
Timestamp: 2026-05-24T01:02:48.955Z
Learning: For this repo’s Java test code (src/test/java/**), do not require CRUD/operation tag annotations like Tag("read") or Tag("write") on test classes or test methods. Treat those CRUD/operation tags as optional filtering vocabulary only. For layer/type distinction, only Tag("integration") and Tag("dao") are required. Code review should avoid flagging missing Tag("read"/"write") (or similar CRUD operation tags) when tests otherwise follow the CI-enforced naming conventions.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
📚 Learning: 2026-03-24T17:34:18.508Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 716
File: src/main/java/io/github/carlos_emr/carlos/eform/actions/RtlPreventions2Action.java:155-156
Timestamp: 2026-03-24T17:34:18.508Z
Learning: In this repository (carlos-emr/carlos), treat `demographic_no` as an internal database surrogate integer key (the PK of the demographic table), not as PHI. When reviewing code, do not flag logging or output of `demographic_no` as PHI exposure. PHI to flag includes clinical/demographic data elements like patient name, date of birth, HIN, and address.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
📚 Learning: 2026-04-07T02:57:10.703Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1111
File: src/main/java/io/github/carlos_emr/carlos/util/JDBCUtil.java:140-142
Timestamp: 2026-04-07T02:57:10.703Z
Learning: When reviewing this repository’s Java code, flag any unprotected use of `SchemaFactory.newInstance(...)` (e.g., instances that do not restrict `XMLConstants.ACCESS_EXTERNAL_DTD` and `XMLConstants.ACCESS_EXTERNAL_SCHEMA` to empty strings). Recommend migrating those call sites to `io.github.carlos_emr.carlos.utility.XmlUtils.createSecureSchemaFactory()` so XML Schema validation is hardened against external entity/schema access. Known migrated call sites include `IndicatorTemplateHandler.java` and `DemographicExportAction42Action.java` (post-PR `#1111`).
Applied to files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
📚 Learning: 2026-05-24T01:02:46.757Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/DateUtilsConvertDate8CharUnitTest.java:39-42
Timestamp: 2026-05-24T01:02:46.757Z
Learning: In this repo, `CarlosUnitTestBase` is intended only for tests that need to mock the `SpringUtils` / `LogAction` statics. For unit tests that only exercise pure static methods (e.g., `DateUtils`, `SafeEncode`, `QueryAppender`, `Textualizer`, `RequestNegotiation`) and do not interact with those statics, do not extend `CarlosUnitTestBase`; keep the test self-contained to avoid unnecessary setup with no benefit.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/security/EncounterPanelPrivilegeRegressionTest.java
Replace the source-inspection regression with behavioral coverage for episode and pregnancy panel denial. Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com (noreply@anthropic.com) Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com (noreply@anthropic.com) Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Ben-Heerema
force-pushed
the
copilot/fix-privilege-check-override
branch
from
e7d89b5 to
bd882ed
Compare
|
@coderabbitai review |
|
@coderabbitai review |
@Ben-Heerema Couldn't start the review: please upgrade to a paid plan to review bot-authored PRs. |
|
✅ Action performedReview finished.
|
|
|
@yingbull I think your concerns are addressed now :) |
|
Grabbing this one and finishing it off |
Signed-off-by: Cecil Foubert <Cfoubert0782@conestogac.on.ca>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Cecil Foubert <Cfoubert0782@conestogac.on.ca>
|
@cubic-dev-ai review |
@CecilFoubert Couldn't start the review: please upgrade to a paid plan to review bot-authored PRs. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java`:
- Around line 406-409: The JSON branch in EctDisplayAction’s response handling
writes bytes using the platform default charset and never sets a JSON content
type. Update the isJsonRequest path to set the response Content-Type to
application/json;charset=UTF-8 before writing, and change the
json.toString().getBytes() write in that block to use UTF-8 explicitly so
non-ASCII data is returned correctly.
- Around line 160-183: The null-demographic rebuild path in
prepareEncounterSessionBean/resolveDemographicNo is still unsafe because
rebuildEncounterSessionBean can be called with a null demographicNo and then
EctSessionBean.setUpEncounterPage dereferences it. Update the flow so a missing
demographicNo is rejected or handled before calling rebuildEncounterSessionBean,
or add an explicit sessionless/general-privilege branch in
prepareEncounterSessionBean to avoid rebuilding when bean is null and no request
parameter is present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d38f2bc1-c77c-40f3-a4e3-887d7788fbee
📒 Files selected for processing (1)
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📜 Review details
⚠️ CI failures not shown inline (10)
GitHub Actions: DCO / DCO Sign-Off: Restore encounter panel privilege enforcement
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mpr_json=$(curl -fsSL \�[0m
�[36;1m -H "Authorization: ***" \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m "https://api.github.com/repos/$REPO/pulls/$PR_NUMBER")�[0m
�[36;1mpr_commit_count=$(jq -r '.commits' <<< "$pr_json")�[0m
�[36;1mbase_ref=$(jq -r '.base.ref' <<< "$pr_json")�[0m
�[36;1m�[0m
�[36;1mif [ "$pr_commit_count" -gt 250 ]; then�[0m
�[36;1m echo "::error::CARLOS EMR DCO check cannot automatically verify $pr_commit_count commits because GitHub's pull request commits API is capped at 250 commits."�[0m
GitHub Actions: DCO / 0_DCO Sign-Off.txt: Restore encounter panel privilege enforcement
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mpr_json=$(curl -fsSL \�[0m
�[36;1m -H "Authorization: ***" \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m "https://api.github.com/repos/$REPO/pulls/$PR_NUMBER")�[0m
�[36;1mpr_commit_count=$(jq -r '.commits' <<< "$pr_json")�[0m
�[36;1mbase_ref=$(jq -r '.base.ref' <<< "$pr_json")�[0m
�[36;1m�[0m
�[36;1mif [ "$pr_commit_count" -gt 250 ]; then�[0m
�[36;1m echo "::error::CARLOS EMR DCO check cannot automatically verify $pr_commit_count commits because GitHub's pull request commits API is capped at 250 commits."�[0m
GitHub Actions: SpotBugs Analysis / SpotBugs + Find Security Bugs: Restore encounter panel privilege enforcement
Conclusion: failure
##[group]Run export REPORT="target/spotbugs-result.xml"
�[36;1mexport REPORT="target/spotbugs-result.xml"�[0m
�[36;1mif [ ! -f "$REPORT" ]; then�[0m
�[36;1m echo "::error::No SpotBugs report found at $REPORT"�[0m
GitHub Actions: SpotBugs Analysis / 0_SpotBugs + Find Security Bugs.txt: Restore encounter panel privilege enforcement
Conclusion: failure
##[group]Run export REPORT="target/spotbugs-result.xml"
�[36;1mexport REPORT="target/spotbugs-result.xml"�[0m
�[36;1mif [ ! -f "$REPORT" ]; then�[0m
�[36;1m echo "::error::No SpotBugs report found at $REPORT"�[0m
GitHub Actions: Automatic Dependency Submission (Maven) / submit-maven: Automatic Dependency Submission (Maven)
Conclusion: failure
##[group]Run if command -v mvn &> /dev/null; then
�[36;1mif command -v mvn &> /dev/null; then�[0m
�[36;1m version=$(mvn -q -v)�[0m
�[36;1m echo "ignore-wrapper=true" >> "$GITHUB_OUTPUT"�[0m
�[36;1melif [[ -x ./mvnw ]]; then�[0m
�[36;1m version=$(./mvnw -q -v)�[0m
�[36;1m echo "ignore-wrapper=false" >> "$GITHUB_OUTPUT"�[0m
�[36;1melse�[0m
�[36;1m echo "::error::No Maven executable found. Please ensure Maven is installed or a Maven wrapper is present."�[0m
GitHub Actions: Automatic Dependency Submission (Maven) / 0_submit-maven.txt: Automatic Dependency Submission (Maven)
Conclusion: failure
##[group]Run if command -v mvn &> /dev/null; then
�[36;1mif command -v mvn &> /dev/null; then�[0m
�[36;1m version=$(mvn -q -v)�[0m
�[36;1m echo "ignore-wrapper=true" >> "$GITHUB_OUTPUT"�[0m
�[36;1melif [[ -x ./mvnw ]]; then�[0m
�[36;1m version=$(./mvnw -q -v)�[0m
�[36;1m echo "ignore-wrapper=false" >> "$GITHUB_OUTPUT"�[0m
�[36;1melse�[0m
�[36;1m echo "::error::No Maven executable found. Please ensure Maven is installed or a Maven wrapper is present."�[0m
GitHub Actions: SonarCloud Analysis / Build and analyze: Restore encounter panel privilege enforcement
Conclusion: failure
##[group]Run if [ -z "$SONAR_TOKEN" ]; then
�[36;1mif [ -z "$SONAR_TOKEN" ]; then�[0m
�[36;1m echo "::error::SONAR_TOKEN is not available in this workflow run."�[0m
GitHub Actions: SonarCloud Analysis / Build and analyze: Restore encounter panel privilege enforcement
Conclusion: failure
Node 20 is being deprecated. This workflow is running with Node 24 by default. If you need to temporarily use Node 20, you can set the ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true environment variable. For more information see: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
Post job cleanup.
(node:4023) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
[command]/usr/bin/tar --posix -cf cache.tzst --exclude cache.tzst -P -C /home/runner/work/carlos/carlos --files-from manifest.txt --use-compress-program zstdmt
/usr/bin/tar: .sonar-cache/e93874b6711c87049339696a507b9c2b/sonar-cvereachability-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/06f6abcba0e155ab188fa8a3d1d759b8/sonar-java-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/be77970dffb3af77b743873d2acfb1b2/sonar-dbdjavafrontend-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/c6ef680a93f0d72a08c373315db8eed6/sonar-xml-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/1d9ea99bf4667c88719b48067a250a82/sonar-architecturejavafrontend-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/c5530adf25b595550fc6dda8e33d4291/sonar-jasmin-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/08a0b3fe78cd804013c377c24792dba2/sonar-commona3scontextcollector-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/b577e400c3b63c02eafc2c98c9f0f197/sonar-textenterprise-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/bc69b468a9274275cc4e49a3f8135d5d/sonar-architecture-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/38784d6093b56436b7506d76adff42dd/sonar-javaa3scontextcollector-plugin.jar: Cannot open: Permission denied
/usr/bin/tar: .sonar-cache/f40304095ec8ece109c8...
GitHub Actions: SonarCloud Analysis / Build and analyze: Restore encounter panel privilege enforcement
Conclusion: failure
file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 20:16:24.780 Invalid character encountered in file /workspace/src/main/webapp/images/icons/valid.png at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 20:16:24.780 Invalid character encountered in file /workspace/src/main/webapp/css/images/sort_both.png at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 20:16:24.811 Invalid character encountered in file /workspace/src/main/webapp/images/2.gif at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 20:16:24.811 Invalid character encountered in file /workspace/src/main/webapp/images/notepad_blank.gif at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 20:16:24.812 Invalid character encountered in file /workspace/src/main/webapp/billing/CA/ON/images/rightarrow.gif at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 20:16:24.816 Invalid character encountered in file /workspace/src/main/webapp/images/icons/132.png at line 1 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 20:16:24.825 Invalid character encountered in file /workspace/src/main/resources/oscar/oscarBilling/ca/bc/MSP/mspEditCodes.properties at line 661 for encoding UTF-8. Please fix file content or configure the encoding to be used using property 'sonar.sourceEncoding'.
[WARNING] 20:16:24.831 Invalid character encountered in file /workspace/src/main/webapp/images/clear.png at line 1 for encoding UTF-8. Please fix file content or configure ...
GitHub Actions: SonarCloud Analysis / 0_Build and analyze.txt: Restore encounter panel privilege enforcement
Conclusion: failure
##[group]Run if [ -z "$SONAR_TOKEN" ]; then
�[36;1mif [ -z "$SONAR_TOKEN" ]; then�[0m
�[36;1m echo "::error::SONAR_TOKEN is not available in this workflow run."�[0m
🧰 Additional context used
📓 Path-based instructions (9)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Useio.github.carlos_emr.carlos.*namespace for ALL new code; specific packages: DAOs inio.github.carlos_emr.carlos.commn.dao.*, Models inio.github.carlos_emr.carlos.commn.model.*, exception: ProviderDao inio.github.carlos_emr.carlos.dao.ProviderDao
UsePathValidationUtilsfor ALL file path operations that include user input
NEVER log or expose PHI (Protected Health Information) in logs or error messages
UseSpringUtils.getBean(ClassName.class)for Spring bean injection in Struts2 actions and non-Spring-managed classes
JavaDoc is required on all public classes and methods; do NOT include@authortags, use@sincetags with git log timestamps, use CARLOS copyright header for new files
**/*.java: Keep Java package names asio.github.carlos_emr.carlos.*for backward compatibility - DO NOT change internal package names
UsePathValidationUtilsfor ALL file path operations to prevent path traversal attacks
PHI (Patient Health Information) must NEVER be logged or exposed in error messages
Use NEW namespaceio.github.carlos_emr.carlos.*for ALL new code - avoid oldorg.oscarehr.*andoscar.*packages
Use sanctioned suffixes for layer naming:*Action,*ViewModelAssembler,*ViewModel,*Loader,*Resolver,*Composer,*Validator,*Persister,*Calculator,*Parser,*ImportService,*Service,*Dao,*Dto,*Command. Never combine two role-suffixes or use forbidden suffixes like*Manager,*Helper,*Utils,*Prep
Public JavaDoc is required for public entrypoint classes and non-obvious public/protected methods; carrier DTOs, getters/setters, and private helpers do not need method-by-method JavaDoc
DO NOT add@authortags in JavaDoc (misleading after Bitbucket→GitHub migration); use git history for accurate dates with@sincetags
Inline comments should explain the why: legacy compatibility, security/compliance constraints, file-format offsets, transaction boundaries, and non-obvious control flow. Do not narrate o...
Files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
**/*.{jsp,java}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use OWASP Encoder for ALL user input output: in JSP use
${e:forHtml(value)}with taglib<%@ taglib uri="owasp.encoder.jakarta" prefix="e" %>, in Java useEncode.forHtml(value), with context-specific variants (forHtmlAttribute,forJavaScript,forCssString,forUri,forUriComponent)Use OWASP Encoder null-safe CARLOS wrappers for all user input encoding:
<carlos:encode>tag,${carlos:forXxx()}EL functions, orSafeEncode.forXxx()Java methods. DO NOT use<e:forXxx>,${e:forXxx()}, orEncode.forXxx()directly in new code.
Files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
**/*.{java,sql}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use parameterized queries ONLY in SQL/HQL operations; never use string concatenation for SQL queries
Files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
src/**/*.java
📄 CodeRabbit inference engine (GEMINI.md)
src/**/*.java: Keep Java packages asio.github.carlos_emr.carlos.*for backward compatibility - do NOT change these internal namespace paths
Preserve existing copyright headers in modified files and preserve GPL version exactly as-is; never remove existing copyright notices
Files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
src/main/java/io/github/carlos_emr/carlos/**/*Action.java
📄 CodeRabbit inference engine (GEMINI.md)
src/main/java/io/github/carlos_emr/carlos/**/*Action.java: ALWAYS usePathValidationUtilsfor ALL file path operations to prevent path traversal attacks
Use the*2Action.javanaming convention for ALL new Struts2 actions; extendActionSupportfromorg.apache.struts2(not legacycom.opensymphony.xwork2)
Files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
{src/main/java/io/github/carlos_emr/carlos/**/*.java,src/main/webapp/**/*.jsp}
📄 CodeRabbit inference engine (GEMINI.md)
PHI (Patient Health Information) must NEVER be logged or exposed in error messages
Files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
src/main/java/io/github/carlos_emr/carlos/**/*.java
📄 CodeRabbit inference engine (GEMINI.md)
src/main/java/io/github/carlos_emr/carlos/**/*.java: All public classes and methods MUST have comprehensive JavaDoc; do NOT add@authortags; use@sincewith accurate git history dates; document@param,@return, and@throwstags with specific types
Add inline comments on separate lines (not same-line) for complex logic; include comprehensive comments for non-obvious algorithms or healthcare-specific calculations
Files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve all existing copyright headers in modified files; do not remove or change GPL version statements
Files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
**/*.{java,jsp,xml,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
New files must use CARLOS project header as defined in
docs/copyright-header-carlos.md
Files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
🧠 Learnings (11)
📚 Learning: 2026-03-06T23:54:01.319Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 563
File: src/main/java/io/github/carlos_emr/carlos/lab/ca/all/parsers/AlphaHandler.java:59-63
Timestamp: 2026-03-06T23:54:01.319Z
Learning: In the carlos-emr/carlos repository, for Java sources under src/main/java, empty no-arg constructors should have only a minimal one-line JavaDoc. Do not add since or other metadata tags to empty constructors, as they provide no meaningful value. Place documentation efforts in the class-level JavaDoc instead.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📚 Learning: 2026-03-13T21:29:23.775Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 590
File: src/main/java/io/github/carlos_emr/carlos/eform/actions/DisplayImage2Action.java:69-69
Timestamp: 2026-03-13T21:29:23.775Z
Learning: When reviewing Java source files, allow since Javadoc tags to reflect substantial rewrites or major API changes (e.g., security overhaul, new API surface). Do not flag the tag as incorrect solely because the file predates the tagged date. If flagging is considered, verify against commit history or release notes to confirm the tag corresponds to a meaningful change, not the original introduction. Apply this guidance across Java sources in the repository (src/main/java/**/*.java).
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📚 Learning: 2026-04-11T22:46:51.202Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1556
File: src/main/java/io/github/carlos_emr/carlos/demographic/dto/DemographicListItemDTO.java:74-179
Timestamp: 2026-04-11T22:46:51.202Z
Learning: In this repository, do not require method-level JavaDoc for plain public JavaBean-style getters and setters in Java files under src/main/java/**/*.java. Their meaning should be covered by field-level documentation and/or the class-level JavaDoc. Only require JavaDoc for non-trivial public methods (e.g., constructors with logic, factory methods, and computed/derived accessors where behavior is not just returning/setting a field). In code reviews, missing JavaDoc on simple getters/setters should not be flagged as a documentation violation.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📚 Learning: 2026-04-11T22:47:04.719Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1556
File: src/main/java/io/github/carlos_emr/carlos/provider/dto/ProviderSummaryDTO.java:57-104
Timestamp: 2026-04-11T22:47:04.719Z
Learning: In this repository, do not require JavaDoc for trivial getter and setter methods. Only JavaDoc for types/classes and for more meaningful public API points is expected—e.g., class-level JavaDoc, constructors, static factory methods, and non-trivial public methods (such as formatting helpers). In code reviews, ignore missing JavaDoc on getters/setters.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📚 Learning: 2026-05-24T01:01:46.756Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/main/java/io/github/carlos_emr/carlos/utility/CachedDateFormats.java:165-216
Timestamp: 2026-05-24T01:01:46.756Z
Learning: For Java sources under src/main/java/**/*.java, follow the repository’s Documentation Standards (CLAUDE.md) regarding Javadoc requirements: do not require mechanical param/return/throws tags for trivial wrapper/delegator public methods that are essentially one-line pass-throughs to a cached or underlying instance (e.g., methods like parseDefault/formatDefault/parse/format in CachedDateFormats.java). Instead, rely on class-level Javadoc to document the overall contracts, including thread-safety, null handling, return value semantics, and any mutation/restoration behavior (such as timezone restoration). Require full per-method Javadoc tag coverage only for non-trivial public methods whose behavior is not obvious from the context/signature.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📚 Learning: 2026-06-25T16:10:08.376Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 3023
File: src/main/java/io/github/carlos_emr/carlos/encounter/oscarConsultationRequest/pageUtil/EConsult2Action.java:287-328
Timestamp: 2026-06-25T16:10:08.376Z
Learning: When reviewing Java code that reconstructs an origin/authority from a `java.net.URI` (e.g., `scheme://host:port`), assume `URI#getHost()` may already return bracketed IPv6 literals such as `[::1]` (not just `::1`). Do not “re-bracket” IPv6 manually when rebuilding the origin from the host returned by `getHost()`. If `getHost()` is `null`, only then apply the malformed-colon/authority fallback rejection logic on that authority-fallback path (the path that runs when `getHost()` is `null`), not on the normal host-based reconstruction path.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📚 Learning: 2026-03-24T17:34:18.508Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 716
File: src/main/java/io/github/carlos_emr/carlos/eform/actions/RtlPreventions2Action.java:155-156
Timestamp: 2026-03-24T17:34:18.508Z
Learning: In this repository (carlos-emr/carlos), treat `demographic_no` as an internal database surrogate integer key (the PK of the demographic table), not as PHI. When reviewing code, do not flag logging or output of `demographic_no` as PHI exposure. PHI to flag includes clinical/demographic data elements like patient name, date of birth, HIN, and address.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📚 Learning: 2026-04-07T02:57:10.703Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1111
File: src/main/java/io/github/carlos_emr/carlos/util/JDBCUtil.java:140-142
Timestamp: 2026-04-07T02:57:10.703Z
Learning: When reviewing this repository’s Java code, flag any unprotected use of `SchemaFactory.newInstance(...)` (e.g., instances that do not restrict `XMLConstants.ACCESS_EXTERNAL_DTD` and `XMLConstants.ACCESS_EXTERNAL_SCHEMA` to empty strings). Recommend migrating those call sites to `io.github.carlos_emr.carlos.utility.XmlUtils.createSecureSchemaFactory()` so XML Schema validation is hardened against external entity/schema access. Known migrated call sites include `IndicatorTemplateHandler.java` and `DemographicExportAction42Action.java` (post-PR `#1111`).
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📚 Learning: 2026-04-04T04:11:45.844Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 937
File: src/main/java/io/github/carlos_emr/carlos/documentManager/actions/ManageDocument2Action.java:728-733
Timestamp: 2026-04-04T04:11:45.844Z
Learning: In PRs, when reviewing logging in *Action/*Manager/*Service/*Dao classes, treat the decision to include document filenames/paths in logs as an architectural/policy choice (not a log-sanitization/injection issue). Do not flag logging of sanitized document filenames/paths that have been passed through LogSanitizer as PHI exposure in the context of individual PRs. Limit review scope for “log sanitization” work to CRLF/log-injection prevention (and related injection-safety concerns), not to removing filename/path values from logs, unless there is a separate, enforceable technical reason beyond the injection-sanitization scope.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📚 Learning: 2026-04-05T04:38:53.740Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1000
File: src/main/java/io/github/carlos_emr/carlos/encounter/oscarMeasurements/pageUtil/EctAddMeasurementType2Action.java:82-85
Timestamp: 2026-04-05T04:38:53.740Z
Learning: In Struts2 action classes (e.g., `EctAddMeasurementType2Action` and similar), it’s acceptable to handle validation failures by (1) storing errors on the current request via `request.setAttribute("actionErrors", new ArrayList<>(getActionErrors()))` and (2) returning a result name like `"failure"` that forwards/dispatches to the target view (JSP) while preserving request attributes. Do NOT flag this as “errors lost on redirect” because it is not a redirect. Only raise error-loss concerns when `response.sendRedirect(...)` (or equivalent redirect behavior) is used and the code does not persist errors via a session/flash mechanism before returning `NONE`.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
📚 Learning: 2026-04-05T04:39:00.147Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1000
File: src/main/java/io/github/carlos_emr/carlos/dxresearch/pageUtil/dxResearchUpdateQuickList2Action.java:0-0
Timestamp: 2026-04-05T04:39:00.147Z
Learning: In this Struts2 codebase, when a Struts2 action returns a named result (e.g., "failure") that is mapped in struts XML to another action/JSP, the default result type is `type="dispatcher"` (server-side forward). In this case, original request parameters are preserved and should be available to the forwarded-to target via `StrutsParameter` binding. During code review, do NOT treat missing query/request parameters in the forwarded-to action as data-loss just because they aren’t explicitly listed; only flag missing parameters as a data-loss issue when the code performs a genuine client-side redirect using `response.sendRedirect(...)` and the action returns `NONE` after calling `sendRedirect` (where request parameters are not preserved across the redirect).
Applied to files:
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
🪛 ast-grep (0.44.0)
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java
[warning] 401-401: Regular expression is compiled from a non-literal, possibly user-controlled value. A crafted regex (or input matched against one) can trigger catastrophic backtracking and hang the thread (ReDoS). Use a hardcoded literal pattern, wrap untrusted text with Pattern.quote(...), or validate/length-limit the input and enforce a matching timeout before passing it to Pattern.compile / String.matches / String.replaceAll / String.replaceFirst.
Context: params.replaceAll(regex, "")
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-java)
[warning] 147-147: Prevent HTTP parameter pollution
Context: request.getParameter("json")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 160-160: Prevent HTTP parameter pollution
Context: request.getParameter("demographicNo")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 171-171: Prevent HTTP parameter pollution
Context: request.getParameter("demographicNo")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 198-198: Prevent HTTP parameter pollution
Context: request.getParameter("curDate")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 203-203: Prevent HTTP parameter pollution
Context: request.getParameter("providerNo")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 215-215: Prevent HTTP parameter pollution
Context: request.getParameter("curProviderNo")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 240-240: Prevent HTTP parameter pollution
Context: request.getParameter("reason")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 249-249: Prevent HTTP parameter pollution
Context: request.getParameter("encType")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 258-258: Prevent HTTP parameter pollution
Context: request.getParameter("userName")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 274-274: Prevent HTTP parameter pollution
Context: request.getParameter(parameterName)
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 283-283: Prevent HTTP parameter pollution
Context: request.getParameter(parameterName)
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 292-292: Prevent HTTP parameter pollution
Context: request.getParameter(parameterName)
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 301-301: Prevent HTTP parameter pollution
Context: request.getParameter("msgId")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 310-310: Prevent HTTP parameter pollution
Context: request.getParameter("source")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 315-315: Prevent HTTP parameter pollution
Context: request.getParameter(parameterName)
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 336-336: Prevent HTTP parameter pollution
Context: request.getParameter("cmd")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
[warning] 366-366: Prevent HTTP parameter pollution
Context: request.getParameter("hC")
Note: [CWE-235] Improper Handling of Extra Parameters. Security best practice.
(http-parameter-pollution)
🔇 Additional comments (2)
src/main/java/io/github/carlos_emr/carlos/encounter/pageUtil/EctDisplayAction.java (2)
400-405: The ast-grep ReDoS hint on Line 401 is a false positive:cmdoriginates fromgetCmd()(a subclass constant, not request input) and is neutralized viaPattern.quote(...); the resulting\bLITERAL\bpattern has no alternation/nested quantifiers, so catastrophic backtracking is not possible.
142-158: LGTM!Also applies to: 198-321, 334-398
Signed-off-by: Cecil Foubert <Cfoubert0782@conestogac.on.ca>
CecilFoubert
force-pushed
the
copilot/fix-privilege-check-override
branch
from
35d601f to
13a8bd4
Compare
|
Episode and pregnancy encounter panels evaluated role privileges, then immediately overwrote the result with
true, making those checks ineffective. This allowed users who could reach the encounter shell to see those panels regardless of_newCasemgmt.episode/_newCasemgmt.pregnancypermissions.This PR restores those panel-specific checks and also tightens the shared encounter entry gate so panel loading requires chart access instead of plain demographic read.
Shared encounter gate
EctDisplayAction, replace the pre-session privilege check from_demographic rto_eChart r.Panel-specific authorization
EctDisplayEpisode2Action.EctDisplayPregnancy2Action.OscarRoleObjectPrivilege.checkPrivilege(...)result now controls whether those panels populate.Effective behavior after this change
_eChart rto load the encounter panel flow at all._newCasemgmt.episode._newCasemgmt.pregnancy.Regression coverage
_eChartis missing._eChartis granted.Related Issues
Fixes the encounter panel privilege bypass described in this PR.
Also incorporates follow-up review feedback to use the correct shared chart-level gate.
How Was This Tested?
EncounterPanelPrivilegeRegressionUnitTest._eChartaccess blocks encounter-panel loading before session mutation_eChartaccess allows the shared flow to proceed_newCasemgmt.episodeprevents episode panel population_newCasemgmt.pregnancyprevents pregnancy panel population