secrets view (UI to manage wave secrets) #2526
Conversation
WalkthroughThis pull request introduces a complete secret store management feature across the Wave Terminal codebase. It includes CLI commands for secret operations (delete, set, get, list), a backend implementation for storing and retrieving secrets with a new DeleteSecret function, RPC interface updates to support deletion via null values in a pointer map, a frontend React component suite with state management for the secret store UI, integration of the new view type into the block registry, and comprehensive documentation covering both usage (wsh-reference) and architectural guidance for creating new view types. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: unknown linters: 'unusedfunc,unusedparams', run 'golangci-lint help linters' to see the list of supported linters Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
aiprompts/newview.md(1 hunks)cmd/wsh/cmd/wshcmd-secret.go(3 hunks)docs/docs/wsh-reference.mdx(1 hunks)frontend/app/block/block.tsx(2 hunks)frontend/app/block/blockutil.tsx(2 hunks)frontend/app/view/secretstore/secretstore-model.ts(1 hunks)frontend/app/view/secretstore/secretstore.tsx(1 hunks)pkg/secretstore/secretstore.go(1 hunks)pkg/wshrpc/wshclient/wshclient.go(1 hunks)pkg/wshrpc/wshrpctypes.go(1 hunks)pkg/wshrpc/wshserver/wshserver.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
pkg/wshrpc/wshrpctypes.go (2)
pkg/wshrpc/wshclient/wshclient.go (1)
SetSecretsCommand(602-605)frontend/app/store/wshclientapi.ts (1)
SetSecretsCommand(501-503)
pkg/wshrpc/wshserver/wshserver.go (3)
pkg/wshrpc/wshclient/wshclient.go (1)
SetSecretsCommand(602-605)frontend/app/store/wshclientapi.ts (1)
SetSecretsCommand(501-503)pkg/secretstore/secretstore.go (2)
DeleteSecret(236-249)SetSecret(218-234)
cmd/wsh/cmd/wshcmd-secret.go (4)
pkg/wshrpc/wshclient/wshclient.go (1)
SetSecretsCommand(602-605)frontend/app/store/wshclientapi.ts (1)
SetSecretsCommand(501-503)pkg/wshrpc/wshrpctypes.go (1)
RpcOpts(345-351)cmd/wsh/cmd/wshcmd-root.go (1)
WriteStdout(74-76)
frontend/app/view/secretstore/secretstore-model.ts (2)
frontend/app/block/blocktypes.ts (1)
BlockNodeModel(7-12)frontend/app/view/secretstore/secretstore.tsx (1)
SecretStoreView(306-377)
frontend/app/view/secretstore/secretstore.tsx (1)
frontend/app/view/secretstore/secretstore-model.ts (1)
SecretStoreViewModel(13-251)
pkg/wshrpc/wshclient/wshclient.go (1)
frontend/app/store/wshclientapi.ts (1)
SetSecretsCommand(501-503)
frontend/app/block/block.tsx (1)
frontend/app/view/secretstore/secretstore-model.ts (1)
SecretStoreViewModel(13-251)
🪛 Gitleaks (8.28.0)
docs/docs/wsh-reference.mdx
[high] 937-937: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
aiprompts/newview.md
[grammar] ~259-~259: Ensure spelling is correct
Context: ...and navigation - Event handling for webview navigation events - Custom styling ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Docsite
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: merge-gatekeeper
🔇 Additional comments (10)
aiprompts/newview.md (1)
1-524: Excellent documentation for the new view architecture.This comprehensive guide provides clear instructions and real-world examples for implementing new view types in Wave Terminal. The structure follows a logical progression from architecture overview through implementation details, with helpful code snippets and best practices.
The documentation correctly reflects the Model-View architecture pattern used in the codebase and provides accurate examples that align with the new SecretStore implementation added in this PR.
frontend/app/block/blockutil.tsx (2)
33-35: LGTM! Icon mapping added for the new Secret Store view.The icon choice ("key") is appropriate and follows the existing pattern.
61-63: LGTM! Display name mapping added for the new Secret Store view.The display name "Secret Store" is clear and consistent with other view names in the codebase.
frontend/app/block/block.tsx (2)
15-15: LGTM! Import added for the new SecretStore view model.
56-56: LGTM! SecretStore view registered in BlockRegistry.The registration follows the established pattern and enables the "secretstore" view type to be used in blocks.
pkg/wshrpc/wshclient/wshclient.go (1)
602-602: LGTM! Signature updated to match interface change.The function signature correctly changed from
map[string]stringtomap[string]*stringto support deletion semantics. As this is generated code, the change appropriately reflects the interface update.cmd/wsh/cmd/wshcmd-secret.go (3)
49-55: LGTM! Delete command definition added.The command structure is consistent with other secret subcommands.
115-115: LGTM! Set command updated to use pointer-based secret map.The change from
map[string]string{name: value}tomap[string]*string{name: &value}correctly implements the new API signature.
141-159: LGTM! Delete command implementation is correct.The function properly:
- Validates the secret name format (lines 147-149)
- Uses
nilpointer value to represent deletion (line 151)- Includes appropriate error handling
- Tracks activity for telemetry
docs/docs/wsh-reference.mdx (1)
898-984: Documentation is correct—no duplication detected.Verification confirms only one
## secretsection exists at line 901. The comprehensive documentation with practical examples and appropriate security warnings is properly integrated.
| await RpcApi.SetSecretsCommand(TabRpcClient, { [selectedSecret]: null }); | ||
| this.closeSecretView(); | ||
| await this.refreshSecrets(); | ||
| } catch (error) { | ||
| globalStore.set(this.errorMessage, `Failed to delete secret: ${error.message}`); | ||
| } finally { | ||
| globalStore.set(this.isLoading, false); | ||
| } |
There was a problem hiding this comment.
Fix null handling for SetSecretsCommand.
RpcApi.SetSecretsCommand is still typed as {[key: string]: string} (see frontend/app/store/wshclientapi.ts), so this call with { [selectedSecret]: null } fails the TypeScript build (null is not assignable to string). Please update the RPC helper to accept string | null (and propagate that through the client types) before relying on the nil-for-delete contract. Otherwise the UI won't compile.
🤖 Prompt for AI Agents
In frontend/app/view/secretstore/secretstore-model.ts around lines 179 to 186,
the call RpcApi.SetSecretsCommand(TabRpcClient, { [selectedSecret]: null })
fails TypeScript because SetSecretsCommand expects values of type string; update
the RPC helper and client types to accept string | null: change the
SetSecretsCommand payload type from {[key: string]: string} to {[key: string]:
string | null} in frontend/app/store/wshclientapi.ts (and any generated or
propagated client typings), ensure the RPC implementation and any serialization
handle null as the delete sentinel, and then update any callers or type imports
accordingly so the UI can pass null for deletions without build errors.
| func DeleteSecret(name string) error { | ||
| if name == "" { | ||
| return fmt.Errorf("secret name cannot be empty") | ||
| } | ||
| if err := initSecretStore(); err != nil { | ||
| return err | ||
| } | ||
| lock.Lock() | ||
| defer lock.Unlock() | ||
|
|
||
| delete(secrets, name) | ||
| requestWrite() | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add name format validation to DeleteSecret for consistency.
The DeleteSecret function validates that the name is not empty but doesn't validate the name format using secretNameRegexp, unlike SetSecret (lines 222-224). This inconsistency could allow deletion attempts with invalid names.
Apply this diff to add format validation:
func DeleteSecret(name string) error {
if name == "" {
return fmt.Errorf("secret name cannot be empty")
}
+ if !secretNameRegexp.MatchString(name) {
+ return fmt.Errorf("secret name must start with a letter and contain only letters, numbers, and underscores")
+ }
if err := initSecretStore(); err != nil {
return err
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func DeleteSecret(name string) error { | |
| if name == "" { | |
| return fmt.Errorf("secret name cannot be empty") | |
| } | |
| if err := initSecretStore(); err != nil { | |
| return err | |
| } | |
| lock.Lock() | |
| defer lock.Unlock() | |
| delete(secrets, name) | |
| requestWrite() | |
| return nil | |
| } | |
| func DeleteSecret(name string) error { | |
| if name == "" { | |
| return fmt.Errorf("secret name cannot be empty") | |
| } | |
| if !secretNameRegexp.MatchString(name) { | |
| return fmt.Errorf("secret name must start with a letter and contain only letters, numbers, and underscores") | |
| } | |
| if err := initSecretStore(); err != nil { | |
| return err | |
| } | |
| lock.Lock() | |
| defer lock.Unlock() | |
| delete(secrets, name) | |
| requestWrite() | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In pkg/secretstore/secretstore.go around lines 236 to 249, DeleteSecret
currently only checks for empty name but should also validate the name against
secretNameRegexp like SetSecret does; update DeleteSecret to return an error
when name does not match secretNameRegexp (e.g., fmt.Errorf("invalid secret
name")) before calling initSecretStore(), preserving existing locking and
requestWrite behavior so deletion only proceeds for valid secret names.
| GetSecretsCommand(ctx context.Context, names []string) (map[string]string, error) | ||
| GetSecretsNamesCommand(ctx context.Context) ([]string, error) | ||
| SetSecretsCommand(ctx context.Context, secrets map[string]string) error | ||
| SetSecretsCommand(ctx context.Context, secrets map[string]*string) error |
There was a problem hiding this comment.
🧩 Analysis chain
Verify frontend TypeScript API compatibility with pointer-based secrets.
The signature change from map[string]string to map[string]*string enables deletion semantics (nil values), but the frontend TypeScript API in frontend/app/store/wshclientapi.ts still shows {[key: string]: string}.
Ensure the frontend TypeScript types are updated to support nullable values (e.g., {[key: string]: string | null}) to properly represent deletions.
🏁 Script executed:
#!/bin/bash
# Check frontend TypeScript API definition for SetSecretsCommand
rg -A 3 "SetSecretsCommand.*data.*key.*string" frontend/app/store/wshclientapi.tsLength of output: 171
</parameter>
</invoke>
---
🏁 Script executed:
```shell
#!/bin/bash
# Verify the actual Go signature at the specified location
rg -n -B2 -A5 "SetSecretsCommand" pkg/wshrpc/wshrpctypes.go
Length of output: 462
🏁 Script executed:
#!/bin/bash
# Check for JSON marshaling or RPC serialization logic around secrets
rg -n -B5 -A10 "setsecrets|SetSecrets" pkg/wshrpc/ --type=goLength of output: 4749
🏁 Script executed:
#!/bin/bash
# Check if there's any special handling in the RPC client call
rg -n -B3 -A10 "wshRpcCall" frontend/app/store/wshclientapi.tsLength of output: 28201
🏁 Script executed:
#!/bin/bash
# Look for any type definitions or interfaces related to secrets in TypeScript
rg -n "string.*null|null.*string|\\?:" frontend/app/store/wshclientapi.ts | head -20Length of output: 2101
🏁 Script executed:
#!/bin/bash
# Check the RPC handler implementation on backend
rg -n -A20 "func.*SetSecretsCommand" pkg/wshrpc/ --type=goLength of output: 3199
🏁 Script executed:
#!/bin/bash
# Check if there's middleware or serialization that handles null values
rg -n -B3 -A10 "json\\.Marshal|json\\.Unmarshal" pkg/wshrpc/ --type=go | head -40Length of output: 1844
🏁 Script executed:
#!/bin/bash
# Check wshclientapi.ts for how data is serialized before sending
cat frontend/app/store/wshclientapi.ts | head -50Length of output: 1988
🏁 Script executed:
#!/bin/bash
# Search for other commands with nullable types in TypeScript to understand pattern
rg -n "\\[key: string\\]:" frontend/app/store/wshclientapi.tsLength of output: 331
🏁 Script executed:
#!/bin/bash
# Check if there are tests showing intended usage with null/undefined
rg -n -B2 -A5 "SetSecretsCommand.*null|SetSecretsCommand.*undefined" --type=tsLength of output: 756
🏁 Script executed:
#!/bin/bash
# Look for any type guards or transformation before RPC call
rg -n -B5 -A10 "setsecrets" frontend/app/store/ --type=tsLength of output: 1486
</parameter>
</invoke>
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the code generator mentioned in wshclientapi.ts
fd -name "*generatets*" -o -name "*generate*ts*"
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check the code generator to understand pointer-to-null conversion
rg -n "\\*string|string\\|null" cmd/generate/ --type=go | head -30Length of output: 53
🏁 Script executed:
#!/bin/bash
# Look for how the generator handles pointer types
rg -n -B5 -A10 "pointer|\\*" cmd/generate/ --type=go | head -50Length of output: 53
Update TypeScript API signature to support nullable values for secret deletion.
The Go backend was changed to accept map[string]*string, enabling nil values for deletion semantics. The TypeScript type at frontend/app/store/wshclientapi.ts:501 still declares {[key: string]: string}, which does not allow null values. However, the frontend code at frontend/app/view/secretstore/secretstore-model.ts:179 already passes null for deletion: SetSecretsCommand(TabRpcClient, { [selectedSecret]: null }).
Change the TypeScript type to {[key: string]: string | null} to match the Go signature and maintain type safety.
🤖 Prompt for AI Agents
In frontend/app/store/wshclientapi.ts around line 501, the TypeScript RPC
signature currently uses {[key: string]: string} but the Go backend accepts
map[string]*string (nil allowed). Update the type to {[key: string]: string |
null} so callers can pass null for secret deletion; adjust the exported
interface/type declaration and any related function signatures to use the new
union type to keep typings consistent with frontend usage (e.g.
secretstore-model.ts).
No description provided.