feat: adds ssh support for git operations #987

dcoric · 2025-04-28T12:28:46Z

This PR implements SSH support for Git operations, resolving #27. The implementation allows users to perform Git operations over SSH protocol, providing an alternative to HTTPS for repository access.

Changes

Added SSH server implementation using ssh2 library
Implemented two authentication methods:
- Public key authentication
- Password authentication
Added SSH configuration options in config.schema.json
Created comprehensive SSH documentation in docs/SSH.md
Added SSH server tests in test/ssh/sshServer.test.js

Configuration

SSH support can be configured in the main configuration file:

{
  "ssh": {
    "enabled": true,
    "port": 2222,
    "hostKey": {
      "privateKeyPath": "./.ssh/host_key",
      "publicKeyPath": "./.ssh/host_key.pub"
    }
  }
}

Usage

Users can connect using standard Git SSH commands. The command format depends on the configured port:

If port is set to 22 (default SSH port):

git clone git@your-proxy:username/repo.git

If using a custom port (e.g., 2222):

git clone ssh://git@your-proxy:2222/username/repo.git

Testing

Added comprehensive tests for SSH functionality:

Server initialization
Authentication methods
Connection handling
Git protocol support

Documentation

Added detailed documentation in docs/SSH.md covering:

Configuration options
Authentication methods
Security features
Usage instructions
Troubleshooting guide

Future Improvements

Move SSH configuration options (keep alive, timeouts, and other params) to config file
Enhance actions for SSH functionality
Increase test coverage
Improved error reporting
Additional security features

This implementation provides a secure and reliable way to perform Git operations over SSH, giving users more flexibility in how they interact with their repositories.

Below is a summary of the most important changes:

SSH Feature Implementation:

Added a new ssh configuration section in config.schema.json and proxy.config.json to enable SSH support, configure the port, and specify host key paths. ([[1]](https://github.com/finos/git-proxy/pull/987/files#diff-be1695b1e63a508d59982601f9e1fb7f58247deecb1e427adb77bcad758ae5e5R82-R111), [[2]](https://github.com/finos/git-proxy/pull/987/files#diff-c465aafa0fe603e2d28b017938f55e5ce3253aac7aa303efeabfc06a4ad52d5fR105-R112))
Created docs/SSH.md to provide comprehensive documentation for configuring, using, and troubleshooting the SSH feature. ([docs/SSH.mdR1-R165](https://github.com/finos/git-proxy/pull/987/files#diff-1d0301881738da7c699d7634669c5156c345a5094d6c012cfa7254cbdf15cbd2R1-R165))
Integrated SSH key management into the CLI, including commands to add and remove SSH keys (addSSHKey function and ssh-key command). ([[1]](https://github.com/finos/git-proxy/pull/987/files#diff-ee51eb1c2264303569f2a8fa9f5bb2de1b6b51eed24eed943d079f341f4139b0R310-R363), [[2]](https://github.com/finos/git-proxy/pull/987/files#diff-ee51eb1c2264303569f2a8fa9f5bb2de1b6b51eed24eed943d079f341f4139b0R494-R524), [[3]](https://github.com/finos/git-proxy/pull/987/files#diff-c3c551630c7afb35431840ab36bd8cc771deb1fa385b73804eaf8da4b8db7e0cR1-R122))

Database Enhancements:

Updated the user schema to include SSH public keys and added functions for managing SSH keys (addPublicKey, removePublicKey, findUserBySSHKey). ([[1]](https://github.com/finos/git-proxy/pull/987/files#diff-d90c8ba033b20fb64a27fb6f16a94afa5ebc2f25bb7af6ec42cd260dcbf46710R43-R46), [[2]](https://github.com/finos/git-proxy/pull/987/files#diff-d90c8ba033b20fb64a27fb6f16a94afa5ebc2f25bb7af6ec42cd260dcbf46710R98-R155), [[3]](https://github.com/finos/git-proxy/pull/987/files#diff-c7d2c320d1f7fc0aad36b8bf616a7289ca4dcf00e6e057ba8a1d354fb25b74c0R84-R86))

Configuration and Code Updates:

Introduced new methods in src/config/index.ts to retrieve SSH-related settings (getSSHConfig, getSSHProxyUrl). ([[1]](https://github.com/finos/git-proxy/pull/987/files#diff-198a1431d7c5e26ea78bc6e54b34b54e6f8ab342dd48ea11013877e8466a87c5R33), [[2]](https://github.com/finos/git-proxy/pull/987/files#diff-198a1431d7c5e26ea78bc6e54b34b54e6f8ab342dd48ea11013877e8466a87c5R48-R58))
Updated package.json to include the ssh2 library and its TypeScript types for SSH server implementation. ([[1]](https://github.com/finos/git-proxy/pull/987/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R75), [[2]](https://github.com/finos/git-proxy/pull/987/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R90))

Documentation Enhancements:

Updated README.md to include a link to the new SSH documentation. ([README.mdR88-R95](https://github.com/finos/git-proxy/pull/987/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R88-R95))

These changes collectively enable the SSH feature, providing an alternative to HTTPS for secure Git operations while enhancing user control and security.

netlify · 2025-04-28T12:29:07Z

✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name	Link
🔨 Latest commit	`b8ba792`
🔍 Latest deploy log	https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/690f3b78838a9e0008cb6c0f

dcoric · 2025-05-09T14:25:22Z

@JamieSlome @coopernetes, please take a look when you get a chance

jescalada

Just a preliminary review of the code changes. I'll run it and test more thoroughly afterwards!

jescalada · 2025-05-31T04:17:41Z

packages/git-proxy-cli/index.ts

+        addSSHKey(argv.username, argv.keyPath);
+      } else if (argv.action === 'remove') {
+        // TODO: Implement remove SSH key
+        console.error('Error: SSH key: Remove action not implemented yet');


Is the required removeSSHKey function implemented now? Maybe all we need to do is plug it in here 😃

jescalada · 2025-05-31T04:53:26Z

src/proxy/ssh/server.js

+
+class SSHServer {
+  constructor() {
+    // TODO: Server config could go to config file


Just bringing attention to this TODO

jescalada · 2025-05-31T05:17:19Z

src/proxy/ssh/server.js

+          // Get remote host from config
+          const remoteUrl = new URL(config.getProxyUrl());
+
+          // TODO: Connection options could go to config


Bringing attention to this TODO. Is there a reason why the default port is 22 in some places, but 2222 in the sample proxy.config.json? 🤔

jescalada · 2025-05-31T05:24:31Z

src/proxy/ssh/server.js

+                  config.getSSHConfig().hostKey.privateKeyPath,
+                ),
+                // Ensure these settings are explicitly set for the proxy connection
+                windowSize: 1024 * 1024, // 1MB window size


Might be best to extract these (and the repeated ones from earlier in the file) into constants so they can be reused.

jescalada · 2025-05-31T05:34:50Z

src/service/routes/users.ts

+// Add SSH public key
+router.post('/:username/ssh-keys', async (req, res) => {
+  if (!req.user) {
+    res.status(401).json({ error: 'Authentication required' });


I'm wondering if this could be 'Login required' to make it a bit more clear that the user should login first. Since it will mainly be used as an API, the flow may not be obvious.

Feel free to ignore this if the flow (including login) is documented!

jescalada · 2025-05-31T05:47:21Z

src/service/routes/users.ts

+  }
+
+  // Strip the comment from the key (everything after the last space)
+  const keyWithoutComment = publicKey.split(' ').slice(0, 2).join(' ');


Should we add some validation here to make sure that the key is in a valid format? Or would this be more appropriate for the db functions?

Right now, invalid formatting would get caught in the try/catch below, but the user-facing error wouldn't be descriptive.

jescalada

Review (Part 2): I managed to confirm that the general flow works as expected, but I'm wondering how to trigger the proxy action chain through SSH.

Other than that, improving user-facing errors and some documentation improvements would be great for UX. 🙂

jescalada · 2025-05-31T12:06:20Z

src/proxy/ssh/server.js

+    // TODO: Server config could go to config file
+    this.server = new ssh2.Server(
+      {
+        hostKeys: [require('fs').readFileSync(config.getSSHConfig().hostKey.privateKeyPath)],


This line blows up when running git-proxy (npm run start) if the file is not present:

[0] Service Listening on 8080 [0] node:fs:562 [0] return binding.open( [0] ^ [0] [0] Error: ENOENT: no such file or directory, open './.ssh/host_key' [0] at Object.openSync (node:fs:562:18) [0] at Object.readFileSync (node:fs:446:35) [0] at new SSHServer (/home/juan/Desktop/Juan/Projects/git-proxy/src/proxy/ssh/server.js:11:34) [0] at proxyPreparations (/home/juan/Desktop/Juan/Projects/git-proxy/src/proxy/index.ts:52:23) [0] at process.processTicksAndRejections (node:internal/process/task_queues:105:5) [0] at async Object.start (/home/juan/Desktop/Juan/Projects/git-proxy/src/proxy/index.ts:68:3) { [0] errno: -2, [0] code: 'ENOENT', [0] syscall: 'open', [0] path: './.ssh/host_key' [0] } [0] [0] Node.js v22.13.1 [0] npm run server exited with code 1 ^C[1] npm run client exited with code SIGINT

Some validation here would be great, perhaps we can recover from the error and let git-proxy run, or stop the server and let the user know that they should either provide a valid file, or disable ssh in the config.

jescalada · 2025-05-31T12:14:49Z

src/proxy/ssh/server.js

+    // TODO: Server config could go to config file
+    this.server = new ssh2.Server(
+      {
+        hostKeys: [require('fs').readFileSync(config.getSSHConfig().hostKey.privateKeyPath)],


Another issue I found, is that the path doesn't seem to resolve when using ~. It might be nice to be able to set the config to ~/.ssh/host_key so that it resolves to the home directory. Not a huge deal though! 👍🏼

jescalada · 2025-05-31T12:25:22Z

src/db/file/users.ts

+    findUser(username)
+      .then((user) => {
+        if (!user) {
+          reject(new Error('User not found'));


This line gets successfully triggered when the user isn't found. However, the CLI just prints this out instead of bubbling up the User not found error message (which is much more descriptive):

Error: SSH key: 'Request failed with status code 500'

jescalada · 2025-05-31T12:39:49Z

src/db/file/users.ts

+        }
+        if (!user.publicKeys.includes(publicKey)) {
+          user.publicKeys.push(publicKey);
+          exports.updateUser(user).then(resolve).catch(reject);


Oddly, the exports here is tripping up my runtime:

[0] Adding SSH key { targetUsername: 'admin', keyWithoutComment: '-----BEGIN OPENSSH' } [0] Error adding SSH key: TypeError: exports.updateUser is not a function [0] at <anonymous> (/home/juan/Desktop/Juan/Projects/git-proxy/src/db/file/users.ts:112:19) [0] at process.processTicksAndRejections (node:internal/process/task_queues:105:5)

Removing the exports causes a TypeError due to how the Promise is handled, but we can fix it like this:

Suggested change

exports.updateUser(user).then(resolve).catch(reject);

updateUser(user).then(() => resolve(user)).catch(reject);

jescalada · 2025-05-31T12:41:36Z

src/db/file/users.ts

+          return;
+        }
+        user.publicKeys = user.publicKeys.filter((key) => key !== publicKey);
+        exports.updateUser(user).then(resolve).catch(reject);


Same as above, this made the code run properly for me:

Suggested change

exports.updateUser(user).then(resolve).catch(reject);

updateUser(user).then(() => resolve(user)).catch(reject);

I might be messing something up in my flow though. I'm executing npm run start for git-proxy and then separately executing the CLI commands via:

node ./packages/git-proxy-cli/index.js ssh-key --action add --username admin --keyPath /home/juan/.ssh/host_key.pub

jescalada · 2025-05-31T13:09:28Z

src/proxy/ssh/server.js

+              });
+
+              proxyGitSsh.on('error', (err) => {
+                console.error('[SSH] Remote SSH error with proxy key:', err);


Upon entering an invalid key, I trigger the following error on line 638:

git-proxy backend

[0] [GitHub SSH Debug] Handshake completed [0] [GitHub SSH Debug] Outbound: Sending SERVICE_REQUEST (ssh-userauth) [0] [GitHub SSH Debug] Inbound: Received EXT_INFO [0] [GitHub SSH Debug] Inbound: Received SERVICE_ACCEPT (ssh-userauth) [0] [GitHub SSH Debug] Outbound: Sending USERAUTH_REQUEST (none) [0] [GitHub SSH Debug] Inbound: Received USERAUTH_FAILURE (publickey) [0] [GitHub SSH Debug] Client: none auth failed [0] [GitHub SSH Debug] Outbound: Sending USERAUTH_REQUEST (publickey -- check) [0] [GitHub SSH Debug] Inbound: Received USERAUTH_FAILURE (publickey) [0] [GitHub SSH Debug] Client: publickey (rsa-sha2-256) auth failed [0] [GitHub SSH Debug] Client: publickey auth failed [0] [SSH] Remote SSH error with proxy key: Error: All configured authentication methods failed [0] at doNextAuth (/home/juan/Desktop/Juan/Projects/git-proxy/node_modules/ssh2/lib/client.js:863:21) [0] at tryNextAuth (/home/juan/Desktop/Juan/Projects/git-proxy/node_modules/ssh2/lib/client.js:1080:7) [0] at USERAUTH_FAILURE (/home/juan/Desktop/Juan/Projects/git-proxy/node_modules/ssh2/lib/client.js:428:11) [0] at 51 (/home/juan/Desktop/Juan/Projects/git-proxy/node_modules/ssh2/lib/protocol/handlers.misc.js:408:16) [0] at Protocol.onPayload (/home/juan/Desktop/Juan/Projects/git-proxy/node_modules/ssh2/lib/protocol/Protocol.js:2059:10) [0] at AESGCMDecipherBinding.decrypt (/home/juan/Desktop/Juan/Projects/git-proxy/node_modules/ssh2/lib/protocol/crypto.js:1086:26) [0] at Protocol.parsePacket [as _parse] (/home/juan/Desktop/Juan/Projects/git-proxy/node_modules/ssh2/lib/protocol/Protocol.js:2028:25) [0] at Protocol.parse (/home/juan/Desktop/Juan/Projects/git-proxy/node_modules/ssh2/lib/protocol/Protocol.js:313:16) [0] at Socket.<anonymous> (/home/juan/Desktop/Juan/Projects/git-proxy/node_modules/ssh2/lib/client.js:773:21) [0] at Socket.emit (node:events:524:28) { [0] level: 'client-authentication' [0] } [0] [SSH Debug] [518855.983928741] Outbound: Sending CHANNEL_DATA (r:0, 51) [0] [SSH Debug] [518855.983928741] Outbound: Sending CHANNEL_EOF (r:0) [0] [SSH Debug] [518855.983928741] Outbound: Sending CHANNEL_CLOSE (r:0) [0] [GitHub SSH Debug] Outbound: Sending DISCONNECT (11) [0] [SSH Debug] [518855.983928741] Inbound: CHANNEL_CLOSE (r:0) [0] [SSH Debug] [518855.983928741] Inbound: Received DISCONNECT (11, "disconnected by user") [0] [SSH Debug] [518855.983928741] Socket ended [0] [SSH] Client disconnected [0] [SSH] Client disconnected [0] [SSH Debug] [518855.983928741] Socket closed [0] [SSH] Client connection closed [0] [GitHub SSH Debug] Socket ended [0] [GitHub SSH Debug] Socket closed

However, the user facing CLI gives an unrelated error:

User-facing CLI

$ git clone ssh://git@localhost:2222/jescalada/git-proxy-experiments.git Cloning into 'git-proxy-experiments'... fatal: protocol error: bad line length character: Erro

Would be great if we could get a more descriptive error for this common scenario (bad key).

In my case, this was because of my GitHub authentication not set up correctly.

jescalada · 2025-05-31T13:39:52Z

src/proxy/ssh/server.js

+
+            // Execute the Git command on remote
+            remoteGitSsh.exec(
+              command,


I might be missing something in my configuration, but when I execute git push, the git-proxy action chain isn't triggered:

$ git push ssh://git@localhost:2222/jescalada/git-proxy-experiments.git Enumerating objects: 5, done. Counting objects: 100% (5/5), done. Delta compression using up to 12 threads Compressing objects: 100% (3/3), done. Writing objects: 100% (3/3), 331 bytes | 331.00 KiB/s, done. Total 3 (delta 1), reused 0 (delta 0), pack-reused 0 remote: Resolving deltas: 100% (1/1), completed with 1 local object. To ssh://localhost:2222/jescalada/git-proxy-experiments.git fa06cb5..ea81914 main -> main

It directly updated my upstream repo instead of going through the proxy's approval process.

That said, after setting up the public key in the GitHub SSH page, the SSH works great! The SSH key setup is worth explaining in detail in the ssh.md guide.

jescalada · 2025-05-31T14:19:01Z

package.json

The test script should be as follows (so that the ssh tests get covered in the PR):

"test": "NODE_ENV=test ts-mocha './test/**/*.test.js' --exit",

Merging the current main into the PR might fix this too.

(I changed this by accident in one of my TS refactor PRs. 😓)

jescalada

Review (Part 3): Just took a look at the unit tests. It would be great to cover some more edge cases and get more granular testing by passing error messages through the ctx.reject (if possible).

If you'd like, I can help out with the documentation refactor and unit test coverage 🙂

jescalada · 2025-06-02T06:27:51Z

src/proxy/ssh/server.js

+        accept();
+      } else {
+        console.log('[SSH] Rejecting unknown global request:', info.type);
+        reject();


Would it be better if we sent the error string as a reject argument? Might be better for more granular testing.

jescalada · 2025-06-02T06:29:56Z

test/ssh/sshServer.test.js

+      expect(mockClient.on.calledWith('authentication')).to.be.true;
+    });
+
+    describe('authentication', () => {


Would be great to add extra tests for authentication failure cases. These are a few passing sample tests I wrote:

Suggested change

describe('authentication', () => {

describe('authentication', () => {

it('should reject invalid public keys', async () => {

const mockCtx = {

method: 'publickey',

key: {

algo: 'ssh-rsa',

data: Buffer.from('invalid-key-data'),

comment: 'invalid-key',

},

accept: sinon.stub(),

reject: sinon.stub(),

};

mockDb.findUserBySSHKey.resolves(null);

server.handleClient(mockClient);

const authHandler = mockClient.on.withArgs('authentication').firstCall.args[1];

await authHandler(mockCtx);

expect(mockCtx.reject.calledOnce).to.be.true;

expect(mockClient.username).to.be.null;

expect(mockClient.userPrivateKey).to.be.null;

});

it('should reject invalid passwords', async () => {

const mockCtx = {

method: 'password',

username: 'test-user',

password: 'invalid-password',

accept: sinon.stub(),

reject: sinon.stub(),

};

mockDb.findUser.resolves({

username: 'test-user',

password: '$2a$10$mockHash',

});

const bcrypt = require('bcryptjs');

sinon.stub(bcrypt, 'compare').resolves(false);

server.handleClient(mockClient);

const authHandler = mockClient.on.withArgs('authentication').firstCall.args[1];

await authHandler(mockCtx);

expect(mockDb.findUser.calledWith('test-user')).to.be.true;

expect(bcrypt.compare.calledWith('invalid-password', '$2a$10$mockHash')).to.be.true;

expect(mockCtx.reject.calledOnce).to.be.true;

});

it('should handle a missing private key', async () => {

const mockCtx = {

method: 'publickey',

key: null,

accept: sinon.stub(),

reject: sinon.stub(),

};

mockDb.findUserBySSHKey.resolves({ username: 'test-user' });

server.handleClient(mockClient);

const authHandler = mockClient.on.withArgs('authentication').firstCall.args[1];

await authHandler(mockCtx);

expect(mockCtx.reject.calledOnce).to.be.true;

expect(mockClient.username).to.be.null;

expect(mockClient.userPrivateKey).to.be.null;

});

Extra authentication test case ideas:

Malformed SSH keys

Currently, malformed keys go through the findUserBySSHKey function which logs [SSH] No user found with this SSH key instead of throwing error

Invalid auth method (neither publickey nor password)

Nonexistent user

jescalada · 2025-06-02T06:51:17Z

test/ssh/sshServer.test.js

+        command: "git-upload-pack 'test/repo'",
+      };
+
+      mockChain.executeChain.resolves({


Would be great to include some cases with error: true/blocked: true:

Sample

I wrote a sample test for this, but it seems to be failing:

it('should handle git-upload-pack command with error in chain', async () => { const mockInfo = { command: "git-upload-pack 'test/repo'", }; mockChain.executeChain.resolves({ error: true, blocked: true, }); server.handleSession(mockAccept, mockReject); const execHandler = mockSession.on.withArgs('exec').firstCall.args[1]; await execHandler(mockAccept, mockReject, mockInfo); expect(mockChain.executeChain.calledWith({ method: 'GET', originalUrl: " 'test/repo", isSSH: true, })).to.be.true; expect(mockStream.end.calledOnce).to.be.true; expect(mockStream.exit.calledOnce).to.be.true; expect(mockStream.exit.calledWith(1)).to.be.true; });

Error:

2) SSHServer handleSession should handle git-upload-pack command with invalid repo: TypeError: stream.write is not a function at SSHServer.<anonymous> (src/proxy/ssh/server.js:664:18) at Generator.next (<anonymous>) at fulfilled (src/proxy/ssh/server.js:5:58) at processTicksAndRejections (node:internal/process/task_queues:105:5)

jescalada · 2025-06-02T07:09:43Z

test/ssh/sshServer.test.js

+
+    it('should handle git-receive-pack command', async () => {
+      const mockInfo = {
+        command: "git-receive-pack 'test/repo'",


Other possible test cases for handleSession:

Invalid repo handling

Currently gets stuck on [GitHub SSH Debug] Client: Trying github.com on port 22 ... without throwing error. executeChain doesn't seem to get called

Edge case tests:

Empty repository name, whitespace inputs

SSH injection (example: command: "git-upload-pack 'repo'; rm -rf /; #",)

- Implement complete SSH server with public key and password authentication - Add SSH key management to user database (both File and MongoDB) - Create SSH CLI tools for key management - Add SSH configuration schema and TypeScript types - Integrate SSH server with main proxy lifecycle - Add REST endpoints for SSH key CRUD operations - Include comprehensive test suite and documentation - Support Git operations over SSH with full proxy chain integration

@ts-expect-error

- Convert SSH server (src/proxy/ssh/server.js -> server.ts) - Convert SSH CLI tool (src/cli/ssh-key.js -> ssh-key.ts) - Add proper TypeScript types and interfaces - Install @types/ssh2 for SSH2 library types - Fix TypeScript compilation errors with type assertions - Update imports to use TypeScript files - Remove @ts-expect-error comment as no longer needed

- Add email and gitAccount fields to SSHUser and AuthenticatedUser interfaces - Improve client connection handling by logging client IP and user details - Refactor handleClient method to accept client connection info - Enhance error handling and logging for better debugging - Update tests to reflect changes in client handling and authentication

- Update keepalive settings to recommended intervals for better connection stability - Implement cleanup of keepalive timers on client disconnects - Modify error handling to allow client recovery instead of closing connections - Improve logging for debugging client key usage and connection errors - Update tests to reflect changes in keepalive behavior and error handling

- Introduce SSH key management to securely store and reuse user SSH keys during the approval process - Add SSHKeyManager and SSHAgent classes for key encryption, storage, and expiration management - Implement captureSSHKey processor to capture and store SSH key information during push actions - Enhance Action and request handling to support SSH-specific user data - Update push action chain to include SSH key capture - Extend PushData model to include encrypted SSH key and expiration details - Provide configuration options for SSH key encryption and management

- Introduce .nvmrc file to specify Node.js version (v20) - Add SSH interface definitions for configuration of SSH proxy server and host keys - Update config generation to include SSH settings - Modify SSH server command handling to improve error reporting and session management - Enhance tests for SSH key capture and server functionality, ensuring robust error handling and edge case coverage

- Add .claude/ to .gitignore to prevent tracking of Claude-related files

…handling in SSH server - Update SSH configuration merging to guarantee 'enabled' is always a boolean value. - Enhance error handling in SSH server to provide clearer error messages when chain execution fails.

Fixes SSH push operations by capturing pack data before executing the security chain. Previously SSH pushes failed because pack data was streamed directly without capture, causing parsePush processor to fail with null body. Changes: - Split push/pull operation handling with proper timing - Capture pack data from SSH streams for push operations - Execute security chain after pack data is available for pushes - Execute security chain before streaming for pulls - Add comprehensive error handling and timeout protection - Forward captured pack data to remote after security approval - Add size limits (500MB) and corruption detection Security: All existing security features now work for SSH pushes including gitleaks scanning, diff analysis, and approval workflows. Test coverage: 91.74% line coverage with comprehensive unit and integration tests covering pack capture, error scenarios, and end-to-end workflows.

Prevents the accidental committing of SSH keys generated during tests.

- Updated the test to use forwardPackDataToRemote for handling git-receive-pack commands. - Added async handling for stream events to ensure proper execution flow. - Skipped the pack data corruption detection test to prevent false positives. - Improved assertions for error messages related to access denial and remote forwarding failures. These changes improve the robustness and reliability of the SSHServer tests.

Added support for maximum pack size limits in proxy configuration, allowing for better control over git operations. Introduced new SSH clone configuration options, including service token credentials for cloning repositories. Updated configuration types to include limits and SSH clone settings. Enhanced the handling of SSH keys during push operations, ensuring proper encryption and management of user keys. Improved error handling and logging for SSH operations, providing clearer feedback during failures. These changes improve the flexibility and security of git operations within the proxy server.

…ocols

dgl · 2025-10-24T04:01:14Z

src/security/SSHKeyManager.ts

+    expiryTime: Date;
+  } {
+    const keyBuffer = Buffer.isBuffer(privateKey) ? privateKey : Buffer.from(privateKey);
+    const encryptionKey = this.getEncryptionKey();


getEncryptionKey says it is for development use (if the env variable isn't set), but I don't see a code path calling generateEncryptionKey which says it is for production. I'd also consider renaming the methods to be more clear that one is development and production, rather than get vs generate which isn't that clear...

dgl · 2025-10-24T04:16:27Z

src/proxy/processors/push-action/pullRemote.ts

+const buildSSHCloneUrl = (remoteUrl: string): string => {
+  const parsed = new URL(remoteUrl);
+  const repoPath = parsed.pathname.replace(/^\//, '');
+  return `git@${parsed.hostname}:${repoPath}`;


I would point out in the docs that the username is currently hardcoded to git and/or add a todo (this works fine for github and gitlab I think, but may not work on other services).

dgl · 2025-10-24T04:21:05Z

src/proxy/processors/push-action/pullRemote.ts

+  await fs.promises.writeFile(keyPath, keyBuffer, { mode: 0o600 });
+
+  const originalGitSSH = process.env.GIT_SSH_COMMAND;
+  process.env.GIT_SSH_COMMAND = `ssh -i ${keyPath} -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null`;


I'd point out in the docs that the SSH TOFU check is skipped. Better would be generating a known hosts file somehow (that way the trust in the TLS certificate is used to trust the SSH keys):

For github in curl + jq:

curl -sL -H "Accept: application/vnd.github+json" \ -H "X-GitHub-Api-Version: 2022-11-28" \ https://api.github.com/meta | jq -r '.ssh_keys[]'

(From https://docs.github.com/en/rest/meta/meta?apiVersion=2022-11-28#get-github-meta-information)

Could maybe work to have a workflow that generates that as part of a release and ships it (with say github.com and gitlab.com at least).

jescalada · 2025-11-03T16:31:07Z

@pGrovesy @grovesy We'd really appreciate a look on this PR! 🙏🏼 #1141 will make reviewing the functionality a lot easier.

jescalada

Just a partial review of the code (Part 1/3). Will go through the rest of the code and test the flow once merge conflicts are resolved!

jescalada · 2025-11-05T12:20:25Z

docs/SSH_KEY_RETENTION.md

@@ -0,0 +1,199 @@
+# SSH Key Retention for Git Proxy


Nit: Replace all instances of "Git Proxy" with "GitProxy" without a space for consistency 👍🏼

jescalada · 2025-11-05T12:25:48Z

docs/SSH_KEY_RETENTION.md

+
+### SSH Configuration
+
+Enable SSH support in `proxy.config.json`:


Wonder if we could make this setup section a bit more "visible"? It'd be great if there was a more user-friendly guide on how to set up the SSH configuration (although this might have been done in the SSH UI PR?).

However, I think this would be best put in a separate file, such as SETTING_UP_SSH.md to make it really obvious for busy developers and AI tools scaninng through the docs 😃

jescalada · 2025-11-05T12:37:35Z

docs/SSH_KEY_RETENTION.md

+- Temporary files are created with restrictive permissions (0600)
+- All temporary files are automatically cleaned up
+
+## API Usage


Is there a reason to have this as part of the docs? Perhaps for using the SSH API in custom actions/plugins? Otherwise I feel like we need to add a good explanation here so it's obvious that this doc is for explaining the SSH rationale/implementation rather than the usage.

jescalada · 2025-11-05T12:46:27Z

docs/SSH_KEY_RETENTION.md

+4. **Key Rotation**: Automatic key rotation based on push frequency
+5. **Integration**: Integration with external SSH key management systems
+
+## Troubleshooting


Might be good to expand on these and provide more details! I'll add some suggestions for it when going through the setup process again.

jescalada · 2025-11-05T12:49:00Z

packages/git-proxy-cli/index.js

        case 401:
-          errorMessage =
-            'Error: Authorise: Authentication required (401): ' + error?.response?.data?.message;
+          errorMessage = 'Error: Authorise: Authentication required';


Any specific reason to remove the error response content? I feel like keeping that would be good especially for the 401 errors which can happen due to many different reasons (some of which are easier than others for the user to fix).

jescalada · 2025-11-05T15:05:13Z

src/config/index.ts

+export const getMaxPackSizeBytes = (): number => {
+  const config = loadFullConfiguration();
+  const configuredValue = config.limits?.maxPackSizeBytes;
+  const fallback = 1024 * 1024 * 1024; // 1 GiB default


What are the implications of having a 1 GB default max pack size? Would this somehow prevent larger operations from going through SSH?

jescalada · 2025-11-05T15:11:03Z

src/config/index.ts

+  const fallback = 1024 * 1024 * 1024; // 1 GiB default
+
+  if (
+    typeof configuredValue === 'number' &&


This validation should happen automatically if maxPackSizeBytes is added to the config.schema.json (and then we import the generated types from src/config/generated/config.ts. We can still use fallbacks in the flow by omitting the "required": [] section in the schema entry.

jescalada · 2025-11-05T15:15:13Z

src/config/index.ts

+  }
+};
+
+export const getSSHProxyUrl = (): string | undefined => {


Is this function being used? 🤔

jescalada · 2025-11-05T15:19:53Z

src/db/file/users.ts

+  });
+};
+
+export const findUserBySSHKey = (sshKey: string): Promise<User | null> => {


Wondering how this handles searching with duplicates - what would happen if for some reason two users shared the same public key (perhaps if two users were logging in from the same device)?

jescalada · 2025-11-05T15:21:02Z

src/db/mongo/users.ts

+  );
+};
+
+export const findUserBySSHKey = async function (sshKey: string): Promise<User | null> {


Same comment as above, does this correctly handle two users with the same key, or other duplicate key scenarios?

jescalada

Code review (Part 2/3). Looking good so far, just a few spots that I found possible ways to improve the implementation! 😃

jescalada · 2025-11-06T04:24:55Z

src/proxy/processors/pre-processor/parseAction.ts

  originalUrl: string;
  method: string;
  headers: Record<string, string>;
+  protocol?: 'https' | 'ssh';


There might be a way to refactor this to use Action instead, since most of the attributes are shared.

jescalada · 2025-11-06T04:29:03Z

src/proxy/processors/push-action/captureSSHKey.ts

+      return action;
+    }
+
+    const privateKeyBuffer = Buffer.isBuffer(privateKeySource)


It seems these are executing Buffer.from(privateKeySource) and such, regardless of whether it's a buffer or not

jescalada · 2025-11-06T04:34:24Z

src/proxy/processors/push-action/captureSSHKey.ts

+    step.log('SSH key information stored for approval process');
+    step.setContent(`SSH key retained until ${encrypted.expiryTime.toISOString()}`);
+
+    privateKeyBuffer.fill(0);


Should we be clearing the publicKeyBuffer memory as well?

jescalada · 2025-11-06T04:37:58Z

src/proxy/processors/push-action/captureSSHKey.ts

+    );
+
+    if (!addedToAgent) {
+      console.warn(


Wonder if it's fine to continue execution here if SSH key caching fails 🤔

jescalada · 2025-11-06T04:43:35Z

src/proxy/processors/push-action/captureSSHKey.ts

+
+    const authContext = req?.authContext ?? {};
+    const sshKeyContext = authContext?.sshKey;
+    const privateKeySource =


Is is necessary to have multiple different sources for the private key? If necessary, it'd be nice to have this fallback chain documented somewhere so it's easier for users to configure.

jescalada · 2025-11-06T07:43:15Z

src/service/SSHKeyForwardingService.ts

+   * @param {any} push The push object
+   * @return {Promise<boolean>} True if successful
+   */
+  private static async executeSSHPushWithProxyKey(push: any): Promise<boolean> {


Same as above, would be great to enforce proper types for push.

jescalada · 2025-11-06T07:43:50Z

src/service/SSHKeyForwardingService.ts

+   * @param {any} push The push object
+   * @return {Promise<boolean>} True if successful
+   */
+  private static async executeHTTPSPush(push: any): Promise<boolean> {


Same as above

jescalada · 2025-11-06T07:46:56Z

src/service/SSHKeyForwardingService.ts

Would be great to define specific errors and throw them from each file/operation. Such as SSHKeyForwardingError, etc.

jescalada · 2025-11-06T07:52:35Z

src/service/urls.js

-    );
+    const protocol = normaliseProtocol(req?.protocol);
+    const host = resolveHost(req);
+    const defaultURL = `${protocol}://${host}`.replace(`:${UI_PORT}`, `:${PROXY_HTTP_PORT}`);


Might want to use a port replacement function rather than just replacing the first occurrence of the port:

https://example.com:3000/path:3000.replace(:3000, :8000)
Result: "https://example.com:8000/path:3000"

const replacePort = (url, oldPort, newPort) => { try { const parsed = new URL(url); if (parsed.port === String(oldPort)) { parsed.port = String(newPort); } return parsed.toString(); } catch (error) { console.error('Failed to parse URL:', error); return url; } };

jescalada · 2025-11-06T07:53:35Z

src/service/urls.ts

Might want to convert this to TS! 😃

…git-proxy; branch 'main' of https://github.com/finos/git-proxy into denis-coric/ssh-flow

github-actions bot added the feature label Apr 28, 2025

This was referenced Apr 28, 2025

Support SSH instead of http only G-Research/git-proxy#25

Open

Security: Api endpoints should not expose user credentials G-Research/git-proxy#4

Open

dcoric linked an issue May 7, 2025 that may be closed by this pull request

Security: Api endpoints should not expose user credentials G-Research/git-proxy#4

Open

dcoric force-pushed the denis-coric/ssh-flow branch from 83a7496 to 399bf6c Compare May 9, 2025 13:15

dcoric force-pushed the denis-coric/ssh-flow branch from 399bf6c to f514691 Compare May 16, 2025 11:04

fabiovincenzi mentioned this pull request May 28, 2025

feat: SSH UI #1033

Closed

jescalada requested changes May 31, 2025

View reviewed changes

jescalada reviewed Jun 2, 2025

View reviewed changes

dcoric linked an issue Jun 6, 2025 that may be closed by this pull request

[Migration] Convert ssh/server to TS G-Research/git-proxy#47

Open

dcoric force-pushed the denis-coric/ssh-flow branch from f514691 to 6667d15 Compare June 16, 2025 11:20

coopernetes mentioned this pull request Jun 25, 2025

Repositories should be identified and retrieved via URL rather than name #950

Closed

fabiovincenzi mentioned this pull request Aug 4, 2025

feat: SSH UI #1141

Closed

dcoric added 7 commits September 12, 2025 15:56

Merge branch 'main' into denis-coric/ssh

af69d45

fix: enhance SSH server tests and client handling

8df000a

feat: add findUserBySSHKey function to user database operations

719103a

dcoric force-pushed the denis-coric/ssh-flow branch from 6667d15 to 2fd1703 Compare September 17, 2025 11:06

dcoric added 2 commits September 17, 2025 13:55

dcoric force-pushed the denis-coric/ssh-flow branch from 5d680a6 to 91b58eb Compare September 19, 2025 14:22

dcoric added 3 commits September 19, 2025 16:23

chore: update .gitignore to exclude Claude directory

b2e7557

- Add .claude/ to .gitignore to prevent tracking of Claude-related files

fix: ensure SSH enabled configuration is a boolean and improve error …

7e3553c

…handling in SSH server - Update SSH configuration merging to guarantee 'enabled' is always a boolean value. - Enhance error handling in SSH server to provide clearer error messages when chain execution fails.

Merge remote-tracking branch 'finos/main' into denis-coric/ssh-flow

2d56a76

dcoric added 10 commits September 25, 2025 14:05

fix: fixes lint and refreshed package-lock.json

61e6a0b

Merge remote-tracking branch 'finos/main' into denis-coric/ssh-flow

27b190b

fix: adds test SSH keys to .gitignore

6192ee9

Prevents the accidental committing of SSH keys generated during tests.

feat: add comprehensive performance tests for HTTP/HTTPS and SSH prot…

2cc7553

…ocols

Merge branch 'main' into denis-coric/ssh-flow

107bac1

Merge remote-tracking branch 'finos/main' into denis-coric/ssh-flow

8698ad1

refactor: rename variables in performance tests for clarity

cd47fb8

dgl reviewed Oct 24, 2025

View reviewed changes

kriswest mentioned this pull request Nov 3, 2025

3rd November 2025 - GitProxy Meeting Minutes #1259

Open

17 tasks

jescalada requested changes Nov 5, 2025

View reviewed changes

jescalada requested changes Nov 6, 2025

View reviewed changes

This was referenced Nov 6, 2025

feat: SSH UI G-Research/git-proxy#63

Open

feat: SSH UI #1276

Open

jescalada added 2 commits November 8, 2025 17:41

Merge branch 'denis-coric/ssh-flow' of https://github.com/G-Research/…

b54952d

…git-proxy; branch 'main' of https://github.com/finos/git-proxy into denis-coric/ssh-flow

test: fix flaky ssh performance test

b8ba792

	exports.updateUser(user).then(resolve).catch(reject);
	updateUser(user).then(() => resolve(user)).catch(reject);

-    describe('authentication', () => {
+    describe('authentication', () => {
+      it('should reject invalid public keys', async () => {
+        const mockCtx = {
+          method: 'publickey',
+          key: {
+            algo: 'ssh-rsa',
+            data: Buffer.from('invalid-key-data'),
+            comment: 'invalid-key',
+          },
+          accept: sinon.stub(),
+          reject: sinon.stub(),
+        };
+        mockDb.findUserBySSHKey.resolves(null);
+        server.handleClient(mockClient);
+        const authHandler = mockClient.on.withArgs('authentication').firstCall.args[1];
+        await authHandler(mockCtx);
+        expect(mockCtx.reject.calledOnce).to.be.true;
+        expect(mockClient.username).to.be.null;
+        expect(mockClient.userPrivateKey).to.be.null;
+      });
+      it('should reject invalid passwords', async () => {
+        const mockCtx = {
+          method: 'password',
+          username: 'test-user',
+          password: 'invalid-password',
+          accept: sinon.stub(),
+          reject: sinon.stub(),
+        };
+        mockDb.findUser.resolves({
+          username: 'test-user',
+          password: '$2a$10$mockHash',
+        });
+        const bcrypt = require('bcryptjs');
+        sinon.stub(bcrypt, 'compare').resolves(false);
+        server.handleClient(mockClient);
+        const authHandler = mockClient.on.withArgs('authentication').firstCall.args[1];
+        await authHandler(mockCtx);
+        expect(mockDb.findUser.calledWith('test-user')).to.be.true;
+        expect(bcrypt.compare.calledWith('invalid-password', '$2a$10$mockHash')).to.be.true;
+        expect(mockCtx.reject.calledOnce).to.be.true;
+      });
+      it('should handle a missing private key', async () => {
+        const mockCtx = {
+          method: 'publickey',
+          key: null,
+          accept: sinon.stub(),
+          reject: sinon.stub(),
+        };
+        mockDb.findUserBySSHKey.resolves({ username: 'test-user' });
+        server.handleClient(mockClient);
+        const authHandler = mockClient.on.withArgs('authentication').firstCall.args[1];
+        await authHandler(mockCtx);
+        expect(mockCtx.reject.calledOnce).to.be.true;
+        expect(mockClient.username).to.be.null;
+        expect(mockClient.userPrivateKey).to.be.null;
+      });


		### SSH Configuration

		Enable SSH support in `proxy.config.json`:

feat: adds ssh support for git operations #987

Are you sure you want to change the base?

feat: adds ssh support for git operations #987

Conversation

dcoric commented Apr 28, 2025

Changes

Configuration

Usage

Testing

Documentation

Future Improvements

SSH Feature Implementation:

Database Enhancements:

Configuration and Code Updates:

Documentation Enhancements:

Uh oh!

netlify bot commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Uh oh!

dcoric commented May 9, 2025

Uh oh!

jescalada left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jescalada left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

git-proxy backend

User-facing CLI

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jescalada left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Sample

Error:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

netlify bot commented Apr 28, 2025 •

edited

Loading

jescalada commented Nov 3, 2025 •

edited

Loading