fix: amber badge fails closed on unverified content#33
Open
Aina-India wants to merge 9 commits into
Open
Conversation
Stand up the first automated tests for the trust-critical signature path, which had zero coverage, and make CI fail on test failure. No behavior changes — this pins current correct behavior and builds the cross-language vector contract that keeps the Go and JS verifiers byte-compatible. - Go: signing_test.go in mirror + publisher — round-trip sign/verify, tamper rejection, untrusted-signer rejection, legacy/multi-sig normalization, and a vector-driven test over the shared fixture. - JS: extract the pure dedup/threshold logic from verify.js into verify-logic.js (single source of truth) and exercise it with a zero-dependency Node runner (node:assert + crypto.webcrypto, Node 16+). verify.js now imports the module; its <script> tags become type=module across all pages (consistent with forms.js). package.json gains "type":"module" so Node treats the shared file as ESM. - Shared contract: testdata/latest.vectors.json (read by both Go and JS) plus scripts/gen-test-vectors.mjs, a deterministic generator using test-only seeds derived from fixed strings (no real keys, nothing secret). - CI: a gating test job (setup-go + go test both packages + JS test) and a post-build assertion that every page loading verify.js uses type=module — a safety net so future templating (grussdorian#16) can't silently break the badge. The duplicate-signature dedup test and fix follow in PR-2 (grussdorian#13); the build-determinism test in PR-8 (grussdorian#18); rollback-refusal coverage in PR-4 (grussdorian#20).
The previous implementation counted every valid (signer, signature) entry, so a latest.json with the same key listed twice could satisfy a threshold-2 requirement with only one real party's key. Fix: track seen signers in a set and skip duplicates before incrementing the valid count. Changed in all three verifier implementations to keep them byte-compatible (Guardrail G3): packages/mirror/signing.go, packages/publisher/signing.go, packages/site/js/verify-logic.js. Adds a duplicate_signer cross-language test vector (threshold 2, one signer repeated twice → expectedValidCount 1, expectedValid false) to testdata/latest.vectors.json, exercised by both Go and JS test suites.
Previously, PoW ran as a page-load widget, producing a token tied only to a random challenge string — entirely detached from the Nostr event eventually submitted. A fresh challenge could be pre-mined and reused for any content, making the anti-spam guarantee meaningless. This commit binds PoW to the actual event being submitted: - pow-worker.js: add NIP-13 mode — mines the event id (SHA-256 of the NIP-01 canonical serialization [0,pubkey,created_at,kind,tags,content]) until it has POW_DIFFICULTY leading zero bits; embeds the nonce as a ["nonce","<n>","<difficulty>"] tag per the NIP-13 spec. - forms.js: rewritten to call mineEventPoW() on submit — PoW is bound to the ephemeral sk/pk and encrypted content of each submission; the legacy getCaptchaToken() call is removed entirely. - join.html / demand.html: remove the pow-widget .fg div and the page-load inline <script> that started the old PoW worker. - test/pow-nip13.test.mjs: regression test covering leadingZeroBits contract, NIP-01 serialization format, and end-to-end mining at difficulty=4 (all 9 assertions pass). - publish.yml: add "JS NIP-13 PoW tests" step to the test job.
grussdorian#20) A validly-signed latest.json can be replayed indefinitely: nothing in the daemon's poll loop or the browser verifier checked how old the publisher's timestamp was. An attacker who captured a signed manifest once could keep re-serving it long after the real content moved on. Changes: - staleness.go: isFreshEnough(l, maxAge) — returns false when l.Timestamp is older than maxAge (zero timestamp or zero maxAge skips the check for backwards compat). - config.go: MaxAge time.Duration field, default 48h via MAX_AGE env var. - daemon.go: call isFreshEnough() after signature verification; log and return without pinning when the manifest is stale. - verify.js: Step 3b — if latest.timestamp is > 48 h old, set the .cjp-badge--outdated state instead of proceeding to the integrity check. Reuses the existing CSS class already in style.css. - staleness_test.go: 5 table-driven tests (fresh/stale/boundary/zero-ts/ zero-maxAge) — all pass.
- Add content/translations/{en,hi,ta,te,bn}.json — all five languages
en: 5 pages; hi/ta/te/bn: 3 pages each (index, join, demand)
- Add packages/site/templates/{index,join,demand,mirror,trust}.html
{{base}}, {{lang_switcher}}, {{canonical_og_block}} placeholders;
join form standardised to <input name="location"> (matches forms.js);
CDN captcha (<frc-captcha> / jsdelivr) auto-removed by template
- Rewrite scripts/build.js to render template × lang matrix → dist/
17 pages, strip-script-before-hash invariant preserved, G4 guardrail
enforced (no CDN refs in dist/), type=module on every verify.js load
- Delete 21 old hand-written HTML files (packages/site/ root + hi/ ta/ te/ bn/)
- CI already asserts type=module on every verify.js script tag (publish.yml)
…rian#16) EN pages had base='', making 'import from "js/forms.js"' a bare specifier the browser refuses. ES module specifiers require a '/', './', or '../' prefix. Changing EN base to './' fixes the import; all hrefs/srcs are unaffected since './mirror.html' and 'mirror.html' are equivalent for browser navigation.
…russdorian#15) Add a CI step that fails the build if any https:// URL pointing at a public CDN (jsdelivr, esm.sh, unpkg, cdnjs) appears in dist/. Matches runtime URL strings only, not // comment mentions of domain names, so the grep is precise and won't false-positive on documentation text. Also remove two stale TODO comments in forms.js and relays.js that referenced CDN imports which were already replaced with local bundles (nostr-tools.bundle.js, age-encryption.bundle.js) in earlier work. Closes grussdorian#15 (CDN captcha auto-removed by PR-5 templates; G4 enforced here).
…ages Add tests/e2e/site.spec.js covering: - All 17 pages in the render matrix (en:5 hi:3 ta:3 te:3 bn:3) load with HTTP 200, correct html[lang], verify badge present, verify.js as type=module, no uncaught JS errors on initial load - Join form: name + location fields present; old state select and frc-captcha absent - Demand form: name + city + country fields present - EN-only 404s: hi/ta/te/bn mirror.html and trust.html return 404 - Mirror lang switcher: non-EN entries fall back to /lang/index.html, not mirror - Lang switcher .cur marking and EN back-link on translated pages - No CDN script src attributes on any page (belt-and-suspenders over the grep gate) Also wire an e2e CI job in publish.yml (runs after build, Node 20, Chromium, uploads playwright-report/ artifact on failure for trace inspection). Fixes found during test run: EN base was '' causing bare module specifier 'js/forms.js' — browser refuses imports without './', '/', or '../' prefix. Fixed in PR-5 (build.js base = './' for EN); all 54 tests now pass locally.
…ssdorian#17) Three paths in verify.js showed green 'verified' without a positive content hash match, silently hiding integrity failures: 1. IPFS integrity.json unreachable → now shows amber 'signed' + schedules one retry after 20s so slow CID propagation can still upgrade to green. 2. Page absent from integrity manifest → amber 'signed'. 3. Page re-fetch blocked (strict CSP / offline) → amber 'signed'. Green 'verified' is now reserved exclusively for actualHash === expectedHash. Also: - Add .cjp-badge--signed CSS rule (orange-amber, distinct from --outdated yellow). - Fix href="trust.html" → href="/trust.html" (root-relative) in sigLabel and the unknown-state link so translated pages (/hi/index.html etc.) don't resolve to a non-existent /hi/trust.html. - Extract fetchIntegrity() and verifyContentHash() as named inner functions for the retry path; no change to the signature verification logic (G3 preserved).
7 tasks
Aina-India
changed the title
Aina-India
changed the title
Aina-India
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three paths in
verify.jsreturned greenverifiedwithout a confirmed content hash match — silently hiding integrity failures:integrity.jsonunreachableactualHash === expectedHashGreen
verifiedis now exclusively a positive content hash match. Any other outcome that has passed signature verification but not content verification shows the new ambersignedstate.Additional fixes:
.cjp-badge--signedCSS (orange-amber#e8a020, distinct from--outdatedyellow#f0c040)href="trust.html"→href="/trust.html"(root-relative) insigLabeland the unknown-state link — fixes broken link from translated pages like/hi/index.htmlwhich would have resolved to non-existent/hi/trust.htmlfetchIntegrity()andverifyContentHash()as named inner functions to support the IPFS propagation retry path cleanlyTest plan
npx playwright test)verify-logic.test.mjs— 7 tests pass (signature logic unchanged)dweb.link,w3s.link,ipfs.io): badge shows orange ⊘ amber, not green ✓?with/trust.htmllink (root-relative, resolves correctly from/hi/index.html)Closes #17. Depends on #30 (PR-5), #31 (PR-6), #32 (PR-E2E).