Skip to content

Update rules with required phrases automatically#3924

Merged
pombredanne merged 91 commits into
developfrom
update-rules-with-required-phrases-auto
Apr 10, 2025
Merged

Update rules with required phrases automatically#3924
pombredanne merged 91 commits into
developfrom
update-rules-with-required-phrases-auto

Conversation

@AyanSinhaMahapatra

@AyanSinhaMahapatra AyanSinhaMahapatra commented Sep 17, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

This is a continuation of #3254 with added required phrases in license rules after review and further manual curations. Also contains improvements in required phrase collection and marking.

Reference: #2637 #2878

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁

@AyanSinhaMahapatra
Add script for adding required phrases automatically
aa9374c 
Add a script which can add required phrases in already existing rules
automatically from required phrases already present in other rules and
license field names. This can be done one license expression at a time.

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases automatically for mit rules
72f290f 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases at bsd-new rules automatically
bd14127 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases automatically for gpl-2.0
20a7860 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases for gpl-2.0-only rules automatically
0a44594 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Skip creating required phrase rules for generic licenses
435255b 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra changed the title Update rules with required phrases auto Update rules with required phrases automatically Sep 17, 2024
@AyanSinhaMahapatra
Add required phrases for genreic license rules automtaically
12eaccf 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases to gpl-1.0-plus rules automatically
ece8c33 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases automatically on gpl/lgpl rules
be94a84 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add more required phrases automatically in permissive rules
584b554 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases automatically to gpl-3.0 rules
c778ce0 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases for lgpl rules automatically
f5e9593 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases for agpl rules automatically
63c23f0 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases to bsd rules automatically
40ced0c 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases in lgpl rules automatically
518116d 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
AyanSinhaMahapatra force-pushed the update-rules-with-required-phrases-auto branch from ea221d4 to 518116d Compare September 18, 2024 13:43
@AyanSinhaMahapatra
Add required phrases automatically for cc rules
3583ac6 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra mentioned this pull request Sep 18, 2024
Closed
4 tasks
@pombredanne

pombredanne commented Oct 4, 2024

Copy link
Copy Markdown
Member

I am pushing shortly a few updates:

  • decouple the creation of new rules from updating existing rules in a separate CLI
  • ensure we skip more rules in the whole process: any rule that cannot be matched approximately and not only tiny rules, and also false positives
  • ensure that no rule get a required phrase addition that would break in the middle of a URL, email, or copyright. This will be done to check that no required phrase injection changes the set of ignorables of a rule and makes the URL not longer a proper URL for instance.
  • extend "skipping" the collection of required phrases flag to skip a rule from both required phrases collection AND injection. This allow to handle exceptions more easily.

@pombredanne
Improve license detection rules
1994095 
Do not damage rules with URLs

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Improve required phrases in paths
4d1b8ac 
Ensure that the leading /usr is not broken with {{ required phrase }}
markers.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Improve required phrases in paths
ef5101e 
Ensure that /usr paths are not broken with {{ required phrase }}
markers.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Improve required phrases in URLs
8e615bb 
Ensure that URLs are not broken with {{ required phrase }} markers.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Correct rule syntax
6831e90 
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Move back required-phrase related code
e05ca6f 
This is code that belongs to required_phrase.py, not to tokenize.py

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Use correct syntax for rules
94cffc9 
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Apache Software Foundation is not required phrase
62643cb 
This creates many false positives.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Remove duplicate imports
7150b82 
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Improve license models
83eadec 
This helps with required phrases handling, addition and generation

* Add new Rule.source attribute to track the "source" of a license rule
  like when adding a new required phrase to a rule
* Add new Rule.is_tiny computed attribute to ytrack tiny, very small
  rules
* Add new Rule.is_approx_matchable property for rules that can only be
  matched exactly
* Add new Rule.is_generic for rules that contain "generic" licenses
* Support required_phrases-related fields in Rule.validate()
* Update index.py accordingly

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Relax required phrase filtering
6eff903 
Only process stopwords this for "is_continuous" rules

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Correct license test expectation
e3fc9c9 
Some rules now have a "is_required_phrase" flag

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Correct license rule expression
94a4d35 
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Update expected license test result
4718fed 
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@AyanSinhaMahapatra AyanSinhaMahapatra mentioned this pull request Nov 5, 2024
Open
@jloehel jloehel mentioned this pull request Nov 28, 2024
Open
@AyanSinhaMahapatra AyanSinhaMahapatra mentioned this pull request Jan 28, 2025
Open
60 tasks
@petergardfjall

petergardfjall commented Feb 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hey! This PR looks promising as I think it has the potential to greatly reduce false positives.
Just to drop an example, having required phrases throughout the rule set would avoid "silly" mistakes like this:

"rule_url": "https://github.com/nexB/scancode-toolkit/tree/develop/src/licensedcode/data/rules/lgpl-2.1_250.RULE",
"matched_text": "// Licensed under the MIT license. \nnamespace Microsoft.OpenApi.Readers.V2"

And I do see quite a lot of these "ghost" detections.

I was just checking in to ask if there is any hope of seeing this PR merged. It has been open for some time.

@pombredanne

pombredanne commented Feb 19, 2025

Copy link
Copy Markdown
Member

@petergardfjall re:

I was j ust checking in to ask if there is any hope of seeing this PR merged. It has been open for some time.

Yes! I am about to have a chat with @AyanSinhaMahapatra on just that.

@pombredanne

pombredanne commented Feb 19, 2025

Copy link
Copy Markdown
Member

The longer story is that there are reconciliations to do wrt. the changes I made and changes that @AyanSinhaMahapatra has not yet pushed.

@AyanSinhaMahapatra
Add required phrases for mpl rules automatically
ac28196 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases automatically for more rules
3438f99 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Merge branch 'develop' into update-rules-with-required-phrases-auto
de19fbe 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
AyanSinhaMahapatra force-pushed the update-rules-with-required-phrases-auto branch from eba882d to de19fbe Compare March 31, 2025 10:31
This was referenced Apr 4, 2025
Merged
Merged
@pombredanne
Improve ISC rules and tests
c821616 
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Update test expectations
c40e404 
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Improve detection of BSD license
4211d02 
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Filter key phrases for continuity correctly
ca1c70f 
The test for a key phrase uninterrupted continuity must be done in all
cases, whether a matched rule is "continuous", or a full "required
phrase".

Fix test and add new test.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
github-advanced-security[bot]
github-advanced-security AI found potential problems Apr 7, 2025
View reviewed changes
Comment thread src/licensedcode/cache.py
"""
Dump this license cache on disk at ``cache_file``.
"""
with open(cache_file, 'wb') as fn:

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](1).

Copilot Autofix

AI over 1 year ago

To fix the problem, we need to ensure that the path constructed from the SCANCODE_LICENSE_INDEX_CACHE environment variable is validated before being used. We can achieve this by normalizing the path and ensuring it is contained within a safe root directory. This will prevent path traversal attacks and ensure that the application only accesses intended directories.

  1. Normalize the licensedcode_cache_dir path using os.path.normpath.
  2. Ensure that the normalized path starts with a predefined safe root directory.
  3. Raise an exception if the path validation fails.
Suggested changeset 1
src/scancode_config.py
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/scancode_config.py b/src/scancode_config.py
--- a/src/scancode_config.py
+++ b/src/scancode_config.py
@@ -183,3 +183,10 @@
 __env_license_cache_dir = os.getenv('SCANCODE_LICENSE_INDEX_CACHE')
-licensedcode_cache_dir = (__env_license_cache_dir or std_license_cache_dir)
+if __env_license_cache_dir:
+    normalized_cache_dir = os.path.normpath(__env_license_cache_dir)
+    safe_root_dir = os.path.normpath(scancode_src_dir)
+    if not normalized_cache_dir.startswith(safe_root_dir):
+        raise ValueError("Invalid cache directory path")
+    licensedcode_cache_dir = normalized_cache_dir
+else:
+    licensedcode_cache_dir = std_license_cache_dir
 
EOF
@@ -183,3 +183,10 @@
__env_license_cache_dir = os.getenv('SCANCODE_LICENSE_INDEX_CACHE')
licensedcode_cache_dir = (__env_license_cache_dir or std_license_cache_dir)
if __env_license_cache_dir:
normalized_cache_dir = os.path.normpath(__env_license_cache_dir)
safe_root_dir = os.path.normpath(scancode_src_dir)
if not normalized_cache_dir.startswith(safe_root_dir):
raise ValueError("Invalid cache directory path")
licensedcode_cache_dir = normalized_cache_dir
else:
licensedcode_cache_dir = std_license_cache_dir

Copilot is powered by AI and may make mistakes. Always verify output.
@pombredanne
Avoid heisen failures in CI
f31d0ef 
Some tests running with a single process and threading are randombly
failiing in the CI that run varisou wrapper node-based envt. that then
spawn lower level tools like our test runners.
This sometimes makes the Python process and threading run in corner
cases that fail. This commits moves these issues out of the way to
avoid failing when this fails.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Avoid failures in failure reporting
f837a38 
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
pombredanne force-pushed the update-rules-with-required-phrases-auto branch from 681bbee to f837a38 Compare April 9, 2025 21:34
@pombredanne

pombredanne commented Apr 10, 2025

Copy link
Copy Markdown
Member

@AyanSinhaMahapatra all green... I am merging now 🙇

@pombredanne
pombredanne merged commit e054254 into develop Apr 10, 2025
@pombredanne
pombredanne deleted the update-rules-with-required-phrases-auto branch April 10, 2025 07:45
@codewithfourtix

This comment was marked as off-topic.

Sign in to view
@Kaushik-Kumar-CEG Kaushik-Kumar-CEG mentioned this pull request May 28, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@Ebaoone Ebaoone Ebaoone approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@AyanSinhaMahapatra @pombredanne @petergardfjall @codewithfourtix @github-advanced-security @Ebaoone