Skip to content

Commit

Permalink
[JENKINS-25348]
Browse files Browse the repository at this point in the history 
Avoid having multiple copies of groovy-sandbox.jar in the Jenkins JVM,
which compromises the security.

I think it makes sense for the script-security plugin to own this
library, so instead let's depend on that plugin to provide a canonical
copy.

There's still a separate fix that needs to happen in script-security
plugin and its client plugins, but in the mean time this fix plugs the
hole as email-ext-plugin is the only other plugin in the jenkinsci org
that brings its own copy of groovy-sandbox
  • Loading branch information
@kohsuke
kohsuke committed Nov 4, 2014
1 parent 99ff8ae commit 985892a
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions pom.xml
View file
Expand Up @@ -120,9 +120,9 @@
<version>1.5</version>
</dependency>
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>groovy-sandbox</artifactId>
<version>1.0</version>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>script-security</artifactId>
<version>1.6</version>
</dependency>
<dependency>
<groupId>org.jsoup</groupId>
Expand Down

5 comments on commit 985892a

@jglick
Copy link
Member

@jglick jglick commented on 985892a Nov 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway the calls to Groovy Sandbox from this plugin are probably worse than useless. It should be properly integrated with script-security.

@slide
Copy link
Member

@slide slide commented on 985892a Nov 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can core commiters please follow the pull request method for changing plugins? It's very annoying to have things changed underneath you with no notice.

@kohsuke
Copy link
Member Author

@kohsuke kohsuke commented on 985892a Nov 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, my apologies. I thought this was an invisible enough change.

@slide
Copy link
Member

@slide slide commented on 985892a Nov 4, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kohsuke
Copy link
Member Author

@kohsuke kohsuke commented on 985892a Nov 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. We touch so many plugins and in majority of them there's no active maintainer (which results in 720 open pull requests as of now across the board) to merge pull requests, so I've developed the "let's commit straight in and accept a revert" mentality.

But there are some plugins that are clearly exceptions to this norm, and email-ext is one of them. I'll be careful going forward.

(And yes, please feel free to revert changes, and I take no offense.)

Please sign in to comment.