Skip to content

fix(installer): strip quotes + reject malformed OP_ITEM/OP_FIELD input#15

Open
busla wants to merge 1 commit into
mainfrom
fix/installer-input-validation
Open

fix(installer): strip quotes + reject malformed OP_ITEM/OP_FIELD input#15
busla wants to merge 1 commit into
mainfrom
fix/installer-input-validation

Conversation

@busla
Copy link
Copy Markdown
Contributor

@busla busla commented May 11, 2026

Summary

Followup from #14. A Windows user reran the installer, saw the migration warning correctly identify their legacy OP_ITEM, but then typed the legacy value back at the prompt (with quotes, then without) and the installer accepted it both times. Runtime defense in claudestart.ps1 silently corrected it at launch, but local.env on disk was still wrong.

Three input-validation gaps surfaced:

  1. Quote characters passed through literally. PowerShell's Read-Host and bash's read both return literal " / ' chars. Copy-pasted values from docs or secret managers often arrive quoted, and the validation only checked the op:// prefix — so a quoted "op://..." failed with the misleading message "must start with op://" (it does — after the quote). Strip surrounding matched quotes inside the prompt helpers.

  2. OP_ITEM prompt accepted 3+ segment paths. Right after the migration explained that the field shouldn't be in OP_ITEM, the prompt happily accepted op://Vault/Item/Field again. Add segment-count validation with a specific message that shows the user exactly how to split their input across the next two prompts.

  3. OP_FIELD prompt accepted full op:// URLs. Reject inputs starting with op:// in both the field-list and no-fields paths, with a hint that the field name is just a label.

Walkthrough of the previously failing scenario

Before this PR (real user session):

1Password item (op://Vault/Item, no field) [op://Employee/liteLLM]: \"op://Employee/liteLLM/API key\"
[WARN]  Must start with op:// — try again              ← misleading: it does, after the quote
1Password item (op://Vault/Item, no field) [op://Employee/liteLLM]: op://Employee/liteLLM/API key
                                                         ← accepted (3 segments — should reject)
1Password field name (case-sensitive) [API key]: op://Employee/liteLLM/API key
                                                         ← accepted (full URI as field — should reject)

After this PR:

1Password item (...) [op://Employee/liteLLM]: \"op://Employee/liteLLM/API key\"
                                                         ← quotes stripped
[WARN]  OP_ITEM should be just op://Vault/Item — you included the field in the path.
[WARN]    Use op://Employee/liteLLM here, then 'API key' in the next prompt.
1Password item (...) [op://Employee/liteLLM]: <enter>
                                                         ← user accepts default
1Password field name (case-sensitive) [API key]: <enter>
                                                         ← clean local.env written

Test plan

  • bash -n install.sh
  • Functional test of quote-stripping covering matched double/single quotes, no quotes, unmatched leading quote, empty quotes, and embedded quotes — all behave correctly
  • Verified by inspection that the failing scenario now hits each new validation path
  • Windows user reruns irm .../install.ps1 | iex and types the legacy value — should get the specific 3-segment rejection
  • macOS user reruns curl .../install.sh | bash — should behave identically

🤖 Generated with Claude Code

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) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant