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

Enhance Alpine package scan results #307

Open
aalexanderr opened this issue Jun 8, 2021 · 19 comments
Open

Enhance Alpine package scan results #307

aalexanderr opened this issue Jun 8, 2021 · 19 comments

Comments

@aalexanderr
Copy link

Alpine packages lack some important info like copyrights or where the source code is located. This info can't be gathered from the packages themselves as its just not there. To get this info we need to:
download aports repo & for each pkg check it out on commit specific to alpine package (via fetchcode)
parse APKBUILD aboutcode-org/scancode-toolkit#2541
download package sources (fetchcode) & amend new info to package's scan results

Discussed a bit with @pombredanne
Most likely @quepop will PR it

The question is- should it be standard behavior when alpine based docker is being scanned or should it be a separate pipeline?

@quepop
Copy link

quepop commented Jun 8, 2021

@quepop
Copy link

quepop commented Jun 10, 2021

@pombredanne should I PR it?

@pombredanne
Copy link
Contributor

@quepop please do! that's much easier for review
Of note:

@quepop
Copy link

quepop commented Jun 10, 2021

@pombredanne I don't think there is any other way. In order to extract source variable value (or any other value) we need to run the script (APKBUILD file), because in some of the alpine packages, source variable is built in a for loop (not to mention that sometimes bash string formatting is used).

From pkg:alpine/[email protected] APKBUILD file:

_arch_keys="
        aarch64:[email protected]
        armhf:[email protected]
 
        x86:[email protected]
        x86,x86_64:[email protected]
        x86_64:[email protected]
 
        ppc64le:[email protected]
 
        s390x:[email protected]
 
        mips64:[email protected]
        "
 
for _i in $_arch_keys; do
        source="$source ${_i#*:}"
done

@pombredanne
Copy link
Contributor

@quepop good point, but do we need these (e.g. these _arch_keys) ?

@quepop
Copy link

quepop commented Jun 10, 2021

@pombredanne We need the source variable to be complete (to be able to download sources and scan them for copyrights). I tried to emphasize (by giving an example) that the only way we get a complete source variable every time is by running the script.

@quepop
Copy link

quepop commented Jun 24, 2021

@pombredanne Regarding aboutcode-org/scancode-toolkit#2541 (comment), how should I name these new files (pipe, pipeline)? My idea is /pipes/alpine_helper.py and /pipelines/alpine_complement.py

@tdruez
Copy link
Contributor

tdruez commented Jun 25, 2021

@quepop There's already a pipes/alpine.py pipes module in which you can add the new functions.
For the pipeline, what about "alpine_packages.py", unless it's too specific?

@quepop
Copy link

quepop commented Jul 1, 2021

@tdruez In the case of multi-image projects we cannot match DiscoveredPackage from the database to the image (and alpine version) they are from - db entry for DiscoveredPackage only include project_id (no image_id or image_index field). Alpine version is only present in project.extra_data.images array. It's a problem because in the aports repository every alpine version is a certain branch and we need to pull them and make checkouts (branch and then commit-id) to be able to extract correct APKBUILDs.

My solution is to add a new database field to the DiscoveredPackage class named image_index which would store an index (project.extra_data.images array) of the image it is from from. Is it acceptable?

@tdruez
Copy link
Contributor

tdruez commented Jul 1, 2021

@quepop what about adding the extra_data field on the DiscoveredPackage model, for consistency with Project and CodebaseResource models?
Let me know if that would work for you and I'll make that change upstream.

@pombredanne
Copy link
Contributor

Repasting from quepop/scancode.io@2f59440#r52902214 for reference :
@quepop

I have been thinking more about this for nexB#191 and sourcing arbitrary bash script this way is too much of a security concern.

An alternative could include:

  • using a container, jail or some sorts of chroot to try to minimize the risk a little
  • implement a simple bash/shell script parser and perform parameter expansion

I think 2. is best and much less involved than having a container depdendency.
Furthermore, there could be other variables of interest in an APKBUILD and we need eventually to parse other shell-based manifests to extract metadata such as PKGBUILD (Arch), ebuild (Gentoo) m4 (Autotools) and a few more.

Therefore I am implementing this that can then be reused here:

@pombredanne
Copy link
Contributor

@quepop what about adding the extra_data field on the DiscoveredPackage model, for consistency with Project and CodebaseResource models?
Let me know if that would work for you and I'll make that change upstream.

👍 ... this is being added too to SCTK FWIW

@quepop
Copy link

quepop commented Jul 1, 2021

@pombredanne sounds great but what about APKBUILDs like these: #307? Will it work?
@tdruez Yes, it would be great.

tdruez referenced this issue in aboutcode-org/scancode.io Jul 1, 2021
tdruez referenced this issue in aboutcode-org/scancode.io Jul 1, 2021
@tdruez
Copy link
Contributor

tdruez commented Jul 1, 2021

@quepop extra_data added on the DiscoveredPackage model aboutcode-org/scancode.io#222
Make sure to use the DiscoveredPackage.update_extra_data() API, see https:/nexB/scancode.io/blob/main/scanpipe/models.py#L197

@quepop
Copy link

quepop commented Jul 5, 2021

@tdruez @pombredanne

There is another issue that i found looking at my database. Some alpine images like docker://alpine:3.13 have subpackages installed from a parent package that is not installed (libcrypto, libssl from openssl). Handling cases like these isn't clear for me (because running the scan on the parent still leaves a problem of deciding what subset of the scan's output is applicable to the parent's children)
I've decided that in cases where a parent package is present, my code will update only its missing information (not its chilrdren).

@pombredanne
Copy link
Contributor

@quepop not sure what to do wrt. subpackages... can you provide a concrete example with links and may sample APKBUILD snippets?

@pombredanne
Copy link
Contributor

re:

@quepop not sure what to do wrt. subpackages... can you provide a concrete example with links and may sample APKBUILD snippets?

Actually the new approach in https:/nexB/scancode-toolkit/blob/b7d070fc788f4c233df1f31d735e1ac5b3aa2d29/src/packagedcode/alpine.py#L144 could be enhanced to also collect these.

  1. collect the variable and resolve that lists all the subpackages such as https://gitlab.alpinelinux.org/alpine/aports/-/blob/9238f96475ab45281ce3d0cdfbf37efcbdeb8888/main/openssl/APKBUILD#L14
  2. for each of these, parse the corresponding bash function with the same name and extract the variables defined inside a back function

Then merge subpackages variables with the base variables.

This way declared licensed, description and other sub-package-specific metadata could be collected.

Note that this would work whereas the approach to just eval the APKBUILD with bash would not be able to collect variables defined inside a function, as this would require to run the function, which is impractical since the function names are not known (unless parsed ...) .

@pombredanne
Copy link
Contributor

See aboutcode-org/scancode.io#272 (comment)

The feature will be implemented instead in PurlDB ...
See #95 and #237

@pombredanne
Copy link
Contributor

Moving this to PurlDB

@pombredanne pombredanne transferred this issue from aboutcode-org/scancode.io Feb 26, 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 a pull request may close this issue.

4 participants