Skip to content

feat(ssh): Add SSH agent forwarding#1332

Merged
jescalada merged 186 commits into
finos:mainfrom
fabiovincenzi:ssh-agent-on-pr987
Jun 10, 2026
Merged

feat(ssh): Add SSH agent forwarding#1332
jescalada merged 186 commits into
finos:mainfrom
fabiovincenzi:ssh-agent-on-pr987

Conversation

@fabiovincenzi

@fabiovincenzi fabiovincenzi commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

Summary

Adds complete SSH protocol support to Git Proxy with SSH agent forwarding.

Key Features

  • SSH Proxy Server (src/proxy/ssh/server.ts) - Full SSH protocol handling
  • SSH Agent Forwarding (src/proxy/ssh/AgentForwarding.ts, AgentProxy.ts) - Securely forwards client SSH credentials to upstream Git servers
  • SSH Key Management - API endpoints, UI components, and CLI tools for user SSH key registration
  • Git Protocol Handling (src/proxy/ssh/GitProtocol.ts) - Parses and processes git-upload-pack/git-receive-pack over SSH

Documentation

Tests

Comprehensive test coverage in test/ssh/:

dcoric and others added 30 commits September 12, 2025 15:56
- 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
- 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.
@jescalada

Copy link
Copy Markdown
Contributor

I found a few bugs worth noting while testing today, most notably:

Concurrent SSH requests failing due to nonexistent channels:

This error happens when running concurrent git commands through SSH (client-side output):

channel_by_id: 2: bad id: channel free
Disconnecting 10.32.1.86 port 2222: data packet referred to nonexistent channel 2
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

I just pushed a fix for this one which was simply due to the incoming and outgoing channel IDs being flipped. It worked with single requests because both use the same channel ID (1) for packet transport.

An error when calling git push when there aren't any commits to push:

juan@pop-os:~/git-proxy-experiments$ git push
Access denied: Unable to parse push. Please contact an administrator for support: Your push has been blocked. Please make sure you are pushing to a single branch.
error: failed to push some refs to 'ssh://10.32.1.86:2222/github.com/jescalada/git-proxy-experiments.git'

When pushing without SSH, this just gives us the expected "Everything up-to-date". I'll look into this further and find a fix 👍🏼

An error after fixing merge conflicts and pushing a merge commit. The first "approval" push was apparently skipped, so there wasn't any push for an admin to approve and break the loop:

juan@pop-os:~/git-proxy-experiments$ git push
Enumerating objects: 10, done.
Counting objects: 100% (10/10), done.
Delta compression using up to 14 threads
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 690 bytes | 690.00 KiB/s, done.
Total 6 (delta 2), reused 0 (delta 0), pack-reused 0
Access denied: Unreferenced commits in pack (2): e9260e6c519ac964e895ec69d926f844638c9c60, 9ed3a288070f3a7f58494d1a50c99435cff1757c.
This usually happens when a branch was made from a commit that hasn't been approved and pushed to the remote.
Please get approval on the commits, push them and try again.
send-pack: unexpected disconnect while reading sideband packet
fatal: the remote end hung up unexpectedly

I'll try reproducing this one again, as it may be due to a buggy push action from before our rcs. Note that the buggy state can be resolved by simply rebasing instead of dealing with merge commits...

fabiovincenzi and others added 4 commits May 29, 2026 14:52
Fixes issue with pushes being blocked on "git push" via SSH when branch was already up-to-date. HTTPS does this automatically when calling GET /info/refs
@jescalada

Copy link
Copy Markdown
Contributor

Just pushed a fix for the error when calling git push on a branch that's up-to-date. Basically, HTTPS handles no-op pushes automatically (via GET /info/refs) and never actually triggers the chain, whereas SSH was executing the chain and erroring out in parsePush.

I fixed it by modifying the endHandler in src/proxy/ssh/server.ts to actually parse the packet lines beforehand, detect and skip no-op pushes and keeping the data integrity and chain execution bits intact.

@kriswest

kriswest commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

@re-vlad

@andypols andypols left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Checked it out and it worked as expected.

jescalada and others added 5 commits June 4, 2026 14:27
Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>
Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>

@kriswest kriswest left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks pretty good to me, a few minor comments and one concern - the disabling of config validation. What is the validation issue with the SSH config thats causing that ? If its a JSON schema issue I may be able to help resolve.

Comment thread config.schema.json
Comment thread config.schema.json Outdated
Comment thread src/config/generated/config.ts Outdated
Comment thread src/config/generated/config.ts Outdated
Comment thread src/config/index.ts Outdated
Comment thread src/proxy/ssh/GitProtocol.ts Outdated
Comment thread src/proxy/ssh/GitProtocol.ts Outdated
Comment thread src/proxy/ssh/knownHosts.ts Outdated
Comment thread src/service/urls.ts Outdated
Comment thread website/docs/ssh-setup.md
fabiovincenzi and others added 8 commits June 7, 2026 13:15
Co-authored-by: Kris West <kristopher.west@natwest.com>
Signed-off-by: Fabio Vincenzi <93596376+fabiovincenzi@users.noreply.github.com>
Co-authored-by: Kris West <kristopher.west@natwest.com>
Signed-off-by: Fabio Vincenzi <93596376+fabiovincenzi@users.noreply.github.com>
@fabiovincenzi

Copy link
Copy Markdown
Contributor Author

Thanks for the review @kriswest! I believe all your comments have been addressed. Let me know if anything else needs attention.

@kriswest kriswest left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM after comments were dealt with

@kriswest

Copy link
Copy Markdown
Contributor

@jescalada's early comments still need to be resolved to merge: #1332 (review)

@jescalada

Copy link
Copy Markdown
Contributor

Just verified that the older comments have been dealt with - ready to merge! 🚀

@jescalada jescalada merged commit 8ac32ba into finos:main Jun 10, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSH: Implement SSH agent forwarding protocol support SSH support

7 participants