[I] LLM found bugs [maven-help-plugin]

["elharo (via GitHub)" <gi...@apache.org>]

elharo opened a new issue, #380:
URL: https://github.com/apache/maven-help-plugin/issues/380

   ### Affected version
   
   HEAD
   
   ### Bug description
   
   Bug Analysis for maven-help-plugin
   
   HIGH Severity
   1. EffectivePomMojo.cleanModel() mutates the live project's Model in-place (EffectivePomMojo.java:210-213)
   private static void cleanModel(Model pom) {
       Properties properties = new SortedProperties();
       properties.putAll(pom.getProperties());
       pom.setProperties(properties);  // <-- modifies the real project model
   }
   Called from writeEffectivePom() at line 182 on project.getModel(). This permanently replaces the project's Properties object with a SortedProperties instance, altering property iteration order for the remainder of the build. If any subsequent mojo or plugin depends on insertion order or object identity of Properties, it will break. This is a data corruption side effect in what should be a read-only display operation.
   2. EffectiveSettingsMojo mutates the original Settings object when showPasswords=true (EffectiveSettingsMojo.java:87-94)
   
   ```
   if (showPasswords) {
       copySettings = settings;  // <-- uses original, not a copy
   } else {
       copySettings = copySettings(settings);
       ...
   }
   // ...
   writeEffectiveSettings(copySettings, writer); // calls cleanSettings() which modifies profiles
   ```
   
   When -DshowPasswords=true, the original Maven Settings object is used directly. writeEffectiveSettings -> cleanSettings() then replaces each profile's Properties with a SortedProperties instance (same pattern as bug #1). This mutates the global shared settings object, corrupting state for subsequent lifecycle steps.
   3. DescribeMojo.lookupPluginDescriptor() error message references null mojo fields instead of PluginInfo (DescribeMojo.java:333-338)
   throw new MojoExecutionException(
       "Error retrieving plugin descriptor for:" + LS + LS + "groupId: '"
           + groupId + "'" + LS + "artifactId: '" + artifactId + "'" + LS + "version: '" + version + "'" + LS + LS, e);
   When the plugin was specified by prefix (e.g., -Dplugin=help), the mojo fields groupId, artifactId, version are all null. The method parameter PluginInfo pi contains the actual values (pi.getGroupId(), etc.) but the error message ignores them. Output: groupId: 'null'\nartifactId: 'null'\nversion: 'null' — a misleading error that defeats debugging.
   MEDIUM Severity
   4. AllProfilesMojo.addProjectPomProfiles() uses parent context for active profiles (AllProfilesMojo.java:153-156)
   if (project.getActiveProfiles() != null) {
       for (Profile profile : project.getActiveProfiles()) {
           activeProfiles.put(profile.getId(), profile);
       }
   }
   project = project.getParent();  // walks up to parent
   When the loop reaches a parent POM, project.getActiveProfiles() returns profiles active in the parent's build context. These may not be active for the child project (different JDK, OS, properties). The output will incorrectly mark parent profiles as active when they aren't applicable to the child.
   5. EffectiveSettingsMojo.copySettings() doesn't deep-copy profiles (EffectiveSettingsMojo.java:156-198)
   Only servers and proxies are manually deep-copied. The profiles list is shared with the original via SettingsUtils.copySettings(). While hidePasswords() only targets servers/proxies, profiles can also contain sensitive data (passwords in profile properties) that could be exposed if the original settings object is later queried expecting the copy to be isolated.
   6. ListDependencyTypesMojo, ListLifecyclePhasesMojo, ListPackagingMojo write to console AND file simultaneously (e.g., ListDependencyTypesMojo.java:95-96)
   getLog().info(LS + "Maven Dependency Types defined:" + LS + LS + descriptionBuffer);
   writeFile(output, descriptionBuffer);  // always called, even when output is null
   These three mojos always write to the log via getLog().info(...) and unconditionally call writeFile(output, ...) (which is a no-op when output is null). Every other mojo (ActiveProfilesMojo, AllProfilesMojo, DescribeMojo, SystemMojo) uses if (output != null) { ... } else { getLog().info(...); }. The dual-write is inconsistent and means content is always printed to console even when writing to a file. They also don't prepend the "Generated by Maven Help Plugin" header that ActiveProfilesMojo and SystemMojo add.
   LOW Severity
   7. DescribeMojo.toLines() delegates to generated HelpMojo via fragile reflection (DescribeMojo.java:747-778)
   Method m = HelpMojo.class.getDeclaredMethod("toLines", String.class, Integer.TYPE, Integer.TYPE, Integer.TYPE);
   m.setAccessible(true);
   List<String> output = (List<String>) m.invoke(HelpMojo.class, text, indent, indentSize, lineLength);
   HelpMojo is generated by maven-plugin-tools at compile time (in target/generated-sources/). It IS available at runtime, but this reflection is fragile: any change in the generated code's toLines signature silently breaks all describe output formatting. The HelpMojo.toLines internally calls repeat() with new StringBuilder(repeat * str.length()) — a classic integer overflow risk that could throw NegativeArraySizeException (caught, but still a brittle chain).
   8. AbstractHelpMojo.LS uses System.getProperty("line.separator") (AbstractHelpMojo.java:55)
   protected static final String LS = System.getProperty("line.separator");
   Should use System.lineSeparator() (Java 7+), which falls back to "\n" if the property is missing. Additionally, DescribeMojoTest inconsistently uses both System.getProperty("line.separator") (line 88) and System.lineSeparator() (line 110).
   9. SortedProperties uses raw types (AbstractEffectiveMojo.java:147-148)
   List list = new ArrayList(keynames);
   Collections.sort(list);
   The @SuppressWarnings({"rawtypes", "unchecked"}) annotation masks the issue rather than using List<Object>.
   10. AbstractHelpMojo.getAetherArtifact() uses deprecated Artifact.LATEST_VERSION (AbstractHelpMojo.java:147)
   version = Artifact.LATEST_VERSION;
   Deprecated in favor of null (which means "latest") in the Aether API.
   11. EffectivePomMojo encoding fallback can return null (EffectivePomMojo.java:111)
   String encoding = output != null ? project.getModel().getModelEncoding() : System.getProperty("file.encoding");
   System.getProperty("file.encoding") can theoretically return null.
   12. AllProfilesMojo — settings profiles added to allProfilesByIds but never tracked for active status (AllProfilesMojo.java:167-173)
   addSettingsProfiles() only populates allProfilesByIds, never activeProfilesByIds. Active detection relies on project.getActiveProfiles() which may or may not include converted settings profiles depending on Maven version and activation mechanisms. This can under-report active settings profiles.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] LLM found bugs [maven-help-plugin]

["elharo (via GitHub)" <gi...@apache.org>]

elharo commented on issue #380:
URL: https://github.com/apache/maven-help-plugin/issues/380#issuecomment-4857079630

   Individual issues have been filed for all 12 bugs:
   
   - #384 — EffectivePomMojo.cleanModel() mutates the live project Model in-place
   - #385 — EffectiveSettingsMojo mutates original Settings when showPasswords=true
   - #386 — DescribeMojo.lookupPluginDescriptor() error message references null mojo fields
   - #387 — AllProfilesMojo.addProjectPomProfiles() uses parent build context for active profiles
   - #388 — EffectiveSettingsMojo.copySettings() does not deep-copy profiles
   - #389 — List*Mojos write to console AND file simultaneously
   - #390 — DescribeMojo.toLines() delegates to generated HelpMojo via fragile reflection
   - #391 — AbstractHelpMojo.LS uses System.getProperty instead of System.lineSeparator()
   - #392 — SortedProperties uses raw types
   - #393 — AbstractHelpMojo.getAetherArtifact() uses deprecated Artifact.LATEST_VERSION
   - #394 — EffectivePomMojo encoding fallback can return null
   - #395 — AllProfilesMojo settings profiles never tracked for active status


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] LLM found bugs [maven-help-plugin]

["elharo (via GitHub)" <gi...@apache.org>]

elharo closed issue #380: LLM found bugs
URL: https://github.com/apache/maven-help-plugin/issues/380


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org