Repository navigation
feat(presentation-mode): mask API keys and tokens for screensharing - #533
Closed
nicdavidson wants to merge 2 commits into
Closed
nicdavidson wants to merge 2 commits into
nicdavidson wants to merge 2 commits into
Conversation
Screensharing the admin UI puts live API keys on someone else's monitor. One toggle in the top bar now covers every credential the UI renders, with a per-field eye to reveal one on demand. Display and clipboard are deliberately separate paths: df-try-it renders displaySnippet and copies snippet, df-mcp-connect renders maskIfPresenting(x) and copies x. A masked curl command still pastes into Postman and works. The value leaves the DOM rather than being blurred — a CSS blur stays selectable and recoverable from a screenshot. Service-config secrets key off the field name, not the schema type: oidc.client_secret ships as `text` and mcp.oauth_client_secret as `string`, so both render as plain visible inputs. `_id` is excluded so client_id stays readable during OAuth setup. This is not a security boundary — the key still arrives in the API response and is visible in devtools. It addresses screenshare exposure. Covered by 34 unit tests (registered in jest.config.ci.js) and e2e/presentation-mode.spec.ts, which asserts the clipboard still carries the exact unmasked command and sweeps the leaking routes, failing loudly if a swept route has no credential left to hide.
Contributor
Author
|
Superseded by #537, which carries these presentation-mode commits (plus a fix for unmasked keys wrapping in the API Keys table) rebased onto current main. |
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
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.
Why
Screensharing the admin UI puts live API keys on someone else's monitor. On a local instance the API Keys page renders 10 full keys as plain text, and
/api-security/api-keysdoes the same.One toggle in the top bar (next to the theme toggles) now masks every credential the UI renders, with a per-field eye to reveal one on demand.
••••••••••••localStorage, so a demo doesn't start exposedThe constraint that shaped the design
Masked curl commands still have to paste into Postman and work.
Display and clipboard are therefore separate paths everywhere:
df-try-itrendersdisplaySnippet, copiessnippetdf-mcp-connectrendersmaskIfPresenting(x), copiesxMasking never touches the model. The curl command itself stays fully legible on screen — URL, method, headers — only the credential is covered.
The value also leaves the DOM rather than being blurred. A CSS blur stays selectable and recoverable from a screenshot; a test asserts the raw key is absent from
innerHTML.Service config
Secrets key off the field name, not the schema type, because the schema can't be trusted:
oidc.client_secretships as typetextandmcp.oauth_client_secretasstring, so both render as plain visible inputs today._idis excluded soclient_idstays readable during OAuth setup.Surfaces covered
API-key table, app details, MCP access key, MCP connect snippets (×5), artifact-card auth header, celebration dialog, try-it curl/Python/JS/MCP snippets, and
df-dynamic-fieldfor service-config credentials.Swagger's raw-spec panel is untouched and doesn't need to be:
dfApiDocsApiKeyis''in both environment files, so its curl carries no real key.Tests
npm run test:ci— 203 passed, up from 169. The 34 new specs are registered injest.config.ci.js, so they actually gate.e2e/presentation-mode.spec.ts— 9 passed against a live instance. The one that matters:It captures the real snippet, enables masking, asserts the credential is gone from screen, clicks Copy, and asserts
clipboard === the original unmasked command. Postman-paste is pinned by exact equality, not by inspection.Also covered: keys absent from
innerHTML; the eye reveals without navigating away (table rows are clickable — the test caught the missingstopPropagation); setting survives reload; off restores the keys; and a data-driven sweep of the leaking routes that fails loudly if a swept route has no credential left to hide, so it can't rot into a no-op.Regression check: full e2e is 32 passed / 5 failed. The 5
nav-overviewsfailures are pre-existing — verified by stashing this work and re-running on a clean tree for the identical result. They are an environment difference in the local instance, unrelated to this change.Not a security boundary
The key still arrives in the API response and is visible in devtools. This addresses screenshare exposure, not exfiltration.