Skip to content

Conversation

@akshaydeo
Copy link
Contributor

@akshaydeo akshaydeo commented Jan 5, 2026

Summary

Added environment variable support for sensitive configuration fields in vector stores and virtual keys, allowing users to securely store credentials outside of configuration files.

Fixes

Closes #1230

Changes

  • Added environment variable resolution for sensitive fields in vector stores:
    • Weaviate: API key
    • Redis: Username and password
    • Qdrant: API key
  • Added environment variable support for virtual key values in governance configuration
  • Removed verbose debug logging in governance config loading

Type of change

  • Feature
  • Security

Affected areas

  • Core (Go)
  • Transports (HTTP)

How to test

  1. Configure vector stores using environment variables:
{
  "type": "weaviate",
  "config": {
    "api_key": "env.WEAVIATE_API_KEY"
  }
}
  1. Set the corresponding environment variables:
export WEAVIATE_API_KEY="your-secret-key"
export REDIS_PASSWORD="your-redis-password"
export QDRANT_API_KEY="your-qdrant-key"
export VIRTUAL_KEY_VALUE="your-virtual-key"
  1. Run the application and verify the credentials are properly resolved

Security considerations

This change improves security by allowing sensitive credentials to be stored in environment variables rather than in configuration files, reducing the risk of credential exposure in source control or configuration management systems.

Checklist

  • I added/updated tests where appropriate
  • I verified builds succeed (Go and UI)

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 5, 2026

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added environment-variable resolution for sensitive configuration fields in vector stores and governance virtual keys, allowing credentials and secrets to be supplied via env vars.
    • Resolution failures are now surfaced with contextual messages to aid troubleshooting.
  • Chores

    • Reduced verbose debug logging for cleaner console output.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Added environment-variable resolution for sensitive config fields: vectorstore configs (Weaviate, Redis, Qdrant) and governance virtual keys; removed verbose debug logging in governance config loading.

Changes

Cohort / File(s) Summary
Vectorstore Configuration
framework/vectorstore/store.go
During JSON unmarshalling, resolve fields prefixed with env. via envutils.ProcessEnvValue for Weaviate APIKey, Redis Username/Password, and Qdrant APIKey. Added imports: strings, github.com/maximhq/bifrost/framework/envutils. Errors from resolution are surfaced with context.
Governance Configuration
transports/bifrost-http/lib/config.go
Removed verbose debug logging in loadGovernanceConfigFromFile. In mergeMCPConfig, resolve VirtualKeys values prefixed with env. via ProcessEnvValue, set VK.Value, and record env usage in config.EnvKeys with ConfigPath governance.virtual_keys[ID].value.

Sequence Diagram

sequenceDiagram
    participant Parser as Config Parser
    participant EnvUtil as envutils.ProcessEnvValue
    participant OS as Environment
    participant Store as Config Struct / Store

    Parser->>Parser: Unmarshal JSON (vectorstore / governance)
    Note over Parser: Detect fields with "env." prefix
    alt Field is "env." reference
        Parser->>EnvUtil: ProcessEnvValue("env.NAME" or similar)
        EnvUtil->>OS: Read environment variable
        OS-->>EnvUtil: Return resolved value / error
        EnvUtil-->>Parser: Resolved value or error
        Parser->>Store: Assign resolved value into config field
    else Literal value (no "env." prefix)
        Parser->>Store: Assign value as-is
    end
    Store-->>Parser: Finalized config (errors bubbled with context if any)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I sniffed the code by moonlit logs,

found secrets tucked in env-bound clogs,
I hopped and fetched the hidden key,
replaced the strings, set config free,
nibble, hop — secure and snappy! 🎩

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'adds env resolution for loadingConfigFromFile' clearly describes the main feature being added—environment variable resolution for loading configuration files—and aligns well with the primary changes in the changeset.
Description check ✅ Passed The PR description includes all required sections: Summary, Changes, Type of change, Affected areas, How to test, and Security considerations. It comprehensively documents the feature and is well-structured according to the template.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 01-05-adds_env_resolution_for_loadingconfigfromfile

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor Author

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

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026

🧪 Test Suite Available

This PR can be tested by a repository admin.

Run tests for PR #1238

@akshaydeo akshaydeo marked this pull request as ready for review January 5, 2026 03:50
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 081fa43 and d7cdaa5.

📒 Files selected for processing (2)
  • framework/vectorstore/store.go
  • transports/bifrost-http/lib/config.go
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)

Files:

  • framework/vectorstore/store.go
  • transports/bifrost-http/lib/config.go
🧠 Learnings (4)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.

Applied to files:

  • framework/vectorstore/store.go
  • transports/bifrost-http/lib/config.go
📚 Learning: 2025-12-29T11:54:55.836Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1153
File: framework/configstore/rdb.go:2221-2246
Timestamp: 2025-12-29T11:54:55.836Z
Learning: In Go reviews, do not flag range-over-int patterns like for i := range n as compile-time errors, assuming Go 1.22+ semantics. Only flag actual range-capable values (slices, arrays, maps, channels, strings) and other compile-time issues. This applies to all Go files across the repository.

Applied to files:

  • framework/vectorstore/store.go
  • transports/bifrost-http/lib/config.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.

Applied to files:

  • transports/bifrost-http/lib/config.go
📚 Learning: 2025-12-29T09:14:16.633Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 888
File: transports/bifrost-http/handlers/middlewares.go:246-256
Timestamp: 2025-12-29T09:14:16.633Z
Learning: In the bifrost HTTP transport, fasthttp.RequestCtx is the primary context carrier and should be passed directly to functions that expect a context.Context. Do not convert to context.Context unless explicitly required. Ensure tracer implementations and related components are designed to accept fasthttp.RequestCtx directly, and document this architectural decision for maintainers.

Applied to files:

  • transports/bifrost-http/lib/config.go
🧬 Code graph analysis (1)
framework/vectorstore/store.go (3)
framework/envutils/utils.go (1)
  • ProcessEnvValue (10-23)
framework/vectorstore/redis.go (1)
  • RedisConfig (22-40)
framework/vectorstore/qdrant.go (1)
  • QdrantConfig (14-19)
🔇 Additional comments (5)
transports/bifrost-http/lib/config.go (1)

948-948: LGTM! Cleanup of verbose debug logging.

The removal of verbose debug logging in governance config loading improves code clarity and reduces noise in production environments.

framework/vectorstore/store.go (4)

8-8: LGTM! Required imports for environment variable processing.

The strings and envutils imports are necessary for the environment variable resolution functionality added below.

Also applies to: 11-11


128-135: Excellent implementation of environment variable resolution.

The Weaviate API key processing:

  • Uses the centralized envutils.ProcessEnvValue helper for consistency
  • Includes proper error handling with contextual error messages
  • Only processes when the field has the env. prefix
  • Enhances security by enabling environment-based credential injection

143-156: Well-implemented environment variable processing for Redis credentials.

The implementation correctly handles both Username and Password fields:

  • Processes each field independently with the same robust pattern
  • Provides clear, field-specific error messages
  • Enables secure credential management for Redis authentication

164-170: Consistent implementation completing environment variable support.

The Qdrant API key processing follows the same robust pattern as Weaviate and Redis, ensuring consistency across all supported vector store types. This completes the security enhancement by enabling environment-based credential injection for all vector stores.

@akshaydeo akshaydeo force-pushed the 01-05-adds_env_resolution_for_loadingconfigfromfile branch from d7cdaa5 to c00b81a Compare January 5, 2026 04:53
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d7cdaa5 and c00b81a.

📒 Files selected for processing (2)
  • framework/vectorstore/store.go
  • transports/bifrost-http/lib/config.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • transports/bifrost-http/lib/config.go
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)

Files:

  • framework/vectorstore/store.go
🧠 Learnings (2)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.

Applied to files:

  • framework/vectorstore/store.go
📚 Learning: 2025-12-29T11:54:55.836Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1153
File: framework/configstore/rdb.go:2221-2246
Timestamp: 2025-12-29T11:54:55.836Z
Learning: In Go reviews, do not flag range-over-int patterns like for i := range n as compile-time errors, assuming Go 1.22+ semantics. Only flag actual range-capable values (slices, arrays, maps, channels, strings) and other compile-time issues. This applies to all Go files across the repository.

Applied to files:

  • framework/vectorstore/store.go
🧬 Code graph analysis (1)
framework/vectorstore/store.go (3)
framework/envutils/utils.go (1)
  • ProcessEnvValue (10-23)
framework/vectorstore/redis.go (1)
  • RedisConfig (22-40)
framework/vectorstore/qdrant.go (1)
  • QdrantConfig (14-19)
🔇 Additional comments (3)
framework/vectorstore/store.go (3)

8-8: LGTM! Necessary imports added.

The strings and envutils imports are correctly added to support environment variable resolution for sensitive configuration fields.

Also applies to: 11-11


128-135: Security improvement for Weaviate API key handling.

The environment variable resolution for APIKey is correctly implemented with appropriate error handling. This allows sensitive credentials to be stored in environment variables instead of configuration files.


142-156: Security improvement for Redis credentials.

Environment variable resolution for both Username and Password is correctly implemented. Each field is processed independently with appropriate error handling, allowing Redis credentials to be securely managed via environment variables.

Copy link
Contributor Author

akshaydeo commented Jan 5, 2026

Merge activity

  • Jan 5, 5:50 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jan 5, 5:51 AM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo akshaydeo merged commit 2c0cbf1 into main Jan 5, 2026
9 checks passed
@akshaydeo akshaydeo deleted the 01-05-adds_env_resolution_for_loadingconfigfromfile branch January 5, 2026 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

We are facing problem with the virtual keys, it seems that if i try to add them from secrets, the chart takes it as plain text “

2 participants