diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ |:-------------------------------------------------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------| | [DL3000](https://github.com/hadolint/hadolint/wiki/DL3000) | Use absolute WORKDIR. | | [DL3001](https://github.com/hadolint/hadolint/wiki/DL3001) | For some bash commands it makes no sense running them in a Docker container like ssh, vim, shutdown, service, ps, free, top, kill, mount, ifconfig. | -| [DL3002](https://github.com/hadolint/hadolint/wiki/DL3002) | Do not switch to root USER. | +| [DL3002](https://github.com/hadolint/hadolint/wiki/DL3002) | Last user should not be root. | | [DL3003](https://github.com/hadolint/hadolint/wiki/DL3003) | Use WORKDIR to switch to a directory. | | [DL3004](https://github.com/hadolint/hadolint/wiki/DL3004) | Do not use sudo as it leads to unpredictable behavior. Use a tool like gosu to enforce root. | | [DL3005](https://github.com/hadolint/hadolint/wiki/DL3005) | Do not use apt-get upgrade or dist-upgrade. | diff --git a/hadolint.cabal b/hadolint.cabal --- a/hadolint.cabal +++ b/hadolint.cabal @@ -2,10 +2,10 @@ -- -- see: https://github.com/sol/hpack -- --- hash: 6d3df6bb7a68f68f6c3b7f0396c1a4f2ef36d21dd6eebe46dff3c194f1a932c4 +-- hash: 91a172ca6bf1a65cb720378c6e3afd17c6f8ecaa619d9e2ca1f9b81e85c2d8a6 name: hadolint -version: 1.7.4 +version: 1.7.5 synopsis: Dockerfile Linter JavaScript API description: A smarter Dockerfile linter that helps you build best practice Docker images. category: Development diff --git a/src/Hadolint/Rules.hs b/src/Hadolint/Rules.hs --- a/src/Hadolint/Rules.hs +++ b/src/Hadolint/Rules.hs @@ -6,6 +6,7 @@ import Control.Arrow ((&&&)) import Data.List (dropWhile, isInfixOf, isPrefixOf, mapAccumL) import Data.List.NonEmpty (toList) +import Data.Maybe (catMaybes) import qualified Hadolint.Bash as Bash import Language.Docker.Syntax @@ -312,16 +313,35 @@ invalidCmds = ["ssh", "vim", "shutdown", "service", "ps", "free", "top", "kill", "mount"] noRootUser :: Rule -noRootUser = instructionRule code severity message check +noRootUser dockerfile = instructionRuleState code severity message check Nothing dockerfile where code = "DL3002" severity = WarningC - message = "Do not switch to root USER" - check (User user) = - not - (Text.isPrefixOf "root:" user || - Text.isPrefixOf "0:" user || user == "root" || user == "0") - check _ = True + message = "Last USER should not be root" + check _ _ (From from) = withState (Just from) True -- Remember the last FROM instruction found + check st@(Just _) line (User user) + | isRoot user && lastUserIsRoot line = withState st False + | otherwise = withState st True + check st _ _ = withState st True + -- + -- + lastUserIsRoot line = (line, True) `elem` rootStages + -- + -- + rootStages :: [(Linenumber, Bool)] + rootStages = + let (_, userTuples) = + mapAccumL buildMap Nothing (map (instruction &&& lineNumber) dockerfile) + in catMaybes userTuples + -- + -- + buildMap _ (From from, _) = withState (Just from) Nothing + buildMap st@(Just _) (User user, line) = withState st (Just (line, isRoot user)) + buildMap st _ = withState st Nothing + -- + -- + isRoot user = + Text.isPrefixOf "root:" user || Text.isPrefixOf "0:" user || user == "root" || user == "0" noCd :: Rule noCd = instructionRule code severity message check diff --git a/test/Spec.hs b/test/Spec.hs --- a/test/Spec.hs +++ b/test/Spec.hs @@ -54,11 +54,83 @@ it "sudo" $ do ruleCatches noSudo "RUN sudo apt-get update" onBuildRuleCatches noSudo "RUN sudo apt-get update" - it "no root" $ ruleCatches noRootUser "USER root" - it "no root" $ ruleCatchesNot noRootUser "USER foo" - it "no root UID" $ ruleCatches noRootUser "USER 0" - it "no root:root" $ ruleCatches noRootUser "USER root:root" - it "no root UID:GID" $ ruleCatches noRootUser "USER 0:0" + + it "last user should not be root" $ + let dockerFile = + [ "FROM scratch" + , "USER root" + ] + in ruleCatches noRootUser $ Text.unlines dockerFile + + it "no root" $ + let dockerFile = + [ "FROM scratch" + , "USER foo" + ] + in ruleCatchesNot noRootUser $ Text.unlines dockerFile + + it "no root UID" $ + let dockerFile = + [ "FROM scratch" + , "USER 0" + ] + in ruleCatches noRootUser $ Text.unlines dockerFile + + it "no root:root" $ + let dockerFile = + [ "FROM scratch" + , "USER root:root" + ] + in ruleCatches noRootUser $ Text.unlines dockerFile + + it "no UID:GID" $ + let dockerFile = + [ "FROM scratch" + , "USER 0:0" + ] + in ruleCatches noRootUser $ Text.unlines dockerFile + + it "can switch back to non root" $ + let dockerFile = + [ "FROM scratch" + , "USER root" + , "RUN something" + , "USER foo" + ] + in ruleCatches noRootUser $ Text.unlines dockerFile + + it "warns on transitive root user" $ + let dockerFile = + [ "FROM debian as base" + , "USER root" + , "RUN something" + , "FROM base" + , "RUN something else" + ] + in ruleCatches noRootUser $ Text.unlines dockerFile + + it "warns on multiple stages" $ + let dockerFile = + [ "FROM debian as base" + , "USER root" + , "RUN something" + , "FROM scratch" + , "USER foo" + , "RUN something else" + ] + in ruleCatches noRootUser $ Text.unlines dockerFile + + it "does not warn when switching in multiple stages" $ + let dockerFile = + [ "FROM debian as base" + , "USER root" + , "RUN something" + , "USER foo" + , "FROM scratch" + , "RUN something else" + ] + in ruleCatches noRootUser $ Text.unlines dockerFile + it "install sudo" $ do ruleCatchesNot noSudo "RUN apt-get install sudo" onBuildRuleCatchesNot noSudo "RUN apt-get install sudo" @@ -623,13 +695,15 @@ in ruleCatchesNot noRootUser $ Text.unlines dockerFile it "ignores only the given rule" $ let dockerFile = - [ "# hadolint ignore=DL3001" + [ "FROM scratch" + , "# hadolint ignore=DL3001" , "USER root" ] in ruleCatches noRootUser $ Text.unlines dockerFile it "ignores only the given rule, when multiple passed" $ let dockerFile = - [ "# hadolint ignore=DL3001,DL3002" + [ "FROM scratch" + , "# hadolint ignore=DL3001,DL3002" , "USER root" ] in ruleCatchesNot noRootUser $ Text.unlines dockerFile @@ -642,7 +716,8 @@ in ruleCatches noRootUser $ Text.unlines dockerFile it "won't ignore the rule if passed invalid rule names" $ let dockerFile = - [ "# hadolint ignore=crazy,DL3002" + [ "FROM scratch" + , "# hadolint ignore=crazy,DL3002" , "USER root" ] in ruleCatches noRootUser $ Text.unlines dockerFile