From cf409a7018408dc903c136dcfd6b2df0f23f2613 Mon Sep 17 00:00:00 2001 From: blankll Date: Tue, 27 Oct 2026 01:52:08 +0800 Subject: [PATCH 1/8] fix: restore consistent AI permission controls and gate per-connection editing - access levels use the shared radio pattern with a short scope description inside each option; Custom edits the same values as the presets instead of forming a separate layer - connection-level custom actions expose Use default (Allow/Deny); system defaults use switches for binary actions - MCP service settings and shared AI defaults live in separate cards - per-connection permission editing requires the local Ultimate entitlement: inline upgrade prompt, draft stripped on save, and save_ai_permissions enforced in Rust with a cleanup carve-out that only clears decisions of connections that no longer exist --- DESIGN.md | 2 +- docs/AI_PERMISSIONS.md | 8 + src-tauri/src/ai_permissions/mod.rs | 156 ++++++++++++++++++ src/__tests__/aiPermissions.test.ts | 112 +++++++++++-- .../ai-permissions/AccessLevelPicker.vue | 71 ++++++++ .../ConnectionPermissionEditor.vue | 86 ++++++---- .../DefaultPermissionEditor.vue | 89 ++++------ src/lang/enUS.ts | 16 +- src/lang/zhCN.ts | 14 +- src/store/connectionStore.ts | 7 +- src/views/setting/mcp-bridge.vue | 96 +++++------ tests/store/connectionStore.test.ts | 20 +++ 12 files changed, 518 insertions(+), 159 deletions(-) create mode 100644 src/components/ai-permissions/AccessLevelPicker.vue diff --git a/DESIGN.md b/DESIGN.md index 966c699d..a4c70b9a 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -331,4 +331,4 @@ Check each affected surface. Documentation-only changes need source, link, and i ### AI permission controls -Connection dialogs expose a collapsed AI permissions section shared by Data Studio and external MCP. Default to system inheritance, show effective allowed actions, and reveal per-action Inherit / Allow / Deny under Customize. The system settings page edits action defaults, without a separate connection allowlist or MCP exposure switch. Preserve shared Select, Label, Button, neutral surfaces, localized English/Chinese copy, keyboard focus, and loading/error/retry states. See [AI permissions](docs/AI_PERMISSIONS.md) for enforcement and migration behavior. +Per-connection permission editing uses the existing local Ultimate gate and inline upgrade prompt; Community connection edits preserve saved permission decisions. Connection dialogs expose a collapsed AI permissions section shared by Data Studio and external MCP. Default to system inheritance, show effective allowed actions, and offer visible radio choices for access levels, including Custom. Each option contains a short description of its scope. Custom reveals per-action Use default / Allow / Deny controls; each inherited option names its current decision. Presets and custom editing change the same permission values and do not form separate policy layers. The system settings page edits action defaults, without a separate connection allowlist or MCP exposure switch. Separate shared AI defaults from MCP service settings using the existing settings cards. Use switches for binary system defaults, and preserve shared RadioGroup, Select, Label, Button, neutral surfaces, localized English/Chinese copy, keyboard focus, and loading/error/retry states. See [AI permissions](docs/AI_PERMISSIONS.md) for enforcement and migration behavior. diff --git a/docs/AI_PERMISSIONS.md b/docs/AI_PERMISSIONS.md index 9aa85622..a25a6bf1 100644 --- a/docs/AI_PERMISSIONS.md +++ b/docs/AI_PERMISSIONS.md @@ -23,3 +23,11 @@ Agent executors are bound to a conversation and its connection/source identities Preparation rejects denied calls before displaying confirmation. A queued call whose confirmation requirements become stricter returns to the existing approval flow. Permission denials are terminal and do not trigger transport retries; approval never bypasses a later denial. Rechecks occur at dispatch and do not cancel operations already dispatched or erase context already sent to a model. The host-hook API is introduced by [data-studio-agent #35](https://github.com/geek-fun/data-studio-agent/pull/35). Merge that runtime implementation before merging its consumers. Keep the exact compatible revision until a release containing these hooks exists; after the runtime is merged and released, update both the manifest and lockfile to that release. Older release tags do not provide this API. + +## Permission controls + +Settings separate MCP service controls from the AI defaults shared by Data Studio and external MCP clients. Access levels use the existing settings radio pattern, with a short description inside each option. Custom is an access choice that reveals individual actions; it edits the same values as the presets. Choosing Custom alone does not change permissions. + +Connection dialogs default to system inheritance. Each custom action offers Use default (with the current decision), Allow, or Deny, alongside its effective decision and source. Within the connection section, saved custom combinations show their action controls automatically. Choosing a preset sets explicit decisions for every action; choosing Use system default clears them. + +Editing AI permission defaults and per-connection decisions requires the existing local Ultimate entitlement, including eligible version-locked releases. Community users see the existing upgrade prompt in connection dialogs. Ordinary connection edits preserve saved decisions. Permission cleanup after deletion remains available only when the connection no longer exists; it cannot reset the permissions of a saved connection. diff --git a/src-tauri/src/ai_permissions/mod.rs b/src-tauri/src/ai_permissions/mod.rs index 51898c8c..de60ca52 100644 --- a/src-tauri/src/ai_permissions/mod.rs +++ b/src-tauri/src/ai_permissions/mod.rs @@ -67,6 +67,41 @@ pub fn get_ai_permissions(app: AppHandle) -> Result { load_permission_policy(&app) } +fn authorize_permission_change( + access: Result<(), String>, + defaults: &Option, + connection_id: Option<&str>, + permissions: &Option, + saved_connections: impl FnOnce() -> Result, String>, +) -> Result<(), String> { + if access.is_ok() + || defaults.is_some() + || permissions + .as_ref() + .is_some_and(|rules| *rules != PermissionRules::default()) + { + return access; + } + let Some(id) = connection_id else { + return access; + }; + match saved_connections()? { + None => Ok(()), + Some(Value::Array(connections)) + if connections + .iter() + .all(|connection| match connection.get("id") { + Some(Value::String(saved)) => saved != id, + Some(Value::Number(saved)) => saved.to_string() != id, + _ => false, + }) => + { + Ok(()) + } + _ => access, + } +} + #[tauri::command] pub fn save_ai_permissions( defaults: Option, @@ -74,6 +109,19 @@ pub fn save_ai_permissions( permissions: Option, app: AppHandle, ) -> Result { + authorize_permission_change( + crate::entitlement::ensure_local_ultimate( + &app.state::(), + "AI permissions", + ), + &defaults, + connection_id.as_deref(), + &permissions, + || { + let store = app.store(".store.dat").map_err(|e| e.to_string())?; + Ok(store.get("connections")) + }, + )?; let _guard = AI_POLICY_LOCK.lock().map_err(|e| e.to_string())?; let mut policy = load_permission_policy_inner(&app)?; if let Some(defaults) = defaults { @@ -93,3 +141,111 @@ pub fn save_ai_permissions( config.save(&dir)?; Ok(policy) } + +#[cfg(test)] +mod entitlement_tests { + use super::*; + use serde_json::json; + + fn denied() -> Result<(), String> { + Err(crate::entitlement::entitlement_required_error( + "AI permissions", + )) + } + + #[test] + fn ultimate_can_edit_defaults_and_connection_decisions_without_cleanup_lookup() { + assert!(authorize_permission_change( + Ok(()), + &Some(PermissionRules::default()), + Some("saved"), + &Some(PermissionRules::explicit(true, false, false, false, false)), + || panic!("editing must not use the cleanup path"), + ) + .is_ok()); + } + + #[test] + fn community_cannot_edit_defaults_or_explicit_connection_decisions() { + assert!(authorize_permission_change( + denied(), + &Some(PermissionRules::default()), + None, + &None, + || panic!("denied defaults must not inspect storage"), + ) + .is_err()); + assert!(authorize_permission_change( + denied(), + &None, + Some("saved"), + &Some(PermissionRules::explicit(false, false, false, false, false)), + || panic!("explicit denials are permission edits, not cleanup"), + ) + .is_err()); + assert!(authorize_permission_change( + denied(), + &None, + Some("new"), + &Some(PermissionRules::explicit(true, false, false, false, false)), + || panic!("new connection permissions also require Ultimate"), + ) + .is_err()); + } + + #[test] + fn community_cannot_reset_a_saved_connections_permissions() { + for (id, connections) in [ + ("saved", json!([{"id":"saved"}])), + ("42", json!([{"id":42}])), + ] { + let error = authorize_permission_change( + denied(), + &None, + Some(id), + &Some(PermissionRules::default()), + || Ok(Some(connections)), + ) + .unwrap_err(); + assert!(error.contains("ENTITLEMENT_REQUIRED")); + } + } + + #[test] + fn community_can_only_cleanup_decisions_for_deleted_connections() { + assert!(authorize_permission_change( + denied(), + &None, + Some("deleted"), + &Some(PermissionRules::default()), + || Ok(Some(json!([{"id":"retained"}]))), + ) + .is_ok()); + assert!( + authorize_permission_change(denied(), &None, Some("last-deleted"), &None, || Ok(None),) + .is_ok() + ); + } + + #[test] + fn unreadable_or_malformed_connections_do_not_authorize_cleanup() { + for connections in [ + json!({}), + json!([{"name":"missing-id"}]), + json!([{"id":null}]), + ] { + assert!( + authorize_permission_change(denied(), &None, Some("saved"), &None, || Ok(Some( + connections + )),) + .is_err() + ); + } + assert!( + authorize_permission_change(denied(), &None, Some("saved"), &None, || Err( + "Storage unavailable".into() + ),) + .is_err() + ); + } +} diff --git a/src/__tests__/aiPermissions.test.ts b/src/__tests__/aiPermissions.test.ts index af4f6e6a..30d1319d 100644 --- a/src/__tests__/aiPermissions.test.ts +++ b/src/__tests__/aiPermissions.test.ts @@ -1,17 +1,29 @@ /** @jest-environment jsdom */ import { invoke } from '@tauri-apps/api/core' +import { openUrl } from '@tauri-apps/plugin-opener' import { flushPromises, mount } from '@vue/test-utils' -import { defineComponent } from 'vue' +import { defineComponent, reactive } from 'vue' import { AI_ACTIONS, aiPreset, resolveAiPermissions } from '@/common/aiPermissions' import ConnectionPermissionEditor from '@/components/ai-permissions/ConnectionPermissionEditor.vue' import DefaultPermissionEditor from '@/components/ai-permissions/DefaultPermissionEditor.vue' import SourcePermissionModal from '@/views/data-studio/components/modify-source-modal.vue' jest.mock('@tauri-apps/api/core', () => ({ invoke: jest.fn() })) -jest.mock('@/common', () => jest.requireActual('@/common/aiPermissions')) +jest.mock('@/common', () => ({ ...jest.requireActual('@/common/aiPermissions'), UPGRADE_URL: 'https://example.test/upgrade' })) +const mockEntitlement = reactive({ isLocalUltimate: true }) +jest.mock('@/store', () => ({ useEntitlementStore: () => mockEntitlement })) +jest.mock('@tauri-apps/plugin-opener', () => ({ openUrl: jest.fn() })) +jest.mock('@/components/upgrade', () => ({ ProBadge: { template: 'Ultimate' } })) jest.mock('vue-i18n', () => ({ useI18n: () => ({ t: (key: string) => key }) })) jest.mock('@/components/ui/button', () => ({ Button: { template: '' } })) jest.mock('@/components/ui/label', () => ({ Label: { template: '' } })) +jest.mock('@/components/ui/radio-group', () => ({ + RadioGroup: { props: ['modelValue', 'disabled'], emits: ['update:modelValue'], template: '
' }, + RadioGroupItem: { props: ['value'], template: '' }, +})) +jest.mock('@/components/ui/switch', () => ({ + Switch: { props: ['checked', 'disabled'], emits: ['update:checked'], template: '' }, +})) jest.mock('@/components/ui/select', () => ({ Select: { props: ['modelValue'], emits: ['update:modelValue'], template: '' }, SelectTrigger: { template: '' }, @@ -57,12 +69,44 @@ const openEditor = async (id?: string) => { describe('aI permission chain', () => { beforeEach(() => { + mockEntitlement.isLocalUltimate = true mockStore.updateSessionSourcePermissions.mockReset() mockStore.detachSourceFromSession.mockReset() mockInvoke.mockReset() mockInvoke.mockResolvedValue(policy) }) + it('gates Community users without loading permissions or blocking ordinary connection saves', async () => { + mockEntitlement.isLocalUltimate = false + const wrapper = mount(ConnectionPermissionEditor, { props: { connectionId: 'dev' } }) + await flushPromises() + expect(mockInvoke).not.toHaveBeenCalled() + expect(wrapper.find('details').exists()).toBe(false) + expect(wrapper.text()).toContain('aiPermissions.ultimateOnly') + expect((wrapper.vm as unknown as { ready: boolean }).ready).toBe(true) + await wrapper.get('button').trigger('click') + expect(openUrl).toHaveBeenCalledWith('https://example.test/upgrade') + mockEntitlement.isLocalUltimate = true + await flushPromises() + expect(mockInvoke).toHaveBeenCalledWith('get_ai_permissions') + expect(wrapper.find('details').exists()).toBe(true) + }) + + it('discards an in-flight permission load when Ultimate access is lost', async () => { + let finishLoad!: (value: typeof policy) => void + mockInvoke.mockImplementationOnce(() => new Promise((resolve) => { + finishLoad = resolve + })) + const wrapper = mount(ConnectionPermissionEditor, { props: { connectionId: 'dev' } }) + mockEntitlement.isLocalUltimate = false + await flushPromises() + finishLoad(policy) + await flushPromises() + expect(wrapper.emitted('update:modelValue')).toBeUndefined() + expect(wrapper.find('details').exists()).toBe(false) + expect((wrapper.vm as unknown as { ready: boolean }).ready).toBe(true) + }) + it('resolves each action with explicit false distinct from inheritance', () => { expect(resolveAiPermissions(aiPreset('full'), { update: false })).toEqual({ read: true, create: true, update: false, delete: true, manage: true }) expect(resolveAiPermissions({}, { read: true })).toEqual(aiPreset('read')) @@ -70,19 +114,28 @@ describe('aI permission chain', () => { expect(AI_ACTIONS.every(action => aiPreset('none')[action] === false)).toBe(true) }) + it('describes every access level inside its own selectable container', async () => { + const wrapper = mount(DefaultPermissionEditor) + await flushPromises() + ;['none', 'read', 'data', 'full', 'custom'].forEach((value) => { + const option = wrapper.get(`label[for="ai-default-preset-${value}"]`) + expect(option.text()).toContain(`aiPermissions.${value}Help`) + expect(option.get('input').attributes('aria-describedby')).toBe(`ai-default-preset-${value}-description`) + }) + }) + it('loads saved decisions and displays effective access from the whole chain', async () => { const wrapper = await openEditor('dev') expect(wrapper.vm.rules).toEqual({ update: true, delete: false }) expect(wrapper.text()).toContain('aiPermissions.effective: aiPermissions.read, aiPermissions.update') - await wrapper.get('button').trigger('click') + expect(wrapper.findAll('select')).toHaveLength(5) expect(wrapper.text()).toContain('aiPermissions.fromConnection') expect(wrapper.text()).toContain('aiPermissions.fromSystem') }) it('supports inherit, explicit allow and explicit deny independently', async () => { const wrapper = await openEditor('dev') - await wrapper.get('button').trigger('click') - const actionSelect = () => wrapper.findAll('select')[3] + const actionSelect = () => wrapper.findAll('select')[2] await actionSelect().setValue('deny') expect(wrapper.vm.rules).toEqual({ update: false, delete: false }) await actionSelect().setValue('inherit') @@ -93,20 +146,55 @@ describe('aI permission chain', () => { it('keeps custom decisions when opening the custom preset', async () => { const wrapper = await openEditor('dev') - await wrapper.get('select').setValue('custom') + await wrapper.get('input[value="custom"]').setValue() expect(wrapper.vm.rules).toEqual({ update: true, delete: false }) - expect(wrapper.findAll('select')).toHaveLength(6) + expect(wrapper.findAll('select')).toHaveLength(5) }) it('new connections inherit and presets preserve explicit decisions', async () => { const wrapper = await openEditor() expect(wrapper.vm.rules).toEqual({}) - await wrapper.get('select').setValue('none') + await wrapper.get('input[value="none"]').setValue() expect(wrapper.vm.rules).toEqual(aiPreset('none')) - await wrapper.get('select').setValue('inherit') + await wrapper.get('input[value="inherit"]').setValue() expect(wrapper.vm.rules).toEqual({}) }) + it('offers Custom from inherited access without changing stored decisions', async () => { + const wrapper = await openEditor() + await wrapper.get('input[value="custom"]').setValue() + expect(wrapper.vm.rules).toEqual({}) + expect(wrapper.findAll('select')).toHaveLength(5) + await wrapper.findAll('select')[1].setValue('allow') + expect(wrapper.vm.rules).toEqual({ create: true }) + await wrapper.get('input[value="inherit"]').setValue() + expect(wrapper.vm.rules).toEqual({}) + expect(wrapper.findAll('select')).toHaveLength(0) + }) + + it('shows custom defaults on load and keeps action editing open when values match a preset', async () => { + mockInvoke.mockResolvedValueOnce({ permission_defaults: { ...aiPreset('read'), create: true }, connection_permissions: {} }) + const wrapper = mount(DefaultPermissionEditor) + await flushPromises() + expect(wrapper.findAll('input[type="checkbox"]')).toHaveLength(5) + await wrapper.get('#ai-default-create').setValue(false) + expect(wrapper.get('[data-value]').attributes('data-value')).toBe('custom') + expect(wrapper.findAll('input[type="checkbox"]')).toHaveLength(5) + await wrapper.get('button').trigger('click') + await flushPromises() + expect(mockInvoke).toHaveBeenLastCalledWith('save_ai_permissions', { defaults: aiPreset('read') }) + }) + + it('entering Custom does not change default permissions or introduce a separate policy', async () => { + const wrapper = mount(DefaultPermissionEditor) + await flushPromises() + await wrapper.get('input[value="custom"]').setValue() + expect(wrapper.findAll('input[type="checkbox"]')).toHaveLength(5) + await wrapper.get('button').trigger('click') + await flushPromises() + expect(mockInvoke).toHaveBeenLastCalledWith('save_ai_permissions', { defaults: aiPreset('read') }) + }) + it('blocks readiness after load failure and recovers through Retry', async () => { mockInvoke.mockRejectedValueOnce(new Error('Unavailable')) const wrapper = await openEditor('dev') @@ -121,11 +209,11 @@ describe('aI permission chain', () => { it('retries failed default saves without discarding the edited permissions', async () => { const wrapper = mount(DefaultPermissionEditor) await flushPromises() - await wrapper.get('select').setValue('full') + await wrapper.get('input[value="full"]').setValue() mockInvoke.mockRejectedValueOnce(new Error('Temporarily unavailable')) await wrapper.findAll('button').find(button => button.text() === 'aiPermissions.save')!.trigger('click') await flushPromises() - expect(wrapper.get('select').element.value).toBe('full') + expect(wrapper.get('[data-value]').attributes('data-value')).toBe('full') await wrapper.findAll('button').find(button => button.text() === 'aiPermissions.retry')!.trigger('click') await flushPromises() expect(mockInvoke).toHaveBeenLastCalledWith('save_ai_permissions', { defaults: aiPreset('full') }) @@ -162,7 +250,7 @@ describe('aI permission chain', () => { it('saves system defaults without overwriting connection decisions', async () => { const wrapper = mount(DefaultPermissionEditor) await flushPromises() - await wrapper.get('select').setValue('none') + await wrapper.get('input[value="none"]').setValue() await wrapper.findAll('button').find(button => button.text() === 'aiPermissions.save')!.trigger('click') await flushPromises() expect(mockInvoke).toHaveBeenLastCalledWith('save_ai_permissions', { defaults: aiPreset('none') }) diff --git a/src/components/ai-permissions/AccessLevelPicker.vue b/src/components/ai-permissions/AccessLevelPicker.vue new file mode 100644 index 00000000..436841d7 --- /dev/null +++ b/src/components/ai-permissions/AccessLevelPicker.vue @@ -0,0 +1,71 @@ + + + + + diff --git a/src/components/ai-permissions/ConnectionPermissionEditor.vue b/src/components/ai-permissions/ConnectionPermissionEditor.vue index e3800590..267a0a4b 100644 --- a/src/components/ai-permissions/ConnectionPermissionEditor.vue +++ b/src/components/ai-permissions/ConnectionPermissionEditor.vue @@ -1,19 +1,25 @@