Skip to content

fix(deep-links): report plugin integrity failures as tampering, not a bad link - #152

Merged
kipavy merged 3 commits into
devfrom
fix/deeplink-confirm-followups
Aug 19, 2026
Merged

fix(deep-links): report plugin integrity failures as tampering, not a bad link#152
kipavy merged 3 commits into
devfrom
fix/deeplink-confirm-followups

Conversation

@kipavy

@kipavy kipavy commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Follow-ups 1 and 3 from the #150 confirm-routes review. Part of #144.

An integrity mismatch read as "bad link"

A plugin-install confirm sheet surfaced every accept failure as the generic settings.plugins.deepLinkInstall.failed, so PluginHashMismatchError and MinAppVersionError were misreported. A hash mismatch is the user's only tamper signal, and it must not look like a link that simply did not work. The settings click path already distinguished both, so the mapping now exists once:

  • New src/plugins/installErrors.tspluginInstallErrorMessage(e, t, fallbackKey), matched on error type rather than message, per the project rule.
  • ConfirmSpec gains errorMessage?: (e, t, fallbackKey) => string; plugin-install sets it to pluginInstallErrorMessage directly.
  • DeepLinkConfirmModal's two setError sites both go through a module-scope failureMessage(spec, e, t) — module scope so the load effect gains no dependency.
  • PluginsSection.notifyError's inline three-way ternary collapses onto the shared helper.

The review proposed errorKeyFor?: (e) => string. A bare key cannot carry versionUnsupported's {version} interpolation, so the seam takes t and returns the message; passing the fallback key in means errorKey is still stated only once.

Two tests that could not fail

Both invite negative tests clicked an accept button that was already disabled, so their "invites nobody" assertions were vacuous. Explicit .disabled assertions added — and to the snippet and plugin negative tests, which had the identical hole.

New coverage: hash mismatch, min-version, and generic failure each produce their own message in the sheet.

Verification

  • node ./node_modules/typescript/bin/tsc --noEmit -p tsconfig.json — exit 0, no output.
  • Full CI=true pnpm vitest run1 failed | 3713 passed. The failure is tests/pluginBundleBuild.test.ts timing out at 5000ms under load; it passes 8/8 run alone. Known flake on pristine dev, unrelated to this diff.

Not covered: the sheets have still never been opened in a running app.


Added after review: follow-ups 2 and 4

A locale change re-ran the sheet's load

The sheet stored its error already translated, which forced t into the load effect's dependencies. t is a new function on every locale change, so changing language over an open sheet ran load again — a second searchUsers or manifest fetch — without resetting loading or loadFailed, leaving accept enabled over a stale result.

Errors are now held as a key plus parameters and translated at render, so t leaves the deps. pluginInstallErrorMessage returns that descriptor rather than a string, which is also what lets it carry versionUnsupported's version. The effect also resets its own state, since a redelivered link can change the intent's identity without remounting the sheet. A test pins the call count: it was 2, it is now 1.

A double-click could cross two sheets

dismissPrompt promoted the next queued intent in the same tick, and join has no load, so its accept button was live in the first frame at the same screen position as the button just clicked — a double-click on Install could land its second half on Join.

Queue promotion now waits 300ms, and only a sheet arriving behind another pays it: a link that arrives on its own still prompts immediately, and unprompted intents are untouched. The delay was chosen over arming every sheet's accept button late, so a single link stays instant.

Re-verified: tsc --noEmit exit 0 with no output; full CI=true pnpm vitest run = 1 failed | 3715 passed, the failure again being tests/pluginBundleBuild.test.ts at 5000ms, which passes 8/8 alone.

kipavy added 3 commits August 19, 2026 10:40
… bad link

A plugin-install confirm sheet surfaced every accept failure as the generic
"deep link install failed", so PluginHashMismatchError and MinAppVersionError
were misreported. A hash mismatch is the user's only tamper signal.

Extract pluginInstallErrorMessage(e, t, fallbackKey), matched on error type
rather than message, and use it from both the settings install path and the
confirm sheet via a new optional errorMessage on ConfirmSpec.

Also add explicit .disabled assertions to the confirm-sheet negative tests:
they clicked an already-disabled accept button, so the "installs/invites
nobody" assertions could not fail.
The sheet stored its error already translated, which forced `t` into the load
effect's dependencies. `t` is a new function on every locale change, so changing
language over an open sheet ran `load` a second time — another searchUsers or
manifest fetch — without resetting `loading` or `loadFailed`, leaving accept
enabled over a stale result.

Errors are now held as a key plus its parameters and translated at render, so
`t` leaves the deps. pluginInstallErrorMessage returns that descriptor rather
than a string, which is also what lets it carry versionUnsupported's `version`.
The effect additionally resets its own state, since a redelivered link can
change the intent's identity without remounting the sheet.
… dismissed one

dismissPrompt promoted the next queued intent in the same tick, and `join` has
no load step, so its accept button was live in the first frame at the same
screen position as the button just clicked. A double-click on Install could
land its second half on Join.

Queue promotion now waits 300ms. Only a sheet arriving behind another pays it:
a link that arrives on its own still prompts immediately, and unprompted
intents are untouched.
@kipavy
kipavy merged commit 3c4f555 into dev Aug 19, 2026
4 checks passed
@kipavy
kipavy deleted the fix/deeplink-confirm-followups branch August 19, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant