From 6b513de632800363c9b558b8ae16d3c2af1f815e Mon Sep 17 00:00:00 2001 From: Tirefire <84106878+tire-fire@users.noreply.github.com> Date: Fri, 13 Feb 2026 13:58:26 +0000 Subject: [PATCH 1/2] Fix gosec v2.23.0 findings --- .github/workflows/test.yml | 5 +++-- engine/checks/web.go | 12 ++++++++++-- www/api/announcements.go | 8 +++++--- www/api/api.go | 22 ++++++++++++++++++++++ www/api/injects.go | 10 ++++++---- www/api/submissions.go | 6 +++--- 6 files changed, 49 insertions(+), 14 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4de3bf38..cf0eb1fe 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -111,8 +111,9 @@ jobs: - name: Run gosec security scanner run: | go install github.com/securego/gosec/v2/cmd/gosec@latest - # Security exceptions are handled via inline // #nosec comments with justifications - gosec -exclude-generated -exclude-dir=divisor ./... + # G117: false positive on intentional struct fields (RefreshToken in Redis DTO, Password in login request) + # G706: false positive with log/slog structured logging (values are auto-escaped) + gosec -exclude=G117,G706 -exclude-generated -exclude-dir=divisor -exclude-dir=.worktrees ./... property-tests: name: Property-Based Tests diff --git a/engine/checks/web.go b/engine/checks/web.go index 8f9f462f..2ec3d9bf 100644 --- a/engine/checks/web.go +++ b/engine/checks/web.go @@ -8,6 +8,7 @@ import ( "log/slog" "math/rand" "net/http" + "net/url" "regexp" "strconv" "strings" @@ -53,7 +54,14 @@ func (c Web) Run(teamID uint, teamIdentifier string, roundID uint, resultsChan c } requestURL := fmt.Sprintf("%s://%s:%d%s", c.Scheme, c.Target, c.Port, u.Path) - req, err := http.NewRequest("GET", requestURL, nil) + parsedURL, err := url.Parse(requestURL) + if err != nil || (parsedURL.Scheme != "http" && parsedURL.Scheme != "https") { + checkResult.Error = "invalid request URL" + checkResult.Debug = "URL failed validation: " + requestURL + response <- checkResult + return + } + req, err := http.NewRequest("GET", parsedURL.String(), nil) if err != nil { checkResult.Error = "error creating web request" checkResult.Debug = err.Error() @@ -66,7 +74,7 @@ func (c Web) Run(teamID uint, teamIdentifier string, roundID uint, resultsChan c // Store request info for timeout debugging checkResult.Debug = fmt.Sprintf("Attempting GET %s", requestURL) - resp, err := client.Do(req) + resp, err := client.Do(req) // #nosec G704 -- URL is validated above; target comes from admin-controlled event.conf if err != nil { checkResult.Error = "web request errored out" if strings.Contains(err.Error(), "Client.Timeout exceeded") { diff --git a/www/api/announcements.go b/www/api/announcements.go index b7968292..5a638735 100644 --- a/www/api/announcements.go +++ b/www/api/announcements.go @@ -7,6 +7,7 @@ import ( "net/http" "os" "path" + "path/filepath" "quotient/engine/db" "slices" "time" @@ -146,12 +147,13 @@ func CreateAnnouncement(w http.ResponseWriter, r *http.Request) { return } - uploadDir := fmt.Sprintf("submissions/announcements/%d", announcement.ID) - if err := os.MkdirAll(uploadDir, 0750); err != nil { + subDir := fmt.Sprintf("%d", announcement.ID) + if err := SafeMkdirAll("submissions/announcements", subDir, 0750); err != nil { WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": "Failed to create directory"}) return } + uploadDir := filepath.Join("submissions/announcements", subDir) for _, fileHeader := range files { file, err := fileHeader.Open() if err != nil { @@ -160,7 +162,7 @@ func CreateAnnouncement(w http.ResponseWriter, r *http.Request) { } defer file.Close() - dst, err := os.Create(fmt.Sprintf("%s/%s", uploadDir, fileHeader.Filename)) + dst, err := SafeCreate(uploadDir, fileHeader.Filename) if err != nil { WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": "Failed to create file on disk"}) return diff --git a/www/api/api.go b/www/api/api.go index 3d752b3c..4b4bca39 100644 --- a/www/api/api.go +++ b/www/api/api.go @@ -2,13 +2,17 @@ package api import ( "encoding/json" + "errors" "fmt" + "io/fs" "log/slog" "net/http" "os" + "path/filepath" "quotient/engine" "quotient/engine/config" "quotient/engine/db" + "strings" ) var ( @@ -56,6 +60,24 @@ func SafeCreate(baseDir, relativePath string) (*os.File, error) { return root.Create(relativePath) } +// SafeMkdirAll creates nested directories within baseDir safely, +// preventing directory traversal attacks using os.Root. +func SafeMkdirAll(baseDir, relativePath string, perm os.FileMode) error { + root, err := os.OpenRoot(baseDir) + if err != nil { + return err + } + defer root.Close() + parts := strings.Split(filepath.ToSlash(filepath.Clean(relativePath)), "/") + for i := range parts { + dir := strings.Join(parts[:i+1], "/") + if err := root.Mkdir(dir, perm); err != nil && !errors.Is(err, fs.ErrExist) { + return err + } + } + return nil +} + // CheckCompetitionStarted returns false and writes error response if competition hasn't started // Admins always have access regardless of competition start time func CheckCompetitionStarted(w http.ResponseWriter, r *http.Request) bool { diff --git a/www/api/injects.go b/www/api/injects.go index 3e3397ef..9294541a 100644 --- a/www/api/injects.go +++ b/www/api/injects.go @@ -7,6 +7,7 @@ import ( "net/http" "os" "path" + "path/filepath" "quotient/engine/db" "slices" "time" @@ -179,12 +180,13 @@ func CreateInject(w http.ResponseWriter, r *http.Request) { return } - uploadDir := fmt.Sprintf("config/injects/%d", inject.ID) - if err := os.MkdirAll(uploadDir, 0750); err != nil { + subDir := fmt.Sprintf("%d", inject.ID) + if err := SafeMkdirAll("config/injects", subDir, 0750); err != nil { WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": "Failed to create directory"}) return } + uploadDir := filepath.Join("config/injects", subDir) for _, fileHeader := range files { file, err := fileHeader.Open() if err != nil { @@ -193,7 +195,7 @@ func CreateInject(w http.ResponseWriter, r *http.Request) { } defer file.Close() - dst, err := os.Create(fmt.Sprintf("%s/%s", uploadDir, fileHeader.Filename)) + dst, err := SafeCreate(uploadDir, fileHeader.Filename) if err != nil { WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": "Failed to create file on disk"}) return @@ -324,7 +326,7 @@ func UpdateInject(w http.ResponseWriter, r *http.Request) { } defer file.Close() - dst, err := os.Create(fmt.Sprintf("%s/%s", uploadDir, fileHeader.Filename)) + dst, err := SafeCreate(uploadDir, fileHeader.Filename) if err != nil { WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": "Failed to create file on disk"}) return diff --git a/www/api/submissions.go b/www/api/submissions.go index 9f66356a..d9784644 100644 --- a/www/api/submissions.go +++ b/www/api/submissions.go @@ -7,7 +7,6 @@ import ( "log/slog" "math" "net/http" - "os" "path/filepath" "quotient/engine/db" "slices" @@ -79,13 +78,14 @@ func CreateSubmission(w http.ResponseWriter, r *http.Request) { return } - uploadDir := fmt.Sprintf("submissions/%d/%d/%d", injectID, team.ID, submission.Version) - err = os.MkdirAll(uploadDir, 0750) + subDir := fmt.Sprintf("%d/%d/%d", injectID, team.ID, submission.Version) + err = SafeMkdirAll("submissions", subDir, 0750) if err != nil { WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": "Error creating directories"}) return } + uploadDir := filepath.Join("submissions", subDir) out, err := SafeCreate(uploadDir, fileHeader.Filename) if err != nil { WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": "Error creating the file"}) From 4d937a7ee124f96c28d0427f94bdb3712517a28b Mon Sep 17 00:00:00 2001 From: Tirefire <84106878+tire-fire@users.noreply.github.com> Date: Fri, 13 Feb 2026 14:26:46 +0000 Subject: [PATCH 2/2] Migrate runner from log to slog --- runner/runner.go | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/runner/runner.go b/runner/runner.go index 4fbc46d7..588b91c3 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -4,7 +4,7 @@ import ( "context" "encoding/json" "fmt" - "log" + "log/slog" "os" "time" @@ -26,7 +26,7 @@ func main() { func runApp(err error) int { if err != nil { - log.Printf("[Runner] Error from reaper: %v", err) + slog.Error("error from reaper", "error", err) return 1 } @@ -51,7 +51,7 @@ func runApp(err error) int { }) ctx := context.Background() - log.Printf("[Runner] Started with ID %s, listening for tasks on Redis at: %s", runnerID, redisAddr) + slog.Info("runner started", "runner_id", runnerID, "redis_addr", redisAddr) go func() { events := rdb.Subscribe(context.Background(), "events") @@ -59,9 +59,9 @@ func runApp(err error) int { eventsChannel := events.Channel() for msg := range eventsChannel { - log.Printf("[Runner] Received message: %v", msg) + slog.Info("received message", "payload", msg.Payload) if msg.Payload == "reset" { - log.Printf("[Runner] Reset event received, quitting...") + slog.Info("reset event received, quitting") os.Exit(0) } else { continue @@ -72,13 +72,13 @@ func runApp(err error) int { for { task, err := getNextTask(ctx, rdb) if err != nil { - log.Printf("[Runner] Error getting task: %v", err) + slog.Error("error getting task", "error", err) continue } runner, err := createRunner(task) if err != nil { - log.Printf("[Runner] Error creating runner: %v", err) + slog.Error("error creating runner", "error", err) continue } @@ -103,8 +103,8 @@ func getNextTask(ctx context.Context, rdb *redis.Client) (*engine.Task, error) { return nil, fmt.Errorf("invalid task format: %w", err) } - log.Printf("[Runner] Received task: RoundID=%d TeamID=%d TeamIdentifier=%s ServiceType=%s", - task.RoundID, task.TeamID, task.TeamIdentifier, task.ServiceType) + slog.Info("received task", "round_id", task.RoundID, "team_id", task.TeamID, + "team_identifier", task.TeamIdentifier, "service_type", task.ServiceType) return &task, nil } @@ -153,7 +153,7 @@ func createRunner(task *engine.Task) (checks.Runner, error) { return nil, fmt.Errorf("failed to unmarshal check data: %w", err) } - log.Printf("[Runner] CheckData: %+v", runner) + slog.Debug("check data", "runner", fmt.Sprintf("%+v", runner)) return runner, nil } @@ -181,8 +181,8 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta // this currently discards all failed attempts for i := range task.Attempts { - log.Printf("[Runner] Running check: RoundID=%d TeamID=%d ServiceType=%s ServiceName=%s Attempt=%d", - task.RoundID, task.TeamID, task.ServiceType, task.ServiceName, i+1) + slog.Info("running check", "round_id", task.RoundID, "team_id", task.TeamID, + "service_type", task.ServiceType, "service_name", task.ServiceName, "attempt", i+1) // Create context with deadline checkCtx, cancel := context.WithDeadline(ctx, task.Deadline) @@ -199,8 +199,8 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta result.ServiceType = task.ServiceType result.RoundID = task.RoundID - log.Printf("[Runner] Check result received: RoundID=%d TeamID=%d ServiceType=%s Status=%v Debug=%s Error=%s", - result.RoundID, result.TeamID, result.ServiceType, result.Status, result.Debug, result.Error) + slog.Info("check result received", "round_id", result.RoundID, "team_id", result.TeamID, + "service_type", result.ServiceType, "status", result.Status, "debug", result.Debug, "error", result.Error) case <-checkCtx.Done(): result.Status = false @@ -211,8 +211,8 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta result.ServiceType = task.ServiceType result.RoundID = task.RoundID - log.Printf("[Runner] Check timed out: RoundID=%d TeamID=%d ServiceType=%s", - task.RoundID, task.TeamID, task.ServiceType) + slog.Warn("check timed out", "round_id", task.RoundID, "team_id", task.TeamID, + "service_type", task.ServiceType) } // Break if successful or deadline passed @@ -224,17 +224,17 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta // Marshal and store result resultJSON, err := json.Marshal(result) if err != nil { - log.Printf("[Runner] Failed to marshal result: %v", err) + slog.Error("failed to marshal result", "error", err) return } if err := rdb.RPush(ctx, "results", resultJSON).Err(); err != nil { - log.Printf("[Runner] Failed to push result to Redis: %v", err) + slog.Error("failed to push result to Redis", "error", err) return } - log.Printf("[Runner] Successfully pushed result: RoundID=%d TeamID=%d ServiceType=%s Status=%v", - result.RoundID, result.TeamID, result.ServiceType, result.Status) + slog.Info("successfully pushed result", "round_id", result.RoundID, "team_id", result.TeamID, + "service_type", result.ServiceType, "status", result.Status) // Update the task key with the final result status result.EndTime = time.Now().Format(time.RFC3339)