Java

[aksOps]

No description provided.

To fix the problem, explicitly restrict the GITHUB_TOKEN permissions for this workflow to the minimum required. Since the jobs only check out code, set up Java, run Maven, and upload artifacts, they only need read access to repository contents. The simplest safe fix is to add a root-level permissions: block with contents: read, which will apply to all jobs that don’t override it.

Concretely:

• Edit .github/workflows/ci-java.yml.
• Insert a permissions: block near the top of the workflow (after name: and before on: or after on:), setting contents: read.
• This single block covers both build and cross-platform jobs, and does not change any functionality of the workflow other than tightening token permissions.

No additional methods, imports, or external definitions are needed—this is purely a YAML configuration change.

Generally, to fix this type of problem, you add a permissions section that explicitly restricts the GITHUB_TOKEN to the least privileges needed. For a typical CI workflow that only checks out code, runs builds/tests, and uploads artifacts, contents: read is sufficient, and you can declare it either at the workflow root (applies to all jobs) or per job.

For this specific workflow in .github/workflows/ci-java.yml, the best fix without changing behavior is to add a workflow-level permissions block right after the name (or after on:) so that both build and cross-platform jobs inherit it. Since the jobs only read repository contents and do not use any write operations (no releases, PR updates, issue modifications, etc.), we can safely set:

permissions:
  contents: read

No additional imports or methods are needed; this is pure YAML configuration. The change will ensure that, regardless of organization/repo defaults, this workflow always runs with a read-only GITHUB_TOKEN for repository contents and satisfies the CodeQL rule.

In general, the fix is to explicitly declare a permissions: block that grants only the minimal scopes required. For this workflow, the job needs to check out code (which uses the GITHUB_TOKEN for repository access) and then run Maven and SonarCloud analysis using an explicit SONAR_TOKEN secret, not the GITHUB_TOKEN. Therefore, the GITHUB_TOKEN only needs read access to the repository contents. If the build pulled GitHub Packages, we might also add packages: read, but there’s no evidence of that here.

The best minimal fix, without altering existing behavior, is to add permissions: contents: read at the job level for the sonar job. That ensures this job’s token can read repository contents but cannot perform write operations like pushing commits, modifying issues, or updating pull requests. Concretely, in .github/workflows/sonarcloud-java.yml, under jobs: sonar:, insert:

    permissions:
      contents: read

between runs-on: ubuntu-latest and steps:. This maintains current functionality (checkout still works, Maven and SonarCloud still run) while constraining the token as recommended.

In general, to fix user-controlled integer arithmetic that might overflow, you either (1) validate or cap the user inputs to a safe range before using them in arithmetic expressions, (2) perform overflow-safe arithmetic with checks, or (3) use a wider type and still cap to logical bounds (like list size). Here, we only need to ensure that pagination arithmetic for start + limit cannot overflow while preserving current API behavior (limit and offset are still int, and negative inputs continue to behave sensibly).

The minimal, behavior-preserving fix is to compute end using an overflow-safe approach that never relies on unchecked int addition. Since edges.size() is an int, we can clamp limit so that start + safeLimit is guaranteed not to exceed edges.size() and cannot overflow. A simple pattern is:

1. Compute start as now: int start = Math.min(Math.max(offset, 0), edges.size()); (also guarding negative offsets).
2. Compute a non-negative safeLimit that respects both limit and the remaining list size without overflowing: for example:
   • First ensure limit is non-negative: int nonNegativeLimit = Math.max(limit, 0);
   • Then cap it to the remaining elements: int safeLimit = Math.min(nonNegativeLimit, edges.size() - start);
3. Compute end as start + safeLimit. Because safeLimit <= edges.size() - start, start + safeLimit stays within [start, edges.size()] and cannot overflow, given that both start and edges.size() are in [0, Integer.MAX_VALUE].

This avoids any unchecked start + limit expression and introduces simple validation on limit/offset without changing the overall semantics for typical (non-pathological) inputs. We only need to change QueryService.listEdges at lines 130–132 in src/main/java/io/github/randomcodespace/iq/query/QueryService.java. No new methods or imports are required.

Note: There is a very similar pattern in GraphController.listEdges (lines 235–239) also using start + limit; however, the CodeQL path we must fix ends at QueryService.listEdges. The instructions require us to only change shown snippets, so we will confine our edits to QueryService.listEdges.

---

In general, to fix this issue, replace the use of MD5 with a strong modern hash algorithm such as SHA‑256. That means changing MessageDigest.getInstance("MD5") to MessageDigest.getInstance("SHA-256"), updating comments and error messages to reflect the algorithm change, and ensuring that all hash outputs remain lowercase hex strings so callers still receive the same type of output (though the actual values and length will differ).

The best way to fix this without changing functionality beyond what is necessary is:

• Keep the public API the same: hash(Path) and hashString(String) still return a lowercase hex string; method names and signatures remain unchanged.
• Switch the underlying digest algorithm from MD5 to SHA‑256 in both methods.
• Update the Javadoc and class‑level comments to describe “SHA‑256” instead of “MD5”.
• Update the RuntimeException messages so they correctly mention SHA‑256.
• No new imports are needed; MessageDigest already supports "SHA-256" and we still use HexFormat.

All changes are confined to src/main/java/io/github/randomcodespace/iq/cache/FileHasher.java, on the lines that specify the algorithm string and the associated Javadoc/comments.

In general, the fix is to replace the weak hash function (MD5) with a strong, modern one such as SHA‑256. For Java’s MessageDigest, this means changing the algorithm string passed to getInstance from "MD5" to "SHA-256" (or similar), and updating any documentation that specifically refers to MD5. SHA‑256 is widely supported in the standard JDK, so this does not require new dependencies.

Concretely for this file:

• In hash(Path file), change MessageDigest.getInstance("MD5") to MessageDigest.getInstance("SHA-256"), and update the Javadoc to describe SHA‑256 instead of MD5.
• In hashString(String content), change MessageDigest.getInstance("MD5") to MessageDigest.getInstance("SHA-256"), and likewise update the Javadoc.
• Adjust the error messages in the NoSuchAlgorithmException handlers so they mention "SHA-256 not available" instead of "MD5 not available".
• Update the class comment to indicate that SHA‑256 (a strong modern hash) is used for content‑change detection; while cryptographic strength is not required for this use, using SHA‑256 removes the warning without altering observable behavior, other than producing different hash values. Callers that persist hashes will see different digests, but functionally the behavior (consistent, deterministic hashing for change detection) is unchanged.

No new methods or imports are needed; MessageDigest already supports "SHA-256" in standard Java.

In general, TOCTOU issues like this are fixed by making the check and the use part of the same critical section, so that no other thread can intervene and change the relevant state between them. Here, that means ensuring that isCached(hash), loadCachedResults(hash), and storeResults(...) for a given cacheRef are executed atomically with respect to other threads that might also be using cacheRef.

The best fix, without changing external behavior, is to synchronize on a monitor that protects access to the shared AnalysisCache instance. Since we do not control AnalysisCache’s implementation here, the simplest, non-invasive option is to synchronize on cacheRef when performing the read/write sequence. We wrap the entire “check cache / possibly analyze / possibly store results” block inside a synchronized (cacheRef) block. This guarantees: (1) no two threads can simultaneously perform isCached/loadCachedResults/storeResults on the same cacheRef, and (2) the state of the cache cannot change between isCached and loadCachedResults for that instance. We keep the outer if (cacheRef != null) as-is, but move all subsequent cache interactions into the synchronized block. No new methods are required; no existing imports change. All modifications are confined to the anonymous task body in src/main/java/io/github/randomcodespace/iq/analyzer/Analyzer.java at lines 221–248.

In general, TOCTOU issues around caches or resources are best solved by avoiding separate “check then act” patterns on shared mutable state. Instead, perform the operation that might fail directly (here, loadCachedResults) and handle the failure (null result) in the same place, or ensure both the check and use occur under a shared lock. Since loadCachedResults already returns null when no cached entry exists, the preceding isCached call is redundant and can be removed.

The best fix here, without changing existing functionality, is to eliminate the TOCTOU by:

• Removing the if (cache.isCached(hash)) { ... } branch.
• Always calling cache.loadCachedResults(hash) once.
• If the result is non-null, use it and increment batchCacheHits.
• If it is null, fall back to analyzeFile and optionally store results in the cache.

This preserves behavior: previously, if isCached(hash) was true but loadCachedResults returned null (due to concurrent change), the code would just skip the cached path and proceed to analyze the file. After the change, we simply detect that via a single loadCachedResults returning null and do the same. All required changes are confined to the lambda submitted to executor in runBatchedWithCache in Analyzer.java, around lines 489–507. No new imports or helper methods are necessary.

