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.

Sign up for GitHub

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

Jump to bottom

storage: set 0o744 for files with exec mode set #1094

Merged
merged 1 commit into from
May 12, 2023
Merged

storage: set 0o744 for files with exec mode set #1094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions internal/controller/storage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,12 @@ import (
const GarbageCountLimit = 1000

const (
// defaultFileMode is the permission mode applied to all files inside an artifact archive.
// defaultFileMode is the permission mode applied to files inside an artifact archive.
defaultFileMode int64 = 0o644
// defaultDirMode is the permission mode applied to all directories inside an artifact archive.
defaultDirMode int64 = 0o755
// defaultExeFileMode is the permission mode applied to executable files inside an artifact archive.
defaultExeFileMode int64 = 0o744
)

// Storage manages artifacts
Expand Down Expand Up @@ -424,6 +426,7 @@ func (s Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFi
if err != nil {
return err
}

// The name needs to be modified to maintain directory structure
// as tar.FileInfoHeader only has access to the base name of the file.
// Ref: https://golang.org/src/archive/tar/common.go?#L626
Expand All @@ -434,21 +437,7 @@ func (s Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFi
return err
}
}
header.Name = relFilePath

// We want to remove any environment specific data as well, this
// ensures the checksum is purely content based.
header.Gid = 0
header.Uid = 0
header.Uname = ""
header.Gname = ""
header.ModTime = time.Time{}
header.AccessTime = time.Time{}
header.ChangeTime = time.Time{}
header.Mode = defaultFileMode
if fi.Mode().IsDir() {
header.Mode = defaultDirMode
}
sanitizeHeader(relFilePath, header)

if err := tw.WriteHeader(header); err != nil {
return err
Expand Down Expand Up @@ -689,3 +678,42 @@ func (wc *writeCounter) Write(p []byte) (int, error) {
wc.written += int64(n)
return n, nil
}

// sanitizeHeader modifies the tar.Header to be relative to the root of the
// archive and removes any environment specific data.
func sanitizeHeader(relP string, h *tar.Header) {
// Modify the name to be relative to the root of the archive,
// this ensures we maintain the same structure when extracting.
h.Name = relP

// We want to remove any environment specific data as well, this
// ensures the checksum is purely content based.
h.Gid = 0
h.Uid = 0
h.Uname = ""
h.Gname = ""
h.ModTime = time.Time{}
h.AccessTime = time.Time{}
h.ChangeTime = time.Time{}

// Override the mode to be the default for the type of file.
setDefaultMode(h)
}

// setDefaultMode sets the default mode for the given header.
func setDefaultMode(h *tar.Header) {
if h.FileInfo().IsDir() {
h.Mode = defaultDirMode
return
}

if h.FileInfo().Mode().IsRegular() {
mode := h.FileInfo().Mode()
if mode&os.ModeType == 0 && mode&0o111 != 0 {
h.Mode = defaultExeFileMode
return
}
h.Mode = defaultFileMode
return
}
}
103 changes: 69 additions & 34 deletions internal/controller/storage_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,14 @@ func TestStorage_Archive(t *testing.T) {
t.Fatalf("error while bootstrapping storage: %v", err)
}

createFiles := func(files map[string][]byte) (dir string, err error) {
type dummyFile struct {
content []byte
mode int64
}

createFiles := func(files map[string]dummyFile) (dir string, err error) {
dir = t.TempDir()
for name, b := range files {
for name, df := range files {
absPath := filepath.Join(dir, name)
if err = os.MkdirAll(filepath.Dir(absPath), 0o750); err != nil {
return
Expand All @@ -120,18 +125,24 @@ func TestStorage_Archive(t *testing.T) {
if err != nil {
return "", fmt.Errorf("could not create file %q: %w", absPath, err)
}
if n, err := f.Write(b); err != nil {
if n, err := f.Write(df.content); err != nil {
f.Close()
return "", fmt.Errorf("could not write %d bytes to file %q: %w", n, f.Name(), err)
}
f.Close()

if df.mode != 0 {
if err = os.Chmod(absPath, os.FileMode(df.mode)); err != nil {
return "", fmt.Errorf("could not chmod file %q: %w", absPath, err)
}
}
}
return
}

matchFiles := func(t *testing.T, storage *Storage, artifact sourcev1.Artifact, files map[string][]byte, dirs []string) {
matchFiles := func(t *testing.T, storage *Storage, artifact sourcev1.Artifact, files map[string]dummyFile, dirs []string) {
t.Helper()
for name, b := range files {
for name, df := range files {
mustExist := !(name[0:1] == "!")
if !mustExist {
name = name[1:]
Expand All @@ -140,7 +151,7 @@ func TestStorage_Archive(t *testing.T) {
if err != nil {
t.Fatalf("failed reading tarball: %v", err)
}
if bs := int64(len(b)); s != bs {
if bs := int64(len(df.content)); s != bs {
t.Fatalf("%q size %v != %v", name, s, bs)
}
if exist != mustExist {
Expand All @@ -150,8 +161,12 @@ func TestStorage_Archive(t *testing.T) {
t.Errorf("tarball contained excluded file %q", name)
}
}
if exist && m != defaultFileMode {
t.Fatalf("%q mode %v != %v", name, m, defaultFileMode)
expectMode := df.mode
if expectMode == 0 {
expectMode = defaultFileMode
}
if exist && m != expectMode {
t.Fatalf("%q mode %v != %v", name, m, expectMode)
}
}
for _, name := range dirs {
Expand Down Expand Up @@ -179,62 +194,62 @@ func TestStorage_Archive(t *testing.T) {

tests := []struct {
name string
files map[string][]byte
files map[string]dummyFile
filter ArchiveFileFilter
want map[string][]byte
want map[string]dummyFile
wantDirs []string
wantErr bool
}{
{
name: "no filter",
files: map[string][]byte{
".git/config": nil,
"file.jpg": []byte(`contents`),
"manifest.yaml": nil,
files: map[string]dummyFile{
".git/config": {},
"file.jpg": {content: []byte(`contents`)},
"manifest.yaml": {},
},
filter: nil,
want: map[string][]byte{
".git/config": nil,
"file.jpg": []byte(`contents`),
"manifest.yaml": nil,
want: map[string]dummyFile{
".git/config": {},
"file.jpg": {content: []byte(`contents`)},
"manifest.yaml": {},
},
},
{
name: "exclude VCS",
files: map[string][]byte{
".git/config": nil,
"manifest.yaml": nil,
files: map[string]dummyFile{
".git/config": {},
"manifest.yaml": {},
},
wantDirs: []string{
"!.git",
},
filter: SourceIgnoreFilter(nil, nil),
want: map[string][]byte{
"!.git/config": nil,
"manifest.yaml": nil,
want: map[string]dummyFile{
"!.git/config": {},
"manifest.yaml": {},
},
},
{
name: "custom",
files: map[string][]byte{
".git/config": nil,
"custom": nil,
"horse.jpg": nil,
files: map[string]dummyFile{
".git/config": {},
"custom": {},
"horse.jpg": {},
},
filter: SourceIgnoreFilter([]gitignore.Pattern{
gitignore.ParsePattern("custom", nil),
}, nil),
want: map[string][]byte{
"!git/config": nil,
"!custom": nil,
"horse.jpg": nil,
want: map[string]dummyFile{
"!git/config": {},
"!custom": {},
"horse.jpg": {},
},
wantErr: false,
},
{
name: "including directories",
files: map[string][]byte{
"test/.gitkeep": nil,
files: map[string]dummyFile{
"test/.gitkeep": {},
},
filter: SourceIgnoreFilter([]gitignore.Pattern{
gitignore.ParsePattern("custom", nil),
Expand All @@ -244,6 +259,26 @@ func TestStorage_Archive(t *testing.T) {
},
wantErr: false,
},
{
name: "sets default file modes",
files: map[string]dummyFile{
"test/file": {
mode: 0o666,
},
"test/executable": {
mode: 0o777,
},
},
want: map[string]dummyFile{
"test/file": {
mode: defaultFileMode,
},
"test/executable": {
mode: defaultExeFileMode,
},
},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down