Skip to content

fix(security): upgrade OIDC and JOSE dependencies - #526

Merged
vuanhphung merged 3 commits into
mainfrom
fix/oidc-jose-upgrade
Nov 2, 2026
Merged

vuanhphung merged 3 commits into
mainfrom
fix/oidc-jose-upgrade

Conversation

@vuanhphung

Copy link
Copy Markdown
Collaborator

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.

Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
@vuanhphung vuanhphung added ai-assisted integration-test Preview the Go integration replay suite on this PR integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) labels Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Go integration tests triggered (thrift+sea / replay). View workflow runs.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Go integration tests triggered (sea / passthrough). View workflow runs.

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread go.mod
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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).

@vuanhphung vuanhphung added the run-e2e Trigger on-demand cross-OS E2E label Nov 2, 2026
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
@github-actions github-actions Bot removed integration-test Preview the Go integration replay suite on this PR integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) labels Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Integration test approval reset.

New commits were pushed to this PR. Label(s) integration-test, integration-test-kernel were removed for security.

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

@vuanhphung vuanhphung added integration-test Preview the Go integration replay suite on this PR integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) labels Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Go integration tests triggered (sea / passthrough). View workflow runs.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Go integration tests triggered (thrift+sea / replay). View workflow runs.

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No issues identified by the review bot.

Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
@github-actions github-actions Bot removed integration-test Preview the Go integration replay suite on this PR integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) labels Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Integration test approval reset.

New commits were pushed to this PR. Label(s) integration-test, integration-test-kernel were removed for security.

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

@vuanhphung vuanhphung added integration-test Preview the Go integration replay suite on this PR integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) labels Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Go integration tests triggered (sea / passthrough). View workflow runs.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Go integration tests triggered (thrift+sea / replay). View workflow runs.

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 // indirect pin is only protected by a comment in go.mod. Nothing in CI checks for go mod tidy drift, 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.

@vuanhphung
vuanhphung enabled auto-merge November 2, 2026 03:32
@vuanhphung
vuanhphung added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 325572a Nov 2, 2026
23 checks passed
@vuanhphung
vuanhphung deleted the fix/oidc-jose-upgrade branch November 2, 2026 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-assisted integration-test Preview the Go integration replay suite on this PR integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) run-e2e Trigger on-demand cross-OS E2E

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant