Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Improvements to install/uninstall process #32

Merged
merged 1 commit into from
Oct 7, 2024
Merged

Improvements to install/uninstall process #32

merged 1 commit into from
Oct 7, 2024

Conversation

jdrodjpl
Copy link
Collaborator

@jdrodjpl jdrodjpl commented Oct 7, 2024

No description provided.

@jdrodjpl jdrodjpl requested a review from galenatjpl October 7, 2024 23:48
@jdrodjpl jdrodjpl merged commit 04f874a into main Oct 7, 2024
1 of 2 checks passed
@jdrodjpl jdrodjpl deleted the install_redo branch October 7, 2024 23:49
}

// Open the log file in append mode
file, err := os.OpenFile(logfile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix AI 4 months ago

To fix the problem, we need to validate the logfile parameter to ensure it does not contain any path traversal sequences or invalid characters. We can achieve this by checking that the resolved path is within a specific directory that is considered safe. This involves resolving the input with respect to that directory and then checking that the resulting path is still within it.

  1. Validate the logfile parameter to ensure it does not contain any path traversal sequences or invalid characters.
  2. Use filepath.Abs to get the absolute path and ensure it starts with the intended directory.
  3. Update the RunTerraformLogOutToFile function to include this validation.
Suggested changeset 1
backend/internal/terraform/runner.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/internal/terraform/runner.go b/backend/internal/terraform/runner.go
--- a/backend/internal/terraform/runner.go
+++ b/backend/internal/terraform/runner.go
@@ -207,4 +207,11 @@
 
+	// Validate logfile path
+	logDir := filepath.Join(appconf.Workdir, "install_logs")
+	absLogfile, err := filepath.Abs(logfile)
+	if err != nil || !strings.HasPrefix(absLogfile, logDir) {
+		return fmt.Errorf("invalid logfile path: %s", logfile)
+	}
+
 	// Open the log file in append mode
-	file, err := os.OpenFile(logfile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+	file, err := os.OpenFile(absLogfile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
 	if err != nil {
EOF
@@ -207,4 +207,11 @@

// Validate logfile path
logDir := filepath.Join(appconf.Workdir, "install_logs")
absLogfile, err := filepath.Abs(logfile)
if err != nil || !strings.HasPrefix(absLogfile, logDir) {
return fmt.Errorf("invalid logfile path: %s", logfile)
}

// Open the log file in append mode
file, err := os.OpenFile(logfile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
file, err := os.OpenFile(absLogfile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
if err != nil {
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
}

// Read the log file
content, err := os.ReadFile(logfile)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
This path depends on a
user-provided value
.

Copilot Autofix AI 4 months ago

To fix the problem, we need to ensure that the appName parameter does not contain any path traversal characters or sequences. This can be achieved by validating the appName parameter to ensure it does not contain any path separators ("/" or "\") or ".." sequences. If the validation fails, we should return an error response.

Steps to fix:

  1. Validate the appName parameter to ensure it does not contain any path separators or ".." sequences.
  2. If the validation fails, return an HTTP error response.
  3. Proceed with the file path construction and file read operation only if the validation passes.
Suggested changeset 1
backend/internal/web/api.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/internal/web/api.go b/backend/internal/web/api.go
--- a/backend/internal/web/api.go
+++ b/backend/internal/web/api.go
@@ -122,2 +122,8 @@
 
+		// Validate appName to prevent path traversal
+		if strings.Contains(appName, "/") || strings.Contains(appName, "\\") || strings.Contains(appName, "..") {
+			http.Error(c.Writer, "Invalid application name", http.StatusBadRequest)
+			return
+		}
+
 		// deploymentID, err := db.FetchDeploymentIDByApplicationName(deploymentName)
EOF
@@ -122,2 +122,8 @@

// Validate appName to prevent path traversal
if strings.Contains(appName, "/") || strings.Contains(appName, "\\") || strings.Contains(appName, "..") {
http.Error(c.Writer, "Invalid application name", http.StatusBadRequest)
return
}

// deploymentID, err := db.FetchDeploymentIDByApplicationName(deploymentName)
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant