segmentation violation message when passing Dockerfile via --rootfs option with a bad location of the Kraftfile to kraft build

[razvand]

Describe the bug

This is a quite specific setup that results in a segmentation violation message being outputted by kraft build. The command must feature:

a) the --rootfs option with a path using the Dockerfile word; the path may be correct or not
b) the --kraftfile / -K option with a path to a missing Kraftfile, i.e. the path is incorect

Running the command will result in a SIGSEGV being delivered, and a segmentation violation message being outputted.

Steps to reproduce

Use the command below anywhere. No need for valid paths:

kraft build --rootfs ./Dockerfile -K /Kraftfile .

It will result in a segmentation violation message being delivered. The Dockerfile may exist or not.

Note that the --rootfs option must be passed a string with the Dockefile argument. These commands result in the same behavior:

kraft build --rootfs ./Dockerfile2 -K /Kraftfile .
kraft build --rootfs ./Dockerfile/a -K /Kraftfile .
kraft build --rootfs ./Dockerfile/a/b/c -K /Kraftfile .

The path to the Kraftfile (the argument to the -K option) must be an missing path. These commands result in the same behavior, as long as the Kraftfile path is incorrect:

kraft build --rootfs ./Dockerfile -K ./Kraftfile .
kraft build --rootfs ./Dockerfile -K ../Kraftfile .
kraft build --rootfs ./Dockerfile -K /usr/share/Kraftfile .

This behavior will not happen if:

A. the Kraftfile exists and
B. the Kraftfile contains the spec and the rootfs options inside

For example, this happens:

$ cat Kraftfile 
spec: v0.6

runtime: base:latest

$ kraft build --rootfs ./Dockerfile -K ./Kraftfile .
 E  could not initialize initramfs builder: could not determine how to build initrd from: ./Dockerfil

Expected behavior

The command should print out an error such as missing Kraftfile or badly formatted Kraftfile, instead of issuing a segmentation violation message.

Which architectures were you using or does this bug affect?

No response

Which operating system were you using or does this bug affect?

linux/debian

Relevant log output

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0xf8 pc=0x1a8e5e7]

goroutine 1 [running]:
kraftkit.sh/internal/cli/kraft/build.(*builderKraftfileRuntime).Prepare(0x0?, {0x25920b8, 0xc000665d40}, 0xc00026e690, {0xc00026b680?, 0xc000458850?, 0xc00010f398?})
        /__w/kraftkit/kraftkit/internal/cli/kraft/build/builder_kraftfile_runtime.go:59 +0x87
kraftkit.sh/internal/cli/kraft/build.(*builderDockerfile).Prepare(0xc00026b680?, {0x25920b8?, 0xc000665d40?}, 0x1d45ea0?, {0xc000408b60?, 0x470873?, 0x1d462e0?})
        /__w/kraftkit/kraftkit/internal/cli/kraft/build/builder_dockerfile.go:50 +0x2a
kraftkit.sh/internal/cli/kraft/build.Build({0x25920b8, 0xc000665d40}, 0x0?, {0xc0005c06e0, 0x1, 0x5})
        /__w/kraftkit/kraftkit/internal/cli/kraft/build/build.go:106 +0x443
kraftkit.sh/internal/cli/kraft/build.(*BuildOptions).Run(0xc00026e690, {0x25920b8, 0xc000665d40}, {0xc0005c06e0?, 0x0?, 0xffffffffffffffff?})
        /__w/kraftkit/kraftkit/internal/cli/kraft/build/build.go:187 +0x4e
kraftkit.sh/cmdfactory.New.func1(0xc000338c08?, {0xc0005c06e0?, 0x0?, 0x0?})
        /__w/kraftkit/kraftkit/cmdfactory/builder.go:372 +0x43
kraftkit.sh/cmdfactory.AttributeFlags.bind.func3(0xc000338c08, {0xc0005c06e0, 0x1, 0x5})
        /__w/kraftkit/kraftkit/cmdfactory/builder.go:554 +0x1cb
github.com/spf13/cobra.(*Command).execute(0xc000338c08, {0xc0005c0690, 0x5, 0x5})
        /go/pkg/mod/github.com/spf13/[email protected]/command.go:985 +0xaca
github.com/spf13/cobra.(*Command).ExecuteC(0xc000338908)
        /go/pkg/mod/github.com/spf13/[email protected]/command.go:1117 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
        /go/pkg/mod/github.com/spf13/[email protected]/command.go:1041
github.com/spf13/cobra.(*Command).ExecuteContext(...)
        /go/pkg/mod/github.com/spf13/[email protected]/command.go:1034
kraftkit.sh/cmdfactory.Main({0x25920b8, 0xc0006653e0}, 0xc000338908)
        /__w/kraftkit/kraftkit/cmdfactory/builder.go:184 +0x6f
kraftkit.sh/internal/cli/kraft.Main({0x7fffb83490de?, 0x360e048?, 0xc0000061c0?})
        /__w/kraftkit/kraftkit/internal/cli/kraft/kraft.go:229 +0xbf5
main.main()
        /__w/kraftkit/kraftkit/cmd/kraft/main.go:19 +0x79

[bishnushah0909]

Root Cause:
The program doesn't properly handle the missing Kraftfile scenario and continues processing, trying to dereference an invalid object (the builderKraftfileRuntime object) when it should have terminated with a more informative error message. This oversight results in a segmentation fault.

You Might Fix it by :

1. Add Null Pointer Checks:
   The code should include validation for the input paths, especially the Kraftfile. If the Kraftfile is missing or invalid, the program should exit gracefully with an error message instead of proceeding to the point where it tries to dereference a nil pointer.
   example:
   if kraftfilePath == nil || !fileExists(kraftfilePath) {
   fmt.Println("Error: Kraftfile is missing or invalid.")
   return
   }

2)Gracefully Handle Missing Files:
there should be checks that ensure:

The --rootfs path is valid (even if it’s just a placeholder).
The -K path is either valid or gives a proper error if it's missing or incorrect.

if rootfsPath == "Dockerfile" && !fileExists(rootfsPath) {
fmt.Println("Error: Dockerfile path is invalid.")
return
}

3)Error Handling in Build Process: The builderKraftfileRuntime should not be initialized if the Kraftfile is missing. Instead, it should handle the error prior to attempting to prepare the build.

4)Improved Error Messaging:
Instead of a segmentation fault, the program should output a user-friendly error message indicating which file is missing or invalid. This would significantly improve the user experience and help in debugging.

5)Reproduce and Test: After applying these changes, try to reproduce the error by providing invalid paths and check that the application now exits with clear error messages instead of a crash.

6)Potential Fix in Code (Example):

func (b *builderKraftfileRuntime) Prepare(ctx context.Context, args ...interface{}) error {
if b == nil {
return fmt.Errorf("builderKraftfileRuntime is nil, cannot prepare build")
}

if b.KraftfilePath == "" || !fileExists(b.KraftfilePath) {
    return fmt.Errorf("Kraftfile path is missing or invalid")
}

// continue with the preparation process...

}

   OR you might even try this code 

func (b *builderKraftfileRuntime) Prepare(ctx context.Context, args ...interface{}) error {
if b == nil {
return fmt.Errorf("builderKraftfileRuntime is nil, cannot prepare build")
}

// Check if Kraftfile path is valid
if b.KraftfilePath == "" || !fileExists(b.KraftfilePath) {
    return fmt.Errorf("Kraftfile path is missing or invalid")
}

// If everything is fine, continue with the build process
return nil

}

func fileExists(path string) bool {
// Your logic to check if the file exists
return path != "" && os.Stat(path) == nil
}

[razvand]

Thanks, @bishnushah0909 , for the input. I'll let @craciunoiuc and @nderjung comment here, as they have a better view of the source code.