Skip to content

fix: auto-redact env-backed values in EnvVar JSON serialization#2540

Merged
akshaydeo merged 2 commits intov1.5.0from
04-07-fix_prevent_env_var_secret_leakage_in_api_responses
Apr 8, 2026
Merged

fix: auto-redact env-backed values in EnvVar JSON serialization#2540
akshaydeo merged 2 commits intov1.5.0from
04-07-fix_prevent_env_var_secret_leakage_in_api_responses

Conversation

@impoiler
Copy link
Copy Markdown
Contributor

@impoiler impoiler commented Apr 7, 2026

Summary

Implements automatic redaction of environment variable values in JSON serialization to prevent secrets from leaking through API responses. Adds comprehensive redaction support for all vector store configurations and fixes Azure configuration reconstruction logic.

Changes

  • Added MarshalJSON method to EnvVar that automatically redacts values sourced from environment variables during JSON serialization
  • Extended vector store redaction to support Redis, Qdrant, and Pinecone configurations in addition to existing Weaviate support
  • Fixed Azure configuration reconstruction logic to check for any Azure field presence instead of only the endpoint field

Type of change

  • Feature
  • Bug fix

Affected areas

  • Core (Go)
  • Transports (HTTP)

How to test

Test the automatic redaction behavior:

# Test EnvVar JSON serialization with environment variables
export TEST_SECRET="sensitive_value"
go test ./core/schemas -run TestEnvVarMarshalJSON

# Test vector store configuration redaction
go test ./transports/bifrost-http/lib -run TestGetVectorStoreConfigRedacted

# Verify overall functionality
go test ./...

Test with various vector store configurations to ensure secrets are properly redacted while preserving non-sensitive configuration values.

Screenshots/Recordings

N/A

Breaking changes

  • Yes
  • No

Related issues

N/A

Security considerations

This change significantly improves security by:

  • Automatically preventing environment variable secrets from appearing in JSON API responses
  • Ensuring comprehensive redaction across all vector store provider configurations
  • Maintaining backward compatibility while adding defense-in-depth for secret handling

The automatic redaction only affects JSON serialization and does not impact database persistence, encryption, or internal LLM request paths.

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
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 7, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1070e4f7-96fe-4a9b-9c3d-db45ee0c1d92

📥 Commits

Reviewing files that changed from the base of the PR and between 38d9f8b and 82740d2.

📒 Files selected for processing (2)
  • core/schemas/envvar.go
  • framework/configstore/clientconfig_redaction_test.go
✅ Files skipped from review due to trivial changes (1)
  • core/schemas/envvar.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • framework/configstore/clientconfig_redaction_test.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Environment-backed configuration values are now properly redacted in JSON output: resolved secret values are removed while environment variable references are preserved, and redaction does not alter in-memory configuration or runtime retrieval.
  • Tests

    • Added comprehensive tests validating redaction behavior across multiple provider types and scenarios, ensuring masked outputs contain env references and no resolved secret strings.

Walkthrough

Added EnvVar.MarshalJSON to emit a redacted value when FromEnv is true (preserving env_var and from_env) and added unit tests validating JSON redaction for env-backed fields across multiple provider configs without mutating in-memory values.

Changes

Cohort / File(s) Summary
Core Implementation
core/schemas/envvar.go
Added func (e EnvVar) MarshalJSON() ([]byte, error) that uses sonic.Marshal on an alias of EnvVar and replaces value with e.Redacted() when FromEnv is true, preserving env_var and from_env.
Test Coverage
framework/configstore/clientconfig_redaction_test.go
New tests (~5) verifying JSON redaction of env-backed fields across provider configs (Azure, Vertex, Bedrock, OpenAI); assert resolved secrets are masked in marshaled JSON, env.<NAME> and from_env:true remain, and original in-memory values remain unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I nibble JSON carrots in the night,
Env names glow while secret bits take flight.
FromEnv whispers, values tucked out of view,
Tests hop around to prove the mask is true.
A tiny rabbit clap — redaction done anew.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding automatic redaction of environment variable values in JSON serialization of EnvVar types.
Description check ✅ Passed The description covers all major template sections including summary, changes, type of change, affected areas, how to test, breaking changes, security considerations, and checklist completion.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 04-07-fix_prevent_env_var_secret_leakage_in_api_responses

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

Copy link
Copy Markdown
Contributor Author

impoiler commented Apr 7, 2026

@impoiler impoiler force-pushed the 04-06-fix_validation_in_all_provider_configuration_forms branch from ca3b8b7 to b02ecee Compare April 7, 2026 07:09
@impoiler impoiler force-pushed the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch from 61d2f30 to 890c519 Compare April 7, 2026 07:09
@impoiler impoiler changed the title fix: prevent env var secret leakage in API responses feat: add automatic secret redaction to EnvVar JSON serialization and expand vector store config redaction Apr 7, 2026
@impoiler impoiler self-assigned this Apr 7, 2026
@impoiler impoiler force-pushed the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch from 890c519 to f9ed215 Compare April 7, 2026 07:40
@impoiler impoiler force-pushed the 04-06-fix_validation_in_all_provider_configuration_forms branch 2 times, most recently from a1fc8b1 to 47c6f82 Compare April 7, 2026 07:59
@impoiler impoiler force-pushed the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch from f9ed215 to 3904313 Compare April 7, 2026 07:59
@impoiler impoiler marked this pull request as ready for review April 7, 2026 07:59
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 7, 2026

Confidence Score: 5/5

Safe to merge — the change is a well-scoped, idempotent security hardening with thorough tests and no impact on persistence or inference paths.

No P0 or P1 findings. The MarshalJSON implementation is correct (value receiver, envVarAlias recursion guard, scoped to JSON serialization only). Double-masking via ProviderConfig.Redacted() + MarshalJSON is idempotent and harmless. Tests cover all key paths including mutation safety.

No files require special attention.

Vulnerabilities

No security concerns identified. The change moves in the correct direction — adding MarshalJSON as a defence-in-depth layer so that resolved env-var secrets cannot reach JSON API responses even if upstream Redacted() call-sites are incomplete. GORM persistence (Value()) and internal inference paths (GetValue()) are unaffected by the new method.

Important Files Changed

Filename Overview
core/schemas/envvar.go Added MarshalJSON (value receiver) that auto-redacts env-backed EnvVar during JSON serialization; uses envVarAlias to avoid infinite recursion in sonic.Marshal; does not affect GORM Value() or GetValue() paths
framework/configstore/clientconfig_redaction_test.go New test file covering: env-backed auto-masking, plain-value passthrough, Vertex field masking, no-mutation guarantee, and a full multi-provider smoke test for zero leaked secrets

Reviews (11): Last reviewed commit: "fix: prevent env var secret leakage in A..." | Re-trigger Greptile

Comment thread transports/bifrost-http/lib/config.go Outdated
@impoiler impoiler force-pushed the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch from 3904313 to f89cd87 Compare April 7, 2026 08:14
@impoiler impoiler force-pushed the 04-06-fix_validation_in_all_provider_configuration_forms branch from 47c6f82 to 8882712 Compare April 7, 2026 09:43
@impoiler impoiler force-pushed the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch from f89cd87 to c662e1e Compare April 7, 2026 09:43
Copy link
Copy Markdown
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

🧹 Nitpick comments (1)
core/schemas/envvar.go (1)

170-170: Simplify (&e).Redacted() to e.Redacted().

In Go, when calling a method with a pointer receiver on an addressable value, the compiler automatically takes the address. Since e is a function parameter (addressable), the explicit (&e) is unnecessary.

