-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: add expiration support to virtual keys #4882
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -165,6 +165,7 @@ type CreateVirtualKeyRequest struct { | |
| RateLimit *CreateRateLimitRequest `json:"rate_limit,omitempty"` | ||
| IsActive *bool `json:"is_active,omitempty"` | ||
| CalendarAligned bool `json:"calendar_aligned,omitempty"` // When true, all budgets reset at clean calendar boundaries | ||
| ExpiresAt *time.Time `json:"expires_at,omitempty"` // Optional expiry; nil means never expires | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the relevant handler and schema sections with line numbers.
git ls-files transports/bifrost-http/handlers/governance.go transports/config.schema.json
echo "---- governance.go ----"
sed -n '140,230p' transports/bifrost-http/handlers/governance.go
echo "---- config.schema.json search ----"
rg -n '"expires_at"|"clear_expires_at"|virtual_keys|additionalProperties' transports/config.schema.jsonRepository: maximhq/bifrost Length of output: 16596 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Show the exact virtual_key object schema section if present.
python3 - <<'PY'
from pathlib import Path
p = Path('transports/config.schema.json')
text = p.read_text()
for needle in ['"virtual_keys"', '"expires_at"', '"clear_expires_at"']:
idx = text.find(needle)
print(f"\n== {needle} ==")
if idx == -1:
print("not found")
continue
start = max(0, text.rfind('\n', 0, idx-500))
end = text.find('\n', idx+1000)
print(text[start:end if end!=-1 else len(text)])
PYRepository: maximhq/bifrost Length of output: 1776 Add 🤖 Prompt for AI AgentsSource: Path instructions |
||
| } | ||
|
|
||
| // UpdateVirtualKeyRequest represents the request body for updating a virtual key | ||
|
|
@@ -193,6 +194,8 @@ type UpdateVirtualKeyRequest struct { | |
| IsActive *bool `json:"is_active,omitempty"` | ||
| CalendarAligned *bool `json:"calendar_aligned,omitempty"` // When true, all budgets reset at clean calendar boundaries | ||
| ResetBudgetUsage *bool `json:"reset_budget_usage,omitempty"` | ||
| ExpiresAt *time.Time `json:"expires_at,omitempty"` // Set a new expiry; nil means "leave unchanged" | ||
| ClearExpiresAt bool `json:"clear_expires_at,omitempty"` // true to remove an existing expiry | ||
| } | ||
|
|
||
| var errVirtualKeyDualAssociation = errors.New("VirtualKey cannot be attached to both Team and Customer") | ||
|
|
@@ -1271,6 +1274,14 @@ func (h *GovernanceHandler) createVirtualKey(ctx *fasthttp.RequestCtx) { | |
| seenDurations[b.ResetDuration] = true | ||
| } | ||
| } | ||
| // Validate expires_at: must be in the future if provided | ||
| if req.ExpiresAt != nil { | ||
| now := time.Now().UTC() | ||
| if !req.ExpiresAt.After(now) { | ||
| SendError(ctx, 400, "expires_at must be a future timestamp") | ||
| return | ||
| } | ||
| } | ||
| // Set defaults: nil means "use DB default (true)" | ||
| isActive := req.IsActive | ||
| if isActive == nil { | ||
|
|
@@ -1297,6 +1308,7 @@ func (h *GovernanceHandler) createVirtualKey(ctx *fasthttp.RequestCtx) { | |
| CustomerID: req.CustomerID, | ||
| IsActive: isActive, | ||
| CalendarAligned: req.CalendarAligned, | ||
| ExpiresAt: req.ExpiresAt, | ||
| } | ||
| if err := h.configStore.CreateVirtualKey(ctx, &vk, tx); err != nil { | ||
| return err | ||
|
|
@@ -1495,6 +1507,19 @@ func (h *GovernanceHandler) updateVirtualKey(ctx *fasthttp.RequestCtx) { | |
| SendError(ctx, 400, "VirtualKey cannot be attached to both Team and Customer") | ||
| return | ||
| } | ||
| // Validate mutually exclusive ExpiresAt and ClearExpiresAt | ||
| if req.ExpiresAt != nil && req.ClearExpiresAt { | ||
| SendError(ctx, 400, "cannot set both expires_at and clear_expires_at") | ||
| return | ||
| } | ||
| // Validate expires_at: must be in the future if provided | ||
| if req.ExpiresAt != nil { | ||
| now := time.Now().UTC() | ||
| if !req.ExpiresAt.After(now) { | ||
| SendError(ctx, 400, "expires_at must be a future timestamp") | ||
| return | ||
| } | ||
| } | ||
| vk, err := h.configStore.GetVirtualKey(ctx, vkID) | ||
| if err != nil { | ||
| if errors.Is(err, configstore.ErrNotFound) { | ||
|
|
@@ -1551,6 +1576,11 @@ func (h *GovernanceHandler) updateVirtualKey(ctx *fasthttp.RequestCtx) { | |
| if req.IsActive != nil { | ||
| vk.IsActive = req.IsActive | ||
| } | ||
| if req.ClearExpiresAt { | ||
| vk.ExpiresAt = nil | ||
| } else if req.ExpiresAt != nil { | ||
| vk.ExpiresAt = req.ExpiresAt | ||
| } | ||
| if req.CalendarAligned != nil { | ||
| vk.CalendarAligned = *req.CalendarAligned | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PreRequestHookshort-circuitThe condition previously returned early for inactive keys (
!virtualKey.IsActiveValue()); this PR replaces that with the expiry check but doesn't preserve the inactive check. As a result, inactive (but not expired) virtual keys now fall through the guard and enter the routing pipeline —stampGovernanceCtxFromVK, routing-rule evaluation,loadBalanceProvider, and MCP tool-allowlist computation all execute beforePreLLMHookeventually blocks the key viaEvaluateVirtualKeyRequest. Load-balancer counters and MCP context values can be mutated for requests that will never be served. Both checks should be in the condition: