Fix KeePass XML parser compatibility - #979
Merged
Merged
Conversation
Contributor
|
The changes in this PR will be included in the next version bump.
|
commit: |
The kdbxweb patch only reaches users because the plugin bundles kdbxweb at build time, and nothing verified that. Plugin test suites had no `test:ci` script, so `turbo test:ci` never ran them - the keepass suite reproduces this bug exactly but had been failing unnoticed. - add `test:ci` to every plugin package that has tests (10 suites) - make turbo's `test:ci` depend on `build`, since plugin tests load dist/ - exclude the website from `test:ci` so it doesn't drag in a full astro build - defer @initKeePass db setup until a kp()/kpBulk() call runs, so an unused instance with an empty password no longer fails the whole schema - document the kdbxweb patch and its tie to the @xmldom/xmldom override
kdbxweb passes the errorHandler object that xmldom removed in 0.9, so the root override (>=0.9.10, added for security) broke every KeePass database. xmldom 0.8.13 shipped the same day as 0.9.10 with all five advisories backported, and it still accepts errorHandler - so a version pin fixes this with no patched dependency to maintain. kdbxweb is the only package in the repo that depends on xmldom, and bun ignores yarn-style scoped overrides, so the pin has to be global.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
varlock-website | 8bebc4b | Commit Preview URL Branch Preview URL |
Aug 07 2026, 07:43 PM |
kdbxweb builds its parser with the errorHandler object that @xmldom/xmldom removed in 0.9, so the root override (>=0.9.10) broke every KeePass database. Pinning xmldom back to 0.8.x fixed it, but put a repo-wide ceiling on a dependency for one plugin's sake. kdbxweb's xmldom import is now redirected at build time to a small wrapper that translates errorHandler into the supported onError callback, so the override stays where it was and the constraint lives in the plugin that has it. Unit tests cover the wrapper directly, since the strict parse behavior it preserves is not otherwise reachable from a valid database file.
Contributor
Author
|
Fixed the CI failure in commit Task list (5/5 completed)
|
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
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.



Fixes #978.
kdbxwebbuilds its parser with theerrorHandlerobject that@xmldom/xmldomremoved in 0.9, so the rootoverridesentry pinning xmldom to>=0.9.10(added as a security floor) broke every KeePass database witherrorHandler object is no longer supported. kdbxweb has not shipped a release since 2021 and upstream still has the old code, so there is no version to upgrade to.Since the plugin bundles kdbxweb, its xmldom import is now redirected at build time to
src/xmldom-compat.ts, a small wrapper that translateserrorHandlerinto the supportedonErrorcallback and re-exports the rest of xmldom untouched. The override stays where it is, and the constraint lives in the one plugin that has it rather than capping xmldom for the whole repo.Two things worth knowing for anyone touching that config: esbuild's
aliasoption cannot express this (it prefix-matches subpaths, so the shim's own import resolves back into the shim, which is why it uses anonResolvehook), and xmldom has to be a direct dependency of the plugin or it will not resolve under bun's isolated installs.The alternative was pinning xmldom to
^0.8.13, the newest release kdbxweb can talk to and free of the advisories that motivated the override. That worked, but put a repo-wide ceiling on a dependency for one plugin's sake.Plugin tests now run in CI
This shipped because no plugin package defined a
test:ciscript, and CI runsturbo test:ci. The keepass suite reproduces this bug exactly, and had been failing unnoticed.test:ciadded to all 10 plugin packages that have tests. 1password and infisical build a separate test bundle first, without which their suites error out and report 31 and 5 tests "skipped".test:cinow depends onbuild, since plugin tests loaddist/plugin.cjsrather thansrc.@varlock/websiteexcluded from the roottest:ci, so that newbuilddependency does not drag a full Astro build into every test run.Unit tests cover the compat wrapper directly. A valid database cannot produce the non-fatal parse errors it guards against (the KDBX HMAC catches corruption first), so the strict-parse behavior is not reachable end to end.
Deferred KeePass setup
@initKeePassresolveddbPath/passwordeagerly at schema load, so an empty master password was fatal even when nothing in the schema read from KeePass. Setup now runs on the firstkp()/kpBulk()call, memoized per instance. This is what the existingunused plugin with empty password causes no errorstest always expected.Verification
Full
test:ci --force: 43/43 tasks, keepass 24/24.bun auditclean for the keepass plugin. Lint,typecheck:all, and the docs build all pass. Confirmed the builtdist/plugin.cjsbundles xmldom 0.9.10 and opens real databases through it.azure/gpt-5.6-sol| 𝕏