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

SPDX FileName should not be absolute #2093

Open
vargenau opened this issue Sep 4, 2023 · 13 comments
Open

SPDX FileName should not be absolute #2093

vargenau opened this issue Sep 4, 2023 · 13 comments
Labels
bug Something isn't working

Comments

@vargenau
Copy link
Contributor

vargenau commented Sep 4, 2023

What happened:

Syft generates the following SPDX (tag:value):

FileName: /usr/share/ca-certificates/mozilla/Actalis_Authentication_Root_CA.crt
SPDXID: SPDXRef-File-...Actalis-Authentication-Root-CA.crt-b2e28e6876228bbb
FileType: TEXT
FileChecksum: SHA1: 511ca95607022a99ed8e68bd63f136c4854cefcb
LicenseConcluded: NOASSERTION
FileComment: layerID: sha256:3f946b95045046b182ad195bfdb24fe56dd6ea12d34e35a0995218d22c05102a

tools-python complains that it is invalid SPDX.

file name must not be an absolute path starting with "/", but is: /usr/share/ca-certificates/mozilla/Actalis_Authentication_Root_CA.crt

The SPDX spec says: "A relative filename".

What you expected to happen:

Have a relative path in FileName.

Steps to reproduce the issue:

syft docker:bitnami/mongodb -o spdx-tag-value > bitnami-mongodb.spdx

Anything else we need to know?:

Environment:

  • Output of syft version: syft 0.89.0
  • OS (e.g: cat /etc/os-release or similar): macos 13.5.1
@vargenau vargenau added the bug Something isn't working label Sep 4, 2023
@m-dhana
Copy link

m-dhana commented Sep 5, 2023

I am also failing similar error, when I run the sbom json file through Ntia-checker.

As per the spdx specs, https://spdx.github.io/spdx-spec/v2.3/file-information/#81-file-name-field, the filename field - "In general, every filename is preceded with a ./"

I am using the syft 0.87.1.

@kzantow
Copy link
Contributor

kzantow commented Sep 5, 2023

Just a note: regardless if there are changes to be made in Syft, it looks like both of these tools are incorrectly validating this field. The spec says:

Identify the full path and filename that corresponds to the file information in this section. The metadata for the file name field is shown in Table 36.

Table 36 — Metadata for the file name field

Required	Yes
Cardinality	1..1
Format		A relative filename with the root of the package archive or directory.
  		In general, every filename is preceded with a ./, see http://www.ietf.org/rfc/rfc3986.txt for syntax.

Nowhere in this text does it say that the filename is required to be a relative path starting with ./.

@vargenau
Copy link
Contributor Author

vargenau commented Sep 5, 2023

Yes, I think the tools should complain if the path starts with "/" and not require that the path starts with "./".

See spdx/Spdx-Java-Library#195

I have asked the question in the SPDX tech mailing list.

@kzantow
Copy link
Contributor

kzantow commented Sep 5, 2023

I don't really think using absolute paths should be considered incorrect. Take an image scan, for example: the files are all absolute paths within the image filesystem. This makes a lot of sense to me, rather than transitioning these to relative paths starting with ./.

@vargenau
Copy link
Contributor Author

vargenau commented Sep 6, 2023

From the SPDX tech mailing list:

I'm curious what the motivation is for paths being relative. If I scan an image, for example, I would expect to see absolute paths to the files within the image filesystem, rather than those being translated to relative paths.

Cheers,
-Keith

There's a couple of reasons why this is helpful:

  1. Since the path is relative to the root of the package a relative path in an image is equivalent to an absolute path.
  2. We don't want paths to refer to things outside of the package (particularly for package managers that don't have as strict of a boundary as images do), e.g. no "/home/william/my-secret-stuff".
  3. If you have the same set of files embedded in multiple packages you can reuse the file elements, this is useful for example if you want to represent a directory of files within an archive and then to represent that same directory of files when it's extracted to a different location.

-- William Bartholomew

@kzantow kzantow changed the title FileName should not be absolute SPDX FileName should not be absolute Sep 6, 2023
@kzantow
Copy link
Contributor

kzantow commented Sep 6, 2023

Thanks @vargenau -- I followed the discussion on the mailing list, it sounds like we're going to need to update these paths to be relative indeed.

@kzantow kzantow moved this to Backlog in OSS Sep 6, 2023
@tpodowd
Copy link

tpodowd commented Sep 21, 2023

Thanks. I'm also having the same issue validating with tools-python.

@vargenau
Copy link
Contributor Author

Hello @kzantow ,

Any progress on fixing this issue?

@stevehipwell
Copy link

👀

@sej7278
Copy link

sej7278 commented Aug 16, 2024

Is there any update on this (just re-tried syft 1.11.0)?

It definitely seems we should have relative ./ paths not / paths, at least with syft scan dir:.

"fileName": "/foo.rpm", makes no sense when that's not where the file is (in the root dir) and not where the files inside install to either.

https://lists.spdx.org/g/Spdx-tech/message/4972

https://spdx.github.io/spdx-spec/v2.3/file-information/#81-file-name-field

The current setup fails both https://tools.spdx.org/app/ntia_checker/ and https://tools.spdx.org/app/validate/ due to not using relative paths and also because . has no version/supplier - which I guess is a side effect (https://github.com/eBay/sbom-scorecard also drops 1% due to the dot directory).

@vargenau
Copy link
Contributor Author

@sej7278
Unfortunately, this bug does not seem to get much interest.
It is a pity, as the SPDX code is invalid.

@sej7278
Copy link

sej7278 commented Aug 20, 2024

@vargenau i guess we could use sed to replace / with ./ on the sbom output, its far from ideal though

@vargenau
Copy link
Contributor Author

@vargenau i guess we could use sed to replace / with ./ on the sbom output, its far from ideal though

Yes, it is what I am doing, but it is ugly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Ready
Development

No branches or pull requests

6 participants