Skip to content

fix(datastore/mysql): prevent SQL injection in GetTargetByScope - #255

Merged
whywaita merged 1 commit into
masterfrom
fix/sql-injection-get-target-by-scope
Jul 7, 2026
Merged

fix(datastore/mysql): prevent SQL injection in GetTargetByScope#255
whywaita merged 1 commit into
masterfrom
fix/sql-injection-get-target-by-scope

Conversation

@whywaita

@whywaita whywaita commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Summary

GetTargetByScope was the only query in this file built with fmt.Sprintf, embedding the scope value directly into the SQL string. Every other query (e.g. GetTarget) uses bind parameters (?); this one used raw string concatenation.

// Before
query := fmt.Sprintf(`SELECT ... FROM targets WHERE scope = "%s"`, scope)
if err := m.Conn.GetContext(ctx, &t, query); err != nil {

scope originates from webhook payloads (repository / organization names) and the target registration API, so it is user-controllable. A value containing a double quote could break out of the string literal and inject SQL.

Changes

  • Switch GetTargetByScope to a ? placeholder, matching GetTarget.
  • Add a regression test asserting that a scope containing SQL meta characters is treated as a literal value and returns datastore.ErrNotFound (neither matching a row nor producing a SQL error).
// After
query := `SELECT ... FROM targets WHERE scope = ?`
if err := m.Conn.GetContext(ctx, &t, query, scope); err != nil {

Testing

  • go build ./... and go vet ./... pass.
  • The added test compiles; the integration test itself runs in CI (it depends on dockertest / MySQL).

…SQL injection

GetTargetByScope built its query with fmt.Sprintf, embedding the scope
value directly into the SQL string while every other query in this file
uses bind parameters. The scope originates from webhook payloads and the
target registration API, so a value containing a double quote could
break out of the string literal and inject SQL.

Switch to a ? placeholder like GetTarget does, and add a regression test
that a scope containing SQL meta characters is treated as a literal
value (returning datastore.ErrNotFound rather than matching or erroring).
@whywaita
whywaita merged commit ae1689f into master Jul 7, 2026
5 checks passed
@whywaita
whywaita deleted the fix/sql-injection-get-target-by-scope branch July 7, 2026 06:16
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