From 0429d880e96d706d1524ffc173440b8a69d5def4 Mon Sep 17 00:00:00 2001 From: Nestor Reyes <108298854+OneWhoNests@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:55:38 -0400 Subject: [PATCH 1/7] fix: support MySQL usernames containing @ MySQL usernames can legally contain "@" (e.g. an account named after an email address), but the connector's user@host composite ID was built as "user@host" and then reassembled everywhere by naively splitting on every "@" and requiring exactly 2 parts. A username like "someone@orion.com" produced an ID like "someone@orion.com@%", which failed to parse and aborted sync during grant processing with "malformed principal ID". Add client.SplitUserHost, which splits on the *last* "@" instead (MySQL host specs never contain "@", so this is unambiguous), and use it at every user@host parsing site: grant/revoke for databases, tables, columns, routines, servers, roles, users, plus principal-ID parsing in grants listing and user deletion. Also widen the user/host identifier validation regex to allow "@". Co-Authored-By: Claude Sonnet 5 --- pkg/client/columns.go | 12 ++--- pkg/client/databases.go | 20 ++++---- pkg/client/helper.go | 14 +++++- pkg/client/helper_test.go | 96 +++++++++++++++++++++++++++++++++++++++ pkg/client/roles.go | 33 +++++++------- pkg/client/routines.go | 20 ++++---- pkg/client/servers.go | 20 ++++---- pkg/client/tables.go | 20 ++++---- pkg/client/users.go | 20 ++++---- pkg/connector/grants.go | 13 +++--- pkg/connector/user.go | 10 ++-- 11 files changed, 192 insertions(+), 86 deletions(-) create mode 100644 pkg/client/helper_test.go diff --git a/pkg/client/columns.go b/pkg/client/columns.go index 249d5d9f..12cd2b88 100644 --- a/pkg/client/columns.go +++ b/pkg/client/columns.go @@ -85,11 +85,11 @@ func (c *Client) ListColumns(ctx context.Context, parentResourceID *v2.ResourceI // If the privilege is "grant", it grants SELECT, INSERT, UPDATE, and REFERENCES privileges. func (c *Client) GrantColumnPrivilege(ctx context.Context, table string, column string, user string, privilege string) error { - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { + userName, host, err := SplitUserHost(user) + if err != nil { return fmt.Errorf("invalid user format: %s", user) } - userGrant := fmt.Sprintf("%s'@'%s", userSplit[0], userSplit[1]) + userGrant := fmt.Sprintf("%s'@'%s", userName, host) var privileges []string if strings.ToLower(privilege) == "grant" { @@ -120,11 +120,11 @@ func (c *Client) GrantColumnPrivilege(ctx context.Context, table string, column } func (c *Client) RevokeColumnPrivilege(ctx context.Context, table string, column string, user string, privilege string) error { - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { + userName, host, err := SplitUserHost(user) + if err != nil { return fmt.Errorf("invalid user format: %s", user) } - userRevoke := fmt.Sprintf("%s'@'%s", userSplit[0], userSplit[1]) + userRevoke := fmt.Sprintf("%s'@'%s", userName, host) var privileges []string if strings.ToLower(privilege) == "grant" { diff --git a/pkg/client/databases.go b/pkg/client/databases.go index e58e396f..5dd211cc 100644 --- a/pkg/client/databases.go +++ b/pkg/client/databases.go @@ -74,15 +74,15 @@ func (c *Client) ListDatabases(ctx context.Context, pager *Pager) ([]*DbModel, s } func (c *Client) GrantDatabasePrivilege(ctx context.Context, database string, user string, privilege string) error { - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { - return fmt.Errorf("invalid user format, expected user@host") + userName, host, err := SplitUserHost(user) + if err != nil { + return fmt.Errorf("invalid user format, expected user@host: %w", err) } - userEsc, err := escapeMySQLUserHost(userSplit[0]) + userEsc, err := escapeMySQLUserHost(userName) if err != nil { return err } - hostEsc, err := escapeMySQLUserHost(userSplit[1]) + hostEsc, err := escapeMySQLUserHost(host) if err != nil { return err } @@ -99,15 +99,15 @@ func (c *Client) GrantDatabasePrivilege(ctx context.Context, database string, us } func (c *Client) RevokeDatabasePrivilege(ctx context.Context, database string, user string, privilege string) error { - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { - return fmt.Errorf("invalid user format, expected user@host") + userName, host, err := SplitUserHost(user) + if err != nil { + return fmt.Errorf("invalid user format, expected user@host: %w", err) } - userEsc, err := escapeMySQLUserHost(userSplit[0]) + userEsc, err := escapeMySQLUserHost(userName) if err != nil { return err } - hostEsc, err := escapeMySQLUserHost(userSplit[1]) + hostEsc, err := escapeMySQLUserHost(host) if err != nil { return err } diff --git a/pkg/client/helper.go b/pkg/client/helper.go index 0b29f19c..ca1ce72d 100644 --- a/pkg/client/helper.go +++ b/pkg/client/helper.go @@ -21,7 +21,7 @@ func escapeMySQLIdent(ident string) (string, error) { } // Helper for user/host. -var validUserHost = regexp.MustCompile(`^[a-zA-Z0-9_%\\.\\-]+$`) +var validUserHost = regexp.MustCompile(`^[a-zA-Z0-9_%\\.@\-]+$`) func escapeMySQLUserHost(ident string) (string, error) { if !validUserHost.MatchString(ident) { @@ -29,3 +29,15 @@ func escapeMySQLUserHost(ident string) (string, error) { } return ident, nil } + +// SplitUserHost splits a "name@host" identifier into its name and host parts. +// Names (MySQL usernames or role names) may themselves legally contain "@", +// but MySQL host specifications (hostnames, IPs, netmasks, or "%" wildcards) +// never do, so splitting on the last "@" unambiguously recovers both parts. +func SplitUserHost(s string) (string, string, error) { + idx := strings.LastIndex(s, "@") + if idx <= 0 || idx == len(s)-1 { + return "", "", fmt.Errorf("invalid user@host format: %s", s) + } + return s[:idx], s[idx+1:], nil +} diff --git a/pkg/client/helper_test.go b/pkg/client/helper_test.go new file mode 100644 index 00000000..b8d3e696 --- /dev/null +++ b/pkg/client/helper_test.go @@ -0,0 +1,96 @@ +package client + +import ( + "testing" +) + +func Test_SplitUserHost(t *testing.T) { + type want struct { + user string + host string + } + tests := []struct { + name string + in string + want want + wantErr bool + }{ + { + name: "simple user and host", + in: "someone@%", + want: want{user: "someone", host: "%"}, + }, + { + name: "username containing @", + in: "someone@orion.com@%", + want: want{user: "someone@orion.com", host: "%"}, + }, + { + name: "username containing multiple @", + in: "a@b@c@10.0.0.1", + want: want{user: "a@b@c", host: "10.0.0.1"}, + }, + { + name: "collapsed comma-separated hosts", + in: "someone@orion.com@localhost,%", + want: want{user: "someone@orion.com", host: "localhost,%"}, + }, + { + name: "no @", + in: "someone", + wantErr: true, + }, + { + name: "empty user", + in: "@%", + wantErr: true, + }, + { + name: "empty host", + in: "someone@", + wantErr: true, + }, + { + name: "empty string", + in: "", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + user, host, err := SplitUserHost(tt.in) + if (err != nil) != tt.wantErr { + t.Fatalf("SplitUserHost(%q) error = %v, wantErr %v", tt.in, err, tt.wantErr) + } + if tt.wantErr { + return + } + if user != tt.want.user || host != tt.want.host { + t.Errorf("SplitUserHost(%q) = (%q, %q), want (%q, %q)", tt.in, user, host, tt.want.user, tt.want.host) + } + }) + } +} + +func Test_escapeMySQLUserHost(t *testing.T) { + tests := []struct { + name string + in string + wantErr bool + }{ + {name: "plain username", in: "someone"}, + {name: "username with @", in: "someone@orion.com"}, + {name: "wildcard host", in: "%"}, + {name: "hostname", in: "%.example.com"}, + {name: "quote injection attempt", in: "someone' OR '1'='1", wantErr: true}, + {name: "space", in: "some one", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := escapeMySQLUserHost(tt.in) + if (err != nil) != tt.wantErr { + t.Errorf("escapeMySQLUserHost(%q) error = %v, wantErr %v", tt.in, err, tt.wantErr) + } + }) + } +} diff --git a/pkg/client/roles.go b/pkg/client/roles.go index 4bd53e99..3dff05ea 100644 --- a/pkg/client/roles.go +++ b/pkg/client/roles.go @@ -3,33 +3,32 @@ package client import ( "context" "fmt" - "strings" ) func (c *Client) GrantRolePrivilege(ctx context.Context, role, user, privilege string) error { - roleParts := strings.Split(role, "@") - if len(roleParts) != 2 { + roleName, roleHostRaw, err := SplitUserHost(role) + if err != nil { return fmt.Errorf("invalid role format: %s", role) } - userParts := strings.Split(user, "@") - if len(userParts) != 2 { + userName, userHostRaw, err := SplitUserHost(user) + if err != nil { return fmt.Errorf("invalid user format: %s", user) } - roleUser, err := escapeMySQLUserHost(roleParts[0]) + roleUser, err := escapeMySQLUserHost(roleName) if err != nil { return err } - roleHost, err := escapeMySQLUserHost(roleParts[1]) + roleHost, err := escapeMySQLUserHost(roleHostRaw) if err != nil { return err } - targetUser, err := escapeMySQLUserHost(userParts[0]) + targetUser, err := escapeMySQLUserHost(userName) if err != nil { return err } - targetHost, err := escapeMySQLUserHost(userParts[1]) + targetHost, err := escapeMySQLUserHost(userHostRaw) if err != nil { return err } @@ -53,29 +52,29 @@ func (c *Client) GrantRolePrivilege(ctx context.Context, role, user, privilege s } func (c *Client) RevokeRolePrivilege(ctx context.Context, role, user, privilege string) error { - roleParts := strings.Split(role, "@") - if len(roleParts) != 2 { + roleName, roleHostRaw, err := SplitUserHost(role) + if err != nil { return fmt.Errorf("invalid role format: %s", role) } - userParts := strings.Split(user, "@") - if len(userParts) != 2 { + userName, userHostRaw, err := SplitUserHost(user) + if err != nil { return fmt.Errorf("invalid user format: %s", user) } - roleUser, err := escapeMySQLUserHost(roleParts[0]) + roleUser, err := escapeMySQLUserHost(roleName) if err != nil { return err } - roleHost, err := escapeMySQLUserHost(roleParts[1]) + roleHost, err := escapeMySQLUserHost(roleHostRaw) if err != nil { return err } - targetUser, err := escapeMySQLUserHost(userParts[0]) + targetUser, err := escapeMySQLUserHost(userName) if err != nil { return err } - targetHost, err := escapeMySQLUserHost(userParts[1]) + targetHost, err := escapeMySQLUserHost(userHostRaw) if err != nil { return err } diff --git a/pkg/client/routines.go b/pkg/client/routines.go index c0b72875..9998955c 100644 --- a/pkg/client/routines.go +++ b/pkg/client/routines.go @@ -98,15 +98,15 @@ func (c *Client) GrantRoutinePrivilege(ctx context.Context, privilege string, sc return err } - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { - return fmt.Errorf("invalid user format, expected user@host") + userName, host, err := SplitUserHost(user) + if err != nil { + return fmt.Errorf("invalid user format, expected user@host: %w", err) } - userEsc, err := escapeMySQLUserHost(userSplit[0]) + userEsc, err := escapeMySQLUserHost(userName) if err != nil { return err } - hostEsc, err := escapeMySQLUserHost(userSplit[1]) + hostEsc, err := escapeMySQLUserHost(host) if err != nil { return err } @@ -134,15 +134,15 @@ func (c *Client) RevokeRoutinePrivilege(ctx context.Context, privilege string, s return err } - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { - return fmt.Errorf("invalid user format, expected user@host") + userName, host, err := SplitUserHost(user) + if err != nil { + return fmt.Errorf("invalid user format, expected user@host: %w", err) } - userEsc, err := escapeMySQLUserHost(userSplit[0]) + userEsc, err := escapeMySQLUserHost(userName) if err != nil { return err } - hostEsc, err := escapeMySQLUserHost(userSplit[1]) + hostEsc, err := escapeMySQLUserHost(host) if err != nil { return err } diff --git a/pkg/client/servers.go b/pkg/client/servers.go index 42101df1..a7a80c66 100644 --- a/pkg/client/servers.go +++ b/pkg/client/servers.go @@ -40,15 +40,15 @@ func (c *Client) ExecContext(ctx context.Context, query string) (sql.Result, err } func (c *Client) GrantServerPrivilege(ctx context.Context, user string, privilege string) error { - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { - return fmt.Errorf("invalid user format, expected user@host") + userName, host, err := SplitUserHost(user) + if err != nil { + return fmt.Errorf("invalid user format, expected user@host: %w", err) } - userEsc, err := escapeMySQLUserHost(userSplit[0]) + userEsc, err := escapeMySQLUserHost(userName) if err != nil { return err } - hostEsc, err := escapeMySQLUserHost(userSplit[1]) + hostEsc, err := escapeMySQLUserHost(host) if err != nil { return err } @@ -60,15 +60,15 @@ func (c *Client) GrantServerPrivilege(ctx context.Context, user string, privileg } func (c *Client) RevokeServerPrivilege(ctx context.Context, user string, privilege string) error { - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { - return fmt.Errorf("invalid user format, expected user@host") + userName, host, err := SplitUserHost(user) + if err != nil { + return fmt.Errorf("invalid user format, expected user@host: %w", err) } - userEsc, err := escapeMySQLUserHost(userSplit[0]) + userEsc, err := escapeMySQLUserHost(userName) if err != nil { return err } - hostEsc, err := escapeMySQLUserHost(userSplit[1]) + hostEsc, err := escapeMySQLUserHost(host) if err != nil { return err } diff --git a/pkg/client/tables.go b/pkg/client/tables.go index 0c2f0dd5..21267f1d 100644 --- a/pkg/client/tables.go +++ b/pkg/client/tables.go @@ -83,15 +83,15 @@ func (c *Client) ListTables(ctx context.Context, parentResourceID *v2.ResourceId } func (c *Client) GrantTablePrivilege(ctx context.Context, table string, user string, privilege string) error { - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { - return fmt.Errorf("invalid user format, expected user@host") + userName, host, err := SplitUserHost(user) + if err != nil { + return fmt.Errorf("invalid user format, expected user@host: %w", err) } - userEsc, err := escapeMySQLUserHost(userSplit[0]) + userEsc, err := escapeMySQLUserHost(userName) if err != nil { return err } - hostEsc, err := escapeMySQLUserHost(userSplit[1]) + hostEsc, err := escapeMySQLUserHost(host) if err != nil { return err } @@ -108,15 +108,15 @@ func (c *Client) GrantTablePrivilege(ctx context.Context, table string, user str } func (c *Client) RevokeTablePrivilege(ctx context.Context, table string, user string, privilege string) error { - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { - return fmt.Errorf("invalid user format, expected user@host") + userName, host, err := SplitUserHost(user) + if err != nil { + return fmt.Errorf("invalid user format, expected user@host: %w", err) } - userEsc, err := escapeMySQLUserHost(userSplit[0]) + userEsc, err := escapeMySQLUserHost(userName) if err != nil { return err } - hostEsc, err := escapeMySQLUserHost(userSplit[1]) + hostEsc, err := escapeMySQLUserHost(host) if err != nil { return err } diff --git a/pkg/client/users.go b/pkg/client/users.go index ee3b177a..10ad4cfa 100644 --- a/pkg/client/users.go +++ b/pkg/client/users.go @@ -228,15 +228,15 @@ func (c *Client) GetHost(ctx context.Context) (string, error) { } func (c *Client) CreateUser(ctx context.Context, user string, password string) error { - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { - return fmt.Errorf("invalid user format, expected user@host") + userName, host, err := SplitUserHost(user) + if err != nil { + return fmt.Errorf("invalid user format, expected user@host: %w", err) } - userEsc, err := escapeMySQLUserHost(userSplit[0]) + userEsc, err := escapeMySQLUserHost(userName) if err != nil { return err } - hostEsc, err := escapeMySQLUserHost(userSplit[1]) + hostEsc, err := escapeMySQLUserHost(host) if err != nil { return err } @@ -248,15 +248,15 @@ func (c *Client) CreateUser(ctx context.Context, user string, password string) e } func (c *Client) DropUser(ctx context.Context, user string) error { - userSplit := strings.Split(user, "@") - if len(userSplit) != 2 { - return fmt.Errorf("invalid user format, expected user@host") + userName, host, err := SplitUserHost(user) + if err != nil { + return fmt.Errorf("invalid user format, expected user@host: %w", err) } - userEsc, err := escapeMySQLUserHost(userSplit[0]) + userEsc, err := escapeMySQLUserHost(userName) if err != nil { return err } - hostEsc, err := escapeMySQLUserHost(userSplit[1]) + hostEsc, err := escapeMySQLUserHost(host) if err != nil { return err } diff --git a/pkg/connector/grants.go b/pkg/connector/grants.go index 71369500..7721b66d 100644 --- a/pkg/connector/grants.go +++ b/pkg/connector/grants.go @@ -22,19 +22,18 @@ func grantsForUserOrRole( var ret []*v2.Grant grantMap := make(map[string]struct{}) - parts := strings.Split(strings.TrimPrefix(resource.Id.Resource, fmt.Sprintf("%s:", resource.Id.ResourceType)), "@") - if len(parts) != 2 { - return nil, fmt.Errorf("malformed principal ID") + idStr := strings.TrimPrefix(resource.Id.Resource, fmt.Sprintf("%s:", resource.Id.ResourceType)) + user, hostPart, err := client.SplitUserHost(idStr) + if err != nil { + return nil, fmt.Errorf("malformed principal ID: %w", err) } - user := parts[0] - hosts := []string{parts[1]} + hosts := []string{hostPart} // If we are collapsing users, we will want to split the host portion of the ID to inspect each real user's grants if collapseUsers { - hosts = strings.Split(parts[1], ",") + hosts = strings.Split(hostPart, ",") } - var err error for _, host := range hosts { err = listGlobalGrants(ctx, resource.ParentResourceId, user, host, grantMap, c) if err != nil { diff --git a/pkg/connector/user.go b/pkg/connector/user.go index cedef0dc..fb5cff7d 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -201,14 +201,14 @@ func (s *userSyncer) Delete(ctx context.Context, resourceId *v2.ResourceId) (ann return nil, fmt.Errorf("baton-mysql: non-user resource passed to user delete") } userID := strings.TrimSpace(strings.Split(resourceId.Resource, ":")[1]) - parts := strings.Split(userID, "@") - if len(parts) != 2 { - return nil, fmt.Errorf("baton-mysql: invalid user ID format, expected 'user@host'") + userPart, hostPart, err := client.SplitUserHost(userID) + if err != nil { + return nil, fmt.Errorf("baton-mysql: invalid user ID format, expected 'user@host': %w", err) } - user, host := strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) + user, host := strings.TrimSpace(userPart), strings.TrimSpace(hostPart) userStr := fmt.Sprintf("%s@%s", user, host) - err := s.client.DropUser(ctx, userStr) + err = s.client.DropUser(ctx, userStr) if err != nil { return nil, fmt.Errorf("drop user failed: %w", err) } From 34f90b7794bcd2fcb89d4a2f90aa1365f033dc56 Mon Sep 17 00:00:00 2001 From: Nestor Reyes <108298854+OneWhoNests@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:55:38 -0400 Subject: [PATCH 2/7] fix: address review feedback on @-in-username PR - GrantColumnPrivilege/RevokeColumnPrivilege now validate user/host via escapeMySQLUserHost, matching every other converted call site. They previously interpolated the raw split values straight into the GRANT/ REVOKE statement, which was a SQL injection vector for a user/host containing a quote. Also wrap the SplitUserHost error with %w instead of dropping it. - validUserHost no longer matches a literal backslash. Combined with the '%s'@'%s' quoting used throughout, a name ending in "\" could escape the closing quote under MySQL's default (non-NO_BACKSLASH_ESCAPES) sql_mode. Backslash was already allowed before this PR; this was a good moment to drop it while rewriting the character class. - userSyncer.Delete now derives the composite ID via TrimPrefix on the resource type, matching grantsForUserOrRole, instead of strings.Split(...)[1], which panics if the ID has no ":" and silently truncates names containing ":". Co-Authored-By: Claude Sonnet 5 --- pkg/client/columns.go | 24 ++++++++++++++++++++---- pkg/client/helper.go | 2 +- pkg/client/helper_test.go | 1 + pkg/connector/user.go | 2 +- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/pkg/client/columns.go b/pkg/client/columns.go index 12cd2b88..2d73162b 100644 --- a/pkg/client/columns.go +++ b/pkg/client/columns.go @@ -87,9 +87,17 @@ func (c *Client) ListColumns(ctx context.Context, parentResourceID *v2.ResourceI func (c *Client) GrantColumnPrivilege(ctx context.Context, table string, column string, user string, privilege string) error { userName, host, err := SplitUserHost(user) if err != nil { - return fmt.Errorf("invalid user format: %s", user) + return fmt.Errorf("invalid user format: %s: %w", user, err) } - userGrant := fmt.Sprintf("%s'@'%s", userName, host) + userEsc, err := escapeMySQLUserHost(userName) + if err != nil { + return err + } + hostEsc, err := escapeMySQLUserHost(host) + if err != nil { + return err + } + userGrant := fmt.Sprintf("%s'@'%s", userEsc, hostEsc) var privileges []string if strings.ToLower(privilege) == "grant" { @@ -122,9 +130,17 @@ func (c *Client) GrantColumnPrivilege(ctx context.Context, table string, column func (c *Client) RevokeColumnPrivilege(ctx context.Context, table string, column string, user string, privilege string) error { userName, host, err := SplitUserHost(user) if err != nil { - return fmt.Errorf("invalid user format: %s", user) + return fmt.Errorf("invalid user format: %s: %w", user, err) + } + userEsc, err := escapeMySQLUserHost(userName) + if err != nil { + return err + } + hostEsc, err := escapeMySQLUserHost(host) + if err != nil { + return err } - userRevoke := fmt.Sprintf("%s'@'%s", userName, host) + userRevoke := fmt.Sprintf("%s'@'%s", userEsc, hostEsc) var privileges []string if strings.ToLower(privilege) == "grant" { diff --git a/pkg/client/helper.go b/pkg/client/helper.go index ca1ce72d..62782192 100644 --- a/pkg/client/helper.go +++ b/pkg/client/helper.go @@ -21,7 +21,7 @@ func escapeMySQLIdent(ident string) (string, error) { } // Helper for user/host. -var validUserHost = regexp.MustCompile(`^[a-zA-Z0-9_%\\.@\-]+$`) +var validUserHost = regexp.MustCompile(`^[a-zA-Z0-9_%.@\-]+$`) func escapeMySQLUserHost(ident string) (string, error) { if !validUserHost.MatchString(ident) { diff --git a/pkg/client/helper_test.go b/pkg/client/helper_test.go index b8d3e696..b5f97ac7 100644 --- a/pkg/client/helper_test.go +++ b/pkg/client/helper_test.go @@ -84,6 +84,7 @@ func Test_escapeMySQLUserHost(t *testing.T) { {name: "hostname", in: "%.example.com"}, {name: "quote injection attempt", in: "someone' OR '1'='1", wantErr: true}, {name: "space", in: "some one", wantErr: true}, + {name: "trailing backslash", in: `someone\`, wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/connector/user.go b/pkg/connector/user.go index fb5cff7d..b3d8d852 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -200,7 +200,7 @@ func (s *userSyncer) Delete(ctx context.Context, resourceId *v2.ResourceId) (ann if resourceId.ResourceType != resourceTypeUser.Id { return nil, fmt.Errorf("baton-mysql: non-user resource passed to user delete") } - userID := strings.TrimSpace(strings.Split(resourceId.Resource, ":")[1]) + userID := strings.TrimSpace(strings.TrimPrefix(resourceId.Resource, fmt.Sprintf("%s:", resourceId.ResourceType))) userPart, hostPart, err := client.SplitUserHost(userID) if err != nil { return nil, fmt.Errorf("baton-mysql: invalid user ID format, expected 'user@host': %w", err) From bc8d0c96e47b898689aabd48e1f09de7319a79df Mon Sep 17 00:00:00 2001 From: Nestor Reyes <108298854+OneWhoNests@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:55:38 -0400 Subject: [PATCH 3/7] fix: resolve staticcheck SA1019 deprecation warnings in user.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rs.WithUserProfile and rs.WithStatus are deprecated (profile/status moved from UserTrait to a Resource-level attribute). CI's lint check flags these regardless of whether the resource-level mirroring happens, which only applies when going through WithUserTrait/NewUserResource — this connector builds *v2.Resource via a struct literal instead, so switch to setting the resource-level fields directly via rs.WithResourceProfile/rs.WithResourceStatus. Verified no SA1019 findings remain repo-wide (golangci-lint), and that the resulting resource has HasProfile()/HasStatus() populated correctly via a standalone check against the real SDK types. Co-Authored-By: Claude Sonnet 5 --- pkg/connector/user.go | 54 ++++++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/pkg/connector/user.go b/pkg/connector/user.go index b3d8d852..34dfae76 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -44,21 +44,14 @@ func (s *userSyncer) List( var annos annotations.Annotations ut, err := rs.NewUserTrait( - rs.WithUserProfile(map[string]interface{}{ - "user": u.User, - "host": u.Host, - "first_name": fmt.Sprintf("%s@%s", u.User, u.Host), - "user_id": fmt.Sprintf("%s@%s", u.User, u.Host), - }), rs.WithUserLogin(u.User), - rs.WithStatus(v2.UserTrait_Status_STATUS_ENABLED), ) if err != nil { return nil, "", nil, err } annos.Update(ut) - ret = append(ret, &v2.Resource{ + resource := &v2.Resource{ DisplayName: fmt.Sprintf("%s@%s", u.User, u.Host), Id: &v2.ResourceId{ ResourceType: s.resourceType.Id, @@ -66,7 +59,23 @@ func (s *userSyncer) List( }, Annotations: annos, ParentResourceId: parentResourceID, - }) + } + + err = rs.WithResourceProfile(map[string]interface{}{ + "user": u.User, + "host": u.Host, + "first_name": fmt.Sprintf("%s@%s", u.User, u.Host), + "user_id": fmt.Sprintf("%s@%s", u.User, u.Host), + })(resource) + if err != nil { + return nil, "", nil, err + } + err = rs.WithResourceStatus(v2.Status_RESOURCE_STATUS_ENABLED, "")(resource) + if err != nil { + return nil, "", nil, err + } + + ret = append(ret, resource) } return ret, nextPageToken, nil, nil @@ -169,14 +178,7 @@ func (o *userSyncer) CreateAccount( func parseIntoUserResource(user *client.User, parent *v2.ResourceId) (*v2.Resource, error) { ut, err := rs.NewUserTrait( - rs.WithUserProfile(map[string]interface{}{ - "user": user.User, - "host": user.Host, - "first_name": fmt.Sprintf("%s@%s", user.User, user.Host), - "user_id": fmt.Sprintf("%s@%s", user.User, user.Host), - }), rs.WithUserLogin(user.User), - rs.WithStatus(v2.UserTrait_Status_STATUS_ENABLED), ) if err != nil { return nil, err @@ -185,7 +187,7 @@ func parseIntoUserResource(user *client.User, parent *v2.ResourceId) (*v2.Resour annos := annotations.Annotations{} annos.Update(ut) - return &v2.Resource{ + resource := &v2.Resource{ DisplayName: fmt.Sprintf("%s@%s", user.User, user.Host), Id: &v2.ResourceId{ ResourceType: resourceTypeUser.Id, @@ -193,7 +195,23 @@ func parseIntoUserResource(user *client.User, parent *v2.ResourceId) (*v2.Resour }, Annotations: annos, ParentResourceId: parent, - }, nil + } + + err = rs.WithResourceProfile(map[string]interface{}{ + "user": user.User, + "host": user.Host, + "first_name": fmt.Sprintf("%s@%s", user.User, user.Host), + "user_id": fmt.Sprintf("%s@%s", user.User, user.Host), + })(resource) + if err != nil { + return nil, err + } + err = rs.WithResourceStatus(v2.Status_RESOURCE_STATUS_ENABLED, "")(resource) + if err != nil { + return nil, err + } + + return resource, nil } func (s *userSyncer) Delete(ctx context.Context, resourceId *v2.ResourceId) (annotations.Annotations, error) { From 76892bed124fccb3d131dda76a97444cb6e6f446 Mon Sep 17 00:00:00 2001 From: Nestor Reyes <108298854+OneWhoNests@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:55:38 -0400 Subject: [PATCH 4/7] refactor: build user resources via rs.NewUserResource Address review feedback: List() and parseIntoUserResource() previously hand-built *v2.Resource via a struct literal, duplicating the same profile-map/status boilerplate in two places and skipping the SDK's NewUserResource/WithUserTrait helper entirely. Consolidated List() to just call parseIntoUserResource() per user, and rewrote parseIntoUserResource() to build through rs.NewUserResource with WithParentResourceID/WithResourceProfile/WithResourceStatus. Verified against a live MySQL container that resources, parent linkage, display name, and the @-in-username grants still round-trip identically after the refactor. Co-Authored-By: Claude Sonnet 5 --- pkg/connector/user.go | 79 ++++++++----------------------------------- 1 file changed, 14 insertions(+), 65 deletions(-) diff --git a/pkg/connector/user.go b/pkg/connector/user.go index 34dfae76..16f97ba8 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -41,40 +41,10 @@ func (s *userSyncer) List( var ret []*v2.Resource for _, u := range users { - var annos annotations.Annotations - - ut, err := rs.NewUserTrait( - rs.WithUserLogin(u.User), - ) - if err != nil { - return nil, "", nil, err - } - annos.Update(ut) - - resource := &v2.Resource{ - DisplayName: fmt.Sprintf("%s@%s", u.User, u.Host), - Id: &v2.ResourceId{ - ResourceType: s.resourceType.Id, - Resource: u.GetID(), - }, - Annotations: annos, - ParentResourceId: parentResourceID, - } - - err = rs.WithResourceProfile(map[string]interface{}{ - "user": u.User, - "host": u.Host, - "first_name": fmt.Sprintf("%s@%s", u.User, u.Host), - "user_id": fmt.Sprintf("%s@%s", u.User, u.Host), - })(resource) + resource, err := parseIntoUserResource(u, parentResourceID) if err != nil { return nil, "", nil, err } - err = rs.WithResourceStatus(v2.Status_RESOURCE_STATUS_ENABLED, "")(resource) - if err != nil { - return nil, "", nil, err - } - ret = append(ret, resource) } @@ -177,41 +147,20 @@ func (o *userSyncer) CreateAccount( } func parseIntoUserResource(user *client.User, parent *v2.ResourceId) (*v2.Resource, error) { - ut, err := rs.NewUserTrait( - rs.WithUserLogin(user.User), + return rs.NewUserResource( + fmt.Sprintf("%s@%s", user.User, user.Host), + resourceTypeUser, + user.GetID(), + []rs.UserTraitOption{rs.WithUserLogin(user.User)}, + rs.WithParentResourceID(parent), + rs.WithResourceProfile(map[string]interface{}{ + "user": user.User, + "host": user.Host, + "first_name": fmt.Sprintf("%s@%s", user.User, user.Host), + "user_id": fmt.Sprintf("%s@%s", user.User, user.Host), + }), + rs.WithResourceStatus(v2.Status_RESOURCE_STATUS_ENABLED, ""), ) - if err != nil { - return nil, err - } - - annos := annotations.Annotations{} - annos.Update(ut) - - resource := &v2.Resource{ - DisplayName: fmt.Sprintf("%s@%s", user.User, user.Host), - Id: &v2.ResourceId{ - ResourceType: resourceTypeUser.Id, - Resource: user.GetID(), - }, - Annotations: annos, - ParentResourceId: parent, - } - - err = rs.WithResourceProfile(map[string]interface{}{ - "user": user.User, - "host": user.Host, - "first_name": fmt.Sprintf("%s@%s", user.User, user.Host), - "user_id": fmt.Sprintf("%s@%s", user.User, user.Host), - })(resource) - if err != nil { - return nil, err - } - err = rs.WithResourceStatus(v2.Status_RESOURCE_STATUS_ENABLED, "")(resource) - if err != nil { - return nil, err - } - - return resource, nil } func (s *userSyncer) Delete(ctx context.Context, resourceId *v2.ResourceId) (annotations.Annotations, error) { From 321d1628343a6d83d6796bb9475789ae3eb5c6bc Mon Sep 17 00:00:00 2001 From: Nestor Reyes <108298854+OneWhoNests@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:55:38 -0400 Subject: [PATCH 5/7] fix: allow empty username in SplitUserHost, wrap role/user split errors SplitUserHost rejected idx <= 0, which also rejected the empty-name case (idx == 0) -- but MySQL's anonymous account is a real, valid entity of the form ''@'host'. The old strings.Split-based check accepted it (split on "@" yields ["", host]), so this was a regression: default MySQL/MariaDB installs ship an anonymous account, and grantsForUserOrRole would now fail the entire sync on it. Only reject when there's no "@" at all (idx < 0) or the host half is empty. Also wrap the SplitUserHost error with %w in GrantRolePrivilege/ RevokeRolePrivilege instead of discarding it, matching every other converted call site. Verified against a live MySQL 8.0 container with an actual anonymous account (''@'localhost'): sync succeeds and its grant is correctly attributed to principal "@localhost". Co-Authored-By: Claude Sonnet 5 --- pkg/client/helper.go | 3 ++- pkg/client/helper_test.go | 6 +++--- pkg/client/roles.go | 8 ++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/client/helper.go b/pkg/client/helper.go index 62782192..1dfb906d 100644 --- a/pkg/client/helper.go +++ b/pkg/client/helper.go @@ -36,7 +36,8 @@ func escapeMySQLUserHost(ident string) (string, error) { // never do, so splitting on the last "@" unambiguously recovers both parts. func SplitUserHost(s string) (string, string, error) { idx := strings.LastIndex(s, "@") - if idx <= 0 || idx == len(s)-1 { + // An empty name is valid: MySQL's anonymous account is ''@'host'. + if idx < 0 || idx == len(s)-1 { return "", "", fmt.Errorf("invalid user@host format: %s", s) } return s[:idx], s[idx+1:], nil diff --git a/pkg/client/helper_test.go b/pkg/client/helper_test.go index b5f97ac7..79b65af4 100644 --- a/pkg/client/helper_test.go +++ b/pkg/client/helper_test.go @@ -41,9 +41,9 @@ func Test_SplitUserHost(t *testing.T) { wantErr: true, }, { - name: "empty user", - in: "@%", - wantErr: true, + name: "empty user (MySQL anonymous account)", + in: "@%", + want: want{user: "", host: "%"}, }, { name: "empty host", diff --git a/pkg/client/roles.go b/pkg/client/roles.go index 3dff05ea..23246192 100644 --- a/pkg/client/roles.go +++ b/pkg/client/roles.go @@ -8,12 +8,12 @@ import ( func (c *Client) GrantRolePrivilege(ctx context.Context, role, user, privilege string) error { roleName, roleHostRaw, err := SplitUserHost(role) if err != nil { - return fmt.Errorf("invalid role format: %s", role) + return fmt.Errorf("invalid role format: %s: %w", role, err) } userName, userHostRaw, err := SplitUserHost(user) if err != nil { - return fmt.Errorf("invalid user format: %s", user) + return fmt.Errorf("invalid user format: %s: %w", user, err) } roleUser, err := escapeMySQLUserHost(roleName) @@ -54,12 +54,12 @@ func (c *Client) GrantRolePrivilege(ctx context.Context, role, user, privilege s func (c *Client) RevokeRolePrivilege(ctx context.Context, role, user, privilege string) error { roleName, roleHostRaw, err := SplitUserHost(role) if err != nil { - return fmt.Errorf("invalid role format: %s", role) + return fmt.Errorf("invalid role format: %s: %w", role, err) } userName, userHostRaw, err := SplitUserHost(user) if err != nil { - return fmt.Errorf("invalid user format: %s", user) + return fmt.Errorf("invalid user format: %s: %w", user, err) } roleUser, err := escapeMySQLUserHost(roleName) From 8abb09790ccacf7608b70c2c4de50226da175f32 Mon Sep 17 00:00:00 2001 From: Nestor Reyes <108298854+OneWhoNests@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:55:38 -0400 Subject: [PATCH 6/7] fix: allow empty username in escapeMySQLUserHost for anonymous accounts SplitUserHost now accepts the empty username of MySQL's anonymous account (''@'host'), but escapeMySQLUserHost's regex still required one-or-more characters, so Grant/Revoke/CreateUser/DropUser against that account failed one step later with "invalid user/host: ".// Every call site feeds escapeMySQLUserHost values derived from SplitUserHost, which already guarantees the host half is non-empty, so loosening the regex to zero-or-more only ever affects the anonymous account's empty username -- it can't accidentally allow an empty host. Verified against a live MySQL 8.0 container: GrantDatabasePrivilege, RevokeDatabasePrivilege, and DropUser all now succeed against a real ''@'localhost' account. Co-Authored-By: Claude Sonnet 5 --- pkg/client/helper.go | 7 +++++-- pkg/client/helper_test.go | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/client/helper.go b/pkg/client/helper.go index 1dfb906d..f75bd16d 100644 --- a/pkg/client/helper.go +++ b/pkg/client/helper.go @@ -20,8 +20,11 @@ func escapeMySQLIdent(ident string) (string, error) { return strings.Join(parts, "."), nil } -// Helper for user/host. -var validUserHost = regexp.MustCompile(`^[a-zA-Z0-9_%.@\-]+$`) +// Helper for user/host. Empty is allowed: every caller derives ident via +// SplitUserHost, which guarantees the host half is always non-empty, so an +// empty string here can only be the username of MySQL's anonymous account +// (''@'host'). +var validUserHost = regexp.MustCompile(`^[a-zA-Z0-9_%.@\-]*$`) func escapeMySQLUserHost(ident string) (string, error) { if !validUserHost.MatchString(ident) { diff --git a/pkg/client/helper_test.go b/pkg/client/helper_test.go index 79b65af4..d2a0dea6 100644 --- a/pkg/client/helper_test.go +++ b/pkg/client/helper_test.go @@ -78,6 +78,7 @@ func Test_escapeMySQLUserHost(t *testing.T) { in string wantErr bool }{ + {name: "empty (MySQL anonymous account username)", in: ""}, {name: "plain username", in: "someone"}, {name: "username with @", in: "someone@orion.com"}, {name: "wildcard host", in: "%"}, From ea0c493e9756dbc1bacb25c9fd58334d98902ff4 Mon Sep 17 00:00:00 2001 From: Nestor Reyes <108298854+OneWhoNests@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:07:25 -0400 Subject: [PATCH 7/7] fix: CreateAccount UserType/empty-name guards, allow IPv6 and netmask hosts Three findings from review: - CreateAccount built client.User without UserType, so GetID() returned ":user@host" instead of "user:user@host". The old strings.Split(id, ":")[1] in Delete tolerated that; TrimPrefix does not, so the ID stayed ":user@host", SplitUserHost yielded ":user", and escapeMySQLUserHost rejected the ":" -- deleting a freshly provisioned account failed. The malformed ID was a pre-existing problem in its own right; setting UserType fixes both. - CreateAccount took username from the account profile with only a type assertion. Now that escapeMySQLUserHost accepts the empty string, an empty username would provision MySQL's anonymous account (''@'host'). Guard against it: empty names are legitimate to read and delete, never to create. - validUserHost rejected ":" and "/", which are legal in MySQL host specs -- IPv6 literals (the stock root@::1) and netmask forms (198.51.100.0/255.255.255.0). Such accounts synced but every grant/revoke/drop against them failed with "invalid user/host". Both characters are inert inside the single-quoted '%s'@'%s' the callers build; "'" and "\" remain excluded. Verified against a live MySQL 8.0 container: grant/revoke round-trips for a v6user@::1 and a netuser@198.51.100.0/255.255.255.0 account, the CreateAccount composite-ID round trip through to DropUser, and that quote-injection and trailing-backslash inputs are still rejected. Full sync over all four edge-case account shapes exits clean. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/client/helper.go | 7 +++++-- pkg/client/helper_test.go | 20 ++++++++++++++++++++ pkg/connector/user.go | 15 ++++++++++++--- 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/pkg/client/helper.go b/pkg/client/helper.go index f75bd16d..39f49ea9 100644 --- a/pkg/client/helper.go +++ b/pkg/client/helper.go @@ -23,8 +23,11 @@ func escapeMySQLIdent(ident string) (string, error) { // Helper for user/host. Empty is allowed: every caller derives ident via // SplitUserHost, which guarantees the host half is always non-empty, so an // empty string here can only be the username of MySQL's anonymous account -// (''@'host'). -var validUserHost = regexp.MustCompile(`^[a-zA-Z0-9_%.@\-]*$`) +// (''@'host'). ":" and "/" are allowed because MySQL host specs include IPv6 +// literals (the stock root@::1) and netmask forms (198.51.100.0/255.255.255.0); +// both are inert inside the single-quoted '%s'@'%s' the callers build. "'" and +// "\" stay excluded, as those are what could break out of that quoting. +var validUserHost = regexp.MustCompile(`^[a-zA-Z0-9_%.@:/\-]*$`) func escapeMySQLUserHost(ident string) (string, error) { if !validUserHost.MatchString(ident) { diff --git a/pkg/client/helper_test.go b/pkg/client/helper_test.go index d2a0dea6..5e118c02 100644 --- a/pkg/client/helper_test.go +++ b/pkg/client/helper_test.go @@ -35,6 +35,21 @@ func Test_SplitUserHost(t *testing.T) { in: "someone@orion.com@localhost,%", want: want{user: "someone@orion.com", host: "localhost,%"}, }, + { + name: "ipv6 loopback host", + in: "root@::1", + want: want{user: "root", host: "::1"}, + }, + { + name: "username with @ and ipv6 host", + in: "someone@orion.com@::1", + want: want{user: "someone@orion.com", host: "::1"}, + }, + { + name: "netmask host", + in: "someone@198.51.100.0/255.255.255.0", + want: want{user: "someone", host: "198.51.100.0/255.255.255.0"}, + }, { name: "no @", in: "someone", @@ -83,6 +98,11 @@ func Test_escapeMySQLUserHost(t *testing.T) { {name: "username with @", in: "someone@orion.com"}, {name: "wildcard host", in: "%"}, {name: "hostname", in: "%.example.com"}, + {name: "ipv4 host", in: "127.0.0.1"}, + {name: "ipv6 loopback host", in: "::1"}, + {name: "ipv6 full host", in: "2001:db8::8a2e:370:7334"}, + {name: "netmask host", in: "198.51.100.0/255.255.255.0"}, + {name: "wildcard octet host", in: "198.51.100.%"}, {name: "quote injection attempt", in: "someone' OR '1'='1", wantErr: true}, {name: "space", in: "some one", wantErr: true}, {name: "trailing backslash", in: `someone\`, wantErr: true}, diff --git a/pkg/connector/user.go b/pkg/connector/user.go index 16f97ba8..e53e25b2 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -107,6 +107,12 @@ func (o *userSyncer) CreateAccount( if !ok { return nil, nil, nil, fmt.Errorf("missing or invalid 'username' in profile") } + // An empty username would create MySQL's anonymous account (''@'host'). + // That is a legitimate entity to read and delete, but never something we + // should provision on request. + if username == "" { + return nil, nil, nil, fmt.Errorf("baton-mysql: 'username' in profile must not be empty") + } host, err := o.client.GetHost(ctx) if err != nil { @@ -124,10 +130,13 @@ func (o *userSyncer) CreateAccount( return nil, nil, nil, fmt.Errorf("create user failed: %w", err) } - // Build resource + // Build resource. UserType must be set: GetID() renders it as the + // ":@" prefix, and omitting it yields ":user@host", + // which every consumer of the composite ID then fails to parse. user := &client.User{ - User: username, - Host: host, + UserType: client.UserType, + User: username, + Host: host, } userResource, err := parseIntoUserResource(user, nil) if err != nil {