Skip to content

fix: fixes connector UI redaction storage issue - #5478

Merged
akshaydeo merged 1 commit into
devfrom
07-23-fix_fixes_connector_ui_redaction_storage_issue
Jul 23, 2026
Merged

akshaydeo merged 1 commit into
devfrom
07-23-fix_fixes_connector_ui_redaction_storage_issue

Conversation

@roroghost17

@roroghost17 roroghost17 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a bug where plain-text SecretVar objects (e.g. {"value": "..."} with no ref/type fields) were not being recognised as SecretVar-shaped during redaction restoration. This caused the UI to persist masked values instead of restoring the real stored secrets when saving Kafka SASL credentials or similar connectors that store secrets as plain strings but return them as value-only objects after a redacted GET.

Changes

  • isSecretVarObject previously required either ref+type or env_var+from_env alongside value, which excluded plain-text SecretVars that marshal as {"value": "..."} alone (since ref/type are omitempty). The function now accepts any map whose keys are exclusively drawn from the known SecretVar field set (value, ref, type, env_var, from_env), with value required to be a string.
  • This ensures that value-only objects round-tripped by the UI after a redacted GET are correctly identified and restored from the existing stored value, rather than being passed through with the masked content.
  • Objects with a non-redacted value (e.g. username shown in clear) pass through unchanged, and intentional updates (new password, env reference) are not clobbered.
  • Tests added for the Kafka SASL credential shape, the FullyRedacted() sentinel (<REDACTED>), and intentional secret rotation/env-ref switching.

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 ./transports/bifrost-http/handlers/...

The new tests cover:

  • Kafka SASL password and ca_cert restored from stored plain strings when the UI sends back value-only masked objects.
  • FullyRedacted() sentinel (<REDACTED>) correctly triggers restoration.
  • Rotated passwords and env-ref switches pass through without being overwritten by the stored value.

Breaking changes

  • Yes
  • No

Security considerations

This change affects how redacted secret values are handled during plugin configuration updates. The fix ensures masked values are never persisted in place of real secrets, and that intentional secret rotations or env-ref changes are not silently discarded. No new secret exposure surface is introduced.

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

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

isSecretVarObject now recognizes validated value-only SecretVar objects and rejects unknown fields. Tests cover restoration of Kafka secret values and fully redacted telemetry or push gateway passwords.

Changes

SecretVar redaction restoration

Layer / File(s) Summary
SecretVar detection and restoration coverage
transports/bifrost-http/handlers/plugins.go, transports/bifrost-http/handlers/plugins_test.go
isSecretVarObject validates string values and allowed fields, while tests verify restoration of Kafka secrets and fully redacted password sentinels.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: bearts

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
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.
Title check ✅ Passed The title is concise and describes the main fix: connector UI redaction storage handling.
Description check ✅ Passed The description matches the template well and includes summary, changes, testing, security, and checklist details.
✨ 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-23-fix_fixes_connector_ui_redaction_storage_issue

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

@CLAassistant

CLAassistant commented Jul 23, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

roroghost17 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@roroghost17
roroghost17 marked this pull request as ready for review July 23, 2026 06:06
@coderabbitai
coderabbitai Bot requested a review from BearTS July 23, 2026 06:07
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 23, 2026
akshaydeo
akshaydeo previously approved these changes Jul 23, 2026

akshaydeo commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Jul 23, 7:34 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 23, 7:36 AM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 23, 7:47 AM UTC: Graphite couldn't merge this PR because it was not satisfying all requirements (PR has had changes requested).
  • Jul 23, 7:58 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 23, 7:59 AM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from 07-21-feat_adds_bifrost_latency_data_to_connectors to graphite-base/5478 July 23, 2026 07:34
@akshaydeo
akshaydeo changed the base branch from graphite-base/5478 to dev July 23, 2026 07:35
@akshaydeo
akshaydeo dismissed stale reviews from coderabbitai[bot] and themself July 23, 2026 07:35

The base branch was changed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@transports/bifrost-http/handlers/plugins_test.go`:
- Around line 188-205: Update the rotated and environment-reference fixtures in
the restoreRedactedFromExisting tests to use schema-valid SecretVar shapes:
represent the rotated password as {"value": "..."} only, and represent the
environment reference with {"env_var": "KAFKA_PASSWORD", "from_env": true}.
Adjust the assertions as needed while preserving the expected pass-through
behavior.
🪄 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: 6a7a03bf-e417-4c1f-a7e3-32882ad2b007

📥 Commits

Reviewing files that changed from the base of the PR and between e607088 and d756bd2.

📒 Files selected for processing (2)
  • transports/bifrost-http/handlers/plugins.go
  • transports/bifrost-http/handlers/plugins_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • transports/bifrost-http/handlers/plugins.go

Comment thread transports/bifrost-http/handlers/plugins_test.go
@akshaydeo
akshaydeo force-pushed the 07-23-fix_fixes_connector_ui_redaction_storage_issue branch from e607088 to d756bd2 Compare July 23, 2026 07:57
@akshaydeo
akshaydeo merged commit a9946e9 into dev Jul 23, 2026
12 of 13 checks passed
@akshaydeo
akshaydeo deleted the 07-23-fix_fixes_connector_ui_redaction_storage_issue branch July 23, 2026 07:59
akshaydeo pushed a commit that referenced this pull request Jul 24, 2026
## Summary

Fixes a bug where plain-text `SecretVar` objects (e.g. `{"value": "..."}` with no `ref`/`type` fields) were not being recognised as `SecretVar`-shaped during redaction restoration. This caused the UI to persist masked values instead of restoring the real stored secrets when saving Kafka SASL credentials or similar connectors that store secrets as plain strings but return them as value-only objects after a redacted GET.

## Changes

- `isSecretVarObject` previously required either `ref`+`type` or `env_var`+`from_env` alongside `value`, which excluded plain-text `SecretVar`s that marshal as `{"value": "..."}` alone (since `ref`/`type` are `omitempty`). The function now accepts any map whose keys are exclusively drawn from the known `SecretVar` field set (`value`, `ref`, `type`, `env_var`, `from_env`), with `value` required to be a string.
- This ensures that value-only objects round-tripped by the UI after a redacted GET are correctly identified and restored from the existing stored value, rather than being passed through with the masked content.
- Objects with a non-redacted value (e.g. username shown in clear) pass through unchanged, and intentional updates (new password, env reference) are not clobbered.
- Tests added for the Kafka SASL credential shape, the `FullyRedacted()` sentinel (`<REDACTED>`), and intentional secret rotation/env-ref switching.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./transports/bifrost-http/handlers/...
```

The new tests cover:
- Kafka SASL `password` and `ca_cert` restored from stored plain strings when the UI sends back value-only masked objects.
- `FullyRedacted()` sentinel (`<REDACTED>`) correctly triggers restoration.
- Rotated passwords and env-ref switches pass through without being overwritten by the stored value.

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

This change affects how redacted secret values are handled during plugin configuration updates. The fix ensures masked values are never persisted in place of real secrets, and that intentional secret rotations or env-ref changes are not silently discarded. No new secret exposure surface is introduced.

## Checklist

- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
## Summary

Fixes a bug where plain-text `SecretVar` objects (e.g. `{"value": "..."}` with no `ref`/`type` fields) were not being recognised as `SecretVar`-shaped during redaction restoration. This caused the UI to persist masked values instead of restoring the real stored secrets when saving Kafka SASL credentials or similar connectors that store secrets as plain strings but return them as value-only objects after a redacted GET.

## Changes

- `isSecretVarObject` previously required either `ref`+`type` or `env_var`+`from_env` alongside `value`, which excluded plain-text `SecretVar`s that marshal as `{"value": "..."}` alone (since `ref`/`type` are `omitempty`). The function now accepts any map whose keys are exclusively drawn from the known `SecretVar` field set (`value`, `ref`, `type`, `env_var`, `from_env`), with `value` required to be a string.
- This ensures that value-only objects round-tripped by the UI after a redacted GET are correctly identified and restored from the existing stored value, rather than being passed through with the masked content.
- Objects with a non-redacted value (e.g. username shown in clear) pass through unchanged, and intentional updates (new password, env reference) are not clobbered.
- Tests added for the Kafka SASL credential shape, the `FullyRedacted()` sentinel (`<REDACTED>`), and intentional secret rotation/env-ref switching.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./transports/bifrost-http/handlers/...
```

The new tests cover:
- Kafka SASL `password` and `ca_cert` restored from stored plain strings when the UI sends back value-only masked objects.
- `FullyRedacted()` sentinel (`<REDACTED>`) correctly triggers restoration.
- Rotated passwords and env-ref switches pass through without being overwritten by the stored value.

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

This change affects how redacted secret values are handled during plugin configuration updates. The fix ensures masked values are never persisted in place of real secrets, and that intentional secret rotations or env-ref changes are not silently discarded. No new secret exposure surface is introduced.

## Checklist

- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
## Summary

Fixes a bug where plain-text `SecretVar` objects (e.g. `{"value": "..."}` with no `ref`/`type` fields) were not being recognised as `SecretVar`-shaped during redaction restoration. This caused the UI to persist masked values instead of restoring the real stored secrets when saving Kafka SASL credentials or similar connectors that store secrets as plain strings but return them as value-only objects after a redacted GET.

## Changes

- `isSecretVarObject` previously required either `ref`+`type` or `env_var`+`from_env` alongside `value`, which excluded plain-text `SecretVar`s that marshal as `{"value": "..."}` alone (since `ref`/`type` are `omitempty`). The function now accepts any map whose keys are exclusively drawn from the known `SecretVar` field set (`value`, `ref`, `type`, `env_var`, `from_env`), with `value` required to be a string.
- This ensures that value-only objects round-tripped by the UI after a redacted GET are correctly identified and restored from the existing stored value, rather than being passed through with the masked content.
- Objects with a non-redacted value (e.g. username shown in clear) pass through unchanged, and intentional updates (new password, env reference) are not clobbered.
- Tests added for the Kafka SASL credential shape, the `FullyRedacted()` sentinel (`<REDACTED>`), and intentional secret rotation/env-ref switching.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./transports/bifrost-http/handlers/...
```

The new tests cover:
- Kafka SASL `password` and `ca_cert` restored from stored plain strings when the UI sends back value-only masked objects.
- `FullyRedacted()` sentinel (`<REDACTED>`) correctly triggers restoration.
- Rotated passwords and env-ref switches pass through without being overwritten by the stored value.

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

This change affects how redacted secret values are handled during plugin configuration updates. The fix ensures masked values are never persisted in place of real secrets, and that intentional secret rotations or env-ref changes are not silently discarded. No new secret exposure surface is introduced.

## Checklist

- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
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