feat/gen ssh key by fiftin · Pull Request #3656 · semaphoreui/semaphore

fiftin · 2026-02-21T14:18:18Z

feat(secrets): generate private ssh key on server
feat(secrets): gen SSH key on server
fix(secrets): show public key for updated secrets too
feat(secrets): don't allow user override plain field

chatgpt-codex-connector · 2026-02-21T14:18:24Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot

Pull request overview

This pull request implements a server-side SSH key generation feature for Semaphore UI. Users can now optionally generate SSH key pairs on the server instead of providing their own private keys. The generated public key is displayed to users for copying to authorized_keys files on target systems.

Changes:

Added server-side SSH key generation using RSA 2048-bit keys with public/private key pair creation
Implemented UI components to display generated public keys in a dialog with copy-to-clipboard functionality
Extended database schema to store public key metadata in the Plain field for SSH keys
Added generate_ssh_key flag to control key generation behavior in the frontend and backend

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
services/server/access_key_svc.go	Implements maybeGenerateSSHPrivateKey function to generate SSH keys and store public key in Plain field
db/AccessKey.go	Adds GenerateSSHKey transient field for API communication
db/sql/access_key.go	Updates SQL queries to persist Plain field for both create and update operations
db/bolt/access_key.go	Contains commented implementation note for BoltDB Plain field handling
api/projects/keys.go	Returns Plain field in create response to enable frontend public key display
web/src/views/project/Keys.vue	Adds public key display dialog with copy functionality after key creation/update
web/src/components/KeyForm.vue	Adds generate_ssh_key checkbox and displays public key for existing generated keys

Copilot · 2026-02-21T15:08:37Z


+    <v-checkbox
+      v-model="item.generate_ssh_key"
+      label="Generate SSH Key"


The checkbox label "Generate SSH Key" is hardcoded and not internationalized, while other labels in the form use the i18n translation function. This creates inconsistency with the rest of the codebase where all user-facing strings are translated.

Consider changing label="Generate SSH Key" to :label="$t('generateSshKey')" and adding the corresponding translation keys.

Suggested change

label="Generate SSH Key"

:label="$t('generateSshKey')"

Copilot · 2026-02-21T15:08:37Z

+      <div style="position: relative">
+        <pre
+          style="
+            overflow: auto;
+            background: gray;
+            color: white;
+            border-radius: 10px;
+            margin-top: 5px;
+          "
+          class="pa-2"
+          >{{ publicKey }}</pre
+        >
+
+        <CopyClipboardButton
+          style="position: absolute; right: 0; top: 0; transform: scale(0.9);"


Inline styles are used extensively for the public key display section. Consider extracting these styles to a scoped <style> section or using CSS classes for better maintainability and consistency. This makes it easier to maintain and update styling across the application.

Suggested change

<div style="position: relative">

<pre

style="

overflow: auto;

background: gray;

color: white;

border-radius: 10px;

margin-top: 5px;

"

class="pa-2"

>{{ publicKey }}</pre

>

<CopyClipboardButton

style="position: absolute; right: 0; top: 0; transform: scale(0.9);"

<div class="public-key-container">

<pre

class="pa-2 public-key-display"

>{{ publicKey }}</pre

>

<CopyClipboardButton

class="public-key-copy-button"

Copilot · 2026-02-21T15:08:37Z

+              style="
+                overflow: auto;
+                background: gray;
+                color: white;
+                border-radius: 10px;
+                margin-top: 5px;
+              "
+              class="pa-2"


Inline styles are duplicated between KeyForm.vue and Keys.vue for displaying the public key. The same styling is applied to the <pre> element in both files (gray background, white color, border-radius, etc.). Consider creating a reusable component or shared styles to avoid this duplication and ensure consistency.

Suggested change

style="

overflow: auto;

background: gray;

color: white;

border-radius: 10px;

margin-top: 5px;

"

class="pa-2"

class="pa-2 mt-1 rounded overflow-auto grey darken-3 white--text"

Copilot · 2026-02-21T15:08:38Z

+      const isGeneratedOnCreate = e && e.action === 'new';
+      const isGeneratedOnUpdate = e && e.action === 'edit' && e.item && e.item.generate_ssh_key;


