T2 (#810): Added registration-launcher (_launcher.jar) for manifest-driven upgrade orchestration#820
T2 (#810): Added registration-launcher (_launcher.jar) for manifest-driven upgrade orchestration#820GOKULRAJ136 wants to merge 3 commits into
Conversation
…est-driven upgrade orchestration Signed-off-by: GOKULRAJ136 <110164849+GOKULRAJ136@users.noreply.github.com>
|
Warning Review limit reached
More reviews will be available in 42 minutes and 15 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds a new ChangesRegistration launcher pipeline
Sequence Diagram(s)sequenceDiagram
participant Initialization
participant StartupEvaluator
participant JreMigrationStager
participant LibUpdater
participant MigrationCleaner
participant NormalStartup
Initialization->>StartupEvaluator: evaluate(rootManifest, rootSignature, libManifest, trustedKey, jreMajorVersion)
alt NORMAL_STARTUP
Initialization->>MigrationCleaner: cleanup(rootDir)
Initialization->>NormalStartup: launch(args)
else MIGRATE_JRE
Initialization->>JreMigrationStager: stage(root, verifiedRootManifest, ...)
else UPDATE_LIB
Initialization->>LibUpdater: update(...)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 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 `@registration/registration-launcher/pom.xml`:
- Around line 32-36: The registration-launcher module’s junit:junit test
dependency is still being resolved to a vulnerable 4.12 via the parent BOM.
Update the parent BOM’s junit.version to 4.13.1 or newer, or migrate any
TemporaryFolder-based tests to JUnit 5’s TemporaryDirectory extension. Use the
junit dependency in registration/registration-launcher and the parent BOM
property that controls its version to locate the change.
In
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ManifestVerifier.java`:
- Around line 71-80: The manifest verification in
ManifestVerifier.verifyManifest should reject any entry name that resolves
outside baseDir instead of blindly using new File(baseDir, name). Normalize the
candidate path against baseDir, verify it remains under the update root, and
treat any ../ or absolute entry as a mismatch before hashing. Update the
existing loop over manifest.getEntries() to use this containment check, and add
regression tests covering both relative traversal and absolute manifest entry
names.
In
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ResumableDownloader.java`:
- Around line 58-66: `ResumableDownloader.download` currently allows a
`readTimeout` of 0, which can make launcher downloads hang forever; tighten the
input validation so both `connectTimeout` and `readTimeout` must be positive and
throw `IllegalArgumentException` otherwise. Apply the same guard in
`LibUpdater.update` before it delegates to `ResumableDownloader.download`, so
invalid timeout values are rejected at the entry point instead of propagating
downstream. Use the existing `download` and `update` methods as the reference
points for the fix.
- Around line 287-296: The writeValidator method currently swallows IOException
when persisting or deleting the .part.meta validator, which can leave the
download state inconsistent. In ResumableDownloader.writeValidator, fail closed
by propagating the exception (or converting it to an unchecked failure) instead
of only logging, so callers can stop before body bytes are written. Keep the
behavior tied to the writeValidator path and its caller flow in the
download/resume logic so partial downloads are not committed with stale or
missing validator metadata.
In
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ZipExtractor.java`:
- Around line 40-63: The ZipExtractor.extract flow currently unpacks entries
before any integrity verification, so it needs hard safety limits during
extraction. Update ZipExtractor.extract to enforce caps on total uncompressed
bytes and maximum entry count while iterating ZipInputStream entries, and fail
fast if limits are exceeded; alternatively, integrate ManifestVerifier-style
validation into the extraction path so files are checked as they are written.
Use the existing ZipExtractor.extract, resolve, and ManifestVerifier flow as the
place to add these guardrails.
- Around line 44-52: The ZipExtractor extraction logic only checks the
normalized path in the entry handling flow, so it can still write through an
existing symlink under the target directory. Update ZipExtractor.extract to
reject symlinked targetDir ancestors or resolved parent paths before calling
Files.createDirectories or Files.newOutputStream, and make sure the check covers
both directory and file entries. Add a regression test near the existing
traversal-case coverage to verify a pre-existing symlink like sub -> elsewhere
cannot be used to extract sub/payload.jar outside the extraction root.
In
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/JreMigrationStager.java`:
- Around line 112-119: The JRE staging logic in JreMigrationStager currently
trusts jre21Temp.exists() as proof that staging is complete, which can skip a
needed re-extraction after an interrupted or tampered run. Update the staging
flow around the jre21Temp and ZipExtractor.extract path to either always rebuild
a clean staging directory or verify a completion marker/checksum tied to the
verified jre21.zip before treating the temp JRE as valid. Ensure the check is
based on actual staged integrity, not just directory presence.
- Around line 121-123: The staging flow in JreMigrationStager.stage() is too
permissive because copyIfMissing() leaves any existing root
migration.exe/rollback.exe untouched and only warns when the verified artifact
is missing, so the caller may launch a stale or incomplete executable. Update
the logic around copyIfMissing() and the migration/rollback copy steps to always
replace the root executables with the verified artifacts from the artifacts
directory, and fail staging if the verified source is unavailable or the copy
cannot be completed. Apply the same replacement behavior in the rollback-related
staging path referenced by the same helper so both executables are guaranteed
current before launch.
In
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/LauncherConfig.java`:
- Around line 49-60: The upgrade URL validation in LauncherConfig currently
relies on a string prefix check, which can be bypassed by crafted templates and
redirect downloads to an unexpected host. Update the logic around the rendered
template in LauncherConfig to parse the final value as a URI, reject any
user-info component, and verify the parsed scheme, host, and port still match
the parsed upgradeServer before allowing it. Keep the existing URL construction
flow with REG_CLIENT_URL and the rendered base value, but replace the
startsWith("https://") guard with URI-based validation and a clear
IllegalArgumentException on mismatch.
In
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/LibUpdater.java`:
- Around line 68-93: The lib update flow in LibUpdater reuses tempDir but only
ensures it exists, so stale extracted jars from a prior failed attempt can
remain and trip findUnexpectedFiles(...) during the next update. Before calling
ZipExtractor.extract(...) in the update path, clear any previously staged
non-control contents from tempDir while preserving the manifest/signature files
needed for verification, so a fresh lib.zip extraction starts from a clean
staging area.
In
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/MigrationCleaner.java`:
- Around line 62-73: The recursive cleanup in deleteRecursively currently
follows directory contents returned by File.listFiles(), which can traverse
symlinks and delete files outside the intended base directory. Update
MigrationCleaner.deleteRecursively to avoid descending into symlinks by
switching to a Files.walkFileTree approach without FOLLOW_LINKS, or by
explicitly rejecting symlink entries before recursion, and keep the existing
delete/warn behavior for regular files and directories. Add a regression test
around the cleanup path that includes a symlink inside jre21_temp or .artifacts
and verifies the target outside baseDir is not removed.
In
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/StartupEvaluator.java`:
- Around line 70-86: The startup decision in StartupEvaluator currently aborts
only when the lib manifest has no version, but a signature-verified root
manifest with no Manifest-Version still falls through to migration logic. Update
the version checks around ManifestVerifier.getVersion(...) so a missing/blank
rootVersion is treated as corruption too, and return a hard abort Evaluation
instead of calling migrationAction(...). Keep the existing normal-startup and
version-mismatch paths intact.
In
`@registration/registration-launcher/src/test/java/io/mosip/registration/launcher/JreMigrationStagerTest.java`:
- Around line 50-72: The happy-path fixture in JreMigrationStagerTest only
prepares jre21.zip, so it does not verify that the migration executables are
staged. Update baseSetup and the verified-happy-path test to include signed
root-manifest entries for migration.exe and rollback.exe, and make the
assertions check that both files are copied into the app root. Use the existing
helpers such as manifestBytes, sign, and stage_verifiedHappyPath to locate and
extend the setup and assertions.
In
`@registration/registration-launcher/src/test/java/io/mosip/registration/launcher/JreVersionDetectorTest.java`:
- Line 6: Update the JreVersionDetectorTest so it verifies currentMajorVersion()
matches the exact parsed value from
majorVersion(System.getProperty("java.version")) rather than only checking it is
>= 11. Use the JreVersionDetector.currentMajorVersion and
JreVersionDetector.majorVersion symbols to locate the test, and make the
assertion compare the delegated result directly so any change in delegation
behavior is caught.
In
`@registration/registration-launcher/src/test/java/io/mosip/registration/launcher/LauncherDialogsTest.java`:
- Around line 17-22: The LauncherDialogsTest class mutates JVM-wide state in
forceHeadless() by setting java.awt.headless and never restoring it. Update the
test setup in forceHeadless() and add matching cleanup in an `@AfterClass` method
to save and restore the previous system property value so later tests in the
same JVM are not affected.
In
`@registration/registration-launcher/src/test/java/io/mosip/registration/launcher/NormalStartupTest.java`:
- Around line 17-23: The `NormalStartup.launch()` test is mutating JVM-wide
system properties and not restoring them afterward, which can leak state into
other tests. Update
`launch_withoutClientOnClasspath_throwsReflectiveOperationException()` to
capture the original values of `java.net.useSystemProxies` and
`logback.configurationFile` before calling `NormalStartup.launch()`, and restore
them in a `finally` block even when the reflective exception is thrown. Use the
`NormalStartup.launch` and
`launch_withoutClientOnClasspath_throwsReflectiveOperationException` symbols to
place the cleanup alongside the existing assertions.
🪄 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: 262ff91d-33c4-4e3e-a78b-1f665dde9250
⛔ Files ignored due to path filters (2)
registration/registration-launcher/src/main/resources/provider.pemis excluded by!**/*.pemregistration/registration-launcher/src/test/resources/provider.pemis excluded by!**/*.pem
📒 Files selected for processing (32)
registration/pom.xmlregistration/registration-launcher/pom.xmlregistration/registration-launcher/src/main/java/io/mosip/registration/controller/Initialization.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/JreMigrationStager.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/JreVersionDetector.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/LauncherConfig.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/LauncherDialogs.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/LibUpdateResult.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/LibUpdater.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/MigrationArtifacts.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/MigrationCleaner.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/NormalStartup.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/StartupAction.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/StartupEvaluator.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/HashUtil.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ManifestVerifier.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ResumableDownloader.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/SignatureVerifier.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ZipExtractor.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/JreMigrationStagerTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/JreVersionDetectorTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/LauncherConfigTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/LauncherDialogsTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/LibUpdaterTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/MigrationCleanerTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/NormalStartupTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/StartupEvaluatorTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/common/HashUtilTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/common/ManifestVerifierTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/common/ResumableDownloaderTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/common/SignatureVerifierTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/common/ZipExtractorTest.java
Signed-off-by: GOKULRAJ136 <110164849+GOKULRAJ136@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
registration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ManifestVerifier.java (1)
143-149: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject symlinked artifacts in
fileMatches.
JreMigrationStager.verifyArtifactsAgainstRootManifest(...)uses this helper as the last integrity gate before those.artifacts/*paths are later unzipped or copied.file.exists()follows symlinks, so a pre-existingjre21.zip -> /somewhere-else/jre21.zipis hashed outside the artifacts directory and can then be swapped after verification but before use. Require a no-follow regular-file check before hashing.Suggested fix
+import java.nio.file.LinkOption; ... public static boolean fileMatches(Manifest manifest, String entryName, File file) throws IOException { Attributes attrs = manifest.getAttributes(entryName); if (attrs == null) { return false; } String expectedHash = attrs.getValue(Attributes.Name.CONTENT_TYPE); - return expectedHash != null && file.exists() && expectedHash.equals(HashUtil.sha256Hex(file)); + return expectedHash != null + && Files.isRegularFile(file.toPath(), LinkOption.NOFOLLOW_LINKS) + && expectedHash.equals(HashUtil.sha256Hex(file)); }🤖 Prompt for 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. In `@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ManifestVerifier.java` around lines 143 - 149, Reject symlinked artifacts in ManifestVerifier.fileMatches by verifying the target is a regular file without following links before computing the SHA-256. Replace the current file.exists() check with a no-follow file type check on the File path, and only call HashUtil.sha256Hex(file) when the path is confirmed to be a regular file. Keep the existing manifest attribute lookup and expected hash comparison intact, and ensure JreMigrationStager.verifyArtifactsAgainstRootManifest still relies on this stricter integrity gate.
♻️ Duplicate comments (1)
registration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ZipExtractor.java (1)
37-51: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAlso reject symlinks in the extraction-root path.
This closes
sub -> /elsewhere, but not a symlinkedtargetDiritself (or an existing symlink in its path). In that casecreateDirectories(...)and later writes still go outside the intended root, andLibUpdater.update(...)reaches this beforeManifestVerifierruns. Please fail fast on symlinks in the extraction-root path and add a regression next toextract_throughPreexistingSymlinkDir_isBlocked().Suggested fix
+import java.nio.file.LinkOption; ... public static void extract(File zipFile, File targetDir) throws IOException { - Files.createDirectories(targetDir.toPath()); - Path targetRoot = targetDir.toPath().toAbsolutePath().normalize(); + Path targetRoot = targetDir.toPath().toAbsolutePath().normalize(); + rejectSymlinkPath(targetRoot, targetDir.getPath()); + Files.createDirectories(targetRoot); ... + private static void rejectSymlinkPath(Path path, String name) throws IOException { + for (Path cursor = path; cursor != null; cursor = cursor.getParent()) { + if (Files.exists(cursor, LinkOption.NOFOLLOW_LINKS) && Files.isSymbolicLink(cursor)) { + throw new IOException("Blocked zip extraction through symbolic link: " + name); + } + } + }Also applies to: 76-82
🤖 Prompt for 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. In `@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ZipExtractor.java` around lines 37 - 51, The extraction logic in ZipExtractor still allows a symlinked extraction root or symlinked parent path, so writes can escape the intended directory before ManifestVerifier runs. Update the ZipExtractor flow around targetDir, targetRoot, and rejectSymlinkAncestor to fail fast if the extraction-root path itself contains any symlink before createDirectories or opening the ZipInputStream. Make the check cover the full root path, not just entries under it, and add a regression test alongside extract_throughPreexistingSymlinkDir_isBlocked() to verify a symlinked targetDir is rejected.
🤖 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
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/LauncherConfig.java`:
- Around line 71-85: Update parseHttpsUri in LauncherConfig to reject any URI
that contains a query or fragment, not just non-https schemes or missing hosts.
Add validation for getQuery() and getFragment() and throw
IllegalArgumentException for both mosip.client.upgrade.server.url and the
rendered mosip.reg.client.url so versionBase cannot build malformed URLs from
values like https://host/...?... or #....
In
`@registration/registration-launcher/src/test/java/io/mosip/registration/launcher/common/ResumableDownloaderTest.java`:
- Around line 399-414: The test for
download_validatorSidecarUnwritable_failsClosed only checks that an IOException
is thrown, so tighten it to verify the fail-closed behavior as well. Replace the
expected-exception annotation with explicit try/catch around download(...) and
then assert the final artifact file is not created and the temporary .part file
does not contain payload bytes. Use the existing download, startServer, and
writeValidator-related setup in ResumableDownloaderTest to locate the scenario
and keep the assertions focused on the side effects.
---
Outside diff comments:
In
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ManifestVerifier.java`:
- Around line 143-149: Reject symlinked artifacts in
ManifestVerifier.fileMatches by verifying the target is a regular file without
following links before computing the SHA-256. Replace the current file.exists()
check with a no-follow file type check on the File path, and only call
HashUtil.sha256Hex(file) when the path is confirmed to be a regular file. Keep
the existing manifest attribute lookup and expected hash comparison intact, and
ensure JreMigrationStager.verifyArtifactsAgainstRootManifest still relies on
this stricter integrity gate.
---
Duplicate comments:
In
`@registration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ZipExtractor.java`:
- Around line 37-51: The extraction logic in ZipExtractor still allows a
symlinked extraction root or symlinked parent path, so writes can escape the
intended directory before ManifestVerifier runs. Update the ZipExtractor flow
around targetDir, targetRoot, and rejectSymlinkAncestor to fail fast if the
extraction-root path itself contains any symlink before createDirectories or
opening the ZipInputStream. Make the check cover the full root path, not just
entries under it, and add a regression test alongside
extract_throughPreexistingSymlinkDir_isBlocked() to verify a symlinked targetDir
is rejected.
🪄 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: f37d18be-cd85-46b4-b877-5c0066b43181
📒 Files selected for processing (21)
registration/registration-launcher/pom.xmlregistration/registration-launcher/src/main/java/io/mosip/registration/controller/Initialization.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/LauncherConfig.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/LibUpdater.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/MigrationCleaner.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/StartupAction.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/StartupEvaluator.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ManifestVerifier.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ResumableDownloader.javaregistration/registration-launcher/src/main/java/io/mosip/registration/launcher/common/ZipExtractor.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/JreMigrationStagerTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/JreVersionDetectorTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/LauncherConfigTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/LauncherDialogsTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/LibUpdaterTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/MigrationCleanerTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/NormalStartupTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/StartupEvaluatorTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/common/ManifestVerifierTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/common/ResumableDownloaderTest.javaregistration/registration-launcher/src/test/java/io/mosip/registration/launcher/common/ZipExtractorTest.java
Signed-off-by: GOKULRAJ136 <110164849+GOKULRAJ136@users.noreply.github.com>
Refer Story #807
Fixes #812
Summary by CodeRabbit