Skip to content
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
51a4a35
refactor(ssh): add PktLineParser and base function to eliminate code …
fabiovincenzi Nov 20, 2025
f6fb9eb
feat(ssh): implement server-side SSH agent forwarding with LazyAgent …
fabiovincenzi Nov 20, 2025
61b3595
feat(ssh): add SSH helper functions for connection setup and validation
fabiovincenzi Nov 20, 2025
3e0e5c0
refactor(ssh): simplify server.ts and pullRemote using helper functions
fabiovincenzi Nov 20, 2025
4a2b273
docs: add SSH proxy architecture documentation
fabiovincenzi Nov 20, 2025
0f3d3b8
fix(ssh): correct ClientWithUser to extend ssh2.Connection instead of…
fabiovincenzi Nov 20, 2025
39be87e
feat: add dependencies for SSH key management
fabiovincenzi Oct 24, 2025
dbef641
feat(db): add PublicKeyRecord type for SSH key management
fabiovincenzi Nov 6, 2025
9545ac2
feat(db): implement SSH key management for File database
fabiovincenzi Nov 6, 2025
24d499c
feat(db): implement SSH key management for MongoDB
fabiovincenzi Nov 6, 2025
df603ef
feat(db): update database wrapper with correct SSH key types
fabiovincenzi Nov 6, 2025
7e5d6d9
feat(api): add SSH key management endpoints
fabiovincenzi Nov 6, 2025
59aef6e
feat(ui): add SSH service for API calls
fabiovincenzi Nov 6, 2025
ebfff2d
feat(ui): add SSH key management UI and clone tabs
fabiovincenzi Nov 6, 2025
0570c4c
feat(cli): update SSH key deletion to use fingerprint
fabiovincenzi Nov 6, 2025
e5da79c
chore: add SSH key fingerprint API and UI updates
fabiovincenzi Nov 20, 2025
ab0bdbe
refactor(ssh): remove explicit SSH algorithm configuration
fabiovincenzi Nov 26, 2025
b72d222
fix(ssh): use existing packet line parser
fabiovincenzi Nov 26, 2025
55d06ab
feat(ssh): improve agent forwarding error message and make it configu…
fabiovincenzi Nov 26, 2025
f6281d6
fix(ssh): use startsWith instead of includes for git-receive-pack det…
fabiovincenzi Nov 26, 2025
5e3e13e
feat(ssh): add SSH host key verification to prevent MitM attacks
fabiovincenzi Nov 26, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
351 changes: 351 additions & 0 deletions docs/SSH_ARCHITECTURE.md

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
"dependencies": {
"@material-ui/core": "^4.12.4",
"@material-ui/icons": "4.11.3",
"@material-ui/lab": "^4.0.0-alpha.61",
"@primer/octicons-react": "^19.19.0",
"@seald-io/nedb": "^4.1.2",
"axios": "^1.12.2",
Expand All @@ -90,6 +91,7 @@
"concurrently": "^9.2.1",
"connect-mongo": "^5.1.0",
"cors": "^2.8.5",
"dayjs": "^1.11.13",
"diff2html": "^3.4.52",
"env-paths": "^3.0.0",
"escape-string-regexp": "^5.0.0",
Expand Down Expand Up @@ -119,6 +121,7 @@
"react-router-dom": "6.30.1",
"simple-git": "^3.28.0",
"ssh2": "^1.16.0",
"sshpk": "^1.18.0",
"uuid": "^11.1.0",
"validator": "^13.15.15",
"yargs": "^17.7.2"
Expand Down
48 changes: 40 additions & 8 deletions src/cli/ssh-key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import * as fs from 'fs';
import * as path from 'path';
import axios from 'axios';
import { utils } from 'ssh2';
import * as crypto from 'crypto';

const API_BASE_URL = process.env.GIT_PROXY_API_URL || 'http://localhost:3000';
const GIT_PROXY_COOKIE_FILE = path.join(
Expand All @@ -23,6 +25,23 @@ interface ErrorWithResponse {
message: string;
}

// Calculate SHA-256 fingerprint from SSH public key
// Note: This function is duplicated in src/service/routes/users.js to keep CLI and server independent
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need to keep the CLI and server independent? Since the CLI is already importing a few things from the parent package.

Perhaps we could extract this function to src/service/routes/utils.ts for better testing and dealing with potential bugs. 🤔

function calculateFingerprint(publicKeyStr: string): string | null {
try {
const parsed = utils.parseKey(publicKeyStr);
if (!parsed || parsed instanceof Error) {
return null;
}
const pubKey = parsed.getPublicSSH();
const hash = crypto.createHash('sha256').update(pubKey).digest('base64');
return `SHA256:${hash}`;
} catch (err) {
console.error('Error calculating fingerprint:', err);
return null;
}
}

async function addSSHKey(username: string, keyPath: string): Promise<void> {
try {
// Check for authentication
Expand Down Expand Up @@ -83,15 +102,28 @@ async function removeSSHKey(username: string, keyPath: string): Promise<void> {
// Read the public key file
const publicKey = fs.readFileSync(keyPath, 'utf8').trim();

// Make the API request
await axios.delete(`${API_BASE_URL}/api/v1/user/${username}/ssh-keys`, {
data: { publicKey },
withCredentials: true,
headers: {
'Content-Type': 'application/json',
Cookie: cookies,
// Strip the comment from the key (everything after the last space)
const keyWithoutComment = publicKey.split(' ').slice(0, 2).join(' ');

// Calculate fingerprint
const fingerprint = calculateFingerprint(keyWithoutComment);
if (!fingerprint) {
console.error('Invalid SSH key format. Unable to calculate fingerprint.');
process.exit(1);
}

console.log(`Removing SSH key with fingerprint: ${fingerprint}`);

// Make the API request using fingerprint in path
await axios.delete(
`${API_BASE_URL}/api/v1/user/${username}/ssh-keys/${encodeURIComponent(fingerprint)}`,
{
withCredentials: true,
headers: {
Cookie: cookies,
},
},
});
);

console.log('SSH key removed successfully!');
} catch (error) {
Expand Down
1 change: 1 addition & 0 deletions src/db/file/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@ export const {
updateUser,
addPublicKey,
removePublicKey,
getPublicKeys,
} = users;
41 changes: 29 additions & 12 deletions src/db/file/users.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import fs from 'fs';
import Datastore from '@seald-io/nedb';

import { User, UserQuery } from '../types';
import { User, UserQuery, PublicKeyRecord } from '../types';
import { DuplicateSSHKeyError, UserNotFoundError } from '../../errors/DatabaseErrors';

const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day
Expand Down Expand Up @@ -181,7 +181,7 @@ export const getUsers = (query: Partial<UserQuery> = {}): Promise<User[]> => {
});
};

export const addPublicKey = (username: string, publicKey: string): Promise<void> => {
export const addPublicKey = (username: string, publicKey: PublicKeyRecord): Promise<void> => {
return new Promise((resolve, reject) => {
// Check if this key already exists for any user
findUserBySSHKey(publicKey)
Expand All @@ -202,20 +202,28 @@ export const addPublicKey = (username: string, publicKey: string): Promise<void>
if (!user.publicKeys) {
user.publicKeys = [];
}
if (!user.publicKeys.includes(publicKey)) {
user.publicKeys.push(publicKey);
updateUser(user)
.then(() => resolve())
.catch(reject);
} else {
resolve();

// Check if key already exists (by key content or fingerprint)
const keyExists = user.publicKeys.some(
(k) =>
k.key === publicKey.key || (k.fingerprint && k.fingerprint === publicKey.fingerprint),
);

if (keyExists) {
reject(new Error('SSH key already exists'));
return;
}

user.publicKeys.push(publicKey);
updateUser(user)
.then(() => resolve())
.catch(reject);
})
.catch(reject);
});
};

export const removePublicKey = (username: string, publicKey: string): Promise<void> => {
export const removePublicKey = (username: string, fingerprint: string): Promise<void> => {
return new Promise((resolve, reject) => {
findUser(username)
.then((user) => {
Expand All @@ -228,7 +236,7 @@ export const removePublicKey = (username: string, publicKey: string): Promise<vo
resolve();
return;
}
user.publicKeys = user.publicKeys.filter((key) => key !== publicKey);
user.publicKeys = user.publicKeys.filter((k) => k.fingerprint !== fingerprint);
updateUser(user)
.then(() => resolve())
.catch(reject);
Expand All @@ -239,7 +247,7 @@ export const removePublicKey = (username: string, publicKey: string): Promise<vo

export const findUserBySSHKey = (sshKey: string): Promise<User | null> => {
return new Promise<User | null>((resolve, reject) => {
db.findOne({ publicKeys: sshKey }, (err: Error | null, doc: User) => {
db.findOne({ 'publicKeys.key': sshKey }, (err: Error | null, doc: User) => {
// ignore for code coverage as neDB rarely returns errors even for an invalid query
/* istanbul ignore if */
if (err) {
Expand All @@ -254,3 +262,12 @@ export const findUserBySSHKey = (sshKey: string): Promise<User | null> => {
});
});
};

export const getPublicKeys = (username: string): Promise<PublicKeyRecord[]> => {
return findUser(username).then((user) => {
if (!user) {
throw new Error('User not found');
}
return user.publicKeys || [];
});
};
14 changes: 8 additions & 6 deletions src/db/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { AuthorisedRepo } from '../config/generated/config';
import { PushQuery, Repo, RepoQuery, Sink, User, UserQuery } from './types';
import { PushQuery, Repo, RepoQuery, Sink, User, UserQuery, PublicKeyRecord } from './types';
import * as bcrypt from 'bcryptjs';
import * as config from '../config';
import * as mongo from './mongo';
Expand Down Expand Up @@ -171,9 +171,11 @@ export const findUserBySSHKey = (sshKey: string): Promise<User | null> =>
sink.findUserBySSHKey(sshKey);
export const getUsers = (query?: Partial<UserQuery>): Promise<User[]> => sink.getUsers(query);
export const deleteUser = (username: string): Promise<void> => sink.deleteUser(username);
export const updateUser = (user: Partial<User>): Promise<void> => sink.updateUser(user);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the Partial here being removed? I think this was changed at some point to allow proper typing for activeDirectory.ts and some test files.

We could alternatively fix the usages, but it makes the most sense for a DB update function (triggered through a PATCH endpoint) to take a partial version of the related entity.

export const addPublicKey = (username: string, publicKey: string): Promise<void> =>
export const updateUser = (user: User): Promise<void> => sink.updateUser(user);
export const addPublicKey = (username: string, publicKey: PublicKeyRecord): Promise<void> =>
sink.addPublicKey(username, publicKey);
export const removePublicKey = (username: string, publicKey: string): Promise<void> =>
sink.removePublicKey(username, publicKey);
export type { PushQuery, Repo, Sink, User } from './types';
export const removePublicKey = (username: string, fingerprint: string): Promise<void> =>
sink.removePublicKey(username, fingerprint);
export const getPublicKeys = (username: string): Promise<PublicKeyRecord[]> =>
sink.getPublicKeys(username);
export type { PushQuery, Repo, Sink, User, PublicKeyRecord } from './types';
1 change: 1 addition & 0 deletions src/db/mongo/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@ export const {
updateUser,
addPublicKey,
removePublicKey,
getPublicKeys,
} = users;
37 changes: 30 additions & 7 deletions src/db/mongo/users.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { OptionalId, Document, ObjectId } from 'mongodb';
import { toClass } from '../helper';
import { User } from '../types';
import { User, PublicKeyRecord } from '../types';
import { connect } from './helper';
import _ from 'lodash';
import { DuplicateSSHKeyError } from '../../errors/DatabaseErrors';
Expand Down Expand Up @@ -71,32 +71,55 @@ export const updateUser = async (user: Partial<User>): Promise<void> => {
await collection.updateOne(filter, { $set: userWithoutId }, options);
};

export const addPublicKey = async (username: string, publicKey: string): Promise<void> => {
export const addPublicKey = async (username: string, publicKey: PublicKeyRecord): Promise<void> => {
// Check if this key already exists for any user
const existingUser = await findUserBySSHKey(publicKey);
const existingUser = await findUserBySSHKey(publicKey.key);

if (existingUser && existingUser.username.toLowerCase() !== username.toLowerCase()) {
throw new DuplicateSSHKeyError(existingUser.username);
}

// Key doesn't exist for other users
const collection = await connect(collectionName);

const user = await collection.findOne({ username: username.toLowerCase() });
if (!user) {
throw new Error('User not found');
}

const keyExists = user.publicKeys?.some(
(k: PublicKeyRecord) =>
k.key === publicKey.key || (k.fingerprint && k.fingerprint === publicKey.fingerprint),
);

if (keyExists) {
throw new Error('SSH key already exists');
}

await collection.updateOne(
{ username: username.toLowerCase() },
{ $addToSet: { publicKeys: publicKey } },
{ $push: { publicKeys: publicKey } },
);
};

export const removePublicKey = async (username: string, publicKey: string): Promise<void> => {
export const removePublicKey = async (username: string, fingerprint: string): Promise<void> => {
const collection = await connect(collectionName);
await collection.updateOne(
{ username: username.toLowerCase() },
{ $pull: { publicKeys: publicKey } },
{ $pull: { publicKeys: { fingerprint: fingerprint } } },
);
};

export const findUserBySSHKey = async function (sshKey: string): Promise<User | null> {
const collection = await connect(collectionName);
const doc = await collection.findOne({ publicKeys: { $eq: sshKey } });
const doc = await collection.findOne({ 'publicKeys.key': { $eq: sshKey } });
return doc ? toClass(doc, User.prototype) : null;
};

export const getPublicKeys = async (username: string): Promise<PublicKeyRecord[]> => {
const user = await findUser(username);
if (!user) {
throw new Error('User not found');
}
return user.publicKeys || [];
};
18 changes: 13 additions & 5 deletions src/db/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ export type QueryValue = string | boolean | number | undefined;

export type UserRole = 'canPush' | 'canAuthorise';

export type PublicKeyRecord = {
key: string;
name: string;
addedAt: string;
fingerprint: string;
};

export class Repo {
project: string;
name: string;
Expand Down Expand Up @@ -58,7 +65,7 @@ export class User {
email: string;
admin: boolean;
oidcId?: string | null;
publicKeys?: string[];
publicKeys?: PublicKeyRecord[];
displayName?: string | null;
title?: string | null;
_id?: string;
Expand All @@ -70,7 +77,7 @@ export class User {
email: string,
admin: boolean,
oidcId: string | null = null,
publicKeys: string[] = [],
publicKeys: PublicKeyRecord[] = [],
_id?: string,
) {
this.username = username;
Expand Down Expand Up @@ -110,7 +117,8 @@ export interface Sink {
getUsers: (query?: Partial<UserQuery>) => Promise<User[]>;
createUser: (user: User) => Promise<void>;
deleteUser: (username: string) => Promise<void>;
updateUser: (user: Partial<User>) => Promise<void>;
addPublicKey: (username: string, publicKey: string) => Promise<void>;
removePublicKey: (username: string, publicKey: string) => Promise<void>;
updateUser: (user: User) => Promise<void>;
addPublicKey: (username: string, publicKey: PublicKeyRecord) => Promise<void>;
removePublicKey: (username: string, fingerprint: string) => Promise<void>;
getPublicKeys: (username: string) => Promise<PublicKeyRecord[]>;
}
Loading
Loading