The logic for determining if a public key should be shown on update (line 156) assumes that if generate_ssh_key is true, a key was generated. However, this flag comes from the request body, not the response. If the backend fails to generate the key (but doesn't return an error), or if the key generation is skipped for some reason, the dialog may still appear with an empty public key. Consider adding an additional check to ensure the public key was actually generated before showing the dialog.

Copilot · 2026-02-21T15:08:38Z

+func maybeGenerateSSHPrivateKey(key *db.AccessKey) error {
+	if !key.GenerateSSHKey || key.Type != db.AccessKeySSH {
+		key.Plain = nil
+		return nil
+	}
+
+	var b bytes.Buffer
+	privateKeyFile := bufio.NewWriter(&b)
+
+	publicKey, err := util.GeneratePrivateKey(privateKeyFile)
+	if err != nil {
+		return err
+	}
+
+	err = privateKeyFile.Flush()
+	if err != nil {
+		return err
+	}
+
+	key.SshKey.PrivateKey = b.String()
+
+	type sshPublicKey struct {
+		PublicKey string `json:"public_key"`
+	}
+
+	plainBytes, err := json.Marshal(sshPublicKey{
+		PublicKey: publicKey,
+	})
+	if err != nil {
+		return err
+	}
+
+	plain := string(plainBytes)
+	key.Plain = &plain
+	return nil
+}


The SSH key generation functionality introduced by this PR lacks automated test coverage. Consider adding tests to verify that maybeGenerateSSHPrivateKey correctly generates keys when GenerateSSHKey is true, sets the Plain field with the public key, and properly handles the case when GenerateSSHKey is false. Tests should also verify that the generated private key is valid and properly encrypted.

Example test cases to add:

Test that GenerateSSHKey=true generates both private and public keys

Test that GenerateSSHKey=false preserves existing private key

Test that Plain field is set correctly with public key JSON

Test that generated keys are valid RSA keys

Copilot · 2026-02-21T15:08:38Z

+      v-model="item.generate_ssh_key"
+      label="Generate SSH Key"
+      v-if="!isReadOnly && item.type === 'ssh'"
+      :disabled="formSaving || !canEditSecrets"


The checkbox for generating SSH keys is only disabled when canEditSecrets is false, but it should also be disabled for existing keys where override_secret is required. When editing an existing key without override_secret checked, users can toggle generate_ssh_key, but this won't have any effect because the secret won't be updated. This creates a confusing user experience.

Consider adding || (!isNew && !item.override_secret) to the disabled condition to make it clear that SSH key generation only works when secrets can be edited.

Suggested change

:disabled="formSaving || !canEditSecrets"

:disabled="formSaving || !canEditSecrets || (!isNew && !item.override_secret)"

Copilot · 2026-02-21T15:08:38Z

+      :max-width="700"
+      v-model="createdPublicKeyDialog"
+      :save-button-text="null"
+      title="Generated SSH Public Key"


The dialog title "Generated SSH Public Key" is hardcoded and not internationalized. All other strings in the file use the i18n translation function (e.g., $t('create'), $t('save')). This string should be extracted to the translation files for consistency and to support multiple languages.

Consider changing title="Generated SSH Public Key" to title="$t('generatedSshPublicKey')" and adding the corresponding translation keys.

Suggested change

title="Generated SSH Public Key"

:title="$t('generatedSshPublicKey')"

cursor

Stale comment

Security review outcome (PR #3656)

Verdict: One high-confidence issue remains in new code: incorrect SQL arguments in UpdateAccessKey when override_secret is set (including the new “generate SSH key” update path). Prior automation threads were cleared so this assessment is the active one.

Slack summary (copy/paste): PR feat/gen_ssh_key — 1 High: db/sql/access_key.go UpdateAccessKey adds an extra key.Plain to the OverrideSecret branch so placeholders no longer match columns (source_storage_id / source_storage_key / source_storage_type get wrong values). Fix: remove the stray args = append(args, key.Plain) or add a matching plain=? in the SET clause. No other medium+ issues validated (Vue JSON.parse is server-shaped JSON; XSS risk low).

No Slack integration available here; summary included above and in this review.

_{Sent by Cursor Automation: Find vulnerabilities}

cursor · 2026-05-02T15:39:23Z

 		query += ", type=?, secret=?, source_storage_id=?, source_storage_key=?, source_storage_type=?"
 		args = append(args, key.Type)
 		args = append(args, key.Secret)
+		args = append(args, key.Plain)


High — SQL update corruption / integrity

This args = append(args, key.Plain) does not match the UPDATE clause (type=?, secret=?, source_storage_id=?, …). Every placeholder after secret is shifted: e.g. source_storage_id is bound to key.Plain (a string), source_storage_key to key.SourceStorageID (int/nil), etc. Any flow that sets OverrideSecret (including generated SSH key rotation) can corrupt access_key metadata, break vault/env references, and cause runtime failures when resolving secrets — availability and trust-boundary impact on the key store.

Fix: Remove this line, or if plain must be updated in the same statement, add plain=? to the SET list in the correct position and keep args aligned.

cursor

Security review outcome (PR #3656)

Verdict: One high-confidence issue remains in new code: incorrect SQL arguments in UpdateAccessKey when override_secret is set (including the new “generate SSH key” update path). Prior automation threads were cleared so this assessment is the active one.

Slack summary (copy/paste): PR feat/gen_ssh_key — 1 High: db/sql/access_key.go UpdateAccessKey adds an extra key.Plain to the OverrideSecret branch so placeholders no longer match columns (source_storage_id / source_storage_key / source_storage_type get wrong values). Fix: remove the stray args = append(args, key.Plain) or add a matching plain=? in the SET clause. No other medium+ issues validated (Vue JSON.parse is server-shaped JSON; XSS risk low).

No Slack integration available here; summary included above and in this review.

_{Sent by Cursor Automation: Find vulnerabilities}

cursor · 2026-05-30T13:41:43Z

 		query += ", type=?, secret=?, source_storage_id=?, source_storage_key=?, source_storage_type=?"
 		args = append(args, key.Type)
 		args = append(args, key.Secret)
+		args = append(args, key.Plain)


High — SQL update corruption / wrong secret storage binding

This args = append(args, key.Plain) does not match the UPDATE clause (type=?, secret=?, source_storage_id=?, …). Every placeholder after secret is shifted: e.g. source_storage_id is bound to key.Plain (a JSON string), source_storage_key to key.SourceStorageID, source_storage_type to key.SourceStorageKey.

Any flow that sets OverrideSecret (including generate SSH key on edit via access_key_svc.Update) can corrupt vault/external-storage metadata for the key. That can misroute secret resolution (credential confusion) or break integrity checks—treat as a security-relevant integrity bug, not just a functional typo.

Fix: Remove this append, or add an explicit plain=? column to the SET list and keep argument order aligned.

fiftin added 4 commits February 21, 2026 18:42

feat(secrets): generate private ssh key on server

2a741b7

feat(secrets): gen SSH key on server

186b1b8

fix(secrets): show public key for updated secrets too

3463fbb

feat(secrets): don't allow user override plain field

1fc0245

fiftin requested a review from Copilot February 21, 2026 15:01

Copilot started reviewing on behalf of fiftin February 21, 2026 15:01 View session

Copilot AI reviewed Feb 21, 2026

View reviewed changes

fiftin mentioned this pull request Feb 24, 2026

SSH CA certificate key store support #3531

Open

Merge branch 'develop' into feat/gen_ssh_key

aaecddd

cursor Bot reviewed May 2, 2026

View reviewed changes

Merge branch 'develop' into feat/gen_ssh_key

1d638a4

cursor Bot reviewed May 30, 2026

View reviewed changes

		const isGeneratedOnCreate = e && e.action === 'new';
		const isGeneratedOnUpdate = e && e.action === 'edit' && e.item && e.item.generate_ssh_key;

	:disabled="formSaving \|\| !canEditSecrets"
	:disabled="formSaving \|\| !canEditSecrets \|\| (!isNew && !item.override_secret)"

	title="Generated SSH Public Key"
	:title="$t('generatedSshPublicKey')"

Uh oh!

Uh oh!

Conversation

fiftin commented Feb 21, 2026

Uh oh!

chatgpt-codex-connector Bot commented Feb 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Security review outcome (PR #3656)

Uh oh!

cursor Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Security review outcome (PR #3656)

Uh oh!

cursor Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cursor Bot left a comment •

edited

Loading