-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: add ssh_signing_key input for SSH commit signing #784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| #!/usr/bin/env bun | ||
|
|
||
| /** | ||
| * Cleanup SSH signing key after action completes | ||
| * This is run as a post step for security purposes | ||
| */ | ||
|
|
||
| import { cleanupSshSigning } from "../github/operations/git-config"; | ||
|
|
||
| async function run() { | ||
| try { | ||
| await cleanupSshSigning(); | ||
| } catch (error) { | ||
| // Don't fail the action if cleanup fails, just log it | ||
| console.error("Failed to cleanup SSH signing key:", error); | ||
| } | ||
| } | ||
|
|
||
| if (import.meta.main) { | ||
| run(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,9 +6,14 @@ | |
| */ | ||
|
|
||
| import { $ } from "bun"; | ||
| import { mkdir, writeFile, rm } from "fs/promises"; | ||
| import { join } from "path"; | ||
| import { homedir } from "os"; | ||
| import type { GitHubContext } from "../context"; | ||
| import { GITHUB_SERVER_URL } from "../api/config"; | ||
|
|
||
| const SSH_SIGNING_KEY_PATH = join(homedir(), ".ssh", "claude_signing_key"); | ||
|
|
||
| type GitUser = { | ||
| login: string; | ||
| id: number; | ||
|
|
@@ -54,3 +59,50 @@ export async function configureGitAuth( | |
|
|
||
| console.log("Git authentication configured successfully"); | ||
| } | ||
|
|
||
| /** | ||
| * Configure git to use SSH signing for commits | ||
| * This is an alternative to GitHub API-based commit signing (use_commit_signing) | ||
| */ | ||
| export async function setupSshSigning(sshSigningKey: string): Promise<void> { | ||
| console.log("Configuring SSH signing for commits..."); | ||
|
|
||
| // Validate SSH key format | ||
| if (!sshSigningKey.trim()) { | ||
| throw new Error("SSH signing key cannot be empty"); | ||
| } | ||
| if ( | ||
| !sshSigningKey.includes("BEGIN") || | ||
| !sshSigningKey.includes("PRIVATE KEY") | ||
| ) { | ||
| throw new Error("Invalid SSH private key format"); | ||
| } | ||
|
|
||
| // Create .ssh directory with secure permissions (700) | ||
| const sshDir = join(homedir(), ".ssh"); | ||
| await mkdir(sshDir, { recursive: true, mode: 0o700 }); | ||
|
|
||
| // Write the signing key atomically with secure permissions (600) | ||
| await writeFile(SSH_SIGNING_KEY_PATH, sshSigningKey, { mode: 0o600 }); | ||
| console.log(`✓ SSH signing key written to ${SSH_SIGNING_KEY_PATH}`); | ||
|
|
||
| // Configure git to use SSH signing | ||
| await $`git config gpg.format ssh`; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Performance: Sequential Git Config Commands These three independent git config operations can be parallelized to save ~20-60ms per run: await Promise.all([
$`git config gpg.format ssh`,
$`git config user.signingkey ${SSH_SIGNING_KEY_PATH}`,
$`git config commit.gpgsign true`,
]); |
||
| await $`git config user.signingkey ${SSH_SIGNING_KEY_PATH}`; | ||
| await $`git config commit.gpgsign true`; | ||
|
|
||
| console.log("✓ Git configured to use SSH signing for commits"); | ||
| } | ||
|
|
||
| /** | ||
| * Clean up the SSH signing key file | ||
| * Should be called in the post step for security | ||
| */ | ||
| export async function cleanupSshSigning(): Promise<void> { | ||
| try { | ||
| await rm(SSH_SIGNING_KEY_PATH, { force: true }); | ||
| console.log("✓ SSH signing key cleaned up"); | ||
| } catch (error) { | ||
| console.log("No SSH signing key to clean up"); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,10 @@ import type { Mode, ModeOptions, ModeResult } from "../types"; | |
| import type { PreparedContext } from "../../create-prompt/types"; | ||
| import { prepareMcpConfig } from "../../mcp/install-mcp-server"; | ||
| import { parseAllowedTools } from "./parse-tools"; | ||
| import { configureGitAuth } from "../../github/operations/git-config"; | ||
| import { | ||
| configureGitAuth, | ||
| setupSshSigning, | ||
| } from "../../github/operations/git-config"; | ||
| import type { GitHubContext } from "../../github/context"; | ||
| import { isEntityContext } from "../../github/context"; | ||
|
|
||
|
|
@@ -79,7 +82,27 @@ export const agentMode: Mode = { | |
|
|
||
| async prepare({ context, githubToken }: ModeOptions): Promise<ModeResult> { | ||
| // Configure git authentication for agent mode (same as tag mode) | ||
| if (!context.inputs.useCommitSigning) { | ||
| // SSH signing takes precedence if provided | ||
| const useSshSigning = !!context.inputs.sshSigningKey; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code Duplication This SSH signing detection and git configuration logic (lines 86-118) is duplicated in tag mode (lines 96-128 in Recommendation: Extract this logic into a shared function like |
||
| const useApiCommitSigning = | ||
| context.inputs.useCommitSigning && !useSshSigning; | ||
|
|
||
| if (useSshSigning) { | ||
| // Setup SSH signing for commits | ||
| await setupSshSigning(context.inputs.sshSigningKey); | ||
|
|
||
| // Still configure git auth for push operations (user/email and remote URL) | ||
| const user = { | ||
| login: context.inputs.botName, | ||
| id: parseInt(context.inputs.botId), | ||
| }; | ||
| try { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent Error Handling Agent mode swallows git configuration errors with a comment "Continue anyway", while tag mode (lines 109-114) properly throws errors. This inconsistency could lead to confusing failures later. Recommendation: Either make both modes consistent, or document why agent mode should continue on error. Users deserve clear error messages when git setup fails. |
||
| await configureGitAuth(githubToken, context, user); | ||
| } catch (error) { | ||
| console.error("Failed to configure git authentication:", error); | ||
| // Continue anyway - git operations may still work with default config | ||
| } | ||
| } else if (!useApiCommitSigning) { | ||
| // Use bot_id and bot_name from inputs directly | ||
| const user = { | ||
| login: context.inputs.botName, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Input Validation Missing
The SSH key should be validated before use to catch misconfigurations early and prevent potential injection issues.
Recommendation: