Skip to content

fix(ociplugins): try every configured key when the command names none - #891

Merged
lonelycode merged 1 commit into
mainfrom
fix/oci-unpinned-try-all-keys
Oct 28, 2026
Merged

lonelycode merged 1 commit into
mainfrom
fix/oci-unpinned-try-all-keys

Conversation

@lonelycode

Copy link
Copy Markdown
Member

Problem

A plugin installed from the marketplace (any oci:// command without ?pubkey=) was checked only against the first default key, the embedded Tyk key. A third-party plugin signed with a key the operator had added (OCI_PLUGINS_PUBKEY_*) failed verification unless the admin edited the install command to add ?pubkey=<ID>. Found while vetting the Next Moca plugin (TykTechnologies/tyk-ai-studio-plugins-ce#2).

Admins should still have to add third-party keys explicitly; they shouldn't have to change the install command.

Change

  • Unpinned verification (pkg/ociplugins/client.go): with no ?pubkey=, each configured default key is tried in order, the Tyk key first, for the index and for the platform-manifest fallback. The key that verified is recorded in the cache entry and logged as public_key. If none verifies, the error names every key tried.
  • Cache hits: an unpinned load accepts a cached entry only while its recorded key is still configured, so removing a key stops the plugins it verified from loading. A pinned load still requires the pinned key. Entries from earlier releases (no key recorded) are served to unpinned loads as before.
  • ?pubkey= keeps its single-key behaviour and now also restricts a plugin to that one key.
  • Community Edition (no signature enforcement) is unchanged.
  • Docs: plugins-security.md, plugins-deployment.md, features/PluginSecurity.md ("Which key is tried"), features/PluginMarketplace.md, microgateway/docs/oci-plugin-workflow.md, and the 2.3.0 changelog.

Tests

New pkg/ociplugins/unpinned_keys_test.go:

  • A plugin signed with an added key installs without ?pubkey=; a Tyk-signed one tries only the Tyk key.
  • A pinned load doesn't try other keys; when no key verifies, the error names each one.
  • Removing a key revokes its cached plugin; pinned and unpinned cache entries are kept apart; legacy entries are served.
  • The platform-manifest fallback works with an added key.
  • TestSignatureKeys and TestDefaultKeyIDsResolve.

go test ./pkg/ociplugins/... ./services/plugin_security/... passes after rebasing on main; the config, services (OCI/plugin) and microgateway internal/config and internal/services tests passed before the rebase.

Bead: midsommar-clean-ucxi. Follow-up filed as midsommar-clean-0388 (record the key's fingerprint, not only its name).

🤖 Generated with Claude Code

Marketplace installs (and any oci:// command without ?pubkey=) were only
checked with the first default key, the embedded Tyk key, so a plugin
signed with a key the operator added (OCI_PLUGINS_PUBKEY_*) failed unless
its install command was edited.

An unpinned check now tries each configured default key in order, the
Tyk key first, for the index and then the platform-manifest fallback, and
records the key that verified. A cache hit without ?pubkey= accepts the
entry while that key is still configured, so removing a key revokes the
plugins it verified; entries from earlier releases ("" recorded) stay
valid. ?pubkey= keeps its single-key behaviour. The Community Edition
path is unchanged.

Bead: midsommar-clean-ucxi

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@probelabs

probelabs Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

This PR fixes an issue where OCI plugins installed without a specified public key (e.g., from the marketplace) were only checked against the first configured key. The change updates the signature verification logic to try every configured public key in order until one succeeds. This allows administrators to trust third-party plugins by adding the publisher's key to the configuration, without needing to modify the plugin's installation command.

Additionally, the key that successfully verifies a plugin is now recorded in the cache. When loading a plugin from the cache, this recorded key is checked against the current configuration. This ensures that if a public key is removed from the configuration, any plugins that were verified by it are effectively revoked and will not be loaded from the cache.

Files Changed Analysis

The changes are well-structured across logic, testing, and documentation:

  • Core Logic (pkg/ociplugins/client.go): The main implementation is here. The signature verification flow is modified to iterate through multiple keys for unpinned plugins. It also introduces logic to record the successful key in the cache and validate it on subsequent loads.
  • New Tests (pkg/ociplugins/unpinned_keys_test.go): A comprehensive new test file has been added to cover the new behaviors, including multi-key verification, cache revocation when a key is removed, and handling of legacy cache entries.
  • Configuration (pkg/ociplugins/config.go): Comments for PluginMetadata have been updated to reflect the new role of SignatureKey.
  • Documentation (6 files): The CHANGELOG and documentation for plugin deployment, security, the marketplace, and the microgateway have all been updated to explain the new, more flexible verification behavior.

Architecture & Impact Assessment

  • What this PR accomplishes: It improves the user experience for installing third-party OCI plugins by allowing signature verification against a set of configured keys, rather than just one. This is particularly useful for marketplace plugins where the installation command is standardized.

  • Key technical changes introduced:

    1. Iterative Verification: For plugins without a ?pubkey= parameter, the system now attempts verification with each configured default public key in sequence (starting with the embedded Tyk key).
    2. Cache Metadata Enhancement: The ID of the public key that successfully verifies a plugin is now stored in the plugin's cache metadata.
    3. Cache Invalidation Logic: When loading a cached plugin, the system now checks if the key it was verified with is still configured. If the key has been removed, the cache entry is considered invalid, forcing a re-fetch and re-verification.
  • Affected system components: The primary impact is on the OCI Plugin Management System, which is a shared component used by both AI Studio and the Microgateway. Any part of the system responsible for loading, caching, or verifying OCI plugins is affected by this change.

Verification Flow

sequenceDiagram
    participant User
    participant OCIClient
    participant Verifier
    participant Cache

    alt Old Flow (Unpinned Plugin)
        User->>OCIClient: FetchPlugin("oci://...")
        OCIClient->>Verifier: VerifySignature(digest, defaultKey[0])
        Verifier-->>OCIClient: Fails (if not signed by first key)
        OCIClient-->>User: Error
    end

    alt New Flow (Unpinned Plugin)
        User->>OCIClient: FetchPlugin("oci://...")
        OCIClient->>OCIClient: signatureKeys() → [key1, key2, ...]
        loop For each key
            OCIClient->>Verifier: VerifySignature(digest, key)
            alt Verification Succeeds
                Verifier-->>OCIClient: Success
                OCIClient->>Cache: StoreMetadata(..., verifiedByKey=key)
                OCIClient-->>User: Success
                break
            end
        end
        alt All keys failed
            OCIClient-->>User: Error (naming all keys tried)
        end
        end
    end

Loading

Scope Discovery & Context Expansion

The changes are concentrated in the pkg/ociplugins package, which serves as a library for OCI plugin handling. The impact of this change extends to all consumers of this library within the codebase, namely AI Studio and the Microgateway, as they both rely on it for fetching and running plugins. The new behavior is governed by the existing OCI_PLUGINS_PUBKEY_* environment variables, making it an enhancement to the existing security configuration framework.

This change improves security posture by introducing a mechanism to revoke trust. By removing a public key from the environment configuration, administrators can now ensure that any cached plugins signed with that key will no longer be loaded, which was not previously possible.

Metadata
  • Review Effort: 3 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-10-28T21:03:14.620Z | Triggered by: pr_opened | Commit: 57d3fef

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs

probelabs Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

Performance Issues (2)

Severity Location Issue
🟡 Warning pkg/ociplugins/client.go:411-422
The `verifyDigest` function sequentially attempts signature verification with each configured public key. Signature verification is a computationally expensive (CPU-bound) operation. If a large number of public keys are configured, this sequential process can introduce significant latency during plugin loading, especially if the correct key is at the end of the list or if verification fails for all keys.
💡 SuggestionFor a small number of keys, the current sequential approach is acceptable. However, to make this more robust against a larger number of keys, consider parallelizing the signature verification. You could launch a goroutine for each key and use a channel to receive the result, returning as soon as the first successful verification completes. This would reduce the worst-case latency from O(N) to roughly O(1) in terms of verification time (plus goroutine overhead).
🟡 Warning pkg/ociplugins/client.go:434
The `signatureKeyAccepted` function uses `slices.Contains` to check if a recorded key exists in the `DefaultPublicKeys` slice. This is an O(N) operation, where N is the number of default public keys. This function is called when loading a plugin from the cache. While the number of keys is currently expected to be small, using a more efficient data structure for lookups is a best practice to prevent potential performance degradation if the number of keys grows.
💡 SuggestionTo optimize this lookup, convert the `DefaultPublicKeys` slice into a map (set) when the `OCIPluginClient` is initialized. This would change the lookup from O(N) to O(1). For example, add a `defaultPublicKeysSet map[string]struct{}` to the `OCIPluginClient` struct, populate it once, and then use it for fast lookups in `signatureKeyAccepted`.
\n\n

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

\n\n

Performance Issues (2)

Severity Location Issue
🟡 Warning pkg/ociplugins/client.go:411-422
The `verifyDigest` function sequentially attempts signature verification with each configured public key. Signature verification is a computationally expensive (CPU-bound) operation. If a large number of public keys are configured, this sequential process can introduce significant latency during plugin loading, especially if the correct key is at the end of the list or if verification fails for all keys.
💡 SuggestionFor a small number of keys, the current sequential approach is acceptable. However, to make this more robust against a larger number of keys, consider parallelizing the signature verification. You could launch a goroutine for each key and use a channel to receive the result, returning as soon as the first successful verification completes. This would reduce the worst-case latency from O(N) to roughly O(1) in terms of verification time (plus goroutine overhead).
🟡 Warning pkg/ociplugins/client.go:434
The `signatureKeyAccepted` function uses `slices.Contains` to check if a recorded key exists in the `DefaultPublicKeys` slice. This is an O(N) operation, where N is the number of default public keys. This function is called when loading a plugin from the cache. While the number of keys is currently expected to be small, using a more efficient data structure for lookups is a best practice to prevent potential performance degradation if the number of keys grows.
💡 SuggestionTo optimize this lookup, convert the `DefaultPublicKeys` slice into a map (set) when the `OCIPluginClient` is initialized. This would change the lookup from O(N) to O(1). For example, add a `defaultPublicKeysSet map[string]struct{}` to the `OCIPluginClient` struct, populate it once, and then use it for fast lookups in `signatureKeyAccepted`.
\n\n ### ✅ Quality Check Passed

No quality issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2026-10-28T21:03:11.432Z | Triggered by: pr_opened | Commit: 57d3fef

💡 TIP: You can chat with Visor using /visor ask <your question>

@lonelycode
lonelycode merged commit 1d8dc51 into main Oct 28, 2026
23 checks passed
@lonelycode
lonelycode deleted the fix/oci-unpinned-try-all-keys branch October 28, 2026 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant