Skip to content

release-25.4: Revert "sql: remove AllowUnsafeInternals deserialization workaround"#164389

Merged
dhartunian merged 1 commit intocockroachdb:release-25.4from
angles-n-daemons:undo-unsafe-internals-deserialization-check
Feb 26, 2026
Merged

release-25.4: Revert "sql: remove AllowUnsafeInternals deserialization workaround"#164389
dhartunian merged 1 commit intocockroachdb:release-25.4from
angles-n-daemons:undo-unsafe-internals-deserialization-check

Conversation

@angles-n-daemons
Copy link
Copy Markdown
Contributor

@angles-n-daemons angles-n-daemons commented Feb 25, 2026

This reverts commit 32d9056 which removed the workaround that forces AllowUnsafeInternals to true during session deserialization.

The workaround is still needed because multi-tenant clusters may still be running mixed versions where older pods serialize sessions without the AllowUnsafeInternals field. When such a session is deserialized on a 25.4 node, protobuf sets the field to its zero value (false), which incorrectly blocks access to internal tables.

Fixes: CRDB-57952
Epic: None
Release note: None

Release justification: Prevents the accidental deserialization of a zero value for a non-zero default session var in a mixed-version multi-tenant cluster.

This reverts commit 32d9056 which removed the workaround that
forces `AllowUnsafeInternals` to `true` during session deserialization.

The workaround is still needed because multi-tenant clusters may still
be running mixed versions where older pods serialize sessions without
the `AllowUnsafeInternals` field. When such a session is deserialized
on a 25.4 node, protobuf sets the field to its zero value (`false`),
which incorrectly blocks access to internal tables.

Fixes: CRDB-57952
Epic: None
Release note: None
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Feb 25, 2026

Thanks for opening a backport.

Before merging, please confirm that it falls into one of the following categories (select one):

  • Non-production code changes OR fixes for serious issues. Non-production includes test-only changes, build system changes, etc. Serious issues are defined in the policy as correctness, stability, or security issues, data corruption/loss, significant performance regressions, breaking working and widely used functionality, or an inability to detect and debug production issues.
  • Other approved changes. These changes must be gated behind a disabled-by-default feature flag unless there is a strong justification not to. Reference the approved ENGREQ ticket in the PR body (e.g., "Fixes ENGREQ-123").

Add a brief release justification to the PR description explaining your selection.

Also, confirm that the change does not break backward compatibility and complies with all aspects of the backport policy.

All backports must be reviewed by the TL and EM for the owning area.

@blathers-crl blathers-crl Bot added backport Label PR's that are backports to older release branches T-observability labels Feb 25, 2026
@angles-n-daemons angles-n-daemons marked this pull request as ready for review February 25, 2026 21:57
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Feb 25, 2026

✅ PR #164389 is compliant with backport policy

Confidence: high
Critical bug criteria met: [Bugs that can cause the DB to return incorrect results or result in suboptimal performance]
Backward compatible: true
Explanation: The pull request complies with the backport policy as it meets the criteria for Release Justification. The PR body contains a specific 'Release justification: Prevents the accidental deserialization of a zero value for a non-zero default session var in a mixed-version multi-tenant cluster.' This indicates that the change addresses a critical bug by preventing incorrect settings in session deserialization in a mixed-version environment, important for stability in multi-tenant clusters. Such a bug falls under the criteria of fixing incorrect results or suboptimal performance. Furthermore, the PR does not introduce compatibility issues or version gate removals; it primarily adds essential corrections to maintain existing functionality reliably during an upgrade phase. The code change sets 'AllowUnsafeInternals' to 'true' to prevent restrictive behavior on internal table access, addressing a bug with potential significant implications.

ENGREQ Check Passed: No ENGREQ required (non-production code or serious issues).

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@dhartunian dhartunian changed the title Revert "sql: remove AllowUnsafeInternals deserialization workaround" release-25.4: Revert "sql: remove AllowUnsafeInternals deserialization workaround" Feb 26, 2026
@dhartunian dhartunian merged commit 5e10517 into cockroachdb:release-25.4 Feb 26, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Label PR's that are backports to older release branches T-observability v25.4.7

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants