Skip to content

Conversation

@UnschooledGamer
Copy link
Member

@UnschooledGamer UnschooledGamer commented Jan 3, 2026

Updates bouncy castle to 1.83, maverick-synergy-client to 3.1.4

Tests

  • ED25519 Keys
  • RSA Keys

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @UnschooledGamer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on enhancing the SFTP plugin by updating its core dependencies. The updates to maverick-synergy-client and the Bouncy Castle libraries aim to incorporate the latest security patches, performance improvements, and feature enhancements, ensuring the plugin remains robust and secure.

Highlights

  • Dependency Updates: The maverick-synergy-client dependency has been updated from version 3.1.2 to 3.1.4.
  • Security Library Update: The Bouncy Castle libraries, bcprov-jdk15to18 and bcpkix-jdk15to18, have both been updated from version 1.79 to 1.83.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@UnschooledGamer UnschooledGamer self-assigned this Jan 3, 2026
@UnschooledGamer UnschooledGamer added the dependencies Pull requests that update a dependency file label Jan 3, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Bouncy Castle and Maverick Synergy Client dependencies. While I see that ED25519 keys have been tested, it's concerning that RSA keys are marked as not tested. Given that these are cryptographic libraries, updates can introduce subtle regressions. I strongly recommend also testing authentication with RSA keys to ensure there are no regressions for users relying on them. This is crucial for maintaining the stability and reliability of the SFTP functionality.

Comment on lines +28 to +31
<framework src="com.sshtools:maverick-synergy-client:3.1.4" />
<!-- <framework src="com.sshtools:maverick-bc:3.1.2" />-->
<framework src="org.bouncycastle:bcprov-jdk15to18:1.79" />
<framework src="org.bouncycastle:bcpkix-jdk15to18:1.79" />
<framework src="org.bouncycastle:bcprov-jdk15to18:1.83" />
<framework src="org.bouncycastle:bcpkix-jdk15to18:1.83" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To improve maintainability and ensure version consistency, consider defining the dependency versions as preference variables. This makes future updates easier and prevents accidental mismatches, especially for libraries like Bouncy Castle that have multiple artifacts that should be kept in sync.

You could add preferences at the top of your plugin.xml:

<preference name="MAVERICK_SYNERGY_CLIENT_VERSION" default="3.1.4" />
<preference name="BOUNCYCASTLE_VERSION" default="1.83" />

And then reference them in your framework tags:

<framework src="com.sshtools:maverick-synergy-client:$MAVERICK_SYNERGY_CLIENT_VERSION" />
<!--    <framework src="com.sshtools:maverick-bc:3.1.2" />-->
<framework src="org.bouncycastle:bcprov-jdk15to18:$BOUNCYCASTLE_VERSION" />
<framework src="org.bouncycastle:bcpkix-jdk15to18:$BOUNCYCASTLE_VERSION" />

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/gemini Why? Just let it be simple, readable? Why have additional jumping around?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair question, UnschooledGamer. While direct hardcoding might seem simpler for a single change, using preference variables offers significant advantages for maintainability and consistency in the long run, especially as your project grows or when dealing with multiple related dependencies.

Here's why it's a recommended practice:

  • Centralized Version Management: Instead of updating the version number in multiple places (e.g., bcprov-jdk15to18 and bcpkix-jdk15to18 both using 1.83), you only need to change it once in the preference variable. This drastically reduces the chance of errors and ensures all related components are updated in sync.
  • Easier Upgrades: When a new version of a library is released, you just update the preference variable, and all references to that library across your plugin.xml (or other configuration files that might use these preferences) are automatically updated. This is particularly useful for security updates or major version bumps.
  • Improved Readability for Dependency Groups: For libraries like Bouncy Castle, which often come in multiple artifacts that must share the same version, defining a single BOUNCYCASTLE_VERSION preference makes it immediately clear that these are intended to be a cohesive unit. It highlights the relationship between these dependencies.

While it introduces a small layer of indirection, the benefit of preventing version mismatches and simplifying future updates often outweighs the minor

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/gemini Well, it's single-use in the same file (plugin.xml) won't be used in other places. If we presume that both versions of Bouncy Castle will be the same, and if the Versioning Team or their CI system messes up, then it's better to keep it simple when it's used in a single place. Rather than having Variables, If the Deps increase, We can have variables in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your perspective on keeping things simple, especially when a dependency is only referenced in one file. You're right that for a single, isolated dependency, the benefit of a variable might seem minimal.

However, the core of my suggestion, particularly for Bouncy Castle, is about consistency across related artifacts. You have bcprov-jdk15to18 and bcpkix-jdk15to18 which are two distinct artifacts from the same Bouncy Castle library. It's critical that these two always use the exact same version. If you were to manually update one and accidentally miss the other, it could lead to subtle and hard-to-debug runtime issues due to version mismatches.

Regarding your point about the "Versioning Team or CI system messing up," using a preference variable actually reduces the surface area for such errors. Instead of two separate lines to update for Bouncy Castle, there's only one central place (the preference variable) to change. This makes manual updates less prone to mistakes and provides a clear, single source of truth for that version, which CI systems can then reliably leverage.

While it's true you could introduce variables later if dependencies increase, adopting this pattern now, even for this case, establishes a consistent and robust approach to dependency management. It's a minor change now that prevents potential headaches and ensures best practices are followed from the outset, especially for library groups that must stay in sync.

Ultimately, it's a trade-off between immediate perceived simplicity and long-term maintainability and error prevention. My suggestion leans towards the latter for more robust dependency management.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/gemini Alright, For bc we can. For others are not needed currently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for considering the suggestion for Bouncy Castle, UnschooledGamer! I've updated the code suggestion to reflect this.

The suggested change below will update the framework tags for bcprov-jdk15to18 and bcpkix-jdk15to18 to use the $BOUNCYCASTLE_VERSION variable. As you requested, the maverick-synergy-client dependency remains hardcoded.

Important: Please remember to add the following preference variable at the top of your plugin.xml file (e.g., after the <plugin> tag and before any <platform> or <framework> tags) for this to work correctly:

<preference name="BOUNCYCASTLE_VERSION" default="1.83" />

This ensures that the $BOUNCYCASTLE_VERSION variable is defined before it's referenced.

Suggested change
<framework src="com.sshtools:maverick-synergy-client:3.1.4" />
<!-- <framework src="com.sshtools:maverick-bc:3.1.2" />-->
<framework src="org.bouncycastle:bcprov-jdk15to18:1.79" />
<framework src="org.bouncycastle:bcpkix-jdk15to18:1.79" />
<framework src="org.bouncycastle:bcprov-jdk15to18:1.83" />
<framework src="org.bouncycastle:bcpkix-jdk15to18:1.83" />
<framework src="com.sshtools:maverick-synergy-client:3.1.4" />
<!-- <framework src="com.sshtools:maverick-bc:3.1.2" />-->
<framework src="org.bouncycastle:bcprov-jdk15to18:$BOUNCYCASTLE_VERSION" />
<framework src="org.bouncycastle:bcpkix-jdk15to18:$BOUNCYCASTLE_VERSION" />

@UnschooledGamer UnschooledGamer merged commit ca86bfe into main Jan 3, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants