[Penify]: Documentation for commit - c90f5bb by penify-dev[bot] · Pull Request #4 · carlos-emr/carlos

penify-dev · 2026-02-02T21:19:49Z

This PR focuses exclusively on updating and refining the documentation throughout the codebase. There are no functional changes to the code itself.

Changes:
src/main/java/io/github/carlos_emr/carlos/eform/actions/AddEForm2Action.java
src/main/java/io/github/carlos_emr/carlos/eform/data/EFormBase.java
src/main/java/io/github/carlos_emr/carlos/eform/util/EFormViewForPdfGenerationServlet.java
src/main/java/io/github/carlos_emr/carlos/sec/LoginFilter.java

🙏 Request:
Please review the changes to ensure that the documentation is clear, accurate, and adheres to your project's standards.
Any feedback regarding areas that might still need clarification or additional details would be highly appreciated.
You can also raise the request on the Penify Community or mail us at support@penify.dev

…dule Fix all 10 blocking issues identified in security review: Security Fixes (XSS & CSRF): - Issue #1: Restore Encode.forHtml() for message body in ViewMessage.jsp to prevent stored XSS - Issue #6: Add Encode.forHtml() for demographic_name in DisplayMessages.jsp - Issue #7: Add Encode.forJavaScript() for JS variables in ViewMessage.jsp - Issue #9: Add OWASP Encoder import and Encode.forHtml() in SentMessage.jsp - Issue #5: Add CSRF tokens to all 5 forms (CreateMessage, DisplayMessages search/list, ViewMessage main/modal) Functional Bug Fixes: - Issue #2: Add null checks for messageNo in btnRead/btnUnread handlers to prevent NPE - Issue #3: Fix validation function case mismatch (validatefields -> validateFields) - Issue #4: Fix attachment validation parseInt bug (parse idParts[2] not array) - Issue #8: Call writeToMessage() before validation to sync editor content HTML/UI Fixes: - Issue #10: Fix button self-closing tags, duplicate class attribute, invalid table nesting, malformed ID attributes with spaces Co-authored-by: Michael Yingbull <yingbull@users.noreply.github.com>

Functional bug fixes: - Fix #1: AddService.jsp used wrong i18n key (AddDepartment.msgDepartmentAdded) that showed "Department saved" after adding a service; changed to correct key AddService.msgServiceAdded - Fix #2: DisplayService.jsp - ${serviceDesc} EL expression always resolved to empty because serviceDesc was a scriptlet-local variable; added pageContext.setAttribute to expose it to EL - Fix #3: AddSpecialist.jsp - ${referralNoMsg} EL expression always empty for the same reason; added pageContext.setAttribute after assignment - Fix Bonus: Remove redundant XSS-vulnerable pre-load block in AddSpecialist.jsp (lines 133-140) that injected request.getAttribute("department") raw into JS; the safely-encoded equivalent at lines 229-235 was already present Defensive fixes: - Fix #4: EctConEditSpecialists2Action - update path lacked try-catch for NumberFormatException on Integer.parseInt(specId) and null check on the DAO.find() result; both added mirroring the delete path pattern - Fix #5: EctConEditSpecialists2Action - soft-delete loop did not emit an audit log entry; added LogAction.addLog(LogConst.DELETE, "specialist", ...) after each successful soft-delete. Also fixed log injection risk: changed string concatenation in warn() to SLF4J parameterized form ({}) - Fix #6: AddDepartment.jsp, AddInstitution.jsp, AddSpecialist.jsp - hidden id/specId fields emitted the literal string "null" in add-mode (Java null printed via <%= %>); guarded with ternary to emit empty string Issue #7 - EditInstitutions.jsp / EctConEditInstitutions2Action: - Renamed checkbox from name="specialists" to name="institutions" - Renamed action field/getter/setter from specialists to institutions - Updated delete-button label key from EditSpecialists.btnDeleteSpecialist (which showed "Delete Specialist" on an institution page) to new key EditInstitutions.btnDeleteInstitution - Added btnDeleteInstitution key to all 5 locale property files Code quality: - Fix #8: EctConTitlebar - made jspVect and displayNameVect private - Fix #9: EctConTitlebar - HTML-encode display names via Encode.forHtml() when generating nav link text; added org.owasp.encoder.Encode import Co-authored-by: Michael Yingbull <yingbull@users.noreply.github.com>

