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

fix: update tar traversal to respect current director entry #225

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

spiffcs
Copy link
Contributor

@spiffcs spiffcs commented Feb 29, 2024

Summary

Users of syft reported upstream that they are now seeing an error for OCI scanning when stereoscope hits a tar entry with . as the current directory.

This PR adds a case for making sure the destination path is not the current directory entry.

It also adds a specific skip for this type given that we don't want to stat . and perform any additional actions.

The current check will fail with a . entry given the following:

`/tmp`
`/tmp/`

/tmp does not begin with the prefix /tmp/ so stereoscope thinks we're escaping and errors

This also prevents the error from surfacing in this case:
anchore/syft#2678

Copy link

github-actions bot commented Feb 29, 2024

Benchmark Test Results

Benchmark results from the latest changes vs base branch
latest: Pulling from library/ubuntu
tar: Option --mtime: Treating date 'UTC 2019-09-16' as 2019-09-16 00:00:00
goos: linux
goarch: amd64
pkg: github.com/anchore/stereoscope/pkg/file
cpu: AMD EPYC 7763 64-Core Processor                
ctr: 
           │ ./.tmp/benchmark-089bb6e.txt │
           │            sec/op            │
TarIndex-2                   36.21µ ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

           │ ./.tmp/benchmark-089bb6e.txt │
           │             B/op             │
TarIndex-2                  5.560Ki ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

           │ ./.tmp/benchmark-089bb6e.txt │
           │          allocs/op           │
TarIndex-2                    93.00 ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

pkg: github.com/anchore/stereoscope/test/integration
                                      │ ./.tmp/benchmark-089bb6e.txt │
                                      │            sec/op            │
SimpleImage_GetImage/docker-archive-2                   1.246m ± ∞ ¹
SimpleImage_GetImage/podman-2                           17.50m ± ∞ ¹
geomean                                                 4.669m
¹ need >= 6 samples for confidence interval at level 0.95

                                      │ ./.tmp/benchmark-089bb6e.txt │
                                      │             B/op             │
SimpleImage_GetImage/docker-archive-2                  326.0Ki ± ∞ ¹
SimpleImage_GetImage/podman-2                          441.6Ki ± ∞ ¹
geomean                                                379.4Ki
¹ need >= 6 samples for confidence interval at level 0.95

                                      │ ./.tmp/benchmark-089bb6e.txt │
                                      │          allocs/op           │
SimpleImage_GetImage/docker-archive-2                   2.609k ± ∞ ¹
SimpleImage_GetImage/podman-2                           2.829k ± ∞ ¹
geomean                                                 2.717k
¹ need >= 6 samples for confidence interval at level 0.95

ctr: failed to dial "/run/containerd/containerd.sock": connection error: desc = "transport: error while dialing: dial unix /run/containerd/containerd.sock: connect: permission denied"
                                                   │ ./.tmp/benchmark-089bb6e.txt │
                                                   │            sec/op            │
SimpleImage_FetchSquashedContents/docker-archive-2                   21.91µ ± ∞ ¹
SimpleImage_FetchSquashedContents/podman-2                           21.33µ ± ∞ ¹
geomean                                                              21.62µ
¹ need >= 6 samples for confidence interval at level 0.95

                                                   │ ./.tmp/benchmark-089bb6e.txt │
                                                   │             B/op             │
SimpleImage_FetchSquashedContents/docker-archive-2                  2.648Ki ± ∞ ¹
SimpleImage_FetchSquashedContents/podman-2                          2.648Ki ± ∞ ¹
geomean                                                             2.648Ki
¹ need >= 6 samples for confidence interval at level 0.95

                                                   │ ./.tmp/benchmark-089bb6e.txt │
                                                   │          allocs/op           │
SimpleImage_FetchSquashedContents/docker-archive-2                    21.00 ± ∞ ¹
SimpleImage_FetchSquashedContents/podman-2                            21.00 ± ∞ ¹
geomean                                                               21.00
¹ need >= 6 samples for confidence interval at level 0.95

Signed-off-by: Christopher Phillips <[email protected]>
Signed-off-by: Christopher Phillips <[email protected]>
@spiffcs spiffcs merged commit fe426d1 into main Feb 29, 2024
7 checks passed
@spiffcs spiffcs deleted the respect-current-directory-entry branch February 29, 2024 17:55
@wagoodman wagoodman added the bug Something isn't working label Apr 23, 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.

3 participants