Bump pgx to v5.10.0 (CWE-306)#10898
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9952cba558
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0 | ||
| github.com/iancoleman/strcase v0.3.0 | ||
| github.com/jackc/pgx/v5 v5.9.2 | ||
| github.com/jackc/pgx/v5 v5.10.0 |
There was a problem hiding this comment.
Configure pgx require_auth when bumping for CWE-306
For deployments using postgres12_pgx without an explicit connectAttributes.require_auth, this bump does not actually restrict PostgreSQL authentication methods: buildDSNAttr only emits user-supplied connect attributes plus the default sslmode, and pgx v5.10.0 leaves RequireAuth empty unless the DSN contains require_auth, so all server-requested auth methods are still accepted. Since this commit is the CWE-306 remediation, Temporal should set a safe require_auth in the generated DSN or expose a first-class default; otherwise the downgrade-to-cleartext-password scenario remains on the default pgx path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We already have a way to set require_auth on ConnectAttributes. We can't set the default since we have variation in use cases.
| github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0 | ||
| github.com/iancoleman/strcase v0.3.0 | ||
| github.com/jackc/pgx/v5 v5.9.2 | ||
| github.com/jackc/pgx/v5 v5.10.0 |
There was a problem hiding this comment.
If I understand it correctly this is for giving the capability of enforcing a client side check for the auth config on the postgresql server side. (and I guess not even sure this is CWE306)
If so, I don't feel this is something we need to issue a security patch release for? wdyt?
What changed?
Bumped pgx dependency to v5.10.0
Why?
Fixes CWE-306
How did you test it?
Potential risks
CI should point any regressions