[fix] Enable (most) wrapping when seeing USER directive (#34)

* Use COPY --chmod=0755 whenever possible when doing entrypoint wrapping.
* Capture any USER command in a Dockerfile. If we're wrapping, and COPY --chmod is not available to us, and the last section has a USER section, then we wrap a manual chmod with USER root / USER <whatever they provided>
* adding new section for 0.1.3 release notes

---------

Co-authored-by: indecisivedragon <liming@crashoverride.com>
Co-authored-by: Miroslav Shubernetskiy <miroslav@miki725.com>

src/chalk_common.nim

@@ -229,6 +229,9 @@ type
     rawSrc*: seq[string]
     rawDst*: string
 
+  DfUserInfo* = ref object of InfoBase
+    str*: string
+
   LabelInfo* = ref object of InfoBase
     labels*: OrderedTable[string, string]
 
@@ -238,6 +241,7 @@ type
     entryPoint*:  EntryPointInfo
     cmd*:         CmdInfo
     shell*:       ShellInfo
+    lastUser*:    DfUserInfo
 
   DockerInvocation* = ref object
     dockerExe*:         string

src/commands/cmd_docker.nim

@@ -37,6 +37,8 @@ proc runMungedDockerInvocation*(ctx: DockerInvocation): int =
   trace("Running docker: " & dockerExeLocation & " " & args.join(" "))
 
   if ctx.dfPassOnStdin:
+    if not ctx.inDockerFile.endswith("\n"):
+      ctx.inDockerFile &= "\n"
     newStdin = ctx.inDockerFile & ctx.addedInstructions.join("\n")
     trace("Passing on stdin: \n" & newStdin)
 

src/docker_base.nim

@@ -12,6 +12,7 @@ import osproc, config, util, reporting
 
 var
   buildXVersion: float  = 0   # Major and minor only
+  dockerVersion: string = ""
 
 const
   hashHeader* = "sha256:"
@@ -62,6 +63,19 @@ proc getBuildXVersion*(): float =
 
   return buildXVersion
 
+proc getDockerVersion*(): string =
+  once:
+    let (output, exitcode) = execCmdEx(dockerExeLocation & " version")
+    if exitcode == 0:
+      let words = output.split(" ")
+
+      for item in words:
+        if '.' in item:
+          dockerVersion = item
+          break
+
+  return dockerVersion
+
 template haveBuildContextFlag*(): bool =
   buildXVersion >= 0.8
 
@@ -105,12 +119,15 @@ proc makeFileAvailableToDocker*(ctx:      DockerInvocation,
     once:
       trace("Docker injection method: --build-context")
 
+    var chmodstr = ""
+
+    if chmod:
+      chmodstr = "--chmod=0755 "
+
     ctx.newCmdLine.add("--build-context")
     ctx.newCmdLine.add("chalkexedir" & $(contextCounter) & "=\"" & dir & "\"")
-    ctx.addedInstructions.add("COPY --from=chalkexedir" & $(contextCounter) &
-      " " & file & " /" & newname)
-    if chmod:
-      ctx.addedInstructions.add("RUN chmod 0755 /" & newname)
+    ctx.addedInstructions.add("COPY " & chmodstr & "--from=chalkexedir" &
+      $(contextCounter) & " " & file & " /" & newname)
     contextCounter += 1
     if move:
       registerTempFile(loc)
@@ -138,10 +155,24 @@ proc makeFileAvailableToDocker*(ctx:      DockerInvocation,
           copyFile(loc, dstLoc)
           trace("Copied " & loc & " to " & dstLoc)
 
-        ctx.addedInstructions.add("COPY " & file & " " & " /" & newname)
-        if chmod:
+        if chmod and getDockerVersion().startswith("2") and
+           getBuildXVersion() > 0:
+          ctx.addedInstructions.add("COPY --chmod=0755 " & file & " " & " /" &
+            newname)
+        elif chmod:
+          let useDirective = ctx.dfSections[^1].lastUser
+
+          # TODO detect user from base image if possible but thats not
+          # trivial as what is a base image is not a trivial question
+          # due to multi-stage build possibilities...
+          if useDirective != nil:
+            ctx.addedInstructions.add("USER root")
+          ctx.addedInstructions.add("COPY " & file & " " & " /" & newname)
           ctx.addedInstructions.add("RUN chmod 0755 /" & newname)
-
+          if useDirective != nil:
+            ctx.addedInstructions.add("USER " & useDirective.str)
+        else:
+          ctx.addedInstructions.add("COPY " & file & " " & " /" & newname)
         registerTempFile(dstLoc)
 
     except:

src/dockerfile.nim

@@ -788,6 +788,8 @@ proc parseAndEval*(s:      Stream,
     of "CMD":
       firstFromCheck()
       res.add(parse.parseCmd(cmd))
+    of "USER":
+      res.add(DfUserInfo(str: unicode.strip(cmd.rawArg)))
     of "ONBUILD":
       firstFromCheck()
       res.add(parse.parseOnBuild(cmd))
@@ -855,6 +857,8 @@ proc evalAndExtractDockerfile*(ctx: DockerInvocation) =
       section.cmd = CmdInfo(obj)
     elif obj of ShellInfo:
       section.shell = ShellInfo(obj)
+    elif obj of DfUserInfo:
+      section.lastUser = DfUserInfo(obj)
     elif obj of LabelInfo:
       for k, v in LabelInfo(obj).labels:
         labels[k] = v

src/docs/core-release-notes.md

@@ -1,3 +1,26 @@
+# Release Notes for Chalk version 0.1.3
+
+## Fixes
+
+- Docker support when installed via `Snap`.
+  [9](https://github.com/crashappsec/chalk/pull/9)
+- Removes error log when using chalk on ARM Linux as chalk fully
+  runs on ARM Linux now.
+  [7](https://github.com/crashappsec/chalk/pull/7)
+- When a `Dockerfile` uses `USER` directive, chalk can now wrap
+  entrypoint in that image
+  (`docker.wrap_entrypoint = true` in chalk config).
+  [34](https://github.com/crashappsec/chalk/pull/34)
+
+## Known Issues
+
+### Containers
+
+- When a `Dockerfile` does not use `USER` directive but base image
+  uses it to change default image user, chalk cannot wrap the
+  image as it on legacy Docker builder (not buildx) as it will
+  fail to `chmod` permissions of chalk during the build.
+
 # Release Notes for Chalk version 0.1.2
 
 This is the first open source release of Chalk. For those who

tests/chalk/runner.py

@@ -345,8 +345,10 @@ def docker_build(
         tag: Optional[str] = None,
         context: Optional[Path] = None,
         expected_success: bool = True,
+        expecting_report: bool = True,
         virtual: bool = False,
         cwd: Optional[Path] = None,
+        args: Optional[dict[str, str]] = None,
         config: Optional[Path] = None,
         buildkit: bool = True,
     ) -> tuple[str, ChalkProgram]:
@@ -358,6 +360,7 @@ def docker_build(
             tag=tag,
             context=context,
             dockerfile=dockerfile,
+            args=args,
             cwd=cwd,
             expected_success=expected_success,
             buildkit=buildkit,
@@ -375,6 +378,7 @@ def docker_build(
                     tag=tag,
                     context=context,
                     dockerfile=dockerfile,
+                    args=args,
                 ),
                 expected_success=expected_success,
                 cwd=cwd,
@@ -384,11 +388,18 @@ def docker_build(
         dockerfile = dockerfile or (
             (cwd or context or Path(os.getcwd())) / "Dockerfile"
         )
-        if expected_success:
+        if expecting_report and expected_success:
             # sanity check that chalk mark includes basic chalk keys
             assert image_hash == result.mark["_CURRENT_HASH"]
             assert image_hash == result.mark["_IMAGE_ID"]
             assert str(dockerfile) == result.mark["DOCKERFILE_PATH"]
+        elif not expecting_report:
+            try:
+                assert not result.reports
+            except json.JSONDecodeError:
+                # we are not expecting any report json to be present in output
+                # so this exception is expected here
+                pass
         return image_hash, result
 
     def docker_push(self, image: str):

tests/data/configs/docker_wrap.conf

@@ -0,0 +1 @@
+docker.wrap_entrypoint = true

tests/data/dockerfiles/valid/sample_3/Dockerfile

@@ -0,0 +1,6 @@
+FROM alpine as base
+
+RUN adduser -D testuser
+RUN id
+USER testuser
+RUN id

tests/data/dockerfiles/valid/split_dockerfiles/base.Dockerfile

@@ -0,0 +1,6 @@
+FROM alpine as base
+
+RUN adduser -D testuser
+RUN id
+USER testuser
+RUN id

tests/data/dockerfiles/valid/split_dockerfiles/test.Dockerfile

@@ -0,0 +1,4 @@
+ARG BASE
+FROM $BASE
+
+ENTRYPOINT ["/bin/bash", "-c"]

tests/test_docker.py

@@ -22,6 +22,7 @@
 from .conf import CONFIGS, DOCKERFILES, REGISTRY
 from .utils.docker import Docker
 from .utils.log import get_logger
+from .utils.os import run
 
 
 logger = get_logger()
@@ -69,6 +70,10 @@ def run_container(*args, **kwargs):
         (DOCKERFILES / "valid" / "sample_1", None, False),
         # PWD=foo && docker build -t test .
         (DOCKERFILES / "valid" / "sample_1", None, True),
+        # PWD=foo && docker build .
+        (DOCKERFILES / "valid" / "sample_3", None, False),
+        # PWD=foo && docker build -t test .
+        (DOCKERFILES / "valid" / "sample_3", None, True),
         # docker build -f foo/Dockerfile foo
         (None, DOCKERFILES / "valid" / "sample_1" / "Dockerfile", False),
         # docker build -f foo/Dockerfile -t test foo
@@ -91,16 +96,59 @@ def test_build(
         cwd=cwd,
         tag=random_hex if tag else None,
         buildkit=buildkit,
+        config=CONFIGS / "docker_wrap.conf",
     )
     assert image_id
 
 
+@pytest.mark.parametrize("buildkit", [True, False])
+@pytest.mark.parametrize(
+    "base, test",
+    [
+        (
+            DOCKERFILES / "valid" / "split_dockerfiles" / "base.Dockerfile",
+            DOCKERFILES / "valid" / "split_dockerfiles" / "test.Dockerfile",
+        ),
+    ],
+)
+def test_composite_build(
+    chalk: Chalk,
+    base: Path,
+    test: Path,
+    buildkit: bool,
+    random_hex: str,
+):
+    image_id, _ = Docker.build(
+        dockerfile=base,
+        buildkit=buildkit,
+        tag=random_hex,
+    )
+    assert image_id
+
+    # TODO this is a known limitation for the moment
+    # we EXPECT this case to fail without buildkit enabled,
+    # as base image adjusts USER and child Dockerfile
+    # does not have any indication that USER was adjusted
+    # and so we cannot detect USER from base image.
+    # In this case chalk falls back to standard docker build
+    # which means there is no chalk report in the output
+    second_image_id, result = chalk.docker_build(
+        dockerfile=test,
+        buildkit=buildkit,
+        args={"BASE": random_hex},
+        config=CONFIGS / "docker_wrap.conf",
+        expecting_report=buildkit,
+    )
+    assert second_image_id
+
+
 @mock.patch.dict(os.environ, {"SINK_TEST_OUTPUT_FILE": "/tmp/sink_file.json"})
 @pytest.mark.parametrize(
     "test_file",
     [
         "valid/sample_1",
         "valid/sample_2",
+        "valid/sample_3",
     ],
 )
 def test_virtual_valid(
@@ -173,12 +221,15 @@ def test_virtual_invalid(
     ).is_file(), "virtual-chalk.json should not have been created!"
 
 
-@pytest.mark.parametrize("test_file", ["valid/sample_1", "valid/sample_2"])
+@pytest.mark.parametrize(
+    "test_file", ["valid/sample_1", "valid/sample_2", "valid/sample_3"]
+)
 def test_nonvirtual_valid(chalk: Chalk, test_file: str, random_hex: str):
     tag = f"{test_file}_{random_hex}"
     image_hash, build = chalk.docker_build(
         dockerfile=DOCKERFILES / test_file / "Dockerfile",
         tag=tag,
+        config=CONFIGS / "docker_wrap.conf",
     )
 
     # artifact is the docker image

tests/utils/docker.py

@@ -21,12 +21,15 @@ def build_cmd(
         tag: Optional[str],
         context: Optional[Path] = None,
         dockerfile: Optional[Path] = None,
+        args: Optional[dict[str, str]] = None,
     ):
         cmd = ["docker", "build"]
         if tag:
             cmd += ["-t", tag]
         if dockerfile:
             cmd += ["-f", str(dockerfile)]
+        for name, value in (args or {}).items():
+            cmd += [f"--build-arg={name}={value}"]
         cmd += [str(context or ".")]
         return cmd
 
@@ -36,6 +39,7 @@ def build(
         tag: Optional[str] = None,
         context: Optional[Path] = None,
         dockerfile: Optional[Path] = None,
+        args: Optional[dict[str, str]] = None,
         cwd: Optional[Path] = None,
         expected_success: bool = True,
         buildkit: bool = True,
@@ -45,7 +49,9 @@ def build(
         """
         return Docker.with_image_id(
             run(
-                Docker.build_cmd(tag=tag, context=context, dockerfile=dockerfile),
+                Docker.build_cmd(
+                    tag=tag, context=context, dockerfile=dockerfile, args=args
+                ),
                 expected_exit_code=int(not expected_success),
                 env=Docker.build_env(buildkit=buildkit),
                 cwd=cwd,