Skip to content

fix: address GitHub security findings (CodeQL #38/#40, Dependabot #207)#163

Open
cawalch wants to merge 3 commits into
mainfrom
fix/security-findings
Open

fix: address GitHub security findings (CodeQL #38/#40, Dependabot #207)#163
cawalch wants to merge 3 commits into
mainfrom
fix/security-findings

Conversation

@cawalch

@cawalch cawalch commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Addresses all open security findings from the GitHub Security tab.

CodeQL #40 — Log injection (ERROR)

File: services/merrymaker-go/internal/http/ui_jobs.go

Replaced three log.Printf calls that interpolated user-controlled jobID/eventID path values into log message strings with structured h.logger().ErrorContext/WarnContext calls. Structured logging treats user values as typed fields, preventing newline-based log injection.

CodeQL #38 — Cookie Secure attribute not set (WARNING)

File: services/merrymaker-go/internal/http/middleware_csrf.go

The CSRF token cookie previously set Secure: isSecure (dynamic, based on TLS/proxy detection). Changed to Secure: true unconditionally — the app is always served over HTTPS in production (direct TLS or proxy-terminated TLS).

Dependabot #207 / CodeQL #43 #44uuid < 11.1.1 (MEDIUM)

File: services/puppeteer-worker/package.json

Added an npm overrides entry to force uuid@^11.1.1 across all transitive dependencies (@google-cloud/storagegaxios, teeny-request). npm audit now reports 0 vulnerabilities.

cawalch added 2 commits June 1, 2026 15:25
- ui_jobs.go: replace log.Printf with structured h.logger() calls for
  user-controlled jobID/eventID values to prevent log injection (CodeQL #40)
- middleware_csrf.go: set CSRF cookie Secure attribute to true unconditionally;
  app is always served over HTTPS in production (CodeQL #38)
- puppeteer-worker: add npm override to force uuid >= 11.1.1, fixing
  buffer bounds check vuln in transitive deps via @google-cloud/storage
  (Dependabot #207, CodeQL #43/#44)
- ui_jobs.go: wrap long ErrorContext call to satisfy golines
- middleware_csrf.go: rename unused r param to _, remove now-unused
  isForwardedHTTPS helper (both became dead code after Secure: true change)
@cawalch

cawalch commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

⚠️ GolangCI-Lint is failing due to a formatting issue:

services/merrymaker-go/internal/http/ui_jobs.go:1119:1: File is not properly formatted (golines)

The line with h.logger().ErrorContext(r.Context(), "JobEventDetails: failed to fetch event", ...) needs to be reformatted. Run golines on the file to fix it.

Also this PR needs a rebase onto the latest main since the base branch has been updated.

@cawalch

cawalch commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

@dependabot rebase

@cawalch cawalch enabled auto-merge (rebase) June 15, 2026 15:53
@cawalch cawalch self-assigned this Jun 15, 2026
@cawalch cawalch requested a review from chriscarlson June 15, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant