TT-17702: Add Lister and Setter logic to Vault and Consul

[vladzabolotnyi]

Description

Ticket: https://tyktech.atlassian.net/browse/TT-17702

Related Issue

Motivation and Context

Test Coverage For This Change

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)
• Documentation updates or improvements.

Checklist

• I have reviewed the guidelines for contributing to this repository.
• Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If PRing from your fork, don't come from your master!
• Make sure you are making a pull request against our master branch (left side). Also, it would be best if you started your change off our latest master.
• My change requires a change to the documentation.
  • I have manually updated the README(s)/documentation accordingly.
  • If you've changed APIs, describe what needs to be updated in the documentation.
• I have updated the documentation accordingly.
• Modules and vendor dependencies have been updated; run go mod tidy && go mod vendor
• When updating library version must provide reason/explanation for this update.
• I have added tests to cover my changes.
• All new and existing tests passed.
• Check your code additions will not fail linting checks:
  • gofmt -s -w .
  • go vet ./...

[github-actions]

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

---

I have read the CLA Document and I hereby sign the CLA

---

Vlad Zabolotnyi seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You can retrigger this bot by commenting recheck in this Pull Request}

[github-actions]

🚨 Jira Linter Failed

Commit: 48d7368
Failed at: 2026-07-17 14:44:41 UTC

The Jira linter failed to validate your PR. Please check the error details below:

🔍 Click to view error details

configuration error: Jira user email is required for API authentication

Next Steps

• Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
• Verify your PR title matches the branch's Jira ticket ID
• Check that the Jira ticket exists and is accessible

---

This comment will be automatically deleted once the linter passes.

[probelabs]

This PR introduces Setter and Lister capabilities to the kv provider interface, enabling write and list operations for secrets, with implementations for Vault and Consul.

Files Changed Analysis

• kv/provider.go: Defines the new Setter interface and modifies the Lister interface to return a map[string]string (key-value pairs) instead of just keys ([]string). It also adds an AsSetter helper function.
• kv/providers/consul/consul.go & vault/vault.go: Implement the Setter and Lister (for Consul) interfaces. The Vault implementation correctly handles differences between KV v1 and v2 APIs. The Consul implementation adds a safeguard against listing the entire KV store with an empty prefix.
• kv/providers/consul/consul_test.go & vault/vault_test.go: Add extensive unit tests for the new Set and List functionalities, covering success cases, error handling, context cancellation, and authentication.
• kv/resolver/reference.go (New File): Introduces a new public API ParseReference to parse kv:// reference strings into their constituent parts (Store, Path, Field) without resolving the value. This is essential for write-back operations that need to target a specific store and key.
• kv/internal/resolve/refs.go & kv/resolver/resolver_test.go: Expose internal parsing logic and add comprehensive tests for the new ParseReference function.

The changes are heavily weighted towards adding new code and tests, with minimal deletions, reflecting the addition of new features.

Architecture & Impact Assessment

• What this PR accomplishes: It extends the key-value store abstraction (kv.Provider) to support writing and listing secrets, not just reading them. This is a foundational change that enables higher-level features like automated secret rotation or dynamic configuration management.

• Key technical changes introduced:

  • Setter Interface: A new Set(ctx, key, value) method allows writing values to a provider.
  • Lister Interface Change: The List method now returns map[string]string, providing both keys and values for enumerated secrets.
  • Reference Parsing: A new resolver.ParseReference function is introduced, allowing systems to deconstruct a kv:// URI without fetching its value. This is critical for routing write operations to the correct backend.

• Affected system components: The primary impact is on the kv package and its consumers. Any service that uses the kv.Resolver or kv.Provider can now potentially write or list secrets if the underlying provider supports it. This change enables new workflows for managing secrets and configuration dynamically.

Component Interaction

graph TD
    subgraph ID1 [Application Layer]
        A[Secret Management Service] --|kv://vault/secret/app#key, newValue|--> B{Write-Back Logic}
    end

    subgraph ID2 [KV Abstraction Layer]
        B -- "1. ParseReference" --> C[resolver.ParseReference]
        C -- "Returns Store: vault, Path: secret/app" --> B
        B -- "2. Get Provider" --> D[Provider Registry]
        D -- "vault" --> E[Vault Provider]
        B -- "3. AsSetter" --> E
        E -- "Setter interface" --> B
        B -- "4. Set("secret/app", "{\"key\":\"newValue\"}")" --> E
    end

    subgraph ID3 [Provider Implementation]
        E -- "5. WriteWithContext" --> F[HashiCorp Vault API]
    end

Loading

Scope Discovery & Context Expansion

The introduction of Setter and ParseReference strongly implies the development of a secret rotation or write-back mechanism. A higher-level service can now:

1. Receive a request to update a secret identified by a kv:// string.
2. Use resolver.ParseReference to identify the target store (e.g., vault) and path.
3. Retrieve the corresponding kv.Provider from a registry.
4. Use kv.AsSetter to get the write interface.
5. Call Set to push the new value to the backend (Vault or Consul).

The change to the Lister interface enhances operational and discovery tools, allowing them to inspect both the keys and values under a given prefix, which is useful for debugging and auditing.

Metadata

• Review Effort: 4 / 5
• Primary Label: feature

---

Powered by Visor from Probelabs

Last updated: 2026-07-17T14:48:01.196Z | Triggered by: pr_opened | Commit: 48d7368

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

Security Issues (3)

Severity	Location	Issue
🟠 Error	kv/providers/consul/consul.go:184-198	The `Set` function writes to a given `key` in Consul without enforcing any path restrictions or prefix validation. If the `key` is controllable by an attacker, this could allow writing to arbitrary paths in the Consul KV store, potentially leading to configuration overwrite, privilege escalation, or denial of service. The scope of the damage is limited only by the permissions of the Consul token in use. 💡 Suggestion Introduce a `write_prefix` in the Consul provider configuration. Before writing, validate that the provided `key` is within this configured prefix. If the prefix is not configured, either disallow writes or log a prominent security warning.
🟡 Warning	kv/providers/vault/vault.go:223-247	The `Set` function for the Vault provider allows writes to any path permitted by the Vault token if `mountPath` is not configured. While it correctly restricts writes to the `mountPath` when configured, not having it configured creates an insecure default where writes are not properly namespaced. This could lead to accidental or malicious writes to unintended secret paths. 💡 Suggestion To promote secure-by-default configuration, consider making `mountPath` mandatory for write operations. The `Set` function could return an error if it's called without a `mountPath` configured. At a minimum, log a security warning when this occurs and update documentation to strongly recommend its use.
🟡 Warning	kv/providers/consul/consul.go:205-224	The `List` function checks for an empty prefix, which is a good mitigation against listing the entire KV store. However, it does not enforce any other restrictions on the prefix. A broad prefix (e.g., '/') could still be provided, leading to high resource consumption on the Consul server and potentially exposing more data than intended by the caller. 💡 Suggestion Consider introducing a configurable `allowed_list_prefixes` or a single `root_prefix` in the provider configuration to ensure that `List` operations are constrained to intended namespaces, preventing overly broad queries.

Architecture Issues (2)

Severity	Location	Issue
🟡 Warning	kv/provider.go:109-113	The `Lister` interface has been changed from returning `[]string` to `map[string]string`. This is a breaking change for any external consumers of this interface. While it's a functional improvement for performance by preventing N+1 lookups, it violates semantic versioning if this is a library intended for external use without a major version bump. 💡 Suggestion If this is a public library, this change should be part of a major version release. If it's for internal consumption and all consumers are updated within this PR or related work, it's acceptable. Consider adding a comment to the interface definition noting the change and the reason for it.
🟡 Warning	kv/providers/vault/vault.go:232-234	The check `if fields == nil` is redundant. `json.Unmarshal` into a `map[string]any` will result in a `nil` map if the input is the JSON literal `null`. However, the preceding error check `if err != nil` would not catch this, as unmarshaling `null` is a valid operation. The subsequent check `if fields == nil` correctly handles this case. The issue is that the error message for `fields == nil` is less specific than the one for the unmarshal error. A non-object JSON value like `"a string"` or `123` will fail the unmarshal, but `null` will pass and then be caught by `fields == nil`. The error message for the `nil` case could be more specific. 💡 Suggestion The check itself is correct, but the error message could be improved to be consistent with the unmarshal error message. For example: `return errors.New("vault: value must be a JSON object, but was null")`. This provides more context to the user about why their input was rejected.

Performance Issues (1)

Severity	Location	Issue
🟡 Warning	kv/providers/consul/consul.go:195-216	The `List` function fetches all key-value pairs under a given prefix and loads them entirely into memory. This is dictated by the `Lister` interface change in `kv/provider.go:113` which now returns `map[string]string`. If a prefix matches a large number of keys, this implementation can lead to excessive memory allocation and high latency, potentially causing Out-Of-Memory errors or service degradation. 💡 Suggestion For use cases that might return a large number of items, consider redesigning the `Lister` interface to support streaming or pagination, for example by returning a channel or an iterator. If changing the interface is not feasible, ensure that this function is only called with prefixes known to match a small number of keys and document this limitation clearly in the `Lister` interface comments.

---

Powered by Visor from Probelabs

Last updated: 2026-07-17T14:46:58.108Z | Triggered by: pr_opened | Commit: 48d7368

💡 TIP: You can chat with Visor using /visor ask <your question>

[sonarqubecloud]

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud