plugins contract: use cli arguments instead of json as input

[qmuntal]

The plugin extensibility spec defines the plugin cli input as a json file, such as:

{
  "contractVersion" : "<major-version.minor-version>",

  // Complete key definition from
  // /notation/config.json /signingKeys/keys with matching key name
  "keyDefinition" : {  
    "name": "mysigningkey",  
    "id" : "keyid",
    "plugin": "com.example.nv2plugin"
  },

  "payload" : "<Base64 encoded payload to be signed>"
}

I'm not convinced that using json as input format is better than just using cli arguments, such as:

notation-{plugin-name}.exe generate-signature --contract 1.0 --key-name "akey" --key-id "1234" {payload}

notation-{plugin-name}.exe generate-envelope --contract 1.0 --key-name "akey" --key-id "1234" \
  --payload-type "application/vnd.oci.descriptor.v1+json" \
  --sig-envelop-type "application/vnd.cncf.notary.v2.jws.v1" \
  {payload}

Using plain cli arguments have the following advantages:

• Manually crafting cli arguments is much easier than crafting a json, which means better debugging and troubleshooting experience.
• Arguments are easier to document than json schemas, at least in CLI contexts. Again, better troubleshooting.
• Decoupling the configuration data model from the plugin contract. Passing the whole keyDefinition as a json object is a bad business, as a change in how we store our config could mean breaking some plugins.

@SteveLasker @gokarnm @shizhMSFT

[SteveLasker]

Thanks @qmuntal. We briefly discussed in the Notary v2 meeting today. @rgnote, @gokarnm and @shizhMSFT were going to review.

[gokarnm]

@qmuntal that is an interesting idea. The think the current approach is more appropriate though as

• The plugin executable is not intended to be a CLI, it is expected to work in conjunction with notation and not execute standalone. We settled on an external executable with stdin/out as request response contract, as a substitute to reflection, or any other dynamic load module and execute feature available in other languages, as Go does not have a cross platform equivalent.
• Passing request parameters using command line arguments has the following drawbacks
  • Various OS have different limits for command line input (seems to be 32KB on windows and 2MB for linux)
  • There may be some implications with how easily command line input can be automatically captured and logged vs. stdin/stdout. This is currently not an issue though as we don't pass any sensitive data like raw keys over this channel.
• Stand alone testing and debugging of plugin can set the request in a file and pipe it to the plugin myplugin generate-signature < request.json
• We can change the encoding of request/response in future. Ideally I'd like to move from JSON to something like CBOR, that does not require use to base-64 encode binary content, for better support for passing binary payload through request and signatures through response.

Decoupling the configuration data model from the plugin contract makes a lot of sense, it allows us to evolve the config without breaking contract, I'm open to ideas here. The simplest approach, maybe to just pass a map with some provision for custom config data for the plugin (current key config does not support that yet).

@shizhMSFT any feedback?

[qmuntal]

Passing request parameters using command line arguments has the following drawbacks
Various OS have different limits for command line input (seems to be 32KB on windows and 2MB for linux)
There may be some implications with how easily command line input can be automatically captured and logged vs. stdin/stdout. This is currently not an issue though as we don't pass any sensitive data like raw keys over this channel.

The first drawback is indeed a hard limitation, but do you think we will ever reach even 1KB input? The largest component of the payload is the digested content, which should be at most 32 bytes when using SHA512.

There may be some implications with how easily command line input can be automatically captured and logged vs. stdin/stdout. This is currently not an issue though as we don't pass any sensitive data like raw keys over this channel.

Do you envision if we will ever pass sensitive data?

I can see that using stdin instead of cli arguments is a meditated design decision, and I can agree with it. Yet, I still want to give on more data point to reinforce the idea that it hurts debugging, at least for plugin authors: neither VSCode Go extension nor most the popular Go debugger, go-delve, support debugging programs which read from stdin: microsoft/vscode-go#219 and go-delve/delve#1274.

Edit: The previous issues with vscode and go-delve are not debugging blockers, as there are multiple hacks on could do to overcome it.

[qmuntal]

Decoupling the configuration data model from the plugin contract makes a lot of sense, it allows us to evolve the config without breaking contract, I'm open to ideas here. The simplest approach, maybe to just pass a map with some provision for custom config data for the plugin (current key config does not support that yet).

What about this:

// signing request
{
   "contractVersion": "<contract version>",
   "keyName": "<key name>",
   "keyId": "<key id>",
   "payload": "<base64 string>",
}

// signing envelope request
{
   "contractVersion": "<contract version>",
   "keyName": "<key name>",
   "keyId": "<key id>",
   "payload": "<base64 string>",
   "payloadType": "<payload type>",
   "signatureEnvelopeType": "<signature envelope type>",
}

[gokarnm]

The first drawback is indeed a hard limitation, but do you think we will ever reach even 1KB input? The largest component of the payload is the digested content, which should be at most 32 bytes when using SHA512.

The payload for image signing are small, as they are detached from the actual image payload. We want to keep the option open to sign arbitrary payloads in future which are not limited to OCI artifacts.

There may be some implications with how easily command line input can be automatically captured and logged vs. stdin/stdout. This is currently not an issue though as we don't pass any sensitive data like raw keys over this channel.

Do you envision if we will ever pass sensitive data?

We won't as part of plugin contract, but we intend to support pass through config for plugins, for users to pass additional plugin specific configuration as CLI params or in key config alongside plugin name. These may contain sensitive data (say kms authentication details), even though we'll advice users/plugin developers not to pass sensitive data.

I can see that using stdin instead of cli arguments is a meditated design decision, and I can agree with it. Yet, I still want to give on more data point to reinforce the idea that it hurts debugging, at least for plugin authors: neither VSCode Go extension nor most the popular Go debugger, go-delve, support debugging programs which read from stdin: microsoft/vscode-go#219 and go-delve/delve#1274.

Edit: The previous issues with vscode and go-delve are not debugging blockers, as there are multiple hacks on could do to overcome it.

Got it. One workaround would be for the plugin developer to support alternate way to pass request that does not rely on stdin (say a --file CLI argument outside of the plugin spec) for most tests. Though plugin developers will still have to run some integration tests which exercises the stdin request path.

[gokarnm]

Decoupling the configuration data model from the plugin contract makes a lot of sense, it allows us to evolve the config without breaking contract, I'm open to ideas here. The simplest approach, maybe to just pass a map with some provision for custom config data for the plugin (current key config does not support that yet).

What about this:

// signing request
{
   "contractVersion": "<contract version>",
   "keyName": "<key name>",
   "keyId": "<key id>",
   "payload": "<base64 string>",
}

// signing envelope request
{
   "contractVersion": "<contract version>",
   "keyName": "<key name>",
   "keyId": "<key id>",
   "payload": "<base64 string>",
   "payloadType": "<payload type>",
   "signatureEnvelopeType": "<signature envelope type>",
}

Looks good, I'd additionally include a map for pass through config for plugin itself.

// key definition for plugin with additional config
"keyDefinition" : {  
    "name": "mysigningkey",  
    "id" : "keyid",
    "plugin": "com.example.nv2plugin"
    "pluginConfig" : {
       "kms-region" : "asia-1"
     }
  }

 // signing request
 {
    "contractVersion": "<contract version>",
    "keyName": "<key name>",
    "keyId": "<key id>",
     "pluginConfig" : {  // map string-string
           "kms-region" : "asia-1"
      }
    "payload": "<base64 string>"
 }

[gokarnm]

@qmuntal can we close this issue?

[qmuntal]

@qmuntal can we close this issue?

Yes!