Skip to content

wizard: _existing returns double-quoted .env values with their quotes, so "Enter keeps current" corrupts them (follow-up to #770) #1220

Closed
#1237
Closed
wizard: _existing returns double-quoted .env values with their quotes, so "Enter keeps current" corrupts them (follow-up to #770)#1220
#1237
Labels
ready-for-agentFully specified, ready for an AFK agent

Description

@bcbrendo
bcbrendo
opened on Oct 8, 2026

Skill

wizard

Harness and version

Not harness-specific: the bug is in plain bash in skills/engineering/wizard/template.sh. Reproduced at f3fc563 with GNU bash 5.3.20 and 3.2.57 (macOS /bin/bash).

Model and effort

N/A. Found by testing the template's _existing and write_env functions directly. No model behaviour is involved.

What happened

This follows up #770. That fix made write_env write single-quoted values. _existing undoes that single-quote form, but it returns any other existing form verbatim. A .env that already holds a common double-quoted dotenv line gives the quotes back as part of the value. If the user presses Enter to keep the current value, write_env stores the quotes as literal characters, and set_secret sends them to CI.

Repro with dummy values (_existing and write_env loaded from template.sh):

ENV_FILE=$(mktemp)
printf 'API_KEY="dummy-value"\n' > "$ENV_FILE"
write_env API_KEY "$(_existing API_KEY)"   # what "Enter keeps current" does
cat "$ENV_FILE"
# API_KEY='"dummy-value"'    <- the credential now contains the quotes

What you expected

The comment above _existing says "with write_env's quoting undone", and ask/ask_secret say "Enter keeps current". Keeping the current value should leave the credential unchanged: API_KEY="dummy-value" should be offered as dummy-value.

Paired regression case: a fix must not break the opposite case. A value that really contains quotes, which write_env stores as API_KEY='"dummy-value"', must read back as "dummy-value". A naive "strip double quotes as well" decodes both layers and changes that credential.

Suggested fix: choose the decoder from how the value is stored, and make the branches exclusive:

if [[ "$value" == \'*\' ]]; then value="${value:1:${#value}-2}"; value="${value//"$sq"/\'}"
elif [[ ${#value} -ge 2 && "$value" == \"*\" ]]; then
  value="${value:1:${#value}-2}"
  [[ "$value" == *[\\\$\"]* ]] && return 1   # escapes or $: can't decode safely; ask again
fi

Tested against both cases above, plus write → read → write round trips of values that contain ", ', $, \ and spaces.

Activity

  1. github-actions
    added
    needs-triageMaintainer needs to evaluate
    on Oct 8, 2026

  2. mattpocock commented on Oct 8, 2026

    @mattpocock
    mattpocock
    on Oct 8, 2026
    Owner

    This was generated by AI during triage.

    Agent brief: wizard template.sh _existing returns a double-quoted .env value (API_KEY="x") with its quotes, so Enter-keeps-current writes them into the credential. Fix _existing to decode the double-quoted form too, with exclusive branches so write_env's single-quoted form (API_KEY='"x"') still reads back as "x". If the double-quoted value holds \, $ or ", return 1 (ask again) rather than guess. Test both cases from the issue. Patch changeset.

  3. mattpocock
    added
    ready-for-agentFully specified, ready for an AFK agent
    and removed
    needs-triageMaintainer needs to evaluate
    on Oct 8, 2026
  4. Mola-maker
    added a commit that references this issue on Oct 9, 2026

    fix(wizard): decode double-quoted .env values in _existing (mattpococ…

    822875a

  5. Mola-maker commented on Oct 9, 2026

    @Mola-maker
    Mola-maker
    on Oct 9, 2026

    I have a working fix for this on a fork branch. Since PR creation here is limited to collaborators, I am linking it instead of opening a PR:

    Branch: https://github.com/Mola-maker/skills/tree/fix/wizard-existing-double-quotes
    Compare: main...Mola-maker:fix/wizard-existing-double-quotes

    The change gives _existing a second, exclusive branch for the double-quoted form: API_KEY="dummy-value" reads back as dummy-value, a single-quoted value that really contains double quotes still reads back as "dummy-value", and a double-quoted value containing escapes, a $ or an inner quote fails safe so the wizard asks again. It follows the shape suggested in the issue and includes a patch changeset.

    Tested by extracting the template library and running 20 cases (the repro above, the paired regression case, write/read/write round trips with spaces, quotes, $ and backslashes, plus edge cases): 3 failures before the fix, all 20 pass after. bash -n is clean.

    Happy for this to be cherry-picked or used as a reference.

  6. mattpocock
    closed this as completedin #1237on Oct 9, 2026
  7. mattpocock
    added a commit that references this issue on Oct 9, 2026

    wizard: template fixes (#1220, #1142, #1081, #1041, #1003) (#1237)

    49dd158
  8. jthurne
    added a commit that references this issue on Oct 10, 2026

    Merge remote-tracking branch 'upstream/main'

    6f3a587
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    ready-for-agentFully specified, ready for an AFK agent

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions