Security upgrades, HTTP static resource fix, and version history enhancements#151
Open
Innovarzweng wants to merge 26 commits into
Open
Security upgrades, HTTP static resource fix, and version history enhancements#151Innovarzweng wants to merge 26 commits into
Innovarzweng wants to merge 26 commits into
Conversation
Introduce SshTransportConfig and HttpsTransportConfig as dedicated TransportConfigCallback implementations, replacing the inline JSch factory in GitRepositoryService. Users can now authenticate via username + PAT (inline or from a credentials file) in addition to SSH. - GitSettings: add AUTH_TYPE_SSH/HTTPS constants, fix isSSH() null check - GitRepositoryService: replace SshSessionFactory field with TransportConfigCallback; rename validateSSHConnection → validateRemoteConnection with private overload to avoid double-build - GitOperations: wire single transportConfig through all JGit commands - GitSettingsTabPanel: add auth type switcher (SSH/HTTPS), HTTPS inline and file-path credential panels, JPasswordField for PAT masking - plugin.xml: bump version to 3.0.1, add Mirth 26.3.1 compatibility
- Add 3 header actions: Reload (fetch + ahead/behind count), Pull (normal merge, conflicts resolved using remote), Push (fetch + rebase + push) - Add RemoteStatus DTO and /remoteStatus, /pull, /push endpoints - Replace pullWithOverwrite with pullNormal for Pull button; auto-resolves conflicts by taking remote version, preserves local unpushed commits - Fix UP_TO_DATE treated as push failure in GitOperations.push() - Fix git repo not re-initialized when Remote URL or Branch changes in settings — now deletes local repo and re-clones to avoid unrelated histories or wrong-branch corruption - Reformat VersionHistoryServletInterface to consistent multiline style
to the getStatistics/getStatisticsPost pipeline. When true, deployed channel statistics are read from persistent storage instead of in-memory counters.
- Add v26_3_1 enum entry to Version.java - Add Migrate26_3_1 no-op migrator and register in ServerMigrator - Update version in mirth-build.properties, mirth.properties (conf + setup) - Update mirthVersion in all connector and plugin xml files (server/src, server/setup, custom-extensions)
Only log when running as root/Administrator (warn). Skip the info-level message for the passing case — it adds noise without actionable signal.
Only warn when no_new_privs is NOT set; skip the info log when it is.
…sages (IRT-753) In Jetty 9, ContextHandler.doScope() skipped dispatch if baseRequest.isHandled() was true, so RequestHandler was never called after StaticResourceHandler served a response. The Jetty 12 EE8 ContextHandler does not provide this guard — HandlerCollection calls all handlers unconditionally, causing RequestHandler to create a spurious channel message on every static resource hit. Adding the isHandled() check at the top of RequestHandler.handle() restores the original behavior.
…ture/security-upgrade
…ingDialog message - Add KeystorePasswordRegenerationTest proving that keystore password regeneration preserves the AES data-encryption key. Both encryptData channel messages and encrypt.properties database passwords remain fully decryptable after re-keying. - Correct KeystoreWarningDialog warning text: previous message incorrectly stated that encryptData messages and encrypted DB passwords would become unreadable after regeneration. They are not affected -- only the SSL/TLS certificate changes. - Fix KeyEncryptorTest: replace SunJCE class import with string literal to avoid Java module system access error.
…ture/security-upgrade
…og, restore encryption note - Indent regenerate checkbox to align with the message text column (icon width + gap) - Restore note clarifying encryptData messages and encrypt.properties passwords are NOT affected by keystore regeneration
…aultConfigurationControllerTests Covers ALREADY_SECURE (non-default and null passwords), REGENERATED for both default password sets (Set A and Set B) including usingDefaultKeystorePassword flag check via reflection, and FileNotFoundException propagation on missing keystore file. saveMirthConfig() enabled by stubbing getConfigurationDir() in @BeforeClass.
Jetty 12 commits the response before the full body is written when no Content-Length is known, causing ERR_HTTP2_PROTOCOL_ERROR in browsers for large resources (>~30 kB). For FILE, DIRECTORY, and CUSTOM resource types, Content-Length / Content-Length-Long is now set before the first write. Skipped when GZIP is active since compressed size is unknown. Also wraps the error-path reset() call in a try/catch for IllegalStateException so a mid-stream write error that already committed the response does not mask the original exception. Adds 22 unit tests (HttpReceiverStaticResourceTest) covering all three resource types, GZIP paths, error paths, and the committed-response guard. Adds 8 integration tests (HttpReceiverStaticResourceIntegrationTest) that spin up a real Jetty 12 server on loopback and assert Content-Length is present on the wire for 100 KB CUSTOM and FILE resources, and that GZIP responses decompress correctly end-to-end.
…HandlerCollection
Replace the broken Jetty 12 handler chain in HttpReceiver.onStart() with the correct
ContextHandlerCollection + DefaultHandler + Handler.Sequence pattern, matching
MirthWebServer.java.
Two bugs compounded:
1. HandlerCollection has no path-based routing; inner ContextHandlers could not reject
requests for paths outside their configured contextPath.
2. A root ContextHandler("/") wrapped the entire tree, accepting every URL and
suppressing any 404 fallback (Jetty 12 has no implicit default 404 unlike Jetty 9).
Fix:
- Replace HandlerCollection + root ContextHandler with ContextHandlerCollection so
Jetty enforces strict prefix matching per context.
- Add explicit DefaultHandler (serveFavIcon=false, showContexts=false) so unmatched
paths return 404 instead of an HTTP 200 context-listing page.
- Move security handler inside each ContextHandler (per-context) instead of wrapping
the whole collection, preserving per-path auth scoping.
- Add setAllowNullPathInfo(true) to the main channel ContextHandler to prevent Jetty
from issuing a 301 redirect when the request path equals the context path exactly.
Update HttpReceiverStaticResourceIntegrationTest.startServer() to use the same
ContextHandlerCollection pattern and add a regression test confirming that requests
outside the resource context path return 404.
…dErrorResponse Jetty 12 requires Content-Length to be set before writing the response body or body bytes may be silently dropped. Adds setContentLength() in the non-gzip branch of sendResponse() and in sendErrorResponse(). Adds three unit tests to HttpReceiverTest covering non-gzip Content-Length, gzip (no Content-Length), and error response Content-Length assertions.
Add setContentLengthLong(file.length()) before the file copy when not gzip-encoding, matching the IRT-828 pattern from StaticResourceHandler. Move fis close to finally so it always runs. Wrap response.reset() in an inner try/catch(IllegalStateException) so a committed response does not mask the original error. Tests: non-gzip GET verifies Content-Length is set; gzip GET verifies it is not; error path verifies reset()+setStatus(500) are called and handle() completes normally. Also fix pre-existing wrong assertion in testContextPathNormalizationHandlesEmpty and replace commons-lang3 FQN calls with String.equalsIgnoreCase in MirthWebServerTest.
Extract the anonymous Swagger UI filter to SwaggerUiFilter (public static class) and add setContentLengthLong + try-with-resources stream close before each Files.copy call — same IRT-828 pattern applied to both the index.html branch and the generic static file branch. Without this fix Jetty 12 commits the response before the full body is written, causing ERR_HTTP2_PROTOCOL_ERROR in browsers for large assets such as lib/swagger-ui-bundle.js (~974 KB).
Buffer JNLP XML to byte[] before writing so Content-Length is known upfront, then write via getOutputStream() instead of getWriter(). Replace the unguarded null-document path in the else branch with sendError(404) + return, eliminating the NPE on non-matching URIs. Update WebStartServletTest: wire getOutputStream() to a ByteArrayOutputStream, track status and contentLength in TestHttpServletResponse, remove try-catch wrappers from three 404-path tests and assert status 404, and add testDoGetCoreSetsContentLength to verify the header is set.
…le not found StaticResourceHandler was calling baseRequest.setHandled(true) unconditionally, even when it returned early without writing a response (e.g. requested file absent under a DIRECTORY resource). This prevented the channel RequestHandler from processing the request, causing a 404 instead of falling through. Introduced a responded flag that is only set to true when content is actually written to the response. The setHandled call is now gated on that flag.
…ough CoreContextHandler.handle() (the Jetty 12 EE8-to-core bridge) returns true once the request path matches the context path, even when the inner EE8 handler never sets isHandled(). Separate ContextHandlers for DIRECTORY static resources therefore block fallthrough to the channel — Handler.Sequence and ContextHandlerCollection both fail for the same reason. Fix: DIRECTORY resources are no longer given their own ContextHandler. They are collected and embedded inside the channel ContextHandler ahead of RequestHandler, in a HandlerCollection subclass that overrides handle() to stop on the first handler that sets isHandled(). HandlerCollection (not an anonymous AbstractHandler) is used so Jetty lifecycle calls (setServer, start) propagate to all inner handlers via addHandler(), preserving correct behaviour when ConstraintSecurityHandler wraps the resource handlers for auth. CUSTOM and FILE resources are unaffected — they keep their own ContextHandler since exact-path matching means fallthrough is never needed. Adds GROUP D integration tests (startServerWithDirectoryAndChannel + ChannelEchoHandler) covering: missing file falls through, existing file served, subdirectory path falls through.
The previous commit only embedded DIRECTORY resources inside the channel ContextHandler. CUSTOM and FILE resources still had their own ContextHandler, which caused the same CoreContextHandler bridge problem: any sub-path that the StaticResourceHandler cannot serve (e.g. GET /test/data/wrong for a CUSTOM resource at /test/data) returned 404 instead of falling through to the channel. Remove the ResourceType.DIRECTORY condition. All static resource handlers are now collected into resourceHandlers and embedded in the channel context's stop-on-first-handled HandlerCollection, regardless of type. The handlers variable and its iteration loop are kept for structural consistency but are now only ever populated by the channel ContextHandler itself.
…urces Commit a200466 extended the channel-embedded handler architecture to all ResourceTypes, but the integration test suite only covered DIRECTORY fallthrough (GROUP D). Add GROUP E tests that prove CUSTOM and FILE sub-path misses also fall through to the channel handler, not to a 404.
…tatic assets Verify that SwaggerUiFilter sets Content-Length and Content-Type when serving index.html and static CSS, and passes through non-static API paths unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/healthcheck) no longer create spurious channel messages — Jetty 12 EE8ContextHandlerdoesn't guard onisHandled()the way Jetty 9 did; added explicit check at top ofRequestHandler.handle()(IRT-753)no_new_privsis active or when running as a normal user26.3.0tomirthVersioncompatibility listsGitStatusTabPanel; add HTTPS/PAT authentication support26.3.1; mergeupdate-version-historybranchTest plan
/healthcheck→text/plain"Success"; confirmcurl GETreturns "Success" and no message is created in the source channel26.3.0custom extensions load without compatibility errors