fix: throw error on gradle command failure instead of returning empty…

[a-oren]

… sbom

Description

GradleProvider used Operations.runProcessGetOutput() which never checked
the process exit code. When Gradle evaluation failed (e.g., unresolvable
version catalogs), the error was silently ignored and an empty SBOM was
returned to the user.
Switched getDependencies() and getProperties() to use
runProcessGetFullOutput() which properly captures stdout, stderr, and
exit code. Non-zero exit codes now throw a RuntimeException with the
Gradle error message.
Also added null-safety defaults in getRoot() for edge cases with
partial properties output.

Related issue (if any): fixes #361

Checklist

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

Additional information

[qodo-code-review]

Review Summary by Qodo

Fix Gradle provider to throw errors on command failure

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Replace runProcessGetOutput() with runProcessGetFullOutput() to capture exit codes
• Throw RuntimeException when Gradle commands fail instead of silently returning empty SBOM
• Add null-safety defaults in getRoot() for missing properties
• Add comprehensive test coverage for Gradle command failures

Diagram

flowchart LR
  A["Gradle Command Execution"] -->|Previously| B["runProcessGetOutput<br/>ignores exit code"]
  A -->|Now| C["runProcessGetFullOutput<br/>captures exit code"]
  B --> D["Silent Failure<br/>Empty SBOM"]
  C --> E{"Exit Code == 0?"}
  E -->|Yes| F["Return Output"]
  E -->|No| G["Throw RuntimeException<br/>with error details"]
  H["Missing Properties"] -->|Now| I["Use Defaults<br/>unknown/0.0.0"]

Loading

File Changes

1. src/main/java/io/github/guacsec/trustifyda/providers/GradleProvider.java 🐞 Bug fix +29/-12

Add exit code validation and error handling

• Replace Operations.runProcessGetOutput() with Operations.runProcessGetFullOutput() in
 getDependencies() and getProperties() methods
• Add exit code validation that throws RuntimeException with descriptive error messages when
 Gradle commands fail
• Add null-safety checks in getRoot() using getOrDefault() for group and version properties
• Add null/empty check for rootName with fallback to "unknown"

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

---

2. src/test/java/io/github/guacsec/trustifyda/providers/Gradle_Provider_Test.java 🧪 Tests +144/-18

Update tests for exit code validation

• Update mock setup to use runProcessGetFullOutput() instead of runProcessGetOutput()
• Change argument matchers from string equality to array-based matchers for command validation
• Return Operations.ProcessExecOutput objects with exit code 0 for successful cases
• Add two new test methods: test_provideStack_throws_on_gradle_failure() and
 test_provideComponent_throws_on_gradle_failure() to verify exception handling
• Add imports for assertThatThrownBy, isNull, Arrays, and @Test annotation

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

---

[qodo-code-review]

Code Review by Qodo

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

1. Timeout exitValue crash ☑ 🐞 Bug ⛯ Reliability

Description

GradleProvider now uses Operations.runProcessGetFullOutput(), whose implementation ignores the
boolean result of waitFor(30s) and then unconditionally calls process.exitValue(); if the process is
still running, this throws IllegalThreadStateException and aborts analysis with an unhelpful runtime
crash. Additionally, because stdout/stderr are fully read before waitFor, the intended timeout may
not actually prevent hangs for long-running Gradle invocations.

Code

src/main/java/io/github/guacsec/trustifyda/providers/GradleProvider.java[R229-238]

+    var result =
+        Operations.runProcessGetFullOutput(
+            Path.of(manifestPath.getParent().toString()), cmdList, null);
+
+    if (result.getExitCode() != 0) {
+      throw new RuntimeException(
+          String.format(
+              "gradle dependencies command failed with exit code %d for manifest '%s': %s",
+              result.getExitCode(), manifestPath, result.getError()));
+    }

Evidence

The PR changes GradleProvider to call runProcessGetFullOutput() for gradle dependencies / `gradle
properties`. In Operations.runProcessGetFullOutput(), the code reads stdout/stderr, then calls
process.waitFor(30L, TimeUnit.SECONDS) but ignores its return value and immediately calls
process.exitValue(). Per the Java Process contract, exitValue throws if the process hasn’t
terminated, so a slow/hung Gradle process can now crash GradleProvider with
IllegalThreadStateException (or hang before reaching waitFor due to blocking stream reads).

