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

[JENKINS-68404] Add script listener to track usage #416

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<url>https://github.com/jenkinsci/${project.artifactId}-plugin</url>
<properties>
<changelist>999999-SNAPSHOT</changelist>
<jenkins.version>2.332.1</jenkins.version>
<jenkins.version>2.355-SNAPSHOT</jenkins.version>
daniel-beck marked this conversation as resolved.
Show resolved Hide resolved
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
</properties>
<licenses>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import hudson.model.AbstractDescribableImpl;
import hudson.model.Descriptor;
import hudson.model.Item;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.util.FormValidation;
import io.jenkins.lib.versionnumber.JavaSpecificationVersion;
Expand Down Expand Up @@ -437,7 +438,16 @@ public Object evaluate(ClassLoader loader, Binding binding, @CheckForNull TaskLi
memoryProtectedLoader = new CleanGroovyClassLoader(loader);
loaderF.set(sh, memoryProtectedLoader);
}
return sh.evaluate(ScriptApproval.get().using(script, GroovyLanguage.get()));
String origin = "UNKNOWN";
if (binding.hasVariable("build")) {
Run run = (Run) binding.getVariable("build");

origin = String.format("build '%s'", run.getExternalizableId());
} else {
LOGGER.log(Level.INFO, "Could not determine origin of the groovy script - missing implementation. Please open an issue for this!");
}

return sh.evaluate(ScriptApproval.get().using(script, GroovyLanguage.get(), origin));
}

} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import jenkins.model.GlobalConfiguration;
import jenkins.model.GlobalConfigurationCategory;
import jenkins.model.ScriptListener;
import jenkins.util.SystemProperties;
import net.sf.json.JSONArray;
import net.sf.json.JSONObject;
Expand Down Expand Up @@ -475,22 +476,37 @@ public String configuring(@NonNull String script, @NonNull Language language, @N
* @param script a possibly unapproved script
* @param language the language in which it is written
* @return {@code script}, for convenience
* @throws UnapprovedUsageException in case it has not yet been approved
* @throws UnapprovedUsageException in case it has not yet been approve
* @deprecated Use {@link #using(String, Language, String)}.
*/
@Deprecated
public synchronized String using(@NonNull String script, @NonNull Language language) throws UnapprovedUsageException {
return using(script, language, "N/A");
}
/**
* Called when a script is about to be used (evaluated).
* @param script a possibly unapproved script
* @param language the language in which it is written
* @param origin A descriptive, trackable identifier of the entity running the script.
* @return {@code script}, for convenience
* @throws UnapprovedUsageException in case it has not yet been approved
*/
public synchronized String using(@NonNull String script, @NonNull Language language, String origin) throws UnapprovedUsageException {
if (script.length() == 0) {
// As a special case, always consider the empty script preapproved, as this is usually the default for new fields,
// and in many cases there is some sensible behavior for an emoty script which we want to permit.
ScriptListener.fireScriptEvent(script, origin, null);

return script;
}
String hash = hash(script, language.getName());
if (!approvedScriptHashes.contains(hash)) {
// Probably need not add to pendingScripts, since generally that would have happened already in configuring.
throw new UnapprovedUsageException(hash);
}
ScriptListener.fireScriptEvent(script, origin, null);
return script;
}

// Only for testing
synchronized boolean isScriptHashApproved(String hash) {
return approvedScriptHashes.contains(hash);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public class SecureGroovyScriptTest {
assertEquals(0, pendingScripts.size());

// Test that the script is executable. If it's not, we will get an UnapprovedUsageException
assertEquals(groovy, ScriptApproval.get().using(groovy, GroovyLanguage.get()));
assertEquals(groovy, ScriptApproval.get().using(groovy, GroovyLanguage.get(), "Testing"));
}

/**
Expand Down Expand Up @@ -243,7 +243,7 @@ public class SecureGroovyScriptTest {

// We didn't add the approved classpath so ...
final UnapprovedUsageException e = assertThrows(UnapprovedUsageException.class,
() -> ScriptApproval.get().using(groovy, GroovyLanguage.get()));
() -> ScriptApproval.get().using(groovy, GroovyLanguage.get(), "Testing"));
assertEquals("script not yet approved for use", e.getMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ boolean findApproved() {

@Override
Script use() {
assertEquals(groovy, ScriptApproval.get().using(groovy, GroovyLanguage.get()));
assertEquals(groovy, ScriptApproval.get().using(groovy, GroovyLanguage.get(), "Testing"));
return this;
}

Expand Down