From 711c92028542a5db477be62a7177afb0a4380e88 Mon Sep 17 00:00:00 2001 From: clawbot Date: Thu, 19 Feb 2026 23:44:47 -0800 Subject: [PATCH] fix: resolve lint issues for make check compliance --- internal/config/config.go | 2 +- internal/docker/client.go | 28 +++---- internal/handlers/api.go | 2 +- internal/handlers/app.go | 9 +- internal/handlers/sanitize.go | 30 +++++++ internal/handlers/sanitize_test.go | 84 +++++++++++++++++++ internal/service/deploy/deploy.go | 1 + .../service/deploy/deploy_cleanup_test.go | 2 +- internal/service/deploy/export_test.go | 4 +- internal/service/notify/notify.go | 4 +- internal/ssh/keygen.go | 2 +- 11 files changed, 142 insertions(+), 26 deletions(-) create mode 100644 internal/handlers/sanitize.go create mode 100644 internal/handlers/sanitize_test.go diff --git a/internal/config/config.go b/internal/config/config.go index b3adafb..d6f919b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -51,7 +51,7 @@ type Config struct { MaintenanceMode bool MetricsUsername string MetricsPassword string - SessionSecret string + SessionSecret string //nolint:gosec // not a hardcoded credential, loaded from env/file CORSOrigins string params *Params log *slog.Logger diff --git a/internal/docker/client.go b/internal/docker/client.go index 10af151..38cc198 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -480,6 +480,20 @@ func (c *Client) CloneRepo( return c.performClone(ctx, cfg) } +// RemoveImage removes a Docker image by ID or tag. +// It returns nil if the image was successfully removed or does not exist. +func (c *Client) RemoveImage(ctx context.Context, imageID string) error { + _, err := c.docker.ImageRemove(ctx, imageID, image.RemoveOptions{ + Force: true, + PruneChildren: true, + }) + if err != nil && !client.IsErrNotFound(err) { + return fmt.Errorf("failed to remove image %s: %w", imageID, err) + } + + return nil +} + func (c *Client) performBuild( ctx context.Context, opts BuildImageOptions, @@ -740,20 +754,6 @@ func (c *Client) connect(ctx context.Context) error { return nil } -// RemoveImage removes a Docker image by ID or tag. -// It returns nil if the image was successfully removed or does not exist. -func (c *Client) RemoveImage(ctx context.Context, imageID string) error { - _, err := c.docker.ImageRemove(ctx, imageID, image.RemoveOptions{ - Force: true, - PruneChildren: true, - }) - if err != nil && !client.IsErrNotFound(err) { - return fmt.Errorf("failed to remove image %s: %w", imageID, err) - } - - return nil -} - func (c *Client) close() error { if c.docker != nil { err := c.docker.Close() diff --git a/internal/handlers/api.go b/internal/handlers/api.go index d97be9b..edf7f46 100644 --- a/internal/handlers/api.go +++ b/internal/handlers/api.go @@ -76,7 +76,7 @@ func deploymentToAPI(d *models.Deployment) apiDeploymentResponse { func (h *Handlers) HandleAPILoginPOST() http.HandlerFunc { type loginRequest struct { Username string `json:"username"` - Password string `json:"password"` + Password string `json:"password"` //nolint:gosec // request field, not a hardcoded credential } type loginResponse struct { diff --git a/internal/handlers/app.go b/internal/handlers/app.go index 72fb07c..14fec9d 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -499,7 +499,8 @@ func (h *Handlers) HandleAppLogs() http.HandlerFunc { return } - _, _ = writer.Write([]byte(logs)) + //nolint:gosec // logs sanitized: ANSI escapes and control chars stripped + _, _ = writer.Write([]byte(SanitizeLogs(logs))) } } @@ -534,7 +535,7 @@ func (h *Handlers) HandleDeploymentLogsAPI() http.HandlerFunc { logs := "" if deployment.Logs.Valid { - logs = deployment.Logs.String + logs = SanitizeLogs(deployment.Logs.String) } response := map[string]any{ @@ -582,7 +583,7 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc { } // Check if file exists - _, err := os.Stat(logPath) + _, err := os.Stat(logPath) //nolint:gosec // logPath is constructed by deploy service, not from user input if os.IsNotExist(err) { http.NotFound(writer, request) @@ -661,7 +662,7 @@ func (h *Handlers) HandleContainerLogsAPI() http.HandlerFunc { } response := map[string]any{ - "logs": logs, + "logs": SanitizeLogs(logs), "status": status, } diff --git a/internal/handlers/sanitize.go b/internal/handlers/sanitize.go new file mode 100644 index 0000000..91f2ddc --- /dev/null +++ b/internal/handlers/sanitize.go @@ -0,0 +1,30 @@ +package handlers + +import ( + "regexp" + "strings" +) + +// ansiEscapePattern matches ANSI escape sequences (CSI, OSC, and single-character escapes). +var ansiEscapePattern = regexp.MustCompile(`(\x1b\[[0-9;]*[a-zA-Z]|\x1b\][^\x07]*\x07|\x1b[^[\]])`) + +// SanitizeLogs strips ANSI escape sequences and non-printable control characters +// from container log output. Newlines (\n), carriage returns (\r), and tabs (\t) +// are preserved. This ensures that attacker-controlled container output cannot +// inject terminal escape sequences or other dangerous control characters. +func SanitizeLogs(input string) string { + // Strip ANSI escape sequences + result := ansiEscapePattern.ReplaceAllString(input, "") + + // Strip remaining non-printable characters (keep \n, \r, \t) + var b strings.Builder + b.Grow(len(result)) + + for _, r := range result { + if r == '\n' || r == '\r' || r == '\t' || r >= ' ' { + b.WriteRune(r) + } + } + + return b.String() +} diff --git a/internal/handlers/sanitize_test.go b/internal/handlers/sanitize_test.go new file mode 100644 index 0000000..8282082 --- /dev/null +++ b/internal/handlers/sanitize_test.go @@ -0,0 +1,84 @@ +package handlers_test + +import ( + "testing" + + "git.eeqj.de/sneak/upaas/internal/handlers" +) + +func TestSanitizeLogs(t *testing.T) { //nolint:funlen // table-driven tests + t.Parallel() + + tests := []struct { + name string + input string + expected string + }{ + { + name: "plain text unchanged", + input: "hello world\n", + expected: "hello world\n", + }, + { + name: "strips ANSI color codes", + input: "\x1b[31mERROR\x1b[0m: something failed\n", + expected: "ERROR: something failed\n", + }, + { + name: "strips OSC sequences", + input: "\x1b]0;window title\x07normal text\n", + expected: "normal text\n", + }, + { + name: "strips null bytes", + input: "hello\x00world\n", + expected: "helloworld\n", + }, + { + name: "strips bell characters", + input: "alert\x07here\n", + expected: "alerthere\n", + }, + { + name: "preserves tabs", + input: "field1\tfield2\tfield3\n", + expected: "field1\tfield2\tfield3\n", + }, + { + name: "preserves carriage returns", + input: "line1\r\nline2\r\n", + expected: "line1\r\nline2\r\n", + }, + { + name: "strips mixed escape sequences", + input: "\x1b[32m2024-01-01\x1b[0m \x1b[1mINFO\x1b[0m starting\x00\n", + expected: "2024-01-01 INFO starting\n", + }, + { + name: "empty string", + input: "", + expected: "", + }, + { + name: "only control characters", + input: "\x00\x01\x02\x03", + expected: "", + }, + { + name: "cursor movement sequences stripped", + input: "\x1b[2J\x1b[H\x1b[3Atext\n", + expected: "text\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := handlers.SanitizeLogs(tt.input) + if got != tt.expected { + t.Errorf("SanitizeLogs(%q) = %q, want %q", tt.input, got, tt.expected) + } + }) + } +} diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index 19a65a4..0959729 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -726,6 +726,7 @@ func (svc *Service) cleanupCancelledDeploy( } else { svc.log.Info("cleaned up build dir from cancelled deploy", "app", app.Name, "path", dirPath) + _ = deployment.AppendLog(ctx, "Cleaned up build directory") } } diff --git a/internal/service/deploy/deploy_cleanup_test.go b/internal/service/deploy/deploy_cleanup_test.go index 1474342..5a49ee5 100644 --- a/internal/service/deploy/deploy_cleanup_test.go +++ b/internal/service/deploy/deploy_cleanup_test.go @@ -32,7 +32,7 @@ func TestCleanupCancelledDeploy_RemovesBuildDir(t *testing.T) { require.NoError(t, os.MkdirAll(deployDir, 0o750)) // Create a file inside to verify full removal - require.NoError(t, os.WriteFile(filepath.Join(deployDir, "work"), []byte("test"), 0o640)) + require.NoError(t, os.WriteFile(filepath.Join(deployDir, "work"), []byte("test"), 0o600)) // Also create a dir for a different deployment (should NOT be removed) otherDir := filepath.Join(buildDir, "99-xyz789") diff --git a/internal/service/deploy/export_test.go b/internal/service/deploy/export_test.go index a5aa241..bd90daa 100644 --- a/internal/service/deploy/export_test.go +++ b/internal/service/deploy/export_test.go @@ -52,10 +52,10 @@ func NewTestServiceWithConfig(log *slog.Logger, cfg *config.Config, dockerClient // cleanupCancelledDeploy for testing. It removes build directories matching // the deployment ID prefix. func (svc *Service) CleanupCancelledDeploy( - ctx context.Context, + _ context.Context, appName string, deploymentID int64, - imageID string, + _ string, ) { // We can't create real models.App/Deployment in tests easily, // so we test the build dir cleanup portion directly. diff --git a/internal/service/notify/notify.go b/internal/service/notify/notify.go index 0cc29da..0d10728 100644 --- a/internal/service/notify/notify.go +++ b/internal/service/notify/notify.go @@ -260,7 +260,7 @@ func (svc *Service) sendNtfy( request.Header.Set("Title", title) request.Header.Set("Priority", svc.ntfyPriority(priority)) - resp, err := svc.client.Do(request) + resp, err := svc.client.Do(request) //nolint:gosec // URL constructed from trusted config, not user input if err != nil { return fmt.Errorf("failed to send ntfy request: %w", err) } @@ -352,7 +352,7 @@ func (svc *Service) sendSlack( request.Header.Set("Content-Type", "application/json") - resp, err := svc.client.Do(request) + resp, err := svc.client.Do(request) //nolint:gosec // URL from trusted webhook config if err != nil { return fmt.Errorf("failed to send slack request: %w", err) } diff --git a/internal/ssh/keygen.go b/internal/ssh/keygen.go index 49e0ee9..ce2d8c0 100644 --- a/internal/ssh/keygen.go +++ b/internal/ssh/keygen.go @@ -12,7 +12,7 @@ import ( // KeyPair contains an SSH key pair. type KeyPair struct { - PrivateKey string + PrivateKey string //nolint:gosec // field name describes SSH key material, not a hardcoded secret PublicKey string }