Assume cosign.key as key field in signing secret

[simonbaird]

User description

I want to use this and have it dwim:

--vsa-signing-key k8s://conforma/vsa-signing-key

Also added a commit to read a password from the signing key secret and use it.

Ref: https://issues.redhat.com/browse/EC-1589

---

PR Type

Enhancement

---

Description

• Default to cosign.key field when no key specified in Kubernetes secret reference

• Allows simplified secret reference syntax without explicit key field

• Added comprehensive test coverage for default key behavior

• Supports both single-key and multi-key secrets with cosign.key fallback

---

Diagram Walkthrough

flowchart LR
  A["k8s://namespace/secret"] -->|"Auto-append cosign.key"| B["k8s://namespace/secret/cosign.key"]
  B -->|"Resolve from K8s"| C["Private key bytes"]
  D["k8s://namespace/secret/key-field"] -->|"Use explicit field"| C

Loading

File Walkthrough

	Relevant files
Enhancement	private_key.go Auto-append cosign.key to secret references                            internal/utils/private_key.go Added fmt and strings imports for string manipulation Implemented logic to auto-append /cosign.key to key references without explicit key field Detects Kubernetes secret format and appends default key field when only namespace and secret name provided Maintains backward compatibility with explicit key field specifications +12/-1
Tests	private_key_test.go Add tests for default cosign.key behavior                                internal/utils/private_key_test.go Updated test case description for multi-key secret to reflect new default behavior Changed expected error message from "contains multiple keys" to "key field not found" Added test case for secret with cosign.key field as default Added test case for mixed-key secret where cosign.key exists among multiple keys Extended mock secret setup to include new test scenarios with cosign.key field Added assertions to verify correct key content retrieval for new test cases +44/-2

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Edge case parsing: The new k8s:// parsing defaults only when len(parts)==2 but does not explicitly validate non-empty namespace/secret or handle trailing slashes/empty segments, which may lead to unexpected behavior for malformed inputs. Referred Code // If the key-field is not specified assume it is "cosign.key" adjustedKeyRef := keyRef if strings.HasPrefix(keyRef, "k8s://") { parts := strings.Split(strings.TrimPrefix(keyRef, "k8s://"), "/") if len(parts) == 2 { adjustedKeyRef = fmt.Sprintf("%s/cosign.key", keyRef) } } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Input validation gaps: keyRef is an external input and the new logic does not explicitly reject invalid k8s:// references with empty namespace/secret segments before passing them to KeyFromKeyRef, which may defer validation and produce less controlled behavior. Referred Code // If the key-field is not specified assume it is "cosign.key" adjustedKeyRef := keyRef if strings.HasPrefix(keyRef, "k8s://") { parts := strings.Split(strings.TrimPrefix(keyRef, "k8s://"), "/") if len(parts) == 2 { adjustedKeyRef = fmt.Sprintf("%s/cosign.key", keyRef) } } return KeyFromKeyRef(ctx, adjustedKeyRef, fs) Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add validation for malformed secret references Add validation to ensure the namespace and secret name parts of a Kubernetes secret reference are non-empty before appending the default key. internal/utils/private_key.go [36-41] if strings.HasPrefix(keyRef, "k8s://") { parts := strings.Split(strings.TrimPrefix(keyRef, "k8s://"), "/") - if len(parts) == 2 { + // A valid reference without a key field has two non-empty parts: namespace and secret name. + if len(parts) == 2 && parts[0] != "" && parts[1] != "" { adjustedKeyRef = fmt.Sprintf("%s/cosign.key", keyRef) } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies an edge case where malformed keyRef inputs could cause incorrect behavior, and the proposed fix makes the new logic more robust by validating the parsed namespace and secret name.	Medium
Update

[codecov]

Codecov Report

❌ Patch coverage is 65.00000% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/utils/private_key.go	62.50%	6 Missing ⚠️
internal/validate/vsa/attest.go	75.00%	1 Missing ⚠️

Flag	Coverage Δ
acceptance	55.78% <50.00%> (-0.02%)	⬇️
generative	19.01% <45.00%> (+0.04%)	⬆️
integration	28.50% <45.00%> (+0.02%)	⬆️
unit	67.69% <45.00%> (-0.04%)	⬇️

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

Files with missing lines	Coverage Δ
internal/validate/vsa/attest.go	88.52% <75.00%> (-1.14%)	⬇️
internal/utils/private_key.go	64.70% <62.50%> (-35.30%)	⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[simonbaird]

Acceptance tests pass for me locally. I just rebased, let's see how it goes.

[joejstuart]

Does this logic need to be here? It looks similar to what's going on in utils.PasswordFromKeyRef.

[simonbaird]

Good catch, thanks!

[Acepresso]

LGTM

[simonbaird]

Let's merge.