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

Respect user configuration for Syft #832

Merged
merged 5 commits into from
Mar 7, 2024

Conversation

chmeliik
Copy link
Contributor

@chmeliik chmeliik commented Feb 21, 2024

STONEBLD-2095

Syft makes many things configurable:
https://github.com/anchore/syft#configuration

For example, users can take advantage of this to get rid of false
positives. This will be useful for the Syft build itself:
redhat-appstudio/rh-syft#21

Currently, our SBOM generation does not respect the user configuration.
Syft reads the config from the current working directory, not from the
target directory (anchore/syft#2465).

Set the working directory to the root of the user's repository to ensure
we respect the configuration.

@chmeliik chmeliik force-pushed the respect-syft-configuration branch 2 times, most recently from bb37a8e to 538a5ec Compare February 21, 2024 10:44
@chmeliik
Copy link
Contributor Author

Testing in redhat-appstudio/rh-syft#22

@chmeliik
Copy link
Contributor Author

Seems to work as intended

$ cosign download sbom registry.redhat.io/rh-syft-tech-preview/syft-rhel9:0.105.0 > sbom-old.json
$ jq < sbom-old.json '.components[].purl | try match("pkg:[^/]*").string catch "<no purl>"' |
  sort | uniq -c | sort -n
      1 "pkg:ebuild"
      1 "pkg:nix"
      2 "pkg:composer"
      2 "pkg:github"
      2 "pkg:otp"
      5 "pkg:swift"
      7 "pkg:pub"
     10 "pkg:cargo"
     12 "<no purl>"
     14 "pkg:conan"
     14 "pkg:nuget"
     21 "pkg:rpm"
     22 "pkg:hex"
     23 "pkg:cocoapods"
     27 "pkg:hackage"
     30 "pkg:pypi"
     43 "pkg:maven"
     47 "pkg:npm"
     54 "pkg:generic"
     55 "pkg:gem"
   1318 "pkg:golang"

$ cosign download sbom quay.io/redhat-user-workloads/rhtap-build-tenant/rh-syft/rh-syft:on-pr-c9185065be17836f3d60b22bf2ea45ef7220413f > sbom-new.json
$ jq < sbom-new.json '.components[].purl | try match("pkg:[^/]*").string catch "<no purl>"' |
  sort | uniq -c | sort -n
      1 "<no purl>"
     20 "pkg:rpm"
   1308 "pkg:golang"

@lcarva
Copy link
Contributor

lcarva commented Feb 21, 2024

@chmeliik, are there any concerns that a user config may generate a less accurate SBOM (intentionally or not)?

@chmeliik
Copy link
Contributor Author

@chmeliik, are there any concerns that a user config may generate a less accurate SBOM (intentionally or not)?

That's a good point. My thinking was that by checking in custom configuration, users would be taking the responsibility for potentially hurting the accuracy of their own SBOMs.

But maybe it shouldn't be allowed anyway, as it allows them to bypass SBOM-based EC checks by hiding problematic packages 🤔

@chmeliik
Copy link
Contributor Author

There are also cases where not respecting the configuration will lead to a less accurate SBOM... (false positives are an inaccuracy too). This may need broader discussion

@chmeliik
Copy link
Contributor Author

Added a topic to the Feb 27 architecture call

@chmeliik
Copy link
Contributor Author

Per discussion on the arch call, respecting the users' configuration sounds worth it.

  • it allows the user to get rid of false positives that Syft finds
    • this helps SBOM accuracy for both hermetic and non-hermetic builds
  • it allows the user to cause false negatives by excluding packages that Syft finds
    • this hurts SBOM accuracy for non-hermetic builds
    • for hermetic builds, the cachi2-generated SBOM should still contain everything that got into the build from outside

Even for hermetic builds, there are ways to hide dependencies if the user really wants to. They can vendor all their dependencies and make sure Syft won't find them. But I think we don't need to build accurate SBOMs for adversarial users, we can assume some level of cooperation.

@chmeliik chmeliik force-pushed the respect-syft-configuration branch from 538a5ec to 87e2e06 Compare February 28, 2024 10:09
@chmeliik chmeliik marked this pull request as ready for review February 28, 2024 10:10
@brunoapimentel
Copy link
Contributor

Upon first review, LGTM. I think we should sync up with the docs team so that this gets properly documented in Konflux (otherwise, users might not even know this is possible).

I will run a few tests here so I can give you my final ack.

@chmeliik
Copy link
Contributor Author

chmeliik commented Mar 1, 2024

Rebased on main (I hope - Github isn't showing the force-push)

@chmeliik chmeliik force-pushed the respect-syft-configuration branch from 87e2e06 to 59d1ac9 Compare March 1, 2024 08:12
@chmeliik
Copy link
Contributor Author

chmeliik commented Mar 1, 2024

/retest

@chmeliik chmeliik force-pushed the respect-syft-configuration branch from 59d1ac9 to 92f0349 Compare March 1, 2024 11:32
@chmeliik
Copy link
Contributor Author

chmeliik commented Mar 1, 2024

just a rebase

@chmeliik
Copy link
Contributor Author

chmeliik commented Mar 1, 2024

/retest

@chmeliik chmeliik force-pushed the respect-syft-configuration branch from 92f0349 to 87278ea Compare March 4, 2024 14:16
@chmeliik
Copy link
Contributor Author

chmeliik commented Mar 4, 2024

/retest

@chmeliik chmeliik force-pushed the respect-syft-configuration branch 2 times, most recently from acea2c4 to 2e885da Compare March 5, 2024 11:44
@chmeliik
Copy link
Contributor Author

chmeliik commented Mar 5, 2024

I believe tests are failing for the same reason as #827 (comment)

The attestation was created 24 minutes after the image was pushed to quay.io

https://quay.io/repository/redhat-user-workloads/build-templates-e2e/test-app-832/devfile-sample-python-basic-qcxh?tab=tags&tag=latest

image

@chmeliik
Copy link
Contributor Author

chmeliik commented Mar 5, 2024

/retest

@chmeliik chmeliik mentioned this pull request Mar 5, 2024
1 task
@chmeliik chmeliik force-pushed the respect-syft-configuration branch from 2e885da to e6a42d0 Compare March 6, 2024 08:13
chmeliik added 5 commits March 7, 2024 10:33
STONEBLD-2095

Syft makes many things configurable:
https://github.com/anchore/syft#configuration

For example, users can take advantage of this to get rid of false
positives. This will be useful for the Syft build itself:
redhat-appstudio/rh-syft#21

Currently, our SBOM generation does not respect the user configuration.
Syft reads the config from the current working directory, not from the
target directory (anchore/syft#2465).

Set the working directory to the root of the user's repository to ensure
we respect the configuration.

---

This also allows the user to - intentionally or otherwise - exclude
packages that should be reported, causing false negatives. That seems
like an acceptable tradeoff, given that:

* For hermetic builds, the SBOM should still report everything that got
  in from outside, regardless of Syft configuration.
* We should assume some level of co-operation from the user, we don't
  have to design accurate SBOMs for users that actively sabotage the
  proces

Signed-off-by: Adam Cmiel <[email protected]>
See the corresponding commit for the buildah task

Note that buildah-rhtap does not currently support hermetic builds,
which makes the tradeoffs (allowing the user to cause false negatives)
more severe. Support for hermetic builds (or some other more accurate
SBOM solution) should eventually be added though, so this concern is
temporary.

Signed-off-by: Adam Cmiel <[email protected]>
See the corresponding commit for the buildah task

Signed-off-by: Adam Cmiel <[email protected]>
See the corresponding commit for the buildah task

Note that rpm-ostree does not currently support hermetic builds, which
makes the tradeoffs (allowing the user to cause false negatives) more
severe. Support for hermetic builds should eventually be added though,
so this concern is temporary.

Signed-off-by: Adam Cmiel <[email protected]>
@chmeliik chmeliik force-pushed the respect-syft-configuration branch from e6a42d0 to c35e315 Compare March 7, 2024 09:33
Copy link

sonarqubecloud bot commented Mar 7, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@chmeliik
Copy link
Contributor Author

chmeliik commented Mar 7, 2024

/retest

@chmeliik chmeliik changed the title tasks/buildah: respect user configuration for Syft Respect user configuration for Syft Mar 7, 2024
@chmeliik
Copy link
Contributor Author

chmeliik commented Mar 7, 2024

/retest

@chmeliik chmeliik merged commit 31067e9 into konflux-ci:main Mar 7, 2024
6 checks passed
@chmeliik chmeliik deleted the respect-syft-configuration branch March 7, 2024 12:41
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.

3 participants