Skip to content

feat(secrets): add local storage mode#75

Merged
vitramir merged 4 commits into
mainfrom
noa/issue-73
Apr 22, 2026
Merged

feat(secrets): add local storage mode#75
vitramir merged 4 commits into
mainfrom
noa/issue-73

Conversation

@casey-brooks

Copy link
Copy Markdown
Contributor

Summary

  • add storage mode selection for secrets create/edit with local value handling
  • prevent plaintext display by keeping local values masked/empty after creation
  • update secrets list to show source and reference for local vs remote

Testing

  • npm run lint
  • npm run test
  • npm run typecheck
  • npm run build

Fixes #73

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • npm run lint (no errors)
  • npm run test (13 files, 56 passed, 0 failed, 0 skipped)
  • npm run typecheck (pass)
  • npm run build (pass)

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes.

Main blockers:

  • Please avoid sending empty-string placeholders for mutually exclusive fields (value vs secretProviderId/remoteName). This is risky with proto3 optional/oneof presence and weakens the invariant. Build a discriminated union payload and omit irrelevant fields.
  • Storage mode detection should reflect the invariant (remote requires BOTH providerId + remoteName). The current providerId || remoteName check masks partial/invalid states.

Nice-to-have:

  • Update the remote name placeholder/help text to match the spec format.
  • Consider surfacing missing provider IDs instead of falling back to a generic label.

Tests (recommended): add a minimal e2e asserting local secrets show "Built-in"/"Stored in console" and that plaintext values never appear in the list/edit UI.

Comment thread src/pages/OrganizationSecretsTab.tsx Outdated
Comment thread src/pages/OrganizationSecretsTab.tsx
Comment thread src/pages/OrganizationSecretsTab.tsx Outdated
Comment thread src/pages/OrganizationSecretsTab.tsx Outdated
@casey-brooks

Copy link
Copy Markdown
Contributor Author

Summary:

  • Tightened storage mode detection to require provider + remote name, surfacing invalid configs with clear labels and edit notices.
  • Built create payloads as discriminated unions (no empty-string placeholders) and updated provider label fallbacks/remote name helper text.
  • Added a minimal local-secret e2e to assert Built-in display and plaintext redaction.

Tests:

noa-lucent
noa-lucent previously approved these changes Apr 22, 2026

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review complete.

  • Storage-mode inference now matches the invariant (remote requires provider+remote name; partial configs surfaced as invalid).
  • Create payloads no longer send empty-string placeholders; irrelevant fields are omitted.
  • Remote name UX updated to match the <mount>/<path>/<key> format.
  • Added e2e coverage for local secrets (and verifies plaintext is not shown).

LGTM.

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Summary:

  • Added Argos screenshots for local secrets list + edit dialog and create dialog storage modes (local/remote).
  • Expanded secrets e2e coverage to capture create/edit UI states without exposing plaintext.

Tests:

noa-lucent
noa-lucent previously approved these changes Apr 22, 2026

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review complete.

New commit adds Argos screenshots for secrets create/edit flows. The screenshots don’t expose plaintext secret values (local value remains empty/masked), and the new create-dialog storage-mode coverage looks good.

LGTM.

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Summary:

  • Replaced storage mode selector with a single Secret provider select (Built-in first) and aligned form logic/validation with built-in vs provider selection.
  • Updated secrets create dialog e2e + Argos coverage for built-in/provider selections without plaintext.

Tests:

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review complete.

Reviewed latest UX change (single provider selector with Built-in first; no separate storage-mode selector).

  • Provider selector cleanly represents storage modes (Built-in vs remote providers) and still enforces the mutual-exclusion invariant in validation + payload construction.
  • Edit flow still prevents mode switching (Built-in <-> remote) via disabled options while allowing remote-only adjustments.
  • Updated e2e coverage reflects the new UX and still asserts no plaintext display.

LGTM.

@vitramir
vitramir merged commit 4131887 into main Apr 22, 2026
2 of 3 checks passed
casey-brooks added a commit that referenced this pull request Apr 22, 2026
* feat(secrets): support local storage

* fix(secrets): tighten storage mode handling

* test(e2e): add secrets argos shots

* feat(secrets): unify provider selection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Console: support Local/Built-in secrets storage mode (in addition to Remote providers)

3 participants