In general, to fix “executing a command with a relative path,” you should ensure the command uses an absolute path, not just the bare executable name, or otherwise ensure it is resolved in a controlled/safe way. For tools like git that may be installed in different locations, a common approach is: (1) resolve the command to an absolute path once (e.g., at startup) using a controlled search strategy, (2) cache that absolute path, and (3) use it in all subsequent ProcessBuilder calls. If resolution fails, fall back to the current behavior (or disable the functionality) in a safe way.

For this specific code, the best fix without changing visible functionality is:

• Introduce a small helper method in BundleCommand that tries to locate git as an absolute path:
  • On Unix-like systems, first try /usr/bin/git and /usr/local/bin/git (common install locations), verifying with Files.isExecutable.
  • If those fail, fall back to "git" as-is (so existing behavior continues where PATH is trusted).
• Use the result of this helper in both bundleSourceFiles and getGitSha instead of the string literal "git".

This keeps behavior compatible (users still only need git in their PATH, or in standard locations), while addressing the CodeQL warning by ensuring that, where possible, an absolute path is used. All needed types (java.nio.file.Files, java.nio.file.Paths) are already partially imported: Files is imported, but we can rely only on Files and Path.of(...) to avoid new imports. The changes are all within src/main/java/io/github/randomcodespace/iq/cli/BundleCommand.java: add a private resolveGitCommand() method near the other private helpers and adjust the two ProcessBuilder instantiations to call it.

In general, to fix this issue we should ensure the ProcessBuilder is invoked with an absolute path to the git executable rather than the bare "git" command name. That way, the OS does not have to consult PATH, avoiding the possibility that a manipulated PATH points to a malicious executable.

The best way here, without changing existing behavior, is to introduce a private helper that determines the path to the Git executable once and then use that in isGitRepo (and likely any other Git invocations in this class). Because we must not change behavior significantly and we only see this snippet, we can implement this as: use a configurable absolute path from CodeIqConfig if available (if such a method exists), else fall back to "git" but pre-resolve it to an absolute path via ProcessBuilder or a small lookup, and finally still fall back to "git" if we cannot resolve it. However, per instructions, we cannot assume new methods on CodeIqConfig that are not shown, so we should keep it self-contained: implement a small resolveGitExecutable() that:

• First checks the GIT_EXECUTABLE environment variable (a common pattern) to get an absolute path if set.
• Otherwise, attempts a minimal lookup by invoking which git on Unix-like systems and where git on Windows, capturing the resolved absolute path.
• If all resolution steps fail, keep using "git" as a final fallback (this preserves current behavior while making the normal case safer).

We then modify isGitRepo(Path root) so that instead of "git" it uses the result of resolveGitExecutable(). This change is localized to FileDiscovery.java, requires no new imports beyond what already exists (we can reuse ProcessBuilder and existing IOException), and does not alter the overall logic: we still run git rev-parse --git-dir in the same directory and interpret the exit code the same way.

In general, to fix this kind of problem you should avoid executing commands by bare name and instead use an absolute path to the executable, or resolve the executable’s location in a controlled manner (e.g., by checking a small set of known safe locations or a trusted configuration value). If resolution fails, the code should behave gracefully (e.g., return null here) rather than executing an arbitrary command from PATH.

For this concrete case, the best fix without changing existing functionality is:

• Add a small helper method inside Analyzer that tries to resolve an absolute path to git:
  • First, check a trusted environment variable like GIT_EXEC_PATH (if present) to build a candidate path.
  • Then, check a small list of typical installation paths (/usr/bin/git, /usr/local/bin/git, C:\Program Files\Git\bin\git.exe, etc.), using Files.isExecutable to ensure the file exists and is executable.
  • Return the first matching absolute path as a String, or null if none is found.
• Update getGitHead to:
  • Call this helper to obtain gitPath.
  • If gitPath is null, log at debug and return null without starting a process.
  • Otherwise, construct the ProcessBuilder with gitPath as the executable instead of "git".

All changes are confined to Analyzer.java:

• Add a private static helper method (e.g., resolveGitExecutable()) somewhere near getGitHead.
• Change the ProcessBuilder construction in getGitHead on lines 744–746 accordingly.
  No new imports are strictly necessary beyond what’s already there, since we already import java.nio.file.Files and java.nio.file.Path.