feat: add PKCS#11/HSM signer support for Fulcio and CTLog by sampras343 · Pull Request #2182 · securesign/secure-sign-operator

sampras343 · 2026-08-04T16:01:06Z

Summary

Add PKCS#11 as a signer backend option for both Fulcio and CTLog, enabling hardware security module (HSM) integration for signing operations. Stacked on #2181 (Auth) and #2175 (Volumes).

API Changes

FulcioPKCS11Config: ConfigRef (crypto11 JSON secret), KeyConfig (ID + Label)
CTlogPKCS11Config: PinSecretRef, PublicKeyRef, TokenLabel, ModulePath
PKCS11 * field on FulcioSigner and CTlogSigner
Signer Type enum extended to file;pkcs11
CEL validation rules enforce mutual exclusivity

Controller Changes

ensure_pkcs11_config actions validate secrets exist before deployment, with content-hash drift detection (not ObservedGeneration) to avoid spurious rotation on unrelated spec changes
Fulcio ensurePKCS11Deployment: --ca=pkcs11ca, crypto11 config mount, CA cert mount, HSM volumes
CTLog: --pkcs11_module_path arg, PKCS#11 protobuf config generation
generate_signer gated with positive Type==file check (not !=pkcs11)
File/PKCS#11 mode switching cleans up stale volumes

Design Decisions

No Persistence field on CTlogPKCS11Config — users configure HSM token persistence via spec.ctlog.volumes
hsm-lib-export init container is user-defined via spec.initContainers, not operator-generated
Drift detection via content hash in condition message, not ObservedGeneration
All HSM-specific logic confined to PKCS#11 actions; shared helpers remain generic

Review Comments Addressed

(from PR #2128):

#	Comment	Status
Rotation bug	CanHandle fires on any spec change	Done — content-hash drift detection
Persistence dead fields	Size/StorageClass/Retain unused	Done — field dropped, use spec.volumes
Negative signer check	Type != pkcs11 fragile	Done — positive Type == file check
Missing CTLog tests	No ensure_pkcs11_config tests	Done — full test suite
Duplicated validation	PinSecretRef/PublicKeyRef blocks identical	Done — validateSecretRef helper
No HSM hardcoding	Generic helpers must stay clean	Done — zero HSM refs in ensure/

Test plan

go build ./... and go vet ./... pass
Fulcio ensure_pkcs11_config tests (CanHandle matrix, Handle valid/invalid, drift detection)
CTLog ensure_pkcs11_config tests (same coverage)
v1alpha1 roundtrip fuzz tests pass with PKCS#11 fields
All existing tests still pass (file mode regression)
Deploy PKCS#11 CR on OCP with SoftHSM — Fulcio + CTLog reach Ready
cosign sign + verify in PKCS#11 mode

Depends on: #2181

🤖 Generated with Claude Code

qodo-for-securesign · 2026-08-04T16:02:51Z

PR Summary by Qodo

Add PKCS#11/HSM signer backend for Fulcio and CTLog

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add PKCS#11 signer mode to Fulcio and CTLog APIs with mutual-exclusion validation.
• Reconcile PKCS#11 secrets/volumes, generate CTLog PKCS#11 config, and deploy workloads in HSM
 mode.
• Add drift detection, conversion roundtrip fuzzing, and comprehensive controller/unit test
 coverage.

Diagram

graph TD
CR["Signing CRs (Securesign/Fulcio/CTLog)"] --> Ctrl["Operator controllers"] --> Ensure["ensure_pkcs11_config"] --> Sec[("K8s Secrets")]
Sec --> Deploy["Deployment & config reconcile"] --> Pods["Fulcio/CTLog workloads"]
Ensure --> Conds{{"Status conditions"}}
subgraph Legend
  direction LR
  _svc["Controller/Action"] ~~~ _sec[("Secret")] ~~~ _cond{{"Condition"}}
end

High-Level Assessment

The following are alternative approaches to this PR:

1. Store drift hash in a dedicated status field (vs condition message)

➕ Avoids parsing/free-form condition messages for hash matching
➕ Easier to evolve format and validate via schema
➕ Reduces risk of false positives if message text changes
➖ Requires adding new status fields and conversion logic
➖ Slightly larger API surface area to maintain

2. Annotate managed Deployment/Secret with desired-spec hash instead of status conditions

➕ Naturally couples drift detection to the reconciled artifact being rolled
➕ Avoids status writes on every reconcile when only comparing desired vs actual
➖ Harder to diagnose from CR status alone
➖ Still needs careful handling of which inputs contribute to the hash

3. Shared PKCS#11 reconciliation helper library for Fulcio + CTLog

➕ Reduces duplication across ensure/deploy/config paths
➕ Encourages consistent volume naming/cleanup and validation behavior
➖ May over-generalize: Fulcio uses crypto11/key selection; CTLog uses tokenLabel-only semantics
➖ Refactor risk while introducing a new feature

Recommendation: Current approach is reasonable for incremental rollout: keep PKCS#11 logic localized to dedicated actions and deploy-time wiring, with explicit CEL mutual-exclusion and strong test coverage. The main improvement worth considering is moving the content-hash out of the condition message into a dedicated status field (or an annotation on the reconciled resources) to reduce reliance on message string matching and ease future evolution.

Files changed (34) +2959 / -33

Enhancement (17) +897 / -30

ctlog_types.goAdd CTLog PKCS#11 signer config and validations +37/-1

Add CTLog PKCS#11 signer config and validations

• Introduces CTlogSignerTypePKCS11 and CTlogPKCS11Config (PIN secret, public key secret, token label, module path). Extends signer type enum and adds CEL XValidations to enforce file vs PKCS#11 mutual exclusivity and required PKCS#11 fields.

api/v1/ctlog_types.go

fulcio_types.goAdd Fulcio PKCS#11 signer config and validations +32/-1

Add Fulcio PKCS#11 signer config and validations

• Introduces FulcioSignerTypePKCS11 plus FulcioPKCS11Config (crypto11 config secret ref and HSM key selection via ID/label). Extends signer type enum and adds CEL XValidations for mutual exclusivity and PKCS#11 certificateChainRef requirement.

api/v1/fulcio_types.go

zz_generated.deepcopy.goRegenerate deep-copies for new PKCS#11 API structs +71/-0

Regenerate deep-copies for new PKCS#11 API structs
• Adds DeepCopy/DeepCopyInto implementations for CTlogPKCS11Config, FulcioPKCS11Config, and PKCS11KeyConfig, and wires PKCS11 pointers into signer DeepCopy methods.
api/v1/zz_generated.deepcopy.go

ctlog_conversion.goPreserve CTLog PKCS#11 signer fields during conversion +1/-0

Preserve CTLog PKCS#11 signer fields during conversion
• Copies restored v1 PKCS#11 signer state into the hub object during ConvertTo to maintain PKCS#11 configuration across API version conversions.
api/v1alpha1/ctlog_conversion.go

fulcio_conversion.goPreserve Fulcio PKCS#11 signer fields during conversion +1/-0

Preserve Fulcio PKCS#11 signer fields during conversion
• Copies restored v1 PKCS#11 signer state into the hub object during ConvertTo to maintain PKCS#11 configuration across API version conversions.
api/v1alpha1/fulcio_conversion.go

securesign_conversion.goPreserve nested Fulcio/CTLog PKCS#11 signer fields in Securesign conversion +2/-0

Preserve nested Fulcio/CTLog PKCS#11 signer fields in Securesign conversion
• Copies restored PKCS#11 signer config for Fulcio and CTLog when converting the Securesign CR, keeping the new signer mode stable across versions.
api/v1alpha1/securesign_conversion.go

constants.goAdd shared HSM volume/mount constants +5/-0

Add shared HSM volume/mount constants
• Defines common HSM volume names and mount paths (hsm-tokens, hsm-lib) for reuse across Fulcio and CTLog reconciliation.
internal/constants/constants.go

constants.goAdd CTLog PKCS#11 condition constant +7/-4

Add CTLog PKCS#11 condition constant
• Introduces PKCS11Condition and a shared resolved reason used by the new ensure_pkcs11_config action to gate PKCS#11 readiness.
internal/controller/ctlog/actions/constants.go

deployment.goDeploy CTLog with PKCS#11 module arg and HSM volumes +70/-0

Deploy CTLog with PKCS#11 module arg and HSM volumes
• When signer type is pkcs11, injects --pkcs11_module_path based on module filename in hsm-lib mount. Adds ensurePKCS11Resources to mount/manage hsm-lib and hsm-tokens and cleans them up when switching back to file mode.
internal/controller/ctlog/actions/deployment.go

ensure_pkcs11_config.goAdd CTLog ensure_pkcs11_config action with content-hash drift detection +186/-0

Add CTLog ensure_pkcs11_config action with content-hash drift detection
• Introduces a new action that validates pin/public key secrets exist and are non-empty, sets Status.PublicKeyRef, invalidates ConfigCondition to force config regen, and sets PKCS11Condition true with a deterministic spec hash to avoid generation-based spurious rotations.
internal/controller/ctlog/actions/ensure_pkcs11_config.go

server_config.goGenerate CTLog server config for PKCS#11 mode and hash inputs +84/-16

Generate CTLog server config for PKCS#11 mode and hash inputs

• Updates prerequisites so PrivateKeyRef is required only for file mode. Adds PKCS#11 config generation path using CreateCtlogPKCS11Config and annotates server config with pkcs11ContentHash derived from secrets and spec fields to trigger updates appropriately.

internal/controller/ctlog/actions/server_config.go

ctlog_controller.goWire CTLog PKCS#11 condition + ensure action into reconciler +5/-1

Wire CTLog PKCS#11 condition + ensure action into reconciler
• Updates condition supplier to include PKCS11Condition when signer type is pkcs11 and registers the new ensure_pkcs11_config action early in the action sequence.
internal/controller/ctlog/ctlog_controller.go

ctlog_config.goAdd CTLog PKCS#11 protobuf config generator +61/-0

Add CTLog PKCS#11 protobuf config generator
• Adds CreateCtlogPKCS11Config to build a prototext LogMultiConfig using keyspb.PKCS11Config, including DER public key extraction from PEM and roots handling without embedding private key material.
internal/controller/ctlog/utils/ctlog_config.go

constants.goAdd Fulcio PKCS#11 condition and volume/mount constants +7/-1

Add Fulcio PKCS#11 condition and volume/mount constants
• Defines Fulcio PKCS#11 condition and standard mount paths/volume names for pkcs11 config and CA cert secrets used by pkcs11 deployment mode.
internal/controller/fulcio/actions/constants.go

deployment.goDeploy Fulcio in PKCS#11 mode with crypto11 config/cert mounts and HSM volumes +164/-5

Deploy Fulcio in PKCS#11 mode with crypto11 config/cert mounts and HSM volumes
• Selects between file and PKCS#11 deployment strategies based on signer type. Implements ensurePKCS11Deployment to set --ca=pkcs11ca args, mount crypto11 config and CA cert secrets, manage hsm-lib/hsm-tokens volumes, and clean up file-mode resources when switching modes.
internal/controller/fulcio/actions/deployment.go

ensure_pkcs11_config.goAdd Fulcio ensure_pkcs11_config action with drift detection +159/-0

Add Fulcio ensure_pkcs11_config action with drift detection
• Validates crypto11 config secret and CA cert secret availability, populates Status.Certificate.CARef, sets CertCondition true, and sets PKCS11Condition true with a deterministic content hash to avoid re-handling on unrelated spec changes.
internal/controller/fulcio/actions/ensure_pkcs11_config.go

fulcio_controller.goWire Fulcio PKCS#11 condition + ensure action into reconciler +5/-1

Wire Fulcio PKCS#11 condition + ensure action into reconciler
• Updates condition supplier to include PKCS11Condition when signer type is pkcs11 and adds the ensure_pkcs11_config action into the Fulcio action pipeline.
internal/controller/fulcio/fulcio_controller.go

Bug fix (3) +23 / -3

generate_signer.goDisable CTLog file-key generation when PKCS#11 signer is selected +3/-0

Disable CTLog file-key generation when PKCS#11 signer is selected
• Adds IsEnabled gating so generate_signer runs only for file mode (or empty type), preventing file-key secret generation in PKCS#11 mode.
internal/controller/ctlog/actions/generate_signer.go

generate_signer.goDisable Fulcio file-key generation when PKCS#11 signer is selected +3/-0

Disable Fulcio file-key generation when PKCS#11 signer is selected
• Adds IsEnabled gating so generate_signer runs only for file mode (or empty type), preventing file signer secret generation in PKCS#11 mode.
internal/controller/fulcio/actions/generate_signer.go

pod_spec.goFix init container reconciliation to avoid infinite update loops +17/-3

Fix init container reconciliation to avoid infinite update loops
• Preserves API-server default ImagePullPolicy unless explicitly set. Uses slices.Clone to preserve nil vs empty slices for Env/VolumeMounts, preventing omitempty round-trip diffs that can trigger endless CreateOrUpdate churn.
internal/utils/kubernetes/ensure/pod_spec.go

Tests (10) +1655 / -0

conversion_roundtrip_test.goAdd fuzzers for PKCS#11 signer roundtrip safety +97/-0

Add fuzzers for PKCS#11 signer roundtrip safety

• Adds dedicated fuzzer functions to generate valid file vs PKCS#11 combinations for FulcioSigner and CTlogSigner. Registers these fuzzers in securesign/ctlog/fulcio conversion roundtrip tests to cover the new fields.

api/v1alpha1/conversion_roundtrip_test.go

deployment_test.goTest CTLog PKCS#11 volumes/mounts, args, and cleanup behavior +155/-0

Test CTLog PKCS#11 volumes/mounts, args, and cleanup behavior
• Adds coverage for PKCS#11 deployment wiring: expected volumes/mount paths, module path argument behavior, cleanup on mode switch, and preserving user-provided PVC for hsm-tokens.
internal/controller/ctlog/actions/deployment_test.go

ensure_pkcs11_config_test.goAdd CTLog PKCS#11 ensure action test suite +434/-0

Add CTLog PKCS#11 ensure action test suite
• Adds CanHandle matrix tests (type gating, state gating, same/different hash) and Handle tests for missing/empty secrets, status updates, and generation-bump regression coverage.
internal/controller/ctlog/actions/ensure_pkcs11_config_test.go

generate_signer_test.goTest CTLog generate_signer enablement for file vs PKCS#11 +30/-0

Test CTLog generate_signer enablement for file vs PKCS#11
• Adds unit tests ensuring the generate_signer action is disabled in PKCS#11 mode and enabled in file/empty-type modes.
internal/controller/ctlog/actions/generate_signer_test.go

server_config_test.goAdd CTLog server-config tests for PKCS#11 config generation +310/-0

Add CTLog server-config tests for PKCS#11 config generation
• Adds tests verifying PKCS#11 mode successfully generates config secrets and that nil pin/public key refs produce errors, including coverage that PKCS#11 mode bypasses PrivateKeyRef requirements.
internal/controller/ctlog/actions/server_config_test.go

ctlog_config_test.goUnit test CTLog PKCS#11 config generator +107/-0

Unit test CTLog PKCS#11 config generator
• Validates generated prototext parses correctly, contains expected PKCS#11 fields, handles invalid PEM input, and supports multiple root certificates.
internal/controller/ctlog/utils/ctlog_config_test.go

ensure_pkcs11_config_test.goAdd Fulcio PKCS#11 ensure action tests +290/-0

Add Fulcio PKCS#11 ensure action tests
• Covers CanHandle behavior (type/state/hash) and Handle outcomes for missing config/cert refs, missing secrets, and status updates (including CARef behavior and non-prepopulation of CertificateChain).
internal/controller/fulcio/actions/ensure_pkcs11_config_test.go

fulcio_deployment_test.goTest Fulcio PKCS#11 deployment wiring and cleanup in file mode +150/-0

Test Fulcio PKCS#11 deployment wiring and cleanup in file mode
• Adds helpers to build a PKCS#11-mode Fulcio deployment and verifies args and expected pkcs11/hsm volumes. Adds regression test ensuring PKCS#11 volumes are absent in file mode and file-mode volumes remain present.
internal/controller/fulcio/actions/fulcio_deployment_test.go

generate_signer_test.goTest Fulcio generate_signer enablement for file vs PKCS#11 +30/-0

Test Fulcio generate_signer enablement for file vs PKCS#11
• Adds unit tests ensuring generate_signer is disabled in PKCS#11 mode and enabled in file/empty-type modes.
internal/controller/fulcio/actions/generate_signer_test.go

pod_spec_test.goAdd regression tests for nil-slice preservation and ImagePullPolicy defaulting +52/-0

Add regression tests for nil-slice preservation and ImagePullPolicy defaulting
• Adds tests ensuring nil Env/VolumeMounts remain nil after reconciliation and that ImagePullPolicy is not overwritten with an empty string when not specified in the spec.
internal/utils/kubernetes/ensure/pod_spec_test.go

Documentation (1) +141 / -0

pkcs11-hsm-support.mdDocument PKCS#11/HSM setup and example manifests +141/-0

Document PKCS#11/HSM setup and example manifests

• Adds end-to-end documentation describing how to provide PKCS#11 module/token persistence via init containers and volumes. Includes a dual Fulcio+CTLog SoftHSM example, operator-managed volume naming, and CTLog token-level key selection caveats.

docs/pkcs11-hsm-support.md

Other (3) +243 / -0

rhtas.redhat.com_ctlogs.yamlExpose CTLog PKCS#11 signer schema in CRD +68/-0

Expose CTLog PKCS#11 signer schema in CRD

• Adds pkcs11 signer schema (modulePath/pinSecretRef/publicKeyRef/tokenLabel) and updates type enum to include pkcs11. Adds x-kubernetes-validations for required/mutually-exclusive signer configuration.

config/crd/bases/rhtas.redhat.com_ctlogs.yaml

rhtas.redhat.com_fulcios.yamlExpose Fulcio PKCS#11 signer schema in CRD +53/-0

Expose Fulcio PKCS#11 signer schema in CRD

• Adds pkcs11 schema (configRef and keyConfig) and updates signer type enum to include pkcs11. Adds x-kubernetes-validations enforcing pkcs11/file mutual exclusion and certificateChainRef requirement.

config/crd/bases/rhtas.redhat.com_fulcios.yaml

rhtas.redhat.com_securesigns.yamlExpose PKCS#11 signer schema for embedded Fulcio/CTLog in Securesign CRD +122/-0

Expose PKCS#11 signer schema for embedded Fulcio/CTLog in Securesign CRD
• Mirrors CTLog and Fulcio signer pkcs11 schemas inside Securesign’s nested specs, including enum updates and x-kubernetes-validations for mutual exclusivity and required fields.
config/crd/bases/rhtas.redhat.com_securesigns.yaml

qodo-for-securesign · 2026-08-04T16:10:34Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~PKCS11 config ignored silently~~ ✗ Dismissed 🐞 Bug ≡ Correctness ⭐ New

Description

CTlogSigner/FulcioSigner validations don’t require type: pkcs11 when pkcs11 is set, so a CR can
be accepted with pkcs11 config but empty/omitted type, and controllers will treat it as
file-mode. This can lead to file-key generation/deploy behavior even though the user provided HSM
configuration.

Code

api/v1/ctlog_types.go[R119-121]

+// +kubebuilder:validation:XValidation:rule="self.type != 'pkcs11' || has(self.pkcs11)",message="pkcs11 configuration is required when type is pkcs11"
+// +kubebuilder:validation:XValidation:rule="self.type != 'pkcs11' || !has(self.file)",message="file configuration must not be set when type is pkcs11"
+// +kubebuilder:validation:XValidation:rule="self.type != 'file' || !has(self.pkcs11)",message="pkcs11 configuration must not be set when type is file"

Relevance

●●● Strong
They commonly accept adding CRD XValidations to prevent invalid states; silent pkcs11 ignore is a
clear misconfig bug.
PR-#1089

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new XValidations only gate on explicit type values; meanwhile both CTLog and Fulcio
controllers treat type == '' as file-mode and only activate PKCS#11 behavior when `type ==
'pkcs11'`, so PKCS#11 config can be accepted yet ignored.
api/v1/ctlog_types.go[118-133]
api/v1/fulcio_types.go[76-99]
internal/controller/ctlog/actions/generate_signer.go[23-35]
internal/controller/ctlog/actions/deployment.go[140-149]
internal/controller/fulcio/actions/generate_signer.go[32-45]
internal/controller/fulcio/actions/deployment.go[60-67]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The CRD XValidations only constrain `pkcs11` when `self.type` is explicitly `'pkcs11'` or `'file'`. They do not reject `has(self.pkcs11)` when `self.type` is empty/omitted, but the controllers treat empty `type` as file-mode.

### Issue Context
- Empty `type` is treated as file-mode (key generation enabled, PKCS#11 deployment paths not taken).
- Users can accidentally supply PKCS#11 config without setting `type: pkcs11`, leading to silently ignored HSM config.

### Fix Focus Areas
- api/v1/ctlog_types.go[118-133]
- api/v1/fulcio_types.go[76-99]

### Proposed fix
1. Add an explicit XValidation to both `CTlogSigner` and `FulcioSigner`:
  - `rule="!has(self.pkcs11) || self.type == 'pkcs11'"`, message like: "type must be pkcs11 when pkcs11 is set".
2. Regenerate CRDs (and ensure the generated `config/crd/bases/*` reflect the new rule).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~CTlog PKCS11 validation bypass~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

CTlog ensurePKCS11Config sets PKCS11Condition=False on missing/empty secrets but returns without an
action error; once status stops changing, ReturnOnChange can continue to later actions. Because
buildPKCS11Config does not validate non-empty PIN data, an empty PIN Secret value can still be
embedded into the generated CTlog config Secret.

Code

internal/controller/ctlog/actions/ensure_pkcs11_config.go[R71-74]

+	// Validate PinSecretRef
+	if _, err := a.validateSecretRef(ctx, instance, p.PinSecretRef, "spec.signer.pkcs11.pinSecretRef"); err != nil {
+		return a.ReturnOnChange(a.PersistStatus)(ctx, instance)
+	}

Relevance

●●● Strong
Team previously accepted tightening CTlog config validation/error handling patterns; this is similar
reliability hardening.
PR-#1406

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The validation path returns ReturnOnChange(PersistStatus) instead of an error; ReturnOnChange
returns Continue() when no status change occurs, and the PKCS#11 server-config builder passes the
PIN through without checking for empty content.
internal/controller/ctlog/actions/ensure_pkcs11_config.go[71-79]
internal/controller/ctlog/actions/ensure_pkcs11_config.go[140-150]
internal/action/base_action.go[95-115]
internal/controller/ctlog/actions/server_config.go[291-321]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ensurePKCS11Config.Handle()` validates PKCS#11 secrets, but on validation failure it only persists a False condition and returns `ReturnOnChange(PersistStatus)` without an error. If the condition update does not change status (same Reason/Message), `ReturnOnChange` yields `Continue()`, allowing subsequent actions to run.

Separately, `server_config.buildPKCS11Config()` reads the PIN Secret but does not reject an empty PIN (it passes `string(pin)` through), so an empty PIN can reach the generated CTlog config.

### Issue Context
- `validateSecretRef()` explicitly treats `len(data)==0` as an error.
- `ReturnOnChange` continues when the status is unchanged.
- `buildPKCS11Config()` does not enforce non-empty PIN/public key.

### Fix Focus Areas
- internal/controller/ctlog/actions/ensure_pkcs11_config.go[71-79]
- internal/controller/ctlog/actions/ensure_pkcs11_config.go[140-150]
- internal/action/base_action.go[95-115]
- internal/controller/ctlog/actions/server_config.go[291-321]

### Suggested change
1. When `validateSecretRef()` fails, return a requeueing result (e.g., `a.RequeueAfter(...)`) or `a.Error(...)` (retriable) so later actions do not proceed with invalid PKCS#11 inputs.
2. Add defensive checks in `buildPKCS11Config()` (or `CreateCtlogPKCS11Config`) to explicitly reject empty PIN and empty public key data.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~HSM tokens volume overwritten~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Fulcio PKCS#11 deployment overwrites the hsm-tokens volume with EmptyDir whenever it isn’t
EmptyDir/PVC, which clobbers user-provided volume types (e.g., CSI/hostPath/ephemeral). This breaks
the advertised behavior of configuring HSM token persistence via spec.volumes.

Code

internal/controller/fulcio/actions/deployment.go[R441-444]

+		hsmTokensVol := kubernetes.FindVolumeByNameOrCreate(&template.Spec, HSMTokensVolumeName)
+		if !ensure.HasVolume(&template.Spec, HSMTokensVolumeName) || (hsmTokensVol.EmptyDir == nil && hsmTokensVol.PersistentVolumeClaim == nil) {
+			hsmTokensVol.VolumeSource = core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{}}
+		}

Relevance

●●● Strong
Overwriting user-provided volume sources contradicts stated design of using spec.volumes; likely
corrected to preserve user volumes.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
User-defined volumes are applied first via ReconcileUserPodResources, but PKCS#11 deployment then
replaces the hsm-tokens volume source unless it is specifically EmptyDir/PVC, which overwrites
other valid user volume sources.
internal/controller/fulcio/actions/deployment.go[154-157]
internal/controller/fulcio/actions/deployment.go[441-448]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
In Fulcio PKCS#11 mode, `ensurePKCS11Deployment()` rewrites the `hsm-tokens` volume to `EmptyDir` unless the existing `VolumeSource` is `EmptyDir` or `PersistentVolumeClaim`. Because user volumes are reconciled earlier, this can override a user-specified `hsm-tokens` volume source.

### Issue Context
`ensureCommonDeployment()` calls `ensure.ReconcileUserPodResources(...)` with `instance.Spec.Volumes`, and then `ensurePKCS11Deployment()` modifies `hsm-tokens`.

### Fix Focus Areas
- internal/controller/fulcio/actions/deployment.go[154-157]
- internal/controller/fulcio/actions/deployment.go[441-448]

### Suggested change
1. Detect whether `hsm-tokens` was user-defined by checking `instance.Spec.Volumes` (similar to CTlog’s implementation) and only default to `EmptyDir` when it is *not* user-defined.
2. Do not overwrite other user `VolumeSource` types; if some sources are unsupported for your use case, fail fast with a clear error/condition instead of silently replacing them.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View action required (1)

4. ~~CA chain updates ignored~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Fulcio PKCS#11 drift detection only hashes spec.signer.pkcs11 fields, so changing
spec.signer.certificateChain.certificateChainRef may not re-run ensurePKCS11Config and
Status.Certificate.CARef (used by deployment mounts) can remain stale. This can mount the wrong CA
cert after a CA chain rotation.

Code

internal/controller/fulcio/actions/ensure_pkcs11_config.go[R115-118]
+		if cfg.ConfigRef != nil {
+			h.Write([]byte(cfg.ConfigRef.Name))
+			h.Write([]byte(cfg.ConfigRef.Key))
+		}

Relevance

●●● Strong
Drift detection omissions causing stale CA mounts match team’s prior focus on correct
config/rotation detection; likely to be fixed.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
CanHandle() only re-triggers when the stored PKCS#11 hash changes; because the hash excludes
certificateChainRef, changing that ref won’t update Status.Certificate.CARef, yet the deployment
uses that status ref to mount the CA Secret.
internal/controller/fulcio/actions/ensure_pkcs11_config.go[32-48]
internal/controller/fulcio/actions/ensure_pkcs11_config.go[80-105]
internal/controller/fulcio/actions/ensure_pkcs11_config.go[111-123]
internal/controller/fulcio/actions/deployment.go[352-360]
internal/controller/fulcio/actions/deployment.go[430-439]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Fulcio PKCS#11 drift detection is based on `computePKCS11Hash()`, but that hash does not include `spec.signer.certificateChain.certificateChainRef`. As a result, CA chain reference updates may not trigger the PKCS#11 config action to run again, leaving `status.certificate.caRef` stale.

### Issue Context
`ensurePKCS11Config.Handle()` copies `spec.signer.certificateChain.certificateChainRef` into `status.certificate.caRef`, and `ensurePKCS11Deployment()` uses `status.certificate.caRef` to mount the CA cert Secret.

### Fix Focus Areas
- internal/controller/fulcio/actions/ensure_pkcs11_config.go[32-48]
- internal/controller/fulcio/actions/ensure_pkcs11_config.go[80-105]
- internal/controller/fulcio/actions/ensure_pkcs11_config.go[111-123]
- internal/controller/fulcio/actions/deployment.go[352-360]
- internal/controller/fulcio/actions/deployment.go[430-439]

### Suggested change
1. Extend `computePKCS11Hash()` to incorporate `spec.signer.certificateChain.certificateChainRef.name` and `.key` (with delimiters).
2. Optionally add clear separators between all concatenated fields to avoid collisions.
3. Add/adjust tests to cover CA chain reference changes triggering `CanHandle()` and updating `status.certificate.caRef`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~PKCS11 hash ignores read failures~~ ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New

Description

configMatchingAnnotations computes pkcs11ContentHash while silently skipping PIN/public-key
secret data on read errors, but still emits a hash. This can falsely mark an existing server-config
secret as “drifted” and trigger repeated regeneration attempts during transient secret-read/RBAC
failures.

Code

internal/controller/ctlog/actions/server_config.go[R425-434]

+	if instance.Spec.Signer.Type == rhtasv1.CTlogSignerTypePKCS11 && instance.Spec.Signer.PKCS11 != nil {
+		p := instance.Spec.Signer.PKCS11
+		h := sha256.New()
+		if pinData, err := kubernetes.GetSecretData(ctx, i.Client, instance.Namespace, p.PinSecretRef); err == nil {
+			h.Write(pinData)
+		}
+		h.Write([]byte{0})
+		if pubKeyData, err := kubernetes.GetSecretData(ctx, i.Client, instance.Namespace, p.PublicKeyRef); err == nil {
+			h.Write(pubKeyData)
+		}

Relevance

●●● Strong
Repo has accepted handling Kubernetes read errors explicitly; ignoring secret-read failures in drift
hashing likely flagged as reliability issue.
PR-#1406

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Secret validity checking depends on annotations derived from configMatchingAnnotations; since
PKCS#11 annotation hashing ignores secret-read errors, expected annotations can change without any
real spec/secret content change, leading to false invalidation.
internal/controller/ctlog/actions/server_config.go[387-391]
internal/controller/ctlog/actions/server_config.go[396-441]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`configMatchingAnnotations` hashes PKCS#11 inputs but ignores `GetSecretData` errors for PIN/public key, producing a deterministic hash that may differ from the one stored when secrets were readable. `validateExistingSecret` treats annotation mismatch as invalid secret and enters the recreation path.

### Issue Context
This is best handled like other dependency failures: don’t treat the config secret as invalid when required inputs are temporarily unreadable; instead surface an error and requeue.

### Fix Focus Areas
- internal/controller/ctlog/actions/server_config.go[387-390]
- internal/controller/ctlog/actions/server_config.go[396-441]

### Proposed fix
1. Change `configMatchingAnnotations(...)` to return `(map[string]string, error)`.
2. When reading `pinSecretRef`/`publicKeyRef` fails, return the error.
3. In `validateExistingSecret`, if `configMatchingAnnotations` returns an error, treat it as an API/dependency error (return error) rather than `errSecretInvalid`.
  - This avoids false drift detection and repeated regeneration attempts caused purely by transient read failures.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. ~~ModulePath accepts invalid values~~ ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New

Description

CTlogPKCS11Config.modulePath is only validated as an absolute path, but deployment logic derives
--pkcs11_module_path from path.Base(modulePath) and assumes it’s a library filename. Inputs like
/ or /usr/lib64/pkcs11/ will produce invalid runtime paths and break CTLog startup.

Code

internal/controller/ctlog/actions/deployment.go[R142-148]

+		if isPKCS11 {
+			p := instance.Spec.Signer.PKCS11
+			if p == nil {
+				return fmt.Errorf("PKCS#11 config not yet resolved")
+			}
+			modulePath := fmt.Sprintf("%s/%s", constants.HSMLibMountPath, path.Base(p.ModulePath))
+			appArgs = append(appArgs, fmt.Sprintf("--pkcs11_module_path=%s", modulePath))

Relevance

●● Moderate
Defensive validation is plausible, but tightening modulePath constraints may be debated/too strict
without precedent.
PR-#1243

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The CRD pattern only enforces an absolute path while deployment derives the runtime module path
using path.Base, which will not be a valid .so filename for many absolute paths (notably
directories/root).
api/v1/ctlog_types.go[105-116]
internal/controller/ctlog/actions/deployment.go[140-149]
docs/pkcs11-hsm-support.md[118-128]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The controller constructs `--pkcs11_module_path` using `path.Base(spec.signer.pkcs11.modulePath)` but the API only enforces `modulePath` starts with `/`. Directory-like values or `/` can yield basenames that are not a shared library filename, causing CTLog to fail at runtime.

### Issue Context
The operator’s documented behavior is to extract the module filename and mount it from the shared `hsm-lib` EmptyDir, so the spec must contain a file path (not just any absolute path).

### Fix Focus Areas
- api/v1/ctlog_types.go[105-116]
- internal/controller/ctlog/actions/deployment.go[140-149]

### Proposed fix
1. Strengthen CRD validation for `modulePath` to require a filename, e.g. a pattern like:
  - `^/.+\\.so(\\.[0-9]+)*$` (adjust as appropriate).
2. Add a controller-side guard before using `path.Base`:
  - reject basenames of `.`, `/`, or ones not matching expected `.so*` suffix, returning a clear error so the user sees why reconcile can’t proceed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7. PIN duplicated into config Secret 🐞 Bug ⛨ Security

Description

CTlog PKCS#11 mode copies the HSM PIN into the generated CTlog server-config Secret (protobuf text),
duplicating sensitive credential material beyond the original pinSecretRef Secret. This increases
the blast radius for any reader of the generated config Secret.

Code

internal/controller/ctlog/utils/ctlog_config.go[R204-208]

+		PrivateKey: mustMarshalAny(&keyspb.PKCS11Config{
+			TokenLabel: tokenLabel,
+			Pin:        pin,
+			PublicKey:  string(publicKeyPEM),
+		}),

Relevance

●● Moderate
Sensitive-data duplication concern is valid, but may be required by CTlog PKCS#11 config format; no
clear precedent.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code reads the PIN secret and serializes the PIN value into the generated CTlog configuration,
which is then stored in a Kubernetes Secret.
internal/controller/ctlog/actions/server_config.go[291-321]
internal/controller/ctlog/utils/ctlog_config.go[178-212]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The CTlog PKCS#11 configuration generation embeds the PIN value into the generated CTlog config (stored in a Kubernetes Secret). This duplicates the PIN into an additional Secret.

### Issue Context
- `buildPKCS11Config()` reads the PIN Secret data and passes it into `CreateCtlogPKCS11Config()`.
- `CreateCtlogPKCS11Config()` places it into `keyspb.PKCS11Config.Pin` and serializes it.

### Fix Focus Areas
- internal/controller/ctlog/actions/server_config.go[291-321]
- internal/controller/ctlog/utils/ctlog_config.go[178-212]

### Suggested change
1. If CTlog supports it, refactor the configuration to reference the PIN via a mounted Secret file (or other indirection) rather than embedding the PIN value into the generated config Secret.
2. If embedding is required, document this explicitly and consider tightening access patterns around the generated server-config Secret (labels, RBAC expectations, and rotation story) to minimize exposure.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous review results

Review updated until commit f44f7c9

Results up to commit 89cd36d ⚖️ Balanced

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~CA chain updates ignored~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Fulcio PKCS#11 drift detection only hashes spec.signer.pkcs11 fields, so changing
spec.signer.certificateChain.certificateChainRef may not re-run ensurePKCS11Config and
Status.Certificate.CARef (used by deployment mounts) can remain stale. This can mount the wrong CA
cert after a CA chain rotation.

Code

internal/controller/fulcio/actions/ensure_pkcs11_config.go[R115-118]
+		if cfg.ConfigRef != nil {
+			h.Write([]byte(cfg.ConfigRef.Name))
+			h.Write([]byte(cfg.ConfigRef.Key))
+		}

Relevance

●●● Strong
Drift detection omissions causing stale CA mounts match team’s prior focus on correct
config/rotation detection; likely to be fixed.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
CanHandle() only re-triggers when the stored PKCS#11 hash changes; because the hash excludes
certificateChainRef, changing that ref won’t update Status.Certificate.CARef, yet the deployment
uses that status ref to mount the CA Secret.
internal/controller/fulcio/actions/ensure_pkcs11_config.go[32-48]
internal/controller/fulcio/actions/ensure_pkcs11_config.go[80-105]
internal/controller/fulcio/actions/ensure_pkcs11_config.go[111-123]
internal/controller/fulcio/actions/deployment.go[352-360]
internal/controller/fulcio/actions/deployment.go[430-439]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Fulcio PKCS#11 drift detection is based on `computePKCS11Hash()`, but that hash does not include `spec.signer.certificateChain.certificateChainRef`. As a result, CA chain reference updates may not trigger the PKCS#11 config action to run again, leaving `status.certificate.caRef` stale.

### Issue Context
`ensurePKCS11Config.Handle()` copies `spec.signer.certificateChain.certificateChainRef` into `status.certificate.caRef`, and `ensurePKCS11Deployment()` uses `status.certificate.caRef` to mount the CA cert Secret.

### Fix Focus Areas
- internal/controller/fulcio/actions/ensure_pkcs11_config.go[32-48]
- internal/controller/fulcio/actions/ensure_pkcs11_config.go[80-105]
- internal/controller/fulcio/actions/ensure_pkcs11_config.go[111-123]
- internal/controller/fulcio/actions/deployment.go[352-360]
- internal/controller/fulcio/actions/deployment.go[430-439]

### Suggested change
1. Extend `computePKCS11Hash()` to incorporate `spec.signer.certificateChain.certificateChainRef.name` and `.key` (with delimiters).
2. Optionally add clear separators between all concatenated fields to avoid collisions.
3. Add/adjust tests to cover CA chain reference changes triggering `CanHandle()` and updating `status.certificate.caRef`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~CTlog PKCS11 validation bypass~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

CTlog ensurePKCS11Config sets PKCS11Condition=False on missing/empty secrets but returns without an
action error; once status stops changing, ReturnOnChange can continue to later actions. Because
buildPKCS11Config does not validate non-empty PIN data, an empty PIN Secret value can still be
embedded into the generated CTlog config Secret.

Code

internal/controller/ctlog/actions/ensure_pkcs11_config.go[R71-74]

+	// Validate PinSecretRef
+	if _, err := a.validateSecretRef(ctx, instance, p.PinSecretRef, "spec.signer.pkcs11.pinSecretRef"); err != nil {
+		return a.ReturnOnChange(a.PersistStatus)(ctx, instance)
+	}

Relevance

●●● Strong
Team previously accepted tightening CTlog config validation/error handling patterns; this is similar
reliability hardening.
PR-#1406

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The validation path returns ReturnOnChange(PersistStatus) instead of an error; ReturnOnChange
returns Continue() when no status change occurs, and the PKCS#11 server-config builder passes the
PIN through without checking for empty content.
internal/controller/ctlog/actions/ensure_pkcs11_config.go[71-79]
internal/controller/ctlog/actions/ensure_pkcs11_config.go[140-150]
internal/action/base_action.go[95-115]
internal/controller/ctlog/actions/server_config.go[291-321]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ensurePKCS11Config.Handle()` validates PKCS#11 secrets, but on validation failure it only persists a False condition and returns `ReturnOnChange(PersistStatus)` without an error. If the condition update does not change status (same Reason/Message), `ReturnOnChange` yields `Continue()`, allowing subsequent actions to run.

Separately, `server_config.buildPKCS11Config()` reads the PIN Secret but does not reject an empty PIN (it passes `string(pin)` through), so an empty PIN can reach the generated CTlog config.

### Issue Context
- `validateSecretRef()` explicitly treats `len(data)==0` as an error.
- `ReturnOnChange` continues when the status is unchanged.
- `buildPKCS11Config()` does not enforce non-empty PIN/public key.

### Fix Focus Areas
- internal/controller/ctlog/actions/ensure_pkcs11_config.go[71-79]
- internal/controller/ctlog/actions/ensure_pkcs11_config.go[140-150]
- internal/action/base_action.go[95-115]
- internal/controller/ctlog/actions/server_config.go[291-321]

### Suggested change
1. When `validateSecretRef()` fails, return a requeueing result (e.g., `a.RequeueAfter(...)`) or `a.Error(...)` (retriable) so later actions do not proceed with invalid PKCS#11 inputs.
2. Add defensive checks in `buildPKCS11Config()` (or `CreateCtlogPKCS11Config`) to explicitly reject empty PIN and empty public key data.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~HSM tokens volume overwritten~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Fulcio PKCS#11 deployment overwrites the hsm-tokens volume with EmptyDir whenever it isn’t
EmptyDir/PVC, which clobbers user-provided volume types (e.g., CSI/hostPath/ephemeral). This breaks
the advertised behavior of configuring HSM token persistence via spec.volumes.

Code

internal/controller/fulcio/actions/deployment.go[R441-444]

+		hsmTokensVol := kubernetes.FindVolumeByNameOrCreate(&template.Spec, HSMTokensVolumeName)
+		if !ensure.HasVolume(&template.Spec, HSMTokensVolumeName) || (hsmTokensVol.EmptyDir == nil && hsmTokensVol.PersistentVolumeClaim == nil) {
+			hsmTokensVol.VolumeSource = core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{}}
+		}

Relevance

●●● Strong
Overwriting user-provided volume sources contradicts stated design of using spec.volumes; likely
corrected to preserve user volumes.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
User-defined volumes are applied first via ReconcileUserPodResources, but PKCS#11 deployment then
replaces the hsm-tokens volume source unless it is specifically EmptyDir/PVC, which overwrites
other valid user volume sources.
internal/controller/fulcio/actions/deployment.go[154-157]
internal/controller/fulcio/actions/deployment.go[441-448]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
In Fulcio PKCS#11 mode, `ensurePKCS11Deployment()` rewrites the `hsm-tokens` volume to `EmptyDir` unless the existing `VolumeSource` is `EmptyDir` or `PersistentVolumeClaim`. Because user volumes are reconciled earlier, this can override a user-specified `hsm-tokens` volume source.

### Issue Context
`ensureCommonDeployment()` calls `ensure.ReconcileUserPodResources(...)` with `instance.Spec.Volumes`, and then `ensurePKCS11Deployment()` modifies `hsm-tokens`.

### Fix Focus Areas
- internal/controller/fulcio/actions/deployment.go[154-157]
- internal/controller/fulcio/actions/deployment.go[441-448]

### Suggested change
1. Detect whether `hsm-tokens` was user-defined by checking `instance.Spec.Volumes` (similar to CTlog’s implementation) and only default to `EmptyDir` when it is *not* user-defined.
2. Do not overwrite other user `VolumeSource` types; if some sources are unsupported for your use case, fail fast with a clear error/condition instead of silently replacing them.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. PIN duplicated into config Secret 🐞 Bug ⛨ Security

Description

CTlog PKCS#11 mode copies the HSM PIN into the generated CTlog server-config Secret (protobuf text),
duplicating sensitive credential material beyond the original pinSecretRef Secret. This increases
the blast radius for any reader of the generated config Secret.

Code

internal/controller/ctlog/utils/ctlog_config.go[R204-208]

+		PrivateKey: mustMarshalAny(&keyspb.PKCS11Config{
+			TokenLabel: tokenLabel,
+			Pin:        pin,
+			PublicKey:  string(publicKeyPEM),
+		}),

Relevance

●● Moderate
Sensitive-data duplication concern is valid, but may be required by CTlog PKCS#11 config format; no
clear precedent.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code reads the PIN secret and serializes the PIN value into the generated CTlog configuration,
which is then stored in a Kubernetes Secret.
internal/controller/ctlog/actions/server_config.go[291-321]
internal/controller/ctlog/utils/ctlog_config.go[178-212]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The CTlog PKCS#11 configuration generation embeds the PIN value into the generated CTlog config (stored in a Kubernetes Secret). This duplicates the PIN into an additional Secret.

### Issue Context
- `buildPKCS11Config()` reads the PIN Secret data and passes it into `CreateCtlogPKCS11Config()`.
- `CreateCtlogPKCS11Config()` places it into `keyspb.PKCS11Config.Pin` and serializes it.

### Fix Focus Areas
- internal/controller/ctlog/actions/server_config.go[291-321]
- internal/controller/ctlog/utils/ctlog_config.go[178-212]

### Suggested change
1. If CTlog supports it, refactor the configuration to reference the PIN via a mounted Secret file (or other indirection) rather than embedding the PIN value into the generated config Secret.
2. If embedding is required, document this explicitly and consider tightening access patterns around the generated server-config Secret (labels, RBAC expectations, and rotation story) to minimize exposure.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

codecov-commenter · 2026-08-04T17:50:06Z

Codecov Report

❌ Patch coverage is 79.48164% with 95 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.84%. Comparing base (76105ab) to head (7411315).

Files with missing lines	Patch %	Lines
api/v1/zz_generated.deepcopy.go	0.00%	43 Missing and 2 partials ⚠️
internal/controller/ctlog/actions/server_config.go	80.00%	6 Missing and 6 partials ⚠️
.../controller/fulcio/actions/ensure_pkcs11_config.go	86.58%	10 Missing and 1 partial ⚠️
internal/controller/fulcio/actions/deployment.go	87.95%	5 Missing and 5 partials ⚠️
...l/controller/ctlog/actions/ensure_pkcs11_config.go	93.75%	3 Missing and 3 partials ⚠️
internal/controller/ctlog/actions/deployment.go	77.77%	2 Missing and 2 partials ⚠️
internal/controller/ctlog/utils/ctlog_config.go	92.10%	2 Missing and 1 partial ⚠️
internal/controller/ctlog/ctlog_controller.go	50.00%	1 Missing and 1 partial ⚠️
internal/controller/fulcio/fulcio_controller.go	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@                     Coverage Diff                     @@
##           sachin/feat/signer-auth    #2182      +/-   ##
===========================================================
+ Coverage                    57.25%   57.84%   +0.58%     
===========================================================
  Files                          288      291       +3     
  Lines                        16249    16693     +444     
===========================================================
+ Hits                          9304     9656     +352     
- Misses                        5978     6048      +70     
- Partials                       967      989      +22

Flag	Coverage Δ
e2e	`65.00% <11.16%> (-2.90%)`	⬇️
unit	`38.46% <78.61%> (+1.12%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sampras343 · 2026-08-05T08:36:25Z

Re: PIN duplicated into config Secret (Qodo finding #4)

Acknowledged — this is an upstream architectural requirement, not something we can change.

The ct_server binary reads the HSM PIN from its protobuf config file via keyspb.PKCS11Config.Pin. This is how the trillian PKCS#11 signer is designed — the PIN must be in the config for ct_server to authenticate with the HSM at startup.

The generated config Secret is:

Owned by the CTLog CR (controller reference) — deleted when the CR is deleted
Access-controlled via Kubernetes RBAC (same as any other Secret)
No more exposed than the original pinSecretRef Secret (both are in the same namespace)

This is consistent with how other operators handle HSM PINs in protobuf configs (e.g., sigstore/scaffolding).

Mitigating the blast radius further would require upstream changes to ct_server to support reading the PIN from an environment variable or mounted file instead of the protobuf config — that's out of scope for this PR.

qodo-for-securesign · 2026-08-05T18:38:36Z

Code review by qodo was updated up to the latest commit 3bed21c

Add PKCS#11/HSM-backed signer support, enabling hardware security module integration for both Fulcio CA signing and CTLog STH signing. API changes: - FulcioPKCS11Config: ConfigRef, KeyConfig (ID/Label), CEL validation - CTlogPKCS11Config: PinSecretRef, PublicKeyRef, TokenLabel, ModulePath - Signer type enum extended to "file;pkcs11" on both components Controller changes: - ensure_pkcs11_config actions: validate secrets, content-hash drift detection, PKCS11Condition lifecycle - Deployment actions: signer-type dispatch, HSM volume/mount wiring, mode-switch cleanup with PKCS11Condition removal - server_config: PKCS11 protobuf config generation with null-byte separated content hash - Shared HSM helpers: EnsureHSMResources / CleanupHSMResources E2E test suite: - test/e2e/pkcs11/ with dedicated //go:build pkcs11 tag - SoftHSM prerequisites: key ceremony Jobs, log extraction, secrets - WithPKCS11Signer fixture for SecureSign CR builder - Makefile test-e2e-pkcs11 target + CI job in main.yml Signed-off-by: Sachin Sampras M <sampras343@gmail.com>

qodo-for-securesign Bot reviewed Aug 4, 2026

View reviewed changes

Comment thread internal/controller/fulcio/actions/ensure_pkcs11_config.go

Comment thread internal/controller/fulcio/actions/deployment.go Outdated

Comment thread internal/controller/ctlog/actions/ensure_pkcs11_config.go

sampras343 force-pushed the sachin/feat/pkcs11-signer branch 2 times, most recently from 30aaea8 to 1552696 Compare August 4, 2026 17:32

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from 1552696 to b492fda Compare August 5, 2026 08:35

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from b492fda to 4145314 Compare August 5, 2026 08:46

sampras343 marked this pull request as draft August 5, 2026 10:25

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from 4145314 to 2237d6c Compare August 5, 2026 10:51

github-actions Bot added the needs-rebase label Aug 5, 2026

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from 2237d6c to 91e8ff8 Compare August 5, 2026 12:44

github-actions Bot removed the needs-rebase label Aug 5, 2026

kdacosta0 mentioned this pull request Aug 5, 2026

feat: add KMS signer support for Fulcio (SECURESIGN-4462) #2184

Draft

github-actions Bot added the needs-rebase label Aug 5, 2026

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from 91e8ff8 to 738921c Compare August 5, 2026 15:06

github-actions Bot removed the needs-rebase label Aug 5, 2026

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from 738921c to 12327f0 Compare August 5, 2026 17:58

sampras343 force-pushed the sachin/feat/pkcs11-signer branch 3 times, most recently from 42d36b4 to 3bed21c Compare August 5, 2026 18:25

sampras343 marked this pull request as ready for review August 5, 2026 18:32

qodo-for-securesign Bot reviewed Aug 5, 2026

View reviewed changes

Comment thread api/v1/ctlog_types.go Outdated

sampras343 force-pushed the sachin/feat/pkcs11-signer branch 2 times, most recently from 0c6827d to 4da6d90 Compare August 5, 2026 19:15

sampras343 force-pushed the sachin/feat/pkcs11-signer branch 2 times, most recently from 73a1295 to bcd9852 Compare August 7, 2026 10:04

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from bcd9852 to 2a8b8fb Compare August 10, 2026 08:34

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from 2a8b8fb to f44f7c9 Compare August 11, 2026 06:58

github-actions Bot added the needs-rebase label Aug 11, 2026

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from f44f7c9 to 099474a Compare August 11, 2026 14:48

github-actions Bot removed the needs-rebase label Aug 11, 2026

sampras343 force-pushed the sachin/feat/pkcs11-signer branch 2 times, most recently from b0241d9 to 6d96543 Compare August 11, 2026 15:33

github-actions Bot added the needs-rebase label Aug 12, 2026

sampras343 mentioned this pull request Aug 12, 2026

feat: add InitContainers, Volumes, VolumeMounts for Fulcio and CTLog #2175

Open

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from 6d96543 to 08517a0 Compare August 12, 2026 13:19

github-actions Bot added needs-rebase and removed needs-rebase labels Aug 12, 2026

sampras343 force-pushed the sachin/feat/pkcs11-signer branch from 08517a0 to 7411315 Compare August 12, 2026 14:25

github-actions Bot removed the needs-rebase label Aug 12, 2026

Navigation Menu

Uh oh!

feat: add PKCS#11/HSM signer support for Fulcio and CTLog - #2182

feat: add PKCS#11/HSM signer support for Fulcio and CTLog#2182
sampras343 wants to merge 1 commit into
sachin/feat/signer-authfrom
sachin/feat/pkcs11-signer

sampras343 commented Aug 4, 2026

Uh oh!

qodo-for-securesign Bot commented Aug 4, 2026 •

edited

Loading

Uh oh!

qodo-for-securesign Bot commented Aug 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Aug 4, 2026 •

edited

Loading

Uh oh!

sampras343 commented Aug 5, 2026

Uh oh!

Uh oh!

qodo-for-securesign Bot commented Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sampras343 commented Aug 4, 2026

Summary

API Changes

Controller Changes

Design Decisions

Review Comments Addressed

Test plan

Uh oh!

qodo-for-securesign Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary by Qodo

Uh oh!

qodo-for-securesign Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sampras343 commented Aug 5, 2026

Re: PIN duplicated into config Secret (Qodo finding #4)

Uh oh!

Uh oh!

qodo-for-securesign Bot commented Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qodo-for-securesign Bot commented Aug 4, 2026 •

edited

Loading

qodo-for-securesign Bot commented Aug 4, 2026 •

edited

Loading

codecov-commenter commented Aug 4, 2026 •

edited

Loading