Skip to content

Commit

Permalink
osbuild-worker-executor: appease gosec
Browse files Browse the repository at this point in the history
Note that gosec IMHO is a bit silly here, the heuristics used are
note very good, i.e. the code is already validating the external
inputs and it's not clear to me that "filepath.Clean()" will help
but it seems to supress the error. I hope gosec provides value
in other places, here it seems to be adding work :/

I also excluded "gosec" from any _test.go files, I do not see
why we should gosec tests?
  • Loading branch information
mvo5 committed Jun 5, 2024
1 parent 2fa36fc commit aca1bab
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 8 deletions.
3 changes: 3 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,6 @@ issues:

# Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
max-same-issues: 0
exclude-rules:
- path: ^cmd/osbuild-worker-executor/.*_test\.go$
linters: ["gosec"]
25 changes: 19 additions & 6 deletions cmd/osbuild-worker-executor/handler_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ func runOsbuild(buildDir string, control *controlJSON, output io.Writer) (string
}
// stream output over http
wf := writeFlusher{w: output, flusher: flusher}
// note that the "filepath.Clean()" is here for gosec:G304
logfPath := filepath.Clean(filepath.Join(buildDir, "build.log"))
// and also write to our internal log
logf, err := os.Create(filepath.Join(buildDir, "build.log"))
logf, err := os.Create(logfPath)
if err != nil {
return "", fmt.Errorf("cannot create log file: %v", err)
}
Expand Down Expand Up @@ -79,6 +81,7 @@ func runOsbuild(buildDir string, control *controlJSON, output io.Writer) (string
return "", err
}

// #nosec G204
cmd = exec.Command(
"tar",
"-Scf",
Expand Down Expand Up @@ -152,7 +155,8 @@ func handleManifestJSON(atar *tar.Reader, buildDir string) error {
}
manifestJSONPath := filepath.Join(buildDir, "manifest.json")

f, err := os.Create(manifestJSONPath)
// note that the filepath.Clean() is here just to appease gosec:G304
f, err := os.Create(filepath.Clean(manifestJSONPath))
if err != nil {
return fmt.Errorf("cannot create manifest.json: %v", err)
}
Expand Down Expand Up @@ -180,23 +184,32 @@ func handleIncludedSources(atar *tar.Reader, buildDir string) error {
}

// ensure we only allow "store/" things
if filepath.Clean(hdr.Name) != strings.TrimSuffix(hdr.Name, "/") {
return fmt.Errorf("name not clean: %v != %v", filepath.Clean(hdr.Name), hdr.Name)
name := hdr.Name
if filepath.Clean(name) != strings.TrimSuffix(name, "/") {
return fmt.Errorf("name not clean: %v != %v", filepath.Clean(name), name)
}
if !strings.HasPrefix(hdr.Name, "store/") {
if !strings.HasPrefix(name, "store/") {
return fmt.Errorf("expected store/ prefix, got %v", hdr.Name)
}
// note that the extra filepath.Clean() is just there to
// appease gosec G305
target := filepath.Join(buildDir, filepath.Clean(name))

// this assume "well" behaving tars, i.e. all dirs that lead
// up to the tar are included etc
target := filepath.Join(buildDir, hdr.Name)
mode := os.FileMode(hdr.Mode)
switch hdr.Typeflag {
case tar.TypeDir:
if err := os.Mkdir(target, mode); err != nil {
return fmt.Errorf("unpack: %w", err)
}
case tar.TypeReg:
// Note that the G304 here is silly, target is carefully
// checked (and is no error in the flow above). Mode is
// not relevant for an information leak. But it seems
// there is no way to get a debug trace from gosec what
// exactly is wrong.
// #nosec G304
f, err := os.OpenFile(target, os.O_RDWR|os.O_CREATE, mode)
if err != nil {
return fmt.Errorf("unpack: %w", err)
Expand Down
5 changes: 3 additions & 2 deletions cmd/osbuild-worker-executor/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ func run(ctx context.Context, args []string, getenv func(string) string) error {

srv := newServer(logger, config)
httpServer := &http.Server{
Addr: net.JoinHostPort(config.Host, config.Port),
Handler: srv,
Addr: net.JoinHostPort(config.Host, config.Port),
Handler: srv,
ReadHeaderTimeout: 10 * time.Second,
}
go func() {
logger.Printf("listening on %s\n", httpServer.Addr)
Expand Down

0 comments on commit aca1bab

Please sign in to comment.