diff --git a/docs/contributing/architecture/data-storage.md b/docs/contributing/architecture/data-storage.md index 063b9b9039..0361e98645 100644 --- a/docs/contributing/architecture/data-storage.md +++ b/docs/contributing/architecture/data-storage.md @@ -243,8 +243,16 @@ The schema is defined by migrations in `packages/worker/migrations/`: are derived at read time from `community_forks` / `community_stars` - `secret_buckets`: encrypted-secret ownership buckets scoped to `user`, `package`, or `session`. Package buckets bind directly to `saved_packages.id`; - package runtimes may use their own package secrets, while user secrets require - an explicit `allowed_packages` grant on every package read path. + package runtimes may use their own package secrets. User secrets are + auto-granted for read/use to self-authored packages (no `community_forks` row + for that `saved_packages.id` + `userId`) and to adopted forks + (`community_forks.adopted_at` set via `community_fork_adopt`; columns in + `0074-community-fork-adoption.sql`). Unadopted community forks + (`community_forks.forked_package_id`, indexed in + `0073-community-forks-forked-package-index.sql`) still require an explicit + `allowed_packages` grant on every package read path. Updating or deleting a + user secret from package code always requires that grant, regardless of fork + or adoption state. App access pattern: @@ -597,8 +605,15 @@ on write unless a migration backfills existing rows. policy inputs (`0009-secret-allowed-hosts.sql`, `0010-secret-allowed-capabilities.sql`, `0023-secret-allowed-packages.sql`). Tightening parse-error behavior requires explicit compatibility review. - `allowed_packages` applies only to user-scoped secrets; package-scoped secrets - are owned exclusively by the package id in their bucket binding. + `allowed_packages` applies only to user-scoped secrets. Unadopted + community-forked packages need it for every package read/use path (provenance + via `community_forks.forked_package_id` + `forker_user_id`; index + `0073-community-forks-forked-package-index.sql`). Self-authored packages and + adopted forks (`community_forks.adopted_at` / `adoption_note` from + `0074-community-fork-adoption.sql`) skip that grant for read/use only. + Mutations from package code (`secret_set` / `secret_delete` / OpenAPI + token-refresh writes) always require the grant. Package-scoped secrets are + owned exclusively by the package id in their bucket binding. - `package_runtime_runs.metadata_json` and `package_runtime_logs.fields_json` (`0037-package-runtime-debug.sql`) store bounded debug metadata and log fields. diff --git a/docs/contributing/secret-host-approval.md b/docs/contributing/secret-host-approval.md index 468913e3b2..c9cda25fb0 100644 --- a/docs/contributing/secret-host-approval.md +++ b/docs/contributing/secret-host-approval.md @@ -21,7 +21,8 @@ authenticated account admin UI. In this repo, that means the user must approve host access through the account secrets experience, such as `/account/secrets` and the focused approval route at -`/account/secrets/approve`. +`/account/secrets/approve`. Capability allowlist changes use the same one-click +"Approve secret access" card on `/account/secrets//?capability=…`. ## What agents should assume diff --git a/docs/guides/account-secret-setup.md b/docs/guides/account-secret-setup.md index cb7d64430f..20c677797c 100644 --- a/docs/guides/account-secret-setup.md +++ b/docs/guides/account-secret-setup.md @@ -48,8 +48,12 @@ Provide the user a URL like: ## Package approval URLs (after a package exists) -When a saved package needs access to one or more **existing** user secrets, send -the user an approval link — do not ask them to recreate the secrets. +Self-authored packages and adopted community forks (`community_fork_adopt`) can +read and use the user's secrets without an `allowed_packages` grant; updating or +deleting a user secret from package code still requires that grant. When an +**unadopted community-forked** package needs access to one or more **existing** +user secrets, either adopt it after reviewing the source or send the user an +approval link — do not ask them to recreate the secrets. - Single secret: `/account/secrets/user/{secretName}?package_id={savedPackageId}&package={kodyId}` diff --git a/docs/guides/integration-bootstrap.md b/docs/guides/integration-bootstrap.md index 9020324f80..8e5d4e69f9 100644 --- a/docs/guides/integration-bootstrap.md +++ b/docs/guides/integration-bootstrap.md @@ -121,11 +121,15 @@ app will depend on: - the agent is using the correct secret names, integration name, and API base URL -An authenticated `execute` smoke test does **not** grant package secret access. -After you save or publish a secret-using package, surface -`pending_secret_package_approvals` (prefer `bulk_approval_url`), wait for the -user to approve, and verify with `packages.invokeChecked` before calling the -work complete. +An authenticated `execute` smoke test does **not** grant package secret access +for unadopted community-forked packages. Self-authored packages and adopted +forks (`community_fork_adopt` after source review) get automatic read/use access +to user secrets (host approval still applies; updating or deleting a user secret +from package code still needs an `allowed_packages` grant). After you save or +publish a secret-using package, read `pending_secret_package_approvals`; when it +is non-null (unadopted community forks), either adopt after review or surface +`bulk_approval_url`, wait when required, and verify with +`packages.invokeChecked` before calling the work complete. ## Important exceptions diff --git a/docs/guides/package-authoring.md b/docs/guides/package-authoring.md index 8dc5259d30..7314e777c3 100644 --- a/docs/guides/package-authoring.md +++ b/docs/guides/package-authoring.md @@ -90,15 +90,23 @@ When a package will use user-scoped secrets (`{{secret:name}}` placeholders or 1. Ensure each secret exists (see `guide: "connect_secret"` / `guide: "secret_backed_integration"`). -2. After save/publish, read `pending_secret_package_approvals` from the tool - result. -3. Send the user `bulk_approval_url` when present; otherwise send each - `approval_url`. -4. Wait for approval, then smoke-test with `packages.invokeChecked(...)`. +2. Self-authored packages (and community forks adopted with + `community_fork_adopt` after a real source review) get automatic read/use + access to user secrets (host approval still applies; `secret_set` / + `secret_delete` still need an `allowed_packages` grant). After save/publish, + read `pending_secret_package_approvals` from the tool result — it is non-null + only for unadopted community forks. +3. When pending approvals are present, either review the fork source and call + `community_fork_adopt` with a `review_summary`, or send the user + `bulk_approval_url` / each `approval_url`. +4. Wait for approval or adoption (when required), then smoke-test with + `packages.invokeChecked(...)`. 5. Only then treat the package as ready to run. Host approval (from an earlier ad hoc `execute` smoke test) is separate from -package approval. Both may be required. +package approval. Unadopted community-forked packages may need both; +self-authored and adopted packages still need host approval when outbound calls +require it. ## Community icon diff --git a/docs/guides/package-lifecycle.md b/docs/guides/package-lifecycle.md index 8d58b02002..af624922a7 100644 --- a/docs/guides/package-lifecycle.md +++ b/docs/guides/package-lifecycle.md @@ -63,8 +63,10 @@ not tied to reusable package behavior. `job_schedule_once` is the one-off convenience form. Use `guide: "package_authoring"` for package shape, README `## Intent`, -visibility guidance, and the secret-using package approval checklist (prefer -bulk approval URLs from `pending_secret_package_approvals` after save/publish). +visibility guidance, and the secret-using package approval checklist +(`pending_secret_package_approvals` is non-null only for unadopted +community-forked packages; prefer `community_fork_adopt` after review, or bulk +approval URLs when present). ## Signals to escalate from `execute` to a package diff --git a/docs/guides/secret-backed-integration.md b/docs/guides/secret-backed-integration.md index 1e786a32d1..f79c5343f3 100644 --- a/docs/guides/secret-backed-integration.md +++ b/docs/guides/secret-backed-integration.md @@ -59,14 +59,21 @@ smoke-test path is unclear. - Prefer plain package exports for simple automations. - Use a package app only when the user actually needs interactive UI, browser-side forms, or hosted callbacks. -8. After the package is saved or published, finish package secret approval. - - An ad hoc `execute` smoke test does **not** grant package secret access. +8. After the package is saved or published, finish package secret approval when + needed. + - Self-authored packages and adopted forks (`community_fork_adopt`) get + automatic read/use access to user secrets (mutations still need an + `allowed_packages` grant); unadopted community forks still need explicit + package approval for read/use, or adoption after review. + - An ad hoc `execute` smoke test does **not** grant package secret access for + community forks. - Read `pending_secret_package_approvals` from `package_save` or - `package_publish_external_push`. - - Prefer `bulk_approval_url` when present (one click for all listed secrets). - Otherwise send each per-secret `approval_url`. - - Wait for the user to approve, then verify with - `packages.invokeChecked(...)` before treating the package as complete. + `package_publish_external_push` (null for self-authored / adopted + packages). + - When present, either review the source and call `community_fork_adopt`, or + send `bulk_approval_url` / each `approval_url`. + - Wait for the user to approve or for adoption (when required), then verify + with `packages.invokeChecked(...)` before treating the package as complete. ## Secret names and value names @@ -174,8 +181,10 @@ Avoid these mistakes: - saving readable config as a secret - saving the downstream package before the smoke test passes - assuming a saved secret automatically approves outbound hosts -- treating an ad hoc `execute` smoke test as package secret approval -- marking a secret-using package complete without sending package approval links - (prefer the bulk approval URL when multiple secrets need access) +- treating an ad hoc `execute` smoke test as package secret approval for a + community-forked package +- marking an unadopted community-forked secret-using package complete without + adopting after review (`community_fork_adopt`) or sending package approval + links (prefer the bulk approval URL when multiple secrets need access) - inventing a provider-specific flow when one or two secrets plus a smoke test would do diff --git a/docs/use/secrets-and-values.md b/docs/use/secrets-and-values.md index 6b54b196e9..98bb0c8080 100644 --- a/docs/use/secrets-and-values.md +++ b/docs/use/secrets-and-values.md @@ -14,9 +14,15 @@ such as **`package`**) returns **metadata only**: names, descriptions, allowed hosts, allowed capabilities — not plaintext values. Package-scoped secrets belong to one saved package and are available only while -that package runs. User-scoped secrets require explicit package approval before -package code can list, read, update, or delete them through mounts, fetch -placeholders, or secret-aware capabilities. +that package runs. User-scoped secrets are available automatically for **reading +and using** (mounts, fetch placeholders, capability inputs) to packages the user +authored themselves, and to community forks the user has adopted with +`community_fork_adopt` after reviewing the source. Unadopted community forks +still require explicit package approval (`allowed_packages`) before those +read/use paths. Updating or deleting a user secret from package code +(`secret_set`, `secret_delete`, OpenAPI token-refresh writes) always requires +the explicit package grant, including for self-authored and adopted packages. +Host and capability approvals are unchanged. **`kody.secret_set(...)`** persists a value that is already available inside execution (for example a refreshed OAuth token). It does not return secret @@ -104,15 +110,20 @@ does not by itself approve new hosts. ## Package approval -User-scoped secrets also need explicit **package** approval before package code -can use them. Saving a secret, approving a host, or succeeding in an ad hoc -execute smoke test does not grant that access. +Unadopted community-forked packages need explicit **package** approval +(`allowed_packages`) before they can read or use user-scoped secrets. +Self-authored packages and adopted forks (`community_fork_adopt` after a real +source review) get that read/use access automatically. Updating or deleting a +user secret from package code always needs the grant. Saving a secret, approving +a host, or succeeding in an ad hoc execute smoke test does not grant package +access. When several secrets need the same package approved, Kody can provide a bulk approval URL shaped like `/account/secrets/approve?package_id=...&names=secretA,secretB`. That page lists every pending secret and approves them in one click. Single-secret links still -work for one-off grants. +work for one-off grants. For community forks, reviewing the source and calling +`community_fork_adopt` is an alternative to sending those approval links. ## Values diff --git a/packages/worker/client/routes/account-approval-shared.node.test.ts b/packages/worker/client/routes/account-approval-shared.node.test.ts index 8f5f847b55..bf79b47fe3 100644 --- a/packages/worker/client/routes/account-approval-shared.node.test.ts +++ b/packages/worker/client/routes/account-approval-shared.node.test.ts @@ -17,6 +17,21 @@ test('buildHostApprovalRequestUrl maps secret approval links and rejects invalid ).toBe( '/account/secrets.json?allowed-host=api.github.com&selected=user%3A%3A%3A%3AgithubAccessToken', ) + expect( + buildHostApprovalRequestUrl( + 'https://example.com/account/secrets/user/cloudflareToken?capability=secret_set', + 'https://example.com', + ), + ).toBe( + '/account/secrets.json?capability=secret_set&selected=user%3A%3A%3A%3AcloudflareToken', + ) + expect( + buildHostApprovalRequestUrl( + '/account/secrets/package/pkg-1/signingSecret?capability=secret_jwt_sign', + ), + ).toBe( + '/account/secrets.json?capability=secret_jwt_sign&selected=package%3A%3Apkg-1%3A%3AsigningSecret', + ) expect(() => buildHostApprovalRequestUrl('/account/secrets?allowed-host=slack.com'), ).toThrow('Invalid approval link.') diff --git a/packages/worker/client/routes/account-approval-shared.ts b/packages/worker/client/routes/account-approval-shared.ts index 4e7dab645b..19c20f11bf 100644 --- a/packages/worker/client/routes/account-approval-shared.ts +++ b/packages/worker/client/routes/account-approval-shared.ts @@ -11,6 +11,7 @@ export type ApprovalView = { requestedHost: string requestedCapability: string | null currentAllowedHosts: Array + currentAllowedCapabilities: Array requestedPackageId: string | null currentAllowedPackages: Array } diff --git a/packages/worker/client/routes/account-secrets.tsx b/packages/worker/client/routes/account-secrets.tsx index 1b22b84bda..b80c78e095 100644 --- a/packages/worker/client/routes/account-secrets.tsx +++ b/packages/worker/client/routes/account-secrets.tsx @@ -298,7 +298,7 @@ function buildSecretsHref(pathname: string, search: string) { return `${pathname}${search}` } -function readCapabilityPrefill(href: string) { +function readRequestedCapability(href: string) { const url = new URL(href, 'http://localhost') const value = url.searchParams.get('capability') return value?.trim() ? value.trim() : null @@ -330,7 +330,7 @@ function getAlreadyAddedNotice(input: { readRequestedHost(input.href), ) const requestedCapability = normalizeSingleAllowedCapability( - readCapabilityPrefill(input.href), + readRequestedCapability(input.href), ) const requestedPackageId = new URL(input.href, 'http://localhost').searchParams @@ -345,7 +345,11 @@ function getAlreadyAddedNotice(input: { ? normalizeAllowedCapabilities( coerceStringRows(input.selectedSecret.allowedCapabilities), ) - : [] + : input.approval + ? normalizeAllowedCapabilities( + coerceStringRows(input.approval.currentAllowedCapabilities), + ) + : [] const allowedPackageIds = input.selectedSecret ? Array.from( new Set( @@ -369,11 +373,15 @@ function getAlreadyAddedNotice(input: { if (hostAlreadyAdded) { items.push(`Host ${requestedHost} is already in allowed hosts.`) } - if ( + const capabilityAlreadyAdded = requestedCapability != null && + requestedHost == null && + requestedPackageId == null && allowedCapabilities.includes(requestedCapability) - ) { - items.push(`Capability ${requestedCapability} is already in allowed kody.`) + if (capabilityAlreadyAdded) { + items.push( + `Capability ${requestedCapability} is already in allowed capabilities.`, + ) } const packageAlreadyAdded = requestedPackageId != null && allowedPackageIds.includes(requestedPackageId) @@ -386,26 +394,11 @@ function getAlreadyAddedNotice(input: { return { items, hostAlreadyAdded, + capabilityAlreadyAdded, packageAlreadyAdded, } } -function applyCapabilityPrefill(state: EditorState, capability: string | null) { - if (!capability) return state - if (state.allowedCapabilities.some((entry) => entry.trim() === capability)) { - return state - } - const nextAllowedCapabilities = - state.allowedCapabilities.length === 1 && - state.allowedCapabilities[0]?.trim() === '' - ? [capability] - : [...state.allowedCapabilities, capability] - return { - ...state, - allowedCapabilities: nextAllowedCapabilities, - } -} - function buildSecretHref( secret: { name: string @@ -603,25 +596,18 @@ export function AccountSecretsRoute(handle: Handle) { function syncEditorState(selection: SelectionState) { deleteSecretCheck.reset() showSecretValue = false - const capabilityPrefill = readCapabilityPrefill(getCurrentHref()) if (selection.isCreating) { - editorState = applyCapabilityPrefill( - createEditorStateFromNewSecretQuery(packageOptions, getCurrentHref()), - capabilityPrefill, + editorState = createEditorStateFromNewSecretQuery( + packageOptions, + getCurrentHref(), ) return } if (selectedSecret) { - editorState = applyCapabilityPrefill( - createEditorStateFromSecret(selectedSecret), - capabilityPrefill, - ) + editorState = createEditorStateFromSecret(selectedSecret) return } - editorState = applyCapabilityPrefill( - createEmptyEditorState(packageOptions), - capabilityPrefill, - ) + editorState = createEmptyEditorState(packageOptions) } function applyPayload( @@ -759,6 +745,10 @@ export function AccountSecretsRoute(handle: Handle) { const isBulkPackageApproval = Boolean(approval.requestedPackageId) && approval.names.length > 1 + const isCapabilityApproval = + Boolean(approval.requestedCapability) && + !approval.requestedPackageId && + !approval.requestedHost applyPayload( payload, selection, @@ -767,12 +757,16 @@ export function AccountSecretsRoute(handle: Handle) { ? isBulkPackageApproval ? `Approved package access for ${approval.names.length} secrets.` : 'Approved requested package.' - : 'Approved requested host.' + : isCapabilityApproval + ? 'Approved requested capability.' + : 'Approved requested host.' : approval.requestedPackageId ? isBulkPackageApproval ? 'Rejected bulk package approval request.' : 'Rejected package approval request.' - : 'Rejected host approval request.', + : isCapabilityApproval + ? 'Rejected capability approval request.' + : 'Rejected host approval request.', ) handle.update() @@ -1101,12 +1095,17 @@ export function AccountSecretsRoute(handle: Handle) { approval && !isRefreshingForLocationChange && !alreadyAddedNotice?.hostAlreadyAdded && + !alreadyAddedNotice?.capabilityAlreadyAdded && !alreadyAddedNotice?.packageAlreadyAdded ? approval : null const requestedPackageMetadata = approvalCard?.requestedPackageId ? packagesById.get(approvalCard.requestedPackageId) : null + const isCapabilityApprovalCard = + Boolean(approvalCard?.requestedCapability) && + !approvalCard?.requestedPackageId && + !approvalCard?.requestedHost return ( @@ -1192,6 +1191,13 @@ export function AccountSecretsRoute(handle: Handle) { ) : null} + ) : isCapabilityApprovalCard ? ( +

+ Requested capability:{' '} + {approvalCard.requestedCapability} for secret{' '} + {approvalCard.name} from the{' '} + {getScopeLabel(approvalCard.scope)} scope. +

) : (

Allow {approvalCard.requestedHost} to receive @@ -1199,7 +1205,7 @@ export function AccountSecretsRoute(handle: Handle) { {getScopeLabel(approvalCard.scope)} scope.

)} - {approvalCard.requestedCapability ? ( + {approvalCard.requestedCapability && !isCapabilityApprovalCard ? (

Requested capability:{' '} {approvalCard.requestedCapability} @@ -1241,6 +1247,13 @@ export function AccountSecretsRoute(handle: Handle) { )} ) + ) : isCapabilityApprovalCard ? ( +

+ Current allowed capabilities:{' '} + {approvalCard.currentAllowedCapabilities.length > 0 + ? approvalCard.currentAllowedCapabilities.join(', ') + : 'none'} +

) : (

Current allowed hosts:{' '} diff --git a/packages/worker/migrations/0073-community-forks-forked-package-index.sql b/packages/worker/migrations/0073-community-forks-forked-package-index.sql new file mode 100644 index 0000000000..002a137cf3 --- /dev/null +++ b/packages/worker/migrations/0073-community-forks-forked-package-index.sql @@ -0,0 +1,4 @@ +-- Lookup path for package secret access: self-authored packages have no +-- community_forks row; forked packages are found by forked_package_id. +CREATE INDEX IF NOT EXISTS idx_community_forks_forked_package_id +ON community_forks(forked_package_id); diff --git a/packages/worker/migrations/0074-community-fork-adoption.sql b/packages/worker/migrations/0074-community-fork-adoption.sql new file mode 100644 index 0000000000..7d1a4874f3 --- /dev/null +++ b/packages/worker/migrations/0074-community-fork-adoption.sql @@ -0,0 +1,5 @@ +-- Marks a community fork as reviewed/trusted by the forker. Provenance stays +-- (fork row remains); adopted forks get self-authored-like secret read/use +-- auto-grant. Mutations still require allowed_packages. +ALTER TABLE community_forks ADD COLUMN adopted_at TEXT; +ALTER TABLE community_forks ADD COLUMN adoption_note TEXT; diff --git a/packages/worker/src/app/account-secrets-data.ts b/packages/worker/src/app/account-secrets-data.ts index 305c06c15c..9748b27007 100644 --- a/packages/worker/src/app/account-secrets-data.ts +++ b/packages/worker/src/app/account-secrets-data.ts @@ -14,6 +14,7 @@ import { import { type SecretScope } from '#mcp/secrets/types.ts' import { normalizeBulkPackageSecretApprovalNames } from '#mcp/secrets/package-approval-url.ts' import { listSavedPackagesByUserId } from '#worker/package-registry/repo.ts' +import { parseAllowedCapabilityName } from '#mcp/secrets/allowed-capabilities.ts' import { normalizeAllowedHosts } from '#mcp/secrets/allowed-hosts.ts' type AuthenticatedUser = NonNullable< @@ -62,6 +63,7 @@ type SecretApprovalView = { requestedCapability: string | null requestedPackageId: string | null currentAllowedHosts: Array + currentAllowedCapabilities: Array currentAllowedPackages: Array } @@ -137,7 +139,9 @@ async function buildAccountSecretsPayload(input: { let approval: SecretApprovalView | null = null let approvalError: string | null = null - const hasApprovalTarget = Boolean(requestedApprovalHost || requestedPackageId) + const hasApprovalTarget = Boolean( + requestedApprovalHost || requestedPackageId || requestedCapability, + ) const hasApprovalSubject = Boolean( input.selectedSecretId || requestedSecretNames.length > 0, ) @@ -239,11 +243,19 @@ type ResolvedSecretApproval = packageId: string storageContext: StorageContext | null } + | { + kind: 'capability' + name: string + scope: SecretScope + capabilityName: string + storageContext: StorageContext | null + } function resolveApprovalRequest(input: { secretId: string | null requestedHost: string | null requestedPackageId: string | null + requestedCapability?: string | null requestedSecretNames?: Array }): ResolvedSecretApproval { const requestedSecretNames = normalizeBulkPackageSecretApprovalNames( @@ -302,7 +314,22 @@ function resolveApprovalRequest(input: { storageContext, } } - throw new Error('Approval request is missing a host or package.') + if (input.requestedCapability) { + const requestedCapability = parseAllowedCapabilityName( + input.requestedCapability, + ) + if (!requestedCapability) { + throw new Error('Invalid approval request capability.') + } + return { + kind: 'capability', + name: parsed.name, + scope: parsed.scope, + capabilityName: requestedCapability, + storageContext, + } + } + throw new Error('Approval request is missing a host, package, or capability.') } async function resolveSecretApprovalView(input: { @@ -319,6 +346,7 @@ async function resolveSecretApprovalView(input: { secretId: input.secretId, requestedHost: input.requestedHost, requestedPackageId: input.requestedPackageId, + requestedCapability: input.requestedCapability, requestedSecretNames: input.requestedSecretNames, }) if (approval.kind === 'package_bulk') { @@ -357,6 +385,7 @@ async function resolveSecretApprovalView(input: { requestedCapability: input.requestedCapability, requestedPackageId: approval.packageId, currentAllowedHosts: [], + currentAllowedCapabilities: [], currentAllowedPackages: [approval.packageId], } satisfies SecretApprovalView } @@ -370,6 +399,7 @@ async function resolveSecretApprovalView(input: { requestedCapability: input.requestedCapability, requestedPackageId: approval.packageId, currentAllowedHosts: firstSecret?.allowedHosts ?? [], + currentAllowedCapabilities: firstSecret?.allowedCapabilities ?? [], currentAllowedPackages: firstSecret?.allowedPackages ?? [], } satisfies SecretApprovalView } @@ -396,9 +426,13 @@ async function resolveSecretApprovalView(input: { names: [approval.name], scope: approval.scope, requestedHost: approval.kind === 'host' ? approval.requestedHost : '', - requestedCapability: input.requestedCapability, + requestedCapability: + approval.kind === 'capability' + ? approval.capabilityName + : input.requestedCapability, requestedPackageId: approval.kind === 'package' ? approval.packageId : null, currentAllowedHosts: secret.allowedHosts, + currentAllowedCapabilities: secret.allowedCapabilities, currentAllowedPackages: secret.allowedPackages, } satisfies SecretApprovalView } diff --git a/packages/worker/src/app/handlers/account-secrets.node.test.ts b/packages/worker/src/app/handlers/account-secrets.node.test.ts index 0f17152d45..4a9e988b9d 100644 --- a/packages/worker/src/app/handlers/account-secrets.node.test.ts +++ b/packages/worker/src/app/handlers/account-secrets.node.test.ts @@ -71,9 +71,16 @@ vi.mock('#mcp/secrets/allowed-hosts.ts', () => ({ ), })) -vi.mock('#mcp/secrets/allowed-capabilities.ts', () => ({ - normalizeAllowedCapabilities: (capabilities: Array) => capabilities, -})) +vi.mock('#mcp/secrets/allowed-capabilities.ts', async (importOriginal) => { + const actual = + await importOriginal< + typeof import('#mcp/secrets/allowed-capabilities.ts') + >() + return { + ...actual, + normalizeAllowedCapabilities: (capabilities: Array) => capabilities, + } +}) vi.mock('#mcp/secrets/host-approval.ts', () => ({ buildSecretHostApprovalUrl: (...args: Array) => @@ -571,6 +578,263 @@ test('account secrets payload includes all packages and package titles and allow }) }) +test('capability approval view, reject, approve, and dedupe mirror host/package flow', async () => { + mockModule.setSecretAllowedCapabilities.mockClear() + mockModule.setSecretAllowedHosts.mockClear() + mockModule.setSecretAllowedPackages.mockClear() + const handler = createAccountSecretsApiHandler(createEnv()) + const secret = { + name: 'cloudflareToken', + scope: 'user' as const, + description: 'Cloudflare token', + packageId: null, + allowedHosts: [], + allowedCapabilities: ['secret_get'], + allowedPackages: [], + createdAt: new Date(0).toISOString(), + updatedAt: new Date(0).toISOString(), + ttlMs: null, + } + + mockModule.listSecrets.mockResolvedValueOnce([secret]) + mockModule.listSecrets.mockResolvedValueOnce([secret]) + mockModule.listSavedPackagesByUserId.mockResolvedValueOnce([]) + mockModule.listPackageSecretsByPackageIds.mockResolvedValueOnce(new Map()) + + const viewResponse = await handler.handler({ + request: new Request( + 'https://example.com/account/secrets.json?selected=user::::cloudflareToken&capability=secret_set', + { method: 'GET' }, + ), + params: {}, + } as never) + + expect(viewResponse.status).toBe(200) + await expect(viewResponse.json()).resolves.toMatchObject({ + ok: true, + approval: { + name: 'cloudflareToken', + scope: 'user', + requestedHost: '', + requestedCapability: 'secret_set', + requestedPackageId: null, + currentAllowedCapabilities: ['secret_get'], + }, + }) + + mockModule.listSecrets.mockResolvedValueOnce([]) + mockModule.listSavedPackagesByUserId.mockResolvedValueOnce([]) + mockModule.listPackageSecretsByPackageIds.mockResolvedValueOnce(new Map()) + + const rejectResponse = await handler.handler({ + request: new Request( + 'https://example.com/account/secrets.json?selected=user::::cloudflareToken&capability=secret_set', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ action: 'reject' }), + }, + ), + params: {}, + } as never) + + expect(rejectResponse.status).toBe(200) + await expect(rejectResponse.json()).resolves.toMatchObject({ ok: true }) + expect(mockModule.setSecretAllowedCapabilities).not.toHaveBeenCalled() + expect(mockModule.setSecretAllowedHosts).not.toHaveBeenCalled() + expect(mockModule.setSecretAllowedPackages).not.toHaveBeenCalled() + + mockModule.listSecrets.mockResolvedValueOnce([ + { + ...secret, + allowedCapabilities: ['secret_get', 'secret_get'], + }, + ]) + mockModule.listSecrets.mockResolvedValueOnce([]) + mockModule.listSavedPackagesByUserId.mockResolvedValueOnce([]) + mockModule.listPackageSecretsByPackageIds.mockResolvedValueOnce(new Map()) + + const approveResponse = await handler.handler({ + request: new Request( + 'https://example.com/account/secrets.json?selected=user::::cloudflareToken&capability=secret_set', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ action: 'approve' }), + }, + ), + params: {}, + } as never) + + expect(approveResponse.status).toBe(200) + await expect(approveResponse.json()).resolves.toMatchObject({ ok: true }) + expect(mockModule.setSecretAllowedCapabilities).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'cloudflareToken', + scope: 'user', + allowedCapabilities: ['secret_get', 'secret_set'], + storageContext: { sessionId: null, appId: null, packageId: null }, + }), + ) + + mockModule.setSecretAllowedCapabilities.mockClear() + mockModule.listSecrets.mockResolvedValueOnce([ + { + ...secret, + allowedCapabilities: ['secret_set'], + }, + ]) + mockModule.listSecrets.mockResolvedValueOnce([]) + mockModule.listSavedPackagesByUserId.mockResolvedValueOnce([]) + mockModule.listPackageSecretsByPackageIds.mockResolvedValueOnce(new Map()) + + const dedupeResponse = await handler.handler({ + request: new Request( + 'https://example.com/account/secrets.json?selected=user::::cloudflareToken&capability=secret_set', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ action: 'approve' }), + }, + ), + params: {}, + } as never) + + expect(dedupeResponse.status).toBe(200) + await expect(dedupeResponse.json()).resolves.toMatchObject({ ok: true }) + expect(mockModule.setSecretAllowedCapabilities).toHaveBeenCalledWith( + expect.objectContaining({ + allowedCapabilities: ['secret_set'], + }), + ) +}) + +test('capability approval rejects invalid targets and defers to host when both are present', async () => { + mockModule.setSecretAllowedCapabilities.mockClear() + mockModule.setSecretAllowedHosts.mockClear() + const handler = createAccountSecretsApiHandler(createEnv()) + + const missingTargetResponse = await handler.handler({ + request: new Request( + 'https://example.com/account/secrets.json?selected=user::::cloudflareToken', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ action: 'approve' }), + }, + ), + params: {}, + } as never) + + expect(missingTargetResponse.status).toBe(400) + await expect(missingTargetResponse.json()).resolves.toMatchObject({ + ok: false, + error: 'Approval request is missing a host, package, or capability.', + }) + expect(mockModule.setSecretAllowedCapabilities).not.toHaveBeenCalled() + + mockModule.listSecrets.mockResolvedValueOnce([ + { + name: 'cloudflareToken', + scope: 'user', + description: 'Cloudflare token', + packageId: null, + allowedHosts: [], + allowedCapabilities: [], + allowedPackages: [], + createdAt: new Date(0).toISOString(), + updatedAt: new Date(0).toISOString(), + ttlMs: null, + }, + ]) + mockModule.listSecrets.mockResolvedValueOnce([]) + mockModule.listSavedPackagesByUserId.mockResolvedValueOnce([]) + mockModule.listPackageSecretsByPackageIds.mockResolvedValueOnce(new Map()) + + const hostPrecedenceResponse = await handler.handler({ + request: new Request( + 'https://example.com/account/secrets.json?selected=user::::cloudflareToken&allowed-host=api.cloudflare.com&capability=secret_set', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ action: 'approve' }), + }, + ), + params: {}, + } as never) + + expect(hostPrecedenceResponse.status).toBe(200) + await expect(hostPrecedenceResponse.json()).resolves.toMatchObject({ + ok: true, + }) + expect(mockModule.setSecretAllowedHosts).toHaveBeenCalled() + expect(mockModule.setSecretAllowedCapabilities).not.toHaveBeenCalled() +}) + +test('capability approval rejects junk and oversized capability names without policy change', async () => { + mockModule.setSecretAllowedCapabilities.mockClear() + const handler = createAccountSecretsApiHandler(createEnv()) + + const junkResponse = await handler.handler({ + request: new Request( + `https://example.com/account/secrets.json?selected=user::::cloudflareToken&capability=${encodeURIComponent('evil name