Skip to content

Commit 179cd7a

Browse files
author
François Chastanet
committed
more secure defer using SafeCloseDeferCallback
1 parent 7fedd2d commit 179cd7a

9 files changed

Lines changed: 42 additions & 25 deletions

File tree

.github/workflows/main.yml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ jobs:
6464
6565
- uses: akatov/commit-status-updater@a9e988ec5454692ff7745a509452422a35172ad6
6666
with:
67-
name: docker-build-${{ env.image_tag }}
67+
name: build-docker-${{ env.image_tag }}
6868
status: pending
6969

7070
- name: Docker meta
@@ -101,7 +101,12 @@ jobs:
101101
- uses: akatov/commit-status-updater@a9e988ec5454692ff7745a509452422a35172ad6
102102
if: ${{ always() }}
103103
with:
104-
name: docker-build-${{ env.image_tag }}
104+
name: build-docker-${{ env.image_tag }}
105+
status: ${{ job.status }}
106+
107+
- uses: akatov/commit-status-updater@a9e988ec5454692ff7745a509452422a35172ad6
108+
with:
109+
name: build-docker
105110
status: ${{ job.status }}
106111

107112
# -------------------------------------------------------

internal/compiler/annotationEmbedGenerate.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
"github.com/fchastanet/bash-compiler/internal/render"
1010
"github.com/fchastanet/bash-compiler/internal/utils/encoding"
11+
"github.com/fchastanet/bash-compiler/internal/utils/errors"
1112
"github.com/fchastanet/bash-compiler/internal/utils/files"
1213
"github.com/fchastanet/bash-compiler/internal/utils/logger"
1314
"github.com/fchastanet/bash-compiler/internal/utils/tar"
@@ -70,8 +71,7 @@ func (annotationEmbedGenerate *annotationEmbedGenerate) renderFile(
7071
if logger.FancyHandleError(err) {
7172
return "", err
7273
}
73-
// skipcq: GO-S2307 // no need Sync as readOnly open
74-
defer file.Close()
74+
defer errors.SafeCloseDeferCallback(file, &err)
7575

7676
md5sum, err := encoding.ChecksumFromFile(file)
7777
if logger.FancyHandleError(err) {
@@ -111,7 +111,7 @@ func (annotationEmbedGenerate *annotationEmbedGenerate) renderDir(
111111
if logger.FancyHandleError(err) {
112112
return "", err
113113
}
114-
defer directoryArchive.Close()
114+
defer errors.SafeCloseDeferCallback(directoryArchive, &err)
115115
err = directoryArchive.Sync()
116116
if err != nil {
117117
return "", err

internal/utils/dotenv/LoadEnvFile.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99

1010
"github.com/a8m/envsubst"
11+
"github.com/fchastanet/bash-compiler/internal/utils/errors"
1112
"github.com/fchastanet/bash-compiler/internal/utils/logger"
1213
)
1314

@@ -21,8 +22,7 @@ func LoadEnvFile(confFile string) error {
2122
if err != nil {
2223
return err
2324
}
24-
// skipcq: GO-S2307 // no need Sync as readOnly open
25-
defer confFileContent.Close()
25+
defer errors.SafeCloseDeferCallback(confFileContent, &err)
2626

2727
variables := make(map[string]string)
2828
scanFile(confFileContent, variables)

internal/utils/dotenv/LoadEnvFile_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func TestLoadFileDependentVarsDefaultValues(t *testing.T) {
5454
defer teardownTest(t)
5555
err := LoadEnvFile("./testsData/dependentVars.txt")
5656
assert.NilError(t, err, "error should be nil")
57-
envValue, exists := os.LookupEnv("FRAMEWORK_ROOT_DIR")
57+
envValue, exists := os.LookupEnv("CUSTOM_ENV_VAR")
5858
assert.Equal(t, exists, true)
5959
assert.Equal(t, envValue, "dummy", envValue)
6060
envValue, exists = os.LookupEnv("SRC_FILE")
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
FRAMEWORK_ROOT_DIR=${MY_VAR:-dummy}
2-
SRC_FILE="${FRAMEWORK_ROOT_DIR}/srcFile"
1+
CUSTOM_ENV_VAR=${MY_VAR:-dummy}
2+
SRC_FILE="${CUSTOM_ENV_VAR}/srcFile"

internal/utils/encoding/checksum_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ func TestChecksumFromFile(t *testing.T) {
3030
)
3131
}
3232

33+
// jscpd:ignore-start
3334
func TestChecksumFromUnknownFile(t *testing.T) {
3435
tempFile, _ := os.CreateTemp("", "test******")
3536
tempFile.Sync()
@@ -40,3 +41,5 @@ func TestChecksumFromUnknownFile(t *testing.T) {
4041
assert.IsType(t, &fs.PathError{Op: "", Path: "", Err: nil}, err)
4142
assert.Equal(t, "", checksum)
4243
}
44+
45+
// jscpd:ignore-end

internal/utils/errors/errors.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package errors
22

3-
import "fmt"
3+
import (
4+
"fmt"
5+
)
46

57
type ValidationError struct {
68
InnerError error
@@ -15,3 +17,15 @@ func (e *ValidationError) Error() string {
1517
e.Context, e.FieldName, e.FieldValue,
1618
)
1719
}
20+
21+
type closeInterface interface {
22+
Close() error
23+
}
24+
25+
func SafeCloseDeferCallback(file closeInterface, err *error) {
26+
// Report the error, if any, from Close, but do so
27+
// only if there isn't already an outgoing error.
28+
if c := file.Close(); *err == nil {
29+
*err = c
30+
}
31+
}

internal/utils/files/files.go

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"errors"
88
"io"
99
"os"
10+
11+
myErrors "github.com/fchastanet/bash-compiler/internal/utils/errors"
1012
)
1113

1214
const (
@@ -90,21 +92,14 @@ func Copy(srcPath string, dstPath string) (err error) {
9092
if err != nil {
9193
return err
9294
}
93-
// skipcq: GO-S2307 // no need Sync as readOnly open
94-
defer r.Close() // ignore error: file was opened read-only.
95+
defer myErrors.SafeCloseDeferCallback(r, &err)
9596

9697
w, err := os.Create(dstPath)
9798
if err != nil {
9899
return err
99100
}
100101

101-
defer func() {
102-
// Report the error, if any, from Close, but do so
103-
// only if there isn't already an outgoing error.
104-
if c := w.Close(); err == nil {
105-
err = c
106-
}
107-
}()
102+
defer myErrors.SafeCloseDeferCallback(w, &err)
108103

109104
_, err = io.Copy(w, r)
110105
return err

internal/utils/tar/tar.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"path/filepath"
1010
"time"
1111

12+
"github.com/fchastanet/bash-compiler/internal/utils/errors"
1213
"github.com/fchastanet/bash-compiler/internal/utils/logger"
1314
)
1415

@@ -17,15 +18,15 @@ func CreateArchive(
1718
relativeDir string,
1819
buf io.Writer,
1920
updateFileInfoHeader func(info *tar.Header, fi fs.FileInfo) error,
20-
) error {
21+
) (err error) {
2122
// Create new Writers for gzip and tar
2223
// These writers are chained. Writing to the tar writer will
2324
// write to the gzip writer which in turn will write to
2425
// the "buf" writer
2526
gw := gzip.NewWriter(buf)
26-
defer gw.Close()
27+
defer errors.SafeCloseDeferCallback(gw, &err)
2728
tw := tar.NewWriter(gw)
28-
defer tw.Close()
29+
defer errors.SafeCloseDeferCallback(tw, &err)
2930

3031
// Iterate over files and add them to the tar archive
3132
for _, file := range files {
@@ -97,8 +98,7 @@ func addToArchive(
9798
if err != nil {
9899
return err
99100
}
100-
// skipcq: GO-S2307 // no need Sync as readOnly open
101-
defer file.Close()
101+
defer errors.SafeCloseDeferCallback(file, &err)
102102

103103
header, err := getFileHeader(fileInfo, filename, relativeDir)
104104
if err != nil {

0 commit comments

Comments
 (0)