Skip to content

fix: read ignore patterns from workspace member Cargo.toml files#375

Open
a-oren wants to merge 2 commits intoguacsec:mainfrom
a-oren:fix/members-ignore
Open

fix: read ignore patterns from workspace member Cargo.toml files#375
a-oren wants to merge 2 commits intoguacsec:mainfrom
a-oren:fix/members-ignore

Conversation

@a-oren
Copy link
Copy Markdown
Contributor

@a-oren a-oren commented Mar 26, 2026

Description

Previously, ignore patterns (exhortignore / trustify-da-ignore) were only read from the root Cargo.toml. In a virtual workspace, member crates have their own Cargo.toml with [dependencies], so ignore comments there were never detected. This caused the Java client to include dependencies that should have been ignored at the member level.

Now, when processing each workspace member for stack analysis, its own Cargo.toml is parsed for ignore patterns and merged with workspace-level ignores before walking the dependency tree.

Related issue (if any): fixes #374

Checklist

  • I have followed this repository's contributing guidelines.
  • I will adhere to the project's code of conduct.

Additional information

Anything else?

@a-oren a-oren requested a review from soul2zimate March 26, 2026 17:19
@qodo-code-review
Copy link
Copy Markdown
Contributor

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Read ignore patterns from workspace member Cargo.toml files

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Read ignore patterns from workspace member Cargo.toml files
• Merge member-level ignores with workspace-level ignores
• Add manifestPath field to CargoPackage model
• Add comprehensive tests for member ignore pattern detection
Diagram
flowchart LR
  A["Virtual Workspace"] -->|"for each member"| B["getMemberIgnoredDeps"]
  B -->|"read member Cargo.toml"| C["Parse ignore patterns"]
  C -->|"merge with workspace ignores"| D["Member ignored deps set"]
  D -->|"pass to processWorkspaceMember"| E["Process member dependencies"]
Loading

Grey Divider

File Changes

1. src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java 🐞 Bug fix +48/-1

Add member-level ignore pattern detection

• Added getMemberIgnoredDeps() method to read and merge ignore patterns from member Cargo.toml
 files
• Modified handleVirtualWorkspace() to call getMemberIgnoredDeps() for each workspace member
• Passes merged member-level and workspace-level ignored dependencies to processWorkspaceMember()
• Includes error handling for missing or unreadable member manifest files

src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java


2. src/main/java/io/github/guacsec/trustifyda/providers/rust/model/CargoPackage.java ✨ Enhancement +1/-0

Add manifestPath field to CargoPackage

• Added manifestPath field to CargoPackage record
• Maps to manifest_path JSON property from Cargo metadata

src/main/java/io/github/guacsec/trustifyda/providers/rust/model/CargoPackage.java


3. src/test/java/io/github/guacsec/trustifyda/providers/CargoProviderCargoParsingTest.java 🧪 Tests +79/-0

Add tests for member ignore pattern detection

• Added testMemberCargoTomlIgnorePatternsDetected() test for inline ignore comments
• Added testMemberIgnorePatternsWithTableFormat() test for table-format dependencies
• Tests verify both exhortignore and trustify-da-ignore patterns are detected
• Tests confirm non-ignored dependencies are not included in ignored set

src/test/java/io/github/guacsec/trustifyda/providers/CargoProviderCargoParsingTest.java


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Mar 26, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Remediation recommended

1. Member Cargo.toml failures ignored 📘 Rule violation ⛯ Reliability
Description
When reading a workspace member’s Cargo.toml fails (I/O error or TOML parse errors), the code
logs/returns workspaceIgnoredDeps and continues, which can silently produce incorrect analysis
results with member-level ignore directives not applied. This uses logging/early returns instead of
failing fast with an exception and actionable context.
Code

src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java[R227-256]

