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)
This commit is contained in:
clawbot 2026-02-19 20:30:11 -08:00
parent fb91246b07
commit b0d84868e9
7 changed files with 21 additions and 20 deletions

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

@ -499,7 +499,7 @@ func (h *Handlers) HandleAppLogs() http.HandlerFunc {
return return
} }
_, _ = writer.Write([]byte(logs)) //nolint:gosec // logs are from trusted container output, not user input _, _ = writer.Write([]byte(logs)) //nolint:gosec // response Content-Type is text/plain, not rendered as HTML
} }
} }

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

@ -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>