Add principal-scoped secret access contract

[shivanshu-risa]

Summary

• add a host-implemented secret access API bound to plugins and exact MCP tools
• separate metadata listing from explicit plaintext retrieval
• add a trusted human-only grant manager contract with owner-only mutations
• expose the scoped providers through PluginContext and sandbox delegation

Verification

• ./gradlew build
• ./gradlew apiCheck

Merge order

This is the foundation PR. Merge it first so release v1.0.94 is available to the host, AI Gateway, and Secret Manager PRs.

[claude]

Review: principal-scoped secret access contract

Nice, carefully-argued contract — the "principal is never an argument" rule, the not-found/not-authorized conflation in getSecret, and the split between SecretAccessProvider and SecretGrantManager are the right shapes for this. Conventions check out: both files end with a newline, no Compose/Android APIs are introduced, the new PluginContext members are nullable with get() = null defaults, build.gradle.kts is correctly left at 1.0.93 so CI's bump-push cuts 1.0.94, and the api/ dump is self-consistent with the sources (mangled Result suffixes, $stable, synthetic ctors and $default statics all line up). I could not run ./gradlew apiCheck in this environment, so I'm trusting your verification there.

The substantive comments below are mostly "now or never": everything here is frozen the moment 1.0.94 publishes.

1. searchSecrets and createSecret clash with SecretDataProvider (SecretAccessProvider.kt:42, :62)

Both interfaces now declare searchSecrets(query: String, limit: Int, offset: Int) and createSecret(request: CreateSecretRequestData) with the same Kotlin parameter lists but different return types (Result<PaginatedSecretsData> vs Result<PaginatedAccessibleSecrets>, Result<Unit> vs Result<String>). A single host class therefore cannot implement both interfaces — Kotlin rejects it on the return type, and the JVM names collide too (the -gIAlu-s / -BWLJW6A suffixes depend on arity plus the Result return, not on the type argument, which is why deleteSecret and deleteOwnedSecret already share a suffix in the dump).

That's workable with two separate implementation objects, but it's an avoidable trap for the host and the Secret Manager PR, and renaming also fixes the naming asymmetry inside this interface — updateOwnedSecret/deleteOwnedSecret are explicit while createSecret/searchSecrets are not. Suggestion: createOwnedSecret, searchAccessibleSecrets (and optionally listAccessibleSecrets/getAccessibleSecret). Free now, a new-overload-forever problem after release.

2. getSecret hands back more than a plaintext value (SecretAccessProvider.kt:54)

AccessibleSecretMetadata's KDoc says "Notes, passwords, recovery codes, and TOTP seeds are intentionally absent", but getSecret returns the full SecretEntryData, which carries notes, metadata.twofaSecret and metadata.recoveryCodes. So a human granting an MCP tool use of one login also hands it the 2FA seed and the recovery codes — i.e. the whole second factor, permanently, since recovery codes don't rotate on revoke. For a surface whose whole premise is least privilege that seems worth an explicit decision rather than inheriting it from the reused type:

• either return a narrower type (AccessibleSecretValue with website/username/password only, plus a separate getSecretTotp-style call for principals that genuinely need it), or
• document in getSecret that it returns the complete record including second-factor material, so the grant UI can warn the human accordingly.

SecretEntryData is a released data class and can never gain or lose components, so the narrower type has to be introduced here if it's wanted at all.

3. Frozen data classes are missing fields the grant UI will want (SecretAccessProvider.kt:138, :111)

