Skip to content

feat: add is_access_profile_managed server-computed flag to VK for RBAC-independent managed-key UI - #5858

Merged
akshaydeo merged 1 commit into
mainfrom
08-05-fix_show_managed_info_on_a_vk_if_its_managed
Aug 5, 2026
Merged

akshaydeo merged 1 commit into
mainfrom
08-05-fix_show_managed_info_on_a_vk_if_its_managed

Conversation

@BearTS

@BearTS BearTS commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Users without access-profile RBAC permissions were not seeing the managed-key lock and notice on virtual keys governed by an access profile. Because the managed state was derived solely from the client-side access-profile lookup (which is RBAC-gated), users lacking that permission saw the key as unmanaged and could attempt edits that should be locked.

This PR introduces a server-computed is_access_profile_managed flag on the VirtualKey response that is populated during the governance read path, independent of any access-profile permission check. The UI uses this flag as the authoritative source for "is this key managed," falling back to the existing profile lookup only for resolving the profile name and actions.

Changes

  • Added IsAccessProfileManaged bool to TableVirtualKey as a non-persisted, computed field (gorm:"-") that is set by applyExternalBudgets from the external resolver result.
  • Added a Managed bool field to ExternalQuotaBudgetResult so resolvers can signal managed status independently of whether the profile carries any budget or rate-limit rows.
  • applyExternalBudgets now sets vk.IsAccessProfileManaged = ext.Managed before overwriting budgets and rate limit, ensuring the flag is present even when the profile has neither.
  • Updated useVirtualKeyUsage so isManagedByProfile is true when either the server flag is set or the client-side profile lookup finds a match. This means users without access-profile permissions still see the lock and notice.
  • Fixed a potential nil-dereference in virtualKeyDetailsSheet where managingProfile.user_id was accessed without first checking that managingProfile is non-nil.
  • ManagedVirtualKeyNotice now receives isManagedByProfile directly so it renders correctly even when managingProfile is undefined.
  • Added is_access_profile_managed?: boolean to the VirtualKey TypeScript interface.
  • Added a new test TestApplyExternalBudgets_ManagedWithNoGovernanceFlagsAndClears verifying that a resolver-managed VK with no AP budget and no AP rate limit is still flagged managed and has its mirror rows cleared.
  • Updated the existing TestApplyExternalBudgets_RateLimitOnlyDropsNativeBudgets test to assert IsAccessProfileManaged=true.

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

  1. Configure a virtual key governed by an access profile.
  2. Log in as a user who does not have RBAC permission to list access profiles.
  3. Navigate to the virtual key detail sheet — the managed-key notice and edit lock should be visible.
  4. Log in as a user who does have access-profile permissions and confirm the profile name still appears alongside the notice.
# Core/Transports
go test ./transports/bifrost-http/handlers/... -run TestApplyExternalBudgets

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build

Breaking changes

  • Yes
  • No

Security considerations

The is_access_profile_managed flag is read-only and never persisted (gorm:"-"). It is populated only on governance read paths and carries no sensitive data — it is a boolean indicator used solely to drive UI state. No auth or secret handling is affected.

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 Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Virtual key details now indicate when a key is managed by an access profile.
    • Managed status remains available when access-profile information is restricted.
  • Bug Fixes

    • Prevented invalid user-detail lookups when a managing profile lacks a user identifier.
    • Cleared outdated native quota and rate-limit data when external governance is unavailable.
    • Improved handling of externally governed virtual keys when governance details are unavailable.

Walkthrough

The change adds a server-computed managed-status flag for virtual keys. Governance processing populates the flag, and the UI uses it when access-profile data is unavailable. Details rendering now requires a defined managing-profile user ID.

Changes

Managed virtual key status

Layer / File(s) Summary
Backend status propagation and coverage
framework/configstore/tables/virtualkey.go, transports/bifrost-http/handlers/governance.go, transports/bifrost-http/handlers/governance_test.go
TableVirtualKey and ExternalQuotaBudgetResult expose managed status. applyExternalBudgets copies the resolver value. Tests cover managed keys without native budgets or rate limits.
Frontend status consumption and rendering
ui/lib/types/governance.ts, ui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.ts, ui/app/_fallbacks/enterprise/components/access-profiles/managedVirtualKeyNotice.tsx, ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx
The UI type includes the server flag. Managed-state detection uses the flag with the resolved profile fallback. The notice receives the managed-state prop. User details rendering requires a defined user_id.

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

Sequence Diagram(s)

sequenceDiagram
  participant ExternalQuotaResolver
  participant applyExternalBudgets
  participant VirtualKeyAPI
  participant useVirtualKeyUsage
  participant ManagedVirtualKeyNotice

  ExternalQuotaResolver->>applyExternalBudgets: returns managed quota result
  applyExternalBudgets->>VirtualKeyAPI: sets is_access_profile_managed
  VirtualKeyAPI->>useVirtualKeyUsage: provides managed-status flag
  useVirtualKeyUsage->>ManagedVirtualKeyNotice: passes isManagedByProfile
Loading

Possibly related PRs

Suggested reviewers: akshaydeo, pratham-mishra04, impoiler

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.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
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 clearly identifies the server-computed flag and its purpose for RBAC-independent managed-key UI behavior.
Description check ✅ Passed The description covers the purpose, changes, affected areas, testing, breaking changes, security, and checklist, but omits screenshots and related issues.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 08-05-fix_show_managed_info_on_a_vk_if_its_managed

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

BearTS commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@BearTS
BearTS marked this pull request as ready for review August 5, 2026 06:11
@BearTS BearTS changed the title fix: show managed info on a vk if its managed feat: add is_access_profile_managed server-computed flag to VK for RBAC-independent managed-key UI Aug 5, 2026

@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: 2

🤖 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/governance.go`:
- Around line 121-124: Update the two remaining managed fixtures in governance
tests around the managed response fixture definitions to explicitly set Managed:
true, ensuring the fixtures at the referenced test cases represent
access-profile-managed VKs.
- Line 956: Update the mutation and customer response paths that serialize
VirtualKeys to call applyExternalBudgets before serialization, matching the
existing list and detail hydration behavior so managed keys retain
IsAccessProfileManaged. Add response tests covering both mutation and customer
VirtualKeys paths.
🪄 Autofix

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: fbe25fbf-a4d5-43af-ba24-ccf5c4cbcb06

📥 Commits

Reviewing files that changed from the base of the PR and between 06d3859 and 167ffbb.

📒 Files selected for processing (6)
  • framework/configstore/tables/virtualkey.go
  • transports/bifrost-http/handlers/governance.go
  • transports/bifrost-http/handlers/governance_test.go
  • ui/app/workspace/virtual-keys/hooks/useVirtualKeyUsage.ts
  • ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx
  • ui/lib/types/governance.ts

Comment thread transports/bifrost-http/handlers/governance.go
Comment thread transports/bifrost-http/handlers/governance.go
@BearTS
BearTS force-pushed the 08-05-fix_show_externalmanaged_rate_limits_and_budgets_if_the_vk_is_externally_managed branch from 06d3859 to e326687 Compare August 5, 2026 06:21
@BearTS
BearTS force-pushed the 08-05-fix_show_managed_info_on_a_vk_if_its_managed branch from 167ffbb to ff6edb4 Compare August 5, 2026 06:21
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@BearTS
BearTS force-pushed the 08-05-fix_show_managed_info_on_a_vk_if_its_managed branch from ff6edb4 to 3bfa186 Compare August 5, 2026 06:24
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 5, 2026
@akshaydeo
akshaydeo changed the base branch from 08-05-fix_show_externalmanaged_rate_limits_and_budgets_if_the_vk_is_externally_managed to graphite-base/5858 August 5, 2026 06:54
@akshaydeo
akshaydeo force-pushed the graphite-base/5858 branch from e326687 to 093fdab Compare August 5, 2026 06:55
@akshaydeo
akshaydeo force-pushed the 08-05-fix_show_managed_info_on_a_vk_if_its_managed branch from 3bfa186 to ccb56a0 Compare August 5, 2026 06:55
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@graphite-app
graphite-app Bot changed the base branch from graphite-base/5858 to main August 5, 2026 06:55
@graphite-app
graphite-app Bot dismissed coderabbitai[bot]’s stale review August 5, 2026 06:55

The base branch was changed.

@akshaydeo
akshaydeo force-pushed the 08-05-fix_show_managed_info_on_a_vk_if_its_managed branch from ccb56a0 to 37a4cc3 Compare August 5, 2026 06:55
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

akshaydeo commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Aug 5, 6:57 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 5, 6:57 AM UTC: @akshaydeo merged this pull request with Graphite.

@coderabbitai
coderabbitai Bot requested a review from impoiler August 5, 2026 06:57
@akshaydeo
akshaydeo merged commit 4ec7341 into main Aug 5, 2026
14 of 15 checks passed
@akshaydeo
akshaydeo deleted the 08-05-fix_show_managed_info_on_a_vk_if_its_managed branch August 5, 2026 06:57
atharvamhaske pushed a commit to atharvamhaske/bifrost that referenced this pull request Aug 13, 2026
…RBAC-independent managed-key UI (maximhq#5858)

## Summary

Users without access-profile RBAC permissions were not seeing the managed-key lock and notice on virtual keys governed by an access profile. Because the managed state was derived solely from the client-side access-profile lookup (which is RBAC-gated), users lacking that permission saw the key as unmanaged and could attempt edits that should be locked.

This PR introduces a server-computed `is_access_profile_managed` flag on the `VirtualKey` response that is populated during the governance read path, independent of any access-profile permission check. The UI uses this flag as the authoritative source for "is this key managed," falling back to the existing profile lookup only for resolving the profile name and actions.

## Changes

- Added `IsAccessProfileManaged bool` to `TableVirtualKey` as a non-persisted, computed field (`gorm:"-"`) that is set by `applyExternalBudgets` from the external resolver result.
- Added a `Managed bool` field to `ExternalQuotaBudgetResult` so resolvers can signal managed status independently of whether the profile carries any budget or rate-limit rows.
- `applyExternalBudgets` now sets `vk.IsAccessProfileManaged = ext.Managed` before overwriting budgets and rate limit, ensuring the flag is present even when the profile has neither.
- Updated `useVirtualKeyUsage` so `isManagedByProfile` is `true` when either the server flag is set or the client-side profile lookup finds a match. This means users without access-profile permissions still see the lock and notice.
- Fixed a potential nil-dereference in `virtualKeyDetailsSheet` where `managingProfile.user_id` was accessed without first checking that `managingProfile` is non-nil.
- `ManagedVirtualKeyNotice` now receives `isManagedByProfile` directly so it renders correctly even when `managingProfile` is undefined.
- Added `is_access_profile_managed?: boolean` to the `VirtualKey` TypeScript interface.
- Added a new test `TestApplyExternalBudgets_ManagedWithNoGovernanceFlagsAndClears` verifying that a resolver-managed VK with no AP budget and no AP rate limit is still flagged managed and has its mirror rows cleared.
- Updated the existing `TestApplyExternalBudgets_RateLimitOnlyDropsNativeBudgets` test to assert `IsAccessProfileManaged=true`.

## Type of change

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

## Affected areas

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

## How to test

1. Configure a virtual key governed by an access profile.
2. Log in as a user who does **not** have RBAC permission to list access profiles.
3. Navigate to the virtual key detail sheet — the managed-key notice and edit lock should be visible.
4. Log in as a user who **does** have access-profile permissions and confirm the profile name still appears alongside the notice.

```sh
# Core/Transports
go test ./transports/bifrost-http/handlers/... -run TestApplyExternalBudgets

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

