Skip to content

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

Draft
coderdan wants to merge 4 commits into
feat/stack-encrypt-eql-json-conformancefrom
feat/stack-encrypt-lifecycle-conformance
Draft

coderdan wants to merge 4 commits 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, including its review fixes at e6ce5d0.

Closes #1480.

@changeset-bot

changeset-bot Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 27ded58

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

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

@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.

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.

1 participant