Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions api/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ func Route(
runnersAPI.Path("").HandlerFunc(runnerController.UpdateRunner).Methods("PUT")
runnersAPI.Path("").HandlerFunc(runners.UnregisterRunner).Methods("DELETE")

// Repository archive endpoint for proxy git client
repositoriesAPI := internalAPI.PathPrefix("/repositories").Subrouter()
repositoriesAPI.Use(runners.RunnerMiddleware)
repositoriesAPI.HandleFunc("/archive", runners.GetRepositoryArchive).Methods("POST")

publicWebHookRouter := r.PathPrefix(webPath + "api").Subrouter()
publicWebHookRouter.Use(StoreMiddleware, JSONMiddleware)
publicWebHookRouter.Path("/integrations/{integration_alias}").HandlerFunc(
Expand Down
263 changes: 263 additions & 0 deletions api/runners/runners.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
package runners

import (
"archive/tar"
"bytes"
"compress/gzip"
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"encoding/json"
"encoding/pem"
"fmt"
"io"
"net/http"
"os"
"os/exec"
"path/filepath"
"time"

"github.com/semaphoreui/semaphore/api/helpers"
"github.com/semaphoreui/semaphore/db"
"github.com/semaphoreui/semaphore/db_lib"
"github.com/semaphoreui/semaphore/pkg/ssh"
"github.com/semaphoreui/semaphore/pkg/task_logger"
"github.com/semaphoreui/semaphore/services/runners"
"github.com/semaphoreui/semaphore/services/server"
Expand Down Expand Up @@ -347,3 +356,257 @@

w.WriteHeader(http.StatusNoContent)
}

type RepositoryRequest struct {
GitURL string `json:"git_url" binding:"required"`
GitBranch string `json:"git_branch" binding:"required"`
SSHKeyID *int `json:"ssh_key_id,omitempty"`
}

type RepositoryResponse struct {
Hash string `json:"hash"`
Message string `json:"message"`
Archive []byte `json:"archive"`
}

func GetRepositoryArchive(w http.ResponseWriter, r *http.Request) {
runner := helpers.GetFromContext(r, "runner").(db.Runner)

var req RepositoryRequest
if !helpers.Bind(w, r, &req) {
return
}

store := helpers.Store(r)

// Create a temporary repository record for cloning
tempRepo := db.Repository{
GitURL: req.GitURL,
GitBranch: req.GitBranch,
}

// Set SSH key if provided
if req.SSHKeyID != nil && runner.ProjectID != nil {
accessKey, err := store.GetAccessKey(*runner.ProjectID, *req.SSHKeyID)
if err != nil {
helpers.WriteErrorStatus(w, "Access key not found", http.StatusNotFound)
return
}
tempRepo.SSHKeyID = *req.SSHKeyID
tempRepo.SSHKey = accessKey
}

// Create a temporary directory for cloning
tempDir, err := os.MkdirTemp("", "semaphore-repo-*")
if err != nil {
log.WithError(err).Error("Failed to create temp directory")
helpers.WriteErrorStatus(w, "Internal server error", http.StatusInternalServerError)
return
}
defer os.RemoveAll(tempDir)

// Create a simple logger for the git operations
logger := &simpleLogger{}

// Create git repository with the appropriate git client (but not proxy to avoid recursion)
var gitClient db_lib.GitClient
switch util.Config.GitClientId {
case util.GoGitClientId:
gitClient = db_lib.CreateGoGitClient(&simpleKeyInstaller{})
default:
gitClient = db_lib.CreateCmdGitClient(&simpleKeyInstaller{})
}

gitRepo := db_lib.GitRepository{
Repository: tempRepo,
Logger: logger,
Client: gitClient,
}

// Create a custom GitRepository that returns our temp directory
customGitRepo := customGitRepository{
GitRepository: gitRepo,
customPath: tempDir,
}

// Clone the repository
err = customGitRepo.Clone()
if err != nil {
log.WithError(err).WithFields(log.Fields{

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by an access to Password
flows to a logging call.
Sensitive data returned by an access to Password
flows to a logging call.

Copilot Autofix

AI about 2 months ago

The best way to fix this problem is to ensure that any data containing sensitive fields—such as a Git URL that may have username:password@hostname—is either omitted, obfuscated, or carefully redacted before being logged. Specifically, when logging errors related to repository operations, we should avoid logging sensitive values, or sanitize the error and associated fields before logging.

In this case, the problematic log line is:

log.WithError(err).WithFields(log.Fields{
	"git_url":    req.GitURL,
	"git_branch": req.GitBranch,
}).Error("Failed to clone repository")

The err itself may include the sensitive password, and req.GitURL is known-safe (doesn't include credentials), but downstream errors (originating from e.g., constructed clone URLs with credentials) could inject sensitive data.

To fix:

  • Sanitize err.Error() before logging:
    • Redact credentials from any URLs embedded in the error string prior to logging.
  • Alternatively, log only a generic error message without the actual error details, or explicitly check and redact known patterns.

Implementation:

  • Introduce a helper to sanitize sensitive data from error messages.
  • Before logging, pass the error through this helper, and log only the sanitized error message.
  • This should be applied to the affected log statement in api/runners/runners.go.

Suggested changeset 1
api/runners/runners.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/api/runners/runners.go b/api/runners/runners.go
--- a/api/runners/runners.go
+++ b/api/runners/runners.go
@@ -27,8 +27,19 @@
 	"github.com/semaphoreui/semaphore/services/tasks"
 	"github.com/semaphoreui/semaphore/util"
 	log "github.com/sirupsen/logrus"
+	"regexp"
 )
 
+
+// sanitizeSensitiveError redacts credentials in known URL patterns from error messages
+func sanitizeSensitiveError(msg string) string {
+	// This regex will replace username:password@ in URLs with username:***@
+	// Covers http/https/git URLs in the error string
+	// e.g., https://user:[email protected] -> https://user:***@github.com
+	re := regexp.MustCompile(`(https?://)([^:/\s]+):([^@/\s]+)@`)
+	return re.ReplaceAllString(msg, "${1}${2}:***@")
+}
+
 func RunnerMiddleware(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 
@@ -432,11 +441,13 @@
 	// Clone the repository
 	err = customGitRepo.Clone()
 	if err != nil {
-		log.WithError(err).WithFields(log.Fields{
+		// Sanitize sensitive data from err before logging
+		safeErrMsg := sanitizeSensitiveError(err.Error())
+		log.WithFields(log.Fields{
 			"git_url":    req.GitURL,
 			"git_branch": req.GitBranch,
-		}).Error("Failed to clone repository")
-		helpers.WriteErrorStatus(w, "Failed to clone repository: "+err.Error(), http.StatusBadRequest)
+		}).Error("Failed to clone repository: " + safeErrMsg)
+		helpers.WriteErrorStatus(w, "Failed to clone repository: "+safeErrMsg, http.StatusBadRequest)
 		return
 	}
 
EOF
@@ -27,8 +27,19 @@
"github.com/semaphoreui/semaphore/services/tasks"
"github.com/semaphoreui/semaphore/util"
log "github.com/sirupsen/logrus"
"regexp"
)


// sanitizeSensitiveError redacts credentials in known URL patterns from error messages
func sanitizeSensitiveError(msg string) string {
// This regex will replace username:password@ in URLs with username:***@
// Covers http/https/git URLs in the error string
// e.g., https://user:[email protected] -> https://user:***@github.com
re := regexp.MustCompile(`(https?://)([^:/\s]+):([^@/\s]+)@`)
return re.ReplaceAllString(msg, "${1}${2}:***@")
}

func RunnerMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

@@ -432,11 +441,13 @@
// Clone the repository
err = customGitRepo.Clone()
if err != nil {
log.WithError(err).WithFields(log.Fields{
// Sanitize sensitive data from err before logging
safeErrMsg := sanitizeSensitiveError(err.Error())
log.WithFields(log.Fields{
"git_url": req.GitURL,
"git_branch": req.GitBranch,
}).Error("Failed to clone repository")
helpers.WriteErrorStatus(w, "Failed to clone repository: "+err.Error(), http.StatusBadRequest)
}).Error("Failed to clone repository: " + safeErrMsg)
helpers.WriteErrorStatus(w, "Failed to clone repository: "+safeErrMsg, http.StatusBadRequest)
return
}

Copilot is powered by AI and may make mistakes. Always verify output.
"git_url": req.GitURL,
"git_branch": req.GitBranch,
}).Error("Failed to clone repository")
helpers.WriteErrorStatus(w, "Failed to clone repository: "+err.Error(), http.StatusBadRequest)
return
}

// Get commit information
hash, err := customGitRepo.GetLastCommitHash()
if err != nil {
log.WithError(err).Error("Failed to get commit hash")
hash = "unknown"
}

message, err := customGitRepo.GetLastCommitMessage()
if err != nil {
log.WithError(err).Error("Failed to get commit message")
message = "unknown"
}

// Create tar.gz archive of the repository
archiveData, err := createRepositoryArchive(tempDir)
if err != nil {
log.WithError(err).Error("Failed to create repository archive")
helpers.WriteErrorStatus(w, "Failed to create archive", http.StatusInternalServerError)
return
}

// Prepare response
response := RepositoryResponse{
Hash: hash,
Message: message,
Archive: archiveData,
}

log.WithFields(log.Fields{
"git_url": req.GitURL,
"git_branch": req.GitBranch,
"commit_hash": hash,
"runner_id": runner.ID,
}).Info("Repository archive served to runner")

helpers.WriteJSON(w, http.StatusOK, response)
}

// Simple logger implementation for git operations
type simpleLogger struct {
status task_logger.TaskStatus
}

type StatusListener = task_logger.StatusListener
type LogListener = task_logger.LogListener
type TaskStatus = task_logger.TaskStatus

func (l *simpleLogger) Log(message string) {
log.Info(message)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by an access to Password
flows to a logging call.
Sensitive data returned by an access to Password
flows to a logging call.

Copilot Autofix

AI about 2 months ago

To prevent leaking passwords in log messages, especially when logging file paths that are derived from URLs containing credentials, we should sanitize such inputs before logging. Specifically, in the context provided, whenever logging a file path (e.g., a requirements file path) that was constructed from a potentially sensitive repository URL, we need to remove any embedded credentials before writing to the log.

The best way to achieve this, without altering existing functionality, is to introduce a helper function that strips credentials from URLs. For example, we can use Go's net/url package (standard library, well-known) to parse the URL and drop the User field, then reconstruct the sanitized URL. We'll apply this sanitization right before logging the filename/path.

Concrete steps:

  • In db_lib/AnsibleApp.go, modify logging statements that log a requirements file path, specifically the call to t.Log(requirementsFilePath + " has no changes. Skip galaxy install process.\n") in function installGalaxyRequirementsFile. Sanitize the path/URL for any embedded credentials.
  • (Optional/future-proofing) If there are other logging statements in the flow where URLs with potential credentials are logged, apply similar sanitization.
  • Define a helper method to sanitize embedded credentials from URLs/paths. Use Go's net/url package for parsing and reconstructing.
  • Add the net/url import if not already present.

No changes are needed in the generic logger (simpleLogger), as the fix should be applied at the source of the potentially problematic log call.


Suggested changeset 1
db_lib/AnsibleApp.go
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/db_lib/AnsibleApp.go b/db_lib/AnsibleApp.go
--- a/db_lib/AnsibleApp.go
+++ b/db_lib/AnsibleApp.go
@@ -9,6 +9,7 @@
 
 	"github.com/semaphoreui/semaphore/db"
 	"github.com/semaphoreui/semaphore/pkg/task_logger"
+	"net/url"
 )
 
 func getMD5Hash(filepath string) (string, error) {
@@ -86,11 +87,23 @@
 	return t.Repository.GetFullPath(t.Template.ID)
 }
 
+// sanitizeRequirementsFilePath removes credentials from URLs that may be embedded in a file path.
+func sanitizeRequirementsFilePath(path string) string {
+	// Only attempt to sanitize if it looks like a URL
+	u, err := url.Parse(path)
+	if err != nil || u.Scheme == "" || u.Host == "" {
+		// Not a URL: return as is
+		return path
+	}
+	u.User = nil
+	return u.String()
+}
+
 func (t *AnsibleApp) installGalaxyRequirementsFile(requirementsType GalaxyRequirementsType, requirementsFilePath string) error {
 	requirementsHashFilePath := fmt.Sprintf("%s_%s.md5", requirementsFilePath, requirementsType)
 
 	if _, err := os.Stat(requirementsFilePath); err != nil {
-		t.Log("No " + requirementsFilePath + " file found. Skip galaxy install process.\n")
+		t.Log("No " + sanitizeRequirementsFilePath(requirementsFilePath) + " file found. Skip galaxy install process.\n")
 		return nil
 	}
 
@@ -108,7 +117,7 @@
 			return err
 		}
 	} else {
-		t.Log(requirementsFilePath + " has no changes. Skip galaxy install process.\n")
+		t.Log(sanitizeRequirementsFilePath(requirementsFilePath) + " has no changes. Skip galaxy install process.\n")
 	}
 
 	return nil
EOF
@@ -9,6 +9,7 @@

"github.com/semaphoreui/semaphore/db"
"github.com/semaphoreui/semaphore/pkg/task_logger"
"net/url"
)

func getMD5Hash(filepath string) (string, error) {
@@ -86,11 +87,23 @@
return t.Repository.GetFullPath(t.Template.ID)
}

// sanitizeRequirementsFilePath removes credentials from URLs that may be embedded in a file path.
func sanitizeRequirementsFilePath(path string) string {
// Only attempt to sanitize if it looks like a URL
u, err := url.Parse(path)
if err != nil || u.Scheme == "" || u.Host == "" {
// Not a URL: return as is
return path
}
u.User = nil
return u.String()
}

func (t *AnsibleApp) installGalaxyRequirementsFile(requirementsType GalaxyRequirementsType, requirementsFilePath string) error {
requirementsHashFilePath := fmt.Sprintf("%s_%s.md5", requirementsFilePath, requirementsType)

if _, err := os.Stat(requirementsFilePath); err != nil {
t.Log("No " + requirementsFilePath + " file found. Skip galaxy install process.\n")
t.Log("No " + sanitizeRequirementsFilePath(requirementsFilePath) + " file found. Skip galaxy install process.\n")
return nil
}

@@ -108,7 +117,7 @@
return err
}
} else {
t.Log(requirementsFilePath + " has no changes. Skip galaxy install process.\n")
t.Log(sanitizeRequirementsFilePath(requirementsFilePath) + " has no changes. Skip galaxy install process.\n")
}

return nil
Copilot is powered by AI and may make mistakes. Always verify output.
}

func (l *simpleLogger) Logf(format string, a ...any) {
log.Infof(format, a...)
}

func (l *simpleLogger) LogWithTime(time time.Time, message string) {
log.Info(message)
}

func (l *simpleLogger) LogfWithTime(time time.Time, format string, a ...any) {
log.Infof(format, a...)
}

func (l *simpleLogger) LogCmd(cmd *exec.Cmd) {
log.Infof("Executing command: %v", cmd)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by an access to Password
flows to a logging call.
Sensitive data returned by an access to Password
flows to a logging call.

Copilot Autofix

AI about 2 months ago

The logging call in func (l *simpleLogger) LogCmd(cmd *exec.Cmd) in api/runners/runners.go currently logs the entire cmd object. Since command arguments may contain sensitive credentials (e.g., in URLs), we should redact credentials before logging, or log only safe, non-sensitive portions of the command. The best fix is to sanitize command arguments before logging.

Approach:

  • Write a helper function in api/runners/runners.go to redact any authentication information in URLs, such as user:password@host constructs.
  • In LogCmd, prepare a string version of the command and apply the sanitization function to all arguments before building/logging.
  • Only make changes inside the shown runners.go code.
  • Add helper function for redacting passwords from URLs using regular expressions.
  • Change LogCmd so that, instead of logging cmd verbatim, it logs the executable and the sanitized arguments.
Suggested changeset 1
api/runners/runners.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/api/runners/runners.go b/api/runners/runners.go
--- a/api/runners/runners.go
+++ b/api/runners/runners.go
@@ -16,6 +16,8 @@
 	"os/exec"
 	"path/filepath"
 	"time"
+	"regexp"
+	"strings"
 
 	"github.com/semaphoreui/semaphore/api/helpers"
 	"github.com/semaphoreui/semaphore/db"
@@ -503,8 +505,20 @@
 	log.Infof(format, a...)
 }
 
+// redactCredentialsFromURL redacts user:password from URLs for logging.
+func redactCredentialsFromURL(s string) string {
+	// Matches protocols http(s)/ftp/git/ssh/... followed by user:password@ and removes credentials
+	re := regexp.MustCompile(`(\w+://)([^/@]+@)`)
+	return re.ReplaceAllString(s, "$1[REDACTED]@")
+}
+
 func (l *simpleLogger) LogCmd(cmd *exec.Cmd) {
-	log.Infof("Executing command: %v", cmd)
+	// Sanitize each argument in Args
+	safeArgs := make([]string, len(cmd.Args))
+	for i, arg := range cmd.Args {
+		safeArgs[i] = redactCredentialsFromURL(arg)
+	}
+	log.Infof("Executing command: %s %s", redactCredentialsFromURL(cmd.Path), strings.Join(safeArgs, " "))
 }
 
 func (l *simpleLogger) SetStatus(status TaskStatus) {
EOF
@@ -16,6 +16,8 @@
"os/exec"
"path/filepath"
"time"
"regexp"
"strings"

"github.com/semaphoreui/semaphore/api/helpers"
"github.com/semaphoreui/semaphore/db"
@@ -503,8 +505,20 @@
log.Infof(format, a...)
}

// redactCredentialsFromURL redacts user:password from URLs for logging.
func redactCredentialsFromURL(s string) string {
// Matches protocols http(s)/ftp/git/ssh/... followed by user:password@ and removes credentials
re := regexp.MustCompile(`(\w+://)([^/@]+@)`)
return re.ReplaceAllString(s, "$1[REDACTED]@")
}

func (l *simpleLogger) LogCmd(cmd *exec.Cmd) {
log.Infof("Executing command: %v", cmd)
// Sanitize each argument in Args
safeArgs := make([]string, len(cmd.Args))
for i, arg := range cmd.Args {
safeArgs[i] = redactCredentialsFromURL(arg)
}
log.Infof("Executing command: %s %s", redactCredentialsFromURL(cmd.Path), strings.Join(safeArgs, " "))
}

func (l *simpleLogger) SetStatus(status TaskStatus) {
Copilot is powered by AI and may make mistakes. Always verify output.
}

func (l *simpleLogger) SetStatus(status TaskStatus) {
l.status = status
}

func (l *simpleLogger) AddStatusListener(listener StatusListener) {
// No-op for simple implementation
}

func (l *simpleLogger) AddLogListener(listener LogListener) {
// No-op for simple implementation
}

func (l *simpleLogger) SetCommit(hash, message string) {
// No-op for simple implementation
}

func (l *simpleLogger) WaitLog() {
// No-op for simple implementation
}

// Simple key installer implementation for git operations
type simpleKeyInstaller struct{}

func (k *simpleKeyInstaller) Install(key db.AccessKey, usage db.AccessKeyRole, logger task_logger.Logger) (ssh.AccessKeyInstallation, error) {
// For now, return a simple implementation that doesn't install keys
// This will work for public repositories or repositories that don't require authentication
return ssh.AccessKeyInstallation{}, nil
}

// Custom GitRepository wrapper that overrides GetFullPath
type customGitRepository struct {
db_lib.GitRepository
customPath string
}

func (c customGitRepository) GetFullPath() string {
return c.customPath
}

func createRepositoryArchive(repoPath string) ([]byte, error) {
var buf bytes.Buffer

// Create gzip writer
gzWriter := gzip.NewWriter(&buf)
defer gzWriter.Close()

// Create tar writer
tarWriter := tar.NewWriter(gzWriter)
defer tarWriter.Close()

// Walk through the repository directory
err := filepath.Walk(repoPath, func(file string, fi os.FileInfo, err error) error {
if err != nil {
return err
}

// Create tar header
header, err := tar.FileInfoHeader(fi, "")
if err != nil {
return err
}

// Calculate relative path
relPath, err := filepath.Rel(repoPath, file)
if err != nil {
return err
}

header.Name = relPath

// Write header
err = tarWriter.WriteHeader(header)
if err != nil {
return err
}

// If it's a regular file, write its contents
if fi.Mode().IsRegular() {
fileData, err := os.Open(file)
if err != nil {
return err
}
defer fileData.Close()

_, err = io.Copy(tarWriter, fileData)
if err != nil {
return err
}
}

return nil
})

if err != nil {
return nil, err
}

// Close writers to flush data
tarWriter.Close()
gzWriter.Close()

return buf.Bytes(), nil
}
8 changes: 8 additions & 0 deletions db_lib/GitClientFactory.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ func CreateDefaultGitClient(keyInstaller AccessKeyInstaller) GitClient {
return CreateGoGitClient(keyInstaller)
case util.CmdGitClientId:
return CreateCmdGitClient(keyInstaller)
case util.ProxyGitClientId:
return CreateProxyGitClient(keyInstaller)
default:
return CreateCmdGitClient(keyInstaller)
}
Expand All @@ -24,3 +26,9 @@ func CreateCmdGitClient(keyInstaller AccessKeyInstaller) GitClient {
keyInstaller: keyInstaller,
}
}

func CreateProxyGitClient(keyInstaller AccessKeyInstaller) GitClient {
return ProxyGitClient{
keyInstaller: keyInstaller,
}
}
Loading
Loading