chore(deps): upgrade cipherstash-client to 0.38.0, drop vendored stack-auth patch (CIP-3233)#409
Open
freshtonic wants to merge 2 commits into
Open
chore(deps): upgrade cipherstash-client to 0.38.0, drop vendored stack-auth patch (CIP-3233)#409freshtonic wants to merge 2 commits into
freshtonic wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThis PR updates the proxy encryption path to use ChangesProxy EQL output migration
Vendored stack-auth cleanup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…tack-auth patch (CIP-3233) Moves Proxy off the `vendor/stack-auth` `[patch.crates-io]` workaround and onto the current released cipherstash-client group, built against the fixed stack-auth. Background: 2.2.4 (PR #408) shipped the CIP-3233 access-key token-refresh fix via a vendored stack-auth patched on top of the 0.34.1-alpha.4 source. cipherstash-client 0.38.0 links stack-auth 0.38.0, which carries the same fix from crates.io, so the vendored copy and patch are no longer needed. Changes: - cipherstash-client / cipherstash-config / cts-common: 0.34.1-alpha.4 -> 0.38.0 (carries the API migration from PR #406's 0.37.0 upgrade; 0.37 -> 0.38 needed no further source changes) - Remove `[patch.crates-io] stack-auth = { path = "vendor/stack-auth" }`, the `exclude = ["vendor/stack-auth"]` workspace entry, and the vendor/stack-auth tree - stack-auth now resolves from crates.io (0.38.0); single version of the cipherstash-client group in the lock (zerokms-protocol 0.12.19) Verified: `cargo check --workspace`, `cargo clippy --workspace --all-targets`, and `cargo test --workspace --lib` (111 proxy unit tests) all pass. Integration tests need a live DB/ZeroKMS and were not run here.
freshtonic
force-pushed
the
james/cip-3233-proxy-drop-vendor-patch
branch
from
256ad08 to
2e11f69
Compare
freshtonic
changed the title
cipherstash-client 0.38.0 emits structured JSONB (SteVec) encrypted
values as {"k":"sv",...} without a top-level `c` field. The pinned EQL
2.3.0-pre.3 enforced a top-level `c` on every encrypted value via
eql_v2._encrypted_check_c, rejecting these payloads with EP0001
("Encrypted column missing ciphertext (c) field") and failing all
JSONB/SteVec integration tests.
EQL 2.3.1 relaxes the check to `(val ? 'c') OR (val ? 'sv')`, accepting
the new SteVec format.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cipherstash-proxy/src/postgresql/messages/bind.rs (1)
84-96: 🗄️ Data Integrity & Integration | 🟠 MajorConfirm EqlOutput serialization format mismatch for EQL 2.3.1
The workspace is pinned to
cipherstash-clientversion 0.38.0, whereEqlOutputserializes viaserde_json::to_valueinto a JSON object containing a top-levelc(ciphertext) field. The required EQL 2.3.1 format expects a structure without this top-levelcwrapper. As written,rewrite()writes the legacy payload, which will fail or corrupt data ineql_v2_encryptedcolumns expecting the new format. Update the serialization logic to produce the schema-free format expected by the EQL 2.3.1 backend, likely requiring manual construction of the JSON payload from theEqlOutputcomponents or updating the dependency if a fix is available.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cipherstash-proxy/src/postgresql/messages/bind.rs` around lines 84 - 96, The bind::rewrite method is serializing EqlOutput with serde_json::to_value, which preserves the legacy top-level c wrapper and does not match the EQL 2.3.1 payload shape. Update rewrite() to emit the schema-free JSON expected by eql_v2_encrypted columns, either by manually building the payload from EqlOutput’s fields or by switching to a dependency/version that already produces the new format. Keep the change localized to bind::rewrite and the EqlOutput serialization path it uses.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Cargo.toml`:
- Around line 46-48: The Cargo.toml dependency update points cipherstash-client,
cipherstash-config, and cts-common at unpublished 0.38.0 versions, which will
break dependency resolution. Revert these entries to a published crates.io
version or restore the [patch.crates-io] override to a valid local/git source
for stack-auth until those crates are available. Keep the existing dependency
names and patch configuration aligned so Cargo can resolve them successfully.
---
Outside diff comments:
In `@packages/cipherstash-proxy/src/postgresql/messages/bind.rs`:
- Around line 84-96: The bind::rewrite method is serializing EqlOutput with
serde_json::to_value, which preserves the legacy top-level c wrapper and does
not match the EQL 2.3.1 payload shape. Update rewrite() to emit the schema-free
JSON expected by eql_v2_encrypted columns, either by manually building the
payload from EqlOutput’s fields or by switching to a dependency/version that
already produces the new format. Keep the change localized to bind::rewrite and
the EqlOutput serialization path it uses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25c7c19a-7daf-43a1-9720-a75b783fae42
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockvendor/stack-auth/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (35)
Cargo.tomlmise.local.example.tomlmise.tomlpackages/cipherstash-proxy/src/error.rspackages/cipherstash-proxy/src/lib.rspackages/cipherstash-proxy/src/postgresql/backend.rspackages/cipherstash-proxy/src/postgresql/context/mod.rspackages/cipherstash-proxy/src/postgresql/frontend.rspackages/cipherstash-proxy/src/postgresql/messages/bind.rspackages/cipherstash-proxy/src/postgresql/messages/data_row.rspackages/cipherstash-proxy/src/proxy/encrypt_config/manager.rspackages/cipherstash-proxy/src/proxy/mod.rspackages/cipherstash-proxy/src/proxy/zerokms/zerokms.rsvendor/stack-auth/.gitignorevendor/stack-auth/Cargo.tomlvendor/stack-auth/LICENSEvendor/stack-auth/README.mdvendor/stack-auth/examples/auto_strategy.rsvendor/stack-auth/examples/device_code.rsvendor/stack-auth/src/access_key.rsvendor/stack-auth/src/access_key_refresher.rsvendor/stack-auth/src/access_key_strategy.rsvendor/stack-auth/src/auto_refresh.rsvendor/stack-auth/src/auto_strategy.rsvendor/stack-auth/src/device_client.rsvendor/stack-auth/src/device_code/mod.rsvendor/stack-auth/src/device_code/protocol.rsvendor/stack-auth/src/device_code/tests.rsvendor/stack-auth/src/lib.rsvendor/stack-auth/src/oauth_refresher.rsvendor/stack-auth/src/oauth_strategy.rsvendor/stack-auth/src/refresher.rsvendor/stack-auth/src/service_token.rsvendor/stack-auth/src/static_token_strategy.rsvendor/stack-auth/src/token.rs
💤 Files with no reviewable changes (22)
- vendor/stack-auth/src/static_token_strategy.rs
- vendor/stack-auth/src/device_code/tests.rs
- vendor/stack-auth/README.md
- vendor/stack-auth/src/auto_strategy.rs
- vendor/stack-auth/src/oauth_refresher.rs
- vendor/stack-auth/.gitignore
- vendor/stack-auth/src/access_key.rs
- vendor/stack-auth/Cargo.toml
- vendor/stack-auth/src/device_client.rs
- vendor/stack-auth/src/device_code/protocol.rs
- vendor/stack-auth/src/device_code/mod.rs
- vendor/stack-auth/src/refresher.rs
- vendor/stack-auth/src/service_token.rs
- vendor/stack-auth/src/oauth_strategy.rs
- vendor/stack-auth/src/access_key_refresher.rs
- vendor/stack-auth/src/access_key_strategy.rs
- vendor/stack-auth/examples/auto_strategy.rs
- vendor/stack-auth/LICENSE
- vendor/stack-auth/src/token.rs
- vendor/stack-auth/src/lib.rs
- vendor/stack-auth/src/auto_refresh.rs
- vendor/stack-auth/examples/device_code.rs
Comment on lines
+46
to
+48
| cipherstash-client = { version = "0.38.0" } | ||
| cipherstash-config = { version = "0.38.0" } | ||
| cts-common = { version = "0.38.0" } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm published versions and that no stale patch/exclude remains
for c in cipherstash-client cipherstash-config cts-common; do
echo "== $c =="
curl -s "https://crates.io/api/v1/crates/$c" | jq -r '.versions[].num' | rg -n '0\.38\.0' || echo "0.38.0 NOT found"
done
echo "== residual stack-auth references =="
rg -n 'stack-auth|vendor/stack-auth' Cargo.toml Cargo.lock 2>/dev/nullRepository: cipherstash/proxy
Length of output: 550
Critical Build Failure: Versions 0.38.0 are unpublished.
The versions 0.38.0 for cipherstash-client, cipherstash-config, and cts-common are not available on crates.io. Removing the [patch.crates-io] override with these versions will cause the build to fail immediately as the resolver cannot find these artifacts.
Required Action:
- Revert the
Cargo.tomlchanges to use a published version or restore the[patch.crates-io]entry with a valid local or git source forstack-authuntil the crates are published. Do not merge this PR in its current state.
[dangerous_changes]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Cargo.toml` around lines 46 - 48, The Cargo.toml dependency update points
cipherstash-client, cipherstash-config, and cts-common at unpublished 0.38.0
versions, which will break dependency resolution. Revert these entries to a
published crates.io version or restore the [patch.crates-io] override to a valid
local/git source for stack-auth until those crates are available. Keep the
existing dependency names and patch configuration aligned so Cargo can resolve
them successfully.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moves Proxy off the
vendor/stack-auth[patch.crates-io]workaround and onto the current releasedcipherstash-clientgroup, built against the fixed stack-auth. This is the CIP-3233 follow-up cleanup.Background: 2.2.4 (#408) shipped the CIP-3233 access-key token-refresh fix via a vendored stack-auth, patched on top of the
0.34.1-alpha.4source thatcipherstash-client 0.34.1-alpha.4pinned.cipherstash-client 0.38.0linksstack-auth 0.38.0, which carries the same fix straight from crates.io — so the vendored copy and the patch are no longer needed.Changes
cipherstash-client/cipherstash-config/cts-common:0.34.1-alpha.4→0.38.0(the current published group). The API migration for the0.34 → 0.37jump is carried over from the (closed) feat: upgrade cipherstash-client to 0.37.0 #406 0.37.0 upgrade — same 10 source-file changes;0.37 → 0.38needed no further source changes.[patch.crates-io] stack-auth = { path = "vendor/stack-auth" }, theexclude = ["vendor/stack-auth"]workspace entry, and the entirevendor/stack-auth/tree.stack-authnow resolves from crates.io at 0.38.0 (registry source, verified in the lock). Single version of the whole cipherstash-client group;zerokms-protocolat0.12.19.Verification
cargo check --workspace— cleancargo clippy --workspace --all-targets— cleancargo test --workspace --lib— 111cipherstash-proxyunit tests passstack-auth(0.38.0, registry source); novendor/orpatch.crates-ioreferences remainNot run:
cipherstash-proxy-integrationtests (need a live Postgres + ZeroKMS; they fail here only withConnectionRefused). Please run the full integration suite in CI / with creds before merge.Notes
Closes the
cipherstash/proxyfollow-up line item on CIP-3233.Summary by CodeRabbit
Bug Fixes
Chores
2.3.1.