- Wrap user-controlled IDs in Encode.forJava() in all warn() log calls in EctConEditSpecialists2Action and EctConEditInstitutions2Action to prevent log injection / control-character forging (issue #2) - Encode contextPath with Encode.forHtmlAttribute() in EctConTitlebar for coding standards compliance (issue #3) - Add comprehensive JavaDoc to EctConTitlebar class, both constructors, and estBar() method per project documentation requirements (issue #4) Co-authored-by: Michael Yingbull <yingbull@users.noreply.github.com>

Addresses items #1-9 and #11-13 from the code review summary: #1 PdfWriterFactory: replace multiple setPageEvent() calls with PdfPageEventForwarder to prevent stampers from silently overwriting each other (confidentiality/promo text was never stamping) #2 PdfWriterFactory: throw IllegalStateException instead of returning null on DocumentException to fail fast and prevent NPEs in callers #3 ImagePDFCreator: wrap all TIFF/non-TIFF processing in try/finally to guarantee document.close() runs even when exceptions are thrown #4 FHIRCommunicationRequestHandler: use try-with-resources for PdfReader to prevent resource leak if getNumberOfPages() throws #5 FrmPDFServlet: guard Integer.parseInt(cfgVal[5]) with try/catch NumberFormatException, falling back to font size 12 with a log warning #6 pom.xml: declare openpdf-html 3.0.2 as an explicit direct dependency (LabPDFCreator directly imports HTMLWorker from this module) #7 FaxImporter: switch Base64.getDecoder() to getMimeDecoder() to handle MIME-formatted (line-wrapped) Base64 payloads from fax providers; add explicit null check for missing document payload #8 LabPDFCreator: remove unused MessageHandler handler parameter from private getTextColor() method and update its call site #9 ReplacedElementFactoryImpl: update outdated comment that still referenced "old lowagie Image class" — now uses OpenPDF 3.x #11 ImagePDFCreator: add null check for ImageInputStream returned by ImageIO.createImageInputStream() before passing to getImageReaders() #12 docs/pdf-dependency-consolidation-plan.md: add blank lines around Group E and Group F tables to satisfy markdownlint MD058 #13 FHIRCommunicationRequestHandler: remove raw fileName from error log to avoid leaking file path information Co-authored-by: Michael Yingbull <yingbull@users.noreply.github.com>

@Autowired

… in PDF rendering (#536) * docs: add PDF dependency consolidation plan Comprehensive plan to migrate from 5+ PDF libraries (iText 5 AGPL, XMLWorker, ultrabuk, Flying Saucer, PDFBox) down to 2 (OpenPDF LGPL + PDFBox). Covers 45 Java files across 4 phases with risk assessment, API migration mapping, and testing strategy. Key decisions: - Replace iText 5.5.13.5 (AGPL) with OpenPDF 2.0.x (LGPL) - Keep PDFBox 2.0.35 for merge/split/encrypt operations - Keep Flying Saucer 9.13.3 (already uses OpenPDF transitively) - Defer Doc2PDF removal and ultrabuk removal to separate PRs - No browser JS needed for PDF rendering (only PDF-embedded Acrobat JS) https://claude.ai/code/session_01TRZBbahEcwdwf4v31NoTDK * feat: migrate 45 files from iText 5 (AGPL) to OpenPDF 2.0.5 (LGPL) Migrate all PDF creation code from com.itextpdf.* to com.lowagie.* (OpenPDF 2.0.5), reducing license risk and consolidating PDF libraries. Changes across 46 files: - Add explicit OpenPDF 2.0.5 dependency (matches Flying Saucer 9.13.3) - Replace all com.itextpdf.text.* imports with com.lowagie.text.* - Replace BaseColor (iText-specific) with java.awt.Color (4 files) - Replace Font.FontFamily enum with Font int constants (1 file) - Replace iText codec.Base64 with java.util.Base64 (FaxImporter) - Consolidate PdfWriterFactory: remove deprecated lowagie overloads, migrate active iText overload to OpenPDF with full functionality - Update inline BadPasswordException reference (FaxImporter) Intentionally deferred: - Doc2PDF.java stays on iText (uses XMLWorkerHelper, separate PR) - ultrabuk-htmltopdf-java kept as-is (separate PR) - iText 5 dependency remains in pom.xml for Doc2PDF only No functional changes to PDF output. All PDF templates (150+ AcroForm PDFs), embedded Acrobat JavaScript (auto-print), and PDF manipulation operations are unaffected by the import migration. https://claude.ai/code/session_01TRZBbahEcwdwf4v31NoTDK * fix: migrate test files from iText to OpenPDF imports Migrate 2 test files missed in the initial bulk migration: - LabPDFCreatorTest.java: com.itextpdf.text.DocumentException → com.lowagie.text - FaxImporterCriticalGapsTest.java: all iText imports → OpenPDF equivalents, replace codec.Base64.encodeBytes() with java.util.Base64.getEncoder() https://claude.ai/code/session_01TRZBbahEcwdwf4v31NoTDK * Update src/main/java/io/github/carlos_emr/carlos/casemgmt/print/OscarChartPrinter.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update src/main/java/io/github/carlos_emr/carlos/encounter/oscarConsultationRequest/pageUtil/EctConsultationFormRequestPrintPdf.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update src/main/java/io/github/carlos_emr/carlos/fax/core/FaxImporter.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * chore: migrate TOTP library from aerogear-otp-java to java-totp (#533) * chore: migrate TOTP library from aerogear-otp-java to java-totp Replace the unmaintained org.jboss.aerogear:aerogear-otp-java:1.0.0 with the actively maintained dev.samstevens.totp:totp:1.7.1 library. Changes: - pom.xml: swap aerogear-otp-java dependency for dev.samstevens.totp - MfaManager.java: replace Base32.random() with DefaultSecretGenerator - Login2Action.java: replace Totp.verify() with DefaultCodeVerifier - Remove aerogear entries from dependency lock files Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> Co-authored-by: Claude <noreply@anthropic.com> * chore: fix minor regressions with checkstyle and filter order (#534) Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: remove HibernateDaoSupport dependency from 26 DAOs for Hibernate 6 prep (#506) * chore: remove HibernateDaoSupport dependency from 26 DAOs for Hibernate 6 prep Replace Spring's HibernateDaoSupport base class with a minimal AbstractHibernateDao that injects SessionFactory directly, removing the primary Hibernate 5→6 migration blocker for 26 DAO files. Changes: - Create AbstractHibernateDao base class with @Autowired SessionFactory - Replace .list() with .getResultList() in HqlQueryHelper (5 occurrences) - Migrate 26 DAOs from HibernateDaoSupport to AbstractHibernateDao - Replace getHibernateTemplate() CRUD calls with currentSession() equivalents - Replace Criteria/Example API with HQL in SecuserroleDaoImpl, ClientReferralDAOImpl - Replace createSQLQuery with createNativeQuery in AbstractQueryHandler - Fix SQL injection in SecroleDaoImpl (string concat → parameterized query) - Update HibernateTestDao: org.hibernate.Query → org.hibernate.query.Query 7 complex DAOs deferred to follow-up PR: ProviderDaoImpl, CaseManagementNoteDAOImpl, SecProviderDaoImpl, LookupDaoImpl, FormsDAOImpl, ProgramClientStatusDAOImpl, DemographicDaoImpl. - AbstractQueryHandler: replace deprecated setResultTransformer/ AliasToEntityMapResultTransformer (removed in Hibernate 6) with setTupleTransformer lambda returning LinkedHashMap<String,Object> - ClientImageDAOImpl.saveClientImage: fix bug saving clientImage instead of existing when a record already exists (would lose field updates) - PopulationReportDaoImpl.HQL_GET_USAGES: entity name cannot be parameterized in HQL; hardcode Admission and fix ?2 to ?1 for date - PopulationReportDaoImpl.getPrevalence/getIncidence: replace ICD-10 code string concatenation with parameterized IN clause to prevent HQL injection - ProgramFunctionalUserDAOImpl.getFunctionalUserByUserType: select pfu.Id (entity PK) not pfu.ProgramId; caller compares result against function.getId() so returning programId made duplicate detection broken Co-authored-by: Michael Yingbull <yingbull@users.noreply.github.com> Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * chore: pdf dep updates Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * fix: detect TIFF by extension before Image.getInstance() in ImagePDFCreator OpenPDF 3.0.x removed the built-in TiffImage codec, so Image.getInstance() throws for TIFF files. The previous code called Image.getInstance() first and the TIFF handler branch was unreachable for actual TIFFs. Fix: check file extension to detect TIFFs before attempting Image.getInstance(), routing TIFF files directly to the ImageIO/TwelveMonkeys handler. Also: - Use a generic error message (no raw path) in the missing-reader exception - Wrap reader.dispose() in a finally block to prevent resource leak Co-authored-by: Michael Yingbull <yingbull@users.noreply.github.com> * fix: validate imagePath with PathValidationUtils in ImagePDFCreator Validate imagePath against DOCUMENT_DIR using PathValidationUtils.validateExistingPath() before any filesystem access in printPdf(). Centralizes validation once before the TIFF and non-TIFF branches, preventing path traversal attacks. Also removes raw path from the non-TIFF error log message to avoid exposing potentially sensitive file paths. Co-authored-by: Michael Yingbull <yingbull@users.noreply.github.com> * fix: resolve review bot findings for OpenPDF migration PR Addresses items #1-9 and #11-13 from the code review summary: #1 PdfWriterFactory: replace multiple setPageEvent() calls with PdfPageEventForwarder to prevent stampers from silently overwriting each other (confidentiality/promo text was never stamping) #2 PdfWriterFactory: throw IllegalStateException instead of returning null on DocumentException to fail fast and prevent NPEs in callers #3 ImagePDFCreator: wrap all TIFF/non-TIFF processing in try/finally to guarantee document.close() runs even when exceptions are thrown #4 FHIRCommunicationRequestHandler: use try-with-resources for PdfReader to prevent resource leak if getNumberOfPages() throws #5 FrmPDFServlet: guard Integer.parseInt(cfgVal[5]) with try/catch NumberFormatException, falling back to font size 12 with a log warning #6 pom.xml: declare openpdf-html 3.0.2 as an explicit direct dependency (LabPDFCreator directly imports HTMLWorker from this module) #7 FaxImporter: switch Base64.getDecoder() to getMimeDecoder() to handle MIME-formatted (line-wrapped) Base64 payloads from fax providers; add explicit null check for missing document payload #8 LabPDFCreator: remove unused MessageHandler handler parameter from private getTextColor() method and update its call site #9 ReplacedElementFactoryImpl: update outdated comment that still referenced "old lowagie Image class" — now uses OpenPDF 3.x #11 ImagePDFCreator: add null check for ImageInputStream returned by ImageIO.createImageInputStream() before passing to getImageReaders() #12 docs/pdf-dependency-consolidation-plan.md: add blank lines around Group E and Group F tables to satisfy markdownlint MD058 #13 FHIRCommunicationRequestHandler: remove raw fileName from error log to avoid leaking file path information Co-authored-by: Michael Yingbull <yingbull@users.noreply.github.com> * chore: dep cleanup Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: migrate from itext to resolve license compliance issue Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: docs Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: docs Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: tests Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: small fixes re: tests Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * Update src/main/java/io/github/carlos_emr/carlos/documentManager/LocalOnlyUserAgent.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> * chore: fixes Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> --------- Signed-off-by: Michael Yingbull <michael@maplecreekmedical.ca> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Michael Yingbull <yingbull@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

- MoveMOHFiles2Action: catch IllegalArgumentException alongside SecurityException so malformed URL-encoded filenames (e.g. %GG) don't bubble up as 500s (#1) - EFormExportZip: add ze.isDirectory() guard to skip directory zip entries (#2) - PrintDemoChartLabel2Action: null-guard System.getProperty("user.home") before passing to new File() to avoid NPE bypassing the classpath fallback (#3) - PathValidationUtilsTest: assert createNewFile() return value is true so test setup failures are caught explicitly (#4) - docs/security-scanning-notes.md: fix broken "Open" cross-reference (#5) - EFormExportZip: wrap both FileOutputStream write paths in try-with-resources so streams are closed on exception (#6) - EFormExportZip.deleteDirectory(): null-guard listFiles() to avoid NPE (#7) - EDTFolder.getFolder(): return null instead of silently defaulting to INBOX for unrecognised folder names; update MoveMOHFiles2Action.getFolderPath() and viewMOHFiles.jsp callers to handle null with explicit error responses (#8) Co-authored-by: Michael Yingbull <yingbull@users.noreply.github.com>

@return

- EFormExportZip: wrap both FileInputStream instances in try-with-resources so streams close on IOException from inputToOutput() (#1) - EFormExportZip: add `continue` when imageFile.exists() so the "skipping image" message is accurate and existing images are not silently overwritten (#2) Also move fis open into the try-with-resources below the existence check - EFormExportZip: use StandardCharsets.UTF_8 in new String(bytes) for explicit, platform-independent eForm HTML encoding (code review note) - EFormExportZip: fix pre-existing double-space in "skipping image" message - MoveMOHFiles2Action.getFolderPath(): document null return in @return (#3) - EDTFolder.getFolder(): add full JavaDoc documenting null return (#4) Agent-Logs-Url: https://github.com/carlos-emr/carlos/sessions/33040b1f-6da0-444b-9393-23b3aa5f3e38 Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>

@return

…s, Allow header Addresses the verified findings from the third-pass full-scope PR review. Audit logs (#3): - Add WARN logs before the SecurityException throws in the three view-gate actions (MsgPreviewPDF2Action, MsgTransferSelectItems2Action, MsgViewCreateMessage2Action) so denied _msg read attempts carry provider context the same way the mutation actions already do. JavaDoc rot (#7): - MsgViewMessage2Action execute(): @return said NONE-via-redirect; happy path now returns SUCCESS (forward). Document both returns + @throws SecurityException. - MsgAttachPDF2Action execute(): add the new NONE (405) path and @throws SecurityException to the Javadoc. - MsgMessengerAdmin2Action execute(): add NONE (405), @throws java.io.IOException (new 405 path), and @throws SecurityException. Struts config guard (#2): - New StrutsMessengerConfigTest parses struts-messenger.xml and asserts no privilege-sensitive <result> routes to /messenger/*.jsp outside /WEB-INF/jsp/. The three intentional utility JSPs (attachmentFrameset, selfCloseAndRefreshOpener, Transfer/error) are explicitly allowlisted per the PR's scope. Also asserts every reference to the relocated gated JSPs lives under /WEB-INF/jsp/. Admin happy paths (#4): - shouldPopulateFetchAttributes_onAdminReadHappyPath: _admin read + default method returns SUCCESS and sets the groups/localContacts request attrs the admin JSP depends on. - shouldInvokeAddMember_onValidAddPost: method=add POST calls messengerGroupManager.addMember with the parsed group id. - shouldInvokeAddGroup_onValidCreatePost: method=create POST calls addGroup with name + parentId. - shouldInvokeRemoveGroup_onValidRemoveGroupPost: method=remove POST without a member param deletes the named group. Uniform Allow: POST header (#9): - Set Allow: POST on the 405 response of MsgAdjustAttachments2Action, MsgTransferPostItems2Action, and MsgMessengerAdmin2Action so all four mutation actions match MsgAttachPDF2Action's RFC-compliant behavior. Extend the existing 405 tests in each suite to assert the header. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@service

Items 1-9 from the PR review-comment triage (must/should/could fix list). Verified each one against the current code first; some were already fixed or invalid; the rest are addressed below. Items already fixed in earlier commits or invalid based on actual code (no change needed): #4 PaymentType actions XSS — already mitigated with nosemgrep marker + Content-Type: application/json + Jackson serialization. #8 BillingCodeSearchViewModelAssembler %% wildcard — Copilot diagnosis was wrong; q.desc is built from `name`, not from `q.code`. #9 GstReportViewModelAssembler null check — already in place at lines 91-93. Items fixed in this commit: #1 escapeXml="false" in three JSPs (billingONfavourite.jsp, billingONEditPrivateCode.jsp, billingOHIPsimulation.jsp). Replaced `<c:out value="${X}" escapeXml="false"/>` with raw EL `${X}` and added a JSP comment block + ViewModel field Javadoc documenting the trust contract: the assembler is the only producer and every user value is wrapped in SafeEncode.forHtml() before concatenation. Identical rendering, but breaks the Semgrep jsp-el-xss `<c:out + escapeXml=false>` pattern match. The structural safety invariant lives on the ViewModel field's Javadoc. #2 CodeQL "uncontrolled data used in path expression" in MoveMOHFiles2Action. Refactored getFile(folderPath, fileName) to use PathValidationUtils.validatePath(fileName, new File(folderPath)) — sanitize-and-validate-within-dir in one step at File-construction time, instead of building File from raw user input and validating after the fact. ScheduleOfBenefitsUpload2Action already calls PathValidationUtils.validateUpload(importFile) and reassigns; that chain is correct, the CodeQL finding was on stale line numbers. #3 Semgrep tainted-session in MoveMOHFiles2Action:247 (the `req.getSession().setAttribute("backupfilepath", folderPath)` line). Annotated with `// nosemgrep:` covering all three semgrep rules that flagged it, plus a justifying comment that folderPath comes from the EDTFolder enum lookup (closed set of property-driven server-side paths), not raw user input. #5 BatchBillingViewModelAssembler not Spring-managed. Added @service annotation; replaced the no-arg ctor's SpringUtils.getBean lookups with mandatory constructor parameters. BatchBill2Action now constructor-injects it instead of `new BatchBillingViewModelAssembler()`. BatchBill2ActionUnitTest updated to pass the assembler mock through the new ctor. #6 BillingCodeLookup misnamed Hashtable methods. Renamed fillCodeDataHashtable → toCodeDataMap (the method returns HashMap<String,String>, not Hashtable). Three call sites inside the same file updated. #7 BillingEDTOBECOutputSpecificationBeanHandler concrete-collection getter. Changed return type from ArrayList<...> to List<...> so callers don't depend on the implementation type. BillingDocumentErrorReportUpload2Action's local variable type adjusted to match. Test failure unblocked along the way: BillingOnJspRoutingTest.shouldUseContextAwareRoutes was asserting on the legacy scriptlet `<%= inrBillingAction %>` shape in inr/reportINR.jsp, but that JSP was already migrated to `${carlos:forHtmlAttribute(reportInrModel.inrBillingActionUrl)}`. Updated the test assertion to match the new ViewModel-based shape. Verified end-to-end: - 471/471 tests in billings.ca.** + billing.CA.ON.** - Spring context boots clean (BatchBill2Action via the new ctor; BatchBillingViewModelAssembler discovered as @service) - Playwright runtime smoke on /billing/CA/ON/ViewBillingONEditPrivateCode (#1 + #6 paths) /billing/CA/ON/ViewBillingONFavourite (#1 path) /billing/CA/ON/BatchBill (#5 path) /billing?billRegion=ON&demographic_no=7 (regression check) — all render with no ERROR lines in catalina.out. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Six must-fix and four should-fix items from the open-comment triage. Each fix is independent and small; verified the full chain compiles and the billing test suite stays green (471/471). Runtime smoke via Playwright: calendar popup now defaults to "today" instead of year-1-BC December; the rest of the affected pages render with no ERROR lines in catalina. out. MUST FIX: #1 OntarioRASettlementService — precompile Q_CODE_PATTERN as a static final java.util.regex.Pattern (was String.matches recompiling per row); switch noErrorBills from ArrayList<String> to LinkedHashSet<String> so the I2/35-with-Q-codes overlap branch doesn't produce duplicate updateBillingStatus(account, "S") writes. Insertion order preserved for any caller that snapshots the audit-log. #2 BillingONFormDemographicLoader — replaced the non-existent Bootstrap class 'alert alert error' with 'alert alert-danger' on the two warning/error <div>s, and rewrote the unclear "referral doctor's no" → "referral doctor's number" copy. Without the fix the warning banner was rendering with no styling at all (no red background) — silent UX defect. #3 BillingCalendarPopupViewModelAssembler — when year/month are missing/unparseable, fall back to today's calendar instead of constructing GregorianCalendar(0, -1, 1) which normalised to year 1 BC December. Test fixture updated ( shouldPreserveLegacyZeroDate → shouldFallBackToToday). #4 BatchBill2Action — distinct i18n key for malformed-row vs the original bad-date case. Added 'billing.batchbilling.badRow' to oscarResources_en.properties; the malformed-row branch now uses it instead of reusing 'billing.batchbilling.badDate'. Bad-date branch unchanged. #5 BillingBCSetup2Action:113 — extended the existing nosemgrep comma list with 'java.lang.security.audit.tainted-session-from- http-request' so all three Semgrep rules on the same line are suppressed (the audit-flavoured one was missing, kept reappearing on every push). #6 ViewOngenreport2Action.java:70 + ViewOnregenreport2Action.java:71 — added response.setHeader("Allow", "POST") before the 405 sendError, matching RFC 7231 §6.5.5 and the convention every other 2Action in this PR follows. SHOULD FIX: #7 ViewBillingON2Action — null-session path now throws SecurityException ("missing session") instead of "missing required sec object (_billing)". The privilege-denied path keeps the original message. Test fixture updated to match. #8 ManageBillingformBilltypeViewModelAssembler — dropped unused LoggedInInfo parameter from assemble(). One caller (ManageBillingformBilltype2Action:67) updated; LoggedInInfo import on the assembler also removed. #9 BillingReportFragmentViewModel — dropped the redundant List.copyOf in the constructor; the builder setters already produce immutable copies. Saves an allocation + iteration per row collection on every report (real cost when the unbilled/billed lists are long). #10 BillingCorrectionRecordService — deleted dead 3-line elemToDel block at line 317-320 (declared, two values added, never read). Verified: 471/471 tests green; Playwright runtime smoke on /billing?billRegion=ON&demographic_no=7, /billing/CA/ON/ViewBillingCalendarPopup (now defaults to current month), /billing/CA/ON/BatchBill — all clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@SuppressWarnings

All 6 are could-fix tier from the open-comment triage — Javadoc / class naming / extra test coverage. No behaviour change in production code beyond the boundary-aware test additions. #1 ErrorPageLoggerUnitTest class Javadoc — rewrote to describe what the tests actually do (log-capture assertions on count, level, message content, throwable identity), instead of the legacy "verify only that the helper runs without throwing" claim. #2 LogSanitizerUnitTest split @link — rejoined the {@link io.github.carlos_emr.carlos.billings.ca.on.validator .BillingValidationException} reference onto a single line so Javadoc tooling can resolve it. #3 ErrorPageLogger ;jsessionid coverage — added two tests: shouldStripJsessionidPathParam_beforeLogging (matrix-param stripping in isolation) and shouldStripBothJsessionidAndQueryString_whenBothPresent (combined ;jsessionid+? case). The other three Copilot bullets (query-string strip, null-request no-op, non-Throwable attr) were already covered by existing tests. #4 ViewBillingShortcutPg12Action → ViewBillingShortcutPg1View2Action — class renamed (the legacy "Pg12" reads like "page 1+2"; new name follows the View*View2Action convention used elsewhere). Updated references in 5 files: the class file, its unit test, the ViewModel constant, the BillingShortcutPg1ViewModelAssemblerUnitTest, and struts-billing.xml mapping. #5 BillingReportControlViewModelAssembler unused rp — removed the @SuppressWarnings("unused") + dead local cast at line 96. The ReportProvider import also became unused and was removed. #6 RateLimitFilterTest boundary cases — added two tests: shouldMatchLoginRate_whenJsessionidMatrixParamPresent (locks in the ; boundary handling so attackers can't bypass /login limits by appending ;jsessionid=…) and shouldNotMatchLoginRate_when PathIsLoginfailed (locks in the prefix-boundary so /loginfailed doesn't accidentally fall under the /login tier). Test placeholder hygiene: previous draft used "DEADBEEF" as a mock session ID — replaced with TEST_SESSION_ID_… across the new tests for clarity and to avoid surprising future readers / log-content scanners. Verified: 543/543 across billings.ca.**, billing.CA.ON.**, ErrorPageLoggerUnitTest, LogSanitizerUnitTest, RateLimitFilterTest. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Applies the nine verified multi-agent code-review findings on the Selenium headless-Chromium eForm PDF renderer. All changes are non-breaking. MUST - #1 Sessionless render of image/background-bearing eForms. Removing session forwarding left EFormImageViewForPdfGenerationServlet returning 401 for the render browser (no session), so forms with ${oscar_image_path}/displayImage backgrounds failed the render. The render token is now render-scoped (peek, not consume-once): one grant authorizes the eForm document plus every loopback asset-image subresource. buildPdfHtml threads the grant onto image URLs; the image servlet accepts a live grant as an alternative to a session on the loopback path (shared template assets only). The renderer still invalidates the token when the render finishes; the 2-min TTL is the backstop. SHOULD - #2 Block WebRTC egress (--disable-features=WebRtc, --force-webrtc-ip-handling-policy=disable_non_proxied_udp, --disable-background-networking) so ICE/STUN/TURN UDP cannot bypass the HTTP dead proxy. - #3 Stop an externally-owned ChromeDriverService if the ChromeDriver ctor throws (sandbox-start failure), preventing an orphaned chromedriver process. COULD - #4 Latch the main-document HTTP status as soon as it is drained so a performance-log flood cannot evict it into a spurious status=null gate failure. - #5 Broaden redactUrls to also strip bare filesystem-style paths from logs. - #6 Wrap-safe deadline comparison and numeric (not lexical) capture-page sort. Inherited from #3164 - #8 EFormSignatureViewForPdfGenerationServlet (digital signatures = PHI) now requires a live render grant on the loopback path, closing the previous always-open by-id enumeration surface. Its only live consumer is the render, which now carries the grant; in-render script remains contained by the egress lockdown, not the grant. - #7 (display/save HTML rewrites) and #9 (CoverPage _edoc preview requirement) are documented as tracked follow-ups: changing #7 would risk breaking rendering, and #9 is an operator role-configuration note. Tests: render-token peek repeatability + invalidation; image servlet grant path (200 with no privilege check) and invalid-grant rejection (401); signature servlet grant gate (serve with grant, 401 without, 403 non-loopback); asset-URL grant threading for both ${oscar_image_path} and /eform/displayImage forms. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>

penify-dev Bot added 2 commits February 2, 2026 21:19

[Penify]: Documentation for commit - c90f5bb

fb3bbe9

[Penify]: Documentation for commit - c90f5bb

b16f362

github-actions Bot mentioned this pull request Feb 4, 2026

Cleared up Caseload to better format #93

Closed

yingbull mentioned this pull request Feb 4, 2026

Review: Add concurrent booking conflict detection for appointments #137

Closed

github-actions Bot mentioned this pull request Feb 6, 2026

Review: Ensure clinical note amendments maintain audit trail #207

Closed

yingbull approved these changes Feb 7, 2026

View reviewed changes

yingbull closed this Feb 7, 2026

claude Bot mentioned this pull request Feb 10, 2026

feat: port server-side tickler pagination from upstream PR #2268 #337

Closed

This was referenced Feb 12, 2026

Migrate and update built-in fax handling services based on initial JunoEMR code, while keeping fax-ws server support. #345

Closed

Redesign provider preferences page with Bootstrap 5 #352

Closed

309 feature oscar 19 messenger #354

Merged

This was referenced Feb 13, 2026

chore: enable the new inboxhub by default and fix startup issue re hrm #353

Merged

Update setLabMacroPrefs.jsp #362

Merged

Integrated fax handler with SRFax support #370

Closed

feat: Update preferences to modern UI with bootstrap and remove popups #376

Closed

This was referenced Feb 25, 2026

469 feature appointment alert #470

Merged

Fix/specialist #429

Merged

This was referenced Apr 12, 2026

sec: suppress 205 CWE-501 trust-boundary Semgrep FPs across 36 Action files #1596

Merged

Fix i18n bundle loading, fill empty keys, and correct JS references #1588

Closed

This was referenced Apr 13, 2026

Add regression tests for RptDownloadCSVServlet empty-filter WHERE clause paths #1660

Closed

sec: gate 40 admin JSPs behind WEB-INF/jsp via Struts2 2Actions #1665

Merged

Dead prescription-module URLs: MyDrugRef remnants + missing JSPs/actions #1677

Closed

cubic-dev-ai Bot mentioned this pull request Apr 14, 2026

sec: gate all BC billing JSPs behind WEB-INF/jsp via Struts2 2Actions (mega) #1632

Merged

7 tasks

This was referenced Apr 17, 2026

Clean up dead prescription-module URLs and remove stale MyDrugRef callers #1810

Merged

Add regression coverage for RptDownloadCSVServlet empty-filter WHERE assembly #1902

Merged

Fix XML upload entrypoint after forms JSP migration #1915

Merged

yingbull mentioned this pull request Jun 24, 2026

[codex] Fix consultation request signatures #3019

Merged

Ben-Heerema mentioned this pull request Jul 2, 2026

Investigate provider-to-patient email functionality in CARLOS #3096

Closed

Copilot AI mentioned this pull request Aug 4, 2026

Merge main into develop to resolve develop→main merge conflicts #3312

Merged

yingbull mentioned this pull request Sep 20, 2026

fix(semgrep): correct CARLOS sanitizer models and run every rule in CI #3801

Merged

8 tasks

Navigation Menu

[Penify]: Documentation for commit - c90f5bb - #4

[Penify]: Documentation for commit - c90f5bb#4
penify-dev[bot] wants to merge 2 commits into
developfrom
penify/auto_doc_c90f5bb_32666

penify-dev Bot commented Feb 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

penify-dev Bot commented Feb 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant