fix: add `FullyRedacted()` for proxy passwords and `MarshalForStorage()` for `ProxyConfig` to prevent partial value leakage in API responses by BearTS · Pull Request #3445 · maximhq/bifrost

BearTS · 2026-05-12T21:01:58Z

Summary

Introduces a FullyRedacted method on EnvVar and a dedicated MarshalForStorage method on ProxyConfig, along with a shared EnvVarAsString helper, to ensure proxy secrets are never partially exposed in API responses and that EnvVar fields are consistently serialized as plain strings when persisting proxy configuration to the database. Previously, json.Marshal was used directly in the GORM BeforeSave hook, which would serialize EnvVar fields as structured objects rather than the flat string format expected in storage. Additionally, the old Redacted() logic on ProxyConfig could leak substrings of literal passwords through partial masking.

Changes

Added EnvVar.FullyRedacted() which replaces any non-empty value with the fixed placeholder <REDACTED>, ensuring no substring of the original secret is exposed. FromEnv and EnvVar metadata are preserved so env references remain visible and round-trip update merges still match via Equals.
Added EnvVarAsString utility function that returns the wire-form string for an *EnvVar: the env var token if sourced from the environment, or the literal value otherwise.
Added ProxyConfig.MarshalForStorage() which uses EnvVarAsString to flatten all EnvVar fields into plain strings for database persistence. json.Marshal on *ProxyConfig is preserved for HTTP API responses where clients expect the full value/env_var/from_env object structure.
Replaced json.Marshal(p.ProxyConfig) with p.ProxyConfig.MarshalForStorage() in the GORM BeforeSave hook.
Simplified ProxyConfig.Redacted() by removing redundant IsFromEnv() branching. Passwords and CA certificates now use FullyRedacted() to guarantee full opacity, while URL and username delegate to .Redacted(). A nil receiver guard was also added.
Applied the same EnvVarAsString simplification to NetworkConfig.MarshalJSON for CACertPEM.

Type of change

Affected areas

How to test

go test ./core/schemas/... ./framework/configstore/...

Verify that after saving a provider with a proxy configuration containing both literal and env.*-sourced fields, the stored proxy_config_json column contains flat strings (e.g. "url": "http://proxy.example.com" or "url": "env.PROXY_URL") rather than structured EnvVar objects.

Verify that the HTTP API response for the same provider still returns the full EnvVar object structure for proxy fields, and that the password field is serialized as {"val":"<REDACTED>"} with no substring of the original value present.

Breaking changes

Yes
No

Related issues

Security considerations

Proxy passwords are now fully opaque in API responses regardless of whether they are literal values or environment-sourced. The old Redacted() path could expose a prefix of a literal password through partial masking; FullyRedacted() eliminates this by always substituting the fixed <REDACTED> placeholder. Storage serialization writes the env.* token rather than the resolved secret value when the field is environment-sourced, avoiding accidental secret persistence in the database.

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 · 2026-05-12T21:02:06Z

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: e072bd67-64d1-488a-8e65-98a3d4c7cd25

📥 Commits

Reviewing files that changed from the base of the PR and between faefe20 and 04eabd5.

📒 Files selected for processing (6)

core/schemas/envvar.go
core/schemas/envvar_test.go
core/schemas/provider.go
core/schemas/proxyconfig_redaction_test.go
core/schemas/utils.go
framework/configstore/tables/provider.go

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

framework/configstore/tables/provider.go
core/schemas/envvar.go
core/schemas/envvar_test.go

📝 Walkthrough

Summary by CodeRabbit

Bug Fixes
- Proxy configuration now produces a consistent stored JSON, safely handles absent values, and serializes environment-backed entries deterministically.
- Redaction is nil-safe and stronger: credentials and certificates are fully obscured; visible fields are masked when set.
Tests
- Added tests validating credential redaction and environment-backed value handling to ensure secure serialization.

Walkthrough

Adds EnvVarAsString and EnvVar.FullyRedacted; implements ProxyConfig.MarshalForStorage; simplifies NetworkConfig.MarshalJSON and ProxyConfig.Redacted to use the new helpers; wires MarshalForStorage into TableProvider.BeforeSave; and adds tests for EnvVar fully-redaction and password redaction.

Changes

EnvVar Serialization for Proxy Config Persistence

Layer / File(s)	Summary
EnvVar string conversion utility `core/schemas/utils.go`	New `EnvVarAsString(*EnvVar) string` normalizes conversion to the wire-form string: nil -> `""`, env-backed -> the env token, otherwise -> `GetValue()`.
EnvVar FullyRedacted + tests `core/schemas/envvar.go`, `core/schemas/envvar_test.go`	Added `EnvVar.FullyRedacted()` to return a copy with `Val` set to `"<REDACTED>"` (preserving `FromEnv`/`EnvVar`), and tests covering nil, empty, literal, and env-backed cases.
ProxyConfig storage serialization & redaction `core/schemas/provider.go`, `core/schemas/proxyconfig_redaction_test.go`	Added `ProxyConfig.MarshalForStorage()` which serializes EnvVar fields into plain strings via `EnvVarAsString`. Refactored `ProxyConfig.Redacted()` to use `IsSet()` with `Redacted()` for URL/Username and `FullyRedacted()` for Password/CACertPEM. `NetworkConfig.MarshalJSON` now uses `EnvVarAsString` for `CACertPEM`. Includes a test ensuring passwords are fully opaque after redaction and marshaling.
Storage persistence integration `framework/configstore/tables/provider.go`	`TableProvider.BeforeSave` now uses `ProxyConfig.MarshalForStorage()` instead of `json.Marshal(p.ProxyConfig)` when populating `ProxyConfigJSON`.

Sequence Diagram

sequenceDiagram
  participant TableProvider as TableProvider.BeforeSave
  participant ProxyConfig as ProxyConfig.MarshalForStorage
  participant EnvVarAsString as EnvVarAsString
  participant Storage as ProxyConfigJSON
  TableProvider->>ProxyConfig: call MarshalForStorage()
  ProxyConfig->>EnvVarAsString: convert URL/Username/Password/CACertPEM
  EnvVarAsString-->>ProxyConfig: string values
  ProxyConfig-->>Storage: JSON bytes for persistence

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibble at EnvVars, tidy each string,
Marshal them neatly so storage can sing.
Secrets get smooched into "" rows,
Tests keep the burrow safe where no password shows.
Hop—config persisted, and off the bunny goes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: adding `FullyRedacted()` and `MarshalForStorage()` methods to prevent secret leakage in API responses.
Description check	✅ Passed	The description covers all required sections: summary, changes, type of change, affected areas, how to test, security considerations, and checklist. All sections are substantively filled out.
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.

_{✏️ 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 05-13-fix_consistency_for_the_way_we_store_envvar_for_proxyconfig

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 and usage tips.}

BearTS · 2026-05-12T21:02:12Z

fix: add FullyRedacted() for proxy passwords and MarshalForStorage() for ProxyConfig to prevent partial value leakage in API responses #3445 👈 (View in Graphite)
dev

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

greptile-apps · 2026-05-12T21:06:16Z

Confidence Score: 5/5

Safe to merge — the changes are targeted security hardening with no logic regressions.

The storage serialization fix (flat strings vs. structured objects in BeforeSave) and the full-opacity password redaction are both correct. EnvVarAsString is semantically equivalent to the existing driver.Valuer Value() method and matches what EnvVar.Scan() parses on load. The ShouldPreserveStored / IsRedacted round-trip for "" works because IsRedacted uses EqualFold, so the new placeholder is recognized. All edge cases (nil receiver, empty value, unresolved env var) are handled and tested.

No files require special attention.

Important Files Changed

Filename	Overview
core/schemas/envvar.go	Adds FullyRedacted() method that replaces Val with "" while preserving FromEnv/EnvVar metadata; nil guard and empty-value case are handled correctly
core/schemas/utils.go	Adds EnvVarAsString helper; semantically equivalent to the existing driver.Valuer Value() method, consistent with what Scan() expects when loading from DB
core/schemas/provider.go	Adds MarshalForStorage() for flat-string DB persistence and refactors Redacted() to use FullyRedacted() for passwords/CA certs and Redacted() for URL/username; nil receiver guard added
framework/configstore/tables/provider.go	BeforeSave hook now calls MarshalForStorage() instead of json.Marshal, fixing the bug where EnvVar fields were stored as structured objects instead of flat strings
core/schemas/envvar_test.go	Adds tests for FullyRedacted covering nil receiver, empty value, literal, and env-sourced cases; good coverage
core/schemas/proxyconfig_redaction_test.go	New test file verifying literal proxy passwords produce "" with no substring leakage in both Val and JSON output

_{Reviews (6): Last reviewed commit: "fix: consistency for the way we store en..." | Re-trigger Greptile}

coderabbitai

Actionable comments posted: 1

🤖 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 `@core/schemas/provider.go`:
- Around line 289-303: Add a nil-receiver guard at the start of
ProxyConfig.Redacted to avoid panics when called on a nil receiver: in function
ProxyConfig.Redacted() (the method that currently dereferences pc and checks
pc.CACertPEM, pc.URL, pc.Username, pc.Password) add an early check if pc == nil
{ return nil } so the method safely returns when invoked on a nil *ProxyConfig
instead of dereferencing and panicking.

🪄 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: 8cd3d72e-6c6d-44af-a938-1d07a00c2ddb

📥 Commits

Reviewing files that changed from the base of the PR and between 1ca12a9 and 72a1761.

📒 Files selected for processing (3)

core/schemas/provider.go
core/schemas/utils.go
framework/configstore/tables/provider.go

The merge-base changed after approval.

akshaydeo · 2026-05-19T07:55:55Z

Merge activity

May 19, 7:55 AM UTC: A user started a stack merge that includes this pull request via Graphite.
May 19, 7:56 AM UTC: @akshaydeo merged this pull request with Graphite.

…()` for `ProxyConfig` to prevent partial value leakage in API responses (#3445) ## Summary Introduces a `FullyRedacted` method on `EnvVar` and a dedicated `MarshalForStorage` method on `ProxyConfig`, along with a shared `EnvVarAsString` helper, to ensure proxy secrets are never partially exposed in API responses and that `EnvVar` fields are consistently serialized as plain strings when persisting proxy configuration to the database. Previously, `json.Marshal` was used directly in the GORM `BeforeSave` hook, which would serialize `EnvVar` fields as structured objects rather than the flat string format expected in storage. Additionally, the old `Redacted()` logic on `ProxyConfig` could leak substrings of literal passwords through partial masking. ## Changes - Added `EnvVar.FullyRedacted()` which replaces any non-empty value with the fixed placeholder `<REDACTED>`, ensuring no substring of the original secret is exposed. `FromEnv` and `EnvVar` metadata are preserved so env references remain visible and round-trip update merges still match via `Equals`. - Added `EnvVarAsString` utility function that returns the wire-form string for an `*EnvVar`: the env var token if sourced from the environment, or the literal value otherwise. - Added `ProxyConfig.MarshalForStorage()` which uses `EnvVarAsString` to flatten all `EnvVar` fields into plain strings for database persistence. `json.Marshal` on `*ProxyConfig` is preserved for HTTP API responses where clients expect the full `value/env_var/from_env` object structure. - Replaced `json.Marshal(p.ProxyConfig)` with `p.ProxyConfig.MarshalForStorage()` in the GORM `BeforeSave` hook. - Simplified `ProxyConfig.Redacted()` by removing redundant `IsFromEnv()` branching. Passwords and CA certificates now use `FullyRedacted()` to guarantee full opacity, while URL and username delegate to `.Redacted()`. A nil receiver guard was also added. - Applied the same `EnvVarAsString` simplification to `NetworkConfig.MarshalJSON` for `CACertPEM`. ## Type of change - [x] Bug fix - [x] Refactor - [ ] Feature - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/schemas/... ./framework/configstore/... ``` Verify that after saving a provider with a proxy configuration containing both literal and `env.*`-sourced fields, the stored `proxy_config_json` column contains flat strings (e.g. `"url": "http://proxy.example.com"` or `"url": "env.PROXY_URL"`) rather than structured `EnvVar` objects. Verify that the HTTP API response for the same provider still returns the full `EnvVar` object structure for proxy fields, and that the `password` field is serialized as `{"val":"<REDACTED>"}` with no substring of the original value present. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Proxy passwords are now fully opaque in API responses regardless of whether they are literal values or environment-sourced. The old `Redacted()` path could expose a prefix of a literal password through partial masking; `FullyRedacted()` eliminates this by always substituting the fixed `<REDACTED>` placeholder. Storage serialization writes the `env.*` token rather than the resolved secret value when the field is environment-sourced, avoiding accidental secret persistence in the database. ## 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

