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

feat: add hierarchical option to merge command #338

Merged
merged 40 commits into from
Feb 5, 2025

Conversation

CBeck-96
Copy link
Collaborator

closes #152

@CBeck-96 CBeck-96 linked an issue Dec 15, 2024 that may be closed by this pull request
@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request settings_changes unittests labels Dec 15, 2024
@CBeck-96 CBeck-96 requested a review from italvi December 15, 2024 01:54
Copy link
Contributor

github-actions bot commented Dec 15, 2024

Coverage

Coverage Report •
FileStmtsMissCoverMissing
__main__.py3852394%240–241, 258, 270, 313, 324–328, 330, 825, 868, 913, 918, 922, 947, 961, 965, 969, 971, 980, 990
merge.py210498%375–376, 379–380
auxiliary
   sbomFunctions.py153696%72, 80, 157, 375, 392, 404
TOTAL206510395% 

Tests Skipped Failures Errors Time
384 2 💤 0 ❌ 0 🔥 14.169s ⏱️

@italvi italvi changed the title Feat: adds hierarchical option to merge command Feat: add hierarchical option to merge command Dec 15, 2024
@italvi italvi changed the title Feat: add hierarchical option to merge command feat: add hierarchical option to merge command Dec 15, 2024
Copy link
Collaborator

@italvi italvi left a comment

Choose a reason for hiding this comment

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

The coverage of 94% got me confused, but it seems we still have 95%, so that's why the pipeline did not break.
image

Further I noticed two bugs:
During testing the hierarchical flag, I observed the following bug:
If I have an SBOM with components within a component of components, i.e.

{
    "$schema": "http://cyclonedx.org/schema/bom-1.4.schema.json",
    "bomFormat": "CycloneDX",
    "specVersion": "1.4",
    "version": 1,
    "metadata": {
        "tools": [
            {
                "vendor": "@cyclonedx",
                "name": "cyclonedx-library",
                "version": "1.13.3",
                "externalReferences": [
                    {
                        "url": "https://github.com/CycloneDX/cyclonedx-javascript-library/issues",
                        "type": "issue-tracker",
                        "comment": "as detected from PackageJson property \"bugs.url\""
                    },
                    {
                        "url": "git+https://github.com/CycloneDX/cyclonedx-javascript-library.git",
                        "type": "vcs",
                        "comment": "as detected from PackageJson property \"repository.url\""
                    },
                    {
                        "url": "https://github.com/CycloneDX/cyclonedx-javascript-library#readme",
                        "type": "website",
                        "comment": "as detected from PackageJson property \"homepage\""
                    }
                ]
            },
            {
                "vendor": "@cyclonedx",
                "name": "cyclonedx-npm",
                "version": "1.9.2",
                "externalReferences": [
                    {
                        "url": "https://github.com/CycloneDX/cyclonedx-node-npm/issues",
                        "type": "issue-tracker",
                        "comment": "as detected from PackageJson property \"bugs.url\""
                    },
                    {
                        "url": "git+https://github.com/CycloneDX/cyclonedx-node-npm.git",
                        "type": "vcs",
                        "comment": "as detected from PackageJson property \"repository.url\""
                    },
                    {
                        "url": "https://github.com/CycloneDX/cyclonedx-node-npm#readme",
                        "type": "website",
                        "comment": "as detected from PackageJson property \"homepage\""
                    }
                ]
            }
        ],
        "component": {
            "type": "application",
            "name": "juice-shop",
            "version": "14.1.1",
            "bom-ref": "[email protected]",
            "author": "Björn Kimminich",
            "description": "Probably the most modern and sophisticated insecure web application",
            "licenses": [
                {
                    "license": {
                        "id": "MIT"
                    }
                }
            ],
            "purl": "pkg:npm/[email protected]?vcs_url=git%2Bhttps%3A//github.com/juice-shop/juice-shop.git",
            "externalReferences": [
                {
                    "url": "https://github.com/juice-shop/juice-shop/issues",
                    "type": "issue-tracker",
                    "comment": "as detected from PackageJson property \"bugs.url\""
                },
                {
                    "url": "git+https://github.com/juice-shop/juice-shop.git",
                    "type": "vcs",
                    "comment": "as detected from PackageJson property \"repository.url\""
                },
                {
                    "url": "https://owasp-juice.shop",
                    "type": "website",
                    "comment": "as detected from PackageJson property \"homepage\""
                }
            ],
            "properties": [
                {
                    "name": "cdx:npm:package:path",
                    "value": ""
                },
                {
                    "name": "cdx:npm:package:private",
                    "value": "true"
                }
            ]
        }
    },
    "components": [
        {
            "type": "library",
            "name": "node-pre-gyp",
            "group": "@mapbox",
            "version": "1.0.10",
            "bom-ref": "@mapbox/[email protected]",
            "author": "Dane Springmeyer",
            "description": "Node.js native addon binary install tool",
            "hashes": [
                {
                    "alg": "SHA-512",
                    "content": "e324a8e028f34adba9accc24df91f9a4f6e4ca68efd521778c62e3eab007a7fc53fd117b4cbedb77d0939b5c43638f4adaa17b8e647f83b93e4a42c44cfea8a4"
                }
            ],
            "licenses": [
                {
                    "license": {
                        "id": "BSD-3-Clause"
                    }
                }
            ],
            "purl": "pkg:npm/%40mapbox/[email protected]",
            "externalReferences": [
                {
                    "url": "https://registry.npmjs.org/@mapbox/node-pre-gyp/-/node-pre-gyp-1.0.10.tgz",
                    "type": "distribution",
                    "comment": "as detected from npm-ls property \"resolved\""
                },
                {
                    "url": "https://github.com/mapbox/node-pre-gyp/issues",
                    "type": "issue-tracker",
                    "comment": "as detected from PackageJson property \"bugs.url\""
                },
                {
                    "url": "git://github.com/mapbox/node-pre-gyp.git",
                    "type": "vcs",
                    "comment": "as detected from PackageJson property \"repository.url\""
                },
                {
                    "url": "https://github.com/mapbox/node-pre-gyp#readme",
                    "type": "website",
                    "comment": "as detected from PackageJson property \"homepage\""
                }
            ],
            "properties": [
                {
                    "name": "cdx:npm:package:path",
                    "value": "node_modules/@mapbox/node-pre-gyp"
                }
            ],
            "components": [
                {
                    "type": "library",
                    "name": "ansi-regex",
                    "version": "5.0.1",
                    "bom-ref": "@mapbox/[email protected]|[email protected]",
                    "author": "Sindre Sorhus",
                    "description": "Regular expression for matching ANSI escape codes",
                    "hashes": [
                        {
                            "alg": "SHA-512",
                            "content": "aae2505e54d25062f62c7f52517a3c570b18e2ca1a9e1828e8b3529bce04d4b05c13cb373b4c29762473c91f73fd9649325316bf7eea38e6fda5d26531410a15"
                        }
                    ],
                    "licenses": [
                        {
                            "license": {
                                "id": "MIT"
                            }
                        }
                    ],
                    "purl": "pkg:npm/[email protected]",
                    "externalReferences": [
                        {
                            "url": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz",
                            "type": "distribution",
                            "comment": "as detected from npm-ls property \"resolved\""
                        },
                        {
                            "url": "https://github.com/chalk/ansi-regex/issues",
                            "type": "issue-tracker",
                            "comment": "as detected from PackageJson property \"bugs.url\""
                        },
                        {
                            "url": "git+https://github.com/chalk/ansi-regex.git",
                            "type": "vcs",
                            "comment": "as detected from PackageJson property \"repository.url\""
                        },
                        {
                            "url": "https://github.com/chalk/ansi-regex#readme",
                            "type": "website",
                            "comment": "as detected from PackageJson property \"homepage\""
                        }
                    ],
                    "properties": [
                        {
                            "name": "cdx:npm:package:path",
                            "value": "node_modules/@mapbox/node-pre-gyp/node_modules/ansi-regex"
                        }
                    ]
                }
            ]
        }
    ]
}

Then a simple merge leads to:

image

meaning the square brackets from the array are also copied.

Second bug: If I have a components within a components of a components so a subsystem of a subsystem, only the first subsystem is copied. Everything else is just dropped.

cdxev/__main__.py Outdated Show resolved Hide resolved
cdxev/__main__.py Outdated Show resolved Hide resolved
tests/auxiliary/helper.py Outdated Show resolved Hide resolved
docs/source/usage/merge.rst Outdated Show resolved Hide resolved
@CBeck-96 CBeck-96 force-pushed the 152-merge-is-not-hierarchical branch from 7ac5388 to 64796a2 Compare January 7, 2025 05:14
@CBeck-96 CBeck-96 requested a review from italvi January 7, 2025 05:26
Copy link
Collaborator

@italvi italvi left a comment

Choose a reason for hiding this comment

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

The hierarchical flag seems to work now, however, if I have a components within a components of a component, i.e.

{
    "components": [
        {
            "type": "library",
            "name": "node-pre-gyp",
            "group": "@mapbox",
            "version": "1.0.10",
            "bom-ref": "@mapbox/[email protected]",
            "author": "Dane Springmeyer",
            "description": "Node.js native addon binary install tool",
            "hashes": [
                {
                    "alg": "SHA-512",
                    "content": "e324a8e028f34adba9accc24df91f9a4f6e4ca68efd521778c62e3eab007a7fc53fd117b4cbedb77d0939b5c43638f4adaa17b8e647f83b93e4a42c44cfea8a4"
                }
            ],
            "licenses": [
                {
                    "license": {
                        "id": "BSD-3-Clause"
                    }
                }
            ],
            "purl": "pkg:npm/%40mapbox/[email protected]",
            "externalReferences": [
                {
                    "url": "https://registry.npmjs.org/@mapbox/node-pre-gyp/-/node-pre-gyp-1.0.10.tgz",
                    "type": "distribution",
                    "comment": "as detected from npm-ls property \"resolved\""
                },
                {
                    "url": "https://github.com/mapbox/node-pre-gyp/issues",
                    "type": "issue-tracker",
                    "comment": "as detected from PackageJson property \"bugs.url\""
                },
                {
                    "url": "git://github.com/mapbox/node-pre-gyp.git",
                    "type": "vcs",
                    "comment": "as detected from PackageJson property \"repository.url\""
                },
                {
                    "url": "https://github.com/mapbox/node-pre-gyp#readme",
                    "type": "website",
                    "comment": "as detected from PackageJson property \"homepage\""
                }
            ],
            "properties": [
                {
                    "name": "cdx:npm:package:path",
                    "value": "node_modules/@mapbox/node-pre-gyp"
                }
            ],
            "components": [
                {
                    "type": "library",
                    "name": "ansi-regex",
                    "version": "5.0.1",
                    "bom-ref": "@mapbox/[email protected]|[email protected]",
                    "author": "Sindre Sorhus",
                    "description": "Regular expression for matching ANSI escape codes",
                    "hashes": [
                        {
                            "alg": "SHA-512",
                            "content": "aae2505e54d25062f62c7f52517a3c570b18e2ca1a9e1828e8b3529bce04d4b05c13cb373b4c29762473c91f73fd9649325316bf7eea38e6fda5d26531410a15"
                        }
                    ],
                    "licenses": [
                        {
                            "license": {
                                "id": "MIT"
                            }
                        }
                    ],
                    "purl": "pkg:npm/[email protected]",
                    "externalReferences": [
                        {
                            "url": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz",
                            "type": "distribution",
                            "comment": "as detected from npm-ls property \"resolved\""
                        },
                        {
                            "url": "https://github.com/chalk/ansi-regex/issues",
                            "type": "issue-tracker",
                            "comment": "as detected from PackageJson property \"bugs.url\""
                        },
                        {
                            "url": "git+https://github.com/chalk/ansi-regex.git",
                            "type": "vcs",
                            "comment": "as detected from PackageJson property \"repository.url\""
                        },
                        {
                            "url": "https://github.com/chalk/ansi-regex#readme",
                            "type": "website",
                            "comment": "as detected from PackageJson property \"homepage\""
                        }
                    ],
                    "properties": [
                        {
                            "name": "cdx:npm:package:path",
                            "value": "node_modules/@mapbox/node-pre-gyp/node_modules/ansi-regex"
                        }
                    ]
                }
            ]
          }
     ]
 }

Then it does not work as you describe it in the documentation: The component "ansi-regex" is still just copied within "node-pre-gyp" and not separately on the highest level.

cdxev/__main__.py Outdated Show resolved Hide resolved
docs/source/usage/merge.rst Outdated Show resolved Hide resolved
cdxev/merge.py Outdated Show resolved Hide resolved
cdxev/merge.py Outdated Show resolved Hide resolved
cdxev/merge.py Show resolved Hide resolved
cdxev/merge.py Outdated Show resolved Hide resolved
cdxev/merge.py Outdated Show resolved Hide resolved
cdxev/merge.py Outdated Show resolved Hide resolved
tests/test_merge.py Outdated Show resolved Hide resolved
tests/integration/test_integration.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@italvi italvi left a comment

Choose a reason for hiding this comment

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

Small changes for the comments, should be "fixed" fast, then I can approve it.

cdxev/merge.py Outdated Show resolved Hide resolved
cdxev/merge.py Outdated Show resolved Hide resolved
docs/source/usage/merge.rst Outdated Show resolved Hide resolved
tests/test_merge.py Outdated Show resolved Hide resolved
cdxev/merge.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@italvi italvi left a comment

Choose a reason for hiding this comment

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

As all requested changes are implemented now: LGTM.

@italvi italvi merged commit 9548ade into main Feb 5, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request settings_changes unittests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge is not hierarchical
2 participants