Dashboard: EventListeners, Interceptors: option to NOT show CEL interceptor details

[bitsofinfo]

If a CEL interceptor handles a sensitive value (i.e. like in a comparison) its exposed in the clear in the dashboard. Might be nice to have an option to obfuscate or toggle displaying on/off interceptor details when the dashboard is installed.

[a-roberts]

Yeah this is a good idea, so a few things about how we could implement this

1) Option at install time?
2) A switch on the page itself?
3) Something more intelligent, like having a known sensitive field name and if you use that we won't display the values?

Open to more suggestions too, @AlanGreene FYI

[AlanGreene]

If the concern is around surfacing sensitive information in the UI we would also need to disable the raw YAML view of the EventListener.

I wonder if there's any guidance in tektoncd/triggers on how to handle sensitive values like this (i.e. so they can be referenced safely rather than inlining them)?

[bigkevmcd]

One possibility, might be to add an extension function "secretValue(ns, name, key)" which would find a named secret, and return the string value.

[a-roberts]

One possibility, might be to add an extension function "secretValue(ns, name, key)" which would find a named secret, and return the string value.

Yeah 👍 this sounds good to me, and I'm wondering if this is specific to just using CEL style interceptors (surely not)

[bigkevmcd]

@bitsofinfo can you describe the specifics of what you're matching on?

[bitsofinfo]

was thinking same thing as @bigkevmcd something that can let you inject a legit k8s secret into an expression (by name) via some little function like that.

use-case, a shared secret/string that is simply conveyed via a header between the remote hook that is calling the EventListener w/ JSON and the CEL interceptors that validate it.

[dibyom]

I wonder if there's any guidance in tektoncd/triggers on how to handle sensitive values like this (i.e. so they can be referenced safely rather than inlining them)?

For secret values, we usually take a reference to a k8s secret (like in GitHub/Gtlab interceptors).

use-case, a shared secret/string that is simply conveyed via a header between the remote hook that is calling the EventListener w/ JSON and the CEL interceptors that validate it.

This is almost exactly what the Gitlab interceptor does...wonder if it makes sense to have a generic shared secret interceptor like that

[bigkevmcd]

@dibyom that would be fairly easy to do, the question is "what algorithm"?

Some are HMAC, some are simple constant-time string comparisons.

Something vaguely like this:

apiVersion: tekton.dev/v1alpha1
kind: EventListener
metadata:
  name: listener-interceptor
spec:
  serviceAccountName: tekton-triggers-example-sa
  triggers:
    - name: foo-trig
      interceptors:
        - sharedsecret:
            header: X-My-Header
            secret:
              name: my-secret
              namespace: my-ns
              key: interceptor-secret
            algorithm: hmac | compare

The complexity comes with algorithm.

[dibyom]

Indeed 😄 For GitLab, its just a string compare (constant time recommended). GitHub/Slack compute a HMAC of the payload and the secret.

Also, It seems like whatever changes we make here would probably go in Triggers and not in dashboard...does it make sense to move this issue to Triggers? @a-roberts @bitsofinfo ?

[a-roberts]

Discussed on the WG call today, we decided for now we should hide this and @AlanGreene did we agree on removing the yaml view button too?

This is until there's an awesome mechanism for changes in Triggers (so @dibyom in response to your question I think a new issue for Triggers should be raised linking to the suggestions here, and this issue remains a dashboard issue for us to handle not exposing one's overlays 😄 )

[AlanGreene]

did we agree on removing the yaml view button too?

yes for now, seems like the safest thing to do

[Megan-Wright]

/assign

[Megan-Wright]

@AlanGreene how does this look for now as a temporary solution until triggers implements a way of knowing whats secure and whats not?

[AlanGreene]

Discussed on slack, we'll hide all details for CEL interceptors as the filter could also contain sensitive values. Other interceptors will continue to be displayed as normal.

[Megan-Wright]

/close

[tekton-robot]

@Megan-Wright: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[bigkevmcd]

I feel it's missing something if you can't see the CEL interceptor details.

I implemented https://github.com/tektoncd/triggers/pull/492