## Summary This PR cuts the `v1.5.11` / `v1.3.11` release across core, framework, and all plugins, and introduces a new Claude skill (`release-checklist`) for pre-release migration safety auditing. ## Changes - **`release-checklist` skill** — Adds `.claude/skills/release-checklist/SKILL.md`, a read-only pre-release audit tool that scans Go-defined database migrations changed in a release for high-scale deadlock/lock-contention risks and boot-time-blocking operations. It produces a structured `PASS`/`WARN`/`FAIL` report with a concrete remediation plan per finding. The skill is designed to grow via an extensible Checks Registry. - **Version bumps** — `core` → `1.5.11`, `framework` → `1.3.11`, `transports` → `1.5.3`, `plugins/governance` → `1.5.11`, `plugins/logging` → `1.5.11`, `plugins/semanticcache` → `1.5.11`, `plugins/otel` → `1.2.11`, `plugins/maxim` → `1.6.11`, `plugins/prompts` → `1.0.11`, and remaining plugins bumped accordingly. - **Changelogs populated** — All per-package changelogs updated with the full set of features and fixes shipping in this release. Key highlights in this release: - Temporary access tokens for scoped, time-limited API access - MCP per-user OAuth flow refactor - Bedrock Mantle inference engine support - Azure Realtime provider with enriched session tracking - Direct access control (DAC) and virtual key rotation - Cluster-aware log metadata and per-node usage aggregation - Feature flag framework - Config-hash-based file value override of DB on restart - Semantic cache plugin rewrite - Numerous streaming stability, Bedrock, Anthropic, and Gemini fixes - AWS SDK and dependency security updates ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Verify version files reflect the new release cat core/version # expect 1.5.11 cat framework/version # expect 1.3.11 cat transports/version # expect 1.5.3 # Core/Transports go test ./... ``` To exercise the new `release-checklist` skill, invoke it via Claude with: ``` /release-checklist origin/dev...HEAD ``` Expected output: a structured report with `PASS`/`WARN`/`FAIL` per check and a Remediation Plan table for any findings. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues #3603, #3565, #3489, #3334, #3335, #3435, #3554, #3590, #3444, #3198, #3581, #3610, #3599, #3567, #3382, #3461 and others listed in the changelogs. ## Security considerations - AWS SDK and dependency security updates are included (#3461). - `FullyRedacted()` for proxy passwords and `MarshalForStorage()` for `ProxyConfig` prevent partial secret leakage in API responses (#3445). - The `release-checklist` skill is strictly read-only and never modifies files. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

BearTS changed the title ~~fix: consistency for the way we store envvar for proxyconfig~~ refactor: add MarshalForStorage for ProxyConfig and extract EnvVarAsString helper to simplify EnvVar serialization May 12, 2026

BearTS marked this pull request as ready for review May 12, 2026 21:03

greptile-apps Bot reviewed May 12, 2026

View reviewed changes

Comment thread core/schemas/provider.go

coderabbitai Bot requested changes May 12, 2026

View reviewed changes

Comment thread core/schemas/provider.go

BearTS force-pushed the 05-13-fix_consistency_for_the_way_we_store_envvar_for_proxyconfig branch from 72a1761 to 4a6db3a Compare May 12, 2026 21:13

coderabbitai Bot previously approved these changes May 12, 2026

View reviewed changes

akshaydeo force-pushed the dev branch from 1ca12a9 to 47a6eb6 Compare May 13, 2026 10:19

akshaydeo requested a review from a team as a code owner May 13, 2026 10:19

akshaydeo force-pushed the dev branch from 47a6eb6 to 59c5f71 Compare May 13, 2026 10:59

BearTS force-pushed the 05-13-fix_consistency_for_the_way_we_store_envvar_for_proxyconfig branch from 4a6db3a to 8ce31f3 Compare May 13, 2026 12:37

coderabbitai Bot previously approved these changes May 13, 2026

View reviewed changes

akshaydeo force-pushed the dev branch from 89f0bbb to 8be492f Compare May 14, 2026 12:51

BearTS force-pushed the 05-13-fix_consistency_for_the_way_we_store_envvar_for_proxyconfig branch from 8ce31f3 to 630608f Compare May 14, 2026 13:08

coderabbitai Bot previously approved these changes May 14, 2026

View reviewed changes

akshaydeo force-pushed the dev branch from 0914e31 to 6b78c0a Compare May 15, 2026 05:59

BearTS force-pushed the 05-13-fix_consistency_for_the_way_we_store_envvar_for_proxyconfig branch from 630608f to faefe20 Compare May 15, 2026 08:00

coderabbitai Bot previously approved these changes May 15, 2026

View reviewed changes

akshaydeo force-pushed the dev branch from 1ddcfeb to 218e73d Compare May 15, 2026 22:04

fix: consistency for the way we store envvar for proxyconfig

04eabd5

BearTS force-pushed the 05-13-fix_consistency_for_the_way_we_store_envvar_for_proxyconfig branch from faefe20 to 04eabd5 Compare May 18, 2026 05:55

coderabbitai Bot approved these changes May 18, 2026

View reviewed changes

akshaydeo approved these changes May 19, 2026

View reviewed changes

akshaydeo merged commit 71d8375 into dev May 19, 2026
16 checks passed

akshaydeo deleted the 05-13-fix_consistency_for_the_way_we_store_envvar_for_proxyconfig branch May 19, 2026 07:56

akshaydeo mentioned this pull request May 20, 2026

changelog #3628

Merged

18 tasks

akshaydeo mentioned this pull request May 20, 2026

v1.5.3 #3531

Merged

coderabbitai Bot mentioned this pull request May 29, 2026

MCP federation: reveal=false redaction allowlist is incomplete — sshKey.privateKey and login.totp returned in plaintext #3864

Open

Navigation Menu

Uh oh!

fix: add `FullyRedacted()` for proxy passwords and `MarshalForStorage()` for `ProxyConfig` to prevent partial value leakage in API responses - #3445

fix: add `FullyRedacted()` for proxy passwords and `MarshalForStorage()` for `ProxyConfig` to prevent partial value leakage in API responses#3445
akshaydeo merged 1 commit into
devfrom
05-13-fix_consistency_for_the_way_we_store_envvar_for_proxyconfig

BearTS commented May 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 12, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

BearTS commented May 12, 2026

Uh oh!

greptile-apps Bot commented May 12, 2026 •

edited

Loading

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

akshaydeo commented May 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

BearTS commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Type of change

Affected areas

How to test

Breaking changes

Related issues

Security considerations

Checklist

Uh oh!

coderabbitai Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

BearTS commented May 12, 2026

Uh oh!

greptile-apps Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akshaydeo commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

BearTS commented May 12, 2026 •

edited

Loading

coderabbitai Bot commented May 12, 2026 •

edited

Loading

greptile-apps Bot commented May 12, 2026 •

edited

Loading

akshaydeo commented May 19, 2026 •

edited

Loading