Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run groovy scripts with no permissions #10969

Merged
merged 2 commits into from May 5, 2015
Merged

Conversation

rmuir
Copy link
Contributor

@rmuir rmuir commented May 5, 2015

Scripts shouldn't really have side effects.
There are a number of simple and complicated ways we can reduce their java permissions, this is the simplest one that I see.

This is not a sandbox, it just runs groovy scripts without priviledges since they should not need them.
A sandbox would be more, and involve API changes.
I did however revive the old sandbox test just as a simple start.

@s1monw
Copy link
Contributor

s1monw commented May 5, 2015

LGTM this a great start

@uschindler
Copy link
Contributor

Phantastic. I know that you know what you know that you are doing! URL#getFile() is bad, but OK for those static checks...

To be sure: Is a equals enough, I would have expected that it should be startsWith("/groovy/script"), especially if one tries to define a class inside his script and maybe defines a custom package.

@Override
public void setUp() throws Exception {
super.setUp();
assumeTrue("security manager is enabled", System.getSecurityManager() != null);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the message should be "security manager is disabled", because the assume would only print the message and ignore test if the security manager is disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1, I will improve this assume.

@rmuir
Copy link
Contributor Author

rmuir commented May 5, 2015

@uschindler yes, groovy always sets this strange "fake" URL. I think its just intended to make it easier to configure in a policy file. That would be hard for us (even if its preferred) because everything there must really be specified as a white-list. Here a black-list (simple check) is easier to reason about.

@uschindler
Copy link
Contributor

OK. Thanks for the info!

rmuir added a commit that referenced this pull request May 5, 2015
Run groovy scripts with no permissions
@rmuir rmuir merged commit 1139498 into elastic:master May 5, 2015
@clintongormley clintongormley added >enhancement :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache labels Jun 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants