feat(decrypt): add trust-confirmation gate before files reach disk#258
Conversation
The download page used to auto-trigger browser downloads the moment decryption finished. Recipients now see a Confirm panel — same Done layout (banner + file list + pill chips of verified attributes) plus two neutral buttons — and decide whether to keep the files. Decline discards the in-memory blob; accept triggers the download as before. When the sender disclosed nothing beyond their email, a strong warning band appears above the buttons. Email alone is a weak claim (anyone with control of the mailbox could have signed), so the gate is the right moment to surface that risk. UX polish that came out of the same round: - Widen the column to 350px and let the Yivi QR fill it instead of capping at 330px — the column now visibly wraps the QR. - New `--pg-success` brand token (light/dark) used only for the accept button's hover/focus tint; decline tints to the existing `--pg-input-error`. - /debug/download-flow: ALL_STATES gains `Confirm` + `Discarded`, scenarios route through Confirm and pause there for the developer to click one of the trust buttons, force-state highlight tracks the live `downloadState` in real time, the page locks to one viewport height, and the global footer is hidden on `/debug/*`. Test: - New `isWeakSenderIdentity` helper alongside `verifiedAttributesFor`, unit-tested for null, email-only, with-private-attrs, and empty-value-attribute cases.
|
Dobby has received the request! Routing to the right specialist now... |
|
/dobby review |
|
Dobby is on it! Routing to the review specialist now. 🧦 |
There was a problem hiding this comment.
Reviewed PR #258 (trust-confirmation gate) — requesting changes. Tests pass (npm run check + 12 unit tests) and the per-rule compliance sweep is clean; three correctness issues in the download page remain. Two are inline below; the most important one can't be attached inline because it lives in an unchanged region of the file:
[bug] Orphaned i18n key + dead CSS in the Ready (QR-scan) sender-section — src/routes/(app)/download/+page.svelte line 332.
This PR deletes the verifiedExtra key from en.json/nl.json and removes the .attr-list / .sender-label-extra CSS, migrating the Confirm and Done sender-sections to the new attr-chips markup. But the identical sender-section in the Ready state (lines 331–339) was not migrated — it still calls $_(…/verifiedExtra) and renders <dl class="attr-list">. When info.sender carries extra disclosed attributes during the QR scan, svelte-i18n now renders the literal string filesharing.decryptpanel.verifiedExtra and the <dl> is unstyled. Migrate this block to the chips markup like the other two. Note: npm run check passes (i18n keys aren't type-checked), so CI won't catch this.
The two inline findings: the Confirm gate shows a "files have been downloaded" message before files reach disk, and isWeakSenderIdentity(null) surfaces email-only warning copy for a truly unsigned file.
| /> | ||
| </svg> | ||
| <p role="status"> | ||
| {$_('filesharing.decryptpanel.doneMessageComplete')} |
There was a problem hiding this comment.
[bug] The Confirm trust-gate shows the green checkmark banner + doneMessageComplete ("Your files have been downloaded and decrypted") while the whole point of the gate is that the files have NOT reached disk yet — the user must still click "Download files". This contradicts the feature description ("before files reach disk") and the CTA below it. Use a decryption-complete / ready-to-download message here, not doneMessageComplete.
| {#if downloadState === 'Done'} | ||
| <div in:fade={{ duration: 300, delay: 200 }}> | ||
| <FileList files={fileList} /> | ||
| {#if isWeakSenderIdentity(senderIdentity)} |
There was a problem hiding this comment.
[nit] isWeakSenderIdentity(null) returns true, so a file with no sender at all renders the trustWarnEmailOnly copy ("The sender only verified an email address…") even though no email/sender exists and the sender-section above is hidden. Edge case (PostGuard files are normally signed) but the copy is inaccurate for a truly unsigned file.
- Confirm gate showed doneMessageComplete ("files have been downloaded
and decrypted") before files reach disk. Add a readyToDownload message
and use it there; the banner no longer contradicts the gate and CTA.
- Migrate the Ready (QR-scan) sender-section to the attr-chips markup. It
still referenced the removed verifiedExtra i18n key and attr-list CSS,
so disclosed attributes rendered as a literal key in an unstyled <dl>.
- Scope isWeakSenderIdentity to senders that actually verified an email.
A missing/unsigned sender no longer triggers the email-only warning,
whose copy does not apply when there is no email to caveat.
An unsigned file (no verifiable sender at all) is the weakest case of all, yet it previously showed no caution. Add a dedicated, stronger warning for it and force the recipient to read before they can accept: - isUnsignedSender() helper (sender has no verified email) + unit tests. - Confirm gate now branches: unsigned => louder trustWarnUnsigned band (thicker border, more saturated fill); email-only keeps the existing trustWarnEmailOnly band; verified senders show none. - For the unsigned case only, the download button starts disabled and fills left-to-right over 5s (TRUST_UNLOCK_MS) before activating, so the user cannot click through without pausing on the warning. Decline stays enabled throughout. Other cases remain instantly clickable. - New i18n key trustWarnUnsigned (en + nl). - debug/download-flow: replace the email-only toggle with a three-way sender-identity selector (strong / email-only / unsigned) and mirror the warning branch + time-locked button so the preview stays faithful; also sync its Confirm banner to readyToDownload and hide sender sections when there is no email.
The 5s greyed/progress-bar download button read as broken UI. Replace it with an explicit confirmation step, and refine the warnings per review: - New shared UnsignedConfirmModal: on an unsigned file, "Download files" opens a modal (red warning, Cancel / Download anyway, Esc + click- outside to close, Cancel focused on open) instead of a single click. Signed files still download on one click. Used by both the download page and the debug preview. - Remove the time-lock state/effect/timer and the locked-button CSS. - Email-only warning is now orange (new --pg-warning token); unsigned stays red but is no longer bold, so severity reads through colour. - Strip em-dashes from both warning messages (en + nl). - New i18n keys: trustConfirmHeader / trustConfirmAccept / trustConfirmCancel.
464aa8f
into
feat/decrypt-show-private-attributes
* docs: document pg-js 2.0 DecryptFileResult + onDownloadProgress + cause-preserving IdentityMismatchError Sources: - encryption4all/postguard-js#86 (DecryptFileResult shape change, onDownloadProgress) - encryption4all/postguard-js#84 (IdentityMismatchError cause preservation) * docs: clarify createEnvelope tier 2 vs tier 3 upload-failure semantics Source: encryption4all/postguard-js#82 (re-throw on tier 3, console.warn on tier 2) * docs: add Runtime config section for postguard-website APP_CONFIG keys Sources: - encryption4all/postguard-website#244 (STAGING) - encryption4all/postguard-website#247 (GLITCHTIP_DSN) - encryption4all/postguard-website#255 (SITE_URL) * docs: surface private signing attributes on FriendlySender (from encryption4all/postguard-js#89) * docs: document /download trust-confirmation gate (from encryption4all/postguard-website#258) --------- Co-authored-by: dobby-yivi-agent[bot] <275734547+dobby-yivi-agent[bot]@users.noreply.github.com>
…257) * feat(decrypt): show private signing attributes on the download page Recipients can now see (and crypto verifies) the sender's name, mobile number, and date of birth alongside the email — the attributes the sender disclosed under the *private* signing identity at sign time. Previously the download page only ever showed the public email even when the sender disclosed more. Three things were needed: 1. Work around privacybydesign/irmamobile#360. The irma app drops optional attributes when the empty `[]` alternative precedes the real one inside a discon. Reorder `SIGN_ATTRIBUTES` so the empty alternative is last in each disjunction, and expand the per- attribute `optional: true` shorthand by hand into `[[attr], []]` for the same reason. Bumping pg-js or any other client wouldn't help — the bug is on the wallet side. Revert once the irmamobile fix has shipped to the app stores. 2. Pick up the corresponding pg-js fix that captures the verified private signing identity returned by `unsealer.unseal(...)`. (Already landed upstream — bump pinned through the lockfile once the new pg-js version is released.) 3. Render the disclosed non-email attributes on the download page with their existing i18n labels (`filesharing.attributes.<type>`) under the already-translated `verifiedExtra` heading, in both the pre-decrypt Ready panel and the post-decrypt Done panel. The filtering/labeling logic lives in a tiny pure helper (`verifiedAttributesFor`) that is unit-tested with vitest. Also adds vitest as a devDep with a minimal config scoped to `src/**` so the project finally has a unit-test framework alongside the existing Playwright e2e setup. New `npm run test:unit` script. * feat(decrypt): add trust-confirmation gate before files reach disk (#258) * feat(decrypt): add trust-confirmation gate before files reach disk The download page used to auto-trigger browser downloads the moment decryption finished. Recipients now see a Confirm panel — same Done layout (banner + file list + pill chips of verified attributes) plus two neutral buttons — and decide whether to keep the files. Decline discards the in-memory blob; accept triggers the download as before. When the sender disclosed nothing beyond their email, a strong warning band appears above the buttons. Email alone is a weak claim (anyone with control of the mailbox could have signed), so the gate is the right moment to surface that risk. UX polish that came out of the same round: - Widen the column to 350px and let the Yivi QR fill it instead of capping at 330px — the column now visibly wraps the QR. - New `--pg-success` brand token (light/dark) used only for the accept button's hover/focus tint; decline tints to the existing `--pg-input-error`. - /debug/download-flow: ALL_STATES gains `Confirm` + `Discarded`, scenarios route through Confirm and pause there for the developer to click one of the trust buttons, force-state highlight tracks the live `downloadState` in real time, the page locks to one viewport height, and the global footer is hidden on `/debug/*`. Test: - New `isWeakSenderIdentity` helper alongside `verifiedAttributesFor`, unit-tested for null, email-only, with-private-attrs, and empty-value-attribute cases. * fix(decrypt): address review on trust-confirmation gate - Confirm gate showed doneMessageComplete ("files have been downloaded and decrypted") before files reach disk. Add a readyToDownload message and use it there; the banner no longer contradicts the gate and CTA. - Migrate the Ready (QR-scan) sender-section to the attr-chips markup. It still referenced the removed verifiedExtra i18n key and attr-list CSS, so disclosed attributes rendered as a literal key in an unstyled <dl>. - Scope isWeakSenderIdentity to senders that actually verified an email. A missing/unsigned sender no longer triggers the email-only warning, whose copy does not apply when there is no email to caveat. * feat(decrypt): warn on unsigned files and time-lock their download An unsigned file (no verifiable sender at all) is the weakest case of all, yet it previously showed no caution. Add a dedicated, stronger warning for it and force the recipient to read before they can accept: - isUnsignedSender() helper (sender has no verified email) + unit tests. - Confirm gate now branches: unsigned => louder trustWarnUnsigned band (thicker border, more saturated fill); email-only keeps the existing trustWarnEmailOnly band; verified senders show none. - For the unsigned case only, the download button starts disabled and fills left-to-right over 5s (TRUST_UNLOCK_MS) before activating, so the user cannot click through without pausing on the warning. Decline stays enabled throughout. Other cases remain instantly clickable. - New i18n key trustWarnUnsigned (en + nl). - debug/download-flow: replace the email-only toggle with a three-way sender-identity selector (strong / email-only / unsigned) and mirror the warning branch + time-locked button so the preview stays faithful; also sync its Confirm banner to readyToDownload and hide sender sections when there is no email. * refactor(decrypt): replace unsigned time-lock with a confirmation modal The 5s greyed/progress-bar download button read as broken UI. Replace it with an explicit confirmation step, and refine the warnings per review: - New shared UnsignedConfirmModal: on an unsigned file, "Download files" opens a modal (red warning, Cancel / Download anyway, Esc + click- outside to close, Cancel focused on open) instead of a single click. Signed files still download on one click. Used by both the download page and the debug preview. - Remove the time-lock state/effect/timer and the locked-button CSS. - Email-only warning is now orange (new --pg-warning token); unsigned stays red but is no longer bold, so severity reads through colour. - Strip em-dashes from both warning messages (en + nl). - New i18n keys: trustConfirmHeader / trustConfirmAccept / trustConfirmCancel. * fix(ci): make test:unit self-contained on a clean checkout test:unit was `vitest run` with no `svelte-kit sync` in front, so on a clean checkout it hard-failed with `[TSCONFIG_ERROR] Tsconfig not found` (the root tsconfig extends the not-yet-generated .svelte-kit/tsconfig.json) and ran 0 tests. Prefix `svelte-kit sync` so it works from a bare `npm ci` and correct the vitest.config.ts comment that claimed otherwise. Addresses review feedback on PR #257. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: run unit tests (test:unit) as a PR gate Adds a test-unit job that runs npm run test:unit on every PR. The script is self-contained (svelte-kit sync && vitest run), so a bare npm ci is enough — no separate sync step needed. * docs(decrypt): correct isUnsignedSender docstring (no time-lock) The docstring claimed the unsigned-sender gate "time-locks the download button", but there is no time delay anywhere in the code. The gate shows the strongest trust warning and routes the download through an extra confirmation modal (UnsignedConfirmModal). Reword to match behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: dobby-yivi-agent[bot] <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Inserts a Confirm state between Decrypting and Done on the download page. Decryption finishes silently into in-memory blobs; the recipient sees the file list and the verified sender, then decides whether to actually keep the files. Decline drops the blob and shows a Discarded message; accept triggers the browser downloads as before.
When the sender disclosed nothing beyond email, the Confirm panel shows a strong warning band above the buttons — email alone is a weak identity claim (anyone with mailbox control could have signed), so the gate is the right moment to surface that.
The two buttons are visually neutral by default; they tint to a new
--pg-successon the accept side and the existing--pg-input-erroron the decline side only on hover or focus, to avoid nudging the choice.UX polish that came out of the same round (all in this PR):
/debug/download-flow:ConfirmandDiscardedadded to the state grid; scenarios now pause on Confirm and let the developer click one of the two buttons to continue; force-state highlight tracks the livedownloadStatein real time (including during scenarios); the page locks to one viewport height so the outer scroll bar goes away; the global footer is hidden on/debug/*via a tiny conditional in(app)/+layout.svelte.Builds on #257 — base is that branch, not main. Once #257 merges, GitHub will auto-retarget this to main.
Test plan
npm run checkandnpm run test:unitare green locally (12 unit tests)./download?…with a file signed for email + fullname + mobile + dateofbirth: Yivi → Decrypting → Confirm. Confirm panel matches Done's body (success banner, file list, pill chips of the three private attributes) plus two neutral buttons. No warning band. Decline ⇒ Discarded; Accept ⇒ Done and browser fans out the downloads./download?…with a file signed for email only: Confirm panel shows the red-bordered warning above the buttons; the rest of the page identical./debug/download-flow: no outer scroll, no footer; force-state and scenario playback both correctly highlight whichever state is on screen; scenarios stop at Confirm; toggling "Weak identity (email only)" shows/hides the warning band in the preview.