Skip to content

test(stack-encrypt): enroll lifecycle and key-I/O conformance - #1493

Open
coderdan wants to merge 1 commit into
feat/stack-encrypt-eql-json-conformancefrom
feat/stack-encrypt-lifecycle-conformance
Open

coderdan wants to merge 1 commit into
feat/stack-encrypt-eql-json-conformancefrom
feat/stack-encrypt-lifecycle-conformance

Conversation

@coderdan

@coderdan coderdan commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Semantic vectors alone do not establish ownership release, cancellation, cleanup or key-I/O behavior. This adds eight lifecycle requirements backed by existing native Rust and Go tests, with a required CI lane and an evidence report that records both passing tests and observation limits.

Adds five focused tests for dropping unpolled and in-flight work, success/error capture release, cleanup after a late authenticated read failure, and bounded KMS chunk concurrency. The runner shares test-inventory and execution checks with EQL conformance, requires every enrolled test to run without skips, and preserves the heap observer's exclusion diagnostics.

The contract explicitly records the existing Go post-decrypt guest-memory gap, native heap-observer exclusions, caller-owned host values, unobserved live HTTP chunking, and future Python/TypeScript enrollment. Existing backend tests remain in place; this changes no production behavior.

Validation:

  • Lifecycle profile: 33 native Rust and 28 Go tests passed.
  • EQL regression: fast lane and native PostgreSQL lane passed.
  • 76 JavaScript runner/CI-wiring tests passed.
  • Rust clippy, rustfmt, Biome and diff checks passed.

Stacked on #1492, rebased onto its head at 07a5a14 (which includes its review fixes).

Closes #1480.

Summary by CodeRabbit

  • Tests

    • Expanded conformance checks for lifecycle behavior, cancellation, cleanup, key I/O, and chunk concurrency across Rust and Go.
    • Conformance runs now provide required CI checks and reports on supported Linux and macOS environments.
  • Documentation

    • Added guidance on lifecycle coverage, evidence requirements, platform support, and known observation limits.

@changeset-bot

changeset-bot Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 247cde3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The PR adds a lifecycle-v1 conformance profile for Rust and Go, tests lifecycle and cleanup behavior, and a runner that validates and executes enrolled suites. It extracts shared evidence helpers from the EQL runner and adds required CI execution with JSON report uploads.

Changes: Stack Encrypt lifecycle conformance

Layer / File(s) Summary
Lifecycle requirements and enrollment
conformance/stack-encrypt/v1/lifecycle-v1.enrollment.json, conformance/stack-encrypt/lifecycle-contract.md, conformance/stack-encrypt/README.md, conformance/stack-encrypt/coverage.md, docs/plans/2026-10-31-stack-encrypt-conformance.md
The lifecycle-v1 enrollment maps Rust and Go tests to lifecycle requirements and records limitations. The contract and supporting documentation describe evidence coverage and observation boundaries.
Shared conformance evidence execution
scripts/stack-conformance-evidence.mjs, scripts/stack-eql-conformance.mjs, scripts/__tests__/stack-conformance-ci.test.mjs
Shared helpers validate evidence selections, inventories, results, and environment variables, then run Rust or Go suites. The EQL runner uses these helpers and validates its profile. Enrollment-file tests now allow profiles without cases files.
Lifecycle runner and CI reports
scripts/stack-lifecycle-conformance.mjs, mise.toml, .github/workflows/tests-golang.yml, scripts/__tests__/stack-lifecycle-conformance.test.mjs
The runner selects lifecycle suites, collects metadata, and writes reports. The mise task and CI workflow run it and upload required JSON reports. Tests check profile selection and CI wiring.
Lifecycle, cleanup, and cancellation probes
packages/stack-encrypt/tests/lifecycle_conformance.rs, packages/stack-encrypt/tests/heap_hygiene.rs, packages/stack-kms/src/futures.rs
Rust tests check capture release after dropped and cancelled encryption work, provider outcomes, heap cleanup after a late field-type error, and release of active chunk futures after cancellation.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant Mise
  participant LifecycleRunner as stack-lifecycle-conformance.mjs
  participant Evidence as stack-conformance-evidence.mjs
  participant RustTests as Rust suites
  participant GoTests as Go suites
  participant Reports as JSON report artifact
  GitHubActions->>Mise: run lifecycle conformance task
  Mise->>LifecycleRunner: invoke runner
  LifecycleRunner->>Evidence: run selected suites
  Evidence->>RustTests: validate inventory and run required tests
  Evidence->>GoTests: run selected tests and validate results
  LifecycleRunner->>Reports: write lifecycle reports
  GitHubActions->>Reports: upload reports and require files
Loading

Possibly related issues

  • Issue 1490: The lifecycle enrollment, required CI lane, and limitation reports provide evidence relevant to its stable Rust release gate.
  • Issue 1393: The PR records Python lifecycle enrollment as not yet covered, but does not add Python evidence.
  • Issue 1479: The PR records TypeScript lifecycle enrollment as a limitation, but does not add a TypeScript runner.

Possibly related PRs

  • cipherstash/stack#1460: This PR extends the conformance index, required CI mode, and Rust/Go evidence checks introduced there.
  • cipherstash/stack#1492: This PR extracts shared evidence validation from the EQL/JSON runner and adds lifecycle execution using the report pattern.
  • cipherstash/stack#1461: This PR enrolls runtime tests from that work as lifecycle evidence for validation-before-I/O and logical batching.

Suggested reviewers: freshtonic, auxesis

Merge Risk: Minimal · up to 247cd

The lifecycle conformance lane appears mergeable with normal checks; no actionable regression is established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: enrolling stack-encrypt lifecycle and key-I/O conformance.
Linked Issues check Passed Directly linked issue #1480 is open and active, so its acceptance items apply. Evidence was read from the head enrollment index (conformance/stack-encrypt/v1/lifecycle-v1.enrollment.json) and lifecycl…
Out of Scope Changes check Passed The PR's own change is a single commit, 247cde3 ("test(encrypt): enroll lifecycle conformance"), on top of the merge base 07a5a14. The CI commits 2995fbb (CRAP and cached Rust runtimes) and 14d9a18 …
Full details: Linked Issues check

Explanation

Directly linked issue #1480 is open and active, so its acceptance items apply. Evidence was read from the head enrollment index (conformance/stack-encrypt/v1/lifecycle-v1.enrollment.json) and lifecycle-contract.md.

  1. Map existing native and Go tests to named requirements [#1480]: Met. The index defines eight requirements. Each maps existing test names across success, error, cancellation, drop, partial output, buffer cleanup and diagnostics. Native suites: lifecycle, heap, pending, guest-abi, kms, runtime, reads, contexts. Go suites: go, go-memory, go-sdk.

  2. Observe validation-before-I/O and logical batching; separate logical calls from physical chunk limits [#1480]: Met for Rust, with a recorded Go limit. The contract states that runtime, read and context cases count provider calls. The logical-batching requirement uses instrumented DataKeySource calls. The chunk tests use the KMS helper with controlled callbacks. The contract states that logical calls are distinct from HTTP requests. Go does not expose native provider counters, and the contract says it does not claim independently instrumented Go key-request counts. This gap is recorded, not hidden.

  3. Retain backend-specific tests; add Python/TypeScript evidence when available [#1480]: Met. Existing backend tests remain. Python and TypeScript are marked python-typescript-lifecycle, status not-enrolled. No such backend evidence is required yet.

  4. Document ownership transfer and erasure limits without promising erasure of caller-owned values [#1480]: Met. The contract states that caller-owned host values are outside the guarantee, that drop witnesses do not prove zeroization, and that Go close is required because GC cleanup is eventual.

  5. Require executable evidence and identify unsupported or unobserved behavior [#1480]: Met. The index lists five limitations with statuses: go-post-decrypt-shadow-stack (known-gap), native-heap-observer-exclusions, caller-owned-host-values, live-provider-http-chunking (not-observed), and python-typescript-lifecycle (not-enrolled). The contract states that the runner fails when an enrolled test is not run or is skipped.

Residual observation, not a failure: the review noted that no enrolled check covers secret-free display or debug output of Rust stack-encrypt public errors. The diagnostics requirement cites guest-abi and Go SDK redaction tests. It does not cite a Rust public error display test. The issue asks only to map existing tests, and this is mapped for the guest and Go SDK layers.

Limit of this review: I did not read the test bodies. Test existence and execution rest on the index and the contract, which state that the runner checks inventory and execution.

Secure Buffer adoption (#1468), key storage and copy removal (#1458), and Python concurrency and parity (#1393, #1394) are separate work. The PR correctly does not implement them.

Full details: Out of Scope Changes check

Explanation

The PR's own change is a single commit, 247cde3 ("test(encrypt): enroll lifecycle conformance"), on top of the merge base 07a5a14. The CI commits 2995fbb (CRAP and cached Rust runtimes) and 14d9a18 (cached cargo-binstall shim) are already in the merge base from #1492. They are not changes introduced by this PR. Their absence from the whole-PR file list confirms this. The author-reported cargo-binstall pin and Ubuntu 24.04 CRAP change therefore do not appear as this PR's changes.

All changed paths from the merge base to head support lifecycle enrollment, the shared runner, or its verification:

  • Test additions: packages/stack-encrypt/tests/heap_hygiene.rs (late read failure case), packages/stack-encrypt/tests/lifecycle_conformance.rs (drop and cancel probes), and packages/stack-kms/src/futures.rs (bounded chunk concurrency and drop test). These add evidence for lifecycle-ownership, cancellation, partial-output, buffer-cleanup and provider-chunk requirements.
  • Runner and shared helpers: scripts/stack-conformance-evidence.mjs, scripts/stack-lifecycle-conformance.mjs, and scripts/stack-eql-conformance.mjs. The EQL script now delegates to the shared helpers and checks its profile. This is refactoring that the PR states is needed to share inventory and execution checks. The author reports EQL behavior is preserved.
  • CI wiring: .github/workflows/tests-golang.yml adds a required lifecycle step with report upload, and mise.toml adds test:conformance:lifecycle.
  • Evidence and documentation: the enrollment index, lifecycle-contract.md, README, coverage.md, the conformance plan, and the naming and wiring tests in scripts/tests.

No production behavior change is described, and no production source hunk is identified as unrelated. The test changes in futures.rs are test-only additions.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 11 functions across 8 files. (7 skipped: 7 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI; Enforced overrides: Organization
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5054ceda-7991-4864-801f-33410eb50ffe
Commits

Reviewing files that changed from the base of the PR and between 27ded58 and 247cde3.

Files selected for processing: 11
  • .github/workflows/tests-golang.yml
  • conformance/stack-encrypt/README.md
  • conformance/stack-encrypt/coverage.md
  • conformance/stack-encrypt/lifecycle-contract.md
  • conformance/stack-encrypt/v1/lifecycle-v1.enrollment.json
  • docs/plans/2026-10-31-stack-encrypt-conformance.md
  • mise.toml
  • scripts/__tests__/stack-conformance-ci.test.mjs
  • scripts/__tests__/stack-lifecycle-conformance.test.mjs
  • scripts/stack-eql-conformance.mjs
  • scripts/stack-lifecycle-conformance.mjs
Files skipped from review as they are similar to previous changes: 1
  • docs/plans/2026-10-31-stack-encrypt-conformance.md

Limit details: You’ve used all 2 included reviews currently available.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Mutation testing (cargo-mutants, --in-diff, stack-auth + stack-encrypt)

No mutants were generated for the changed lines.

@coderdan

coderdan commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Review: enroll lifecycle and key-I/O conformance

Reviewed 91911cd48 against feat/stack-encrypt-eql-json-conformance, on two axes: the repo's coding standards, and #1480 plus the conformance plan. Locally both vitest files passed (10 tests), and nextest passed on stack-kms futures plus the lifecycle_conformance and heap_hygiene binaries. Two hand mutations: .buffered(n+1) makes concurrency_limit_bounds_chunks_and_drop_releases_them fail, so that test discriminates; the heap test result is under Spec (c).

Standards

No breach of a documented rule.

  • Checked and fine. The lifecycle step runs in wasi-check, which sets MISE_ENV: test, and stack-conformance-evidence.mjs:126 calls cargo through mise x --env test with --locked. The MISE_ENV gap flagged on test(stack-encrypt): enroll EQL and JSON conformance #1492 doesn't recur here, and stack-lifecycle-conformance.test.mjs:97 pins it.
  • Also fine. The new mise task has a CI caller. No issue numbers were added. No changeset is needed.

Code smells (judgement calls):

  • Duplicated code. stack-lifecycle-conformance.mjs:14-69 and stack-eql-conformance.mjs:42-131 repeat the same main(): check the environment, set STACK_CONFORMANCE_REQUIRED, clear the update variables, build the report, loop over suites, write to STACK_CONFORMANCE_REPORT_DIR. The PR already extracts a shared evidence module, so prepareRequiredEnv() and writeReport() belong there. So does EQL's metadata(workspace) helper, which lifecycle inlines (:29-31).
  • One fact, two keys. The Cargo.lock hash is lock_sha256 in the lifecycle report (:39) and root_lock_sha256 in the EQL report (:70).
  • Two tests for the same decision. Whether to hash the Go guests is language !== 'rust' in lifecycle (:43) but [...selected.values()].some(s => s.language === 'go') in EQL (:74). The selection-based form is the more robust one.
  • A re-export kept for one import. stack-eql-conformance.mjs:17-22 re-exports four evidence functions, apparently only so the old EQL test's import keeps working. The new lifecycle test imports from the module directly.
  • Layout. packages/stack-kms/src/futures.rs:92-93 has no blank line before the new #[test], unlike the tests around it. lifecycle_conformance.rs also runs struct/impl blocks together (:32-43, :69-70, :91-92).

Spec

Verified:

  • The CI step is required. It has no if: and no continue-on-error, and the report upload fails when the report is missing.
  • Path filters cover the enrolled code, including packages/stack-kms/** and stack-guest-abi.
  • Moving the shared logic out of the EQL script keeps its behaviour.
  • The cancel test's provider_drops is a real observation. It would fail if the provider future were detached or spawned.
  • Python/TypeScript are recorded as not-enrolled, and the Go post-decrypt skip is kept out of the passing evidence.

(a) Missing or partial:

  • "Map … secret-free diagnostics." lifecycle-diagnostics-v1 cites native evidence only from the guest ABI (lifecycle-profiles.json about lines 190-207). Nothing enrolled checks that stack-encrypt's own Error Display/Debug output is free of plaintext.
  • "Explicitly identify unsupported/unobserved behavior." lifecycle-contract.md says in prose that Go has no native provider counters and that Go logical batching is not independently counted. Neither is in the limitations index, so neither appears in reports.

(c) Looks implemented, but weak or wrong:

  • The heap test misses a leaked prefix. Adding std::mem::forget(fields) before the FieldType return in open_record (record.rs about line 2600) still lets a_late_read_type_failure_leaves_no_plaintext_on_the_heap (heap_hygiene.rs:341) pass. The heap observer sees only frees, so a leaked decoded prefix goes undetected. Yet the contract says the batch "releases its decoded prefix without marked heap residue" (lifecycle-contract.md:15). Either reword it to "freed blocks are wiped" or add leak accounting.
  • The new drop-witness tests prove less than the contract claims. The witnesses are test-owned values moved into a Pending::map closure (lifecycle_conformance.rs:107-116), so "released exactly once" is mostly what Rust ownership already guarantees unless Pending leaks. The tests never observe the operation's own captures (rows, plan, keys). The contract's "The drop witnesses belong to fulfilment closures and provider futures in real record operations. They establish ownership release" (lifecycle-contract.md:23-24) overstates this.
  • A misleading error message. The shared selections() throws No EQL conformance evidence selected (stack-conformance-evidence.mjs:59) for the lifecycle profile too.

(b) Scope creep: none of substance. lib: true suite support and the per-suite Go package/success_output fields are needed to enrol the unit suites.

Summary

  • Standards: no breaches of documented rules, 5 smells. The biggest is the duplicated main() across the two runner scripts.
  • Spec: 2 partial requirements, 3 that look weak or wrong. The worst is that the heap test misses a leaked decoded prefix, though the contract says the prefix is released.

coderdan commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the shared CI bootstrap failure in 7a3dec9 (also merged into #1493). Every failed job stopped before tests with No version is set for shim: cargo-binstall: EQL setup left a cached shim, but the root test environment selected no version. mise.test.toml now pins the prebuilt cargo-binstall installer at 1.26.2, so mise can resolve it before installing nextest and the other cargo tools.

Verified with CI's exact mise 2026.4.0 in isolated tool/Cargo directories: the original configuration reproduces the failure; adding the pin installs and runs cargo-nextest 0.9.146 successfully. All 47 relevant CI/tooling regression tests and formatting checks pass. Fresh CI runs have been triggered on both PRs.

coderdan commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in 65d4937 and carried into the stacked PR #1493.

The CRAP failures were executable compatibility failures after coverage tests passed: cargo-crap 0.6.1's downloaded Linux binary requires GLIBC 2.39. Both CRAP jobs now run on Ubuntu 24.04 with matching mise cache keys, and check cargo crap --version before collecting coverage. The scoring thresholds and coverage scope are unchanged.

Also fixed the PostgreSQL job's cached Rust toolchain failure: it now reconciles both configured Rust toolchains and verifies cargo before restoring the build cache.

Verified in GitHub Actions on both PRs: CRAP (stack-auth), CRAP (stack-encrypt), the full WASI/Go job, and Go rows/queries plus JSON conformance in PostgreSQL all passed. Locally, all 116 relevant CI regression tests and the 10 child conformance-runner tests passed.

@coderdan
coderdan marked this pull request as ready for review November 3, 2026 11:33
@coderdan
coderdan requested a review from a team as a code owner November 3, 2026 11:33
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T11:34:46.624436Z 27ded58 Draft marked ready
🔒 Security Review ✅ Completed 2026-11-03T11:35:05.678971Z 27ded58 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cipherstash-agent cipherstash-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review attestation

Reviewed 27ded58 · posted from Claude Code · claude-sonnet-5-5 · effort medium
Sources: code-review · claude-sonnet-5-5
Skills: code-review, review-cross-check

No findings.


Generated by Claude Code

Enrolls native Rust and Go WASI lifecycle, cleanup and key-I/O evidence
as the lifecycle-v1 profile, and moves the execution and non-vacuity
checks the EQL/JSON runner had into scripts/stack-conformance-evidence.mjs
so both runners share them.

The enrollment index is conformance/stack-encrypt/v1/lifecycle-v1.enrollment.json,
following the <profile>.<role>.json convention, and carries its profile
in its header; each runner now checks its index's profile, since the
shared selection no longer knows which profile it is reading. The naming
guard requires a cases file only beside expected outputs and runner
inputs: an enrollment index needs none, and lifecycle-v1 has no cases.
@coderdan
coderdan force-pushed the feat/stack-encrypt-lifecycle-conformance branch from 27ded58 to 247cde3 Compare November 3, 2026 11:59
@coderdan

coderdan commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto #1492 (07a5a142f), which is now on top of #1491's file-naming convention. This was a force push. The branch is now one linear commit, 247cde3d2, in place of b05d943c0 and the three merge commits from #1492. None of those merges resolved a conflict, so dropping them loses nothing.

Before committing, I checked the result against the old head 27ded587c for every file this PR changes. The Rust, Go workflow, mise.toml and stack-conformance-evidence.mjs changes are identical. The other differences are all deliberate:

Run locally on 247cde3d2, with all four guests built:

  • mise run test:conformance:lifecycle: passed. It ran 19 Go, 3 Go SDK and 6 Go memory tests, plus the native lane.
  • mise run test:conformance:eql: passed, with 21 Go tests.
  • 701 of 701 tests in the stack-kms lib and the stack-encrypt conformance, json_operators, lifecycle and heap-hygiene binaries.
  • 124 CI-wiring script tests pass, and Biome is clean.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stack-encrypt conformance: define lifecycle, cleanup and key-I/O requirements

2 participants