+    try {
+      TomlParseResult memberToml = Toml.parse(memberManifest);
+      if (memberToml.hasErrors()) {
+        return workspaceIgnoredDeps;
+      }
+      String memberContent = Files.readString(memberManifest, StandardCharsets.UTF_8);
+      Set<String> memberIgnored = getIgnoredDependencies(memberToml, memberContent);
+      if (memberIgnored.isEmpty()) {
+        return workspaceIgnoredDeps;
+      }
+      if (debugLoggingIsNeeded()) {
+        log.info(
+            "Found "
+                + memberIgnored.size()
+                + " ignored dependencies in member "
+                + memberPkg.name()
+                + ": "
+                + memberIgnored);
+      }
+      Set<String> merged = new HashSet<>(workspaceIgnoredDeps);
+      merged.addAll(memberIgnored);
+      return merged;
+    } catch (IOException e) {
+      log.warning(
+          "Failed to read member Cargo.toml for ignore patterns: "
+              + memberManifest
+              + ": "
+              + e.getMessage());
+      return workspaceIgnoredDeps;
+    }
Evidence
PR Compliance ID 4 requires throwing exceptions (with context) when required preconditions fail
rather than only logging and continuing. The new getMemberIgnoredDeps() returns
workspaceIgnoredDeps when memberToml.hasErrors() and catches IOException with only
log.warning(...), so the analysis proceeds without applying member ignore patterns.

src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java[227-256]
Best Practice: Repository guidelines

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getMemberIgnoredDeps()` currently continues when a member `Cargo.toml` cannot be read or parsed, which can cause member-level ignore patterns to be skipped without stopping execution.

## Issue Context
Compliance requires failing fast with an exception (including relevant context) when required preconditions fail; logging should not replace error signaling.

## Fix Focus Areas
- src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java[227-256]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Member ignore read can abort 🐞 Bug ⛯ Reliability
Description
getMemberIgnoredDeps() can throw unchecked exceptions (e.g., InvalidPathException/SecurityException)
because Path.of(...) and some file checks are outside the try/catch and only IOException is caught;
this can bubble up and cause addDependencies() to stop processing the entire workspace dependency
walk. Result: SBOM may miss most/all workspace member dependencies if one member manifest path is
problematic.
Code

src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java[R217-256]

+  private Set<String> getMemberIgnoredDeps(
+      String memberId, Map<String, CargoPackage> packageMap, Set<String> workspaceIgnoredDeps) {
+    CargoPackage memberPkg = packageMap.get(memberId);
+    if (memberPkg == null || memberPkg.manifestPath() == null) {
+      return workspaceIgnoredDeps;
+    }
+    Path memberManifest = Path.of(memberPkg.manifestPath());
+    if (!Files.isRegularFile(memberManifest)) {
+      return workspaceIgnoredDeps;
+    }
+    try {
+      TomlParseResult memberToml = Toml.parse(memberManifest);
+      if (memberToml.hasErrors()) {
+        return workspaceIgnoredDeps;
+      }
+      String memberContent = Files.readString(memberManifest, StandardCharsets.UTF_8);
+      Set<String> memberIgnored = getIgnoredDependencies(memberToml, memberContent);
+      if (memberIgnored.isEmpty()) {
+        return workspaceIgnoredDeps;
+      }
+      if (debugLoggingIsNeeded()) {
+        log.info(
+            "Found "
+                + memberIgnored.size()
+                + " ignored dependencies in member "
+                + memberPkg.name()
+                + ": "
+                + memberIgnored);
+      }
+      Set<String> merged = new HashSet<>(workspaceIgnoredDeps);
+      merged.addAll(memberIgnored);
+      return merged;
+    } catch (IOException e) {
+      log.warning(
+          "Failed to read member Cargo.toml for ignore patterns: "
+              + memberManifest
+              + ": "
+              + e.getMessage());
+      return workspaceIgnoredDeps;
+    }
Evidence
getMemberIgnoredDeps builds the Path and performs file checks before the try/catch and only catches
IOException; any unchecked exception will propagate out of handleVirtualWorkspace’s loop.
addDependencies catches Exception at the top-level and stops further dependency processing after
logging, so one failure can drop dependency analysis for the whole workspace invocation.

src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java[105-151]
src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java[213-257]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`getMemberIgnoredDeps()` can throw unchecked exceptions (e.g., `InvalidPathException`, `SecurityException`) because `Path.of(...)` and file checks occur outside the `try` block and only `IOException` is caught. In a virtual workspace, this can bubble up and cause `addDependencies()` to stop processing the rest of the members.

### Issue Context
This code runs inside the STACK virtual-workspace loop; an exception during ignore extraction should degrade to “no extra ignores for that member”, not abort the entire dependency walk.

### Fix Focus Areas
- src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java[213-257]
- src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java[105-151]

### Suggested fix
- Move `Path.of(memberPkg.manifestPath())` and any filesystem access inside the `try`.
- Catch broader exceptions (at least `RuntimeException`/`InvalidPathException`/`SecurityException` in addition to `IOException`) and return `workspaceIgnoredDeps`.
- Optionally, handle exceptions per-member in the `for` loop (so one bad member doesn’t stop processing other members).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

3. Merge behavior not tested 🐞 Bug ⚙ Maintainability
Description
The added tests validate ignore-pattern extraction via reflective access to the private
getIgnoredDependencies(), but they don’t test the new behavior that merges workspace-level ignores
with member-level ignores via getMemberIgnoredDeps(). This leaves the primary PR behavior unguarded
against regressions.
Code

src/test/java/io/github/guacsec/trustifyda/providers/CargoProviderCargoParsingTest.java[R792-806]

+    java.lang.reflect.Method method =
+        CargoProvider.class.getDeclaredMethod(
+            "getIgnoredDependencies", TomlParseResult.class, String.class);
+    method.setAccessible(true);
+
+    TomlParseResult tomlResult = Toml.parse(memberCargoToml);
+    String content = Files.readString(memberCargoToml, StandardCharsets.UTF_8);
+
+    @SuppressWarnings("unchecked")
+    Set<String> ignoredDeps = (Set<String>) method.invoke(provider, tomlResult, content);
+
+    assertTrue(ignoredDeps.contains("serde"), "serde should be ignored (exhortignore)");
+    assertFalse(ignoredDeps.contains("tokio"), "tokio should NOT be ignored");
+    assertTrue(ignoredDeps.contains("reqwest"), "reqwest should be ignored (trustify-da-ignore)");
+    assertEquals(2, ignoredDeps.size(), "Should find exactly 2 ignored dependencies in member");
Evidence
The new tests invoke only getIgnoredDependencies via reflection and never exercise
getMemberIgnoredDeps/virtual-workspace integration, despite the production change being the
merge-and-apply logic per member.

src/test/java/io/github/guacsec/trustifyda/providers/CargoProviderCargoParsingTest.java[770-847]
src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java[213-249]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Current tests verify member TOML ignore detection, but they don’t verify the new `getMemberIgnoredDeps()` merge behavior (workspace ignored deps + member ignored deps) that is used during virtual-workspace STACK processing.

### Issue Context
Without a merge/integration test, regressions like “member ignores not merged” or “workspace ignores lost” won’t be caught.

### Fix Focus Areas
- src/test/java/io/github/guacsec/trustifyda/providers/CargoProviderCargoParsingTest.java[770-847]
- src/main/java/io/github/guacsec/trustifyda/providers/CargoProvider.java[213-249]

### Suggested fix
- Add a unit test that:
 - Creates two Cargo.toml files: one representing workspace root (workspace ignored set can be a predefined Set in the test), and one member Cargo.toml with an ignore comment.
 - Constructs a minimal `packageMap` containing a `CargoPackage` with `manifestPath` pointing to the member Cargo.toml.
 - Invokes `getMemberIgnoredDeps` (via reflection or by widening visibility to package-private) and asserts the returned Set contains both workspace and member ignored deps.
- Prefer testing `getMemberIgnoredDeps` directly over only testing `getIgnoredDependencies`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo


CargoProvider provider = new CargoProvider(memberCargoToml);

java.lang.reflect.Method method =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just minor suggestion: we should not use reflection in tests

@a-oren a-oren requested a review from soul2zimate March 29, 2026 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cargo virtual workspace: exhortignore patterns in member Cargo.toml files are not respected

2 participants