Skip to content

feat: make sensitive fields secret-picker-only #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TerrifiedBug merged 3 commits into from
Mar 6, 2026
Merged

feat: make sensitive fields secret-picker-only #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b199eb
feat: make sensitive fields secret-picker-only, remove plaintext input
TerrifiedBug Mar 6, 2026
8b309f9
fix: address code review feedback on SecretPickerInput
TerrifiedBug Mar 6, 2026
67b725a
fix: use autoComplete="new-password" instead of "off"
TerrifiedBug Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/components/config-forms/field-renderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,6 @@ export function FieldRenderer({
<SecretPickerInput
value={(value as string) ?? ""}
onChange={(v) => onChange(v)}
placeholder={placeholder}
/>
) : isCertFile ? (
<CertPickerInput
Expand Down
232 changes: 176 additions & 56 deletions src/components/config-forms/secret-picker-input.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
"use client";

import { useState } from "react";
import { useQuery } from "@tanstack/react-query";
import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query";
import { useTRPC } from "@/trpc/client";
import { useEnvironmentStore } from "@/stores/environment-store";
import { Lock, X } from "lucide-react";
import { Lock, X, AlertTriangle, Plus } from "lucide-react";
import { toast } from "sonner";

import { Input } from "@/components/ui/input";
import { Button } from "@/components/ui/button";
import { Badge } from "@/components/ui/badge";
import { Label } from "@/components/ui/label";
import {
Popover,
PopoverContent,
Expand All @@ -29,12 +31,15 @@ export function makeSecretRef(name: string): string {
interface SecretPickerInputProps {
value: string;
onChange: (value: string) => void;
placeholder?: string;
}

export function SecretPickerInput({ value, onChange, placeholder }: SecretPickerInputProps) {
export function SecretPickerInput({ value, onChange }: SecretPickerInputProps) {
const [popoverOpen, setPopoverOpen] = useState(false);
const [showCreate, setShowCreate] = useState(false);
const [newName, setNewName] = useState("");
const [newValue, setNewValue] = useState("");
const trpc = useTRPC();
const queryClient = useQueryClient();
const environmentId = useEnvironmentStore((s) => s.selectedEnvironmentId);

const secretsQuery = useQuery(
Expand All @@ -45,9 +50,38 @@ export function SecretPickerInput({ value, onChange, placeholder }: SecretPicker
);
const secrets = secretsQuery.data ?? [];

const createMutation = useMutation(
trpc.secret.create.mutationOptions({
onSuccess: (created) => {
queryClient.invalidateQueries({ queryKey: trpc.secret.list.queryKey({ environmentId: environmentId! }) });
onChange(makeSecretRef(created.name));
setPopoverOpen(false);
resetCreateForm();
toast.success(`Secret "${created.name}" created`);
},
onError: (err) => toast.error(err.message),
})
);

function resetCreateForm() {
setShowCreate(false);
setNewName("");
setNewValue("");
}

function handleCreate() {
if (!environmentId || !newName.trim() || !newValue.trim()) return;
createMutation.mutate({
environmentId,
name: newName.trim(),
value: newValue.trim(),
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

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

Silent whitespace stripping on secret value

newValue.trim() is called before sending the secret value to the server. While accidental leading/trailing whitespace is common in pasted API keys, there are secret formats (e.g. some base64-padded tokens or generated passwords) where the stored value must be byte-exact. Silently trimming means a user who pastes such a value will get a secret that doesn't work, with no indication of why.

The name field trimming (same line) is unambiguously correct — names should never have surrounding whitespace. The value trim is the debatable one.

Consider trimming name only, or at minimum alerting the user if trimming actually changed their input:

Suggested change
value: newValue.trim(),
name: newName.trim(),
value: newValue,
Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/components/config-forms/secret-picker-input.tsx
Line: 77

Comment:
**Silent whitespace stripping on secret value**

`newValue.trim()` is called before sending the secret value to the server. While accidental leading/trailing whitespace is common in pasted API keys, there are secret formats (e.g. some base64-padded tokens or generated passwords) where the stored value must be byte-exact. Silently trimming means a user who pastes such a value will get a secret that doesn't work, with no indication of why.

The `name` field trimming (same line) is unambiguously correct — names should never have surrounding whitespace. The value trim is the debatable one.

Consider trimming `name` only, or at minimum alerting the user if trimming actually changed their input:

```suggestion
      name: newName.trim(),
      value: newValue,
```

How can I resolve this? If you propose a fix, please make it concise.

});
}

const secretRef = typeof value === "string" ? parseSecretRef(value) : null;
const isPlaintextLegacy = !!value && !secretRef;

// When a secret reference is active, show a badge instead of the input
// State 1: Secret reference selected — show badge
if (secretRef) {
return (
<div className="flex items-center gap-2">
Expand All @@ -69,59 +103,145 @@ export function SecretPickerInput({ value, onChange, placeholder }: SecretPicker
);
}

return (
<div className="flex items-center gap-1">
<Input
type="password"
value={value}
onChange={(e) => onChange(e.target.value)}
placeholder={placeholder}
className="flex-1"
/>
{environmentId && (
<Popover open={popoverOpen} onOpenChange={setPopoverOpen}>
<PopoverTrigger asChild>
// Shared popover content
const pickerPopover = environmentId ? (
<Popover open={popoverOpen} onOpenChange={(open) => { setPopoverOpen(open); if (!open) resetCreateForm(); }}>
<PopoverTrigger asChild>
{isPlaintextLegacy ? (
<Button type="button" variant="outline" size="sm">
<Lock className="h-3.5 w-3.5 mr-2" />
Select secret to replace
</Button>
) : (
<Button type="button" variant="outline" size="sm" className="text-muted-foreground">
<Lock className="h-3.5 w-3.5 mr-2" />
Select secret...
</Button>
)}
</PopoverTrigger>
<PopoverContent align="start" className="w-72 p-0">
<div className="p-3 pb-2">
<p className="text-sm font-medium">Select Secret</p>
<p className="text-xs text-muted-foreground">
Choose a secret from this environment
</p>
</div>
<div className="max-h-48 overflow-y-auto border-t">
{secrets.length === 0 && !secretsQuery.isLoading ? (
<p className="p-3 text-xs text-muted-foreground text-center">
No secrets yet
</p>
) : secretsQuery.isLoading ? (
<p className="p-3 text-xs text-muted-foreground text-center">
Loading...
</p>
Comment on lines +130 to +137
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

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

Brief "No secrets yet" flash on first open

The condition order can show "No secrets yet" for a frame before "Loading..." when the popover first opens. When popoverOpen flips to true, the query's enabled flag transitions from false → true. In the same render where PopoverContent appears, secretsQuery.isLoading may still be false (the fetch hasn't started yet), so secrets.length === 0 && !secretsQuery.isLoading evaluates to true and "No secrets yet" briefly flashes before the loading state kicks in.

A more robust guard is to also check whether the query is enabled but hasn't yet returned data (secretsQuery.isPending):

Suggested change
{secrets.length === 0 && !secretsQuery.isLoading ? (
<p className="p-3 text-xs text-muted-foreground text-center">
No secrets yet
</p>
) : secretsQuery.isLoading ? (
<p className="p-3 text-xs text-muted-foreground text-center">
Loading...
</p>
{secretsQuery.isPending ? (
<p className="p-3 text-xs text-muted-foreground text-center">
Loading...
</p>
) : secrets.length === 0 ? (
<p className="p-3 text-xs text-muted-foreground text-center">
No secrets yet
</p>
) : (
Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/components/config-forms/secret-picker-input.tsx
Line: 130-137

Comment:
**Brief "No secrets yet" flash on first open**

The condition order can show "No secrets yet" for a frame before "Loading..." when the popover first opens. When `popoverOpen` flips to `true`, the query's `enabled` flag transitions from `false` → `true`. In the same render where `PopoverContent` appears, `secretsQuery.isLoading` may still be `false` (the fetch hasn't started yet), so `secrets.length === 0 && !secretsQuery.isLoading` evaluates to `true` and "No secrets yet" briefly flashes before the loading state kicks in.

A more robust guard is to also check whether the query is enabled but hasn't yet returned data (`secretsQuery.isPending`):

```suggestion
          {secretsQuery.isPending ? (
            <p className="p-3 text-xs text-muted-foreground text-center">
              Loading...
            </p>
          ) : secrets.length === 0 ? (
            <p className="p-3 text-xs text-muted-foreground text-center">
              No secrets yet
            </p>
          ) : (
```

How can I resolve this? If you propose a fix, please make it concise.

) : (
secrets.map((secret) => (
<button
key={secret.id}
type="button"
className="w-full text-left px-3 py-2 text-sm hover:bg-accent transition-colors font-mono"
onClick={() => {
onChange(makeSecretRef(secret.name));
setPopoverOpen(false);
resetCreateForm();
}}
>
{secret.name}
</button>
))
)}
</div>
<div className="border-t p-2">
{showCreate ? (
<div className="space-y-2">
<div className="space-y-1">
<Label className="text-xs">Name</Label>
<Input
value={newName}
onChange={(e) => setNewName(e.target.value)}
placeholder="MY_SECRET_NAME"
className="h-8 text-xs font-mono"
/>
</div>
<div className="space-y-1">
<Label className="text-xs">Value</Label>
<Input
type="password"
autoComplete="new-password"
value={newValue}
onChange={(e) => setNewValue(e.target.value)}
placeholder="secret value"
className="h-8 text-xs"
/>
</div>
<div className="flex gap-2">
<Button
type="button"
size="sm"
className="h-7 text-xs flex-1"
disabled={!newName.trim() || !newValue.trim() || createMutation.isPending}
onClick={handleCreate}
>
{createMutation.isPending ? "Creating..." : "Create & Use"}
</Button>
<Button
type="button"
variant="ghost"
size="sm"
className="h-7 text-xs"
onClick={resetCreateForm}
>
Cancel
</Button>
</div>
</div>
) : (
<Button
type="button"
variant="outline"
size="icon"
className="h-9 w-9 shrink-0"
aria-label="Use a secret"
variant="ghost"
size="sm"
className="w-full h-8 text-xs"
onClick={() => setShowCreate(true)}
>
<Lock className="h-3.5 w-3.5" />
<Plus className="h-3.5 w-3.5 mr-1.5" />
Create new secret
</Button>
</PopoverTrigger>
<PopoverContent align="end" className="w-64 p-0">
<div className="p-3 pb-2">
<p className="text-sm font-medium">Use Secret</p>
<p className="text-xs text-muted-foreground">
Select a secret from this environment
</p>
</div>
<div className="max-h-48 overflow-y-auto border-t">
{secrets.length === 0 ? (
<p className="p-3 text-xs text-muted-foreground text-center">
{secretsQuery.isLoading ? "Loading..." : "No secrets available"}
</p>
) : (
secrets.map((secret) => (
<button
key={secret.id}
type="button"
className="w-full text-left px-3 py-2 text-sm hover:bg-accent transition-colors font-mono"
onClick={() => {
onChange(makeSecretRef(secret.name));
setPopoverOpen(false);
}}
>
{secret.name}
</button>
))
)}
</div>
</PopoverContent>
</Popover>
)}
</div>
);
)}
</div>
</PopoverContent>
</Popover>
) : null;

// State 2: Legacy plaintext value — show warning + picker
if (isPlaintextLegacy) {
return (
<div className="space-y-2">
<div className="flex items-center gap-2">
<Badge variant="destructive" className="flex items-center gap-1.5 px-3 py-1.5">
<AlertTriangle className="h-3 w-3" />
<span className="text-xs">Plaintext value — select a secret to replace</span>
</Badge>
<Button
type="button"
variant="ghost"
size="icon"
className="h-7 w-7"
aria-label="Clear value"
onClick={() => onChange("")}
>
<X className="h-3.5 w-3.5" />
</Button>
</div>
{pickerPopover ?? (
<p className="text-xs text-muted-foreground">
Select an environment to choose a replacement secret
</p>
)}
</div>
);
}

// State 3: No value — show picker button
return pickerPopover ?? <p className="text-xs text-muted-foreground">No environment selected</p>;
}
Loading