Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-63366

withCredentials masking should filter before other log filters

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      The implementation of the withCredentials step performs credentials masking by using a ConsoleLogFilter instance that is merged with any existing filters into the context of that step. This merging is done via BodyInvoker.mergeConsoleLogFilters(), though it is currently done by adding the credentials masking log filter as a subsequent filter. By nesting multiple withCredentials steps, the outer-most step's log filter will apply to the logs first even though the inner-most step's log filter should logically take precedence. For example, suppose we have some pipeline snippet:

      withCredentials([string(credentialsId: '...', variable: 'OUTER')]) {
        // ...
        withCredentials([string(credentialsId: '...', variable: 'INNER')]) {
          // ...
        }
      }
      

      Depending on the contents of the individual credentials, if the outer step takes precedence over the inner step, then this might change the logs of the inner step to be ineffective in its own attempts at filtering.

      At first, I thought I had found a problem with how this feature interacted with TaskListenerDecorator from workflow-api, but it turns out that updating BindingStep in this plugin to reverse the ordering of how it merges the ConsoleLogFilter has nearly the same effect as converting that class to use the TaskListenerDecorator API and merging the decorators in the same ordering (masking first). As the resulting ConsoleLogFilter gets converted to a TaskListenerDecorator in DefaultStepContext which would take precedence over any registered TaskListenerDecorator instances anyways.

        Attachments

          Activity

          Show
          jvz Matt Sicker added a comment - https://github.com/jenkinsci/credentials-binding-plugin/pull/107

            People

            Assignee:
            jvz Matt Sicker
            Reporter:
            jvz Matt Sicker
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated: