From 0945b9da3aee3458fe2ed15a0813b130cc39182d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B3n=20Levy?= Date: Mon, 11 May 2026 11:03:50 +0000 Subject: [PATCH] fix(installer): strip quotes + reject malformed OP_ITEM/OP_FIELD input Followup from #14. The migration warning showed the correct split, but a Windows user typed the legacy 3-segment value back at the prompt (with quotes the first time), and the installer accepted it. Runtime defense in claudestart.ps1 silently fixed it at launch, but local.env on disk was still wrong. Three input-validation gaps in the prompts: 1. Read-Host / read returned literal quote chars from copy-pasted values (e.g. "op://...") so the first attempt failed with the misleading "must start with op://". Strip surrounding matched quotes inside prompt_default / Prompt-Default. 2. OP_ITEM prompt only checked the op:// prefix. A 3+ segment path passed through, undoing the migration that just happened. Now the prompt loops with a specific message that shows exactly how to split the input across the next two prompts. 3. OP_FIELD prompt accepted anything, including a full op:// URL. Reject inputs starting with op:// in both the field-list and no-fields paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- install.ps1 | 42 +++++++++++++++++++++++++++++++++++++----- install.sh | 44 +++++++++++++++++++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/install.ps1 b/install.ps1 index 84b3a7c..c3fa1ee 100644 --- a/install.ps1 +++ b/install.ps1 @@ -77,7 +77,14 @@ function Prompt-Default($question, $default) { if ([Console]::IsInputRedirected) { return $default } $reply = Read-Host " $question [$default]" if ([string]::IsNullOrWhiteSpace($reply)) { return $default } - return $reply.Trim() + # Strip whitespace and surrounding matched quotes — copy-pasted secret + # references and shell-like values often include literal quote chars. + $reply = $reply.Trim() + if (($reply.StartsWith('"') -and $reply.EndsWith('"')) -or + ($reply.StartsWith("'") -and $reply.EndsWith("'"))) { + $reply = $reply.Substring(1, $reply.Length - 2) + } + return $reply } function Read-Existing($key) { @@ -119,8 +126,15 @@ function Prompt-OpField { $fields = Get-OpFields $OpItem if (-not $fields -or $fields.Count -eq 0) { - Write-Warn "Could not enumerate fields for $OpItem (op missing, not signed in, or item not found)" - return (Prompt-Default "1Password field name (case-sensitive)" $Default) + Write-Warn "Could not enumerate fields for ${OpItem} (op missing, not signed in, or item not found)" + while ($true) { + $reply = Prompt-Default "1Password field name (case-sensitive)" $Default + if ($reply -like 'op://*') { + Write-Warn "Field name is just the label (e.g. 'API Key'), not a full op:// path" + continue + } + return $reply + } } Write-Host "" @@ -140,6 +154,10 @@ function Prompt-OpField { Write-Warn "Number out of range — try again" continue } + if ($reply -like 'op://*') { + Write-Warn "Field name is just the label (e.g. 'API Key'), not a full op:// path" + continue + } return $reply } } @@ -180,8 +198,22 @@ function Prompt-LocalConfig { while ($true) { $opItem = Prompt-Default "1Password item (op://Vault/Item, no field)" $defaultItem - if ($opItem -like 'op://*') { break } - Write-Warn "Must start with op:// — try again" + if (-not ($opItem -like 'op://*')) { + Write-Warn "Must start with op:// — try again" + continue + } + $vSegs = ($opItem -replace '^op://', '').Split('/') + if ($vSegs.Count -gt 2) { + $hintField = ($vSegs | Select-Object -Skip 2) -join '/' + Write-Warn "OP_ITEM should be just op://Vault/Item — you included the field in the path." + Write-Warn " Use op://$($vSegs[0])/$($vSegs[1]) here, then '$hintField' in the next prompt." + continue + } + if ($vSegs.Count -lt 2 -or -not $vSegs[0] -or -not $vSegs[1]) { + Write-Warn "OP_ITEM needs both Vault and Item — got '$opItem'" + continue + } + break } $opField = Prompt-OpField $opItem $defaultField diff --git a/install.sh b/install.sh index 846808f..6ad3bc9 100644 --- a/install.sh +++ b/install.sh @@ -91,7 +91,13 @@ prompt_default() { IFS= read -r reply <&3 || reply="" exec 3<&- fi - printf '%s\n' "${reply:-$default}" + reply="${reply:-$default}" + # Strip surrounding matched quotes — copy-pasted values from docs/secret + # managers often arrive with quote chars that break naive validation. + if [[ "$reply" == \"*\" || "$reply" == \'*\' ]]; then + reply="${reply:1:${#reply}-2}" + fi + printf '%s\n' "$reply" } read_existing() { @@ -134,8 +140,16 @@ prompt_op_field() { fields=$(list_op_fields "$op_item") if [[ -z "$fields" ]]; then warn "Could not enumerate fields for $op_item (op missing, not signed in, or item not found)" - prompt_default "1Password field name (case-sensitive)" "$default_field" - return + while :; do + local reply + reply=$(prompt_default "1Password field name (case-sensitive)" "$default_field") + if [[ "$reply" == op://* ]]; then + warn "Field name is just the label (e.g. 'API Key'), not a full op:// path" + continue + fi + printf '%s\n' "$reply" + return + done fi echo >&2 @@ -161,6 +175,10 @@ prompt_op_field() { warn "Number out of range — try again" continue fi + if [[ "$reply" == op://* ]]; then + warn "Field name is just the label (e.g. 'API Key'), not a full op:// path" + continue + fi printf '%s\n' "$reply" return done @@ -200,8 +218,24 @@ prompt_local_config() { while :; do op_item=$(prompt_default "1Password item (op://Vault/Item, no field)" "${current_item:-op://Employee/ai.apro.is litellm}") - [[ "$op_item" == op://* ]] && break - warn "Must start with op:// — try again" + if [[ "$op_item" != op://* ]]; then + warn "Must start with op:// — try again" + continue + fi + local _validate_stripped="${op_item#op://}" + local -a _validate_segs=() + IFS='/' read -ra _validate_segs <<< "$_validate_stripped" + if (( ${#_validate_segs[@]} > 2 )); then + local _hint_field="${_validate_stripped#${_validate_segs[0]}/${_validate_segs[1]}/}" + warn "OP_ITEM should be just op://Vault/Item — you included the field in the path." + warn " Use op://${_validate_segs[0]}/${_validate_segs[1]} here, then '${_hint_field}' in the next prompt." + continue + fi + if (( ${#_validate_segs[@]} < 2 )) || [[ -z "${_validate_segs[0]}" || -z "${_validate_segs[1]}" ]]; then + warn "OP_ITEM needs both Vault and Item — got '$op_item'" + continue + fi + break done op_field=$(prompt_op_field "$op_item" "${current_field:-API Key}")