forked from ungleich-public/cdist
		
	__file: make file uploading and attribute changes more atomic
Fixes ungleich-public/cdist#331 Signed-off-by: Steven Armstrong <steven@armstrong.cc>
This commit is contained in:
		
					parent
					
						
							
								bd44c023d3
							
						
					
				
			
			
				commit
				
					
						22039284f5
					
				
			
		
					 2 changed files with 33 additions and 9 deletions
				
			
		| 
						 | 
					@ -1,7 +1,7 @@
 | 
				
			||||||
#!/bin/sh -e
 | 
					#!/bin/sh -e
 | 
				
			||||||
#
 | 
					#
 | 
				
			||||||
# 2011-2012 Nico Schottelius (nico-cdist at schottelius.org)
 | 
					# 2011-2012 Nico Schottelius (nico-cdist at schottelius.org)
 | 
				
			||||||
# 2013 Steven Armstrong (steven-cdist armstrong.cc)
 | 
					# 2013-2022 Steven Armstrong (steven-cdist armstrong.cc)
 | 
				
			||||||
#
 | 
					#
 | 
				
			||||||
# This file is part of cdist.
 | 
					# This file is part of cdist.
 | 
				
			||||||
#
 | 
					#
 | 
				
			||||||
| 
						 | 
					@ -89,10 +89,26 @@ if [ "$state_should" = "present" ] || [ "$state_should" = "exists" ]; then
 | 
				
			||||||
      touch "$__object/files/set-attributes"
 | 
					      touch "$__object/files/set-attributes"
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      # upload file to temp location
 | 
					      # upload file to temp location
 | 
				
			||||||
      tempfile_template="${destination}.cdist.XXXXXXXXXX"
 | 
					      upload_destination="$(mktemp -u "${destination}.cdist.XXXXXXXXXX")"
 | 
				
			||||||
 | 
					      # Yes, we are aware that this is a race condition.
 | 
				
			||||||
 | 
					      # However:
 | 
				
			||||||
 | 
					      # a) cdist usually writes to directories that are not user writable
 | 
				
			||||||
 | 
					      #    (probably > 99.9%)
 | 
				
			||||||
 | 
					      # b) if they are user owned, the user / attacker always wins
 | 
				
			||||||
 | 
					      #    (probably < 0.1%)
 | 
				
			||||||
 | 
					      # c) the only case which we could improve are tmp directories and we
 | 
				
			||||||
 | 
					      #    don't think managing tmp directories with cdist is a typical case
 | 
				
			||||||
 | 
					      #    ("the rest %)"
 | 
				
			||||||
      cat << DONE
 | 
					      cat << DONE
 | 
				
			||||||
destination_upload="\$($__remote_exec $__target_host "mktemp $tempfile_template")"
 | 
					$__remote_exec $__target_host test -e $upload_destination && {
 | 
				
			||||||
 | 
					   echo "Refusing to upload file to existing destination: $upload_destination" >&2
 | 
				
			||||||
 | 
					   exit 1
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
DONE
 | 
					DONE
 | 
				
			||||||
 | 
					      # Tell gencode-remote to where we uploaded the file so it can move
 | 
				
			||||||
 | 
					      # it to its final destination.
 | 
				
			||||||
 | 
					      echo "$upload_destination" > "$__object/files/upload-destination"
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      if [ "$upload_file" ]; then
 | 
					      if [ "$upload_file" ]; then
 | 
				
			||||||
         echo upload >> "$__messages_out"
 | 
					         echo upload >> "$__messages_out"
 | 
				
			||||||
         # IPv6 fix
 | 
					         # IPv6 fix
 | 
				
			||||||
| 
						 | 
					@ -103,12 +119,8 @@ DONE
 | 
				
			||||||
             my_target_host="${__target_host}"
 | 
					             my_target_host="${__target_host}"
 | 
				
			||||||
         fi
 | 
					         fi
 | 
				
			||||||
         cat << DONE
 | 
					         cat << DONE
 | 
				
			||||||
$__remote_copy "$source" "${my_target_host}:\$destination_upload"
 | 
					$__remote_copy "$source" "${my_target_host}:${upload_destination}"
 | 
				
			||||||
DONE
 | 
					DONE
 | 
				
			||||||
      fi
 | 
					      fi
 | 
				
			||||||
# move uploaded file into place
 | 
					 | 
				
			||||||
cat << DONE
 | 
					 | 
				
			||||||
$__remote_exec $__target_host "rm -rf \"$destination\"; mv \"\$destination_upload\" \"$destination\""
 | 
					 | 
				
			||||||
DONE
 | 
					 | 
				
			||||||
   fi
 | 
					   fi
 | 
				
			||||||
fi
 | 
					fi
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -1,7 +1,7 @@
 | 
				
			||||||
#!/bin/sh -e
 | 
					#!/bin/sh -e
 | 
				
			||||||
#
 | 
					#
 | 
				
			||||||
# 2011-2013 Nico Schottelius (nico-cdist at schottelius.org)
 | 
					# 2011-2013 Nico Schottelius (nico-cdist at schottelius.org)
 | 
				
			||||||
# 2013 Steven Armstrong (steven-cdist armstrong.cc)
 | 
					# 2013-2022 Steven Armstrong (steven-cdist armstrong.cc)
 | 
				
			||||||
#
 | 
					#
 | 
				
			||||||
# This file is part of cdist.
 | 
					# This file is part of cdist.
 | 
				
			||||||
#
 | 
					#
 | 
				
			||||||
| 
						 | 
					@ -62,6 +62,13 @@ set_mode() {
 | 
				
			||||||
 | 
					
 | 
				
			||||||
case "$state_should" in
 | 
					case "$state_should" in
 | 
				
			||||||
    present|exists)
 | 
					    present|exists)
 | 
				
			||||||
 | 
					        if [ -f "$__object/files/upload-destination" ]; then
 | 
				
			||||||
 | 
					            final_destination="$destination"
 | 
				
			||||||
 | 
					            # We change the 'global' $destination variable here so we can
 | 
				
			||||||
 | 
					            # change attributes of the new/uploaded file before moving it
 | 
				
			||||||
 | 
					            # to it's final destination.
 | 
				
			||||||
 | 
					            destination="$(cat "$__object/files/upload-destination")"
 | 
				
			||||||
 | 
					        fi
 | 
				
			||||||
        # Note: Mode - needs to happen last as a chown/chgrp can alter mode by
 | 
					        # Note: Mode - needs to happen last as a chown/chgrp can alter mode by
 | 
				
			||||||
        #  clearing S_ISUID and S_ISGID bits (see chown(2))
 | 
					        #  clearing S_ISUID and S_ISGID bits (see chown(2))
 | 
				
			||||||
        for attribute in group owner mode; do
 | 
					        for attribute in group owner mode; do
 | 
				
			||||||
| 
						 | 
					@ -81,6 +88,11 @@ case "$state_should" in
 | 
				
			||||||
                fi
 | 
					                fi
 | 
				
			||||||
            fi
 | 
					            fi
 | 
				
			||||||
        done
 | 
					        done
 | 
				
			||||||
 | 
					        if [ -f "$__object/files/upload-destination" ]; then
 | 
				
			||||||
 | 
					            # move uploaded file into place
 | 
				
			||||||
 | 
					            printf 'rm -rf "%s"\n' "$final_destination"
 | 
				
			||||||
 | 
					            printf 'mv -T "%s" "%s"\n' "$destination" "$final_destination"
 | 
				
			||||||
 | 
					        fi
 | 
				
			||||||
        if [ -f "$__object/files/set-attributes" ]; then
 | 
					        if [ -f "$__object/files/set-attributes" ]; then
 | 
				
			||||||
            # set-attributes is created if file is created or uploaded in gencode-local
 | 
					            # set-attributes is created if file is created or uploaded in gencode-local
 | 
				
			||||||
            fire_onchange=1
 | 
					            fire_onchange=1
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue