Compare commits

...

2 Commits

Author SHA1 Message Date
clawbot
b0d84868e9 fix: sanitize container log output and fix lint issues
- Update nolint comment on log streaming to accurately describe why
  gosec is suppressed (text/plain Content-Type, not HTML)
- Replace <script type="text/plain"> with data attribute for initial
  logs to prevent </script> breakout from attacker-controlled log data
- Move RemoveImage before unexported methods (funcorder)
- Fix file permissions in test (gosec G306)
- Rename unused parameters in export_test.go (revive)
- Add required blank line before assignment (wsl)
2026-02-19 20:30:11 -08:00
clawbot
fb91246b07 chore: code cleanup and best practices (closes #45)
- Fix gofmt formatting across 4 files
- Add nolint annotations with justifications for all gosec findings
- Resolve all 7 pre-existing linter warnings
- make check now passes cleanly
2026-02-19 20:27:07 -08:00
11 changed files with 27 additions and 26 deletions

View File

@@ -51,7 +51,7 @@ type Config struct {
MaintenanceMode bool MaintenanceMode bool
MetricsUsername string MetricsUsername string
MetricsPassword string MetricsPassword string
SessionSecret string SessionSecret string //nolint:gosec // not a hardcoded credential, loaded from env/file
CORSOrigins string CORSOrigins string
params *Params params *Params
log *slog.Logger log *slog.Logger

View File

@@ -480,6 +480,20 @@ func (c *Client) CloneRepo(
return c.performClone(ctx, cfg) 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( func (c *Client) performBuild(
ctx context.Context, ctx context.Context,
opts BuildImageOptions, opts BuildImageOptions,
@@ -740,20 +754,6 @@ func (c *Client) connect(ctx context.Context) error {
return nil 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 { func (c *Client) close() error {
if c.docker != nil { if c.docker != nil {
err := c.docker.Close() err := c.docker.Close()

View File

@@ -76,7 +76,7 @@ func deploymentToAPI(d *models.Deployment) apiDeploymentResponse {
func (h *Handlers) HandleAPILoginPOST() http.HandlerFunc { func (h *Handlers) HandleAPILoginPOST() http.HandlerFunc {
type loginRequest struct { type loginRequest struct {
Username string `json:"username"` Username string `json:"username"`
Password string `json:"password"` Password string `json:"password"` //nolint:gosec // request field, not a hardcoded credential
} }
type loginResponse struct { type loginResponse struct {

View File

@@ -499,7 +499,7 @@ func (h *Handlers) HandleAppLogs() http.HandlerFunc {
return return
} }
_, _ = writer.Write([]byte(logs)) _, _ = writer.Write([]byte(logs)) //nolint:gosec // response Content-Type is text/plain, not rendered as HTML
} }
} }
@@ -582,7 +582,7 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
} }
// Check if file exists // 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) { if os.IsNotExist(err) {
http.NotFound(writer, request) http.NotFound(writer, request)

View File

@@ -726,6 +726,7 @@ func (svc *Service) cleanupCancelledDeploy(
} else { } else {
svc.log.Info("cleaned up build dir from cancelled deploy", svc.log.Info("cleaned up build dir from cancelled deploy",
"app", app.Name, "path", dirPath) "app", app.Name, "path", dirPath)
_ = deployment.AppendLog(ctx, "Cleaned up build directory") _ = deployment.AppendLog(ctx, "Cleaned up build directory")
} }
} }

View File

@@ -32,7 +32,7 @@ func TestCleanupCancelledDeploy_RemovesBuildDir(t *testing.T) {
require.NoError(t, os.MkdirAll(deployDir, 0o750)) require.NoError(t, os.MkdirAll(deployDir, 0o750))
// Create a file inside to verify full removal // 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) // Also create a dir for a different deployment (should NOT be removed)
otherDir := filepath.Join(buildDir, "99-xyz789") otherDir := filepath.Join(buildDir, "99-xyz789")

View File

@@ -52,10 +52,10 @@ func NewTestServiceWithConfig(log *slog.Logger, cfg *config.Config, dockerClient
// cleanupCancelledDeploy for testing. It removes build directories matching // cleanupCancelledDeploy for testing. It removes build directories matching
// the deployment ID prefix. // the deployment ID prefix.
func (svc *Service) CleanupCancelledDeploy( func (svc *Service) CleanupCancelledDeploy(
ctx context.Context, _ context.Context,
appName string, appName string,
deploymentID int64, deploymentID int64,
imageID string, _ string,
) { ) {
// We can't create real models.App/Deployment in tests easily, // We can't create real models.App/Deployment in tests easily,
// so we test the build dir cleanup portion directly. // so we test the build dir cleanup portion directly.

View File

@@ -260,7 +260,7 @@ func (svc *Service) sendNtfy(
request.Header.Set("Title", title) request.Header.Set("Title", title)
request.Header.Set("Priority", svc.ntfyPriority(priority)) 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 { if err != nil {
return fmt.Errorf("failed to send ntfy request: %w", err) 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") 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 { if err != nil {
return fmt.Errorf("failed to send slack request: %w", err) return fmt.Errorf("failed to send slack request: %w", err)
} }

View File

@@ -12,7 +12,7 @@ import (
// KeyPair contains an SSH key pair. // KeyPair contains an SSH key pair.
type KeyPair struct { type KeyPair struct {
PrivateKey string PrivateKey string //nolint:gosec // field name describes SSH key material, not a hardcoded secret
PublicKey string PublicKey string
} }

View File

@@ -369,7 +369,7 @@ document.addEventListener("alpine:init", () => {
init() { init() {
// Read initial logs from script tag (avoids escaping issues) // Read initial logs from script tag (avoids escaping issues)
const initialLogsEl = this.$el.querySelector(".initial-logs"); const initialLogsEl = this.$el.querySelector(".initial-logs");
this.logs = initialLogsEl?.textContent || "Loading..."; this.logs = initialLogsEl?.dataset.logs || "Loading...";
// Set up scroll tracking // Set up scroll tracking
this.$nextTick(() => { this.$nextTick(() => {

View File

@@ -98,7 +98,7 @@
title="Scroll to bottom" title="Scroll to bottom"
>↓ Follow</button> >↓ Follow</button>
</div> </div>
{{if .Logs.Valid}}<script type="text/plain" class="initial-logs">{{.Logs.String}}</script>{{end}} {{if .Logs.Valid}}<div hidden class="initial-logs" data-logs="{{.Logs.String}}"></div>{{end}}
</div> </div>
{{end}} {{end}}
</div> </div>