feat(desktop): encrypted Agent Provider settings panel#576
Closed
tlongwell-block wants to merge 3 commits into
Closed
feat(desktop): encrypted Agent Provider settings panel#576tlongwell-block wants to merge 3 commits into
tlongwell-block wants to merge 3 commits into
Conversation
Adds a Settings → Agent Provider panel to the desktop GUI for configuring the `sprout-agent` provider, model, API key, and behavior knobs. Settings are encrypted at rest with NIP-44 self-encryption (the user's own nostr key) and injected into the sprout-agent child's env at spawn time. The panel also gains automatic provider-URL detection based on the API key format the user pastes. ## Backend (`desktop/src-tauri/src/commands/agent_provider_settings/`) - `mod.rs` — IPC types + plaintext `StoredSettings` (Drop zeroizes `api_key`; no Debug derive on input/stored). v2 envelope binds the plaintext to its owner pubkey for rollback / envelope-swap protection. - `storage.rs` — envelope read/write, NIP-44 encrypt/decrypt with `Zeroizing<String>` for plaintext, atomic-rename writes, file-size cap, `normalize_origin` (rejects non-loopback `http://`, userinfo, query, fragment), `validate_input` (provider whitelist, control-char rejection, size caps for key/model/base_url/system_prompt, positive- int knobs). `validate_stored` mirrors the same rules on the decrypted blob at spawn time — fails closed on a rolled-back pre-validation envelope so a redirected `http://api.example.com/v1` cannot escape. - `commands.rs` — `get_*`, `save_*`, `delete_*`, `get_*_env_presence` Tauri commands. The save command trims whitespace + zeroes the input api_key before validation can early-return. - `spawn.rs` — `LoadForSpawn` enum + `EnvPairs` newtype whose Drop zeroizes every value buffer. `apply_to_command` hands each env pair to `Command::env` by reference, zeroizing the local buffer after. Spawn policy: Ok → strip OWNED_AGENT_ENV_VARS + ACP-level vars then inject; None → no-op; IdentityMismatch / Error → fail closed (strip inherited owned vars, inject nothing). - `tests.rs` — round-trip envelope I/O, identity-rotation, save-time validation (oversized prompt, zero timeouts, tiny history bytes, unknown provider, control chars, oversized fields, api-key whitespace trim, owner_pubkey v2), `apply_to_command` × `LoadForSpawn` matrix (Ok/None/IdentityMismatch-fails-closed/Error-fails-closed, openai dialect), `stored_to_env_pairs` for each dialect, R7 `validate_stored` coverage (non-loopback http, control chars in key/model/base_url, userinfo/query in base_url, unknown provider, oversized prompt, empty-key + loopback local accepted). ## Runtime integration (`desktop/src-tauri/src/managed_agents/runtime.rs`) - `build_agent_command` calls `agent_provider_settings::apply_to_command` exactly when the harness is `sprout-agent`. ACP-level vars (SPROUT_AGENT_PROVIDER etc.) are stripped from inherited parent env before injection so a stale shell `ANTHROPIC_API_KEY` never shadows saved settings. `respond-to` gate env (`SPROUT_ACP_RESPOND_TO[_ALLOWLIST]`) threads through with the new `owner_hex: Option<&str>` parameter (origin/main merge). ## Frontend - `lib/detectProvider.ts` — pure key-format detector. Recognizes Anthropic, OpenAI (legacy/proj/svcacct via fixed infix), OpenRouter, Groq, xAI, Cerebras, Together, Perplexity, Fireworks (medium), bare sk- → DeepSeek (low + ambiguity-aware), plus localhost/127.0.0.1 patterns for Ollama / vLLM / llama.cpp. Includes ADMIN_ONLY_PROVIDER_ID sentinel for `sk-ant-admin01-` which we explicitly refuse to save. Key format wins over a prefilled default base URL; an explicit non- default base URL wins back for medium-confidence keys (e.g. Fireworks + api.openai.com host). Fixture strings construct the OpenAI infix via concat so GitHub's secret scanner doesn't regex-match an inline OpenAI-shaped service-account/project key (`detectProvider.test.mjs`, `settings-agent-provider.spec.ts`). - `lib/providerCatalog.ts` — declarative catalog (id, label, dialect, isLocal, default model + base URL, key-shape hint). Drives the picker, the auto-fill on detection, the local-provider placeholder enforcement, and the per-provider model field default. - `lib/agentProviderFormState.ts` — FormState shape + reducers. `applyProviderSwitch` is the single source of truth for what gets reset on a provider change (model when empty or still previous default; baseUrl when new provider has a default OR user hasn't edited it; clears previous default host for null-default providers; drops apiKey on switch TO a local provider). Used by both the manual picker and the auto-detect effect so the policy can't drift. - `lib/agentProviderSettingsApi.ts` + `hooks/useAgentProviderSettings.ts` — typed IPC wrappers + React-Query hooks (load / save / delete / envPresence). - `ui/AgentProviderSettingsCard.tsx` — the panel itself. Empty state with shell-env hint, identity-rotation banner, load-error banner, detected-provider badge, reveal/hide toggle, advanced section, inline provider-change warning, confirm-clear dialog. On save success the plaintext is wiped from form state + reveal toggles off, independent of any React-Query refresh effect (covers the structural- sharing identical-redacted-view edge case). - `ui/AgentProviderAdvancedFields.tsx`, `AgentProviderBanners.tsx`, `AgentProviderClearDialog.tsx` — split components. ## Per-agent dialog (sprout-agent special case) - `agents/ui/CreateAgentDialogSections.tsx` — Model + System prompt inputs are hidden for sprout-agent paths (those are owned globally). A note line points users to Settings → Agent Provider. - `agents/ui/EditAgentDialog.tsx` — passes `selectedProviderId="custom"` to the shared runtime fields so the agent-command input stays editable for existing rows; the system-prompt hide still resolves via `isSproutAgentPath`'s `agentCommand` arm. - `agents/ui/ManagedAgentRow.tsx` — "Model managed by Sprout settings" link is a span with role="button" + stopPropagation (was a nested `<button>` inside the row button — both invalid HTML and double- triggering). - `agents/lib/resolveAcpProviderId.ts` — TS / Rust alignment for inline-args resolution (Rust `known_acp_provider` strips args; TS now matches). ## Tests - Rust: 328 tests passing (R7 added 7; full agent_provider_settings suite at 44/44). - TS / node-test: 55 cases for the form-state reducer + provider detector. - Playwright integration: 12 settings-agent-provider scenarios (empty state + detection + save round-trip, identity-rotation banner, provider-change-warning, key-format-beats-prefilled-baseUrl, load- error banner, clear flow + Escape cancel, rotation banner a11y, local-provider switch with saved key, manual switch reset, detected- provider model reset, post-save key-input clear). ## just ci summary - Rust: 321/321 + 7 new validate_stored tests - Mobile: 336/336 - Desktop / web: format + biome + file-size + ts-check all green - Playwright integration agents + settings-agent-provider: 19/19 green ## Codex review history - Reviews #1, #2, #3, #5, #6 surfaced and fixed: non-loopback HTTP at save AND spawn, identity-mismatch fails closed, local-provider key leak prevention, inline-args resolver alignment, Zeroize on api_key before early-return, no Debug derive, control-char / length caps + trim, owner_pubkey v2 envelope, local-provider switch unblock, EditAgentDialog command field, model reset on detection switch, nested-button fix, `applyProviderSwitch` reducer extraction. - Review #7 (P2-UI + P2-Rust): clear form.apiKey + revealKey on save success; validate decrypted settings at spawn time. - Review #8: 9/10, no blocking findings. Signed-off-by: Tyler Longwell <109685178+tlongwell-block@users.noreply.github.com> Co-authored-by: Dawn <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@sprout-oss.stage.blox.sqprod.co>
8188cb7 to
3dceb02
Compare
Builds on the v1 encrypted Agent Provider settings panel by replacing the
single-config record with a multi-profile wrapper. Users can configure any
number of named provider profiles (Anthropic work, OpenAI personal, local
Ollama, etc.), mark one as default, and pin a specific profile per
sprout-agent at create/edit time. The default is what spawns when an agent
has no pin.
## Plaintext schema (v3)
`ProfilesPlaintext { schema_version: 3, owner_pubkey, default_profile_id,
profiles: [NamedProfile { id, label, created_at, updated_at, settings }] }`
The per-profile `settings` slot is exactly the v1/v2 `StoredSettings` shape
(api_key Zeroized on Drop, no Debug derive). Legacy v1/v2 plaintexts
migrate idempotently on first read: wrap as the sole profile, label
"Default", mark default, stamp `owner_pubkey` with the envelope pubkey.
A mismatched legacy owner_pubkey aborts as IdentityMismatch — we refuse to
launder a stale-identity plaintext into a clean v3 envelope.
The wrapper-level `owner_pubkey` is the authoritative identity check.
Per-profile `settings.owner_pubkey` continues to drive v2 defense-in-depth
at the spawn path (`load_for_spawn`).
## Capacity
No explicit count cap on `profiles.len()`. The real bound is the JSON-
serialized `ProfilesPlaintext` fitting under `MAX_PLAINTEXT_BYTES = 48 KiB`
(headroom under NIP-44 v2's 65535-byte plaintext limit). Measured:
- 1 typical profile (sk-ant key + Anthropic defaults, no system_prompt): 855 B
- 72 typical profiles fit; 73 → save rejected with the size-cap error
## UI
- **Settings → Agent Provider** card now lists profiles with per-row
Set-default / Edit / Delete. Two-click destructive arm on row Delete.
- **Add / Edit profile dialog** (`AgentProviderProfileDialog`) carries
label + provider + key + model + base URL + advanced knobs. Auto-detect
from key format (Anthropic, OpenAI, OpenRouter, Groq, xAI, Cerebras,
Together, Perplexity, Fireworks, bare sk- → DeepSeek, plus localhost
patterns for Ollama / vLLM / llama.cpp) drives provider + base URL +
default model in one synchronized form-state reducer.
- **In-dialog Delete** mirrors the row's two-click arm; disarms on cancel
or profile switch.
- **CreateAgentDialog / EditAgentDialog** show a Sprout Profile picker
for sprout-agent rows only. Picker uses a PENDING sentinel so the
`<select>` stays controlled during load / identity-mismatch. Save is
blocked when sprout-agent has no pin AND no valid default; symmetric
in Create and Edit.
- **ManagedAgentRow** shows the resolved profile label for sprout-agent
rows (warning color + "(none set — edit to fix)" when the default is
null or dangling). Non-sprout rows fire zero settings queries.
- **No-default banner**, **identity-rotation banner**, **load-error
banner**, and **clear-all dialog** carry over from v1, retargeted at
the wrapper.
## Hydration race fix (R11 + R12 + R13)
The profile dialog is mounted once per panel and switches between profile
ids by prop. Without gating, profile A's form values stay mounted while
B's query loads — a fast Save can submit A's values under B's id. Worse,
`invalidateQueries` keeps the previous data while refetch is in flight, so
a cross-window mutation + slow get_profile reseeds A's form under B again.
Fix lives in `useProfileDialogHydration`:
- Three explicit effects: close-reset, profile-switch-reset, hydrate-once.
- `hydrationStale` blocks Save during the race window.
- `viewIsFresh = isFetched && !isFetching` gates the seed against the
stale-but-served refetch case.
- `apiKey` + `revealKey` are scrubbed on close AND on profile switch.
- In-dialog Delete arm is reset by a scope-keyed effect on any
`(open, profileId)` transition (the earlier scope-marker approach
failed when the parent passed profileId=null on close and the same
profileId on reopen — scope key matched, armed state persisted).
A `agentProviderProfileReadDelayMs` test knob makes the race window
observable in e2e.
## Backend (`commands/agent_provider_settings/`)
- `storage_profiles.rs` — wrapper IO + v1/v2 → v3 migration; `validate_label`
(trim, non-empty, ≤64 B, no control chars); `new_profile_id` (UUID v4);
`ParseError::{IdentityMismatch, Other}` (typed dispatch, no string-match).
- `commands_write.rs` — create/update/delete/set-default IPC, all routed
through `validate_label` and a pre-flight `validate_input` mirroring
v1's save-time rules.
- `spawn.rs` (extended) — `resolve_target_profile` chooses the profile
by explicit id, else default, else fails closed. Per-profile owner
cross-check still runs at spawn.
- Pin validation on agent create/update: Ok / Unknown / Indeterminate
policy. Auto-clear of a stale pin only runs when a valid default
exists; otherwise the warning persists and the user must pick.
## Tests
- Rust: 69/69 in `agent_provider_settings` (full sweep 353/353).
Covers migration v1→v3 + v2→v3 (accept, idempotent, mismatched owner
aborts), `resolve_target_profile` policy matrix, auto-default first-
save, schema-dispatch reject on v4+, `validate_label` edge cases,
per-profile owner_pubkey cross-check at spawn.
- Playwright: 18/18 in `settings-agent-provider`. Includes the
hydration-race regression test (pre-fills a valid label so
`hydrationStale` is the only remaining gate), the cancel→reopen
delete-disarm regression test (R13), in-dialog Delete arm/commit,
no-default banner after deleting the default, clear-all flow,
identity-rotation banner, provider-change warning, post-close
apiKey + reveal-state scrub.
- `just check`: green.
## Codex review history
7 rounds (R7 through R13) on top of v1's 8. Final verdict R14 was
deferred; the last scored round was R13 at 8.3/10 with one MED
regression caught and folded.
Highlights by round:
- R7 (backend pin validation, typed parse error)
- R8 (block save on missing-pin-no-default + trim pin at IPC boundary)
- R9 (gate missing-state on status==='ok', block unstartable create,
split sprout-only query, in-dialog delete UX)
- R10 (symmetric Edit gate, flag dangling default on row)
- R11 (HIGH: kill profile-hydration race, scrub secrets on close)
- R12 (MED: stale-but-served fresh-query gate, sharper race test)
- R13 (MED regression: scope-marker delete-armed memory bug, replaced
with explicit scope-keyed effect)
Per-round review prompts + verdicts live in `.review/` (gitignored).
Signed-off-by: Tyler Longwell <109685178+tlongwell-block@users.noreply.github.com>
Co-authored-by: Dawn <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@sprout-oss.stage.blox.sqprod.co>
Pulls in 8 commits from origin/main: - 1858e98 fix(desktop): drive unread badges from live subscription, not refetched lastMessageAt (#581) - 9e76a08 fix(desktop): refine header scaling and shadow (#573) - b74ec95 fix(desktop): keep day dividers below header (#574) - aad564b Move agent activity below composer (#579) - bda98da docs(nips): NIP-AE — Agent Engrams (#575) - 1b87a09 refactor: extract shared @mention resolver into sprout-sdk (#580) - 2ee7356 fix: add default-run to sprout-relay so `cargo run -p sprout-relay` works (#577) - f0549b5 feat(desktop): channel hover state and right-click mark-unread context menu (#578) No conflicts. * origin/main: fix(desktop): drive unread badges from live subscription, not refetched lastMessageAt (#581) fix(desktop): refine header scaling and shadow (#573) fix(desktop): keep day dividers below header (#574) Move agent activity below composer (#579) docs(nips): NIP-AE — Agent Engrams (#575) refactor: extract shared @mention resolver into sprout-sdk (#580) fix: add default-run to sprout-relay so `cargo run -p sprout-relay` works (#577) feat(desktop): channel hover state and right-click mark-unread context menu (#578)
tlongwell-block
added a commit
that referenced
this pull request
May 15, 2026
Adds env_vars: BTreeMap<String, String> on both PersonaRecord and ManagedAgentRecord. Precedence at spawn: parent env < persona < agent (last write wins). Reserved keys — Sprout's identity/secrets — are rejected at save time and stripped at runtime so a typo or malicious value can't swap the agent's nsec. Backend - managed_agents/env_vars.rs (new): merged_user_env, RESERVED_ENV_KEYS, is_reserved_env_key, validate_user_env_keys, 16 unit tests. - managed_agents/runtime.rs: merges persona env → agent env at spawn via env_vars::merged_user_env. - managed_agents/types.rs: env_vars field on PersonaRecord and ManagedAgentRecord; included in ManagedAgentSummary so the frontend list/edit reload sees saved state. - commands/personas.rs, commands/agents.rs, commands/agent_models.rs: validate_user_env_keys at create/update; merged_user_env also used by sprout-acp models discovery and provider deploy so credentials in persona env flow into both. Reserved keys: SPROUT_PRIVATE_KEY, NOSTR_PRIVATE_KEY, SPROUT_AUTH_TAG, SPROUT_API_TOKEN, SPROUT_ACP_PRIVATE_KEY, SPROUT_ACP_API_TOKEN. Case-insensitive match — lowercase variants of these specific keys are almost certainly typos, not legitimate use. Behavior knobs (GOOSE_MODE, SPROUT_TOOLSETS, SPROUT_ACP_MODEL, SPROUT_ACP_SYSTEM_PROMPT) remain freely overridable. Two-layer enforcement: 1. Save-time — validate_user_env_keys rejects reserved keys with a clear error listing the offending keys, surfaced in the dialog. 2. Runtime — merged_user_env strips reserved keys with a warning log. Defense in depth for older on-disk records that predate validation. Frontend - features/agents/ui/EnvVarsEditor.tsx (new): reusable key/value editor with add/remove rows and validation hints. - PersonaDialog, CreateAgentDialog, EditAgentDialog: embed the editor. - shared/api/types.ts, tauri.ts, tauriPersonas.ts: envVars field on create/update payloads. envVars: undefined on update = 'don't touch' so editing unrelated fields can't wipe saved credentials. Tests - 16 unit tests in env_vars.rs covering merge precedence, reserved-key stripping (persona + agent + case-insensitive), and the validator. - 1 e2e test (desktop/tests/e2e/persona-env-vars.spec.ts) drives the EnvVarsEditor through the Persona dialog. - All 296 desktop crate unit tests pass. Also bumps the MessageComposer.tsx size limit from 700→710. The composer-autofocus PR (#572) landed on origin/main while this branch was in review and pushed it 3 lines over the limit. Unrelated to this feature; bumping here so CI is green. Supersedes #576. Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
tlongwell-block
added a commit
that referenced
this pull request
May 15, 2026
Adds env_vars: BTreeMap<String, String> on both PersonaRecord and ManagedAgentRecord. Precedence at spawn: parent env < persona < agent (last write wins). Reserved keys — Sprout's identity/secrets — are rejected at save time and stripped at runtime so a typo or malicious value can't swap the agent's nsec. Backend - managed_agents/env_vars.rs (new): merged_user_env, RESERVED_ENV_KEYS, is_reserved_env_key, validate_user_env_keys, 16 unit tests. - managed_agents/runtime.rs: merges persona env → agent env at spawn via env_vars::merged_user_env. - managed_agents/types.rs: env_vars field on PersonaRecord and ManagedAgentRecord; included in ManagedAgentSummary so the frontend list/edit reload sees saved state. - commands/personas.rs, commands/agents.rs, commands/agent_models.rs: validate_user_env_keys at create/update; merged_user_env also used by sprout-acp models discovery and provider deploy so credentials in persona env flow into both. Reserved keys: SPROUT_PRIVATE_KEY, NOSTR_PRIVATE_KEY, SPROUT_AUTH_TAG, SPROUT_API_TOKEN, SPROUT_ACP_PRIVATE_KEY, SPROUT_ACP_API_TOKEN. Case-insensitive match — lowercase variants of these specific keys are almost certainly typos, not legitimate use. Behavior knobs (GOOSE_MODE, SPROUT_TOOLSETS, SPROUT_ACP_MODEL, SPROUT_ACP_SYSTEM_PROMPT) remain freely overridable. Two-layer enforcement: 1. Save-time — validate_user_env_keys rejects reserved keys with a clear error listing the offending keys, surfaced in the dialog. 2. Runtime — merged_user_env strips reserved keys with a warning log. Defense in depth for older on-disk records that predate validation. Frontend - features/agents/ui/EnvVarsEditor.tsx (new): reusable key/value editor with add/remove rows and validation hints. - PersonaDialog, CreateAgentDialog, EditAgentDialog: embed the editor. - shared/api/types.ts, tauri.ts, tauriPersonas.ts: envVars field on create/update payloads. envVars: undefined on update = 'don't touch' so editing unrelated fields can't wipe saved credentials. Tests - 16 unit tests in env_vars.rs covering merge precedence, reserved-key stripping (persona + agent + case-insensitive), and the validator. - 1 e2e test (desktop/tests/e2e/persona-env-vars.spec.ts) drives the EnvVarsEditor through the Persona dialog. - All 296 desktop crate unit tests pass. Also bumps the MessageComposer.tsx size limit from 700→710. The composer-autofocus PR (#572) landed on origin/main while this branch was in review and pushed it 3 lines over the limit. Unrelated to this feature; bumping here so CI is green. Supersedes #576. Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
tlongwell-block
added a commit
that referenced
this pull request
May 15, 2026
Adds env_vars: BTreeMap<String, String> on both PersonaRecord and ManagedAgentRecord. Precedence at spawn: parent env < persona < agent (last write wins). Reserved keys — Sprout's identity/secrets — are rejected at save time and stripped at runtime so a typo or malicious value can't swap the agent's nsec. Backend - managed_agents/env_vars.rs (new): merged_user_env, RESERVED_ENV_KEYS, is_reserved_env_key, validate_user_env_keys, 17 unit tests. - managed_agents/runtime.rs: merges persona env → agent env at spawn via env_vars::merged_user_env. - managed_agents/types.rs: env_vars field on PersonaRecord and ManagedAgentRecord; included in ManagedAgentSummary so the frontend list/edit reload sees saved state. - managed_agents/backend.rs: extends provider stderr/error redaction to scrub user-supplied env values (redact_secrets_with + env_secrets_from_request). Without this, a provider that echoes its request in failure messages could surface an ANTHROPIC_API_KEY-style secret unredacted via spawnError/last_error. - commands/personas.rs, commands/agents.rs, commands/agent_models.rs: validate_user_env_keys at create/update; merged_user_env also used by sprout-acp models discovery and provider deploy so credentials in persona env flow into both. Reserved keys - SPROUT_PRIVATE_KEY, NOSTR_PRIVATE_KEY (agent identity) - SPROUT_AUTH_TAG (NIP-OA relay auth) - SPROUT_API_TOKEN, SPROUT_ACP_PRIVATE_KEY, SPROUT_ACP_API_TOKEN - SPROUT_ACP_AGENT_OWNER (owner enforcement for legacy records without auth_tag — overriding would change who the agent responds to) Case-insensitive match — lowercase variants of these specific keys are almost certainly typos, not legitimate use. Behavior knobs (GOOSE_MODE, SPROUT_TOOLSETS, SPROUT_ACP_MODEL, SPROUT_ACP_SYSTEM_PROMPT) remain freely overridable. Two-layer enforcement: 1. Save-time — validate_user_env_keys rejects reserved keys with a clear error listing the offending keys, surfaced in the dialog. 2. Runtime — merged_user_env strips reserved keys with a warning log. Defense in depth for older on-disk records that predate validation. Frontend - features/agents/ui/EnvVarsEditor.tsx (new): reusable key/value editor with add/remove rows and validation hints. - PersonaDialog, CreateAgentDialog, EditAgentDialog: embed the editor. - shared/api/types.ts, tauri.ts, tauriPersonas.ts: envVars field on create/update payloads. envVars: undefined on update = 'don't touch' so editing unrelated fields can't wipe saved credentials. Tests - 17 unit tests in env_vars.rs covering merge precedence, reserved-key stripping (persona + agent + case-insensitive), owner-key protection for legacy records, and the validator. - 5 new unit tests in backend.rs covering redact_secrets_with (user env values scrubbed, short values skipped, overlapping secrets handled) and env_secrets_from_request (string extraction + missing-shape). - 1 e2e test (desktop/tests/e2e/persona-env-vars.spec.ts) drives the EnvVarsEditor through the Persona dialog. - All 302 desktop crate unit tests pass. Also bumps the MessageComposer.tsx size limit from 700→710 — the composer-autofocus PR (#572) landed on origin/main while this branch was in review and pushed it 3 lines over. Unrelated to this feature; bumping here so CI is green. Bumps backend.rs limit from 530→640 for the new redaction helpers + their tests. Supersedes #576. Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
tlongwell-block
added a commit
that referenced
this pull request
May 15, 2026
Adds env_vars: BTreeMap<String, String> on both PersonaRecord and ManagedAgentRecord. Precedence at spawn: parent env < persona < agent (last write wins). Reserved keys — Sprout's identity/secrets — are rejected at save time and stripped at runtime so a typo or malicious value can't swap the agent's nsec. Backend - managed_agents/env_vars.rs (new): merged_user_env, RESERVED_ENV_KEYS, is_reserved_env_key, validate_user_env_keys, 17 unit tests. - managed_agents/runtime.rs: merges persona env → agent env at spawn via env_vars::merged_user_env. - managed_agents/types.rs: env_vars field on PersonaRecord and ManagedAgentRecord; included in ManagedAgentSummary so the frontend list/edit reload sees saved state. - managed_agents/backend.rs: extends provider stderr/error redaction to scrub user-supplied env values (redact_secrets_with + env_secrets_from_request). Without this, a provider that echoes its request in failure messages could surface an ANTHROPIC_API_KEY-style secret unredacted via spawnError/last_error. - commands/personas.rs, commands/agents.rs, commands/agent_models.rs: validate_user_env_keys at create/update; merged_user_env also used by sprout-acp models discovery and provider deploy so credentials in persona env flow into both. Reserved keys - SPROUT_PRIVATE_KEY, NOSTR_PRIVATE_KEY (agent identity) - SPROUT_AUTH_TAG (NIP-OA relay auth) - SPROUT_API_TOKEN, SPROUT_ACP_PRIVATE_KEY, SPROUT_ACP_API_TOKEN - SPROUT_ACP_AGENT_OWNER (owner enforcement for legacy records without auth_tag — overriding would change who the agent responds to) Case-insensitive match — lowercase variants of these specific keys are almost certainly typos, not legitimate use. Behavior knobs (GOOSE_MODE, SPROUT_TOOLSETS, SPROUT_ACP_MODEL, SPROUT_ACP_SYSTEM_PROMPT) remain freely overridable. Two-layer enforcement: 1. Save-time — validate_user_env_keys rejects reserved keys with a clear error listing the offending keys, surfaced in the dialog. 2. Runtime — merged_user_env strips reserved keys with a warning log. Defense in depth for older on-disk records that predate validation. Frontend - features/agents/ui/EnvVarsEditor.tsx (new): reusable key/value editor with add/remove rows and validation hints. - PersonaDialog, CreateAgentDialog, EditAgentDialog: embed the editor. - shared/api/types.ts, tauri.ts, tauriPersonas.ts: envVars field on create/update payloads. envVars: undefined on update = 'don't touch' so editing unrelated fields can't wipe saved credentials. Tests - 17 unit tests in env_vars.rs covering merge precedence, reserved-key stripping (persona + agent + case-insensitive), owner-key protection for legacy records, and the validator. - 5 new unit tests in backend.rs covering redact_secrets_with (user env values scrubbed, short values skipped, overlapping secrets handled) and env_secrets_from_request (string extraction + missing-shape). - 1 e2e test (desktop/tests/e2e/persona-env-vars.spec.ts) drives the EnvVarsEditor through the Persona dialog. - All 302 desktop crate unit tests pass. Also bumps the MessageComposer.tsx size limit from 700→710 — the composer-autofocus PR (#572) landed on origin/main while this branch was in review and pushed it 3 lines over. Unrelated to this feature; bumping here so CI is green. Bumps backend.rs limit from 530→640 for the new redaction helpers + their tests. Supersedes #576. Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
tlongwell-block
added a commit
that referenced
this pull request
May 15, 2026
Adds env_vars: BTreeMap<String, String> on both PersonaRecord and ManagedAgentRecord. Precedence at spawn: parent env < persona < agent (last write wins). Reserved keys — Sprout's identity, code-execution surface, and security gates — are rejected at save time and stripped at runtime so a typo or malicious value can't swap the agent's nsec, code, relay, or respond-to gate. Backend - managed_agents/env_vars.rs (new): merged_user_env, RESERVED_ENV_KEYS, is_reserved_env_key, validate_user_env_keys, 20 unit tests. - managed_agents/runtime.rs: merges persona env → agent env at spawn via env_vars::merged_user_env. - managed_agents/types.rs: env_vars field on PersonaRecord and ManagedAgentRecord; included in ManagedAgentSummary so the frontend list/edit reload sees saved state. - managed_agents/backend.rs: extends provider stderr/error redaction to scrub user-supplied env values (redact_secrets_with + env_secrets_from_request + redact_env_values_in). Uses single-pass str::replace to avoid a non-terminating loop when a user env value is a substring of '[REDACTED]'. - commands/personas.rs, commands/agents.rs, commands/agent_models.rs: validate_user_env_keys at create/update; merged_user_env also used by sprout-acp models discovery and provider deploy so credentials in persona env flow into both. Model-discovery stderr is now redacted through redact_env_values_in before being formatted into the frontend-visible error. Reserved keys (three categories) - Identity / secrets: SPROUT_PRIVATE_KEY, NOSTR_PRIVATE_KEY, SPROUT_AUTH_TAG, SPROUT_API_TOKEN, SPROUT_ACP_PRIVATE_KEY, SPROUT_ACP_API_TOKEN. - Code-execution surface: SPROUT_ACP_AGENT_COMMAND, SPROUT_ACP_AGENT_ARGS, SPROUT_ACP_MCP_COMMAND, SPROUT_RELAY_URL. - Security gates: SPROUT_ACP_RESPOND_TO, SPROUT_ACP_RESPOND_TO_ALLOWLIST, SPROUT_ACP_AGENT_OWNER (legacy-record owner fallback). Case-insensitive match — lowercase variants of these specific keys are almost certainly typos, not legitimate use. Behavior knobs (GOOSE_MODE, SPROUT_TOOLSETS, SPROUT_ACP_MODEL, SPROUT_ACP_SYSTEM_PROMPT) remain freely overridable. Two-layer enforcement: 1. Save-time — validate_user_env_keys rejects reserved keys with a clear error listing the offending keys, surfaced in the dialog. 2. Runtime — merged_user_env strips reserved keys with a warning log. Defense in depth for older on-disk records that predate validation. Frontend - features/agents/ui/EnvVarsEditor.tsx (new): reusable key/value editor with add/remove rows and validation hints. - PersonaDialog, CreateAgentDialog, EditAgentDialog: embed the editor. - shared/api/types.ts, tauri.ts, tauriPersonas.ts: envVars field on create/update payloads. envVars: undefined on update = 'don't touch' so editing unrelated fields can't wipe saved credentials. Tests - 20 unit tests in env_vars.rs covering merge precedence, reserved-key stripping for each category (identity, code-execution, security gates, legacy owner, relay URL, case-insensitive), and the validator. - 6 new unit tests in backend.rs covering redact_secrets_with (user env values scrubbed, short values skipped, overlapping secrets handled, termination when value is substring of marker), env_secrets_from_request (string extraction + missing-shape), and redact_env_values_in. - 1 e2e test (desktop/tests/e2e/persona-env-vars.spec.ts) drives the EnvVarsEditor through the Persona dialog. - All 307 desktop crate unit tests pass. Also bumps the MessageComposer.tsx size limit from 700→710 (the composer-autofocus PR #572 landed on origin/main mid-review and pushed it 3 lines over). Bumps backend.rs limit from 530→700 for the new redaction helpers + their tests. Supersedes #576. Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
tlongwell-block
added a commit
that referenced
this pull request
May 15, 2026
Adds env_vars: BTreeMap<String, String> on both PersonaRecord and ManagedAgentRecord. Precedence at spawn: parent env < persona < agent (last write wins). Reserved keys — Sprout's identity, code-execution surface, and security gates — are rejected at save time and stripped at runtime so a typo or malicious value can't swap the agent's nsec, code, relay, or respond-to gate. Backend - managed_agents/env_vars.rs (new): merged_user_env, RESERVED_ENV_KEYS, is_reserved_env_key, validate_user_env_keys, 20 unit tests. - managed_agents/runtime.rs: merges persona env → agent env at spawn via env_vars::merged_user_env. - managed_agents/types.rs: env_vars field on PersonaRecord and ManagedAgentRecord; included in ManagedAgentSummary so the frontend list/edit reload sees saved state. - managed_agents/backend.rs: extends provider stderr/error redaction to scrub user-supplied env values (redact_secrets_with + env_secrets_from_request + redact_env_values_in). Uses single-pass str::replace to avoid a non-terminating loop when a user env value is a substring of '[REDACTED]'. - commands/personas.rs, commands/agents.rs, commands/agent_models.rs: validate_user_env_keys at create/update; merged_user_env also used by sprout-acp models discovery and provider deploy so credentials in persona env flow into both. Model-discovery stderr is now redacted through redact_env_values_in before being formatted into the frontend-visible error. Reserved keys (three categories) - Identity / secrets: SPROUT_PRIVATE_KEY, NOSTR_PRIVATE_KEY, SPROUT_AUTH_TAG, SPROUT_API_TOKEN, SPROUT_ACP_PRIVATE_KEY, SPROUT_ACP_API_TOKEN. - Code-execution surface: SPROUT_ACP_AGENT_COMMAND, SPROUT_ACP_AGENT_ARGS, SPROUT_ACP_MCP_COMMAND, SPROUT_RELAY_URL. - Security gates: SPROUT_ACP_RESPOND_TO, SPROUT_ACP_RESPOND_TO_ALLOWLIST, SPROUT_ACP_AGENT_OWNER (legacy-record owner fallback). Case-insensitive match — lowercase variants of these specific keys are almost certainly typos, not legitimate use. Behavior knobs (GOOSE_MODE, SPROUT_TOOLSETS, SPROUT_ACP_MODEL, SPROUT_ACP_SYSTEM_PROMPT) remain freely overridable. Two-layer enforcement: 1. Save-time — validate_user_env_keys rejects reserved keys with a clear error listing the offending keys, surfaced in the dialog. 2. Runtime — merged_user_env strips reserved keys with a warning log. Defense in depth for older on-disk records that predate validation. Frontend - features/agents/ui/EnvVarsEditor.tsx (new): reusable key/value editor with add/remove rows and validation hints. - PersonaDialog, CreateAgentDialog, EditAgentDialog: embed the editor. - shared/api/types.ts, tauri.ts, tauriPersonas.ts: envVars field on create/update payloads. envVars: undefined on update = 'don't touch' so editing unrelated fields can't wipe saved credentials. Tests - 20 unit tests in env_vars.rs covering merge precedence, reserved-key stripping for each category (identity, code-execution, security gates, legacy owner, relay URL, case-insensitive), and the validator. - 6 new unit tests in backend.rs covering redact_secrets_with (user env values scrubbed, short values skipped, overlapping secrets handled, termination when value is substring of marker), env_secrets_from_request (string extraction + missing-shape), and redact_env_values_in. - 1 e2e test (desktop/tests/e2e/persona-env-vars.spec.ts) drives the EnvVarsEditor through the Persona dialog. - All 307 desktop crate unit tests pass. Also bumps the MessageComposer.tsx size limit from 700→710 (the composer-autofocus PR #572 landed on origin/main mid-review and pushed it 3 lines over). Bumps backend.rs limit from 530→700 for the new redaction helpers + their tests. Supersedes #576. Signed-off-by: Tyler Longwell <tlongwell@squareup.com> Security: reject malformed env var keys Tightens validate_user_env_keys to require POSIX-shaped keys ([A-Za-z_][A-Za-z0-9_]*). Closes a denylist bypass: Rust's Command::env(k, v) accepts a key containing '=' and writes it verbatim into the child's environ block. A key like SPROUT_AUTH_TAG=x with value forged lands as SPROUT_AUTH_TAG=x=forged in the child env, so getenv("SPROUT_AUTH_TAG") returns "x=forged" — bypassing the reserved- key string compare. Confirmed exploitable for SPROUT_AUTH_TAG (legacy agents) and any other reserved key Sprout doesn't always set with the canonical name first. Two-layer fix: 1. validate_user_env_keys rejects malformed keys at save time, listing each invalid key with a clear regex hint. 2. merged_user_env strips malformed keys at spawn time as defense in depth for on-disk records that predate the validator. +6 unit tests pinning the bypass and adjacent edge cases (empty, whitespace, NUL, leading digit, non-ASCII, =-in-key). 313 unit tests pass.
tlongwell-block
added a commit
that referenced
this pull request
May 15, 2026
Adds env_vars: BTreeMap<String, String> on both PersonaRecord and ManagedAgentRecord. Precedence at spawn: parent env < persona < agent (last write wins). Reserved keys — Sprout's identity, code-execution surface, and security gates — are rejected at save time and stripped at runtime so a typo or malicious value can't swap the agent's nsec, code, relay, or respond-to gate. Backend - managed_agents/env_vars.rs (new): merged_user_env, RESERVED_ENV_KEYS, is_reserved_env_key, validate_user_env_keys, 20 unit tests. - managed_agents/runtime.rs: merges persona env → agent env at spawn via env_vars::merged_user_env. - managed_agents/types.rs: env_vars field on PersonaRecord and ManagedAgentRecord; included in ManagedAgentSummary so the frontend list/edit reload sees saved state. - managed_agents/backend.rs: extends provider stderr/error redaction to scrub user-supplied env values (redact_secrets_with + env_secrets_from_request + redact_env_values_in). Uses single-pass str::replace to avoid a non-terminating loop when a user env value is a substring of '[REDACTED]'. - commands/personas.rs, commands/agents.rs, commands/agent_models.rs: validate_user_env_keys at create/update; merged_user_env also used by sprout-acp models discovery and provider deploy so credentials in persona env flow into both. Model-discovery stderr is now redacted through redact_env_values_in before being formatted into the frontend-visible error. Reserved keys (three categories) - Identity / secrets: SPROUT_PRIVATE_KEY, NOSTR_PRIVATE_KEY, SPROUT_AUTH_TAG, SPROUT_API_TOKEN, SPROUT_ACP_PRIVATE_KEY, SPROUT_ACP_API_TOKEN. - Code-execution surface: SPROUT_ACP_AGENT_COMMAND, SPROUT_ACP_AGENT_ARGS, SPROUT_ACP_MCP_COMMAND, SPROUT_RELAY_URL. - Security gates: SPROUT_ACP_RESPOND_TO, SPROUT_ACP_RESPOND_TO_ALLOWLIST, SPROUT_ACP_AGENT_OWNER (legacy-record owner fallback). Case-insensitive match — lowercase variants of these specific keys are almost certainly typos, not legitimate use. Behavior knobs (GOOSE_MODE, SPROUT_TOOLSETS, SPROUT_ACP_MODEL, SPROUT_ACP_SYSTEM_PROMPT) remain freely overridable. Two-layer enforcement: 1. Save-time — validate_user_env_keys rejects reserved keys with a clear error listing the offending keys, surfaced in the dialog. 2. Runtime — merged_user_env strips reserved keys with a warning log. Defense in depth for older on-disk records that predate validation. Frontend - features/agents/ui/EnvVarsEditor.tsx (new): reusable key/value editor with add/remove rows and validation hints. - PersonaDialog, CreateAgentDialog, EditAgentDialog: embed the editor. - shared/api/types.ts, tauri.ts, tauriPersonas.ts: envVars field on create/update payloads. envVars: undefined on update = 'don't touch' so editing unrelated fields can't wipe saved credentials. Tests - 20 unit tests in env_vars.rs covering merge precedence, reserved-key stripping for each category (identity, code-execution, security gates, legacy owner, relay URL, case-insensitive), and the validator. - 6 new unit tests in backend.rs covering redact_secrets_with (user env values scrubbed, short values skipped, overlapping secrets handled, termination when value is substring of marker), env_secrets_from_request (string extraction + missing-shape), and redact_env_values_in. - 1 e2e test (desktop/tests/e2e/persona-env-vars.spec.ts) drives the EnvVarsEditor through the Persona dialog. - All 307 desktop crate unit tests pass. Also bumps the MessageComposer.tsx size limit from 700→710 (the composer-autofocus PR #572 landed on origin/main mid-review and pushed it 3 lines over). Bumps backend.rs limit from 530→700 for the new redaction helpers + their tests. Supersedes #576. Signed-off-by: Tyler Longwell <tlongwell@squareup.com> Security: reject malformed env var keys Tightens validate_user_env_keys to require POSIX-shaped keys ([A-Za-z_][A-Za-z0-9_]*). Closes a denylist bypass: Rust's Command::env(k, v) accepts a key containing '=' and writes it verbatim into the child's environ block. A key like SPROUT_AUTH_TAG=x with value forged lands as SPROUT_AUTH_TAG=x=forged in the child env, so getenv("SPROUT_AUTH_TAG") returns "x=forged" — bypassing the reserved- key string compare. Confirmed exploitable for SPROUT_AUTH_TAG (legacy agents) and any other reserved key Sprout doesn't always set with the canonical name first. Two-layer fix: 1. validate_user_env_keys rejects malformed keys at save time, listing each invalid key with a clear regex hint. 2. merged_user_env strips malformed keys at spawn time as defense in depth for on-disk records that predate the validator. +6 unit tests pinning the bypass and adjacent edge cases (empty, whitespace, NUL, leading digit, non-ASCII, =-in-key). 313 unit tests pass.
tlongwell-block
added a commit
that referenced
this pull request
May 15, 2026
Adds env_vars: BTreeMap<String, String> on both PersonaRecord and ManagedAgentRecord. Precedence at spawn: parent env < persona < agent (last write wins). Reserved keys — Sprout's identity, code-execution surface, and security gates — are rejected at save time and stripped at runtime so a typo or malicious value can't swap the agent's nsec, code, relay, or respond-to gate. Backend - managed_agents/env_vars.rs (new): merged_user_env, RESERVED_ENV_KEYS, is_reserved_env_key, validate_user_env_keys, 20 unit tests. - managed_agents/runtime.rs: merges persona env → agent env at spawn via env_vars::merged_user_env. - managed_agents/types.rs: env_vars field on PersonaRecord and ManagedAgentRecord; included in ManagedAgentSummary so the frontend list/edit reload sees saved state. - managed_agents/backend.rs: extends provider stderr/error redaction to scrub user-supplied env values (redact_secrets_with + env_secrets_from_request + redact_env_values_in). Uses single-pass str::replace to avoid a non-terminating loop when a user env value is a substring of '[REDACTED]'. - commands/personas.rs, commands/agents.rs, commands/agent_models.rs: validate_user_env_keys at create/update; merged_user_env also used by sprout-acp models discovery and provider deploy so credentials in persona env flow into both. Model-discovery stderr is now redacted through redact_env_values_in before being formatted into the frontend-visible error. Reserved keys (three categories) - Identity / secrets: SPROUT_PRIVATE_KEY, NOSTR_PRIVATE_KEY, SPROUT_AUTH_TAG, SPROUT_API_TOKEN, SPROUT_ACP_PRIVATE_KEY, SPROUT_ACP_API_TOKEN. - Code-execution surface: SPROUT_ACP_AGENT_COMMAND, SPROUT_ACP_AGENT_ARGS, SPROUT_ACP_MCP_COMMAND, SPROUT_RELAY_URL. - Security gates: SPROUT_ACP_RESPOND_TO, SPROUT_ACP_RESPOND_TO_ALLOWLIST, SPROUT_ACP_AGENT_OWNER (legacy-record owner fallback). Case-insensitive match — lowercase variants of these specific keys are almost certainly typos, not legitimate use. Behavior knobs (GOOSE_MODE, SPROUT_TOOLSETS, SPROUT_ACP_MODEL, SPROUT_ACP_SYSTEM_PROMPT) remain freely overridable. Two-layer enforcement: 1. Save-time — validate_user_env_keys rejects reserved keys with a clear error listing the offending keys, surfaced in the dialog. 2. Runtime — merged_user_env strips reserved keys with a warning log. Defense in depth for older on-disk records that predate validation. Frontend - features/agents/ui/EnvVarsEditor.tsx (new): reusable key/value editor with add/remove rows and validation hints. - PersonaDialog, CreateAgentDialog, EditAgentDialog: embed the editor. - shared/api/types.ts, tauri.ts, tauriPersonas.ts: envVars field on create/update payloads. envVars: undefined on update = 'don't touch' so editing unrelated fields can't wipe saved credentials. Tests - 20 unit tests in env_vars.rs covering merge precedence, reserved-key stripping for each category (identity, code-execution, security gates, legacy owner, relay URL, case-insensitive), and the validator. - 6 new unit tests in backend.rs covering redact_secrets_with (user env values scrubbed, short values skipped, overlapping secrets handled, termination when value is substring of marker), env_secrets_from_request (string extraction + missing-shape), and redact_env_values_in. - 1 e2e test (desktop/tests/e2e/persona-env-vars.spec.ts) drives the EnvVarsEditor through the Persona dialog. - All 307 desktop crate unit tests pass. Also bumps the MessageComposer.tsx size limit from 700→710 (the composer-autofocus PR #572 landed on origin/main mid-review and pushed it 3 lines over). Bumps backend.rs limit from 530→700 for the new redaction helpers + their tests. Supersedes #576. Signed-off-by: Tyler Longwell <tlongwell@squareup.com> Security: reject malformed env var keys Tightens validate_user_env_keys to require POSIX-shaped keys ([A-Za-z_][A-Za-z0-9_]*). Closes a denylist bypass: Rust's Command::env(k, v) accepts a key containing '=' and writes it verbatim into the child's environ block. A key like SPROUT_AUTH_TAG=x with value forged lands as SPROUT_AUTH_TAG=x=forged in the child env, so getenv("SPROUT_AUTH_TAG") returns "x=forged" — bypassing the reserved- key string compare. Confirmed exploitable for SPROUT_AUTH_TAG (legacy agents) and any other reserved key Sprout doesn't always set with the canonical name first. Two-layer fix: 1. validate_user_env_keys rejects malformed keys at save time, listing each invalid key with a clear regex hint. 2. merged_user_env strips malformed keys at spawn time as defense in depth for on-disk records that predate the validator. +6 unit tests pinning the bypass and adjacent edge cases (empty, whitespace, NUL, leading digit, non-ASCII, =-in-key). 313 unit tests pass.
tlongwell-block
added a commit
that referenced
this pull request
May 15, 2026
Adds env_vars: BTreeMap<String, String> on both PersonaRecord and ManagedAgentRecord. Precedence at spawn: parent env < persona < agent (last write wins). Reserved keys — Sprout's identity, code-execution surface, and security gates — are rejected at save time and stripped at runtime so a typo or malicious value can't swap the agent's nsec, code, relay, or respond-to gate. Backend - managed_agents/env_vars.rs (new): merged_user_env, RESERVED_ENV_KEYS, is_reserved_env_key, validate_user_env_keys, 20 unit tests. - managed_agents/runtime.rs: merges persona env → agent env at spawn via env_vars::merged_user_env. - managed_agents/types.rs: env_vars field on PersonaRecord and ManagedAgentRecord; included in ManagedAgentSummary so the frontend list/edit reload sees saved state. - managed_agents/backend.rs: extends provider stderr/error redaction to scrub user-supplied env values (redact_secrets_with + env_secrets_from_request + redact_env_values_in). Uses single-pass str::replace to avoid a non-terminating loop when a user env value is a substring of '[REDACTED]'. - commands/personas.rs, commands/agents.rs, commands/agent_models.rs: validate_user_env_keys at create/update; merged_user_env also used by sprout-acp models discovery and provider deploy so credentials in persona env flow into both. Model-discovery stderr is now redacted through redact_env_values_in before being formatted into the frontend-visible error. Reserved keys (three categories) - Identity / secrets: SPROUT_PRIVATE_KEY, NOSTR_PRIVATE_KEY, SPROUT_AUTH_TAG, SPROUT_API_TOKEN, SPROUT_ACP_PRIVATE_KEY, SPROUT_ACP_API_TOKEN. - Code-execution surface: SPROUT_ACP_AGENT_COMMAND, SPROUT_ACP_AGENT_ARGS, SPROUT_ACP_MCP_COMMAND, SPROUT_RELAY_URL. - Security gates: SPROUT_ACP_RESPOND_TO, SPROUT_ACP_RESPOND_TO_ALLOWLIST, SPROUT_ACP_AGENT_OWNER (legacy-record owner fallback). Case-insensitive match — lowercase variants of these specific keys are almost certainly typos, not legitimate use. Behavior knobs (GOOSE_MODE, SPROUT_TOOLSETS, SPROUT_ACP_MODEL, SPROUT_ACP_SYSTEM_PROMPT) remain freely overridable. Two-layer enforcement: 1. Save-time — validate_user_env_keys rejects reserved keys with a clear error listing the offending keys, surfaced in the dialog. 2. Runtime — merged_user_env strips reserved keys with a warning log. Defense in depth for older on-disk records that predate validation. Frontend - features/agents/ui/EnvVarsEditor.tsx (new): reusable key/value editor with add/remove rows and validation hints. - PersonaDialog, CreateAgentDialog, EditAgentDialog: embed the editor. - shared/api/types.ts, tauri.ts, tauriPersonas.ts: envVars field on create/update payloads. envVars: undefined on update = 'don't touch' so editing unrelated fields can't wipe saved credentials. Tests - 20 unit tests in env_vars.rs covering merge precedence, reserved-key stripping for each category (identity, code-execution, security gates, legacy owner, relay URL, case-insensitive), and the validator. - 6 new unit tests in backend.rs covering redact_secrets_with (user env values scrubbed, short values skipped, overlapping secrets handled, termination when value is substring of marker), env_secrets_from_request (string extraction + missing-shape), and redact_env_values_in. - 1 e2e test (desktop/tests/e2e/persona-env-vars.spec.ts) drives the EnvVarsEditor through the Persona dialog. - All 307 desktop crate unit tests pass. Also bumps the MessageComposer.tsx size limit from 700→710 (the composer-autofocus PR #572 landed on origin/main mid-review and pushed it 3 lines over). Bumps backend.rs limit from 530→700 for the new redaction helpers + their tests. Supersedes #576. Signed-off-by: Tyler Longwell <tlongwell@squareup.com> Security: reject malformed env var keys Tightens validate_user_env_keys to require POSIX-shaped keys ([A-Za-z_][A-Za-z0-9_]*). Closes a denylist bypass: Rust's Command::env(k, v) accepts a key containing '=' and writes it verbatim into the child's environ block. A key like SPROUT_AUTH_TAG=x with value forged lands as SPROUT_AUTH_TAG=x=forged in the child env, so getenv("SPROUT_AUTH_TAG") returns "x=forged" — bypassing the reserved- key string compare. Confirmed exploitable for SPROUT_AUTH_TAG (legacy agents) and any other reserved key Sprout doesn't always set with the canonical name first. Two-layer fix: 1. validate_user_env_keys rejects malformed keys at save time, listing each invalid key with a clear regex hint. 2. merged_user_env strips malformed keys at spawn time as defense in depth for on-disk records that predate the validator. +6 unit tests pinning the bypass and adjacent edge cases (empty, whitespace, NUL, leading digit, non-ASCII, =-in-key). 313 unit tests pass.
tlongwell-block
added a commit
that referenced
this pull request
May 15, 2026
Adds env_vars: BTreeMap<String, String> on both PersonaRecord and ManagedAgentRecord. Precedence at spawn: parent env < persona < agent (last write wins). Reserved keys — Sprout's identity, code-execution surface, and security gates — are rejected at save time and stripped at runtime so a typo or malicious value can't swap the agent's nsec, code, relay, or respond-to gate. Backend - managed_agents/env_vars.rs (new): merged_user_env, RESERVED_ENV_KEYS, is_reserved_env_key, validate_user_env_keys, 20 unit tests. - managed_agents/runtime.rs: merges persona env → agent env at spawn via env_vars::merged_user_env. - managed_agents/types.rs: env_vars field on PersonaRecord and ManagedAgentRecord; included in ManagedAgentSummary so the frontend list/edit reload sees saved state. - managed_agents/backend.rs: extends provider stderr/error redaction to scrub user-supplied env values (redact_secrets_with + env_secrets_from_request + redact_env_values_in). Uses single-pass str::replace to avoid a non-terminating loop when a user env value is a substring of '[REDACTED]'. - commands/personas.rs, commands/agents.rs, commands/agent_models.rs: validate_user_env_keys at create/update; merged_user_env also used by sprout-acp models discovery and provider deploy so credentials in persona env flow into both. Model-discovery stderr is now redacted through redact_env_values_in before being formatted into the frontend-visible error. Reserved keys (three categories) - Identity / secrets: SPROUT_PRIVATE_KEY, NOSTR_PRIVATE_KEY, SPROUT_AUTH_TAG, SPROUT_API_TOKEN, SPROUT_ACP_PRIVATE_KEY, SPROUT_ACP_API_TOKEN. - Code-execution surface: SPROUT_ACP_AGENT_COMMAND, SPROUT_ACP_AGENT_ARGS, SPROUT_ACP_MCP_COMMAND, SPROUT_RELAY_URL. - Security gates: SPROUT_ACP_RESPOND_TO, SPROUT_ACP_RESPOND_TO_ALLOWLIST, SPROUT_ACP_AGENT_OWNER (legacy-record owner fallback). Case-insensitive match — lowercase variants of these specific keys are almost certainly typos, not legitimate use. Behavior knobs (GOOSE_MODE, SPROUT_TOOLSETS, SPROUT_ACP_MODEL, SPROUT_ACP_SYSTEM_PROMPT) remain freely overridable. Two-layer enforcement: 1. Save-time — validate_user_env_keys rejects reserved keys with a clear error listing the offending keys, surfaced in the dialog. 2. Runtime — merged_user_env strips reserved keys with a warning log. Defense in depth for older on-disk records that predate validation. Frontend - features/agents/ui/EnvVarsEditor.tsx (new): reusable key/value editor with add/remove rows and validation hints. - PersonaDialog, CreateAgentDialog, EditAgentDialog: embed the editor. - shared/api/types.ts, tauri.ts, tauriPersonas.ts: envVars field on create/update payloads. envVars: undefined on update = 'don't touch' so editing unrelated fields can't wipe saved credentials. Tests - 20 unit tests in env_vars.rs covering merge precedence, reserved-key stripping for each category (identity, code-execution, security gates, legacy owner, relay URL, case-insensitive), and the validator. - 6 new unit tests in backend.rs covering redact_secrets_with (user env values scrubbed, short values skipped, overlapping secrets handled, termination when value is substring of marker), env_secrets_from_request (string extraction + missing-shape), and redact_env_values_in. - 1 e2e test (desktop/tests/e2e/persona-env-vars.spec.ts) drives the EnvVarsEditor through the Persona dialog. - All 307 desktop crate unit tests pass. Also bumps the MessageComposer.tsx size limit from 700→710 (the composer-autofocus PR #572 landed on origin/main mid-review and pushed it 3 lines over). Bumps backend.rs limit from 530→700 for the new redaction helpers + their tests. Supersedes #576. Signed-off-by: Tyler Longwell <tlongwell@squareup.com> Security: reject malformed env var keys Tightens validate_user_env_keys to require POSIX-shaped keys ([A-Za-z_][A-Za-z0-9_]*). Closes a denylist bypass: Rust's Command::env(k, v) accepts a key containing '=' and writes it verbatim into the child's environ block. A key like SPROUT_AUTH_TAG=x with value forged lands as SPROUT_AUTH_TAG=x=forged in the child env, so getenv("SPROUT_AUTH_TAG") returns "x=forged" — bypassing the reserved- key string compare. Confirmed exploitable for SPROUT_AUTH_TAG (legacy agents) and any other reserved key Sprout doesn't always set with the canonical name first. Two-layer fix: 1. validate_user_env_keys rejects malformed keys at save time, listing each invalid key with a clear regex hint. 2. merged_user_env strips malformed keys at spawn time as defense in depth for on-disk records that predate the validator. +6 unit tests pinning the bypass and adjacent edge cases (empty, whitespace, NUL, leading digit, non-ASCII, =-in-key). 313 unit tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a Settings → Agent Provider panel to the desktop GUI for configuring the
sprout-agentprovider, model, API key, and behavior knobs. The panel ships in two layered commits:3dceb02— encrypted Agent Provider settings panel. Single global config per identity. NIP-44 v2 self-encrypted at rest. Auto-detect provider + base URL from the API key shape.9013310— multi-provider profiles for sprout-agent. Many named profiles per identity; one marked as default; per-sprout-agent pin selector in Create/Edit. Legacy single-config envelopes migrate idempotently on first read.What you get
sk-ant-admin01-admin keys.Threat model
Zeroizing<String>→ NIP-44 envelope on disk → on spawn, decrypt →StoredSettings(Drop zeroizes) →EnvPairs(Drop zeroizes every value buffer) → handed toCommand::envby reference, source buffer zeroized after each pair.validate_stored): non-loopbackhttp://rejected, control chars in key/model/base_url rejected, userinfo/query/fragment in URL rejected, size caps, positive-int knobs. Closes a rollback-to-pre-validation-envelope attack.owner_pubkeyat the wrapper level (authoritative) AND per-profile (v2 defense-in-depth). Migration refuses to launder a mismatched legacy owner_pubkey into a clean v3 envelope under the current pubkey.useProfileDialogHydration): close-reset / profile-switch-reset / hydrate-once, plushydrationStaleandviewIsFresh = isFetched && !isFetchinggates on Save./proc/<pid>/environ. A separate fd-handoff feature is tracked.Plaintext schema (v3 wrapper)
Legacy v1/v2 plaintexts migrate idempotently on first read: wrap as the sole profile, label "Default", mark default, stamp
owner_pubkeywith the envelope pubkey. A mismatched legacy owner_pubkey aborts asIdentityMismatch(typed dispatch — no string-matching on error text).Capacity
No explicit count cap on
profiles.len(). The real bound is the JSON-serializedProfilesPlaintextfitting underMAX_PLAINTEXT_BYTES = 48 KiB(headroom under NIP-44 v2's 65535-byte plaintext limit). Measured against the real serializer:A profile with a max-sized 32 KB
system_prompteats most of the budget on its own (~1 fat + ~15 typical fit alongside it). Typical sprout-agent profiles carry no system_prompt — the agent defaults it.If we ever need to feel infinite: raise
MAX_PLAINTEXT_BYTES(still bounded by NIP-44 v2's limit) or chunk across multiple envelopes. Not worth doing speculatively.UI
AgentProviderProfileDialog) carries label + provider + key + model + base URL + advanced knobs. Auto-detect from key format (Anthropic, OpenAI legacy/proj/svcacct, OpenRouter, Groq, xAI, Cerebras, Together, Perplexity, Fireworks, baresk-→ DeepSeek, plus localhost/127.0.0.1 patterns for Ollama / vLLM / llama.cpp) drives provider + base URL + default model through a singleapplyProviderSwitchreducer so the policy can't drift between the manual picker and the auto-detect effect.<select>stays controlled during load / identity-mismatch. Save is blocked symmetrically in Create and Edit when sprout-agent has no pin AND no valid default.Backend (
desktop/src-tauri/src/commands/agent_provider_settings/)mod.rs— IPC types +StoredSettings(Drop zeroizesapi_key; no Debug derive) + v3 wrapper types.storage.rs— envelope read/write, NIP-44 encrypt/decrypt withZeroizing<String>, atomic-rename writes, file-size cap,normalize_origin,validate_input,validate_stored.storage_profiles.rs— wrapper IO + v1/v2 → v3 migration;validate_label(trim, non-empty, ≤64 B, no control chars);new_profile_id(UUID v4);ParseError::{IdentityMismatch, Other}typed dispatch.commands.rs/commands_write.rs— Tauri commands for state read, create/update/delete profile, set-default, clear, env-presence. All writes pre-flighted throughvalidate_label+validate_input.spawn.rs—LoadForSpawnenum +EnvPairsnewtype whose Drop zeroizes every value buffer.resolve_target_profilechooses by explicit pin id → default → fail closed.Tests
agent_provider_settingsmodule alone is 69 tests — envelope round-trip, identity rotation, save-time validation matrix, migration v1→v3 + v2→v3 (accept, idempotent, mismatched owner aborts),resolve_target_profilepolicy matrix, auto-default first-save, schema-dispatch reject on v4+,validate_labeledge cases, per-profile owner_pubkey cross-check at spawn,validate_stored(non-loopback http, control chars, userinfo/query, unknown provider, oversized prompt).applyProviderSwitchreducer +detectProvider(all key shapes + base-URL override + loopback hosts + ambiguity rules).settings-agent-provider+ 7 agents scenarios. Includes the hydration-race regression test (pre-fills a valid label sohydrationStaleis the only remaining gate), the cancel→reopen delete-disarm regression test (R13), in-dialog Delete arm/commit, no-default banner after deleting the default, clear-all flow, identity-rotation banner, provider-change warning, post-close apiKey + reveal-state scrub, two-profile list with Set-default / Edit / Delete.just check: green.Screenshots
Live in
desktop/screenshots/agent-provider/and posted in the main channel — two-profile list, auto-detect-from-key add dialog, provider-change warning on issuer switch, no-default banner after deleting the default.Codex review history
15 rounds across the two commits. Final verdicts:
v1 panel (
3dceb02) — R1–R8, final R8 verdict 9/10, no blocking findings. Surfaced and fixed: non-loopback HTTP at save AND spawn, identity-mismatch fails closed, local-provider key leak prevention, inline-args resolver alignment (TS↔Rust), Zeroize on api_key before early-return, no Debug derive on input/stored types, control-char / length caps + trim, owner_pubkey v2 envelope, local-provider switch unblock, EditAgentDialog command field, model reset on detection switch, nested-<button>fix,applyProviderSwitchreducer extraction, post-save apiKey + revealKey scrub,validate_storedat spawn.Multi-provider (
9013310) — R7–R13:ParseError.missingdetection onstatus==='ok'; picker PENDING sentinel; block unstartable Create; split sprout-only query; in-dialog Delete UX.useProfileDialogHydration(close-reset / switch-reset / hydrate-once); scrub apiKey + revealKey on close AND profile switch.viewIsFresh = isFetched && !isFetching); sharper race test (pre-fills label sohydrationStaleis the only remaining gate).(open, profileId)transition. Regression test pins the fix.The per-round review prompts + verdicts live in
.review/(gitignored).Signed-off-by: Tyler Longwell 109685178+tlongwell-block@users.noreply.github.com
Co-authored-by: Dawn c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@sprout-oss.stage.blox.sqprod.co