feat: make /allowlist and /redaction pages for cli#77
feat: make /allowlist and /redaction pages for cli#77
Conversation
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 28773915 | Triggered | Generic High Entropy Secret | b7a4332 | fluid-cli/internal/tui/redaction.go | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
There was a problem hiding this comment.
Pull request overview
Adds dedicated TUI screens for managing read-only command allowlists and log redaction patterns, and exposes subcommand restriction metadata for UI display.
Changes:
- Introduces
/allowlistand/redactionTUI flows (models, views, close messages) with persistence via config save. - Removes allowlist/redaction fields from the generic settings screen.
- Exposes subcommand restriction data from
shared/readonlythroughfluid-cli/internal/readonly, with tests.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/readonly/validate.go | Exposes subcommand restriction metadata to callers. |
| fluid-cli/internal/readonly/validate.go | Adapts shared restriction metadata for the CLI (map → sorted slice). |
| fluid-cli/internal/readonly/validate_test.go | Adds coverage for restriction metadata shape/sorting. |
| fluid-cli/internal/tui/messages.go | Adds close messages for new allowlist/redaction screens. |
| fluid-cli/internal/tui/model.go | Wires /allowlist and /redaction commands and delegates to new sub-models. |
| fluid-cli/internal/tui/allowlist.go | New allowlist editor screen (list/add/delete) backed by config. |
| fluid-cli/internal/tui/allowlist_test.go | Tests allowlist editor behaviors. |
| fluid-cli/internal/tui/redaction.go | New redaction pattern editor with live preview backed by config. |
| fluid-cli/internal/tui/redaction_test.go | Tests redaction editor behaviors. |
| fluid-cli/internal/tui/settings.go | Removes allowlist/redaction fields from settings UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result := make(map[string]map[string]bool) | ||
| for k, v := range subcommandRestrictions { | ||
| result[k] = v |
| result := make(map[string][]string, len(readonly.SubcommandRestrictions())) | ||
| for cmd, subs := range readonly.SubcommandRestrictions() { |
| m.addErr = "command already in allowlist" | ||
| return m, nil | ||
| } | ||
| } | ||
| m.userCmds = append(m.userCmds, newCmd) | ||
| sort.Strings(m.userCmds) | ||
| m.cfg.ExtraAllowedCommands = m.userCmds | ||
| m.mode = allowlistModeList |
| case "d": | ||
| if m.mode == allowlistModeList { | ||
| totalBuiltins := len(m.builtinCmds) | ||
| if m.selected >= totalBuiltins { | ||
| idx := m.selected - totalBuiltins | ||
| m.userCmds = append(m.userCmds[:idx], m.userCmds[idx+1:]...) | ||
| m.cfg.ExtraAllowedCommands = m.userCmds | ||
| if m.selected >= len(m.builtinCmds)+len(m.userCmds) && m.selected > 0 { | ||
| m.selected-- | ||
| } | ||
| m.ensureVisible() |
| case " ": | ||
| if m.mode == redactionModeList && m.selected == 0 { | ||
| m.cfg.Redact.Enabled = !m.cfg.Redact.Enabled | ||
| } | ||
| return m, nil | ||
|
|
||
| case "e": | ||
| if m.mode == redactionModeList && m.selected == 0 { | ||
| m.cfg.Redact.Enabled = !m.cfg.Redact.Enabled | ||
| } |
| m.previewBefore = "" | ||
| m.previewAfter = "" | ||
| m.previewErr = "" | ||
| m.selected = 1 + len(m.customPatterns) - 1 |
| if m.addErr != "" { | ||
| b.WriteString(m.styles.error.Render(m.addErr)) | ||
| b.WriteString("\n") | ||
| } | ||
|
|
||
| if m.previewBefore != "" && m.previewErr == "" { | ||
| b.WriteString("\n") | ||
| b.WriteString(m.styles.section.Render("--- Live Preview ---")) | ||
| b.WriteString("\n") | ||
|
|
||
| b.WriteString(m.styles.dimmed.Render("Before: ")) | ||
| b.WriteString(m.previewBefore) | ||
| b.WriteString("\n") | ||
|
|
||
| b.WriteString(m.styles.dimmed.Render("After: ")) | ||
| b.WriteString(m.previewAfter) | ||
| b.WriteString("\n") | ||
| } |
| func renderHighlighted(text string, style lipgloss.Style) string { | ||
| re := regexp.MustCompile(`\b\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\b|` + | ||
| `\b[A-Za-z0-9+/]{20,}=*\b|` + | ||
| `\bAKIA[0-9A-Z]{16}\b|` + | ||
| `-----BEGIN [A-Z ]+PRIVATE KEY-----|` + | ||
| `\b(postgres|mysql|mongodb|redis)://[^\s]+`) | ||
| locs := re.FindAllStringIndex(text, -1) | ||
| if len(locs) == 0 { | ||
| return text | ||
| } | ||
| return renderHighlightedLocs(text, locs, style) | ||
| } |
| // Handle AllowlistCloseMsg | ||
| if closeMsg, ok := msg.(AllowlistCloseMsg); ok { | ||
| m.inAllowlist = false | ||
| m.state = StateIdle | ||
| if closeMsg.Saved { | ||
| m.cfg = m.allowlistModel.GetConfig() | ||
| if err := m.cfg.Save(m.configPath); err != nil { | ||
| m.addSystemMessage(fmt.Sprintf("Failed to save config: %v", err)) | ||
| } else { | ||
| m.addSystemMessage("Allowlist saved.") | ||
| } | ||
| } else { | ||
| m.addSystemMessage("Allowlist cancelled.") | ||
| } | ||
| m.updateViewportContent(false) | ||
| m.textarea.Focus() | ||
| return m, nil | ||
| } | ||
|
|
||
| // Handle RedactionCloseMsg | ||
| if closeMsg, ok := msg.(RedactionCloseMsg); ok { | ||
| m.inRedaction = false | ||
| m.state = StateIdle | ||
| if closeMsg.Saved { | ||
| m.cfg = m.redactionModel.GetConfig() | ||
| if err := m.cfg.Save(m.configPath); err != nil { | ||
| m.addSystemMessage(fmt.Sprintf("Failed to save config: %v", err)) | ||
| } else { | ||
| m.addSystemMessage("Redaction patterns saved.") | ||
| } | ||
| } else { | ||
| m.addSystemMessage("Redaction cancelled.") | ||
| } | ||
| m.updateViewportContent(false) | ||
| m.textarea.Focus() | ||
| return m, nil |
Code ReviewGood overall structure — the two new TUI screens are well-organized, follow the existing Bubbletea patterns, and are backed by reasonable test coverage. A few issues worth addressing before merge: Bugs1. m.selected = 1 + len(m.customPatterns) - 1 // = len(m.customPatterns)Custom patterns start at index m.selected = 1 + len(builtinPatterns) + len(m.customPatterns) - 12. Both handlers do 3. Unused if m.inAllowlist {
var cmd tea.Cmd // declared but never used
allowlistModel, cmd := m.allowlistModel.Update(msg) // shadows it with :=The Design / Correctness Issues4. Immediate config mutation before Ctrl+S In both models, When the user presses Esc (
5. The highlight regex in 6. Other slash commands in the codebase only match with the leading Minor7. 8. Section headers always render even when scrolled past them ( What's Good
|
- Add Kafka capture config and sandbox stub REST handlers with tests - Add kafkastub manager and transport for daemon-side Redpanda simulation - Add daemon Kafka orchestration and agent client integration - Add Redpanda e2e integration test and demo setup/reset scripts - Add customer logo images and LogoMarquee to landing page - Move original product demo to /product route; update scripted demo colors - Fix tickerMockStore to implement new Kafka store methods Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove unused wrapper functions (attachKafkaStubs, kafkaBrokerConfigForRequest, captureStatusToLocal/State) - Fix unchecked error returns on deferred Close calls throughout daemon, kafkastub, and integration tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code ReviewThis PR adds /allowlist and /redaction TUI pages, a Kafka capture/stub system (daemon + API + proto), cloud-init ISO generation, and several daemon improvements. Solid work overall with good test coverage for the TUI components. A few issues worth addressing before merging. Critical: Nil pointer dereference in three REST handlersFile: The pattern below is repeated in cfg, err := s.store.GetKafkaCaptureConfig(r.Context(), id)
if err != nil || cfg.OrgID != org.ID { // PANIC if err != nil (cfg is nil)When Security: AllowAutoTopicCreation in Kafka producerFile: The Kafka writer has Test coverage gaps for REST handlersFile: Only 2 of the 9 handlers have tests (
The project CLAUDE.md states every code change needs tests. Minor: Duplicate newKafkaManager with inconsistent signaturesTwo private functions with the same name exist in different packages:
The inconsistency means the agent package always uses default redaction with no way to override it for testing. Recommend aligning to the injected pattern used in daemon. Minor: StatePaused declared but never usedFile:
Minor: Direct sentinel comparison instead of errors.IsFile: The daemon uses Nit: PR scope vs titleThe PR title says "make /allowlist and /redaction pages for cli" but the diff also includes Kafka capture/stub infrastructure, cloud-init ISO generation, daemon identity keys, proto changes, and web assets. Consider updating the title or splitting into smaller PRs to make review and bisecting easier. |
- Rename all directories: fluid-cli→deer-cli, fluid-daemon→deer-daemon - Update all Go module paths to github.com/aspectrr/deer.sh - Fix proto go_package: fluid.sh/proto/gen/go/fluid/v1;fluidv1 → deer.sh/proto/gen/go/deer/v1;deerv1 - Regenerate all .pb.go files with buf generate (source paths now deer/v1/...) - Integrate Kafka/Redpanda demo from feature/local-kafka-demo branch: - demo/ directory: docker-compose, Logstash pipeline, Kibana setup, weather-producer, prepare-source - scripts/demo/start.sh and stop.sh with all fluid→deer replacements - Root Makefile with demo-start/stop/reset targets - Replace all fluid-demo→deer-demo, fluid-daemon→deer-daemon references in demo scripts - Update CI job names and working-directory paths throughout Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review: feat: make /allowlist and /redaction pages for cliGood work overall — the two new TUI pages are well-structured, follow the existing Bubble Tea patterns in the codebase, and come with solid test coverage. Here are my findings, ordered by severity. Critical1. The TUI in Either wire the new config fields into command validation before merging, or gate the UI behind a "coming soon" notice so users aren't misled. Major2. Duplicate/divergent pattern definitions between
Derive 3. Live preview shows
Minor4. The field exists in 5. In 6.
Nit
SummaryThe blocking issue is #1 — the subcommand configuration is inert at runtime. #2 and #3 will cause user confusion (highlighted patterns don't match real redaction; preview output format differs). The rest are polish items. Happy to discuss any of these in more detail. |
PR Review: feat: make /allowlist and /redaction pages for cliSummaryThis PR adds two new TUI pages ( Bugs / Correctness[allowlist.go] Mutation before save confirmation The allowlist model directly mutates Fix: deep-copy the relevant config fields on init and restore on cancel, or defer all writes to The same issue applies to [redaction.go] Incomplete IPv6 regex The builtin IPv6 pattern Code Quality[config.go] Unclear bool semantics for ExtraAllowedSubcommandsMode map[string]bool `yaml:"extra_allowed_subcommands_mode"`A raw type SubcommandMode string
const (
SubcommandModeAllowlist SubcommandMode = "allowlist"
SubcommandModeBlocklist SubcommandMode = "blocklist"
)
ExtraAllowedSubcommandsMode map[string]SubcommandMode[redaction.go] Builtin patterns defined in TUI layer The 6 builtin redaction patterns are defined inside [redaction.go] API key pattern is vendor-specific
Test CoverageCoverage looks solid overall (15 tests for allowlist, 9 for redaction). A few gaps worth addressing:
PR ScopeThis PR bundles four unrelated changes:
Bundling makes the diff very hard to review and complicates Minor
What's Good
|
- Change default model from claude-opus-4.6 to z-ai/glm-4.7 - Update context window from 1000000 to 203000 tokens - Change compact model to z-ai/glm-4.5-air:free - Update system prompt to position Deer as ELK-stack Consultant 💘 Generated with Crush Assisted-by: GLM-4.7 via Crush <crush@charm.land>
- Rename "Fluid" to "Deer" throughout the codebase - Add comprehensive ELK-stack demo with weather pipeline - Implement chat logging and history tracking - Add macOS-native sandbox scripts and improvements - Update demo scripts and Logstash pipeline configurations - Add Redpanda caching and setup scripts - Update proto definitions with simple_kafka_broker support - Improve TUI and agent interactions - Add multiple data validation and transformation filters 💘 Generated with Crush Assisted-by: GLM-4.7 via Crush <crush@charm.land>
- Update macOS native demo script - Regenerate route tree after changes 💘 Generated with Crush Assisted-by: GLM-4.7 via Crush <crush@charm.land>
Fixes linter errors where defer Close() calls weren't checking return values. 💘 Generated with Crush Assisted-by: GLM-4.7 via Crush <crush@charm.land>
💘 Generated with Crush Assisted-by: GLM-4.7 via Crush <crush@charm.land>
…rand Bug fixes: - Fix missing SimpleKafkaBroker field in CreateSandboxStream (remote.go) - Fix unbounded blocking on network approval channel with context cancellation (agent.go) - Fix SourceVM field missing from error done message (agent.go) - Fix orchestrator panic on short sandbox IDs (orchestrator.go) - Fix unary CreateSandbox ignoring snapshot mode (daemon server.go) - Fix redaction highlight regex diverged from builtin patterns (redaction.go) - Fix keypress leaking between add/detail inputs in allowlist (allowlist.go) - Fix missing validation on kafka config update (kafka_handlers.go) - Fix sandbox ID validation in readiness server (readiness.go) - Fix product.tsx setTimeout not cleaned up on unmount Architecture fixes: - Fix TOCTOU race in gRPC SendAndWait - load stream inside mutex (stream.go) - Fix goroutine leak in kafkastub manager on rapid config updates (manager.go) - Fix time.Sleep not respecting context cancellation in IP discovery (ip.go) - Fix vmHostCache unbounded growth - clear before refresh (helpers.go) - Fix kafka hooks using context.Background() with no timeout (daemon/kafka.go, agent/kafka.go) - Fix chatlog logger swallowing errors + expensive fsync on every write (logger.go) Tests added: - chatlog/logger_test.go: 12 tests covering all methods + concurrent writes - kafka_handlers_test.go: 10 tests for GET/UPDATE/DELETE kafka configs Complete fluid→deer rebrand across all modules: - cloudinit.go: ~80 in-VM references (systemd units, scripts, paths, logs) - sourcevm/manager.go, helpers.go: fluid-readonly → deer-readonly SSH user - readonly/prepare.go, shell.go: fluid-readonly user/shell creation - sshkeys/manager.go: fluid-readonly principal - agent/client.go: temp dir name - config/config.go: variable rename fluidDir → deerDir - snapshotpull/: snapshot naming prefix - kafkastub/transport: group ID prefix - provider/lxc/: user creation and shell commands - Web docs: install commands, APT keys, bridge names, CA paths - SECURITY.md, AGENTS.md, RELEASING.md: all references updated 💘 Generated with Crush Assisted-by: GLM-5.1 via Crush <crush@charm.land>
Description
Type of Change
Checklist
Release Notes
Labels