Harden CAYWResource against XSS and other security issues - #15868
Merged
Merged
Conversation
Siedlerchr
requested review from
koppor and
palukku
and removed request for
koppor
May 31, 2026 18:02
Contributor
Review Summary by QodoHarden CAYW against XSS and library path security issues
WalkthroughsDescription• Harden CAYW browser extension against XSS attacks with security headers • Validate and restrict library path access with user confirmation dialog • Escape HTML content in EntryResource to prevent XSS vulnerabilities • Add security-focused tests for library path validation Diagramflowchart LR
A["CAYW Request"] --> B["Validate Library Path"]
B --> C{Path Served or Allowed?}
C -->|Yes| D["Load Library"]
C -->|No| E["Show User Dialog"]
E --> F{User Decision}
F -->|Allow| D
F -->|Block| G["Reject Request"]
D --> H["Add Security Headers"]
H --> I["Return Response"]
G --> I
File Changes1. jabsrv/src/main/java/org/jabref/http/server/cayw/CAYWResource.java
|
Contributor
Code Review by Qodo
1. Hardcoded INVALID_LIBRARY_PATH_ERROR
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| private static final String CAYW_CONTENT_SECURITY_POLICY = "default-src 'none'; frame-ancestors 'none'; base-uri 'none'"; | ||
| private static final String X_CONTENT_TYPE_OPTIONS = "X-Content-Type-Options"; | ||
| private static final String NO_SNIFF = "nosniff"; | ||
| private static final String INVALID_LIBRARY_PATH_ERROR = "The 'librarypath' parameter must reference a currently served library file."; |
Contributor
There was a problem hiding this comment.
1. Hardcoded invalid_library_path_error 📘 Rule violation ⚙ Maintainability
The new INVALID_LIBRARY_PATH_ERROR message is hardcoded and returned to clients via BadRequestException, making it user-facing but not localizable. This violates the requirement to route user-facing/validation strings through localization (with placeholder-based formatting when needed).
Agent Prompt
## Issue description
A user-facing validation/error message for the `librarypath` parameter is introduced as a hardcoded English string (`INVALID_LIBRARY_PATH_ERROR`) and returned via HTTP `BadRequestException`, which bypasses JabRef localization.
## Issue Context
The compliance rules require all user-facing strings (including validation/integrity messages) to be localized.
## Fix Focus Areas
- jabsrv/src/main/java/org/jabref/http/server/cayw/CAYWResource.java[79-249]
- jablib/src/main/resources/l10n/JabRef_en.properties[200-208]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+286
to
+331
| CompletableFuture<LibraryPathAccessPromptResult> future = new CompletableFuture<>(); | ||
| try { | ||
| Platform.runLater(() -> { | ||
| Alert alert = new Alert(Alert.AlertType.CONFIRMATION); | ||
| alert.setTitle(Localization.lang("Security warning")); | ||
| alert.setHeaderText(Localization.lang("You are about to open a local file.")); | ||
| Label fileLabel = new Label(Localization.lang("File: %0", requestedLibraryPath)); | ||
| CheckBox dontAskAgain = new CheckBox(Localization.lang("Do not ask again")); | ||
| alert.getDialogPane().setContent(new VBox(10, fileLabel, dontAskAgain)); | ||
|
|
||
| ButtonType allowButton = new ButtonType(Localization.lang("Allow"), ButtonBar.ButtonData.YES); | ||
| ButtonType allowAllButton = new ButtonType(Localization.lang("Allow all"), ButtonBar.ButtonData.APPLY); | ||
| ButtonType disallowButton = new ButtonType(Localization.lang("Disallow"), ButtonBar.ButtonData.NO); | ||
| alert.getButtonTypes().setAll(allowButton, allowAllButton, disallowButton); | ||
|
|
||
| ButtonType selectedButton = alert.showAndWait().orElse(disallowButton); | ||
| future.complete(new LibraryPathAccessPromptResult(selectedButton, dontAskAgain.isSelected())); | ||
| }); | ||
| } catch (IllegalStateException exception) { | ||
| LOGGER.warn("JavaFX toolkit not initialized for CAYW security prompt.", exception); | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| LibraryPathAccessPromptResult promptResult = future.get(); | ||
| if (promptResult.selectedButton().getButtonData() == ButtonBar.ButtonData.APPLY) { | ||
| allowAllLibraryPaths = true; | ||
| return true; | ||
| } | ||
|
|
||
| boolean shouldAllow = promptResult.selectedButton().getButtonData() == ButtonBar.ButtonData.YES; | ||
| if (promptResult.dontAskAgain()) { | ||
| if (shouldAllow) { | ||
| TRUSTED_LIBRARY_PATHS.add(requestedLibraryPath); | ||
| } else { | ||
| BLOCKED_LIBRARY_PATHS.add(requestedLibraryPath); | ||
| } | ||
| } | ||
| return shouldAllow; | ||
| } catch (InterruptedException exception) { | ||
| Thread.currentThread().interrupt(); | ||
| LOGGER.warn("Interrupted while waiting for CAYW security prompt.", exception); | ||
| } catch (ExecutionException exception) { | ||
| LOGGER.warn("Failed to evaluate CAYW security prompt.", exception); | ||
| } | ||
| return false; |
Contributor
There was a problem hiding this comment.
3. Prompt blocks request thread 🐞 Bug ☼ Reliability
promptForLibraryPathAccess blocks the request thread on future.get() with no timeout, so any request requiring user confirmation can hang a server thread indefinitely. Exceptions inside the Platform.runLater callback can also prevent completing the future, permanently stalling the request.
Agent Prompt
### Issue description
`promptForLibraryPathAccess` waits indefinitely (`future.get()`) for a JavaFX dialog result. This can exhaust the server thread pool and hang requests forever if the dialog never completes or the UI callback throws.
### Issue Context
CAYW requests are handled by the embedded Grizzly server; blocking the request thread until a user responds means remote callers can keep server threads occupied. Additionally, exceptions thrown inside the `runLater` callback won’t be caught by the surrounding try/catch and can leave the `CompletableFuture` uncompleted.
### Fix Focus Areas
- jabsrv/src/main/java/org/jabref/http/server/cayw/CAYWResource.java[277-332]
- jabsrv/src/main/java/org/jabref/http/server/Server.java[139-171]
### Concrete fix guidance
- Replace `future.get()` with a bounded wait (e.g., `future.get(timeout, TimeUnit.SECONDS)`), and treat timeout as “disallow”.
- Wrap the body of the `Platform.runLater` runnable in a `try/catch` and call `future.completeExceptionally(e)` on failure.
- Consider using `future.orTimeout(...)` (if available) to ensure the future cannot block indefinitely.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
calixtus
reviewed
Jun 1, 2026
| @NullMarked | ||
| @AllowedToUseAwt("Requires java.awt.datatransfer.Clipboard") | ||
| @Path("better-bibtex/cayw") | ||
| @jakarta.ws.rs.Path("better-bibtex/cayw") |
Member
Author
There was a problem hiding this comment.
Yes, it clashes otherwise with nio Path
Member
|
Except removing import of path looks okish for me, but im no expert for this |
palukku
reviewed
Jun 1, 2026
| ### Fixed | ||
|
|
||
| - EndNote and Refer importers now respect the citation key preferences for unwanted characters. [#15743](https://github.com/JabRef/jabref/pull/15743) | ||
| - We hardened CAYW browser extension communication by validating custom `librarypath` access and adding an allow/disallow confirmation dialog for opening local files. [#15295](https://github.com/JabRef/jabref/issues/15295) |
Member
There was a problem hiding this comment.
It's not a Browser extension, it is just an endpoint to call from different applications, such as vscode, texmaker and others
Comment on lines
+257
to
+259
| if (srvStateManager instanceof JabRefSrvStateManager) { | ||
| return true; | ||
| } |
Member
There was a problem hiding this comment.
Why we want to always allow access when running in standalone and therefore not prompt the user?
Member
Author
|
No gui mode?
Philip ***@***.***> schrieb am Mo., 1. Juni 2026, 17:35:
… ***@***.**** commented on this pull request.
------------------------------
In CHANGELOG.md
<#15868 (comment)>:
> @@ -34,6 +34,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
### Fixed
- EndNote and Refer importers now respect the citation key preferences for unwanted characters. [#15743](#15743)
+- We hardened CAYW browser extension communication by validating custom `librarypath` access and adding an allow/disallow confirmation dialog for opening local files. [#15295](#15295)
It's not a Browser extension, it is just an endpoint to call from
different applications, such as vscode, texmaker and others
------------------------------
In jabsrv/src/main/java/org/jabref/http/server/cayw/CAYWResource.java
<#15868 (comment)>:
> + if (srvStateManager instanceof JabRefSrvStateManager) {
+ return true;
+ }
Why we want to always allow access when running in standalone and
therefore not prompt the user?
—
Reply to this email directly, view it on GitHub
<#15868?email_source=notifications&email_token=AACOFZHXKXKR63YJJJAZ4G345WPCHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBQGI2DGNZTHE3KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4402437396>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACOFZD7Z2JPKBHK4JIYPQL45WPCHAVCNFSM6AAAAACZUTDQKOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DIMBSGQZTOMZZGY>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AACOFZDP7HO32PFQW4UNBQD45WPCHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBQGI2DGNZTHE3KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/AACOFZB7KG3UVFS6UTHEOWL45WPCHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBQGI2DGNZTHE3KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Member
But if the cayw endpoint is called, there will be a gui for selection of the entries, so we can also show the prompt. For the cayw to work, there has to be gui no matter if it runs along jabgui or standalone |
Member
Author
|
Okay, got it, I thought in cli mode we don't have a gui |
Siedlerchr
enabled auto-merge
June 4, 2026 09:42
Siedlerchr
added
the
status: ready-for-review
Member
Author
|
@palukku can you approve? |
palukku
approved these changes
Jun 7, 2026
palukku
left a comment
palukku
left a comment
Member
There was a problem hiding this comment.
Works in CLI as well as GUI mode. (No dark mode in both, but does not bother me, can be fixed later).
github-actions
Bot
added
the
status: to-be-merged
5 tasks
Siedlerchr
added a commit
that referenced
this pull request
Jun 8, 2026
* upstream/main: (22 commits) Chore(deps): Bump com.github.andygoossens:gradle-modernizer-plugin from 1.13.0 to 1.14.0 in /build-logic (#15922) Chore(deps): Bump jablib/src/main/resources/csl-styles from `5bb8c99` to `e98c9e1` (#15900) Add ability to view citation previews on hover in 'Citations' tab (#15914) Chore(deps): Bump dev.langchain4j:langchain4j-bom in /versions (#15925) Chore(deps): Bump org.glassfish.grizzly:grizzly-bom in /versions (#15923) Chore(deps): Bump com.uber.nullaway:nullaway in /versions (#15924) SLR : Added performRawSearchQuery to Search Based Fetcher interface (#15916) Fix typos in .jbang/JabKitLauncher.java (#15919) Fix permission (#15918) New Crowdin updates (#15912) Harden CAYWResource against XSS and other security issues (#15868) Fix Cleanup, LastOpenedFiles and XMP reset and import (#15874) Add guard blocking PowerShell here-strings in Bash git commits (#15898) Fix ImporterPreferences reset and import (#15908) Chore(deps): Bump com.uber.nullaway:nullaway in /versions (#15906) Fix: Preserve field focus when navigating entries with keyboard shortcuts (#14943) (#15732) Localization consitency for some of the keys (#15824) chore(deps): update dependency org.glassfish.grizzly:grizzly-http-server to v5.0.2 (#15911) chore(deps): update dependency org.glassfish.grizzly:grizzly-framework to v5.0.2 (#15910) Chore(deps): Bump net.ltgt.nullaway from 3.0.0 to 3.1.0 in /jablib (#15905) ...
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
3 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.
Fixes some of the XSS security issues mentioned. https://github.com/JabRef/jabref/security
Harden security and show a user dialog (currently only valid until restart of jabref)
Related issues and pull requests
Closes _____
PR Description
Steps to test
CAYW tests
AI usage
GPT 5.3 coded helped me to analyze the security issues and to fix them. All code was reviewed by me
Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)