fix(cert): address PR #1688 review feedback

- clean up the partial certificate directory when the initial write
  fails, not just the database row
- log a warning when the existing self-signed private key cannot be
  reused so operators notice the public-key fingerprint has changed
- defensively copy the model's Domains and IPAddresses slices in
  SelfSignedOptionsFromModel
- require an explicit "Save now" confirmation after generating from the
  site editor, and write the directives into the editor first so the
  user can review the diff before saving

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
0xJacky
2026-05-23 12:46:47 +08:00
parent 3184994aee
commit 91a77aa770
3 changed files with 50 additions and 11 deletions
+6
View File
@@ -2,6 +2,7 @@ package certificate
import (
"net/http"
"os"
"path/filepath"
"strings"
@@ -72,6 +73,11 @@ func GenerateSelfSignedCert(c *gin.Context) {
keyPath := filepath.Join(dir, "private.key")
if err = writeSelfSignedFiles(certPath, keyPath, opts); err != nil {
// remove the partial directory so a failed generation leaves no orphan files
if rmErr := os.RemoveAll(dir); rmErr != nil {
logger.Errorf("self-signed cert directory cleanup failed for id %d at %s: %v",
certModel.ID, dir, rmErr)
}
// roll back the row so a failed generation leaves no orphan record
if rollbackErr := db.Delete(certModel).Error; rollbackErr != nil {
logger.Errorf("self-signed cert rollback failed for id %d: %v", certModel.ID, rollbackErr)
@@ -1,5 +1,6 @@
<script setup lang="ts">
import type { Cert } from '@/api/cert'
import { Modal } from 'ant-design-vue'
import SelfSignedCertForm from '@/views/certificate/components/SelfSignedCertForm.vue'
import { useTLSDirectives } from '../../composables/useTLSDirectives'
import { useSiteEditorStore } from '../SiteEditor/store'
@@ -8,6 +9,7 @@ const editorStore = useSiteEditorStore()
const { curDirectivesMap } = storeToRefs(editorStore)
const { ensureTLSDirectives } = useTLSDirectives()
const { message } = useGlobalApp()
const [modal, ContextHolder] = Modal.useModal()
const refForm = useTemplateRef('refForm')
@@ -20,20 +22,44 @@ function open() {
refForm.value?.open()
}
async function onCreated(certificate: Cert) {
function onCreated(certificate: Cert) {
// Write the TLS directives into the editor first so the user can see the
// pending diff regardless of whether they choose to save now or review.
ensureTLSDirectives(certificate.ssl_certificate_path, certificate.ssl_certificate_key_path)
try {
await editorStore.save()
message.success($gettext('Self-signed certificate applied'))
}
catch {
message.error($gettext('Certificate written but failed to save site configuration'))
}
modal.confirm({
title: $gettext('Save the site configuration now?'),
content: $gettext(
'The certificate has been generated at %{path} and the ssl_certificate '
+ 'directives have been added to the current server block. Save the '
+ 'configuration now, or review the changes in the editor and save manually.',
{ path: certificate.ssl_certificate_path },
),
okText: $gettext('Save now'),
cancelText: $gettext('Review first'),
centered: true,
async onOk() {
try {
await editorStore.save()
message.success($gettext('Self-signed certificate applied'))
}
catch {
message.error($gettext(
'Saving the site configuration failed; the certificate directives are in '
+ 'the editor — review the changes and retry from the Save button.',
))
}
},
onCancel() {
message.info($gettext('Certificate directives added to the editor; review and save when ready.'))
},
})
}
</script>
<template>
<div class="self-signed-cert">
<ContextHolder />
<AFormItem :label="$gettext('Self-signed certificate')">
<AButton @click="open">
{{ $gettext('Generate self-signed certificate') }}
+10 -3
View File
@@ -16,6 +16,7 @@ import (
"github.com/0xJacky/Nginx-UI/model"
"github.com/go-acme/lego/v5/certcrypto"
"github.com/uozi-tech/cosy"
"github.com/uozi-tech/cosy/logger"
)
const (
@@ -119,21 +120,27 @@ func RegenerateSelfSigned(certModel *model.Cert) (certPEM, keyPEM []byte, err er
signer, parseErr := loadSelfSignedKey(certModel.SSLCertificateKeyPath)
if parseErr != nil || signer == nil {
// fall back to a fresh key when the existing key cannot be reused
// Fall back to a fresh key when the existing key cannot be reused.
// Log a warning so operators notice that the certificate's public key
// (and therefore its fingerprint) has changed.
logger.Warnf("self-signed key %s could not be reused, generating a fresh key: %v",
certModel.SSLCertificateKeyPath, parseErr)
return GenerateSelfSigned(opts)
}
return signSelfSigned(opts, signer)
}
// SelfSignedOptionsFromModel builds SelfSignedOptions from a persisted Cert.
// The model's slices are defensively copied so options consumers cannot
// observe concurrent mutations on the persisted Cert.
func SelfSignedOptionsFromModel(certModel *model.Cert) SelfSignedOptions {
opts := SelfSignedOptions{
DNSNames: certModel.Domains,
DNSNames: append([]string(nil), certModel.Domains...),
KeyType: certModel.GetKeyType(),
ValidityDays: SelfSignedDefaultValidityDays,
}
if certModel.SelfSignedConfig != nil {
opts.IPAddresses = certModel.SelfSignedConfig.IPAddresses
opts.IPAddresses = append([]string(nil), certModel.SelfSignedConfig.IPAddresses...)
if certModel.SelfSignedConfig.ValidityDays > 0 {
opts.ValidityDays = certModel.SelfSignedConfig.ValidityDays
}