feat(#45): ZK-lite Secure Disclosure — responsible vulnerability disclosure#54
feat(#45): ZK-lite Secure Disclosure — responsible vulnerability disclosure#54Nicolas0315 wants to merge 3 commits intomainfrom
Conversation
…ponsible vulnerability disclosure - Add SecureDisclosureManager with full lifecycle management - ZK-lite proof: commitment = SHA-256(category||severity||nonce||secret) proof = SHA-256(category||severity||nonce||blindingFactor) attackDetails never included in public proof - markPatched() + revealFullReport() enforce patch-first disclosure - revokeProof() with ImmutableLedger revocation event - getTimeline() returns full discovery→proven→patched→revealed chain - 29 unit tests covering all acceptance criteria and edge cases Closes #45
Allow callers to specify the disclosing party's ID. Falls back to 'system' for backwards compatibility.
Nicolas0315
left a comment
There was a problem hiding this comment.
RALPH PR Review — #54 ZK-lite Secure Disclosure
Verdict: NEEDS CHANGES (1 bug, 1 design note)
🐛 Bug: patchedAt timestamp is always wrong
In revealFullReport(), the patchedAt field is set to new Date().toISOString() — the reveal time, not the actual patch time.
Since the full timeline (discoveredAt → provenAt → patchedAt → revealedAt) is the core audit value of this module, having patchedAt === revealedAt defeats the purpose.
Fix: Store patchedAt on the ZkLiteProof object when markPatched() runs:
// In ZkLiteProof interface — add:
patchedAt?: string;
// In markPatched():
zkProof.patchedAt = new Date().toISOString(); // store here
// In revealFullReport():
patchedAt: zkProof.patchedAt ?? now, // use stored value📝 Design Note: nonce is not stored — post-hoc verification limited
verifyProof() only checks that hashes are 64 hex chars (valid SHA-256 format), not that they were honestly generated. A third party cannot independently verify the commitment after the fact.
Minimal fix: Add a JSDoc note to verifyProof() clarifying it is a "format/state integrity check, not cryptographic verification", so callers don't rely on it for security decisions.
Better fix: Store nonce in ZkLiteProof (not sensitive — security comes from blindingFactor), enabling verifyProof() to re-derive and confirm the proof hash.
✅ What's good
attackDetailscorrectly excluded from publicproof✅- State machine enforcement (no step skipping, no double-revoke) ✅
- 29 well-structured tests covering all lifecycle paths ✅
- No new dependencies, no breaking changes ✅
- ImmutableLedger integration is clean ✅
The patchedAt bug is the only blocker. Nonce note can be a follow-up. After those two items this is merge-ready.
🤖 RALPH Review — Claude Opus 4.6
Fixes two issues raised in RALPH review of PR #54: 1. Bug fix: patchedAt was set to the reveal time (now) in revealFullReport(). - Add patchedAt?: string to ZkLiteProof interface - Set zkProof.patchedAt = now in markPatched() - Use zkProof.patchedAt ?? now in revealFullReport() - This ensures the full audit timeline (discovered→proven→patched→revealed) is accurate; patchedAt now correctly reflects when the patch was applied 2. JSDoc improvement: verifyProof() was misleadingly described as 'verification'. - Add explicit warning that this is a format/state integrity check only, not cryptographic verification - Note the nonce limitation and suggest future extension if needed 3. Test: add 'patchedAt from markPatched, not revealFullReport' regression test All 217 tests pass / ESLint clean
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Nicolas0315
left a comment
There was a problem hiding this comment.
RALPH Follow-up Review — PR #54 ZK-lite Secure Disclosure
Verdict: APPROVE ✅ — Previous blockers resolved
Changes since last review (commit 25fcee3)
🐛 Bug Fix: patchedAt timestamp corrected ✅
Added patchedAt?: string to the ZkLiteProof interface. markPatched() now stores the actual patch timestamp on the proof object. revealFullReport() consumes zkProof.patchedAt ?? now — ensuring the audit timeline correctly records when the patch was applied, not when the report was revealed.
Regression test added: "should set patchedAt to the time markPatched was called, not revealFullReport" — asserts patchedAt <= revealedAt and that patchedAt falls within the markPatched call window.
📝 Design Note: verifyProof() JSDoc updated ✅
Added an explicit warning clarifying this is a "format/state integrity check, not cryptographic verification", notes the nonce limitation, and recommends the future extension path (storing nonce in ZkLiteProof).
KS40e Score: 17/18
| Axis | Status |
|---|---|
| Security | ✅ attackDetails still excluded from proof; no new surface |
| Quality | ✅ 217/217 tests pass (30 for this module, +1 regression) |
| Style | ✅ ESLint clean, consistent conventions |
| Risk | ✅ No breaking changes, additive fix only |
| CrossModal | ✅ New test directly validates the fix in the implementation |
| Integration (−1) |
This PR is now merge-ready. The single blocker (patchedAt bug) is fixed and covered by a regression test. The nonce limitation is documented for future reference.
🤖 RALPH Follow-up Review
Summary
ZK-lite hash commitment scheme for responsible vulnerability disclosure.
Proves "this vulnerability of this severity exists" without revealing attack details.
Changes
src/lib/vuln/ZkLiteDisclosure.tsSecureDisclosureManager, types, ZK-lite proof schemesrc/lib/vuln/__tests__/ZkLiteDisclosure.test.tssrc/lib/vuln/index.tsHow it works
Verifiers can confirm
categoryandseverityare public. TheattackDetailsnever appear inproof.Lifecycle:
createProof→markPatched→revealFullReportAll events recorded to
ImmutableLedger(existing infrastructure).Acceptance Criteria
revealFullReportenforcespatchedstate prerequisite)getTimeline)revokeProofwith ledger event)Self-Review (RALPH/Opus — Request)
Security ✅
crypto.randomUUID()Quality ✅
Style ✅
ImmutableLedger+crypto.subtle)Risk ✅
Test Plan
vitest run)Closes #45
🤖 Generated by RALPH Issue Pipeline
Branch:
ralph/issue-45