feat(app): mask secrets in API key and MCP install snippets with a shared RevealSnippet - #2988
feat(app): mask secrets in API key and MCP install snippets with a shared RevealSnippet#2988brandon-pereira wants to merge 3 commits into
Conversation
… snippets Introduce a shared RevealSnippet compound component that masks secrets in code snippets until revealed, while copy always yields the real value. Adopt it across the Team Settings API-key fields and the MCP install snippets (CLI command, deep-link JSON fallback, plain JSON). Masking is driven entirely by the secrets prop: omitted/empty renders a plain snippet, a bare string masks the whole value via defaultRedact, and [real, redacted] pairs mask specific substrings.
🦋 Changeset detectedLatest commit: 4ebe870 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
Greptile SummaryThe PR introduces a shared reveal-aware credential snippet and adopts it for Team Settings API keys and MCP installation instructions.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/app/src/components/RevealSnippet/RevealSnippet.tsx | Implements the compound reveal, redaction, input, code, and copy controls, including resetting reveal state when the value changes. |
| packages/app/src/components/TeamSettings/ApiKeysSection.tsx | Replaces plaintext ingestion and personal-access-key rendering with masked read-only fields. |
| packages/app/src/components/ClickStackOnboarding/McpInstallPanel.tsx | Passes the deployment access key into every human-readable MCP installation snippet for masking. |
| packages/app/src/components/ClickStackOnboarding/installSnippets.ts | Documents the intentional presence of encoded credentials in installation deep links. |
| packages/app/src/components/RevealSnippet/tests/RevealSnippet.test.tsx | Covers masking, revealing, copying, nullable inputs, disabled reveal controls, and re-masking after credential changes. |
Reviews (2): Last reviewed commit: "test(app): cover copy-without-reveal + M..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Deep Review✅ No critical issues found. The previously flagged P1 (reveal state surviving key rotation) is resolved in this branch — 🟡 P2 — recommended
🔵 P3 nitpicks (1)
Reviewers (7): correctness, testing, maintainability, project-standards, kieran-typescript, security, previous-comments. Testing gaps:
Notes: The Cursor/VS Code deep links carry the base64/URL-encoded key in the DOM |
E2E Test Results✅ All tests passed • 306 passed • 1 skipped • 1229s
Tests ran across 4 shards in parallel. |
- Reset reveal state when the value changes (e.g. API key rotation) so a freshly issued credential is never shown without a fresh reveal (P1). - De-export defaultRedact, SecretInput, RevealRenderProps (knip: unused exports); they remain internal. - Drop an `as HTMLInputElement` cast in the ApiKeysSection test in favor of the exact masked-string assertion (which already covers length).
…daction - Add tests asserting Copy and Input place the REAL value on the clipboard while the snippet is still masked (the core copy-without-reveal promise). - Add McpInstallPanel consumer tests: the access key is masked in both the command and JSON-fallback snippets, and revealed only on an explicit action. - Clamp defaultRedact's visible prefix to half the value so short inputs are still masked instead of shown in full. - Redact longest secrets first so overlapping real values are order-independent. - Re-export defaultRedact and use it in the ApiKeysSection test instead of a local re-implementation. - Document the intended residual: MCP deep links carry the (encoded) key in their href by necessity; only the JSON fallback is reveal-gated.
| // Shared muted-at-rest treatment for the Reveal and Copy buttons. Uses the | ||
| // themed text color (not Mantine "dimmed") so it stays legible in light/dark | ||
| // and across brands; both buttons carry `c="inherit"` to pick up this color. | ||
| .reveal { |
There was a problem hiding this comment.
would setting variant="link" not also give us this?
| } | ||
|
|
||
| return ( | ||
| <Button |
There was a problem hiding this comment.
OOC what would happen if someone triple clicked the text? I assume that wouldn't copy with the plaintext, but instead have the ***** ?
2 participants
What & why
Credentials were rendered in plain text across the app — the Team Settings API-key cards showed the full key, and the MCP install snippets inlined the personal access key directly into the copyable command/JSON. That's a shoulder-surfing risk on a screen-share or in a screenshot.
This adds a shared
RevealSnippetcomponent that masks secrets in a snippet until the user explicitly reveals them, while copy always hands back the real value (so nobody has to reveal a key just to copy it). It's adopted across every credential surface touched here:Design
RevealSnippetis a compound component —RevealSnippet.Code,.Reveal,.Copy, and a self-contained.Inputshare reveal state via context, so the surrounding layout (header row, panel chrome, button placement) is entirely up to the caller.Masking is driven by a single
secretsprop, with a deliberately safe default:secrets[][value](bare string)defaultRedact(prefix + dots) — for a bare API key[[real, redacted], …]Bare-string and pair entries can be mixed, and
nullentries are skipped sosecrets={[maybeKey]}is safe (e.g. a ClickHouse Cloud deployment with no key). The masked stand-in is padded to the real length so the field width doesn't jump on reveal.Screenshots
Testing
RevealSnippet(masking, reveal toggle, bare-string vs. pair vs. null secrets,canReveal=false, render-prop override) and updatedApiKeysSectiontests (masked value on the input, inline reveal).tsc --noEmit, ESLint, and unit tests all pass.