From 361fdcce1b14769d652421a064b198145a4122cc Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Wed, 24 May 2023 17:07:46 -0700 Subject: [PATCH] Maintain file permissions when renaming Temporary and autosave files do not neccessarily have the correct permissions set to replace existing project files. This updates the permissions to match the existing values where possible Fixes https://gitlab.com/kicad/code/kicad/-/issues/13574 (cherry picked from commit 48ecd742eb86d74049c6d14f54598e3e48fbb49d) --- common/eda_base_frame.cpp | 4 +++ eeschema/files-io.cpp | 4 +++ kicad/project_tree_item.cpp | 1 + libs/kiplatform/gtk/io.cpp | 27 +++++++++++++++- libs/kiplatform/include/kiplatform/io.h | 9 +++++- libs/kiplatform/msw/io.cpp | 40 +++++++++++++++++++++++- libs/kiplatform/osx/io.mm | 36 +++++++++++++++++++++ pagelayout_editor/files.cpp | 7 ++++- pcbnew/exporters/step/step_pcb_model.cpp | 7 ++++- pcbnew/files.cpp | 7 +++++ pcbnew/footprint_info_impl.cpp | 5 +++ pcbnew/plugins/kicad/pcb_plugin.cpp | 5 +++ 12 files changed, 147 insertions(+), 5 deletions(-) diff --git a/common/eda_base_frame.cpp b/common/eda_base_frame.cpp index e84c32ecaa..129d658088 100644 --- a/common/eda_base_frame.cpp +++ b/common/eda_base_frame.cpp @@ -63,6 +63,7 @@ #include #include #include +#include #include #include @@ -1303,6 +1304,9 @@ void EDA_BASE_FRAME::CheckForAutoSaveFile( const wxFileName& aFileName ) // the file name. if( response == wxYES ) { + // Preserve the permissions of the current file + KIPLATFORM::IO::DuplicatePermissions( aFileName.GetFullPath(), autoSaveFileName.GetFullPath() ); + if( !wxRenameFile( autoSaveFileName.GetFullPath(), aFileName.GetFullPath() ) ) { wxMessageBox( _( "The auto save file could not be renamed to the board file name." ), diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp index 9e56acbaeb..d846abc1ea 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -69,6 +69,8 @@ #include #include +#include + #if wxCHECK_VERSION( 3, 1, 7 ) #include "widgets/filedlg_hook_save_project.h" #else @@ -727,6 +729,8 @@ bool SCH_EDIT_FRAME::saveSchematicFile( SCH_SHEET* aSheet, const wxString& aSave if( success ) { + // Preserve the permissions of the current file + KIPLATFORM::IO::DuplicatePermissions( schematicFileName.GetFullPath(), tempFile ); // Replace the original with the temporary file we just wrote success = wxRenameFile( tempFile, schematicFileName.GetFullPath() ); diff --git a/kicad/project_tree_item.cpp b/kicad/project_tree_item.cpp index fdf1500dcf..f29b42d39b 100644 --- a/kicad/project_tree_item.cpp +++ b/kicad/project_tree_item.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include diff --git a/libs/kiplatform/gtk/io.cpp b/libs/kiplatform/gtk/io.cpp index 9950ee32b3..47cc4a5ab0 100644 --- a/libs/kiplatform/gtk/io.cpp +++ b/libs/kiplatform/gtk/io.cpp @@ -23,6 +23,8 @@ #include #include +#include +#include FILE* KIPLATFORM::IO::SeqFOpen( const wxString& aPath, const wxString& aMode ) { @@ -31,4 +33,27 @@ FILE* KIPLATFORM::IO::SeqFOpen( const wxString& aPath, const wxString& aMode ) posix_fadvise( fileno( fp ), 0, 0, POSIX_FADV_SEQUENTIAL ); return fp; -} \ No newline at end of file +} + +bool KIPLATFORM::IO::DuplicatePermissions( const wxString &aSrc, const wxString &aDest ) +{ + struct stat sourceStat; + if( stat( aSrc.fn_str(), &sourceStat ) == 0 ) + { + mode_t permissions = sourceStat.st_mode & ( S_IRWXU | S_IRWXG | S_IRWXO ); + if( chmod( aDest.fn_str(), permissions ) == 0 ) + { + return true; + } + else + { + // Handle error + return false; + } + } + else + { + // Handle error + return false; + } +} diff --git a/libs/kiplatform/include/kiplatform/io.h b/libs/kiplatform/include/kiplatform/io.h index 23993a812a..846bb4b9c4 100644 --- a/libs/kiplatform/include/kiplatform/io.h +++ b/libs/kiplatform/include/kiplatform/io.h @@ -37,7 +37,14 @@ namespace IO * to say linux and it's posix_fadvise */ FILE* SeqFOpen( const wxString& aPath, const wxString& mode ); + + /** + * Duplicates the file security data from one file to another ensuring that they are + * the same between both. This assumes that the user has permission to set #aDest + * @return true if the process was successful + */ + bool DuplicatePermissions( const wxString& aSrc, const wxString& aDest ); } // namespace IO } // namespace KIPLATFORM -#endif // KIPLATFORM_IO_H_ \ No newline at end of file +#endif // KIPLATFORM_IO_H_ diff --git a/libs/kiplatform/msw/io.cpp b/libs/kiplatform/msw/io.cpp index b91f5c61ec..4ec06b2161 100644 --- a/libs/kiplatform/msw/io.cpp +++ b/libs/kiplatform/msw/io.cpp @@ -68,4 +68,42 @@ FILE* KIPLATFORM::IO::SeqFOpen( const wxString& aPath, const wxString& aMode ) // Fallback for MSYS2 return wxFopen( aPath, aMode ); #endif -} \ No newline at end of file +} + +bool KIPLATFORM::IO::DuplicatePermissions( const wxString &aSrc, const wxString &aDest ) +{ + bool retval = false; + PSECURITY_DESCRIPTOR pSD = nullptr; + DWORD dwSize = 0; + + // Retrieve the security descriptor from the source file + if( GetFileSecurity( sourceFilePath.wc_str(), + OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION, + NULL, 0, &dwSize ) ) + { + pSD = static_cast( new BYTE[dwSize] ); + + if( !pSD ) + return false; + + if( !GetFileSecurity( sourceFilePath.wc_str(), + OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION + | DACL_SECURITY_INFORMATION, pSD, dwSize, &dwSize ) ) + { + delete[] pSD; + return false; + } + + // Assign the retrieved security descriptor to the destination file + if( !SetFileSecurity( destFilePath.wc_str(), + OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION + | DACL_SECURITY_INFORMATION, pSD ) ) + { + retval = false; + } + + delete[] pSD; + } + + return retval; +} diff --git a/libs/kiplatform/osx/io.mm b/libs/kiplatform/osx/io.mm index 0be0791c82..3e24ade364 100644 --- a/libs/kiplatform/osx/io.mm +++ b/libs/kiplatform/osx/io.mm @@ -19,10 +19,46 @@ #include +#import + #include #include FILE* KIPLATFORM::IO::SeqFOpen( const wxString& aPath, const wxString& aMode ) { return wxFopen( aPath, aMode ); +} + +bool AssignPermissions(const wxString& sourceFilePath, const wxString& destFilePath) +{ + NSString *sourcePath = [NSString stringWithUTF8String:sourceFilePath.utf8_str()]; + NSString *destPath = [NSString stringWithUTF8String:destFilePath.utf8_str()]; + + NSFileManager *fileManager = [NSFileManager defaultManager]; + + NSError *error; + NSDictionary *sourceAttributes = [fileManager attributesOfItemAtPath:sourcePath error:&error]; + + if( !sourceAttributes ) + { + NSLog(@"Error retrieving source file attributes: %@", error); + return false; + } + + NSNumber *permissions = sourceAttributes[NSFilePosixPermissions]; + + if (permissions == nil) + { + return false; + } + + if ([fileManager setAttributes:@{NSFilePosixPermissions: permissions} ofItemAtPath:destPath error:&error]) + { + return true; + } + else + { + NSLog(@"Error assigning permissions: %@", error); + return false; + } } \ No newline at end of file diff --git a/pagelayout_editor/files.cpp b/pagelayout_editor/files.cpp index 0103aa60f4..8a5291451e 100644 --- a/pagelayout_editor/files.cpp +++ b/pagelayout_editor/files.cpp @@ -32,6 +32,8 @@ #include #include +#include + #include "pl_editor_frame.h" #include "pl_editor_id.h" #include "properties_frame.h" @@ -307,6 +309,9 @@ bool PL_EDITOR_FRAME::SaveDrawingSheetFile( const wxString& aFullFileName ) return false; } + // Preserve the permissions of the current file + KIPLATFORM::IO::DuplicatePermissions( aFullFileName, tempFile ); + if( !wxRenameFile( tempFile, aFullFileName ) ) return false; @@ -325,4 +330,4 @@ void PL_EDITOR_FRAME::DoWithAcceptedFiles() { for( const wxFileName& file : m_AcceptedFiles ) OpenProjectFiles( { file.GetFullPath() }, KICTL_CREATE ); -} \ No newline at end of file +} diff --git a/pcbnew/exporters/step/step_pcb_model.cpp b/pcbnew/exporters/step/step_pcb_model.cpp index 0a89efff2e..cec2b37e81 100644 --- a/pcbnew/exporters/step/step_pcb_model.cpp +++ b/pcbnew/exporters/step/step_pcb_model.cpp @@ -38,6 +38,7 @@ #include #include +#include #include "step_pcb_model.h" #include "streamwrapper.h" @@ -733,6 +734,10 @@ bool STEP_PCB_MODEL::WriteSTEP( const wxString& aFileName ) if( success ) { + + // Preserve the permissions of the current file + KIPLATFORM::IO::DuplicatePermissions( fn.GetFullPath(), tmpfname ); + if( !wxRenameFile( tmpfname, fn.GetFullName(), true ) ) { ReportMessage( wxString::Format( wxT( "Cannot rename temporary file '%s' to '%s'.\n" ), @@ -1226,4 +1231,4 @@ TDF_Label STEP_PCB_MODEL::transferModel( Handle( TDocStd_Document )& source, }; return component; -} \ No newline at end of file +} diff --git a/pcbnew/files.cpp b/pcbnew/files.cpp index eed94946e6..427daffcdd 100644 --- a/pcbnew/files.cpp +++ b/pcbnew/files.cpp @@ -23,6 +23,8 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ +#include + #include #include #include @@ -63,6 +65,8 @@ #include "zone_filler.h" #include // For ::ResolvePossibleSymlinks() +#include + #include #include #include @@ -1101,6 +1105,9 @@ bool PCB_EDIT_FRAME::SavePcbFile( const wxString& aFileName, bool addToHistory, return false; } + // Preserve the permissions of the current file + KIPLATFORM::IO::DuplicatePermissions( pcbFileName.GetFullPath(), tempFile ); + // If save succeeded, replace the original with what we just wrote if( !wxRenameFile( tempFile, pcbFileName.GetFullPath() ) ) { diff --git a/pcbnew/footprint_info_impl.cpp b/pcbnew/footprint_info_impl.cpp index ddabcd5ae5..a375bccb4b 100644 --- a/pcbnew/footprint_info_impl.cpp +++ b/pcbnew/footprint_info_impl.cpp @@ -34,6 +34,8 @@ #include #include +#include + #include #include #include @@ -337,6 +339,9 @@ void FOOTPRINT_LIST_IMPL::WriteCacheToFile( const wxString& aFilePath ) txtStream.Flush(); outStream.Close(); + // Preserve the permissions of the current file + KIPLATFORM::IO::DuplicatePermissions( aFilePath, tmpFileName.GetFullPath() ); + if( !wxRenameFile( tmpFileName.GetFullPath(), aFilePath, true ) ) { // cleanup in case rename failed diff --git a/pcbnew/plugins/kicad/pcb_plugin.cpp b/pcbnew/plugins/kicad/pcb_plugin.cpp index fc13df542b..004a0e76f3 100644 --- a/pcbnew/plugins/kicad/pcb_plugin.cpp +++ b/pcbnew/plugins/kicad/pcb_plugin.cpp @@ -57,6 +57,8 @@ #include #include +#include + // For some reason wxWidgets is built with wxUSE_BASE64 unset so expose the wxWidgets // base64 code. Needed for PCB_BITMAP #define wxUSE_BASE64 1 @@ -131,6 +133,9 @@ void FP_CACHE::Save( FOOTPRINT* aFootprint ) // and it is fully inexplicable. See if this dodges the error. wxMilliSleep( 250L ); + // Preserve the permissions of the current file + KIPLATFORM::IO::DuplicatePermissions( fn.GetFullPath(), tempFileName ); + if( !wxRenameFile( tempFileName, fn.GetFullPath() ) ) { wxString msg = wxString::Format( _( "Cannot rename temporary file '%s' to '%s'" ),