Repository navigation
feat(web): support separate decryption keys - #139
Conversation
There was a problem hiding this comment.
Thanks for the thorough work here — the receiver-side flow (missing key → manual entry prompt) is exactly the right shape, and keeping the raw key out of React Query cache identities is a good catch. However, I'd like to take a different overall approach, which should significantly shrink this PR:
1. No configuration — always show both options
I don't want this to be deployment/config-driven. Instead, the create-success view should always show three fields:
- The combined URL (key in fragment) — current behavior
- The key-free URL
- The decryption key on its own
The sender chooses how to share at share time, rather than the deployer choosing at build time. A URL without a key already implicitly tells the recipient to enter it separately — the key prompt you built renders whenever the fragment is missing, so no query parameter and no flag are needed. This removes VITE_SEPARATE_DECRYPTION_KEY and with it the Dockerfile.web, docker-compose.yaml, README, config.ts, and config.test.ts changes.
2. Hard-break ?key= support instead of the compatibility layer
Rather than carrying the deprecated query-param format forward, let's remove support for it entirely (planned as its own release/changeset for changelog clarity — happy to coordinate sequencing). Rationale: secrets are TTL-bound so there's no long tail of old links, and a ?key= link has already leaked the key to server logs by the time the client sees it — reading it client-side only masks that. With the new key prompt, an old ?key= link degrades gracefully: the recipient lands on the prompt and can still enter the key manually.
That means legacyKeyBootstrap.ts and the legacy branches in resolveDecryptionKey can go. The one piece worth keeping is a small scrub: if a key query param is present, history.replaceState it away and show a warning toast that the link format is unsafe — without ever reading the value into app state. That's a few lines in main.tsx, not a module.
3. Please trim the unrelated changes
A few things in here are separate concerns that make the diff hard to review — details inline, but broadly: the clipboardCopy rework, the router remount machinery, and the generic createFailed error handling. Happy to see any of those as their own PRs with their own rationale. (Rewording existing locale strings is fine where this feature changes their meaning — e.g. success.description.main must change once auto-copy is removed — just avoid general copy edits beyond that.)
One change I do want to keep from your approach: dropping the automatic copy-to-clipboard on create. Letting the user explicitly choose what to copy is safer and fits the three-field design.
Core of what survives: buildSecretLinks (simplified to always return both URLs), the DecryptionKeyPrompt component and missing-key flow in View.tsx, the new locale keys, and keeping the key out of mutation cache identity. That's a much smaller, focused PR.
|
Thanks — I rebuilt the follow-up around the requested shape. The create-success view now always shows the combined URL, key-free URL, and decryption key; QR remains combined; and automatic clipboard copying is gone. Legacy I also removed the deployment/config/Docker/README work, legacy bootstrap, router-private remount machinery, clipboard-helper changes, and unrelated generic error change. The password input remains controlled with the accessibility additions, locale edits are limited to affected behavior, missing/wrong-key recovery remains available, and raw keys stay out of React Query cache identities and create mutation results. Local validation passed for the frozen install; all 10 web tests; web/core build, typecheck, and lint; full workspace build and typecheck; targeted Prettier; Changesets; diff checks; and browser QA covering query scrubbing/history, warning visibility, manual and wrong-key flows, password retry, the three share fields, combined QR, no automatic clipboard access, and cross-vault SPA navigation. The exact diff also passed an independent sealed Grok 4.5 review with no material findings. Known baseline limitations are recorded in the updated PR body: workspace-wide lint/format remain red on unchanged CRLF-formatted files, core Jest discovers no tests from this hidden Windows worktree, Vite emits its existing chunk-size warning, and remote CI results were not part of the frozen local validation evidence. I also updated the AI-assistance disclosure in the PR body. |
Maintainer follow-upI applied a few polish changes directly on this branch: Create success
Read flow
Legacy
|
Align create/read flows for separate decryption keys, migrate legacy ?key= into the fragment with an upgrade warning, consolidate the changeset, and drop brittle View tests.
d9bf370 to
c22072d
Compare
Summary
?key=links by scrubbing every key parameter before router initialization, warning the recipient, and falling back to manual key entry.Fixes #54
Security
keyexists, deletes every occurrence, and replaces the current history entry before importing the module that creates the router. It never retrieves or stores the query value.gcTime: 0.Validation
no-explicit-anywarning in unchangedpackages/core/src/client.ts)git diff --checkKnown limitations
AI assistance
AI assistance: OpenAI Codex implemented and reviewed the maintainer-requested follow-up. Grok 4.5 provided sealed security/design analysis and an independent packet-only review of the exact final diff. I reviewed the final patch and validation results and take responsibility for the submission.