avoid getting creds from environment during autoenable

When autoenabling special remotes of type S3, weddav, or glacier, do not
take login credentials from environment variables, as the user may not be
expecting the autoenable to happen, and may have those set for other
purposes.

Annex/SpecialRemote.hs

@@ -89,7 +89,7 @@ autoEnable = do
 			(Just name, Right t) -> whenM (canenable u) $ do
 				showSideAction $ "Auto enabling special remote " ++ name
 				dummycfg <- liftIO dummyRemoteGitConfig
-				tryNonAsync (setup t (Enable c) (Just u) Nothing c dummycfg) >>= \case
+				tryNonAsync (setup t (AutoEnable c) (Just u) Nothing c dummycfg) >>= \case
 					Left e -> warning (show e)
 					Right (_c, _u) ->
 						when (cu /= u) $

CHANGELOG

@@ -4,6 +4,10 @@ git-annex (8.20210311) UNRELEASED; urgency=medium
   * import: When the previously exported tree contained a submodule,
     preserve it in the imported tree so it does not get deleted.
   * export --json: Fill in the file field.
+  * When autoenabling special remotes of type S3, weddav, or glacier,
+    do not take login credentials from environment variables, as the user
+    may not be expecting the autoenable to happen, and may have those
+    set for other purposes.
 
  -- Joey Hess <id@joeyh.name>  Fri, 12 Mar 2021 12:06:37 -0400
 

Creds.hs

@@ -1,6 +1,6 @@
 {- Credentials storage
  -
- - Copyright 2012-2020 Joey Hess <id@joeyh.name>
+ - Copyright 2012-2021 Joey Hess <id@joeyh.name>
  -
  - Licensed under the GNU AGPL version 3 or higher.
  -}
@@ -25,6 +25,7 @@ import Annex.Common
 import qualified Annex
 import Types.Creds
 import Types.RemoteConfig
+import Types.Remote (SetupStage(..))
 import Annex.SpecialRemote.Config
 import Annex.Perms
 import Utility.FileMode
@@ -52,21 +53,25 @@ data CredPairStorage = CredPairStorage
  - that. Also caches them locally.
  -
  - The creds are found from the CredPairStorage storage if not provided,
- - so may be provided by an environment variable etc.
+ - so may be provided by an environment variable etc. When autoenabling,
+ - the user would not expect to provide creds, so none are found.
  -
  - The remote's configuration should have already had a cipher stored in it
  - if that's going to be done, so that the creds can be encrypted using the
  - cipher. The EncryptionIsSetup is witness to that being the case.
  -}
 setRemoteCredPair
-	:: EncryptionIsSetup
+	:: SetupStage
+	-> EncryptionIsSetup
 	-> ParsedRemoteConfig
 	-> RemoteGitConfig
 	-> CredPairStorage
 	-> Maybe CredPair
 	-> Annex RemoteConfig
-setRemoteCredPair encsetup pc gc storage mcreds = unparsedRemoteConfig <$>
+setRemoteCredPair ss encsetup pc gc storage mcreds = case ss of
-	setRemoteCredPair' pc encsetup gc storage mcreds
+	AutoEnable _ -> pure (unparsedRemoteConfig pc)
+	_ -> unparsedRemoteConfig <$>
+		setRemoteCredPair' pc encsetup gc storage mcreds
 
 setRemoteCredPair'
 	:: ParsedRemoteConfig

Remote/Git.hs

@@ -136,7 +136,11 @@ gitSetup Init mu _ c _ = do
 	if isNothing mu || mu == Just u
 		then return (c, u)
 		else error "git remote did not have specified uuid"
-gitSetup (Enable _) (Just u) _ c _ = do
+gitSetup (Enable _) mu _ c _ = enableRemote mu c
+gitSetup (AutoEnable _) mu _ c _ = enableRemote mu c
+
+enableRemote :: Maybe UUID -> RemoteConfig -> Annex (RemoteConfig, UUID)
+enableRemote (Just u) c = do
 	inRepo $ Git.Command.run
 		[ Param "remote"
 		, Param "add"
@@ -144,7 +148,7 @@ gitSetup (Enable _) (Just u) _ c _ = do
 		, Param $ maybe (giveup "no location") fromProposedAccepted (M.lookup locationField c)
 		]
 	return (c, u)
-gitSetup (Enable _) Nothing _ _ _ = error "unable to enable git remote with no specified uuid"
+enableRemote Nothing _ = error "unable to enable git remote with no specified uuid"
 
 {- It's assumed to be cheap to read the config of non-URL remotes, so this is
  - done each time git-annex is run in a way that uses remotes, unless

Remote/GitLFS.hs

@@ -150,6 +150,7 @@ mySetup ss mu _ c gc = do
 	let failinitunlessforced msg = case ss of
 		Init -> unlessM (Annex.getState Annex.force) (giveup msg)
 		Enable _ -> noop
+		AutoEnable _ -> noop
 	case (isEncrypted pc, Git.GCrypt.urlPrefix `isPrefixOf` url) of
 		(False, False) -> noop
 		(True, True) -> Remote.GCrypt.setGcryptEncryption pc remotename

Remote/Glacier.hs

@@ -123,7 +123,7 @@ glacierSetup' ss u mcreds c gc = do
 	(c', encsetup) <- encryptionSetup (c `M.union` defaults) gc
 	pc <- either giveup return . parseRemoteConfig c'
 		=<< configParser remote c'
-	c'' <- setRemoteCredPair encsetup pc gc (AWS.creds u) mcreds
+	c'' <- setRemoteCredPair ss encsetup pc gc (AWS.creds u) mcreds
 	pc' <- either giveup return . parseRemoteConfig c''
 		=<< configParser remote c''
 	case ss of

Remote/Helper/ExportImport.hs

@@ -87,11 +87,8 @@ adjustExportImportRemoteType rt = rt { setup = setup' }
 						| configured pc && encryptionIsEnabled pc ->
 							giveup $ "cannot enable both encryption and " ++ fromProposedAccepted configfield
 						| otherwise -> cont
-					Enable oldc -> do
+					Enable oldc -> enable oldc pc configured configfield cont
-						oldpc <- parsedRemoteConfig rt oldc
+					AutoEnable oldc -> enable oldc pc configured configfield cont
-						if configured pc /= configured oldpc
-							then giveup $ "cannot change " ++ fromProposedAccepted configfield ++ " of existing special remote"
-							else cont
 				, if configured pc
 					then giveup $ fromProposedAccepted configfield ++ " is not supported by this special remote"
 					else cont
@@ -100,6 +97,12 @@ adjustExportImportRemoteType rt = rt { setup = setup' }
 			checkconfig importSupported importTree importTreeField $
 				setup rt st mu cp c gc
 	
+	enable oldc pc configured configfield cont = do
+		oldpc <- parsedRemoteConfig rt oldc
+		if configured pc /= configured oldpc
+			then giveup $ "cannot change " ++ fromProposedAccepted configfield ++ " of existing special remote"
+			else cont
+
 -- | Adjust a remote to support exporttree=yes and/or importree=yes.
 adjustExportImport :: Remote -> RemoteStateHandle -> Annex Remote
 adjustExportImport r rs = do

Remote/S3.hs

@@ -273,7 +273,7 @@ s3Setup'  ss u mcreds c gc
 		(c', encsetup) <- encryptionSetup (c `M.union` defaults) gc
 		pc <- either giveup return . parseRemoteConfig c'
 			=<< configParser remote c'
-		c'' <- setRemoteCredPair encsetup pc gc (AWS.creds u) mcreds
+		c'' <- setRemoteCredPair ss encsetup pc gc (AWS.creds u) mcreds
 		pc' <- either giveup return . parseRemoteConfig c''
 			=<< configParser remote c''
 		info <- extractS3Info pc'
@@ -287,14 +287,14 @@ s3Setup'  ss u mcreds c gc
 		showNote "Internet Archive mode"
 		pc <- either giveup return . parseRemoteConfig c
 			=<< configParser remote c
-		c' <- setRemoteCredPair noEncryptionUsed pc gc (AWS.creds u) mcreds
+		c' <- setRemoteCredPair ss noEncryptionUsed pc gc (AWS.creds u) mcreds
 		-- Ensure user enters a valid bucket name, since
 		-- this determines the name of the archive.org item.
 		let validbucket = replace " " "-" $ map toLower $
 			maybe (giveup "specify bucket=") fromProposedAccepted
 				(M.lookup bucketField c')
 		let archiveconfig = 
-			-- IA acdepts x-amz-* as an alias for x-archive-*
+			-- IA accepts x-amz-* as an alias for x-archive-*
 			M.mapKeys (Proposed . replace "x-archive-" "x-amz-" . fromProposedAccepted) $
 			-- encryption does not make sense here
 			M.insert encryptionField (Proposed "none") $
@@ -1273,11 +1273,8 @@ enableBucketVersioning ss info _ _ _ = do
 	case ss of
 		Init -> when (versioning info) $
 			enableversioning (bucket info)
-		Enable oldc -> do
+		Enable oldc -> checkunchanged oldc
-			oldpc <- parsedRemoteConfig remote oldc
+		AutoEnable oldc -> checkunchanged oldc
-			oldinfo <- extractS3Info oldpc
-			when (versioning info /= versioning oldinfo) $
-				giveup "Cannot change versioning= of existing S3 remote."
   where
 	enableversioning b = do
 #if MIN_VERSION_aws(0,21,1)
@@ -1296,6 +1293,12 @@ enableBucketVersioning ss info _ _ _ = do
 			]
 #endif
 
+	checkunchanged oldc = do
+		oldpc <- parsedRemoteConfig remote oldc
+		oldinfo <- extractS3Info oldpc
+		when (versioning info /= versioning oldinfo) $
+			giveup "Cannot change versioning= of existing S3 remote."
+
 -- If the remote has versioning enabled, but the version ID is for some
 -- reason not being recorded, it's not safe to perform an action that
 -- will remove the unversioned file. The file may be the only copy of an

Remote/WebDAV.hs

@@ -128,7 +128,7 @@ gen r u rc gc rs = do
 		chunkconfig = getChunkConfig c
 
 webdavSetup :: SetupStage -> Maybe UUID -> Maybe CredPair -> RemoteConfig -> RemoteGitConfig -> Annex (RemoteConfig, UUID)
-webdavSetup _ mu mcreds c gc = do
+webdavSetup ss mu mcreds c gc = do
 	u <- maybe (liftIO genUUID) return mu
 	url <- maybe (giveup "Specify url=")
 		(return . fromProposedAccepted)
@@ -138,7 +138,7 @@ webdavSetup _ mu mcreds c gc = do
 	creds <- maybe (getCreds pc gc u) (return . Just) mcreds
 	testDav url creds
 	gitConfigSpecialRemote u c' [("webdav", "true")]
-	c'' <- setRemoteCredPair encsetup pc gc (davCreds u) creds
+	c'' <- setRemoteCredPair ss encsetup pc gc (davCreds u) creds
 	return (c'', u)
 
 store :: DavHandleVar -> ChunkConfig -> Storer

Types/Remote.hs

@@ -48,7 +48,7 @@ import Utility.SafeCommand
 import Utility.Url
 import Utility.DataUnits
 
-data SetupStage = Init | Enable RemoteConfig
+data SetupStage = Init | Enable RemoteConfig | AutoEnable RemoteConfig
 
 {- There are different types of remotes. -}
 data RemoteTypeA a = RemoteType

doc/bugs/presence_of_AWS_creds_ruins_access_to_public_urls/comment_2_9e694fdee4a40581a86d21b962f99f25._comment

@@ -0,0 +1,16 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 2"""
+ date="2021-03-17T13:37:01Z"
+ content="""
+Ok, I've made autoenable not take creds from the environment, which will
+avoid the problem.
+
+If there are any external special remotes that might behave similarly,
+it would need an extension to the external special remote protocol to
+support them. Currently `INITREMOTE` is sent during auto-enable, and so
+the protocol would need to have `ENABLEREMOTE` and `AUTOENBLEREMOTE` added
+to it. Since that would need an extension and I don't know if any externals
+actually look at env vars etc at (auto)enable time, I've skipped doing it
+for now.
+"""]]

doc/special_remotes/S3.mdwn

@@ -8,12 +8,11 @@ for usage examples.
 ## configuration
 
 The standard environment variables `AWS_ACCESS_KEY_ID` and
-`AWS_SECRET_ACCESS_KEY` are used to supply login credentials
+`AWS_SECRET_ACCESS_KEY` are used to supply login credentials for S3. You
-for S3. You need to set these only when running
+need to set these only when running `git annex initremote` (or
-`git annex initremote`, as they will be cached in a file only you
+`enableremote`), as they will be cached in a file only you can read inside
-can read inside the local git repository. If you’re working with
+the local git repository. If you’re working with temporary security
-temporary security credentials, you can also set the `AWS_SESSION_TOKEN`
+credentials, you can also set the `AWS_SESSION_TOKEN` environment variable.
-environment variable.
 
 A number of parameters can be passed to `git annex initremote` to configure
 the S3 remote.

doc/special_remotes/glacier.mdwn

@@ -15,8 +15,8 @@ download the data.
 The standard environment variables `AWS_ACCESS_KEY_ID` and 
 `AWS_SECRET_ACCESS_KEY` are used to supply login credentials
 for Amazon. You need to set these only when running
-`git annex initremote`, as they will be cached in a file only you
+`git annex initremote` (or `enableremote`), as they will be cached in
-can read inside the local git repository.
+a file only you can read inside the local git repository.
 
 A number of parameters can be passed to `git annex initremote` to configure
 the Glacier remote.

doc/special_remotes/webdav.mdwn

@@ -3,9 +3,9 @@ This special remote type stores file contents in a WebDAV server.
 ## configuration
 
 The environment variables `WEBDAV_USERNAME` and `WEBDAV_PASSWORD` are used
-to supply login credentials. You need to set these only when running
+to supply login credentials. You need to set these only when running `git
-`git annex initremote`, as they will be cached in a file only you 
+annex initremote` (or `enableremote`), as they will be cached in a file
-can read inside the local git repository.
+only you can read inside the local git repository.
 
 A number of parameters can be passed to `git annex initremote` to configure
 the webdav remote.