The `is_access_profile_managed` flag is read-only and never persisted (`gorm:"-"`). It is populated only on governance read paths and carries no sensitive data — it is a boolean indicator used solely to drive UI state. No auth or secret handling is affected.

## Checklist

- [ ] 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)
- [ ] I verified the CI pipeline passes locally if applicable
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
…RBAC-independent managed-key UI (maximhq#5858)

## Summary

Users without access-profile RBAC permissions were not seeing the managed-key lock and notice on virtual keys governed by an access profile. Because the managed state was derived solely from the client-side access-profile lookup (which is RBAC-gated), users lacking that permission saw the key as unmanaged and could attempt edits that should be locked.

This PR introduces a server-computed `is_access_profile_managed` flag on the `VirtualKey` response that is populated during the governance read path, independent of any access-profile permission check. The UI uses this flag as the authoritative source for "is this key managed," falling back to the existing profile lookup only for resolving the profile name and actions.

## Changes

- Added `IsAccessProfileManaged bool` to `TableVirtualKey` as a non-persisted, computed field (`gorm:"-"`) that is set by `applyExternalBudgets` from the external resolver result.
- Added a `Managed bool` field to `ExternalQuotaBudgetResult` so resolvers can signal managed status independently of whether the profile carries any budget or rate-limit rows.
- `applyExternalBudgets` now sets `vk.IsAccessProfileManaged = ext.Managed` before overwriting budgets and rate limit, ensuring the flag is present even when the profile has neither.
- Updated `useVirtualKeyUsage` so `isManagedByProfile` is `true` when either the server flag is set or the client-side profile lookup finds a match. This means users without access-profile permissions still see the lock and notice.
- Fixed a potential nil-dereference in `virtualKeyDetailsSheet` where `managingProfile.user_id` was accessed without first checking that `managingProfile` is non-nil.
- `ManagedVirtualKeyNotice` now receives `isManagedByProfile` directly so it renders correctly even when `managingProfile` is undefined.
- Added `is_access_profile_managed?: boolean` to the `VirtualKey` TypeScript interface.
- Added a new test `TestApplyExternalBudgets_ManagedWithNoGovernanceFlagsAndClears` verifying that a resolver-managed VK with no AP budget and no AP rate limit is still flagged managed and has its mirror rows cleared.
- Updated the existing `TestApplyExternalBudgets_RateLimitOnlyDropsNativeBudgets` test to assert `IsAccessProfileManaged=true`.

## Type of change

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

## Affected areas

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

## How to test

1. Configure a virtual key governed by an access profile.
2. Log in as a user who does **not** have RBAC permission to list access profiles.
3. Navigate to the virtual key detail sheet — the managed-key notice and edit lock should be visible.
4. Log in as a user who **does** have access-profile permissions and confirm the profile name still appears alongside the notice.

```sh
# Core/Transports
go test ./transports/bifrost-http/handlers/... -run TestApplyExternalBudgets

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

The `is_access_profile_managed` flag is read-only and never persisted (`gorm:"-"`). It is populated only on governance read paths and carries no sensitive data — it is a boolean indicator used solely to drive UI state. No auth or secret handling is affected.

## Checklist

- [ ] 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)
- [ ] I verified the CI pipeline passes locally if applicable
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
…RBAC-independent managed-key UI (maximhq#5858)

## Summary

Users without access-profile RBAC permissions were not seeing the managed-key lock and notice on virtual keys governed by an access profile. Because the managed state was derived solely from the client-side access-profile lookup (which is RBAC-gated), users lacking that permission saw the key as unmanaged and could attempt edits that should be locked.

This PR introduces a server-computed `is_access_profile_managed` flag on the `VirtualKey` response that is populated during the governance read path, independent of any access-profile permission check. The UI uses this flag as the authoritative source for "is this key managed," falling back to the existing profile lookup only for resolving the profile name and actions.

## Changes

- Added `IsAccessProfileManaged bool` to `TableVirtualKey` as a non-persisted, computed field (`gorm:"-"`) that is set by `applyExternalBudgets` from the external resolver result.
- Added a `Managed bool` field to `ExternalQuotaBudgetResult` so resolvers can signal managed status independently of whether the profile carries any budget or rate-limit rows.
- `applyExternalBudgets` now sets `vk.IsAccessProfileManaged = ext.Managed` before overwriting budgets and rate limit, ensuring the flag is present even when the profile has neither.
- Updated `useVirtualKeyUsage` so `isManagedByProfile` is `true` when either the server flag is set or the client-side profile lookup finds a match. This means users without access-profile permissions still see the lock and notice.
- Fixed a potential nil-dereference in `virtualKeyDetailsSheet` where `managingProfile.user_id` was accessed without first checking that `managingProfile` is non-nil.
- `ManagedVirtualKeyNotice` now receives `isManagedByProfile` directly so it renders correctly even when `managingProfile` is undefined.
- Added `is_access_profile_managed?: boolean` to the `VirtualKey` TypeScript interface.
- Added a new test `TestApplyExternalBudgets_ManagedWithNoGovernanceFlagsAndClears` verifying that a resolver-managed VK with no AP budget and no AP rate limit is still flagged managed and has its mirror rows cleared.
- Updated the existing `TestApplyExternalBudgets_RateLimitOnlyDropsNativeBudgets` test to assert `IsAccessProfileManaged=true`.

## Type of change

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

## Affected areas

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

## How to test

1. Configure a virtual key governed by an access profile.
2. Log in as a user who does **not** have RBAC permission to list access profiles.
3. Navigate to the virtual key detail sheet — the managed-key notice and edit lock should be visible.
4. Log in as a user who **does** have access-profile permissions and confirm the profile name still appears alongside the notice.

```sh
# Core/Transports
go test ./transports/bifrost-http/handlers/... -run TestApplyExternalBudgets

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

The `is_access_profile_managed` flag is read-only and never persisted (`gorm:"-"`). It is populated only on governance read paths and carries no sensitive data — it is a boolean indicator used solely to drive UI state. No auth or secret handling is affected.

## Checklist

- [ ] 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)
- [ ] 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.

2 participants