Repository navigation
test(python): verify the two engine facts the spec depends on (#1384) - #1414
Conversation
|
|
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
 |
The spec in #1341 depends on two facts that had not been checked. The first is that the extension can read the engine's decrypted record frame with the public stack-encrypt API. The second is the sync contract: an interrupt waits for a GIL-released call, and a dropped Pending wipes its plaintext. Both facts hold. A Sink written outside the engine reads the frame. A probe function behind an off-by-default cargo feature is sent SIGINT during the call. The interrupt arrives after the call returns. The wipe is from source reading only. The issue's wording of the first fact is corrected: a sealed value's kind is inside its encryption, so the kind is read after decrypting. The findings, how to run each probe again, and what was not tested are in languages/python/docs/. Refs #1384 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
) The codes follow the amendment on #1341. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eeab4bd to
909bd36
Compare
cipherstash-agent
left a comment
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation
Reviewed 909bd36 · posted from Claude Code · claude-sonnet-5-5 · effort medium
Sources: code-review · claude-sonnet-5-5
Skills: code-review, review-cross-check, review-post
Findings (3): 0 critical, 0 major, 3 minor — 3 inline, 0 below.
Verified against the PR diff and the head file content only; the engine source was not available in this session, so two further candidate findings (frame passthrough round trip, maturin feature merge) were not confirmed and are not posted.
Source: code-review · claude-sonnet-5-5
Generated by Claude Code
cipherstash-bot
left a comment
cipherstash-bot
left a comment
There was a problem hiding this comment.
Recommendation: 🟡 merge after changes
Change before merging: the frame reader probe must cover a typed null and a sequence, and the SIGINT timer in the Python test must be cancelled in a finally block. The other points can wait.
Other findings not posted as comments
languages/python/tests/test_gate_keyboard_interrupt.py:30: the tests assume SIGINT raisesKeyboardInterrupt. If the launcher ignores SIGINT, both tests fail withDID NOT RAISE. Installsignal.default_int_handlerin a fixture and restore it afterwards.languages/python/tests/test_gate_keyboard_interrupt.py:21: no CI job builds withgate-probe, so the tests always skip in CI. Add a CI step that builds with the feature and fails when the tests skip.languages/python/docs/gate-facts/keyboard-interrupt.md:12: part 2 of fact 2 (a droppedPendingwipes its plaintext) has no executable test. The doc already says so. Add a heap-hygiene test besidepackages/stack-encrypt/tests/heap_hygiene.rs.languages/python/crates/engine/src/lib.rs:32: the probe builds a new tokio runtime on each call and panics if the build fails. Optional: keep one runtime in aOnceLockand return aPyResult.languages/python/crates/core/tests/gate_frame_reader.rs:201: the malformed-frame test checks only truncation. Optional: add a case for trailing bytes and an empty buffer.
How this review was made
| Agent | Model | Review type | Result |
|---|---|---|---|
| claude | claude-opus-5-5 | test-gap | 2 found, 1 posted |
| claude | claude-opus-5-5 | rust | 3 found, 2 posted |
| claude | claude-opus-5-5 | python | 2 found, 0 posted |
| claude | claude-opus-5-5 | infracode | 0 found, 0 posted |
| codex | gpt-5.6-terra | test-gap | 1 found, 0 posted |
| codex | gpt-5.6-terra | rust | 0 found, 0 posted |
| codex | gpt-5.6-terra | python | 1 found, 1 posted |
| codex | gpt-5.6-terra | infracode | 0 found, 0 posted |
Synthesis: claude-sonnet-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 0 posted finding(s) were raised by two or more models.
Plain language: the reader test did not run; only the word-and-shape check was applied.
Stack: position 3 of 15 (#1405, #1406, *️⃣ #1414, #1415, #1433, #1437, #1453, #1499, #1509, #1510, #1511, #1513, #1516, #1517, #1582). *️⃣ marks this pull request.
Context loaded: the description, 2 linked issue(s) and 6 discussion entries.
Summary
The Python SDK spec (#1341) depends on two facts about the engine that nobody had checked. This PR checks both, with a probe for each, and records the results in
languages/python/docs/gate-facts/. Both facts hold. No SDK code changes, and the normal wheel build is unchanged.The engine is the Rust crate
stack-encrypt. The engine encrypts and decrypts. The extension is the Rust code compiled into the Python package. Python calls the extension, and the extension calls the engine.KeyboardInterrupt) during an engine call is raised only after the call finishes. Also, a request object (Pending) that is dropped early wipes the plaintext it contains.Changes
crates/core/tests/gate_frame_reader.rs: the probe for fact 1. A reader written outside the engine walks a record and records each field's type.crates/engine/src/lib.rsandcrates/engine/Cargo.toml: the probe for fact 2. A function that blocks with Python's global interpreter lock (GIL) released, behind a new cargo feature,gate-probe. The feature is off by default, so the function is not in a normal wheel.tests/test_gate_keyboard_interrupt.py: sends Ctrl-C during that call. The tests skip themselves when the feature is not built.docs/gate-facts/frame-reader.mdanddocs/gate-facts/keyboard-interrupt.md: the findings, how to run each probe again, and what was not tested.Cargo.lock: one line, fortokioas an optional dependency of the probe feature.Verification
All of this was run on macOS 14 (arm64) with Python 3.14.7:
cargo test --locked -p stackencrypt-core --test gate_frame_reader: 2 passed.--features gate-probe: 2 passed. Ctrl-C was sent 0.2 seconds into a 1-second call. The interrupt was raised after 1.01 seconds. As a control, the same signal interrupted a plain Pythontime.sleep(1.0)after 0.21 seconds.mise run test: 4 passed and 2 skipped. The 2 skipped tests are the probe tests, in a normal build.mise run test:cargoandcargo fmt --check: both pass.cargo clippy -D warnings, with and without the probe feature: passes.pnpm run test:scriptson Node 22: 83 of 83 test files pass.Not tested:
Related
Closes #1384. Refs #1341 (the spec). Stacked on #1406.
Review notes
Start with the two files in
docs/gate-facts/. Each one says what was checked, how, and what was not.🤖 Generated with Claude Code