From a1bfedaf248f3e2c54edfb30ee7019cb9d697105 Mon Sep 17 00:00:00 2001 From: megh Date: Thu, 19 Oct 2023 15:22:03 -0600 Subject: [PATCH 1/9] fixes transaction error due to redirect --- Changelog.md | 1 + .../ContainerRegistryApi/Authorization.hs | 23 +++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Changelog.md b/Changelog.md index dfe9054319..1316b17349 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,6 +12,7 @@ - golang: Updates go.mod parser to be compatible with golang v1.21. ([#1304](https://github.com/fossas/fossa-cli/pull/1304)) - `fossa list-targets`: list-target command supports `--format` option with: `ndjson`, `text`, and `legacy`. ([#1296](https://github.com/fossas/fossa-cli/pull/1296)) +- container scanning: fixes transaction errors due to redirection ([]()) ## v3.8.17 diff --git a/src/Control/Carrier/ContainerRegistryApi/Authorization.hs b/src/Control/Carrier/ContainerRegistryApi/Authorization.hs index 81142989cf..fa0f63f562 100644 --- a/src/Control/Carrier/ContainerRegistryApi/Authorization.hs +++ b/src/Control/Carrier/ContainerRegistryApi/Authorization.hs @@ -46,7 +46,7 @@ import Network.HTTP.Client ( applyBearerAuth, parseRequest, ) -import Network.HTTP.Types (methodGet, statusCode) +import Network.HTTP.Types (methodGet, statusCode, methodHead) import Network.HTTP.Types.Header ( hAuthorization, hWWWAuthenticate, @@ -99,16 +99,19 @@ applyAuthToken (Just (BearerAuthToken token)) r = -- If we don't strip auth headers, on redirect, depending on how -- blobs/manifest are retrieved cloud vendor may throw 'Bad Request' error. stripAuthHeaderOnRedirect :: Request -> Request -stripAuthHeaderOnRedirect r = - if ((isAwsECR || isAzure) && method r == methodGet) - then r{shouldStripHeaderOnRedirect = (== hAuthorization)} - else r - where - isAwsECR :: Bool - isAwsECR = "amazonaws.com" `isInfixOf` decodeUtf8 (host r) +stripAuthHeaderOnRedirect r = r{shouldStripHeaderOnRedirect = (== hAuthorization)} + -- if ((isAwsECR || isAzure || isDocker) && (method r == methodGet || method r == methodHead)) + -- then r{shouldStripHeaderOnRedirect = (== hAuthorization)} + -- else r + -- where + -- isAwsECR :: Bool + -- isAwsECR = "amazonaws.com" `isInfixOf` decodeUtf8 (host r) + + -- isAzure :: Bool + -- isAzure = "azurecr.io" `isInfixOf` decodeUtf8 (host r) - isAzure :: Bool - isAzure = "azurecr.io" `isInfixOf` decodeUtf8 (host r) + -- isDocker :: Bool + -- isDocker = "docker.io" `isInfixOf` decodeUtf8 (host r) -- | Generates Auth Token For Request. -- From 6a79b359350148d47e82971ec172096507fff944 Mon Sep 17 00:00:00 2001 From: megh Date: Thu, 19 Oct 2023 15:23:02 -0600 Subject: [PATCH 2/9] redundant comments --- .../Carrier/ContainerRegistryApi/Authorization.hs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/Control/Carrier/ContainerRegistryApi/Authorization.hs b/src/Control/Carrier/ContainerRegistryApi/Authorization.hs index fa0f63f562..8e43a5a752 100644 --- a/src/Control/Carrier/ContainerRegistryApi/Authorization.hs +++ b/src/Control/Carrier/ContainerRegistryApi/Authorization.hs @@ -100,18 +100,6 @@ applyAuthToken (Just (BearerAuthToken token)) r = -- blobs/manifest are retrieved cloud vendor may throw 'Bad Request' error. stripAuthHeaderOnRedirect :: Request -> Request stripAuthHeaderOnRedirect r = r{shouldStripHeaderOnRedirect = (== hAuthorization)} - -- if ((isAwsECR || isAzure || isDocker) && (method r == methodGet || method r == methodHead)) - -- then r{shouldStripHeaderOnRedirect = (== hAuthorization)} - -- else r - -- where - -- isAwsECR :: Bool - -- isAwsECR = "amazonaws.com" `isInfixOf` decodeUtf8 (host r) - - -- isAzure :: Bool - -- isAzure = "azurecr.io" `isInfixOf` decodeUtf8 (host r) - - -- isDocker :: Bool - -- isDocker = "docker.io" `isInfixOf` decodeUtf8 (host r) -- | Generates Auth Token For Request. -- From c0c6fcced8b8259aadbbb9ae24f710aee063197d Mon Sep 17 00:00:00 2001 From: megh Date: Thu, 19 Oct 2023 19:58:29 -0600 Subject: [PATCH 3/9] Updates changelog and fixes linting and formatting errors --- Changelog.md | 2 +- src/Control/Carrier/ContainerRegistryApi/Authorization.hs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Changelog.md b/Changelog.md index 1316b17349..23b28e8ae6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,7 +12,7 @@ - golang: Updates go.mod parser to be compatible with golang v1.21. ([#1304](https://github.com/fossas/fossa-cli/pull/1304)) - `fossa list-targets`: list-target command supports `--format` option with: `ndjson`, `text`, and `legacy`. ([#1296](https://github.com/fossas/fossa-cli/pull/1296)) -- container scanning: fixes transaction errors due to redirection ([]()) +- container scanning: Fixes a network error defect, when interacting with container registry redirects. ([#1308](https://github.com/fossas/fossa-cli/pull/1308/)) ## v3.8.17 diff --git a/src/Control/Carrier/ContainerRegistryApi/Authorization.hs b/src/Control/Carrier/ContainerRegistryApi/Authorization.hs index 8e43a5a752..ef782e125c 100644 --- a/src/Control/Carrier/ContainerRegistryApi/Authorization.hs +++ b/src/Control/Carrier/ContainerRegistryApi/Authorization.hs @@ -33,20 +33,20 @@ import Data.Aeson (FromJSON (parseJSON), decode', eitherDecode, withObject, (.:) import Data.ByteString.Lazy qualified as ByteStringLazy import Data.Map (Map) import Data.Map qualified as Map -import Data.String.Conversion (ConvertUtf8 (decodeUtf8), encodeUtf8, toString, toText) -import Data.Text (Text, isInfixOf) +import Data.String.Conversion (encodeUtf8, toString, toText) +import Data.Text (Text) import Data.Text qualified as Text import Data.Void (Void) import Effect.Logger (Logger) import Network.HTTP.Client ( Manager, - Request (host, method, shouldStripHeaderOnRedirect), + Request (method, shouldStripHeaderOnRedirect), Response (responseBody, responseHeaders, responseStatus), applyBasicAuth, applyBearerAuth, parseRequest, ) -import Network.HTTP.Types (methodGet, statusCode, methodHead) +import Network.HTTP.Types (statusCode) import Network.HTTP.Types.Header ( hAuthorization, hWWWAuthenticate, From 1f12810d0f557afd89e062409b55f42cd2568e39 Mon Sep 17 00:00:00 2001 From: meghfossa Date: Mon, 30 Oct 2023 14:12:18 -0600 Subject: [PATCH 4/9] saving wip --- .../Analysis/ContainerScanningSpec.hs | 102 ++++++++++++++++++ src/Container/Docker/SourceParser.hs | 2 +- test/Container/Docker/SourceParserSpec.hs | 10 ++ 3 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 integration-test/Analysis/ContainerScanningSpec.hs diff --git a/integration-test/Analysis/ContainerScanningSpec.hs b/integration-test/Analysis/ContainerScanningSpec.hs new file mode 100644 index 0000000000..7ae43f50af --- /dev/null +++ b/integration-test/Analysis/ContainerScanningSpec.hs @@ -0,0 +1,102 @@ +{-# LANGUAGE DataKinds #-} + + +module Analysis.ContainerScanningSpec (spec) where + +import Test.Hspec ( it, shouldBe, Spec, fdescribe ) +import Control.Effect.ContainerRegistryApi + ( ContainerRegistryApi, getImageManifest, ContainerRegistryApiF ) +import Control.Effect.Diagnostics (Diagnostics) +import Control.Algebra (Has) +import Control.Carrier.Lift (Lift) +import Data.Text (Text) +import Container.Docker.SourceParser + ( RepoDigest(..), RegistryImageSource, parseImageUrl ) +import Control.Carrier.Diagnostics + ( fromEitherShow, runDiagnostics, DiagnosticsC ) +import Container.Docker.OciManifest (OciManifestV2(..), OciManifestConfig(..)) +import Text.Megaparsec (ParseErrorBundle, parse) +import Data.Void (Void) +import Control.Carrier.Stack ( runStack, StackC ) +import Control.Carrier.StickyLogger + ( ignoreStickyLogger, IgnoreStickyLoggerC ) +import Effect.Logger ( ignoreLogger, IgnoreLoggerC ) +import Effect.Exec ( runExecIO, ExecIOC ) +import Control.Carrier.Reader (runReader, ReaderC) +import Control.Carrier.ContainerRegistryApi (runContainerRegistryApi) +import Control.Carrier.Finally ( runFinally, FinallyC ) +import Diag.Result ( Result(..), renderFailure ) +import Effect.ReadFS ( runReadFSIO, ReadFSIOC ) +import Control.Carrier.ContainerRegistryApi.Common (RegistryCtx) +import Discovery.Filters (AllFilters) +import Control.Carrier.Simple (SimpleC) +import Type.Operator (type ($)) +import Control.Effect.Lift (sendIO) +import Data.String.Conversion ( toText ) +import System.Environment (lookupEnv) + +spec :: Spec +spec = testPrivateRepos + +testPrivateRepos :: Spec +testPrivateRepos = + fdescribe "private registries" $ do + it "dockerhub" $ do + img <- withAuth "FOSSA_DOCKERHUB_WWW_STYLE_USER_PASS" dockerHubImage + res <- runEff $ getImageConfig "amd64" img + case res of + Failure ws eg -> fail (show (renderFailure ws eg "An issue occurred")) + Success _ res' -> res' `shouldBe` dockerHubImageConfigDigest + +dockerHubImage :: Text +dockerHubImage = "index.docker.io/fossabot/container-test-fixture:0" + +dockerHubImageConfigDigest :: RepoDigest +dockerHubImageConfigDigest = RepoDigest "sha256:792d29ec0ccee20718b0233b1ca0c633a57009bbb4d99c247b0ec1e3f562b19b" + +-- + +type EffectStack m = + FinallyC + $ SimpleC ContainerRegistryApiF + $ ReaderC RegistryCtx + $ ReaderC AllFilters + $ ExecIOC + $ ReadFSIOC + $ DiagnosticsC + $ IgnoreLoggerC + $ IgnoreStickyLoggerC + $ StackC IO + +runEff :: EffectStack IO a -> IO (Result a) +runEff = runStack + . ignoreStickyLogger + . ignoreLogger + . runDiagnostics + . runReadFSIO + . runExecIO + . runReader mempty + . runContainerRegistryApi + . runFinally + +decodeStrict :: Text -> Text -> Either (ParseErrorBundle Text Void) RegistryImageSource +decodeStrict arch = parse (parseImageUrl arch) mempty + +getImageConfig :: + ( Has (Lift IO) sig m + , Has Diagnostics sig m + , Has ContainerRegistryApi sig m + ) => + Text -> + Text -> + m RepoDigest +getImageConfig arch img = + configDigest . ociConfig + <$> (getImageManifest =<< fromEitherShow (decodeStrict arch img)) + +withAuth :: Has (Lift IO) sig m => String -> Text -> m Text +withAuth authEnvKey target = do + auth <- sendIO $ lookupEnv authEnvKey + case auth of + Nothing -> pure target + Just auth' -> pure $ (toText auth') <> "@" <> target diff --git a/src/Container/Docker/SourceParser.hs b/src/Container/Docker/SourceParser.hs index 37819f0e45..d3db0d10d7 100644 --- a/src/Container/Docker/SourceParser.hs +++ b/src/Container/Docker/SourceParser.hs @@ -246,7 +246,7 @@ parseAuthCred :: Parser (Text, Text) parseAuthCred = do user <- toText <$> some alphaNumChar void (char ':') - password <- toText <$> (some alphaNumChar) + password <- toText <$> some (alphaNumChar <|> char '_' <|> char '-') void (char '@') pure (user, password) diff --git a/test/Container/Docker/SourceParserSpec.hs b/test/Container/Docker/SourceParserSpec.hs index c8fede2ae3..6c124f8921 100644 --- a/test/Container/Docker/SourceParserSpec.hs +++ b/test/Container/Docker/SourceParserSpec.hs @@ -127,6 +127,16 @@ spec = do fixtureArch ) + "https://user:pass-pass_pass@ghcr.io/fossas/haskell-dev-tools:9.0.2" + `shouldParseInto` ( RegistryImageSource + "ghcr.io" + defaultHttpScheme + (Just ("user", "pass-pass_pass")) + "fossas/haskell-dev-tools" + (mkTagRef "9.0.2") + fixtureArch + ) + fixtureArch :: Text fixtureArch = "amd64" From bf154cf4ff18fda45c29d0f4de43dc19a55ff943 Mon Sep 17 00:00:00 2001 From: meghfossa Date: Mon, 30 Oct 2023 14:28:49 -0600 Subject: [PATCH 5/9] update integeration tests --- integration-test/Analysis/ContainerScanningSpec.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-test/Analysis/ContainerScanningSpec.hs b/integration-test/Analysis/ContainerScanningSpec.hs index 7ae43f50af..f6a31134e0 100644 --- a/integration-test/Analysis/ContainerScanningSpec.hs +++ b/integration-test/Analysis/ContainerScanningSpec.hs @@ -3,7 +3,7 @@ module Analysis.ContainerScanningSpec (spec) where -import Test.Hspec ( it, shouldBe, Spec, fdescribe ) +import Test.Hspec ( it, shouldBe, Spec, describe ) import Control.Effect.ContainerRegistryApi ( ContainerRegistryApi, getImageManifest, ContainerRegistryApiF ) import Control.Effect.Diagnostics (Diagnostics) @@ -40,7 +40,7 @@ spec = testPrivateRepos testPrivateRepos :: Spec testPrivateRepos = - fdescribe "private registries" $ do + describe "private registries" $ do it "dockerhub" $ do img <- withAuth "FOSSA_DOCKERHUB_WWW_STYLE_USER_PASS" dockerHubImage res <- runEff $ getImageConfig "amd64" img From 0e1dca56a1e1eb9c32fede7b0946a70f4028a98d Mon Sep 17 00:00:00 2001 From: meghfossa Date: Mon, 30 Oct 2023 14:29:24 -0600 Subject: [PATCH 6/9] updates changelog --- Changelog.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 23b28e8ae6..c29e4ba9e7 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,8 @@ # FOSSA CLI Changelog +## Unreleased +- container scanning: Fixes a network error defect, when interacting with container registry redirects. ([#1308](https://github.com/fossas/fossa-cli/pull/1308/)) + ## v3.8.20 - container scanning: Fixes registry network calls, to ensure `fossa-cli` uses `Accept` header on `HEAD` network calls. ([#1309](https://github.com/fossas/fossa-cli/pull/1309)) @@ -12,7 +15,6 @@ - golang: Updates go.mod parser to be compatible with golang v1.21. ([#1304](https://github.com/fossas/fossa-cli/pull/1304)) - `fossa list-targets`: list-target command supports `--format` option with: `ndjson`, `text`, and `legacy`. ([#1296](https://github.com/fossas/fossa-cli/pull/1296)) -- container scanning: Fixes a network error defect, when interacting with container registry redirects. ([#1308](https://github.com/fossas/fossa-cli/pull/1308/)) ## v3.8.17 From 74316e50aaaca50862c6669099e330ee453650e7 Mon Sep 17 00:00:00 2001 From: meghfossa Date: Mon, 30 Oct 2023 16:00:24 -0600 Subject: [PATCH 7/9] adds comments --- integration-test/Analysis/ContainerScanningSpec.hs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/integration-test/Analysis/ContainerScanningSpec.hs b/integration-test/Analysis/ContainerScanningSpec.hs index f6a31134e0..f2c74e3d00 100644 --- a/integration-test/Analysis/ContainerScanningSpec.hs +++ b/integration-test/Analysis/ContainerScanningSpec.hs @@ -38,10 +38,13 @@ import System.Environment (lookupEnv) spec :: Spec spec = testPrivateRepos +-- Integeration tests to catch, if registry api implementation changes +-- abruptly. testPrivateRepos :: Spec testPrivateRepos = describe "private registries" $ do it "dockerhub" $ do + -- Refer to 1Password for creds or Github Action Env Keys img <- withAuth "FOSSA_DOCKERHUB_WWW_STYLE_USER_PASS" dockerHubImage res <- runEff $ getImageConfig "amd64" img case res of From 641c5cdf50d4ce4bf60b95d164574a7e82fc844f Mon Sep 17 00:00:00 2001 From: meghfossa Date: Mon, 30 Oct 2023 16:11:20 -0600 Subject: [PATCH 8/9] fix fmt --- .../Analysis/ContainerScanningSpec.hs | 89 +++++++++++-------- 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/integration-test/Analysis/ContainerScanningSpec.hs b/integration-test/Analysis/ContainerScanningSpec.hs index f2c74e3d00..a65ef743f4 100644 --- a/integration-test/Analysis/ContainerScanningSpec.hs +++ b/integration-test/Analysis/ContainerScanningSpec.hs @@ -1,39 +1,49 @@ {-# LANGUAGE DataKinds #-} - module Analysis.ContainerScanningSpec (spec) where -import Test.Hspec ( it, shouldBe, Spec, describe ) -import Control.Effect.ContainerRegistryApi - ( ContainerRegistryApi, getImageManifest, ContainerRegistryApiF ) -import Control.Effect.Diagnostics (Diagnostics) +import Container.Docker.OciManifest (OciManifestConfig (..), OciManifestV2 (..)) +import Container.Docker.SourceParser ( + RegistryImageSource, + RepoDigest (..), + parseImageUrl, + ) import Control.Algebra (Has) -import Control.Carrier.Lift (Lift) -import Data.Text (Text) -import Container.Docker.SourceParser - ( RepoDigest(..), RegistryImageSource, parseImageUrl ) -import Control.Carrier.Diagnostics - ( fromEitherShow, runDiagnostics, DiagnosticsC ) -import Container.Docker.OciManifest (OciManifestV2(..), OciManifestConfig(..)) -import Text.Megaparsec (ParseErrorBundle, parse) -import Data.Void (Void) -import Control.Carrier.Stack ( runStack, StackC ) -import Control.Carrier.StickyLogger - ( ignoreStickyLogger, IgnoreStickyLoggerC ) -import Effect.Logger ( ignoreLogger, IgnoreLoggerC ) -import Effect.Exec ( runExecIO, ExecIOC ) -import Control.Carrier.Reader (runReader, ReaderC) import Control.Carrier.ContainerRegistryApi (runContainerRegistryApi) -import Control.Carrier.Finally ( runFinally, FinallyC ) -import Diag.Result ( Result(..), renderFailure ) -import Effect.ReadFS ( runReadFSIO, ReadFSIOC ) import Control.Carrier.ContainerRegistryApi.Common (RegistryCtx) -import Discovery.Filters (AllFilters) +import Control.Carrier.Diagnostics ( + DiagnosticsC, + fromEitherShow, + runDiagnostics, + ) +import Control.Carrier.Finally (FinallyC, runFinally) +import Control.Carrier.Lift (Lift) +import Control.Carrier.Reader (ReaderC, runReader) import Control.Carrier.Simple (SimpleC) -import Type.Operator (type ($)) +import Control.Carrier.Stack (StackC, runStack) +import Control.Carrier.StickyLogger ( + IgnoreStickyLoggerC, + ignoreStickyLogger, + ) +import Control.Effect.ContainerRegistryApi ( + ContainerRegistryApi, + ContainerRegistryApiF, + getImageManifest, + ) +import Control.Effect.Diagnostics (Diagnostics) import Control.Effect.Lift (sendIO) -import Data.String.Conversion ( toText ) +import Data.String.Conversion (toText) +import Data.Text (Text) +import Data.Void (Void) +import Diag.Result (Result (..), renderFailure) +import Discovery.Filters (AllFilters) +import Effect.Exec (ExecIOC, runExecIO) +import Effect.Logger (IgnoreLoggerC, ignoreLogger) +import Effect.ReadFS (ReadFSIOC, runReadFSIO) import System.Environment (lookupEnv) +import Test.Hspec (Spec, describe, it, shouldBe) +import Text.Megaparsec (ParseErrorBundle, parse) +import Type.Operator (type ($)) spec :: Spec spec = testPrivateRepos @@ -42,14 +52,14 @@ spec = testPrivateRepos -- abruptly. testPrivateRepos :: Spec testPrivateRepos = - describe "private registries" $ do - it "dockerhub" $ do - -- Refer to 1Password for creds or Github Action Env Keys - img <- withAuth "FOSSA_DOCKERHUB_WWW_STYLE_USER_PASS" dockerHubImage - res <- runEff $ getImageConfig "amd64" img - case res of - Failure ws eg -> fail (show (renderFailure ws eg "An issue occurred")) - Success _ res' -> res' `shouldBe` dockerHubImageConfigDigest + describe "private registries" $ do + it "dockerhub" $ do + -- Refer to 1Password for creds or Github Action Env Keys + img <- withAuth "FOSSA_DOCKERHUB_WWW_STYLE_USER_PASS" dockerHubImage + res <- runEff $ getImageConfig "amd64" img + case res of + Failure ws eg -> fail (show (renderFailure ws eg "An issue occurred")) + Success _ res' -> res' `shouldBe` dockerHubImageConfigDigest dockerHubImage :: Text dockerHubImage = "index.docker.io/fossabot/container-test-fixture:0" @@ -72,7 +82,8 @@ type EffectStack m = $ StackC IO runEff :: EffectStack IO a -> IO (Result a) -runEff = runStack +runEff = + runStack . ignoreStickyLogger . ignoreLogger . runDiagnostics @@ -99,7 +110,7 @@ getImageConfig arch img = withAuth :: Has (Lift IO) sig m => String -> Text -> m Text withAuth authEnvKey target = do - auth <- sendIO $ lookupEnv authEnvKey - case auth of - Nothing -> pure target - Just auth' -> pure $ (toText auth') <> "@" <> target + auth <- sendIO $ lookupEnv authEnvKey + case auth of + Nothing -> pure target + Just auth' -> pure $ (toText auth') <> "@" <> target From e35cd8857cfff9ae4252f2629060c611a2c03c01 Mon Sep 17 00:00:00 2001 From: meghfossa Date: Mon, 30 Oct 2023 21:00:26 -0600 Subject: [PATCH 9/9] adds missing module in test suite --- spectrometer.cabal | 1 + 1 file changed, 1 insertion(+) diff --git a/spectrometer.cabal b/spectrometer.cabal index 545a4edd90..b13df4c678 100644 --- a/spectrometer.cabal +++ b/spectrometer.cabal @@ -669,6 +669,7 @@ test-suite integration-tests Analysis.CarthageSpec Analysis.ClojureSpec Analysis.CocoapodsSpec + Analysis.ContainerScanningSpec Analysis.ElixirSpec Analysis.ErlangSpec Analysis.FixtureExpectationUtils