Skip to content

COR-1258: inactive Maven profile sections no longer leak into the base dependency graph - #145

Merged
juangaitanv merged 1 commit into
mainfrom
cor-1258-profile-section-leaks
Aug 3, 2026
Merged

juangaitanv merged 1 commit into
mainfrom
cor-1258-profile-section-leaks

Conversation

@juangaitanv

Copy link
Copy Markdown
Contributor

Summary

Addresses the two open automated-review findings on #142 (review):

  • Inactive profile dependencyManagement leaked into direct dependencies: split_dependency_management ran on the raw POM before <profiles> were stripped, so a profile-only managed version (e.g. 9.9.9) pinned a versionless base dependency. Sections are now stripped first; the dependency stays unresolved instead of taking the inactive profile's version.
  • Reporting-plugin dependencies emitted as application dependencies: <reporting> joins <profiles>/<build> in the stripped set.

Follow-through from the same reordering:

  • parse_pom_properties now also reads the stripped text — a profile's <properties> block could previously resolve ${...} placeholders in base dependencies (same leak, one line above the fix). Profile-only placeholders now pass through unresolved.
  • The section-tag list is one shared constant (NON_DEPENDENCY_SECTIONS) used by both parse_pom_dependencies and pom_project_version, so the two lists can't drift.

Effective-POM territory (profile activation, parent chains, imported BOMs) stays out of scope, tracked in COR-1733.

Test plan

  • New fixture java-maven-profile-mgmt + test asserting an inactive profile's managed version does not pin a base dependency and profile-only properties do not resolve base placeholders — verified to fail against the old code (lib@9.9.9) and pass with the fix
  • java-maven-plugin-deps fixture extended with a <reporting> plugin dependency; exclusion asserted alongside the existing build/profile cases
  • cargo test — 555 passed, 0 failed; clippy (strict) and fmt clean

…y and dependencyManagement extraction

An inactive profile's dependencyManagement (and properties) were parsed
from the raw POM before section stripping, so a profile-only managed
version could pin a versionless base dependency, and reporting-plugin
dependencies surfaced as application dependencies. Strip sections first
and feed properties and management extraction from the stripped text.

Addresses the two open review findings on PR #142.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No actionable findings. I verified the merge-base diff and the Maven parser call path: stripping now precedes both property and dependencyManagement extraction, the new inactive-profile fixture covers both leak paths, and the reporting fixture asserts exclusion of the nested reporting dependency (report-helper). The shared section list leaves project-level properties/management behavior covered by the existing Maven fixtures.

Validation: RUSTUP_TOOLCHAIN=stable ./harness test passed all 555 tests; clippy and format checks passed. The PR's Rust test job and completed platform builds are also passing.

Open in Web View Automation 

Sent by Cursor Automation: pr-flow

@corgea-security corgea-security left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated review risk: 2/5.

No actionable findings. The supplied diff consistently prevents profile, build, and reporting sections from influencing base dependency/property extraction, with targeted regression coverage.

No critical or high-priority changes were found.

@corgea-security corgea-security added the dennis-reviewed Dennis completed an automated review label Aug 3, 2026

@corgea-security corgea-security left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approved by Dennis: high policy risk and automated risk 2/5.

@juangaitanv
juangaitanv merged commit 015c6ca into main Aug 3, 2026
19 checks passed
@juangaitanv
juangaitanv deleted the cor-1258-profile-section-leaks branch August 3, 2026 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dennis-reviewed Dennis completed an automated review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants