Skip to content

fix: repair bare wildcard model-list rows - #5107

Merged
akshaydeo merged 1 commit into
maximhq:devfrom
eyeveil:fix/4318-allowed-models-star
Jul 12, 2026
Merged

akshaydeo merged 1 commit into
maximhq:devfrom
eyeveil:fix/4318-allowed-models-star

Conversation

@eyeveil

@eyeveil eyeveil commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Repairs legacy virtual-key provider-config rows that contain a bare * in JSON-serialized model-list columns. Those rows cannot be deserialized by GORM and can block the provider administration surface until the database is repaired manually.

Changes

  • Add an idempotent data-repair migration converting bare * values to canonical ["*"] JSON in allowed_models and blacklisted_models.
  • Leave valid JSON and all other values untouched.
  • Add regression coverage for the current serializer round trip and for loading both repaired legacy corruption variants.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

Framework/configstore is affected.

How to test

cd framework
go test ./configstore -count=1

Expected: the configstore package passes, including the wildcard round-trip and repair migration tests.

Screenshots/Recordings

Not applicable; no UI changes.

Breaking changes

  • Yes
  • No

Related issues

Fixes #4318

Security considerations

No credential material is read or changed. The migration only canonicalizes two malformed model-list values so affected configuration rows can be loaded again.

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

…eak admin PUTs

Legacy v1.5.10 rows persisted allowed_models as the bare string * in a
serializer:json column; loading them aborts with invalid character '*'
and poisons every subsequent PUT /api/providers for that provider. Adds
a data-repair migration rewriting bare * to canonical ["*"] for both
allowed_models and blacklisted_models (current write path already
round-trips correctly; only legacy rows are affected).

Affected packages:
- framework/configstore/migrations.go

Fixes maximhq#4318
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6011ae2e-59fc-44b1-93b3-a64dcde68d34

📥 Commits

Reviewing files that changed from the base of the PR and between e0d2ac1 and b96ea55.

📒 Files selected for processing (2)
  • framework/configstore/migrations.go
  • framework/configstore/migrations_test.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Fixed compatibility with legacy wildcard model configuration values.
    • Existing provider configurations using wildcard allowlists or blocklists are automatically converted to the supported format.
    • Wildcard settings now load correctly and preserve their intended unrestricted or block-all behavior.

Walkthrough

The change adds a registered migration that repairs bare wildcard values in virtual key provider configuration columns, alongside tests covering canonical wildcard persistence, deserialization, and legacy-row repair.

Changes

Wildcard persistence repair

Layer / File(s) Summary
Canonical wildcard round trip
framework/configstore/migrations_test.go
Tests verify wildcard allowlist and blocklist values persist as ["*"] and deserialize into unrestricted and block-all semantics.
Legacy wildcard repair migration
framework/configstore/migrations.go, framework/configstore/migrations_test.go
Registers a forward-only migration that rewrites bare * values in both model columns, logs affected rows, and verifies repaired rows deserialize correctly.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MigrationRunner
  participant migrationRepairBareWildcardAllowedModels
  participant Database
  MigrationRunner->>migrationRepairBareWildcardAllowedModels: run registered migration
  migrationRepairBareWildcardAllowedModels->>Database: update bare '*' values
  Database-->>migrationRepairBareWildcardAllowedModels: return affected row counts
  migrationRepairBareWildcardAllowedModels-->>MigrationRunner: complete migration
Loading

Suggested reviewers: danpiths, Pratham-Mishra04

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the PR's main fix: repairing bare wildcard model-list rows.
Description check ✅ Passed The description covers summary, changes, testing, related issue, breaking changes, and security considerations.
Linked Issues check ✅ Passed The migration and tests address #4318 by repairing bare '*' values to JSON arrays and covering both affected columns.
Out of Scope Changes check ✅ Passed The diff stays within configstore migration and regression tests, with no obvious unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The migration is narrow, idempotent, and covered by tests for both repaired columns.
  • The no-op rollback is documented and avoids restoring the malformed value.

Important Files Changed

Filename Overview
framework/configstore/migrations.go Adds the final repair migration for exact bare wildcard values in virtual-key provider config model-list columns.
framework/configstore/migrations_test.go Adds tests for wildcard GORM round trips and for repairing both affected provider-config columns.

Reviews (1): Last reviewed commit: "[fix]: configstore - repair bare wildcar..." | Re-trigger Greptile

@akshaydeo

Copy link
Copy Markdown
Contributor

@eyeveil we never allowed blank * in that field - how did you land up in this scenario ?

@akshaydeo
akshaydeo merged commit b9d2aaf into maximhq:dev Jul 12, 2026
5 checks passed
akshaydeo pushed a commit that referenced this pull request Jul 14, 2026
…eak admin PUTs (#5107)

Legacy v1.5.10 rows persisted allowed_models as the bare string * in a
serializer:json column; loading them aborts with invalid character '*'
and poisons every subsequent PUT /api/providers for that provider. Adds
a data-repair migration rewriting bare * to canonical ["*"] for both
allowed_models and blacklisted_models (current write path already
round-trips correctly; only legacy rows are affected).

Affected packages:
- framework/configstore/migrations.go

Fixes #4318
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
…eak admin PUTs (maximhq#5107)

Legacy v1.5.10 rows persisted allowed_models as the bare string * in a
serializer:json column; loading them aborts with invalid character '*'
and poisons every subsequent PUT /api/providers for that provider. Adds
a data-repair migration rewriting bare * to canonical ["*"] for both
allowed_models and blacklisted_models (current write path already
round-trips correctly; only legacy rows are affected).

Affected packages:
- framework/configstore/migrations.go

Fixes maximhq#4318
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
…eak admin PUTs (maximhq#5107)

Legacy v1.5.10 rows persisted allowed_models as the bare string * in a
serializer:json column; loading them aborts with invalid character '*'
and poisons every subsequent PUT /api/providers for that provider. Adds
a data-repair migration rewriting bare * to canonical ["*"] for both
allowed_models and blacklisted_models (current write path already
round-trips correctly; only legacy rows are affected).

Affected packages:
- framework/configstore/migrations.go

Fixes maximhq#4318
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants