Skip to content

fix: sync pgx/v5 migration — remove pgproto3/v2 (CVE-2026-32286)#985

Open
buger wants to merge 1 commit intorelease-1.14from
fix/pgx-v5-sync-1.14
Open

fix: sync pgx/v5 migration — remove pgproto3/v2 (CVE-2026-32286)#985
buger wants to merge 1 commit intorelease-1.14from
fix/pgx-v5-sync-1.14

Conversation

@buger
Copy link
Copy Markdown
Member

@buger buger commented Apr 21, 2026

Summary

  • Sync pgx/v5 migration from release-1.14.1 (PR [TT-16932] Fix CVE-2026-32286 #959) to release-1.14
  • Upgrade storage v1.3.0 → v1.3.1, replace pgx/v4 with pgx/v5
  • Update gorm drivers (postgres v1.2.0 → v1.5.0, mysql v1.0.3 → v1.3.2)
  • Update gorm fork with pgx/v5 error translation support
  • Add monthEncodePlan for pgx/v5 simple protocol compatibility
  • Removes pgproto3/v2 — the last remaining HIGH CVE

Test plan

  • go build ./... passes
  • pgproto3/v2 no longer in go.mod
  • CI passes

🤖 Generated with Claude Code

Cherry-pick changes from release-1.14.1 (PR #959):
- Upgrade storage v1.3.0 → v1.3.1
- Replace pgx/v4 with pgx/v5
- Update gorm drivers (postgres, mysql)
- Update gorm fork with pgx/v5 error translation
- Add monthEncodePlan for pgx/v5 simple protocol compatibility
- Add pgx/v5 and MySQL test suites

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

🚨 Jira Linter Failed

Commit: 0381549
Failed at: 2026-04-21 09:51:13 UTC

The Jira linter failed to validate your PR. Please check the error details below:

🔍 Click to view error details
failed to get Jira issue: failed to fetch Jira issue CVE-2026: Issue does not exist or you do not have permission to see it.: request failed. Please analyze the request body for more details. Status code: 404

Next Steps

  • Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
  • Verify your PR title matches the branch's Jira ticket ID
  • Check that the Jira ticket exists and is accessible

This comment will be automatically deleted once the linter passes.

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Apr 21, 2026

This PR resolves a high-severity vulnerability (CVE-2026-32286) by upgrading the PostgreSQL driver from pgx/v4 to pgx/v5, which removes the vulnerable pgproto3/v2 dependency. The changes are synced from a previous PR (#959) into the release-1.14 branch.

Beyond the primary security fix, the PR includes several significant updates:

  • Dependency Upgrades: Updates gorm drivers for PostgreSQL (v1.2.0 → v1.5.0) and MySQL (v1.0.3 → v1.3.2), and the TykTechnologies/storage module.
  • GORM Fork Sync: The internal gorm fork is updated to a version that supports pgx/v5 error translation.
  • Driver Compatibility Fix: A custom encoding plan (monthEncodePlan) is introduced in pumps/sql.go to handle time.Month values correctly with the pgx/v5 simple protocol, preventing a potential data type mismatch.
  • CI Enhancements: The GitHub Actions workflow is refactored to use service containers for dependencies like Postgres and Redis, and adds a MySQL service to expand test coverage.
  • Extensive New Tests: Two new comprehensive test files, pumps/sql_pgxv5_test.go and pumps/sql_mysql_test.go, have been added. These files introduce dozens of test cases that validate migration idempotency, batch writes, upsert logic, connection pooling, data type handling, and error translation for both database drivers.

Files Changed Analysis

  • go.mod / go.sum: Reflect the core dependency changes, most notably replacing jackc/pgx/v4 with jackc/pgx/v5 and removing jackc/pgproto3/v2. Versions for gorm drivers, testify, and the gorm fork are also updated.
  • pumps/sql.go: Contains the critical logic for establishing a PostgreSQL connection. It now includes a workaround for a pgx/v5 behavioral change by injecting a custom type encoder for time.Month to ensure it's treated as an integer.
  • pumps/sql_pgxv5_test.go: A large new file (~680 lines) adding extensive test coverage for the pgx/v5 driver, covering migrations, batch inserts, upserts (ON CONFLICT), connection pooling, sharding, error translation, and various data type encodings.
  • pumps/sql_mysql_test.go: A new file (~230 lines) adding robust tests for the MySQL pump, validating the upgraded driver against core functionality and edge cases like strict-mode zero-date handling.
  • .github/workflows/ci-test.yml: The CI pipeline is modernized, replacing third-party actions with built-in service containers and adding MySQL to the test matrix.

Architecture & Impact Assessment

This PR primarily accomplishes a critical security remediation by upgrading a core data persistence dependency. The impact is concentrated on the SQL pump, which is responsible for writing analytics data to PostgreSQL and MySQL.

Key Technical Changes

  1. Driver Upgrade: The central change is the replacement of pgx/v4 with pgx/v5.
  2. Compatibility Shim: A non-trivial workaround was implemented in pumps/sql.go to handle time.Month encoding, indicating a subtle breaking change in the new driver that required custom handling.
  3. Test-Driven Validation: The addition of over 900 lines of new, targeted tests for both PostgreSQL and MySQL demonstrates a robust validation strategy for the upgraded dependencies. This significantly improves confidence in the stability of the data pumps.

Affected System Components

The change directly affects the SQLPump, SQLAggregatePump, and GraphSQLPump components that rely on the common SQL connection logic. Any Tyk setup that uses PostgreSQL or MySQL as a data sink for analytics will be impacted.

graph TD
    subgraph Tyk Pump Application
        A[Data Pumps] --> B[pumps/sql.go];
        B --> C[GORM ORM];
        C --> D[gorm.io/driver/postgres];
        D --> E[jackc/pgx/v5];
    end
    E --> F[PostgreSQL Database];

    style E fill:#f9f,stroke:#333,stroke-width:2px
    style B fill:#f9f,stroke:#333,stroke-width:2px
Loading

Scope Discovery & Context Expansion

The scope of this PR extends beyond a simple version bump. The need for a custom encoding plan in pumps/sql.go and an updated gorm fork for error translation highlights the subtle complexities of the pgx/v5 migration. The proactive addition of comprehensive test suites for both MySQL and PostgreSQL suggests that the author was not only fixing the CVE but also hardening the data persistence layer against future regressions. These new tests now form a valuable asset for validating any future changes to the SQL pumps.

Metadata
  • Review Effort: 4 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-04-21T09:52:43.545Z | Triggered by: pr_opened | Commit: 0381549

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Apr 21, 2026

Security Issues (3)

Severity Location Issue
🟠 Error pumps/sql.go:105-107
The custom `monthEncodePlan.Encode` method is implemented in a way that will cause a panic at runtime. The associated wrapper function (lines 125-131) converts the `time.Month` value to an `int`. The `pgx` driver then calls the `Encode` method with this `int` value. However, the `Encode` method performs a type assertion `value.(time.Month)`, which will fail because the `value` it receives is an `int`, not a `time.Month`. This will crash the pump when it tries to write an analytics record with a `Month` field using the simple protocol for PostgreSQL.
💡 SuggestionThe implementation of the wrapper function and the `Encode` method are inconsistent. To fix this, the wrapper should pass the original `time.Month` value, and the `Encode` method should perform the conversion to `int`, which aligns with the standard pattern for this type of workaround.

Modify the wrapper function to pass the original value through, and adjust the Encode method to perform the type assertion and conversion correctly.

🟡 Warning .github/workflows/ci-test.yml:38-49
Hardcoded credentials (`tyk123`, `root`) are used for the test databases (PostgreSQL and MySQL) in the CI workflow. While this is a non-production environment, storing credentials in version control is against security best practices. These credentials could be inadvertently exposed or misused.
💡 SuggestionReplace the hardcoded passwords with GitHub secrets. Store the passwords in the repository's secrets settings and reference them in the workflow file (e.g., `POSTGRES_PASSWORD: ${{ secrets.TEST_DB_PASSWORD }}`). This prevents credentials from being present in the codebase.
🟡 Warning .github/workflows/ci-test.yml:96
The PostgreSQL test connection string includes `sslmode=disable`, which prevents the use of TLS for the database connection. While this is for a local database inside a CI runner and the risk is low, it's a best practice to use secure configurations in all environments to prevent accidental insecure deployments and to align testing with production settings.
💡 SuggestionFor improved security posture and consistency with production environments, consider using a more secure SSL mode like `sslmode=require` if the CI architecture supports it, even for local connections. This is a defense-in-depth measure.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

Performance Issues (1)

Severity Location Issue
🟡 Warning pumps/sql.go:144-154
The `AfterConnect` callback allocates a new function literal and a new slice for every new database connection. While connection creation is infrequent in a pooled environment, these allocations are unnecessary and can be avoided by defining the function and slice as package-level variables.
💡 SuggestionDefine the `TryWrapEncodePlanFunc` and its containing slice as a package-level variable to prevent allocations in the `AfterConnect` callback, which runs every time a new connection is established.

Powered by Visor from Probelabs

Last updated: 2026-04-21T09:52:38.425Z | Triggered by: pr_opened | Commit: 0381549

💡 TIP: You can chat with Visor using /visor ask <your question>

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