src/main/java/io/github/guacsec/trustifyda/providers/GradleProvider.java[224-261]
src/main/java/io/github/guacsec/trustifyda/tools/Operations.java[195-238]

Agent prompt

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

## Issue description
`Operations.runProcessGetFullOutput()` ignores the `waitFor(timeout)` result and calls `exitValue()` unconditionally, which can throw `IllegalThreadStateException` if the process is still running. GradleProvider now depends on this method for `gradle dependencies` and `gradle properties`, so slow or stuck Gradle runs can crash analysis or hang.
### Issue Context
The PR routes Gradle execution through `runProcessGetFullOutput()` to capture exit codes, but that helper’s timeout handling is unsafe.
### Fix Focus Areas
- src/main/java/io/github/guacsec/trustifyda/tools/Operations.java[195-238]
- src/main/java/io/github/guacsec/trustifyda/providers/GradleProvider.java[224-261]
### Implementation notes
- Check the boolean return of `process.waitFor(…, …)`.
- If `false`, destroy the process (and ideally its descendants) and return a `ProcessExecOutput` with a sentinel exit code, or throw a clear exception (e.g., `RuntimeException("Command timed out…")`).
- Avoid potential blocking on stdout/stderr reads.
- Either call `waitFor` first and then read streams, or consume stdout/stderr concurrently (e.g., separate threads / executors) before waiting.
- Preserve the interrupt flag if catching `InterruptedException` (e.g., `Thread.currentThread().interrupt()`) before throwing.

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

---

2. Temp files leak on errors ☑ 🐞 Bug ⛯ Reliability

Description

getDependencies() and getProperties() create temp files before running Gradle, but on non-zero exit
code they throw immediately without deleting the newly created temp files. This increases temp-file
leakage in the exact failure scenarios this PR is targeting (e.g., Gradle evaluation errors).

Code

src/main/java/io/github/guacsec/trustifyda/providers/GradleProvider.java[R225-238]

var tempFile = Files.createTempFile("TRUSTIFY_DA_graph_", null);
-    // the command will create the dependency tree in the temp file
String gradleCommand = gradleExecutable + " dependencies";
-
String[] cmdList = gradleCommand.split("\\s+");
-    String gradleOutput =
-        Operations.runProcessGetOutput(Path.of(manifestPath.getParent().toString()), cmdList);
-    Files.writeString(tempFile, gradleOutput);
+    var result =
+        Operations.runProcessGetFullOutput(
+            Path.of(manifestPath.getParent().toString()), cmdList, null);
+
+    if (result.getExitCode() != 0) {
+      throw new RuntimeException(
+          String.format(
+              "gradle dependencies command failed with exit code %d for manifest '%s': %s",
+              result.getExitCode(), manifestPath, result.getError()));
+    }

Evidence

In getDependencies(), the temp file is created first, then the Gradle command is executed and
checked; on failure the method throws and never deletes tempFile. The same pattern exists in
getProperties() with propsTempFile. Because the exception is thrown before returning the path,
callers cannot clean up these temp files.

src/main/java/io/github/guacsec/trustifyda/providers/GradleProvider.java[224-262]

Agent prompt

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

## Issue description
Temp files are created before running Gradle, but failure now throws early, leaving those temp files behind.
### Issue Context
This PR intentionally turns Gradle failures into exceptions. Those same failure cases now leak temp files because cleanup doesn’t run.
### Fix Focus Areas
- src/main/java/io/github/guacsec/trustifyda/providers/GradleProvider.java[224-262]
### Implementation notes
- Prefer creating the temp file *after* confirming exit code == 0.
- Example: run gradle, check exit code, then `Files.createTempFile(...)` and `Files.writeString(...)`.
- If you must create the temp file up front, ensure deletion in the failure path:
- Wrap execution in try/catch, `Files.deleteIfExists(tempFile)` before rethrow.
- Apply the same pattern to both `getDependencies()` and `getProperties()`.

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

---

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

[soul2zimate]

Could you take a look at the qodo review comments ? look like legitimate to me

[soul2zimate]

could you also fix the potential temp files leak on errors ? thanks

[soul2zimate]

@a-oren thanks for the quick fixes, but the changes fail gradle- related integration tests in CI due to the timeout.