diff --git a/docs/ssh-commit-signing.md b/docs/ssh-commit-signing.md new file mode 100644 index 000000000..ac9e948f7 --- /dev/null +++ b/docs/ssh-commit-signing.md @@ -0,0 +1,160 @@ +# SSH Commit Signing + +ByteRover signs commits with an SSH key. When enabled, every commit carries a +cryptographic signature and shows as **Verified** in the ByteRover UI and via +`git verify-commit`. + +## Quick start + +```bash +# 1. Generate (skip if you already have a key) +ssh-keygen -t ed25519 -C "you@example.com" -f ~/.ssh/id_ed25519_signing + +# 2. Register the public key with ByteRover +brv signing-key add --key ~/.ssh/id_ed25519_signing.pub --title "My laptop" + +# 3. Tell brv where the private key lives +brv vc config user.signingkey ~/.ssh/id_ed25519_signing + +# 4. Sign every commit automatically +brv vc config commit.sign true +``` + +From here `brv vc commit -m "..."` produces a signed commit. + +--- + +## Supported key formats + +| Key format | Direct (file) | Via ssh-agent | +| --------------------------------------------------- | :-----------: | :-----------: | +| Ed25519, OpenSSH format, **unencrypted** | ✅ | ✅ | +| Ed25519, OpenSSH format, passphrase-protected | ❌ | ✅ | +| RSA / ECDSA, any format | ❌ | ✅ | + +If your key falls into a row that only supports the ssh-agent column, load it +into the agent before signing: + +```bash +ssh-add ~/.ssh/id_rsa # macOS / Linux / WSL +``` + +On Windows PowerShell with the OpenSSH agent service running: + +```powershell +Get-Service ssh-agent | Set-Service -StartupType Automatic +Start-Service ssh-agent +ssh-add $HOME\.ssh\id_rsa +``` + +`brv vc commit --sign` automatically prefers ssh-agent when it is available +(`SSH_AUTH_SOCK` set on Unix, `\\.\pipe\openssh-ssh-agent` on Windows) — you +do not need to change any brv config. + +--- + +## Passphrase-protected keys (without ssh-agent) + +For an unencrypted Ed25519 key on disk you do not need a passphrase. For any +other passphrase-protected key, **use ssh-agent** (above) — that is the +expected path for the vast majority of users. + +The narrow exception is an Ed25519 key saved in legacy PEM/PKCS8 format +(rather than the modern OpenSSH format that `ssh-keygen` produces by +default). Keys in that format support direct passphrase entry: + +```bash +# Pass once via flag +brv vc commit -m "msg" --sign --passphrase "$MY_PASS" + +# Or via env var (preferred for CI / scripts — keeps the secret out of shell history) +BRV_SSH_PASSPHRASE="$MY_PASS" brv vc commit -m "msg" --sign +``` + +`brv` does **not** prompt interactively for the passphrase — `brv vc` is a +non-interactive oclif command. If a passphrase is required and neither +`--passphrase` nor `BRV_SSH_PASSPHRASE` is provided, the command exits with a +clear error pointing to both options. + +--- + +## Cross-platform notes + +### macOS / Linux + +Default key location: `~/.ssh/id_ed25519`. ssh-agent is started by your shell +or DE; check with `ssh-add -l`. + +### Windows (PowerShell, native OpenSSH) + +- Install OpenSSH from "Optional Features" if not present. +- Default key location: `$HOME\.ssh\id_ed25519`. +- The agent runs as a Windows service, not a per-shell process. + +### WSL + +WSL has its own ssh-agent independent of the Windows agent. Either: + +- Generate keys inside WSL and use `ssh-add` there, or +- Bridge to the Windows agent via `npiperelay` + `socat` (community guides + exist) — beyond this doc's scope. + +When pointing brv at a key, use the WSL path (`/mnt/c/...` for Windows-side +files, plain `~/...` for WSL-side). + +--- + +## Existing git SSH signing config + +If you already configured `git config gpg.format ssh` and +`git config user.signingKey ...`, brv can import directly: + +```bash +brv vc config --import-git-signing +``` + +This reads `user.signingKey` and `commit.gpgSign` from your git config and +copies them into brv's project config — no manual setup. + +--- + +## Verification + +Verify any signed commit with the standard `git verify-commit`: + +```bash +# Build an allowed_signers file once +echo "you@example.com $(cat ~/.ssh/id_ed25519_signing.pub)" > ~/.config/brv/allowed_signers + +# Verify +cd .brv/context-tree +git -c gpg.ssh.allowedSignersFile=~/.config/brv/allowed_signers verify-commit HEAD +``` + +Expected: `Good "git" signature for you@example.com with ED25519 key SHA256:...`. + +--- + +## Removing a key + +```bash +brv signing-key list # list registered keys + IDs +brv signing-key remove # remove from ByteRover +``` + +This deletes the public key from ByteRover only. The private key on disk and +any `brv vc config user.signingkey` setting are untouched. + +--- + +## Troubleshooting + +Symptom column shows the leading substring of each error — the actual +output continues with a key path or key-type detail. Match by prefix. + +| Symptom (starts with) | Likely cause | Fix | +| --- | --- | --- | +| `Error: Encrypted OpenSSH private keys are not supported for direct signing. Load the key into ssh-agent first: ssh-add …` | brv cannot decrypt OpenSSH-format encrypted keys natively. | Run the `ssh-add` command from the error message, then retry. | +| `Error: Unsupported OpenSSH key type: …` | RSA / ECDSA / non-Ed25519 OpenSSH keys are not parsed natively. | Load the key into ssh-agent (`ssh-add `). | +| `Error: Signing key requires a passphrase. Provide it via the --passphrase flag or the BRV_SSH_PASSPHRASE environment variable, then retry.` | PEM-format key needs a passphrase and none was supplied. | Pass `--passphrase` or set `BRV_SSH_PASSPHRASE`. | +| `Could not verify signature.` from `git verify-commit` | `allowed_signers` file missing or wrong fingerprint. | Re-create as shown in **Verification**. | diff --git a/package-lock.json b/package-lock.json index 340bc1518..2bbd51b48 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1587,6 +1587,7 @@ "integrity": "sha512-CGOfOJqWjg2qW/Mb6zNsDm+u5vFQ8DxXfbM09z69p5Z6+mE1ikP2jUXw+j42Pf1XTYED2Rni5f95npYeuwMDQA==", "inBundle": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.29.0", "@babel/generator": "^7.29.0", @@ -6116,6 +6117,7 @@ "integrity": "sha512-2I0gnIVPtfnMw9ee9h1dJG7tp81+8Ob3OJb3Mv37rx5L40/b0i7djjCVvGOVqc9AEIQyvyu1i6ypKdFw8R8gQw==", "inBundle": true, "license": "MIT", + "peer": true, "engines": { "node": "^14.21.3 || >=16" }, @@ -6205,6 +6207,7 @@ "resolved": "https://registry.npmjs.org/@oclif/core/-/core-4.5.4.tgz", "integrity": "sha512-78YYJls8+KG96tReyUsesKKIKqC0qbFSY1peUSrt0P2uGsrgAuU9axQ0iBQdhAlIwZDcTyaj+XXVQkz2kl/O0w==", "license": "MIT", + "peer": true, "dependencies": { "ansi-escapes": "^4.3.2", "ansis": "^3.17.0", @@ -6465,6 +6468,7 @@ "resolved": "https://registry.npmjs.org/@types/react/-/react-18.3.28.tgz", "integrity": "sha512-z9VXpC7MWrhfWipitjNdgCauoMLRdIILQsAEV+ZesIzBq/oUlxk0m3ApZuMFCXdnS4U7KrI+l3WRUEGQ8K1QKw==", "license": "MIT", + "peer": true, "dependencies": { "@types/prop-types": "*", "csstype": "^3.2.2" @@ -6654,6 +6658,7 @@ "resolved": "https://registry.npmjs.org/react/-/react-18.3.1.tgz", "integrity": "sha512-wS+hAgJShR0KhEvPJArfuPVN1+Hz1t0Y6n5jLrGQbkb4urgPE/0Rve+1kMB1v/oWgHgm4WIcV+i7F2pTVj+2iQ==", "license": "MIT", + "peer": true, "dependencies": { "loose-envify": "^1.1.0" }, @@ -8970,6 +8975,7 @@ "integrity": "sha512-ZsJzA5thDQMSQO788d7IocwwQbI8B5OPzmqNvpf3NY/+MHDAS759Wo0gd2WQeXYt5AAAQjzcrTVC6SKCuYgoCQ==", "inBundle": true, "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~6.21.0" } @@ -9030,6 +9036,7 @@ "integrity": "sha512-MWtvHrGZLFttgeEj28VXHxpmwYbor/ATPYbBfSFZEIRK0ecCFLl2Qo55z52Hss+UV9CRN7trSeq1zbgx7YDWWg==", "inBundle": true, "license": "MIT", + "peer": true, "dependencies": { "csstype": "^3.2.2" } @@ -9353,6 +9360,7 @@ "integrity": "sha512-n1H6IcDhmmUEG7TNVSspGmiHHutt7iVKtZwRppD7e04wha5MrkV1h3pti9xQLcCMt6YWsncpoT0HMjkH1FNwWQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.46.0", "@typescript-eslint/types": "8.46.0", @@ -9885,6 +9893,7 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -9930,6 +9939,7 @@ "resolved": "https://registry.npmjs.org/ai/-/ai-5.0.129.tgz", "integrity": "sha512-IARdFetNTedDfqpByNMm9p0oHj7JS+SpOrbgLdQdyCiDe70Xk07wnKP4Lub1ckCrxkhAxY3yxOHllGEjbpXgpQ==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@ai-sdk/gateway": "2.0.35", "@ai-sdk/provider": "2.0.1", @@ -10377,6 +10387,7 @@ "integrity": "sha512-Cg7TFGpIr01vOQNODXOOaGz2NpCU5gl8x1qJFbb6hbZxR7XrcE2vtbAsTAbJ7/xwJtUuJEw8K8Zr/AE0LHlesg==", "inBundle": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/runtime": "^7.12.5", "cosmiconfig": "^7.0.0", @@ -10828,6 +10839,7 @@ ], "inBundle": true, "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.10.12", "caniuse-lite": "^1.0.30001782", @@ -12877,6 +12889,7 @@ "integrity": "sha512-XyLmROnACWqSxiGYArdef1fItQd47weqB7iwtfr9JHwRrqIXZdcFMvvEcL9xHCmL0SNsOvF0c42lWyM1U5dgig==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -13432,6 +13445,7 @@ "integrity": "sha512-whOE1HFo/qJDyX4SnXzP4N6zOWn79WhnCUY/iDR0mPfQZO8wcYE4JClzI2oZrhBnnMUCBCHZhO6VQyoBU95mZA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@rtsao/scc": "^1.1.0", "array-includes": "^3.1.9", @@ -14064,6 +14078,7 @@ "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "inBundle": true, "license": "MIT", + "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -15493,6 +15508,7 @@ "integrity": "sha512-am5zfg3yu6sqn5yjKBNqhnTX7Cv+m00ox+7jbaKkrLMRJ4rAdldd1xPd/JzbBWspqaQv6RSTrgFN95EsfhC+7w==", "inBundle": true, "license": "MIT", + "peer": true, "engines": { "node": ">=16.9.0" } @@ -15771,6 +15787,7 @@ "resolved": "https://registry.npmjs.org/ink/-/ink-6.5.1.tgz", "integrity": "sha512-wF3j/DmkM8q5E+OtfdQhCRw8/0ahkc8CUTgEddxZzpEWPslu7YPL3t64MWRoI9m6upVGpfAg4ms2BBvxCdKRLQ==", "license": "MIT", + "peer": true, "dependencies": { "@alcalzone/ansi-tokenize": "^0.2.1", "ansi-escapes": "^7.2.0", @@ -21206,7 +21223,8 @@ "version": "0.0.1", "resolved": "https://registry.npmjs.org/quickjs-wasi/-/quickjs-wasi-0.0.1.tgz", "integrity": "sha512-fBWNLTBkxkLAhe1AzF1hyXEvuA+N+vV1WMP2D6iiMUblvmOt8Pp5t8zUcgvz7aYA1ldUdxDlgUse15dmcKjkNg==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/rambda": { "version": "7.5.0", @@ -21281,6 +21299,7 @@ "integrity": "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ==", "inBundle": true, "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -21344,6 +21363,7 @@ "integrity": "sha512-AXJdLo8kgMbimY95O2aKQqsz2iWi9jMgKJhRBAxECE4IFxfcazB2LmzloIoibJI3C12IlY20+KFaLv+71bUJeQ==", "inBundle": true, "license": "MIT", + "peer": true, "dependencies": { "scheduler": "^0.27.0" }, @@ -23028,6 +23048,7 @@ "resolved": "https://registry.npmjs.org/socket.io/-/socket.io-4.8.3.tgz", "integrity": "sha512-2Dd78bqzzjE6KPkD5fHZmDAKRNe3J15q+YHDrIsy9WEkqttc7GY+kT9OBLSMaPbQaEd0x1BjcmtMtXkfpc+T5A==", "license": "MIT", + "peer": true, "dependencies": { "accepts": "~1.3.4", "base64id": "~2.0.0", @@ -23898,6 +23919,7 @@ "integrity": "sha512-vzCjQO/rgUuK9sf8VJZvjqiqiHFaZLnOiimmUuOKODxWL8mm/xua7viT7aqX7dgPY60otQjUotzFMmCB4VdmqQ==", "dev": true, "license": "BSD-2-Clause", + "peer": true, "dependencies": { "@jridgewell/source-map": "^0.3.3", "acorn": "^8.15.0", @@ -24274,6 +24296,7 @@ "integrity": "sha512-5C1sg4USs1lfG0GFb2RLXsdpXqBSEhAaA/0kPL01wxzpMqLILNxIxIOKiILz+cdg/pLnOUxFYOR5yhHU666wbw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "~0.27.0", "get-tsconfig": "^4.7.5" @@ -24451,6 +24474,7 @@ "devOptional": true, "inBundle": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -24975,6 +24999,7 @@ "integrity": "sha512-Bby3NOsna2jsjfLVOHKes8sGwgl4TT0E6vvpYgnAYDIF/tie7MRaFthmKuHx1NSXjiTueXH3do80FMQgvEktRg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.27.0", "fdir": "^6.5.0", @@ -25401,6 +25426,7 @@ "integrity": "sha512-PlXPeEWMXMZ7sPYOHqmDyCJzcfNrUr3fGNKtezX14ykXOEIvyK81d+qydx89KY5O71FKMPaQ2vBfBFI5NHR63A==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "fast-deep-equal": "^3.1.3", "fast-uri": "^3.0.1", @@ -25517,6 +25543,7 @@ "integrity": "sha512-cIFJOD1DESzpjOBl763Kp1AH7UE/0fcdHe6rZXUdQ9c50uvgigvW97u3IcSeBwOkgqL/PXPBktBCh0KEu5L8XQ==", "dev": true, "license": "MIT", + "peer": true, "bin": { "rollup": "dist/bin/rollup" }, @@ -26046,6 +26073,7 @@ "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "inBundle": true, "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/src/oclif/commands/signing-key/add.ts b/src/oclif/commands/signing-key/add.ts new file mode 100644 index 000000000..2bc3c2853 --- /dev/null +++ b/src/oclif/commands/signing-key/add.ts @@ -0,0 +1,83 @@ +import {Command, Flags} from '@oclif/core' +import {readFileSync} from 'node:fs' + +import {extractPublicKey, resolveHome} from '../../../shared/ssh/index.js' +import {type IVcSigningKeyResponse, VcEvents} from '../../../shared/transport/events/vc-events.js' +import {formatConnectionError, withDaemonRetry} from '../../lib/daemon-client.js' + +export default class SigningKeyAdd extends Command { + public static description = 'Add an SSH public key to your Byterover account for commit signing' + public static examples = [ + '<%= config.bin %> <%= command.id %> --key ~/.ssh/id_ed25519 --title "Dev laptop"', + '<%= config.bin %> <%= command.id %> -k ~/.ssh/id_ed25519.pub', + ] +public static flags = { + key: Flags.string({ + char: 'k', + description: + 'Path to the SSH private key (used to derive the public key) or a .pub file', + required: true, + }), + title: Flags.string({ + char: 't', + description: 'Human-readable label for the key (defaults to the key comment)', + }), + } + + public async run(): Promise { + const {flags} = await this.parse(SigningKeyAdd) + const keyPath = resolveHome(flags.key) + + let publicKey: string | undefined + let {title} = flags + + try { + if (keyPath.endsWith('.pub')) { + // Public key file — read directly + const raw = readFileSync(keyPath, 'utf8').trim() + publicKey = raw + // Extract comment as default title (third field in authorized_keys format) + const parts = raw.split(' ') + if (!title && parts.length >= 3) title = parts.slice(2).join(' ') + } else { + // Private key file — extract public key without decryption (works for encrypted keys too) + const extracted = await extractPublicKey(keyPath) + const b64 = extracted.publicKeyBlob.toString('base64') + publicKey = `${extracted.keyType} ${b64}` + if (!title) title = extracted.comment ?? `My ${extracted.keyType} key` + } + } catch (error) { + this.error( + `Failed to read key file: ${error instanceof Error ? error.message : String(error)}`, + ) + } + + // Narrow explicitly: this.error() returns `never` but oclif's type + // declaration doesn't expose that, so without the guard TS treats + // publicKey as possibly undefined below. + if (!publicKey) { + this.error('Failed to determine public key material from key file') + } + + if (!title) title = 'My SSH key' + + try { + const response = await withDaemonRetry(async (client) => + client.requestWithAck(VcEvents.SIGNING_KEY, { + action: 'add', + publicKey, + title, + }), + ) + + if (response.action === 'add' && response.key) { + this.log('✅ Signing key added successfully') + this.log(` Title: ${response.key.title}`) + this.log(` Fingerprint: ${response.key.fingerprint}`) + this.log(` Type: ${response.key.keyType}`) + } + } catch (error) { + this.error(formatConnectionError(error)) + } + } +} diff --git a/src/oclif/commands/signing-key/list.ts b/src/oclif/commands/signing-key/list.ts new file mode 100644 index 000000000..39415a94e --- /dev/null +++ b/src/oclif/commands/signing-key/list.ts @@ -0,0 +1,45 @@ +import {Command} from '@oclif/core' + +import {type IVcSigningKeyResponse, VcEvents} from '../../../shared/transport/events/vc-events.js' +import {formatConnectionError, withDaemonRetry} from '../../lib/daemon-client.js' + +export default class SigningKeyList extends Command { + public static description = 'List SSH signing keys registered on your Byterover account' + public static examples = ['<%= config.bin %> <%= command.id %>'] + + public async run(): Promise { + try { + const response = await withDaemonRetry(async (client) => + client.requestWithAck(VcEvents.SIGNING_KEY, {action: 'list'}), + ) + + if (response.action !== 'list' || !response.keys) { + this.error('Unexpected response from daemon') + } + + const {keys} = response + + if (keys.length === 0) { + this.log('No signing keys registered.') + this.log(' Run: brv signing-key add --key ~/.ssh/id_ed25519') + return + } + + this.log(`\nSigning keys (${keys.length}):\n`) + for (const key of keys) { + const lastUsed = key.lastUsedAt + ? `Last used: ${new Date(key.lastUsedAt).toLocaleDateString()}` + : 'Never used' + this.log(` [${key.id}]`) + this.log(` Title: ${key.title}`) + this.log(` Fingerprint: ${key.fingerprint}`) + this.log(` Type: ${key.keyType}`) + this.log(` ${lastUsed}`) + this.log(` Added: ${new Date(key.createdAt).toLocaleDateString()}`) + this.log('') + } + } catch (error) { + this.error(formatConnectionError(error)) + } + } +} diff --git a/src/oclif/commands/signing-key/remove.ts b/src/oclif/commands/signing-key/remove.ts new file mode 100644 index 000000000..3e58dda9f --- /dev/null +++ b/src/oclif/commands/signing-key/remove.ts @@ -0,0 +1,48 @@ +import {Args, Command, Flags} from '@oclif/core' + +import {type IVcSigningKeyResponse, VcEvents} from '../../../shared/transport/events/vc-events.js' +import {formatConnectionError, withDaemonRetry} from '../../lib/daemon-client.js' + +export default class SigningKeyRemove extends Command { + public static args = { + id: Args.string({ + description: 'Key ID to remove (from brv signing-key list)', + required: true, + }), + } + public static description = 'Remove an SSH signing key from your Byterover account' + public static examples = [ + '<%= config.bin %> <%= command.id %> --yes', + '# Get key ID from: brv signing-key list', + ] + public static flags = { + yes: Flags.boolean({ + default: false, + description: 'Confirm the destructive removal (required — oclif commands never prompt).', + }), + } + + public async run(): Promise { + const {args, flags} = await this.parse(SigningKeyRemove) + + if (!flags.yes) { + this.error( + `Refusing to remove signing key '${args.id}' without explicit --yes confirmation. ` + + 'This action is irreversible. Re-run with --yes to proceed.', + ) + } + + try { + await withDaemonRetry(async (client) => + client.requestWithAck(VcEvents.SIGNING_KEY, { + action: 'remove', + keyId: args.id, + }), + ) + + this.log(`✅ Signing key removed: ${args.id}`) + } catch (error) { + this.error(formatConnectionError(error)) + } + } +} diff --git a/src/oclif/commands/vc/commit.ts b/src/oclif/commands/vc/commit.ts index 3977a2308..7730b73bd 100644 --- a/src/oclif/commands/vc/commit.ts +++ b/src/oclif/commands/vc/commit.ts @@ -1,16 +1,30 @@ import {Command, Flags} from '@oclif/core' -import {type IVcCommitResponse, VcEvents} from '../../../shared/transport/events/vc-events.js' +import {type IVcCommitRequest, type IVcCommitResponse, VcErrorCode, VcEvents} from '../../../shared/transport/events/vc-events.js' import {formatConnectionError, withDaemonRetry} from '../../lib/daemon-client.js' export default class VcCommit extends Command { public static description = 'Save staged changes as a commit' - public static examples = ['<%= config.bin %> <%= command.id %> -m "Add project architecture notes"'] + public static examples = [ + '<%= config.bin %> <%= command.id %> -m "Add project architecture notes"', + '<%= config.bin %> <%= command.id %> -m "Signed commit" --sign', + '<%= config.bin %> <%= command.id %> -m "Unsigned commit" --no-sign', + '<%= config.bin %> <%= command.id %> -m "Signed (encrypted key)" --sign --passphrase "$MY_PASS"', + 'BRV_SSH_PASSPHRASE="$MY_PASS" <%= config.bin %> <%= command.id %> -m "Signed (env)" --sign', + ] public static flags = { message: Flags.string({ char: 'm', description: 'Commit message', }), + passphrase: Flags.string({ + description: + 'SSH key passphrase — WARNING: visible in process list and shell history. Prefer BRV_SSH_PASSPHRASE env var or ssh-agent.', + }), + sign: Flags.boolean({ + allowNo: true, + description: 'Sign the commit with your configured SSH key. Use --no-sign to override commit.sign=true.', + }), } public static strict = false @@ -26,13 +40,37 @@ export default class VcCommit extends Command { this.error('Usage: brv vc commit -m ""') } + const {sign} = flags + const passphrase = flags.passphrase ?? process.env.BRV_SSH_PASSPHRASE + const payload: IVcCommitRequest = { + message, + ...(sign === undefined ? {} : {sign}), + ...(passphrase ? {passphrase} : {}), + } + try { const result = await withDaemonRetry(async (client) => - client.requestWithAck(VcEvents.COMMIT, {message}), + client.requestWithAck(VcEvents.COMMIT, payload), ) - this.log(`[${result.sha.slice(0, 7)}] ${result.message}`) + const sigIndicator = result.signed ? ' 🔏' : '' + this.log(`[${result.sha.slice(0, 7)}] ${result.message}${sigIndicator}`) } catch (error) { + // oclif commands run non-interactively. Surface a clear actionable error + // for missing passphrase instead of prompting — interactive entry belongs + // in the TUI layer. + if ( + error instanceof Error && + 'code' in error && + typeof error.code === 'string' && + error.code === VcErrorCode.PASSPHRASE_REQUIRED + ) { + this.error( + 'Signing key requires a passphrase. Provide it via the --passphrase flag ' + + 'or the BRV_SSH_PASSPHRASE environment variable, then retry.', + ) + } + this.error(formatConnectionError(error)) } } diff --git a/src/oclif/commands/vc/config.ts b/src/oclif/commands/vc/config.ts index 738350f52..2a3b28ad2 100644 --- a/src/oclif/commands/vc/config.ts +++ b/src/oclif/commands/vc/config.ts @@ -1,27 +1,72 @@ -import {Args, Command} from '@oclif/core' +import {Args, Command, Flags} from '@oclif/core' +import {existsSync, readFileSync} from 'node:fs' -import {isVcConfigKey, type IVcConfigResponse, VcEvents} from '../../../shared/transport/events/vc-events.js' +import {extractPublicKey, resolveHome} from '../../../shared/ssh/index.js' +import {isVcConfigKey, type IVcConfigResponse, type IVcSigningKeyResponse, VcErrorCode, VcEvents} from '../../../shared/transport/events/vc-events.js' import {formatConnectionError, withDaemonRetry} from '../../lib/daemon-client.js' +/** + * Returns the transport-error code (e.g. VcErrorCode.SIGNING_KEY_ALREADY_EXISTS) + * from errors coming back through the daemon transport. TransportRequestError + * preserves `.code`; other error shapes return undefined. + */ +function getTransportErrorCode(error: unknown): string | undefined { + if (error instanceof Error && 'code' in error) { + const {code} = error as Error & {code?: unknown} + if (typeof code === 'string') return code + } + + return undefined +} + export default class VcConfig extends Command { public static args = { - key: Args.string({description: 'Config key (user.name or user.email)', required: true}), + key: Args.string({ + description: 'Config key: user.name, user.email, user.signingkey, commit.sign', + required: false, + }), value: Args.string({description: 'Value to set (omit to read current value)'}), } - public static description = 'Get or set commit author for ByteRover version control' - public static examples = [ +public static description = 'Get or set commit author / signing config for ByteRover version control' +public static examples = [ '<%= config.bin %> <%= command.id %> user.name "Your Name"', '<%= config.bin %> <%= command.id %> user.email "you@example.com"', + '<%= config.bin %> <%= command.id %> user.signingkey ~/.ssh/id_ed25519', + '<%= config.bin %> <%= command.id %> commit.sign true', '<%= config.bin %> <%= command.id %> user.name', - '<%= config.bin %> <%= command.id %> user.email', + '<%= config.bin %> <%= command.id %> --import-git-signing', ] +public static flags = { + 'import-git-signing': Flags.boolean({ + description: + 'Import SSH signing config from local or global git config ' + + '(user.signingKey + gpg.format=ssh + commit.gpgSign)', + exclusive: ['key'], + }), + } public async run(): Promise { - const {args} = await this.parse(VcConfig) + const {args, flags} = await this.parse(VcConfig) const {key, value} = args + // --import-git-signing mode: reads local/global gitconfig and imports into brv config + if (flags['import-git-signing']) { + await this.runImport() + return + } + + if (!key) { + this.error( + 'Usage: brv vc config [value]\n' + + 'Keys: user.name, user.email, user.signingkey, commit.sign\n' + + 'Or: brv vc config --import-git-signing', + ) + } + if (!isVcConfigKey(key)) { - this.error(`Unknown key '${key}'. Allowed: user.name, user.email.`) + this.error( + `Unknown key '${key}'. Allowed: user.name, user.email, user.signingkey, commit.sign.`, + ) } try { @@ -29,8 +74,86 @@ export default class VcConfig extends Command { client.requestWithAck(VcEvents.CONFIG, {key, value}), ) - this.log(`${result.value}`) + if (result.hint) this.log(` Hint: ${result.hint}`) + this.log(result.value) + } catch (error) { + this.error(formatConnectionError(error)) + } + } + + /** + * Import SSH signing configuration from the local or global git config. + * Reads: user.signingKey, gpg.format, commit.gpgSign via `git config --get` + */ + private async runImport(): Promise { + try { + const result = await withDaemonRetry(async (client) => + client.requestWithAck(VcEvents.CONFIG, {importGitSigning: true}), + ) + + if (result.hint) this.log(` ${result.hint}`) + this.log(`✅ Imported signing config from git:`) + this.log(` user.signingkey = ${result.value}`) + + // Attempt to register the key automatically + this.log(`\n⏳ Attempting to register signing key with ByteRover server...`) + try { + const keyPath = resolveHome(result.value) + let publicKey: string + let title = 'My SSH key' + + const pubPath = keyPath.endsWith('.pub') ? keyPath : `${keyPath}.pub` + + if (existsSync(pubPath)) { + const raw = readFileSync(pubPath, 'utf8').trim() + publicKey = raw + const parts = raw.split(' ') + if (parts.length >= 3) title = parts.slice(2).join(' ') + } else { + // No .pub sidecar — extract public key without decryption (works for encrypted keys) + const extracted = await extractPublicKey(keyPath) + const b64 = extracted.publicKeyBlob.toString('base64') + publicKey = `${extracted.keyType} ${b64}` + title = extracted.comment ?? `My ${extracted.keyType} key` + } + + const response = await withDaemonRetry(async (client) => + client.requestWithAck(VcEvents.SIGNING_KEY, { + action: 'add', + publicKey, + title, + }), + ) + + if (response.action === 'add' && response.key) { + this.log('✅ Signing key registered successfully on server') + } + } catch (error) { + if (getTransportErrorCode(error) === VcErrorCode.SIGNING_KEY_ALREADY_EXISTS) { + this.log('✅ Signing key is already registered on server') + } else { + const errMsg = error instanceof Error ? error.message : String(error) + this.log(`⚠️ Could not automatically register key: ${errMsg}`) + this.log(` You may need to run: brv signing-key add --key ${result.value}`) + } + } } catch (error) { + const msg = error instanceof Error ? error.message : String(error) + // Provide helpful guidance when no signing key is configured in git + if (msg.includes('not set') || msg.includes('not found')) { + this.log('ℹ️ No SSH signing key found in your git config.') + this.log('') + this.log('To configure SSH signing in git:') + this.log(' git config user.signingKey ~/.ssh/id_ed25519') + this.log(' git config gpg.format ssh') + this.log(' git config commit.gpgSign true') + this.log('') + this.log('Or set it directly in brv:') + this.log(' brv vc config user.signingkey ~/.ssh/id_ed25519') + this.log(' brv vc config commit.sign true') + return + } + this.error(formatConnectionError(error)) } } diff --git a/src/server/core/interfaces/services/i-git-service.ts b/src/server/core/interfaces/services/i-git-service.ts index 72dc93326..21efa1e0b 100644 --- a/src/server/core/interfaces/services/i-git-service.ts +++ b/src/server/core/interfaces/services/i-git-service.ts @@ -54,7 +54,16 @@ export type AheadBehind = {ahead: number; behind: number} // --- Params types --- export type InitGitParams = BaseGitParams & {defaultBranch?: string} export type AddGitParams = BaseGitParams & {filePaths: string[]} -export type CommitGitParams = BaseGitParams & {author?: {email: string; name: string}; message: string} +export type CommitGitParams = { + author?: {email: string; name: string} + message: string + /** + * Optional sign callback — receives the raw commit payload (the text that would be signed) + * and must return an armored SSH signature (-----BEGIN SSH SIGNATURE----- ... -----END SSH SIGNATURE-----). + * When provided, the commit is created with a gpgsig header containing the returned signature. + */ + onSign?: (payload: string) => Promise +} & BaseGitParams export type LogGitParams = BaseGitParams & {depth?: number; filepath?: string; ref?: string} export type PushGitParams = BaseGitParams & {branch?: string; remote?: string} export type PullGitParams = BaseGitParams & {allowUnrelatedHistories?: boolean; author?: {email: string; name: string}; branch?: string; remote?: string} diff --git a/src/server/core/interfaces/services/i-http-client.ts b/src/server/core/interfaces/services/i-http-client.ts index d8e79ab6c..9e35ecadf 100644 --- a/src/server/core/interfaces/services/i-http-client.ts +++ b/src/server/core/interfaces/services/i-http-client.ts @@ -16,6 +16,14 @@ export type HttpRequestConfig = { * - Response parsing */ export interface IHttpClient { + /** + * Performs an HTTP DELETE request. + * @param url The URL to request + * @param config Optional request configuration (headers, timeout) + * @returns A promise that resolves to the response data + */ + delete: (url: string, config?: HttpRequestConfig) => Promise + /** * Performs an HTTP GET request. * @param url The URL to request diff --git a/src/server/core/interfaces/services/i-signing-key-service.ts b/src/server/core/interfaces/services/i-signing-key-service.ts new file mode 100644 index 000000000..e1c467ffe --- /dev/null +++ b/src/server/core/interfaces/services/i-signing-key-service.ts @@ -0,0 +1,17 @@ +// ── DTOs ───────────────────────────────────────────────────────────────────── + +export type SigningKeyResource = { + createdAt: string + fingerprint: string + id: string + keyType: string + lastUsedAt?: string + publicKey: string + title: string +} + +export interface ISigningKeyService { + addKey(title: string, publicKey: string): Promise + listKeys(): Promise + removeKey(keyId: string): Promise +} diff --git a/src/server/core/interfaces/vc/i-vc-git-config-store.ts b/src/server/core/interfaces/vc/i-vc-git-config-store.ts index 845384122..a07af5b76 100644 --- a/src/server/core/interfaces/vc/i-vc-git-config-store.ts +++ b/src/server/core/interfaces/vc/i-vc-git-config-store.ts @@ -1,7 +1,11 @@ -// Both optional — user sets name/email separately, one at a time (like git config) +// Both optional — user sets fields separately, one at a time (like git config) export interface IVcGitConfig { + /** Auto-sign all commits when true (default: false) */ + commitSign?: boolean email?: string name?: string + /** Path to SSH private key for signing (e.g., "~/.ssh/id_ed25519") */ + signingKey?: string } export interface IVcGitConfigStore { diff --git a/src/server/infra/git/isomorphic-git-service.ts b/src/server/infra/git/isomorphic-git-service.ts index 660f4d3ef..7e522a905 100644 --- a/src/server/infra/git/isomorphic-git-service.ts +++ b/src/server/infra/git/isomorphic-git-service.ts @@ -257,7 +257,21 @@ export class IsomorphicGitService implements IGitService { let sha: string try { - sha = await git.commit({author, dir, fs, message: params.message, ...(parent ? {parent} : {})}) + sha = await git.commit({ + author, + dir, + fs, + message: params.message, + ...(parent ? {parent} : {}), + ...(params.onSign + ? { + onSign: async ({payload}: {payload: string}) => ({ + signature: await params.onSign!(payload), + }), + signingKey: 'ssh-signing', + } + : {}), + }) } catch (error) { if (error instanceof git.Errors.UnmergedPathsError) { const paths = error.data.filepaths.join(', ') diff --git a/src/server/infra/http/authenticated-http-client.ts b/src/server/infra/http/authenticated-http-client.ts index a08aae7b7..9cf8f8281 100644 --- a/src/server/infra/http/authenticated-http-client.ts +++ b/src/server/infra/http/authenticated-http-client.ts @@ -49,6 +49,30 @@ export class AuthenticatedHttpClient implements IHttpClient { this.sessionKey = sessionKey } + /** + * Performs an HTTP DELETE request with authentication headers. + * @param url The URL to request + * @param config Optional request configuration (headers, timeout) + * @returns A promise that resolves to the response data + * @throws Error if the request fails + */ + public async delete(url: string, config?: HttpRequestConfig): Promise { + try { + const axiosConfig: AxiosRequestConfig = { + headers: this.buildHeaders(config?.headers), + httpAgent: ProxyConfig.getProxyAgent(), + httpsAgent: ProxyConfig.getProxyAgent(), + proxy: false, + timeout: config?.timeout, + } + + const response = await axios.delete(url, axiosConfig) + return response.data + } catch (error) { + throw this.handleError(error) + } + } + /** * Performs an HTTP GET request with authentication headers. * @param url The URL to request diff --git a/src/server/infra/iam/http-signing-key-service.ts b/src/server/infra/iam/http-signing-key-service.ts new file mode 100644 index 000000000..ccd43e5c2 --- /dev/null +++ b/src/server/infra/iam/http-signing-key-service.ts @@ -0,0 +1,104 @@ +import type {IHttpClient} from '../../core/interfaces/services/i-http-client.js' +import type {ISigningKeyService, SigningKeyResource} from '../../core/interfaces/services/i-signing-key-service.js' + +import {VcErrorCode} from '../../../shared/transport/events/vc-events.js' +import {VcError} from '../../core/domain/errors/vc-error.js' + +// IAM API wraps all responses in {success, data} +type ApiEnvelope = { + data: T + success: boolean +} + +// IAM API response wrappers (inside data envelope) +type CreateSigningKeyData = { + signing_key: RawSigningKeyResource +} + +type ListSigningKeysData = { + signing_keys: RawSigningKeyResource[] +} + +// IAM returns snake_case; map to camelCase +type RawSigningKeyResource = { + created_at: string + fingerprint: string + id: string + key_type: string + last_used_at?: string + public_key: string + title: string +} + +function mapResource(raw: RawSigningKeyResource): SigningKeyResource { + return { + createdAt: raw.created_at, + fingerprint: raw.fingerprint, + id: raw.id, + keyType: raw.key_type, + lastUsedAt: raw.last_used_at, + publicKey: raw.public_key, + title: raw.title, + } +} + +/** + * HTTP client for the IAM signing key CRUD API. + * + * API base: /api/v3/users/me/signing-keys (configured via BRV_API_BASE_URL / httpClient base URL) + */ +export class HttpSigningKeyService implements ISigningKeyService { + private readonly httpClient: IHttpClient + private readonly iamBaseUrl: string + + constructor(httpClient: IHttpClient, iamBaseUrl: string) { + this.httpClient = httpClient + this.iamBaseUrl = iamBaseUrl.replace(/\/$/, '') + } + + async addKey(title: string, publicKey: string): Promise { + let response: ApiEnvelope + try { + response = await this.httpClient.post>( + `${this.iamBaseUrl}/api/v3/users/me/signing-keys`, + /* eslint-disable camelcase */ + {public_key: publicKey, title}, + /* eslint-enable camelcase */ + ) + } catch (error) { + // AuthenticatedHttpClient collapses non-2xx axios errors into Error + // instances whose message carries the HTTP status. Translate the + // duplicate-key signal (409) into a structured VcError so callers can + // branch on the code instead of regex-matching English substrings. + if (error instanceof Error && /\b409\b|conflict/i.test(error.message)) { + throw new VcError( + 'This SSH public key is already registered with your Byterover account.', + VcErrorCode.SIGNING_KEY_ALREADY_EXISTS, + ) + } + + throw error + } + + if (!response.success) { + throw new Error('IAM signing-key add request failed (response.success=false)') + } + + return mapResource(response.data.signing_key) + } + + async listKeys(): Promise { + const response = await this.httpClient.get>( + `${this.iamBaseUrl}/api/v3/users/me/signing-keys`, + ) + if (!response.success) { + throw new Error('IAM signing-key list request failed (response.success=false)') + } + + return (response.data.signing_keys ?? []).map((raw) => mapResource(raw)) + } + + async removeKey(keyId: string): Promise { + await this.httpClient.delete(`${this.iamBaseUrl}/api/v3/users/me/signing-keys/${keyId}`) + } +} diff --git a/src/server/infra/process/feature-handlers.ts b/src/server/infra/process/feature-handlers.ts index 2b503cb85..967fd2d21 100644 --- a/src/server/infra/process/feature-handlers.ts +++ b/src/server/infra/process/feature-handlers.ts @@ -62,6 +62,7 @@ import { PushHandler, ResetHandler, ReviewHandler, + SigningKeyHandler, SourceHandler, SpaceHandler, StatusHandler, @@ -310,6 +311,14 @@ export async function setupFeatureHandlers({ webAppUrl: envConfig.webAppUrl, }).setup() + // Signing Key handler — creates a fresh authenticated HTTP client per request + // using the current session key from tokenStore (consistent with PushHandler pattern). + new SigningKeyHandler({ + iamBaseUrl: envConfig.iamBaseUrl, + tokenStore, + transport, + }).setup() + new ContextTreeHandler({ contextFileReader, contextTreeService, diff --git a/src/server/infra/ssh/index.ts b/src/server/infra/ssh/index.ts new file mode 100644 index 000000000..023559598 --- /dev/null +++ b/src/server/infra/ssh/index.ts @@ -0,0 +1,5 @@ +export {SigningKeyCache} from './signing-key-cache.js' +export {SshAgentSigner, tryGetSshAgentSigner} from './ssh-agent-signer.js' +export {isPassphraseError, parseSSHPrivateKey, probeSSHKey, resolveHome} from './ssh-key-parser.js' +export {signCommitPayload} from './sshsig-signer.js' +export type {ParsedSSHKey, SSHKeyProbe, SSHKeyType, SSHSignatureResult} from './types.js' diff --git a/src/server/infra/ssh/signing-key-cache.ts b/src/server/infra/ssh/signing-key-cache.ts new file mode 100644 index 000000000..f14a00fd9 --- /dev/null +++ b/src/server/infra/ssh/signing-key-cache.ts @@ -0,0 +1,112 @@ +import type {ParsedSSHKey} from './types.js' + +interface CacheEntry { + expiresAt: number + key: ParsedSSHKey +} + +/** + * In-memory TTL cache for parsed SSH private keys. + * + * Option C Path B: after successful file-based parsing (with passphrase), + * cache the ParsedSSHKey object (which holds an opaque crypto.KeyObject) + * so subsequent commits within the TTL window require no passphrase prompt. + * + * Scoping: entries are keyed by (projectPath, keyPath). Two different projects + * that happen to share an identical signing-key path MUST NOT share cached + * state — a user switching projects should not inherit the previous project's + * decrypted key object. The null byte separator is safe because POSIX and + * Windows paths cannot contain \0. + * + * Security properties: + * - Stored in daemon process memory only — never written to disk + * - crypto.KeyObject is opaque and not directly extractable + * - Passphrase is never stored — only the decrypted key object + * - Cleared entirely on daemon restart + * - Per-project, per-key invalidation when user changes config + */ +export class SigningKeyCache { + private readonly cache = new Map() + private readonly ttlMs: number + + /** + * @param ttlMs - Cache TTL in milliseconds. + * Default: 30 minutes. Rationale: a user signing many commits in one session + * should not need to re-parse the key file on every commit. 30 minutes balances + * convenience against the window in which a compromised daemon process could use + * the cached key object. The passphrase itself is never stored — only the opaque + * crypto.KeyObject produced after decryption. + */ + constructor(ttlMs: number = 30 * 60 * 1000) { + this.ttlMs = ttlMs + } + + /** Null byte cannot appear in POSIX or Windows path components. */ + private static composeKey(projectPath: string, keyPath: string): string { + return `${projectPath}\0${keyPath}` + } + + /** + * Current number of cached (non-expired) keys. + */ + get size(): number { + const now = Date.now() + let count = 0 + for (const [, entry] of this.cache) { + if (now <= entry.expiresAt) count++ + } + + return count + } + + /** + * Get a cached key for a (project, key) pair. + * Returns undefined if the entry does not exist or has expired. + */ + get(projectPath: string, keyPath: string): ParsedSSHKey | undefined { + const now = Date.now() + this.sweep(now) + const entry = this.cache.get(SigningKeyCache.composeKey(projectPath, keyPath)) + return entry && now <= entry.expiresAt ? entry.key : undefined + } + + /** + * Invalidate a specific (project, key) pair (e.g., when user changes config). + */ + invalidate(projectPath: string, keyPath: string): void { + this.cache.delete(SigningKeyCache.composeKey(projectPath, keyPath)) + } + + /** + * Clear all cached keys (e.g., on explicit logout or security reset). + */ + invalidateAll(): void { + this.cache.clear() + } + + /** + * Cache a parsed key for a (project, key) pair. + * Resets TTL if the entry was already cached. + * Also sweeps expired entries to prevent memory leaks. + */ + set(projectPath: string, keyPath: string, key: ParsedSSHKey): void { + const now = Date.now() + this.sweep(now) + this.cache.set(SigningKeyCache.composeKey(projectPath, keyPath), { + expiresAt: now + this.ttlMs, + key, + }) + } + + /** + * Evict every cache entry whose TTL has already elapsed at `now`. Called + * from both get() and set() so long-running daemons that are read-heavy + * (cache once, sign many) still reclaim memory for keys the user has + * stopped using. + */ + private sweep(now: number): void { + for (const [k, entry] of this.cache) { + if (now > entry.expiresAt) this.cache.delete(k) + } + } +} diff --git a/src/server/infra/ssh/ssh-agent-signer.ts b/src/server/infra/ssh/ssh-agent-signer.ts new file mode 100644 index 000000000..c87d5abf7 --- /dev/null +++ b/src/server/infra/ssh/ssh-agent-signer.ts @@ -0,0 +1,275 @@ +import {createHash} from 'node:crypto' +import net from 'node:net' + +import type {SSHSignatureResult} from './types.js' + +import {getPublicKeyMetadata} from './ssh-key-parser.js' +import {SSHSIG_HASH_ALGORITHM, SSHSIG_MAGIC_PREAMBLE} from './sshsig-constants.js' + +const SSHSIG_MAGIC = Buffer.from(SSHSIG_MAGIC_PREAMBLE) + +// SSH agent protocol message types +const SSH_AGENTC_REQUEST_IDENTITIES = 11 +const SSH_AGENTC_SIGN_REQUEST = 13 +const SSH_AGENT_IDENTITIES_ANSWER = 12 +const SSH_AGENT_SIGN_RESPONSE = 14 +const SSH_AGENT_FAILURE = 5 + +// SSH agent sign flags +const SSH_AGENT_RSA_SHA2_512 = 4 + +/** + * Read a uint32 big-endian from a buffer at offset. + */ +function readUInt32(buf: Buffer, offset: number): number { + return buf.readUInt32BE(offset) +} + +/** + * Write a uint32 big-endian prefix then data (SSH wire string). + */ +function sshString(data: Buffer | string): Buffer { + const buf = Buffer.isBuffer(data) ? data : Buffer.from(data, 'utf8') + const len = Buffer.allocUnsafe(4) + len.writeUInt32BE(buf.length, 0) + return Buffer.concat([len, buf]) +} + +/** + * Low-level SSH agent client over a Unix domain socket. + */ +class SshAgentClient { + private readonly socketPath: string + + constructor(socketPath: string) { + this.socketPath = socketPath + } + + /** List all identities (public keys) currently held by the agent. */ + async listIdentities(): Promise> { + const request = Buffer.from([SSH_AGENTC_REQUEST_IDENTITIES]) + const response = await this.request(request) + + if (response[0] !== SSH_AGENT_IDENTITIES_ANSWER) { + throw new Error(`Agent returned unexpected message type: ${response[0]}`) + } + + const count = readUInt32(response, 1) + const identities: Array<{blob: Buffer; comment: string; fingerprint: string}> = [] + let offset = 5 + + for (let i = 0; i < count; i++) { + const blobLen = readUInt32(response, offset) + offset += 4 + const blob = response.subarray(offset, offset + blobLen) + offset += blobLen + + const commentLen = readUInt32(response, offset) + offset += 4 + const comment = response.subarray(offset, offset + commentLen).toString('utf8') + offset += commentLen + + // Compute SHA256 fingerprint to match against our key + const hash = createHash('sha256').update(blob).digest('base64').replace(/=+$/, '') + const fingerprint = `SHA256:${hash}` + + identities.push({blob, comment, fingerprint}) + } + + return identities + } + + /** Send a request to the agent and receive the full response. */ + async request(payload: Buffer): Promise { + const MAX_RESPONSE_SIZE = 1024 * 1024 // 1 MB + + return new Promise((resolve, reject) => { + const socket = net.createConnection(this.socketPath) + const chunks: Buffer[] = [] + let settled = false + + const settle = (fn: () => void): void => { + if (settled) return + settled = true + socket.destroy() + fn() + } + + socket.once('connect', () => { + // Prefix payload with uint32 length + const lenBuf = Buffer.allocUnsafe(4) + lenBuf.writeUInt32BE(payload.length, 0) + socket.write(Buffer.concat([lenBuf, payload])) + }) + + socket.on('data', (chunk: Buffer) => { + chunks.push(chunk) + const accumulated = Buffer.concat(chunks) + + if (accumulated.length >= 4) { + const responseLen = readUInt32(accumulated, 0) + + if (responseLen > MAX_RESPONSE_SIZE) { + settle(() => reject(new Error(`Agent response too large: ${responseLen} bytes`))) + return + } + + if (accumulated.length >= 4 + responseLen) { + const body = accumulated.subarray(4, 4 + responseLen) + if (body.length === 0) { + settle(() => reject(new Error('Agent returned empty response body'))) + return + } + + settle(() => resolve(body)) + } + } + }) + + socket.once('error', (err) => settle(() => reject(err))) + socket.once('close', () => { + settle(() => reject(new Error('Agent socket closed without response'))) + }) + + // Timeout after 3 seconds + socket.setTimeout(3000, () => { + settle(() => reject(new Error('SSH agent request timed out'))) + }) + }) + } + + /** Request the agent to sign data with a specific key blob. */ + async sign(keyBlob: Buffer, data: Buffer, flags: number = 0): Promise { + const request = Buffer.concat([ + Buffer.from([SSH_AGENTC_SIGN_REQUEST]), + sshString(keyBlob), + sshString(data), + (() => { + const f = Buffer.allocUnsafe(4) + f.writeUInt32BE(flags, 0) + return f + })(), + ]) + + const response = await this.request(request) + + if (response[0] === SSH_AGENT_FAILURE) { + throw new Error('SSH agent refused to sign (key may not be loaded)') + } + + if (response[0] !== SSH_AGENT_SIGN_RESPONSE) { + throw new Error(`Agent returned unexpected sign response type: ${response[0]}`) + } + + // Response: byte SSH_AGENT_SIGN_RESPONSE + string(signature) + const sigLen = readUInt32(response, 1) + if (sigLen > response.length - 5) { + throw new Error( + `Agent signature truncated: header claims ${sigLen} bytes, body has ${response.length - 5}`, + ) + } + + return response.subarray(5, 5 + sigLen) + } +} + +/** + * High-level signer that uses ssh-agent to produce sshsig-format signatures. + */ +export class SshAgentSigner { + private readonly agent: SshAgentClient + private readonly keyBlob: Buffer + private readonly keyType: string + + constructor(agent: SshAgentClient, keyBlob: Buffer, keyType: string) { + this.agent = agent + this.keyBlob = keyBlob + this.keyType = keyType + } + + /** + * Sign a commit payload using the ssh-agent, producing an armored sshsig signature. + */ + async sign(payload: string): Promise { + const NAMESPACE = 'git' + + // 1. Hash commit payload with the spec-mandated SHA-512. + const messageHash = createHash(SSHSIG_HASH_ALGORITHM).update(Buffer.from(payload, 'utf8')).digest() + + // 2. Build signed-data structure per PROTOCOL.sshsig §2 (6-byte SSHSIG preamble) + const signedData = Buffer.concat([ + SSHSIG_MAGIC, + sshString(NAMESPACE), + sshString(''), + sshString(SSHSIG_HASH_ALGORITHM), + sshString(messageHash), + ]) + + // 3. Choose sign flags + const flags = this.keyType === 'ssh-rsa' ? SSH_AGENT_RSA_SHA2_512 : 0 + + // 4. Ask agent to sign the signed data blob + const agentSignature = await this.agent.sign(this.keyBlob, signedData, flags) + + // 5. The agent returns a full SSH signature blob already + // (string(key-type) + string(raw-sig)) + // We use it directly as the sshsig signature field + + // 6. Build sshsig binary envelope + const versionBuf = Buffer.allocUnsafe(4) + versionBuf.writeUInt32BE(1, 0) + + const sshsigBinary = Buffer.concat([ + SSHSIG_MAGIC, + versionBuf, + sshString(this.keyBlob), + sshString(NAMESPACE), + sshString(''), + sshString(SSHSIG_HASH_ALGORITHM), + sshString(agentSignature), + ]) + + // 7. Armor + const base64 = sshsigBinary.toString('base64') + const lines = base64.match(/.{1,76}/g) ?? [base64] + const armored = [ + '-----BEGIN SSH SIGNATURE-----', + ...lines, + '-----END SSH SIGNATURE-----', + ].join('\n') + + return {armored, raw: sshsigBinary} + } +} + +/** + * Try to get an SshAgentSigner for the key at the given path. + * + * Priority chain path A: connect to ssh-agent → find matching key → return signer. + * Returns undefined (non-throwing) if: + * - $SSH_AUTH_SOCK is not set + * - Agent is unreachable + * - The key at keyPath is not loaded in the agent + */ +export async function tryGetSshAgentSigner(keyPath: string): Promise { + const agentSocket = process.env.SSH_AUTH_SOCK ?? (process.platform === 'win32' ? String.raw`\\.\pipe\openssh-ssh-agent` : undefined) + if (!agentSocket) return undefined + + try { + const agent = new SshAgentClient(agentSocket) + + // Derive public key fingerprint from key file (reads only public key — no passphrase needed) + const parsed = await getPublicKeyMetadata(keyPath).catch(() => {}) + if (!parsed) return undefined + + // Find matching identity in agent + const identities = await agent.listIdentities() + const match = identities.find((id) => id.fingerprint === parsed.fingerprint) + if (!match) return undefined + + return new SshAgentSigner(agent, match.blob, parsed.keyType) + } catch { + // Agent unavailable — degrade gracefully to cache/file path + return undefined + } +} diff --git a/src/server/infra/ssh/ssh-key-parser.ts b/src/server/infra/ssh/ssh-key-parser.ts new file mode 100644 index 000000000..254f28b55 --- /dev/null +++ b/src/server/infra/ssh/ssh-key-parser.ts @@ -0,0 +1,11 @@ +// All implementations have moved to src/shared/ssh/key-parser.ts. +// Re-export everything so existing server imports (ssh-agent-signer, vc-handler, etc.) continue to work. +export { + computeFingerprint, + extractPublicKey, + getPublicKeyMetadata, + isPassphraseError, + parseSSHPrivateKey, + probeSSHKey, + resolveHome, +} from '../../../shared/ssh/key-parser.js' diff --git a/src/server/infra/ssh/sshsig-constants.ts b/src/server/infra/ssh/sshsig-constants.ts new file mode 100644 index 000000000..beb7726b9 --- /dev/null +++ b/src/server/infra/ssh/sshsig-constants.ts @@ -0,0 +1,27 @@ +// PROTOCOL.sshsig spec constants. See: +// https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.sshsig +// +// MAGIC_PREAMBLE is `byte[6] "SSHSIG"` (no null terminator) for both the +// envelope and the signed-data structure. Adding a null byte produces +// signatures that fail `ssh-keygen -Y verify` and `git verify-commit`. +// +// Exported as a string rather than a Buffer so the spec value cannot be +// mutated across module boundaries — Buffer is indexable-mutable. Each +// importing module materialises its own module-private Buffer once at load. +export const SSHSIG_MAGIC_PREAMBLE = 'SSHSIG' + +// Hash algorithm embedded in the sshsig signed-data structure. Fixed at +// `sha512`, not configurable, for three reasons: +// +// 1. Ed25519 (our primary supported key type) MANDATES SHA-512 as part of +// the EdDSA algorithm itself — there is no other choice. +// 2. RSA signing via ssh-agent uses the `RSA-SHA2-512` agent flag +// (see ssh-agent-signer.ts). OpenSSH's default. +// 3. Every OpenSSH verifier from 2017 onwards accepts sha512-labelled +// sshsig signatures; there is no consumer-compat benefit to offering +// sha256 as an alternative. +// +// If a future key type or verifier needs sha256, thread this constant +// through signCommitPayload / SshAgentSigner.sign as a parameter — do not +// re-hardcode in each signer. +export const SSHSIG_HASH_ALGORITHM = 'sha512' diff --git a/src/server/infra/ssh/sshsig-signer.ts b/src/server/infra/ssh/sshsig-signer.ts new file mode 100644 index 000000000..9ab794280 --- /dev/null +++ b/src/server/infra/ssh/sshsig-signer.ts @@ -0,0 +1,82 @@ +import {createHash, sign} from 'node:crypto' + +import type {ParsedSSHKey, SSHSignatureResult} from './types.js' + +import {SSHSIG_HASH_ALGORITHM, SSHSIG_MAGIC_PREAMBLE} from './sshsig-constants.js' + +const SSHSIG_MAGIC = Buffer.from(SSHSIG_MAGIC_PREAMBLE) +const SSHSIG_VERSION = 1 +const NAMESPACE = 'git' + +/** + * Encode a Buffer or string as an SSH wire-format length-prefixed string. + * Format: uint32(len) + bytes + */ +function sshString(data: Buffer | string): Buffer { + const buf = Buffer.isBuffer(data) ? data : Buffer.from(data, 'utf8') + const lenBuf = Buffer.allocUnsafe(4) + lenBuf.writeUInt32BE(buf.length, 0) + return Buffer.concat([lenBuf, buf]) +} + +/** + * Create an SSH signature for a commit payload using the sshsig format. + * + * The returned armored signature is suitable for embedding directly as + * the `gpgsig` header value in a git commit object. + * + * @param payload - The raw commit object text (as passed by isomorphic-git's onSign callback) + * @param key - Parsed SSH key from ssh-key-parser + */ +export function signCommitPayload(payload: string, key: ParsedSSHKey): SSHSignatureResult { + // 1. Hash the commit payload with the spec-mandated SHA-512. + // isomorphic-git passes payload as a string; convert to bytes first. + const messageHash = createHash(SSHSIG_HASH_ALGORITHM).update(Buffer.from(payload, 'utf8')).digest() + + // 2. Build the "signed data" structure per PROTOCOL.sshsig §2 + // This is what the private key actually signs — NOT the raw payload. + const signedData = Buffer.concat([ + SSHSIG_MAGIC, // 6-byte preamble per spec + sshString(NAMESPACE), // "git" + sshString(''), // reserved (empty) + sshString(SSHSIG_HASH_ALGORITHM), // "sha512" + sshString(messageHash), // H(payload) + ]) + + // 3. Sign the signed data with the private key + // Ed25519: sign(null, data, key) — algorithm is implicit in the key + // RSA: sign('sha512', data, key) — must specify hash explicitly + // ECDSA: sign(null, data, key) — algorithm follows the curve + const isRsa = key.keyType === 'ssh-rsa' + const rawSignature = sign(isRsa ? 'sha512' : null, signedData, key.privateKeyObject) + + // 4. Build the SSH signature blob (key-type-specific wrapper) + // Ed25519: string("ssh-ed25519") + string(64-byte-sig) + // RSA: string("rsa-sha2-512") + string(rsa-sig) ← NOT "ssh-rsa" + // ECDSA: string("ecdsa-sha2-nistp256") + string(ecdsa-sig) + const blobKeyType = isRsa ? 'rsa-sha2-512' : key.keyType + const signatureBlob = Buffer.concat([sshString(blobKeyType), sshString(rawSignature)]) + + // 5. Build the full sshsig binary envelope per PROTOCOL.sshsig §3 + const versionBuf = Buffer.allocUnsafe(4) + versionBuf.writeUInt32BE(SSHSIG_VERSION, 0) + + const sshsigBinary = Buffer.concat([ + SSHSIG_MAGIC, // magic preamble + versionBuf, // version = 1 + sshString(key.publicKeyBlob), // public key blob + sshString(NAMESPACE), // "git" + sshString(''), // reserved + sshString(SSHSIG_HASH_ALGORITHM), // "sha512" + sshString(signatureBlob), // wrapped signature + ]) + + // 6. Armor with PEM-style headers (76-char line wrapping, as ssh-keygen does) + const base64 = sshsigBinary.toString('base64') + const lines = base64.match(/.{1,76}/g) ?? [base64] + const armored = ['-----BEGIN SSH SIGNATURE-----', ...lines, '-----END SSH SIGNATURE-----'].join( + '\n', + ) + + return {armored, raw: sshsigBinary} +} diff --git a/src/server/infra/ssh/types.ts b/src/server/infra/ssh/types.ts new file mode 100644 index 000000000..0ff35cb1a --- /dev/null +++ b/src/server/infra/ssh/types.ts @@ -0,0 +1,9 @@ +// Re-exported from shared so server modules continue to work via their existing import paths. +export type { + ParsedSSHKey, + SSHKeyProbe, + SSHKeyProbeResult, + SSHKeyProbeResultFound, + SSHKeyType, + SSHSignatureResult, +} from '../../../shared/ssh/types.js' diff --git a/src/server/infra/transport/handlers/index.ts b/src/server/infra/transport/handlers/index.ts index c5fdf33fa..ca9b2b118 100644 --- a/src/server/infra/transport/handlers/index.ts +++ b/src/server/infra/transport/handlers/index.ts @@ -26,6 +26,8 @@ export {ResetHandler} from './reset-handler.js' export type {ResetHandlerDeps} from './reset-handler.js' export {ReviewHandler} from './review-handler.js' export type {ReviewHandlerDeps} from './review-handler.js' +export {SigningKeyHandler} from './signing-key-handler.js' +export type {SigningKeyHandlerDeps} from './signing-key-handler.js' export {SourceHandler} from './source-handler.js' export type {SourceHandlerDeps} from './source-handler.js' export {SpaceHandler} from './space-handler.js' diff --git a/src/server/infra/transport/handlers/signing-key-handler.ts b/src/server/infra/transport/handlers/signing-key-handler.ts new file mode 100644 index 000000000..209b01fde --- /dev/null +++ b/src/server/infra/transport/handlers/signing-key-handler.ts @@ -0,0 +1,91 @@ +import type {ITokenStore} from '../../../core/interfaces/auth/i-token-store.js' +import type {ISigningKeyService, SigningKeyResource} from '../../../core/interfaces/services/i-signing-key-service.js' +import type {ITransportServer} from '../../../core/interfaces/transport/i-transport-server.js' + +import { + type IVcSigningKeyRequest, + type IVcSigningKeyResponse, + type SigningKeyItem, + VcEvents, +} from '../../../../shared/transport/events/vc-events.js' +import {NotAuthenticatedError} from '../../../core/domain/errors/task-error.js' +import {AuthenticatedHttpClient} from '../../http/authenticated-http-client.js' +import {HttpSigningKeyService} from '../../iam/http-signing-key-service.js' + +export type SigningKeyHandlerDeps = { + iamBaseUrl: string + /** Optional test seam: inject a pre-built service to bypass HTTP client construction. */ + signingKeyService?: ISigningKeyService + tokenStore: ITokenStore + transport: ITransportServer +} + +function toSigningKeyItem(resource: SigningKeyResource): SigningKeyItem { + return { + createdAt: resource.createdAt, + fingerprint: resource.fingerprint, + id: resource.id, + keyType: resource.keyType, + lastUsedAt: resource.lastUsedAt, + publicKey: resource.publicKey, + title: resource.title, + } +} + +/** + * Handles vc:signing-key events from the CLI. + * Creates a fresh authenticated HTTP client per request to use the current session key. + */ +export class SigningKeyHandler { + private readonly iamBaseUrl: string + private readonly injectedService?: ISigningKeyService + private readonly tokenStore: ITokenStore + private readonly transport: ITransportServer + + constructor(deps: SigningKeyHandlerDeps) { + this.iamBaseUrl = deps.iamBaseUrl + this.injectedService = deps.signingKeyService + this.tokenStore = deps.tokenStore + this.transport = deps.transport + } + + setup(): void { + this.transport.onRequest( + VcEvents.SIGNING_KEY, + async (data) => { + const signingKeyService = await this.createService() + return this.handle(signingKeyService, data) + }, + ) + } + + private async createService(): Promise { + const token = await this.tokenStore.load() + if (!token?.isValid()) throw new NotAuthenticatedError() + if (this.injectedService) return this.injectedService + const httpClient = new AuthenticatedHttpClient(token.sessionKey) + return new HttpSigningKeyService(httpClient, this.iamBaseUrl) + } + + private async handle( + service: ISigningKeyService, + data: IVcSigningKeyRequest, + ): Promise { + switch (data.action) { + case 'add': { + const key = await service.addKey(data.title, data.publicKey) + return {action: 'add', key: toSigningKeyItem(key)} + } + + case 'list': { + const keys = await service.listKeys() + return {action: 'list', keys: keys.map((k) => toSigningKeyItem(k))} + } + + case 'remove': { + await service.removeKey(data.keyId) + return {action: 'remove'} + } + } + } +} diff --git a/src/server/infra/transport/handlers/vc-handler.ts b/src/server/infra/transport/handlers/vc-handler.ts index df86a4e1d..abfe6ea7f 100644 --- a/src/server/infra/transport/handlers/vc-handler.ts +++ b/src/server/infra/transport/handlers/vc-handler.ts @@ -1,5 +1,7 @@ +import {execFile} from 'node:child_process' import fs from 'node:fs' -import {join} from 'node:path' +import path, {join} from 'node:path' +import {promisify} from 'node:util' import type {ITokenStore} from '../../../core/interfaces/auth/i-token-store.js' import type {IContextTreeService} from '../../../core/interfaces/context-tree/i-context-tree-service.js' @@ -59,6 +61,16 @@ import {NotAuthenticatedError} from '../../../core/domain/errors/task-error.js' import {VcError} from '../../../core/domain/errors/vc-error.js' import {ensureContextTreeGitignore, ensureGitignoreEntries} from '../../../utils/gitignore.js' import {buildCogitRemoteUrl, isValidBranchName, parseUserFacingUrl} from '../../git/cogit-url.js' +import { + isPassphraseError, + type ParsedSSHKey, + parseSSHPrivateKey, + probeSSHKey, + resolveHome, + signCommitPayload, + SigningKeyCache, + tryGetSshAgentSigner, +} from '../../ssh/index.js' import {type ProjectBroadcaster, type ProjectPathResolver, resolveRequiredProjectPath} from './handler-types.js' /** @@ -83,9 +95,12 @@ function classifyIsomorphicGitError(error: unknown, notFoundCode: VcErrorCodeTyp return undefined } -const FIELD_MAP: Record = { +// FIELD_MAP maps vc config keys to IVcGitConfig field names +const FIELD_MAP: Record = { + 'commit.sign': 'commitSign', 'user.email': 'email', 'user.name': 'name', + 'user.signingkey': 'signingKey', } export interface IVcHandlerDeps { @@ -95,6 +110,7 @@ export interface IVcHandlerDeps { gitService: IGitService projectConfigStore: IProjectConfigStore resolveProjectPath: ProjectPathResolver + signingKeyCache?: SigningKeyCache spaceService: ISpaceService teamService: ITeamService tokenStore: ITokenStore @@ -113,6 +129,7 @@ export class VcHandler { private readonly gitService: IGitService private readonly projectConfigStore: IProjectConfigStore private readonly resolveProjectPath: ProjectPathResolver + private readonly signingKeyCache: SigningKeyCache private readonly spaceService: ISpaceService private readonly teamService: ITeamService private readonly tokenStore: ITokenStore @@ -127,6 +144,7 @@ export class VcHandler { this.gitService = deps.gitService this.projectConfigStore = deps.projectConfigStore this.resolveProjectPath = deps.resolveProjectPath + this.signingKeyCache = deps.signingKeyCache ?? new SigningKeyCache() this.spaceService = deps.spaceService this.teamService = deps.teamService this.tokenStore = deps.tokenStore @@ -189,21 +207,6 @@ export class VcHandler { this.transport.onRequest(VcEvents.STATUS, (_data, clientId) => this.handleStatus(clientId)) } - private async buildAuthorHint(existing?: IVcGitConfig): Promise { - try { - const token = await this.tokenStore.load() - if (token?.isValid()) { - const email = existing?.email ?? token.userEmail - const name = existing?.name ?? token.userName ?? token.userEmail - return `Run: brv vc config user.name '${name}' and brv vc config user.email '${email}'.` - } - } catch { - // not logged in - } - - return 'Run: brv vc config user.name and brv vc config user.email .' - } - private buildNoRemoteMessage(nextStep: string): string { return ( `No remote configured.\n\nTo connect to cloud:\n` + @@ -638,31 +641,153 @@ export class VcHandler { await this.guardUnmergedAndMarkers(directory) const config = await this.vcGitConfigStore.get(projectPath) - if (!config?.name || !config.email) { - const hint = await this.buildAuthorHint(config) - throw new VcError(`Commit author not configured. ${hint}`, VcErrorCode.USER_NOT_CONFIGURED) + const author = await this.resolveAuthor(config) + + // ── Option C SSH Signing ──────────────────────────────────────────────── + // + // Determine whether to sign this commit: + // data.sign=true → force sign + // data.sign=false → force no-sign + // undefined → use config.commitSign (false by default — matches git behaviour where + // setting user.signingKey alone does NOT enable auto-signing) + const shouldSign = data.sign ?? config?.commitSign ?? false + + let onSign: ((payload: string) => Promise) | undefined + + if (shouldSign) { + const keyPath = config?.signingKey ? resolveHome(config.signingKey) : undefined + + if (!keyPath) { + throw new VcError( + 'Signing key not configured. Run: brv vc config user.signingkey ~/.ssh/id_ed25519', + VcErrorCode.SIGNING_KEY_NOT_CONFIGURED, + ) + } + + // Path A: try SSH agent first (zero-prompt, works even for encrypted keys) + const agentSigner = await tryGetSshAgentSigner(keyPath) + if (agentSigner) { + onSign = async (payload: string) => { + const result = await agentSigner.sign(payload) + return result.armored + } + } else { + // Path B/C: cache or file-based parsing + const parsed = await this.resolveSigningKey(projectPath, keyPath, data.passphrase) + onSign = async (payload: string) => { + const result = signCommitPayload(payload, parsed) + return result.armored + } + } } const commit = await this.gitService.commit({ - author: {email: config.email, name: config.name}, + author, directory, message: data.message, + ...(onSign ? {onSign} : {}), }) - return {message: commit.message, sha: commit.sha} + return {message: commit.message, sha: commit.sha, ...(shouldSign ? {signed: true} : {})} } private async handleConfig(data: IVcConfigRequest, clientId: string): Promise { const projectPath = resolveRequiredProjectPath(this.resolveProjectPath, clientId) - const field = FIELD_MAP[data.key] + // ── --import-git-signing branch ───────────────────────────────────────── + // Reads `user.signingKey` and `commit.gpgSign` from local/global git config + // and writes them into the brv VcGitConfigStore. + if (data.importGitSigning) { + return this.handleImportGitSigning(projectPath) + } + + const field = FIELD_MAP[data.key.toLowerCase()] if (!field) { - throw new VcError(`Unknown key '${data.key}'. Allowed: user.name, user.email.`, VcErrorCode.INVALID_CONFIG_KEY) + throw new VcError( + `Unknown key '${data.key}'. Allowed: user.name, user.email, user.signingkey, commit.sign.`, + VcErrorCode.INVALID_CONFIG_KEY, + ) } if (data.value !== undefined) { - // SET: read existing → merge single field → write back + // SET: read existing → coerce value → merge → write back const existing = (await this.vcGitConfigStore.get(projectPath)) ?? {} + + // Coerce 'commit.sign' value to boolean + if (field === 'commitSign') { + const boolValue = data.value === 'true' || data.value === '1' + if (data.value !== 'true' && data.value !== 'false' && data.value !== '1' && data.value !== '0') { + throw new VcError( + `Invalid value for commit.sign: '${data.value}'. Use true, false, 1, or 0.`, + VcErrorCode.INVALID_CONFIG_VALUE, + ) + } + + await this.vcGitConfigStore.set(projectPath, {...existing, commitSign: boolValue}) + return {key: data.key, value: String(boolValue)} + } + + // Coerce 'user.signingkey': resolve ~ to home and validate file exists + if (field === 'signingKey') { + let resolvedPath = resolveHome(data.value) + if (resolvedPath.endsWith('.pub')) { + resolvedPath = resolvedPath.slice(0, -4) + } + + // Reject relative paths — they silently resolve against the daemon's + // CWD and would break the next time the daemon starts from a + // different directory. Matches git's own `user.signingKey` semantic. + if (!path.isAbsolute(resolvedPath)) { + throw new VcError( + `Signing key path must be absolute. Got: ${resolvedPath}`, + VcErrorCode.INVALID_CONFIG_VALUE, + ) + } + + const probe = await probeSSHKey(resolvedPath) + if (!probe.exists) { + throw new VcError( + `SSH key not found at: ${resolvedPath}`, + VcErrorCode.SIGNING_KEY_NOT_FOUND, + ) + } + + // Derive fingerprint for display hint (non-blocking if parse fails) + let hint: string | undefined + if (probe.opensshEncrypted) { + const agent = await tryGetSshAgentSigner(resolvedPath) + if (!agent) { + hint = 'Warning: Key is encrypted OpenSSH format. You must load it into ssh-agent to sign commits.' + } + } else { + try { + const parsed = await parseSSHPrivateKey(resolvedPath) + // Cache the parsed key for immediate use + this.signingKeyCache.set(projectPath, resolvedPath, parsed) + hint = `Fingerprint: ${parsed.fingerprint}` + } catch (error) { + // Encrypted key — need passphrase for fingerprint; skip hint silently. + // Anything else (corrupt file, unexpected format, race between probe + // and parse) surfaces as a hint so the user gets a breadcrumb at + // config time rather than a surprise at `brv vc commit --sign`. + if (!isPassphraseError(error)) { + hint = `Could not compute fingerprint: ${error instanceof Error ? error.message : String(error)}` + } + } + } + + // Evict any cached ParsedSSHKey bound to the PREVIOUS key path — + // otherwise a stale entry keeps the old decrypted key object in daemon + // memory until its 30-min TTL expires. + if (existing.signingKey && existing.signingKey !== resolvedPath) { + this.signingKeyCache.invalidate(projectPath, existing.signingKey) + } + + await this.vcGitConfigStore.set(projectPath, {...existing, signingKey: resolvedPath}) + return {hint, key: data.key, value: resolvedPath} + } + + // Regular string fields (user.name, user.email) const merged = {...existing, [field]: data.value} await this.vcGitConfigStore.set(projectPath, merged) return {key: data.key, value: data.value} @@ -670,12 +795,12 @@ export class VcHandler { // GET const config = await this.vcGitConfigStore.get(projectPath) - const value = config?.[field] - if (value === undefined) { + const rawValue = config?.[field] + if (rawValue === undefined) { throw new VcError(`'${data.key}' is not set.`, VcErrorCode.CONFIG_KEY_NOT_SET) } - return {key: data.key, value} + return {key: data.key, value: String(rawValue)} } private async handleDiff(data: IVcDiffRequest, clientId: string): Promise { @@ -788,6 +913,86 @@ export class VcHandler { return {remote} } + /** + * Import SSH signing config from local/global git config. + * Reads these git keys (falls back local → global): + * user.signingKey → brv user.signingkey + * commit.gpgSign → brv commit.sign + * gpg.format → must be 'ssh' (warns otherwise) + */ + private async handleImportGitSigning(projectPath: string): Promise { + const execFileAsync = promisify(execFile) + + async function readGitConfig(key: string): Promise { + try { + const {stdout} = await execFileAsync('git', ['config', '--get', key], {cwd: projectPath}) + return stdout.trim() || undefined + } catch { + return undefined + } + } + + const signingKey = await readGitConfig('user.signingKey') + const gpgFormat = await readGitConfig('gpg.format') + const gpgSign = await readGitConfig('commit.gpgSign') + + if (!signingKey) { + throw new VcError( + 'No user.signingKey found in git config. Configure it with: git config user.signingKey ~/.ssh/id_ed25519', + VcErrorCode.SIGNING_KEY_NOT_FOUND, + ) + } + + if (gpgFormat && gpgFormat !== 'ssh') { + throw new VcError( + `git config gpg.format is '${gpgFormat}', not 'ssh'. brv only supports SSH commit signing.`, + VcErrorCode.INVALID_CONFIG_VALUE, + ) + } + + let resolvedPath = resolveHome(signingKey) + if (resolvedPath.endsWith('.pub')) { + resolvedPath = resolvedPath.slice(0, -4) + } + + // Reject relative paths imported from git config. See handleConfig for rationale. + if (!path.isAbsolute(resolvedPath)) { + throw new VcError( + `Signing key path from git config must be absolute. Got: ${resolvedPath}`, + VcErrorCode.INVALID_CONFIG_VALUE, + ) + } + + const probe = await probeSSHKey(resolvedPath) + if (!probe.exists) { + throw new VcError( + `SSH key from git config not found at: ${resolvedPath}`, + VcErrorCode.SIGNING_KEY_NOT_FOUND, + ) + } + + // Read and merge into brv config + const existing = (await this.vcGitConfigStore.get(projectPath)) ?? {} + const commitSign = gpgSign === 'true' || gpgSign === '1' + const updated: typeof existing = {...existing, commitSign, signingKey: resolvedPath} + await this.vcGitConfigStore.set(projectPath, updated) + + let hint = `commit.sign: ${String(commitSign)}` + try { + const parsed = await parseSSHPrivateKey(resolvedPath) + this.signingKeyCache.set(projectPath, resolvedPath, parsed) + hint = `Fingerprint: ${parsed.fingerprint} | commit.sign: ${String(commitSign)}` + } catch (error) { + // Encrypted key: fingerprint unavailable without passphrase, that's fine. + // Anything else: surface alongside commit.sign so the user notices now. + if (!isPassphraseError(error)) { + hint = `${hint} | Could not compute fingerprint: ${error instanceof Error ? error.message : String(error)}` + } + } + + return {hint, key: 'user.signingkey', value: resolvedPath} + } + private async handleInit(clientId: string): Promise { const projectPath = resolveRequiredProjectPath(this.resolveProjectPath, clientId) @@ -883,14 +1088,11 @@ export class VcHandler { await this.guardUnmergedAndMarkers(directory) - const config = await this.vcGitConfigStore.get(projectPath) - if (!config?.name || !config.email) { - const hint = await this.buildAuthorHint(config) - throw new VcError(`Commit author not configured. ${hint}`, VcErrorCode.USER_NOT_CONFIGURED) - } + const mergeConfig = await this.vcGitConfigStore.get(projectPath) + const mergeAuthor = await this.resolveAuthor(mergeConfig) await this.gitService.commit({ - author: {email: config.email, name: config.name}, + author: mergeAuthor, directory, message: data.message, }) @@ -925,14 +1127,11 @@ export class VcHandler { } const config = await this.vcGitConfigStore.get(projectPath) - if (!config?.name || !config.email) { - const hint = await this.buildAuthorHint(config) - throw new VcError(`Commit author not configured. ${hint}`, VcErrorCode.USER_NOT_CONFIGURED) - } + const mergeStartAuthor = await this.resolveAuthor(config) const result = await this.gitService.merge({ allowUnrelatedHistories: data.allowUnrelatedHistories, - author: {email: config.email, name: config.name}, + author: mergeStartAuthor, branch: data.branch, directory, message: data.message, @@ -970,10 +1169,14 @@ export class VcHandler { throw new VcError(this.buildNoRemoteMessage('brv vc pull origin main'), VcErrorCode.NO_REMOTE) } - // Soft resolve author: use vc config if available, otherwise let pull() fallback to getAuthor() from auth token. - // Unlike commit/merge, pull only needs author when creating a merge commit (not for up-to-date or fast-forward). + // Resolve author for merge commits during pull (fallback to auth token if config not set) const config = await this.vcGitConfigStore.get(projectPath) - const author = config?.name && config?.email ? {email: config.email, name: config.name} : undefined + let author: undefined | {email: string; name: string} + try { + author = await this.resolveAuthor(config) + } catch { + // Fallback: proceed without author if resolution fails + } // If explicit branch provided, use it directly (skip tracking resolution) const remote = data?.remote ?? 'origin' @@ -1312,6 +1515,32 @@ export class VcHandler { } } + /** + * Resolve commit author: config values first, then fallback to auth token. + * Throws USER_NOT_CONFIGURED only when neither source has name+email. + */ + private async resolveAuthor(config?: IVcGitConfig): Promise<{email: string; name: string}> { + if (config?.name && config.email) { + return {email: config.email, name: config.name} + } + + try { + const token = await this.tokenStore.load() + if (token?.isValid()) { + const email = config?.email ?? token.userEmail + const name = config?.name ?? token.userName ?? token.userEmail + if (email && name) return {email, name} + } + } catch { + // not logged in + } + + throw new VcError( + 'Commit author not configured. Run: brv vc config user.name and brv vc config user.email .', + VcErrorCode.USER_NOT_CONFIGURED, + ) + } + /** * Resolve clone request data into a clean cogit URL + team/space info. * Accepts either a URL or explicit teamName/spaceName. @@ -1445,6 +1674,57 @@ export class VcHandler { throw new VcError('Cannot determine branch for pull. Check out a branch first.', VcErrorCode.NO_BRANCH_RESOLVED) } + /** + * File-based signing key resolution (paths B and C only). + * Path A (ssh-agent) is handled in handleCommit before calling this method. + * + * Path B: in-memory TTL cache (passphrase not needed after first use) + * Path C: parse key file (may require passphrase — delegates to CLI caller via PASSPHRASE_REQUIRED error) + */ + private async resolveSigningKey( + projectPath: string, + keyPath: string, + passphrase?: string, + ): Promise { + // Path B: in-memory TTL cache — scoped by (project, key) pair so a user + // switching projects never inherits a decrypted KeyObject from elsewhere. + const cachedKey = this.signingKeyCache.get(projectPath, keyPath) + if (cachedKey) return cachedKey + + // Path C: parse key file + const probe = await probeSSHKey(keyPath) + if (!probe.exists) { + throw new VcError( + `SSH key file not found: ${keyPath}`, + VcErrorCode.SIGNING_KEY_NOT_FOUND, + ) + } + + if (probe.needsPassphrase) { + // Encrypted OpenSSH native format (bcrypt KDF + AES cipher) is not supported for + // direct decryption. Tell the user to load the key into ssh-agent instead. + if (probe.opensshEncrypted) { + throw new VcError( + `Encrypted OpenSSH private keys are not supported for direct signing. ` + + `Load the key into ssh-agent first: ssh-add ${keyPath}`, + VcErrorCode.SIGNING_KEY_NOT_SUPPORTED, + ) + } + + if (!passphrase) { + throw new VcError( + `SSH key at ${keyPath} requires a passphrase. Retry with your passphrase.`, + VcErrorCode.PASSPHRASE_REQUIRED, + ) + } + } + + const parsed = await parseSSHPrivateKey(keyPath, passphrase) + // Store in cache (passphrase is not stored — only the decrypted KeyObject) + this.signingKeyCache.set(projectPath, keyPath, parsed) + return parsed + } + private async resolveTargetBranch(requestedBranch: string | undefined, directory: string): Promise { const trimmed = requestedBranch?.trim() diff --git a/src/server/infra/vc/file-vc-git-config-store.ts b/src/server/infra/vc/file-vc-git-config-store.ts index 844ea17a4..6fba36116 100644 --- a/src/server/infra/vc/file-vc-git-config-store.ts +++ b/src/server/infra/vc/file-vc-git-config-store.ts @@ -21,7 +21,12 @@ function projectKey(projectPath: string): string { function isIVcGitConfig(value: unknown): value is IVcGitConfig { if (typeof value !== 'object' || value === null) return false const v = value as Record - return (v.name === undefined || typeof v.name === 'string') && (v.email === undefined || typeof v.email === 'string') + return ( + (v.name === undefined || typeof v.name === 'string') && + (v.email === undefined || typeof v.email === 'string') && + (v.signingKey === undefined || typeof v.signingKey === 'string') && + (v.commitSign === undefined || typeof v.commitSign === 'boolean') + ) } export class FileVcGitConfigStore implements IVcGitConfigStore { @@ -46,7 +51,7 @@ export class FileVcGitConfigStore implements IVcGitConfigStore { public async set(projectPath: string, config: IVcGitConfig): Promise { const projectDir = join(this.deps.getDataDir(), 'projects', projectKey(projectPath)) await mkdir(projectDir, {recursive: true}) - await writeFile(join(projectDir, 'vc-git-config.json'), JSON.stringify(config, null, 2), 'utf8') + await writeFile(join(projectDir, 'vc-git-config.json'), JSON.stringify(config, null, 2), {encoding: 'utf8', mode: 0o600}) } private configPath(projectPath: string): string { diff --git a/src/shared/ssh/index.ts b/src/shared/ssh/index.ts new file mode 100644 index 000000000..d47a8a1c1 --- /dev/null +++ b/src/shared/ssh/index.ts @@ -0,0 +1,10 @@ +export { + computeFingerprint, + extractPublicKey, + getPublicKeyMetadata, + parseSSHPrivateKey, + probeSSHKey, + resolveHome, +} from './key-parser.js' + +export type {ParsedSSHKey, SSHKeyProbe, SSHKeyType, SSHSignatureResult} from './types.js' diff --git a/src/shared/ssh/key-parser.ts b/src/shared/ssh/key-parser.ts new file mode 100644 index 000000000..a896eecc1 --- /dev/null +++ b/src/shared/ssh/key-parser.ts @@ -0,0 +1,502 @@ +import {createHash, createPrivateKey, createPublicKey} from 'node:crypto' +import {constants} from 'node:fs' +import {access, readFile} from 'node:fs/promises' +import {homedir} from 'node:os' +import path from 'node:path' + +import type {ParsedSSHKey, SSHKeyProbe, SSHKeyType} from './types.js' + +// ── OpenSSH private key format parser ──────────────────────────────────────── +// Spec: https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.key +// +// Binary layout: +// "openssh-key-v1\0" (magic) +// string ciphername ("none" if unencrypted) +// string kdfname ("none" if unencrypted) +// string kdfoptions (empty if unencrypted) +// uint32 nkeys (number of keys, usually 1) +// string pubkey (SSH wire-format public key) +// string private_keys (encrypted or plaintext private key data) +// +// Private key data (plaintext, nkeys=1): +// uint32 check1 +// uint32 check2 (must equal check1) +// string keytype (e.g., "ssh-ed25519") +// [key-type-specific private key fields] +// string comment +// [padding bytes: 1,2,3,...] + +const OPENSSH_MAGIC = 'openssh-key-v1\0' + +/** Encode a value as an SSH wire-format length-prefixed string. */ +function sshStr(data: Buffer | string): Buffer { + const b = Buffer.isBuffer(data) ? data : Buffer.from(data) + const len = Buffer.allocUnsafe(4) + len.writeUInt32BE(b.length, 0) + return Buffer.concat([len, b]) +} + +const VALID_SSH_KEY_TYPES: ReadonlySet = new Set([ + 'ecdsa-sha2-nistp256', + 'ecdsa-sha2-nistp384', + 'ecdsa-sha2-nistp521', + 'ssh-ed25519', + 'ssh-rsa', +]) + +/** Type predicate — true when `s` is one of the SSH key types we accept. */ +export function isValidSSHKeyType(s: string): s is SSHKeyType { + return VALID_SSH_KEY_TYPES.has(s) +} + +/** Read a uint32 big-endian from a buffer at offset; returns [value, newOffset] */ +function readUInt32(buf: Buffer, offset: number): [number, number] { + return [buf.readUInt32BE(offset), offset + 4] +} + +/** Read an SSH wire-format length-prefixed string; returns [Buffer, newOffset] */ +function readSSHString(buf: Buffer, offset: number): [Buffer, number] { + const [len, afterLen] = readUInt32(buf, offset) + return [buf.subarray(afterLen, afterLen + len), afterLen + len] +} + +/** Parse the binary OpenSSH private key format (unencrypted only). */ +function parseOpenSSHKey(raw: string): { + cipherName: string + keyType: SSHKeyType + privateKeyBlob: Buffer + publicKeyBlob: Buffer +} { + // Strip PEM armor + const b64 = raw + .replace('-----BEGIN OPENSSH PRIVATE KEY-----', '') + .replace('-----END OPENSSH PRIVATE KEY-----', '') + .replaceAll(/\s+/g, '') + const buf = Buffer.from(b64, 'base64') + + // Verify magic + const magic = buf.subarray(0, OPENSSH_MAGIC.length).toString() + if (magic !== OPENSSH_MAGIC) { + throw new Error('Not an OpenSSH private key (wrong magic bytes)') + } + + let offset = OPENSSH_MAGIC.length + + // ciphername + let cipherNameBuf: Buffer + ;[cipherNameBuf, offset] = readSSHString(buf, offset) + const cipherName = cipherNameBuf.toString() + + // kdfname (skip value — only offset matters) + ;[, offset] = readSSHString(buf, offset) + + // kdfoptions (skip value) + ;[, offset] = readSSHString(buf, offset) + + // nkeys + let nkeys: number + ;[nkeys, offset] = readUInt32(buf, offset) + if (nkeys !== 1) { + throw new Error(`OpenSSH key file contains ${nkeys} keys; only single-key files are supported`) + } + + // public key blob (SSH wire format) + let publicKeyBlob: Buffer + ;[publicKeyBlob, offset] = readSSHString(buf, offset) + + // private key blob (may be encrypted) + let privateKeyBlob: Buffer + ;[privateKeyBlob, offset] = readSSHString(buf, offset) + + // Read key type from public key blob to identify the key + const [keyTypeBuf] = readSSHString(publicKeyBlob, 0) + const keyTypeStr = keyTypeBuf.toString() + if (!isValidSSHKeyType(keyTypeStr)) { + throw new Error(`Unknown SSH key type: '${keyTypeStr}'`) + } + + return {cipherName, keyType: keyTypeStr, privateKeyBlob, publicKeyBlob} +} + +/** + * Convert an OpenSSH Ed25519 private key blob to a Node.js-loadable format. + * + * Ed25519 private key blob layout (plaintext): + * uint32 check1 + * uint32 check2 + * string "ssh-ed25519" + * string pubkey (32 bytes) + * string privkey (64 bytes: 32-byte seed + 32-byte pubkey) + * string comment + * padding bytes + */ +function opensshEd25519ToNodeKey(privateKeyBlob: Buffer): { + privateKeyPkcs8: Buffer + publicKeyBlob: Buffer +} { + let offset = 0 + + // check1 and check2 must match (used to verify decryption) + const [check1] = readUInt32(privateKeyBlob, offset) + offset += 4 + const [check2] = readUInt32(privateKeyBlob, offset) + offset += 4 + + if (check1 !== check2) { + throw new Error('OpenSSH key decryption check failed (wrong passphrase?)') + } + + // key type + let keyTypeBuf: Buffer + ;[keyTypeBuf, offset] = readSSHString(privateKeyBlob, offset) + const keyType = keyTypeBuf.toString() + + if (keyType !== 'ssh-ed25519') { + throw new Error(`Expected ssh-ed25519 key type, got: ${keyType}`) + } + + // public key (32 bytes) + let pubKeyBytes: Buffer + ;[pubKeyBytes, offset] = readSSHString(privateKeyBlob, offset) + + // private key (64 bytes: first 32 = seed) + let privKeyBytes: Buffer + ;[privKeyBytes, offset] = readSSHString(privateKeyBlob, offset) + + // The Ed25519 "private key" in OpenSSH format is the 64-byte concatenation + // of: seed (32 bytes) + public key (32 bytes). + // Node.js needs a DER-encoded ASN.1 PKCS8 structure for Ed25519. + // + // PKCS8 for Ed25519: + // SEQUENCE { + // INTEGER 0 (version) + // SEQUENCE { OID 1.3.101.112 (id-EdDSA) } + // OCTET STRING wrapping OCTET STRING (32-byte seed) + // } + const seed = privKeyBytes.subarray(0, 32) + + // ASN.1 encoding for Ed25519 private key in PKCS8 format + // This is the known fixed ASN.1 header for Ed25519 PKCS8 + const pkcs8Header = Buffer.from('302e020100300506032b657004220420', 'hex') + const pkcs8Der = Buffer.concat([pkcs8Header, seed]) + + // SSH wire format public key blob: string("ssh-ed25519") + string(32-byte-pubkey) + const publicKeyBlob = Buffer.concat([sshStr('ssh-ed25519'), sshStr(pubKeyBytes)]) + + return {privateKeyPkcs8: pkcs8Der, publicKeyBlob} +} + +/** Detect whether a PEM key parsing error indicates an encrypted key needing a passphrase. */ +export function isPassphraseError(err: unknown): boolean { + if (!(err instanceof Error)) return false + + // Node.js crypto errors expose an `code` property. Whitelist exactly the two + // OpenSSL codes that mean "passphrase is the problem": + // - ERR_OSSL_BAD_DECRYPT — wrong / empty-string passphrase on encrypted PEM + // - ERR_OSSL_CRYPTO_INTERRUPTED_OR_CANCELLED — encrypted PEM with NO passphrase + // argument (OpenSSL aborts the read instead + // of erroring; misleadingly named) + // A broader prefix match like `code.startsWith('ERR_OSSL')` would false-match + // `ERR_OSSL_UNSUPPORTED` (malformed/unparseable PEM) and other format-level + // failures, causing probeSSHKey to incorrectly return needsPassphrase:true. + const code = 'code' in err && typeof err.code === 'string' ? err.code : '' + if (code === 'ERR_OSSL_BAD_DECRYPT' || code === 'ERR_OSSL_CRYPTO_INTERRUPTED_OR_CANCELLED') { + return true + } + + // Fallback: string matching for compatibility across Node.js/OpenSSL versions. + // + // Do NOT add `/unsupported/` here — several callers (parseOpenSSHKey, + // parseSSHPrivateKey) throw `"Unsupported OpenSSH key type: ..."` / + // `"Unsupported PEM key type: ..."` for keys this build cannot parse natively. + // Matching that text would re-introduce the same false-positive that the + // code-based whitelist above is guarding against. + const msg = err.message.toLowerCase() + return /bad decrypt|passphrase|bad password|interrupted or cancelled/.test(msg) +} + +/** + * Translate a Node.js crypto error into a user-grade Error when the code + * matches a known cause. Otherwise returns the original error untouched so + * unrecognised failures still surface for debugging. + * + * Purpose: the raw OpenSSL messages (`error:1C800064:Provider routines::bad + * decrypt`, `error:1E08010C:DECODER routines::unsupported`) are unreadable to + * end users. AC9-b and AC9-c of ENG-2002 require actionable English. + */ +function formatCryptoError(err: unknown, keyPath: string): Error { + if (!(err instanceof Error)) return new Error(String(err)) + + const code = 'code' in err && typeof err.code === 'string' ? err.code : '' + + if (code === 'ERR_OSSL_BAD_DECRYPT') { + return new Error(`Wrong passphrase for SSH key at ${keyPath}.`) + } + + if (code === 'ERR_OSSL_UNSUPPORTED') { + return new Error( + `File at ${keyPath} is not a valid SSH private key ` + + `(unrecognised or malformed PEM / PKCS8 body).`, + ) + } + + return err +} + +// ── Public API ───────────────────────────────────────────────────────────── + +/** + * Check if a key file exists and whether it requires a passphrase. + * Does NOT load the private key material beyond the initial probe. + */ +export async function probeSSHKey(keyPath: string): Promise { + try { + await access(keyPath, constants.R_OK) + } catch { + return {exists: false} + } + + try { + const raw = await readFile(keyPath, 'utf8') + + if (raw.includes('BEGIN OPENSSH PRIVATE KEY')) { + // OpenSSH format: check cipherName field + const parsed = parseOpenSSHKey(raw) + + if (parsed.keyType !== 'ssh-ed25519') { + throw new Error(`Unsupported OpenSSH key type: ${parsed.keyType}. Only ssh-ed25519 is supported natively. Please load this key into ssh-agent instead.`) + } + + const needsPassphrase = parsed.cipherName !== 'none' + return { + exists: true, + needsPassphrase, + ...(needsPassphrase ? {opensshEncrypted: true} : {}), + } + } + + // PEM/PKCS8 format (RSA, ECDSA with traditional headers) + const pk = createPrivateKey({format: 'pem', key: raw}) + if (pk.asymmetricKeyType !== 'ed25519') { + throw new Error(`Unsupported PEM key type: ${pk.asymmetricKeyType}. Only ed25519 is supported natively. Please load this key into ssh-agent instead.`) + } + + return {exists: true, needsPassphrase: false} + } catch (error: unknown) { + if (isPassphraseError(error)) { + return {exists: true, needsPassphrase: true} + } + + throw formatCryptoError(error, keyPath) + } +} + +/** + * Parse an SSH private key file into a usable signing key. + * Supports: + * - OpenSSH native format (Ed25519 only in v1; RSA/ECDSA to follow) + * - Standard PEM (PKCS#8, traditional RSA/ECDSA) + * + * Throws if passphrase is required but not provided, or if format is unsupported. + */ +export async function parseSSHPrivateKey( + keyPath: string, + passphrase?: string, +): Promise { + const raw = await readFile(keyPath, 'utf8') + + // ── OpenSSH native format ────────────────────────────────────────────── + if (raw.includes('BEGIN OPENSSH PRIVATE KEY')) { + const {cipherName, keyType, privateKeyBlob} = parseOpenSSHKey(raw) + + if (cipherName !== 'none') { + // Encrypted OpenSSH keys require AES-256-CTR + bcrypt KDF decryption (out of scope for v1). + // resolveSigningKey short-circuits on opensshEncrypted before reaching here, + // so this is a safety net for direct callers. + throw new Error( + 'Encrypted OpenSSH private keys are not yet supported. ' + + 'Please use an unencrypted key or load it via ssh-agent.', + ) + } + + if (keyType !== 'ssh-ed25519') { + throw new Error( + `Unsupported OpenSSH key type: ${keyType}. Only ssh-ed25519 is supported in v1.`, + ) + } + + const {privateKeyPkcs8, publicKeyBlob: sshPublicKeyBlob} = + opensshEd25519ToNodeKey(privateKeyBlob) + + const privateKeyObject = createPrivateKey({ + format: 'der', + key: privateKeyPkcs8, + type: 'pkcs8', + }) + + const fingerprint = computeFingerprint(sshPublicKeyBlob) + + return { + fingerprint, + keyType, + privateKeyObject, + publicKeyBlob: sshPublicKeyBlob, + } + } + + // ── Standard PEM format (PKCS8, RSA, ECDSA) ─────────────────────────── + let privateKeyObject + try { + privateKeyObject = createPrivateKey({ + format: 'pem', + key: raw, + ...(passphrase ? {passphrase} : {}), + }) + } catch (error: unknown) { + throw formatCryptoError(error, keyPath) + } + + const publicKey = createPublicKey(privateKeyObject) + + // For non-Ed25519 keys in standard PEM, derive SSH wire format manually + const asymKeyType = privateKeyObject.asymmetricKeyType + let publicKeyBlob: Buffer + let keyType: SSHKeyType + + if (asymKeyType === 'ed25519') { + // KeyObject.export has an overload returning string | Buffer depending on + // format. DER output is binary; Buffer.from(string) would silently + // UTF-8-encode and corrupt the DER bytes, so narrow via isBuffer. + const exported = publicKey.export({format: 'der', type: 'spki'}) + if (!Buffer.isBuffer(exported)) { + throw new TypeError('Expected Buffer from DER export of Ed25519 public key') + } + + const derPub = exported + // Ed25519 SPKI DER = 12-byte ASN.1 header + 32-byte raw public key + const rawPubBytes = derPub.subarray(12) + + keyType = 'ssh-ed25519' + publicKeyBlob = Buffer.concat([sshStr('ssh-ed25519'), sshStr(rawPubBytes)]) + } else { + throw new Error(`Unsupported key type for PEM parsing: ${asymKeyType}`) + } + + const fingerprint = computeFingerprint(publicKeyBlob) + + return {fingerprint, keyType, privateKeyObject, publicKeyBlob} +} + +/** Compute SHA256 fingerprint from SSH wire-format public key blob. */ +export function computeFingerprint(publicKeyBlob: Buffer): string { + const hash = createHash('sha256').update(publicKeyBlob).digest('base64').replace(/=+$/, '') + return `SHA256:${hash}` +} + +/** + * Extract the SSH public key blob and type from a private key file without decryption. + * + * Priority: + * 1. `.pub` sidecar file (authorized_keys format) — also captures the comment field + * 2. OpenSSH native private key format — public key lives in the unencrypted file + * header, so this works even when the private key is encrypted (no passphrase needed) + * 3. PEM/PKCS8 format — requires an unencrypted private key + * + * Throws if the file cannot be read or the format is unrecognised. + */ +export async function extractPublicKey(keyPath: string): Promise<{ + comment?: string + keyType: string + publicKeyBlob: Buffer +}> { + // 1. .pub sidecar + const pubPath = `${keyPath}.pub` + try { + const rawPub = await readFile(pubPath, 'utf8') + const parts = rawPub.trim().split(' ') + if (parts.length >= 2) { + const keyType = parts[0] + const publicKeyBlob = Buffer.from(parts[1], 'base64') + const comment = parts.length >= 3 ? parts.slice(2).join(' ') : undefined + return {comment, keyType, publicKeyBlob} + } + } catch { + // No sidecar — fall through + } + + const raw = await readFile(keyPath, 'utf8') + + // 2. OpenSSH native format: public key is in the unencrypted file header + if (raw.includes('BEGIN OPENSSH PRIVATE KEY')) { + const {keyType, publicKeyBlob} = parseOpenSSHKey(raw) + return {keyType, publicKeyBlob} + } + + // 3. PEM/PKCS8 format: requires the private key to be unencrypted + // + // An encrypted PEM without a .pub sidecar has no cheap way to derive the + // public key — the material lives inside the encrypted body. Rather than + // let parseSSHPrivateKey's "Wrong passphrase" (misleading — the caller + // never entered one) or a raw OpenSSL code reach the CLI, translate + // passphrase-class failures into an actionable hint pointing at the + // canonical ssh-keygen workaround. ENG-2002 AC9-b/c. + try { + const parsed = await parseSSHPrivateKey(keyPath) + return {keyType: parsed.keyType, publicKeyBlob: parsed.publicKeyBlob} + } catch (error) { + if (isPassphraseError(error)) { + throw new Error( + `Cannot extract public key from encrypted PEM at ${keyPath}. ` + + `Generate a .pub sidecar first:\n\n ssh-keygen -y -f ${keyPath} > ${keyPath}.pub`, + ) + } + + throw error + } +} + +/** + * Attempt to extract public key metadata (fingerprint and keyType) from a key path, + * checking for a .pub file first, then attempting to parse an OpenSSH private key + * (which contains the public key even if the private key is encrypted). + */ +export async function getPublicKeyMetadata(keyPath: string): Promise { + const pubPath = keyPath.endsWith('.pub') ? keyPath : `${keyPath}.pub` + try { + const rawPub = await readFile(pubPath, 'utf8') + const parts = rawPub.trim().split(' ') + if (parts.length >= 2 && isValidSSHKeyType(parts[0])) { + const blob = Buffer.from(parts[1], 'base64') + return {fingerprint: computeFingerprint(blob), keyType: parts[0]} + } + } catch { + // Ignore error, fallback to private key + } + + try { + const raw = await readFile(keyPath, 'utf8') + if (raw.includes('BEGIN OPENSSH PRIVATE KEY')) { + const parsed = parseOpenSSHKey(raw) + return { + fingerprint: computeFingerprint(parsed.publicKeyBlob), + keyType: parsed.keyType, + } + } + } catch { + return undefined + } + + return undefined +} + +/** + * Resolve ~ to the user's home directory in a key path. + */ +export function resolveHome(keyPath: string): string { + if (keyPath === '~') return homedir() + if (keyPath.startsWith('~/') || keyPath.startsWith('~\\')) { + return path.join(homedir(), keyPath.slice(2)) + } + + return keyPath +} diff --git a/src/shared/ssh/types.ts b/src/shared/ssh/types.ts new file mode 100644 index 000000000..bbcf72a86 --- /dev/null +++ b/src/shared/ssh/types.ts @@ -0,0 +1,39 @@ +import type * as crypto from 'node:crypto' + +export type SSHKeyType = + | 'ecdsa-sha2-nistp256' + | 'ecdsa-sha2-nistp384' + | 'ecdsa-sha2-nistp521' + | 'ssh-ed25519' + | 'ssh-rsa' + +export type ParsedSSHKey = { + /** SHA256 fingerprint — used for display and matching with IAM */ + fingerprint: string + /** Key type identifier (e.g., 'ssh-ed25519') */ + keyType: SSHKeyType + /** Node.js crypto KeyObject — opaque, not extractable */ + privateKeyObject: crypto.KeyObject + /** Raw public key blob in SSH wire format (for embedding in sshsig) */ + publicKeyBlob: Buffer +} + +export type SSHSignatureResult = { + /** Armored SSH signature (-----BEGIN SSH SIGNATURE----- ... -----END SSH SIGNATURE-----) */ + armored: string + /** Raw sshsig binary (before base64 armoring) */ + raw: Buffer +} + +export type SSHKeyProbeResult = { + exists: false +} + +export type SSHKeyProbeResultFound = { + exists: true + needsPassphrase: boolean + /** True when the key is OpenSSH native format AND encrypted (bcrypt KDF + AES cipher). */ + opensshEncrypted?: boolean +} + +export type SSHKeyProbe = SSHKeyProbeResult | SSHKeyProbeResultFound diff --git a/src/shared/transport/events/vc-events.ts b/src/shared/transport/events/vc-events.ts index b3dbe8261..3133f9c97 100644 --- a/src/shared/transport/events/vc-events.ts +++ b/src/shared/transport/events/vc-events.ts @@ -14,6 +14,7 @@ export const VcErrorCode = { INVALID_ACTION: 'ERR_VC_INVALID_ACTION', INVALID_BRANCH_NAME: 'ERR_VC_INVALID_BRANCH_NAME', INVALID_CONFIG_KEY: 'ERR_VC_INVALID_CONFIG_KEY', + INVALID_CONFIG_VALUE: 'ERR_VC_INVALID_CONFIG_VALUE', INVALID_REF: 'ERR_VC_INVALID_REF', INVALID_REMOTE_URL: 'ERR_VC_INVALID_REMOTE_URL', MERGE_CONFLICT: 'ERR_VC_MERGE_CONFLICT', @@ -28,9 +29,14 @@ export const VcErrorCode = { NOTHING_STAGED: 'ERR_VC_NOTHING_STAGED', NOTHING_TO_PUSH: 'ERR_VC_NOTHING_TO_PUSH', NOTHING_TO_RESET: 'ERR_VC_NOTHING_TO_RESET', + PASSPHRASE_REQUIRED: 'ERR_VC_PASSPHRASE_REQUIRED', PULL_FAILED: 'ERR_VC_PULL_FAILED', PUSH_FAILED: 'ERR_VC_PUSH_FAILED', REMOTE_ALREADY_EXISTS: 'ERR_VC_REMOTE_ALREADY_EXISTS', + SIGNING_KEY_ALREADY_EXISTS: 'ERR_VC_SIGNING_KEY_ALREADY_EXISTS', + SIGNING_KEY_NOT_CONFIGURED: 'ERR_VC_SIGNING_KEY_NOT_CONFIGURED', + SIGNING_KEY_NOT_FOUND: 'ERR_VC_SIGNING_KEY_NOT_FOUND', + SIGNING_KEY_NOT_SUPPORTED: 'ERR_VC_SIGNING_KEY_NOT_SUPPORTED', UNCOMMITTED_CHANGES: 'ERR_VC_UNCOMMITTED_CHANGES', UNRELATED_HISTORIES: 'ERR_VC_UNRELATED_HISTORIES', USER_NOT_CONFIGURED: 'ERR_VC_USER_NOT_CONFIGURED', @@ -57,6 +63,7 @@ export const VcEvents = { PUSH: 'vc:push', REMOTE: 'vc:remote', RESET: 'vc:reset', + SIGNING_KEY: 'vc:signing-key', STATUS: 'vc:status', } as const @@ -90,31 +97,83 @@ export interface IVcAddResponse { export interface IVcCommitRequest { message: string + /** Passphrase for encrypted SSH key — only sent on retry after PASSPHRASE_REQUIRED error */ + passphrase?: string + /** Override signing config: true = force sign, false = force no-sign, undefined = use config */ + sign?: boolean } export interface IVcCommitResponse { message: string sha: string + /** True when the commit was cryptographically signed with an SSH key. */ + signed?: boolean } -export type VcConfigKey = 'user.email' | 'user.name' +// Canonical form is all-lowercase (matches the `git config` CLI convention and +// the FIELD_MAP lookup key in vc-handler.ts). Camel-case variants from legacy +// callers are accepted at runtime via case-insensitive lookup below, but are +// not part of the exported union so new code cannot depend on them. +export type VcConfigKey = 'commit.sign' | 'user.email' | 'user.name' | 'user.signingkey' -export const VC_CONFIG_KEYS: readonly string[] = ['user.name', 'user.email'] satisfies readonly VcConfigKey[] +export const VC_CONFIG_KEYS: readonly string[] = [ + 'user.name', + 'user.email', + 'user.signingkey', + 'commit.sign', +] satisfies readonly VcConfigKey[] export function isVcConfigKey(key: string): key is VcConfigKey { - return VC_CONFIG_KEYS.includes(key) + return (VC_CONFIG_KEYS as readonly string[]).includes(key.toLowerCase()) } -export interface IVcConfigRequest { +/** Import SSH signing settings from local/global git config (no key/value needed). */ +export interface IVcConfigImportRequest { + importGitSigning: true +} + +/** Get or set a single config key. */ +export interface IVcConfigKeyRequest { + importGitSigning?: false + /** Config key (e.g., 'user.email', 'user.signingkey') */ key: VcConfigKey + /** Value to set. Omit for GET. For 'commit.sign': 'true' or 'false'. */ value?: string } +export type IVcConfigRequest = IVcConfigImportRequest | IVcConfigKeyRequest + export interface IVcConfigResponse { + /** Optional display hint (e.g., fingerprint after setting signingkey) */ + hint?: string key: string value: string } +// ── Signing Key Management ───────────────────────────────────────────────── + +export type VcSigningKeyAction = 'add' | 'list' | 'remove' + +export type IVcSigningKeyRequest = + | {action: 'add'; publicKey: string; title: string} + | {action: 'list'} + | {action: 'remove'; keyId: string} + +export type SigningKeyItem = { + createdAt: string + fingerprint: string + id: string + keyType: string + lastUsedAt?: string + publicKey: string + title: string +} + +export type IVcSigningKeyResponse = + | {action: 'add'; key: SigningKeyItem} + | {action: 'list'; keys: SigningKeyItem[]} + | {action: 'remove'} + export interface IVcPushRequest { branch?: string setUpstream?: boolean diff --git a/src/tui/components/inline-prompts/index.ts b/src/tui/components/inline-prompts/index.ts index 6a6c14241..025f1ab46 100644 --- a/src/tui/components/inline-prompts/index.ts +++ b/src/tui/components/inline-prompts/index.ts @@ -10,6 +10,8 @@ export {InlineFileSelector} from './inline-file-selector.js' export type {InlineFileSelectorProps} from './inline-file-selector.js' export {InlineInput} from './inline-input.js' export type {InlineInputProps} from './inline-input.js' +export {InlinePassword} from './inline-password.js' +export type {InlinePasswordProps} from './inline-password.js' export {InlineSearch} from './inline-search.js' export type {InlineSearchProps} from './inline-search.js' export {InlineSelect} from './inline-select.js' diff --git a/src/tui/components/inline-prompts/inline-password.tsx b/src/tui/components/inline-prompts/inline-password.tsx new file mode 100644 index 000000000..1ffea6abf --- /dev/null +++ b/src/tui/components/inline-prompts/inline-password.tsx @@ -0,0 +1,84 @@ +/** + * InlinePassword Component + * + * Masked-input variant of InlineInput for secrets (e.g., SSH key passphrases). + * + * Renders `*` for each typed character; the real value is never written to + * the Ink text stream. Uses ink's useInput directly (same rationale as + * InlineInput — paste chunks arriving in a single React batch must + * accumulate via a functional state updater). + */ + +import {Box, Text, useInput} from 'ink' +import React, {useCallback, useState} from 'react' + +import {useTheme} from '../../hooks/index.js' +import {stripBracketedPaste} from '../../utils/index.js' + +export interface InlinePasswordProps { + /** The prompt message shown before the masked field */ + message: string + /** Escape-key handler (caller typically aborts the surrounding flow) */ + onCancel?: () => void + /** Called with the raw (unmasked) value when user presses Enter */ + onSubmit: (value: string) => void +} + +export function InlinePassword({ + message, + onCancel, + onSubmit, +}: InlinePasswordProps): React.ReactElement { + const { + theme: {colors}, + } = useTheme() + const [value, setValue] = useState('') + + const handleSubmit = useCallback(() => { + setValue((currentValue) => { + const cleaned = stripBracketedPaste(currentValue) + // Empty passphrase is almost always a mistake; do not submit. + if (cleaned.length === 0) return currentValue + setTimeout(() => onSubmit(cleaned), 0) + return currentValue + }) + }, [onSubmit]) + + useInput((input, key) => { + if (key.escape) { + if (onCancel) onCancel() + return + } + + if (key.upArrow || key.downArrow || key.tab || (key.shift && key.tab)) { + return + } + + if (key.return) { + handleSubmit() + return + } + + if (key.backspace || key.delete) { + setValue((prev) => prev.slice(0, -1)) + return + } + + if (input && !key.ctrl && !key.meta) { + const cleaned = stripBracketedPaste(input) + if (cleaned) { + setValue((prev) => prev + cleaned) + } + } + }) + + return ( + + + ? + {message} + {'*'.repeat(value.length)} + + + ) +} diff --git a/src/tui/features/vc/commit/components/vc-commit-flow-state.ts b/src/tui/features/vc/commit/components/vc-commit-flow-state.ts new file mode 100644 index 000000000..89df95b06 --- /dev/null +++ b/src/tui/features/vc/commit/components/vc-commit-flow-state.ts @@ -0,0 +1,83 @@ +/** + * Pure state machine for the TUI commit flow. + * + * Split out of vc-commit-flow.tsx so the transitions are unit-testable + * without spinning up an Ink tree. The component owns side effects + * (firing mutations, rendering Ink nodes); this module owns decisions. + * + * Required by ENG-2002 §Signing Flow step 2: encrypted-key passphrase + * prompting must live in the TUI (not oclif). + */ + +import {VcErrorCode} from '../../../../../shared/transport/events/vc-events.js' +import {formatTransportError, getTransportErrorCode} from '../../../../utils/error-messages.js' + +/** Cap matches the retry limit the pre-B1 oclif command used to enforce. */ +export const MAX_PASSPHRASE_RETRIES = 3 + +export type CommitFlowState = + | {attempt: number; kind: 'awaiting-passphrase'} + | {attempt: number; kind: 'committing'} + | {kind: 'done'; message: string; outcome: 'cancelled' | 'error' | 'success'} + +export type CommitFlowEvent = + | {error: unknown; type: 'commit-error'} + | {message: string; sha: string; signed?: boolean; type: 'commit-success'} + | {type: 'passphrase-cancelled'} + | {type: 'passphrase-submitted'} + +export const initialCommitFlowState: CommitFlowState = {attempt: 0, kind: 'committing'} + +export function reduceCommitFlow( + state: CommitFlowState, + event: CommitFlowEvent, +): CommitFlowState { + if (state.kind === 'done') return state + + switch (event.type) { + case 'commit-error': { + const code = getTransportErrorCode(event.error) + if (code === VcErrorCode.PASSPHRASE_REQUIRED && state.kind === 'committing') { + // Daemon re-rejected after a passphrase retry: cap the loop. + if (state.attempt >= MAX_PASSPHRASE_RETRIES) { + return { + kind: 'done', + message: `Too many failed passphrase attempts (${MAX_PASSPHRASE_RETRIES}).`, + outcome: 'error', + } + } + + return {attempt: state.attempt + 1, kind: 'awaiting-passphrase'} + } + + return { + kind: 'done', + message: `Failed to commit: ${formatTransportError(event.error)}`, + outcome: 'error', + } + } + + case 'commit-success': { + const signed = event.signed ? ' 🔏' : '' + return { + kind: 'done', + message: `[${event.sha.slice(0, 7)}] ${event.message}${signed}`, + outcome: 'success', + } + } + + case 'passphrase-cancelled': { + if (state.kind !== 'awaiting-passphrase') return state + return { + kind: 'done', + message: 'Passphrase entry cancelled.', + outcome: 'cancelled', + } + } + + case 'passphrase-submitted': { + if (state.kind !== 'awaiting-passphrase') return state + return {attempt: state.attempt, kind: 'committing'} + } + } +} diff --git a/src/tui/features/vc/commit/components/vc-commit-flow.tsx b/src/tui/features/vc/commit/components/vc-commit-flow.tsx index 69aabca7c..cdfac9873 100644 --- a/src/tui/features/vc/commit/components/vc-commit-flow.tsx +++ b/src/tui/features/vc/commit/components/vc-commit-flow.tsx @@ -1,11 +1,12 @@ import {Text, useInput} from 'ink' import Spinner from 'ink-spinner' -import React, {useEffect} from 'react' +import React, {useCallback, useEffect, useReducer, useRef} from 'react' import type {CustomDialogCallbacks} from '../../../../types/commands.js' -import {formatTransportError} from '../../../../utils/error-messages.js' +import {InlinePassword} from '../../../../components/inline-prompts/index.js' import {useExecuteVcCommit} from '../api/execute-vc-commit.js' +import {initialCommitFlowState, reduceCommitFlow} from './vc-commit-flow-state.js' type VcCommitFlowProps = CustomDialogCallbacks & { message: string @@ -13,33 +14,84 @@ type VcCommitFlowProps = CustomDialogCallbacks & { export function VcCommitFlow({message, onCancel, onComplete}: VcCommitFlowProps): React.ReactNode { const commitMutation = useExecuteVcCommit() + const [state, dispatch] = useReducer(reduceCommitFlow, initialCommitFlowState) + // Escape aborts only while committing (not mid-passphrase-entry — the input + // component owns Escape there so the user can cancel the prompt). useInput((_, key) => { - if (key.escape && !commitMutation.isPending) { + if (key.escape && state.kind === 'committing' && !commitMutation.isPending) { onCancel() } }) - const fired = React.useRef(false) + const fireCommit = useCallback( + (passphrase?: string) => { + commitMutation.mutate( + passphrase === undefined ? {message} : {message, passphrase}, + { + onError(error) { + dispatch({error, type: 'commit-error'}) + }, + onSuccess(result) { + dispatch({ + message: result.message, + sha: result.sha, + type: 'commit-success', + ...(result.signed ? {signed: true} : {}), + }) + }, + }, + ) + }, + [commitMutation, message], + ) + + // First commit attempt + const fired = useRef(false) useEffect(() => { if (fired.current) return fired.current = true - commitMutation.mutate( - {message}, - { - onError(error) { - onComplete(`Failed to commit: ${formatTransportError(error)}`) - }, - onSuccess(result) { - onComplete(`[${result.sha.slice(0, 7)}] ${result.message}`) - }, - }, + fireCommit() + }, [fireCommit]) + + // Terminal state → bubble up to dialog manager. + // + // Also drop the passphrase from React-Query's mutation.variables. The + // library retains variables until `reset()` or GC (~5 min), so without + // this the plaintext passphrase keeps living in TUI memory after the + // commit succeeds — visible to devtools, error boundaries, diagnostic + // dumpers. Cleared even on error/cancel terminal states for symmetry. + useEffect(() => { + if (state.kind === 'done') { + commitMutation.reset() + onComplete(state.message) + } + }, [state, onComplete, commitMutation]) + + if (state.kind === 'awaiting-passphrase') { + return ( + dispatch({type: 'passphrase-cancelled'})} + onSubmit={(pp) => { + dispatch({type: 'passphrase-submitted'}) + fireCommit(pp) + }} + /> ) - }, []) + } - return ( - - Committing... - - ) + if (state.kind === 'committing') { + return ( + + Committing... + + ) + } + + // Terminal — useEffect above bubbles the outcome; render nothing + return null } + +// Re-export so consumers (e.g., tests) can inspect the underlying state shape. +export type {CommitFlowState} from './vc-commit-flow-state.js' diff --git a/test/commands/signing-key/remove.test.ts b/test/commands/signing-key/remove.test.ts new file mode 100644 index 000000000..9eda0b92f --- /dev/null +++ b/test/commands/signing-key/remove.test.ts @@ -0,0 +1,41 @@ +import type {Config} from '@oclif/core' + +import {Config as OclifConfig} from '@oclif/core' +import {expect} from 'chai' +import {restore, stub} from 'sinon' + +import SigningKeyRemove from '../../../src/oclif/commands/signing-key/remove.js' + +describe('signing-key remove --yes guard', () => { + let config: Config + + before(async () => { + config = await OclifConfig.load(import.meta.url) + }) + + afterEach(() => { + restore() + }) + + it('refuses without --yes and does not touch the network', async () => { + const cmd = new SigningKeyRemove(['fake-id'], config) + const errorStub = stub(cmd, 'error').throws(new Error('STOP')) + + let thrown: unknown + try { + await cmd.run() + } catch (error) { + thrown = error + } + + expect(thrown).to.be.instanceOf(Error) + expect(errorStub.calledOnce).to.be.true + const [firstArg] = errorStub.firstCall.args as [string] + expect(firstArg).to.match(/--yes/) + expect(firstArg).to.match(/irreversible/i) + }) + + it('declares the --yes flag on the command', () => { + expect(SigningKeyRemove.flags).to.have.property('yes') + }) +}) diff --git a/test/unit/infra/iam/http-signing-key-service.test.ts b/test/unit/infra/iam/http-signing-key-service.test.ts new file mode 100644 index 000000000..8d46cade9 --- /dev/null +++ b/test/unit/infra/iam/http-signing-key-service.test.ts @@ -0,0 +1,198 @@ +/** + * HttpSigningKeyService Unit Tests + */ + +import {expect} from 'chai' +import {createSandbox, type SinonSandbox, type SinonStub} from 'sinon' + +import type {IHttpClient} from '../../../../src/server/core/interfaces/services/i-http-client.js' + +import {VcError} from '../../../../src/server/core/domain/errors/vc-error.js' +import {HttpSigningKeyService} from '../../../../src/server/infra/iam/http-signing-key-service.js' +import {VcErrorCode} from '../../../../src/shared/transport/events/vc-events.js' + +type Stubbed = {[K in keyof T]: SinonStub & T[K]} + +const IAM_BASE_URL = 'https://iam.example.com' +const KEYS_PATH = '/api/v3/users/me/signing-keys' + +/* eslint-disable camelcase */ +const RAW_KEY = { + created_at: '2024-01-01T00:00:00Z', + fingerprint: 'SHA256:abc123', + id: 'key-id-1', + key_type: 'ssh-ed25519', + public_key: 'ssh-ed25519 AAAA... test@example.com', + title: 'My laptop', +} +/* eslint-enable camelcase */ + +describe('HttpSigningKeyService', () => { + let sandbox: SinonSandbox + let httpClient: Stubbed + let service: HttpSigningKeyService + + beforeEach(() => { + sandbox = createSandbox() + httpClient = { + delete: sandbox.stub().resolves(), + get: sandbox.stub(), + post: sandbox.stub(), + put: sandbox.stub().resolves(), + } + service = new HttpSigningKeyService(httpClient as unknown as IHttpClient, IAM_BASE_URL) + }) + + afterEach(() => { + sandbox.restore() + }) + + describe('addKey()', () => { + it('POSTs to the signing-keys endpoint with snake_case body', async () => { + httpClient.post.resolves({ + data: {signing_key: RAW_KEY}, // eslint-disable-line camelcase + success: true, + }) + + await service.addKey('My laptop', 'ssh-ed25519 AAAA...') + + expect(httpClient.post.calledOnce).to.be.true + const [url, body] = httpClient.post.firstCall.args + expect(url).to.equal(`${IAM_BASE_URL}${KEYS_PATH}`) + expect(body).to.deep.equal({ + public_key: 'ssh-ed25519 AAAA...', // eslint-disable-line camelcase + title: 'My laptop', + }) + }) + + it('maps snake_case API response to camelCase SigningKeyResource', async () => { + httpClient.post.resolves({ + data: {signing_key: RAW_KEY}, // eslint-disable-line camelcase + success: true, + }) + + const result = await service.addKey('My laptop', 'ssh-ed25519 AAAA...') + + expect(result).to.deep.equal({ + createdAt: '2024-01-01T00:00:00Z', + fingerprint: 'SHA256:abc123', + id: 'key-id-1', + keyType: 'ssh-ed25519', + lastUsedAt: undefined, + publicKey: 'ssh-ed25519 AAAA... test@example.com', + title: 'My laptop', + }) + }) + + // Regression test for PR #435 review comment #19: dup-detection at the + // CLI used to sniff four different English substrings on the error + // message. The IAM server already communicates duplicate intent via HTTP + // 409; the HTTP adapter is the right layer to translate that into a + // structured VcError whose code the CLI can match exactly. + it('throws VcError SIGNING_KEY_ALREADY_EXISTS when underlying client reports HTTP 409', async () => { + httpClient.post.rejects(new Error('HTTP 409: Conflict')) + + let caught: unknown + try { + await service.addKey('My laptop', 'ssh-ed25519 AAAA...') + } catch (error) { + caught = error + } + + expect(caught).to.be.instanceOf(VcError) + if (caught instanceof VcError) { + expect(caught.code).to.equal(VcErrorCode.SIGNING_KEY_ALREADY_EXISTS) + } + }) + + it('passes through other HTTP errors unchanged', async () => { + const original = new Error('HTTP 500: Internal Server Error') + httpClient.post.rejects(original) + + let caught: unknown + try { + await service.addKey('My laptop', 'ssh-ed25519 AAAA...') + } catch (error) { + caught = error + } + + expect(caught).to.equal(original) + }) + }) + + describe('listKeys()', () => { + it('GETs from the signing-keys endpoint', async () => { + httpClient.get.resolves({ + data: {signing_keys: [RAW_KEY]}, // eslint-disable-line camelcase + success: true, + }) + + await service.listKeys() + + expect(httpClient.get.calledOnce).to.be.true + expect(httpClient.get.firstCall.args[0]).to.equal(`${IAM_BASE_URL}${KEYS_PATH}`) + }) + + it('maps each key from snake_case to camelCase', async () => { + httpClient.get.resolves({ + data: {signing_keys: [RAW_KEY]}, // eslint-disable-line camelcase + success: true, + }) + + const keys = await service.listKeys() + + expect(keys).to.have.length(1) + expect(keys[0]).to.include({fingerprint: 'SHA256:abc123', keyType: 'ssh-ed25519'}) + }) + + it('returns empty array when signing_keys field is missing', async () => { + httpClient.get.resolves({ + data: {}, + success: true, + }) + + const keys = await service.listKeys() + + expect(keys).to.deep.equal([]) + }) + + // Regression test for PR #435 review comment #20: even though + // AuthenticatedHttpClient throws on non-2xx responses, a 200 OK with + // `{success: false}` would previously slip past listKeys unnoticed and + // return []. Callers had no way to distinguish "no keys" from "request + // silently failed". Surface that state as an explicit throw. + it('throws when response envelope reports success:false', async () => { + httpClient.get.resolves({ + data: {signing_keys: []}, // eslint-disable-line camelcase + success: false, + }) + + let caught: unknown + try { + await service.listKeys() + } catch (error) { + caught = error + } + + expect(caught).to.be.instanceOf(Error) + }) + }) + + describe('removeKey()', () => { + it('DELETEs to the signing-keys/{id} endpoint', async () => { + await service.removeKey('key-id-1') + + expect(httpClient.delete.calledOnce).to.be.true + expect(httpClient.delete.firstCall.args[0]).to.equal(`${IAM_BASE_URL}${KEYS_PATH}/key-id-1`) + }) + }) + + describe('URL construction', () => { + it('strips trailing slash from base URL', () => { + const svc = new HttpSigningKeyService(httpClient as unknown as IHttpClient, 'https://iam.example.com/') + httpClient.get.resolves({data: {signing_keys: []}, success: true}) // eslint-disable-line camelcase + svc.listKeys() + expect(httpClient.get.firstCall.args[0]).to.equal(`https://iam.example.com${KEYS_PATH}`) + }) + }) +}) diff --git a/test/unit/infra/ssh/signing-key-cache.test.ts b/test/unit/infra/ssh/signing-key-cache.test.ts new file mode 100644 index 000000000..7b31a2e32 --- /dev/null +++ b/test/unit/infra/ssh/signing-key-cache.test.ts @@ -0,0 +1,178 @@ +import {expect} from 'chai' + +import type {ParsedSSHKey} from '../../../../src/server/infra/ssh/types.js' + +import {SigningKeyCache} from '../../../../src/server/infra/ssh/signing-key-cache.js' + +// Minimal stub ParsedSSHKey — only needs the shape for caching tests +function makeFakeKey(id: string): ParsedSSHKey { + return { + fingerprint: `SHA256:${id}`, + keyType: 'ssh-ed25519', + privateKeyObject: {} as never, + publicKeyBlob: Buffer.from(id), + } +} + +const P1 = '/projects/alpha' +const P2 = '/projects/beta' + +describe('SigningKeyCache', () => { + describe('get() / set()', () => { + it('returns null for unknown key path', () => { + const cache = new SigningKeyCache() + expect(cache.get(P1, '/nonexistent/key')).to.be.undefined + }) + + it('returns stored key immediately after set()', () => { + const cache = new SigningKeyCache() + const key = makeFakeKey('abc') + cache.set(P1, '/home/user/.ssh/id_ed25519', key) + expect(cache.get(P1, '/home/user/.ssh/id_ed25519')).to.equal(key) + }) + + it('different paths within the same project are stored independently', () => { + const cache = new SigningKeyCache() + const key1 = makeFakeKey('k1') + const key2 = makeFakeKey('k2') + cache.set(P1, '/path/to/key1', key1) + cache.set(P1, '/path/to/key2', key2) + expect(cache.get(P1, '/path/to/key1')).to.equal(key1) + expect(cache.get(P1, '/path/to/key2')).to.equal(key2) + }) + + it('overwriting a (project, key) pair replaces the entry', () => { + const cache = new SigningKeyCache() + const key1 = makeFakeKey('v1') + const key2 = makeFakeKey('v2') + cache.set(P1, '/same/path', key1) + cache.set(P1, '/same/path', key2) + expect(cache.get(P1, '/same/path')).to.equal(key2) + }) + }) + + describe('project isolation (ENG-2002 M2)', () => { + it('same keyPath across two projects does NOT share cached entries', () => { + const cache = new SigningKeyCache() + const keyInAlpha = makeFakeKey('alpha') + const keyInBeta = makeFakeKey('beta') + cache.set(P1, '/home/user/.ssh/id_ed25519', keyInAlpha) + cache.set(P2, '/home/user/.ssh/id_ed25519', keyInBeta) + + expect(cache.get(P1, '/home/user/.ssh/id_ed25519')).to.equal(keyInAlpha) + expect(cache.get(P2, '/home/user/.ssh/id_ed25519')).to.equal(keyInBeta) + }) + + it('get() from a different project returns null even with identical keyPath', () => { + const cache = new SigningKeyCache() + cache.set(P1, '/shared/path', makeFakeKey('p1')) + expect(cache.get(P2, '/shared/path')).to.be.undefined + }) + + it('invalidate() is project-scoped', () => { + const cache = new SigningKeyCache() + const keyInAlpha = makeFakeKey('alpha') + const keyInBeta = makeFakeKey('beta') + cache.set(P1, '/shared/path', keyInAlpha) + cache.set(P2, '/shared/path', keyInBeta) + + cache.invalidate(P1, '/shared/path') + + expect(cache.get(P1, '/shared/path')).to.be.undefined + expect(cache.get(P2, '/shared/path')).to.equal(keyInBeta) + }) + }) + + describe('size', () => { + it('is 0 for empty cache', () => { + const cache = new SigningKeyCache() + expect(cache.size).to.equal(0) + }) + + it('counts non-expired entries across projects', () => { + const cache = new SigningKeyCache() + cache.set(P1, '/a', makeFakeKey('a')) + cache.set(P2, '/a', makeFakeKey('b')) + expect(cache.size).to.equal(2) + }) + }) + + describe('TTL expiry', () => { + it('returns null after TTL expires', async function () { + this.timeout(3000) + + const cache = new SigningKeyCache(50) + const key = makeFakeKey('ttl-test') + cache.set(P1, '/ttl/test', key) + + expect(cache.get(P1, '/ttl/test')).to.equal(key) + + await new Promise((resolve) => { setTimeout(resolve, 100) }) + + expect(cache.get(P1, '/ttl/test')).to.be.undefined + }) + + it('returns key before TTL expires', async function () { + this.timeout(3000) + + const cache = new SigningKeyCache(500) + const key = makeFakeKey('ttl-alive') + cache.set(P1, '/ttl/alive', key) + + await new Promise((resolve) => { setTimeout(resolve, 50) }) + + expect(cache.get(P1, '/ttl/alive')).to.equal(key) + }) + }) + + describe('expired entry cleanup on set()', () => { + it('removes expired entries when a new key is set', async function () { + this.timeout(3000) + + const cache = new SigningKeyCache(50) + cache.set(P1, '/old', makeFakeKey('old')) + + await new Promise((resolve) => { setTimeout(resolve, 100) }) + + cache.set(P1, '/new', makeFakeKey('new')) + + expect(cache.size).to.equal(1) + expect(cache.get(P1, '/old')).to.be.undefined + expect(cache.get(P1, '/new')).to.not.be.undefined + }) + }) + + describe('invalidate()', () => { + it('removes a specific (project, key) pair from cache', () => { + const cache = new SigningKeyCache() + const key = makeFakeKey('to-clear') + cache.set(P1, '/invalidate/me', key) + cache.invalidate(P1, '/invalidate/me') + expect(cache.get(P1, '/invalidate/me')).to.be.undefined + }) + + it('does not affect other cached keys when invalidating one', () => { + const cache = new SigningKeyCache() + const key1 = makeFakeKey('keep') + const key2 = makeFakeKey('remove') + cache.set(P1, '/keep', key1) + cache.set(P1, '/remove', key2) + cache.invalidate(P1, '/remove') + expect(cache.get(P1, '/keep')).to.equal(key1) + expect(cache.get(P1, '/remove')).to.be.undefined + }) + }) + + describe('invalidateAll()', () => { + it('clears all entries across all projects', () => { + const cache = new SigningKeyCache() + cache.set(P1, '/a', makeFakeKey('a')) + cache.set(P1, '/b', makeFakeKey('b')) + cache.set(P2, '/c', makeFakeKey('c')) + cache.invalidateAll() + expect(cache.size).to.equal(0) + expect(cache.get(P1, '/a')).to.be.undefined + expect(cache.get(P2, '/c')).to.be.undefined + }) + }) +}) diff --git a/test/unit/infra/ssh/ssh-agent-signer.test.ts b/test/unit/infra/ssh/ssh-agent-signer.test.ts new file mode 100644 index 000000000..8439e06a7 --- /dev/null +++ b/test/unit/infra/ssh/ssh-agent-signer.test.ts @@ -0,0 +1,533 @@ +import {expect} from 'chai' +import {mkdtempSync, writeFileSync} from 'node:fs' +import net from 'node:net' +import {tmpdir} from 'node:os' +import {join} from 'node:path' + +import {tryGetSshAgentSigner} from '../../../../src/server/infra/ssh/ssh-agent-signer.js' +import {parseSSHPrivateKey} from '../../../../src/server/infra/ssh/ssh-key-parser.js' + +// ── Test key ──────────────────────────────────────────────────────────────── +// Same Ed25519 test key used across SSH tests (NOT a production key). +const TEST_OPENSSH_ED25519_KEY = `-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQgAAAJgCtf3VArX9 +1QAAAAtzc2gtZWQyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQg +AAEB01GDi+m4swI3lsGv870+yJFfAJP0CcFSDPcTyCUpaBSYh9Posmi46km6A8piXvKIn +BgiWuHXbhM5iNo3PGM1CAAAAEHRlc3RAZXhhbXBsZS5jb20BAgMEBQ== +-----END OPENSSH PRIVATE KEY-----` + +// ── SSH agent protocol constants ───────────────────────────────────────────── +const SSH_AGENTC_REQUEST_IDENTITIES = 11 +const SSH_AGENTC_SIGN_REQUEST = 13 +const SSH_AGENT_IDENTITIES_ANSWER = 12 +const SSH_AGENT_SIGN_RESPONSE = 14 + +function writeUInt32BE(value: number): Buffer { + const buf = Buffer.allocUnsafe(4) + buf.writeUInt32BE(value, 0) + return buf +} + +function sshString(data: Buffer | string): Buffer { + const buf = Buffer.isBuffer(data) ? data : Buffer.from(data, 'utf8') + return Buffer.concat([writeUInt32BE(buf.length), buf]) +} + +// ── Mock SSH agent server ──────────────────────────────────────────────────── + +/** + * Creates a minimal mock SSH agent that responds to IDENTITIES and SIGN requests. + * Returns the socket path and a cleanup function. + */ +function createMockAgent( + identities: Array<{blob: Buffer; comment: string}>, + signResponse: Buffer, +): {cleanup: () => void; socketPath: string} { + const tempDir = mkdtempSync(join(tmpdir(), 'brv-mock-agent-')) + const socketPath = join(tempDir, 'agent.sock') + + const server = net.createServer((conn) => { + const chunks: Buffer[] = [] + + conn.on('data', (chunk) => { + chunks.push(chunk) + const accumulated = Buffer.concat(chunks) + if (accumulated.length < 4) return + const msgLen = accumulated.readUInt32BE(0) + if (accumulated.length < 4 + msgLen) return + + // Consume the message + const payload = accumulated.slice(4, 4 + msgLen) + chunks.length = 0 + if (accumulated.length > 4 + msgLen) { + chunks.push(accumulated.slice(4 + msgLen)) + } + + const msgType = payload[0] + + if (msgType === SSH_AGENTC_REQUEST_IDENTITIES) { + // Build identities response + const parts: Buffer[] = [ + Buffer.from([SSH_AGENT_IDENTITIES_ANSWER]), + writeUInt32BE(identities.length), + ] + for (const id of identities) { + parts.push(sshString(id.blob), sshString(id.comment)) + } + + const body = Buffer.concat(parts) + conn.write(Buffer.concat([writeUInt32BE(body.length), body])) + } else if (msgType === SSH_AGENTC_SIGN_REQUEST) { + // Return the pre-configured sign response + const body = Buffer.concat([ + Buffer.from([SSH_AGENT_SIGN_RESPONSE]), + sshString(signResponse), + ]) + conn.write(Buffer.concat([writeUInt32BE(body.length), body])) + } + }) + }) + + server.listen(socketPath) + + return { + cleanup() { + server.close() + }, + socketPath, + } +} + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe('ssh-agent-signer', () => { +describe('tryGetSshAgentSigner()', () => { + let tempDir: string + let keyPath: string + let originalAuthSock: string | undefined + + before(() => { + tempDir = mkdtempSync(join(tmpdir(), 'brv-agent-test-')) + keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, TEST_OPENSSH_ED25519_KEY, {mode: 0o600}) + originalAuthSock = process.env.SSH_AUTH_SOCK + }) + + afterEach(() => { + if (originalAuthSock === undefined) { + delete process.env.SSH_AUTH_SOCK + } else { + process.env.SSH_AUTH_SOCK = originalAuthSock + } + }) + + it('returns null when SSH_AUTH_SOCK is not set', async () => { + delete process.env.SSH_AUTH_SOCK + const result = await tryGetSshAgentSigner(keyPath) + expect(result).to.be.undefined + }) + + it('returns null when agent is unreachable', async () => { + process.env.SSH_AUTH_SOCK = '/nonexistent/agent.sock' + const result = await tryGetSshAgentSigner(keyPath) + expect(result).to.be.undefined + }) + + it('returns null when agent has no matching key', async () => { + // Agent with a different key blob + const unrelatedBlob = Buffer.concat([ + sshString('ssh-ed25519'), + sshString(Buffer.alloc(32, 0xff)), // fake pubkey + ]) + const agent = createMockAgent( + [{blob: unrelatedBlob, comment: 'unrelated'}], + Buffer.alloc(0), + ) + process.env.SSH_AUTH_SOCK = agent.socketPath + + try { + const result = await tryGetSshAgentSigner(keyPath) + expect(result).to.be.undefined + } finally { + agent.cleanup() + } + }) + + it('returns a signer when agent has the matching key', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + + const fakeSignature = Buffer.concat([ + sshString('ssh-ed25519'), + sshString(Buffer.alloc(64, 0xab)), // fake sig bytes + ]) + const agent = createMockAgent( + [{blob: parsed.publicKeyBlob, comment: 'test@example.com'}], + fakeSignature, + ) + process.env.SSH_AUTH_SOCK = agent.socketPath + + try { + const signer = await tryGetSshAgentSigner(keyPath) + expect(signer).to.not.be.undefined + } finally { + agent.cleanup() + } + }) +}) + +describe('SshAgentSigner.sign()', () => { + let tempDir: string + let keyPath: string + let originalAuthSock: string | undefined + + before(() => { + tempDir = mkdtempSync(join(tmpdir(), 'brv-agentsign-test-')) + keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, TEST_OPENSSH_ED25519_KEY, {mode: 0o600}) + originalAuthSock = process.env.SSH_AUTH_SOCK + }) + + afterEach(() => { + if (originalAuthSock === undefined) { + delete process.env.SSH_AUTH_SOCK + } else { + process.env.SSH_AUTH_SOCK = originalAuthSock + } + }) + + it('produces armored output with correct SSH SIGNATURE headers', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + const fakeSignature = Buffer.concat([ + sshString('ssh-ed25519'), + sshString(Buffer.alloc(64, 0xab)), + ]) + const agent = createMockAgent( + [{blob: parsed.publicKeyBlob, comment: 'test'}], + fakeSignature, + ) + process.env.SSH_AUTH_SOCK = agent.socketPath + + try { + const signer = await tryGetSshAgentSigner(keyPath) + expect(signer).to.not.be.undefined + + const result = await signer!.sign('test commit payload') + expect(result.armored).to.match(/^-----BEGIN SSH SIGNATURE-----/) + expect(result.armored.trim()).to.match(/-----END SSH SIGNATURE-----$/) + } finally { + agent.cleanup() + } + }) + + it('raw envelope starts with 6-byte SSHSIG magic (no null)', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + const fakeSignature = Buffer.concat([ + sshString('ssh-ed25519'), + sshString(Buffer.alloc(64, 0xab)), + ]) + const agent = createMockAgent( + [{blob: parsed.publicKeyBlob, comment: 'test'}], + fakeSignature, + ) + process.env.SSH_AUTH_SOCK = agent.socketPath + + try { + const signer = await tryGetSshAgentSigner(keyPath) + const result = await signer!.sign('test payload') + + // Envelope magic: 6 bytes 'SSHSIG' (no null) + const magic = result.raw.subarray(0, 6).toString() + expect(magic).to.equal('SSHSIG') + // 7th byte should be version (uint32BE = 0x00), NOT a null terminator + // Version is uint32BE(1) = [0x00, 0x00, 0x00, 0x01] + expect(result.raw.readUInt32BE(6)).to.equal(1) + } finally { + agent.cleanup() + } + }) + + it('signed data sent to agent starts with 6-byte SSHSIG magic followed by namespace', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + + // We'll capture the signed data sent to the agent's sign method + let capturedSignData: Buffer | undefined + const captureSocketPath = join(tempDir, 'capture-agent.sock') + const server = net.createServer((conn) => { + const chunks: Buffer[] = [] + conn.on('data', (chunk) => { + chunks.push(chunk) + const accumulated = Buffer.concat(chunks) + if (accumulated.length < 4) return + const msgLen = accumulated.readUInt32BE(0) + if (accumulated.length < 4 + msgLen) return + + const payload = accumulated.slice(4, 4 + msgLen) + chunks.length = 0 + + const msgType = payload[0] + + if (msgType === SSH_AGENTC_REQUEST_IDENTITIES) { + const body = Buffer.concat([ + Buffer.from([SSH_AGENT_IDENTITIES_ANSWER]), + writeUInt32BE(1), + sshString(parsed.publicKeyBlob), + sshString('test'), + ]) + conn.write(Buffer.concat([writeUInt32BE(body.length), body])) + } else if (msgType === SSH_AGENTC_SIGN_REQUEST) { + // Parse the sign request to capture the data blob + let offset = 1 + const blobLen = payload.readUInt32BE(offset) + offset += 4 + blobLen + const dataLen = payload.readUInt32BE(offset) + offset += 4 + capturedSignData = payload.slice(offset, offset + dataLen) + + // Return a fake signature + const fakeSignature = Buffer.concat([ + sshString('ssh-ed25519'), + sshString(Buffer.alloc(64, 0xab)), + ]) + const body = Buffer.concat([ + Buffer.from([SSH_AGENT_SIGN_RESPONSE]), + sshString(fakeSignature), + ]) + conn.write(Buffer.concat([writeUInt32BE(body.length), body])) + } + }) + }) + server.listen(captureSocketPath) + process.env.SSH_AUTH_SOCK = captureSocketPath + + try { + const signer = await tryGetSshAgentSigner(keyPath) + expect(signer).to.not.be.undefined + await signer!.sign('test payload for magic check') + + // Per PROTOCOL.sshsig the signed-data structure is: + // byte[6] MAGIC_PREAMBLE ("SSHSIG" — NO null terminator) + // string namespace + // ... + // Asserting only the first 7 bytes is unsafe because the namespace length + // prefix happens to begin with 0x00 (uint32 BE of "git" = 0x00000003), so a + // wrong 7-byte 'SSHSIG\0' check would pass spuriously. + expect(capturedSignData).to.not.be.undefined + expect(capturedSignData!.subarray(0, 6).toString()).to.equal('SSHSIG') + const namespaceLen = capturedSignData!.readUInt32BE(6) + expect(namespaceLen).to.equal(3) + expect(capturedSignData!.subarray(10, 13).toString()).to.equal('git') + } finally { + server.close() + } + }) + + it('different payloads produce different signatures', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + const fakeSignature = Buffer.concat([ + sshString('ssh-ed25519'), + sshString(Buffer.alloc(64, 0xab)), + ]) + const agent = createMockAgent( + [{blob: parsed.publicKeyBlob, comment: 'test'}], + fakeSignature, + ) + process.env.SSH_AUTH_SOCK = agent.socketPath + + try { + const signer = await tryGetSshAgentSigner(keyPath) + // Note: with a static mock signature, the armored outputs will differ + // because the signed data (and thus the data sent to agent) differs, + // but the agent returns the same fake sig. The raw envelope is identical + // except for the data we pass to agent. Since we capture the full + // envelope, the sshsig binary will be the same. + // This test verifies the function runs successfully for different payloads. + const r1 = await signer!.sign('payload one') + const r2 = await signer!.sign('payload two') + expect(r1.armored).to.be.a('string') + expect(r2.armored).to.be.a('string') + } finally { + agent.cleanup() + } + }) + + it('armored body contains valid base64', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + const fakeSignature = Buffer.concat([ + sshString('ssh-ed25519'), + sshString(Buffer.alloc(64, 0xab)), + ]) + const agent = createMockAgent( + [{blob: parsed.publicKeyBlob, comment: 'test'}], + fakeSignature, + ) + process.env.SSH_AUTH_SOCK = agent.socketPath + + try { + const signer = await tryGetSshAgentSigner(keyPath) + const result = await signer!.sign('test') + const lines = result.armored.split('\n') + const bodyLines = lines.filter( + (l) => !l.startsWith('-----') && l.trim().length > 0, + ) + const b64 = bodyLines.join('') + expect(() => Buffer.from(b64, 'base64')).to.not.throw() + } finally { + agent.cleanup() + } + }) + + // Regression test for PR #435 review comment #25: the sign-request flags + // field is the only place where the RSA-via-agent code path diverges from + // the ed25519 path. Without this assertion, a refactor from + // `=== 'ssh-rsa'` to `=== 'rsa'` or `.startsWith('rsa-')` would go green + // across every other test — because ed25519 uses flags=0 either way — + // while RSA users silently sign with SHA-1 and downstream verification + // rejects the signature. RSA is the v1 recovery path for encrypted + // ~/.ssh/id_rsa users; breaking it silently means v1 ships with no RSA + // support for that population. + it('sets SSH_AGENT_RSA_SHA2_512 in flags for ssh-rsa keys (full agent round-trip)', async () => { + // Build a fake ssh-rsa wire-format public key blob. Content is + // opaque to this test — only the fingerprint match to the agent + // identity matters for finding the signer, and the keyType on the + // .pub sidecar drives the flags branch inside sign(). + const rsaPubBlob = Buffer.concat([ + sshString('ssh-rsa'), + sshString(Buffer.alloc(3, 0x01)), // fake exponent + sshString(Buffer.alloc(128, 0x02)), // fake modulus + ]) + + const rsaKeyDir = mkdtempSync(join(tmpdir(), 'brv-rsa-agent-test-')) + const rsaKeyPath = join(rsaKeyDir, 'id_rsa') + writeFileSync(rsaKeyPath, 'fake rsa key material — never parsed in this path', {mode: 0o600}) + writeFileSync( + `${rsaKeyPath}.pub`, + `ssh-rsa ${rsaPubBlob.toString('base64')} test@rsa`, + {mode: 0o644}, + ) + + // Mock agent that records the flags field from the first SIGN request. + let capturedFlags: number | undefined + const socketDir = mkdtempSync(join(tmpdir(), 'brv-rsa-mock-agent-')) + const socketPath = join(socketDir, 'agent.sock') + const server = net.createServer((conn) => { + const chunks: Buffer[] = [] + conn.on('data', (chunk) => { + chunks.push(chunk) + const acc = Buffer.concat(chunks) + if (acc.length < 4) return + const msgLen = acc.readUInt32BE(0) + if (acc.length < 4 + msgLen) return + const payload = acc.subarray(4, 4 + msgLen) + chunks.length = 0 + + if (payload[0] === SSH_AGENTC_REQUEST_IDENTITIES) { + const body = Buffer.concat([ + Buffer.from([SSH_AGENT_IDENTITIES_ANSWER]), + writeUInt32BE(1), + sshString(rsaPubBlob), + sshString('test@rsa'), + ]) + conn.write(Buffer.concat([writeUInt32BE(body.length), body])) + } else if (payload[0] === SSH_AGENTC_SIGN_REQUEST) { + // Parse: [type=13][uint32 len][blob][uint32 len][data][uint32 flags] + let offset = 1 + const blobLen = payload.readUInt32BE(offset) + offset += 4 + blobLen + const dataLen = payload.readUInt32BE(offset) + offset += 4 + dataLen + capturedFlags = payload.readUInt32BE(offset) + + // Return minimally valid signature envelope. + const sig = Buffer.concat([sshString('ssh-rsa'), sshString(Buffer.alloc(256, 0xbb))]) + const body = Buffer.concat([Buffer.from([SSH_AGENT_SIGN_RESPONSE]), sshString(sig)]) + conn.write(Buffer.concat([writeUInt32BE(body.length), body])) + } + }) + }) + server.listen(socketPath) + process.env.SSH_AUTH_SOCK = socketPath + + try { + const signer = await tryGetSshAgentSigner(rsaKeyPath) + expect(signer, 'tryGetSshAgentSigner must resolve with an RSA-capable signer').to.not.be.undefined + if (!signer) throw new Error('unreachable') + + await signer.sign('hello RSA') + + // Flag value comes from the SSH_AGENT_RSA_SHA2_512 constant (4) in + // ssh-agent-signer.ts — pinned here so a refactor that drops the + // branch (e.g. `=== 'rsa'`) regresses immediately. + expect(capturedFlags, 'RSA signing must set SSH_AGENT_RSA_SHA2_512 flag').to.equal(4) + } finally { + server.close() + } + }) + + // Regression test for PR #435 review comment #22: sign() used to blindly + // return response.subarray(5, 5 + sigLen) without checking that sigLen + // actually fits in the response buffer. A truncated agent response + // silently yielded a short signature; downstream verification would + // then fail with a generic "bad signature" error instead of surfacing + // the agent-boundary problem at its source. + it('throws when agent response claims a signature length larger than the body', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + + // Build a malformed SIGN_RESPONSE: type byte + sigLen(=100) + only 3 bytes of signature. + // The outer request/response length prefix is added by the mock server. + const malformedBody = Buffer.concat([ + Buffer.from([SSH_AGENT_SIGN_RESPONSE]), + writeUInt32BE(100), + Buffer.from('ABC'), + ]) + + const tempDir2 = mkdtempSync(join(tmpdir(), 'brv-mock-agent-trunc-')) + const socketPath = join(tempDir2, 'agent.sock') + const server = net.createServer((conn) => { + const chunks: Buffer[] = [] + conn.on('data', (chunk) => { + chunks.push(chunk) + const acc = Buffer.concat(chunks) + if (acc.length < 4) return + const msgLen = acc.readUInt32BE(0) + if (acc.length < 4 + msgLen) return + const payload = acc.subarray(4, 4 + msgLen) + chunks.length = 0 + + if (payload[0] === SSH_AGENTC_REQUEST_IDENTITIES) { + const body = Buffer.concat([ + Buffer.from([SSH_AGENT_IDENTITIES_ANSWER]), + writeUInt32BE(1), + sshString(parsed.publicKeyBlob), + sshString('comment'), + ]) + conn.write(Buffer.concat([writeUInt32BE(body.length), body])) + } else if (payload[0] === SSH_AGENTC_SIGN_REQUEST) { + conn.write(Buffer.concat([writeUInt32BE(malformedBody.length), malformedBody])) + } + }) + }) + server.listen(socketPath) + process.env.SSH_AUTH_SOCK = socketPath + + try { + const signer = await tryGetSshAgentSigner(keyPath) + expect(signer, 'signer should be constructed').to.not.be.undefined + if (!signer) throw new Error('unreachable') + + let caught: Error | undefined + try { + await signer.sign('payload') + } catch (error) { + if (error instanceof Error) caught = error + } + + expect(caught, 'sign() must reject on truncated response').to.be.instanceOf(Error) + if (!caught) throw new Error('unreachable') + expect(caught.message).to.match(/truncat|signature.*byte/i) + } finally { + server.close() + } + }) +}) +}) diff --git a/test/unit/infra/ssh/ssh-key-parser.test.ts b/test/unit/infra/ssh/ssh-key-parser.test.ts new file mode 100644 index 000000000..8e4abcd56 --- /dev/null +++ b/test/unit/infra/ssh/ssh-key-parser.test.ts @@ -0,0 +1,451 @@ +import {expect} from 'chai' +import {generateKeyPairSync, sign} from 'node:crypto' +import {mkdtempSync, rmSync, writeFileSync} from 'node:fs' +import {tmpdir} from 'node:os' +import {join} from 'node:path' + +import {extractPublicKey, parseSSHPrivateKey, probeSSHKey, resolveHome} from '../../../../src/server/infra/ssh/ssh-key-parser.js' + +// ── Test helpers ────────────────────────────────────────────────────────────── + +function writeU32(value: number): Buffer { + const buf = Buffer.allocUnsafe(4) + buf.writeUInt32BE(value, 0) + return buf +} + +function sshStr(data: Buffer | string): Buffer { + const b = Buffer.isBuffer(data) ? data : Buffer.from(data) + return Buffer.concat([writeU32(b.length), b]) +} + +function makeEncryptedOpenSSHKey(): string { + const pubBlob = Buffer.concat([sshStr('ssh-ed25519'), sshStr(Buffer.alloc(32, 0xaa))]) + const buf = Buffer.concat([ + Buffer.from('openssh-key-v1\0', 'binary'), + sshStr('aes256-ctr'), + sshStr('bcrypt'), + sshStr(Buffer.alloc(0)), + writeU32(1), + sshStr(pubBlob), + sshStr(Buffer.alloc(64, 0xbb)), + ]) + return `-----BEGIN OPENSSH PRIVATE KEY-----\n${buf.toString('base64')}\n-----END OPENSSH PRIVATE KEY-----` +} + +/** + * A real Ed25519 private key in OpenSSH native format (unencrypted). + * Generated with: ssh-keygen -t ed25519 -f /tmp/brv_test_key -N "" -C "test@example.com" + * This key is used ONLY for unit testing — never for any real credentials. + * Fingerprint: SHA256:R573at4sJuUgWnT+H8ivsX1khl0dKCW9KzJwDz00nmg + */ +const TEST_OPENSSH_ED25519_KEY = `-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQgAAAJgCtf3VArX9 +1QAAAAtzc2gtZWQyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQg +AAEB01GDi+m4swI3lsGv870+yJFfAJP0CcFSDPcTyCUpaBSYh9Posmi46km6A8piXvKIn +BgiWuHXbhM5iNo3PGM1CAAAAEHRlc3RAZXhhbXBsZS5jb20BAgMEBQ== +-----END OPENSSH PRIVATE KEY-----` + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe('ssh-key-parser', () => { +describe('probeSSHKey()', () => { + let tempDir: string + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), 'brv-ssh-test-')) + }) + + afterEach(() => { + rmSync(tempDir, {force: true, recursive: true}) + }) + + it('returns {exists: false} for non-existent file', async () => { + const result = await probeSSHKey(join(tempDir, 'missing_key')) + expect(result).to.deep.equal({exists: false}) + }) + + it('returns {exists: true, needsPassphrase: false} for unencrypted OpenSSH key', async () => { + const keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, TEST_OPENSSH_ED25519_KEY, {mode: 0o600}) + const result = await probeSSHKey(keyPath) + expect(result.exists).to.be.true + if (!result.exists) throw new Error('unreachable') + expect(result.needsPassphrase).to.be.false + }) + + it('returns opensshEncrypted:true for encrypted OpenSSH-format key', async () => { + const pubBlob = Buffer.concat([sshStr('ssh-ed25519'), sshStr(Buffer.alloc(32, 0xaa))]) + const buf = Buffer.concat([ + Buffer.from('openssh-key-v1\0', 'binary'), + sshStr('aes256-ctr'), + sshStr('bcrypt'), + sshStr(Buffer.alloc(0)), + writeU32(1), + sshStr(pubBlob), + sshStr(Buffer.alloc(64, 0xbb)), + ]) + const pem = `-----BEGIN OPENSSH PRIVATE KEY-----\n${buf.toString('base64')}\n-----END OPENSSH PRIVATE KEY-----` + const keyPath = join(tempDir, 'id_openssh_enc2') + writeFileSync(keyPath, pem, {mode: 0o600}) + + const result = await probeSSHKey(keyPath) + expect(result.exists).to.be.true + if (!result.exists) throw new Error('unreachable') + expect(result.opensshEncrypted).to.be.true + }) + + it('returns needsPassphrase:true for an encrypted PEM key with no passphrase argument', async () => { + // Pins the no-passphrase code path: createPrivateKey on an encrypted PEM + // emits ERR_OSSL_CRYPTO_INTERRUPTED_OR_CANCELLED (NOT a "user cancelled + // prompt" — OpenSSL just aborts the read). isPassphraseError must catch + // this code, otherwise probeSSHKey would surface a raw crypto error + // instead of asking the caller for a passphrase. + const {privateKey} = generateKeyPairSync('ed25519', { + privateKeyEncoding: {cipher: 'aes-256-cbc', format: 'pem', passphrase: 'secret', type: 'pkcs8'}, + publicKeyEncoding: {format: 'pem', type: 'spki'}, + }) + const keyPath = join(tempDir, 'id_ed25519_pem_enc') + writeFileSync(keyPath, privateKey, {mode: 0o600}) + + const result = await probeSSHKey(keyPath) + expect(result.exists).to.be.true + if (!result.exists) throw new Error('unreachable') + expect(result.needsPassphrase).to.be.true + }) + + it('throws "Unsupported OpenSSH key type" for unencrypted RSA OpenSSH key (does not false-prompt for passphrase)', async () => { + // Regression test for ENG-2002 B6: the isPassphraseError regex used to include + // /unsupported/, which false-matched parseOpenSSHKey's own error string and + // caused probeSSHKey to incorrectly return needsPassphrase:true for RSA/ECDSA + // OpenSSH-format keys, triggering a spurious passphrase prompt. + const pubBlob = Buffer.concat([sshStr('ssh-rsa'), sshStr(Buffer.alloc(64, 0xaa))]) + const buf = Buffer.concat([ + Buffer.from('openssh-key-v1\0', 'binary'), + sshStr('none'), // cipher: NOT encrypted + sshStr('none'), // kdf + sshStr(Buffer.alloc(0)), + writeU32(1), + sshStr(pubBlob), + sshStr(Buffer.alloc(64, 0xbb)), + ]) + const pem = `-----BEGIN OPENSSH PRIVATE KEY-----\n${buf.toString('base64')}\n-----END OPENSSH PRIVATE KEY-----` + const keyPath = join(tempDir, 'id_rsa_openssh') + writeFileSync(keyPath, pem, {mode: 0o600}) + + let caught: unknown + try { + await probeSSHKey(keyPath) + } catch (error) { + caught = error + } + + if (!(caught instanceof Error)) { + expect.fail('probeSSHKey must throw for unsupported key type, not return needsPassphrase:true') + } + + expect(caught.message).to.match(/Unsupported OpenSSH key type/) + }) + + it('throws a user-grade error (not raw OpenSSL code) for a file that is not a valid SSH key', async () => { + // Regression test for ENG-2002 AC9-b + C2. Node.js crypto emits + // ERR_OSSL_UNSUPPORTED for any PEM body it cannot decode. The raw error + // ("error:1E08010C:DECODER routines::unsupported") is not user-grade; + // probeSSHKey must translate it to actionable English. + const malformedPem = '-----BEGIN PRIVATE KEY-----\nQUFBQQ==\n-----END PRIVATE KEY-----' + const keyPath = join(tempDir, 'malformed_pem') + writeFileSync(keyPath, malformedPem, {mode: 0o600}) + + let caught: unknown + try { + await probeSSHKey(keyPath) + } catch (error) { + caught = error + } + + if (!(caught instanceof Error)) { + expect.fail('probeSSHKey must throw for malformed PEM, not return needsPassphrase:true') + } + + expect(caught.message).to.match(/not a valid SSH private key/i) + expect(caught.message).to.not.match(/ERR_OSSL|DECODER routines/) + }) + + it('throws "Unsupported OpenSSH key type" for unencrypted ECDSA OpenSSH key (does not false-prompt for passphrase)', async () => { + // Same regression as above, but exercising ecdsa-sha2-nistp256. + const pubBlob = Buffer.concat([sshStr('ecdsa-sha2-nistp256'), sshStr(Buffer.alloc(65, 0xaa))]) + const buf = Buffer.concat([ + Buffer.from('openssh-key-v1\0', 'binary'), + sshStr('none'), + sshStr('none'), + sshStr(Buffer.alloc(0)), + writeU32(1), + sshStr(pubBlob), + sshStr(Buffer.alloc(64, 0xbb)), + ]) + const pem = `-----BEGIN OPENSSH PRIVATE KEY-----\n${buf.toString('base64')}\n-----END OPENSSH PRIVATE KEY-----` + const keyPath = join(tempDir, 'id_ecdsa_openssh') + writeFileSync(keyPath, pem, {mode: 0o600}) + + let caught: unknown + try { + await probeSSHKey(keyPath) + } catch (error) { + caught = error + } + + if (!(caught instanceof Error)) { + expect.fail('probeSSHKey must throw for unsupported key type, not return needsPassphrase:true') + } + + expect(caught.message).to.match(/Unsupported OpenSSH key type/) + }) + + it('returns {exists: true, needsPassphrase: true} for encrypted OpenSSH key', async () => { + // Construct a minimal OpenSSH key with cipherName = 'aes256-ctr' to simulate encrypted key. + // Must include a valid public key blob + private key blob so parseOpenSSHKey doesn't crash. + const pubBlob = Buffer.concat([sshStr('ssh-ed25519'), sshStr(Buffer.alloc(32, 0xaa))]) + + const buf = Buffer.concat([ + Buffer.from('openssh-key-v1\0', 'binary'), // magic + sshStr('aes256-ctr'), // ciphername + sshStr('bcrypt'), // kdfname + sshStr(Buffer.alloc(0)), // kdfoptions (empty) + writeU32(1), // nkeys = 1 + sshStr(pubBlob), // public key blob + sshStr(Buffer.alloc(64, 0xbb)), // private key blob (encrypted placeholder) + ]) + const b64 = buf.toString('base64') + const pem = `-----BEGIN OPENSSH PRIVATE KEY-----\n${b64}\n-----END OPENSSH PRIVATE KEY-----` + + const keyPath = join(tempDir, 'id_ed25519_enc') + writeFileSync(keyPath, pem, {mode: 0o600}) + + const result = await probeSSHKey(keyPath) + expect(result.exists).to.be.true + if (!result.exists) throw new Error('unreachable') + expect(result.needsPassphrase).to.be.true + }) +}) + +// ── parseSSHPrivateKey tests ────────────────────────────────────────────────── + +describe('parseSSHPrivateKey()', () => { + let tempDir: string + let keyPath: string + + // Single setup: every test in this block is read-only against keyPath, so + // before/after (lifecycle once) is correct — beforeEach would re-create the + // key file unnecessarily and slow the suite. New tests added here MUST stay + // read-only; if you need to mutate, use a separate tempDir per test. + before(() => { + tempDir = mkdtempSync(join(tmpdir(), 'brv-ssh-parse-test-')) + keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, TEST_OPENSSH_ED25519_KEY, {mode: 0o600}) + }) + + after(() => { + rmSync(tempDir, {force: true, recursive: true}) + }) + + it('returns a ParsedSSHKey with correct shape', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + + expect(parsed).to.have.keys(['fingerprint', 'keyType', 'privateKeyObject', 'publicKeyBlob']) + }) + + it('keyType is ssh-ed25519', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + expect(parsed.keyType).to.equal('ssh-ed25519') + }) + + it('fingerprint is SHA256:... format', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + expect(parsed.fingerprint).to.match(/^SHA256:[A-Za-z0-9+/]+$/) + }) + + it('publicKeyBlob starts with ssh-ed25519 keytype string (SSH wire format)', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + // First 4 bytes = length of "ssh-ed25519" = 11 + const len = parsed.publicKeyBlob.readUInt32BE(0) + const keyType = parsed.publicKeyBlob.subarray(4, 4 + len).toString() + expect(keyType).to.equal('ssh-ed25519') + }) + + it('privateKeyObject is a valid KeyObject that can sign', async () => { + const parsed = await parseSSHPrivateKey(keyPath) + + // Ed25519 uses sign(null, data, key) — algorithm is implicit + const sig = sign(null, Buffer.from('test payload'), parsed.privateKeyObject) + expect(sig).to.be.instanceOf(Buffer) + expect(sig.length).to.be.greaterThan(0) + }) + + it('throws for missing file', async () => { + let threw = false + try { + await parseSSHPrivateKey('/nonexistent/key') + } catch { + threw = true + } + + expect(threw).to.be.true + }) + + it('throws a user-grade error (not raw OpenSSL code) when given a wrong passphrase for an encrypted PEM key', async () => { + // Regression test for ENG-2002 AC9-c. createPrivateKey with a wrong + // passphrase emits ERR_OSSL_BAD_DECRYPT whose raw message + // ("error:1C800064:Provider routines::bad decrypt") is not user-grade; + // parseSSHPrivateKey must translate it to actionable English. + const {privateKey} = generateKeyPairSync('ed25519', { + privateKeyEncoding: {cipher: 'aes-256-cbc', format: 'pem', passphrase: 'rightpass', type: 'pkcs8'}, + publicKeyEncoding: {format: 'pem', type: 'spki'}, + }) + const encKeyPath = join(tempDir, 'id_ed25519_pem_wrongpp') + writeFileSync(encKeyPath, privateKey, {mode: 0o600}) + + let caught: unknown + try { + await parseSSHPrivateKey(encKeyPath, 'wrongpass') + } catch (error) { + caught = error + } + + if (!(caught instanceof Error)) { + expect.fail('parseSSHPrivateKey must throw when given a wrong passphrase') + } + + expect(caught.message).to.match(/passphrase/i) + expect(caught.message).to.not.match(/ERR_OSSL|Provider routines|bad decrypt/i) + }) +}) + +// ── extractPublicKey tests ──────────────────────────────────────────────────── + +describe('extractPublicKey()', () => { + let tempDir: string + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), 'brv-extract-test-')) + }) + + afterEach(() => { + rmSync(tempDir, {force: true, recursive: true}) + }) + + it('extracts public key from encrypted OpenSSH key with no .pub sidecar', async () => { + const keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, makeEncryptedOpenSSHKey(), {mode: 0o600}) + + const result = await extractPublicKey(keyPath) + + expect(result.keyType).to.equal('ssh-ed25519') + expect(result.publicKeyBlob).to.be.instanceOf(Buffer) + expect(result.publicKeyBlob.length).to.be.greaterThan(0) + expect(result.comment).to.be.undefined + }) + + it('prefers .pub sidecar over OpenSSH header when sidecar exists', async () => { + const keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, makeEncryptedOpenSSHKey(), {mode: 0o600}) + writeFileSync( + `${keyPath}.pub`, + 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIERWc7ZeFmViDVndPNPdfAHZi8z9dBhCdlBjVf+xWrUd user@laptop', + {mode: 0o644}, + ) + + const result = await extractPublicKey(keyPath) + + expect(result.keyType).to.equal('ssh-ed25519') + expect(result.comment).to.equal('user@laptop') + // Blob should match what's in the .pub file + const expectedBlob = Buffer.from('AAAAC3NzaC1lZDI1NTE5AAAAIERWc7ZeFmViDVndPNPdfAHZi8z9dBhCdlBjVf+xWrUd', 'base64') + expect(result.publicKeyBlob.equals(expectedBlob)).to.be.true + }) + + it('extracts public key from unencrypted OpenSSH key with no sidecar', async () => { + const keyPath = join(tempDir, 'id_ed25519_unenc') + writeFileSync(keyPath, TEST_OPENSSH_ED25519_KEY, {mode: 0o600}) + + const result = await extractPublicKey(keyPath) + + expect(result.keyType).to.equal('ssh-ed25519') + expect(result.publicKeyBlob).to.be.instanceOf(Buffer) + expect(result.comment).to.be.undefined + }) + + it('throws for a non-existent file', async () => { + let threw = false + try { + await extractPublicKey(join(tempDir, 'no_such_key')) + } catch { + threw = true + } + + expect(threw).to.be.true + }) + + // Regression test for ENG-2002 AC9-b/c (PR #435 review comment #17). + // Before the fix, the PEM branch of extractPublicKey called + // parseSSHPrivateKey(keyPath) with no passphrase. For an encrypted PEM key + // without a .pub sidecar, Node's createPrivateKey emitted either + // ERR_OSSL_BAD_DECRYPT (user-facing: "Wrong passphrase ..." — misleading, + // nobody entered one) or ERR_OSSL_CRYPTO_INTERRUPTED_OR_CANCELLED (raw + // OpenSSL code leaks to the CLI — violates AC9 "no raw OpenSSL codes"). + // + // The fix must detect passphrase-class errors from parseSSHPrivateKey and + // replace them with an actionable hint pointing at `ssh-keygen -y -f`. + it('throws actionable hint (not raw OpenSSL / misleading "wrong passphrase") for encrypted PEM with no sidecar', async () => { + const {privateKey} = generateKeyPairSync('ed25519', { + privateKeyEncoding: {cipher: 'aes-256-cbc', format: 'pem', passphrase: 'secret', type: 'pkcs8'}, + publicKeyEncoding: {format: 'pem', type: 'spki'}, + }) + const keyPath = join(tempDir, 'id_ed25519_pem_enc_no_sidecar') + writeFileSync(keyPath, privateKey, {mode: 0o600}) + + let caught: Error | undefined + try { + await extractPublicKey(keyPath) + } catch (error) { + if (error instanceof Error) caught = error + } + + expect(caught, 'extractPublicKey must throw on encrypted PEM without sidecar').to.be.instanceOf(Error) + if (!caught) throw new Error('unreachable') + // Actionable hint — tells the user exactly how to unblock themselves. + expect(caught.message).to.include('ssh-keygen -y -f') + expect(caught.message).to.include(keyPath) + // Must not misleadingly blame a passphrase the user never entered. + expect(caught.message).to.not.match(/Wrong passphrase/i) + // AC9: no raw OpenSSL codes or OpenSSL routine names in user-facing error. + expect(caught.message).to.not.match(/ERR_OSSL|DECODER routines|Provider routines|bad decrypt/i) + }) +}) + +// ── resolveHome tests ───────────────────────────────────────────────────────── + +describe('resolveHome()', () => { + it('replaces leading ~ with HOME', () => { + const home = process.env.HOME ?? '/home/user' + const result = resolveHome('~/.ssh/id_ed25519') + expect(result).to.equal(`${home}/.ssh/id_ed25519`) + }) + + it('replaces bare ~ with HOME', () => { + const home = process.env.HOME ?? '/home/user' + const result = resolveHome('~') + expect(result).to.equal(home) + }) + + it('does not modify absolute paths', () => { + const result = resolveHome('/home/user/.ssh/id_ed25519') + expect(result).to.equal('/home/user/.ssh/id_ed25519') + }) + + it('does not modify relative paths', () => { + const result = resolveHome('keys/id_ed25519') + expect(result).to.equal('keys/id_ed25519') + }) +}) +}) diff --git a/test/unit/infra/ssh/sshsig-signer.test.ts b/test/unit/infra/ssh/sshsig-signer.test.ts new file mode 100644 index 000000000..2d84a69f0 --- /dev/null +++ b/test/unit/infra/ssh/sshsig-signer.test.ts @@ -0,0 +1,230 @@ +import {expect} from 'chai' +import {execFileSync} from 'node:child_process' +import {createHash, verify as cryptoVerify, generateKeyPairSync} from 'node:crypto' +import {mkdtempSync, rmSync, writeFileSync} from 'node:fs' +import {tmpdir} from 'node:os' +import {join} from 'node:path' + +import type {ParsedSSHKey} from '../../../../src/server/infra/ssh/types.js' + +import {parseSSHPrivateKey} from '../../../../src/server/infra/ssh/ssh-key-parser.js' +import {signCommitPayload} from '../../../../src/server/infra/ssh/sshsig-signer.js' + +/** + * Returns true iff `ssh-keygen` is on PATH and supports the `-Y` subcommand family. + * + * Probe: `ssh-keygen -Y check-novalidate` with no further args. The binary exits + * non-zero with "Too few arguments..." but only when both the binary AND the -Y + * subcommand family are available. ENOENT (binary missing) surfaces as `code === 'ENOENT'`. + */ +function isSshKeygenAvailable(): boolean { + try { + execFileSync('ssh-keygen', ['-Y', 'check-novalidate'], {stdio: 'ignore'}) + return true + } catch (error) { + if (error instanceof Error && 'code' in error && (error as {code: string}).code === 'ENOENT') { + return false + } + + // Non-zero exit (expected: missing namespace) — binary is present and supports -Y. + return true + } +} + +/** Parse an SSH wire-format length-prefixed string from a buffer. Returns [value, nextOffset]. */ +function readSSHString(buf: Buffer, offset: number): [Buffer, number] { + const len = buf.readUInt32BE(offset) + return [buf.subarray(offset + 4, offset + 4 + len), offset + 4 + len] +} + +/** + * Extract the signature blob key-type string and raw signature bytes from + * an armored sshsig output. Returns {keyType, rawSig}. + */ +function extractSigFromArmored(armored: string): {keyType: string; rawSig: Buffer} { + const lines = armored.split('\n') + const b64 = lines.filter((l) => !l.startsWith('-----') && l.trim().length > 0).join('') + const raw = Buffer.from(b64, 'base64') + + // Envelope layout (all length-prefixed SSH strings): + // 'SSHSIG' (6 bytes) + version uint32 (4 bytes) + pubkey + namespace + reserved + hash-algo + sig-blob + let offset = 10 // skip magic (6) + version (4) + ;[, offset] = readSSHString(raw, offset) // pubkey + ;[, offset] = readSSHString(raw, offset) // namespace + ;[, offset] = readSSHString(raw, offset) // reserved + ;[, offset] = readSSHString(raw, offset) // hash-algo + const [sigBlob] = readSSHString(raw, offset) // signature blob + + // Signature blob: string(key-type) + string(raw-sig) + const [keyTypeBuf, afterKeyType] = readSSHString(sigBlob, 0) + const [rawSig] = readSSHString(sigBlob, afterKeyType) + return {keyType: keyTypeBuf.toString(), rawSig} +} + +/** + * Real Ed25519 key for testing — NOT a production key. + * Generated with: ssh-keygen -t ed25519 -f /tmp/brv_test_key -N "" -C "test@example.com" + * Fingerprint: SHA256:R573at4sJuUgWnT+H8ivsX1khl0dKCW9KzJwDz00nmg + */ +const TEST_OPENSSH_ED25519_KEY = `-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQgAAAJgCtf3VArX9 +1QAAAAtzc2gtZWQyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQg +AAEB01GDi+m4swI3lsGv870+yJFfAJP0CcFSDPcTyCUpaBSYh9Posmi46km6A8piXvKIn +BgiWuHXbhM5iNo3PGM1CAAAAEHRlc3RAZXhhbXBsZS5jb20BAgMEBQ== +-----END OPENSSH PRIVATE KEY-----` + +describe('signCommitPayload()', () => { + let tempDir: string + let keyPath: string + + // Helper: parse key once and reuse across tests + let parsedKey: Awaited> + + before(async () => { + tempDir = mkdtempSync(join(tmpdir(), 'brv-sshsig-test-')) + keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, TEST_OPENSSH_ED25519_KEY, {mode: 0o600}) + parsedKey = await parseSSHPrivateKey(keyPath) + }) + + it('returns an SSHSignatureResult with armored and raw fields', async () => { + const result = signCommitPayload('tree abc123\nauthor Test\n\ninitial commit\n', parsedKey) + expect(result).to.have.keys(['armored', 'raw']) + }) + + it('armored signature starts with -----BEGIN SSH SIGNATURE-----', async () => { + const result = signCommitPayload('test payload', parsedKey) + expect(result.armored).to.match(/^-----BEGIN SSH SIGNATURE-----/) + }) + + it('armored signature ends with -----END SSH SIGNATURE-----', async () => { + const result = signCommitPayload('test payload', parsedKey) + expect(result.armored.trim()).to.match(/-----END SSH SIGNATURE-----$/) + }) + + it('raw buffer starts with SSHSIG magic (6 bytes)', async () => { + const result = signCommitPayload('test payload', parsedKey) + const magic = result.raw.subarray(0, 6).toString() + expect(magic).to.equal('SSHSIG') + }) + + it('different payloads produce different signatures', async () => { + const r1 = signCommitPayload('payload one', parsedKey) + const r2 = signCommitPayload('payload two', parsedKey) + expect(r1.armored).to.not.equal(r2.armored) + }) + + it('produces a valid base64 body in the armored output', async () => { + const result = signCommitPayload('test', parsedKey) + const lines = result.armored.split('\n') + // Remove BEGIN/END headers and join + const bodyLines = lines.filter( + (l) => !l.startsWith('-----') && l.trim().length > 0, + ) + const b64 = bodyLines.join('') + // Valid base64 should be decodable + expect(() => Buffer.from(b64, 'base64')).to.not.throw() + }) + + it('raw buffer is a Buffer', async () => { + const result = signCommitPayload('test', parsedKey) + expect(result.raw).to.be.instanceOf(Buffer) + }) + + describe('with RSA key', () => { + let rsaKey: ParsedSSHKey + + before(() => { + const {privateKey} = generateKeyPairSync('rsa', {modulusLength: 2048}) + // publicKeyBlob is used for embedding in the envelope only — a placeholder is fine + rsaKey = { + fingerprint: 'SHA256:placeholder', + keyType: 'ssh-rsa', + privateKeyObject: privateKey, + publicKeyBlob: Buffer.alloc(0), + } + }) + + it('signature blob key-type is rsa-sha2-512 (not ssh-rsa)', () => { + const result = signCommitPayload('test payload', rsaKey) + const {keyType} = extractSigFromArmored(result.armored) + expect(keyType).to.equal('rsa-sha2-512') + }) + + it('RSA signature is verifiable with sha512 algorithm', async () => { + const payload = 'tree abc\nauthor Test\n\ncommit message\n' + const result = signCommitPayload(payload, rsaKey) + const {rawSig} = extractSigFromArmored(result.armored) + + // Reconstruct signed data (same structure sshsig-signer builds internally) + function sshString(data: Buffer | string): Buffer { + const buf = Buffer.isBuffer(data) ? data : Buffer.from(data, 'utf8') + const lenBuf = Buffer.allocUnsafe(4) + lenBuf.writeUInt32BE(buf.length, 0) + return Buffer.concat([lenBuf, buf]) + } + + const messageHash = createHash('sha512').update(Buffer.from(payload, 'utf8')).digest() + const signedData = Buffer.concat([ + // 6-byte preamble per PROTOCOL.sshsig (no null terminator). + Buffer.from('SSHSIG'), + sshString('git'), + sshString(''), + sshString('sha512'), + sshString(messageHash), + ]) + + // Extract public key from our private key and verify + const {createPublicKey} = await import('node:crypto') + const pub = createPublicKey(rsaKey.privateKeyObject) + const valid = cryptoVerify('sha512', signedData, pub, rawSig) + expect(valid).to.be.true + }) + }) + + // Round-trip suite: feed our armored signature back to the OpenSSH reference + // verifier (`ssh-keygen -Y check-novalidate`) and assert acceptance. This is + // the only test in the file that validates against an external verifier — + // every other assertion is structural and can pass while the signature is + // cryptographically invalid (the trap earlier reviews of this code missed). + describe('round-trip with ssh-keygen', () => { + let roundtripDir: string + let roundtripKeyPath: string + let roundtripKey: Awaited> + + before(async function () { + if (!isSshKeygenAvailable()) { + this.skip() + } + + roundtripDir = mkdtempSync(join(tmpdir(), 'brv-sshsig-roundtrip-')) + roundtripKeyPath = join(roundtripDir, 'id_ed25519') + execFileSync('ssh-keygen', ['-q', '-t', 'ed25519', '-N', '', '-C', 'roundtrip', '-f', roundtripKeyPath]) + roundtripKey = await parseSSHPrivateKey(roundtripKeyPath) + }) + + after(() => { + if (roundtripDir) rmSync(roundtripDir, {force: true, recursive: true}) + }) + + it('ssh-keygen -Y check-novalidate accepts the signature', () => { + const payload = 'tree abc123\nparent def456\nauthor Test 0 +0000\n\nround-trip test payload\n' + const {armored} = signCommitPayload(payload, roundtripKey) + + const sigPath = join(roundtripDir, 'commit.sig') + writeFileSync(sigPath, armored) + + // ssh-keygen reads message from stdin, signature from -s file. + // -n git: namespace must match what signCommitPayload uses. + // check-novalidate: verifies cryptographic validity without an allowed_signers file. + const stdout = execFileSync( + 'ssh-keygen', + ['-Y', 'check-novalidate', '-n', 'git', '-s', sigPath], + {input: payload, stdio: ['pipe', 'pipe', 'pipe']}, + ).toString() + + expect(stdout).to.match(/Good "git" signature/i) + }) + }) +}) diff --git a/test/unit/infra/transport/handlers/signing-key-handler.test.ts b/test/unit/infra/transport/handlers/signing-key-handler.test.ts new file mode 100644 index 000000000..6721f362b --- /dev/null +++ b/test/unit/infra/transport/handlers/signing-key-handler.test.ts @@ -0,0 +1,288 @@ +/** + * SigningKeyHandler Unit Tests + */ + +import {expect} from 'chai' +import {createSandbox, type SinonSandbox, type SinonStub} from 'sinon' + +import type {ITokenStore} from '../../../../../src/server/core/interfaces/auth/i-token-store.js' +import type {ISigningKeyService, SigningKeyResource} from '../../../../../src/server/core/interfaces/services/i-signing-key-service.js' +import type {ITransportServer, RequestHandler} from '../../../../../src/server/core/interfaces/transport/i-transport-server.js' + +import {AuthToken} from '../../../../../src/server/core/domain/entities/auth-token.js' +import {NotAuthenticatedError} from '../../../../../src/server/core/domain/errors/task-error.js' +import {SigningKeyHandler} from '../../../../../src/server/infra/transport/handlers/signing-key-handler.js' +import {type SigningKeyItem, VcEvents} from '../../../../../src/shared/transport/events/vc-events.js' + +type Stubbed = {[K in keyof T]: SinonStub & T[K]} + +const IAM_BASE_URL = 'https://iam.example.com' + +const FAKE_KEY: SigningKeyResource = { + createdAt: '2024-01-01T00:00:00Z', + fingerprint: 'SHA256:abc123', + id: 'key-id-1', + keyType: 'ssh-ed25519', + publicKey: 'ssh-ed25519 AAAA... test@example.com', + title: 'My laptop', +} + +function makeValidToken(): AuthToken { + return new AuthToken({ + accessToken: 'valid-access-token', + expiresAt: new Date(Date.now() + 3_600_000), + refreshToken: 'refresh', + sessionKey: 'session-key-123', + userEmail: 'test@example.com', + userId: 'user-1', + }) +} + +interface TestDeps { + requestHandler: RequestHandler + signingKeyService: Stubbed + tokenStore: Stubbed + transport: Stubbed +} + +function makeDeps(sandbox: SinonSandbox): TestDeps { + const requestHandlers: Record = {} + + const transport: Stubbed = { + broadcastToProject: sandbox.stub(), + close: sandbox.stub().resolves(), + emitToClient: sandbox.stub(), + emitToProject: sandbox.stub(), + initialize: sandbox.stub().resolves(), + offRequest: sandbox.stub(), + onRequest: sandbox.stub().callsFake((event: string, handler: RequestHandler) => { + requestHandlers[event] = handler + }), + } as unknown as Stubbed + + const tokenStore: Stubbed = { + clear: sandbox.stub().resolves(), + load: sandbox.stub().resolves(makeValidToken()), + save: sandbox.stub().resolves(), + } + + const signingKeyService: Stubbed = { + addKey: sandbox.stub().resolves(FAKE_KEY), + listKeys: sandbox.stub().resolves([FAKE_KEY]), + removeKey: sandbox.stub().resolves(), + } + + return { + requestHandler: requestHandlers[VcEvents.SIGNING_KEY], + signingKeyService, + tokenStore, + transport, + } +} + +function makeHandlerWithInjectedService(sb: SinonSandbox): { + getRequestHandler: () => RequestHandler + signingKeyService: Stubbed +} { + const requestHandlers: Record = {} + + const signingKeyService: Stubbed = { + addKey: sb.stub().resolves(FAKE_KEY), + listKeys: sb.stub().resolves([FAKE_KEY]), + removeKey: sb.stub().resolves(), + } + + const tokenStore: Stubbed = { + clear: sb.stub().resolves(), + load: sb.stub().resolves(makeValidToken()), + save: sb.stub().resolves(), + } + + const transport: Stubbed = { + broadcastToProject: sb.stub(), + close: sb.stub().resolves(), + emitToClient: sb.stub(), + emitToProject: sb.stub(), + initialize: sb.stub().resolves(), + offRequest: sb.stub(), + onRequest: sb.stub().callsFake((event: string, h: RequestHandler) => { + requestHandlers[event] = h + }), + } as unknown as Stubbed + + const handler = new SigningKeyHandler({ + iamBaseUrl: IAM_BASE_URL, + signingKeyService, + tokenStore, + transport, + }) + handler.setup() + + return { + getRequestHandler: () => requestHandlers[VcEvents.SIGNING_KEY], + signingKeyService, + } +} + +function makeHandler(sandbox: SinonSandbox, deps: TestDeps): {getRequestHandler: () => RequestHandler; handler: SigningKeyHandler} { + const requestHandlers: Record = {} + const {transport} = deps + // Re-wire to capture the registered handler + transport.onRequest.callsFake((event: string, h: RequestHandler) => { + requestHandlers[event] = h + }) + + const handler = new SigningKeyHandler({ + iamBaseUrl: IAM_BASE_URL, + tokenStore: deps.tokenStore, + transport, + }) + handler.setup() + + return { + getRequestHandler: () => requestHandlers[VcEvents.SIGNING_KEY], + handler, + } +} + +describe('SigningKeyHandler', () => { + let sandbox: SinonSandbox + + beforeEach(() => { + sandbox = createSandbox() + }) + + afterEach(() => { + sandbox.restore() + }) + + describe('auth guard', () => { + it('throws NotAuthenticatedError when token is missing', async () => { + const deps = makeDeps(sandbox) + deps.tokenStore.load.resolves() + const {getRequestHandler} = makeHandler(sandbox, deps) + + try { + await getRequestHandler()({action: 'list'}, 'client-1') + expect.fail('Expected error') + } catch (error) { + expect(error).to.be.instanceOf(NotAuthenticatedError) + } + }) + + it('throws NotAuthenticatedError when token.isValid() returns false (expired)', async () => { + const deps = makeDeps(sandbox) + const expiredToken = new AuthToken({ + accessToken: 'acc', + expiresAt: new Date(Date.now() - 1000), // expired + refreshToken: 'ref', + sessionKey: 'sess', + userEmail: 'e@e.com', + userId: 'u1', + }) + deps.tokenStore.load.resolves(expiredToken) + const {getRequestHandler} = makeHandler(sandbox, deps) + + try { + await getRequestHandler()({action: 'list'}, 'client-1') + expect.fail('Expected error') + } catch (error) { + expect(error).to.be.instanceOf(NotAuthenticatedError) + } + }) + }) + + describe('setup', () => { + it('registers handler for VcEvents.SIGNING_KEY', () => { + const deps = makeDeps(sandbox) + const {getRequestHandler} = makeHandler(sandbox, deps) + expect(getRequestHandler()).to.be.a('function') + expect(deps.transport.onRequest.calledWith(VcEvents.SIGNING_KEY)).to.be.true + }) + }) + + describe('action routing (via injectable service seam)', () => { + it('add action calls service.addKey and returns mapped key', async () => { + const {getRequestHandler, signingKeyService} = makeHandlerWithInjectedService(sandbox) + + const result = await getRequestHandler()( + {action: 'add', publicKey: 'ssh-ed25519 AAAA... test@example.com', title: 'My laptop'}, + 'client-1', + ) as {action: string; key: SigningKeyItem} + + expect(signingKeyService.addKey.calledOnce).to.be.true + expect(signingKeyService.addKey.calledWith('My laptop', 'ssh-ed25519 AAAA... test@example.com')).to.be.true + expect(result.action).to.equal('add') + expect(result.key.id).to.equal(FAKE_KEY.id) + expect(result.key.fingerprint).to.equal(FAKE_KEY.fingerprint) + }) + + it('list action calls service.listKeys and returns mapped keys', async () => { + const {getRequestHandler, signingKeyService} = makeHandlerWithInjectedService(sandbox) + + const result = await getRequestHandler()( + {action: 'list'}, + 'client-1', + ) as {action: string; keys: SigningKeyItem[]} + + expect(signingKeyService.listKeys.calledOnce).to.be.true + expect(result.action).to.equal('list') + expect(result.keys).to.have.length(1) + expect(result.keys[0].id).to.equal(FAKE_KEY.id) + }) + + it('remove action calls service.removeKey with keyId', async () => { + const {getRequestHandler, signingKeyService} = makeHandlerWithInjectedService(sandbox) + + const result = await getRequestHandler()( + {action: 'remove', keyId: 'key-id-1'}, + 'client-1', + ) as {action: string} + + expect(signingKeyService.removeKey.calledOnce).to.be.true + expect(signingKeyService.removeKey.calledWith('key-id-1')).to.be.true + expect(result.action).to.equal('remove') + }) + + it('still enforces auth guard even when service is injected', async () => { + const requestHandlers: Record = {} + const signingKeyService: Stubbed = { + addKey: sandbox.stub().resolves(FAKE_KEY), + listKeys: sandbox.stub().resolves([FAKE_KEY]), + removeKey: sandbox.stub().resolves(), + } + const tokenStore: Stubbed = { + clear: sandbox.stub().resolves(), + load: sandbox.stub().resolves(), // no token + save: sandbox.stub().resolves(), + } + const transport: Stubbed = { + broadcastToProject: sandbox.stub(), + close: sandbox.stub().resolves(), + emitToClient: sandbox.stub(), + emitToProject: sandbox.stub(), + initialize: sandbox.stub().resolves(), + offRequest: sandbox.stub(), + onRequest: sandbox.stub().callsFake((event: string, h: RequestHandler) => { + requestHandlers[event] = h + }), + } as unknown as Stubbed + + const handler = new SigningKeyHandler({ + iamBaseUrl: IAM_BASE_URL, + signingKeyService, + tokenStore, + transport, + }) + handler.setup() + + try { + await requestHandlers[VcEvents.SIGNING_KEY]({action: 'list'}, 'client-1') + expect.fail('Expected NotAuthenticatedError') + } catch (error) { + expect(error).to.be.instanceOf(NotAuthenticatedError) + expect(signingKeyService.listKeys.called).to.be.false + } + }) + }) +}) diff --git a/test/unit/infra/transport/handlers/vc-handler.test.ts b/test/unit/infra/transport/handlers/vc-handler.test.ts index 8c7a7125b..7df27e7d7 100644 --- a/test/unit/infra/transport/handlers/vc-handler.test.ts +++ b/test/unit/infra/transport/handlers/vc-handler.test.ts @@ -28,11 +28,13 @@ import {BrvConfig} from '../../../../../src/server/core/domain/entities/brv-conf import {GitAuthError, GitError} from '../../../../../src/server/core/domain/errors/git-error.js' import {NotAuthenticatedError} from '../../../../../src/server/core/domain/errors/task-error.js' import {VcError} from '../../../../../src/server/core/domain/errors/vc-error.js' +import {SigningKeyCache} from '../../../../../src/server/infra/ssh/signing-key-cache.js' import {VcHandler} from '../../../../../src/server/infra/transport/handlers/vc-handler.js' import { type IVcBranchRequest, type IVcBranchResponse, type IVcCheckoutResponse, + type IVcConfigResponse, type IVcFetchResponse, type IVcMergeRequest, type IVcMergeResponse, @@ -187,7 +189,7 @@ function makeDeps(sandbox: SinonSandbox, projectPath: string): TestDeps { } } -function makeVcHandler(deps: TestDeps): VcHandler { +function makeVcHandler(deps: TestDeps, signingKeyCache?: SigningKeyCache): VcHandler { return new VcHandler({ broadcastToProject: deps.broadcastToProject, contextTreeService: deps.contextTreeService, @@ -195,6 +197,7 @@ function makeVcHandler(deps: TestDeps): VcHandler { gitService: deps.gitService, projectConfigStore: deps.projectConfigStore, resolveProjectPath: deps.resolveProjectPath, + ...(signingKeyCache ? {signingKeyCache} : {}), spaceService: deps.spaceService, teamService: deps.teamService, tokenStore: deps.tokenStore, @@ -704,7 +707,7 @@ describe('VcHandler', () => { } }) - it('should throw VcError USER_NOT_CONFIGURED with pre-filled hint when logged in', async () => { + it('should resolve author from auth token when config is missing and user is logged in', async () => { const deps = makeDeps(sandbox, projectPath) deps.gitService.isInitialized.resolves(true) deps.gitService.status.resolves({ @@ -724,16 +727,11 @@ describe('VcHandler', () => { deps.tokenStore.load.resolves(mockToken) makeVcHandler(deps).setup() - try { - await deps.requestHandlers[VcEvents.COMMIT]({message: 'test'}, CLIENT_ID) - expect.fail('Expected error') - } catch (error) { - expect(error).to.be.instanceOf(VcError) - if (error instanceof VcError) { - expect(error.code).to.equal(VcErrorCode.USER_NOT_CONFIGURED) - expect(error.message).to.include('login@example.com') - } - } + await deps.requestHandlers[VcEvents.COMMIT]({message: 'test'}, CLIENT_ID) + + expect(deps.gitService.commit.calledOnce).to.be.true + const commitArgs = deps.gitService.commit.firstCall.args[0] + expect(commitArgs.author).to.deep.equal({email: 'login@example.com', name: 'login@example.com'}) }) it('should throw VcError GIT_NOT_INITIALIZED when git not initialized', async () => { @@ -752,6 +750,157 @@ describe('VcHandler', () => { } }) + it('should NOT sign when signingKey is set but commitSign is absent and sign flag is not passed', async () => { + const deps = makeDeps(sandbox, projectPath) + deps.gitService.isInitialized.resolves(true) + deps.gitService.status.resolves({ + files: [{path: 'a.md', staged: true, status: 'added'}], + isClean: false, + }) + // signingKey is configured but commit.sign is NOT set — should NOT auto-sign + deps.vcGitConfigStore.get.resolves({email: 'test@example.com', name: 'Test', signingKey: '/some/key'}) + makeVcHandler(deps).setup() + + await deps.requestHandlers[VcEvents.COMMIT]({message: 'test'}, CLIENT_ID) + + // gitService.commit should have been called without an onSign callback + expect(deps.gitService.commit.calledOnce).to.be.true + const commitArgs = deps.gitService.commit.firstCall.args[0] + expect(commitArgs.onSign).to.be.undefined + }) + + it('should throw SIGNING_KEY_NOT_SUPPORTED for encrypted OpenSSH key (not PASSPHRASE_REQUIRED)', async () => { + // Build a synthetic encrypted OpenSSH key (aes256-ctr cipher) so probeSSHKey detects it + const sshStr = (s: Buffer | string) => { + const b = Buffer.isBuffer(s) ? s : Buffer.from(s) + const len = Buffer.allocUnsafe(4) + len.writeUInt32BE(b.length, 0) + return Buffer.concat([len, b]) + } + + const writeU32 = (v: number) => { + const b = Buffer.allocUnsafe(4) + b.writeUInt32BE(v, 0) + return b + } + + const pubBlob = Buffer.concat([sshStr('ssh-ed25519'), sshStr(Buffer.alloc(32, 0xaa))]) + const encKeyBuf = Buffer.concat([ + Buffer.from('openssh-key-v1\0', 'binary'), + sshStr('aes256-ctr'), + sshStr('bcrypt'), + sshStr(Buffer.alloc(0)), + writeU32(1), + sshStr(pubBlob), + sshStr(Buffer.alloc(64, 0xbb)), + ]) + const encKeyPem = `-----BEGIN OPENSSH PRIVATE KEY-----\n${encKeyBuf.toString('base64')}\n-----END OPENSSH PRIVATE KEY-----` + const realTmpDir = fs.mkdtempSync(join(tmpdir(), 'brv-enc-key-test-')) + try { + const encKeyPath = join(realTmpDir, 'enc_key') + writeFileSync(encKeyPath, encKeyPem, {mode: 0o600}) + + const deps = makeDeps(sandbox, projectPath) + deps.gitService.isInitialized.resolves(true) + deps.gitService.status.resolves({ + files: [{path: 'a.md', staged: true, status: 'added'}], + isClean: false, + }) + deps.vcGitConfigStore.get.resolves({ + commitSign: true, + email: 'test@example.com', + name: 'Test', + signingKey: encKeyPath, + }) + makeVcHandler(deps).setup() + + try { + await deps.requestHandlers[VcEvents.COMMIT]({message: 'signed commit', sign: true}, CLIENT_ID) + expect.fail('Expected error') + } catch (error) { + expect(error).to.be.instanceOf(VcError) + if (error instanceof VcError) { + expect(error.code).to.equal(VcErrorCode.SIGNING_KEY_NOT_SUPPORTED) + } + } + } finally { + rmSync(realTmpDir, {force: true, recursive: true}) + } + }) + + it('should propagate "Unsupported OpenSSH key type" for RSA OpenSSH key, NOT throw PASSPHRASE_REQUIRED (ENG-2002 B7)', async () => { + // Regression test for ENG-2002 B7: before B6's regex fix, probeSSHKey + // false-marked RSA/ECDSA OpenSSH keys as needsPassphrase:true and + // handleCommit threw PASSPHRASE_REQUIRED, triggering a spurious passphrase + // prompt in the CLI. After B6, the unsupported-key-type error must + // propagate cleanly — no PASSPHRASE_REQUIRED conversion anywhere. + const sshStr = (s: Buffer | string) => { + const b = Buffer.isBuffer(s) ? s : Buffer.from(s) + const len = Buffer.allocUnsafe(4) + len.writeUInt32BE(b.length, 0) + return Buffer.concat([len, b]) + } + + const writeU32 = (v: number) => { + const b = Buffer.allocUnsafe(4) + b.writeUInt32BE(v, 0) + return b + } + + // Unencrypted ssh-rsa OpenSSH key (cipher='none') so the only failure mode + // is the unsupported-keytype check, not encryption handling. + const pubBlob = Buffer.concat([sshStr('ssh-rsa'), sshStr(Buffer.alloc(64, 0xaa))]) + const rsaKeyBuf = Buffer.concat([ + Buffer.from('openssh-key-v1\0', 'binary'), + sshStr('none'), + sshStr('none'), + sshStr(Buffer.alloc(0)), + writeU32(1), + sshStr(pubBlob), + sshStr(Buffer.alloc(64, 0xbb)), + ]) + const rsaKeyPem = `-----BEGIN OPENSSH PRIVATE KEY-----\n${rsaKeyBuf.toString('base64')}\n-----END OPENSSH PRIVATE KEY-----` + const realTmpDir = fs.mkdtempSync(join(tmpdir(), 'brv-rsa-key-test-')) + try { + const rsaKeyPath = join(realTmpDir, 'rsa_key') + writeFileSync(rsaKeyPath, rsaKeyPem, {mode: 0o600}) + + const deps = makeDeps(sandbox, projectPath) + deps.gitService.isInitialized.resolves(true) + deps.gitService.status.resolves({ + files: [{path: 'a.md', staged: true, status: 'added'}], + isClean: false, + }) + deps.vcGitConfigStore.get.resolves({ + commitSign: true, + email: 'test@example.com', + name: 'Test', + signingKey: rsaKeyPath, + }) + makeVcHandler(deps).setup() + + try { + await deps.requestHandlers[VcEvents.COMMIT]({message: 'signed commit', sign: true}, CLIENT_ID) + expect.fail('Expected error') + } catch (error: unknown) { + // Narrow first so subsequent property access doesn't need `as`. + if (!(error instanceof Error)) { + expect.fail(`Expected Error instance, got ${typeof error}`) + } + + // After B6, probeSSHKey throws a plain Error (not a VcError) for + // unsupported keytypes. Pin both the type and the message so any + // future wrapping into VcError is caught and reviewed deliberately + // (a wrap that uses code=PASSPHRASE_REQUIRED would re-introduce the + // CLI retry loop this test guards against). + expect(error).to.not.be.instanceOf(VcError) + expect(error.message).to.match(/Unsupported OpenSSH key type/) + } + } finally { + rmSync(realTmpDir, {force: true, recursive: true}) + } + }) + it('should throw VcError MERGE_CONFLICT when index has unmerged entries', async () => { const deps = makeDeps(sandbox, projectPath) deps.gitService.isInitialized.resolves(true) @@ -866,6 +1015,251 @@ describe('VcHandler', () => { } } }) + + it('should import git signing config across OS and strip .pub extension', async () => { + const realProjectPath = fs.mkdtempSync(join(tmpdir(), 'brv-test-config-')) + + // Create a real git repository to satisfy execFile('git') in the handler + mkdirSync(realProjectPath, {recursive: true}) + const {execSync} = await import('node:child_process') + execSync('git init', {cwd: realProjectPath}) + + // Set up fake keys + const keyPath = join(realProjectPath, 'fake_key') + const pubPath = `${keyPath}.pub` + // A structurally valid fake ed25519 openssh private key so parseSSHPrivateKey can probe it + const fakePrivateKey = `-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACBEVnO2XhZlYg1Z3TzT3XwB2YvM/XQYQnZQY1X/sVq1HQAAAJB6q16Aeqte +gAAAAAtzc2gtZWQyNTUxOQAAACBEVnO2XhZlYg1Z3TzT3XwB2YvM/XQYQnZQY1X/sVq1HQ +AAAEDe9Y3Z4YwZQy0YvTz/Q0ZQY1X/sVq1HQZWYg1Z3TzT3XwB2YvM/XQYQnZQY1X/sVq1 +HQBEVnO2XhZlYg1Z3TzT3XwB2YvM/XQYQnZQY1X/sVq1HQBEVnO2XhZlYg1Z3TzT3XwB2Y +vM/XQYQnZQY1X/sVq1HQAAABF0ZXN0QGV4YW1wbGUuY29tAQI= +-----END OPENSSH PRIVATE KEY-----` + writeFileSync(keyPath, fakePrivateKey, {mode: 0o600}) + writeFileSync(pubPath, 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIERWc7ZeFmViDVndPNPdfAHZi8z9dBhCdlBjVf+xWrUd', {mode: 0o644}) + + // Emulate git configuration where the user pointed to the .pub file + execSync(`git config user.signingkey "${pubPath}"`, {cwd: realProjectPath}) + execSync(`git config commit.gpgsign true`, {cwd: realProjectPath}) + + const deps = makeDeps(sandbox, realProjectPath) + deps.vcGitConfigStore.get.resolves({}) + makeVcHandler(deps).setup() + + const result = await deps.requestHandlers[VcEvents.CONFIG]({importGitSigning: true}, CLIENT_ID) as IVcConfigResponse + + expect(deps.vcGitConfigStore.set.calledOnce).to.be.true + const savedConfig = deps.vcGitConfigStore.set.firstCall.args[1] + expect(savedConfig.commitSign).to.be.true + // Should strip .pub and save the private key path + expect(savedConfig.signingKey).to.equal(keyPath) + expect(result.value).to.equal(keyPath) + }) + + it('handleImportGitSigning: throws SIGNING_KEY_NOT_FOUND when signingKey path does not exist on disk', async () => { + const realProjectPath = fs.mkdtempSync(join(tmpdir(), 'brv-test-import-badpath-')) + mkdirSync(realProjectPath, {recursive: true}) + const {execSync} = await import('node:child_process') + execSync('git init', {cwd: realProjectPath}) + // Point signingKey to a non-existent path (local config overrides global) + execSync('git config user.signingKey "/tmp/definitely-does-not-exist-brv-test"', {cwd: realProjectPath}) + + const deps = makeDeps(sandbox, realProjectPath) + deps.vcGitConfigStore.get.resolves({}) + makeVcHandler(deps).setup() + + try { + await deps.requestHandlers[VcEvents.CONFIG]({importGitSigning: true}, CLIENT_ID) + expect.fail('Expected error') + } catch (error) { + expect(error).to.be.instanceOf(VcError) + if (error instanceof VcError) { + expect(error.code).to.equal(VcErrorCode.SIGNING_KEY_NOT_FOUND) + } + } + }) + + it('handleImportGitSigning: throws INVALID_CONFIG_VALUE when gpg.format is not ssh', async () => { + const realProjectPath = fs.mkdtempSync(join(tmpdir(), 'brv-test-import-gpgformat-')) + mkdirSync(realProjectPath, {recursive: true}) + const {execSync} = await import('node:child_process') + execSync('git init', {cwd: realProjectPath}) + + const keyPath = join(realProjectPath, 'fake_key') + const fakeKey = `-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQgAAAJgCtf3VArX9 +1QAAAAtzc2gtZWQyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQg +AAEB01GDi+m4swI3lsGv870+yJFfAJP0CcFSDPcTyCUpaBSYh9Posmi46km6A8piXvKIn +BgiWuHXbhM5iNo3PGM1CAAAAEHRlc3RAZXhhbXBsZS5jb20BAgMEBQ== +-----END OPENSSH PRIVATE KEY-----` + writeFileSync(keyPath, fakeKey, {mode: 0o600}) + execSync(`git config user.signingkey "${keyPath}"`, {cwd: realProjectPath}) + execSync('git config gpg.format gpg', {cwd: realProjectPath}) + + const deps = makeDeps(sandbox, realProjectPath) + deps.vcGitConfigStore.get.resolves({}) + makeVcHandler(deps).setup() + + try { + await deps.requestHandlers[VcEvents.CONFIG]({importGitSigning: true}, CLIENT_ID) + expect.fail('Expected error') + } catch (error) { + expect(error).to.be.instanceOf(VcError) + if (error instanceof VcError) { + expect(error.code).to.equal(VcErrorCode.INVALID_CONFIG_VALUE) + expect(error.message).to.include('gpg') + } + } + }) + + // Regression test for PR #435 review comment #14: when the user changes + // user.signingkey, the cache entry for the PREVIOUS key path must be + // evicted. Otherwise the old ParsedSSHKey sits in daemon memory until its + // 30-min TTL expires — low-severity leak, but the cache exposes an + // invalidate(projectPath, keyPath) API for exactly this purpose. + it('handleConfig user.signingkey: invalidates cache entry for the previous key path', async () => { + const realProjectPath = fs.mkdtempSync(join(tmpdir(), 'brv-test-config-cache-invalidate-')) + const oldKeyPath = join(realProjectPath, 'old_key') + const newKeyPath = join(realProjectPath, 'new_key') + const fakeKey = `-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQgAAAJgCtf3VArX9 +1QAAAAtzc2gtZWQyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQg +AAEB01GDi+m4swI3lsGv870+yJFfAJP0CcFSDPcTyCUpaBSYh9Posmi46km6A8piXvKIn +BgiWuHXbhM5iNo3PGM1CAAAAEHRlc3RAZXhhbXBsZS5jb20BAgMEBQ== +-----END OPENSSH PRIVATE KEY-----` + writeFileSync(oldKeyPath, fakeKey, {mode: 0o600}) + writeFileSync(newKeyPath, fakeKey, {mode: 0o600}) + + const cache = new SigningKeyCache() + // Parse once via the real parser so we have an authentic ParsedSSHKey to seed. + const {parseSSHPrivateKey} = await import('../../../../../src/server/infra/ssh/ssh-key-parser.js') + const parsed = await parseSSHPrivateKey(oldKeyPath) + cache.set(projectPath, oldKeyPath, parsed) + + const deps = makeDeps(sandbox, projectPath) + deps.vcGitConfigStore.get.resolves({signingKey: oldKeyPath}) + makeVcHandler(deps, cache).setup() + + await deps.requestHandlers[VcEvents.CONFIG]({key: 'user.signingkey', value: newKeyPath}, CLIENT_ID) + + // Old entry must be gone (get() returns a falsy value once the entry + // is evicted — null today, undefined after review comment #11). + expect(cache.get(projectPath, oldKeyPath), 'old cache entry should be invalidated').to.not.be.ok + + rmSync(realProjectPath, {force: true, recursive: true}) + }) + + // Regression test for PR #435 review comment #16: user.signingkey paths + // must be absolute. Relative paths silently resolve against the daemon's + // CWD and break if the daemon restarts from a different working + // directory. Matches git's own `user.signingKey` semantic. + it('handleConfig user.signingkey: rejects non-absolute path with INVALID_CONFIG_VALUE', async () => { + const deps = makeDeps(sandbox, projectPath) + deps.vcGitConfigStore.get.resolves({}) + makeVcHandler(deps).setup() + + try { + await deps.requestHandlers[VcEvents.CONFIG]({key: 'user.signingkey', value: './id_ed25519'}, CLIENT_ID) + expect.fail('Expected VcError') + } catch (error) { + expect(error).to.be.instanceOf(VcError) + if (error instanceof VcError) { + expect(error.code).to.equal(VcErrorCode.INVALID_CONFIG_VALUE) + expect(error.message).to.match(/absolute/i) + } + } + + expect(deps.vcGitConfigStore.set.called, 'must not write config when path is rejected').to.be.false + }) + + // Regression test for PR #435 review comment #24. handleImportGitSigning + // reads via `git config --get `, which resolves local → global → + // system. The existing tests all set the value in local repo config — + // the real-world common setup is a single `user.signingKey` in + // ~/.gitconfig. If someone refactored the implementation to skip the + // global lookup, every existing test still passed while the most + // common user configuration would ship broken. + it('handleImportGitSigning: reads signingKey from GIT_CONFIG_GLOBAL when no local repo config exists', async () => { + const realProjectPath = fs.mkdtempSync(join(tmpdir(), 'brv-test-import-global-')) + mkdirSync(realProjectPath, {recursive: true}) + const {execSync} = await import('node:child_process') + execSync('git init', {cwd: realProjectPath}) + // Fresh repo has no local user.signingKey, so resolution falls through + // to GIT_CONFIG_GLOBAL — no explicit --unset needed. + + const keyPath = join(realProjectPath, 'global_key') + const fakeKey = `-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQgAAAJgCtf3VArX9 +1QAAAAtzc2gtZWQyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQg +AAEB01GDi+m4swI3lsGv870+yJFfAJP0CcFSDPcTyCUpaBSYh9Posmi46km6A8piXvKIn +BgiWuHXbhM5iNo3PGM1CAAAAEHRlc3RAZXhhbXBsZS5jb20BAgMEBQ== +-----END OPENSSH PRIVATE KEY-----` + writeFileSync(keyPath, fakeKey, {mode: 0o600}) + + // Write ONLY a global gitconfig (pointed to via GIT_CONFIG_GLOBAL). + const globalConfigPath = join(realProjectPath, 'fake_gitconfig') + writeFileSync(globalConfigPath, `[user]\n\tsigningKey = ${keyPath}\n[gpg]\n\tformat = ssh\n[commit]\n\tgpgSign = true\n`) + + // Point git's global-config lookup at our temp file for the duration + // of this test (handler spawns git as a child process and inherits env). + const originalGlobal = process.env.GIT_CONFIG_GLOBAL + process.env.GIT_CONFIG_GLOBAL = globalConfigPath + + try { + const deps = makeDeps(sandbox, realProjectPath) + deps.vcGitConfigStore.get.resolves({}) + makeVcHandler(deps).setup() + + await deps.requestHandlers[VcEvents.CONFIG]({importGitSigning: true}, CLIENT_ID) + + expect(deps.vcGitConfigStore.set.calledOnce, 'config must be written').to.be.true + const savedConfig = deps.vcGitConfigStore.set.firstCall.args[1] + expect(savedConfig.signingKey).to.equal(keyPath) + expect(savedConfig.commitSign).to.equal(true) + } finally { + if (originalGlobal === undefined) { + delete process.env.GIT_CONFIG_GLOBAL + } else { + process.env.GIT_CONFIG_GLOBAL = originalGlobal + } + + rmSync(realProjectPath, {force: true, recursive: true}) + } + }) + + it('handleImportGitSigning: does NOT set commitSign when gpgsign is explicitly false', async () => { + const realProjectPath = fs.mkdtempSync(join(tmpdir(), 'brv-test-import-nosign-')) + mkdirSync(realProjectPath, {recursive: true}) + const {execSync} = await import('node:child_process') + execSync('git init', {cwd: realProjectPath}) + + const keyPath = join(realProjectPath, 'fake_key') + const fakeKey = `-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQgAAAJgCtf3VArX9 +1QAAAAtzc2gtZWQyNTUxOQAAACAmIfT6LJouOpJugPKYl7yiJwYIlrh124TOYjaNzxjNQg +AAEB01GDi+m4swI3lsGv870+yJFfAJP0CcFSDPcTyCUpaBSYh9Posmi46km6A8piXvKIn +BgiWuHXbhM5iNo3PGM1CAAAAEHRlc3RAZXhhbXBsZS5jb20BAgMEBQ== +-----END OPENSSH PRIVATE KEY-----` + writeFileSync(keyPath, fakeKey, {mode: 0o600}) + execSync(`git config user.signingkey "${keyPath}"`, {cwd: realProjectPath}) + // Explicitly set gpgsign=false in local repo to override global config + execSync('git config commit.gpgSign false', {cwd: realProjectPath}) + + const deps = makeDeps(sandbox, realProjectPath) + deps.vcGitConfigStore.get.resolves({}) + makeVcHandler(deps).setup() + + await deps.requestHandlers[VcEvents.CONFIG]({importGitSigning: true}, CLIENT_ID) + + const savedConfig = deps.vcGitConfigStore.set.firstCall.args[1] + // commitSign should be false since gpgsign is explicitly false + expect(savedConfig.commitSign).to.equal(false) + }) }) describe('handlePush', () => { @@ -3120,6 +3514,7 @@ describe('VcHandler', () => { try { deps.gitService.listBranches.resolves([{isCurrent: false, isRemote: false, name: 'feature'}]) deps.vcGitConfigStore.get.resolves() + deps.tokenStore.load.resolves() makeVcHandler(deps).setup() try { diff --git a/test/unit/shared/transport/events/vc-events.test.ts b/test/unit/shared/transport/events/vc-events.test.ts new file mode 100644 index 000000000..d5c7a109e --- /dev/null +++ b/test/unit/shared/transport/events/vc-events.test.ts @@ -0,0 +1,38 @@ +import {expect} from 'chai' + +import {isVcConfigKey, VC_CONFIG_KEYS} from '../../../../../src/shared/transport/events/vc-events.js' + +describe('VcConfigKey', () => { + it('canonical keys are all lowercase (no dual camel-case entries)', () => { + // M4: camel-case variants used to be part of the exported set and caused + // duplicate entries in VC_CONFIG_KEYS. Canonical is lowercase only. + expect(VC_CONFIG_KEYS).to.deep.equal([ + 'user.name', + 'user.email', + 'user.signingkey', + 'commit.sign', + ]) + }) + + describe('isVcConfigKey() — lenient case-insensitive parse', () => { + it('accepts the canonical lowercase form', () => { + expect(isVcConfigKey('user.signingkey')).to.be.true + }) + + it('accepts a camel-case variant (backward compat for git-style spelling)', () => { + expect(isVcConfigKey('user.signingKey')).to.be.true + }) + + it('accepts an all-uppercase variant', () => { + expect(isVcConfigKey('USER.SIGNINGKEY')).to.be.true + }) + + it('rejects an unrelated key', () => { + expect(isVcConfigKey('user.notARealKey')).to.be.false + }) + + it('rejects the empty string', () => { + expect(isVcConfigKey('')).to.be.false + }) + }) +}) diff --git a/test/unit/tui/features/vc/commit/vc-commit-flow-state.test.ts b/test/unit/tui/features/vc/commit/vc-commit-flow-state.test.ts new file mode 100644 index 000000000..f1b85c01a --- /dev/null +++ b/test/unit/tui/features/vc/commit/vc-commit-flow-state.test.ts @@ -0,0 +1,148 @@ +import {expect} from 'chai' + +import {VcErrorCode} from '../../../../../../src/shared/transport/events/vc-events.js' +import { + type CommitFlowState, + initialCommitFlowState, + MAX_PASSPHRASE_RETRIES, + reduceCommitFlow, +} from '../../../../../../src/tui/features/vc/commit/components/vc-commit-flow-state.js' + +function errorWithCode(code: string): Error & {code: string} { + const err = new Error('Simulated transport error') as Error & {code: string} + err.code = code + return err +} + +describe('reduceCommitFlow()', () => { + describe('from initial (committing, attempt 0)', () => { + it('commit-success → done(success) with formatted SHA + message + unsigned tag', () => { + const result = reduceCommitFlow(initialCommitFlowState, { + message: 'hello', + sha: 'abcdef1234567890', + type: 'commit-success', + }) + + expect(result).to.deep.equal({ + kind: 'done', + message: '[abcdef1] hello', + outcome: 'success', + }) + }) + + it('commit-success with signed:true includes the signing indicator', () => { + const result = reduceCommitFlow(initialCommitFlowState, { + message: 'signed msg', + sha: 'deadbeefcafe', + signed: true, + type: 'commit-success', + }) + + expect(result.kind).to.equal('done') + if (result.kind !== 'done') throw new Error('unreachable') + expect(result.message).to.equal('[deadbee] signed msg 🔏') + }) + + it('commit-error with PASSPHRASE_REQUIRED → awaiting-passphrase(attempt=1)', () => { + const result = reduceCommitFlow(initialCommitFlowState, { + error: errorWithCode(VcErrorCode.PASSPHRASE_REQUIRED), + type: 'commit-error', + }) + + expect(result).to.deep.equal({attempt: 1, kind: 'awaiting-passphrase'}) + }) + + it('commit-error with a non-passphrase code → done(error)', () => { + const result = reduceCommitFlow(initialCommitFlowState, { + error: errorWithCode('SOMETHING_ELSE'), + type: 'commit-error', + }) + + expect(result.kind).to.equal('done') + if (result.kind !== 'done') throw new Error('unreachable') + expect(result.outcome).to.equal('error') + expect(result.message).to.match(/Failed to commit/) + }) + }) + + describe('from awaiting-passphrase', () => { + const awaiting: CommitFlowState = {attempt: 1, kind: 'awaiting-passphrase'} + + it('passphrase-submitted → committing (attempt preserved)', () => { + const result = reduceCommitFlow(awaiting, {type: 'passphrase-submitted'}) + expect(result).to.deep.equal({attempt: 1, kind: 'committing'}) + }) + + it('passphrase-cancelled → done(cancelled)', () => { + const result = reduceCommitFlow(awaiting, {type: 'passphrase-cancelled'}) + expect(result).to.deep.equal({ + kind: 'done', + message: 'Passphrase entry cancelled.', + outcome: 'cancelled', + }) + }) + }) + + describe('retry cap', () => { + it('PASSPHRASE_REQUIRED after MAX_PASSPHRASE_RETRIES attempts → done(error, "Too many failed…")', () => { + const atCap: CommitFlowState = {attempt: MAX_PASSPHRASE_RETRIES, kind: 'committing'} + const result = reduceCommitFlow(atCap, { + error: errorWithCode(VcErrorCode.PASSPHRASE_REQUIRED), + type: 'commit-error', + }) + + expect(result.kind).to.equal('done') + if (result.kind !== 'done') throw new Error('unreachable') + expect(result.outcome).to.equal('error') + expect(result.message).to.match(/Too many failed passphrase attempts/) + expect(result.message).to.include(String(MAX_PASSPHRASE_RETRIES)) + }) + + it('PASSPHRASE_REQUIRED at attempt < MAX → back to awaiting-passphrase (attempt+1)', () => { + const belowCap: CommitFlowState = {attempt: 1, kind: 'committing'} + const result = reduceCommitFlow(belowCap, { + error: errorWithCode(VcErrorCode.PASSPHRASE_REQUIRED), + type: 'commit-error', + }) + + expect(result).to.deep.equal({attempt: 2, kind: 'awaiting-passphrase'}) + }) + }) + + describe('terminal state is absorbing', () => { + const done: CommitFlowState = {kind: 'done', message: 'x', outcome: 'success'} + + it('ignores commit-success once done', () => { + const result = reduceCommitFlow(done, { + message: 'new', + sha: '1234567890', + type: 'commit-success', + }) + expect(result).to.equal(done) + }) + + it('ignores commit-error once done', () => { + const result = reduceCommitFlow(done, {error: new Error('x'), type: 'commit-error'}) + expect(result).to.equal(done) + }) + + it('ignores passphrase events once done', () => { + expect(reduceCommitFlow(done, {type: 'passphrase-submitted'})).to.equal(done) + expect(reduceCommitFlow(done, {type: 'passphrase-cancelled'})).to.equal(done) + }) + }) + + describe('out-of-order events are no-ops', () => { + it('passphrase-submitted while committing → unchanged', () => { + const committing: CommitFlowState = {attempt: 0, kind: 'committing'} + const result = reduceCommitFlow(committing, {type: 'passphrase-submitted'}) + expect(result).to.equal(committing) + }) + + it('passphrase-cancelled while committing → unchanged', () => { + const committing: CommitFlowState = {attempt: 0, kind: 'committing'} + const result = reduceCommitFlow(committing, {type: 'passphrase-cancelled'}) + expect(result).to.equal(committing) + }) + }) +})