Copilot commented on code in PR #903:
URL: https://github.com/apache/maven-wagon/pull/903#discussion_r3740655224
##########
wagon-providers/wagon-ssh/src/main/java/org/apache/maven/wagon/providers/ssh/jsch/AbstractJschWagon.java:
##########
@@ -123,23 +125,20 @@ public void openConnectionInternal() throws
AuthenticationException {
throw new AuthenticationException(e.getMessage());
}
- // can only pick one method of authentication
- if (privateKey != null && privateKey.exists()) {
- fireSessionDebug("Using private key: " + privateKey);
- try {
- sch.addIdentity(privateKey.getAbsolutePath(),
authenticationInfo.getPassphrase());
- } catch (JSchException e) {
- throw new AuthenticationException("Cannot connect. Reason: " +
e.getMessage(), e);
- }
+ // Can only pick one method of authentication, so pick them in order
of how deliberate they are:
+ // a key named in the settings first, then the agent, and only then a
key file that merely happened
+ // to be lying in ~/.ssh. Letting a found key file outrank the agent
means an agent is never reached
+ // on a machine that has one of those files, which is most of them.
+ boolean privateKeyConfigured = authenticationInfo.getPrivateKey() !=
null;
Review Comment:
The new authentication precedence (configured key > agent > discovered
~/.ssh key) and the agent-selection logic aren’t covered by existing unit
tests. There are tests around private-key configuration errors, but none
exercise the new agent path or verify that an available agent overrides an
auto-discovered key file, so regressions here would likely slip through.
##########
wagon-providers/wagon-ssh-common/src/main/java/org/apache/maven/wagon/providers/ssh/ScpHelper.java:
##########
@@ -102,23 +102,28 @@ public static File getPrivateKey(AuthenticationInfo
authenticationInfo) throws F
return privateKey;
}
+ /**
+ * The key types <code>ssh-keygen</code> produces, most recent first.
<code>id_dsa</code> is not among
+ * them: ssh-dss has been disabled by default in OpenSSH for years, so a
DSA key is the one least likely
+ * to be accepted by the server we are about to reach.
+ */
+ private static final String[] PRIVATE_KEY_NAMES = {"id_ed25519",
"id_ecdsa", "id_rsa"};
+
Review Comment:
Key discovery now prefers id_ed25519/id_ecdsa/id_rsa, but there’s no test
coverage asserting the discovery order and behavior when multiple key types are
present in the configured ~/.ssh directory. Given this affects both wagon-ssh
(embedded JSch) and wagon-ssh-external (ssh -i selection), it would be good to
add a unit test that sets wagon.privateKeyDirectory to a temp dir and verifies
the selected key name under different file combinations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
