-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
e2e: Avoid hard-coding ImageCacheDir #17243
e2e: Avoid hard-coding ImageCacheDir #17243
Conversation
test/e2e/common_test.go
Outdated
@@ -103,6 +103,7 @@ func TestLibpod(t *testing.T) { | |||
|
|||
var _ = SynchronizedBeforeSuite(func() []byte { | |||
// make cache dir | |||
ImageCacheDir = filepath.Join(os.TempDir(), "imagecachedir") | |||
if err := os.MkdirAll(ImageCacheDir, 0777); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The permissions on this directory seems to be too loose. I don't think we should allow different users on the system to share the ImageCacheDir, since it could be an attack route to attack each other.
This should be per user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change ImageCacheDir permissions from 0777
to 0700
.
As far as I can see, this change is no ploblem.
881d41c
to
97a684f
Compare
Needs a rebase. |
97a684f
to
6cbf036
Compare
6cbf036
to
89ebe82
Compare
- ImageCacheDir is hard-coded as "/tmp/podman/imagecachedir". To avoid this hard-coding, I changed it to "os.TempDir()/imagecachedir". - Change ImageCacheDir permissions from 0777 to 0700. This directory should be used by per-user. Signed-off-by: Toshiki Sonoda <[email protected]>
89ebe82
to
2682d3a
Compare
/approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry wrong button, LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, rhatdan, sstosh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
|
||
// ImageCacheDir is initialized at runtime. | ||
// e.g., filepath.Join(os.TempDir(), "imagecachedir") | ||
// This directory should be used by per-user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is technically not true, but oh well, too late
ImageCacheDir is hard-coded as "/tmp/podman/imagecachedir".
To avoid this hard-coding, I changed it to "os.TempDir()/imagecachedir".
Change ImageCacheDir permissions from 0777 to 0700.
This directory should be used by per-user.
Signed-off-by: Toshiki Sonoda [email protected]
Related to #17089
Does this PR introduce a user-facing change?