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

Resolve symlinks when fetching file contents #782

Merged
merged 9 commits into from
Feb 24, 2022

Conversation

wagoodman
Copy link
Contributor

@wagoodman wagoodman commented Jan 30, 2022

It was noticed that file-classifications was empty for /bin/busybox when cataloging the busybox image with the classifications cataloger enabled. Since we have a busybox-binary classifier, this was unexpected. Primarily this is due to the fact that /bin/busybox is a symlink to /bin/[ and the classifier filters files considered by filename (in this case **/busybox, which /bin/[ would never match on). The existing behavior of source.FileResolver when using FetchContents() is to only return contents for unresolved links.

This PR changes this behavior to resolve symlinks first (for all resolver implementations) before attempting to fetch contents. As a result the file-digests cataloger has been adjusted to only report digests for regular files (don't follow links), which is closest to today's behavior.

Open question: should we have the digests cataloger report digests for resolved paths? or only regular files?
Answer: No --it would be confusing to see a symlink have content, and this would be redundant (the real file will be additionally be in the list and the link destination is already reported)

Additional changes:

  • bumped stereoscope to pull in mimetype fix for empty io.Readers
  • the cwd and root directories used when creating the directory resolver are evaluated for possible access via links, such that normalizing response paths is consistent with the real path and not the link path

@wagoodman wagoodman added the bug Something isn't working label Jan 30, 2022
@wagoodman wagoodman requested a review from a team January 30, 2022 16:49
@wagoodman wagoodman self-assigned this Jan 30, 2022
@github-actions
Copy link

github-actions bot commented Jan 30, 2022

Benchmark Test Results

Benchmark results from the latest changes vs base branch
name                                                       old time/op    new time/op    delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2              1.30ms ± 1%    1.82ms ± 1%  +40.31%  (p=0.016 n=4+5)
ImagePackageCatalogers/python-package-cataloger-2            2.97ms ± 0%    4.20ms ±17%  +41.34%  (p=0.008 n=5+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2    1.03ms ± 1%    1.40ms ± 2%  +36.19%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         802µs ± 2%    1091µs ± 1%  +36.04%  (p=0.008 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                     933µs ± 1%    1275µs ± 2%  +36.71%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                      846µs ± 2%    1141µs ± 5%  +34.93%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                      23.4ms ± 4%    29.8ms ± 6%  +27.45%  (p=0.008 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                     1.28ms ± 2%    1.74ms ± 2%  +35.86%  (p=0.008 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2          1.98µs ± 1%    2.57µs ±10%  +29.94%  (p=0.008 n=5+5)

name                                                       old alloc/op   new alloc/op   delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2               252kB ± 0%     253kB ± 0%   +0.32%  (p=0.008 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2            1.06MB ± 0%    1.07MB ± 0%   +0.30%  (p=0.008 n=5+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2     253kB ± 0%     253kB ± 0%     ~     (p=0.151 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         208kB ± 0%     208kB ± 0%   +0.21%  (p=0.008 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                     254kB ± 0%     255kB ± 0%   +0.34%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                      236kB ± 0%     236kB ± 0%   +0.24%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                      4.19MB ± 0%    4.20MB ± 0%     ~     (p=0.222 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                     1.30MB ± 0%    1.30MB ± 0%   +0.23%  (p=0.008 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2            608B ± 0%      608B ± 0%     ~     (all equal)

name                                                       old allocs/op  new allocs/op  delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2               6.33k ± 0%     6.33k ± 0%     ~     (p=0.444 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2             21.4k ± 0%     21.4k ± 0%     ~     (p=0.603 n=5+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2     7.25k ± 0%     7.25k ± 0%     ~     (p=0.095 n=5+4)
ImagePackageCatalogers/javascript-package-cataloger-2         5.36k ± 0%     5.36k ± 0%     ~     (all equal)
ImagePackageCatalogers/dpkgdb-cataloger-2                     7.10k ± 0%     7.10k ± 0%     ~     (all equal)
ImagePackageCatalogers/rpmdb-cataloger-2                      6.82k ± 0%     6.82k ± 0%     ~     (all equal)
ImagePackageCatalogers/java-cataloger-2                       86.8k ± 0%     86.8k ± 0%     ~     (p=0.587 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                      7.37k ± 0%     7.37k ± 0%     ~     (p=1.000 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2            14.0 ± 0%      14.0 ± 0%     ~     (all equal)

@wagoodman wagoodman changed the title Resolve symlinks when fetching file contents [WIP] Resolve symlinks when fetching file contents Jan 31, 2022
@wagoodman wagoodman force-pushed the fix-resolver-fetch-contents-for-links branch from 74c48cf to e66674d Compare February 15, 2022 23:28
@wagoodman wagoodman changed the title [WIP] Resolve symlinks when fetching file contents Resolve symlinks when fetching file contents Feb 15, 2022
@wagoodman wagoodman force-pushed the fix-resolver-fetch-contents-for-links branch from fd8b93d to 895e805 Compare February 17, 2022 14:18
@wagoodman wagoodman force-pushed the fix-resolver-fetch-contents-for-links branch from 0c6d686 to 57cdf45 Compare February 18, 2022 19:12
@wagoodman wagoodman marked this pull request as ready for review February 21, 2022 16:36
@wagoodman wagoodman merged commit 99bb93d into main Feb 24, 2022
@wagoodman wagoodman deleted the fix-resolver-fetch-contents-for-links branch February 24, 2022 15:02
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants