fix(core): route seed access through the zeroizing wrapper (#41) by beardthelion · Pull Request #64 · Gitlawb/node

beardthelion · 2026-06-20T03:03:50Z

Summary

Keypair::seed_bytes() handed out a bare [u8; 32] copy of the Ed25519 private seed. Because [u8; 32] is Copy with no Drop, each caller's copy was released without being scrubbed, leaving secret key material in freed memory. This removes that accessor and routes seed access through the existing zeroizing wrapper.

Motivation & context

Closes #41

to_seed() -> Zeroizing<[u8; 32]> already existed right beside seed_bytes() and scrubs the copy on drop. There was no reason to keep an unzeroized accessor: it was duplicated, and the only two callers were both in gitlawb-core. Removing it makes to_seed() the single seed accessor and closes the exposure window to core dumps, swap, and memory-disclosure bugs.

Kind of change

What changed

Crate touched: gitlawb-core.

Removed Keypair::seed_bytes() (raw, unzeroized seed accessor) in identity.rs.
Migrated the two callers to to_seed(): the envelope-decryption path in encrypt.rs (open_blob) and a crypto agreement test. The migration is mechanical: Zeroizing<[u8; 32]> deref-coerces to &[u8; 32], which is what x25519_secret_from_seed already takes.
Added to_seed_is_sole_seed_accessor documenting and guarding the single-accessor invariant.

No behavior change to envelope encryption/decryption: the X25519 secret derived from the seed is byte-identical, covered by the existing round-trip tests.

How a reviewer can verify

cargo test --workspace
cargo clippy --workspace --all-targets -- -D warnings
# Confirm no raw accessor remains:
git grep -n '\.seed_bytes()' crates/   # only the unrelated p2p local var, no Keypair method

Before you request review

Scope is one logical change; no unrelated churn
cargo test --workspace passes locally
New behavior is covered by tests (required for fixes)
cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings are clean
Commit titles use Conventional Commits (feat(...), fix(...), docs(...))
Docs / .env.example updated if behavior or config changed (or N/A)
Checked existing PRs so this isn't a duplicate

Protocol & signing impact

Touches DID / did:key, Ed25519 / RFC 9421 signatures, UCAN, ref certs, or P2P wire formats
Discussed in an issue before implementation
Backward-compatible with existing nodes and previously signed history

This is in-memory hygiene only. It does not change the signature scheme, key format, DID derivation, or any wire format. The bytes produced and consumed are identical; only the container type of a transient changes.

Notes for reviewers

There is a related, deliberately out-of-scope leak one hop downstream: x25519_secret_from_seed returns the derived X25519 secret as a bare [u8; 32]. It will be filed as its own follow-up issue to keep this PR scoped to exactly #41.

Summary by CodeRabbit

Security
- Strengthened cryptographic seed handling by switching to a secure, automatically-cleared seed accessor for envelope decryption.
Breaking Changes
- Removed the previous raw seed accessor; seed retrieval is now provided only through the secure accessor.
Tests
- Updated and added unit tests to confirm deterministic seed derivation and consistent key reconstruction from the derived seed.

Remove Keypair::seed_bytes(), which returned a bare [u8; 32] copy of the Ed25519 private seed. Because [u8; 32] is Copy with no Drop, each caller's copy was released without being scrubbed, leaving secret key material in freed memory. The zeroizing accessor to_seed() -> Zeroizing<[u8; 32]> already existed beside it; this makes it the sole seed accessor and migrates the two in-crate callers (envelope open path and a crypto test) to it via deref coercion. No behavior change to envelope encryption: the derived X25519 secret is byte-identical, covered by the existing round-trip tests. Adds an identity test pinning to_seed() as the only seed accessor.

coderabbitai · 2026-06-20T03:04:01Z

Warning

Review limit reached

@beardthelion, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 38 minutes and 47 seconds. Learn how PR review limits work.

To continue reviewing without waiting, enable usage-based billing in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7e4d454e-9186-4afd-858f-b93bd00fc968

📥 Commits

Reviewing files that changed from the base of the PR and between 4f81445 and e4f5e87.

📒 Files selected for processing (1)

crates/gitlawb-node/src/api/repos.rs

📝 Walkthrough

Walkthrough

seed_bytes() returning a plain [u8; 32] is removed from Keypair; to_seed() returning Zeroizing<[u8; 32]> becomes the sole seed accessor. The open_blob function and its companion test in encrypt.rs are updated to call to_seed() instead.

Changes

Keypair Seed Zeroization Hardening

Layer / File(s)	Summary
Keypair: remove `seed_bytes`, promote `to_seed` `crates/gitlawb-core/src/identity.rs`	Removes `pub fn seed_bytes(&self) -> [u8; 32]`, replaces it with `to_seed()` returning `Zeroizing<[u8; 32]>`, updates docs to designate `to_seed()` as the sole seed accessor for envelope decryption, and adds a test asserting the return type is `Zeroizing<[u8; 32]>` with deterministic seed values.
`open_blob` and test: migrate to `to_seed()` `crates/gitlawb-core/src/encrypt.rs`	Replaces `keypair.seed_bytes()` with `keypair.to_seed()` in `open_blob` for X25519 secret derivation, and updates the `ed25519_to_x25519_keypair_agrees` test to derive seed via `to_seed()` for consistency.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 A seed once bare, now wrapped with care,
Zeroizing guards what must not linger there.
seed_bytes gone — no plain array remains,
to_seed alone now holds the reins.
The key forgets itself on drop — hooray!
Memory-safe hops make for a safer day. 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: removing unzeroized seed access and routing through a zeroizing wrapper for security hardening.
Description check	✅ Passed	The PR description comprehensively covers motivation, changes, verification steps, and protocol impact, aligning well with the provided template sections.
Linked Issues check	✅ Passed	The PR successfully addresses issue `#41` by removing the unzeroized `seed_bytes()` accessor and migrating both identified callers in gitlawb-core to the zeroizing `to_seed()` alternative.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue `#41`: removing `seed_bytes()`, migrating its two callers, and adding a test to guard the single-accessor invariant.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/zeroize-seed-bytes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

…-trip The to_seed_is_sole_seed_accessor test was named for a compile-time property its body couldn't assert, and its from_seed -> DID check duplicated from_seed_round_trip. Replace it with a type-annotated binding that fails to compile if to_seed ever returns a bare [u8; 32] again, plus the determinism check, which is the only behavior it uniquely covered.

main grew a third seed_bytes() caller (repos.rs, from #40) after this branch was cut. Route it through to_seed() too: node_seed is moved into a long-lived tokio::spawn pin task, so the Zeroizing wrapper scrubs the copy when that task ends. encrypt_and_pin takes &[u8; 32]; the wrapper deref-coerces, so the call site is unchanged.

beardthelion · 2026-06-22T17:36:00Z

@kevincodex1 #64 is ready when you have a minute. It removes the unzeroized seed_bytes() accessor and routes seed access through to_seed() -> Zeroizing, so the seed copy scrubs on drop.

One thing to flag: main grew a third caller since I branched (the encrypted-pin path in repos.rs, from #40), so I merged main in and migrated that one too. It moves the seed into a long-lived spawn task, so wrapping it in Zeroizing is the same hardening rather than just a compile fix. Behavior is byte-identical, covered by the existing round-trip tests; full CI is green and CodeRabbit had no actionable comments.

It's also the base of #66 (the X25519 follow-up), so merging this unblocks that one.

kevincodex1

LGTM-

x25519_secret_from_seed derived the X25519 private scalar from the Ed25519 seed and returned it as a bare [u8; 32]. Because [u8; 32] is Copy with no Drop (and sha2's digest output likewise has no zeroize), the scalar, the SHA-512 digest, and the returned temporary were released without being scrubbed, leaving secret-derived material in freed memory. This is the same Copy-no-Drop class fixed for the raw seed in #41, on the derived secret. Return Zeroizing<[u8; 32]>, build the scalar directly into a zeroizing buffer so no bare secret local persists, and explicitly wipe the SHA-512 digest before it drops. Callers deref the result into crypto_box::SecretKey, which already scrubs its own copy. No behavior change: the derived scalar and all decryption output are byte-identical, covered by the existing ed25519_to_x25519_keypair_agrees and seal_open_round_trip_for_recipients tests. Stacked on #41 (#64).

beardthelion mentioned this pull request Jun 20, 2026

fix(core): zeroize the derived X25519 secret (#65) #66

Merged

17 tasks

beardthelion added kind:security Vulnerability fix or hardening crate:core gitlawb-core — identity, certs, encrypt, DID/UCAN subsystem:identity DID/UCAN, http-sig auth, push authorization sev:high Major break or real security/trust risk, no easy workaround labels Jun 22, 2026

beardthelion added 3 commits June 22, 2026 11:52

Merge remote-tracking branch 'origin/main' into fix/zeroize-seed-bytes

38e654a

kevincodex1 approved these changes Jun 23, 2026

View reviewed changes

kevincodex1 merged commit c9f43b0 into main Jun 23, 2026
13 checks passed

beardthelion mentioned this pull request Jun 23, 2026

fix(core): zeroize the derived X25519 secret (#65) #91

Merged

github-actions Bot mentioned this pull request Jun 30, 2026

chore(main): release 0.4.0 #131

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(core): route seed access through the zeroizing wrapper (#41)#64

fix(core): route seed access through the zeroizing wrapper (#41)#64
kevincodex1 merged 4 commits into
mainfrom
fix/zeroize-seed-bytes

beardthelion commented Jun 20, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 20, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

beardthelion commented Jun 22, 2026

Uh oh!

kevincodex1 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

beardthelion commented Jun 20, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation & context

Kind of change

What changed

How a reviewer can verify

Before you request review

Protocol & signing impact

Notes for reviewers

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

beardthelion commented Jun 22, 2026

Uh oh!

kevincodex1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

beardthelion commented Jun 20, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 20, 2026 •

edited

Loading