Repository navigation
Conversation
The Python SDK has tests for each call, but nothing used it the way an application does. This FastAPI service shares one Cipher between request threads, uses a Pydantic class as both the request body and the model, and stores customers in PostgreSQL with EQL v4. The email is encrypted and searchable by exact match, and a unique index on its equality term refuses duplicates. The live tests need real credentials, so they are run by hand and CI does not run them. AGENTS.md and SECURITY.md list the new example. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The SDK README shows how to declare a SQLAlchemy model, but not how to insert or query one. This second app shows the hand-written code that a SQLAlchemy user needs today: an EQL column type, encrypt before an INSERT, a cast on the query term, and decrypt after a SELECT. Both apps share one table, and the tests run against both. The error handlers move to errors.py, so that both apps use the same ones. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
cipherstash-agent
left a comment
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation
Reviewed 10cafea · posted from Claude Code · claude-opus-5-5 · effort medium
Sources: code-review · claude-opus-5-5 (findings verified at head by review-cross-check delegates · claude-opus-5-5)
Skills: code-review, rust-security, review-cross-check, review-post
Findings (5): 5 minor — 5 inline, 0 below.
rust-security was loaded but the PR contains no Rust; only its logging-discipline rule was applied to the Python logging path. Two further code-review findings were refuted at head (no endpoint input makes PostgreSQL quote a value in diag.message_primary; dropping hide_input_in_errors from either Search leaks nothing because errors.py replaces the 422 body), and four design questions (connection pooling in app.py, the errors.py:17 docstring's general claim about PostgreSQL primary messages, Dependabot not covering this example's uv.lock, keyword-DSN DATABASE_URL) are left to the author and not posted as findings.
Generated by Claude Code
| docker compose down --volumes | ||
| docker compose up --detach --wait | ||
| # Built from the tree, because the released copy moves only at a version bump. | ||
| mise -C ../../../../packages/eql run build |
There was a problem hiding this comment.
minor setup deletes the database volume (line 12) before the EQL build here, so any build failure (for example an untrusted packages/eql/mise.toml, which the README never mentions) leaves an empty database with no bundle or table. Move the build above docker compose down --volumes, and add mise trust ../../../../packages/eql (which pulls EQL's own rust = latest toolchain) to the README prerequisites.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| # hide_parameters leaves the bound values out of SQLAlchemy's error text. | ||
| # PostgreSQL's own detail can still quote the start of a rejected value. | ||
| engine = sa.create_engine( | ||
| os.environ["DATABASE_URL"].replace("postgresql://", "postgresql+psycopg://", 1), |
There was a problem hiding this comment.
minor .replace("postgresql://", ...) leaves a postgres:// URL (as Heroku and others issue) untouched, so importing this module fails with NoSuchModuleError: Can't load plugin: sqlalchemy.dialects:postgres while app.py accepts the same value. Use sa.engine.make_url(os.environ["DATABASE_URL"]).set(drivername="postgresql+psycopg") and pass the URL object, not str(url), which masks the password as ***.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| def test_error_logs_contain_no_plaintext(client, caplog): | ||
| client.post("/customers", json=customer()) | ||
| naive = {**customer(), "created_at": "2026-01-01T00:00:00"} | ||
|
|
||
| with caplog.at_level(logging.INFO): | ||
| client.post("/customers", json=customer()) | ||
| client.post("/customers", json=naive) | ||
|
|
||
| assert "customer_error" in caplog.text | ||
| assert EMAIL not in caplog.text | ||
| assert NAME not in caplog.text |
There was a problem hiding this comment.
minor The setup POSTs are unchecked, and "customer_error" in caplog.text is always satisfied by the naive-datetime request (the engine refuses it before any SQL), so this test never shows that the duplicate (409) path logged anything. Assert each status (201, 409, 422) and check the 409 path's line, e.g. sqlstate=23505; the same unchecked setup lets test_a_different_email_is_not_found (line 62) pass on a failed insert, and lines 70, 80, 89, 161, 172-173 fail with TypeError/KeyError instead of the status.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| session.add(plaintext) | ||
| session.commit() | ||
|
|
||
| assert EMAIL not in str(caught.value) |
There was a problem hiding this comment.
minor This passes while the error text still holds plaintext: psycopg's DataError includes DETAIL: Token "ada" is invalid. and CONTEXT: JSON data, line 1: ada..., and hide_parameters=True hides only SQLAlchemy's parameter line (reproduced with the locked psycopg 3.3.8 / SQLAlchemy 2.1.8). Assert what is true instead ("SQL parameters hidden" in str(caught.value)) and rename the test so it does not claim the exception is plaintext-free.
Source: code-review · claude-opus-5-5
Generated by Claude Code
|
|
||
| ## Run the tests | ||
|
|
||
| The tests use the real ZeroKMS service and the example's PostgreSQL. They run against both apps. They empty the `customers` table. CI does not run them. |
There was a problem hiding this comment.
minor CI never runs these tests, but the example sits under languages/python/**, tests-python.yml's pull_request path filter, so an example-only change runs the full Python matrix, the live job and six wheel builds while covering none of it. Add '!languages/python/examples/**' to that filter (checking python-ci.test.mjs and workflow-paths-filter-parity.test.mjs), or add a job that runs the example.
Source: code-review · claude-opus-5-5
Generated by Claude Code
Summary
This PR adds a small FastAPI service that uses the Python SDK (
stackencrypt) the way a real application does. It stores customers in PostgreSQL. The email is encrypted, and you can still search it by exact match. The same app is written twice: once with psycopg, and once with SQLAlchemy.Changes
app.py: the psycopg version. Request threads share oneCipher, and one Pydantic class is both the request body and the model.app_sqlalchemy.py: the SQLAlchemy version. It shows the code a SQLAlchemy user has to write by hand today: an EQL column type, encryption before an INSERT, a cast on the query term, and decryption after a SELECT.schema.sql: one table for both apps. A unique index on the email's equality term refuses duplicate emails.errors.py: error handlers that both apps use.tests/test_app.py: one set of tests, run against both apps.compose.yaml,mise.toml,pyproject.toml,uv.lock: a local PostgreSQL, the tasks, and the dependencies. The example installsstackencryptfrom this checkout.AGENTS.mdandSECURITY.mdnow list the example.Verification
The tests need real CipherStash credentials, so CI does not run them. They were run by hand against a local PostgreSQL with EQL v4.
Related
Refs #1341
Review notes
The example is internal and is not published, so there is no changeset. The SQLAlchemy app shows the manual steps on purpose. A later PR (
feat/python-sqlalchemy) replaces them withstackencrypt.sqlalchemy.🤖 Generated with Claude Code