Copilot commented on code in PR #902:
URL: https://github.com/apache/maven-wagon/pull/902#discussion_r3740658218


##########
wagon-providers/wagon-ssh/src/main/java/org/apache/maven/wagon/providers/ssh/jsch/AbstractJschWagon.java:
##########
@@ -252,6 +251,76 @@ public void openConnectionInternal() throws 
AuthenticationException {
         }
     }
 
+    private void useIdentityFile(JSch sch, File privateKey) throws 
AuthenticationException {
+        fireSessionDebug("Using private key: " + privateKey);
+
+        // getPrivateKey() substitutes an empty passphrase when the settings 
name none, which is
+        // indistinguishable from "this key is not encrypted". Handing that on 
rejects every encrypted key
+        // outright. Passing null instead lets an unencrypted key through and 
leaves an encrypted one to be
+        // unlocked when it is used, which is where the user can still be 
asked for the passphrase.
+        String passphrase = authenticationInfo.getPassphrase();
+        if (passphrase != null && passphrase.isEmpty()) {
+            passphrase = null;
+        }
+
+        try {
+            sch.addIdentity(privateKey.getAbsolutePath(), passphrase);
+        } catch (JSchException e) {
+            throw new AuthenticationException("Cannot connect. Reason: " + 
e.getMessage(), e);
+        }
+    }
+
+    /**
+     * The identities held by a running SSH agent, or <code>null</code> when 
no agent can be reached or the
+     * one reached holds none, in which case the caller falls back to a key 
file.
+     * <p>
+     * Three kinds of agent are tried in turn: the OpenSSH agent named by 
<code>SSH_AUTH_SOCK</code>, the
+     * Win32 OpenSSH agent, and Pageant. Every one of them is optional and 
each has its own prerequisites --
+     * the first needs a Unix domain socket, which the JDK itself provides 
only from Java 16, and Pageant
+     * needs JNA, which is not a dependency of this provider. Each is 
therefore both constructed and used
+     * inside its own guard, so that a missing class is one skipped agent 
rather than a failure to load.
+     */
+    private IdentityRepository agentIdentityRepository() {
+        IdentityRepository repository = identitiesFrom(SSHAgentConnector::new);
+        if (repository == null) {
+            repository = identitiesFrom(WindowsSSHAgentConnector::new);
+        }
+        if (repository == null) {
+            repository = identitiesFrom(PageantConnector::new);
+        }
+        return repository;
+    }
+
+    /**
+     * Creates one kind of {@link AgentConnector}, in a way that must only be 
invoked from inside
+     * {@link #identitiesFrom}: the classes involved may be absent at runtime, 
so the first mention of one
+     * has to sit where a {@link LinkageError} is caught.
+     */

Review Comment:
   `agentIdentityRepository()` passes method references like 
`SSHAgentConnector::new` / `PageantConnector::new` into `identitiesFrom(...)`, 
but those references can trigger class resolution *before* 
`identitiesFrom(...)` enters its `try/catch`. If the connector class (or its 
transitive deps like JNA for Pageant) is missing at runtime, a 
`LinkageError`/`NoClassDefFoundError` can be thrown outside the guard, 
preventing the intended “skip this agent” fallback.



##########
wagon-providers/wagon-ssh-common/src/main/java/org/apache/maven/wagon/providers/ssh/ScpHelper.java:
##########
@@ -103,23 +103,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:
   `findPrivateKey()` now prefers `id_ed25519` over RSA. On Java runtimes 
without Ed25519 support (and without an installed provider like BouncyCastle), 
selecting `~/.ssh/id_ed25519` will fail even if a usable `id_rsa` is also 
present. Since this module targets older Java baselines as well, it’s safer to 
skip `id_ed25519` when the runtime can’t support it, rather than hard-failing 
key-based auth.



-- 
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]

Reply via email to