fix: set SecretTypePlainText for plain-text JSON and non-prefixed secret values

[BearTS]

Summary

Ensures that SecretVar instances parsed from plain-text values (i.e., JSON objects with a value field but no type, ref, or from_env) are explicitly tagged with SecretTypePlainText rather than left with an empty SecretType. Previously, these cases fell through without a type assignment, making it ambiguous whether a secret was intentionally plain text or simply uninitialized.

Changes

• When parsing a JSON secret object that has no type, ref, or from_env field, SecretType is now explicitly set to SecretTypePlainText.
• When parseSecretRef returns a plain string value (no env. or vault. prefix), the returned SecretVar now carries SecretTypePlainText.
• During UnmarshalJSON, if SecretType remains empty after processing a recognized JSON structure, it is defaulted to SecretTypePlainText.

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 ./core/schemas/...

Verify that a SecretVar parsed from a plain string or a JSON object like {"value": "my-secret"} has SecretType == SecretTypePlainText rather than an empty string.

Breaking changes

• Yes
• No

Security considerations

This change makes secret type classification more explicit. Code that previously relied on an empty SecretType to identify plain-text secrets should be reviewed to ensure it handles SecretTypePlainText correctly.

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]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Plain-text secret values now consistently keep their type set during parsing and JSON loading.
  • Existing secret inputs without an explicit type are now treated as plain text more reliably.
  • Serialized secret values may now include an explicit plain-text type field for better consistency across formats.

Walkthrough

Modifies parseSecretRef and SecretVar.UnmarshalJSON in core/schemas/secretvar.go to explicitly set SecretType to SecretTypePlainText for plain-value inputs instead of leaving it empty, affecting serialized JSON output for such values.

Changes

Secret Type Defaulting

Layer / File(s)	Summary
parseSecretRef plain text defaulting core/schemas/secretvar.go	Both the JSON-compat object fallback and the plain string fallback in parseSecretRef now explicitly set SecretType to SecretTypePlainText.
UnmarshalJSON plain text defaulting core/schemas/secretvar.go	SecretVar.UnmarshalJSON now assigns SecretTypePlainText when SecretType remains empty after resolving env/vault references.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

• maximhq/bifrost#4927: Both PRs modify core/schemas/secretvar.go's parseSecretRef/SecretVar JSON handling affecting how plain inputs are classified via SecretType.

Suggested reviewers: danpiths, akshaydeo

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise, specific, and accurately reflects the main change: defaulting plain-text SecretVar values to SecretTypePlainText.
Description check	✅ Passed	The description follows the template closely and includes summary, changes, testing, breaking changes, security, and checklist details.
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-06-fix_return_secretvar_type_as_plain_text

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: set SecretTypePlainText for plain-text JSON and non-prefixed secret values #4946 👈 (View in Graphite)
• dev

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

[greptile-apps]

Confidence Score: 4/5

Safe to merge; the logic change is correct and narrows a real ambiguity in plain-text secret classification.

The three touched paths are logically correct and consistent with the existing Scan() implementation. The only gap is that no test explicitly asserts SecretType == SecretTypePlainText for the new cases, and the implicit change in MarshalJSON output (now emitting type:plain_text for round-tripped plain secrets) is not covered by a round-trip test.

core/schemas/secretvar_test.go — needs test cases that assert SecretType == SecretTypePlainText and verify the updated MarshalJSON round-trip output.

Important Files Changed

Filename

Overview

core/schemas/secretvar.go

Three-point fix making SecretTypePlainText explicit: new else branch in parseSecretRef's JSON path, explicit type on plain-string return, and a trailing guard in UnmarshalJSON. Logic is correct and consistent with the already-explicit Scan() path, but no new test cases verify SecretType == SecretTypePlainText, and the JSON serialization change (now emitting type:plain_text) is not covered by a round-trip test.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Input string] --> B{Is valid JSON with 'value' field?}
    B -- Yes --> C{Has explicit type field?}
    C -- Yes --> D[Set SecretType = raw.SecretType]
    C -- No --> E{Has ref field?}
    E -- Yes --> F[Set SecretType = inferSecretType ref]
    E -- No --> G{from_env && env_var set?}
    G -- Yes --> H[Set SecretType = SecretTypeEnv, return]
    G -- No --> I{Val starts with 'env.' == env_var?}
    I -- Yes --> J[Set SecretType = SecretTypeEnv, return]
    I -- No --> K["NEW: Set SecretType = SecretTypePlainText (else branch)"]
    D --> L{Resolve vault/env refs}
    F --> L
    K --> L
    L --> M{"NEW: if SecretType == '' set SecretTypePlainText"}
    M --> N[return]
    B -- No --> O{Starts with 'vault.'?}
    O -- Yes --> P[SecretType = SecretTypeVault]
    O -- No --> Q{Starts with 'env.'?}
    Q -- Yes --> R[SecretType = SecretTypeEnv]
    Q -- No --> S["NEW: SecretType = SecretTypePlainText (was empty)"]

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[Input string] --> B{Is valid JSON with 'value' field?}
    B -- Yes --> C{Has explicit type field?}
    C -- Yes --> D[Set SecretType = raw.SecretType]
    C -- No --> E{Has ref field?}
    E -- Yes --> F[Set SecretType = inferSecretType ref]
    E -- No --> G{from_env && env_var set?}
    G -- Yes --> H[Set SecretType = SecretTypeEnv, return]
    G -- No --> I{Val starts with 'env.' == env_var?}
    I -- Yes --> J[Set SecretType = SecretTypeEnv, return]
    I -- No --> K["NEW: Set SecretType = SecretTypePlainText (else branch)"]
    D --> L{Resolve vault/env refs}
    F --> L
    K --> L
    L --> M{"NEW: if SecretType == '' set SecretTypePlainText"}
    M --> N[return]
    B -- No --> O{Starts with 'vault.'?}
    O -- Yes --> P[SecretType = SecretTypeVault]
    O -- No --> Q{Starts with 'env.'?}
    Q -- Yes --> R[SecretType = SecretTypeEnv]
    Q -- No --> S["NEW: SecretType = SecretTypePlainText (was empty)"]

Loading

_{Reviews (1): Last reviewed commit: "fix: return secretvar type as plain text" | Re-trigger Greptile}

[coderabbitai]

🧹 Nitpick comments (1)

core/schemas/secretvar.go (1)

85-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add table-driven tests for the new defaulting behavior.

None of the three new defaulting paths (plain-value JSON object, non-prefixed plain string, and the UnmarshalJSON catch-all) have accompanying test cases in the diff, even though they change observable JSON serialization output (type field now populated instead of omitted). As per coding guidelines, Go core changes should include "table-driven coverage for behavior changes."

Also applies to: 99-99, 360-362

🤖 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 `@core/schemas/secretvar.go` around lines 85 - 87, Add table-driven tests for
the new defaulting behavior in secretvar.go. Cover the three observable paths
introduced by SecretVar.UnmarshalJSON and related defaulting logic: plain-value
JSON objects, non-prefixed plain string inputs, and the catch-all path that now
sets SecretType instead of leaving it omitted. Use the SecretVar and
UnmarshalJSON symbols to verify the serialized JSON now includes the expected
type field for each case, matching the new defaulting behavior.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@core/schemas/secretvar.go`:
- Around line 85-87: Add table-driven tests for the new defaulting behavior in
secretvar.go. Cover the three observable paths introduced by
SecretVar.UnmarshalJSON and related defaulting logic: plain-value JSON objects,
non-prefixed plain string inputs, and the catch-all path that now sets
SecretType instead of leaving it omitted. Use the SecretVar and UnmarshalJSON
symbols to verify the serialized JSON now includes the expected type field for
each case, matching the new defaulting behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 88c7b94c-8bf6-4d20-a4e7-50ef85219e4c

📥 Commits

Reviewing files that changed from the base of the PR and between 2d85bf1 and a8f1d3c.

📒 Files selected for processing (1)

• core/schemas/secretvar.go

[Pratham-Mishra04]

Merge activity

• Jul 6, 11:15 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jul 6, 11:16 AM UTC: @Pratham-Mishra04 merged this pull request with Graphite.