feat: add NewPluginPrimitiveSigner and KeySpecFromPlugin for raw-signature use cases

[dallasd1]

Add two helper functions to the signer package so external callers like the notation CLI can produce raw plugin-backed signatures and certificate chains without going through the JWS/COSE envelope path.

The default behavior in the signing path for JWS/COSE signing remains unchanged.

• signer.NewPluginPrimitiveSigner returns a signature.Signer that calls plugin.GenerateSignature and returns raw signature bytes + certificate chain

• signer.KeySpecFromPlugin calls plugin.DescribeKey and returns the decoded keySpec, enforcing that the plugin's response references the same keyID that was requested

[bketelsen]

Some blockers - KeyID validation is dropped from new public method, no input validation for NewPluginPrimitiveSigner()

[shizhMSFT]

Caution

AI-generated review. This review was produced by an AI agent (Claude Opus 4.7, shizh-reviewer skill) on behalf of @shizhMSFT. It is posted as COMMENT (not REQUEST_CHANGES) — please weigh findings on their merits and treat them as starting points, not gating concerns.

Thanks @dallasd1. notation-go is more flexed than notation-core-go, but exporting an internal type into the public API of the most-imported library in the Notary Project ecosystem still deserves careful thought. A few concerns and one design question.

Design question first

Do we actually need to export pluginPrimitiveSigner?

The stated reason is that the dm-verity flow lives in notaryproject/notation (the CLI) and needs a signature.Signer. Two alternatives that don't expand notation-go's public API:

1. Move the dm-verity command into a signer/dmverity subpackage of notation-go and call the existing pluginPrimitiveSigner internally. The CLI then composes via a public function like signer.NewDMVeritySigner(plugin, keyID, opts) that returns []byte directly. This keeps pluginPrimitiveSigner private.
2. Add a single narrow public function signer.SignPrimitive(ctx, plugin, keyID, payload, pluginConfig) ([]byte, []*x509.Certificate, error) that internally constructs and calls pluginPrimitiveSigner. The caller never holds the type; we can refactor later without breaking compat.

Both reduce the API surface from "a struct + constructor + helper + KeySpec method" to one function. Per clig.dev reasoning applied to libraries: public API surface is a permanent contract; we should add the smallest amount that solves the use case. Right now this PR exports more than is strictly needed (see comments below). Worth at least answering "did we consider these?" before merge.

Concerns (if we proceed with the export)

1. GetKeySpecFromPlugin drops the keyID round-trip check. The existing PluginSigner.getKeySpec (signer/plugin.go:155–157) compares s.keyID to descKeyResp.KeyID and rejects mismatches. The new helper does not. A malicious or buggy plugin can return a different key's spec; the caller will then sign with the wrong spec/algorithm. This is the kind of TOCTOU-adjacent issue that has bitten supply-chain tooling before.
2. NewPluginPrimitiveSigner performs no input validation. NewPluginSigner (signer/plugin.go:66–78) rejects nil plugin and empty keyID. The new constructor accepts both and only fails later, deep inside Sign(). It also accepts an arbitrary signature.KeySpec value from the caller without sanity-checking it — the suggestion below adds local validation via proto.HashAlgorithmFromKeySpec. Note: this is purely a "did you pass me a valid (Type, Size) pair?" check; it does not cross-validate the keySpec against what the plugin would report — for that, the caller must use KeySpecFromPlugin first.
3. *PluginPrimitiveSigner returned from constructor. Constructors that return concrete pointers commit us to every exported field/method/struct-tag. Return signature.Signer (the interface the type already implements). Less to maintain; same usefulness.

Non-blocking design tension

• context.Context baked into the public struct. The original code already does this internally, so exporting it doesn't introduce the anti-pattern — it propagates it. Sign(payload []byte) ignores its surroundings and uses s.ctx, so timeouts/cancellations from the calling goroutine cannot reach the plugin RPC. Resolving this properly (either passing ctx to Sign(ctx, payload) or hiding the ctx behind constructor scope) means changing signature.Signer, which is out of scope for this PR. Worth flagging as a tracking issue for the next signing-API revision.

Naming

• GetKeySpecFromPlugin → KeySpecFromPlugin (Go style: no Get prefix on getters; cf. https://go.dev/wiki/CodeReviewComments#getters)
• NewPluginPrimitiveSigner is fine, but consider NewSignaturePrimitiveSigner or just folding into NewPluginSigner as a mode flag. "PluginPrimitive" reads as two adjectives stacked; users will get it wrong.

Verdict: COMMENT (AI review only — not gating).

[Copilot]

Pull request overview

This PR exports a previously-internal “primitive” plugin-backed signer so external callers (e.g., notation CLI) can generate raw signatures + certificate chains for formats like PKCS#7, while keeping the existing JWS/COSE signing flow unchanged.

Changes:

• Export PluginPrimitiveSigner (formerly unexported) and wire it into the existing plugin signing path.
• Add helper APIs NewPluginPrimitiveSigner() and KeySpecFromPlugin() for external consumers.
• Extend plugin signer unit tests to cover the new exported signer and helper behaviors.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
signer/plugin.go	Exports the primitive signer type and adds helper constructor / key-spec retrieval function for external PKCS#7-style signing flows.
signer/plugin_test.go	Updates mocks and adds new tests for NewPluginPrimitiveSigner and KeySpecFromPlugin.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dallasd1]

Thanks for the review @bketelsen! Your feedback should be addressed concerning the validation checks and test coverage.

[dallasd1]

Thanks for the feedback @shizhMSFT! The comments have been addressed along with unexporting PluginPrimitiveSigner in favor of returning signature.Signer