feat(dashboard): view and edit non-secret environment values after save#4157
Conversation
🦋 Changeset detectedLatest commit: 8b29525 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
f1391dd to
becc405
Compare
b83fb5c to
861233e
Compare
becc405 to
71cdc9c
Compare
861233e to
30625e1
Compare
There was a problem hiding this comment.
cubic analysis
Review completed against the latest diff
Linked issue analysis
Linked issue: AGE-2986: feat: allow viewing/revealing non-secret env var values after save
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Add an is_secret flag to the API types / OpenAPI / SDK so clients can mark entries secret vs non-secret | OpenAPI, Goa design, generated server types, and TS SDK types were updated to include is_secret / isSecret. |
| DB schema migration adding environment_entries.is_secret (so storage layer can persist the flag) | Queries and repo/models assume an is_secret column, but the migration file itself is not present in this PR — the PR is stacked on a separate migration PR (#4154) that must be merged first. | |
| ✅ | Store non-secret values plaintext and keep secret values encrypted (invariant: is_secret ⇔ ciphertext) | Create and update code now conditionally encrypts values based on the is_secret flag; storage parameters include is_secret. |
| ✅ | Reads return cleartext for non-secret entries and redacted format for secret entries | List/decode paths check entry.IsSecret and only decrypt/redact when appropriate; response types include is_secret and value semantics updated. |
| ✅ | Flip rules: secret → non-secret requires a new value in the same request; non-secret → secret allowed without a new value (encrypts stored plaintext) | Update handler enforces the secret→non check and allows non-secret→secret without a value. Tests exercise both directions. |
| ✅ | Preserve rule: omitting value on an existing entry preserves stored value (matches headers preserve semantics) | Update logic uses existing value when value omitted; tests verify omitted-value preserves secret and omitted-is_secret preserves secrecy. |
| ✅ | Dashboard: Secret checkbox on new-entry form (default on), per-row lock/unlock toggle, cleartext display + copy for non-secret rows, unlocking a secret forces entering a new value | Dashboard changes add newEntryIsSecret checkbox, per-row lock/unlock UI and handlers, copy button for non-secret rows, and helper text requiring a new value when unlocking secrets. |
| ✅ | API/SDK regen and client examples updated to include is_secret; generated clients/types reflect new behavior | Generated server/client code and CLI examples include is_secret/isSecret fields and validation changes; SDK models updated. |
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
## Summary Adds an `is_secret` boolean column (`NOT NULL DEFAULT true`) to `environment_entries`. This is the schema half of AGE-2986 (allow viewing/revealing non-secret environment variable values after save, prompted by Pylon #11730). The application code that reads and writes the column ships in speakeasy-api#4157, stacked on this PR. Beyond the migration itself, this PR carries the sqlc regeneration (CI's dirty-files check requires generated models to match the schema) and, because the regenerated `EnvironmentEntry` struct gains a field, two one-line `IsSecret: entry.IsSecret` passthroughs in `shared.go` to satisfy exhaustruct. Both are behaviorally inert: nothing reads the field until speakeasy-api#4157. ## Notes for reviewers - The `DEFAULT true` doubles as the backfill: on Postgres 11+ this is a metadata-only change (no table rewrite), and every pre-existing row reads as secret, which preserves today's encrypt-and-redact behavior exactly. Nothing changes for any caller until the follow-up PR lands. - The default stays `true` permanently (the analogous `remote_mcp_server_headers.is_secret` defaults to `false`, but that column existed from table creation). The application always sets the flag explicitly on insert, so the DB default only ever matters for the backfill. - Migration generated with `mise db:diff`; no hand edits. Linear: AGE-2986 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
🚀 Preview Environment (PR #4157)Preview URL: https://pr-4157.dev.getgram.ai
Gram Preview Bot |
There was a problem hiding this comment.
cubic analysis
1 issue found across 36 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/dashboard/src/pages/mcp/MCPEnvironmentSettings.tsx">
<violation number="1" location="client/dashboard/src/pages/mcp/MCPEnvironmentSettings.tsx:364">
P2: Saving a named blank secret variable closes the sheet but creates neither its environment entry nor MCP config. Include empty secret entries in `systemEntries` (while continuing to reject/validate blank readable values) so placeholder rows reach the API.</violation>
</file>
Linked issue analysis
Linked issue: AGE-2986: feat: allow viewing/revealing non-secret env var values after save
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Non-secret environment variable values are readable after save; secret values remain redacted on reads. | API and server types now include is_secret and value semantics; dashboard shows cleartext for non-secret entries and redacted previews for secrets; tests and manual E2E exercised the scenario. |
| ✅ | Storage invariant: non-secret values stored plaintext, secret values encrypted at rest (is_secret ⇔ ciphertext at rest). | DB/queries and repo code now include is_secret and write/read paths that preserve encryption semantics. |
| ✅ | Flip rules: changing secret→non-secret requires supplying a new value in the same request; non-secret→secret allowed without a value and encrypts stored value in place. | Create/Update handlers enforce supplied-value requirement when flipping to non-secret; tests cover both flip directions. |
| ✅ | Preserve rule: omitting value on an existing entry preserves the stored value (no accidental overwrite). | Update logic reads existing value for omitted entries and ListEnvironmentEntriesForUpdate adds a FOR UPDATE path to avoid races; tests assert preservation behavior. |
| ✅ | Back-compat: omitted is_secret defaults to secret for new entries and keeps current secrecy for existing entries. | Handlers default omitted flag to true for new entries and use current stored secrecy for existing ones; comments and tests reflect this policy. |
| ✅ | Dashboard UX: per-variable view/edit flow with a view action exposing server-returned value, modal editing that enforces secret/readable rules, and changes to AddVariableSheet to default new entries to secret. | Client adds EnvironmentVariableDialog, updates Environment page and rows, adds Eye/EyeOff and copy behavior, and default-is-secret behavior in AddVariableSheet; modal enforces rules described in PR. |
| ✅ | Tests and lint/type-check: handler-level tests, client lint/type-check, and E2E checks exercising the core scenario. | PR claims 52 handler-level tests (including new ones for flips/preserve), client lint and type-check pass, and manual E2E verification was performed. |
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Linked issue analysis
Linked issue: AGE-2986: feat: allow viewing/revealing non-secret env var values after save
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Non-secret environment variable values can be revealed after save (eye icon shows cleartext) while secret values only show the redacted preview. | Environment pages and MCP rows now track per-row reveal state and render MASK vs actual value depending on isSecret and reveal state. |
| ✅ | Secret variables are visually marked (Sensitive badge) | Rows render a 'Sensitive' Badge when an entry is secret and stored. |
| ✅ | Editing uses a shared EnvironmentVariableDialog with a Secret toggle; making a secret variable readable requires entering a new value; leaving the value blank on a secret preserves the stored value. | EnvironmentVariableDialog enforces needsNewValue and validates blank handling; it seeds value empty for stored secrets and requires new value to flip secrecy. |
| ✅ | Copy-to-clipboard is available for readable (non-secret) values and is disabled/locked for secrets | Copy action is included but disabled for secrets; dialog also shows CopyButton only when entry is non-secret and not dirty. |
| ✅ | Delete action requires confirmation and deletes the stored value | Delete flows open a confirmation dialog that triggers updateEnvironment with entriesToRemove to remove the variable. |
| ✅ | MCP server environment settings: value rows gain mask/eye reveal, Sensitive badge, kebab menu actions, and editing uses the shared dialog; MCP Save now only stages MCP metadata | MCP tab and rows were modified to use dialog-driven edits, show badges and reveal behavior, and dialog usage captures environment slug at click time; MCP metadata save logic remains responsible for header/mode staging. |
| ✅ | Add Variable sheet: per-row Secret switch (default on) and it rejects an empty non-secret value before submitting | AddVariableSheet initializes entries with isSecret true, includes a per-row Switch, and validates non-secret empty values with an error message before calling onAddVariables. |
Adds an is_secret flag to environment entries. Secret entries keep today's encrypt-and-redact behavior; non-secret entries are stored in plaintext and returned in cleartext so their values stay readable after save. Flipping a secret entry to non-secret requires supplying a new value so environment write access never doubles as secret read access, while flipping a non-secret entry to secret encrypts the stored value in place. Omitting the value on an existing entry preserves the stored one, with the entry rows locked (FOR UPDATE) so a concurrent rotation cannot be reverted by the read-back. Callers that never send is_secret behave exactly as before: entries default to secret. Server half of AGE-2986; the dashboard UI ships separately on top of this in #4157. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3f0cd99 to
74aa437
Compare
Remove the development-artifact design doc, assert the IsSecret flag in the create-environment response, and cover rejection of empty non-secret values. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dashboard half of AGE-2986, on top of the server's is_secret support. The environment detail page and the MCP server environment settings both move value editing into a shared dialog with a Secret toggle, masked values behind an eye reveal, a copy action for readable values, and per-row edit and delete actions with delete confirmation. Secret values keep the redacted-preview behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
74aa437 to
92b152d
Compare
…e change The actions menu only renders in the system state, so switching a row to omit or user unmounted it and shifted the value input right. Keep the wrapper in the layout for every state and hide it with visibility so the row width stays stable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces the hand-rolled bordered rows on the environment detail page with the DotTable/DotRow pattern used by Sources and other list pages. Each row shows a lock icon for secret entries and a code icon for readable ones, with Key and Value columns and the reveal toggle grouped next to the row actions menu. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DemoRecorded against the local What it shows:
|
Demo: MCP server Authentication tabRecorded against the local What it shows:
|
There was a problem hiding this comment.
cubic analysis
All reported issues were addressed across 9 files
Linked issue analysis
Linked issue: AGE-2986: feat: allow viewing/revealing non-secret env var values after save
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Non-secret environment variable values can be revealed after save (eye icon shows cleartext) while secret values only show the redacted preview. | Environment pages and MCP rows now track per-row reveal state and render MASK vs actual value depending on isSecret and reveal state. |
| ✅ | Secret variables are visually marked (Sensitive badge) | Rows render a 'Sensitive' Badge when an entry is secret and stored. |
| ✅ | Editing uses a shared EnvironmentVariableDialog with a Secret toggle; making a secret variable readable requires entering a new value; leaving the value blank on a secret preserves the stored value. | EnvironmentVariableDialog enforces needsNewValue and validates blank handling; it seeds value empty for stored secrets and requires new value to flip secrecy. |
| ✅ | Copy-to-clipboard is available for readable (non-secret) values and is disabled/locked for secrets | Copy action is included but disabled for secrets; dialog also shows CopyButton only when entry is non-secret and not dirty. |
| ✅ | Delete action requires confirmation and deletes the stored value | Delete flows open a confirmation dialog that triggers updateEnvironment with entriesToRemove to remove the variable. |
| ✅ | MCP server environment settings: value rows gain mask/eye reveal, Sensitive badge, kebab menu actions, and editing uses the shared dialog; MCP Save now only stages MCP metadata | MCP tab and rows were modified to use dialog-driven edits, show badges and reveal behavior, and dialog usage captures environment slug at click time; MCP metadata save logic remains responsible for header/mode staging. |
| ✅ | Add Variable sheet: per-row Secret switch (default on) and it rejects an empty non-secret value before submitting | AddVariableSheet initializes entries with isSecret true, includes a per-row Switch, and validates non-secret empty values with an error message before calling onAddVariables. |
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Pull request was closed


Summary
Dashboard half of AGE-2986. Stacked on #4228, which adds the server's
is_secretsupport (this PR's base; retarget tomainonce #4228 merges).Environment variable values can now be viewed after save when they are not secret, on both surfaces that manage them:
Environment detail page
EnvironmentVariableDialog) with a Secret toggle. Making a secret variable readable requires typing a new value (the server enforces this too); leaving the value blank on a secret keeps the stored value.MCP server environment settings
Known gap
The
MoreActionslock icon for the disabled copy action renders via the sharedcomponents/ui/more-actionsicon map; no per-item tooltip explains why it is locked. Left as-is pending a call on extending the shared component.Testing
pnpm -F dashboard lint, type-check, and 737 tests green.🤖 Generated with Claude Code