fix: PowerShell parser bug + migrate legacy 3-segment OP_ITEM#14
Merged
Conversation
Followups from #13. Two issues users hit on main: 1. install.ps1:127 used "$OpItem:" — PowerShell parses $Var: as a drive-qualified variable lookup ($env:USER syntax) and errors out on the entire script. Wrap with ${OpItem} to delimit. 2. Pre-#13 local.env files have OP_ITEM with the field baked into the path (e.g. op://V/Item/API Key). Post-#13 the wrapper appends /${OP_FIELD} itself, producing bogus lookups like op://V/Item/API Key/API Key. The installer offered the legacy value as the default, so users who hit enter kept it broken. Fixed in two places (defense in depth): - install.sh / install.ps1: detect 3+ segment OP_ITEM in existing local.env, split trailing segment(s) into OP_FIELD, show user what changed before prompting. - claude-env.sh / claudestart.ps1: silently apply the same split after sourcing local.env, so users who pull new code without reinstalling still get correct behavior on next launch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 11, 2026
busla
added a commit
that referenced
this pull request
May 11, 2026
…prompt (#16) * 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) <noreply@anthropic.com> * feat(installer): collapse OP_ITEM+OP_FIELD into one secret-reference prompt The two-prompt flow (item path, then field name) was a synthetic split of what 1Password already gives users as one value: the desktop app's "Copy Secret Reference" produces op://Vault/Item/Field directly, wrapped in double quotes for paste-safety. Replace both prompts with a single one that accepts that exact format, then split into OP_ITEM (vault/item) and OP_FIELD (everything after) at write time. The internal storage and runtime contract stay the same — local.env still has separate variables, claude-env.sh / claudestart.ps1 still append /<project> with /<OP_FIELD> as fallback. Only the collection mechanism changes. Drops list_op_fields/prompt_op_field (bash) and Get-OpFields/Prompt- OpField (PowerShell), since interactive field enumeration was only useful when the user had to type the field name from memory. With Copy Secret Reference, the user pastes vault, item, and field together. The migration warning block is also removed: the installer just uses whatever existing local.env produces (legacy 3-segment OP_ITEM, modern split, or the team default) as the prompt's bracketed default. Net -138 lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
Two regressions reported after #13 merged:
install.ps1:127had"$OpItem:"— PowerShell parses$Var:as a drive-qualified variable lookup (the$env:USERsyntax).OpItemisn't a drive, so the parser errors out andirm | iexaborts before any code runs. Fixed by wrapping with${OpItem}.local.envfiles put the field in the path (OP_ITEM=op://V/Item/API Key). Post-feat: make 1P field name configurable via OP_FIELD #13 the wrapper appends/${OP_FIELD}itself, producing bogus paths likeop://V/Item/API Key/API Key. The installer offered the legacy value as the default, so hitting enter preserved the broken state.Migration approach (defense in depth)
install.sh,install.ps1): detect 3+ segmentOP_ITEMin existinglocal.env, split the trailing segment(s) intoOP_FIELD, show the before/after on stderr before prompting. The user sees what changed and can still override.claude-env.sh,claudestart.ps1): silently apply the same split after sourcinglocal.env. Covers users who pull new wrapper code without rerunning the installer — their nextclaudeinvocation just works.A 4-segment
op://V/I/Section/Fieldis also handled (yieldsOP_FIELD=Section/Field).Test plan
bash -nsyntax check oninstall.shandclaude-env.shinstall.ps1no longer contains any$Var:patterns outside the legitimate$env:NAMEonesirm .../install.ps1 | iex— installer should parse, detect legacy value, show migration, write cleanlocal.envclaude(without reinstall) — runtime should silently split andop readshould succeedcurl .../install.sh | bash— installer should show migration warning and rewritelocal.env🤖 Generated with Claude Code