Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Checkout to Commit #234

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add Checkout to Commit #234

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa10513
add checkout to commit
hateofhades Dec 26, 2024
ac25be4
clippy
hateofhades Dec 26, 2024
6d93136
add commands
hateofhades Dec 26, 2024
468eeba
Add checkout
hateofhades Dec 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src-tauri/src/git/git_commit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
use super::{
git_commit_author::{GitCommitAuthor, GitCommitAuthorType},
git_files::GitFilesRequired,
git_folders::{GitFolders, GIT_FOLDER},
git_project::GitProject,
git_tree::GitTree,
object::{GitObject, Header},
};
use crate::errors::git_object_error::{CommitError, GitObjectError};
use core::fmt;
use serde::{Deserialize, Serialize};
use std::{
fs::{self, OpenOptions},
io::Write,
path::{Path, PathBuf},
};

pub enum CommitPrefix {
Tree,
Expand Down Expand Up @@ -149,6 +157,35 @@

Ok(history)
}

/**
* Checkout all the files in a commit
*/
pub fn checkout(&self, project: &GitProject) -> Result<(), GitObjectError> {
let files =
GitTree::from_hash(project, &self.get_tree_hash())?.get_object_blobs(project, None);

let _ = files.iter().for_each(|file| {
let path = PathBuf::from(project.get_directory()).join(&file.0);
let file_in_fs = OpenOptions::new().write(true).create(true).open(path);

Check warning on line 170 in src-tauri/src/git/git_commit.rs

View check run for this annotation

Codecov / codecov/patch

src-tauri/src/git/git_commit.rs#L164-L170 

Added lines #L164 - L170 were not covered by tests

if let Ok(mut file_in_fs) = file_in_fs {
let _ = file_in_fs.write_all(file.1.data());
}
});

let head_path = PathBuf::from(project.get_directory())
.join(GIT_FOLDER)
.join(GitFilesRequired::HEAD.as_ref());
OpenOptions::new()
.write(true)
.open(head_path)
.map_err(|_| GitObjectError::InvalidCommitFile(CommitError::InvalidContent))?
.write_all(self.get_hash().as_bytes())
.map_err(|_| GitObjectError::InvalidHash)?;

Check warning on line 185 in src-tauri/src/git/git_commit.rs

View check run for this annotation

Codecov / codecov/patch

src-tauri/src/git/git_commit.rs#L172-L185 

Added lines #L172 - L185 were not covered by tests

Ok(())
}

Check warning on line 188 in src-tauri/src/git/git_commit.rs

View check run for this annotation

Codecov / codecov/patch

src-tauri/src/git/git_commit.rs#L187-L188 

Added lines #L187 - L188 were not covered by tests
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Add error handling and security measures to checkout implementation.

The current implementation has several critical issues that need to be addressed:

  1. File operations silently ignore errors using let _
  2. Missing directory creation for nested files
  3. No cleanup mechanism on partial failure
  4. Potential path traversal vulnerability

Here's a suggested implementation with proper error handling and security measures:

 pub fn checkout(&self, project: &GitProject) -> Result<(), GitObjectError> {
     let files =
         GitTree::from_hash(project, &self.get_tree_hash())?.get_object_blobs(project, None);
 
-    let _ = files.iter().for_each(|file| {
-        let path = PathBuf::from(project.get_directory()).join(&file.0);
-        let file_in_fs = OpenOptions::new().write(true).create(true).open(path);
-
-        if let Ok(mut file_in_fs) = file_in_fs {
-            let _ = file_in_fs.write_all(file.1.data());
-        }
-    });
+    // Track created files for cleanup on failure
+    let mut created_files = Vec::new();
+    
+    // Result to track any failures during file operations
+    let result: Result<(), GitObjectError> = files.iter().try_for_each(|file| {
+        let path = PathBuf::from(project.get_directory()).join(&file.0);
+        
+        // Validate path is within project directory
+        if !path.starts_with(project.get_directory()) {
+            return Err(GitObjectError::InvalidPath);
+        }
+        
+        // Create parent directories
+        if let Some(parent) = path.parent() {
+            fs::create_dir_all(parent).map_err(|_| GitObjectError::FileSystemError)?;
+        }
+        
+        // Create and write file
+        let mut file_in_fs = OpenOptions::new()
+            .write(true)
+            .create(true)
+            .truncate(true)
+            .open(&path)
+            .map_err(|_| GitObjectError::FileSystemError)?;
+            
+        file_in_fs.write_all(file.1.data())
+            .map_err(|_| GitObjectError::FileSystemError)?;
+            
+        created_files.push(path);
+        Ok(())
+    });
+    
+    // If any operation failed, cleanup created files
+    if result.is_err() {
+        for path in created_files {
+            let _ = fs::remove_file(path);
+        }
+        return result;
+    }

     let head_path = PathBuf::from(project.get_directory())
         .join(GIT_FOLDER)
         .join(GitFilesRequired::HEAD.as_ref());
     OpenOptions::new()
         .write(true)
         .open(head_path)
         .map_err(|_| GitObjectError::InvalidCommitFile(CommitError::InvalidContent))?
         .write_all(self.get_hash().as_bytes())
         .map_err(|_| GitObjectError::InvalidHash)?;

     Ok(())
 }

Committable suggestion skipped: line range outside the PR's diff.

💡 Codebase verification

File safety checks needed in checkout implementation

Based on the codebase analysis, the current implementation lacks important safety checks before overwriting files. The error handling is focused on git-specific errors (GitObjectError, CommitError) but doesn't handle file system conflicts. Specifically:

  • No existence check before creating/overwriting files
  • No error propagation for file write failures (using let _ to ignore results)
  • No backup mechanism for existing files

Key locations that need attention:

  • git_commit.rs:164-188: Add file existence checks before OpenOptions::new()
  • git_commit.rs:169: Propagate file write errors instead of ignoring them with let _
  • Consider adding a new error variant in GitObjectError for file system conflicts
🔗 Analysis chain

Verify: Check for potential file conflicts.

The current implementation might overwrite existing files without warning. Consider:

  1. Adding a dry-run mode
  2. Detecting and handling conflicts
  3. Providing a way to backup existing files

Let's verify if there are any similar implementations in the codebase:

Let me try a different search strategy to verify this concern.

Let me try one final search to check for any file existence checks or error handling patterns.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for similar checkout or file writing implementations
ast-grep --pattern 'fn $name($$$) {
  $$$
  OpenOptions::new().$$$
  $$$
}'

# Search for conflict handling patterns
rg -A 5 'backup|conflict|overwrite'

Length of output: 115

Script:

#!/bin/bash
# Search for file operations in git-related files
fd -e rs git_ --exec rg -A 5 'OpenOptions::new\(\)'

# Search for any existing conflict handling
fd -e rs git_ --exec rg -A 5 'conflict|exists|backup'

# Look for other checkout implementations
ast-grep --pattern 'fn checkout($$$) {
  $$$
}'

Length of output: 908

Script:

#!/bin/bash
# Check for file existence verifications
fd -e rs git_ --exec rg -B 5 -A 5 'Path::new|PathBuf::from.*exists'

# Look for error handling related to files
fd -e rs git_ --exec rg -B 2 -A 2 'GitObjectError::|GitError::'

# Check the error types definition
rg -B 2 -A 2 'enum (GitObjectError|GitError|CommitError)'

Length of output: 7415

}

impl GitObject for GitCommit {
Expand Down
Loading