Correctly determine $PASS_COLUMN_NAME, Fixing #22

[lewart3]

Better and much simpler logic.

However, needs testing across all supported MariaDB and MySQL versions.

Future: if mysql.global_priv table exists, use that instead:

SELECT JSON_VALUE(Priv, '$.authentication_string') authentication_string FROM mysql.global_priv;

Summary by Sourcery

Simplify and harden detection of the authentication column in the mysql.user table when generating security recommendations.

Bug Fixes:

• Correctly determine the password/authentication column in mysql.user across different MySQL and MariaDB versions, avoiding skipped checks when schemas differ.

Enhancements:

• Replace version-based logic with schema inspection of mysql.user to select the appropriate authentication column and provide clearer feedback when it cannot be determined.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors how the password/authentication column is determined for the mysql.user table by directly inspecting existing columns instead of relying on server version parsing and ad‑hoc shell queries, and improves the skip messaging when no known auth column is found.

Sequence diagram for determining PASS_COLUMN_NAME in security_recommendations

sequenceDiagram
    participant Security_recommendations
    participant select_table_columns_db
    participant MySQL_server

    Security_recommendations->>select_table_columns_db: get mysql_user_columns(mysql, user)
    select_table_columns_db->>MySQL_server: SELECT COLUMN_NAME FROM information_schema.columns WHERE TABLE_SCHEMA = mysql AND TABLE_NAME = user
    MySQL_server-->>select_table_columns_db: column_name list
    select_table_columns_db-->>Security_recommendations: @mysql_user_columns

    alt authentication_string column exists
        Security_recommendations->>Security_recommendations: PASS_COLUMN_NAME = authentication_string
    else Password column exists
        Security_recommendations->>Security_recommendations: PASS_COLUMN_NAME = Password
    else no known auth column
        Security_recommendations->>Security_recommendations: badprint Skipped because could not determine auth column
        Security_recommendations-->>Security_recommendations: return
    end

    Security_recommendations->>Security_recommendations: debugprint Password column = PASS_COLUMN_NAME

Loading

Flow diagram for new PASS_COLUMN_NAME detection logic

flowchart TD
    A[start security_recommendations] --> B[Call select_table_columns_db with mysql and user]
    B --> C[Receive mysql_user_columns]
    C --> D{Does mysql_user_columns contain authentication_string?}
    D -->|yes| E[Set PASS_COLUMN_NAME to authentication_string]
    D -->|no| F{Does mysql_user_columns contain Password?}
    F -->|yes| G[Set PASS_COLUMN_NAME to Password]
    F -->|no| H[badprint Skipped because could not determine auth column]
    H --> I[return from security_recommendations]
    E --> J[debugprint Password column = PASS_COLUMN_NAME]
    G --> J
    J --> K[Continue security checks]

Loading

File-Level Changes

Change	Details	Files
Simplify and harden detection of the authentication/password column in mysql.user	Remove version-based logic and shell calls to information_schema.columns to infer the auth column Use select_table_columns_db('mysql','user') to retrieve the mysql.user column list directly Prefer authentication_string when present; otherwise fall back to Password If no known auth column is found, emit a badprint message and return early from security_recommendations Log the resolved column name via debugprint as before	mysqltuner.pl

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The previous implementation handled the rare but possible case where both authentication_string and password existed by using an IF(plugin=...) expression; the new code always picks authentication_string if present, which may break on instances still relying on the legacy password column, so consider preserving a combined expression when both columns are present.
• The new detection only matches a capitalized Password column; since older MySQL/MariaDB installations sometimes expose password in varying cases, consider making the check case-insensitive or verifying the exact column name(s) returned by select_table_columns_db to avoid false negatives.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The previous implementation handled the rare but possible case where both `authentication_string` and `password` existed by using an `IF(plugin=...)` expression; the new code always picks `authentication_string` if present, which may break on instances still relying on the legacy `password` column, so consider preserving a combined expression when both columns are present.
- The new detection only matches a capitalized `Password` column; since older MySQL/MariaDB installations sometimes expose `password` in varying cases, consider making the check case-insensitive or verifying the exact column name(s) returned by `select_table_columns_db` to avoid false negatives.

## Individual Comments

### Comment 1
<location> `mysqltuner.pl:3312-3315` </location>
<code_context>
-            return;
-        }
+    my @mysql_user_columns = select_table_columns_db( 'mysql', 'user' );
+    if ( grep { /^authentication_string$/msx } @mysql_user_columns ) {
+        $PASS_COLUMN_NAME = 'authentication_string';
     }
+    elsif ( grep { /^Password$/msx } @mysql_user_columns ) {
+        $PASS_COLUMN_NAME = 'Password';
+    }
</code_context>

<issue_to_address>
**issue (bug_risk):** Potential change in behavior when both `authentication_string` and `password` columns exist

The prior query used:

```sql
IF(plugin='mysql_native_password', authentication_string, password)
```

which supported mixed setups where some users still only had `password` populated. The new logic always prefers `authentication_string` when the column exists and never falls back to `password`, so users with only `password` set may now be treated as having empty credentials.

Please consider reinstating a conditional expression (or equivalent query construction) when both columns exist, so that `authentication_string` is preferred for `mysql_native_password` users while still correctly handling accounts that only use `password`.
</issue_to_address>

### Comment 2
<location> `mysqltuner.pl:3315` </location>
<code_context>
+    if ( grep { /^authentication_string$/msx } @mysql_user_columns ) {
+        $PASS_COLUMN_NAME = 'authentication_string';
     }
+    elsif ( grep { /^Password$/msx } @mysql_user_columns ) {
+        $PASS_COLUMN_NAME = 'Password';
+    }
</code_context>

<issue_to_address>
**issue (bug_risk):** Column name case might not match typical `mysql.user` schema

The previous implementation matched `COLUMN_NAME = 'password'` (lowercase), but the new logic only checks for `Password` (capital P). On common MySQL setups the column is actually lowercase, so this may stop detecting the password column on some versions.

To preserve compatibility, either:
* Match `password` (lowercase) as before, or
* Do a case-insensitive match (e.g., `lc $_ eq 'password'`) so this works regardless of column-name casing.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}