SecretPrincipalGrantData has no secretId (so a UI holding rows from several listGrants calls can't tell them apart without keeping the key alongside), no display name (the grant list must join against listPrincipals, and a principal that has since been uninstalled renders as a raw id — exactly the row a human most needs to recognise in order to revoke it), and no access level, while AccessibleSecretMetadata.accessLevel already anticipates that the policy will grow. grantSecret likewise has no level parameter. Per the repo's own rule that data classes never evolve across the boundary, adding secretId, displayName: String? = null and accessLevel: String = "use" now — and a defaulted accessLevel on grantSecret — costs nothing; adding them post-release costs a whole new envelope type.

4. Missing changelog block above version in build.gradle.kts

Every release since at least 1.0.85 documents itself in the comment block above the version line, including the gating story ("New types only: ApiClassLoader/minApiVersion, no host release" for 1.0.92). This PR adds none, and it's the entry that most needs one because the gating here is mixed: the four new types and two new interfaces resolve from the api jar (minApiVersion 1.0.94), but PluginContext.secretAccessProvider/secretGrantManager are new members on a @HostImplemented type and so need minBossVersion for whichever BossConsole release lands the host PR. Worth stating explicitly for the AI Gateway and Secret Manager PRs that will consume it.

5. The "returns null until then" guidance is not what happens below the floor (PluginContext.kt:225, :241)

Until then it returns null and callers must leave secret-backed features unconfigured

Per AGENTS.md, a plugin in ai.rever.boss.plugin.* that names a new member of a @HostImplemented type on a host that lacks it is rejected wholesale by the host's BinaryCompatibilityValidator — it doesn't degrade to null at the call site, and a plugin-side null check or try/catch doesn't help. Null is only what you get from a host that has the member and hasn't wired an implementation. Suggest rewording to say that, and pointing at the optional-adapter pattern (Toolbox's downloadCenterProvider, 1.0.85) for anyone who wants one build to span the floor. Also, the secretGrantManager KDoc says nothing about gating at all, though it has the same minBossVersion requirement.

6. Consider @HostImplemented on the new data carriers

PaginatedAccessibleSecrets, AccessibleSecretMetadata, SecretPrincipalData and SecretPrincipalGrantData are all constructed by the host, so the host compiles them in and serves its pinned copies parent-first — which is exactly what the annotation documents ("any type the host compiles in, whoever implements it"). Precedent: BrokerInfo, BrokeredCredential, TransferInfo, AiAvailableModel. Without the marker, a later reader may reasonably assume a field addition is jar-only/minApiVersion when it actually needs a host release.

7. No tests (src/test/kotlin/ai/rever/boss/plugin/api/)

New API surfaces in this repo generally ship with a contract test — SecretDataProviderDefaultsTest, DownloadCenterTypesTest, AiModelPricingTest. A small SecretAccessTypesTest would pin the things that are binary contract and easy to drift: the limit = 50, offset = 0 defaults on both paging methods, the expirationDate = null / tags = emptyList() / description = null data-class defaults, and a fake implementation asserting that getSecret can return Result.success(null) (the nullable-inside-Result shape is the kind of thing worth having one compiled witness for). Cheap, and test.yml already runs ./gradlew build.

8. How does the host recognise the trusted Secret Manager context? (SecretAccessProvider.kt:95)

The KDoc's guarantee is that secretGrantManager is non-null only for "the trusted Secret Manager UI context". If the host decides that from the manifest plugin id, then a sideloaded plugin declaring that same id gets the grant manager and can grant every vault secret to a principal it controls — the one call in this API that escalates privilege. Not something this PR can enforce, but since the KDoc is the normative statement of the boundary, it's worth saying how the check must be made (store provenance / signature, not manifest id alone) so the host PR implements it that way. Same question, lower stakes, for how an mcp_tool principal id is bound to an invocation.

9. Revocation vs plaintext already handed out (SecretAccessProvider.kt:19-21)

"Revoking a grant takes effect on the next operation. Implementations must not expose a long-lived plaintext cache that outlives authorization" reads as a guarantee, but it can only bind the host: once getSecret returns, the plugin holds the plaintext and revocation can't claw it back. Suggest saying that plainly, plus a caller-facing "refetch per use, never persist". This repo already has the stronger pattern for exactly this problem — BrokeredCredentialProvider (short-lived, host-minted) and BrowserHandle.fillCredentials (use without reading). If a future "use without reading" path for agent/MCP callers is on the roadmap, a sentence here saying getSecret is the fallback rather than the preferred path would set expectations before consumers build on it.

Nits

• AccessibleSecretMetadata (:84) and SecretPrincipalData (:130) put defaulted parameters before required ones, so the defaults are only reachable with named arguments — matches SecretEntryData's existing shape, so fine if intentional, just noting the synthetic ctor is in the dump forever.
• accessLevel and principalType are open strings: worth documenting the canonical case ("owner" vs "OWNER") and that consumers must treat an unrecognised value fail-closed (least privilege), not as owner.
• limit/offset have no documented clamping or negative-value behaviour. Consistent with SecretDataProvider, so optional, but this surface is the one where a hostile caller supplies them.
• The PR description mentions "sandbox delegation", but there's nothing sandbox-related in this diff — I assume that lives in the host PR; worth correcting so reviewers don't go looking for it here.

Nothing above is a correctness bug in the code as written — the items I'd most want resolved before this publishes are (1), (2) and (3), since they're the ones that can't be fixed after 1.0.94 ships.

[claude]

Review: principal-scoped secret access contract (1/2)

Contract-only change and the security shape is the right one. What's good: the principal is not a parameter on any method, so untrusted code can't impersonate one — and the KDoc says why, which is the part that survives future edits. getSecret returning success(null) for both "missing" and "unauthorized" closes the id-probing oracle. Metadata listing (no password/notes/TOTP in AccessibleSecretMetadata) is genuinely separated from plaintext retrieval, and grant administration is a separate interface from use. updateOwnedSecret/deleteOwnedSecret put the ownership requirement in the name, so a grant can't be mistaken for write access. Evolution hygiene is right too: both interfaces @HostImplemented, new types plus new PluginContext members only, no released data class touched, apiDump regenerated in the same commit, version left at 1.0.93 for release CI to bump, new file ends with a newline.

Findings ordered by how expensive they are to fix after release.

1. PluginContext KDoc states the wrong failure mode below the floor

PluginContext.kt:227 — "Until then it returns null and callers must leave secret-backed features unconfigured."

Per this repo's own doctrine that isn't what happens. PluginContext is @HostImplemented (PluginContext.kt:42) and served parent-first from the host's pinned copy, so on a host predating the property the getter does not exist: reading it throws NoSuchMethodError, and the host's BinaryCompatibilityValidator member-checks every ai.rever.boss.plugin.* class in the jar and rejects the whole plugin before it runs. Compare downloadCenterProvider (PluginContext.kt:122-141), which says outright "the failure is not null", and projectSearchProvider (PluginContext.kt:177-190).

This matters because the wording invites the one mitigation that doesn't work (if (context.secretAccessProvider != null)), and the plugin that writes it won't load at all on an older host. Suggest mirroring the downloadCenterProvider note on both members: name both floors (minApiVersion for the types, minBossVersion for the property), say that below the Boss floor the plugin is rejected rather than getting null, and point at the out-of-package + catch (LinkageError) adapter shape. Keep the "don't fall back to secretDataProvider" sentence — good instruction, wrong premise under it.

2. createSecret and searchSecrets collide with SecretDataProvider after erasure

Byte-identical signatures in the regenerated dump, different Kotlin return types:

5677 SecretAccessProvider.createSecret-gIAlu-s   (CreateSecretRequestData;Continuation;)Object  // Result<String>
5693 SecretDataProvider.createSecret-gIAlu-s     (CreateSecretRequestData;Continuation;)Object  // Result<Unit>
5682 SecretAccessProvider.searchSecrets-BWLJW6A  (String;II;Continuation;)Object  // Result<PaginatedAccessibleSecrets>
5705 SecretDataProvider.searchSecrets-BWLJW6A    (String;II;Continuation;)Object  // Result<PaginatedSecretsData>

Same name, same mangling suffix, same descriptor — so no single class can implement both interfaces: Kotlin reports an inherited-declaration clash and no override satisfies both contracts. If the host PR planned to let its existing secret provider also serve the scoped surface, it needs a separate object or adapter. That may be the intent anyway (a principal-bound instance probably shouldn't be the same object), but it's an unstated constraint a downstream PR hits at compile time. Cheap now, impossible later: createOwnedSecret and searchAccessibleSecrets, which also read better beside updateOwnedSecret/deleteOwnedSecret.

3. SecretPrincipalGrantData.grantedByUserId is non-null, but owner rows have no granter

SecretAccessProvider.kt:138-145 — listGrants returns "Immutable creator ownership plus grants", and accessLevel = "owner" means the bound principal created the secret. No granting human exists there, so the host must put something untrue in a non-null String: "", or the vault owner's id, which then renders in Secret Manager as if a human granted it. Data-class constructors are frozen binary contracts here (SecretDataProvider.kt:146-148 says exactly that), so this is the only moment to change it: val grantedByUserId: String? = null plus a KDoc line that null means creator ownership. Settle grantedAt on owner rows the same way (creation time seems fine — just say it).

4. Grants are permanent-until-revoked with no room to add expiry

Same frozen-constructor argument, forward-looking: no expiresAt on SecretPrincipalGrantData, no expiry parameter on grantSecret (SecretAccessProvider.kt:111-116). If time-boxed grants ever become desirable ("let this MCP tool use this token for an hour"), the only additive route left is a parallel envelope type — the tax SecretEntryWithAccessData pays today. expiresAt: String? = null costs nothing now and null means no expiry; a later method with a default body can carry the grant-with-expiry call. If unbounded grants are deliberate policy, say so in the KDoc.

[claude]

Review: principal-scoped secret access contract (2/2)

5. Open-string enums with no constants

accessLevel (owner/use) and principalType (plugin/mcp_tool) as open strings is the right call given sealed hierarchies can't evolve across this boundary — but with no constants the host, AI Gateway and Secret Manager PRs each hardcode the literals, and nothing says whether comparison is exact, case-insensitive, or trimmed. One typo becomes a silent authorization mismatch that can fail open on a != "owner" check.

object SecretAccessLevel { const val OWNER = "owner"; const val USE = "use" }
object SecretPrincipalType { const val PLUGIN = "plugin"; const val MCP_TOOL = "mcp_tool" }

const val inlines into the consumer's bytecode, so it needs no runtime resolution and costs nothing below any floor. Also document exact-lowercase comparison and what an unrecognized value means (treat as no access).

6. No tests, against a consistent repo convention

Comparable contract additions here all ship one: SecretDataProviderDefaultsTest, ProjectSearchProviderDefaultsTest, DownloadCenterTypesTest, AiModelPricingTest, EditorTabPluginDefaultsTest. PRs run ./gradlew build, so this is the layer that would have caught finding 2, and it pins the defaults three downstream PRs are about to depend on. A SecretAccessProviderDefaultsTest covering: a minimal PluginContext implementation returns null for both new members (the graceful-degradation contract); listSecrets()/searchSecrets("q") reach a recording fake as limit = 50, offset = 0; AccessibleSecretMetadata defaults (expirationDate == null, tags.isEmpty()) and accessLevel == "use" on a default-constructed grant; and getSecret for an unauthorized id is isSuccess with a null value rather than a failure — the distinction the whole no-oracle design rests on.

7. Missing 1.0.94 changelog entry in build.gradle.kts

AGENTS.md makes build.gradle.kts the source of truth "(the changelog comment above version documents each release)", and every recent release has a block there — the 1.0.92 entry is the model: what's new, why that shape, and "New types only: ApiClassLoader/minApiVersion, no host release". Here the floor lives only in the PR description, and that same entry's story about the number moving twice while a branch sat unmerged is itself the argument for writing it into the file.

Related: the new KDoc says "the API release that introduces SecretAccessProvider" rather than a number, while DiffTabConfig.kt:26, ProjectSearchProvider.kt:43 and AiModelPricing.kt:177 all name theirs. Put minApiVersion 1.0.94 in the KDoc; minBossVersion can stay prose until that release exists, with a TODO so it isn't forgotten.

8. Semantics the three downstream PRs must agree on, currently unspecified

• grantSecret on an already-granted (principal, secret) pair — idempotent success or failure? revokeSecret on a nonexistent grant — same question. UIs retry these.
• grantSecret with a principal absent from listPrincipals() must fail (the "grant to an uninstalled plugin id" hole).
• What Result.failure means on getSecret. Null is missing-or-unauthorized; add that failure is reserved for store/transport errors, so no caller renders a failure as "access denied" or as "allowed but broken".
• limit bounds — whether the host clamps, and to what. SecretDataProvider has the same gap, but a new surface is the place to fix it: an agent-driven caller will pass limit = 100000.
• Whether getSecret is audited. The KDoc requires revocation to bite "on the next operation" and bans a long-lived plaintext cache, which implies per-call authorization; saying whether that call is also logged tells the host implementer what to build.

9. Minor

• SecretGrantManager and its two types (SecretAccessProvider.kt:102+) are a distinct human-only, trusted-UI-only surface living in a file named for the other interface; a SecretGrantManager.kt would be more discoverable. Optional.
• Naming: neighbours use the Data suffix (PaginatedSecretsData, SecretEntryData, SecretShareData), and the new envelope types drop it while the new principal types keep it. Type names freeze at release too.
• SecretAccessProvider.kt:88-89: expirationDate/tags have defaults but are followed by three required params, so they're usable only with named arguments. Host-produced type — move them last or drop the defaults.
• secretGrantManager access control is entirely host-side (PluginContext.kt:241), as the KDoc acknowledges. For the host PR: key it on the trusted internal context/plugin identity and never on anything manifest-declared (a manifest is plugin-authored input), and have Secret Manager treat null as "not trusted" and hide grant UI rather than render a half-working one.
• The description mentions "sandbox delegation" but this diff only touches PluginContext, and no sandbox-delegating context exists in this repo — so that lands host-side. Flagging only so it isn't assumed done by this merge: if the host's sandbox context delegates provider-by-provider, both members need adding there or they read as null inside the sandbox.

Overall: a well-reasoned model whose docs carry the reasoning, which is what matters for a contract other PRs build on. I'd fix 1-3 before merge — 1 because a wrong failure mode in KDoc produces rejected plugins, 2 and 3 because method names and data-class constructors freeze the moment this releases. 6 and 7 are the repo conventions this would otherwise be the exception to.

Note: I could not run ./gradlew build/apiCheck in this environment, so dump consistency was reviewed by reading rather than verified by execution; finding 2 is read directly off the committed dump.

🤖 Generated with Claude Code

[claude]

Review: principal-scoped secret access contract

Read the full diff (api/boss-plugin-api.api, PluginContext.kt, SecretAccessProvider.kt, build.gradle.kts) against AGENTS.md. The core design is the strongest part of this PR and worth naming before the findings: the principal is not an argument to any method, and the KDoc says why ("accepting a plugin id, tool name, or MCP name from the caller would let untrusted code impersonate another principal"). getSecret collapsing not-found and not-authorized into the same null closes the id-probing oracle. accessLevel/principalType as open strings is the right call given the "never evolve sealed hierarchies across the boundary" rule, and new envelope types instead of new components on SecretEntryData follows the rule the neighbouring file already documents. No Compose/Android surface here, and files end with a newline.

Findings below, most consequential first. I could not run ./gradlew build/apiCheck in this environment, so I am relying on your verification and CI for those; the .api dump entries do match what the Kotlin in the diff would emit (synthetic constructors, the -gIAlu-s/-0E7RQCE/-BWLJW6A Result manglings, $stable, the DefaultImpls holder for the defaulted-arg methods).

1. The manual version bump to 1.0.94 is probably off by one, and downstream PRs are pinning to it

AGENTS.md and the changelog block in build.gradle.kts both say the same thing: "Release CI bump-pushes before building, so main's version below is the version already released and this merge cuts the next one." main is at 1.0.93, so merging this PR without commit 3 releases 1.0.94. With commit 3, CI bump-pushes to 1.0.95 and the published release is 1.0.95 -- while the PR description and the merge-order plan tell the host, AI Gateway, and Secret Manager PRs to gate on minApiVersion 1.0.94.

That failure mode is not soft. A consumer declaring minApiVersion: 1.0.94 installs happily on a host whose newest api jar is 1.0.94 (which would contain none of these types), and then BinaryCompatibilityValidator fails to resolve ai/rever/boss/plugin/api/SecretAccessProvider in the constant pool and rejects the whole plugin -- no per-call-site degradation.

Suggest dropping commit 3 and reading the floor off the actual published tag before the dependent PRs hard-code it.

2. secretGrantManager's KDoc is missing the gating note, and neither new member names its numbers

PluginContext is @HostImplemented, so both new properties resolve from the host's pinned copy, parent-first. secretAccessProvider documents both floors; secretGrantManager documents only the trust rule. The Secret Manager plugin will be the first consumer, and

val grants = context.secretGrantManager ?: return   // does NOT save you

is exactly the null check AGENTS.md warns is useless here -- below the implementing minBossVersion, resolving the getter is what fails and the plugin is rejected outright. Worth adding the same two-floor paragraph, and once the release number is settled, naming the numbers the way the neighbours do (projectSearchProvider says "minApiVersion 1.0.87", downloadCenterProvider likewise) rather than "the API release that introduces...". These docs are where consumers copy their manifest floors from.

3. getSecret hands back notes and TOTP material, which the rest of the design deliberately withholds

AccessibleSecretMetadata's doc is explicit: "Notes, passwords, recovery codes, and TOTP seeds are intentionally absent." But getSecret returns Result<SecretEntryData?>, and SecretEntryData carries notes, plus metadata: SecretMetadataData? with twofaSecret and recoveryCodes.

So the metadata surface is scrubbed and the retrieval surface is not. A human clicking "grant use of my GitHub login to this MCP tool" in the grant picker is consenting to a credential; they are unlikely to read that as "...and my free-form notes, my TOTP seed, and my recovery codes". For an owner row (the principal created the secret) that is fine -- it is the use-grant path where it over-delivers.

This is the cheapest possible moment to narrow it, since a data class crossing this boundary can never gain components after release:

/** Plaintext for an authorized secret. Deliberately narrower than [SecretEntryData]. */
data class AccessibleSecretValue(
    val id: String,
    val website: String,
    val username: String,
    val password: String,
    val expirationDate: String? = null,
    val tags: List<String> = emptyList(),
)

If reusing SecretEntryData is a considered decision (fewer types, one conversion less in the host), please say so in the KDoc -- that a use grant conveys notes and 2FA material -- so the grant UI can word its confirmation honestly.

4. Owner rows in SecretPrincipalGrantData are under-specified (commit 2)

listGrants now returns "immutable creator ownership plus grants", but the type still describes a granted row:

• grantedAt: String and grantedByUserId: String are non-null, and for an owner row there is no granting human. One host will write "", another the vault owner's id, another the creation timestamp -- and the Secret Manager UI cannot tell which. Either make grantedByUserId: String? (free now, a hard break later) or pin it in the doc: "for owner rows, grantedAt is the secret's creation time and grantedByUserId is the vault owner".
• Nothing says what revokeSecret does when handed an owner row. The prose says ownership is immutable, so presumably it fails -- but the UI rendering this list has no field telling it which rows are revocable, so it will draw a revoke button on an owner row and surface an error the user cannot act on. A revocable: Boolean = true, or a documented "revoking an owner row always fails", closes that.
• Naming: the type now models owner rows too, so something like SecretPrincipalAccessData reads truer. Renameable today, frozen after release.

5. No tests, and this repo has a convention for exactly this shape of change

src/test/kotlin/ai/rever/boss/plugin/api/ has SecretDataProviderDefaultsTest, ProjectSearchProviderDefaultsTest, DownloadCenterTypesTest and a dozen more. ProjectSearchProviderDefaultsTest's own KDoc states the reason they exist: default values are part of the contract and apiCheck cannot see one change.

This PR ships a security-relevant default -- accessLevel: String = "use" on SecretPrincipalGrantData. Flip that literal to "owner" in a future tidy-up and every grant row silently claims immutable creator access, with no signature change for apiCheck to catch. Worth a small SecretAccessProviderDefaultsTest pinning:

• a minimal object : PluginContext { } returns null for secretAccessProvider and secretGrantManager -- the fail-closed default;
• listSecrets/searchSecrets pass limit = 50, offset = 0 through verbatim (same trick as ProjectSearchProviderDefaultsTest: capture what the impl sees);
• SecretPrincipalGrantData(...).accessLevel == "use", AccessibleSecretMetadata(...).tags.isEmpty(), expirationDate == null.

6. Missing changelog comment for this release

AGENTS.md: "the changelog comment above version documents each release." The block in build.gradle.kts currently ends at the 1.0.92 notes, and commit 3 moves the number without adding an entry. That block is where the gating split gets recorded, and this change has a split worth recording explicitly: the five new types are jar-only (minApiVersion, ApiClassLoader, no host release), but the two new PluginContext properties are not -- they need a BossConsole release and minBossVersion. Consumers of the next three PRs will read that comment.

Smaller notes

• @HostImplemented on the new data carriers. BrokeredCredentialProvider annotates BrokerInfo and BrokeredCredential, since the host compiles the whole api package in via plugin-api-core. The four new data classes here are unannotated (as are SecretDataProvider's carriers, so precedent is mixed) -- worth a conscious call rather than an accident, given the annotation is what tells the next author these constructors are frozen.
• Do not let the error channel undo the probe defence. getSecret returning null for both not-found and not-authorized is good; add that the Result.failure message must not distinguish them either, or the oracle just moves.
• limit is unbounded. listSecrets(limit = Int.MAX_VALUE) is a tool-driven full-view dump; worth documenting that the host clamps, and to what.
• Grants have no expiry or scope. Adding a parameter to a @HostImplemented interface method later is a host-release change, so now is when it is cheap to decide whether expiresAt: String? belongs on grantSecret -- a grant to an MCP tool for one task is a natural thing to want time-boxed.
• The PR description mentions "sandbox delegation" but there are no sandbox file changes in this diff. Presumably host-side -- worth trimming the description, or double-checking that whatever delegating PluginContext wrapper exists there overrides both new properties, since an un-overridden delegate inherits the null default and the feature silently never appears.
• File organisation. Two interfaces plus three data classes in SecretAccessProvider.kt is within repo norm (DownloadCenterProvider.kt holds five), but SecretGrantManager is a different trust tier -- a separate SecretGrantManager.kt would make "this one is human-UI-only" harder to miss when someone skims for a provider to use.

None of 2-6 block the design; 1 is the one I would resolve before merge, since three other PRs are about to pin a number to it.

🤖 Generated with Claude Code

[shivanshu-risa]

Closing this draft to keep the current rollout limited to the AI Gateway and Secret Manager plugins. Principal-scoped secret access will be redesigned and handled in a separate security-hardening round.