refactor: add validation to vault key concatenation

[nmgaston]

This pull request introduces robust validation and normalization for path-based secret keys in the Vault secrets client, ensuring that secret operations are safe from path traversal and namespace escape issues. The changes also refactor the code to consistently use these validation and path-building utilities across all secret management methods.

Key improvements include:

Path Validation and Security

• Added the validatePathKey function to strictly validate path-based secret keys, preventing empty keys, unsupported characters, absolute paths, path traversal, and non-normalized paths. This helps prevent security vulnerabilities related to improper key usage.
• Introduced the buildScopedSecretPath function to safely join and normalize secret paths under the base namespace, ensuring secrets cannot escape their intended scope.

Refactoring Secret Operations

• Updated all secret management methods (GetKeyValue, SetKeyValue, DeleteKeyValue, GetObject, and SetObject) to use the new path validation and building utilities, replacing previous ad-hoc path concatenation logic. This centralizes and standardizes path handling throughout the codebase.

Error Handling Improvements

• Added a new sentinel error ErrInvalidPathKey and improved error messages for invalid key scenarios, making issues easier to diagnose and debug.

Code Organization

• Refactored the implementation by splitting path-based and field-based key-value logic into separate helper methods (setPathBasedKeyValue, setFieldKeyValue), improving readability and maintainability.

These changes significantly increase the safety and reliability of secret storage operations by enforcing strict key validation and consistent path handling.

[codecov]

Codecov Report

❌ Patch coverage is 97.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.96%. Comparing base (cf8a2af) to head (3424eac).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/secrets/vault/secrets.go	97.50%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1189      +/-   ##
==========================================
+ Coverage   49.57%   50.96%   +1.39%     
==========================================
  Files         146      146              
  Lines       13455    13603     +148     
==========================================
+ Hits         6670     6933     +263     
+ Misses       6207     6076     -131     
- Partials      578      594      +16     

☔ 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.

[Copilot]

Pull request overview

This PR hardens Vault KV v2 secret-path handling by validating and safely joining path-based keys under the configured Vault namespace, and expands tests to cover the new validation and common Vault interactions.

Changes:

• Added path-key validation and a scoped path-join helper to prevent path traversal/namespace escape when building Vault secret paths.
• Refactored Vault read/write/delete logic to use the scoped path builder for both path-based and field-based secret storage.
• Replaced prior mocking with HTTP-level tests using httptest to validate request paths and payload shapes.

Reviewed changes

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

File	Description
pkg/secrets/vault/secrets.go	Adds key/path validation + scoped path builder; updates Vault secret operations to use validated paths.
pkg/secrets/vault/secrets_test.go	Adds extensive tests for key validation, scoped path building, and Vault API interactions via httptest.

Suppressed comments (1)

pkg/secrets/vault/secrets.go:297

• SetObject's docstring says the key must contain "/" to specify a path-based secret, but the implementation accepts slash-free keys and will write to {basePath}/{key}. Consider enforcing the documented contract to prevent accidental writes to unexpected locations.

// SetObject stores a map of string values at a path-based secret.
// The key must contain "/" to specify the path: {basePath}/{key}.
func (c *Client) SetObject(key string, data map[string]string) error {
	secretPath, err := buildScopedSecretPath(c.path, key)
	if err != nil {

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.