Closed dependabot[bot] closed 9 months ago
Does https://github.com/jenkinsci/plugin-pom/pull/859#pullrequestreview-1718532470 still apply? Were there new false positives that were particularly problematic? https://github.com/spotbugs/spotbugs/releases/tag/4.8.2
Does #859 (review) still apply? Were there new false positives that were particularly problematic? https://github.com/spotbugs/spotbugs/releases/tag/4.8.2
Jenkins CLI reports two new spotbugs warnings with spotbugs 4.8.2:
[INFO] BugInstance size is 2
[INFO] Error size is 0
[INFO] Total bugs: 2
[ERROR] Medium: Exception thrown in class hudson.cli.FullDuplexHttpStream at new hudson.cli.FullDuplexHttpStream(URL, String, String) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks. [hudson.cli.FullDuplexHttpStream, hudson.cli.FullDuplexHttpStream] At FullDuplexHttpStream.java:[line 49]At FullDuplexHttpStream.java:[line 49] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.util.QuotedStringTokenizer at new hudson.util.QuotedStringTokenizer(String, String, boolean, boolean) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks. [hudson.util.QuotedStringTokenizer, hudson.util.QuotedStringTokenizer] At QuotedStringTokenizer.java:[line 106]At QuotedStringTokenizer.java:[line 106] CT_CONSTRUCTOR_THROW
When I suppress those two spotbugs warnings with a suppression in the source file, then the following additional spotbugs warnings appear:
[ERROR] Medium: Exception thrown in class hudson.EnvVars at new hudson.EnvVars(String[]) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks. [hudson.En\
vVars, hudson.EnvVars] At EnvVars.java:[line 135]At EnvVars.java:[line 135] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.Launcher$LocalLauncher$1 at new hudson.Launcher$LocalLauncher$1(Launcher$LocalLauncher, String, ExecutorService, InputStream, OutputStream, OutputStream, Process, EnvVars, Thread) wi\
ll leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks. [hudson.Launcher$LocalLauncher$1, hudson.Launcher$LocalLauncher$1] At Launcher.java:[line 1031]At Lau\
ncher.java:[line 1031] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.PluginManager at new hudson.PluginManager(ServletContext, File) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Final\
izer attacks. [hudson.PluginManager, hudson.PluginManager] At PluginManager.java:[line 370]At PluginManager.java:[line 370] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Primitive field hudson.PluginManager.pluginUploaded is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility. [hudson.PluginManager] At PluginMa\
nager.java:[line 348] PA_PUBLIC_PRIMITIVE_ATTRIBUTE
[ERROR] Medium: Exception thrown in class hudson.PluginWrapper at new hudson.PluginWrapper(PluginManager, File, Manifest, URL, ClassLoader, File, List, List) will leave the constructor. The object under construction remains partial\
ly initialized and may be vulnerable to Finalizer attacks. [hudson.PluginWrapper, hudson.PluginWrapper] At PluginWrapper.java:[line 493]At PluginWrapper.java:[line 493] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Primitive field hudson.ProxyConfiguration.noProxyHost is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility. [hudson.ProxyConfiguration] At P\
roxyConfiguration.java:[line 152] PA_PUBLIC_PRIMITIVE_ATTRIBUTE
[ERROR] Medium: Exception thrown in class hudson.cli.CLIAction$ServerSideImpl at new hudson.cli.CLIAction$ServerSideImpl(PlainCLIProtocol$Output, Authentication) will leave the constructor. The object under construction remains par\
tially initialized and may be vulnerable to Finalizer attacks. [hudson.cli.CLIAction$ServerSideImpl, hudson.cli.CLIAction$ServerSideImpl] At CLIAction.java:[line 208]At CLIAction.java:[line 208] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Primitive field hudson.cli.CLICommand.locale is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility. [hudson.cli.CLICommand] At CLICommand.jav\
a:[line 231] PA_PUBLIC_PRIMITIVE_ATTRIBUTE
[ERROR] Medium: Primitive field hudson.cli.CLICommand.stderr is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility. [hudson.cli.CLICommand] At CLICommand.jav\
a:[line 230] PA_PUBLIC_PRIMITIVE_ATTRIBUTE
[ERROR] Medium: Primitive field hudson.cli.CLICommand.stdin is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility. [hudson.cli.CLICommand] At CLICommand.java\
:[line 228] PA_PUBLIC_PRIMITIVE_ATTRIBUTE
[ERROR] Medium: Primitive field hudson.cli.CLICommand.stdout is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility. [hudson.cli.SessionIdCommand] At SessionI\
dCommand.java:[line 21] PA_PUBLIC_PRIMITIVE_ATTRIBUTE
[ERROR] Medium: Exception thrown in class hudson.cli.Connection at new hudson.cli.Connection(Socket) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks\
. [hudson.cli.Connection, hudson.cli.Connection] At Connection.java:[line 74]At Connection.java:[line 74] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Primitive field hudson.cli.CopyJobCommand.dst is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility. [hudson.cli.CopyJobCommand] At CopyJobCo\
mmand.java:[line 56] PA_PUBLIC_PRIMITIVE_ATTRIBUTE
[ERROR] Medium: Primitive field hudson.cli.CreateJobCommand.name is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility. [hudson.cli.CreateJobCommand] At Crea\
teJobCommand.java:[line 52] PA_PUBLIC_PRIMITIVE_ATTRIBUTE
[ERROR] Medium: Primitive field hudson.cli.SetBuildDescriptionCommand.description is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility. [hudson.cli.SetBuild\
DescriptionCommand] At SetBuildDescriptionCommand.java:[line 38] PA_PUBLIC_PRIMITIVE_ATTRIBUTE
[ERROR] Medium: Primitive field hudson.cli.SetBuildDisplayNameCommand.displayName is public and set from inside the class, which makes it too exposed. Consider making it private to limit external accessibility. [hudson.cli.SetBuild\
DisplayNameCommand] At SetBuildDisplayNameCommand.java:[line 36] PA_PUBLIC_PRIMITIVE_ATTRIBUTE
[ERROR] Medium: Exception thrown in class hudson.lifecycle.UnixLifecycle at new hudson.lifecycle.UnixLifecycle() will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Final\
izer attacks. [hudson.lifecycle.UnixLifecycle, hudson.lifecycle.UnixLifecycle] At UnixLifecycle.java:[line 63]At UnixLifecycle.java:[line 63] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.model.AbstractBuild at new hudson.model.AbstractBuild(AbstractProject) will leave the constructor. The object under construction remains partially initialized and may be vulnerable t\
o Finalizer attacks. [hudson.model.AbstractBuild, hudson.model.AbstractBuild] At AbstractBuild.java:[line 161]At AbstractBuild.java:[line 161] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.model.AbstractBuild at new hudson.model.AbstractBuild(AbstractProject, File) will leave the constructor. The object under construction remains partially initialized and may be vulner\
able to Finalizer attacks. [hudson.model.AbstractBuild, hudson.model.AbstractBuild] At AbstractBuild.java:[line 177]At AbstractBuild.java:[line 177] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.model.Build at new hudson.model.Build(Project) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks. [hu\
dson.model.Build, hudson.model.Build] At Build.java:[line 94]At Build.java:[line 94] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.model.Build at new hudson.model.Build(Project, File) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attack\
s. [hudson.model.Build, hudson.model.Build] At Build.java:[line 105]At Build.java:[line 105] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.model.Descriptor at new hudson.model.Descriptor() will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks. \
[hudson.model.Descriptor, hudson.model.Descriptor] At Descriptor.java:[line 291]At Descriptor.java:[line 291] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.model.Fingerprint at new hudson.model.Fingerprint(Run, String, byte[]) will leave the constructor. The object under construction remains partially initialized and may be vulnerable t\
o Finalizer attacks. [hudson.model.Fingerprint, hudson.model.Fingerprint] At Fingerprint.java:[line 877]At Fingerprint.java:[line 877] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.model.FingerprintMap$FingerprintParams at new hudson.model.FingerprintMap$FingerprintParams(Run, String) will leave the constructor. The object under construction remains partially i\
nitialized and may be vulnerable to Finalizer attacks. [hudson.model.FingerprintMap$FingerprintParams, hudson.model.FingerprintMap$FingerprintParams] At FingerprintMap.java:[line 106]At FingerprintMap.java:[line 106] CT_CONSTRUCTOR\
_THROW
[ERROR] Medium: Exception thrown in class hudson.model.FreeStyleBuild at new hudson.model.FreeStyleBuild(FreeStyleProject) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks. [hudson.model.FreeStyleBuild, hudson.model.FreeStyleBuild] At FreeStyleBuild.java:[line 36]At FreeStyleBuild.java:[line 36] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.model.FreeStyleBuild at new hudson.model.FreeStyleBuild(FreeStyleProject, File) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks. [hudson.model.FreeStyleBuild, hudson.model.FreeStyleBuild] At FreeStyleBuild.java:[line 39]At FreeStyleBuild.java:[line 39] CT_CONSTRUCTOR_THROW
[ERROR] Medium: Exception thrown in class hudson.model.FullDuplexHttpChannel at new hudson.model.FullDuplexHttpChannel(UUID, boolean) will leave the constructor. The object under construction remains partially initialized and may be vulnerable to Finalizer attacks. [hudson.model.FullDuplexHttpChannel, hudson.model.FullDuplexHttpChannel] At FullDuplexHttpChannel.java:[line 50]At FullDuplexHttpChannel.java:[line 50] CT_CONSTRUCTOR_THROW
****
The CT_CONSTRUCTOR_THROW
messages seem pretty annoying. Discussion in https://github.com/spotbugs/spotbugs/issues/2695. https://wiki.sei.cmu.edu/confluence/display/java/OBJ11-J.+Be+wary+of+letting+constructors+throw+exceptions seems to relate to libraries used with SecurityManager
which is dead and certainly does not apply to Jenkins; we do not expect untrusted code to be running inside the controller JVM, and it does not seem plausible that finalizer abuse would happen by accident.
I guess we cannot exclude specific detectors at the POM level? https://github.com/jenkinsci/plugin-pom/blob/d1bd47d0ccba8ed7ba2f3f06387c3ae1dd03f75e/pom.xml#L1018 is per repo. The trick in #629 seems to be too broad. https://spotbugs.github.io/spotbugs-maven-plugin/spotbugs-mojo.html#optional-parameters does not seem to support excluding only a pattern with inline config. Ah but wait! https://spotbugs.readthedocs.io/en/latest/detectors.html#constructorthrow seems to be in a detector by itself, so maybe try adding ConstructorThrow
to https://github.com/jenkinsci/plugin-pom/blob/d1bd47d0ccba8ed7ba2f3f06387c3ae1dd03f75e/pom.xml#L120 and see if that trims the new warnings to a more reasonable set?
(https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#pa-primitive-field-is-public-pa-public-primitive-attribute seems a reasonable warning in general; for a CLICommand
it is expected due to args4j.)
Thanks for approving the pull request @jglick . I'd like to first merge and release the parent pom pull request that is used for Jenkins core and watch it through the process of updating the repositories that depend on it. That may expose other changes that we need to make before this is merged and released.
Does that seem OK to you?
Would you be willing to review and approve the Jenkins core parent pom pull request:
Does that seem OK to you?
Whatever you think makes sense. I do not have much to add.
A newer version of com.github.spotbugs:spotbugs-maven-plugin exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.
@dependabot recreate
Superseded by #884.
Bumps com.github.spotbugs:spotbugs-maven-plugin from 4.7.3.6 to 4.8.2.0.
Release notes
Sourced from com.github.spotbugs:spotbugs-maven-plugin's releases.
... (truncated)
Commits
4737e86
[maven-release-plugin] prepare release spotbugs-maven-plugin-4.8.2.06052ca1
[pom] Sort order of maven reporting api/impl4ed4adc
Merge pull request #687 from hazendaz/master8b483fc
[pom] Remove clean goal from invoker as its never existed7ef8b3a
Merge pull request #686 from hazendaz/master2f28d9c
[GHA] For maven wrapper downloads, just use maven there to do that instead of...c0b13b8
Merge pull request #684 from spotbugs/renovate/maven-3.x1df4aad
Merge pull request #685 from spotbugs/renovate/mavenversiondb1181d
Update mavenVersion to v3.9.622ff278
Update dependency maven to v3.9.6You can trigger a rebase of this PR by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show