Repository navigation
fix(security): upgrade OIDC and JOSE dependencies - #526
Conversation
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
|
Go integration tests triggered ( |
|
Go integration tests triggered ( |
There was a problem hiding this comment.
Verdict: 1 Low
Looks good, with one low-severity gap: there's no CHANGELOG entry. The driver uses go-oidc only in auth/oauth/oauth.go, through ClientContext, InsecureIssuerURLContext, NewProvider, Endpoint and ScopeOfflineAccess. All of these are still in go-oidc v3.21.0, and since the driver never verifies tokens, the stricter JOSE v4 parsing doesn't affect it.
| github.com/apache/arrow/go/v12 v12.0.1 | ||
| github.com/apache/thrift v0.24.0 | ||
| github.com/coreos/go-oidc/v3 v3.5.0 | ||
| github.com/coreos/go-oidc/v3 v3.21.0 |
There was a problem hiding this comment.
🔵 Low — Earlier security bumps of dependencies all got a CHANGELOG line: klauspost/compress, thrift, x/crypto, x/mod, and go-jose itself. This PR moves the transitive JOSE dependency up a major version (v3 → v4), bumps go-oidc by 16 minor versions and bumps x/oauth2, but adds nothing under ## Unreleased.
The major-version JOSE change and the oauth2 bump show up in downstream module graphs. Consumers scanning for the go-jose/v3 advisories would benefit from a note. Suggested entry: - Upgrade go-oidc to v3.21.0 (go-jose v4.1.6) and x/oauth2 to v0.36.0 to address go-jose/v3 security findings (databricks/databricks-sql-go#526).
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
|
Integration test approval reset. New commits were pushed to this PR. Label(s) A maintainer must re-review and re-add a label to preview tests again. (The real gate runs in the merge queue.) Latest commit: 435a12e |
|
Go integration tests triggered ( |
|
Go integration tests triggered ( |
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
|
Integration test approval reset. New commits were pushed to this PR. Label(s) A maintainer must re-review and re-add a label to preview tests again. (The real gate runs in the merge queue.) Latest commit: b418dcb |
|
Go integration tests triggered ( |
|
Go integration tests triggered ( |
There was a problem hiding this comment.
Verdict: 1 Nit
Looks good. This is a clean dependency bump with no blocking issues and nothing to post inline. The driver only uses oidc.NewProvider, ClientContext, InsecureIssuerURLContext, Endpoint() and ScopeOfflineAccess (auth/oauth/oauth.go), and all of these still exist in v3.21.0. Nothing imports go-jose or x/crypto directly, so the JOSE v4 parsing and RSA key-size changes don't affect this code. One nit: the x/crypto pin in go.mod is only protected by a comment, so a future go mod tidy could quietly remove it.
Other findings
- ⚪ Nit — The
golang.org/x/crypto v0.55.0 // indirectpin is only protected by a comment in go.mod. Nothing in CI checks forgo mod tidydrift, so the next contributor who runs tidy will quietly remove the pin. MVS would then pick an older x/crypto from transitive go.mod files, and the scanner would start flagging it again. Consider a CI check, or point to this pin in CONTRIBUTING.md.
Resolve the go-jose/v3 security findings by upgrading go-oidc to v3.21.0 and pinning go-jose/v4 to v4.1.6. Update x/oauth2 to the required v0.36.0, refresh checksums, and retain the existing x/crypto security pin. Supersedes #338.
Reviewed upstream compatibility changes: JOSE v4 tightens token parsing, and v4.1.6 caps RSA JWKs at 4096 bits. The driver uses go-oidc for discovery and endpoint extraction; it does not call its token-verification or JWKS APIs. The dependencies' Go 1.25 floor fits the driver's existing Go 1.27.3 requirement.
Validation: full pure-Go unit suite, driver/auth race tests, and golangci-lint passed locally. Integration replay and live kernel tests will run through the requested PR labels.
This PR was created with GitHub MCP.