Enforce execution-principal secret access

[shivanshu-risa]

Summary

• add plugin and exact-MCP-tool ownership to encrypted secrets while leaving existing ownerless rows human/browser-only
• add explicit human use grants, immutable creator ownership, owner-only mutations, and immediate revocation
• bind scoped providers in PluginContext and narrow captured providers during MCP dispatch
• remove broad secret and secret-related Supabase surfaces from ordinary plugin contexts
• route AI settings search to the AI Gateway panel

Verification

• targeted host compile and desktop tests for scoped providers, Supabase blocking, and settings search
• composeApp and plugin-sandbox ktlint checks
• pgTAP regression suite added for user isolation, plugin/tool isolation, grants, revocation, and ACLs
• local pgTAP execution requires the Supabase Docker database; the CLI was available but localhost:54322 was not running

Dependencies and merge order

Depends on risa-labs-inc/boss-plugin-api#62 and its v1.0.94 release. Merge this after that API PR.

[supabase]

Updates to Preview Branch (codex/secret-principal-enforcement) ↗︎

Deployments	Status	Updated
Database	✅	Wed, 16 Sep 2026 04:08:57 UTC
Services	✅	Wed, 16 Sep 2026 04:08:57 UTC
APIs	✅	Wed, 16 Sep 2026 04:08:57 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks	Status	Updated
Configurations	✅	Wed, 16 Sep 2026 04:09:06 UTC
Migrations	✅	Wed, 16 Sep 2026 04:09:13 UTC
Seeding	✅	Wed, 16 Sep 2026 04:09:13 UTC
Edge Functions	✅	Wed, 16 Sep 2026 04:09:19 UTC

---

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

[shivanshu-risa]

Stacked dependency: merge boss-plugin-api#62 and wait for the v1.0.94 release before rerunning this PR's host build.

[github-actions]

Claude diff review of 501235f (no tests executed).

Review of PR #749 @ 501235f

Scope note: this is a diff-only read. I could not resolve PluginContext, SupabaseDataProvider, update_secret, create_secret, can_access_secret, or the existing test fixtures, so several items below are explicitly flagged as unverified.

---

Confirmed defects

1. supabaseDataProvider became by lazy — a null delegate is cached forever

composeApp/.../TrackingPluginContext.kt (~L381)

override val supabaseDataProvider: SupabaseDataProvider? by lazy {
    delegate.supabaseDataProvider?.let { provider -> ... }
}

The previous form was get() = delegate.supabaseDataProvider, re-read on every access. With by lazy, the first access wins permanently. If delegate.supabaseDataProvider is null at first touch (pre-auth, pre-Supabase-init, signed-out window), every later access returns null even after the delegate is populated. The wrapping itself doesn't need caching — wrap in a get(), or memoize keyed on delegate identity.

Same pattern risk in secretAccessProvider by lazy (~L357): SecretExecutionPrincipal.plugin(pluginId) has require(id.isNotBlank()), so a blank pluginId turns this into an IllegalArgumentException thrown from a property getter at an arbitrary later point, rather than at context construction.

2. MCP narrowing replaces the principal, so a tool loses its own plugin's secrets

composeApp/.../mcp/McpToolRegistryImpl.kt (~L1044) + SecretAccessProviderImpl.principal() (~L84)

private suspend fun principal(): SecretExecutionPrincipal =
    currentCoroutineContext()[SecretExecutionPrincipalContext]?.principal ?: pluginPrincipal

Inside a tool handler the principal becomes mcp_tool:<provider>/<tool> only. The DB check (execution_principal_can_use_secret) matches a single principal, so any secret the plugin created for itself (execution_owner_type='plugin') is unreadable during its own MCP tool execution. The pgTAP test at execution_principal_secrets_test.sql:~120 actually asserts this exclusion for sibling tools; the plugin→tool case has the same effect and no fallback/union. A plugin shipping both a panel and MCP tools will silently start failing secret reads unless a human grants every tool separately. Either union the plugin+tool principals in the predicate, or document this as required behaviour.

Corollary in the other direction: if a handler hands work to a scope created outside the invocation (pluginScope.launch, GlobalScope), the context element is dropped and the call silently widens back to the plugin principal. The failure mode of losing the element is "more access", which is the wrong default for a security boundary.

3. SecretExecutionPrincipalContext is public and unvalidated — principal impersonation

composeApp/.../SecretAccessProviderImpl.kt (~L20-35)

data class SecretExecutionPrincipal(val type: String, val id: String) { ... }
class SecretExecutionPrincipalContext(val principal: SecretExecutionPrincipal) : ...

Both the data class and the context element have public constructors, and principal() accepts whatever element is present with no check that it is a narrowing of the bound principal and no check that it was installed by the host. Any code that can reference (or reflectively construct) these host classes can do

withContext(SecretExecutionPrincipalContext(SecretExecutionPrincipal.plugin("ai-gateway"))) { … }

and read another principal's secrets through its own provider instance. The KDoc claim "Never constructed from plugin arguments" / "installed by trusted host dispatchers" is not enforced by the type system. Make the element's constructor internal (and even then, use an unexported token object as the key holder), and reject elements that are not derived from pluginPrincipal. Whether a dynamic plugin can link against composeApp classes depends on the loader's class filtering, which I could not verify from this diff — but reflection over a public class in the app classloader is the cheap path.

4. create_execution_secret can silently produce an ownerless secret

supabase/migrations/20260916000000_execution_principal_secrets.sql (~L214-228)

created_id := (result->>'secret_id')::uuid;
UPDATE public.secrets
SET execution_owner_type = p_principal_type, execution_owner_id = p_principal_id
WHERE id = created_id AND user_id = auth.uid();
RETURN result;

No ROW_COUNT/existence check. If create_secret assigns a different user_id (shared/org creation path), returns a differently-named key, or the row is otherwise not matched, the function returns success: true and the caller gets a secret id it can never read, update, or delete — and which is now an invisible ownerless row. Raise/return failure when FOUND is false.

5. The update path can wipe optional fields and disable 2FA

composeApp/.../ExecutionSecretService.kt (update, requestParameters)

mutation("update_execution_secret") { principalParameters(principal); put("p_secret_id", request.secretId); requestParameters(request) }
...
request.notes?.let { put("p_notes", it) }
if (request.tags.isNotEmpty()) put("p_tags", …)
put("p_twofa_enabled", request.twofaEnabled)
if (request.recoveryCodes.isNotEmpty()) put("p_recovery_codes", …)

Omitted parameters fall back to the SQL defaults (NULL, false), which update_execution_secret forwards verbatim to public.update_secret(...). Unless update_secret treats NULL as "leave unchanged" (I could not verify its body), a plugin updating only the password will clear notes, clear tags, clear recovery codes and set twofa_enabled = false. Also note p_password is always sent, so a partial update always rewrites the password with whatever the caller has. This needs either verified NULL-means-unchanged semantics in update_secret or explicit "unchanged" sentinels — plus a pgTAP test (see §13).

6. getPluginAPI gating only matches the exact Class object

composeApp/.../TrackingPluginContext.kt (~L579)

when (apiClass) {
    SecretDataProvider::class.java -> secretDataProvider?.let(apiClass::cast)
    …
    else -> delegate.getPluginAPI(apiClass)
}

when (apiClass) is reference equality on the Class. If the underlying registry resolves by isInstance/assignability (the usual implementation), a plugin can request any other type the real provider object satisfies — a sub-interface, the concrete impl class, or a broad supertype — and the else branch hands back the unfiltered delegate object, bypassing the boundary this override exists to create. The check should be assignability-based in both directions, e.g. reject when apiClass.isAssignableFrom(SecretDataProvider::class.java) || SecretDataProvider::class.java.isAssignableFrom(apiClass), and likewise for SupabaseDataProvider. (Unverified: the delegate's lookup semantics.)

7. SecretSafeSupabaseDataProvider is a substring deny-list on a non-exhaustive wrapper

composeApp/.../TrackingPluginContext.kt (~L641-676)

private fun String.isSecretSurface(): Boolean {
    val normalized = lowercase().filter { it.isLetterOrDigit() || it == '_' }
    return normalized.contains("secret") || normalized.contains("credential")
}

Three separate problems:

• False negatives. Anything sensitive not spelled "secret"/"credential" passes: vault.*, api_keys, tokens, passwords, *_encrypted, try_decrypt_text, safe_decrypt_recovery_codes. A deny-list here is the wrong shape for a security control; an allow-list (or a host-side registry of plugin-callable RPCs) is.
• False positives / regression. Every plugin previously had unfiltered select/rpc. Any existing plugin reading a table or calling an RPC whose name merely contains "credential" (e.g. a brokered-credential lookup) now fails with SecurityException, with no logging and no allow-list escape. There's no inventory in the diff of what breaks.
• Wrapper completeness. Only select and rpc are overridden. If SupabaseDataProvider has (or later gains) any other member — including a default-implemented one — the wrapper silently doesn't forward it (functional break) or doesn't filter it (security hole). The test's RecordingSupabase suggests the interface currently has exactly two members, but that's an inference, and the construction is not fail-closed against future additions.

8. The DB trusts the client-supplied principal

supabase/migrations/...sql (~L352-374)

GRANT EXECUTE ON FUNCTION public.list_execution_secrets(text,text,text,integer,integer) TO authenticated, service_role;
GRANT EXECUTE ON FUNCTION public.get_execution_secret(text,text,uuid) TO authenticated, service_role;

p_principal_type/p_principal_id are ordinary arguments from an authenticated session. The column comment says "Never accepted from an untrusted plugin UI" (~L30), but the RPC accepts it from any holder of the user's JWT. So the entire plugin/tool boundary is enforced only inside the host process. A plugin that can reach the session token (authDataProvider, or its own HTTP client) can impersonate any principal and read plaintext for every secret the user can access — the SecretSafeSupabaseDataProvider filter in §7 doesn't apply to direct HTTP. This may be an accepted trade-off, but it is not what the migration comment asserts, and it should be stated explicitly in AGENTS.md next to the new guarantees ("Grants never confer update or delete rights" etc.), because it bounds all of them.

Related: get_execution_secret returns plaintext to a machine principal with no audit row. list_secret_execution_grants shows who was granted, never who used it.

9. granted_by FK has no ON DELETE action

supabase/migrations/...sql (~L54)

granted_by uuid NOT NULL REFERENCES auth.users(id),

Default NO ACTION. A grant made by user A on a secret owned/shared by user B blocks deletion of user A's auth row (the cascade from secrets won't reach it). ON DELETE CASCADE or a nullable column with SET NULL is the usual fix.

10. hasMore is wrong on an exact-boundary page

composeApp/.../SecretAccessProviderImpl.kt (~L70-78)

hasMore = it.size >= bounded

A final page that happens to be exactly bounded rows reports hasMore = true, so callers make one extra empty round trip (and any UI driven by it shows a phantom "load more"). Fetch bounded + 1 and trim, or return a count.

11. CI is gated on an unpublished artifact

gradle/libs.versions.toml (~L175-179): boss-plugin-api = "1.0.94" with the comment "The API PR must merge and publish before this host PR can pass CI." This is self-declared as a hard merge-order dependency; the new secretAccessProvider / secretGrantManager / SecretAccessProvider / SecretGrantManager / SecretPrincipalData / AccessibleSecretMetadata / PaginatedAccessibleSecrets symbols all come from it. Nothing in this diff can compile until then.

Also plugin-platform/plugin-api-core/build.gradle.kts inserts the flat-layout local jar path first in siblingPaths, so a stale local boss-plugin-api-1.0.94.jar in a dev workspace now takes precedence over the other layouts. Low impact, but it is a priority change, not just an addition.

---

Uncertain observations (need local verification)

12. Missing withContext import. McpToolRegistryImpl.kt (~L1044) now calls withContext(...); the diff adds only the two SecretExecutionPrincipal* imports. If kotlinx.coroutines.withContext isn't already imported in that file, this doesn't compile.

13. SettingsSearchIndexShapeTest query list. SettingsSearchIndexShapeTest.kt:150-156 only changed the asserted panel to AI_GATEWAY; the loop's query list is outside the hunk. SettingsSearchEntries.kt removed the keywords "secret manager" and "vault" (~L195). If either string is in that query list, the test now fails. Please confirm.

14. PanelIds.AI_GATEWAY = PanelId("ai-gateway", 25) (PanelIds.kt:26). Every neighbouring entry uses 2 (SECRET_MANAGER, ADMIN_ROLE_MANAGEMENT, ROLE_CREATION). 25 looks like a typo or a semantically different second argument; whatever that parameter means (version? min version? ordinal?), the inconsistency deserves a comment or a fix.

15. metadata decoding for secrets without metadata. get_execution_secret returns COALESCE(<jsonb_build_object…>, '{}'::jsonb) (~L180-190), which Kotlin decodes into SecretEntry.metadata. If SecretMetadata's fields (twofa_enabled etc.) lack @Serializable defaults, every getSecret on a secret with no secret_metadata row throws during decode and the whole call fails. Returning SQL NULL instead of '{}' would be safer. Similarly, if SecretEntry has any non-default field not in the RPC's column list (user_id, can_manage, …), decoding fails.

16. list_execution_secrets scans the whole secrets table. (~L130-146) The only predicate is public.execution_principal_can_use_secret(s.id, …), a per-row function call containing its own EXISTS subqueries, with no index-usable pre-filter (no user_id, no execution_owner_*). At any real table size this is a full scan + N function calls per list/search. The new idx_secrets_execution_owner index can't be used by this plan.

17. Principal id built by string concatenation. SecretExecutionPrincipal.mcpTool (SecretAccessProviderImpl.kt:~26) produces "$providerId/$toolName", and secretPrincipalCatalog (TrackingPluginContext.kt:~424) duplicates that format independently. Two issues: (a) if either format changes, grants silently stop matching — the catalog should call SecretExecutionPrincipal.mcpTool(...).id; (b) if providerId or toolName may contain /, ids are ambiguous (a + b/c collides with a/b + c), so a grant can land on the wrong tool. Worth a delimiter-safe encoding or a validation assert.

18. Plugin-id-based trust. HUMAN_SECRET_PROVIDER_PLUGINS / SECRET_MANAGER_PLUGIN_ID (TrackingPluginContext.kt:~72-78) grant the broad SecretDataProvider and the grant manager purely on a string match against the plugin's own declared id. If nothing verifies plugin identity (signature, install provenance) before pluginId reaches this context, a sideloaded plugin declaring ai.rever.boss.plugin.dynamic.secretmanager gets the full human vault and SecretGrantManager, i.e. it can grant itself every secret. Please confirm where pluginId originates and whether it's attested.

19. Sandbox path. SandboxedPluginContext.kt (~L131-135) just forwards secretAccessProvider/secretGrantManager to delegate. That's correct only if every sandboxed plugin's delegate is a TrackingPluginContext; it also does not wrap supabaseDataProvider. And if the sandbox marshals calls across a process boundary, the coroutine-context narrowing from §2 cannot survive the hop — in which case sandboxed MCP tools fall back to the plugin principal.

20. Silent denials. secretDataProvider returns null for non-allowlisted plugins with no log line, and SecretSafeSupabaseDataProvider returns Result.failure(SecurityException(...)) whose message may surface in plugin UI. For a security boundary, denials should be logged (sanitized) at minimum; right now a plugin breaking after this change produces no host-side evidence.

21. ILIKE wildcards in p_query are unescaped (~L143). Not an escalation (the predicate still gates rows), but %/_ in a search string behave as wildcards.

22. SecretGrantManagerImpl.grantSecret requires a currently-loaded principal (SecretAccessProviderImpl.kt:~170) — filter { it.isEnabled && it.healthy && !it.isIncompatible } in secretPrincipalCatalog. A user cannot pre-grant to a temporarily disabled plugin, and the error is a bare IllegalArgumentException("Unknown secret principal"). Deliberate? revokeSecret correctly does not check.

23. execution_principal_can_use_secret grant model (~L98-100): revoked from authenticated/service_role, granted only to postgres. This works only because the SECURITY DEFINER callers share an owner that retains implicit EXECUTE. If migrations run as supabase_admin rather than postgres in some environment, the explicit GRANT … TO postgres is dead and correctness rests entirely on owner-default privileges. Fine, but fragile enough to note.

---

Test gaps

The riskiest logic in this PR is untested by the diff:

• No test of the TrackingPluginContext gate at all — the central security decision. Missing: secretDataProvider == null for an ordinary plugin; non-null for secretmanager/fluckbrowser; secretGrantManager == null for everyone but Secret Manager; getPluginAPI(SecretDataProvider::class.java) blocked; getPluginAPI(SupabaseDataProvider::class.java) returns the wrapped provider for ordinary plugins and the raw one for Secret Manager.
• No test of the MCP registry narrowing. SecretAccessProviderImplTest (SecretAccessProviderImplTest.kt:26-38) simulates the context element by hand; nothing exercises McpToolRegistryCore.executeUncapped, so a regression that drops the withContext wrapper (or the element being lost across a dispatcher/scope hop) would pass CI. Also untested: the widening fallback in §2.
• No negative test for impersonation (§3) — e.g. that a plugin-installed context element for a different principal is rejected.
• ExecutionSecretService is entirely untested — the JSON parameter mapping, including the field-omission behaviour in §5, and the MutationResponse.success == false → Result.failure path.
• pgTAP: no owner happy-path for update/delete. execution_principal_secrets_test.sql:80-95 only asserts that a use grant is refused. An argument-order or arity mistake in public.update_secret(p_secret_id, …, NULL, false) (~L268) or public.delete_secret would not be caught. Add: owner updates succeed and the changed field round-trips; owner deletes succeed.
• pgTAP: no test for create_execution_secret when the ownership UPDATE matches nothing (§4), for the unauthenticated branches, for p_limit/p_offset clamping, or for p_query filtering.
• SecretSafeSupabaseDataProviderTest (SecretSafeSupabaseDataProviderTest.kt:14-33) tests only names that already contain the magic substrings. It would be more useful as a test over the actual set of secret-bearing tables/RPCs in the schema (vault.decrypted_secrets, secret_metadata, secret_tags, safe_decrypt_*, try_decrypt_text), which is where the deny-list's false negatives live.
• SecretGrantManagerImpl has no test for the unknown-principal rejection or the distinctBy/sort behaviour.

---

Documentation

AGENTS.md (~L536-563) now asserts guarantees that the code only partially delivers: "Ownerless legacy secrets are invisible to execution principals until a human explicitly grants use" is true at the RPC layer but not against direct HTTP with the user's JWT (§8), and "bound to the calling plugin or exact MCP tool" is bound only by an unauthenticated, plugin-forgeable coroutine context element (§3). The deleted paragraph about no version floor on secret-manager also leaves an unanswered question for the new arrangement: the AI-provider settings signpost now points at PanelIds.AI_GATEWAY and is filtered on that panel being registered (SettingsSearchEntries.kt:~185), so users without the ai-gateway plugin installed get "No matching settings" for api key/anthropic/claude again — the exact failure the signpost was introduced to prevent. Worth stating whether that's accepted.

[shivanshu-risa]

Closing this draft to keep the current rollout limited to the AI Gateway and Secret Manager plugins. Host-level secret hardening will be redesigned and handled separately.