Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces OWASP dependency-check with direct JAR introspection and optional Maven Central POM lookup; removes vulnerability tracking and bundled dependency-check artifacts; updates merge logic and binary output schema to drop vulnerability links. ChangesJAR Analysis and Merge
Packaging and Repository Cleanup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/fosslight_binary/_jar_analysis.py (1)
208-209: ⚡ Quick winElevate missing
no_nvd.propertieslog level to warning.If this file goes missing, behavior silently degrades;
debugcan hide packaging/runtime regressions during normal operation.Proposed tweak
- else: - logger.debug(f"no_nvd.properties not found at {_NO_NVD_PROPERTIES}.") + else: + logger.warning(f"no_nvd.properties not found at {_NO_NVD_PROPERTIES}.")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_binary/_jar_analysis.py` around lines 208 - 209, The current else branch that logs the missing no_nvd.properties file uses logger.debug which can hide missing-file issues; update the logging call in the else branch that references _NO_NVD_PROPERTIES to logger.warning (or logger.warn) and craft a concise message indicating the file was not found so missing no_nvd.properties is elevated to a warning level; locate the else block that currently calls logger.debug(f"no_nvd.properties not found at {_NO_NVD_PROPERTIES}.") and replace it with a warning-level log using the same _NO_NVD_PROPERTIES symbol.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_binary/_binary.py`:
- Around line 73-75: The items.append call in _binary.py currently inserts one
extra empty placeholder causing no-OSS rows to have more columns than OSS rows;
update the items.append for the no-OSS case to match the exact column count used
for OSS rows by removing the extra empty placeholder (adjust the list built in
items.append that includes self.source_name_or_path, exclude, self.comment,
self.tlsh, self.checksum so the number and order of elements match the OSS-row
construction).
---
Nitpick comments:
In `@src/fosslight_binary/_jar_analysis.py`:
- Around line 208-209: The current else branch that logs the missing
no_nvd.properties file uses logger.debug which can hide missing-file issues;
update the logging call in the else branch that references _NO_NVD_PROPERTIES to
logger.warning (or logger.warn) and craft a concise message indicating the file
was not found so missing no_nvd.properties is elevated to a warning level;
locate the else block that currently calls logger.debug(f"no_nvd.properties not
found at {_NO_NVD_PROPERTIES}.") and replace it with a warning-level log using
the same _NO_NVD_PROPERTIES symbol.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 571242a5-3051-445d-b7cf-50d887450601
📒 Files selected for processing (6)
.gitignorepyproject.tomlsrc/fosslight_binary/_binary.pysrc/fosslight_binary/_jar_analysis.pysrc/fosslight_binary/binary_analysis.pysrc/fosslight_binary/no_nvd.properties
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fosslight_binary/_jar_analysis.py (1)
87-96:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTreat checksum matches as matched before creating the fallback row.
When
bin.checksum == sha1, the OSS list is merged butfoundstaysFalse, so the fallback block still appends a syntheticBinaryItem. That can duplicate the same jar in the report whenever the path differs but the checksum matches.Proposed fix
else: if bin.checksum == sha1: + found = True merge_oss_and_vul_items(bin, oss_list) if not found:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_binary/_jar_analysis.py` around lines 87 - 96, The code merges OSS items when bin.checksum == sha1 but never marks the file as found, causing a fallback BinaryItem to be appended; inside the checksum match branch (the if comparing bin.checksum and sha1) set found = True (and optionally break/return from the loop if appropriate) so the subsequent fallback block that creates a new BinaryItem and appends to not_found_bin is skipped; update the logic around merge_oss_and_vul_items, the checksum comparison, and the variable found to reflect a successful match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/fosslight_binary/_jar_analysis.py`:
- Around line 87-96: The code merges OSS items when bin.checksum == sha1 but
never marks the file as found, causing a fallback BinaryItem to be appended;
inside the checksum match branch (the if comparing bin.checksum and sha1) set
found = True (and optionally break/return from the loop if appropriate) so the
subsequent fallback block that creates a new BinaryItem and appends to
not_found_bin is skipped; update the logic around merge_oss_and_vul_items, the
checksum comparison, and the variable found to reflect a successful match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 346e0196-47fd-4429-812a-c7da5584c8d1
📒 Files selected for processing (6)
.reuse/dep5pyproject.tomlsrc/fosslight_binary/_binary.pysrc/fosslight_binary/_jar_analysis.pysrc/fosslight_binary/binary_analysis.pysrc/fosslight_binary/no_nvd.properties
✅ Files skipped from review due to trivial changes (3)
- .reuse/dep5
- src/fosslight_binary/no_nvd.properties
- pyproject.toml
bjk7119
force-pushed
the
timeout
branch
4 times, most recently
from
35a8ba3 to
6bf9cdf
Compare
bjk7119
added
enhancement
bjk7119
force-pushed
the
timeout
branch
2 times, most recently
from
63cb97e to
1b7e70a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fosslight_binary/binary_analysis.py (1)
28-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove dead
get_java_versionfunction and its supporting imports.With OWASP dependency-check removed,
get_java_versionhas no remaining callers. Theimport subprocessandimport restatements (lines 28–29) exist solely to support this function. Removing them alongside the rest of the OWASP removal keeps the PR self-consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_binary/binary_analysis.py` around lines 28 - 29, Remove the dead get_java_version function and its unused supporting imports by deleting the get_java_version definition and removing the top-level imports of subprocess and re (which only existed to support that function); update any remaining imports or linting configuration if necessary to ensure no unused-import warnings remain and verify no other code references get_java_version.
🧹 Nitpick comments (2)
src/fosslight_binary/_jar_analysis.py (2)
281-336: 💤 Low value
successis never set toFalse; the caller's failure branch is dead.
analyze_jar_fileinitializessuccess = Trueand never reassigns it, sobinary_analysis.pylines 240-243’selse: logger.warning("Could not find OSS information for some jar files.")is unreachable. Either drivesuccessfrom real failure conditions (e.g., all retries exhausted, or_central_network_warnedgot tripped) or drop the flag entirely and just returnjar_items.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_binary/_jar_analysis.py` around lines 281 - 336, analyze_jar_file currently returns success=True unconditionally; change it to set success=False whenever a jar does not produce OSS info after all processing and retries: in the initial for jar_path loop, if _process_one_jar returns (None, needs_retry=False) for a non-excluded rel_path then set success=False; in the retry loop, when attempt >= _MAX_RETRY and you call _process_one_jar with search_timeout=0, if that call returns result is None set success=False; also set success=False if a jar exhausts retries (i.e., needs_retry remains True across attempts and never yields a result). Use the existing symbols analyze_jar_file, success, jar_items, retry_queue, _process_one_jar and _MAX_RETRY to locate where to assign success=False so the caller can detect failures.
54-60: 💤 Low valueJAR manifest parser doesn't handle continuation lines per specification.
Per the JAR specification, manifest values longer than 72 bytes are wrapped onto subsequent lines that start with a single space. The current parser treats each physical line independently, silently truncating continued values (e.g., long
Bundle-DocURLorImplementation-URL). Unfold continuation lines before splitting:♻️ Suggested implementation
def _parse_manifest_bytes(manifest_bytes): info = {} - for line in manifest_bytes.decode('utf-8', errors='replace').splitlines(): - if ':' in line: - k, _, v = line.partition(':') - info[k.strip()] = v.strip() + text = manifest_bytes.decode('utf-8', errors='replace') + # Unfold continuation lines (per JAR manifest spec: lines starting with a single space). + unfolded = re.sub(r'\r?\n ', '', text) + for line in unfolded.splitlines(): + if ':' in line: + k, _, v = line.partition(':') + info[k.strip()] = v.strip() return info(requires
import re)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_binary/_jar_analysis.py` around lines 54 - 60, The _parse_manifest_bytes function currently treats each physical line independently and drops manifest continuation lines; update _parse_manifest_bytes to first "unfold" the manifest by decoding bytes, splitting into physical lines, and joining any line that begins with a single space to the previous physical line (concatenate without the leading space) before splitting key/value pairs; then parse each unfolded logical line with the existing partition(':') logic to populate info. Ensure you operate on the decoded string and preserve existing error='replace' behavior used in manifest_bytes.decode.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_binary/_jar_analysis.py`:
- Around line 339-366: In merge_binary_list, avoid empty-SHA1 false positives
and duplicates by only using the checksum branch when sha1 is non-empty; when
bin.checksum == sha1 set bin.found_in_jar_analysis = True and mark found = True
so the item isn't appended to not_found_bin, and then call
bin.set_oss_items(oss_list) as before (use the existing symbols:
merge_binary_list, sha1, bin.checksum, bin.found_in_jar_analysis, found,
set_oss_items, not_found_bin, BinaryItem).
- Around line 313-323: The retry-max-out branch currently calls
_process_one_jar(jar_path, rel_path, sha1='', search_timeout=0) which force-sets
an empty sha1 into the resulting jar_items entry; instead preserve the real sha1
and prevent a Central lookup by either passing the original sha1 through (sha1
variable) and using a flag to suppress central search, or add a
skip_central=True parameter to _process_one_jar and call
_process_one_jar(jar_path, rel_path, sha1, skip_central=True) (or equivalent) so
the real SHA-1 is kept and Central search is disabled when retries are
exhausted.
- Line 12: The code currently imports xml.etree.ElementTree and calls
ET.fromstring in _parse_pom_bytes which is vulnerable; replace the import with
defusedxml.ElementTree (import defusedxml.ElementTree as ET) and update
_parse_pom_bytes to use ET.fromstring (or ET.parse if switching to file-like
input) so entity expansion and external-entity attacks are mitigated; also add
defusedxml to project dependencies (pyproject.toml) so the secure parser is
installed.
---
Outside diff comments:
In `@src/fosslight_binary/binary_analysis.py`:
- Around line 28-29: Remove the dead get_java_version function and its unused
supporting imports by deleting the get_java_version definition and removing the
top-level imports of subprocess and re (which only existed to support that
function); update any remaining imports or linting configuration if necessary to
ensure no unused-import warnings remain and verify no other code references
get_java_version.
---
Nitpick comments:
In `@src/fosslight_binary/_jar_analysis.py`:
- Around line 281-336: analyze_jar_file currently returns success=True
unconditionally; change it to set success=False whenever a jar does not produce
OSS info after all processing and retries: in the initial for jar_path loop, if
_process_one_jar returns (None, needs_retry=False) for a non-excluded rel_path
then set success=False; in the retry loop, when attempt >= _MAX_RETRY and you
call _process_one_jar with search_timeout=0, if that call returns result is None
set success=False; also set success=False if a jar exhausts retries (i.e.,
needs_retry remains True across attempts and never yields a result). Use the
existing symbols analyze_jar_file, success, jar_items, retry_queue,
_process_one_jar and _MAX_RETRY to locate where to assign success=False so the
caller can detect failures.
- Around line 54-60: The _parse_manifest_bytes function currently treats each
physical line independently and drops manifest continuation lines; update
_parse_manifest_bytes to first "unfold" the manifest by decoding bytes,
splitting into physical lines, and joining any line that begins with a single
space to the previous physical line (concatenate without the leading space)
before splitting key/value pairs; then parse each unfolded logical line with the
existing partition(':') logic to populate info. Ensure you operate on the
decoded string and preserve existing error='replace' behavior used in
manifest_bytes.decode.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b3bb40cd-6b41-48ff-ae83-053441964c1e
⛔ Files ignored due to path filters (81)
src/fosslight_binary/third_party/dependency-check/lib/aho-corasick-double-array-trie-1.2.3.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/android-json-0.0.20131108.vaadin1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/annotations-26.0.2-1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/ant-1.10.15.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/bcpg-jdk18on-1.78.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/bcprov-jdk18on-1.78.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-cli-1.10.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-codec-1.19.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-collections-3.2.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-collections4-4.5.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-compress-1.27.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-dbcp2-2.13.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-digester-2.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-io-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-jcs3-core-3.2.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-lang3-3.19.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-logging-1.3.4.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-pool2-2.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-text-1.14.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-validator-1.10.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/compiler-0.9.6.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/cpe-parser-3.0.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/dependency-check-cli-12.1.7.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/dependency-check-core-12.1.7.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/dependency-check-utils-12.1.7.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/error_prone_annotations-2.41.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/failureaccess-1.0.3.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/gson-2.9.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/guava-33.5.0-jre.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/h2-2.3.232.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/httpclient5-5.5.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/httpclient5-cache-5.4.3.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/httpcore5-5.3.6.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/httpcore5-h2-5.3.4.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/j2objc-annotations-3.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-annotations-2.20.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-core-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-databind-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-dataformat-yaml-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-datatype-jsr310-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-module-blackbird-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jakarta.json-2.0.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jakarta.transaction-api-1.3.3.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/javax.activation-api-1.2.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/javax.inject-1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/javax.ws.rs-api-2.0.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jaxb-api-2.3.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jcs3-slf4j-1.0.5.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jdiagnostics-1.0.7.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jmustache-1.16.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/joda-time-2.14.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jsoup-1.21.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jspecify-1.0.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jsr305-3.0.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jul-to-slf4j-1.7.36.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/listenablefuture-9999.0-empty-to-avoid-conflict-with-guava.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/logback-classic-1.2.13.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/logback-core-1.2.13.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-analysis-common-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-core-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-facet-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-queries-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-queryparser-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-sandbox-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/minlog-1.3.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/open-vulnerability-clients-7.3.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/ossindex-service-api-1.8.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/ossindex-service-client-1.8.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/package-url-java-1.2.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/packager-core-0.21.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/packager-rpm-0.21.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/packageurl-java-1.5.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/pecoff4j-0.0.2.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/retirejs-core-3.0.4.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/semver4j-5.8.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/slf4j-api-1.7.36.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/snakeyaml-2.4.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/spotbugs-annotations-4.9.6.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/toml4j-0.7.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/velocity-engine-core-2.4.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/xz-1.9.jaris excluded by!**/*.jar
📒 Files selected for processing (16)
.gitignore.reuse/dep5cli.specpyproject.tomlsrc/fosslight_binary/__init__.pysrc/fosslight_binary/_binary.pysrc/fosslight_binary/_binary_dao.pysrc/fosslight_binary/_jar_analysis.pysrc/fosslight_binary/binary_analysis.pysrc/fosslight_binary/third_party/dependency-check/LICENSE.txtsrc/fosslight_binary/third_party/dependency-check/NOTICE.txtsrc/fosslight_binary/third_party/dependency-check/README.mdsrc/fosslight_binary/third_party/dependency-check/bin/completion-for-dependency-check.shsrc/fosslight_binary/third_party/dependency-check/bin/dependency-check.batsrc/fosslight_binary/third_party/dependency-check/bin/dependency-check.shsrc/fosslight_binary/third_party/dependency-check/licenses/commons-cli/LICENSE.txt
💤 Files with no reviewable changes (9)
- src/fosslight_binary/third_party/dependency-check/licenses/commons-cli/LICENSE.txt
- src/fosslight_binary/third_party/dependency-check/NOTICE.txt
- src/fosslight_binary/third_party/dependency-check/bin/dependency-check.bat
- src/fosslight_binary/third_party/dependency-check/README.md
- src/fosslight_binary/third_party/dependency-check/bin/dependency-check.sh
- src/fosslight_binary/third_party/dependency-check/LICENSE.txt
- src/fosslight_binary/third_party/dependency-check/bin/completion-for-dependency-check.sh
- cli.spec
- src/fosslight_binary/init.py
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- .reuse/dep5
🚧 Files skipped from review as they are similar to previous changes (1)
- src/fosslight_binary/_binary.py
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/fosslight_binary/_jar_analysis.py (3)
353-360:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRetry max-out path drops the actual JAR SHA-1.
When retries are exhausted, line 357 calls
_process_one_jar(jar_path, rel_path, sha1='', ...), discarding the already-computed SHA-1. The resultingjar_items[rel_path]entry will have"sha1": "", which corrupts SHA-1 matching inmerge_binary_listand loses observability.🛠️ Proposed fix
Pass the real SHA-1 through but skip Central lookup via
search_timeout=0:- result, _ = _process_one_jar(jar_path, rel_path, sha1='', search_timeout=0) + # Skip Central lookup but preserve the real SHA-1 + result, _ = _process_one_jar(jar_path, rel_path, sha1=sha1, search_timeout=0)With
search_timeout=0,_search_central_by_sha1will time out immediately, but the SHA-1 will be preserved in the returned result dict (line 315).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_binary/_jar_analysis.py` around lines 353 - 360, The retry-max path is discarding the computed SHA-1 by calling _process_one_jar(jar_path, rel_path, sha1='', ...); change that call to pass the actual computed sha1 variable (the SHA-1 you computed earlier for this JAR) while still setting search_timeout=0 so _search_central_by_sha1 is skipped immediately; ensure jar_items[rel_path] is set from that result so the preserved "sha1" field is kept for merge_binary_list and observability.
12-12:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
defusedxml.ElementTreefor secure XML parsing.The module parses POM files from arbitrary JAR files using
ET.fromstring(line 42), which is vulnerable to XML entity expansion attacks. For a binary scanner processing untrusted artifacts, this is a security concern.🛡️ Proposed fix
-import xml.etree.ElementTree as ET +import defusedxml.ElementTree as ETAlso add
defusedxmltopyproject.tomldependencies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_binary/_jar_analysis.py` at line 12, Replace the insecure xml.etree.ElementTree usage with defusedxml by changing the import in src/fosslight_binary/_jar_analysis.py to import defusedxml.ElementTree as ET and keep using ET.fromstring where POMs are parsed (the call currently flagged); also add defusedxml to the project dependencies in pyproject.toml so the secure parser is installed. Ensure no other xml.etree.ElementTree imports remain in this module and run tests that exercise the POM parsing to verify behavior is unchanged.
383-393:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSHA-1 matching branch has multiple bugs.
The
elsebranch (lines 391-393) has three related defects:
- Empty SHA-1 false matches: When
sha1is""(from retry fallback or failed computation),bin.checksum == sha1can match unrelated binaries with empty checksums.found_in_jar_analysisnot set: Only the path-match branch sets this flag. Per_binary_dao.py, OSS items are cleared whennot item.found_in_jar_analysis, so SHA-1-matched OSS data will be wiped during DB processing.- Duplicate BinaryItem appended: When SHA-1 matches but path doesn't,
foundstaysFalse, causing a duplicateBinaryItemto be appended.🐛 Proposed fix
for bin in bin_list: if bin.source_name_or_path == key: found = True for oss in oss_list: if oss.name and oss.license: bin.found_in_jar_analysis = True break bin.set_oss_items(oss_list) - else: - if bin.checksum == sha1: - bin.set_oss_items(oss_list) + elif sha1 and bin.checksum == sha1: + found = True + for oss in oss_list: + if oss.name and oss.license: + bin.found_in_jar_analysis = True + break + bin.set_oss_items(oss_list)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_binary/_jar_analysis.py` around lines 383 - 393, The SHA-1 matching branch in the for-loop over bin_list incorrectly matches empty sha1 values, fails to mark matched binaries as found_in_jar_analysis, and leaves the outer found flag false which can cause duplicates; update the loop (the block referencing bin.source_name_or_path, key, sha1, bin.checksum, bin.set_oss_items and the found variable) so that you only attempt checksum matching when sha1 is non-empty, when checksum matches set bin.found_in_jar_analysis = True and set found = True (so the binary isn’t appended again), and keep path-match as the primary branch that also sets found and found_in_jar_analysis before calling bin.set_oss_items.
🧹 Nitpick comments (1)
src/fosslight_binary/_jar_analysis.py (1)
383-383: 💤 Low valueAvoid shadowing Python builtin
bin.Rename the loop variable to avoid shadowing the builtin function.
♻️ Suggested change
- for bin in bin_list: - if bin.source_name_or_path == key: + for bin_item in bin_list: + if bin_item.source_name_or_path == key:(Apply throughout the function)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_binary/_jar_analysis.py` at line 383, The loop variable "bin" in the iteration "for bin in bin_list:" shadows Python's builtin bin(); rename that variable (e.g., to "bin_entry" or "binary_item") and update every usage inside the same function (all references to bin, any assignments, method calls, appends, etc.) to the new name so the builtin is no longer shadowed; apply this rename consistently for every occurrence tied to "bin_list" within the function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/fosslight_binary/_jar_analysis.py`:
- Around line 353-360: The retry-max path is discarding the computed SHA-1 by
calling _process_one_jar(jar_path, rel_path, sha1='', ...); change that call to
pass the actual computed sha1 variable (the SHA-1 you computed earlier for this
JAR) while still setting search_timeout=0 so _search_central_by_sha1 is skipped
immediately; ensure jar_items[rel_path] is set from that result so the preserved
"sha1" field is kept for merge_binary_list and observability.
- Line 12: Replace the insecure xml.etree.ElementTree usage with defusedxml by
changing the import in src/fosslight_binary/_jar_analysis.py to import
defusedxml.ElementTree as ET and keep using ET.fromstring where POMs are parsed
(the call currently flagged); also add defusedxml to the project dependencies in
pyproject.toml so the secure parser is installed. Ensure no other
xml.etree.ElementTree imports remain in this module and run tests that exercise
the POM parsing to verify behavior is unchanged.
- Around line 383-393: The SHA-1 matching branch in the for-loop over bin_list
incorrectly matches empty sha1 values, fails to mark matched binaries as
found_in_jar_analysis, and leaves the outer found flag false which can cause
duplicates; update the loop (the block referencing bin.source_name_or_path, key,
sha1, bin.checksum, bin.set_oss_items and the found variable) so that you only
attempt checksum matching when sha1 is non-empty, when checksum matches set
bin.found_in_jar_analysis = True and set found = True (so the binary isn’t
appended again), and keep path-match as the primary branch that also sets found
and found_in_jar_analysis before calling bin.set_oss_items.
---
Nitpick comments:
In `@src/fosslight_binary/_jar_analysis.py`:
- Line 383: The loop variable "bin" in the iteration "for bin in bin_list:"
shadows Python's builtin bin(); rename that variable (e.g., to "bin_entry" or
"binary_item") and update every usage inside the same function (all references
to bin, any assignments, method calls, appends, etc.) to the new name so the
builtin is no longer shadowed; apply this rename consistently for every
occurrence tied to "bin_list" within the function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 573c26a1-e99f-48e9-81ca-ec96ef8246bb
⛔ Files ignored due to path filters (81)
src/fosslight_binary/third_party/dependency-check/lib/aho-corasick-double-array-trie-1.2.3.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/android-json-0.0.20131108.vaadin1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/annotations-26.0.2-1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/ant-1.10.15.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/bcpg-jdk18on-1.78.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/bcprov-jdk18on-1.78.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-cli-1.10.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-codec-1.19.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-collections-3.2.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-collections4-4.5.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-compress-1.27.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-dbcp2-2.13.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-digester-2.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-io-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-jcs3-core-3.2.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-lang3-3.19.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-logging-1.3.4.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-pool2-2.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-text-1.14.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/commons-validator-1.10.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/compiler-0.9.6.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/cpe-parser-3.0.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/dependency-check-cli-12.1.7.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/dependency-check-core-12.1.7.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/dependency-check-utils-12.1.7.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/error_prone_annotations-2.41.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/failureaccess-1.0.3.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/gson-2.9.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/guava-33.5.0-jre.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/h2-2.3.232.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/httpclient5-5.5.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/httpclient5-cache-5.4.3.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/httpcore5-5.3.6.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/httpcore5-h2-5.3.4.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/j2objc-annotations-3.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-annotations-2.20.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-core-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-databind-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-dataformat-yaml-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-datatype-jsr310-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jackson-module-blackbird-2.20.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jakarta.json-2.0.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jakarta.transaction-api-1.3.3.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/javax.activation-api-1.2.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/javax.inject-1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/javax.ws.rs-api-2.0.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jaxb-api-2.3.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jcs3-slf4j-1.0.5.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jdiagnostics-1.0.7.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jmustache-1.16.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/joda-time-2.14.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jsoup-1.21.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jspecify-1.0.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jsr305-3.0.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/jul-to-slf4j-1.7.36.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/listenablefuture-9999.0-empty-to-avoid-conflict-with-guava.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/logback-classic-1.2.13.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/logback-core-1.2.13.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-analysis-common-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-core-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-facet-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-queries-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-queryparser-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/lucene-sandbox-9.12.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/minlog-1.3.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/open-vulnerability-clients-7.3.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/ossindex-service-api-1.8.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/ossindex-service-client-1.8.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/package-url-java-1.2.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/packager-core-0.21.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/packager-rpm-0.21.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/packageurl-java-1.5.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/pecoff4j-0.0.2.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/retirejs-core-3.0.4.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/semver4j-5.8.0.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/slf4j-api-1.7.36.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/snakeyaml-2.4.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/spotbugs-annotations-4.9.6.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/toml4j-0.7.2.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/velocity-engine-core-2.4.1.jaris excluded by!**/*.jarsrc/fosslight_binary/third_party/dependency-check/lib/xz-1.9.jaris excluded by!**/*.jar
📒 Files selected for processing (16)
.gitignore.reuse/dep5cli.specpyproject.tomlsrc/fosslight_binary/__init__.pysrc/fosslight_binary/_binary.pysrc/fosslight_binary/_binary_dao.pysrc/fosslight_binary/_jar_analysis.pysrc/fosslight_binary/binary_analysis.pysrc/fosslight_binary/third_party/dependency-check/LICENSE.txtsrc/fosslight_binary/third_party/dependency-check/NOTICE.txtsrc/fosslight_binary/third_party/dependency-check/README.mdsrc/fosslight_binary/third_party/dependency-check/bin/completion-for-dependency-check.shsrc/fosslight_binary/third_party/dependency-check/bin/dependency-check.batsrc/fosslight_binary/third_party/dependency-check/bin/dependency-check.shsrc/fosslight_binary/third_party/dependency-check/licenses/commons-cli/LICENSE.txt
💤 Files with no reviewable changes (9)
- src/fosslight_binary/third_party/dependency-check/LICENSE.txt
- cli.spec
- src/fosslight_binary/third_party/dependency-check/README.md
- src/fosslight_binary/third_party/dependency-check/bin/dependency-check.sh
- src/fosslight_binary/third_party/dependency-check/NOTICE.txt
- src/fosslight_binary/third_party/dependency-check/licenses/commons-cli/LICENSE.txt
- src/fosslight_binary/init.py
- src/fosslight_binary/third_party/dependency-check/bin/dependency-check.bat
- src/fosslight_binary/third_party/dependency-check/bin/completion-for-dependency-check.sh
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- .reuse/dep5
🚧 Files skipped from review as they are similar to previous changes (2)
- pyproject.toml
- src/fosslight_binary/binary_analysis.py
Contributor
|
추후 merge시 squash 로 머지 권장드립니다. |
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/fosslight_binary/_jar_analysis.py (1)
386-404:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix the checksum-only merge branch before this ships.
When only the checksum branch on Line 396 matches,
foundstaysFalseandfound_in_jar_analysisis never set. That can both append a duplicateBinaryItemand let downstream code discard the OSS data again. It also still allows empty-SHA1 matches.🛠️ Minimal fix
for key, value in jar_items.items(): found = False oss_list = value["oss_list"] sha1 = value.get("sha1", "") for bin in bin_list: if bin.source_name_or_path == key: found = True for oss in oss_list: if oss.name and oss.license: bin.found_in_jar_analysis = True break bin.set_oss_items(oss_list) - else: - if bin.checksum == sha1: - bin.set_oss_items(oss_list) + elif sha1 and bin.checksum == sha1: + found = True + for oss in oss_list: + if oss.name and oss.license: + bin.found_in_jar_analysis = True + break + bin.set_oss_items(oss_list)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_binary/_jar_analysis.py` around lines 386 - 404, The checksum-only branch currently leaves found=False and doesn't mark the matched BinaryItem as found in jar analysis or prevent duplicates; modify the else branch that tests bin.checksum == sha1 to first skip empty sha1 values, and when a non-empty sha1 matches set bin.set_oss_items(oss_list), set bin.found_in_jar_analysis = True and set found = True (and optionally break out once matched) so the existing BinaryItem is updated and not re-added to not_found_bin; reference BinaryItem, bin_list, oss_list, sha1, bin.checksum, found_in_jar_analysis, set_oss_items, source_name_or_path.
🧹 Nitpick comments (1)
pyproject.toml (1)
30-30: ⚡ Quick winAdd a version constraint for the security-critical
defusedxmldependency.The
defusedxmllibrary is imported insrc/fosslight_binary/_jar_analysis.pyand actively used for secure XML parsing of Maven POM files. Since this is a security-focused library, add a minimum version constraint to ensure reproducible builds and guard against potential regressions.Suggested version constraint
- "defusedxml", + "defusedxml>=0.7.0",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` at line 30, Update the pyproject.toml dependency entry for defusedxml to include a minimum version constraint (e.g., change "defusedxml" to "defusedxml>=0.7.1") so builds are reproducible and you get the security fixes; ensure this matches the import usage in src/fosslight_binary/_jar_analysis.py which relies on defusedxml for secure XML parsing of Maven POMs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_binary/_jar_analysis.py`:
- Around line 193-205: In _process_one_jar ensure the skip_central flag is
honored for the entire fallback path: do not call _exists_in_central when
skip_central is True. Specifically, after using _search_central_by_sha1 (or when
skip_central short-circuits to central_info={}), guard any subsequent call to
_exists_in_central with if not skip_central and otherwise set
confirmed_in_central = False (or use central_info if available) so the
POM-inspection fallback does not trigger another Maven Central network call;
adjust logic around variables central_info, timed_out, confirmed_in_central and
the _exists_in_central(...) call in _process_one_jar accordingly.
---
Duplicate comments:
In `@src/fosslight_binary/_jar_analysis.py`:
- Around line 386-404: The checksum-only branch currently leaves found=False and
doesn't mark the matched BinaryItem as found in jar analysis or prevent
duplicates; modify the else branch that tests bin.checksum == sha1 to first skip
empty sha1 values, and when a non-empty sha1 matches set
bin.set_oss_items(oss_list), set bin.found_in_jar_analysis = True and set found
= True (and optionally break out once matched) so the existing BinaryItem is
updated and not re-added to not_found_bin; reference BinaryItem, bin_list,
oss_list, sha1, bin.checksum, found_in_jar_analysis, set_oss_items,
source_name_or_path.
---
Nitpick comments:
In `@pyproject.toml`:
- Line 30: Update the pyproject.toml dependency entry for defusedxml to include
a minimum version constraint (e.g., change "defusedxml" to "defusedxml>=0.7.1")
so builds are reproducible and you get the security fixes; ensure this matches
the import usage in src/fosslight_binary/_jar_analysis.py which relies on
defusedxml for secure XML parsing of Maven POMs.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca7158bb-256c-4579-9996-6ef2c71fa3a0
📒 Files selected for processing (2)
pyproject.tomlsrc/fosslight_binary/_jar_analysis.py
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Refactor
Chores