diff --git a/skill-guides/orca-cli/references/plugins.md b/skill-guides/orca-cli/references/plugins.md index d418c57b38a..e816a032acf 100644 --- a/skill-guides/orca-cli/references/plugins.md +++ b/skill-guides/orca-cli/references/plugins.md @@ -127,8 +127,8 @@ ORCA plugin uninstall acme.notes-panel --json a version tag to the newest version tag and from a branch to its newest commit; a commit pin stays. An update from the same repository that asks for no new access keeps its approval. One that adds capabilities, a higher execution level, commands, shortcuts or panels, or that comes from another -repository, fails with `approval_required` listing exactly what grows; the running version keeps -working until it is approved. `--all` updates everything it can and exits 3 with one +repository (even a theme or language pack), fails with `approval_required` listing exactly what +changes; the running version keeps working until it is approved. `--all` updates everything it can and exits 3 with one `--approve` command per plugin still waiting. `plugin uninstall` and `plugin catalog rm` never need approval. `plugin catalog add` always diff --git a/src/cli/bundled-skill-guides.ts b/src/cli/bundled-skill-guides.ts index 1522ceb6f59..168c08fc525 100644 --- a/src/cli/bundled-skill-guides.ts +++ b/src/cli/bundled-skill-guides.ts @@ -882,8 +882,8 @@ const ORCA_CLI_PLUGINS_REFERENCE_MARKDOWN = [ "a version tag to the newest version tag and from a branch to its newest commit; a commit pin stays.", "An update from the same repository that asks for no new access keeps its approval. One that adds", "capabilities, a higher execution level, commands, shortcuts or panels, or that comes from another", - "repository, fails with `approval_required` listing exactly what grows; the running version keeps", - "working until it is approved. `--all` updates everything it can and exits 3 with one", + "repository (even a theme or language pack), fails with `approval_required` listing exactly what", + "changes; the running version keeps working until it is approved. `--all` updates everything it can and exits 3 with one", "`--approve` command per plugin still waiting.", "", "`plugin uninstall` and `plugin catalog rm` never need approval. `plugin catalog add` always", diff --git a/src/main/plugins/catalog/plugin-marketplace-install-origin.ts b/src/main/plugins/catalog/plugin-marketplace-install-origin.ts index e128e613306..961f3cf9853 100644 --- a/src/main/plugins/catalog/plugin-marketplace-install-origin.ts +++ b/src/main/plugins/catalog/plugin-marketplace-install-origin.ts @@ -2,6 +2,7 @@ import type { PluginInstallSource, PluginLockEntry } from '../../../shared/plugins/installation/plugin-install-lockfile' +import { isSamePluginSource } from '../../../shared/plugins/catalog/plugin-marketplace' import { marketplaceSourceId } from './plugin-marketplace-store' type MarketplaceInstallSource = Extract @@ -18,3 +19,12 @@ export function marketplaceInstallOf(entry: PluginLockEntry | null): Marketplace plugin } } + +/** Whether a catalog source would replace a marketplace install that came from another repository. */ +export function marketplaceInstallMoved( + entry: PluginLockEntry | null, + source: { url: string; path?: string } +): boolean { + const installed = marketplaceInstallOf(entry) + return installed !== null && !isSamePluginSource(installed.plugin, source) +} diff --git a/src/main/plugins/catalog/plugin-marketplace-installer.test.ts b/src/main/plugins/catalog/plugin-marketplace-installer.test.ts index 5a5ca03a734..f644789a770 100644 --- a/src/main/plugins/catalog/plugin-marketplace-installer.test.ts +++ b/src/main/plugins/catalog/plugin-marketplace-installer.test.ts @@ -13,6 +13,8 @@ import type { PluginImageCache } from '../plugin-image-cache' import { PluginMarketplaceInstaller } from './plugin-marketplace-installer' import { PluginMarketplaceService } from './plugin-marketplace-service' +const revokeConsent = vi.fn(async (_pluginKey: string) => undefined) + const git = vi.hoisted( (): { checkout: ReturnType @@ -151,7 +153,8 @@ async function setup( marketplace: service, userDataPath: root, hostVersion: pluginHostVersion('1.4.0'), - ...(imageCache ? { imageCache } : {}) + ...(imageCache ? { imageCache } : {}), + revokeConsent }), sourceId: added.id } @@ -171,6 +174,7 @@ beforeEach(() => { git.icon = false git.paths = null telemetry.track.mockReset() + revokeConsent.mockClear() git.checkout.mockReset() git.checkout.mockImplementation( async ({ destination, path }: { destination: string; path?: string }) => { @@ -277,4 +281,59 @@ describe('PluginMarketplaceInstaller', () => { expect(result).toMatchObject({ ok: false, error: expect.stringContaining('removed') }) expect((await readPluginLockfile(pluginsDir)).plugins).toEqual({}) }) + + it('flags an update whose catalog moved the plugin to a different repository', async () => { + const { marketplace: service, installer, sourceId } = await setup() + await installer.install(await installer.preview(sourceId, 'community.notes')) + + git.pluginUrl = 'git@github.com:Community/Notes' + git.marketplaceCommit = 'e'.repeat(40) + await service.refreshSource(sourceId) + git.payload = 'respelled' + const respelledPreview = await installer.previewInstalledUpdate('community.notes') + expect(respelledPreview).not.toHaveProperty('repositoryMoved') + const respelled = await installer.install(respelledPreview) + expect(respelled).toMatchObject({ ok: true }) + expect(respelled).not.toHaveProperty('repositoryMoved') + expect(revokeConsent).not.toHaveBeenCalled() + + git.pluginUrl = 'https://github.com/someone-else/notes.git' + git.marketplaceCommit = 'd'.repeat(40) + await service.refreshSource(sourceId) + git.payload = 'moved' + const movedPreview = await installer.previewInstalledUpdate('community.notes') + expect(movedPreview).toMatchObject({ repositoryMoved: true }) + // Install links and catalog cards preview through `preview`, which must flag the move too. + await expect(installer.preview(sourceId, 'community.notes')).resolves.toMatchObject({ + repositoryMoved: true + }) + await expect(installer.install(movedPreview)).resolves.toMatchObject({ + ok: true, + repositoryMoved: true + }) + expect(revokeConsent).toHaveBeenCalledExactlyOnceWith('community.notes') + }) + + it('drops the approval before a moved repository publishes, and installs nothing if it cannot', async () => { + const { root, marketplace: service, installer, sourceId } = await setup() + await installer.install(await installer.preview(sourceId, 'community.notes')) + git.pluginUrl = 'https://github.com/someone-else/notes.git' + git.marketplaceCommit = 'd'.repeat(40) + await service.refreshSource(sourceId) + git.commit = 'b'.repeat(40) + const moved = await installer.preview(sourceId, 'community.notes') + const pluginsDir = join(root, 'plugins') + revokeConsent.mockImplementationOnce(async () => { + // The previous release must still be the published one when approval is dropped. + expect((await readPluginLockfile(pluginsDir)).plugins['community.notes']).toMatchObject({ + resolvedCommit: 'a'.repeat(40) + }) + throw new Error('settings unavailable') + }) + + await expect(installer.install(moved)).rejects.toThrow('settings unavailable') + expect((await readPluginLockfile(pluginsDir)).plugins['community.notes']).toMatchObject({ + resolvedCommit: 'a'.repeat(40) + }) + }) }) diff --git a/src/main/plugins/catalog/plugin-marketplace-installer.ts b/src/main/plugins/catalog/plugin-marketplace-installer.ts index 48416370912..46f0bcdc778 100644 --- a/src/main/plugins/catalog/plugin-marketplace-installer.ts +++ b/src/main/plugins/catalog/plugin-marketplace-installer.ts @@ -29,8 +29,11 @@ import type { PluginMarketplaceListing, PluginMarketplaceService } from './plugin-marketplace-service' -import { marketplaceInstallOf, type MarketplaceInstall } from './plugin-marketplace-install-origin' -import { isSamePluginSource } from '../../../shared/plugins/catalog/plugin-marketplace' +import { + marketplaceInstallMoved, + marketplaceInstallOf, + type MarketplaceInstall +} from './plugin-marketplace-install-origin' import type { PluginLockEntry, PluginLockfile @@ -59,7 +62,7 @@ export type PluginMarketplaceInstallPreview = { images?: PluginHostImages /** What each recipe file can run, read before the staging tree is deleted; null if unreadable. */ vmRecipeIdentities?: PluginVmRecipeIdentities - /** Update previews only: the catalog now points at a different repository than the install. */ + /** The catalog now points at a different repository than the installed copy came from. */ repositoryMoved?: true } @@ -77,6 +80,7 @@ export class PluginMarketplaceInstaller { private readonly hostVersion: string private readonly blockedPluginReason: (pluginKey: string) => string | null private readonly imageCache: PluginImageCache | null + private readonly revokeConsent: (pluginKey: string) => Promise constructor(options: { marketplace: PluginMarketplaceService @@ -84,8 +88,10 @@ export class PluginMarketplaceInstaller { hostVersion: string blockedPluginReason?: (pluginKey: string) => string | null imageCache?: PluginImageCache + revokeConsent: (pluginKey: string) => Promise }) { this.imageCache = options.imageCache ?? null + this.revokeConsent = options.revokeConsent this.marketplace = options.marketplace this.userDataPath = options.userDataPath this.hostVersion = options.hostVersion @@ -125,6 +131,8 @@ export class PluginMarketplaceInstaller { inspection.manifest.contributes.vmRecipes.length > 0 ? await readPluginVmRecipeIdentities(stagingDirectory, inspection.manifest) : undefined + // Why every preview, not only updates: an install link to an installed plugin replaces it too. + const moved = marketplaceInstallMoved(await this.readLockEntry(pluginKey), listing.source) return { marketplaceSourceId, marketplaceName: listing.marketplaceName, @@ -140,7 +148,8 @@ export class PluginMarketplaceInstaller { bundled: listing.bundled, ...(listing.blockedByKillList ? { blockedByKillList: listing.blockedByKillList } : {}), ...(images ? { images } : {}), - ...(vmRecipeIdentities !== undefined ? { vmRecipeIdentities } : {}) + ...(vmRecipeIdentities !== undefined ? { vmRecipeIdentities } : {}), + ...(moved ? { repositoryMoved: true as const } : {}) } } finally { await rm(stagingDirectory, { recursive: true, force: true }) @@ -172,9 +181,9 @@ export class PluginMarketplaceInstaller { } // Why read first: the install rewrites the lock entry this comparison needs. const priorEntry = await this.readLockEntry(preview.pluginKey) - const installed = marketplaceInstallOf(priorEntry) - const repositoryMoved = - installed !== null && !isSamePluginSource(installed.plugin, listing.source) + const repositoryMoved = marketplaceInstallMoved(priorEntry, listing.source) + // Why revoke before publishing: a content-only fingerprint does not name its repository. + await (repositoryMoved ? this.revokeConsent(preview.pluginKey) : undefined) const result = await installPluginFromMarketplace({ pluginsDir: getUserPluginsDir(this.userDataPath), hostVersion: this.hostVersion, @@ -230,10 +239,7 @@ export class PluginMarketplaceInstaller { if (!installed) { throw new Error(`plugin ${pluginKey} was not installed from a marketplace`) } - const preview = await this.preview(installed.sourceId, pluginKey) - return isSamePluginSource(installed.plugin, preview.source) - ? preview - : { ...preview, repositoryMoved: true } + return this.preview(installed.sourceId, pluginKey) } /** The own-catalog listing an installed plugin would update from, read from caches only. */ diff --git a/src/main/plugins/catalog/plugin-private-marketplace.integration.test.ts b/src/main/plugins/catalog/plugin-private-marketplace.integration.test.ts index d5be67248cd..89aab9295ff 100644 --- a/src/main/plugins/catalog/plugin-private-marketplace.integration.test.ts +++ b/src/main/plugins/catalog/plugin-private-marketplace.integration.test.ts @@ -131,7 +131,8 @@ describe('private Git marketplace integration', () => { const installer = new PluginMarketplaceInstaller({ marketplace, userDataPath, - hostVersion: '1.4.0' + hostVersion: '1.4.0', + revokeConsent: async () => undefined }) const preview = await installer.preview(registered.id, pluginKey) diff --git a/src/main/plugins/catalog/plugin-update-approval.test.ts b/src/main/plugins/catalog/plugin-update-approval.test.ts index 65d55e79534..a3672555231 100644 --- a/src/main/plugins/catalog/plugin-update-approval.test.ts +++ b/src/main/plugins/catalog/plugin-update-approval.test.ts @@ -62,6 +62,7 @@ async function runUpdate(options: { repositoryMoved?: boolean dev?: boolean disabledDuringInstall?: boolean + enableIfData?: boolean }) { const previous = discovered(options.before, 'sha-old', options.dev) const next = discovered(options.after, 'sha-new') @@ -93,7 +94,8 @@ async function runUpdate(options: { } return installed() }, - approve + approve, + ...(options.enableIfData ? { enableIfData: {} } : {}) }) return { outcome, approve, consents, next } @@ -142,6 +144,23 @@ describe('catalog update approval', () => { }) } + for (const [type, sample] of Object.entries(SAMPLES)) { + it(`never carries or one-click grants approval when a ${type} plugin moves repository`, async () => { + const before = sample() + const after = { ...sample(), version: '1.1.0' } + for (const enableIfData of [false, true]) { + const { outcome, approve } = await runUpdate({ + before, + after, + repositoryMoved: true, + enableIfData + }) + expect(outcome).toMatchObject({ ok: true, needsReview: true }) + expect(approve).not.toHaveBeenCalled() + } + }) + } + it('asks again when the execution profile rises', async () => { const { outcome, approve } = await runUpdate({ before: SAMPLES.data(), diff --git a/src/main/plugins/plugin-cli-install.integration.test.ts b/src/main/plugins/plugin-cli-install.integration.test.ts index 9beffba6571..0127486a3b4 100644 --- a/src/main/plugins/plugin-cli-install.integration.test.ts +++ b/src/main/plugins/plugin-cli-install.integration.test.ts @@ -139,7 +139,11 @@ async function harness() { }, 'v1.0.0' ) - await sshShim(root, { '/acme/notes.git': notes, '/acme/locale.git': locale }) + const repositories: Record = { + '/acme/notes.git': notes, + '/acme/locale.git': locale + } + await sshShim(root, repositories) const userDataPath = join(root, 'user-data') let discovered: DiscoveredPlugin[] = [] const consents: Record = {} @@ -174,7 +178,10 @@ async function harness() { installer: new PluginMarketplaceInstaller({ marketplace, userDataPath, - hostVersion: HOST_VERSION + hostVersion: HOST_VERSION, + revokeConsent: async (pluginKey) => { + delete consents[pluginKey] + } }) }, applyConsent: async ({ pluginKey, reviewedFingerprint }) => { @@ -184,7 +191,7 @@ async function harness() { caller: { kind: 'local-cli' } } const lock = async () => (await readPluginLockfile(getUserPluginsDir(userDataPath))).plugins - return { deps, notes, consents, lock } + return { root, deps, notes, locale, repositories, consents, lock } } async function approvalOf(promise: Promise): Promise { @@ -295,6 +302,66 @@ describe('orca plugin install and update over Git', () => { expect(outcome).toMatchObject({ status: 'updated', version: '1.1.0', approval: 'approve-flag' }) }) + it('asks again before a catalog update moves even a content-only plugin to another repository', async () => { + const { root, deps, locale, repositories, consents, lock } = await harness() + const catalog = join(root, 'catalog') + const moved = join(root, 'moved-locale') + await mkdir(catalog, { recursive: true }) + await git(catalog, ['init', '--quiet']) + const listCatalog = (url: string, tag: string) => + commitTag( + catalog, + { + 'orca-marketplace.json': JSON.stringify({ + name: 'Team Plugins', + owner: 'team', + plugins: [{ id: 'acme.locale', source: { kind: 'git', url, ref: 'v1.0.0' } }] + }) + }, + tag + ) + await listCatalog(LOCALE_URL, 'c1') + await execFileAsync('git', ['clone', '--quiet', locale, moved]) + await commitTag(moved, { 'locale.json': JSON.stringify({ settings: { title: 'Def' } }) }, 'v9') + await git(moved, ['tag', '--force', 'v1.0.0']) + await sshShim(root, { + ...repositories, + '/team/catalog.git': catalog, + '/someone-else/locale.git': moved + }) + await deps.catalogs.marketplace.addSource({ + kind: 'git', + url: 'ssh://git@example.invalid/team/catalog.git', + ref: 'master' + }) + await installPluginFromCli(deps, { kind: 'catalog', pluginKey: 'acme.locale' }, undefined) + const approved = consents['acme.locale'] + expect(approved).toBeDefined() + + await listCatalog('ssh://git@example.invalid/someone-else/locale.git', 'c2') + const required = await approvalOf(updatePluginFromCli(deps, 'acme.locale', undefined)) + + expect(required.data.update?.repositoryMoved?.to).toContain('someone-else/locale') + expect(required.message).toContain('It now comes from a different repository') + expect(consents['acme.locale']).toBe(approved) + const installedCommit = (await lock())['acme.locale']?.resolvedCommit + const outcome = await updatePluginFromCli( + deps, + 'acme.locale', + required.data.approvalFingerprint + ) + expect(outcome).toMatchObject({ status: 'updated', approval: 'content-only' }) + expect((await lock())['acme.locale']?.resolvedCommit).not.toBe(installedCommit) + // Dropped before the moved files published, then granted again by this explicit approval. + expect(consents['acme.locale']).toBeDefined() + expect(deps.service.recordApprovalAudit).toHaveBeenLastCalledWith( + expect.objectContaining({ + summary: expect.stringContaining('content-only plugin enabled via CLI (update)'), + outcome: 'ok' + }) + ) + }) + it('reports a private repository this host cannot read as git_auth_failed naming the host', async () => { const { deps } = await harness() diff --git a/src/main/plugins/plugin-cli-install.ts b/src/main/plugins/plugin-cli-install.ts index ad5ee365c3c..2ca58ef58b6 100644 --- a/src/main/plugins/plugin-cli-install.ts +++ b/src/main/plugins/plugin-cli-install.ts @@ -172,7 +172,10 @@ export async function settlePluginCliCandidate( growth !== null && repositoryMoved === null && !pluginUpdateGrowsAccess(growth) // Why: updating a plugin that is not running grants nothing; it stays as the user left it. const grants = action === 'install' || predecessor !== null - const needsFlag = grants && !keepsApproval && pluginNeedsCliApproval(candidate.manifest) + // Why a move needs the flag for every kind: a content-only fingerprint does not name its source. + const needsFlag = + grants && + (repositoryMoved !== null || (!keepsApproval && pluginNeedsCliApproval(candidate.manifest))) if (needsFlag) { const update: PluginApprovalUpdateInfo | undefined = previous ? { fromVersion: previous.version, growth, repositoryMoved } diff --git a/src/main/plugins/plugin-enablement.test.ts b/src/main/plugins/plugin-enablement.test.ts index 0814b09fc4d..f99e8d165c5 100644 --- a/src/main/plugins/plugin-enablement.test.ts +++ b/src/main/plugins/plugin-enablement.test.ts @@ -4,7 +4,7 @@ import { getDefaultSettings } from '../../shared/constants' import type { GlobalSettings } from '../../shared/global-settings-types' import { pluginManifestSchema } from '../../shared/plugins/manifest/plugin-manifest' import type { Store } from '../persistence/loading-store/store' -import { applyPluginConsent } from './plugin-enablement' +import { applyPluginConsent, revokePluginConsent } from './plugin-enablement' import type { ValidDiscoveredPlugin } from './plugin-discovery' import type { PluginService } from './plugin-service' @@ -114,3 +114,24 @@ describe('applyPluginConsent', () => { expect(pluginService.reconcileActivationState).toHaveBeenCalledOnce() }) }) + +describe('revokePluginConsent', () => { + it('drops only this plugin’s approval, leaves it enabled, and stops it running', async () => { + const harness = createStore() + harness.store.updateSettings({ + pluginConsents: { [pluginKey]: 'sha256-old', 'samples.other': 'sha256-other' } + }) + const pluginService = createPluginService(() => 'sha256-old') + await revokePluginConsent({ store: harness.store, pluginService, pluginKey }) + expect(harness.getSettings().pluginConsents).toEqual({ 'samples.other': 'sha256-other' }) + expect(harness.getSettings().disabledPlugins).not.toContain(pluginKey) + expect(pluginService.reconcileActivationState).toHaveBeenCalledOnce() + }) + + it('writes nothing when the plugin had no approval', async () => { + const harness = createStore() + const pluginService = createPluginService(() => 'sha256-old') + await revokePluginConsent({ store: harness.store, pluginService, pluginKey }) + expect(harness.updateSettings).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/plugins/plugin-enablement.ts b/src/main/plugins/plugin-enablement.ts index 29b22965bea..feb936622a5 100644 --- a/src/main/plugins/plugin-enablement.ts +++ b/src/main/plugins/plugin-enablement.ts @@ -49,6 +49,26 @@ export async function applyPluginConsent(input: { await pluginService.reconcileActivationState() } +/** Drops a plugin's recorded approval without disabling it, so it waits for review. */ +export async function revokePluginConsent(input: { + store: Store + pluginService: PluginService | null + pluginKey: string +}): Promise { + const { store, pluginService, pluginKey } = input + // Why throw: an install that cannot drop the old approval must not publish. + if (!pluginService) { + throw new Error('plugin service is not running') + } + const consents = normalizePluginConsents(store.getSettings().pluginConsents) + if (!(pluginKey in consents)) { + return + } + delete consents[pluginKey] + store.updateSettings({ pluginConsents: consents }, { notifyListeners: true }) + await pluginService.reconcileActivationState() +} + /** Enabling cannot bypass missing/stale consent. */ export async function applyPluginEnablement(input: { store: Store diff --git a/src/main/startup/main-process-plugins.ts b/src/main/startup/main-process-plugins.ts index da23a33093c..4be52d336fd 100644 --- a/src/main/startup/main-process-plugins.ts +++ b/src/main/startup/main-process-plugins.ts @@ -19,7 +19,7 @@ import { getPluginsDataDir } from '../plugins/plugin-discovery' import { bundledPluginRootForApp } from '../plugins/plugin-bundled-bootstrap' import { approveBundledPluginsFromStore } from '../plugins/plugin-bundled-approval' import { resolvePluginHostEntryPath } from '../plugins/workers/plugin-host-process' -import { applyPluginConsent } from '../plugins/plugin-enablement' +import { applyPluginConsent, revokePluginConsent } from '../plugins/plugin-enablement' import { clearRemovedPluginSettings } from '../plugins/plugin-removal-settings' import { clearPluginFrameStorage } from '../plugins/panels/plugin-frame-guest-sessions' import { bindPluginRpcServices } from './main-process-plugin-rpc' @@ -102,7 +102,9 @@ export async function initializeMainProcessPlugins(runtime: OrcaRuntimeService): userDataPath: app.getPath('userData'), hostVersion: pluginHostVersion(app.getVersion()), blockedPluginReason: (pluginKey) => state.pluginKillListService?.reason(pluginKey) ?? null, - imageCache: pluginImageCache + imageCache: pluginImageCache, + revokeConsent: (pluginKey) => + revokePluginConsent({ store, pluginService: state.pluginService, pluginKey }) }) const bundledPluginRoot = bundledPluginRootForApp(app) state.pluginService = new PluginService({ diff --git a/src/renderer/src/components/plugins/dialogs/PluginUpdateAllDialog.test.tsx b/src/renderer/src/components/plugins/dialogs/PluginUpdateAllDialog.test.tsx index 6b466524378..b246ee93c30 100644 --- a/src/renderer/src/components/plugins/dialogs/PluginUpdateAllDialog.test.tsx +++ b/src/renderer/src/components/plugins/dialogs/PluginUpdateAllDialog.test.tsx @@ -308,6 +308,45 @@ describe('PluginUpdateAllDialog', () => { expect(container.querySelector('[data-plugin-update="a"]')?.textContent).toContain('Updated') }) + it('opens a lone content-only update from a moved repository to its own review', async () => { + const installMarketplacePlugin = vi.fn(({ pluginKey }: { pluginKey: string }) => + Promise.resolve({ ok: true, pluginKey, consentFingerprint: 'f', needsReview: true }) + ) + const consent = vi.fn().mockResolvedValue([]) + installApi({ + previewMarketplaceUpdate: vi.fn(() => + Promise.resolve({ + ...preview('a', []), + source: { kind: 'git', url: 'https://example.com/someone-else/theme.git', ref: 'main' }, + repositoryMoved: true as const + }) + ), + installMarketplacePlugin, + consent + }) + const theme = entry('a', []) + theme.installed!.source = { + kind: 'marketplace', + reference: 'https://example.com/acme/theme.git', + resolvedCommit: 'r0', + contentHash: 'h0' + } + const container = await renderDialog([theme]) + + // Same access, so only the source line explains why this one asks again. + expect(container.querySelector('h2')?.textContent).toBe('Review update to a') + expect(container.textContent).toContain( + 'New source: https://example.com/someone-else/theme.git (was https://example.com/acme/theme.git)' + ) + expect(installMarketplacePlugin).not.toHaveBeenCalled() + await act(async () => button(container, 'Approve & update')?.click()) + expect(consent).toHaveBeenCalledWith({ + pluginKey: 'a', + reviewedFingerprint: 'f', + decision: 'approve' + }) + }) + it('never approves bytes other than the ones it reviewed', async () => { const consent = vi.fn().mockResolvedValue([]) installApi({ diff --git a/src/renderer/src/components/plugins/state/use-plugin-install-action.test.tsx b/src/renderer/src/components/plugins/state/use-plugin-install-action.test.tsx new file mode 100644 index 00000000000..28bd6cda177 --- /dev/null +++ b/src/renderer/src/components/plugins/state/use-plugin-install-action.test.tsx @@ -0,0 +1,148 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot } from 'react-dom/client' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { + PluginMarketplaceHostInstallPreview, + PluginMarketplaceHostListing +} from '../../../../../preload/api-types' +import { pluginManifestSchema } from '../../../../../shared/plugins/manifest/plugin-manifest' +import { + settlePluginInstallTrust, + usePluginInstallTrustStore +} from '@/store/plugin-install-trust-request' +import type { PluginInstallTrustPrompt } from '../presentation/plugin-install-trust' +import { usePluginInstallAction } from './use-plugin-install-action' + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), warning: vi.fn() } })) +vi.mock('./plugin-theme-apply', () => ({ applyPluginThemesNow: vi.fn() })) +import { toast } from 'sonner' +import { applyPluginThemesNow } from './plugin-theme-apply' + +const listing: PluginMarketplaceHostListing = { + marketplaceSourceId: 'source', + marketplaceName: 'Catalog', + marketplaceOwner: 'acme', + marketplaceCommit: 'b'.repeat(40), + pluginKey: 'acme.dusk', + source: { kind: 'git', url: 'https://example.invalid/dusk.git', ref: 'main' }, + categories: [], + official: false, + bundled: false, + release: { + version: '1.0.0', + commit: 'c'.repeat(40), + contentHash: 'd'.repeat(64), + execution: 'data', + minimumOrca: '1.0.0', + types: ['app-theme'] + } +} + +const preview: PluginMarketplaceHostInstallPreview = { + ...listing, + resolvedCommit: 'c'.repeat(40), + contentHash: 'd'.repeat(64), + consentFingerprint: 'sha256-shown', + manifest: pluginManifestSchema.parse({ + manifestVersion: 1, + id: 'dusk', + publisher: 'acme', + name: 'Dusk', + version: '1.0.0', + engines: { orca: '>=1.0.0' }, + pluginApi: 1, + contributes: { themes: [{ id: 'dusk', title: 'Dusk', path: 'themes/dusk.json' }] } + }) +} + +const codePreview: PluginMarketplaceHostInstallPreview = { + ...preview, + manifest: pluginManifestSchema.parse({ + ...preview.manifest, + contributes: { commands: [{ id: 'go', title: 'Go', action: 'sidebar.left.toggle' }] } + }) +} + +function setup(isFromInstallLink?: (pluginKey: string) => boolean) { + const onInstalled = vi.fn().mockResolvedValue(undefined) + const fetchPreview = vi.fn().mockResolvedValue({ ok: true, preview }) + const hook: { current: ReturnType | null } = { current: null } + function Probe(): null { + hook.current = usePluginInstallAction({ fetchPreview, onInstalled, isFromInstallLink }) + return null + } + act(() => createRoot(document.createElement('div')).render()) + return { hook, onInstalled, fetchPreview } +} + +function installResult(on: boolean) { + return { + ok: true as const, + pluginKey: listing.pluginKey, + version: '1.0.0', + contentHash: 'd'.repeat(64), + consentFingerprint: 'sha256-shown', + resolvedCommit: 'c'.repeat(40), + needsReview: !on + } +} + +// Answers every trust ask the way the current test chose, recording what was shown. +const asked: PluginInstallTrustPrompt[] = [] +let answer = true +usePluginInstallTrustStore.subscribe(({ pending }) => { + if (pending) { + asked.push(pending.prompt) + queueMicrotask(() => settlePluginInstallTrust(answer)) + } +}) + +beforeEach(() => { + asked.length = 0 + answer = true + vi.mocked(toast.success).mockClear() + vi.mocked(toast.warning).mockClear() + vi.mocked(applyPluginThemesNow).mockReset().mockResolvedValue(false) + Object.defineProperty(window, 'api', { + configurable: true, + value: { + plugins: { + installMarketplacePlugin: vi.fn().mockResolvedValue(installResult(true)), + consent: vi.fn().mockResolvedValue([]) + } + } + }) +}) + +describe('usePluginInstallAction', () => { + it('leaves an install link’s replacement from a moved repository off and unapproved', async () => { + const codeListing = { + ...listing, + release: { ...listing.release!, execution: 'declarative' as const } + } + const movedCode = { ...codePreview, repositoryMoved: true as const } + for (const [target, moved, approve] of [ + [listing, { ...preview, repositoryMoved: true as const }, undefined], + [codeListing, movedCode, { preview: movedCode }] + ] as const) { + const { hook, fetchPreview, onInstalled } = setup(() => true) + fetchPreview.mockResolvedValue({ ok: true, preview: moved }) + vi.mocked(window.api.plugins.installMarketplacePlugin).mockResolvedValue(installResult(false)) + await act(async () => hook.current?.install(target, approve)) + expect(window.api.plugins.installMarketplacePlugin).toHaveBeenLastCalledWith( + expect.not.objectContaining({ enableIfData: true }) + ) + expect(window.api.plugins.consent).not.toHaveBeenCalled() + expect(applyPluginThemesNow).not.toHaveBeenCalled() + expect(hook.current?.phaseOf(listing.pluginKey)).toBeNull() + expect(onInstalled).toHaveBeenCalledWith(listing.pluginKey) + expect(toast.warning).toHaveBeenLastCalledWith( + 'Dusk 1.0.0 comes from a different repository', + expect.objectContaining({ description: 'It stays off until you review it.' }) + ) + expect(toast.success).not.toHaveBeenCalled() + } + }) +}) diff --git a/src/renderer/src/components/plugins/state/use-plugin-install-action.ts b/src/renderer/src/components/plugins/state/use-plugin-install-action.ts index 64680d3694d..ff01335a3d0 100644 --- a/src/renderer/src/components/plugins/state/use-plugin-install-action.ts +++ b/src/renderer/src/components/plugins/state/use-plugin-install-action.ts @@ -135,6 +135,8 @@ export function usePluginInstallAction({ setPhase(pluginKey, { kind: 'installing' }) } const execution = listing.release?.execution ?? pluginExecutionProfile(preview.manifest) + // Why: an installed copy came from another repository; what replaces it waits for review. + const moved = preview.repositoryMoved === true try { const result = await window.api.plugins.installMarketplacePlugin({ marketplaceSourceId: preview.marketplaceSourceId, @@ -143,7 +145,7 @@ export function usePluginInstallAction({ resolvedCommit: preview.resolvedCommit, ...(isFromInstallLink?.(pluginKey) ? { fromInstallLink: true as const } : {}), // Why the fingerprint: main turns on only the exact bytes this preview showed. - ...(execution === 'data' + ...(execution === 'data' && !moved ? { enableIfData: true as const, expectedFingerprint: preview.consentFingerprint } : {}) }) @@ -154,6 +156,7 @@ export function usePluginInstallAction({ // reviewed on the page after install rather than approved unseen. if ( approve && + !moved && execution !== 'data' && preview.manifest.contributes.vmRecipes.length === 0 ) { @@ -170,6 +173,23 @@ export function usePluginInstallAction({ } await onInstalled(pluginKey) setPhase(pluginKey, null) + if (moved) { + toast.warning( + translate( + 'auto.components.settings.PluginUpdateReviewToast.movedTitle', + '{{value0}} {{value1}} comes from a different repository', + { value0: preview.manifest.name, value1: preview.manifest.version } + ), + { + duration: PLUGIN_ACTION_TOAST_DURATION_MS, + description: translate( + 'auto.components.settings.PluginUpdateReviewToast.movedInstalled', + 'It stays off until you review it.' + ) + } + ) + return + } const { themes, iconThemes, terminalThemes } = preview.manifest.contributes const themed = themes.length + iconThemes.length + terminalThemes.length > 0 // Why: a click to install a theme should show it at once; its toast offers Switch back. diff --git a/src/renderer/src/i18n/locales/de.json b/src/renderer/src/i18n/locales/de.json index 71bdfc3dbbe..f0ef0cb1b5c 100644 --- a/src/renderer/src/i18n/locales/de.json +++ b/src/renderer/src/i18n/locales/de.json @@ -13271,7 +13271,8 @@ "oneDescription": "Veröffentlicht von {{value0}}. Es wird erst aktualisiert, wenn du es prüfst.", "manyDescription": "Sie werden erst aktualisiert, wenn du sie prüfst.", "review": "Prüfen", - "dismiss": "Schließen" + "dismiss": "Schließen", + "movedInstalled": "Es bleibt aus, bis du es prüfst." }, "PluginThemeUseNow": { "assignedLight": "{{value0}} als helles Design festgelegt", diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index c2082042438..b25d53cf57a 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -13709,7 +13709,8 @@ "oneDescription": "Published by {{value0}}. It won’t update until you review it.", "manyDescription": "They won’t update until you review them.", "review": "Review", - "dismiss": "Dismiss" + "dismiss": "Dismiss", + "movedInstalled": "It stays off until you review it." }, "PluginInstallTrustDialog": { "titleFolder": "Make sure you trust this folder", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 927d7548882..89e4576f3b8 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -13012,7 +13012,8 @@ "oneDescription": "Publicado por {{value0}}. No se actualizará hasta que lo revises.", "manyDescription": "No se actualizarán hasta que las revises.", "review": "Revisar", - "dismiss": "Descartar" + "dismiss": "Descartar", + "movedInstalled": "Seguirá desactivado hasta que lo revises." }, "PluginInstallTrustDialog": { "titleFolder": "Asegúrate de que confías en esta carpeta", diff --git a/src/renderer/src/i18n/locales/fr.json b/src/renderer/src/i18n/locales/fr.json index 3a217c13a9e..c53f002f72e 100644 --- a/src/renderer/src/i18n/locales/fr.json +++ b/src/renderer/src/i18n/locales/fr.json @@ -13236,7 +13236,8 @@ "oneDescription": "Publié par {{value0}}. Il ne sera pas mis à jour tant que vous ne l’aurez pas vérifié.", "manyDescription": "Elles ne seront pas installées tant que vous ne les aurez pas vérifiées.", "review": "Vérifier", - "dismiss": "Ignorer" + "dismiss": "Ignorer", + "movedInstalled": "Il reste désactivé jusqu’à ce que vous l’examiniez." }, "PluginInstallTrustDialog": { "titleFolder": "Assurez-vous de faire confiance à ce dossier", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 493489739bf..eadb4b31c4f 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -12725,7 +12725,8 @@ "oneDescription": "公開元: {{value0}}。確認するまで更新されません。", "manyDescription": "確認するまで更新されません。", "review": "確認", - "dismiss": "閉じる" + "dismiss": "閉じる", + "movedInstalled": "確認するまでオフのままです。" }, "PluginInstallTrustDialog": { "titleFolder": "このフォルダーを信頼できることを確認してください", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index a0498c5158e..762a0a7b2a7 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -12725,7 +12725,8 @@ "oneDescription": "게시자: {{value0}}. 검토하기 전까지 업데이트되지 않습니다.", "manyDescription": "검토하기 전까지 업데이트되지 않습니다.", "review": "검토", - "dismiss": "닫기" + "dismiss": "닫기", + "movedInstalled": "검토할 때까지 꺼진 상태로 유지됩니다." }, "PluginInstallTrustDialog": { "titleFolder": "이 폴더를 신뢰하는지 확인하세요", diff --git a/src/renderer/src/i18n/locales/pt.json b/src/renderer/src/i18n/locales/pt.json index b73c2a81cbd..01af6b68987 100644 --- a/src/renderer/src/i18n/locales/pt.json +++ b/src/renderer/src/i18n/locales/pt.json @@ -9594,7 +9594,8 @@ "oneDescription": "Publicado por {{value0}}. Não será atualizado até você revisar.", "manyDescription": "Não serão atualizadas até você revisá-las.", "review": "Revisar", - "dismiss": "Dispensar" + "dismiss": "Dispensar", + "movedInstalled": "Ele fica desativado até você revisá-lo." }, "PluginInstallTrustDialog": { "titleFolder": "Confirme que você confia nesta pasta", diff --git a/src/renderer/src/i18n/locales/ru.json b/src/renderer/src/i18n/locales/ru.json index 1345c36ee14..d4918e6eaec 100644 --- a/src/renderer/src/i18n/locales/ru.json +++ b/src/renderer/src/i18n/locales/ru.json @@ -13635,7 +13635,8 @@ "oneDescription": "Издатель: {{value0}}. Обновление не будет установлено, пока вы его не проверите.", "manyDescription": "Они не будут установлены, пока вы их не проверите.", "review": "Проверить", - "dismiss": "Закрыть" + "dismiss": "Закрыть", + "movedInstalled": "Он останется выключенным, пока вы его не проверите." }, "PluginThemeUseNow": { "assignedLight": "{{value0}} установлена как светлая тема", diff --git a/src/renderer/src/i18n/locales/tr.json b/src/renderer/src/i18n/locales/tr.json index dd7b4aa84a3..a95d2611163 100644 --- a/src/renderer/src/i18n/locales/tr.json +++ b/src/renderer/src/i18n/locales/tr.json @@ -13257,7 +13257,8 @@ "oneDescription": "Yayıncı: {{value0}}. Siz inceleyene kadar güncellenmeyecek.", "manyDescription": "Siz inceleyene kadar güncellenmeyecekler.", "review": "İncele", - "dismiss": "Kapat" + "dismiss": "Kapat", + "movedInstalled": "Siz inceleyene kadar kapalı kalır." }, "PluginThemeUseNow": { "assignedLight": "{{value0}} açık temanız olarak ayarlandı", diff --git a/src/renderer/src/i18n/locales/zh-Hant.json b/src/renderer/src/i18n/locales/zh-Hant.json index 7f4be4d8a79..b24fd613228 100644 --- a/src/renderer/src/i18n/locales/zh-Hant.json +++ b/src/renderer/src/i18n/locales/zh-Hant.json @@ -13094,7 +13094,8 @@ "oneDescription": "發佈者:{{value0}}。在你審查之前不會更新。", "manyDescription": "在你審查之前不會更新。", "review": "審查", - "dismiss": "關閉" + "dismiss": "關閉", + "movedInstalled": "在你審查之前會保持關閉。" }, "PluginThemeUseNow": { "assignedLight": "已將 {{value0}} 設為淺色主題", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 00e53854931..0f9e8dcfd21 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -12725,7 +12725,8 @@ "oneDescription": "发布者:{{value0}}。在你审核之前不会更新。", "manyDescription": "在你审核之前不会更新。", "review": "审核", - "dismiss": "关闭" + "dismiss": "关闭", + "movedInstalled": "在你审查之前会保持关闭。" }, "PluginInstallTrustDialog": { "titleFolder": "请确认你信任此文件夹", diff --git a/src/shared/plugins/approval/plugin-cli-approval.ts b/src/shared/plugins/approval/plugin-cli-approval.ts index 99e7cfc081b..23361aa56de 100644 --- a/src/shared/plugins/approval/plugin-cli-approval.ts +++ b/src/shared/plugins/approval/plugin-cli-approval.ts @@ -193,7 +193,9 @@ export function buildPluginApprovalRequest( contentIdentity: subject.contentIdentity, isDev: subject.isDev, grants, - why: `It can run code or act in Orca, so Orca does not enable it without an explicit approval bound to exactly these files and this access.${devNote}`, + why: pluginNeedsCliApproval(subject.manifest) + ? `It can run code or act in Orca, so Orca does not enable it without an explicit approval bound to exactly these files and this access.${devNote}` + : 'It now comes from a different repository or folder, so Orca does not keep it on without an explicit approval bound to exactly these files.', approveFlag, nextSteps: [ 'Approving grants the access listed above. Confirm with your user first unless they already asked for exactly this plugin and access.',