Skip to content

sec/golangci#3817

Open
fiftin wants to merge 7 commits intodevelopfrom
sec/golangci
Open

sec/golangci#3817
fiftin wants to merge 7 commits intodevelopfrom
sec/golangci

Conversation

@fiftin
Copy link
Copy Markdown
Collaborator

@fiftin fiftin commented May 1, 2026

  • sec(be): fixing bugs found by golangci
  • sec(be): pass golangci
  • fix(be): add missed err return
  • fix(be): remove extra conversion
  • style(be): code style

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Security review (sec/golangci)

Outcome: No medium, high, or critical vulnerabilities identified in the added or modified code paths for this PR.

Prior threads: No remaining findings from earlier automation runs apply to the current diff after validation.

Notes

  • Removed admin /debug/gc and /debug/pprof/dump routes reduces attack surface (heap profiles and forced GC were previously reachable behind the admin API).
  • Runner decrypt failures now fail closed with helpers.WriteError; the default branch logs server-side and returns 400 with no JSON error body, so plaintext secrets are not written to the response.
  • Other edits are error-handling, import/lint, or equivalent refactors without new trust boundaries.

Slack summary (copy/paste):

PR 3817 (sec/golangci) — security review: Clean. No medium+ findings. Admin debug GC/pprof endpoints removed (smaller surface). Runner decrypt error handling uses generic API error responses without leaking secret material in the default path.

Open in Web View Automation 

Sent by Cursor Automation: Find vulnerabilities

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Security review (automation)

Reviewed added/modified code for injection, authz/authn bypasses, secret leakage, SSRF/XSS/path traversal, and unsafe deserialization.

Outcome: No medium, high, or critical vulnerabilities identified with a plausible attacker-controlled path to impact in this diff.

Notes (non-finding):

  • Removing /api/debug/gc and /api/debug/pprof/dump from adminAPI reduces privileged operational attack surface.
  • Runner decrypt failures now return via helpers.WriteError instead of continuing; default branch logs server-side without echoing raw crypto errors to the client.
  • services/export/ProjectUser.go change from val.User.ID to val.ID matches embedded User on UserWithProjectRole and is equivalent for user identity.

Prior automation threads: validated against current diff; nothing warrants re-reporting with new evidence.

Open in Web View Automation 

Sent by Cursor Automation: Find vulnerabilities

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security review (automation)

Outcome: No medium, high, or critical vulnerabilities were identified in the added or modified code for this revision.

Checks performed

  • Traced runner-facing paths (GetRunner decrypt failures now return via helpers.WriteError; generic errors map to HTTP 400 with no JSON body, avoiding error-detail leakage to the client).
  • Reviewed runner registration (enabledActive), config/env wiring, and SQL/Bolt error-handling fixes; no new authz bypass or injection surface identified.
  • Confirmed removal of admin /debug routes is a hardening change.

Slack summary (copy-paste)
PR 3817 (sec/golangci): Security pass — no medium+ findings. Changes are mostly lint/style, error handling, runner enabled flag, and removal of debug admin endpoints (reduces exposure). Prior automation threads cleaned up in favor of this assessment.

Open in Web View Automation 

Sent by Cursor Automation: Find vulnerabilities

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