Fix/secretnav#136
Conversation
Adds a new `infisical tui` command that launches an interactive terminal UI powered by Bubble Tea and Gemini AI. Users can manage secrets via natural language prompts instead of memorizing CLI flags. Features: - AI prompt bar (Gemini 3.1) for NL -> CLI command translation - Secret browser with keyboard navigation and search/filter - Secret detail view with reveal/mask toggle - Create, update, and delete secrets - Environment switcher (dev/staging/prod) - Command preview with safety confirmation for destructive ops - Production environment warning banners - Help modal with keyboard shortcut reference - Auth detection and setup guidance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dit log Secret values are now redacted before reaching the Gemini API via a sanitize → placeholder → hydrate pipeline. Commands are validated against an allowlist and checked for shell injection before execution. All AI- generated commands are logged to ~/.itui/audit.log. Adds standalone `itui` binary entry point (cmd/itui/main.go). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR adds ITUI (Infisical Terminal UI), an AI-powered terminal interface that uses Google Gemini to translate natural language prompts into Infisical CLI commands. The feature includes a Bubble Tea-based TUI with secret browsing, environment switching, AI prompt bar, and command execution with an audit log.
Confidence Score: 1/5
Important Files Changed
Last reviewed commit: 128ffd8 |
| case components.SecretCreatedMsg: | ||
| return m, m.executeCommand( | ||
| fmt.Sprintf("infisical secrets set %s=%s --env=%s --path=%s", | ||
| msg.Key, msg.Value, m.ctx.Environment, m.ctx.Path), | ||
| ) |
There was a problem hiding this comment.
Command injection via unsanitized form input
User-provided msg.Key and msg.Value are directly interpolated into a command string via fmt.Sprintf without any escaping or quoting. A secret value containing spaces (e.g., hello world) would be split into separate arguments by splitArgs, truncating the value. More critically, a value like foo --env=prod would inject additional flags, potentially targeting a different environment than intended.
Since the executor uses exec.Command (not a shell), shell metacharacters aren't directly exploitable, but argument injection is. The value should be passed as a properly quoted argument, or better yet, the Executor.Run method should be called directly with properly separated arguments instead of constructing a raw command string:
| case components.SecretCreatedMsg: | |
| return m, m.executeCommand( | |
| fmt.Sprintf("infisical secrets set %s=%s --env=%s --path=%s", | |
| msg.Key, msg.Value, m.ctx.Environment, m.ctx.Path), | |
| ) | |
| case components.SecretCreatedMsg: | |
| return m, m.executeCommand( | |
| fmt.Sprintf("infisical secrets set %s='%s' --env=%s --path=%s", | |
| msg.Key, strings.ReplaceAll(msg.Value, "'", "'\\''"), m.ctx.Environment, m.ctx.Path), | |
| ) |
| "secrets delete": true, | ||
| "secrets folders": true, | ||
| "export": true, | ||
| "run": true, |
There was a problem hiding this comment.
run subcommand in allowlist enables arbitrary command execution
The run command passes everything after -- to the system as a subprocess (e.g., infisical run --env=dev -- rm -rf /). Since ValidateCommand only checks the subcommand prefix and shell metacharacters, but not the payload after --, the AI model could be prompt-injected into generating a run command with an arbitrary and dangerous subprocess.
Consider either:
- Removing
runfrom the allowlist entirely, or - Adding validation that specifically inspects and restricts what comes after
--inruncommands
| func NewAIClient(apiKey string) *AIClient { | ||
| client, err := genai.NewClient(context.Background(), &genai.ClientConfig{ | ||
| APIKey: apiKey, | ||
| Backend: genai.BackendGeminiAPI, | ||
| }) | ||
| if err != nil { | ||
| return &AIClient{model: "gemini-2.5-flash"} | ||
| } | ||
|
|
||
| return &AIClient{ | ||
| client: client, | ||
| model: "gemini-2.5-flash", | ||
| } |
There was a problem hiding this comment.
Silent error swallowing returns broken client
When genai.NewClient fails, the error is silently discarded and a &AIClient{model: "gemini-2.5-flash"} is returned with a nil client field. The caller (NewModel in itui.go) stores this and only discovers the problem later when Translate is called. This makes debugging harder — the user won't know their API key is invalid or that the client failed to initialize.
Consider returning the error or logging a warning so the user knows initialization failed:
func NewAIClient(apiKey string) (*AIClient, error) {
client, err := genai.NewClient(context.Background(), &genai.ClientConfig{
APIKey: apiKey,
Backend: genai.BackendGeminiAPI,
})
if err != nil {
return nil, fmt.Errorf("failed to create AI client: %w", err)
}
return &AIClient{client: client, model: "gemini-2.5-flash"}, nil
}| if item, ok := m.secretBrowser.SelectedItem(); ok { | ||
| isProd := strings.EqualFold(m.ctx.Environment, "prod") || strings.EqualFold(m.ctx.Environment, "production") | ||
| cmd := fmt.Sprintf("infisical secrets delete %s --env=%s --path=%s --type=shared", item.KeyName, m.ctx.Environment, m.ctx.Path) | ||
| m.confirmDialog.Show(cmd, fmt.Sprintf("Delete secret '%s' from %s?", item.KeyName, m.ctx.Environment), true, isProd) |
There was a problem hiding this comment.
Delete command also vulnerable to argument injection
Similar to the SecretCreatedMsg handler, item.KeyName is interpolated directly into the command string. If a secret key name contains spaces or flag-like patterns (unlikely but possible depending on the backend), this could produce malformed or unintended commands. Consider using the Executor.Run method directly with properly separated args for all programmatically-constructed commands.
| if isWriteOp { | ||
| // Look for "to VALUE" pattern (most common) | ||
| toPattern := regexp.MustCompile(`(?i)\bto\s+(\S+(?:\S*://\S+|\S+))`) |
There was a problem hiding this comment.
Regex compiled inside function on every call
toPattern and eqPattern are compiled with regexp.MustCompile inside SanitizePrompt, meaning they are re-compiled every time the function is called. Since these are static patterns, they should be compiled once as package-level variables (similar to valuePatterns at the top of the file) for better performance.
| if isWriteOp { | |
| // Look for "to VALUE" pattern (most common) | |
| toPattern := regexp.MustCompile(`(?i)\bto\s+(\S+(?:\S*://\S+|\S+))`) | |
| if isWriteOp { | |
| // Look for "to VALUE" pattern (most common) | |
| if matches := writeToPattern.FindStringSubmatchIndex(sanitized); matches != nil { |
Where writeToPattern and writeEqPattern are declared as package-level vars alongside valuePatterns.
Additional Comments (1)
Two compiled binary files ( |
Description 📣
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets