Skip to content

feat: initial dig-session facade (unlock/enroll/sign/inject) - #1

Merged
MichaelTaylor3d merged 2 commits into
mainfrom
feat/dig-session-init
Jul 20, 2026
Merged

feat: initial dig-session facade (unlock/enroll/sign/inject)#1
MichaelTaylor3d merged 2 commits into
mainfrom
feat/dig-session-init

Conversation

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor

Task

New crate dig-session (#1249, seam-7 session/keystore extraction) — the DIG session/keystore layer: unlock identity keys + inject signing primitives. Composes dig-keystore + dig-identity behind one curated, custody-safe facade. GPL-2.0-only. Tracks super-repo issue DIG-Network/dig_ecosystem#1249.

Public API

  • Session::unlock::<K: KeyScheme>(backend, path, password) -> Result<UnlockedIdentity<K>> — generic load -> unlock -> SignerHandle.
  • Session::enroll_identity(backend, path, password, seed) -> Result<UnlockedIdentity<L1WalletBls>> — derive the canonical dig-identity key once, store it, return it unlocked.
  • UnlockedIdentity<K>: public_key, sign, signing_fn, inject_into (yields a bare SigningFn primitive so consumers stay identity-agnostic, #908). Zeroizing, redacting Debug, no Clone.
  • SigningFn<K> = Arc<dyn Fn(&[u8]) -> K::Signature + Send + Sync>.
  • SessionError / Result.

Deviation from the locked design (custody-critical) — needs gate attention

The locked design said "store the DERIVED sk via Keystore::create under BlsSigning". That is INCORRECT: BlsSigning reconstructs its stored bytes via chia_bls::SecretKey::from_seed (a one-way KDF) on every unlock, so storing an already-derived identity sk under BlsSigning re-derives a DIFFERENT key (the documented #64/#57 pitfall) whose public key would NOT match the DID-anchored identity key.

Resolution: the identity path stores the derived sk under L1WalletBls (the from_bytes faithful round-trip scheme). Session::unlock stays generic, so BlsSigning is still available for seed-derived validator keys. A regression test (C-2) asserts the enrolled public key equals dig_identity::public_key_bytes(derive_identity_sk(master)), and would fail on any revert to BlsSigning.

Scope

No seal/decap (belongs to dig-message, same crate level). First cut = unlock/sign/inject only.

Verification

  • cargo test: 8 integration tests + 1 doctest green.
  • cargo fmt --check: clean. cargo clippy --all-targets -D warnings: clean.
  • cargo llvm-cov: 98.18% lines / 96.34% regions (floor 80%).
  • Deps crates.io-only, reference-down (dig-keystore 0.4.1, dig-identity 0.4.2, both live). Layering legal.

Blast radius

New standalone crate, zero existing callers. Not yet a superproject submodule (main adds it after publish). No cross-repo symbol edits.

Draft until CI green; do NOT merge (awaiting triple gate). Do NOT publish to crates.io yet.

MichaelTaylor3d and others added 2 commits July 20, 2026 10:34
Introduce the DIG session/keystore layer: a custody-safe facade composing
dig-keystore (encrypted storage) + dig-identity (canonical BLS derivation).

- Session::unlock<K> — generic load -> unlock -> SignerHandle
- Session::enroll_identity — derive the canonical identity key once and store it
  under L1WalletBls (from_bytes round-trip), NOT BlsSigning (from_seed would
  re-derive a different key — dig_ecosystem #64/#57)
- UnlockedIdentity — zeroizing, redacting Debug, no Clone; sign + inject a bare
  SigningFn primitive so consumers stay identity-agnostic (#908)

No seal/decap (belongs to dig-message). SPEC.md is the normative contract.
Full CI gate set + release/publish workflows from commit 1.

Closes #1249

Co-Authored-By: Claude <noreply@anthropic.com>
The org has GitHub Advanced Security default setup enabled (as do sibling
crates dig-keystore/dig-identity, which ship no codeql.yml). An advanced
CodeQL config cannot upload SARIF while default setup is enabled, so the
custom workflow failed. Remove it and let default setup provide CodeQL.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review July 20, 2026 17:41

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

CORRECTNESS GATE — PASS (custody-adjacent, independent verify)

Verdict: PASS. Reviewed against #1249 acceptance criteria, §2.5 readable-code, §5.1 zeroize/custody, #908 identity-agnostic boundary. All 4 required checks green (version-increment, coverage 98.18% >=80%, commitlint, test suite); zero open review threads.

L1WalletBls scheme choice — CORRECT (verified against dig-keystore 0.4.1 source)

  • L1WalletBls::secret_to_secret_key uses SecretKey::from_bytes (l1_wallet_bls.rs:87-98) — a faithful round-trip of the exact derived scalar. Enrollment stores derive_identity_sk(master).to_bytes() (session.rs:72-74) and unlock reconstructs it byte-identically.
  • BlsSigning::secret_to_bls_secret_key runs SecretKey::from_seed (bls_signing.rs:51-61) — storing the already-derived key there would re-derive a DIFFERENT key. The chosen scheme avoids exactly the #64/#57 double-derivation bug.
  • Regression test C-2 (enrolled_public_key_matches_dig_identity_canonical, tests/facade.rs:41-63) computes the expected pubkey INDEPENDENTLY via dig_identity::public_key_bytes(derive_identity_sk(master)) and asserts equality — it genuinely proves parity and WOULD fail under a revert to BlsSigning. Confirmed real, not vanity.

Zeroize delegation — SUFFICIENT, marker trait NOT needed

  • The only secret material lives in SignerHandle's Zeroizing<Vec<u8>> (signer.rs:19-23), wiped on drop. UnlockedIdentity wraps that handle + a PublicKey (non-secret) — no independent secret buffer to zeroize, so #[derive(ZeroizeOnDrop)] would add nothing.
  • signing_fn clones the SignerHandle into the Arc closure (unlocked.rs:67-70); the clone is itself a Zeroizing buffer wiped when the closure drops. Enrollment's transient Zeroizing secret (session.rs:74) is also wiped.
  • No Clone on UnlockedIdentity; Debug redacts secret AND public key (unlocked.rs:85-92, test C-8). Delegation is the correct design here.

API boundary (#908) — no identity type leaks

  • SigningFn<K> = Arc<dyn Fn(&[u8]) -> K::Signature + Send + Sync> (unlocked.rs:16). K::Signature resolves to chia_bls::Signature — a primitive. inject_into hands a consumer only the closure. Confirmed no dig-session/dig-identity type crosses the boundary.

Other gates

  • Reference-down legal (L10 -> L00 foundation); crates.io-only deps (dig-keystore 0.4.1, dig-identity 0.4.2, verified in Cargo.lock); chia-bls unified at 0.26.0 across the graph (no dual-version type mismatch on the dig_keystore::bls re-exports).
  • GPL-2.0-only correct (transitively links GPL dig-identity). SPEC.md normative + matches code. release.yml + publish.yml both carry workflow_dispatch (new-crate first-release race covered). readable-code: intent-revealing, WHY-comments present, small single-purpose modules.

Non-gating nit (does NOT block merge)

  • chia-bls = "0.26" is declared as a direct dependency in Cargo.toml:34 but is never referenced in src/ (only in doc-comment prose); the BLS types are re-exported from dig_keystore::bls. Harmless (resolves to the same 0.26.0 already in-tree) but it is an unused direct dep — safe to drop in a follow-up. Not a blocker.

No approval event posted (self-approval 422); this PASS review + zero unresolved threads unblocks the orchestrator's squash-merge once branch protection shows all-green.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

HOLD — merge deferred pending final gate leg + harness reconciliation

Status: NOT merged. dig-session is custody-critical (session unlock/sign for the DID-anchored identity key), so it merges only on a complete, real (non-glitched) gate set.

Gate state on this HEAD (383c48c7be47d9f7f3504abaa8d96adcd95b968e) — two of three legs cleared with real, tool-backed verdicts:

  • Correctness gate — PASS. All 4 required checks green (version-increment, coverage 98.18%, commitlint, test suite); zero open review threads. Confirmed the L1WalletBls scheme choice is correct (faithful from_bytes scalar round-trip; BlsSigning would double-derive via from_seed — the #64/#57 pitfall), zeroize delegation to SignerHandle's Zeroizing is sufficient, no dig-identity type leaks the public API. Non-gating nit: unused chia-bls direct dep (Cargo.toml:34).
  • Adversarial verify (independent Opus, custody dual-gate leg) — COULD NOT REFUTE. Traced the full key chain and proved the enrolled/unlocked public key equals dig_identity::public_key_bytes(derive_identity_sk(master)); SignerHandle is not re-exported (no expose_secret reachable), no Clone/Serialize, Debug redacts → #908 boundary intact. One LOW non-blocking nit: the identity_sk.to_bytes() stack temporary at session.rs:74 is not zeroized (follow-up hardening).
  • Security audit — PENDING (leg still running).

Resume condition: the security-audit leg returns a real (non-glitched) verdict on 383c48c7+. If it PASSes, dig-session is fully gated and mergeable — the two already-passed legs do NOT need re-running. Only re-run a leg that genuinely glitched.

Context: a harness-level env-glitch (some fresh gate contexts returning "No tools needed"/context-mode MCP "No such tool") is tracked in #1324. It did NOT affect this PR's correctness or adversarial legs (both executed tools fully). PR branch + HEAD are durable; no work is at risk.

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

CORRECTNESS GATE — PASS

Fresh-context correctness review of the new dig-session crate (L10 seam-7, custody-adjacent). All checklist items clear.

1. L1WalletBls scheme choice — CORRECT (custody-critical)

Verified against the actual dependency source, not just the docs:

  • dig-keystore L1WalletBls::secret_to_secret_key reconstructs via SecretKey::from_bytes (scheme/l1_wallet_bls.rs:97) — exact round-trip of the stored scalar.
  • BlsSigning reconstructs via SecretKey::from_seed (scheme/bls_signing.rs:60) — would re-derive a different key from the same bytes.
  • Keystore::create(Some(secret), …) stores the provided bytes verbatim after a == SECRET_LEN(32) check (keystore.rs:146-157) — no re-derivation on the write path.
  • enroll_identity (src/session.rs:72-85) derives derive_identity_sk(master_secret_key_from_seed(seed)) once, stores identity_sk.to_bytes() (a canonical in-range BLS scalar, so from_bytes accepts it) under L1WalletBls.

Storing the already-derived key under L1WalletBls (round-trip) rather than the locked design's BlsSigning (one-way from_seed) is the correct deviation — BlsSigning would silently yield a key whose pubkey does not match the DID-anchored identity. Documented at src/lib.rs:22-38 and SPEC.md:59-64.

2. C-2 regression is genuine, not tautological

tests/facade.rs:41-63 computes expected independently via dig_identity::public_key_bytes(derive_identity_sk(master_secret_key_from_seed(SEED))) and asserts the enrolled pubkey equals it. Reverting storage to BlsSigning would re-derive on unlock and this assertion would fail. Real tripwire.

3. inject_into / SigningFn — identity-agnostic (#908) confirmed

SigningFn<K> = Arc<dyn Fn(&[u8]) -> K::Signature + Send + Sync> (src/unlocked.rs:16). K::Signature resolves to the primitive chia_bls::Signature; no dig-session/dig-identity type appears in the injected signature. Consumers stay identity-agnostic.

4. Zeroize delegation — SUFFICIENT

UnlockedIdentity wraps SignerHandle<K> (secret in Zeroizing<Vec<u8>>, wiped on drop — dig-keystore/src/signer.rs:19-23) plus a non-secret PublicKey. No Clone on UnlockedIdentity; signing_fn clones the SignerHandle into an Arc closure, each buffer independently zeroized on last-drop; Debug redacts (src/unlocked.rs:85-92, test C-8). A literal #[derive(ZeroizeOnDrop)] is unnecessary because the only secret-bearing field self-zeroizes via delegation. Correct.

5. Layering / deps / license

Reference-down only (L10 → L00 dig-keystore 0.4.1, dig-identity 0.4.2), crates.io versions, no git deps, #![forbid(unsafe_code)], GPL-2.0-only. Correct.

6. Tests / coverage / SPEC / CI

C-1..C-8 all present and real (tests/facade.rs). CI green: fmt, clippy -D warnings, nextest, doctests, cargo doc, coverage gated >=80% (reported 98.18%). Branch protection: 0 required approvals, strict=true, required_conversation_resolution=true, required-check names match. release.yml/publish.yml carry workflow_dispatch. GHAS default setup (no custom codeql.yml) with least-privilege permissions: contents: read. 0 open review threads.

Non-gating notes (no action required)

  • Cargo.toml enables zeroize's derive feature but the crate uses only Zeroizing (no #[derive(Zeroize)]). Harmless unused feature.
  • identity_sk.to_bytes() yields a transient [u8;32] on the stack before the Zeroizing Vec copy; the array copy isn't explicitly zeroized (chia-bls SecretKey itself zeroizes on drop). Defense-in-depth only.

Verdict: PASS. Merge remains the orchestrator's action (checks green, zero unresolved threads).

@MichaelTaylor3d
MichaelTaylor3d merged commit 10dbf23 into main Jul 20, 2026
4 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the feat/dig-session-init branch July 20, 2026 18:01

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correctness gate (fresh context, custody-adjacent L10 session crate). VERDICT: CHANGES-REQUIRED — one gating finding on the stated zeroize criterion (item 2); everything else verified green.

VERIFIED GREEN:

  • Custody round-trip (item 1): enroll stores identity_sk.to_bytes(); L1WalletBls reconstructs via SecretKey::from_bytes (faithful), NOT from_seed. Confirmed against dig-keystore 0.4.1 source (l1_wallet_bls.rs from_bytes; bls_signing.rs from_seed). C-2 test asserts equality vs public_key_bytes(derive_identity_sk(master)) and genuinely fails if reverted to BlsSigning. 8 tests + doctest pass locally.
  • API shape (item 3): SigningFn leaks no key type; inject_into hands only the closure; #908 boundary held.
  • Derivation (item 4): m/12381'/8444'/9'/0' via dig_identity::derive_identity_sk; verified by the canonical-match test.
  • No Clone on UnlockedIdentity; Debug redacts secret + omits pubkey.
  • Reference-down: deps dig-keystore 0.4.1 / dig-identity 0.4.2 / chia-bls / zeroize / thiserror, all crates.io, no git dep. GPL-2.0-only. Branch protection strict / 0 approvals / conversation-resolution + linear. 4 required checks green. 0 pre-existing threads.

Comment thread src/session.rs

// Derive the canonical identity signing key ONCE.
let master = master_secret_key_from_seed(seed);
let identity_sk = derive_identity_sk(&master);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

GATING (item-2 zeroize): the master and identity_sk locals are chia_bls::SecretKey, which in chia-bls 0.26 is #[derive(PartialEq, Eq, Clone)] over a blst_scalar with NO Zeroize/Drop impl. Both hold the raw identity signing scalar and are dropped WITHOUT wiping when enroll_identity returns, leaving key material in freed stack memory. Only the extracted secret bytes (line 74) are wrapped in Zeroizing, so the stated criterion 'enroll ... derived master sk zeroized on all paths' is not met, and the Cargo.toml claim 'zeroize derived secrets on drop' is only partially delivered.

Resolve by either: (a) minimizing/wiping — e.g. derive into the bytes and explicitly overwrite the SecretKey memory via a helper, or gate on an upstream chia-bls zeroize-capable version; or (b) if wiping an un-Zeroize-able upstream type is genuinely infeasible at 0.26, document the residual exposure explicitly in the enroll doc-comment + DEVELOPMENT_LOG and narrow the Cargo.toml claim. loop-security/adversarial run separately and own the final custody call on severity.

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