diff --git a/packages/bruno-api-docs/e2e/components/variable-card/variable-card.component.ts b/packages/bruno-api-docs/e2e/components/variable-card/variable-card.component.ts index 1f1ea38f..debc4ce5 100644 --- a/packages/bruno-api-docs/e2e/components/variable-card/variable-card.component.ts +++ b/packages/bruno-api-docs/e2e/components/variable-card/variable-card.component.ts @@ -7,6 +7,9 @@ export class VariableCardComponent extends BaseComponent { readonly scopeBadge = this.card.getByTestId('variable-info-card-scope'); readonly value = this.card.getByTestId('variable-info-card-value'); readonly copyButton = this.card.getByTestId('variable-info-card-copy'); + // Present only while the copy button is showing its confirmation, which clears + // itself after a second, so a test should assert it in one step. + readonly copiedTick = this.card.getByTestId('variable-info-card-copy-tick'); readonly revealToggle = this.card.getByTestId('variable-info-card-reveal'); readonly note = this.card.getByTestId('variable-info-card-note'); readonly warning = this.card.getByTestId('variable-info-card-warning'); diff --git a/packages/bruno-api-docs/e2e/tests/playground/playground-variables.spec.ts b/packages/bruno-api-docs/e2e/tests/playground/playground-variables.spec.ts index f8cc71ed..9f8a1536 100644 --- a/packages/bruno-api-docs/e2e/tests/playground/playground-variables.spec.ts +++ b/packages/bruno-api-docs/e2e/tests/playground/playground-variables.spec.ts @@ -84,4 +84,208 @@ test.describe('Playground variables: highlight, hover card and inline edit', () await playground.variable.value.click(); await expect(playground.variable.editField).toHaveCount(0); }); + + test('clicking the value puts the caret at the end, not the start', async ({ playground }) => { + await playground.variable.hoverInputToken('host'); + await playground.variable.value.click(); + + const caret = await playground.variable.editField.evaluate( + (el: HTMLTextAreaElement) => ({ start: el.selectionStart, end: el.selectionEnd, length: el.value.length }) + ); + + expect(caret.length).toBeGreaterThan(0); + expect(caret.start).toBe(caret.length); + expect(caret.end).toBe(caret.length); + }); + + test('the copy control stays available while the value is being edited', async ({ playground }) => { + await playground.variable.hoverInputToken('host'); + await expect(playground.variable.copyButton).toBeVisible(); + + await playground.variable.startEditing('https://edited.example.com'); + + await expect(playground.variable.editField).toBeVisible(); + await expect(playground.variable.copyButton).toBeVisible(); + }); + + test('an unset secret starts blank with a reveal control and takes a typed value', async ({ playground }) => { + await playground.variable.hoverInputToken('unsetSecret'); + + await expect(playground.variable.value).toHaveText(''); + await expect(playground.variable.revealToggle).toBeVisible(); + + await playground.variable.editTo('typed-secret'); + + await expect(playground.variable.value).toHaveText('*'.repeat('typed-secret'.length)); + + await playground.variable.revealToggle.click(); + await expect(playground.variable.value).toHaveText('typed-secret'); + }); + + test('an external secret is editable and keeps its Secret scope', async ({ playground }) => { + await playground.variable.hoverInputToken('vaultKey'); + + await expect(playground.variable.scopeBadge).toHaveText('Secret'); + await expect(playground.variable.value).toHaveText(''); + + await playground.variable.editTo('vault-value'); + + await expect(playground.variable.value).toHaveText('*'.repeat('vault-value'.length)); + }); + + // The field contains the asterisks themselves rather than the secret, so the + // secret is never present in the page while it is hidden. + test('the edit field holds only mask characters while the secret is hidden', async ({ playground }) => { + await playground.variable.hoverInputToken('unsetSecret'); + await playground.variable.startEditing('typed-secret'); + + const state = await playground.variable.editField.evaluate((el: HTMLTextAreaElement) => ({ + value: el.value, + caret: el.selectionStart + })); + + expect(state.value).toBe('*'.repeat('typed-secret'.length)); + expect(state.caret).toBe(state.value.length); + }); + + // Use key presses, not fill(): fill() replaces the whole value at once and so + // never exercises deleting a single character through the mask. + test('Backspace and Delete remove characters from a masked secret', async ({ playground }) => { + await playground.variable.hoverInputToken('unsetSecret'); + await playground.variable.startEditing('abcdef'); + + await playground.variable.editField.press('Backspace'); + await expect(playground.variable.editField).toHaveValue('*'.repeat(5)); + + await playground.variable.editField.evaluate((el: HTMLTextAreaElement) => el.setSelectionRange(0, 0)); + await playground.variable.editField.press('Delete'); + await expect(playground.variable.editField).toHaveValue('*'.repeat(4)); + + await playground.variable.editField.press('Enter'); + await playground.variable.hoverInputToken('unsetSecret'); + await playground.variable.revealToggle.click(); + + await expect(playground.variable.value).toHaveText('bcde'); + }); + + // The app confirms every copy, so a variable with nothing in it still ticks. + test('confirms a copy even when the secret has no value yet', async ({ playground, context }) => { + await context.grantPermissions(['clipboard-read', 'clipboard-write']); + + await playground.variable.hoverInputToken('unsetSecret'); + await expect(playground.variable.value).toHaveText(''); + await expect(playground.variable.copyButton).toBeVisible(); + await expect(playground.variable.copiedTick).toHaveCount(0); + + await playground.variable.copyButton.click(); + + await expect(playground.variable.copiedTick).toBeVisible(); + }); + + // The field holds asterisks, so a plain selection copy would put those on the + // clipboard instead of the secret. + test('copying a selection out of a masked secret yields the real value', async ({ page, playground, context }) => { + await context.grantPermissions(['clipboard-read', 'clipboard-write']); + + await playground.variable.hoverInputToken('unsetSecret'); + await playground.variable.startEditing('abcdef'); + + await playground.variable.editField.press('ControlOrMeta+a'); + await playground.variable.editField.press('ControlOrMeta+c'); + + await expect.poll(() => page.evaluate(() => navigator.clipboard.readText())).toBe('abcdef'); + }); + + test('cutting from a masked secret removes the real characters', async ({ page, playground, context }) => { + await context.grantPermissions(['clipboard-read', 'clipboard-write']); + + await playground.variable.hoverInputToken('unsetSecret'); + await playground.variable.startEditing('abcdef'); + + await playground.variable.editField.evaluate((el: HTMLTextAreaElement) => el.setSelectionRange(2, 4)); + await playground.variable.editField.press('ControlOrMeta+x'); + + await expect.poll(() => page.evaluate(() => navigator.clipboard.readText())).toBe('cd'); + await expect(playground.variable.editField).toHaveValue('*'.repeat(4)); + + await playground.variable.editField.press('Enter'); + await playground.variable.hoverInputToken('unsetSecret'); + await playground.variable.revealToggle.click(); + + await expect(playground.variable.value).toHaveText('abef'); + }); + + test('typing into the middle of a masked secret edits the real value', async ({ playground }) => { + await playground.variable.hoverInputToken('unsetSecret'); + await playground.variable.startEditing('abcdef'); + + await playground.variable.editField.evaluate((el: HTMLTextAreaElement) => el.setSelectionRange(3, 3)); + await playground.variable.editField.pressSequentially('XY'); + await playground.variable.editField.press('Enter'); + + await playground.variable.hoverInputToken('unsetSecret'); + await playground.variable.revealToggle.click(); + + await expect(playground.variable.value).toHaveText('abcXYdef'); + }); + + test('a typed secret stays masked in the card until revealed', async ({ playground }) => { + await playground.variable.hoverInputToken('unsetSecret'); + await playground.variable.editTo('typed-secret'); + + await expect(playground.variable.card).not.toContainText('typed-secret'); + + await playground.variable.revealToggle.click(); + + await expect(playground.variable.card).toContainText('typed-secret'); + + await playground.variable.revealToggle.click(); + + await expect(playground.variable.card).not.toContainText('typed-secret'); + await expect(playground.variable.value).toHaveText('*'.repeat('typed-secret'.length)); + }); + + // A secret nobody filled in has no value to send, so the request keeps the + // `{{name}}` reference instead of quietly sending an empty value. Environment + // secrets and external secrets behave the same way here. + test('leaves an unfilled secret unresolved in the request', async ({ page, responsePane }) => { + const sent: string[] = []; + await page.route('**/customers/**', (route) => { + sent.push(route.request().url()); + return route.fulfill({ status: 200, headers: { 'content-type': 'application/json' }, body: '{}' }); + }); + + await responsePane.send(); + + await expect.poll(() => sent.length).toBeGreaterThan(0); + expect(sent[0]).toContain('s={{unsetSecret}}'); + expect(sent[0]).toContain('k={{vaultKey}}'); + }); + + test('sends a typed secret with the request', async ({ page, playground, responsePane }) => { + const sent: string[] = []; + await page.route('**/customers/**', (route) => { + sent.push(route.request().url()); + return route.fulfill({ status: 200, headers: { 'content-type': 'application/json' }, body: '{}' }); + }); + + await playground.variable.hoverInputToken('unsetSecret'); + await playground.variable.editTo('typed-secret'); + + await expect(playground.variable.value).toHaveText('*'.repeat('typed-secret'.length)); + + // A card is already open from the edit above, so the hover helper's wait for + // a visible card returns immediately. Wait for the name to change before + // editing, or the edit lands on the previous variable. + await playground.variable.hoverInputToken('vaultKey'); + await expect(playground.variable.name).toHaveText('vaultKey'); + await playground.variable.editTo('vault-value'); + await expect(playground.variable.value).toHaveText('*'.repeat('vault-value'.length)); + + await responsePane.send(); + + await expect.poll(() => sent.length).toBeGreaterThan(0); + expect(sent[0]).toContain('s=typed-secret'); + expect(sent[0]).toContain('k=vault-value'); + }); }); diff --git a/packages/bruno-api-docs/e2e/tests/variableInfoPopup/variableInfoPopup.spec.ts b/packages/bruno-api-docs/e2e/tests/variableInfoPopup/variableInfoPopup.spec.ts index 6f660e5a..3c06e409 100644 --- a/packages/bruno-api-docs/e2e/tests/variableInfoPopup/variableInfoPopup.spec.ts +++ b/packages/bruno-api-docs/e2e/tests/variableInfoPopup/variableInfoPopup.spec.ts @@ -75,6 +75,16 @@ test.describe('Variable hover card', () => { await expect(variableCard.copyButton).toHaveCount(0); }); + // Only the playground can fill an external secret in, so the docs leave it unresolved. + test('leaves an external secret undefined', async ({ requestPage }) => { + const { variableCard } = requestPage; + await variableCard.hoverToken('vaultKey'); + + await expect(variableCard.card).toBeVisible(); + await expect(variableCard.scopeBadge).toHaveText('Undefined'); + await expect(variableCard.note).toHaveText('Variable is not defined'); + }); + test('shows an (empty) placeholder with no copy for a defined variable that has no value', async ({ requestPage }) => { const { variableCard } = requestPage; await variableCard.hoverToken('emptyValue'); @@ -133,6 +143,21 @@ test.describe('Variable hover card', () => { await expect.poll(() => page.evaluate(() => navigator.clipboard.readText())).toBe('2024-01'); }); + test('turns the copy glyph into a green tick while copied, then reverts', async ({ page, requestPage }) => { + const { variableCard } = requestPage; + await page.context().grantPermissions(['clipboard-read', 'clipboard-write']); + + await variableCard.pinToken('apiVersion'); + await expect(variableCard.card).toBeVisible(); + + await expect(variableCard.copiedTick).toHaveCount(0); + + await variableCard.copyButton.click(); + + await expect(variableCard.copiedTick).toBeVisible(); + await expect(variableCard.copiedTick).toHaveCount(0, { timeout: 3000 }); + }); + test('is read-only — no editor inside the card', async ({ requestPage }) => { const { variableCard } = requestPage; await variableCard.pinToken('host'); diff --git a/packages/bruno-api-docs/src/components/Playground/Content/Views/Common/LargeResponseWarning/LargeResponseWarning.tsx b/packages/bruno-api-docs/src/components/Playground/Content/Views/Common/LargeResponseWarning/LargeResponseWarning.tsx index ff20d1cb..98e21488 100644 --- a/packages/bruno-api-docs/src/components/Playground/Content/Views/Common/LargeResponseWarning/LargeResponseWarning.tsx +++ b/packages/bruno-api-docs/src/components/Playground/Content/Views/Common/LargeResponseWarning/LargeResponseWarning.tsx @@ -50,18 +50,6 @@ export const LargeResponseWarning: React.FC = ({ resp View - + ); diff --git a/packages/bruno-api-docs/src/components/Playground/Content/Views/Common/LargeResponseWarning/StyledWrapper.ts b/packages/bruno-api-docs/src/components/Playground/Content/Views/Common/LargeResponseWarning/StyledWrapper.ts index 36869169..8fe95177 100644 --- a/packages/bruno-api-docs/src/components/Playground/Content/Views/Common/LargeResponseWarning/StyledWrapper.ts +++ b/packages/bruno-api-docs/src/components/Playground/Content/Views/Common/LargeResponseWarning/StyledWrapper.ts @@ -11,7 +11,6 @@ export const StyledWrapper = styled.div` text-align: center; .warning-icon { - margin-bottom: 1rem; color: var(--oc-status-warning-text); } diff --git a/packages/bruno-api-docs/src/components/Playground/Content/Views/EnvironmentsView/EnvironmentsView.tsx b/packages/bruno-api-docs/src/components/Playground/Content/Views/EnvironmentsView/EnvironmentsView.tsx index 59d4c74e..b998b42a 100644 --- a/packages/bruno-api-docs/src/components/Playground/Content/Views/EnvironmentsView/EnvironmentsView.tsx +++ b/packages/bruno-api-docs/src/components/Playground/Content/Views/EnvironmentsView/EnvironmentsView.tsx @@ -14,8 +14,8 @@ import { variableTypeColumn } from '../Common/VariableTypeControl/variableTypeCo import { GlobeIcon } from '../../../../../assets/icons'; import { useAppDispatch } from '../../../../../store/hooks'; import { cx } from '../../../../../utils/cx'; -import { envVariableToRow, envRowToVariable } from '../../../../../utils/environments'; -import { isSecretVariable } from '../../../../../utils/variableResolution'; +import { envVariableToRow, envRowToVariable, mergeExternalSecretRows } from '../../../../../utils/environments'; +import { isSecretVariable, isExternalSecretActive, type ExternalSecretEntry } from '../../../../../utils/variableResolution'; import { updateCollectionEnvironments } from '@slices/playground'; const ENV_TABS = [ @@ -66,12 +66,15 @@ const EnvironmentsView: React.FC = ({ collection, compact const secretProviderType = selectedEnvironment?.externalSecrets?.type as SecretProviderType | undefined; const secretPointerField = (secretProviderType && SECRET_POINTER_FIELD[secretProviderType]) || 'secretName'; - const externalRows: KeyValueRow[] = (selectedEnvironment?.externalSecrets?.variables ?? []).map( - (variable: { name?: string; disabled?: boolean }, index: number) => ({ + // Same predicate the resolver uses, so the checkbox and the request agree on + // which entries are live. + const externalEntries = (selectedEnvironment?.externalSecrets?.variables ?? []) as ExternalSecretEntry[]; + const externalRows: KeyValueRow[] = externalEntries.map( + (variable, index: number) => ({ id: `ext-${index}`, name: variable.name ?? '', value: (variable as Record)[secretPointerField] ?? '', - enabled: variable.disabled !== true + enabled: isExternalSecretActive(variable) }) ); @@ -101,11 +104,7 @@ const EnvironmentsView: React.FC = ({ collection, compact applyToSelectedEnv({ externalSecrets: { ...(selectedEnvironment?.externalSecrets ?? {}), - variables: rows.map((row) => ({ - name: row.name, - [secretPointerField]: row.value, - disabled: !row.enabled - })) + variables: mergeExternalSecretRows(selectedEnvironment?.externalSecrets?.variables, rows, secretPointerField) } }); diff --git a/packages/bruno-api-docs/src/components/VariableInfoCard/StyledWrapper.ts b/packages/bruno-api-docs/src/components/VariableInfoCard/StyledWrapper.ts index dcdb111c..2d97f4df 100644 --- a/packages/bruno-api-docs/src/components/VariableInfoCard/StyledWrapper.ts +++ b/packages/bruno-api-docs/src/components/VariableInfoCard/StyledWrapper.ts @@ -66,6 +66,7 @@ export const StyledWrapper = styled.div` } .var-value-display { + min-height: 1.25rem; padding-right: 1.5rem; font-family: var(--font-sans); font-size: 0.8125rem; @@ -95,7 +96,7 @@ export const StyledWrapper = styled.div` display: block; width: 100%; margin: 0; - padding: 0; + padding: 0 1.5rem 0 0; font-family: var(--font-sans); font-size: 0.8125rem; font-weight: 400; @@ -128,7 +129,8 @@ export const StyledWrapper = styled.div` height: 1em; } - .var-icons .copy-button { + .var-icons .copy-button, + .var-icons .reveal-button { display: inline-flex; align-items: center; justify-content: center; @@ -143,17 +145,25 @@ export const StyledWrapper = styled.div` transition: opacity 0.15s ease, color 0.15s ease; } - .var-icons .copy-button:hover { + .var-icons .copy-button:hover, + .var-icons .reveal-button:hover { opacity: 1; color: var(--text-primary); background: transparent; } - .var-icons .copy-button:focus-visible { + .var-icons .copy-button:focus-visible, + .var-icons .reveal-button:focus-visible { outline: 0.125rem solid var(--primary-color); outline-offset: 0.0625rem; } + .var-icons .copy-button[data-copied] { + opacity: 1; + color: var(--oc-status-success-text); + cursor: default; + } + .var-readonly-note { margin-top: 0.25rem; font-size: 0.6875rem; diff --git a/packages/bruno-api-docs/src/components/VariableInfoCard/VariableInfoCard.spec.tsx b/packages/bruno-api-docs/src/components/VariableInfoCard/VariableInfoCard.spec.tsx index d505603b..276df54f 100644 --- a/packages/bruno-api-docs/src/components/VariableInfoCard/VariableInfoCard.spec.tsx +++ b/packages/bruno-api-docs/src/components/VariableInfoCard/VariableInfoCard.spec.tsx @@ -24,6 +24,7 @@ const collection: any = { { name: 'host', value: 'https://dev.test' }, { name: 'endpoint', value: '{{host}}/v1' }, { name: 'bearer_token', value: 'super-secret', secret: true }, + { name: 'unsetSecret', secret: true }, { name: 'emptyValue', value: '' }, { name: 'variantValue', @@ -32,7 +33,11 @@ const collection: any = { { title: 'second', value: 'two' } ] } - ] + ], + externalSecrets: { + type: 'aws-secrets-manager', + variables: [{ name: 'vaultKey', secretName: 'prod/api-key' }] + } } ] } @@ -84,6 +89,23 @@ describe('VariableInfoCard', () => { expect(root.querySelector(selector('copy'))).toBeNull(); }); + // The docs reach this card through two different providers, so both have to + // leave external secrets unresolved. PageRouter uses the second one. + it('does not resolve an external secret on either docs provider', () => { + expect(part(useRenderToDom(cardTree('vaultKey')), 'scope').text).toBe('Undefined'); + + const store = createOpenCollectionStore(); + store.dispatch(setActiveEnv('Dev')); + const docsItemTree = ( + + + + + + ); + expect(part(useRenderToDom(docsItemTree), 'scope').text).toBe('Undefined'); + }); + it('pretty-prints an object-typed value', () => { const value = part(useRenderToDom(cardTree('profile')), 'value').text; expect(JSON.parse(value)).toEqual({ city: 'NYC', zip: 10001 }); @@ -182,10 +204,32 @@ describe('VariableInfoCard (editable)', () => { expect(value.classList.contains('var-value-placeholder')).toBe(false); }); - it('never makes a secret variable editable', () => { - const value = part(useRenderToDom(editableCardTree('bearer_token')), 'value'); - expect(value.text).toBe('(Secret)'); - expect(value.classList.contains('var-value-editable')).toBe(false); + it('masks a secret, offers reveal and copy, and makes it editable', () => { + const root = useRenderToDom(editableCardTree('bearer_token')); + const value = part(root, 'value'); + expect(value.text).toBe('*'.repeat('super-secret'.length)); + expect(root.toString()).not.toContain('super-secret'); + expect(value.classList.contains('var-value-editable')).toBe(true); + expect(root.querySelector(selector('reveal'))).not.toBeNull(); + expect(root.querySelector(selector('copy'))).not.toBeNull(); + }); + + it('shows a declared external secret as blank and editable, with reveal and copy', () => { + const root = useRenderToDom(editableCardTree('vaultKey')); + expect(part(root, 'scope').text).toBe('Secret'); + expect(part(root, 'value').text).toBe(''); + expect(part(root, 'value').classList.contains('var-value-editable')).toBe(true); + expect(root.querySelector(selector('reveal'))).not.toBeNull(); + expect(root.querySelector(selector('copy'))).not.toBeNull(); + expect(root.querySelector(selector('note'))).toBeNull(); + }); + + // The app shows copy on every card, so an unfilled secret keeps it too. + it('offers copy on a secret with no value yet', () => { + const root = useRenderToDom(editableCardTree('unsetSecret')); + expect(part(root, 'value').text).toBe(''); + expect(root.querySelector(selector('copy'))).not.toBeNull(); + expect(root.querySelector(selector('reveal'))).not.toBeNull(); }); it('never makes a read-only scope (process.env) editable', () => { diff --git a/packages/bruno-api-docs/src/components/VariableInfoCard/VariableInfoCard.tsx b/packages/bruno-api-docs/src/components/VariableInfoCard/VariableInfoCard.tsx index 2b9066a1..0f48053e 100644 --- a/packages/bruno-api-docs/src/components/VariableInfoCard/VariableInfoCard.tsx +++ b/packages/bruno-api-docs/src/components/VariableInfoCard/VariableInfoCard.tsx @@ -1,11 +1,15 @@ import React, { useLayoutEffect, useRef, useState } from 'react'; import { useResolvedVariables } from '../../hooks'; import { CopyButton } from '../../ui/CopyButton/CopyButton'; +import { EyeIcon, EyeOffIcon } from '../../assets/icons'; import { SCOPE_LABELS, INVALID_NAME_WARNING } from '../../constants'; import type { VariableScope } from '../../utils/variableResolution'; import { StyledWrapper } from './StyledWrapper'; -const EDITABLE_SCOPES = new Set(['environment', 'collection', 'folder', 'request']); +const EDITABLE_SCOPES = new Set(['environment', 'collection', 'folder', 'request', '$secrets']); + +/** These scopes read their values from the active environment, so one must be selected to edit them. */ +const ENV_BOUND_SCOPES = new Set(['environment', '$secrets']); interface VariableInfoCardProps { name: string; @@ -13,12 +17,22 @@ interface VariableInfoCardProps { testId?: string; } -const getReadOnlyNote = (scope: VariableScope, activeEnvName: string | null): string | null => { - if (scope === 'process.env' || scope === 'oauth2' || scope === '$secrets') return 'read-only'; +/** + * The `$secrets` scope covers two different things. A literal `{{$secrets.x}}` + * reference points at a secrets provider that a browser cannot reach, so it is + * read-only. An external secret declared on the environment can be filled in + * from the playground, so it is not. `canEdit` is what tells them apart. + */ +const getReadOnlyNote = (scope: VariableScope, activeEnvName: string | null, canEdit: boolean): string | null => { + if (scope === 'process.env' || scope === 'oauth2') return 'read-only'; + if (scope === '$secrets' && !canEdit) return 'read-only'; if (scope === 'undefined') return activeEnvName ? 'Variable is not defined' : 'No active environment'; return null; }; +/** One asterisk per character, keeping line breaks so a multi-line value still looks multi-line. */ +const maskValue = (value: string): string => value.replace(/[^\n]/g, '*'); + export const VariableInfoCard: React.FC = ({ name, editable = false, @@ -27,8 +41,12 @@ export const VariableInfoCard: React.FC = ({ const { lookup, activeEnvName, updateVariable, canWrite } = useResolvedVariables(); const info = lookup(name); const [editing, setEditing] = useState(false); + const [revealed, setRevealed] = useState(false); const [draft, setDraft] = useState(''); const editRef = useRef(null); + const selectionRef = useRef({ start: 0, end: 0 }); + // Where the caret should sit after the next render, or null to leave it alone. + const caretRef = useRef(null); useLayoutEffect(() => { const el = editRef.current; @@ -37,17 +55,39 @@ export const VariableInfoCard: React.FC = ({ el.style.height = `${el.scrollHeight}px`; }, [editing, draft]); + // Setting a textarea's value moves the caret to the end, so any position we + // want to keep has to be reapplied once React has written the new value. + useLayoutEffect(() => { + const el = editRef.current; + const caret = caretRef.current; + if (!el || caret === null) return; + caretRef.current = null; + el.setSelectionRange(caret, caret); + }, [editing, draft]); + const canEdit = editable && canWrite && info.valid - && !info.secret && info.simpleString && EDITABLE_SCOPES.has(info.scope) - && (info.scope !== 'environment' || !!activeEnvName); + && (!ENV_BOUND_SCOPES.has(info.scope) || !!activeEnvName); + + // Secrets are only fillable in the playground. The rendered docs show the + // original "(Secret)" placeholder with no mask, reveal or copy. + const secretFillable = editable && info.secret; + const masked = secretFillable && !revealed; + const displayValue = masked ? maskValue(info.value) : info.value; + // A variable can be secret because it *is* one, or because its value mentions + // one. In the second case the raw value is a template such as + // `Bearer {{token}}`, which holds nothing sensitive, so it is edited in plain + // text rather than masked. + const maskWhileEditing = masked && info.rawValue === info.value; + const editValue = maskWhileEditing ? maskValue(draft) : draft; const startEditing = () => { setDraft(info.rawValue); + caretRef.current = info.rawValue.length; setEditing(true); }; @@ -56,7 +96,63 @@ export const VariableInfoCard: React.FC = ({ if (draft !== info.rawValue) updateVariable(info.name, draft); }; + const rememberSelection = (event: React.SyntheticEvent) => { + const el = event.currentTarget; + selectionRef.current = { start: el.selectionStart ?? 0, end: el.selectionEnd ?? 0 }; + }; + + /** + * While masked, the field contains asterisks rather than the secret, so an + * edit has to be translated back onto the real string. There is one asterisk + * per character, so a position in the field is the same position in the value. + * + * The edited range runs from wherever the change began to wherever the caret + * ended up. Reading it from the caret afterwards, rather than from the + * selection beforehand, is what makes Backspace and Delete work: they remove a + * character next to an empty selection, which the selection alone cannot + * describe. + */ + const handleEditChange = (event: React.ChangeEvent) => { + const el = event.target; + const next = el.value; + if (!maskWhileEditing) { + setDraft(next); + return; + } + + const caret = el.selectionStart ?? 0; + const from = Math.min(selectionRef.current.start, caret); + const inserted = next.slice(from, caret); + const removedCount = draft.length - (next.length - inserted.length); + caretRef.current = caret; + setDraft(draft.slice(0, from) + inserted + draft.slice(from + removedCount)); + }; + + /** + * Copying out of a masked field would otherwise put asterisks on the + * clipboard. Because a position in the field is the same position in the + * value, the selected range can be taken from the real string instead. + */ + const writeSelectionToClipboard = (event: React.ClipboardEvent): [number, number] | null => { + const el = event.currentTarget; + const start = el.selectionStart ?? 0; + const end = el.selectionEnd ?? 0; + if (!maskWhileEditing || start === end) return null; + event.preventDefault(); + event.clipboardData.setData('text/plain', draft.slice(start, end)); + return [start, end]; + }; + + const handleEditCut = (event: React.ClipboardEvent) => { + const range = writeSelectionToClipboard(event); + if (!range) return; + const [start, end] = range; + caretRef.current = start; + setDraft(draft.slice(0, start) + draft.slice(end)); + }; + const handleEditKeyDown = (event: React.KeyboardEvent) => { + rememberSelection(event); if (event.key === 'Enter' && !event.shiftKey) { event.preventDefault(); commit(); @@ -111,19 +207,38 @@ export const VariableInfoCard: React.FC = ({ ); } - const readOnlyNote = getReadOnlyNote(info.scope, activeEnvName); - const emptyLabel = info.value === '' ? '(empty)' : null; - const placeholder = info.secret ? '(Secret)' : canEdit ? null : emptyLabel; + const readOnlyNote = getReadOnlyNote(info.scope, activeEnvName, canEdit); + const emptyLabel = !secretFillable && info.value === '' ? '(empty)' : null; + const placeholder = info.secret && !editable ? '(Secret)' : canEdit ? null : emptyLabel; + + // The playground always offers copy, matching the app, even with nothing yet to + // copy. The docs keep their original rule: a value, and never for a secret. + const showCopy = editable || (info.value !== '' && !info.secret); - const copyButton = ( + const icons = showCopy && (
- + {secretFillable && ( + + )} + {showCopy && ( + + )}
); @@ -139,47 +254,45 @@ export const VariableInfoCard: React.FC = ({ className="var-value-edit" data-testid={`${testId}-edit`} aria-label={`Edit ${info.name}`} - value={draft} + value={editValue} autoFocus rows={1} spellCheck={false} - onChange={(event) => setDraft(event.target.value)} + autoComplete="off" + onChange={handleEditChange} onKeyDown={handleEditKeyDown} + onSelect={rememberSelection} + onCopy={writeSelectionToClipboard} + onCut={handleEditCut} onBlur={commit} /> ); const editableDisplayNode = ( - <> -
{ - event.preventDefault(); - startEditing(); - }} - onKeyDown={(event) => { - if (event.key !== 'Enter' && event.key !== ' ') return; - event.preventDefault(); - startEditing(); - }} - > - {emptyLabel ?? info.value} -
- {copyButton} - +
{ + event.preventDefault(); + startEditing(); + }} + onKeyDown={(event) => { + if (event.key !== 'Enter' && event.key !== ' ') return; + event.preventDefault(); + startEditing(); + }} + > + {emptyLabel ?? displayValue} +
); const readOnlyDisplayNode = ( - <> -
- {info.value} -
- {copyButton} - +
+ {displayValue} +
); const editableNode = editing ? editFieldNode : editableDisplayNode; @@ -188,7 +301,10 @@ export const VariableInfoCard: React.FC = ({ return ( {header} -
{valueNode}
+
+ {valueNode} + {icons} +
{readOnlyNote && (
{readOnlyNote} diff --git a/packages/bruno-api-docs/src/e2eFixtures/variablesCollection.ts b/packages/bruno-api-docs/src/e2eFixtures/variablesCollection.ts index e0a678ef..158d38eb 100644 --- a/packages/bruno-api-docs/src/e2eFixtures/variablesCollection.ts +++ b/packages/bruno-api-docs/src/e2eFixtures/variablesCollection.ts @@ -11,8 +11,13 @@ export const variablesFixtureCollection = { { name: 'host', value: 'https://api.dev.example.com' }, { name: 'endpoint', value: '{{host}}/v1' }, { name: 'bearer_token', value: 'super-secret-token', secret: true }, + { name: 'unsetSecret', secret: true }, { name: 'emptyValue', value: '' } - ] + ], + externalSecrets: { + type: 'aws-secrets-manager', + variables: [{ name: 'vaultKey', secretName: 'prod/payment/api-key' }] + } } ] }, @@ -30,7 +35,8 @@ export const variablesFixtureCollection = { variables: [ { name: 'apiVersion', value: '2024-01' }, { name: 'profile', value: { type: 'object', data: { city: 'NYC', zip: 10001 } } }, - { name: 'exampleOnly', value: 'example-value' } + { name: 'exampleOnly', value: 'example-value' }, + { name: 'collectionSecret', value: 'collection-secret-value', secret: true } ] }, items: [ @@ -40,7 +46,10 @@ export const variablesFixtureCollection = { seq: 1, request: { headers: [{ name: 'X-Folder-Scope', value: '{{folderScope}}' }], - variables: [{ name: 'folderScope', value: 'from-folder' }] + variables: [ + { name: 'folderScope', value: 'from-folder' }, + { name: 'folderSecret', value: 'folder-secret-value', secret: true } + ] }, items: [ { @@ -48,7 +57,7 @@ export const variablesFixtureCollection = { type: 'http', seq: 1, method: 'GET', - url: '{{host}}/customers/{{userId}}?v={{apiVersion}}', + url: '{{host}}/customers/{{userId}}?v={{apiVersion}}&k={{vaultKey}}&s={{unsetSecret}}', auth: { type: 'bearer', token: '{{bearer_token}}' }, headers: [ { name: 'Authorization', value: 'Bearer {{bearer_token}}' }, @@ -57,7 +66,11 @@ export const variablesFixtureCollection = { { name: 'X-Profile', value: '{{profile}}' }, { name: 'X-Home', value: '{{process.env.HOME}}' }, { name: 'X-Random', value: '{{$randomInt}}' }, - { name: 'X-Empty', value: '{{emptyValue}}' } + { name: 'X-Empty', value: '{{emptyValue}}' }, + { name: 'X-Collection-Secret', value: '{{collectionSecret}}' }, + { name: 'X-Folder-Secret', value: '{{folderSecret}}' }, + { name: 'X-Vault-Key', value: '{{vaultKey}}' }, + { name: 'X-Unset-Secret', value: '{{unsetSecret}}' } ], body: { type: 'json', diff --git a/packages/bruno-api-docs/src/hooks/useCopy.ts b/packages/bruno-api-docs/src/hooks/useCopy.ts index 46d314f9..2fc03c2f 100644 --- a/packages/bruno-api-docs/src/hooks/useCopy.ts +++ b/packages/bruno-api-docs/src/hooks/useCopy.ts @@ -20,8 +20,12 @@ function useCopy({ }; }, []); + useEffect(() => { + setCopied((was) => (was ? false : was)); + }, [text, getText]); + const copyResponse = useCallback(async () => { - if (disabled || !navigator.clipboard || !(text || getText)) return; + if (disabled || !navigator.clipboard) return; try { await navigator.clipboard.writeText(text ? text : getText ? getText() : ''); setCopied(true); diff --git a/packages/bruno-api-docs/src/hooks/useVariableResolver.tsx b/packages/bruno-api-docs/src/hooks/useVariableResolver.tsx index 4b7b6dd8..816fab12 100644 --- a/packages/bruno-api-docs/src/hooks/useVariableResolver.tsx +++ b/packages/bruno-api-docs/src/hooks/useVariableResolver.tsx @@ -16,9 +16,10 @@ import { singleReferenceName, detectSpecialScope, isValidVariableName, + isExternalSecretActive, formatEntryValue, referencesSecret, - isSecretVariable, + type ExternalSecretEntry, type ScopedVariableModel, type VariableScope, type VariableSource @@ -110,14 +111,26 @@ const makeResolver = ( }; }; -const collectionAndEnvSources = (collection: OpenCollection | null, activeEnvName: string | null): VariableSource[] => { +/** Presents an environment's external secrets as ordinary secret variables. */ +const externalSecretVariables = (environment: Environment | undefined): SecretVariable[] => + ((environment?.externalSecrets?.variables ?? []) as ExternalSecretEntry[]) + .filter((entry) => entry.name && isExternalSecretActive(entry)) + .map((entry) => ({ name: entry.name, secret: true, value: entry.value ?? '' }) as unknown as SecretVariable); + +const collectionAndEnvSources = ( + collection: OpenCollection | null, + activeEnvName: string | null, + withExternalSecrets = false +): VariableSource[] => { const collectionVariables = (collection?.request?.variables ?? []) as (Variable | SecretVariable)[]; const environments = (collection?.config?.environments ?? []) as Environment[]; const activeEnvironment = environments.find((environment) => environment.name === activeEnvName); - return [ - { scope: 'collection', variables: collectionVariables }, - { scope: 'environment', variables: activeEnvironment?.variables } - ]; + const sources: VariableSource[] = [{ scope: 'collection', variables: collectionVariables }]; + if (withExternalSecrets) { + sources.push({ scope: '$secrets', variables: externalSecretVariables(activeEnvironment) }); + } + sources.push({ scope: 'environment', variables: activeEnvironment?.variables }); + return sources; }; const folderVariables = (folder: Item): (Variable | SecretVariable)[] => @@ -194,21 +207,23 @@ export const ItemVariableResolverProvider: React.FC<{ const activeEnvName = useAppSelector(selectActiveEnvName); const showVars = useAppSelector(selectShowVars); + // Both the docs pages and the playground mount this provider; only the + // playground passes `writable`, and only it can supply an external secret. const model = useMemo(() => { - const sources: VariableSource[] = collectionAndEnvSources(collection, activeEnvName); + const sources: VariableSource[] = collectionAndEnvSources(collection, activeEnvName, writable); for (const folder of ancestry) { sources.push({ scope: 'folder', variables: folderVariables(folder) }); } if (item) sources.push(itemSource(item)); return buildScopedVariableModel(sources); - }, [collection, activeEnvName, ancestry, item]); + }, [collection, activeEnvName, ancestry, item, writable]); const resolver = useMemo(() => makeResolver(model, showVars, activeEnvName), [model, showVars, activeEnvName]); const updateVariable = useCallback( (name: string, value: string) => { const { name: varName, scope } = resolver.lookup(name); - if (scope === 'environment') { + if (scope === 'environment' || scope === '$secrets') { if (activeEnvName) dispatch(setPlaygroundVariable({ scope, name: varName, value, envName: activeEnvName })); } else if (scope === 'collection') { dispatch(setPlaygroundVariable({ scope, name: varName, value })); @@ -217,7 +232,7 @@ export const ItemVariableResolverProvider: React.FC<{ if (itemUuid) dispatch(setPlaygroundVariable({ scope, name: varName, value, itemUuid })); } else if (scope === 'folder') { const owner = [...ancestry].reverse().find((folder) => - folderVariables(folder).some((v) => v.name === varName && !v.disabled && !isSecretVariable(v)) + folderVariables(folder).some((v) => v.name === varName && !v.disabled) ); const itemUuid = getItemUuid(owner); if (itemUuid) dispatch(setPlaygroundVariable({ scope, name: varName, value, itemUuid })); diff --git a/packages/bruno-api-docs/src/runner/index.ts b/packages/bruno-api-docs/src/runner/index.ts index 25f0fa37..a122d46b 100644 --- a/packages/bruno-api-docs/src/runner/index.ts +++ b/packages/bruno-api-docs/src/runner/index.ts @@ -6,9 +6,19 @@ import ScriptRuntime from '../scripting/runtime/script-runtime'; import AssertRuntime, { type AssertionResult } from '../scripting/runtime/assert-runtime'; import { getTreePathFromCollectionToItem, mergeHeaders, mergeScripts, mergeAuth, interpolateVars } from './utils'; import { getCollectionFolderRequestVariables } from './utils/variable-merger'; -import { coerceVariableValue, parseValueByDataType } from '../utils/variableDataType'; +import { coerceVariableValue, parseValueByDataType, type CoercedVariableValue } from '../utils/variableDataType'; +import { externalSecretValues, type ExternalSecretEntry } from '../utils/variableResolution'; +import type { VariableValueOrVariants, VariableValueType } from '@opencollection/types/common/variables'; import { getRequestScripts, getRequestAssertions, scriptsArrayToObject } from '../utils/schemaHelpers'; +interface DeclaredEnvironmentVariable { + name?: string; + value?: VariableValueOrVariants; + type?: VariableValueType; + secret?: boolean; + disabled?: boolean; +} + export interface RunRequestOptions { item: HttpRequest; collection: OpenCollectionCollection; @@ -210,20 +220,26 @@ export class RequestRunner { } private getEnvironmentVariables(environment?: Environment): Record { - if (!environment?.variables) return {}; - - return environment.variables.reduce((vars, variable: any) => { + // External secrets are referenced as ordinary `{{name}}` variables, so they + // go in first and a variable declared on the environment with the same name + // takes precedence over them. + const externalSecrets = externalSecretValues( + environment?.externalSecrets?.variables as ExternalSecretEntry[] | undefined + ); + if (!environment?.variables) return externalSecrets; + + return environment.variables.reduce((vars, variable: DeclaredEnvironmentVariable) => { const name = variable.name; if (name && !variable.disabled) { // Coerce typed values (number/boolean/object) to native, like folder/collection/request vars. // A secret carries its data type as a sibling `type` (value is a plain string), whereas a // non-secret nests it inside the value — so coerce each from the right place. vars[name] = variable.secret - ? parseValueByDataType(variable.value, variable.type) + ? parseValueByDataType(variable.value as CoercedVariableValue, variable.type) : coerceVariableValue(variable.value); } return vars; - }, {} as Record); + }, { ...externalSecrets } as Record); } private async preprocessRequest( diff --git a/packages/bruno-api-docs/src/store/slices/playground.spec.ts b/packages/bruno-api-docs/src/store/slices/playground.spec.ts index 78f76d9e..f6fcd42b 100644 --- a/packages/bruno-api-docs/src/store/slices/playground.spec.ts +++ b/packages/bruno-api-docs/src/store/slices/playground.spec.ts @@ -31,6 +31,10 @@ const makeCollection = () => const envVariables = (store: ReturnType) => selectHydratedCollection(store.getState())!.config!.environments![0].variables!; +const envExternalSecrets = (store: ReturnType) => + (selectHydratedCollection(store.getState())!.config!.environments![0].externalSecrets! + .variables as unknown) as { name: string; value?: string; secretName?: string }[]; + describe('resetPlaygroundEnvironments', () => { it('restores the original environments after an edit', () => { const store = createOpenCollectionStore(); @@ -155,16 +159,33 @@ describe('setPlaygroundVariable', () => { expect(item.variables.find((v) => v.name === 'rv').value).toBe('2'); }); - it('never writes to a secret variable', () => { + it('writes a session value to a secret variable, keeping it marked secret', () => { const collection = makeCollection(); collection.config.environments[0].variables.push({ name: 'sec', secret: true }); const store = createOpenCollectionStore(); store.dispatch(setPlaygroundCollection(collection)); - store.dispatch(setPlaygroundVariable({ scope: 'environment', name: 'sec', value: 'leak', envName: 'Dev' })); + store.dispatch(setPlaygroundVariable({ scope: 'environment', name: 'sec', value: 'typed', envName: 'Dev' })); + + const sec = envVariables(store).find((v) => v.name === 'sec') as { value: string; secret: boolean }; + expect(sec.value).toBe('typed'); + expect(sec.secret).toBe(true); + }); + + it('writes a session value to an external secret pointer', () => { + const collection = makeCollection(); + collection.config.environments[0].externalSecrets = { + type: 'aws-secrets-manager', + variables: [{ name: 'vaultKey', secretName: 'prod/api-key' }] + }; + const store = createOpenCollectionStore(); + store.dispatch(setPlaygroundCollection(collection)); + + store.dispatch(setPlaygroundVariable({ scope: '$secrets', name: 'vaultKey', value: 'typed', envName: 'Dev' })); - const sec = envVariables(store).find((v: any) => v.name === 'sec'); - expect((sec as any).value).toBeUndefined(); + const [vaultKey] = envExternalSecrets(store); + expect(vaultKey.value).toBe('typed'); + expect(vaultKey.secretName).toBe('prod/api-key'); }); }); diff --git a/packages/bruno-api-docs/src/store/slices/playground.ts b/packages/bruno-api-docs/src/store/slices/playground.ts index c433c8d4..710ee035 100644 --- a/packages/bruno-api-docs/src/store/slices/playground.ts +++ b/packages/bruno-api-docs/src/store/slices/playground.ts @@ -8,7 +8,6 @@ import type { Variable, SecretVariable } from '@opencollection/types/common/vari import type { RootState } from '../store'; import { hydrateWithUUIDs, findAndUpdateItem } from '../../utils/fileUtils'; import { isFolder, getRequestVariables } from '../../utils/schemaHelpers'; -import { isSecretVariable } from '../../utils/variableResolution'; import type { ResponseBodyFormat } from '@/constants'; export type ViewMode = 'playground' | 'environments' | 'folder-settings' | 'collection-settings' | 'example'; @@ -236,7 +235,7 @@ const playgroundSlice = createSlice({ setPlaygroundVariable: ( state: PlaygroundState, action: PayloadAction<{ - scope: 'environment' | 'collection' | 'folder' | 'request'; + scope: 'environment' | 'collection' | 'folder' | 'request' | '$secrets'; name: string; value: string; envName?: string; @@ -244,14 +243,19 @@ const playgroundSlice = createSlice({ }> ) => { const { scope, name, value, envName, itemUuid } = action.payload; + // Secret variables are writable. Their values only ever live on this + // in-memory copy of the collection, which is rebuilt from the source on + // reload and never written back, so nothing typed here is persisted. const setInList = (list: (Variable | SecretVariable)[] | undefined): void => { const enabled = (list ?? []).filter((v) => v.name === name && !v.disabled); - const variable = enabled[enabled.length - 1]; - if (variable && !isSecretVariable(variable)) variable.value = value; + const variable = enabled[enabled.length - 1] as (Variable & { value?: string }) | undefined; + if (variable) variable.value = value; }; const apply = (collection: OpenCollectionCollection | null) => { if (!collection) return; - if (scope === 'environment') { + if (scope === '$secrets') { + setInList(readEnvironments(collection)?.find((env) => env.name === envName)?.externalSecrets?.variables); + } else if (scope === 'environment') { setInList(readEnvironments(collection)?.find((env) => env.name === envName)?.variables); } else if (scope === 'collection') { setInList(collection.request?.variables as (Variable | SecretVariable)[] | undefined); diff --git a/packages/bruno-api-docs/src/ui/CopyButton/CopyButton.spec.tsx b/packages/bruno-api-docs/src/ui/CopyButton/CopyButton.spec.tsx index 2151fd71..48f07f37 100644 --- a/packages/bruno-api-docs/src/ui/CopyButton/CopyButton.spec.tsx +++ b/packages/bruno-api-docs/src/ui/CopyButton/CopyButton.spec.tsx @@ -1,18 +1,24 @@ import React from 'react'; -import { renderToStaticMarkup } from 'react-dom/server'; import { describe, it, expect } from 'vitest'; +import { useRenderToDom } from '../../hooks/useRenderToDom'; +import { getByTestId } from '../../test-utils/dom'; import { CopyButton } from './CopyButton'; describe('CopyButton', () => { it('renders an accessible button with the default copy label', () => { - const html = renderToStaticMarkup(); - expect(html).toContain('), 'copy-button'); + expect(button.tagName.toLowerCase()).toBe('button'); + expect(button.getAttribute('type')).toBe('button'); + expect(button.getAttribute('aria-label')).toBe('Copy'); }); it('uses the provided label', () => { - const html = renderToStaticMarkup(); - expect(html).toContain('aria-label="Copy code"'); + const button = getByTestId(useRenderToDom(), 'copy-button'); + expect(button.getAttribute('aria-label')).toBe('Copy code'); + }); + + it('is not marked copied on first render', () => { + const root = useRenderToDom(); + expect(getByTestId(root, 'copy-button').getAttribute('data-copied')).toBeFalsy(); }); }); diff --git a/packages/bruno-api-docs/src/ui/CopyButton/CopyButton.tsx b/packages/bruno-api-docs/src/ui/CopyButton/CopyButton.tsx index 86b63488..c195b780 100644 --- a/packages/bruno-api-docs/src/ui/CopyButton/CopyButton.tsx +++ b/packages/bruno-api-docs/src/ui/CopyButton/CopyButton.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useRef, useState } from 'react'; +import React from 'react'; import { StyledWrapper } from './StyledWrapper'; import { IconCheck, IconCopy } from '@tabler/icons'; import useCopy from '@/hooks/useCopy'; @@ -37,10 +37,15 @@ export const CopyButton: React.FC = ({ className={cx('copy-button', className)} onClick={copyResponse} aria-label={copied ? copiedLabel : label} + data-copied={copied ? 'true' : undefined} data-testid={testId} style={style} > - {copied ? : } + {copied ? ( + + ) : ( + + )} ); }; diff --git a/packages/bruno-api-docs/src/ui/CopyButton/StyledWrapper.ts b/packages/bruno-api-docs/src/ui/CopyButton/StyledWrapper.ts index eab43ce3..6928ea6e 100644 --- a/packages/bruno-api-docs/src/ui/CopyButton/StyledWrapper.ts +++ b/packages/bruno-api-docs/src/ui/CopyButton/StyledWrapper.ts @@ -18,6 +18,11 @@ export const StyledWrapper = styled.button` background-color: var(--badge-bg); } + &[data-copied] { + color: var(--oc-status-success-text); + cursor: default; + } + &:focus-visible { outline: 2px solid var(--primary-color); outline-offset: 1px; diff --git a/packages/bruno-api-docs/src/utils/environments.spec.ts b/packages/bruno-api-docs/src/utils/environments.spec.ts index 48263e0e..530b5c8e 100644 --- a/packages/bruno-api-docs/src/utils/environments.spec.ts +++ b/packages/bruno-api-docs/src/utils/environments.spec.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest'; -import { getEnvironmentVariables, envVariableToRow, envRowToVariable } from './environments'; +import { getEnvironmentVariables, envVariableToRow, envRowToVariable, mergeExternalSecretRows } from './environments'; +import { isExternalSecretActive } from './variableResolution'; describe('getEnvironmentVariables', () => { it('splits regular and secret variables', () => { @@ -274,3 +275,53 @@ describe('envVariableToRow / envRowToVariable round-trip', () => { expect(out.value).toBe('new-token'); }); }); + +describe('mergeExternalSecretRows', () => { + const existing = [ + { name: 'vaultKey', secretName: 'prod/api-key', value: 'typed-this-session' }, + { name: 'dbPassword', secretName: 'prod/db' } + ]; + + it('keeps a session value when another field on the row is edited', () => { + const rows = [ + { name: 'vaultKey', value: 'prod/api-key-renamed', enabled: true }, + { name: 'dbPassword', value: 'prod/db', enabled: true } + ]; + + const out = mergeExternalSecretRows(existing, rows, 'secretName') as Record[]; + + expect(out[0].value).toBe('typed-this-session'); + expect(out[0].secretName).toBe('prod/api-key-renamed'); + expect(out[1].value).toBeUndefined(); + }); + + it('carries the session value through a disable toggle', () => { + const rows = [{ name: 'vaultKey', value: 'prod/api-key', enabled: false }]; + + const out = mergeExternalSecretRows(existing, rows, 'secretName') as Record[]; + + expect(out[0].value).toBe('typed-this-session'); + expect(out[0].disabled).toBe(true); + }); + + // An entry holding both `enabled: false` and `disabled: false` reads as off, + // which would switch the secret off even though the table shows it on. + it('drops a legacy enabled key rather than contradicting the row toggle', () => { + const legacy = [{ name: 'apiKey', secretName: 'prod/api-key', enabled: false }]; + const rows = [{ name: 'apiKey', value: 'prod/api-key', enabled: true }]; + + const [out] = mergeExternalSecretRows(legacy, rows, 'secretName') as Record[]; + + expect(out.enabled).toBeUndefined(); + expect(out.disabled).toBe(false); + expect(isExternalSecretActive(out as { disabled?: boolean; enabled?: boolean })).toBe(true); + }); + + it('adds a brand new row with no carried fields', () => { + const rows = [{ name: 'fresh', value: 'prod/fresh', enabled: true }]; + + const out = mergeExternalSecretRows(existing, rows, 'secretName') as Record[]; + + expect(out).toEqual([{ name: 'fresh', secretName: 'prod/fresh', disabled: false }]); + }); +}); diff --git a/packages/bruno-api-docs/src/utils/environments.ts b/packages/bruno-api-docs/src/utils/environments.ts index aea95e93..d10a5f91 100644 --- a/packages/bruno-api-docs/src/utils/environments.ts +++ b/packages/bruno-api-docs/src/utils/environments.ts @@ -3,7 +3,7 @@ import type { Variable, VariableValueType } from '@opencollection/types/common/v import { MANAGER_LABELS } from '../constants'; import { getDescription, getVariableTypeLabel } from './request'; import { descriptionText, resolveDescription } from './description'; -import { isSecretVariable, unwrapVariableValue } from './variableResolution'; +import { isSecretVariable, unwrapVariableValue, type ExternalSecretEntry } from './variableResolution'; import { rowToVariable, toDataType } from './variableDataType'; const humanizeManager = (type: string | undefined): string => { @@ -76,6 +76,33 @@ interface ExternalSecretsConfig { variables?: { name?: string; secretName?: string; enabled?: boolean; type?: VariableValueType }[]; } +/** + * Rebuild an environment's external secrets after the table has been edited. + * + * A table row holds only a name, a pointer and a toggle, so anything else on the + * entry has to be carried across by hand. In particular the hover card stores a + * typed-in `value` there, which a plain rebuild would throw away. + * + * `enabled` is the exception and is dropped on purpose: the toggle is written + * back as `disabled`, and an entry carrying both could contradict itself. + */ +export const mergeExternalSecretRows = ( + existing: ExternalSecretEntry[] | undefined, + rows: { name: string; value: string; enabled: boolean }[], + pointerField: string +): Record[] => { + const byName = new Map((existing ?? []).map((variable) => [variable.name, variable])); + return rows.map((row) => { + const { enabled: _legacyToggle, ...carried } = byName.get(row.name) ?? {}; + return { + ...carried, + name: row.name, + [pointerField]: row.value, + disabled: !row.enabled + }; + }); +}; + export interface EnvironmentVariableRow { name: string; value: string; diff --git a/packages/bruno-api-docs/src/utils/variableResolution.test.ts b/packages/bruno-api-docs/src/utils/variableResolution.test.ts index 4f7ae2b5..6205a1a5 100644 --- a/packages/bruno-api-docs/src/utils/variableResolution.test.ts +++ b/packages/bruno-api-docs/src/utils/variableResolution.test.ts @@ -1,5 +1,7 @@ import { describe, it, expect } from 'vitest'; import { + isExternalSecretActive, + externalSecretValues, resolveVariables, singleReferenceName, unwrapVariableTyped, @@ -132,3 +134,40 @@ describe('formatEntryValue', () => { expect(formatEntryValue(entry, {})).toBe(JSON.stringify(JSON.parse(entry.value), null, 2)); }); }); + +describe('isExternalSecretActive', () => { + it('treats an entry with neither toggle as active', () => { + expect(isExternalSecretActive({ name: 'apiKey' })).toBe(true); + }); + + it('honours the disabled spelling written by the environments editor', () => { + expect(isExternalSecretActive({ name: 'apiKey', disabled: true })).toBe(false); + expect(isExternalSecretActive({ name: 'apiKey', disabled: false })).toBe(true); + }); + + // The sample collection in this repo writes `enabled`, so a secret switched + // off that way must not reach the request. + it('honours the enabled spelling written by the sample collection', () => { + expect(isExternalSecretActive({ name: 'apiKey', enabled: false })).toBe(false); + expect(isExternalSecretActive({ name: 'apiKey', enabled: true })).toBe(true); + }); +}); + +describe('externalSecretValues', () => { + it('keeps a secret deliberately cleared to an empty string', () => { + expect(externalSecretValues([{ name: 'apiKey', value: '' }])).toEqual({ apiKey: '' }); + }); + + it('skips a secret that was never filled in, leaving the reference unresolved', () => { + expect(externalSecretValues([{ name: 'apiKey' }])).toEqual({}); + }); + + it('skips a switched-off secret under either spelling', () => { + expect(externalSecretValues([{ name: 'a', value: 'x', disabled: true }])).toEqual({}); + expect(externalSecretValues([{ name: 'b', value: 'x', enabled: false }])).toEqual({}); + }); + + it('returns an empty map when the environment declares none', () => { + expect(externalSecretValues(undefined)).toEqual({}); + }); +}); diff --git a/packages/bruno-api-docs/src/utils/variableResolution.ts b/packages/bruno-api-docs/src/utils/variableResolution.ts index 3ce2608e..eec06136 100644 --- a/packages/bruno-api-docs/src/utils/variableResolution.ts +++ b/packages/bruno-api-docs/src/utils/variableResolution.ts @@ -103,7 +103,47 @@ export type VariableScope | '$secrets' | 'undefined'; -export type ConcreteScope = 'collection' | 'environment' | 'folder' | 'request'; +/** Scopes a variable can actually be declared in, `$secrets` being an environment's external secrets. */ +export type ConcreteScope = 'collection' | 'environment' | 'folder' | 'request' | '$secrets'; + +/** + * One entry from an environment's "external secrets" list: a secret whose real + * value lives in a secrets manager such as AWS Secrets Manager or Vault, rather + * than in the collection itself. + * + * - `name` is how the collection refers to it, written as `{{name}}`. + * - `value` is filled in only when someone types one in the playground. A browser + * cannot reach the secrets manager, so there is no other way to get a value. + * - The on/off switch is written as `enabled` by some collections and `disabled` + * by others, so either may be present. Read it with `isExternalSecretActive` + * rather than checking a field directly. + * + * The key used to look the secret up in the manager is stored under a field whose + * name varies by provider (`secretName`, `path`, and others), so it is not listed + * here. Nothing in the playground reads it. + */ +export interface ExternalSecretEntry { + name?: string; + value?: string; + disabled?: boolean; + enabled?: boolean; +} + +/** True unless either spelling of the toggle says the secret is switched off. */ +export const isExternalSecretActive = (entry: ExternalSecretEntry): boolean => + entry.disabled !== true && entry.enabled !== false; + +/** + * External secret values for interpolation, keyed by the name the collection + * references. An entry someone cleared to an empty string is kept, because that + * is a value they chose. An entry nobody ever filled in is skipped, so + * `{{name}}` is sent as-is rather than as an empty string. + */ +export const externalSecretValues = (entries: ExternalSecretEntry[] | undefined): Record => + (entries ?? []).reduce((values, entry) => { + if (entry.name && isExternalSecretActive(entry) && entry.value !== undefined) values[entry.name] = entry.value; + return values; + }, {} as Record); export interface VariableSource { scope: ConcreteScope;