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

[DO NOT MERGE] Check Restricted annotation instead #7

Closed

Conversation

daniel-beck
Copy link

@daniel-beck daniel-beck commented Sep 18, 2017

Because jenkinsci/plugin-pom#61


This changes the behavior of this scanner to find usages of @Restricted annotated APIs of core in plugins. Before the above fix to the plugins POM, this was possible without breaking the build.

DO NOT MERGE, this should either be a separate long-living branch ("restricted-usage-in-plugins") or be integrated into regular deprecated API checks. Not sure which. Currently there's no facility to annotate the type of finding, so merging into the same reports would be weird.

Notable violations:

  • jenkins/util/SystemProperties

  • jenkins/util/xml/XMLUtils

  • hudson/model/Computer$DisplayExecutor

Plus quite a few usages of Messages, which were all @Restricted in jenkinsci/jenkins#2656

PR build output (without evil CSS): https://ci.jenkins.io/job/Infra/job/deprecated-usage-in-plugins/job/PR-7/1/artifact/output/usage-by-plugin.html

@stephenc
Copy link

@daniel-beck if jenkins/util/xml/XMLUtils is @Restricted how are plugins supposed to prevent SECURITY-167 from plugins? is that @Restricted just a hold-over from backporting?

@daniel-beck
Copy link
Author

@stephenc

is that @Restricted just a hold-over from backporting

Not sure. The annotation was originally requested by @jglick, but no note on whether it was supposed to be temporary.

@jglick
Copy link

jglick commented Sep 18, 2017

Do not recall. Probably just a matter of not introducing APIs in security releases. Could be unrestricted in trunk if necessary.

@jglick
Copy link

jglick commented Sep 18, 2017

Apparent example usage

For now, credentials must use a private copy of XMLUtils, with a TODO comment requesting the standard version linking to a core PR (then, when merged, Jenkins version) which unrestricts it. But ideally the API would be a bit higher-level, so you could create a safe HierarchicalStreamReader directly from an InputStream; or XStream2 could even have an unmarshal overload taking InputStream.

@mjdetullio
Copy link

If you decide changes to multi-branch-project-plugin are necessary, I can reopen the ticket to remove it from the plugins library so you can avoid that work.

@daniel-beck
Copy link
Author

@mjdetullio

changes to multi-branch-project-plugin are necessary

No strong opinion, this just means the plugin uses something from core not considered public (and therefore stable) API, this may or may not break at some time in the future, but it will impact modernization of the plugin. No action needed I guess if the plugin is abandoned and discourages further use.

@oleg-nenashev
Copy link

SystemProperties is baad :( I had a plan to make this API publicly accessible via a library (e.g. to reuse it in Remoting and other core components), but I have never got to it. Usage of this API should be safe for now, but it will blow up the things if we remove it from the core as planned. The plugin needs to be fixed for sure

@vivek
Copy link

vivek commented Sep 19, 2017

@daniel-beck SystemProperties is only on release/1.2 branch and older release branch. Its been removed from current master 1.3-SNAPSHOT branch. So I guess we are good?

@daniel-beck
Copy link
Author

@vivek Well, you've already done what this should get you to do, so, yes 👍

Copy link

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

I would rather vote for a separate branch for now. The current report is already useful, so having it in a branch with docs LGTM.

🐝

@jenkinsadmin
Copy link

The context from the Jenkins Pipeline run is:

	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:106)
	at org.apache.maven.cli.MavenCli.execute(MavenCli.java:863)
	at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288)
	at org.apache.maven.cli.MavenCli.main(MavenCli.java:199)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
	at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
	at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 02:12 min
[INFO] Finished at: 2017-12-01T20:21:25+00:00
[INFO] Final Memory: 36M/1243M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.4.0:java (default-cli) on project deprecated-usage-in-plugins: An exception occured while executing the Java class. null: InvocationTargetException: error in opening zip file -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // stage
[Pipeline] stage
[Pipeline] { (Archive)
Stage 'Archive' skipped due to earlier failure(s)
[Pipeline] }
[Pipeline] // stage
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // timeout
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // node
[Pipeline] End of Pipeline

GitHub has been notified of this commit’s build result

ERROR: script returned exit code 1
Finished: FAILURE

powered by the Comment Logger

@jenkinsadmin
Copy link

The context from the Jenkins Pipeline run is:

Also:   hudson.remoting.Channel$CallSiteStackTrace: Remote call to ubuntu-jenkinsinfra78c880
		at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1693)
		at hudson.remoting.UserResponse.retrieve(UserRequest.java:310)
		at hudson.remoting.Channel.call(Channel.java:908)
		at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler.execute(RemoteGitImpl.java:146)
		at sun.reflect.GeneratedMethodAccessor459.invoke(Unknown Source)
		at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
		at java.lang.reflect.Method.invoke(Method.java:498)
		at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler.invoke(RemoteGitImpl.java:132)
		at com.sun.proxy.$Proxy240.execute(Unknown Source)
		at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:1083)
		at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1123)
		at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:113)
		at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:85)
		at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:75)
		at org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution$1$1.call(AbstractSynchronousNonBlockingStepExecution.java:47)
		at hudson.security.ACL.impersonate(ACL.java:260)
		at org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution$1.run(AbstractSynchronousNonBlockingStepExecution.java:44)
		at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
		at java.util.concurrent.FutureTask.run(FutureTask.java:266)
		at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
		at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
		at java.lang.Thread.run(Thread.java:745)
Caused: org.eclipse.jgit.errors.TranslationBundleLoadingException: Loading of translation bundle failed for [org.eclipse.jgit.internal.JGitText, en_US]
	at org.eclipse.jgit.nls.TranslationBundle.load(TranslationBundle.java:168)
	at org.eclipse.jgit.nls.GlobalBundleCache.lookupBundle(GlobalBundleCache.java:96)
	at org.eclipse.jgit.nls.NLS.get(NLS.java:132)
	at org.eclipse.jgit.nls.NLS.getBundleFor(NLS.java:118)
	at org.eclipse.jgit.internal.JGitText.get(JGitText.java:59)
	at org.eclipse.jgit.internal.storage.file.WindowCursor.open(WindowCursor.java:158)
	at org.eclipse.jgit.lib.ObjectReader.open(ObjectReader.java:227)
	at org.eclipse.jgit.revwalk.RevWalk.parseAny(RevWalk.java:859)
	at org.eclipse.jgit.transport.FetchProcess.askForIsComplete(FetchProcess.java:340)
	at org.eclipse.jgit.transport.FetchProcess.executeImp(FetchProcess.java:160)
	at org.eclipse.jgit.transport.FetchProcess.execute(FetchProcess.java:122)
	at org.eclipse.jgit.transport.Transport.fetch(Transport.java:1201)
	at org.eclipse.jgit.api.FetchCommand.call(FetchCommand.java:128)
	at org.jenkinsci.plugins.gitclient.JGitAPIImpl$5.execute(JGitAPIImpl.java:1464)
	at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:153)
	at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:146)
	at hudson.remoting.UserRequest.perform(UserRequest.java:207)
	at hudson.remoting.UserRequest.perform(UserRequest.java:53)
	at hudson.remoting.Request$2.run(Request.java:358)
	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Finished: FAILURE

powered by the Comment Logger

@daniel-beck daniel-beck mentioned this pull request Jan 25, 2018
@jenkinsadmin
Copy link

Build failed; the context from the latest run is:

Expand to view
Downloaded curseforge-publisher.hpi, 900 Kb
Downloaded custom-job-icon.hpi, 1267 Kb
Downloaded cucumber-testresult-plugin.hpi, 3101 Kb
Downloaded cucumber-trends-report.hpi, 481 Kb
Downloaded custom-tools-plugin.hpi, 53 Kb
Downloaded custom-view-tabs.hpi, 23 Kb
Downloaded customize-build-now.hpi, 8 Kb
Downloaded CustomHistory.hpi, 22 Kb
Downloaded cucumber-perf.hpi, 1770 Kb
Downloaded customized-build-message.hpi, 94 Kb
Downloaded cvs.hpi, 605 Kb
Downloaded cygpath.hpi, 7 Kb
Downloaded cygwin-process-killer.hpi, 24 Kb
Downloaded darcs.hpi, 69 Kb
Downloaded daily-quote.hpi, 6 Kb
Downloaded dashboard-view.hpi, 556 Kb
Downloaded database-drizzle.hpi, 160 Kb
wrapper script does not seem to be touching the log file in /home/jenkins/workspace/cated-usage-in-plugins_PR-7-MYPJYXNH36LBLPJGO3ONSARW7PFVVXLFUKYQMOHHXJMZ6XP6S5NQ@tmp/durable-e64c40ec
(JENKINS-48300: if on a laggy filesystem, consider -Dorg.jenkinsci.plugins.durabletask.BourneShellScript.HEARTBEAT_CHECK_INTERVAL=300)
Downloaded datadog.hpi, 810 Kb
Downloaded database-postgresql.hpi, 517 Kb
Downloaded database-sqlite.hpi, 5428 Kb
Downloaded database.hpi, 8510 Kb
Downloaded database-mysql.hpi, 934 Kb
Downloaded cucumber-living-documentation.hpi, 28161 Kb
Downloaded database-h2.hpi, 1212 Kb
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // stage
[Pipeline] stage
[Pipeline] { (Archive)
Stage 'Archive' skipped due to earlier failure(s)
[Pipeline] }
[Pipeline] // stage
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // timeout
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // node
[Pipeline] End of Pipeline

GitHub has been notified of this commit’s build result

ERROR: script returned exit code -1
Finished: FAILURE

Powered by the Comment Logger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants