fix: skip plaintext fallback lookup for secret-based virtual key values

[BearTS]

Summary

Prevents plaintext database fallback lookups when a virtual key value appears to be a secret reference (e.g., a vault or environment variable reference). Without this guard, a value like vault.my-secret or env.MY_VAR would be passed as a raw string into a WHERE value = ? query, which could never match a real row but still leaks information about the lookup path and wastes a database round-trip.

Changes

• In GetVirtualKeyByValue, when a hash-based lookup returns no record, the value is parsed as a SecretVar. If it originates from a secret reference, the function returns ErrNotFound immediately instead of falling back to a plaintext query.
• In GetVirtualKeyQuotaByValue, the same early-exit behavior is applied by checking for vault. and env. prefixes directly, consistent with the rationale in GetVirtualKeyByValue.

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

go test ./framework/configstore/...

• Submit a request using a virtual key value formatted as vault.<secret-name> or env.<VAR_NAME>.
• Confirm the response returns a not-found error without triggering a secondary plaintext database query.
• Confirm that normal virtual key values (non-secret-reference strings) still fall back to plaintext lookup as expected.

Breaking changes

• Yes
• No

Security considerations

Secret references (vault/env-style values) should never be stored or queried as plaintext in the database. Allowing them to reach a WHERE value = ? clause could expose the reference string in query logs or slow query logs. This change ensures such values are short-circuited before any plaintext lookup occurs.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c47109e0-4559-4249-8b64-b520e0093126

📥 Commits

Reviewing files that changed from the base of the PR and between 3c3e488 and 87a6943.

📒 Files selected for processing (2)

• core/schemas/secretvar.go
• framework/configstore/rdb.go

🚧 Files skipped from review as they are similar to previous changes (1)

• framework/configstore/rdb.go

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added support for recognizing secret-style values without resolving them, improving how secret references are handled across the app.

• Bug Fixes

  • Secret references no longer fall back to plain-text matching when a lookup by hashed value fails, reducing incorrect matches and unexpected results.
  • Improved parsing of secret inputs, including compatibility with older formats and JSON-encoded values.

Walkthrough

Secret reference parsing now happens without side effects, and virtual key lookups in the config store return ErrNotFound immediately for secret-referenced values instead of falling back to plaintext matching.

Changes

Secret reference lookup fix

Layer / File(s)	Summary
Secret reference parsing core/schemas/secretvar.go	parseSecretRef classifies plain strings, env.*/vault.* prefixes, and legacy JSON secret forms, IsSecretRef exposes side-effect-free detection, and NewSecretVar resolves vault/env values only after parsing.
Secret-reference short-circuit in virtual key lookups framework/configstore/rdb.go	GetVirtualKeyByValue and GetVirtualKeyQuotaByValue return ErrNotFound after a missed hash lookup when schemas.IsSecretRef(value) is true, skipping the plaintext fallback query.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: danpiths, akshaydeo

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately reflects the main change to virtual key lookup behavior.
Description check	✅ Passed	The description matches the template well, covering summary, changes, testing, security, and breaking changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 07-05-fix_check_for_if_vk_value_is_a_secret

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

_{Comment @coderabbitai help to get the list of available commands.}

[BearTS]

• fix: skip plaintext fallback lookup for secret-based virtual key values #4927 👈 (View in Graphite)
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Confidence Score: 5/5

Safe to merge — the guard is correctly placed after a hash miss and only skips the plaintext fallback for vault/env reference strings, which should never match real virtual-key rows anyway.

The parseSecretRef refactoring is behaviorally equivalent to the old NewSecretVar across all input forms, and the IsSecretRef check in both lookup functions is logically sound. No correctness or security regressions were found.

Both changed files look correct; adding test cases to secretvar_test.go and rdb_test.go for the new code paths would be valuable before the migration window closes and the plaintext fallback is removed entirely.

Important Files Changed

Filename	Overview
core/schemas/secretvar.go	Refactors NewSecretVar by extracting parseSecretRef (classify-only, no side effects) and adds public IsSecretRef. The refactoring is behaviorally equivalent to the old code across all three secret formats (plain prefix, JSON-encoded, and backward-compat env_var/from_env). No test coverage added for the new IsSecretRef API or the parseSecretRef edge cases.
framework/configstore/rdb.go	Adds IsSecretRef guard to GetVirtualKeyByValue and GetVirtualKeyQuotaByValue after a hash miss, short-circuiting the plaintext fallback query for vault./env. values. Change is correctly placed, avoids the vault HTTP amplification risk flagged in the previous review thread, and does not affect normal plaintext-key lookups.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Incoming request with virtual key value] --> B[Compute SHA-256 hash]
    B --> C{Hash lookup\nvalue_hash = ?}
    C -- Found --> D[Return virtual key]
    C -- ErrRecordNotFound --> E{IsSecretRef?\nvault.* or env.*}
    E -- Yes --> F[Return ErrNotFound\nno plaintext query]
    E -- No --> G[Plaintext fallback\nvalue = ?]
    G -- Found --> D
    G -- ErrRecordNotFound --> H[Return ErrNotFound]
    G -- Other error --> I[Return error]
    C -- Other error --> I

    style F fill:#f9a,stroke:#c66
    style E fill:#adf,stroke:#06a

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Incoming request with virtual key value] --> B[Compute SHA-256 hash]
    B --> C{Hash lookup\nvalue_hash = ?}
    C -- Found --> D[Return virtual key]
    C -- ErrRecordNotFound --> E{IsSecretRef?\nvault.* or env.*}
    E -- Yes --> F[Return ErrNotFound\nno plaintext query]
    E -- No --> G[Plaintext fallback\nvalue = ?]
    G -- Found --> D
    G -- ErrRecordNotFound --> H[Return ErrNotFound]
    G -- Other error --> I[Return error]
    C -- Other error --> I

    style F fill:#f9a,stroke:#c66
    style E fill:#adf,stroke:#06a

Loading

_{Reviews (2): Last reviewed commit: "fix: check for if vk value is a secret" | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

framework/configstore/rdb.go (2)

3300-3311: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add table-driven test coverage for the new short-circuit branch.

No test changes accompany this behavior change. This is a security/auth-adjacent lookup path (virtual key resolution), and the new early-return branch (secret-referenced value → immediate ErrNotFound, skipping plaintext fallback) is exactly the kind of behavior change that should have dedicated coverage — e.g. cases for: secret-formatted value with no hash match, plain legacy value with no hash match (still falls back), and a value matching both hash and legacy row.

As per path instructions, framework/** changes should include "tests that cover edge cases and failure paths," and per coding guidelines, **/*.go changes should have "table-driven coverage for behavior changes."

Also applies to: 3330-3341

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@framework/configstore/rdb.go` around lines 3300 - 3311, Add table-driven
tests for the virtual key lookup behavior in rdb.go around the virtual key
resolution path, especially the new early-return branch in the value_hash lookup
flow. Cover at least: a secret-formatted value with no hash match returning
ErrNotFound without falling back to plaintext, a plain legacy value with no hash
match still falling back to the value lookup, and a value that can match both
hashed and legacy rows preferring the hash match. Place the coverage near the
existing lookup tests for the relevant resolver/query logic so the behavior of
the short-circuit branch is locked in.

Sources: Coding guidelines, Path instructions

---

3302-3302: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Local variable s shadows the method receiver s *RDBConfigStore.

Both GetVirtualKeyByValue and GetVirtualKeyQuotaByValue use s as the receiver name; the new if s := schemas.NewSecretVar(value); ... redeclares s inside that scope. Harmless today since only return executes in that block, but it's a common source of confusion/bugs if the branch is extended later.

♻️ Suggested rename to avoid shadowing

-			if s := schemas.NewSecretVar(value); s != nil && s.IsFromSecret() {
+			if sv := schemas.NewSecretVar(value); sv != nil && sv.IsFromSecret() {
 				return nil, ErrNotFound
 			}

Also applies to: 3332-3332

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@framework/configstore/rdb.go` at line 3302, The new local variable named s in
GetVirtualKeyByValue and GetVirtualKeyQuotaByValue shadows the RDBConfigStore
receiver s, which makes the block harder to read and can cause confusion if the
branch grows later. Rename the local result from schemas.NewSecretVar(value) to
a non-conflicting identifier in both methods and keep the existing secret
check/return behavior unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@framework/configstore/rdb.go`:
- Around line 3302-3304: Add table-driven tests for GetVirtualKeyByValue and
GetVirtualKeyQuotaByValue to cover secret-backed inputs and the plaintext
fallback path. Verify that env.* and vault.* values are treated as secret-backed
via schemas.NewSecretVar and return ErrNotFound without falling back to
plaintext matching, while a normal plaintext virtual key row still resolves
successfully. Use the existing lookup helpers and virtual key/quota lookup
functions to keep the test anchored to the current behavior.

---

Nitpick comments:
In `@framework/configstore/rdb.go`:
- Around line 3300-3311: Add table-driven tests for the virtual key lookup
behavior in rdb.go around the virtual key resolution path, especially the new
early-return branch in the value_hash lookup flow. Cover at least: a
secret-formatted value with no hash match returning ErrNotFound without falling
back to plaintext, a plain legacy value with no hash match still falling back to
the value lookup, and a value that can match both hashed and legacy rows
preferring the hash match. Place the coverage near the existing lookup tests for
the relevant resolver/query logic so the behavior of the short-circuit branch is
locked in.
- Line 3302: The new local variable named s in GetVirtualKeyByValue and
GetVirtualKeyQuotaByValue shadows the RDBConfigStore receiver s, which makes the
block harder to read and can cause confusion if the branch grows later. Rename
the local result from schemas.NewSecretVar(value) to a non-conflicting
identifier in both methods and keep the existing secret check/return behavior
unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ff2bd17f-0efc-4ca1-94e1-983f41640297

📥 Commits

Reviewing files that changed from the base of the PR and between d87f109 and 3c3e488.

📒 Files selected for processing (1)

• framework/configstore/rdb.go