12 KiB
Software Quality Remediation Plan
Date: 2026-07-17 Scope: Address remaining open audit findings (F-H1 through F-H4, F-M1–F-M5, F-L1–F-L8) — all software-quality issues, not cryptographic-correctness gaps. Prerequisite: F-C1 (reclassified), F-C2/F-C3 (fixed), F-C4 (resolved) are done.
Priority tiers
The findings fall into three tiers by impact:
- Tier 1 (must-fix for production): F-H1 (tests), F-H2 (id checks), F-H3 (XSS), F-H4 (CSP/SRI), F-L8 (doc leftover)
- Tier 2 (should-fix): F-L2 (24-word default), F-L4 (e-tag id check), F-L5 (Content-Type), F-L6 (relay close-as-success), F-M5 (Falcon UI warning)
- Tier 3 (nice-to-have / design decisions): F-M1 (canonical attestation), F-M2 (block_height verification), F-M3 (xpub enforcement), F-M4 (truncation construction), F-L1 (zeroing), F-L3 (input validation), F-L7 (vendor nostr-login-lite)
Tier 1: Must-fix
F-H1: Add a test suite
What: Create a test file using Node's built-in test runner (no new dependencies). Update package.json test script.
File: test/pq-crypto.test.mjs (new)
Tests to include:
- BIP32→PQ-seed derivation known-answer: fixed mnemonic → expected BIP32 child private keys (verify determinism + correct path).
- PQ keygen determinism: same seed → same pubkeys for all 5 algorithms.
- PQ sign→verify round-trip: for each of ML-DSA-44, ML-DSA-65, SLH-DSA-128s, Falcon-512: sign a test message, verify passes; tamper message → verify fails; tamper signature → verify fails; wrong pubkey → verify fails.
verifyNIPQRContentaccept/reject:- Accept: genuine kind 1 event with valid PQ sigs + valid secp256k1 sig + matching e/sha256 tags.
- Reject: tampered content, tampered PQ sig, wrong pubkey, mismatched e tag, mismatched sha256 tag.
computeEventId/verifyNostrEvent: known Nostr test vector (from NIP-01 spec or a known event).- OTS parser tests:
isDetachedOtsFiletrue on valid prefix, false on random bytes;parseOtsFileonhello-world.txt.ots→ expected target digest + 1 Bitcoin attestation at height 358391;parseOtsFileonincomplete.txt.ots→ pending attestation. verifyOtsProofF-C3 binding: wrong expected digest →verified: falsewith "Target digest mismatch".
package.json change: "test": "node --test test/"
F-H2: verifyNostrEvent check event.id
What: Add an id check to verifyNostrEvent.
Change in pq-crypto.mjs:
export function verifyNostrEvent(event) {
const serialized = JSON.stringify([0, event.pubkey, event.created_at, event.kind, event.tags, event.content]);
const hash = sha256(new TextEncoder().encode(serialized));
const computedId = bytesToHex(hash);
if (event.id && event.id.toLowerCase() !== computedId) return false; // NEW
const sig = hexToBytes(event.sig);
const pubkey = hexToBytes(event.pubkey);
return schnorr.verify(sig, hash, pubkey);
}
Also fix F-L4 in verifyNIPQRContent: assert kind1Event.id (if present) equals computed id, or ignore it and rely on the computed id.
F-H3: Fix XSS — replace innerHTML with textContent for dynamic strings
What: Audit all innerHTML usages in index.html and verify.html. Replace with textContent where the content includes variables derived from relay/localStorage/user input. Keep innerHTML only for static HTML structure (no variables).
Key sites:
setStatus— used witherror.messagefrom relays. Change totextContent.www/index.html:750—otsNotice.innerHTMLwithpendingOts.eventId. Change to build the element withtextContentfor the variable part.setStatus— same pattern. Change totextContent.www/verify.htmlOTS info/badge — usesinnerHTMLwithhashMatchNoteetc. Change totextContent.
Approach: Create a helper function setText(element, text) that uses textContent, and replace all innerHTML calls that include dynamic data. For cases that need HTML structure (e.g., a link inside a status message), build the DOM with createElement + textContent for the variable parts.
F-H4: Add CSP and SRI
What:
- Add a CSP meta tag to both
index.htmlandverify.html:(Note:<meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self'; connect-src wss: https:; img-src 'self' data:; style-src 'self' 'unsafe-inline'; font-src 'self' data:;">style-src 'unsafe-inline'is needed because the pages have inline<style>blocks. Ideally these would be extracted to CSS files, but that's a larger refactor. The CSP still prevents script injection.) - Add SRI
integrityattributes to the bundle script tag. Compute the SHA-384 hash ofwww/pq-crypto.bundle.jsand add:Wait — module scripts with<script type="module" integrity="sha384-<hash>" src="./pq-crypto.bundle.js"></script>integrityneed thecrossoriginattribute too. Since the bundle is same-origin, this should work. The/nostr-login-lite/scripts are not in the repo so we can't compute their SRI — flag this as F-L7 (vendor them or document the deployment dependency).
Note: The CSP connect-src must allow wss: (relays), https: (OTS calendars, Bitcoin APIs, mempool.space, blockstream.info). The script-src 'self' blocks inline scripts — but both pages have inline <script type="module"> blocks. This is a problem. Options:
- (a) Extract the inline scripts to separate
.jsfiles (cleaner, but larger change). - (b) Use a CSP nonce: generate a per-page-load nonce and add
nonce="<random>"to the inline script tags +'nonce-<random>'to the CSP. This is the standard approach for inline scripts. - (c) Use
'unsafe-inline'for script-src (defeats much of the CSP purpose for XSS, but still blocks external script injection).
Recommendation: Option (b) — CSP nonce for the inline module scripts. This gives real XSS protection (an injected script without the nonce won't run) while keeping the inline scripts. The nonce is generated server-side or per-page-load in the HTML. Since this is a static site, the nonce can be generated in a small inline bootstrap script that sets it on the CSP meta tag and the module script tag. Actually, for a static site, the cleanest approach is (a) — extract the inline scripts to .js files. This is more work but gives the strongest CSP.
Decision needed: extract inline scripts to files (stronger, more work) or use nonces (pragmatic)? I'll plan for (a) extraction since it's the right long-term answer, but note (b) as an alternative.
F-L8: Fix why_what_how.md Component 4 leftover
What: why_what_how.md:64 says "the new identity signs to endorse the PQ keys" — successor-model language.
Fix: Reword to: "The old identity signs to authorize the migration. (The raw-nsec migration is a design proposal, not yet implemented.)" Or add a "Design only" note.
Tier 2: Should-fix
F-L2: Default to 24-word mnemonics (or warn)
What: generateSeedPhrase defaults to 128 bits (12 words). Change to 256 bits (24 words), or add a UI warning that 12-word gives only ~64-bit PQ security.
Recommendation: Add a UI warning rather than changing the default — 12 words is more user-friendly for a demo, and the warning educates. Add a note near the seed display: "For maximum post-quantum security, consider a 24-word seed phrase (12 words provides ~64-bit PQ security under Grover's algorithm)."
F-L4: e tag validation check kind1Event.id
What: In verifyNIPQRContent, assert kind1Event.id (if present) equals the computed id.
Fix: Add after computing computedKind1Id:
if (kind1Event.id && kind1Event.id.toLowerCase() !== computedKind1Id.toLowerCase()) {
eTagValid = false;
}
F-L5: Fix timestampEvent Content-Type
What: timestampEvent sends Content-Type: application/x-www-form-urlencoded with a binary body.
Fix: Change to Content-Type: application/octet-stream.
F-L6: Relay publishing — don't treat close as success
What: publishToRelays treats ws.onclose as success.
Fix: Treat close-without-OK as failure or "unknown". Only count as success on an explicit OK message.
F-M5: Falcon-512 draft-status UI warning
What: Add a note in the UI (near the Falcon key display) that Falcon-512 is based on FIPS 206 (draft) and may need re-issuing if the standard changes.
Tier 3: Nice-to-have / design decisions
F-M1: Canonical structured attestation
What: Sign a canonical JSON object (or its hash) instead of human-readable prose. This is a design change that affects the event format — it would require re-issuing any existing events. Recommend deferring unless there's a concrete need (e.g., the prose text needs to change).
F-M2: block_height verification
What: Verify the block height was current at signing time. This requires OTS verification (now implemented) to establish the signing time, then comparing to the claimed block height. Low priority — the block height is informational; the real anchor is the OTS proof.
F-M3: xpub enforcement
What: Add a code-level warning or check that the xpub at m/44'/1237'/0'/0' is never exported. Already documented — code enforcement is difficult since the xpub isn't published by this app. Recommend deferring.
F-M4: Truncation construction
What: Replace "take first N bytes" with HKDF-Expand on the concatenated children. Design change that would break existing key derivation — recommend deferring unless doing a major version bump.
F-L1: Zero secrets on sign-out
What: Clear pqKeys, pqSeed, pqMnemonic references on sign-out. The sign-out flow already clears some state; ensure all secret references are nulled. (JS can't guarantee memory zeroing, but clearing references helps.)
F-L3: Input validation for hexToBytes/base64ToBytes
What: Validate input format and length; throw clear errors on malformed input.
F-L7: Vendor /nostr-login-lite/ scripts
What: Copy the nostr-login-lite scripts into the repo (with SRI) or document the deployment dependency. This is a deployment/reproducibility issue, not a security issue per se.
Execution order
- F-H2 + F-L4:
verifyNostrEventid check +etag id check (small, high-value, inpq-crypto.mjs) - F-L5: Fix Content-Type (one-line change in
pq-crypto.mjs) - F-L6: Fix relay close-as-success (in
index.html) - F-H3: XSS — replace
innerHTMLwithtextContent(in both HTML files) - F-H4: CSP + SRI — extract inline scripts to
.jsfiles, add CSP meta tag, add SRI to bundle (both HTML files) - F-L8: Fix
why_what_how.mdleftover (one-line doc fix) - F-L2: Add 12-word PQ security warning (UI text in
index.html) - F-M5: Add Falcon draft-status warning (UI text in
index.html) - F-L1: Clear secret references on sign-out (in
index.html) - F-L3: Input validation (in
pq-crypto.mjs) - F-H1: Add test suite (new file +
package.json) - Rebuild bundle and verify tests pass
Tier 3 items (F-M1, F-M2, F-M3, F-M4, F-L7) are deferred unless you want them included.
Mermaid diagram
flowchart TD
H2[F-H2: verifyNostrEvent id check] --> L4[F-L4: e-tag id check]
L4 --> L5[F-L5: Content-Type fix]
L5 --> L6[F-L6: relay close-as-success]
L6 --> H3[F-H3: XSS - textContent]
H3 --> H4[F-H4: CSP + SRI]
H4 --> L8[F-L8: doc leftover]
L8 --> L2[F-L2: 12-word warning]
L2 --> M5[F-M5: Falcon warning]
M5 --> L1[F-L1: clear secrets]
L1 --> L3[F-L3: input validation]
L3 --> H1[F-H1: test suite]
H1 --> BUILD[Rebuild bundle + run tests]
style H2 fill:#fcc
style H3 fill:#fcc
style H4 fill:#fcc
style H1 fill:#fcc
style BUILD fill:#cfc