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

Groovy Parser cannot access console log

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Minor
    • Resolution: Fixed
    • Component/s: warnings-ng-plugin
    • Labels:
      None
    • Environment:
      Jenkins ver. 2.138.3 (Docker Hub)
      Wanings NG 1.0.0-beta5
    • Similar Issues:

      Description

      (discussion is split from JENKINS-54759)

      Issue

      console parsers currently run on the master and for security reasons, this excludes the customizable Groovy Parser.
      for an user this is surprising and adding a custom warning parser will need changes in the buildscript to dump output to a log file, ideally adding a parser would not otherwise affect existing jobs .

      1. not piping would be the most clear, and similar to the scripts you would run on a local build
      2. piping it just to a file will remove all visible status during execution
      3. naively splitting it (eg. | tee file.log) will mean the error state of the script doing the build will vanish. see here
      4. additionally storing and restoring the errors state of the script adds boilerplate code

      just to reiterate how invasive this change is, here is my current (POSIX Shell) workaround with variant 4):

      before:

      sh buildscript.sh ARGS...
      

      after:

      pipe_retval() {
        local LOGFILE=$1; shift
        (((("$@" 2>&1; echo $? >&3) | tee "$LOGFILE" >&4) 3>&1) | (read xs; exit $xs)) 4>&1
      }
      
      pipe_retval build.log sh buildscript.sh ARGS...
      

        Attachments

          Issue Links

            Activity

            Hide
            mattphi Matt Phillips added a comment - - edited

            The workaround (pipe to tee) works nicely for jobs that don't allow parallel executions - I just tee to a file called build.log and then parse that.  I have many parameterized jobs that allow parallel executions.  I could prepend the build number to the log file (like build_12454.log), but then I'll need to write another script to clean up old logs in the workspace.  Has anyone else solved this?

            EDIT: I invented a solution -

            1. TEE the output to build_${BUILD_NUMBER}.log
            2. Use your groovy parser to parse build_${BUILD_NUMBER}.log
            3. Use the Archive the Artifacts post build steps to archive build_${BUILD_NUMBER}.log (I do this just in case something is lost in the TEE command and I want to debug that)
            4. Use the Post build task plugin to call del build_%BUILD_NUMBER%.log for log files that match .*

            EDIT2: Ignore everything I've said.  It turns out when Jenkins is running parallel instances of a job, it creates new workspace folders for each instance (e.g. workspace, workspace@2, workspace@3, etc...)

            I will revert to just tee'ing to a file called build.log - and it will get overwritten with each new build!

            It was a journey, but I got there in the end!

            Show
            mattphi Matt Phillips added a comment - - edited The workaround (pipe to tee) works nicely for jobs that don't allow parallel executions - I just tee to a file called build.log and then parse that.  I have many parameterized jobs that allow parallel executions.  I could prepend the build number to the log file (like build_12454.log), but then I'll need to write another script to clean up old logs in the workspace.  Has anyone else solved this? EDIT : I invented a solution - TEE the output to build_${BUILD_NUMBER}.log Use your groovy parser to parse build_${BUILD_NUMBER}.log Use the Archive the Artifacts  post build steps to archive build_${BUILD_NUMBER}.log (I do this just in case something is lost in the TEE command and I want to debug that) Use the Post build task plugin to call  del build_%BUILD_NUMBER%.log for log files that match . * EDIT2 : Ignore everything I've said.  It turns out when Jenkins is running parallel instances of a job, it creates new workspace folders for each instance (e.g. workspace, workspace@2, workspace@3, etc...) I will revert to just tee'ing to a file called build.log - and it will get overwritten with each new build! It was a journey, but I got there in the end!
            Hide
            grunwald Daniel Grunwald added a comment -

            I just investigated how to migrate from the deprecated Warnings Plug-in to WarningsNG, and hit this issue.
            The workaround is not acceptable for us; we'd have to adjust shell scripts in hundreds of jobs.

            We will have to continue using the deprecated plugin (despite it having unfixed security vulnerabilities) until this issue is fixed.

            Wouldn't it be sufficient for security if Groovy parsers from the per-job configuration are prohibited? Those from the system configuration could still be allowed.

            Alternatively, would it be possible to support custom regexes without the Groovy scripting? We just need to fill out the values for filename, message etc. using the regex capture groups. That could be done by configuring filename/message/etc. separately in textboxes using $1 placeholders (regex replace style). That would be easier for users and avoid all the security issues from Groovy scripts.

            Show
            grunwald Daniel Grunwald added a comment - I just investigated how to migrate from the deprecated Warnings Plug-in to WarningsNG, and hit this issue. The workaround is not acceptable for us; we'd have to adjust shell scripts in hundreds of jobs. We will have to continue using the deprecated plugin (despite it having unfixed security vulnerabilities) until this issue is fixed. – Wouldn't it be sufficient for security if Groovy parsers from the per-job configuration are prohibited? Those from the system configuration could still be allowed. Alternatively, would it be possible to support custom regexes without the Groovy scripting? We just need to fill out the values for filename, message etc. using the regex capture groups. That could be done by configuring filename/message/etc. separately in textboxes using $1 placeholders (regex replace style). That would be easier for users and avoid all the security issues from Groovy scripts.
            Hide
            drulli Ulli Hafner added a comment - - edited

            Wouldn't it be sufficient for security if Groovy parsers from the per-job configuration are prohibited? Those from the system configuration could still be allowed.

            You cannot define groovy parsers in the job configuration. So this issue is already about the parsers in the system configuration.

            Alternatively, would it be possible to support custom regexes without the Groovy scripting? We just need to fill out the values for filename, message etc. using the regex capture groups. That could be done by configuring filename/message/etc. separately in textboxes using $1 placeholders (regex replace style).

            Yes, that would be another approach that is mostly secure. (Well there might be a https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS but that is a different topic). But that still needs to be implemented. You can create a new feature request, but I am not sure if someone finds the time to implement that.

            Another solution would be to use a script to transform the log output to the JSON native format. Or even better: create a small Java Class that implements that parser for your console output.

            (The last solution depends on a fix for JENKINS-44450, then console log could be parsed on the agent.)

            Show
            drulli Ulli Hafner added a comment - - edited Wouldn't it be sufficient for security if Groovy parsers from the per-job configuration are prohibited? Those from the system configuration could still be allowed. You cannot define groovy parsers in the job configuration. So this issue is already about the parsers in the system configuration. Alternatively, would it be possible to support custom regexes without the Groovy scripting? We just need to fill out the values for filename, message etc. using the regex capture groups. That could be done by configuring filename/message/etc. separately in textboxes using $1 placeholders (regex replace style). Yes, that would be another approach that is mostly secure. (Well there might be a https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS but that is a different topic). But that still needs to be implemented. You can create a new feature request, but I am not sure if someone finds the time to implement that. Another solution would be to use a script to transform the log output to the JSON native format. Or even better: create a small Java Class that implements that parser for your console output. (The last solution depends on a fix for JENKINS-44450 , then console log could be parsed on the agent.)
            Hide
            pjdarton pjdarton added a comment -

            I've just stumbled upon this. IMO this functional deficit is a critical defect, not a minor bug.

            Background: We've been using the old (deprecated) Warnings plugin for ages and it recently stopped working (incompatible changes in the Jenkins core) so we're now being forced to "jump ship" to the next-gen warnings plugin.  I'm currently re-coding our (thankfully few!) groovy parsers to work with the new plugin ... only to find that the core functionality provided by the old plugin (the ability to parse the console log) doesn't seem to be supported yet.
            That's a blocking issue for me; I had (foolishly) assumed that this plugin's functionality was a superset of the other plugins that it claimed to replace.

            I've read the description above and the "for security reasons" argument has little credibility - these parsers are only configurable by someone with admin access and such a person can run any Groovy code they wish on the primary Jenkins node if they so chose (and/or can authorise groovy calls for use in sandboxed mode).
            i.e. There's nothing inherently insecure about having an admin-approved snippet of groovy code being able to parse the console.
            It's no different than if it was parsing a file of a build that'd run on the primary Jenkins node - it's all running in the same place, and the only thing that's running is 100% approved by the admin. By all means run it in a sandboxed environment where the parser is passed in the console and outputs optional warnings (and only those), but the code is admin-approved because it's admin-submitted.

            I believe that there may be an argument for allowing the admin to choose whether or not a parser is limited to just the console, or just files, or both, depending on the groovy code they've configured (they might want to stop users from creating builds that use parsers incorrectly), but the ability to parse the console content is of vital importance; I'm shocked/disapointed that it's absent in a plugin that's been around this long, especially as this plugin is described as a replacement for the old warning plugin where this aspect "just worked".

            IMO the suggested workaround to "re-code every single job you've got" is unreasonable; we've got hundreds of them and, as a Jenkins admin, I can't 100% rely on my users to correctly pipe everything into a file during evry build step - not all output goes to stdout, you need to capture stderr too and that's all too easy to forget; that's why it's important to scan the console itself - it's much harder for the user to mess that up.

            Show
            pjdarton pjdarton added a comment - I've just stumbled upon this. IMO this functional deficit is a critical defect, not a minor bug. Background: We've been using the old (deprecated) Warnings plugin for ages and it recently stopped working (incompatible changes in the Jenkins core) so we're now being forced to "jump ship" to the next-gen warnings plugin.  I'm currently re-coding our (thankfully few!) groovy parsers to work with the new plugin ... only to find that the core functionality provided by the old plugin (the ability to parse the console log) doesn't seem to be supported yet. That's a blocking issue for me; I had (foolishly) assumed that this plugin's functionality was a superset of the other plugins that it claimed to replace. I've read the description above and the "for security reasons" argument has little credibility - these parsers are only configurable by someone with admin access and such a person can run any Groovy code they wish on the primary Jenkins node if they so chose (and/or can authorise groovy calls for use in sandboxed mode). i.e. There's nothing inherently insecure about having an admin-approved snippet of groovy code being able to parse the console. It's no different than if it was parsing a file of a build that'd run on the primary Jenkins node - it's all running in the same place, and the only thing that's running is 100% approved by the admin. By all means run it in a sandboxed environment where the parser is passed in the console and outputs optional warnings (and only those), but the code is admin-approved because it's admin-submitted. I believe that there may be an argument for allowing the admin to choose whether or not a parser is limited to just the console, or just files, or both, depending on the groovy code they've configured (they might want to stop users from creating builds that use parsers incorrectly), but the ability to parse the console content is of vital importance; I'm shocked/disapointed that it's absent in a plugin that's been around this long, especially as this plugin is described as a replacement for the old warning plugin where this aspect "just worked". IMO the suggested workaround to "re-code every single job you've got" is unreasonable; we've got hundreds of them and, as a Jenkins admin, I can't 100% rely on my users to correctly pipe everything into a file during evry build step - not all output goes to stdout, you need to capture stderr too and that's all too easy to forget; that's why it's important to scan the console itself - it's much harder for the user to mess that up.
            Hide
            pjdarton pjdarton added a comment -

            I've submitted a couple of PRs.

            The first is a simple cosmetic bugfix - the (non-pipeline) job configuration form for using a groovy parser failed to mention that the field validation methods required @POST so they were causing the WebUI to report "ERROR" and not call the validators.

            The second is a partial fix for this issue - it allows the Jenkins administrator to decide whether or not to allow these parsers to scan the console log. Different folks will have different ideas on that matter and now they can choose.

            Ideally, in due course, someone will submit a PR that wraps the groovy parsing in a sandbox so that it can be safely run anywhere, at which point the need for the configurable field will disappear, but until then we can empower Jenkins admins to chose whether they'll take on the job of carefully policing what groovy code they put in those parsers, or simply ban the groovy parsers from scanning the console.

            I invite anyone interested in this issue to pitch in with comments and/or code-reviews; while I'm no stranger to Jenkins plugins, I am a stranger to this Jenkins plugin so I hope I'm not treading on anyone's toes here...

            Show
            pjdarton pjdarton added a comment - I've submitted a couple of PRs. The first is a simple cosmetic bugfix - the (non-pipeline) job configuration form for using a groovy parser failed to mention that the field validation methods required @POST so they were causing the WebUI to report "ERROR" and not call the validators. The second is a partial fix for this issue - it allows the Jenkins administrator to decide whether or not to allow these parsers to scan the console log. Different folks will have different ideas on that matter and now they can choose. Ideally, in due course, someone will submit a PR that wraps the groovy parsing in a sandbox so that it can be safely run anywhere, at which point the need for the configurable field will disappear, but until then we can empower Jenkins admins to chose whether they'll take on the job of carefully policing what groovy code they put in those parsers, or simply ban the groovy parsers from scanning the console. I invite anyone interested in this issue to pitch in with comments and/or code-reviews; while I'm no stranger to Jenkins plugins, I am a stranger to this Jenkins plugin so I hope I'm not treading on anyone's toes here...

              People

              Assignee:
              drulli Ulli Hafner
              Reporter:
              nolange79 Norbert Lange
              Votes:
              7 Vote for this issue
              Watchers:
              11 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: