Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Almalinux advisories #1491

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

ambuj-1211
Copy link
Collaborator

Fix #1201
@ziadhany @TG1999 @keshav-space This is the basic nit to adds Alma Linux advisories into vulnerablecode database, please review it to make further changes.

Added almalinux advisories and tests for it
Signed-off-by: ambuj <kulshreshthaak.12@gmail.com>
@ambuj-1211
Copy link
Collaborator Author

@ziadhany as this importer uses osv.py importer so shall I add AlmaLinux:8 and AlmaLinux:9 in supported ecosystems in osv.py

@ziadhany
Copy link
Collaborator

ziadhany commented Jun 22, 2024

@ambuj-1211 yes, you should add it to this PURL_TYPE_BY_OSV_ECOSYSTEM dict.

Signed-off-by: ambuj <kulshreshthaak.12@gmail.com>
@ziadhany
Copy link
Collaborator

@ambuj-1211
I looked into your code. Instead of using uppercase, you should use lowercase. then you are going to pass the test.

PURL_TYPE_BY_OSV_ECOSYSTEM = {
     ....
    "almalinux:8": "almalinux:8",
    "almalinux:9": "almalinux:9",
}

but you will face another issue. you need to add support for almalinux in univers.

https://github.com/nexB/univers/blob/205d7c48835dfeb6b694c9196728d2b4fa0a011a/src/univers/version_range.py#L1254:L1258

Signed-off-by: ambuj <kulshreshthaak.12@gmail.com>
Signed-off-by: ambuj <kulshreshthaak.12@gmail.com>
Signed-off-by: ambuj <kulshreshthaak.12@gmail.com>
@TG1999 TG1999 requested a review from ziadhany July 9, 2024 15:22
Signed-off-by: ambuj <kulshreshthaak.12@gmail.com>
return dedupe(fixed_versions)


def get_affected_version_range(affected_pkg, raw_id, supported_ecosystem):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ziadhany unable to figure out how to find affected version range

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ambuj-1211, we don't handle getting the affected version range from introduced and fixed in OSV. We only obtain this using versions.

I recommend handling this in a separate issue. Please create one for it.

Additionally, please review the following link:
https://github.com/nexB/vulnerablecode/blob/main/vulnerabilities/tests/test_osv.py#L270

If we don't have a affected versions set this as null.

Copy link
Collaborator Author

@ambuj-1211 ambuj-1211 Jul 13, 2024

Choose a reason for hiding this comment

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

@ziadhany I have done some changes, let me know if it require something else and also tell me if I need to squash the commits to make the history cleaner. And do I need to select update with rebase for this branch?

Signed-off-by: ambuj <kulshreshthaak.12@gmail.com>
Signed-off-by: ambuj <kulshreshthaak.12@gmail.com>
@TG1999
Copy link
Contributor

TG1999 commented Jul 22, 2024

@ziadhany can this be merged ?

@ziadhany
Copy link
Collaborator

ziadhany commented Aug 5, 2024

@ziadhany can this be merged ?

I still need to review this code

…tion

- Added a detailed docstring to the `parse_advisory_data` function in the `almalinux-importer` module.
- The docstring includes a clear description of the function's purpose, arguments, return value, and an example usage.
- Improved the readability and structure of the example output in the docstring to ensure clarity and consistency.

This documentation enhancement makes the `parse_advisory_data` function easier to understand and use, aiding future development and maintenance.

Signed-off-by: ambuj <kulshreshthaak.12@gmail.com>
Copy link
Collaborator

@ziadhany ziadhany left a comment

Choose a reason for hiding this comment

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

@ambuj-1211

Why not use the existing OSV script directly or modify it to support Almalinux, instead of rewriting the entire code?

import json
import logging
from pathlib import Path
from typing import Any
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
from typing import Any

Comment on lines +19 to +21
from univers.version_range import RANGE_CLASS_BY_SCHEMES
from univers.version_range import RpmVersionRange
from univers.versions import InvalidVersion
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ambuj-1211 please remove the unused imports.

Suggested change
from univers.version_range import RANGE_CLASS_BY_SCHEMES
from univers.version_range import RpmVersionRange
from univers.versions import InvalidVersion

from vulnerabilities.utils import get_cwe_id

logger = logging.getLogger(__name__)
BASE_URL = "https://github.com/AlmaLinux/osv-database"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please keep the URL within the importer class.

try:
self.clone(repo_url=self.BASE_URL)
base_path = Path(self.vcs_response.dest_dir)
advisory_dirs = base_path / "tree/master/advisories"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ambuj-1211 This path isn't functioning; the importer isn't actually importing any data.

Suggested change
advisory_dirs = base_path / "tree/master/advisories"
advisory_dirs = base_path / "advisories/"

Please ensure you run the importer and verify the data is correctly imported.

spdx_license_expression = "MIT License"
license_url = "https://github.com/AlmaLinux/osv-database/blob/master/LICENSE"
importer_name = "Alma Linux Importer"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
repo_url = "git+https://github.com/AlmaLinux/osv-database"



class AlmaImporter(Importer):
spdx_license_expression = "MIT License"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please refer to https://spdx.org/licenses/ before writing the SPDX license expression.

Suggested change
spdx_license_expression = "MIT License"
spdx_license_expression = "MIT"

@ziadhany
Copy link
Collaborator

ziadhany commented Sep 9, 2024

@ambuj-1211 Update the OSV get_affected_purl function to add support for AlmaLinux, just like we did for Maven.

def get_affected_purl(affected_pkg, raw_id):

@TG1999 TG1999 added this to the v36.0.0 - 3-next milestone Oct 15, 2024
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.

Collect advisories for AlmaLinux
3 participants