✨ Suggested simplification
 	if e.FromEnv {
 		// Redact the resolved value but keep the env var reference and from_env flag
 		// so the UI still knows which env var backs this field.
-		redacted := (&e).Redacted()
+		redacted := e.Redacted()
 		if redacted != nil {
 			out = envVarAlias(*redacted)
 		}
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/schemas/envvar.go` at line 170, The call uses an unnecessary explicit
address operator; replace (&e).Redacted() with e.Redacted() because the
parameter e is addressable and the compiler will automatically take its address
for a pointer-receiver method; update the invocation in the function where
redacted := (&e).Redacted() appears to use redacted := e.Redacted() referencing
the Redacted method on the env var type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@core/schemas/envvar.go`:
- Line 175: Replace the direct call to sonic.Marshal in the custom MarshalJSON
implementation with the project wrapper MarshalSorted to preserve WASM/native
compatibility (i.e., change the return from sonic.Marshal(out) to
MarshalSorted(out)); also simplify the Redacted invocation by calling
e.Redacted() instead of (&e).Redacted() since Go will auto-address for
pointer-receiver methods—update the MarshalJSON (and any nearby uses)
accordingly.

---

Nitpick comments:
In `@core/schemas/envvar.go`:
- Line 170: The call uses an unnecessary explicit address operator; replace
(&e).Redacted() with e.Redacted() because the parameter e is addressable and the
compiler will automatically take its address for a pointer-receiver method;
update the invocation in the function where redacted := (&e).Redacted() appears
to use redacted := e.Redacted() referencing the Redacted method on the env var
type.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 48c94cae-a136-48ed-8763-f2a605b8aad9

📥 Commits

Reviewing files that changed from the base of the PR and between f89cd87 and c662e1e.

📒 Files selected for processing (2)
  • core/schemas/envvar.go
  • framework/configstore/clientconfig_redaction_test.go
✅ Files skipped from review due to trivial changes (1)
  • framework/configstore/clientconfig_redaction_test.go

Comment thread core/schemas/envvar.go
@impoiler impoiler force-pushed the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch from c662e1e to 548ec55 Compare April 7, 2026 10:32
Copy link
Copy Markdown
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.

♻️ Duplicate comments (1)
core/schemas/envvar.go (1)

175-175: ⚠️ Potential issue | 🟠 Major

Use the core/schemas marshal wrapper in MarshalJSON (Line 175).

Direct sonic.Marshal here bypasses the package abstraction expected for custom marshalers in this package and can diverge from platform-specific behavior in the stack.

Suggested fix
-	return sonic.Marshal(out)
+	return MarshalSorted(out)

Based on learnings: In core/schemas, custom MarshalJSON methods should use the package marshal wrapper (and prior stack feedback for this exact line required MarshalSorted for compatibility).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/schemas/envvar.go` at line 175, The MarshalJSON method is calling
sonic.Marshal(out) directly; replace that call with the core/schemas package
marshal wrapper (use the package's MarshalSorted wrapper used across this
package) so the EnvVar.MarshalJSON method uses schemas.MarshalSorted(out) (or
the package's exported marshal helper) instead of sonic.Marshal to preserve
platform-specific ordering/behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@core/schemas/envvar.go`:
- Line 175: The MarshalJSON method is calling sonic.Marshal(out) directly;
replace that call with the core/schemas package marshal wrapper (use the
package's MarshalSorted wrapper used across this package) so the
EnvVar.MarshalJSON method uses schemas.MarshalSorted(out) (or the package's
exported marshal helper) instead of sonic.Marshal to preserve platform-specific
ordering/behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dfede440-09e2-4599-ab74-dd59e5ca4eeb

📥 Commits

Reviewing files that changed from the base of the PR and between c662e1e and 548ec55.

📒 Files selected for processing (2)
  • core/schemas/envvar.go
  • framework/configstore/clientconfig_redaction_test.go
✅ Files skipped from review due to trivial changes (1)
  • framework/configstore/clientconfig_redaction_test.go

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 7, 2026
@impoiler impoiler force-pushed the 04-06-fix_validation_in_all_provider_configuration_forms branch from 8882712 to 2c660e6 Compare April 7, 2026 15:38
@impoiler impoiler force-pushed the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch 2 times, most recently from 1f09dc1 to d60aba7 Compare April 8, 2026 04:50
@impoiler impoiler force-pushed the 04-06-fix_validation_in_all_provider_configuration_forms branch from 2c660e6 to bf47ff7 Compare April 8, 2026 04:50
@impoiler impoiler force-pushed the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch from d60aba7 to 937c66f Compare April 8, 2026 05:18
@impoiler impoiler force-pushed the 04-06-fix_validation_in_all_provider_configuration_forms branch from bf47ff7 to 20058ea Compare April 8, 2026 05:18
@impoiler impoiler changed the title feat: add automatic secret redaction to EnvVar JSON serialization and expand vector store config redaction fix: auto-redact env-backed values in EnvVar JSON serialization Apr 8, 2026
@impoiler impoiler force-pushed the 04-06-fix_validation_in_all_provider_configuration_forms branch from 20058ea to bf2c06c Compare April 8, 2026 05:56
@impoiler impoiler force-pushed the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch from 937c66f to 38d9f8b Compare April 8, 2026 05:56
Copy link
Copy Markdown
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.

🧹 Nitpick comments (1)
framework/configstore/clientconfig_redaction_test.go (1)

173-174: Use a non-AWS-shaped fixture value to avoid secret-scanner false positives.

Line 173 uses an AKIA...-shaped token, which can trigger automated secret scanning noise even in tests.

Proposed fixture-safe diff
-	t.Setenv("LEAK_TEST_BEDROCK_ACCESS", "AKIAIOSFODNN7LEAKED1")
+	t.Setenv("LEAK_TEST_BEDROCK_ACCESS", "aws-access-key-redaction-fixture")
...
-		"AKIAIOSFODNN7LEAKED1",
+		"aws-access-key-redaction-fixture",

Also applies to: 222-223

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@framework/configstore/clientconfig_redaction_test.go` around lines 173 - 174,
The test uses an AWS-shaped fixture "AKIA..." which can trigger secret scanners;
update the t.Setenv calls in clientconfig_redaction_test.go (the
t.Setenv("LEAK_TEST_BEDROCK_ACCESS", ...) and any other occurrences around the
same test such as the ones at lines referenced) to use a clearly synthetic,
non-AWS-shaped value (e.g., "TEST_BEDROCK_ACCESS_ABC123") and likewise replace
any other fixtures with suspicious prefixes (e.g., change
"sk-leaked-openai-key-1234567890" to "TEST_OPENAI_KEY_ABC123") so all t.Setenv
calls in this file use harmless test tokens instead of real-patterned secrets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@framework/configstore/clientconfig_redaction_test.go`:
- Around line 173-174: The test uses an AWS-shaped fixture "AKIA..." which can
trigger secret scanners; update the t.Setenv calls in
clientconfig_redaction_test.go (the t.Setenv("LEAK_TEST_BEDROCK_ACCESS", ...)
and any other occurrences around the same test such as the ones at lines
referenced) to use a clearly synthetic, non-AWS-shaped value (e.g.,
"TEST_BEDROCK_ACCESS_ABC123") and likewise replace any other fixtures with
suspicious prefixes (e.g., change "sk-leaked-openai-key-1234567890" to
"TEST_OPENAI_KEY_ABC123") so all t.Setenv calls in this file use harmless test
tokens instead of real-patterned secrets.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 04d5d309-fbca-42b0-bd2c-94353165d845

📥 Commits

Reviewing files that changed from the base of the PR and between 937c66f and 38d9f8b.

📒 Files selected for processing (2)
  • core/schemas/envvar.go
  • framework/configstore/clientconfig_redaction_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • core/schemas/envvar.go

@impoiler impoiler force-pushed the 04-06-fix_validation_in_all_provider_configuration_forms branch from bf2c06c to 2c064ab Compare April 8, 2026 13:27
@impoiler impoiler force-pushed the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch from 38d9f8b to 82740d2 Compare April 8, 2026 13:27
Copy link
Copy Markdown
Contributor

akshaydeo commented Apr 8, 2026

Merge activity

  • Apr 8, 2:07 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Apr 8, 2:09 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo akshaydeo changed the base branch from 04-06-fix_validation_in_all_provider_configuration_forms to graphite-base/2540 April 8, 2026 14:08
@akshaydeo akshaydeo changed the base branch from graphite-base/2540 to v1.5.0 April 8, 2026 14:08
@akshaydeo akshaydeo dismissed stale reviews from Pratham-Mishra04 and coderabbitai[bot] April 8, 2026 14:08

The base branch was changed.

@akshaydeo akshaydeo merged commit 75cb667 into v1.5.0 Apr 8, 2026
15 of 16 checks passed
@akshaydeo akshaydeo deleted the 04-07-fix_prevent_env_var_secret_leakage_in_api_responses branch April 8, 2026 14:09
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.

3 participants