Merge pull request #3851 from jglick/ConsoleNote.encodeToBytes-AnonymousClassWarnings-JENKINS-55257

[JENKINS-55257] If ConsoleNote.encodeToBytes is called at all from an agent, it should not use AnonymousClassWarnings

core/src/main/java/hudson/console/ConsoleNote.java

@@ -53,8 +53,11 @@ import com.jcraft.jzlib.GZIPInputStream;
 import com.jcraft.jzlib.GZIPOutputStream;
 import hudson.remoting.ClassFilter;
 import jenkins.security.HMACConfidentialKey;
+import jenkins.util.JenkinsJVM;
 import jenkins.util.SystemProperties;
 import org.jenkinsci.remoting.util.AnonymousClassWarnings;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * Data that hangs off from a console output.
@@ -130,7 +133,8 @@ public abstract class ConsoleNote<T> implements Serializable, Describable<Consol
      * Disables checking of {@link #MAC} so do not set this flag unless you completely trust all users capable of affecting build output,
      * which in practice means that all SCM committers as well as all Jenkins users with any non-read-only access are consider administrators.
      */
-    static /* nonfinal for tests & script console */ boolean INSECURE = SystemProperties.getBoolean(ConsoleNote.class.getName() + ".INSECURE");
+    @Restricted(NoExternalUse.class)
+    public static /* nonfinal for tests & script console */ boolean INSECURE = SystemProperties.getBoolean(ConsoleNote.class.getName() + ".INSECURE");
 
     /**
      * When the line of a console output that this annotation is attached is read by someone,
@@ -181,7 +185,8 @@ public abstract class ConsoleNote<T> implements Serializable, Describable<Consol
 
     private ByteArrayOutputStream encodeToBytes() throws IOException {
         ByteArrayOutputStream buf = new ByteArrayOutputStream();
-        try (ObjectOutputStream oos = AnonymousClassWarnings.checkingObjectOutputStream(new GZIPOutputStream(buf))) {
+        try (OutputStream gzos = new GZIPOutputStream(buf);
+             ObjectOutputStream oos = JenkinsJVM.isJenkinsJVM() ? AnonymousClassWarnings.checkingObjectOutputStream(gzos) : new ObjectOutputStream(gzos)) {
             oos.writeObject(this);
         }
 
@@ -190,7 +195,7 @@ public abstract class ConsoleNote<T> implements Serializable, Describable<Consol
         DataOutputStream dos = new DataOutputStream(new Base64OutputStream(buf2,true,-1,null));
         try {
             buf2.write(PREAMBLE);
-            if (Jenkins.getInstanceOrNull() != null) { // else we are in another JVM and cannot sign; result will be ignored unless INSECURE
+            if (JenkinsJVM.isJenkinsJVM()) { // else we are in another JVM and cannot sign; result will be ignored unless INSECURE
                 byte[] mac = MAC.mac(buf.toByteArray());
                 dos.writeInt(- mac.length); // negative to differentiate from older form
                 dos.write(mac);

test/src/test/java/jenkins/slaves/OldRemotingAgentTest.java

@@ -24,10 +24,19 @@
 package jenkins.slaves;
 
 import hudson.EnvVars;
+import hudson.Launcher;
+import hudson.MarkupText;
+import hudson.console.ConsoleAnnotationDescriptor;
+import hudson.console.ConsoleAnnotator;
+import hudson.console.ConsoleNote;
+import hudson.model.AbstractBuild;
+import hudson.model.BuildListener;
 import hudson.model.Computer;
+import hudson.model.FreeStyleBuild;
 import hudson.model.FreeStyleProject;
 import hudson.model.Label;
 import hudson.model.Slave;
+import hudson.model.TaskListener;
 import hudson.model.labels.LabelAtom;
 import hudson.node_monitors.AbstractAsyncNodeMonitorDescriptor;
 import hudson.node_monitors.AbstractNodeMonitorDescriptor;
@@ -35,6 +44,7 @@ import hudson.node_monitors.NodeMonitor;
 import hudson.slaves.ComputerLauncher;
 import hudson.tasks.BatchFile;
 import hudson.tasks.Shell;
+import jenkins.security.MasterToSlaveCallable;
 import org.codehaus.plexus.util.FileUtils;
 import org.junit.Before;
 import org.junit.Rule;
@@ -43,9 +53,12 @@ import org.junit.rules.TemporaryFolder;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.SimpleCommandLauncher;
+import org.jvnet.hudson.test.TestBuilder;
+import org.jvnet.hudson.test.TestExtension;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.StringWriter;
 import java.lang.reflect.Method;
 import java.net.URISyntaxException;
 import java.util.Collection;
@@ -92,6 +105,55 @@ public class OldRemotingAgentTest {
         NodeMonitorAssert.assertMonitors(NodeMonitor.getAll(), agent.getComputer());
     }
 
+    @Issue("JENKINS-55257")
+    @Test
+    public void remoteConsoleNote() throws Exception {
+        Slave agent = j.createOnlineSlave();
+        FreeStyleProject project = j.createFreeStyleProject();
+        project.setAssignedLabel(agent.getSelfLabel());
+        project.getBuildersList().add(new TestBuilder() {
+            @Override
+            public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
+                build.getWorkspace().act(new RemoteConsoleNotePrinter(listener));
+                return true;
+            }
+        });
+        FreeStyleBuild b = j.buildAndAssertSuccess(project);
+        StringWriter sw = new StringWriter();
+        // The note will not actually work by default; we just want to ensure that the attempt is ignored without breaking the build.
+        // But for purposes of testing, check that the note really made it into the log.
+        boolean insecureOriginal = ConsoleNote.INSECURE;
+        ConsoleNote.INSECURE = true;
+        try {
+            b.getLogText().writeHtmlTo(0, sw);
+        } finally {
+            ConsoleNote.INSECURE = insecureOriginal;
+        }
+        assertThat(sw.toString(), containsString("@@@ANNOTATED@@@"));
+    }
+    private static final class RemoteConsoleNotePrinter extends MasterToSlaveCallable<Void, IOException> {
+        private final TaskListener listener;
+        RemoteConsoleNotePrinter(TaskListener listener) {
+            this.listener = listener;
+        }
+        @Override
+        public Void call() throws IOException {
+            listener.annotate(new RemoteConsoleNote());
+            listener.getLogger().println();
+            return null;
+        }
+    }
+    public static final class RemoteConsoleNote extends ConsoleNote<Object> {
+        @Override
+        public ConsoleAnnotator<Object> annotate(Object context, MarkupText text, int charPos) {
+            text.addMarkup(charPos, "@@@ANNOTATED@@@");
+            return null;
+        }
+
+        @TestExtension("remoteConsoleNote")
+        public static final class DescriptorImpl extends ConsoleAnnotationDescriptor {}
+    }
+
     //TODO: move the logic to JTH
     private class JenkinsRuleWithOldAgent extends JenkinsRule {